Patch for autoload.v3 to allow export of function

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Patch for autoload.v3 to allow export of function

Matthew Persico
I would like to submit a patch to the autoload.v3 script.

The modification would allow the export of the autoloaded function to subshells. 

autoload() would now take a -x option, pass it into _aload() which would then make the approrpiate calls to export -f. 

Where should it be discussed and how does one format and submit a patch (fork, clone, pull request or patch submission on the savanah site or something else)?

Thank uou
--
Matthew O. Persico
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Patch for autoload.v3 to allow export of function

Eduardo A. Bustamante López
On Wed, Feb 8, 2017 at 11:27 AM, Matthew Persico
<[hidden email]> wrote:
[...]
>
> Where should it be discussed and how does one format and submit a patch
> (fork, clone, pull request or patch submission on the savanah site or
> something else)?

Hi Matthew, you can send patches to this list, or to Chet Ramey. You
can see the archives (https://lists.gnu.org/archive/html/bug-bash/)
for example on how people submit patches, but it's basically just
email the patch.

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Patch for autoload.v3 to allow export of function

Matthew Persico
Greetings!

I have finally gotten around to working on autoload again to ensure that
there are no issues. But I have a question about the _AUTOLOADS array,
which is used to track what has been autoloaded and is the source of
information for the -p command.

The code executes a linear scan of the array _AUTOLOADS each time it has to
find and element for removal or to make sure it is not about to add a
duplicate entry. This is hardly very efficient. Would it be OK to convert
that to an associative array (hash to us Perlers) indexed on function name?

I ask before submitting the patch because I am worried about backward
compatibility; I do not know when hashes were added to bash nor do I know
what the policy is about how far back compatibility has to be maintained.

Thank you!


--
Matthew O. Persico
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Patch for autoload.v3 to allow export of function

Chet Ramey
On 7/1/17 5:50 PM, Matthew Persico wrote:

> Greetings!
>
> I have finally gotten around to working on autoload again to ensure that
> there are no issues. But I have a question about the _AUTOLOADS array,
> which is used to track what has been autoloaded and is the source of
> information for the -p command.
>
> The code executes a linear scan of the array _AUTOLOADS each time it has to
> find and element for removal or to make sure it is not about to add a
> duplicate entry. This is hardly very efficient. Would it be OK to convert
> that to an associative array (hash to us Perlers) indexed on function name?
>
> I ask before submitting the patch because I am worried about backward
> compatibility; I do not know when hashes were added to bash nor do I know
> what the policy is about how far back compatibility has to be maintained.

Go ahead and submit the patch; if backwards compatibility is an issue, just
name it `autoload.v4'.  Associative arrays have been in bash since
bash-4.0, which was released in 2009, so including an updated version with
future bash distributions should not limit its usefulness.

Chet
--
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU    [hidden email]    http://cnswww.cns.cwru.edu/~chet/

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Patch for autoload.v3 to allow export of function

Matthew Persico
You'll notice from the dates on this chain that it has been a while that I
have been working on this. There are a number of reasons (like having to
work at my $job), but one of those reasons is a design one.

If you look at the code, there seven functions. Of those, only three are
used to implement autoloading. The other four are for bookkeeping purposes
only, to support the -p (dump) and -u (to remove a function from being
tracked in autoload) options.

The tracking is done with a bash array _AUTOLOADS. All insertions and
removals are processing a linear array - loop and search until you find the
item, copying the other items to a new array. It's all very inefficient,
but since we're dealing with shell, there's not much to do about speed
anyway. At one point, I converted _AUTOLOADS to a bash associative array,
but efficiency isn't the real problem here.

The real problem is that that arrays (associative or otherwise) cannot be
exported to subshells (such as starting up a new xterm), BUT the functions
can. So if you end up with -p and -u unsupportable in subshells, unless you
not only run the autoload commands in the bash profile for logins, but also
in the bashrc for subshells so that you can rebuild _AUTOLOADS, which takes
a lot of extra code, I might add, in the load phase, to make sure we don't
reload the second time around and make sure we get the array right.

Well, as you can see, I've been down the rathole and back and was about to
abandon the work until I came up with a way to avoid all this issues; an
easy way to tell if a function was an autoloaded function no matter how
many shells deep it's been exported. I can even tell whether it is still in
the shim stage or has been executed. But I fear no sane person will agree
with my solution:

Code injection.

It goes like this:

The current set of steps is

1 - Create the shim at autoload time.
2 - First time it's called, the shim finds the file with the function text
and sources it in.
3 - The shim then runs the function with whatever arguments were passed in.

What I propose to do between steps 2 and 3 is

2.1 - After sourcing the function in, type the function to get its text
2.2 - type appears to pretty print function code. No matter how sloppy or
compact your function is on disk, type presents it to you in a particular
format. To wit, for a function 'foo'

foo is a function
foo ()
{
    first line of code
    ....
}

That's regular. Therefore it can be parsed.

2.3 - With the output of the type command, I strip out line 1 and after
line 3, I add the text

local AUTOLOADED='foo'

2.4 - evaling the modified code updates the function to include that line

2.5 - Determining if a function is autoloaded is as simple as executing
(something like)

$ type foo | grep 'local AUTOLOADED='foo'.

which is REALLY REALLY fast, even under Cygwin on Windows 10. Yeah, that's
one of the environments I'm using for testing.

No more unexportable arrays and hashes. Self documenting. And 'AUTOLOADED'
can be changed to something less likely to clash with existing code. And we
document that any function you are going to autoload must avoid using the
variable AUTOLOADED, or whatever it is eventually.

So, before I post code here, what say you all? Is the idea of doing this
code injection so hideous that it would never be accepted, or is it worth
submitting the code for perusal?

Thank you for your time,

On Sun, Jul 2, 2017 at 12:50 PM, Chet Ramey <[hidden email]> wrote:

> On 7/1/17 5:50 PM, Matthew Persico wrote:
> > Greetings!
> >
> > I have finally gotten around to working on autoload again to ensure that
> > there are no issues. But I have a question about the _AUTOLOADS array,
> > which is used to track what has been autoloaded and is the source of
> > information for the -p command.
> >
> > The code executes a linear scan of the array _AUTOLOADS each time it has
> to
> > find and element for removal or to make sure it is not about to add a
> > duplicate entry. This is hardly very efficient. Would it be OK to convert
> > that to an associative array (hash to us Perlers) indexed on function
> name?
> >
> > I ask before submitting the patch because I am worried about backward
> > compatibility; I do not know when hashes were added to bash nor do I know
> > what the policy is about how far back compatibility has to be maintained.
>
> Go ahead and submit the patch; if backwards compatibility is an issue, just
> name it `autoload.v4'.  Associative arrays have been in bash since
> bash-4.0, which was released in 2009, so including an updated version with
> future bash distributions should not limit its usefulness.
>
> Chet
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
>                  ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, UTech, CWRU    [hidden email]    http://cnswww.cns.cwru.edu/~
> chet/
>



--
Matthew O. Persico
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Patch for autoload.v3 to allow export of function

Chet Ramey
On 8/11/17 6:55 PM, Matthew Persico wrote:

> What I propose to do between steps 2 and 3 is
>
> 2.1 - After sourcing the function in, type the function to get its text
> 2.2 - type appears to pretty print function code. No matter how sloppy or
> compact your function is on disk, type presents it to you in a particular
> format. To wit, for a function 'foo'
>
> foo is a function
> foo ()
> {
>     first line of code
>     ....
> }
>
> That's regular. Therefore it can be parsed.
>
> 2.3 - With the output of the type command, I strip out line 1 and after
> line 3, I add the text
>
> local AUTOLOADED='foo'
>
> 2.4 - evaling the modified code updates the function to include that line
>
> 2.5 - Determining if a function is autoloaded is as simple as executing
> (something like)
>
> $ type foo | grep 'local AUTOLOADED='foo'.
>
> which is REALLY REALLY fast, even under Cygwin on Windows 10. Yeah, that's
> one of the environments I'm using for testing.
>
> No more unexportable arrays and hashes. Self documenting. And 'AUTOLOADED'
> can be changed to something less likely to clash with existing code. And we
> document that any function you are going to autoload must avoid using the
> variable AUTOLOADED, or whatever it is eventually.
>
> So, before I post code here, what say you all? Is the idea of doing this
> code injection so hideous that it would never be accepted, or is it worth
> submitting the code for perusal?

Of course it's worth submitting the code for folks to look at. It might be
possible to add a couple more checks to insulate it against possible future
changes to the `type' output.

--
``The lyf so short, the craft so long to lerne.'' - Chaucer
                 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRU    [hidden email]    http://cnswww.cns.cwru.edu/~chet/

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Patch for autoload.v3 to allow export of function

Matthew Persico
Please see the attached 'autoload.v4' and 'autoload.v4.t'.

On Mon, Aug 14, 2017 at 10:30 AM, Chet Ramey <[hidden email]> wrote:

> On 8/11/17 6:55 PM, Matthew Persico wrote:
>
> > What I propose to do between steps 2 and 3 is
> >
> > 2.1 - After sourcing the function in, type the function to get its text
> > 2.2 - type appears to pretty print function code. No matter how sloppy or
> > compact your function is on disk, type presents it to you in a particular
> > format. To wit, for a function 'foo'
> >
> > foo is a function
> > foo ()
> > {
> >     first line of code
> >     ....
> > }
> >
> > That's regular. Therefore it can be parsed.
> >
> > 2.3 - With the output of the type command, I strip out line 1 and after
> > line 3, I add the text
> >
> > local AUTOLOADED='foo'
> >
> > 2.4 - evaling the modified code updates the function to include that line
> >
> > 2.5 - Determining if a function is autoloaded is as simple as executing
> > (something like)
> >
> > $ type foo | grep 'local AUTOLOADED='foo'.
> >
> > which is REALLY REALLY fast, even under Cygwin on Windows 10. Yeah,
> that's
> > one of the environments I'm using for testing.
> >
> > No more unexportable arrays and hashes. Self documenting. And
> 'AUTOLOADED'
> > can be changed to something less likely to clash with existing code. And
> we
> > document that any function you are going to autoload must avoid using the
> > variable AUTOLOADED, or whatever it is eventually.
> >
> > So, before I post code here, what say you all? Is the idea of doing this
> > code injection so hideous that it would never be accepted, or is it worth
> > submitting the code for perusal?
>
> Of course it's worth submitting the code for folks to look at. It might be
> possible to add a couple more checks to insulate it against possible future
> changes to the `type' output.
>
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
>                  ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, UTech, CWRU    [hidden email]    http://cnswww.cns.cwru.edu/~
> chet/
>


--
Matthew O. Persico

autoload.v4 (28K) Download Attachment
autoload.v4.t (5K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Patch for autoload.v3 to allow export of function

Matthew Persico
It has finally occurred to me after 24 hours that attachments are pretty
useless in this forum. I will repost this evening with the contents in the
mail body. And, since this is not longer a patch but really a rerwite, I'll
use a fresh thread.

On Wed, Aug 16, 2017 at 1:56 AM, Matthew Persico <[hidden email]>
wrote:

> Please see the attached 'autoload.v4' and 'autoload.v4.t'.
>
> On Mon, Aug 14, 2017 at 10:30 AM, Chet Ramey <[hidden email]> wrote:
>
>> On 8/11/17 6:55 PM, Matthew Persico wrote:
>>
>> > What I propose to do between steps 2 and 3 is
>> >
>> > 2.1 - After sourcing the function in, type the function to get its text
>> > 2.2 - type appears to pretty print function code. No matter how sloppy
>> or
>> > compact your function is on disk, type presents it to you in a
>> particular
>> > format. To wit, for a function 'foo'
>> >
>> > foo is a function
>> > foo ()
>> > {
>> >     first line of code
>> >     ....
>> > }
>> >
>> > That's regular. Therefore it can be parsed.
>> >
>> > 2.3 - With the output of the type command, I strip out line 1 and after
>> > line 3, I add the text
>> >
>> > local AUTOLOADED='foo'
>> >
>> > 2.4 - evaling the modified code updates the function to include that
>> line
>> >
>> > 2.5 - Determining if a function is autoloaded is as simple as executing
>> > (something like)
>> >
>> > $ type foo | grep 'local AUTOLOADED='foo'.
>> >
>> > which is REALLY REALLY fast, even under Cygwin on Windows 10. Yeah,
>> that's
>> > one of the environments I'm using for testing.
>> >
>> > No more unexportable arrays and hashes. Self documenting. And
>> 'AUTOLOADED'
>> > can be changed to something less likely to clash with existing code.
>> And we
>> > document that any function you are going to autoload must avoid using
>> the
>> > variable AUTOLOADED, or whatever it is eventually.
>> >
>> > So, before I post code here, what say you all? Is the idea of doing this
>> > code injection so hideous that it would never be accepted, or is it
>> worth
>> > submitting the code for perusal?
>>
>> Of course it's worth submitting the code for folks to look at. It might be
>> possible to add a couple more checks to insulate it against possible
>> future
>> changes to the `type' output.
>>
>> --
>> ``The lyf so short, the craft so long to lerne.'' - Chaucer
>>                  ``Ars longa, vita brevis'' - Hippocrates
>> Chet Ramey, UTech, CWRU    [hidden email]
>> http://cnswww.cns.cwru.edu/~chet/
>>
>
>
>
> --
> Matthew O. Persico
>



--
Matthew O. Persico
Loading...