Bug 690695 - ghostscript-gpl-8.70 partially ignores LDFLAGS
Summary: ghostscript-gpl-8.70 partially ignores LDFLAGS
Status: RESOLVED FIXED
Alias: None
Product: Ghostscript
Classification: Unclassified
Component: Build Process (show other bugs)
Version: 8.70
Hardware: PC Linux
: P4 blocker
Assignee: Hin-Tak Leung
QA Contact: Bug traffic
URL: http://bugs.gentoo.org/show_bug.cgi?i...
Keywords:
: 691208 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-08-07 04:40 UTC by Timo Gurr
Modified: 2010-05-03 04:21 UTC (History)
2 users (show)

See Also:
Customer:
Word Size: ---


Attachments
ghostscript-gpl-8.64-respect-gsc-ldflags.patch (713 bytes, patch)
2009-08-07 04:40 UTC, Timo Gurr
Details | Diff
0001-Fix-unix-so-build-regression.patch (1.81 KB, patch)
2010-01-11 04:14 UTC, Hib Eris
Details | Diff
an alternative patch to fix 'make so' (1.08 KB, patch)
2010-02-12 17:19 UTC, Hin-Tak Leung
Details | Diff
0001-Fix-unix-so-build-regression.patch (1.81 KB, patch)
2010-03-24 22:06 UTC, Hib Eris
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Timo Gurr 2009-08-07 04:40:22 UTC
Gentoo's package manager Portage reports various QA issues when installing a
package. We got a bugreport about partially ignored LDFLAGS, see URL and
original bugreport for gs 8.64 at http://bugs.gentoo.org/show_bug.cgi?id=209803.

 * QA Notice: Files built without respecting LDFLAGS have been detected
 *  Please include the following list of files in your report:         
 * /usr/bin/gsx

Attaching a fix to solve this issue, written by Peter Volkov (Gentoo Developer).
Comment 1 Timo Gurr 2009-08-07 04:40:49 UTC
Created attachment 5284 [details]
ghostscript-gpl-8.64-respect-gsc-ldflags.patch
Comment 2 Ralph Giles 2009-08-07 11:29:31 UTC
Thanks for the patch. I've committed a variation as r9948.
Comment 3 Hib Eris 2010-01-11 04:13:33 UTC
The fix for this bug in r9948 caused a regression in building
dynamically linked gsc/gsx with 'make so'.

When I do:
> configure
> make so
> cd sobin
> export LD_LIBRARY_PATH=`pwd`
> ./gsc ../examples/tiger.eps
Segmentation fault

I will attach a patch to fix this.
Comment 4 Hib Eris 2010-01-11 04:14:23 UTC
Created attachment 5862 [details]
0001-Fix-unix-so-build-regression.patch
Comment 5 Hib Eris 2010-02-03 01:55:00 UTC
Please see the issue in comment 3.

I am not sure this is critical for you, but as the 8.71 freeze is here, I think
it is a good idea to look at this before releasing.
Comment 6 Hin-Tak Leung 2010-02-12 17:19:52 UTC
Created attachment 5952 [details]
an alternative patch to fix 'make so'

It does similiar thing as attachment 5862 [details] - the important thing is that the
shared library needed to be compiled with a different LDFLAGS compared to those
two binaries which depends on the library. I think this is a better patch than
5862 as it also applies user-supplied LDFLAGS to those two binaries, but feel
free to comment and/or differ.
Comment 7 Hib Eris 2010-02-13 10:56:13 UTC
In reply to comment 6:

The make rules to build $(GSSOC_XE) and $(GSSOX_XE) already contain LDFLAGS, so
I think there is no need to specify LDFLAGS in the 'so' target.

Personally I think we should never redefine the user variable LDFLAGS anywhere.
The variable LDFLAGS must be part of the command lines that do the linking only.

Therefore I think patch 5862 is better. But that being said, I would be happy if
any of these patches fixing this bug would be applied, either 5862 or 5952. 


Comment 8 Hin-Tak Leung 2010-02-13 13:18:32 UTC
Hmm, there were two things which I didn't like about patch 5862 - introducing
another variable (GS_XE_LDFLAGS) which is essentially single-use and does not
differ at all from LDFLAGS_SO, and it modifies the GS_XE target - see this
sentence - 'Shared object is built by redefining GS_XE in a recursive make'
It appears that the GS_XE target is used for many things.

Actually just using LDFLAGS_SO where GS_XE_LDFLAGS is , is probably best, except
again, the GS_XE target is used for multiple binaries, and also the
include-order in most files is unixlink.mak then unix-dll.mak .

In any case, the bottom of the problem is that shared libraries needs a
different LDFLAGS from executables.
Comment 9 Hib Eris 2010-02-17 03:08:01 UTC
In reply to comment 8:

I disagree with you :-). You say you do not like the fact that patch 5862
changes the GS_XE target, but in patch 5952 you do essentially the same thing
and worse: by redefining LDFLAGS you add LDFLAGS_SO to it. What's worse about
this, is that potentially, this LDFLAGS_SO gets passed to all other targets that
use LDFLAGS. To prevent this, it is wise to use a target specific LDFLAGS:
GS_XE_LDFLAGS.

As you noted, shared objects are build by redefining GS_XE. In my opinion,
because executables need different LDFLAGS then shared objects, when you
redefine GS_XE, you should also redefine GS_XE_LDFLAGS, which is what I did in
patch 5862.

Keeping both LDFLAGS_SO and GS_XE_LDFLAGS, while GS_XE_LDFLAGS=LDFLAGS_SO is
done because GS_XE_LDFLAGS is a 'build variable', whereas LDFLAGS_SO is a
parameter that can change depending on the linker you build with or the platform
you build on.




Comment 10 Hin-Tak Leung 2010-02-17 12:37:14 UTC
I am not sure I follow all the arguments. There are 3 ways the make system
inherits variables definitions - as environment variables (export VAR1=value1),
specified on the command line of invocation of make (e.g. "make VAR1=value1) or
within the Makefile. the variable LDFLAGS has special/conventional meaning
because there is an implicit linker rule if none is specified in the Makefile
(and in such cases, the linker will take it from the environment variable).
LDFLAGS to mean 'the (current) linker flags', given we have to have a different
set of flags for shared libraries, there is a need for a 2nd variable _SO (or at
least an 'additional' part to it), I am not sure where a 3rd one fit in,
especially when it changes between invocations. Anyhow, even reverting r9948 is
preferable to the current state where 'make so' does not give correct executables.

I am against making more changes/additons to _the body of_ the GE_XE target,
since it is the most important one.
Comment 11 Hin-Tak Leung 2010-03-22 12:24:07 UTC
*** Bug 691208 has been marked as a duplicate of this bug. ***
Comment 12 Hib Eris 2010-03-24 22:06:09 UTC
Created attachment 6124 [details]
0001-Fix-unix-so-build-regression.patch

New patch, fixing a minor mistake.
Comment 13 Hin-Tak Leung 2010-03-24 22:29:26 UTC
(In reply to comment #12)
> Created an attachment (id=6124) [details]
> 0001-Fix-unix-so-build-regression.patch
> 
> New patch, fixing a minor mistake.

I'd prefer a solution that does not modify the GS_XE target itself. The reason being that the target is invoked multiple times with different environments to build different things, and to make it formally more complicated (and keeping track of how/where it is invoked and which variables are defined to what values) by introducing another variable which takes different values per invocation.

But in any case, any solution is better than doing nothing.
Comment 14 Hin-Tak Leung 2010-05-02 03:12:51 UTC
Grabbing a Ralph's bugs.
Comment 15 Hin-Tak Leung 2010-05-03 04:19:45 UTC
My patch (attachment 5952 [details]) committed as r11169 , since I'll likely inherit new 'Build Process' bugs - I'd prefer to fix things I break, rather than others break.
Comment 16 Hin-Tak Leung 2010-05-03 04:21:04 UTC
Meant to close with the last comment.