#2 Convert spec to work with rewritten Toolbox
Closed a month ago by rishi. Opened 5 months ago by harrymichal.
rpms/ harrymichal/toolbox convert-to-golang-version  into  master

file modified
+1

@@ -11,3 +11,4 @@ 

  /toolbox-0.0.16.tar.xz

  /toolbox-0.0.17.tar.xz

  /toolbox-0.0.18.tar.xz

+ /toolbox-0.0.91.tar.xz

file modified
+1 -1

@@ -1,1 +1,1 @@ 

- SHA512 (toolbox-0.0.18.tar.xz) = 4b47e950bbe2dcf31d2cb155664df822f01708188615fae3304289986176002bd2ffd8b630ad2453c8cc20b93e92e2c10f38948515dede67f55b44cd4a697e5c

+ SHA512 (toolbox-0.0.91.tar.xz) = 5448abb21016003960484203cb550b47a679c852beb205d187a374870bb37c6c68fa25da24d193d557f196906d5b19a9457548987d058945ac4624a27ad7861f

@@ -0,0 +1,73 @@ 

+ From 4a1fc0c94f48c281acd1588f35c3bb95a1a1e2f2 Mon Sep 17 00:00:00 2001

+ From: =?UTF-8?q?Harry=20M=C3=ADchal?= <harrymichal@seznam.cz>

+ Date: Sat, 27 Jun 2020 16:17:56 +0200

+ Subject: [PATCH] Don't use Go's semantic import versioning

+ 

+ Fedora doesn't support Go modules when building Go programs. This

+ means that source code using semantic import versioning can't be built.

+ 

+ https://github.com/containers/toolbox/pull/484

+ ---

+  src/cmd/create.go      | 2 +-

+  src/go.mod             | 2 +-

+  src/go.sum             | 4 ++--

+  src/pkg/utils/utils.go | 2 +-

+  4 files changed, 5 insertions(+), 5 deletions(-)

+ 

+ diff --git a/src/cmd/create.go b/src/cmd/create.go

+ index 80e4f8c36ca4..98df9d248466 100644

+ --- a/src/cmd/create.go

+ +++ b/src/cmd/create.go

+ @@ -28,7 +28,7 @@ import (

+  	"github.com/containers/toolbox/pkg/podman"

+  	"github.com/containers/toolbox/pkg/shell"

+  	"github.com/containers/toolbox/pkg/utils"

+ -	"github.com/godbus/dbus/v5"

+ +	"github.com/godbus/dbus"

+  	"github.com/sirupsen/logrus"

+  	"github.com/spf13/cobra"

+  )

+ diff --git a/src/go.mod b/src/go.mod

+ index 07891e13a612..45e490f38907 100644

+ --- a/src/go.mod

+ +++ b/src/go.mod

+ @@ -6,7 +6,7 @@ require (

+  	github.com/HarryMichal/go-version v1.0.0

+  	github.com/acobaugh/osrelease v0.0.0-20181218015638-a93a0a55a249

+  	github.com/briandowns/spinner v1.10.0

+ -	github.com/godbus/dbus/v5 v5.0.3

+ +	github.com/godbus/dbus v4.1.0+incompatible

+  	github.com/sirupsen/logrus v1.4.2

+  	github.com/spf13/cobra v0.0.5

+  	golang.org/x/sys v0.0.0-20190422165155-953cdadca894

+ diff --git a/src/go.sum b/src/go.sum

+ index 5ee0d2179a39..5be421de9b13 100644

+ --- a/src/go.sum

+ +++ b/src/go.sum

+ @@ -15,8 +15,8 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs

+  github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys=

+  github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4=

+  github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=

+ -github.com/godbus/dbus/v5 v5.0.3 h1:ZqHaoEF7TBzh4jzPmqVhE/5A1z9of6orkAe5uHoAeME=

+ -github.com/godbus/dbus/v5 v5.0.3/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=

+ +github.com/godbus/dbus v4.1.0+incompatible h1:WqqLRTsQic3apZUK9qC5sGNfXthmPXzUZ7nQPrNITa4=

+ +github.com/godbus/dbus v4.1.0+incompatible/go.mod h1:/YcGZj5zSblfDWMMoOzV4fas9FZnQYTkDnsGvmh2Grw=

+  github.com/hashicorp/hcl v1.0.0/go.mod h1:E5yfLk+7swimpb2L/Alb/PJmXilQ/rhwaUYs4T20WEQ=

+  github.com/inconshreveable/mousetrap v1.0.0 h1:Z8tu5sraLXCXIcARxBp/8cbvlwVa7Z1NHg9XEKhtSvM=

+  github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8=

+ diff --git a/src/pkg/utils/utils.go b/src/pkg/utils/utils.go

+ index 08de2997865a..d97d7b86a71b 100644

+ --- a/src/pkg/utils/utils.go

+ +++ b/src/pkg/utils/utils.go

+ @@ -31,7 +31,7 @@ import (

+  

+  	"github.com/acobaugh/osrelease"

+  	"github.com/containers/toolbox/pkg/shell"

+ -	"github.com/godbus/dbus/v5"

+ +	"github.com/godbus/dbus"

+  	"github.com/sirupsen/logrus"

+  	"golang.org/x/sys/unix"

+  )

+ -- 

+ 2.25.4

+ 

@@ -0,0 +1,30 @@ 

+ From 38d6d4702c05dfa7dd48bdd70d57348ad24ca877 Mon Sep 17 00:00:00 2001

+ From: Debarshi Ray <rishi@fedoraproject.org>

+ Date: Tue, 30 Jun 2020 18:30:26 +0200

+ Subject: [PATCH] pkg/utils: Make it build on aarch64

+ 

+ The syscall.Dup2 wrapper isn't defined on aarch64, which breaks the

+ build as:

+   ../../pkg/utils/utils.go:551:12: undefined: syscall.Dup2

+ 

+ https://github.com/containers/toolbox/pull/486

+ ---

+  src/pkg/utils/utils.go | 2 +-

+  1 file changed, 1 insertion(+), 1 deletion(-)

+ 

+ diff --git a/src/pkg/utils/utils.go b/src/pkg/utils/utils.go

+ index 08de2997865a..6d38b709fb7a 100644

+ --- a/src/pkg/utils/utils.go

+ +++ b/src/pkg/utils/utils.go

+ @@ -548,7 +548,7 @@ func ShowManual(manual string) error {

+  	stderrFdInt := int(stderrFd)

+  	stdoutFd := os.Stdout.Fd()

+  	stdoutFdInt := int(stdoutFd)

+ -	if err := syscall.Dup2(stdoutFdInt, stderrFdInt); err != nil {

+ +	if err := syscall.Dup3(stdoutFdInt, stderrFdInt, 0); err != nil {

+  		return errors.New("failed to redirect standard error to standard output")

+  	}

+  

+ -- 

+ 2.25.4

+ 

@@ -0,0 +1,45 @@ 

+ From e3cadcdeb59996eb28ee6499d43ebc4b9b656a21 Mon Sep 17 00:00:00 2001

+ From: Debarshi Ray <rishi@fedoraproject.org>

+ Date: Mon, 29 Jun 2020 17:57:47 +0200

+ Subject: [PATCH] build: Make the build flags match Fedora's %{gobuild} for

+  PPC64

+ 

+ The Go toolchain doesn't play well with passing compiler and linker

+ flags via environment variables. The linker flags require a second

+ level of quoting, which leaves the build system without a quote level

+ to assign the flags to an environment variable like GOFLAGS.

+ 

+ This is one reason why Fedora doesn't have a RPM macro with only the

+ flags. The %{gobuild} RPM macro includes the entire 'go build ...'

+ invocation.

+ 

+ The Go toolchain also doesn't like the LDFLAGS environment variable as

+ exported by Fedora's %{meson} RPM macro.

+ 

+ Note that these flags are only meant for the "ppc64" CPU architecture,

+ and should be kept updated to match Fedora's Go guidelines. Use

+ 'rpm --eval "%{gobuild}"' to expand the %{gobuild} macro.

+ 

+ For some reason, when built on Koji, the final binary gets created as

+ ../src/src instead of ../src/toolbox, but it doesn't happen when

+ building locally with 'rpmbuild -ba ...'. Hence it's necessary to

+ explicitly specify the name of the output binary.

+ ---

+  src/go-build-wrapper | 3 ++-

+  1 file changed, 2 insertions(+), 1 deletion(-)

+ 

+ diff --git a/src/go-build-wrapper b/src/go-build-wrapper

+ index 9bc4e68a6f2a..d62d684b78d3 100755

+ --- a/src/go-build-wrapper

+ +++ b/src/go-build-wrapper

+ @@ -27,5 +27,6 @@ if ! cd "$1"; then

+      exit 1

+  fi

+  

+ -go build -o "$2"

+ +unset LDFLAGS

+ +go build -compiler gc -tags="rpm_crashtraceback ${BUILDTAGS:-}" -ldflags "${LDFLAGS:-} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \n') -extldflags '-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld '" -a -v -x -o "$2/toolbox"

+  exit "$?"

+ -- 

+ 2.25.4

+ 

@@ -0,0 +1,44 @@ 

+ From f4471c7921bf5a779a586a9375b5fbb252a35857 Mon Sep 17 00:00:00 2001

+ From: Debarshi Ray <rishi@fedoraproject.org>

+ Date: Mon, 29 Jun 2020 17:57:47 +0200

+ Subject: [PATCH] build: Make the build flags match Fedora's %{gobuild}

+ 

+ The Go toolchain doesn't play well with passing compiler and linker

+ flags via environment variables. The linker flags require a second

+ level of quoting, which leaves the build system without a quote level

+ to assign the flags to an environment variable like GOFLAGS.

+ 

+ This is one reason why Fedora doesn't have a RPM macro with only the

+ flags. The %{gobuild} RPM macro includes the entire 'go build ...'

+ invocation.

+ 

+ The Go toolchain also doesn't like the LDFLAGS environment variable as

+ exported by Fedora's %{meson} RPM macro.

+ 

+ Note that these flags are meant for every CPU architecture other than

+ PPC64, and should be kept updated to match Fedora's Go guidelines. Use

+ 'rpm --eval "%{gobuild}"' to expand the %{gobuild} macro.

+ 

+ For some reason, when built on Koji, the final binary gets created as

+ ../src/src instead of ../src/toolbox, but it doesn't happen when

+ building locally with 'rpmbuild -ba ...'. Hence it's necessary to

+ explicitly specify the name of the output binary.

+ ---

+  src/go-build-wrapper | 3 ++-

+  1 file changed, 2 insertions(+), 1 deletion(-)

+ 

+ diff --git a/src/go-build-wrapper b/src/go-build-wrapper

+ index 9bc4e68a6f2a..41aed1ca3d9f 100755

+ --- a/src/go-build-wrapper

+ +++ b/src/go-build-wrapper

+ @@ -27,5 +27,6 @@ if ! cd "$1"; then

+      exit 1

+  fi

+  

+ -go build -o "$2"

+ +unset LDFLAGS

+ +go build -buildmode pie -compiler gc -tags="rpm_crashtraceback ${BUILDTAGS:-}" -ldflags "${LDFLAGS:-} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \n') -extldflags '-Wl,-z,relro -Wl,--as-needed  -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld '" -a -v -x -o "$2/toolbox"

+  exit "$?"

+ -- 

+ 2.25.4

+ 

file modified
+39 -5

@@ -1,16 +1,33 @@ 

  Name:          toolbox

- Version:       0.0.18

- Release:       5%{?dist}

+ Version:       0.0.91

+ 

+ %global goipath github.com/containers/%{name}

+ %gometa

+ 

+ Release:       1%{?dist}

  Summary:       Unprivileged development environment

  

  License:       ASL 2.0

  URL:           https://github.com/containers/%{name}

  Source0:       https://github.com/containers/%{name}/releases/download/%{version}/%{name}-%{version}.tar.xz

  

- BuildArch:     noarch

+ Patch0:        toolbox-Make-it-build-on-aarch64.patch

+ 

+ # Fedora specific

+ Patch100:      toolbox-Don-t-use-Go-s-semantic-import-versioning.patch

+ Patch101:      toolbox-Make-the-build-flags-match-Fedora-s-gobuild.patch

+ Patch102:      toolbox-Make-the-build-flags-match-Fedora-s-gobuild-for-PPC64.patch

  

  BuildRequires: ShellCheck

+ BuildRequires: golang >= 1.13

  BuildRequires: golang-github-cpuguy83-md2man

+ BuildRequires: golang(github.com/HarryMichal/go-version)

+ BuildRequires: golang(github.com/acobaugh/osrelease)

+ BuildRequires: golang(github.com/briandowns/spinner) >= 1.10.0

+ BuildRequires: golang(github.com/godbus/dbus) >= 5.0.3

+ BuildRequires: golang(github.com/sirupsen/logrus) >= 1.4.2

+ BuildRequires: golang(github.com/spf13/cobra) >= 0.0.5

+ BuildRequires: golang(golang.org/x/sys/unix)

  BuildRequires: meson

  BuildRequires: pkgconfig(bash-completion)

  BuildRequires: systemd

@@ -93,10 +110,24 @@ 

  

  

  %prep

- %autosetup

+ %setup -q

+ %patch0 -p1

+ %patch100 -p1

+ 

+ %ifnarch ppc64

+ %patch101 -p1

+ %else

+ %patch102 -p1

+ %endif

+ 

+ %gomkdir

  

  

  %build

+ export GO111MODULE=off

+ export GOPATH=%{gobuilddir}:%{gopath}

+ ln -s src/cmd cmd

+ ln -s src/pkg pkg

  %meson --buildtype=plain -Dprofile_dir=%{_sysconfdir}/profile.d

  %meson_build

  

@@ -110,7 +141,7 @@ 

  

  

  %files

- %doc NEWS README.md

+ %doc CODE-OF-CONDUCT.md NEWS README.md SECURITY.md

  %license COPYING

  %{_bindir}/%{name}

  %{_datadir}/bash-completion

@@ -125,6 +156,9 @@ 

  

  

  %changelog

+ * Tue Jun 30 2020 Harry Míchal <harrymichal@seznam.cz> - 0.0.91-1

+ - Update to 0.0.91

+ 

  * Sat Jun 27 2020 Debarshi Ray <rishi@fedoraproject.org> - 0.0.18-5

  - Remove ExclusiveArch to match Podman

  

The new version of Toolbox is written in Go and that requires the .spec
file to be updated. The new version does not use Meson (that'll maybe
change in the future) so the all the necessarry files have to be copied
manually by the .spec file.

This should wait until we merge the 'rewrite-in-go' branch with the new Toolbox to also update the 'sources' file with the right checksum.

rebased onto c9c8087

5 months ago

rebased onto 9e78ebd

5 months ago

It's just sad that it's not straightforward to get 'go build' to work the Meson. :/

Expecting distributions to manually install the files one by one is asking for trouble. Sooner or later somebody will get it wrong.

Did we update our minimum Podman requirement? We still have checks and fallbacks for Podman 1.5.0.

I don't think we increased the requirement.

EDIT: I'm not sure why I increased it.

rebased onto 391fe35

3 months ago

rebased onto 340078a

3 months ago

It might be that we need to override the auto-generated %{gourl} value because running rpmlint toolbox.spec throws this error:

toolbox.spec: W: invalid-url Source0: https://github.com/containers/toolbox/archive/v0.0.90/toolbox-0.0.90.tar.gz HTTP Error 404: Not Found

Yes, exactly. I've just been looking at that.

rebased onto 34314f5

3 months ago

rebased onto b686c0c

2 months ago

rebased onto 627679b

a month ago

rebased onto 601cc76

a month ago

rebased onto bf7636b

a month ago

rebased onto 7584e56

a month ago

rebased onto f1281c4

a month ago

This should match the current state of the spec file in master. It requires a 0.0.91 release to be published upstream before merging.

Umm... what's the story behind this _build directory? I don't see it mentioned anywhere else? Did I miss something?

My guess is that it's the default build directory for projects using Go macros. Its purpose is that it is linked to src/github.com/containers/toolbox to make invocations of go build and others work.

rebased onto fe83a07

a month ago

rebased onto 36544b5

a month ago

I added a patch that I wanted upstream but @rishi didn't like the idea much because it was too Fedora-specific.

The builds are mostly fine, just aarch64 is failing due to an incompatible syscall made in src/pkg/utils/utils.go on line 551: syscall.Dup2. I think we can ignore that for now but we should fix that in the future.

My guess is that it's the default build directory for
projects using Go macros. Its purpose is that it is linked
to src/github.com/containers/toolbox to make invocations
of go build and others work.

Yeah, the _build directory is created by %{goprep} macro.

In fact, %{goprep} calls %{gomkdir} which in turn creates the directory and sets up the symbolic links.

The builds are mostly fine, just aarch64 is failing due to an
incompatible syscall made in src/pkg/utils/utils.go on
line 551: syscall.Dup2. I think we can ignore that for now
but we should fix that in the future.

Wait, what? syscall.Dup2 doesn't work on aarch64? Really?

According to the build logs, yes.

Well, we can't ignore that for now because if it fails to build on aarch64 then we just won't be able to ship the RPM.

Ah, silly me. That's a real bummer :-(

Ah, silly me. That's a real bummer :-(

Don't worry. :)

https://github.com/containers/toolbox/pull/486 seems to have fixed it.

It would be good to avoid these exotic Go-specific RPM macros because they make the build instructions appear more opaque and becomes a hindrance for those who are not experts in the ins and outs of packaging Go code in Fedora.

There's no reason why we can't stick to the more common macros.

We should mimic the entire %{gobuild} invocation, and not just this subset that's necessary for the build ID. See rpm --eval "%{gobuild}".

For extra fun, you'll notice that the value of %{gobuild} is different for PPC64.

rebased onto ba60453

a month ago

I took the liberty to make our go build ... invocation match Fedora's %{gobuild, and replaced some of the Go-specific macros with their more generic counterparts.

Did I manage to confuse Pagure by force pushing to the branch and merging with the Git CLI? :)

Anyway, merged!

Thanks @harrymichal for all your hard work and heavy lifting on this. I am sorry that it took so long, but better late than never, I suppose.

Pull-Request has been closed by rishi

a month ago