diff mbox series

[bug#35282] gnu: docker: Patch paths of xz and docker-proxy.

Message ID 87h8b0nn1z.fsf@gmail.com
State Accepted
Headers show
Series [bug#35282] gnu: docker: Patch paths of xz and docker-proxy. | expand

Checks

Context Check Description
cbaines/applying patch fail Apply failed

Commit Message

Maxim Cournoyer April 15, 2019, 12:12 a.m. UTC 
This patch series make a couple cosmetic/performance changes to the
docker package, and then goes on to patch the references to the
docker-proxy binary and the xz compression tool.

Prior to this patch, importing a docker image compressed using xz such
as in:

--8<---------------cut here---------------start------------->8---
docker load < some-docker-image.tar.xz
--8<---------------cut here---------------end--------------->8---

Would fail unless xz was found in the system's profile.

Thanks,

Maxim

Comments

Danny Milosavljevic April 15, 2019, 5:56 a.m. UTC | #1 
>diff --git a/gnu/packages/docker.scm b/gnu/packages/docker.scm
>index 10aa3aa5b4..6e598e4d18 100644
>--- a/gnu/packages/docker.scm
>+++ b/gnu/packages/docker.scm
>@@ -375,7 +375,7 @@ built-in registry server of Docker.")
>                                  ((substitute-LookPath source-text package
>                                                        relative-path)
>                                   #`(substitute* source-files
>-                                      ((#,(string-append "exec\\.LookPath\\(\""
>+                                      ((#,(string-append "\\<exec\\.LookPath\\(\""
>                                                          (syntax->datum
>                                                           #'source-text)
>                                                          "\")"))
>@@ -389,7 +389,7 @@ built-in registry server of Docker.")
>                                  ((substitute-LookPath source-text package
>                                                        relative-path)
>                                   #`(substitute* source-files
>-                                      ((#,(string-append "exec\\.Command\\(\""
>+                                      ((#,(string-append "\\<exec\\.Command\\(\""
>                                                          (syntax->datum
>                                                           #'source-text)
                                                          "\"")
Then it wouldn't match those:

./builder/dockerfile/copy_windows.go:   cmd := reexec.Command("windows-fix-permissions", source, destination, identity.SID)
./daemon/graphdriver/overlay2/mount.go: cmd := reexec.Command("docker-mountfrom", dir)
./daemon/graphdriver/windows/windows.go:                cmd := reexec.Command(append([]string{"docker-windows-write-layer", d.info.HomeDir, id}, parentLayerPaths...)...)
./pkg/chrootarchive/archive_unix.go:    cmd := reexec.Command("docker-untar", dest)
./pkg/chrootarchive/diff_unix.go:       cmd := reexec.Command("docker-applyLayer", dest)

Why did you change it?
Maxim Cournoyer April 15, 2019, 7:05 p.m. UTC | #2 
Hello Danny!

Danny Milosavljevic <dannym@scratchpost.org> writes:

>>diff --git a/gnu/packages/docker.scm b/gnu/packages/docker.scm
>>index 10aa3aa5b4..6e598e4d18 100644
>>--- a/gnu/packages/docker.scm
>>+++ b/gnu/packages/docker.scm
>>@@ -375,7 +375,7 @@ built-in registry server of Docker.")
>>                                  ((substitute-LookPath source-text package
>>                                                        relative-path)
>>                                   #`(substitute* source-files
>>-                                      ((#,(string-append "exec\\.LookPath\\(\""
>>+                                      ((#,(string-append "\\<exec\\.LookPath\\(\""
>>                                                          (syntax->datum
>>                                                           #'source-text)
>>                                                          "\")"))
>>@@ -389,7 +389,7 @@ built-in registry server of Docker.")
>>                                  ((substitute-LookPath source-text package
>>                                                        relative-path)
>>                                   #`(substitute* source-files
>>-                                      ((#,(string-append "exec\\.Command\\(\""
>>+                                      ((#,(string-append "\\<exec\\.Command\\(\""
>>                                                          (syntax->datum
>>                                                           #'source-text)
>                                                           "\"")
> Then it wouldn't match those:
>
> ./builder/dockerfile/copy_windows.go:   cmd := reexec.Command("windows-fix-permissions", source, destination, identity.SID)
> ./daemon/graphdriver/overlay2/mount.go: cmd := reexec.Command("docker-mountfrom", dir)
> ./daemon/graphdriver/windows/windows.go:                cmd := reexec.Command(append([]string{"docker-windows-write-layer", d.info.HomeDir, id}, parentLayerPaths...)...)
> ./pkg/chrootarchive/archive_unix.go:    cmd := reexec.Command("docker-untar", dest)
> ./pkg/chrootarchive/diff_unix.go:       cmd := reexec.Command("docker-applyLayer", dest)

Those are currently commented out (not in the actual code), so it was
working with the change, but thanks for bringing this to my attention.

> Why did you change it?

As the commit message says, it was to harmonize the macros' regexes with
the other regexes found in later substitute* uses.  I didn't see the
need to have different flavors of the same.

I suggest amending my patch with:

--8<---------------cut here---------------start------------->8---
@@ -389,7 +389,7 @@ built-in registry server of Docker.")
                                  ((substitute-LookPath source-text package
                                                        relative-path)
                                   #`(substitute* source-files
-                                      ((#,(string-append "\\<exec\\.Command\\(\""
+                                      ((#,(string-append "\\<(re)?exec\\.Command\\(\""
                                                          (syntax->datum
                                                           #'source-text)
                                                          "\""))
--8<---------------cut here---------------end--------------->8---

To be future proof.

What do you think?

Maxim
diff mbox series

Patch

From 646c93fa6a2a75b877153da6c006fd3a17c8dd32 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Sat, 13 Apr 2019 22:58:55 -0400
Subject: [PATCH 7/7] gnu: docker: Refer to xz by its absolute path.

* gnu/packages/docker.scm (docker)[inputs]: Add xz.
[phases]{patch-paths}: Patch the reference to xz.
---
 gnu/packages/docker.scm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/docker.scm b/gnu/packages/docker.scm
index 9dde4c6cb0..78bff8a323 100644
--- a/gnu/packages/docker.scm
+++ b/gnu/packages/docker.scm
@@ -371,6 +371,9 @@  built-in registry server of Docker.")
                 (string-append "var userlandProxyCommandName = \""
                                (assoc-ref inputs "docker-proxy")
                                "/bin/proxy\"\n")))
+             (substitute* "pkg/archive/archive.go"
+               (("string\\{\"xz")
+                (string-append "string{\"" (assoc-ref inputs "xz") "/bin/xz")))
              (let ((source-files (filter (lambda (name)
                                            (not (string-contains name "test")))
                                          (find-files "." "\\.go$"))))
@@ -530,7 +533,8 @@  built-in registry server of Docker.")
        ("runc" ,runc)
        ("util-linux" ,util-linux)
        ("lvm2" ,lvm2)
-       ("xfsprogs" ,xfsprogs)))
+       ("xfsprogs" ,xfsprogs)
+       ("xz" ,xz)))
     (native-inputs
      `(("eudev" ,eudev)      ; TODO: Should be propagated by lvm2 (.pc -> .pc)
        ("go" ,go)
-- 
2.20.1