From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Bavier Subject: Re: [PATCH 01/13] gnu: subversion: Propagate env variables to hooks. Date: Mon, 24 Nov 2014 23:42:55 -0600 Message-ID: References: <1416548468-28421-1-git-send-email-bavier@member.fsf.org> <1416548468-28421-2-git-send-email-bavier@member.fsf.org> <871towp5ze.fsf@gnu.org> <87ppcgtcoj.fsf@gmail.com> <87k32njy58.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=001a1133e61005c9920508a8626c Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:52350) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xt8tu-0004B7-7u for guix-devel@gnu.org; Tue, 25 Nov 2014 00:42:59 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xt8ts-0003hZ-EX for guix-devel@gnu.org; Tue, 25 Nov 2014 00:42:58 -0500 In-Reply-To: <87k32njy58.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: =?UTF-8?Q?Ludovic_Court=C3=A8s?= Cc: Guix-devel , Eric Bavier --001a1133e61005c9920508a8626c Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Sat, Nov 22, 2014 at 5:02 AM, Ludovic Court=C3=A8s wrote: > Eric Bavier skribis: > > > On Fri, Nov 21, 2014 at 4:24 PM, Eric Bavier > wrote: > > > >> > >> Ludovic Court=C3=A8s writes: > >> > >> > Eric Bavier skribis: > >> > > >> >> * gnu/packages/patches/subversion-propagate-env-to-hooks.patch: New > >> patch. > >> >> * gnu-system.am (dist_patch_DATA): Add it. > >> >> * gnu/packages/version-control.scm (subversion): Use it. > >> > > >> > [...] > >> > > >> >> +++ b/gnu/packages/patches/subversion-propagate-env-to-hooks.patch > >> >> @@ -0,0 +1,14 @@ > >> >> +* Hooks need to inherit environment variables such as PATH, > otherwise > >> simple > >> >> + things like `ls` might fail in a hook. > >> > > >> > That looks good, but I want to make sure we=E2=80=99re not changing = something > >> > that was purposefully made this way. > >> > >> If the behavior is similar in 1.8.10, I'll get in contact with the > >> developers. > >> > > > > According to the subversion documentation, the desired behavior is to > > execute hooks in an empty environment for security reasons (see > > > http://svnbook.red-bean.com/en/1.7/svn.reposadmin.create.html#svn.reposad= min.create.hooks > > ). > > OK, so I think it=E2=80=99s important to stick to this behavior. > Agreed. > > > So it looks like this is a small bug in libtool. The wrapper scripts > > libtool generates for programs use `ls' to resolve symbolic links and i= s > > the only utility program invoked without an absolute file name (the onl= y > > other utility being sed). If such a wrapper script is executed with no > > PATH set, then we get "ls: command not found" errors. I don't think it= 's > > unreasonable to expect libtool's wrappers to be able to execute in an > empty > > environment. > > I see. So the patch is there =E2=80=9Cjust=E2=80=9D to allow tests to ru= n, right? > Only for tests to pass, yes. > Second, the problem should affect everyone, including FHS systems, no? > The culprit, I think, is a small difference in behavior of bash. If PATH is unset (such as within svn's hook environment), then `bash -c 'echo $PATH'` on an FHS system prints something like "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", but Guix's bash prints "/no-such-path". In the first case, `ls` will resolve to "/bin/ls", but will not be found in the second. > How do people work around it? > Given that behavior, people have nothing to work around. Does Nix's bash behave differently? I'm not sure that our bash needs to be changed in any way. It's current behavior seems in line with Guix's way of things. I was able to get the tests to pass by simply patching the references to ls that libtool emits in its wrappers. I think this might be the way to go for now, while also submitting a bug to libtool. Thoughts? `~Eric --=20 Please avoid sending me Word or PowerPoint attachments. See http://www.gnu.org/philosophy/no-word-attachments.html --001a1133e61005c9920508a8626c Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Sat, Nov 22, 2014 at 5:02 AM, Ludovic Court=C3=A8s <ludo@gnu.org>= wrote:
Eric Bavier <ericbavier@gmail.com> skribis:

> On Fri, Nov 21, 2014 at 4:24 PM, Eric Bavier <ericbavier@gmail.com> wrote:
>
>>
>> Ludovic Court=C3=A8s writes:
>>
>> > Eric Bavier <ericb= avier@gmail.com> skribis:
>> >
>> >> * gnu/packages/patches/subversion-propagate-env-to-hooks.= patch: New
>> patch.
>> >> * gnu-= system.am (dist_patch_DATA): Add it.
>> >> * gnu/packages/version-control.scm (subversion): Use it.<= br> >> >
>> > [...]
>> >
>> >> +++ b/gnu/packages/patches/subversion-propagate-env-to-ho= oks.patch
>> >> @@ -0,0 +1,14 @@
>> >> +* Hooks need to inherit environment variables such as PA= TH, otherwise
>> simple
>> >> +=C2=A0 things like `ls` might fail in a hook.
>> >
>> > That looks good, but I want to make sure we=E2=80=99re not ch= anging something
>> > that was purposefully made this way.
>>
>> If the behavior is similar in 1.8.10, I'll get in contact with= the
>> developers.
>>
>
> According to the subversion documentation, the desired behavior is to<= br> > execute hooks in an empty environment for security reasons (see
> http://svnbook.red-bean.c= om/en/1.7/svn.reposadmin.create.html#svn.reposadmin.create.hooks
> ).

OK, so I think it=E2=80=99s important to stick to this behavior.
=

Agreed.
=C2=A0

> So it looks like this is a small bug in libtool.=C2=A0 The wrapper scr= ipts
> libtool generates for programs use `ls' to resolve symbolic links = and is
> the only utility program invoked without an absolute file name (the on= ly
> other utility being sed).=C2=A0 If such a wrapper script is executed w= ith no
> PATH set, then we get "ls: command not found" errors.=C2=A0 = I don't think it's
> unreasonable to expect libtool's wrappers to be able to execute in= an empty
> environment.

I see.=C2=A0 So the patch is there =E2=80=9Cjust=E2=80=9D to allow t= ests to run, right?

Only for tests to p= ass, yes.
=C2=A0
Second, the problem should affect everyone, including FHS systems, no?
<= /blockquote>

The culprit, I think, is a small difference= in behavior of bash.=C2=A0 If PATH is unset (such as within svn's hook= environment), then `bash -c 'echo $PATH'` on an FHS system prints = something like "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbi= n:/bin", but Guix's bash prints "/no-such-path".=C2=A0 I= n the first case, `ls` will resolve to "/bin/ls", but will not be= found in the second.
=C2=A0
How do people work around it?

Given tha= t behavior, people have nothing to work around.=C2=A0 Does Nix's bash b= ehave differently?

I'm not sure that our bash needs t= o be changed in any way.=C2=A0 It's current behavior seems in line with= Guix's way of things.

I was able to get the tests to= pass by simply patching the references to ls that libtool emits in its wra= ppers.=C2=A0 I think this might be the way to go for now, while also submit= ting a bug to libtool.

Thoughts?

=
`~Eric

--
Please= avoid sending me Word or PowerPoint attachments.
See http://www.gnu.org/philoso= phy/no-word-attachments.html
--001a1133e61005c9920508a8626c--