12:05 p.m.
bcl opened a new pull-request against the project: `go-rpm-macros` that you are
following:
``
Add BUILDTAGS to %gobuildflags
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
9:49 a.m.
obudai commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
FTR I think this is a very useful addition for packages for which we need to specify a tag
while building them in Fedora.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
9:52 a.m.
ngompa commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
Please prefix it with a `GO_` namespace.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
10:19 a.m.
ngompa commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
Or rather, use `GO` prefix instead...
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
4:09 p.m.
bcl commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
No -- this needs to match the go-rpm-macros used in c9s:
https://gitlab.com/redhat/centos-stream/rpms/go-rpm-macros/-/blob/c9s/rem...
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
5:07 p.m.
ngompa commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
No -- this needs to match the go-rpm-macros used in c9s:
https://gitlab.com/redhat/centos-stream/rpms/go-rpm-macros/-/blob/c9s/rem...
No, `c9s` needs to match what _we_ do. You have done this backwards.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
5:31 p.m.
bcl commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
*I* have not done anything other than try to make things match so that I don't have to
kludge my spec file.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
5:33 p.m.
ngompa commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
:weary:
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
6:06 p.m.
bcl commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
https://bugzilla.redhat.com/show_bug.cgi?id=1999335
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
6:47 p.m.
linkdupont commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
+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`.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
6:56 a.m.
jcajka commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
I second the @ngompa 's take here.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
3:52 a.m.
qulogic commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
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.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
6:37 p.m.
linkdupont commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
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.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
6:51 p.m.
linkdupont commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
+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-comp...
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
2:32 a.m.
jcajka commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
@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.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
9:54 a.m.
linkdupont commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
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?
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
7:05 a.m.
rishi commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
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](https://pagure.io/go-rpm-macros/c/67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2):
```
commit 67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2
Author: Daniel P. Berrangé <berrange(a)redhat.com>
AuthorDate: Wed Sep 18 16:49:58 2019 +0100
Commit: Daniel P. Berrangé <berrange(a)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(a)redhat.com>
```
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
7:16 a.m.
rishi commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
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](https://pagure.io/go-rpm-macros/c/67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2)
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.
```
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
7:27 a.m.
rishi commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
Since commit
[c593262127afd28d](https://pagure.io/go-rpm-macros/c/c593262127afd28d9f0b52a78a3cf0ef5b14673f)
or PR [#35](https://pagure.io/go-rpm-macros/pull-request/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. :)
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
7:30 a.m.
rishi commented on the pull-request: `Add BUILDTAGS to %gobuildflags` that you are
following:
``
Anyway, I think this pull request should be closed because `BUILDTAGS` is already part of
`%gobuildflags` these days. See commit
[c593262127afd28d](https://pagure.io/go-rpm-macros/c/c593262127afd28d9f0b52a78a3cf0ef5b14673f)
or PR [#35](https://pagure.io/go-rpm-macros/pull-request/35).
The open question seems to be whether `BUILDTAGS` should be renamed to `GOBUILDTAGS`, but
that wasn't what this pull request was about.
``
To reply, visit the link below or just reply to this email
https://pagure.io/go-rpm-macros/pull-request/34
427
days inactive
972
days old
golang@lists.fedoraproject.org
19 comments
7 participants
participants (7)
-
Brian C. Lane -
Debarshi Ray
-
Elliott Sales de Andrade
-
Jakub Čajka
-
Link Dupont
-
Neal Gompa
-
Ondřej Budai