Bug 1548437

Commit 020aac ("Import LDFLAGS from redhat-rpm-config (#1548437)") has this:

+ LDFLAGS="$LDFLAGS -pie -Wl,-z,now"; export LDFLAGS

Please use $RPM_LD_FLAGS or something like that, to pick up future flag changes.

Reading through the comments again, I am still not completely sure what I should change to what.

Is the correct wording supposed to be the following?

LDFLAGS="$LDFLAGS $RPM_LD_FLAGS -pie -Wl,-z,now"; export LDFLAGS

Are there any flags that I should be looking for in the build logs to verify the functionality?

Do we need it for rawhide, or for Fedora 28 also?

(In reply to Jakub Jelen from comment #2)
> Reading through the comments again, I am still not completely sure what I
> should change to what.
> 
> Is the correct wording supposed to be the following?
> 
> LDFLAGS="$LDFLAGS $RPM_LD_FLAGS -pie -Wl,-z,now"; export LDFLAGS

No, you should use $RPM_LD_FLAGS only.  Simply inherit what's defined in redhat-rpm-config, so that we can make future distribution-wide changes if necessary.

This was probably fixed by the rebase to cyrus-sasl-2.1.27, where I can notice all the flags are properly set (in rawhide build):

libtool: link: gcc -shared  -fPIC -DPIC  .libs/auxprop.o .libs/canonusr.o .libs/checkpw.o .libs/client.o .libs/common.o .libs/config.o .libs/external.o .libs/md5.o .libs/saslutil.o .libs/server.o .libs/seterror.o .libs/dlopen.o  -Wl,--whole-archive ../common/.libs/libplugin_common.a -Wl,--no-whole-archive  -L/usr/lib64/ -ldl -lresolv -lcrypt -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -O2 -g -fstack-protector-strong -grecord-gcc-switches -m64 -mtune=generic -mcet -Wl,-z -Wl,relro -Wl,-z -Wl,now   -Wl,-soname -Wl,libsasl2.so.3 -o .libs/libsasl2.so.3.0.0

Should the section Compiler Flags [1] of the Packaging Guidelines be updated to list these variables? It lists only the  %{optflags}, which is deprecated.
What about the RPMMacros page [2]? I don't see the $RPM_LD_FLAGS there either.

Anyway to verify I understand what is needed to fix:

 * The line above should probably look like this:

+LDFLAGS="$RPM_LD_FLAGS $LDFLAGS"; export LDFLAGS

Is it everything what is asked in this bug?

[1] https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags
[2] https://fedoraproject.org/wiki/Packaging:RPMMacros#Build_flags_macros_and_variables

(In reply to Jakub Jelen from comment #4)
> This was probably fixed by the rebase to cyrus-sasl-2.1.27, where I can
> notice all the flags are properly set (in rawhide build):
> 
> libtool: link: gcc -shared  -fPIC -DPIC  .libs/auxprop.o .libs/canonusr.o
> .libs/checkpw.o .libs/client.o .libs/common.o .libs/config.o
> .libs/external.o .libs/md5.o .libs/saslutil.o .libs/server.o
> .libs/seterror.o .libs/dlopen.o  -Wl,--whole-archive
> ../common/.libs/libplugin_common.a -Wl,--no-whole-archive  -L/usr/lib64/
> -ldl -lresolv -lcrypt -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -Wl,-z,now
> -specs=/usr/lib/rpm/redhat/redhat-hardened-ld -O2 -g
> -fstack-protector-strong -grecord-gcc-switches -m64 -mtune=generic -mcet
> -Wl,-z -Wl,relro -Wl,-z -Wl,now   -Wl,-soname -Wl,libsasl2.so.3 -o
> .libs/libsasl2.so.3.0.0
> 
> Should the section Compiler Flags [1] of the Packaging Guidelines be updated
> to list these variables? It lists only the  %{optflags}, which is deprecated.
> What about the RPMMacros page [2]? I don't see the $RPM_LD_FLAGS there
> either.

https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/master/f/buildflags.md is the canonical reference.  I asked the packaging people to reference the redhat-rpm-config documentation, but that hasn't happened yet:

https://pagure.io/packaging-committee/issue/743

Thanks for the Packaging:RPMMacros reference, I had not seen this before.

> Anyway to verify I understand what is needed to fix:
> 
>  * The line above should probably look like this:
> 
> +LDFLAGS="$RPM_LD_FLAGS $LDFLAGS"; export LDFLAGS
> 
> Is it everything what is asked in this bug?

I checked cyrus-sasl-lib-2.1.27-0.1rc7.fc29.x86_64, and /usr/lib64/libsasl2.so.3.0.0 now looks okay.  Thanks.

Would you please create an update for cyrus-sasl-2.1.27-0.1rc7.fc28, so that this change makes it into Fedora 28?  So far, it has only been built.

(In reply to Florian Weimer from comment #6)
> Would you please create an update for cyrus-sasl-2.1.27-0.1rc7.fc28, so that
> this change makes it into Fedora 28?  So far, it has only been built.

Disregard that.  It looks like this package was built before the Bodhi activation point, and we simply did not have a successful Fedora 28 compose since then.