unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping.
@ 2020-09-13  5:39 Brendan Tildesley
  2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Brendan Tildesley @ 2020-09-13  5:39 UTC (permalink / raw)
  To: 43367

I'm attempting to fix a bug where wrap-program produces ..X-real-real 
files by mistakenly wrapping already wrapped files. I haven't fully 
tested these because it requires rebuilding everything which takes hours 
to days and core-updates is stuck on mesa now anyway. Perhaps I'll try 
testing on master. Also there may be other places where .X-real files 
are accidentally wrapped, which will now error.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files.
  2020-09-13  5:39 [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Brendan Tildesley
@ 2020-09-13  5:45 ` Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 2/5] utils: Rename wrapper? to wrapped-program? Brendan Tildesley
                     ` (3 more replies)
  2020-09-13  9:40 ` [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Danny Milosavljevic
  2021-04-22  9:06 ` bug#43367: " Ludovic Courtès
  2 siblings, 4 replies; 9+ messages in thread
From: Brendan Tildesley @ 2020-09-13  5:45 UTC (permalink / raw)
  To: 43367

*    guix/build/utils.scm: (wrap-program): Error if wrap-program was
mistakenly passed a .X-real file. This prevents and forces us to fix
cases where a double wrapped ..X-real-real file is created, such as can
be seen with:

find /gnu/ -iname '.*-real-real'
---
 guix/build/utils.scm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index e872cfffd3..822191f4de 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -1194,6 +1194,9 @@ with definitions for VARS."
        (format #f "export ~a=\"$~a${~a:+:}~a\""
                var var var (string-join rest ":")))))
 
+  (when (wrapped-program? prog)
+    (error (string-append prog " is a wrapper. Refusing to wrap.")))
+
   (if already-wrapped?
 
       ;; PROG is already a wrapper: add the new "export VAR=VALUE" lines just
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [bug#43367] [PATCH 2/5] utils: Rename wrapper? to wrapped-program?.
  2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
@ 2020-09-13  5:45   ` Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 3/5] glib-or-gtk-build-system: Don't double wrap programs Brendan Tildesley
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Brendan Tildesley @ 2020-09-13  5:45 UTC (permalink / raw)
  To: 43367

* guix/build/utils.scm (wrap-program): The wrapper? procedure is
incorrectly named as it actually checks to see if prog is the
original program that was moved, not the wrapper.

* guix/build/python-build-system: (wrap): Use renamed wrapped-program?.
---
 guix/build/python-build-system.scm | 2 +-
 guix/build/utils.scm               | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/guix/build/python-build-system.scm b/guix/build/python-build-system.scm
index 62e7a7b305..d1dbbc1de2 100644
--- a/guix/build/python-build-system.scm
+++ b/guix/build/python-build-system.scm
@@ -196,7 +196,7 @@ when running checks after installing the package."
   (define (list-of-files dir)
     (find-files dir (lambda (file stat)
                       (and (eq? 'regular (stat:type stat))
-                           (not (wrapper? file))))))
+                           (not (wrapped-program? file))))))
 
   (define bindirs
     (append-map (match-lambda
diff --git a/guix/build/utils.scm b/guix/build/utils.scm
index 822191f4de..4cd227a668 100644
--- a/guix/build/utils.scm
+++ b/guix/build/utils.scm
@@ -90,7 +90,7 @@
             patch-/usr/bin/file
             fold-port-matches
             remove-store-references
-            wrapper?
+            wrapped-program?
             wrap-program
             wrap-script
 
@@ -1118,8 +1118,8 @@ known as `nuke-refs' in Nixpkgs."
   (program    wrap-error-program)
   (type       wrap-error-type))
 
-(define (wrapper? prog)
-  "Return #t if PROG is a wrapper as produced by 'wrap-program'."
+(define (wrapped-program? prog)
+  "Return #t if PROG is a program that was moved and wrapped by 'wrap-program'."
   (and (file-exists? prog)
        (let ((base (basename prog)))
          (and (string-prefix? "." base)
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [bug#43367] [PATCH 3/5] glib-or-gtk-build-system: Don't double wrap programs.
  2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 2/5] utils: Rename wrapper? to wrapped-program? Brendan Tildesley
@ 2020-09-13  5:45   ` Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 4/5] rakudo-build-system: " Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 5/5] qt-build-system: " Brendan Tildesley
  3 siblings, 0 replies; 9+ messages in thread
From: Brendan Tildesley @ 2020-09-13  5:45 UTC (permalink / raw)
  To: 43367

* guix/build/glib-or-gtk-build-system.scm (wrap-all-programs): If a
package definition was modified to insert an additional wrap phase
before glib-or-gtk...'s wrap phase instead of after, glib-or-gtk...'s
wrap phase will double wrap the .X-real file from the earlier wrap
phase. Filtering out such wrapped programs means these .X-real files
should fix this and mean packagers don't have to worry about ensuring
their wrap phases are put afterwards.
---
 guix/build/glib-or-gtk-build-system.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/guix/build/glib-or-gtk-build-system.scm b/guix/build/glib-or-gtk-build-system.scm
index ba680fd1a9..ccb3138fe2 100644
--- a/guix/build/glib-or-gtk-build-system.scm
+++ b/guix/build/glib-or-gtk-build-system.scm
@@ -142,8 +142,9 @@ add a dependency of that output on GLib and GTK+."
       (unless (member output glib-or-gtk-wrap-excluded-outputs)
         (let* ((bindir       (string-append directory "/bin"))
                (libexecdir   (string-append directory "/libexec"))
-               (bin-list     (append (find-files bindir ".*")
-                                     (find-files libexecdir ".*")))
+               (bin-list     (filter (negate wrapped-program?)
+                                     (append (find-files bindir ".*")
+                                             (find-files libexecdir ".*"))))
                (datadirs     (data-directories
                               (alist-cons output directory inputs)))
                (gtk-mod-dirs (gtk-module-directories
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [bug#43367] [PATCH 4/5] rakudo-build-system: Don't double wrap programs.
  2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 2/5] utils: Rename wrapper? to wrapped-program? Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 3/5] glib-or-gtk-build-system: Don't double wrap programs Brendan Tildesley
@ 2020-09-13  5:45   ` Brendan Tildesley
  2020-09-13  5:45   ` [bug#43367] [PATCH 5/5] qt-build-system: " Brendan Tildesley
  3 siblings, 0 replies; 9+ messages in thread
From: Brendan Tildesley @ 2020-09-13  5:45 UTC (permalink / raw)
  To: 43367

* guix/build/rakudo-build-system.scm (wrap): Don't return any potential
already wrapped-programs in the list-of-files to wrap.
---
 guix/build/rakudo-build-system.scm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/guix/build/rakudo-build-system.scm b/guix/build/rakudo-build-system.scm
index dbdeb1ccd2..b2c090f946 100644
--- a/guix/build/rakudo-build-system.scm
+++ b/guix/build/rakudo-build-system.scm
@@ -97,7 +97,8 @@
     (map (cut string-append dir "/" <>)
          (or (scandir dir (lambda (f)
                             (let ((s (stat (string-append dir "/" f))))
-                              (eq? 'regular (stat:type s)))))
+                              (and (eq? 'regular (stat:type s))
+                                   (not (wrapped-program? f))))))
              '())))
 
   (define bindirs
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [bug#43367] [PATCH 5/5] qt-build-system: Don't double wrap programs.
  2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
                     ` (2 preceding siblings ...)
  2020-09-13  5:45   ` [bug#43367] [PATCH 4/5] rakudo-build-system: " Brendan Tildesley
@ 2020-09-13  5:45   ` Brendan Tildesley
  3 siblings, 0 replies; 9+ messages in thread
From: Brendan Tildesley @ 2020-09-13  5:45 UTC (permalink / raw)
  To: 43367

* guix/build/qt-build-system.scm (wrap-all-programs): Excluded wrapped
programs from the list of files to wrap if they exist to avoid double
wrapping.
---
 guix/build/qt-build-system.scm | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/guix/build/qt-build-system.scm b/guix/build/qt-build-system.scm
index 005157b0a4..4738ca09c9 100644
--- a/guix/build/qt-build-system.scm
+++ b/guix/build/qt-build-system.scm
@@ -83,7 +83,10 @@ add a dependency of that output on Qt."
   (define (find-files-to-wrap directory)
     (append-map
      (lambda (dir)
-       (if (directory-exists? dir) (find-files dir ".*") (list)))
+       (if (directory-exists? dir)
+           (find-files dir (lambda (file stat)
+                             (not (wrapped-program? file))))
+           '()))
      (list (string-append directory "/bin")
            (string-append directory "/sbin")
            (string-append directory "/libexec")
-- 
2.28.0





^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping.
  2020-09-13  5:39 [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Brendan Tildesley
  2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
@ 2020-09-13  9:40 ` Danny Milosavljevic
  2020-09-13 12:30   ` Brendan Tildesley
  2021-04-22  9:06 ` bug#43367: " Ludovic Courtès
  2 siblings, 1 reply; 9+ messages in thread
From: Danny Milosavljevic @ 2020-09-13  9:40 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: 43367

[-- Attachment #1: Type: text/plain, Size: 621 bytes --]

On Sun, 13 Sep 2020 15:39:15 +1000
Brendan Tildesley <mail@brendan.scot> wrote:

> I'm attempting to fix a bug where wrap-program produces ..X-real-real 
> files by mistakenly wrapping already wrapped files. I haven't fully 
> tested these because it requires rebuilding everything which takes hours 
> to days and core-updates is stuck on mesa now anyway. Perhaps I'll try 
> testing on master. Also there may be other places where .X-real files 
> are accidentally wrapped, which will now error.

But can't a thing be wrapped once for one reason and another time for another
reason and that should be fine?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping.
  2020-09-13  9:40 ` [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Danny Milosavljevic
@ 2020-09-13 12:30   ` Brendan Tildesley
  0 siblings, 0 replies; 9+ messages in thread
From: Brendan Tildesley @ 2020-09-13 12:30 UTC (permalink / raw)
  To: Danny Milosavljevic; +Cc: 43367

[-- Attachment #1: Type: text/plain, Size: 3669 bytes --]

On 13/9/20 7:40 pm, Danny Milosavljevic wrote:
> On Sun, 13 Sep 2020 15:39:15 +1000
> Brendan Tildesley <mail@brendan.scot> wrote:
>
>> I'm attempting to fix a bug where wrap-program produces ..X-real-real
>> files by mistakenly wrapping already wrapped files. I haven't fully
>> tested these because it requires rebuilding everything which takes hours
>> to days and core-updates is stuck on mesa now anyway. Perhaps I'll try
>> testing on master. Also there may be other places where .X-real files
>> are accidentally wrapped, which will now error.
> But can't a thing be wrapped once for one reason and another time for another
> reason and that should be fine?

Yes, perhaps I should have explained that this is still possible and 
works fine. When a program is wrapped a second time, it will append to 
the existed wrapper, rather than creating a new file and moving the old 
one. repeated applications of wrap-program after the first one simply 
append. I'll illustrate how this can go wrong though: suppose we have 
/bin/foo and we we are in a repl and run:

(wrap-program "/bin/foo" `("BAR" = ("baz")) => /bin/.foo-real doesn't 
exist so /bin/foo is moved to /bin/.foo-real, a new /bin/foo is created 
that is a wrapper that then launches /bin.foo-real.

(wrap-program "/bin/foo" `("BAR" = ("baz")) => /bin/.foo-real exists so 
/bin/foo is assumed to already be a wrapper so variables are appended to 
/bin/foo.

(wrap-program "/bin/foo" `("BAR" = ("baz")) => same thing again, 
variables are appended

; Now suppose we then run:

(wrap-program "/bin/.foo-real" `("BAR" = ("baz")) => 
/bin/..foo-real-real doesn't exist, so /bin/.foo-real is moved to 
/bin/..foo-real-real and /bin/.foo-real is created again as another wrapper.

This should never be done intentionally I think, but sometimes there is 
code that uses (find-files dir ".") to find binaries to wrap, and this 
is run after a previous existing wrap phase, so the both /bin/foo and 
/bin/.foo-real are wrapped again. Generally everything will continue 
working though despite all this though.

You run this to find some of these double wrapped packages:

find /gnu/ -maxdepth 4 -iname '.*-real-real'

So I thought it best to error whenever this happens instead of allowing it.

An example of this causing an issue is when Prafulla Giri posted a 
patch[0] to fix a bug with Calibre. Their code ought to be correct, but 
it resulted in double wrapping. I created my own patch by overwriting 
the python-build-systems wrap phase and duplicating some code. Andreas 
ended up accepting my patch instead.

... Actually I just realised Prafulla's patch could have been fixed in a 
much simpler way by adjusting the (find-files ...) bit and avoided 
duplication. ...

Anyway, with these patches, Prafulla's patch would have caused an error 
and forced them to fix it, for example, by changing

(find-files "." ".")

to

(find-files "." (lambda (file stat) (not (wrapper? file))))
or
(find-files "." (lambda (file stat) (not (string-prefix "." (basename file))))

----------

So, the main change here is making (wrap-program ".foo-real") an error. 
If you cannot think of a good reason why that should ever be run, I 
think its good to block it. bugs that can slip through easily and lurk 
in the background usually not causing problems are not good in my 
opinion. After that has been decided we need to ensure all build systems 
don't misuse wrap-program that way. I notice some build systems actually 
only pass 'regular files, others allow symlinks or any file. I'm not 
really sure what the exact find-files filter should be.

[0] https://lists.gnu.org/archive/html/guix-patches/2020-09/msg00219.html


[-- Attachment #2: Type: text/html, Size: 4788 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#43367: [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping.
  2020-09-13  5:39 [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Brendan Tildesley
  2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
  2020-09-13  9:40 ` [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Danny Milosavljevic
@ 2021-04-22  9:06 ` Ludovic Courtès
  2 siblings, 0 replies; 9+ messages in thread
From: Ludovic Courtès @ 2021-04-22  9:06 UTC (permalink / raw)
  To: Brendan Tildesley; +Cc: 43367-done

Hi Brendan,

Brendan Tildesley <mail@brendan.scot> skribis:

> I'm attempting to fix a bug where wrap-program produces ..X-real-real
> files by mistakenly wrapping already wrapped files. I haven't fully 
> tested these because it requires rebuilding everything which takes
> hours to days and core-updates is stuck on mesa now anyway. Perhaps
> I'll try testing on master. Also there may be other places where
> .X-real files are accidentally wrapped, which will now error.

The patch series LGTM and I’ve applied it on ‘core-updates’.  I’m
building things now and will push shortly if everything goes well.

Thank you and sorry for the loooong delay!

Ludo’.




^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-04-22  9:08 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-13  5:39 [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Brendan Tildesley
2020-09-13  5:45 ` [bug#43367] [PATCH 1/5] utils: wrap-program: Refuse to wrap .X-real files Brendan Tildesley
2020-09-13  5:45   ` [bug#43367] [PATCH 2/5] utils: Rename wrapper? to wrapped-program? Brendan Tildesley
2020-09-13  5:45   ` [bug#43367] [PATCH 3/5] glib-or-gtk-build-system: Don't double wrap programs Brendan Tildesley
2020-09-13  5:45   ` [bug#43367] [PATCH 4/5] rakudo-build-system: " Brendan Tildesley
2020-09-13  5:45   ` [bug#43367] [PATCH 5/5] qt-build-system: " Brendan Tildesley
2020-09-13  9:40 ` [bug#43367] [core-updates]: [PATCH 0/5]: Prevent wrap-progam from double-wrapping Danny Milosavljevic
2020-09-13 12:30   ` Brendan Tildesley
2021-04-22  9:06 ` bug#43367: " 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).