Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1548437 - cyrus-sasl: Incomplete injection of Fedora build flags
Summary: cyrus-sasl: Incomplete injection of Fedora build flags
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: cyrus-sasl
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jakub Jelen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: Fedora28BuildFlags
TreeView+ depends on / blocked
 
Reported: 2018-02-23 14:11 UTC by Florian Weimer
Modified: 2018-03-19 17:50 UTC (History)
4 users (show)

Fixed In Version: cyrus-sasl-2.1.27-0.1rc7
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-03-19 17:50:33 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

Description Florian Weimer 2018-02-23 14:11:55 UTC
/usr/lib64/libsasl2.so.3.0.0 in cyrus-sasl-lib-2.1.26-37.fc28.x86_64 has not been linked with the standard Fedora linker flags (LDFLAGS) from redhat-rpm-config:

gcc -shared  auxprop.lo canonusr.lo checkpw.lo client.lo common.lo config.lo external.lo md5.lo saslutil.lo server.lo seterror.lo dlopen.lo plugin_common.lo  -L/usr/lib64/ -ldl -lresolv -lcrypt -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lkrb5support -lc  -Wl,-soname -Wl,libsasl2.so.3 -o .libs/libsasl2.so.3.0.0

Looks like LDFLAGS is not used there in the package makefile.

Furthermore, the %build section in the RPM spec file does not import the LDFLAGS from redhat-rpm-config:

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

See https://src.fedoraproject.org/rpms/redhat-rpm-config/blob/master/f/buildflags.md for information on RPM macros and environment variables provided by the build environment.

Comment 1 Florian Weimer 2018-03-07 10:23:56 UTC
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.

Comment 2 Jakub Jelen 2018-03-07 12:08:40 UTC
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?

Comment 3 Florian Weimer 2018-03-07 12:33:57 UTC
(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.

Comment 4 Jakub Jelen 2018-03-07 17:12:36 UTC
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

Comment 5 Florian Weimer 2018-03-09 16:54:28 UTC
(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.

Comment 6 Florian Weimer 2018-03-09 17:18:39 UTC
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.

Comment 7 Florian Weimer 2018-03-09 17:27:18 UTC
(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.


Note You need to log in before you can comment on or make changes to this bug.