rpms / aerc

Clone
Source Code
GIT

#6 [DRAFT] Use upstream Makefile to build aerc
Merged a year ago by gotmax23. Opened a year ago by gotmax23.
rpms/ gotmax23/aerc flags  into  rawhide

file modified
+19 -13 
@@ -59,20 +59,26 @@ 
 

  %endif
 

  

  %build
 

- export BUILDTAGS=notmuch
 

- export LDFLAGS="\
 

-     -X main.Version=%{version} \
 

-     -X main.Date="$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d)" \
 

-     -X git.sr.ht/~rjarry/aerc/config.shareDir=%{_datadir} \
 

-     -X git.sr.ht/~rjarry/aerc/config.libexecDir=%{_libexecdir} \
 

-     "
 

- %gobuild -o aerc %{goipath}
 

- 

- # The go macros interfere with C build flags.
 

- # Reset LDFLAGS and set other missing build flags.
 

- unset LDFLAGS
 

  %set_build_flags
 

- %make_build wrap colorize doc
 

+ # GO_BUILDTAGS: Enable notmuch explicitly instead of relying on auto-detection
 

+ #               in build script.
 

+ # GO_LDFLAGS: Set to an empty string so the C LDFLAGS set by %%set_build_flags
 

+ #             aren't read instead.
 

+ GO_BUILDTAGS=notmuch GO_LDFLAGS=""
 

+ 

+ # BUILD_OPTS: Set `go build` flags
 

+ # DATE: Set DATE based on SOURCE_DATE_EPOCH. The Makefile sets it based on the
 

+ #       current time.
 

+ # GOFLAGS: Set to an empty string. We want to clear the definition from the Makefile.
 

+ # GO_EXTRA_LDFLAGS: Set `go build -ldflags` argument
 

+ # Other values should be self-explanatory
 

+ %make_build \
 

+   BUILD_OPTS=%{gobuild_baseflags_shescaped} \
 

+   DATE="$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d)" \
 

+   GOFLAGS= \
 

+   GO_EXTRA_LDFLAGS=%{gobuild_ldflags_shescaped} \
 

+   PREFIX=%{_prefix} \
 

+   VERSION=%{version} \
 

  

  %install
 

  export PREFIX=%{_prefix}

This takes advantage of https://pagure.io/go-rpm-macros/pull-request/62. CI
will fail until that's merged and built.

/cc @rjarry

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/092449cff2c54a7084c08ff4b08edc2c

Based on what is there in the upstream makefile, I think this should be change to:

%set_build_flags
%make_build \
VERSION=%{version} \
DATE=$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d) \
BUILD_OPTS=%{gobuild_baseflags_shescaped} \
GO_EXTRA_LDFLAGS=%{gobuild_ldflags_shescaped}

That way, it will build everything, including docs, compiled filters and the main binary.

I am assuming that the comment about goflags interfering with C flags is not a problem anymore.

What do you think?

I cannot edit my comment, but here is what I had in mind:

diff --git a/aerc.spec b/aerc.spec
index 8f9c391f97fa..b1f5f2c2313e 100644
--- a/aerc.spec
+++ b/aerc.spec
@@ -60,19 +60,12 @@ sed -i "s|golang.org/x/crypto|github.com/ProtonMail/go-crypto|" $(find . -name "

 %build
 export BUILDTAGS=notmuch
-export LDFLAGS="\
-    -X main.Version=%{version} \
-    -X main.Date="$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d)" \
-    -X git.sr.ht/~rjarry/aerc/config.shareDir=%{_datadir} \
-    -X git.sr.ht/~rjarry/aerc/config.libexecDir=%{_libexecdir} \
-    "
-%gobuild -o aerc %{goipath}
-
-# The go macros interfere with C build flags.
-# Reset LDFLAGS and set other missing build flags.
-unset LDFLAGS
 %set_build_flags
-%make_build wrap colorize doc
+%make_build \
+       PREFIX=%{_prefix} \
+       VERSION=%{version} \
+       DATE=$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d) \
+       BUILD_OPTS=%{gobuild_baseflags_shescaped} \
+       GO_EXTRA_LDFLAGS=%{gobuild_ldflags_shescaped}

 %install
 export PREFIX=%{_prefix}
Edited a year ago by rjarry

@gotmax23, in the upstream makefile, the go build options are split in two (well three) variables that you can override and/or update.

GOFLAGS ?= $(shell contrib/goflags.sh)
BUILD_OPTS ?= -trimpath
GO_LDFLAGS :=
GO_LDFLAGS += -X main.Version=$(VERSION)
GO_LDFLAGS += -X main.Date=$(DATE)
GO_LDFLAGS += -X git.sr.ht/~rjarry/aerc/config.shareDir=$(SHAREDIR)
GO_LDFLAGS += -X git.sr.ht/~rjarry/aerc/config.libexecDir=$(LIBEXECDIR)
GO_LDFLAGS += $(GO_EXTRA_LDFLAGS)

aerc: $(gosrc)
    $(GO) build $(BUILD_OPTS) $(GOFLAGS) -ldflags "$(GO_LDFLAGS)" -o aerc

I am thinking that %{gobuild_baseflags_shescaped} should probably be used to replace GOFLAGS instead of BUILD_OPTS. Otherwise, the -tags option will be specified twice.

That would give:

%build
export BUILDTAGS=notmuch
%set_build_flags
%make_build \
    VERSION=%{version} \
    DATE=$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d) \
    GOFLAGS=%{gobuild_baseflags_shescaped} \
    GO_EXTRA_LDFLAGS=%{gobuild_ldflags_shescaped}

And BUILD_OPTS would remain set to -trimpath which is not part of %{gobuild_baseflags_shescaped}. Would that be ok?

That way, it will build everything, including docs, compiled filters and the main binary.

I am assuming that the comment about goflags interfering with C flags is not a problem anymore.

Hmm, actually I think it is. I need to look into that.

I cannot edit my comment

You should be able to edit comment after refreshing the page. You can also use Ctrl+Enter to send a comment and it'll refresh the page after submitting. The Pagure forge has some quirks...

I am thinking that %{gobuild_baseflags_shescaped} should probably be used to replace GOFLAGS instead of BUILD_OPTS. Otherwise, the -tags option will be specified twice.

That's what I tried first, but it doesn't work due to https://github.com/golang/go/issues/26849. Our flags contain quoted strings. I don't think it's a problem if -tags is specified twice; go will just read the rightmost argument.

That's what I tried first, but it doesn't work due to https://github.com/golang/go/issues/26849. Our flags contain quoted strings. I don't think it's a problem if -tags is specified twice; go will just read the rightmost argument.

Ok, then you should force GOFLAGS="" in the command, since it comes after $(BUILD_OPTS). This will prevent contrib/goflags.sh from being evaluated.

%build
export BUILDTAGS=notmuch
%set_build_flags
%make_build \
    PREFIX=%{_prefix} \
    VERSION=%{version} \
    DATE=$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d) \
    BUILD_OPTS=%{gobuild_baseflags_shescaped} \
    GOFLAGS= \
    GO_EXTRA_LDFLAGS=%{gobuild_ldflags_shescaped}
Edited a year ago by rjarry

rebased onto b721586
a year ago

The last push here along with the latest iteration of my go-rpm-macros patch should address your feedback.

Test build: https://copr.fedorainfracloud.org/coprs/gotmax23/go-rpm-macros-3.4.0_2.20240224.c9593e5/build/7059446/

Thanks!

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/75268127970c4482b47d930f947f473f

I think it needs some small changes:

--- aerc.spec   2024-02-26 00:45:30.348608898 +0100
+++ aerc.spec.fixed 2024-02-26 00:50:24.640549473 +0100
@@ -62,20 +62,23 @@ sed -i "s|golang.org/x/crypto|github.com
 %set_build_flags
 # GO_BUILDTAGS: Enable notmuch explicitly instead of relying on auto-detection
 #               in build script.
-# GO_LDFLAGS: Set to an empty string so the C LDFLAGS set by %%set_build_flags
-#             aren't read instead.
-export GO_BUILDTAGS=notmuch GO_LDFLAGS=""
+export GO_BUILDTAGS=notmuch

 # BUILD_OPTS: Set `go build` flags
-# GOFLAGS: Set to an empty string. We want to clear the definition from the Makefile.
-# GO_LDFLAGS: Set `go build -ldflags` argument
+# GOFLAGS: Set to an empty string to disable notmuch auto-detection.
+# GO_LDFLAGS: Do not override to preserve the ldflags set in the Makefile.
+# GO_EXTRA_LDFLAGS: For extra `go build -ldflags` arguments.
 # DATE: Set DATE based on SOURCE_DATE_EPOCH. The Makefile sets it based on the
-#       current time.
+#       current time. It is used my the Makefile to set variables via GO_LDFLAGS.
+# VERSION: The package version. It is used my the Makefile to set variables
+#          via GO_LDFLAGS.
 %make_build \
   BUILD_OPTS=%{gobuild_baseflags_shescaped} \
   GOFLAGS= \
-  GO_LDFLAGS=%{gobuild_ldflags_shescaped} \
+  GO_EXTRA_LDFLAGS=%{gobuild_ldflags_shescaped} \
   DATE="$(date -d "@${SOURCE_DATE_EPOCH}" +%Y-%m-%d)" \
+  VERSION=%{version} \
+  PREFIX=%{_prefix}

 %install
 export PREFIX=%{_prefix}
Edited a year ago by rjarry

rebased onto 781f199
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/886ab0274392499498671ca0fbc7fd25

@rjarry PTAL

Trailing backslash ?

Can you unset GO_LDFLAGS instead? So that it is clear that we don't shadow the value set in the makefile?

Other than these two minor nits, it looks good to me. Thanks!

Trailing backslash ?

That's on purpose to make it cleaner to add extra options in the future.

Can you unset GO_LDFLAGS instead? So that it is clear that we don't shadow the value set in the makefile?

They have to defined but empty for the goflags macros to work properly, but they don't need to be exported. I can fix that.

Other than these two minor nits, it looks good to me. Thanks!

Cool!

rebased onto 53c793f
a year ago

Build failed. More information on how to proceed and troubleshoot errors available at https://fedoraproject.org/wiki/Zuul-based-ci
https://fedora.softwarefactory-project.io/zuul/buildset/c1befdf7406f451884b2e40d0655a08d

Pull-Request has been merged by gotmax23
a year ago
Metadata
None
No Tags
Changes Summary 1
+19 -13
file changed
aerc.spec
Powered by Pagure 5.14.1
Documentation • File an Issue • About this Instance • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.