Subject: Re: [bug#70115] [PATCH] gnu: skopeo: Update to 1.15.0.

From: Tomas Volf <[email-masked]>
Date: Mon, 1 Apr 2024 15:21:03 +0200

On 2024-04-01 20:37:45 +0800, Z572 wrote:
>
> Tomas Volf <[email-masked]> writes:
>
> > The package is also converted to gnu-build-system in order to take advantage
> > of already existing Makefile instead of reinventing (parts) of it in Guile.
> >
> > * gnu/packages/virtualization.scm (skopeo)[version]: Update to 1.15.0.
> > [build-system]: Use gnu-build-system.
> > [native-inputs]: Add go-1.21, sort.
> > [inputs]: Add bash-minimal.
> > [arguments]<#:import-path, #:install-source?>: Delete.
> > <#:make-flags, #:test-target, #:imported-modules>: New arguments.
> > <#:phases>{'configure}: Delete.
> > {'set-env, 'cc-to-gcc, 'wrap-skopeo, 'remove-go-references}: New phases.
> >
> > Change-Id: I1010e1f4fbdc093646c2879bdf30125ab2e88bdd
> > ---
> > This commit updates the skopeo, and also converts it to use
> > gnu-build-system.  Both is done in one commit, since 1.15 did not
> > built just by straight-forward update, and due to the intended rewrite
> > the time spent debugging that would be wasted.
> >
> > This (gnu-build-system) matches how podman and buildah are done in #70112
> > bringing all github.com/containers/* projects on the same model.
> >
> >  gnu/packages/virtualization.scm | 80 ++++++++++++++++++++++-----------
> >  1 file changed, 54 insertions(+), 26 deletions(-)
> >
> > diff --git a/gnu/packages/virtualization.scm b/gnu/packages/virtualization.scm
> > index 251fe00aa0..60f43c19cf 100644
> > --- a/gnu/packages/virtualization.scm
> > +++ b/gnu/packages/virtualization.scm
> > @@ -151,6 +151,7 @@ (define-module (gnu packages virtualization)
> >    #:use-module (guix gexp)
> >    #:use-module (guix git-download)
> >    #:use-module (guix packages)
> > +  #:use-module (guix modules)
> >    #:use-module (guix utils)
> >    #:use-module (srfi srfi-1)
> >    #:use-module (srfi srfi-26)
> > @@ -2257,7 +2258,7 @@ (define-public umoci
> >  (define-public skopeo
> >    (package
> >      (name "skopeo")
> > -    (version "1.2.3")
> > +    (version "1.15.0")
> >      (source (origin
> >                (method git-fetch)
> >                (uri (git-reference
> > @@ -2266,12 +2267,15 @@ (define-public skopeo
> >                (file-name (git-file-name name version))
> >                (sha256
> >                 (base32
> > -                "0n22sdif437ddg5ch0ipwim3fg0n6ihc9bfi52qkhy3r1grz04hs"))))
> > -    (build-system go-build-system)
> > +                "1f9n3ysdmll7vq8dmgpv03m8aqq3w9cfvbmxxpwmnv1nlfc67ihq"))))
> > +    (build-system gnu-build-system)
> >      (native-inputs
> > -     (list pkg-config go-github-com-go-md2man))
> > +     (list go-1.21
> > +           go-github-com-go-md2man
> > +           pkg-config))
> >      (inputs
> > -     (list btrfs-progs
> > +     (list bash-minimal
> > +           btrfs-progs
> >             eudev
> >             libassuan
> >             libselinux
> > @@ -2280,27 +2284,51 @@ (define-public skopeo
> >             glib
> >             gpgme))
> >      (arguments
> > -     '(#:import-path "github.com/containers/skopeo"
> > -       #:install-source? #f
> > -       #:tests? #f                                ; The tests require Docker
> > -       #:phases
> > -       (modify-phases %standard-phases
> > -         (replace 'build
> > -           (lambda* (#:key import-path #:allow-other-keys)
> > -             (with-directory-excursion (string-append "src/" import-path)
> > -               (invoke "make" "bin/skopeo"))))
> > -         (add-after 'build 'build-docs
> > -           (lambda* (#:key import-path #:allow-other-keys)
> > -             (with-directory-excursion (string-append "src/" import-path)
> > -               (invoke "make" "docs"))))
> > -         (replace 'install
> > -           (lambda* (#:key import-path outputs #:allow-other-keys)
> > -             (with-directory-excursion (string-append "src/" import-path)
> > -               (let ((out (assoc-ref outputs "out")))
> > -                 (install-file "default-policy.json"
> > -                               (string-append out "/etc/containers"))
> > -                 (invoke "make" "install-binary" "install-completions" "install-docs"
> > -                         (string-append "PREFIX=" out)))))))))
> > +     (list
> > +      #:make-flags
> > +      #~(list (string-append "CC=" #$(cc-for-target))
> > +              "PREFIX="
> > +              (string-append "DESTDIR=" #$output)
> > +              (string-append "GOMD2MAN="
> > +                             #$go-github-com-go-md2man "/bin/go-md2man"))
> > +      #:tests? #f                       ; The tests require Docker
> > +      #:test-target "test-unit"
> > +      #:imported-modules
> > +      (source-module-closure `(,@%gnu-build-system-modules
> > +                               (guix build go-build-system)))
> > +      #:phases
> > +      #~(modify-phases %standard-phases
> > +          (delete 'configure)
> > +          (add-after 'unpack 'set-env
> > +            (lambda _
> > +              ;; When running go, things fail because HOME=/homeless-shelter.
> > +              (setenv "HOME" "/tmp")
> > +              ;; Required for detecting btrfs in hack/btrfs* due to bug in GNU
> > +              ;; Make <4.4 causing CC not to be propagated into $(shell ...)
> > +              ;; calls.  Can be removed once we update to >4.3.
> > +              ;;
> > +              ;; This techically does nothing *now*, but after upstream
> > +              ;; issue[1] is solved and 'cc-to-gcc phase is removed, it will
> > +              ;; start being required.
> > +              ;; 1: https://github.com/containers/skopeo/issues/2278
> > +              (setenv "CC" #$(cc-for-target))))
> > +          (add-after 'unpack 'cc-to-gcc
> > +            (lambda _
> > +              (for-each (lambda (file)
> > +                          (substitute* file
> > +                            (("^cc -" all)
> > +                             (string-append "g" all))))
>
> I don't think this is a good idea. Why not "$(CC)", this env can get
> from set-env phase.

I mean, sure, I could do that.  I do not think it really matters, since I *know*
we are building with gcc, but will change it.

>
> > +                        '("hack/btrfs_tag.sh"
> > +                          "hack/btrfs_installed_tag.sh"
> > +                          "hack/libdm_tag.sh"
> > +                          "hack/libsubid_tag.sh"))))
> > +          (add-after 'install 'wrap-skopeo
> > +            (lambda _
> > +              (wrap-program (string-append #$output "/bin/skopeo")
> > +                `("PATH" suffix
> > +                  ("/run/setuid-programs")))))
>
> It would be nice to add some comments about why need wrap /run/setuid-programs .

Will do.

>
> > +          (add-after 'install 'remove-go-references
> > +            (@@ (guix build go-build-system) remove-go-references)))))
> >      (home-page "https://github.com/containers/skopeo")
> >      (synopsis "Interact with container images and container image registries")
> >      (description
>

Thanks for review :)

Tomas

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.