[bug#33893,v5,2/4] gnu: Add docker-engine.

* gnu/packages/docker.scm (docker-engine): New variable.
(%docker-version): New variable.
---
 gnu/packages/docker.scm | 168 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 167 insertions(+), 1 deletion(-)

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> * gnu/packages/docker.scm (docker-engine): New variable.
> (%docker-version): New variable.

[...]

> +              ;(("LookPath") "Guix_doesnt_want_LookPath")

No longer needed?

> +         (replace 'configure
> +           (lambda _
> +             (setenv "DOCKER_GITCOMMIT" (string-append "v" ,%docker-version))
> +             (setenv "AUTO_GOPATH" "1")

Could you add a comment saying what AUTO_GOPATH does?

> +         (replace 'build
> +           (lambda _
> +             ;(invoke "hack/make.sh" "binary")
> +             ; FIXME: bash -c 'hack/validate/default && hack/make.sh'

It’s not clear to me what should be fixed; perhaps a leftover?

> +         (replace 'check
> +           (lambda _
> +             ; FIXME: Those don't find any of the go packages
> +             ; needed.  Probably GOPATH/GOROOT related.
> +             ;(invoke "hack/test/unit")
> +             #t))

That’s potentially problematic.  :-)  Any idea how difficult it would be
to run these tests?

> +         (replace 'install
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let* ((out (assoc-ref outputs "out"))
> +                    (out-bin (string-append out "/bin")))
> +               (install-file "bundles/dynbinary-daemon/dockerd" out-bin)
> +               (install-file "bundles/dynbinary-daemon/dockerd-dev" out-bin))
> +             ;(setenv "DOCKER_MAKE_INSTALL_PREFIX" (assoc-ref outputs "out"))
> +             ; TODO: KEEPBUNDLE=1
> +             ;./source/bundles/dynbinary-daemon/dockerd
> +             ;(invoke "hack/make.sh" "install-binary")

Comments can be removed?

Otherwise LGTM, thanks!

Ludo’.

Hi Ludo,

On Sun, 06 Jan 2019 21:20:35 +0100
Ludovic Courtès <ludo@gnu.org> wrote:

> Danny Milosavljevic <dannym@scratchpost.org> skribis:
> 
> > * gnu/packages/docker.scm (docker-engine): New variable.
> > (%docker-version): New variable.  
> 
> [...]
> 
> > +              ;(("LookPath") "Guix_doesnt_want_LookPath")  
> 
> No longer needed?

It was meant as a detector in order to make compilation fail when, in future
versions, docker wants to invok new stuff that we didn't patch yet.
Should we do that?

> > +         (replace 'configure
> > +           (lambda _
> > +             (setenv "DOCKER_GITCOMMIT" (string-append "v" ,%docker-version))
> > +             (setenv "AUTO_GOPATH" "1")  
> 
> Could you add a comment saying what AUTO_GOPATH does?

Yes, I'll add one.

> > +         (replace 'build
> > +           (lambda _
> > +             ;(invoke "hack/make.sh" "binary")
> > +             ; FIXME: bash -c 'hack/validate/default && hack/make.sh'  
> 
> It’s not clear to me what should be fixed; perhaps a leftover?

Yeah, I meant to check what hack/validate/default does and it seems to do
developer-specific tests (commit message formatted the right way etc), so
I guess we can just not invoke it.

> > +         (replace 'check
> > +           (lambda _
> > +             ; FIXME: Those don't find any of the go packages
> > +             ; needed.  Probably GOPATH/GOROOT related.
> > +             ;(invoke "hack/test/unit")
> > +             #t))  
> 
> That’s potentially problematic.  :-)  Any idea how difficult it would be
> to run these tests?

Go has peculiar ideas of how the directory layout is supposed to be set up.
I could probably figure it out - but if someone with more Go knowledge could
step forward it would be much faster.

> > +         (replace 'install
> > +           (lambda* (#:key outputs #:allow-other-keys)
> > +             (let* ((out (assoc-ref outputs "out"))
> > +                    (out-bin (string-append out "/bin")))
> > +               (install-file "bundles/dynbinary-daemon/dockerd" out-bin)
> > +               (install-file "bundles/dynbinary-daemon/dockerd-dev" out-bin))
> > +             ;(setenv "DOCKER_MAKE_INSTALL_PREFIX" (assoc-ref outputs "out"))
> > +             ; TODO: KEEPBUNDLE=1
> > +             ;./source/bundles/dynbinary-daemon/dockerd
> > +             ;(invoke "hack/make.sh" "install-binary")  
> 
> Comments can be removed?

Yeah.

Thanks!

Hello,

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Sun, 06 Jan 2019 21:20:35 +0100
> Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Danny Milosavljevic <dannym@scratchpost.org> skribis:
>> 
>> > * gnu/packages/docker.scm (docker-engine): New variable.
>> > (%docker-version): New variable.  
>> 
>> [...]
>> 
>> > +              ;(("LookPath") "Guix_doesnt_want_LookPath")  
>> 
>> No longer needed?
>
> It was meant as a detector in order to make compilation fail when, in future
> versions, docker wants to invok new stuff that we didn't patch yet.
> Should we do that?

I see, it sounds like a good idea.  Also add a comment explaining the
rationale.

>> > +         (replace 'build
>> > +           (lambda _
>> > +             ;(invoke "hack/make.sh" "binary")
>> > +             ; FIXME: bash -c 'hack/validate/default && hack/make.sh'  
>> 
>> It’s not clear to me what should be fixed; perhaps a leftover?
>
> Yeah, I meant to check what hack/validate/default does and it seems to do
> developer-specific tests (commit message formatted the right way etc), so
> I guess we can just not invoke it.

OK.

>> > +         (replace 'check
>> > +           (lambda _
>> > +             ; FIXME: Those don't find any of the go packages
>> > +             ; needed.  Probably GOPATH/GOROOT related.
>> > +             ;(invoke "hack/test/unit")
>> > +             #t))  
>> 
>> That’s potentially problematic.  :-)  Any idea how difficult it would be
>> to run these tests?
>
> Go has peculiar ideas of how the directory layout is supposed to be set up.
> I could probably figure it out - but if someone with more Go knowledge could
> step forward it would be much faster.

I see Leo is Cc’d so we’ll see.  :-)

Thank you,
Ludo’.

Hi Ludo,
Hi Leo,

On Tue, 08 Jan 2019 09:42:14 +0100
Ludovic Courtès <ludo@gnu.org> wrote:

> > Go has peculiar ideas of how the directory layout is supposed to be set up.
> > I could probably figure it out - but if someone with more Go knowledge could
> > step forward it would be much faster.  
> 
> I see Leo is Cc’d so we’ll see.  :-)

Nevermind, I've fixed it and learned something in the process:

Linux doesn't actually know the current working directory as a string.
It only knows the inode, so if you call getcwd, what libc actually does is
it opendirs "..", then finds the entry with the same inode number as
the current directory, and then returns the name of that entry.

Now, gopath uses symlinks to set up their preferred directory hierarchy
in such a way:

ln -s ../../../.. .gopath/src/github.com/docker/docker

Now if you chdir into ".gopath/src/github.com/docker/docker" and then Go later
does getcwd, it will appear as if the chdir did not succeed (because it will
just use the old working directory because it has the same inode).

So Go was erroring out because the directory structure there was *still* wrong.

Solution: Set environment variable PWD to the correct name of the directory.

I've pushed this patchset to master.

I'll try to add a system test next - let's see.

Howdy!

Danny Milosavljevic <dannym@scratchpost.org> skribis:

> On Tue, 08 Jan 2019 09:42:14 +0100
> Ludovic Courtès <ludo@gnu.org> wrote:
>
>> > Go has peculiar ideas of how the directory layout is supposed to be set up.
>> > I could probably figure it out - but if someone with more Go knowledge could
>> > step forward it would be much faster.  
>> 
>> I see Leo is Cc’d so we’ll see.  :-)
>
> Nevermind, I've fixed it and learned something in the process:
>
> Linux doesn't actually know the current working directory as a string.
> It only knows the inode, so if you call getcwd, what libc actually does is
> it opendirs "..", then finds the entry with the same inode number as
> the current directory, and then returns the name of that entry.

Are you sure?  In the Linux port of glibc I see this:

--8<---------------cut here---------------start------------->8---
char *
__getcwd (char *buf, size_t size)
{
  char *path;
  char *result;

  // […]

  retval = INLINE_SYSCALL (getcwd, 2, path, alloc_size);
--8<---------------cut here---------------end--------------->8---

And indeed, there’s a ‘getcwd’ syscall:

--8<---------------cut here---------------start------------->8---
$ strace -e getcwd guile -c '(getcwd)'
getcwd("/home/ludo", 100)               = 11
+++ exited with 0 +++
--8<---------------cut here---------------end--------------->8---

> Now, gopath uses symlinks to set up their preferred directory hierarchy
> in such a way:
>
> ln -s ../../../.. .gopath/src/github.com/docker/docker
>
> Now if you chdir into ".gopath/src/github.com/docker/docker" and then Go later
> does getcwd, it will appear as if the chdir did not succeed (because it will
> just use the old working directory because it has the same inode).
>
> So Go was erroring out because the directory structure there was *still* wrong.
>
> Solution: Set environment variable PWD to the correct name of the directory.

Great that you found a solution.

Thanks for taking the time to address this!

Ludo’.

Hi Ludo,

On Thu, 10 Jan 2019 09:50:49 +0100
Ludovic Courtès <ludo@gnu.org> wrote:

> Howdy!
> 
> Danny Milosavljevic <dannym@scratchpost.org> skribis:
> 
> > On Tue, 08 Jan 2019 09:42:14 +0100
> > Ludovic Courtès <ludo@gnu.org> wrote:
> >  
> >> > Go has peculiar ideas of how the directory layout is supposed to be set up.
> >> > I could probably figure it out - but if someone with more Go knowledge could
> >> > step forward it would be much faster.    
> >> 
> >> I see Leo is Cc’d so we’ll see.  :-)  
> >
> > Nevermind, I've fixed it and learned something in the process:
> >
> > Linux doesn't actually know the current working directory as a string.
> > It only knows the inode, so if you call getcwd, what libc actually does is
> > it opendirs "..", then finds the entry with the same inode number as
> > the current directory, and then returns the name of that entry.  

According to the POSIX standard ;)

> Are you sure?  In the Linux port of glibc I see this:
> 
> --8<---------------cut here---------------start------------->8---
> char *
> __getcwd (char *buf, size_t size)
> {
>   char *path;
>   char *result;
> 
>   // […]
> 
>   retval = INLINE_SYSCALL (getcwd, 2, path, alloc_size);
> --8<---------------cut here---------------end--------------->8---
> 
> And indeed, there’s a ‘getcwd’ syscall:
> 
> --8<---------------cut here---------------start------------->8---
> $ strace -e getcwd guile -c '(getcwd)'
> getcwd("/home/ludo", 100)               = 11
> +++ exited with 0 +++
> --8<---------------cut here---------------end--------------->8---

Huh.  I guess it boils down to whether the Linux "process" structure
has the cwd in it as a string or as an inode.

In Linux sources:

static inline void get_fs_pwd(struct fs_struct *fs, struct path *pwd)
{
        spin_lock(&fs->lock);
        *pwd = fs->pwd;
        path_get(pwd);
        spin_unlock(&fs->lock);
}

static void get_fs_root_and_pwd_rcu(struct fs_struct *fs, struct path *root,
                                    struct path *pwd)
{
        unsigned seq;

        do {
                seq = read_seqcount_begin(&fs->seq);
                *root = fs->root;
                *pwd = fs->pwd;
        } while (read_seqcount_retry(&fs->seq, seq));
}

struct path {
        struct vfsmount *mnt;
        struct dentry *dentry;
} __randomize_layout;

SYSCALL_DEFINE2(getcwd, char __user *, buf, unsigned long, size)
{
        int error;
        struct path pwd, root;
        char *page = __getname();

        if (!page)
                return -ENOMEM;

        rcu_read_lock();
        get_fs_root_and_pwd_rcu(current->fs, &root, &pwd);

        error = -ENOENT;
        if (!d_unlinked(pwd.dentry)) {
                unsigned long len;
                char *cwd = page + PATH_MAX;
                int buflen = PATH_MAX;

                prepend(&cwd, &buflen, "\0", 1);
                error = prepend_path(&pwd, &root, &cwd, &buflen);
                rcu_read_unlock();

                if (error < 0)
                        goto out;

                /* Unreachable from current root */
                if (error > 0) {
                        error = prepend_unreachable(&cwd, &buflen);
                        if (error)
                                goto out;
                }

                error = -ERANGE;
                len = PATH_MAX + page - cwd;
                if (len <= size) {
                        error = len;
                        if (copy_to_user(buf, cwd, len))
                                error = -EFAULT;
                }
        } else {
                rcu_read_unlock();
        }

out:
        __putname(page);
        return error;
}

/*
 * Replace the fs->{pwdmnt,pwd} with {mnt,dentry}. Put the old values.
 * It can block.
 */
void set_fs_pwd(struct fs_struct *fs, const struct path *path)
{
        struct path old_pwd;

        path_get(path);
        spin_lock(&fs->lock);
        write_seqcount_begin(&fs->seq);
        old_pwd = fs->pwd;
        fs->pwd = *path; <----------------- !!!!
        write_seqcount_end(&fs->seq);
        spin_unlock(&fs->lock);

        if (old_pwd.dentry)
                path_put(&old_pwd);
}

int ksys_chdir(const char __user *filename)
{
        struct path path;
        int error;
        unsigned int lookup_flags = LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
retry:
        error = user_path_at(AT_FDCWD, filename, lookup_flags, &path);
        if (error)
                goto out;

        error = inode_permission(path.dentry->d_inode, MAY_EXEC | MAY_CHDIR);
        if (error)
                goto dput_and_out;

        set_fs_pwd(current->fs, &path); <----------------- !!!

dput_and_out:
        path_put(&path);
        if (retry_estale(error, lookup_flags)) {
                lookup_flags |= LOOKUP_REVAL;
                goto retry;
        }
out:
        return error;
}

SYSCALL_DEFINE1(chdir, const char __user *, filename)
{
        return ksys_chdir(filename);
}


SYSCALL_DEFINE1(fchdir, unsigned int, fd)
{
        struct fd f = fdget_raw(fd);
        int error;

        error = -EBADF;
        if (!f.file)
                goto out;

        error = -ENOTDIR;
        if (!d_can_lookup(f.file->f_path.dentry))
                goto out_putf;

        error = inode_permission(file_inode(f.file), MAY_EXEC | MAY_CHDIR);
        if (!error)
                set_fs_pwd(current->fs, &f.file->f_path);
out_putf:
        fdput(f);
out:
        return error;
}

Interesting!

> Thanks for taking the time to address this!

No problem :)

On Thu, Jan 10, 2019 at 03:22:10AM +0100, Danny Milosavljevic wrote:
> Hi Ludo,
> Hi Leo,
> 
> On Tue, 08 Jan 2019 09:42:14 +0100
> Ludovic Courtès <ludo@gnu.org> wrote:
> 
> > > Go has peculiar ideas of how the directory layout is supposed to be set up.
> > > I could probably figure it out - but if someone with more Go knowledge could
> > > step forward it would be much faster.  
> > 
> > I see Leo is Cc’d so we’ll see.  :-)

Indeed, Go is very particular about this...

> Nevermind, I've fixed it and learned something in the process:

Okay, good :) Let me know if you have more Go questions.