#21 Expose Go build flags directly to spec files as a macro
Merged 5 months ago by ngompa. Opened 5 months ago by berrange.
berrange/go-rpm-macros flags  into  master

@@ -20,22 +20,28 @@ 

  #

  # SPDX-License-Identifier: GPL-3.0-or-later

  

+ # This *must* be all on one line, as it will be used in shell

+ # assignments. eg

+ #

+ #    GOBUILDFLAGS="%gobuildflags" %configure

+ #

+ # Or

+ #

+ #    %make GOBUILDFLAGS="%gobuildflags"

+ #

+ %gobuildflags() %{expand:%{gocompilerflags} -tags=\\"rpm_crashtraceback \\" -ldflags \\"${LDFLAGS:-}%{?currentgoldflags} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \\n') -extldflags '%__global_ldflags %{?__golang_extldflags}'\\" -a -v -x}

+ 

  # Define commands for building

  # BUILD_ID can be generated for golang build no matter of debuginfo

  %gobuild(o:) %{expand:

    # https://bugzilla.redhat.com/show_bug.cgi?id=995136#c12

    %global _dwz_low_mem_die_limit 0

-   %ifnarch ppc64

-   %{?gobuilddir:GOPATH="%{gobuilddir}:${GOPATH:+${GOPATH}:}%{?gopath}"} GO111MODULE=off \\

-   go build -buildmode pie -compiler gc -tags="rpm_crashtraceback ${BUILDTAGS:-}" -ldflags "${LDFLAGS:-}%{?currentgoldflags} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \\n') -extldflags '%__global_ldflags %{?__golang_extldflags}'" -a -v -x %{?**};

-   %else

    %{?gobuilddir:GOPATH="%{gobuilddir}:${GOPATH:+${GOPATH}:}%{?gopath}"} GO111MODULE=off \\

-   go build                -compiler gc -tags="rpm_crashtraceback ${BUILDTAGS:-}" -ldflags "${LDFLAGS:-}%{?currentgoldflags} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \\n') -extldflags '%__global_ldflags %{?__golang_extldflags}'" -a -v -x %{?**};

-   %endif

+   go build %{?gocompilerflags} -tags="rpm_crashtraceback ${BUILDTAGS:-}" -ldflags "${LDFLAGS:-}%{?currentgoldflags} -B 0x$(head -c20 /dev/urandom|od -An -tx1|tr -d ' \\n') -extldflags '%__global_ldflags %{?__golang_extldflags}'" -a -v -x %{?**};

  }

  ${workroot}${GOPATH:+:${GOPATH}}

  

  # Define commands for testing

- %gotestflags      -buildmode pie -compiler gc

+ %gotestflags      %{gocompilerflags}

  %gotestextldflags %__global_ldflags %{?__golang_extldflags}

  %gotest() GO111MODULE=off go test %{gotestflags} -ldflags "${LDFLAGS:-}%{?currentgoldflags} -extldflags '%{gotestextldflags}'" %{?**};

@@ -0,0 +1,2 @@ 

+ 

+ %gocompilerflags -compiler gc

@@ -0,0 +1,2 @@ 

+ 

+ %gocompilerflags -buildmode pie -compiler gc

The commit message in the second patch has details of key rationale for this change. Essentially though in libvirt, we are integrating Go code for parts of our project and this code is built by our existing build system. It is not possible to have RPM directly run "go build", so we need to pass Go build flags to make.

Along with these patches, the Fedora rawhide go-rpm-macros spec file would need the following change

diff --git a/go-rpm-macros.spec b/go-rpm-macros.spec
index 83e02d7..8de41a6 100644
--- a/go-rpm-macros.spec
+++ b/go-rpm-macros.spec
@@ -7,7 +7,9 @@ Version:   3.0.8
 %global _docdir_fmt     %{name}

 # Master definition that will be written to macro files
-%global golang_arches   %{ix86} x86_64 %{arm} aarch64 ppc64le s390x
+%global golang_pie_arches  %{ix86} x86_64 %{arm} aarch64 s390x
+%global golang_nopie_arches ppc64le
+%global golang_arches   %{golang_pie_arches} %{golang_nopie_arches}
 %global gccgo_arches    %{mips}
 # Go sources can contain arch-specific files and our macros will package the
 # correct files for each architecture. Therefore, move gopath to _libdir and
@@ -125,6 +127,12 @@ install -m 0755 -vp   rpm/*\.{prov,deps} \
 %ifarch %{golang_arches}
 install -m 0644 -vp   rpm/macros.d/macros.go-compilers-golang \
                       %{buildroot}%{_rpmconfigdir}/macros.d/macros.go-compiler-golang
+%ifarch %{golang_pie_arches}
+rm -f %{buildroot}/%{_rpmconfigdir}/macros.d/macros.go-compilers-golang-nopie
+%endif
+%ifarch %{golang_nopie_arches}
+rm -f %{buildroot}/%{_rpmconfigdir}/macros.d/macros.go-compilers-golang-pie
+%endif
 %endif

 %ifarch %{gccgo_arches}

I've tested this with the trivial app using a specfile that does this:

%make_build GOBUILDFLAGS="%gobuildflags"

which calls a makefile which does this

all:
        go build $(GOBUILDFLAGS) -o demo demo.go

Pull-Request has been merged by ngompa

5 months ago

BTW, i just realized my suggested spec file change is not quite right. The original code turned off "pie" for "ppc64", not "ppc64le".

Yes, the patch snippet should be something like:

diff --git a/go-rpm-macros.spec b/go-rpm-macros.spec
index 83e02d7..8de41a6 100644
--- a/go-rpm-macros.spec
+++ b/go-rpm-macros.spec
@@ -7,7 +7,9 @@ Version:   3.0.8
 %global _docdir_fmt     %{name}

 # Master definition that will be written to macro files
-%global golang_arches   %{ix86} x86_64 %{arm} aarch64 ppc64le s390x
+%global golang_pie_arches  %{ix86} x86_64 %{arm} aarch64 ppc64le s390x
+%global golang_nopie_arches %{nil}
+%global golang_arches   %{golang_pie_arches} %{golang_nopie_arches}
 %global gccgo_arches    %{mips}
 # Go sources can contain arch-specific files and our macros will package the
 # correct files for each architecture. Therefore, move gopath to _libdir and
@@ -125,6 +127,12 @@ install -m 0755 -vp   rpm/*\.{prov,deps} \
 %ifarch %{golang_arches}
 install -m 0644 -vp   rpm/macros.d/macros.go-compilers-golang \
                       %{buildroot}%{_rpmconfigdir}/macros.d/macros.go-compiler-golang
+%ifarch %{golang_pie_arches}
+rm -f %{buildroot}/%{_rpmconfigdir}/macros.d/macros.go-compilers-golang-nopie
+%endif
+%ifarch %{golang_nopie_arches}
+rm -f %{buildroot}/%{_rpmconfigdir}/macros.d/macros.go-compilers-golang-pie
+%endif
 %endif

 %ifarch %{gccgo_arches}

@berrange That’s awfully nice, both the runtime ifnarch removal and the generalization to pie/nopie macros

If you think the PR is cooked and @jcajka agrees with it I see no problem in merging it

I probably need to check if the mechanism can be used in go-rpm-macros for tests, having a common way to do things is always good.

Remember that you’re going to lose GOPATH and GO111MODULE assignments, and depending on the Go module weirdness of the day, that may not end well when building within koji.

My only thought for future improvements is to also expose a %gotestflags macro with the equivalent set of flags for testing..... except such a named macro is already in use just setting the compile mode flags right now. I wondered whether you consider that current macro part of the "api promise" or just an internal thing we're able to expand the semantics of ?

My only thought for future improvements is to also expose a %gotestflags macro with the equivalent set of flags for testing..... except such a named macro is already in use just setting the compile mode flags right now. I wondered whether you consider that current macro part of the "api promise" or just an internal thing we're able to expand the semantics of ?

This is question for @nim as he is only one whom might know.

Changes LGTM, assuming that the land in to the Fedora will be co-ordinated and announced(if there are any forward of backwards implications) by the maintainers of the respective packages there.