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