FTR I think this is a very useful addition for packages for which we need to specify a tag while building them in Fedora.

Please prefix it with a GO_ namespace.

Or rather, use GO prefix instead...

No -- this needs to match the go-rpm-macros used in c9s:

https://gitlab.com/redhat/centos-stream/rpms/go-rpm-macros/-/blob/c9s/remove-fedora-dependency-automation.patch#L215

No -- this needs to match the go-rpm-macros used in c9s:

https://gitlab.com/redhat/centos-stream/rpms/go-rpm-macros/-/blob/c9s/remove-fedora-dependency-automation.patch#L215

No, c9s needs to match what we do. You have done this backwards.

I have not done anything other than try to make things match so that I don't have to kludge my spec file.

:weary:

https://bugzilla.redhat.com/show_bug.cgi?id=1999335

+1 to GOBUILDTAGS. The macros already make use of GOBUILDFLAGS, so this follows convention. It also avoids the (albeit narrow) situation where BUILDTAGS is already assumed to mean "C build tags" and a spec file is attempting to modify both CGo build configurations and Go build configurations. It's trivial to adjust existing spec files to use GOBUILDTAGS instead of BUILDTAGS.

I second the @ngompa 's take here.

I'm actually confused why %gobuildflags exists, because %gobuild below that doesn't use it. In fact, %gobuild uses an exact copy of gobuildflags with BUILDTAGS already there.

I guess gobuildflags is there if you need to somehow call go build manually? But in that case adding BUILDTAGS to gobuildflags is only halfway to making things consistent. gobuild should also be changed to use gobuildflags, or things will fall out of sync again.

I'm actually confused why %gobuildflags exists, because %gobuild below that doesn't use it. In fact, %gobuild uses an exact copy of gobuildflags with BUILDTAGS already there.

I guess gobuildflags is there if you need to somehow call go build manually? But in that case adding BUILDTAGS to gobuildflags is only halfway to making things consistent. gobuild should also be changed to use gobuildflags, or things will fall out of sync again.

This does look like an optimization that could be made to tidy up the macro a bit. Feel free to PR this if you feel like it's a good idea. I traced through the commit history and found the commit[1] where the %gobuildflags macro was created. It looks like it might have been simply an oversight to not update the %gobuild macro to use the new %gobuildflags macro.

This is the last we should say on this subject on this PR though; this is not actually related to this PR and should be discussed on a separate PR or issue.

1: https://pagure.io/go-rpm-macros/c/67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2

+1 to GOBUILDTAGS. The macros already make use of GOBUILDFLAGS, so this follows convention. It also avoids the (albeit narrow) situation where BUILDTAGS is already assumed to mean "C build tags" and a spec file is attempting to modify both CGo build configurations and Go build configurations. It's trivial to adjust existing spec files to use GOBUILDTAGS instead of BUILDTAGS.

I think I might be changing my opinion on this. Looking at the macros again, internally, they look for variables such as BUILDTAGS and LDFLAGS[1]. The GOBUILDFLAGS mention only actually occurs in a comment, proposing an example usage of declaring a make or autoconf variable called GOBUILDFLAGS (the assumption being that the Makefile or configure.ac is looking for the variable GOBUILDFLAGS in order to pass it to a call to the compiler directly). The %gobuildflags macro and the %gobuild macro both look for LDFLAGS internally to expand the value it passed to the -ldflags option. Therefore, updating the %gobuildflags macro to use BUILDTAGS, just like the %gobuild macro is already doing, makes a lot of sense.

1: https://pagure.io/go-rpm-macros/blob/master/f/rpm/macros.d/macros.go-compilers-golang#_43

@linkdupont looking at the macros. LDFLAGS should have been passed in to the -ldflags='-extldflags "$LDFLAGS"'. I belive this is a bug in macros as generic "C" ldflags shouldn't really work well with the GC's internal linker. AFAIK at least there is no guarantee that they will. I don't know how I have missed that.

Would a value in the environment variable LDFLAGS get pulled in by the -extldflags '%__global_ldflags %{?__golang_extldflags}' part? Does %__global_ldflags or %__global_extldflags resolve to including $LDFLAGS?

I'm actually confused why %gobuildflags exists, because
%gobuild below that doesn't use it. In fact, %gobuild
uses an exact copy of gobuildflags with BUILDTAGS
already there.

A bit of Git forensics reveals:

commit 67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2
Author:     Daniel P. Berrangé <berrange@redhat.com>
AuthorDate: Wed Sep 18 16:49:58 2019 +0100
Commit:     Daniel P. Berrangé <berrange@redhat.com>
CommitDate: Thu Sep 19 11:36:33 2019 +0100

    macros: define a %gobuildflags macro

    Using the %gobuild macro is fine for a project where the go
    code is the only thing being built, and can be built directly
    by invoking the Go toolchain from RPM.

    In more complex cases though, the Go code is just a small part
    of the project and the Go toolchain is invoked by a build
    system such as make (possibly automake), or meson. In such a
    case we need to be able to tell this build system what flags
    to pass to the compiler.

    The %gobuildflags macros services this purpose allowing a
    RPM spec todo

      GOBUILDFLAGS="%gobuildflags" %configure

    or

      %make GOBUILDFLAGS="%gobuildflags"

    Ideally the %gobuild macro would in turn reference the
    %gobuildflags macro, but that does not appear possible
    given the semantics around quote expansion and escaping
    across RPM and shell.

    Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>

Speaking as someone who maintains a code base where go build is invoked by Meson ...

I traced through the commit history and found the commit[1]
where the %gobuildflags macro was created. It looks like it
might have been simply an oversight to not update the
%gobuild macro to use the new %gobuildflags macro.

Maybe something has changed since then, and my memory is a bit foggy because it's been a while since my deep dive into the Go toolchain and the RPM macros, but it was definitely not an oversight to have %gobuildflags duplicate %gobuild. The commit says:

    Ideally the %gobuild macro would in turn reference the
    %gobuildflags macro, but that does not appear possible
    given the semantics around quote expansion and escaping
    across RPM and shell.

Since commit c593262 or PR #35, %gobuild does use %gobuildflags. It's been a while since that change, so I suppose at least %gobuild still works. I am struggling to spot any relevant change to %gobuildflags, so I guess it's a happy ending. :)

Anyway, I think this pull request should be closed because BUILDTAGS is already part of %gobuildflags these days. See commit c593262 or PR #35.

The open question seems to be whether BUILDTAGS should be renamed to GOBUILDTAGS, but that wasn't what this pull request was about.