From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jelle Licht Subject: bug#23723: patch-shebang phase breaks symlinks Date: Tue, 14 Jun 2016 10:22:33 +0200 Message-ID: References: <87fusowopk.fsf@fsfe.org> <87r3c51agw.fsf@gnu.org> <87r3c33l79.fsf@fsfe.org> <8737oiog2n.fsf@gnu.org> <87bn3427iu.fsf@fsfe.org> <878ty8jj92.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=047d7b3a8900ecff3f053538b441 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:39034) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCjcr-0005ME-7U for bug-guix@gnu.org; Tue, 14 Jun 2016 04:23:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCjcm-0002Ch-U5 for bug-guix@gnu.org; Tue, 14 Jun 2016 04:23:08 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:55488) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCjcm-0002CI-Qh for bug-guix@gnu.org; Tue, 14 Jun 2016 04:23:04 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1bCjck-0006Su-9k for bug-guix@gnu.org; Tue, 14 Jun 2016 04:23:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <878ty8jj92.fsf@gnu.org> List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 23723@debbugs.gnu.org --047d7b3a8900ecff3f053538b441 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Seems like a good policy in general. I'll apply your patch and fix up node then. Thanks a lot for the quick follow up. - Jelle On Jun 14, 2016 9:57 AM, "Ludovic Court=C3=A8s" wrote: > Jelle Licht skribis: > > > Ludovic Court=C3=A8s writes: > > > >> Jelle Licht skribis: > >> > >>> Ludovic Court=C3=A8s writes: > >>>> Hello! > >>>> > >>>> Jelle Licht skribis: > >>>> > >>>>> It seems that the patch-shebang functionality does not deal > gracefully > >>>>> with symlinks: it just overwrites them! > >>>>> > >>>>> After struggling somewhat with getting the recently packaged node > 6.0.0 > >>>>> to behave, I found out that `patch-shebang' in (guix build > >>>>> gnu-build-system) does not work properly on symlinks. > >>>> > >>>> There=E2=80=99s =E2=80=98patch-shebangs=E2=80=99 (plural) in this fi= le, but it explicitly > >>>> touches only regular files (see =E2=80=98list-of-files=E2=80=99). > >>>> > >>> > >>> It seems I made a mistake when writing the bug report; I am talking > >>> about the `patch-shebang' defined in (guix build utils). My apologies= . > >>> > >>> Also, seeing as my experience with the stat utility and similarly > styled > >>> programming libraries was lacking, I decided to play around with the > >>> definition of `list-of-files': It actually does include symlinks, as > >>> (stat:type (stat "some-symlinked-file")) gives us a plain old 'regula= r. > >>> Looking into this a bit more, it seems that calling `stat' gives the > >>> exact same results on both the linked-to-file and the symlink to that > >>> file. > >>> > >>> For the particular problem I ran into to be fixed, it is imperative > that > >>> `list-of-files' of `patch-shebangs' includes the symlink; it does aft= er > >>> all need to be patched. The way this patching currently happens just > >>> clobbers symlinks. > >> > >> My bad, indeed, =E2=80=98list-of-files=E2=80=99 should use =E2=80=98ls= tat=E2=80=99 instead of =E2=80=98stat=E2=80=99. > > > > This would be one way of fixing this bug. I'd rather see that > > `patch-shebang' in (guix build utils) checks for symlinks, and if so, > > patches the actual file instead of the symlink. This is the approach I > > currently use in my tree to use node 6.0. Would there be any downside t= o > > this approach? > > Both would work, but I think the =E2=80=9Cspirit=E2=80=9D is that symlink= s are supposed > to be transparent, and tools/procedures that operate on files shouldn=E2= =80=99t > try to do anything smart about symlinks. Thus I have a slight > preference for pushing the smartness to the edges. WDYT? > > Thanks, > Ludo=E2=80=99. > --047d7b3a8900ecff3f053538b441 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

Seems like a good policy in general.
I'll apply your patch and fix up node then.
Thanks a lot for the quick follow up.
- Jelle

On Jun 14, 2016 9:57 AM, "Ludovic Court=C3= =A8s" <ludo@gnu.org> wrote:<= br type=3D"attribution">
Jelle Licht <jlicht@fsfe.org> skribis:

> Ludovic Court=C3=A8s <ludo@gnu.org<= /a>> writes:
>
>> Jelle Licht <jlicht@fsfe.org= > skribis:
>>
>>> Ludovic Court=C3=A8s <ludo@= gnu.org> writes:
>>>> Hello!
>>>>
>>>> Jelle Licht <jlicht@= fsfe.org> skribis:
>>>>
>>>>> It seems that the patch-shebang functionality does not= deal gracefully
>>>>> with symlinks: it just overwrites them!
>>>>>
>>>>> After struggling somewhat with getting the recently pa= ckaged node 6.0.0
>>>>> to behave, I found out that `patch-shebang' in (gu= ix build
>>>>> gnu-build-system) does not work properly on symlinks.<= br> >>>>
>>>> There=E2=80=99s =E2=80=98patch-shebangs=E2=80=99 (plural) = in this file, but it explicitly
>>>> touches only regular files (see =E2=80=98list-of-files=E2= =80=99).
>>>>
>>>
>>> It seems I made a mistake when writing the bug report; I am ta= lking
>>> about the `patch-shebang' defined in (guix build utils). M= y apologies.
>>>
>>> Also, seeing as my experience with the stat utility and simila= rly styled
>>> programming libraries was lacking, I decided to play around wi= th the
>>> definition of `list-of-files': It actually does include sy= mlinks, as
>>> (stat:type (stat "some-symlinked-file")) gives us a = plain old 'regular.
>>> Looking into this a bit more, it seems that calling `stat'= gives the
>>> exact same results on both the linked-to-file and the symlink = to that
>>> file.
>>>
>>> For the particular problem I ran into to be fixed, it is imper= ative that
>>> `list-of-files' of `patch-shebangs' includes the symli= nk; it does after
>>> all need to be patched. The way this patching currently happen= s just
>>> clobbers symlinks.
>>
>> My bad, indeed, =E2=80=98list-of-files=E2=80=99 should use =E2=80= =98lstat=E2=80=99 instead of =E2=80=98stat=E2=80=99.
>
> This would be one way of fixing this bug. I'd rather see that
> `patch-shebang' in (guix build utils) checks for symlinks, and if = so,
> patches the actual file instead of the symlink. This is the approach I=
> currently use in my tree to use node 6.0. Would there be any downside = to
> this approach?

Both would work, but I think the =E2=80=9Cspirit=E2=80=9D is that symlinks = are supposed
to be transparent, and tools/procedures that operate on files shouldn=E2=80= =99t
try to do anything smart about symlinks.=C2=A0 Thus I have a slight
preference for pushing the smartness to the edges.=C2=A0 WDYT?

Thanks,
Ludo=E2=80=99.
--047d7b3a8900ecff3f053538b441--