[bug#39807] [PATCH] guix: pack: Only wrap executable files.

[Eric Bavier]

On Mon, 2020-07-27 at 16:42 -0500, Eric Bavier wrote:
> Call for help: the test does not pass!  I get this error:
> 
>   hello: run.c:284: exec_in_user_namespace: Unexpected error: No such
> file or directory.
> 
> Could someone more familiar with user namespaces, etc help me work
> this out?

After following a helpful suggestion to try using `strace`, turns out the issue 
is not with user namespaces but mostly
with string manipulation :)  Apologies in advance for wall-of-text.

  $ unshare -mrf strace -s 80 -o trace -ff \
  >  sh -c 'mount -t tmpfs -o ro none "/gnu/store"; 
            /tmp/pack-dir/opt/bin/hello'

and in one the log file corresponding to the exec of the wrapper `hello` I see

  readlink("/proc/self/exe",
           
"/tmp/pack-dir/gnu/store/80kbbxnzn3kgs1jkc6m6ydw2m44lnfaq-wrapperR/bin/hello", 
4095) = 75
  lstat("/gnu/store/zc92ghli8ws31qshf4bhzw1npzqhs4my-test/bin//hello", 
        0x7ffe308a4980) = -1 ENOENT (No such file or directory)

and in the log corresponding to the child after forking in 
exec_in_user_namespace we see the call that leads to the
above error:

  mount("/tmp/pack-dir/gnu/store/80", "/tmp/guix-exec-YMr7WJ//gnu/store", 
0x4810a7, MS_RDONLY|MS_BIND|MS_REC, NULL) = -1
ENOENT (No such file or directory)
  write(2, "hello: run.c:284: exec_in_user_namespace: Unexpected error: No such 
file or dire"..., 87) = 87

So exec_in_user_namespace is trying to mount "/tmp/pack-dir/gnu/store/80", 
which is not a directory.

In gnu/packages/aux-files/run-in-namespace.c:620-626 we try to calculate the 
name of the relocated store directory.  So
far this calculation seems to "accidentaly" work:

/tmp/pack-dir/gnu/store/78xrsg1z...-emacs-no-x-27.1R/bin/emacs
             /gnu/store/w9csar3m...-emacs-no-x-27.1/bin//emacs

The "R" suffix appended to the wrapper store directory name and the 
double-slash we get from find-files (c.f.
guix/scripts/pack.scm:881) "cancel out".   But we might not be so fortunate and 
can get something like this:

                          |
/tmp/pack-dir/gnu/store/80|kbbxnz...-wrapperR/bin/hello (self)
                /gnu/store|/zc92ghli...-test/bin//hello (@PROG@)
                /gnu/store|                     (original_store)
                          |

Because the manifest entry used in the tests added in this patch enters the 
"else" case of `wrapped-package` (c.f.
guix/scripts/pack.scm:904) the index calculation strays and we get a 
non-directory mount point.  I can make the test
pass by using a slightly longer name of "testing" for the file-union :)

I don't think we can enforce a stricter match between the wrapper and target 
store item names to ensure their lengths
are the same, right?  It seems like we maybe want to ignore @WRAPPED_PROGRAM@ 
and use only /proc/self/exe and
original_store to find the relocated store directory?  A regex search might be 
too costly.  We could use strstr to
search for the first occurrence of original_store, if we don't mind assuming 
that most people will probably not unpack
into $HOME/.guix/gnu/store/mine/packs/foo e.g.

--- a/gnu/packages/aux-files/run-in-namespace.c
+++ b/gnu/packages/aux-files/run-in-namespace.c
@@ -619,10 +619,8 @@ main (int argc, char *argv[])
 
   /* SELF is something like "/home/ludo/.local/gnu/store/…-foo/bin/ls" and we
      want to extract "/home/ludo/.local/gnu/store".  */
-  size_t index = strlen (self)
-    - strlen ("@WRAPPED_PROGRAM@") + strlen (original_store);
   char *store = strdup (self);
-  store[index] = '\0';
+  strstr (store, original_store)[sizeof original_store - 1] = '\0';
 
   struct stat statbuf;
 

WDYT?


`~Eric