go-rpm-macros

Clone
Source Code
GIT

#35 fix(macros): use gobuildflags macro in %gobuild
Merged 2 years ago by ngompa. Opened 2 years ago by linkdupont.
linkdupont/go-rpm-macros use-gobuildflags-macro  into  master

file modified
+2 -2 
@@ -29,7 +29,7 @@ 
 

  #
 

  #    %make GOBUILDFLAGS="%gobuildflags"
 

  #
 

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

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

Can you please change this to GOBUILDTAGS and GOLDFLAGS?

Edited 2 years ago by ngompa 

  

  # Turn off Go modules
 

  %gomodulesmode GO111MODULE=off
 
@@ -40,7 +40,7 @@ 
 

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

    %global _dwz_low_mem_die_limit 0
 

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

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

+   go build %{gobuildflags} %{?**};
 

  }
 

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

Rather than defining the build flags twice in two locations, use the %gobuildflags macro inside the %gobuild macro. Note that this moves the build flags as declared in %gobuild into %gobuildflags (notably enabling the use of ${BUILDTAGS} when using %gobuildflags in a standalone manner as documented in the macro comments). This move will maintain compatibility with any spec file that currently assumes it is able to define $BUILDTAGS before calling %gobuild.

Can you please change this to GOBUILDTAGS and GOLDFLAGS?
Edited 2 years ago by ngompa

I'm dodging that issue and just cleaning up the DRY violation part. :stuck_out_tongue_closed_eyes: But seriously, I'm not sure where I stand on that. If we were to change it to GOBUILDTAGS, are we certain there aren't existing spec files that don't already make use of the existing BUILDTAGS variable? I think I agree it should be called GOBUILDTAGS because they are build tags explicitly for the Go compiler. But I want to look through existing specs to understand what kind of a breaking change (if any) that rename would cause.

Sure, Let's figure that out and make it happen soon though. Frankly, I want these macros to make more sense...

:thumbsup:

Pull-Request has been merged by ngompa
2 years ago

I think this helps to normalize the macros by removing redundancy. I'd like to figure out the LDFLAGS use too though. It looks like the macro is assuming LDFLAGS to mean Go compiler LDFLAGS. Which seems reasonable in the context of a Go spec file. So does that mean that the macro can also assume that BUILDTAGS means Go compiler BUILDTAGS, because it's in the context of a Go spec file? There /is/ already a way to define external linker flags by setting __golang_extldflags to a value in the spec file (not sure I agree with that name either, but there it is...), so I believe it is safe to assume that LDFLAGS means "pass these values to the Go compiler's -ldflags flag.

Sure, Let's figure that out and make it happen soon though. Frankly, I want these macros to make more sense...

For the record, at the current time, the following spec files use BUILDTAGS in some capacity.

buildah.spec
go-compilers.spec
golang-github-prometheus.spec
hugo.spec
grafana.spec
moby-engine.spec
weldr-client.spec
golang-github-prometheus-node-exporter.spec
oci-seccomp-bpf-hook.spec
cri-tools.spec
stargz-snapshotter.spec
podman.spec
snapd.spec
runc.spec
syncthing.spec
source-to-image.spec
skopeo.spec
reg.spec
pack.spec
Metadata
None
No Tags
Changes Summary 1
+2 -2
file changed
rpm/macros.d/macros.go-compilers-golang
Powered by Pagure 5.14.1
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.