* [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts.
@ 2016-02-06 17:26 Jan Nieuwenhuizen
2016-02-09 10:41 ` Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Jan Nieuwenhuizen @ 2016-02-06 17:26 UTC (permalink / raw)
To: guix-devel
[-- Attachment #1: Type: text/plain, Size: 298 bytes --]
Hi,
When patch-shebang encounters a script that is a symlink, say
bin/script -> ../lib/foo/thescript
it will change it into a file with rewritten #! . That breaks whenever
`thescript' assumes it lives in lib/foo.
Attached is a patch that has patch-shebangs skip symlinks.
Greetings, Jan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-build-system-do-not-patch-symlinks.-Fixes-locati.patch --]
[-- Type: text/x-diff, Size: 998 bytes --]
From 5a1793944b6ba1368a355edfa5be1b5c542ba48c Mon Sep 17 00:00:00 2001
From: Jan Nieuwenhuizen <janneke@gnu.org>
Date: Sat, 6 Feb 2016 15:59:51 +0100
Subject: [PATCH] gnu-build-system: do not patch symlinks. Fixes
location-aware scripts.
* guix/build/gnu-build-system.scm (patch-shebangs): avoid patching symlinks.
Fixes scripts
---
guix/build/gnu-build-system.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guix/build/gnu-build-system.scm b/guix/build/gnu-build-system.scm
index 2abaa6e..34edff7 100644
--- a/guix/build/gnu-build-system.scm
+++ b/guix/build/gnu-build-system.scm
@@ -303,7 +303,7 @@ makefiles."
(define (list-of-files dir)
(map (cut string-append dir "/" <>)
(or (scandir dir (lambda (f)
- (let ((s (stat (string-append dir "/" f))))
+ (let ((s (lstat (string-append dir "/" f))))
(eq? 'regular (stat:type s)))))
'())))
--
2.1.4
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts.
2016-02-06 17:26 [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts Jan Nieuwenhuizen
@ 2016-02-09 10:41 ` Ludovic Courtès
2016-02-09 19:19 ` Jan Nieuwenhuizen
0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2016-02-09 10:41 UTC (permalink / raw)
To: Jan Nieuwenhuizen; +Cc: guix-devel
Jan Nieuwenhuizen <janneke@gnu.org> skribis:
> When patch-shebang encounters a script that is a symlink, say
>
> bin/script -> ../lib/foo/thescript
>
> it will change it into a file with rewritten #! . That breaks whenever
> `thescript' assumes it lives in lib/foo.
Out of curiosity, what package was this?
> From 5a1793944b6ba1368a355edfa5be1b5c542ba48c Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <janneke@gnu.org>
> Date: Sat, 6 Feb 2016 15:59:51 +0100
> Subject: [PATCH] gnu-build-system: do not patch symlinks. Fixes
> location-aware scripts.
>
> * guix/build/gnu-build-system.scm (patch-shebangs): avoid patching symlinks.
> Fixes scripts
Since this is a rebuild-the-world change, I applied to to a new
‘core-updates’ branch (and adjusted the commit log.)
Thank you!
Ludo’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts.
2016-02-09 10:41 ` Ludovic Courtès
@ 2016-02-09 19:19 ` Jan Nieuwenhuizen
2016-02-09 20:59 ` Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Jan Nieuwenhuizen @ 2016-02-09 19:19 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]
Ludovic Courtès writes:
> Out of curiosity, what package was this?
I encountered it first in jison, a javascript parser generator.
> Since this is a rebuild-the-world change, I applied to to a new
> ‘core-updates’ branch (and adjusted the commit log.)
Yes, I got bitten by that, trying to test it. I had an interesting
learning experience getting to know guix and getting this to work and
reverted to using
(#phases
(replace 'patch-source-shebangs
;;patch-source-shebangs-no-symlinks
(lambda* (#:key outputs #:allow-other-keys)
(for-each patch-shebang
(remove (lambda (file)
(or (not (file-exists? file)) ;dangling symlink
;;(file-is-symlink? file)
(and (file-exists? file)
(eq? 'symlink (stat:type (lstat file))))
(file-is-directory? file)))
(find-files ".")))))
in the package itself. Rebuilding the world is no fun when you want to
get things done.
When I actually got to test it, it appears that something like the above
is still necessary. It seems that the previous patch merely avoids
visiting any symlinked directories, while this version actually looks
at the script to be patched.
Greetings, Jan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-build-system-do-not-patch-symlinks.-Fixes-locati.patch --]
[-- Type: text/x-diff, Size: 998 bytes --]
From 5a1793944b6ba1368a355edfa5be1b5c542ba48c Mon Sep 17 00:00:00 2001
From: Jan Nieuwenhuizen <janneke@gnu.org>
Date: Sat, 6 Feb 2016 15:59:51 +0100
Subject: [PATCH] gnu-build-system: do not patch symlinks. Fixes
location-aware scripts.
* guix/build/gnu-build-system.scm (patch-shebangs): avoid patching symlinks.
Fixes scripts
---
guix/build/gnu-build-system.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/guix/build/gnu-build-system.scm b/guix/build/gnu-build-system.scm
index 2abaa6e..34edff7 100644
--- a/guix/build/gnu-build-system.scm
+++ b/guix/build/gnu-build-system.scm
@@ -303,7 +303,7 @@ makefiles."
(define (list-of-files dir)
(map (cut string-append dir "/" <>)
(or (scandir dir (lambda (f)
- (let ((s (stat (string-append dir "/" f))))
+ (let ((s (lstat (string-append dir "/" f))))
(eq? 'regular (stat:type s)))))
'())))
--
2.1.4
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts.
2016-02-09 19:19 ` Jan Nieuwenhuizen
@ 2016-02-09 20:59 ` Ludovic Courtès
2016-02-09 22:15 ` Jan Nieuwenhuizen
0 siblings, 1 reply; 6+ messages in thread
From: Ludovic Courtès @ 2016-02-09 20:59 UTC (permalink / raw)
To: Jan Nieuwenhuizen; +Cc: guix-devel
Jan Nieuwenhuizen <janneke@gnu.org> skribis:
> Ludovic Courtès writes:
>
>> Out of curiosity, what package was this?
>
> I encountered it first in jison, a javascript parser generator.
How does the script determines its location? Using $0 is unreliable,
and using /proc/self/exe is non portable.
>> Since this is a rebuild-the-world change, I applied to to a new
>> ‘core-updates’ branch (and adjusted the commit log.)
>
> Yes, I got bitten by that, trying to test it. I had an interesting
> learning experience getting to know guix and getting this to work and
> reverted to using
>
> (#phases
> (replace 'patch-source-shebangs
> ;;patch-source-shebangs-no-symlinks
> (lambda* (#:key outputs #:allow-other-keys)
> (for-each patch-shebang
> (remove (lambda (file)
> (or (not (file-exists? file)) ;dangling symlink
> ;;(file-is-symlink? file)
> (and (file-exists? file)
> (eq? 'symlink (stat:type (lstat file))))
> (file-is-directory? file)))
> (find-files ".")))))
>
> in the package itself. Rebuilding the world is no fun when you want to
> get things done.
Yeah. In such cases it’s easier to simply delete or customize the
faulty phase like you did above.
> When I actually got to test it, it appears that something like the above
> is still necessary. It seems that the previous patch merely avoids
> visiting any symlinked directories, while this version actually looks
> at the script to be patched.
[...]
> diff --git a/guix/build/gnu-build-system.scm b/guix/build/gnu-build-system.scm
> index 2abaa6e..34edff7 100644
> --- a/guix/build/gnu-build-system.scm
> +++ b/guix/build/gnu-build-system.scm
> @@ -303,7 +303,7 @@ makefiles."
> (define (list-of-files dir)
> (map (cut string-append dir "/" <>)
> (or (scandir dir (lambda (f)
> - (let ((s (stat (string-append dir "/" f))))
> + (let ((s (lstat (string-append dir "/" f))))
> (eq? 'regular (stat:type s)))))
> '())))
This is exactly what the previous patch does (applied as c13a9feb.)
Am I missing something?
Anyway, it seems clear enough that the effect of this one-liner is to
prevent symlinks from being patched. :-)
Ludo’.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts.
2016-02-09 20:59 ` Ludovic Courtès
@ 2016-02-09 22:15 ` Jan Nieuwenhuizen
2016-02-10 21:30 ` Ludovic Courtès
0 siblings, 1 reply; 6+ messages in thread
From: Jan Nieuwenhuizen @ 2016-02-09 22:15 UTC (permalink / raw)
To: Ludovic Courtès; +Cc: guix-devel
[-- Attachment #1: Type: text/plain, Size: 954 bytes --]
Ludovic Courtès writes:
> How does the script determines its location? Using $0 is unreliable,
> and using /proc/self/exe is non portable.
It uses node.js's __dirname. I would have to dive into node.js
internals to figure that out... I would think it does /proc/self/exe
and has fallbacks for other platforms?
>> diff --git a/guix/build/gnu-build-system.scm b/guix/build/gnu-build-system.scm
>> index 2abaa6e..34edff7 100644
> This is exactly what the previous patch does (applied as c13a9feb.)
>
> Am I missing something?
Ah sorry.. Yes sent you the same patch, second patch attached now.
> Anyway, it seems clear enough that the effect of this one-liner is to
> prevent symlinks from being patched. :-)
Yes so I thought too, until I stumbled into this. Then I saw that
the first patch only considers what files to run scandir on ... any
of those files can be symlinks and those are caught here.
Greetings, Jan
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-build-system-gnu-Do-not-patch-symlinks-v2.patch --]
[-- Type: text/x-diff, Size: 1450 bytes --]
From 2d17c6bd7c7dd466c0aee14beaa47055af0ceb6d Mon Sep 17 00:00:00 2001
From: Jan Nieuwenhuizen <janneke@gnu.org>
Date: Sun, 7 Feb 2016 16:45:25 +0100
Subject: [PATCH] build-system/gnu: Do not patch symlinks, v2.
This fixes location-aware scripts.
* guix/build/gnu-build-system.scm (file-is-symlink?): new function.
(patch-source-shebangs): use it to skip symlinks.
---
guix/build/gnu-build-system.scm | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/guix/build/gnu-build-system.scm b/guix/build/gnu-build-system.scm
index 34edff7..427f020 100644
--- a/guix/build/gnu-build-system.scm
+++ b/guix/build/gnu-build-system.scm
@@ -166,6 +166,10 @@ things like the ABI being used."
(find-files "." "^configure$")))
#t)
+(define (file-is-symlink? file)
+ (and (file-exists? file)
+ (eq? 'symlink (stat:type (lstat file)))))
+
(define* (patch-source-shebangs #:key source #:allow-other-keys)
"Patch shebangs in all source files; this includes non-executable
files such as `.in' templates. Most scripts honor $SHELL and
@@ -174,6 +178,7 @@ $CONFIG_SHELL, but some don't, such as `mkinstalldirs' or Automake's
(for-each patch-shebang
(remove (lambda (file)
(or (not (file-exists? file)) ;dangling symlink
+ (file-is-symlink? file)
(file-is-directory? file)))
(find-files "."))))
--
2.1.4
[-- Attachment #3: Type: text/plain, Size: 154 bytes --]
--
Jan Nieuwenhuizen <janneke@gnu.org> | GNU LilyPond http://lilypond.org
Freelance IT http://JoyofSource.com | Avatar® http://AvatarAcademy.nl
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts.
2016-02-09 22:15 ` Jan Nieuwenhuizen
@ 2016-02-10 21:30 ` Ludovic Courtès
0 siblings, 0 replies; 6+ messages in thread
From: Ludovic Courtès @ 2016-02-10 21:30 UTC (permalink / raw)
To: Jan Nieuwenhuizen; +Cc: guix-devel
Jan Nieuwenhuizen <janneke@gnu.org> skribis:
> Ludovic Courtès writes:
>
>> How does the script determines its location? Using $0 is unreliable,
>> and using /proc/self/exe is non portable.
>
> It uses node.js's __dirname. I would have to dive into node.js
> internals to figure that out... I would think it does /proc/self/exe
> and has fallbacks for other platforms?
That's plausible.
> From 2d17c6bd7c7dd466c0aee14beaa47055af0ceb6d Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <janneke@gnu.org>
> Date: Sun, 7 Feb 2016 16:45:25 +0100
> Subject: [PATCH] build-system/gnu: Do not patch symlinks, v2.
>
> This fixes location-aware scripts.
>
> * guix/build/gnu-build-system.scm (file-is-symlink?): new function.
> (patch-source-shebangs): use it to skip symlinks.
This one changes ‘patch-source-shebangs’, which is about patching files
in the source tree.
It makes sense to avoid patching symlinks there too, but are you sure
this has anything to do with the problem you noticed?
Thanks,
Ludo’.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-10 21:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-06 17:26 [PATCH] gnu-build-system: do not patch symlinks. Fixes location-aware scripts Jan Nieuwenhuizen
2016-02-09 10:41 ` Ludovic Courtès
2016-02-09 19:19 ` Jan Nieuwenhuizen
2016-02-09 20:59 ` Ludovic Courtès
2016-02-09 22:15 ` Jan Nieuwenhuizen
2016-02-10 21:30 ` Ludovic Courtès
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).