Re: [E-devel] 1.22 schedule proposal - updated

2019-02-15 Thread Jonathan Aquilina
Hi Stefan,

I thought you were making a few changes to it given we are doing 5 month major 
releases and then weekly point releases.

Regards,
Jonathan.

-Original Message-
From: Stefan Schmidt  
Sent: 15 February 2019 19:56
To: enlightenment-devel@lists.sourceforge.net
Subject: Re: [E-devel] 1.22 schedule proposal - updated

Hello Jonathan.

On 15.02.19 16:41, Jonathan Aquilina wrote:
> Hi Stefan,
> 
> Not to derail the below in terms of the scripts that will be used to generate 
> the release do you have an ETA for these scripts or will it be sometime next 
> week?

Not sure what you are talking about here. It will be the same release script I 
used all the time. I pointed you several times to it. Last time when we had or 
IRC chat.

regards
Stefan Schmidt

> -Original Message-
> From: Stefan Schmidt 
> Sent: 15 February 2019 16:37
> To: enlightenment-devel@lists.sourceforge.net
> Subject: Re: [E-devel] 1.22 schedule proposal - updated
> 
> Hello.
> 
> We pushed back on the initial schedule to not get everyone caught by surprise 
> of a freeze.
> 
> We had some extra weeks now and I wonder if the following updated schedule 
> would work?
> 
> === Schedule ===
> 2018-08-17 Merge window for 1.22 opens
> 2019-02-20 Notice about soon ending merge window
> 2019-02-28 Merge window is over. Freeze in place.
> * Only bug fixes from this point
> * Alpha release tarball
> * One month stabilization phase starts
> 2019-03-07 Beta1 release tarball
> * Only critical fixes from this point
> 2019-03-14 Beta2 release tarball
> 2019-03-21 EFL 1.22 is out
> 
> As a sidenote I would consider this the last release with autotools. We will 
> also have a meson based tarball ready at that point which we would appreciate 
> testing from packagers. After the release is out we should remove autotools 
> support from master to focus on one build system only.
> Autotools will only be kept use in the 1.22.x releases for maintenance.
> 
> If this schedule does not work for you for a good reason speak up now.
> 
> regards
> Stefan Schmidt
> 
> 
> ___
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 
> 
> ___
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 


___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] [EGIT] [core/efl] master 02/02: Revert the env object because it's broken portability - please redo

2019-02-15 Thread Cedric Bail
On Friday, February 15, 2019 3:58 AM, Carsten Haitzler  
wrote:
> On Fri, 15 Feb 2019 12:29:21 +0100 Marcel Hollerbach m...@bu5hm4n.de said:
> > On 2/15/19 11:27 AM, Carsten Haitzler (The Rasterman) wrote:
> > > On Thu, 14 Feb 2019 19:05:56 +0100 Marcel Hollerbach m...@bu5hm4n.de said:
> > > > On 2/14/19 11:10 AM, Carsten Haitzler (The Rasterman) wrote:
> > > > > On Wed, 13 Feb 2019 17:40:33 +0100 Marcel Hollerbach m...@bu5hm4n.de
> > > > > said:
> > > > > > On 2/13/19 11:00 AM, Carsten Haitzler (The Rasterman) wrote:
> > > > > > > On Tue, 12 Feb 2019 22:05:06 +0100 Marcel Hollerbach 
> > > > > > > m...@bu5hm4n.de
> > > > > > > said:
> > > > > > No, but honestly, i am not trying to make life in C harder. Moving 
> > > > > > the
> > > > > > POV from the amount of
> > > > > > chars per API call, into something like usage range of our 
> > > > > > abstraction
> > > > > > shows you that efl_task is now usable in a wider range, and even 
> > > > > > efl_env
> > > > > > is more powerfull than before. Further more, compare the bindings 
> > > > > > code
> > > > > > before and after the patches, you will see that the code integrates 
> > > > > > much
> > > > > > nicer now, while its a little more (expressive) code in C.
> > > > > 

> > > > > i don't believe it is nicer. it's an extra object to create and hook 
> > > > > in.
> > > > > it's less obvious to the developer in non-c langs - as opposed to just
> > > > > setting env on an obj u have u have to know to create another then
> > > > > configure it then set it... etc. or similar. in every case it is 
> > > > > probably
> > > > > more code for the developer. this direction is really worrying.
> > > > > We have a corepoint where we disagree here. I would say that an
> > > > > principle like
> > > > > "Composition over inheritance" or SOLID or GRASP do have a reason to 
> > > > > exist.
> > > > > And what they pretty much advertice is that seperation of objects also
> > > > > seperates
> > > > > your concerns, which makes testing etc. etc. easier.
> > > 

> > > that's subjective. there is an objective measure that there is more code 
> > > to
> > > write in c and more moving parts to get to know and to then get wrong. it 
> > > is
> > > more code. even if the args array and content is consumed so caller 
> > > doesn't
> > > have to clean up anything (where your design requires partial cleanup by
> > > the caller) it's more. i converted the exe and threads examples that used
> > > args (wiki still has the old code) -
> > > https://phab.enlightenment.org/w/efl-loops-threads/ . exe needed an extra 
> > > 5
> > > lines of code (65->70), the threads example needed 16 more lines of code
> > > (177->193). considering most of the code was other stuff not related to
> > > args or env... compare:
> > > Eo *obj = efl_add(EFL_EXE_CLASS, app,
> > > efl_task_arg_append(efl_added, "./test.sh"),
> > > efl_task_env_set(efl_added, "BLAH", "blahvalue"),
> > > efl_task_flags_set(efl_added,
> > > EFL_TASK_FLAGS_USE_STDOUT | EFL_TASK_FLAGS_USE_STDIN),
> > > efl_event_callback_add(efl_added, EFL_IO_READER_EVENT_CAN_READ_CHANGED,
> > > _read_change, NULL), eina_future_then(efl_task_run(efl_added), _task_exit,
> > > efl_added) );
> > > vs
> > > Eina_Array *args = eina_array_new(1);
> > > eina_array_push(args, eina_stringshare_add("./test.sh"));
> > > Eo *env = efl_duplicate(efl_env_self());
> > > efl_core_env_set(env, "BLAH", "blahvalue");
> > > Eo *obj = efl_add(EFL_EXE_CLASS, app,
> > > efl_core_command_line_command_array_set(efl_added,
> > > args), efl_exe_env_set(efl_added, env),
> > > efl_task_flags_set(efl_added,
> > > EFL_TASK_FLAGS_USE_STDOUT | EFL_TASK_FLAGS_USE_STDIN),
> > > efl_event_callback_add(efl_added, EFL_IO_READER_EVENT_CAN_READ_CHANGED,
> > > _read_change, NULL), eina_future_then(efl_task_run(efl_added), _task_exit,
> > > efl_added) ); efl_unref(env);
> > 

> > Wondefull lineout to use a api how you should not. thx for that!
> > The code should rather be:
> > 

> > Eo *env = efl_duplicate(efl_env_self());
> > efl_core_env_set(env, "BLAH", "blahvalue");
> > Eo *obj = efl_add(EFL_EXE_CLASS, app,
> > 

> > 

> > efl_core_command_line_command_string_set(efl_added, "./test.sh"),
> > efl_exe_env_set(efl_added, env),
> > efl_task_flags_set(efl_added,
> > EFL_TASK_FLAGS_USE_STDOUT | EFL_TASK_FLAGS_USE_STDIN),
> > efl_event_callback_add(efl_added,
> > EFL_IO_READER_EVENT_CAN_READ_CHANGED, _read_change, NULL),
> > eina_future_then(efl_task_run(efl_added),
> > _task_exit, efl_added)
> > );
> > efl_unref(env);
> > Further more, if you start this exe a few times, please save the env
> > object somewhere, so you don't repeat the work.
> 

> the original code sets args separately - read it. the threads test sets 2
> arguments separately so you don't have to go escaping the command string 
> before
> setting it. it's an illustration that where code is trying to save bothering
> escaping stuff for the commandline by setting args one by one and letting efl
> take care of 

Re: [E-devel] 1.22 schedule proposal - updated

2019-02-15 Thread Simon Lees


On 16/02/2019 02:07, Stefan Schmidt wrote:
> Hello.
> 
> We pushed back on the initial schedule to not get everyone caught by
> surprise of a freeze.
> 
> We had some extra weeks now and I wonder if the following updated
> schedule would work?
> 
> === Schedule ===
> 2018-08-17 Merge window for 1.22 opens
> 2019-02-20 Notice about soon ending merge window
> 2019-02-28 Merge window is over. Freeze in place.
> * Only bug fixes from this point
> * Alpha release tarball
> * One month stabilization phase starts
> 2019-03-07 Beta1 release tarball
> * Only critical fixes from this point
> 2019-03-14 Beta2 release tarball
> 2019-03-21 EFL 1.22 is out
> 
> As a sidenote I would consider this the last release with autotools. We
> will also have a meson based tarball ready at that point which we would
> appreciate testing from packagers. After the release is out we should
> remove autotools support from master to focus on one build system only.
> Autotools will only be kept use in the 1.22.x releases for maintenance.
> 

Id be happy to test a meson tarball whenever there is one available.

-- 

Simon Lees (Simotek)http://simotek.net

Emergency Update Team   keybase.io/simotek
SUSE Linux   Adelaide Australia, UTC+10:30
GPG Fingerprint: 5B87 DB9D 88DC F606 E489 CEC5 0922 C246 02F0 014B



signature.asc
Description: OpenPGP digital signature
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] 1.22 schedule proposal - updated

2019-02-15 Thread Stefan Schmidt
Hello Jonathan.

On 15.02.19 16:41, Jonathan Aquilina wrote:
> Hi Stefan,
> 
> Not to derail the below in terms of the scripts that will be used to generate 
> the release do you have an ETA for these scripts or will it be sometime next 
> week?

Not sure what you are talking about here. It will be the same release
script I used all the time. I pointed you several times to it. Last time
when we had or IRC chat.

regards
Stefan Schmidt

> -Original Message-
> From: Stefan Schmidt  
> Sent: 15 February 2019 16:37
> To: enlightenment-devel@lists.sourceforge.net
> Subject: Re: [E-devel] 1.22 schedule proposal - updated
> 
> Hello.
> 
> We pushed back on the initial schedule to not get everyone caught by surprise 
> of a freeze.
> 
> We had some extra weeks now and I wonder if the following updated schedule 
> would work?
> 
> === Schedule ===
> 2018-08-17 Merge window for 1.22 opens
> 2019-02-20 Notice about soon ending merge window
> 2019-02-28 Merge window is over. Freeze in place.
> * Only bug fixes from this point
> * Alpha release tarball
> * One month stabilization phase starts
> 2019-03-07 Beta1 release tarball
> * Only critical fixes from this point
> 2019-03-14 Beta2 release tarball
> 2019-03-21 EFL 1.22 is out
> 
> As a sidenote I would consider this the last release with autotools. We will 
> also have a meson based tarball ready at that point which we would appreciate 
> testing from packagers. After the release is out we should remove autotools 
> support from master to focus on one build system only.
> Autotools will only be kept use in the 1.22.x releases for maintenance.
> 
> If this schedule does not work for you for a good reason speak up now.
> 
> regards
> Stefan Schmidt
> 
> 
> ___
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 
> 
> ___
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 


___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] 1.22 schedule proposal - updated

2019-02-15 Thread Jonathan Aquilina
Hi Stefan,

Not to derail the below in terms of the scripts that will be used to generate 
the release do you have an ETA for these scripts or will it be sometime next 
week?

-Original Message-
From: Stefan Schmidt  
Sent: 15 February 2019 16:37
To: enlightenment-devel@lists.sourceforge.net
Subject: Re: [E-devel] 1.22 schedule proposal - updated

Hello.

We pushed back on the initial schedule to not get everyone caught by surprise 
of a freeze.

We had some extra weeks now and I wonder if the following updated schedule 
would work?

=== Schedule ===
2018-08-17 Merge window for 1.22 opens
2019-02-20 Notice about soon ending merge window
2019-02-28 Merge window is over. Freeze in place.
* Only bug fixes from this point
* Alpha release tarball
* One month stabilization phase starts
2019-03-07 Beta1 release tarball
* Only critical fixes from this point
2019-03-14 Beta2 release tarball
2019-03-21 EFL 1.22 is out

As a sidenote I would consider this the last release with autotools. We will 
also have a meson based tarball ready at that point which we would appreciate 
testing from packagers. After the release is out we should remove autotools 
support from master to focus on one build system only.
Autotools will only be kept use in the 1.22.x releases for maintenance.

If this schedule does not work for you for a good reason speak up now.

regards
Stefan Schmidt


___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] 1.22 schedule proposal - updated

2019-02-15 Thread The Rasterman
On Fri, 15 Feb 2019 16:37:06 +0100 Stefan Schmidt 
said:

i think that sounds sensible.

> Hello.
> 
> We pushed back on the initial schedule to not get everyone caught by
> surprise of a freeze.
> 
> We had some extra weeks now and I wonder if the following updated
> schedule would work?
> 
> === Schedule ===
> 2018-08-17 Merge window for 1.22 opens
> 2019-02-20 Notice about soon ending merge window
> 2019-02-28 Merge window is over. Freeze in place.
> * Only bug fixes from this point
> * Alpha release tarball
> * One month stabilization phase starts
> 2019-03-07 Beta1 release tarball
> * Only critical fixes from this point
> 2019-03-14 Beta2 release tarball
> 2019-03-21 EFL 1.22 is out
> 
> As a sidenote I would consider this the last release with autotools. We
> will also have a meson based tarball ready at that point which we would
> appreciate testing from packagers. After the release is out we should
> remove autotools support from master to focus on one build system only.
> Autotools will only be kept use in the 1.22.x releases for maintenance.
> 
> If this schedule does not work for you for a good reason speak up now.
> 
> regards
> Stefan Schmidt
> 
> 
> ___
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 


-- 
- Codito, ergo sum - "I code, therefore I am" --
Carsten Haitzler - ras...@rasterman.com



___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] Goodbye

2019-02-15 Thread Stephen Houston
Thank you for all of your valuable contributions.  It was a privilege to
work with you and get to know you.  Best of luck in your future endeavors!

Stephen

On Thu, Feb 14, 2019 at 10:00 PM Derek Foreman <
derek.foreman.sams...@gmail.com> wrote:

> As some of you already know, today is my last day working for Samsung, and
> I no longer have a financial incentive to contribute to EFL.
>
> One of my primary reasons for leaving this position is that it was tied to
> a continued requirement to work on EFL, and I've grown weary of doing that,
> so I won't be lingering as a community developer.
>
> Over the years there have been times when I've felt EFL was on the cusp of
> outgrowing its hobby project roots and making a shift towards grown-up
> standards (incorporating peer review, CI, a push for better documentation),
> followed by events that felt like major setbacks on that path.  Some days
> it felt like we were reducing the insurmountable mountain of technical
> debt, and other days hopelessly watching it increase.
>
> EFL is currently in a situation where leadership is frequently at odds with
> the majority of contributors, and this leads to developers finding
> themselves worrying that work they've been paid to do (and maybe sometimes
> have even done well) will be unilaterally reverted, possibly with threats
> of commit right revocation. This makes contributing pointlessly stressful,
> and drives developers away.
>
> I hope I've left my small corners of the project a little better than I
> found them, and I thank you all for an opportunity to contribute.  I've met
> some excellent people in my time here, and hope to keep in touch.  I'll
> still be around on all the regular IRC servers, feel free to contact me*.
>
> Best of luck,
> Derek
>
> * Just not about EFL.  Ever.  Seriously.
>
> ___
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>

___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] 1.22 schedule proposal - updated

2019-02-15 Thread Stefan Schmidt
Hello.

We pushed back on the initial schedule to not get everyone caught by
surprise of a freeze.

We had some extra weeks now and I wonder if the following updated
schedule would work?

=== Schedule ===
2018-08-17 Merge window for 1.22 opens
2019-02-20 Notice about soon ending merge window
2019-02-28 Merge window is over. Freeze in place.
* Only bug fixes from this point
* Alpha release tarball
* One month stabilization phase starts
2019-03-07 Beta1 release tarball
* Only critical fixes from this point
2019-03-14 Beta2 release tarball
2019-03-21 EFL 1.22 is out

As a sidenote I would consider this the last release with autotools. We
will also have a meson based tarball ready at that point which we would
appreciate testing from packagers. After the release is out we should
remove autotools support from master to focus on one build system only.
Autotools will only be kept use in the 1.22.x releases for maintenance.

If this schedule does not work for you for a good reason speak up now.

regards
Stefan Schmidt


___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel


Re: [E-devel] [EGIT] [core/efl] master 02/04: cmdline iface - fix to consume input array AND strings totally

2019-02-15 Thread Daniel Kolesa
On Fri, Feb 15, 2019, at 13:02, Carsten Haitzler wrote:
> On Fri, 15 Feb 2019 12:05:22 +0100 Marcel Hollerbach  said:
> 
> > Thank you for taking care of this. However:
> > 
> > - you broke the C# bindings. (efl_csharp_application.cs:108)
> > - You forgot to annotate that the array has the ownership of the array
> > (array @owned; != (array
> > @owned;
> 
> shouldn't it be:
> 
> array @owned
> 
> ?

It should be array @owned

since stringshare implicitly emits const.

> 
> > Greetings,
> >bu5hm4n
> > 
> > On 2/15/19 11:27 AM, Carsten Haitzler wrote:
> > > raster pushed a commit to branch master.
> > > 
> > > http://git.enlightenment.org/core/efl.git/commit/?id=8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> > > 
> > > commit 8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> > > Author: Carsten Haitzler (Rasterman) 
> > > Date:   Thu Feb 14 11:28:23 2019 +
> > > 
> > > cmdline iface - fix to consume input array AND strings totally
> > > 
> > > strings often enough are generated e.g. via "%s/%s" or "%i" or similar
> > > etc. ... i have poitned to examples, so move to make all strings
> > > consistently stringshared, fix a bug added to the efl thread code
> > > where it accessed and freed array even tho array was consumed (but not
> > > strings) in the set, and the code used free to consume not
> > > stringshare_del. fix other code and tests to match
> > > 
> > > EXCTLY the kind of bugs and mistakes with this kind of design that i
> > > said would happen more often just happened...
> > > ---
> > >  src/lib/ecore/efl_core_command_line.c  |  3 +++
> > >  src/lib/ecore/efl_core_command_line.eo |  2 +-
> > >  src/lib/ecore/efl_loop.c   |  5 -
> > >  src/lib/ecore/efl_thread.c |  4 +---
> > >  src/tests/ecore/efl_app_test_cml.c | 14 +++---
> > >  5 files changed, 16 insertions(+), 12 deletions(-)
> > > 
> > > diff --git a/src/lib/ecore/efl_core_command_line.c
> > > b/src/lib/ecore/efl_core_command_line.c index 24cab90b0e..bd6d21f2d4 
> > > 100644
> > > --- a/src/lib/ecore/efl_core_command_line.c
> > > +++ b/src/lib/ecore/efl_core_command_line.c
> > > @@ -223,6 +223,8 @@ _efl_core_command_line_command_array_set(Eo *obj
> > > EINA_UNUSED, Efl_Core_Command_L
> > > eina_stringshare_del(eina_array_pop(pd->command));
> > > eina_array_free(pd->command); pd->command = NULL;
> > > + for (;i < (array ? eina_array_count(array) : 0); ++i)
> > > +   eina_stringshare_del(content);
> > >   eina_array_free(array);
> > >   return EINA_FALSE;
> > >}
> > > @@ -236,6 +238,7 @@ _efl_core_command_line_command_array_set(Eo *obj
> > > EINA_UNUSED, Efl_Core_Command_L _remove_invalid_chars(param);
> > >  eina_array_push(pd->command, eina_stringshare_add(param));
> > >  free(param);
> > > +eina_stringshare_del(content);
> > >   }
> > > pd->string_command = eina_strbuf_release(command);
> > > pd->filled = EINA_TRUE;
> > > diff --git a/src/lib/ecore/efl_core_command_line.eo
> > > b/src/lib/ecore/efl_core_command_line.eo index 436720d9bd..25b7c88b6e 
> > > 100644
> > > --- a/src/lib/ecore/efl_core_command_line.eo
> > > +++ b/src/lib/ecore/efl_core_command_line.eo
> > > @@ -60,7 +60,7 @@ mixin @beta Efl.Core.Command_Line {
> > >  return : bool; [[On success $true, $false otherwise]]
> > >}
> > >values {
> > > -array : array @owned; [[An array where every array field
> > > is an argument]]
> > > +array : array @owned; [[An array where every
> > > array field is an argument]] }
> > >  }
> > >  @property command_string {
> > > diff --git a/src/lib/ecore/efl_loop.c b/src/lib/ecore/efl_loop.c
> > > index 342a6f7725..1096c62bdb 100644
> > > --- a/src/lib/ecore/efl_loop.c
> > > +++ b/src/lib/ecore/efl_loop.c
> > > @@ -390,8 +390,11 @@ ecore_loop_arguments_send(int argc, const char 
> > > **argv)
> > > cml = eina_array_new(argc);
> > > for (i = 0; i < argc; i++)
> > >   {
> > > -Eina_Stringshare *arg = eina_stringshare_add(argv[i]);
> > > +Eina_Stringshare *arg;
> > > +
> > > +arg = eina_stringshare_add(argv[i]);
> > >  eina_array_push(arga, arg);
> > > +arg = eina_stringshare_add(argv[i]);
> > >  eina_array_push(cml, arg);
> > >   }
> > >  
> > > diff --git a/src/lib/ecore/efl_thread.c b/src/lib/ecore/efl_thread.c
> > > index a324af4f58..421c92dba7 100644
> > > --- a/src/lib/ecore/efl_thread.c
> > > +++ b/src/lib/ecore/efl_thread.c
> > > @@ -277,11 +277,9 @@ _efl_thread_main(void *data, Eina_Thread t)
> > >it->func, it->user_data);
> > >   }
> > > efl_core_command_line_command_array_set(obj, thdat->argv);
> > > +   thdat->argv = NULL;
> > > efl_future_then(obj, efl_loop_job(obj),
> > > .success = _efl_loop_arguments_send);
> > > -
> > > -   while (thdat->argv && eina_array_count(thdat->argv))
>

Re: [E-devel] [EGIT] [core/efl] master 02/04: cmdline iface - fix to consume input array AND strings totally

2019-02-15 Thread The Rasterman
On Fri, 15 Feb 2019 12:05:22 +0100 Marcel Hollerbach  said:

> Thank you for taking care of this. However:
> 
> - you broke the C# bindings. (efl_csharp_application.cs:108)
> - You forgot to annotate that the array has the ownership of the array
> (array @owned; != (array
> @owned;

shouldn't it be:

array @owned

?

> Greetings,
>bu5hm4n
> 
> On 2/15/19 11:27 AM, Carsten Haitzler wrote:
> > raster pushed a commit to branch master.
> > 
> > http://git.enlightenment.org/core/efl.git/commit/?id=8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> > 
> > commit 8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> > Author: Carsten Haitzler (Rasterman) 
> > Date:   Thu Feb 14 11:28:23 2019 +
> > 
> > cmdline iface - fix to consume input array AND strings totally
> > 
> > strings often enough are generated e.g. via "%s/%s" or "%i" or similar
> > etc. ... i have poitned to examples, so move to make all strings
> > consistently stringshared, fix a bug added to the efl thread code
> > where it accessed and freed array even tho array was consumed (but not
> > strings) in the set, and the code used free to consume not
> > stringshare_del. fix other code and tests to match
> > 
> > EXCTLY the kind of bugs and mistakes with this kind of design that i
> > said would happen more often just happened...
> > ---
> >  src/lib/ecore/efl_core_command_line.c  |  3 +++
> >  src/lib/ecore/efl_core_command_line.eo |  2 +-
> >  src/lib/ecore/efl_loop.c   |  5 -
> >  src/lib/ecore/efl_thread.c |  4 +---
> >  src/tests/ecore/efl_app_test_cml.c | 14 +++---
> >  5 files changed, 16 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/lib/ecore/efl_core_command_line.c
> > b/src/lib/ecore/efl_core_command_line.c index 24cab90b0e..bd6d21f2d4 100644
> > --- a/src/lib/ecore/efl_core_command_line.c
> > +++ b/src/lib/ecore/efl_core_command_line.c
> > @@ -223,6 +223,8 @@ _efl_core_command_line_command_array_set(Eo *obj
> > EINA_UNUSED, Efl_Core_Command_L
> > eina_stringshare_del(eina_array_pop(pd->command));
> > eina_array_free(pd->command); pd->command = NULL;
> > + for (;i < (array ? eina_array_count(array) : 0); ++i)
> > +   eina_stringshare_del(content);
> >   eina_array_free(array);
> >   return EINA_FALSE;
> >}
> > @@ -236,6 +238,7 @@ _efl_core_command_line_command_array_set(Eo *obj
> > EINA_UNUSED, Efl_Core_Command_L _remove_invalid_chars(param);
> >  eina_array_push(pd->command, eina_stringshare_add(param));
> >  free(param);
> > +eina_stringshare_del(content);
> >   }
> > pd->string_command = eina_strbuf_release(command);
> > pd->filled = EINA_TRUE;
> > diff --git a/src/lib/ecore/efl_core_command_line.eo
> > b/src/lib/ecore/efl_core_command_line.eo index 436720d9bd..25b7c88b6e 100644
> > --- a/src/lib/ecore/efl_core_command_line.eo
> > +++ b/src/lib/ecore/efl_core_command_line.eo
> > @@ -60,7 +60,7 @@ mixin @beta Efl.Core.Command_Line {
> >  return : bool; [[On success $true, $false otherwise]]
> >}
> >values {
> > -array : array @owned; [[An array where every array field
> > is an argument]]
> > +array : array @owned; [[An array where every
> > array field is an argument]] }
> >  }
> >  @property command_string {
> > diff --git a/src/lib/ecore/efl_loop.c b/src/lib/ecore/efl_loop.c
> > index 342a6f7725..1096c62bdb 100644
> > --- a/src/lib/ecore/efl_loop.c
> > +++ b/src/lib/ecore/efl_loop.c
> > @@ -390,8 +390,11 @@ ecore_loop_arguments_send(int argc, const char **argv)
> > cml = eina_array_new(argc);
> > for (i = 0; i < argc; i++)
> >   {
> > -Eina_Stringshare *arg = eina_stringshare_add(argv[i]);
> > +Eina_Stringshare *arg;
> > +
> > +arg = eina_stringshare_add(argv[i]);
> >  eina_array_push(arga, arg);
> > +arg = eina_stringshare_add(argv[i]);
> >  eina_array_push(cml, arg);
> >   }
> >  
> > diff --git a/src/lib/ecore/efl_thread.c b/src/lib/ecore/efl_thread.c
> > index a324af4f58..421c92dba7 100644
> > --- a/src/lib/ecore/efl_thread.c
> > +++ b/src/lib/ecore/efl_thread.c
> > @@ -277,11 +277,9 @@ _efl_thread_main(void *data, Eina_Thread t)
> >it->func, it->user_data);
> >   }
> > efl_core_command_line_command_array_set(obj, thdat->argv);
> > +   thdat->argv = NULL;
> > efl_future_then(obj, efl_loop_job(obj),
> > .success = _efl_loop_arguments_send);
> > -
> > -   while (thdat->argv && eina_array_count(thdat->argv))
> > free(eina_array_pop(thdat->argv));
> > -   eina_array_free(thdat->argv);
> > free(thdat->event_cb);
> > thdat->event_cb = NULL;
> >  
> > diff --git a/src/tests/ecore/efl_app_test_cml.c
> > b/src/tests/ecore/efl_app_test_cml.c index 1b7cebf552..33024dabb8 100644
> > --- a/src/tests/ecore/efl_app_test_cml.c
> > +++ b/src/tests/ecore/efl_app_test_cml.c
> > @@

Re: [E-devel] [EGIT] [core/efl] master 02/02: Revert the env object because it's broken portability - please redo

2019-02-15 Thread The Rasterman
On Fri, 15 Feb 2019 12:29:21 +0100 Marcel Hollerbach  said:

> 
> 
> On 2/15/19 11:27 AM, Carsten Haitzler (The Rasterman) wrote:
> > On Thu, 14 Feb 2019 19:05:56 +0100 Marcel Hollerbach  said:
> > 
> >>
> >>
> >> On 2/14/19 11:10 AM, Carsten Haitzler (The Rasterman) wrote:
> >>> On Wed, 13 Feb 2019 17:40:33 +0100 Marcel Hollerbach 
> >>> said:
> >>>
>  Hi,
> 
>  On 2/13/19 11:00 AM, Carsten Haitzler (The Rasterman) wrote:
> > On Tue, 12 Feb 2019 22:05:06 +0100 Marcel Hollerbach 
> > said:
> >
> >> Here is the try to get a technical discussion:
> >
> > let's solve these before landing, shll we? if they were just bugs i
> > would have fixed them, but as there are "bugs by design" it needs to go
> > back to the drawing board.
> 
>  Yes, that is why i am sticking things into review, and tbh. a ownership
>  discussion is nothing that sends back things onto the drawing board :)
> >>>
> >>> i think it does because it makes the code even more onerous on the caller
> >>> either way. if caller totally owns everything they need to alloc array,
> >>> alloc/dup strings (all most likely esp strings generated like "%s/%s" for
> >>> paths, then pass, then free every string in array then free array.
> >>>
> >>> if ownership is totally xfrred to the env object they still have to alloc
> >>> array and every string anyway, not do the free work though. it's still
> >>> much more code than the current api. i fail to get this whole "let's make
> >>> it more work for developers because we want some pure interface".
> >>>
> >>> the point of a library is to make life easy, not hard.
> >>>
> >> 2.:
> >>Yes, this is a design decision for now, the string that is forming a
> >> command is not taken care of, reason for this is, that in most usecases
> >> that i have seen the strings are eitherway allocated on the stack of
> >> the function, or are global (argv) and are also not heap allocated (in
> >> our meaning). If this does not work out in other usecases, then i can
> >> happily agree with a patch to change that :)
> >
> > rethink ownership. but what you have is wrong. i have examples where
> > they are generated on the stack with multiple args .. so parent does
> > have to allocate space and cant use a single fixed size buffer. if
> > ownership goes to the array consistently then it's at least consistent
> > (and the api consumes the array entirely including strings), but it's
> > not like this. i brought this up and it was not addressed before
> > landing the patch.
> 
>  Can you write some pseudocode? Because your example suites perfectly
>  well what this is written for.
> >>>
> >>> i pointed to actual src code files in rage and terminology where some args
> >>> are %s/%s or %i and we pass more than just 1 of these as args thus
> >>> requiring multiple static buffers or duplicating strings. the code
> >>> currently generates the cmdline as a single string - but it really should
> >>> build args (some examples will not get escaping right, but with the arg
> >>> by arg api this would be done right by efl avoid the need for the app to
> >>> get it right).
> >>
> >> You even answered yourself to this on irc with using strdupa, so the
> > 
> > strdupa is not portable (alloca is) so caller will actually have to write
> > much more code to do this if they wish to be portable. also alloca/strdupa
> > are very bad ideas if you have to loop over N arguments as the stack could
> > be blown by putting too much on it on smaller stack systems. imagine you
> > have 1000 file paths each 40-80 bytes each. i've seen small stack systems
> > (256k total - i think this was qemu misc binary emulation in the past in
> > scratchbox) and i have actually moved allocations off the stack to make
> > these work before in efl. as an illustration it can be done, but i'm trying
> > to get across the work/pain your api design creates and you just don't seem
> > to understand how much that is.
> > 
> >> only "overhead" here is, to create the array. Which pretty much boils
> >> down again to the argument of usability, where i am saying that a
> >> usability of an array is always better than the API, you can append
> >> prepend, insert etc. etc.. Further more, in bindings code, you can just
> >> use native arrays, that might come from somewhere else, and convert
> >> them nicely into a eina.array. Take a look at how this is done in c#
> >> land, and see how handy this is :)
> > 
> > but it makes c worse. efl was created to make writing e easier. efl is now
> > moving to make it harder. not only is there a legacy -> eo api move needed
> > now, the eo api code in various places is longer and harder, or is it no
> > better where it could be much better but due to design choices like the
> > above it won't be.
> 
> Okay, this is against every single piece of logic, you now have the full
> possibility of dealing with your arguements in fo

Re: [E-devel] [EGIT] [core/efl] master 02/04: cmdline iface - fix to consume input array AND strings totally

2019-02-15 Thread Marcel Hollerbach
Thank you for taking care of this. However:

- you broke the C# bindings. (efl_csharp_application.cs:108)
- You forgot to annotate that the array has the ownership of the array
(array @owned; != (array
@owned;

Greetings,
   bu5hm4n

On 2/15/19 11:27 AM, Carsten Haitzler wrote:
> raster pushed a commit to branch master.
> 
> http://git.enlightenment.org/core/efl.git/commit/?id=8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> 
> commit 8e98c7eef9cdc6b337841fdd11d5b2c8a3079067
> Author: Carsten Haitzler (Rasterman) 
> Date:   Thu Feb 14 11:28:23 2019 +
> 
> cmdline iface - fix to consume input array AND strings totally
> 
> strings often enough are generated e.g. via "%s/%s" or "%i" or similar
> etc. ... i have poitned to examples, so move to make all strings
> consistently stringshared, fix a bug added to the efl thread code
> where it accessed and freed array even tho array was consumed (but not
> strings) in the set, and the code used free to consume not
> stringshare_del. fix other code and tests to match
> 
> EXCTLY the kind of bugs and mistakes with this kind of design that i
> said would happen more often just happened...
> ---
>  src/lib/ecore/efl_core_command_line.c  |  3 +++
>  src/lib/ecore/efl_core_command_line.eo |  2 +-
>  src/lib/ecore/efl_loop.c   |  5 -
>  src/lib/ecore/efl_thread.c |  4 +---
>  src/tests/ecore/efl_app_test_cml.c | 14 +++---
>  5 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/src/lib/ecore/efl_core_command_line.c 
> b/src/lib/ecore/efl_core_command_line.c
> index 24cab90b0e..bd6d21f2d4 100644
> --- a/src/lib/ecore/efl_core_command_line.c
> +++ b/src/lib/ecore/efl_core_command_line.c
> @@ -223,6 +223,8 @@ _efl_core_command_line_command_array_set(Eo *obj 
> EINA_UNUSED, Efl_Core_Command_L
>eina_stringshare_del(eina_array_pop(pd->command));
>   eina_array_free(pd->command);
>   pd->command = NULL;
> + for (;i < (array ? eina_array_count(array) : 0); ++i)
> +   eina_stringshare_del(content);
>   eina_array_free(array);
>   return EINA_FALSE;
>}
> @@ -236,6 +238,7 @@ _efl_core_command_line_command_array_set(Eo *obj 
> EINA_UNUSED, Efl_Core_Command_L
>  _remove_invalid_chars(param);
>  eina_array_push(pd->command, eina_stringshare_add(param));
>  free(param);
> +eina_stringshare_del(content);
>   }
> pd->string_command = eina_strbuf_release(command);
> pd->filled = EINA_TRUE;
> diff --git a/src/lib/ecore/efl_core_command_line.eo 
> b/src/lib/ecore/efl_core_command_line.eo
> index 436720d9bd..25b7c88b6e 100644
> --- a/src/lib/ecore/efl_core_command_line.eo
> +++ b/src/lib/ecore/efl_core_command_line.eo
> @@ -60,7 +60,7 @@ mixin @beta Efl.Core.Command_Line {
>  return : bool; [[On success $true, $false otherwise]]
>}
>values {
> -array : array @owned; [[An array where every array field is 
> an argument]]
> +array : array @owned; [[An array where every 
> array field is an argument]]
>}
>  }
>  @property command_string {
> diff --git a/src/lib/ecore/efl_loop.c b/src/lib/ecore/efl_loop.c
> index 342a6f7725..1096c62bdb 100644
> --- a/src/lib/ecore/efl_loop.c
> +++ b/src/lib/ecore/efl_loop.c
> @@ -390,8 +390,11 @@ ecore_loop_arguments_send(int argc, const char **argv)
> cml = eina_array_new(argc);
> for (i = 0; i < argc; i++)
>   {
> -Eina_Stringshare *arg = eina_stringshare_add(argv[i]);
> +Eina_Stringshare *arg;
> +
> +arg = eina_stringshare_add(argv[i]);
>  eina_array_push(arga, arg);
> +arg = eina_stringshare_add(argv[i]);
>  eina_array_push(cml, arg);
>   }
>  
> diff --git a/src/lib/ecore/efl_thread.c b/src/lib/ecore/efl_thread.c
> index a324af4f58..421c92dba7 100644
> --- a/src/lib/ecore/efl_thread.c
> +++ b/src/lib/ecore/efl_thread.c
> @@ -277,11 +277,9 @@ _efl_thread_main(void *data, Eina_Thread t)
>it->func, it->user_data);
>   }
> efl_core_command_line_command_array_set(obj, thdat->argv);
> +   thdat->argv = NULL;
> efl_future_then(obj, efl_loop_job(obj),
> .success = _efl_loop_arguments_send);
> -
> -   while (thdat->argv && eina_array_count(thdat->argv)) 
> free(eina_array_pop(thdat->argv));
> -   eina_array_free(thdat->argv);
> free(thdat->event_cb);
> thdat->event_cb = NULL;
>  
> diff --git a/src/tests/ecore/efl_app_test_cml.c 
> b/src/tests/ecore/efl_app_test_cml.c
> index 1b7cebf552..33024dabb8 100644
> --- a/src/tests/ecore/efl_app_test_cml.c
> +++ b/src/tests/ecore/efl_app_test_cml.c
> @@ -23,13 +23,13 @@ _construct_array(void)
>  {
> Eina_Array *array = eina_array_new(16);
>  
> -   eina_array_push(array, "/bin/sh");
> -   eina_array_push(array, "-C");
> -   eina_array_push(array, "foo");
> -   eina_array_push(array, "-

Re: [E-devel] [EGIT] [core/efl] master 02/02: Revert the env object because it's broken portability - please redo

2019-02-15 Thread Marcel Hollerbach


On 2/15/19 11:27 AM, Carsten Haitzler (The Rasterman) wrote:
> On Thu, 14 Feb 2019 19:05:56 +0100 Marcel Hollerbach  said:
> 
>>
>>
>> On 2/14/19 11:10 AM, Carsten Haitzler (The Rasterman) wrote:
>>> On Wed, 13 Feb 2019 17:40:33 +0100 Marcel Hollerbach  said:
>>>
 Hi,

 On 2/13/19 11:00 AM, Carsten Haitzler (The Rasterman) wrote:
> On Tue, 12 Feb 2019 22:05:06 +0100 Marcel Hollerbach 
> said:
>
>> Here is the try to get a technical discussion:
>
> let's solve these before landing, shll we? if they were just bugs i would
> have fixed them, but as there are "bugs by design" it needs to go back to
> the drawing board.

 Yes, that is why i am sticking things into review, and tbh. a ownership
 discussion is nothing that sends back things onto the drawing board :)
>>>
>>> i think it does because it makes the code even more onerous on the caller
>>> either way. if caller totally owns everything they need to alloc array,
>>> alloc/dup strings (all most likely esp strings generated like "%s/%s" for
>>> paths, then pass, then free every string in array then free array.
>>>
>>> if ownership is totally xfrred to the env object they still have to alloc
>>> array and every string anyway, not do the free work though. it's still much
>>> more code than the current api. i fail to get this whole "let's make it
>>> more work for developers because we want some pure interface".
>>>
>>> the point of a library is to make life easy, not hard.
>>>
>> 2.:
>>Yes, this is a design decision for now, the string that is forming a
>> command is not taken care of, reason for this is, that in most usecases
>> that i have seen the strings are eitherway allocated on the stack of the
>> function, or are global (argv) and are also not heap allocated (in our
>> meaning). If this does not work out in other usecases, then i can
>> happily agree with a patch to change that :)
>
> rethink ownership. but what you have is wrong. i have examples where they
> are generated on the stack with multiple args .. so parent does have to
> allocate space and cant use a single fixed size buffer. if ownership goes
> to the array consistently then it's at least consistent (and the api
> consumes the array entirely including strings), but it's not like this. i
> brought this up and it was not addressed before landing the patch.

 Can you write some pseudocode? Because your example suites perfectly
 well what this is written for.
>>>
>>> i pointed to actual src code files in rage and terminology where some args
>>> are %s/%s or %i and we pass more than just 1 of these as args thus requiring
>>> multiple static buffers or duplicating strings. the code currently
>>> generates the cmdline as a single string - but it really should build args
>>> (some examples will not get escaping right, but with the arg by arg api
>>> this would be done right by efl avoid the need for the app to get it right).
>>
>> You even answered yourself to this on irc with using strdupa, so the
> 
> strdupa is not portable (alloca is) so caller will actually have to write much
> more code to do this if they wish to be portable. also alloca/strdupa are very
> bad ideas if you have to loop over N arguments as the stack could be blown by
> putting too much on it on smaller stack systems. imagine you have 1000 file
> paths each 40-80 bytes each. i've seen small stack systems (256k total - i
> think this was qemu misc binary emulation in the past in scratchbox) and i 
> have
> actually moved allocations off the stack to make these work before in efl. as
> an illustration it can be done, but i'm trying to get across the work/pain 
> your
> api design creates and you just don't seem to understand how much that is.
> 
>> only "overhead" here is, to create the array. Which pretty much boils
>> down again to the argument of usability, where i am saying that a
>> usability of an array is always better than the API, you can append
>> prepend, insert etc. etc.. Further more, in bindings code, you can just
>> use native arrays, that might come from somewhere else, and convert
>> them nicely into a eina.array. Take a look at how this is done in c#
>> land, and see how handy this is :)
> 
> but it makes c worse. efl was created to make writing e easier. efl is now
> moving to make it harder. not only is there a legacy -> eo api move needed 
> now,
> the eo api code in various places is longer and harder, or is it no better
> where it could be much better but due to design choices like the above it 
> won't
> be.

Okay, this is against every single piece of logic, you now have the full
possibility of dealing with your arguements in form of an array, which
enables you to do many many things you cannot do before, just with
standard API, nothing new. And you claim now just out of the blue, this
is worse. You tell me things like design principles where the majority
of researchers agree 

Re: [E-devel] [EGIT] [core/efl] master 02/02: Revert the env object because it's broken portability - please redo

2019-02-15 Thread The Rasterman
On Thu, 14 Feb 2019 21:32:02 + Cedric Bail  said:

> On Thursday, February 14, 2019 10:05 AM, Marcel Hollerbach 
> wrote:
> > On 2/14/19 11:10 AM, Carsten Haitzler (The Rasterman) wrote:
> > > On Wed, 13 Feb 2019 17:40:33 +0100 Marcel Hollerbach m...@bu5hm4n.de said:
> > > > On 2/13/19 11:00 AM, Carsten Haitzler (The Rasterman) wrote:
> > > > > On Tue, 12 Feb 2019 22:05:06 +0100 Marcel Hollerbach m...@bu5hm4n.de
> > > > > said:
> > > > > > Here is the try to get a technical discussion:
> > > > > 
> 
> > > > > let's solve these before landing, shll we? if they were just bugs i
> > > > > would have fixed them, but as there are "bugs by design" it needs to
> > > > > go back to the drawing board.
> > > > 
> 
> > > > Yes, that is why i am sticking things into review, and tbh. a ownership
> > > > discussion is nothing that sends back things onto the drawing board :)
> > > 
> 
> > > i think it does because it makes the code even more onerous on the caller
> > > either way. if caller totally owns everything they need to alloc array,
> > > alloc/dup strings (all most likely esp strings generated like "%s/%s" for
> > > paths, then pass, then free every string in array then free array.
> > > if ownership is totally xfrred to the env object they still have to alloc
> > > array and every string anyway, not do the free work though. it's still
> > > much more code than the current api. i fail to get this whole "let's make
> > > it more work for developers because we want some pure interface".
> > > the point of a library is to make life easy, not hard.
> > > 
> 
> > > > > > 2.:
> > > > > > Yes, this is a design decision for now, the string that is forming a
> > > > > > command is not taken care of, reason for this is, that in most
> > > > > > usecases that i have seen the strings are eitherway allocated on
> > > > > > the stack of the function, or are global (argv) and are also not
> > > > > > heap allocated (in our meaning). If this does not work out in other
> > > > > > usecases, then i can happily agree with a patch to change that :)
> > > > > 
> 
> > > > > rethink ownership. but what you have is wrong. i have examples where
> > > > > they are generated on the stack with multiple args .. so parent does
> > > > > have to allocate space and cant use a single fixed size buffer. if
> > > > > ownership goes to the array consistently then it's at least
> > > > > consistent (and the api consumes the array entirely including
> > > > > strings), but it's not like this. i brought this up and it was not
> > > > > addressed before landing the patch.
> > > > 
> 
> > > > Can you write some pseudocode? Because your example suites perfectly
> > > > well what this is written for.
> > > 
> 
> > > i pointed to actual src code files in rage and terminology where some
> > > args are %s/%s or %i and we pass more than just 1 of these as args thus
> > > requiring multiple static buffers or duplicating strings. the code
> > > currently generates the cmdline as a single string - but it really should
> > > build args (some examples will not get escaping right, but with the arg
> > > by arg api this would be done right by efl avoid the need for the app to
> > > get it right).
> > 
> 
> > You even answered yourself to this on irc with using strdupa, so the
> > only "overhead" here is, to create the array. Which pretty much boils
> > down again to the argument of usability, where i am saying that a
> > usability of an array is always better than the API, you can append
> > prepend, insert etc. etc.. Further more, in bindings code, you can just
> > use native arrays, that might come from somewhere else, and convert
> > them nicely into a eina.array. Take a look at how this is done in c#
> > land, and see how handy this is :)
> 
> I think everyone forget about the existence of Eina_Slstr which cover this
> use case pretty well in C. You can directly use :
>eina_array_push(env, eina_slstr_printf("%s/%s", SOME_PATH, file));

you're rigvht that that could also be used, but that's almost the same as if you
also used eina_stringshare_add() and had the array_set() consume the entire
array AND strings inside (take ownership one way or another). with slstr you
still have to eina_slstr_local_clear() after the call so it adds another line to
remember there and forget to do. so it's about equivalent with a bit more code
and another thing to go wrong. i've made it actually do the "consume strings -
require them to be stringshared" and the problem i still see is it's 71% more
code to launch an exe (or 250% more code if we just talk about code that
handles setting 1 env var and 1 arg). the slstr way would raise that to 85%
more code (or 300% more depending how you count it) as you need to add the
clear too.

it's more to go wrong and one of my fixes to this handling inside efl is
PRECISELY a fix to this "things going wrong" that i said would happen with this
kind of design and exactly that did happen from someone who designed it and
wrote the code for it 

Re: [E-devel] [EGIT] [core/efl] master 02/02: Revert the env object because it's broken portability - please redo

2019-02-15 Thread The Rasterman
On Thu, 14 Feb 2019 19:05:56 +0100 Marcel Hollerbach  said:

> 
> 
> On 2/14/19 11:10 AM, Carsten Haitzler (The Rasterman) wrote:
> > On Wed, 13 Feb 2019 17:40:33 +0100 Marcel Hollerbach  said:
> > 
> >> Hi,
> >>
> >> On 2/13/19 11:00 AM, Carsten Haitzler (The Rasterman) wrote:
> >>> On Tue, 12 Feb 2019 22:05:06 +0100 Marcel Hollerbach 
> >>> said:
> >>>
>  Here is the try to get a technical discussion:
> >>>
> >>> let's solve these before landing, shll we? if they were just bugs i would
> >>> have fixed them, but as there are "bugs by design" it needs to go back to
> >>> the drawing board.
> >>
> >> Yes, that is why i am sticking things into review, and tbh. a ownership
> >> discussion is nothing that sends back things onto the drawing board :)
> > 
> > i think it does because it makes the code even more onerous on the caller
> > either way. if caller totally owns everything they need to alloc array,
> > alloc/dup strings (all most likely esp strings generated like "%s/%s" for
> > paths, then pass, then free every string in array then free array.
> > 
> > if ownership is totally xfrred to the env object they still have to alloc
> > array and every string anyway, not do the free work though. it's still much
> > more code than the current api. i fail to get this whole "let's make it
> > more work for developers because we want some pure interface".
> > 
> > the point of a library is to make life easy, not hard.
> > 
>  2.:
> Yes, this is a design decision for now, the string that is forming a
>  command is not taken care of, reason for this is, that in most usecases
>  that i have seen the strings are eitherway allocated on the stack of the
>  function, or are global (argv) and are also not heap allocated (in our
>  meaning). If this does not work out in other usecases, then i can
>  happily agree with a patch to change that :)
> >>>
> >>> rethink ownership. but what you have is wrong. i have examples where they
> >>> are generated on the stack with multiple args .. so parent does have to
> >>> allocate space and cant use a single fixed size buffer. if ownership goes
> >>> to the array consistently then it's at least consistent (and the api
> >>> consumes the array entirely including strings), but it's not like this. i
> >>> brought this up and it was not addressed before landing the patch.
> >>
> >> Can you write some pseudocode? Because your example suites perfectly
> >> well what this is written for.
> > 
> > i pointed to actual src code files in rage and terminology where some args
> > are %s/%s or %i and we pass more than just 1 of these as args thus requiring
> > multiple static buffers or duplicating strings. the code currently
> > generates the cmdline as a single string - but it really should build args
> > (some examples will not get escaping right, but with the arg by arg api
> > this would be done right by efl avoid the need for the app to get it right).
> 
> You even answered yourself to this on irc with using strdupa, so the

strdupa is not portable (alloca is) so caller will actually have to write much
more code to do this if they wish to be portable. also alloca/strdupa are very
bad ideas if you have to loop over N arguments as the stack could be blown by
putting too much on it on smaller stack systems. imagine you have 1000 file
paths each 40-80 bytes each. i've seen small stack systems (256k total - i
think this was qemu misc binary emulation in the past in scratchbox) and i have
actually moved allocations off the stack to make these work before in efl. as
an illustration it can be done, but i'm trying to get across the work/pain your
api design creates and you just don't seem to understand how much that is.

> only "overhead" here is, to create the array. Which pretty much boils
> down again to the argument of usability, where i am saying that a
> usability of an array is always better than the API, you can append
> prepend, insert etc. etc.. Further more, in bindings code, you can just
> use native arrays, that might come from somewhere else, and convert
> them nicely into a eina.array. Take a look at how this is done in c#
> land, and see how handy this is :)

but it makes c worse. efl was created to make writing e easier. efl is now
moving to make it harder. not only is there a legacy -> eo api move needed now,
the eo api code in various places is longer and harder, or is it no better
where it could be much better but due to design choices like the above it won't
be.

> >> You have:
> >> - Allocated strings on the stack,
> >> You do:
> >> - Allocate an eina_array
> >> - Add all your stack strings into it
> >> - Pass it to the function
> >> - Be happy with it.
> > 
> > but they arent on the stack because there is > 1 arg. they are not alloca'd
> > or in static buffers if we do multiple.
> > 
>  4.: this will be fixed :)
> 
>  For anyone new to this discussion: This is about getting the abstraction
>  of environment variables / command