Opened 7 years ago

Closed 6 years ago

#14296 closed defect (fixed)

Force consistency of $LD and $AS with $CC

Reported by: jdemeyer Owned by: GeorgSWeber
Priority: major Milestone: sage-5.13
Component: build Keywords:
Cc: leif, jpflori Merged in: sage-5.13.beta3
Authors: Jeroen Demeyer Reviewers: Leif Leonhardy, Jean-Pierre Flori
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

Sometimes, a system-wide GCC can be configured to use a different linker than simply ld (using configure --with-ld=/path/to/linker). The path to the linker used by GCC can be discovered by

buildbot@mark:~$ gcc -print-file-name=ld
/usr/local/binutils-2.22/sparc-SunOS-ultrasparc3-gcc-4.6.2-without-zlib/bin/ld

Unfortunately, there can be an inconsistency between $LD and the linker used by GCC, leading to build failures. We should do the following:

  1. If $LD is unset, set it to $CC -print-file-name=ld by default instead of ld.
  2. If $LD is set and the value differs from $CC -print-file-name=ld, force building of the GCC spkg (define these values to be equal if the output of command -v VALUE is equal).

Take care to support the case that $CC doesn't support the -print-file-name option.


Analogously, the same for the assembler $AS.

Attachments (1)

14296_ld_as.patch (3.5 KB) - added by jdemeyer 6 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:2 in reply to: ↑ description ; follow-up: Changed 7 years ago by leif

Replying to jdemeyer:

Unfortunately, there can be an inconsistency between $LD and the linker used by GCC, leading to build failures.

This affects only a few spkgs. IMHO we should fix these, by changing their spkg-install scripts, or -- much better -- patching the upstream sources and reporting the fixes upstream.

We should do the following:

  1. If $LD is unset, set it to $CC -print-file-name=ld by default instead of ld.

Agreed.

  1. If $LD is set and the value differs from $CC -print-file-name=ld, force building of the GCC spkg (define these values to be equal if the output of command -v VALUE is equal).

That's a bit overkill. Ok for a buildbot, but otherwise we should just issue a warning (or, more complicated, error out unless some override setting is active).

comment:3 in reply to: ↑ 2 ; follow-up: Changed 7 years ago by jdemeyer

Replying to leif:

Replying to jdemeyer:

Unfortunately, there can be an inconsistency between $LD and the linker used by GCC, leading to build failures.

This affects only a few spkgs. IMHO we should fix these, by changing their spkg-install scripts

Is it really a bug? I'd say that configuring GCC to use a different linker from $LD is an user configuration error...

comment:4 follow-up: Changed 7 years ago by leif

Replying to leif:

Replying to jdemeyer:

We should do the following:

  1. If $LD is unset, set it to $CC -print-file-name=ld by default instead of ld.

Agreed.

Thinking more about that, it's probably too surprising (not respecting PATH), at least we shouldn't do that silently (if which ld differs from what GCC returns, unless GCC returns just ld, i.e., no absolute path).

comment:5 follow-up: Changed 7 years ago by jdemeyer

Replying to leif:

  1. If $LD is set and the value differs from $CC -print-file-name=ld, force building of the GCC spkg (define these values to be equal if the output of command -v VALUE is equal).

That's a bit overkill. Ok for a buildbot, but otherwise we should just issue a warning (or, more complicated, error out unless some override setting is active).

Warnings will just be ignored. And why abort with an error if we can "fix" the issue (albeit in a complicated way) by building GCC?

comment:6 in reply to: ↑ 3 Changed 7 years ago by leif

Replying to jdemeyer:

Replying to leif:

Replying to jdemeyer:

Unfortunately, there can be an inconsistency between $LD and the linker used by GCC, leading to build failures.

This affects only a few spkgs. IMHO we should fix these, by changing their spkg-install scripts

Is it really a bug? I'd say that configuring GCC to use a different linker from $LD is an user configuration error...

Nope. AFAIK the affected packages just create (incompatible) linker scripts for the wrong linker, so their linker detection appears to be broken, or they at least feed the linker scripts to the wrong linker.

comment:7 in reply to: ↑ 4 ; follow-up: Changed 7 years ago by jdemeyer

Replying to leif:

Thinking more about that, it's probably too surprising

Given that many (most?) packages actually link with $CC and therefore use the ld configured in $CC, I don't see the problem.

comment:8 in reply to: ↑ 7 Changed 7 years ago by leif

Replying to jdemeyer:

Replying to leif:

Thinking more about that, it's probably too surprising

Given that many (most?) packages actually link with $CC and therefore use the ld configured in $CC, I don't see the problem.

Well, if all spkgs used $CC, $CXX or $FC etc. for linking, there wouldn't be a problem (modulo generating linker scripts for the wrong, i.e., not "GCC's" linker), so setting LD would be pretty pointless.

comment:9 in reply to: ↑ 5 Changed 7 years ago by leif

Replying to jdemeyer:

Replying to leif:

  1. If $LD is set and the value differs from $CC -print-file-name=ld, force building of the GCC spkg (define these values to be equal if the output of command -v VALUE is equal).

That's a bit overkill. Ok for a buildbot, but otherwise we should just issue a warning (or, more complicated, error out unless some override setting is active).

Warnings will just be ignored. And why abort with an error if we can "fix" the issue (albeit in a complicated way) by building GCC?

Well, letting the user unset LD or change / correct its setting is certainly "easier" (and less surprising) than Sage automagically building its own GCC.

comment:10 follow-up: Changed 7 years ago by jdemeyer

leif, would you agree with keeping the changes to sage-env in 14296_ld_as.patch but changing the detection in spkg/install to an error if the values of as or ld don't match (where there would never be an error if we're building GCC, since the newly-built GCC would use the setting of $LD as default linker).

Last edited 7 years ago by jdemeyer (previous) (diff)

comment:11 in reply to: ↑ 10 Changed 7 years ago by leif

In sage-env, stdout of command -v ... should also be redirected to /dev/null.

Replying to jdemeyer:

leif, would you agree with keeping the changes to sage-env in 14296_ld_as.patch

This should be ok, although a user might still intentionally have a different ld or as first in his/her PATH. (Then we'd have to document that setting AS and/or LD might be necessary to get the previous behaviour.)

but changing the detection in spkg/install to an error if the values of as or ld don't match (where there would never be an error if we're building GCC, since the newly-built GCC would use the setting of $LD as default linker).

Hmmm. It should be possible to override the error (probably by setting SAGE_PORT).

There are some subtle issues with the comparisons, not sure how artificial the following scenarios are:

  • AS and/or LD are set to (e.g.) as -foo or ld -foo, respectively (likewise /path/to/as -foo etc.).

Then e.g. command -v "$AS" gives an error (whilst command -v $AS probably doesn't.)

  • AS and/or LD are set to something like as-2.22, while (e.g.) GCC was configured with something else.
  • Opposite of the previous: GCC was configured to (e.g.) use as-2.22 (or /path/to/as, or /path/to/as-2.22), while e.g. AS is meanwhile just as, as-2.23, or /other/path/to/as etc.
  • Probably more weirdnesses.

Filename mismatches don't necessarily mean that the programs are different, or incompatible.

I don't think one couldn't work around the above situations by changing the setup, but this might be difficult for "ordinary" users, especially those without admin rights.


How does this fit your "If a user sets $VAR, assume he/she knows what he/she's doing, so don't override it; he/she doesn't need a nanny."? ;-)

comment:12 Changed 7 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Status changed from new to needs_review

comment:13 Changed 7 years ago by leif

  • Reviewers set to Leif Leonhardy

Now looks ok (and reasonable) to me.

In case users report trouble, we can always "fine-tune" the behaviour later.

Still, the documentation (on env vars, in the Installation Guide IIRC) should get updated accordingly.

Minor issue: In the messages, I wouldn't use \$PROG but PROG, since the former refers to the value of a variable rather than to the variable itself.

Will (have to) test this laterTM...

comment:14 Changed 7 years ago by jpflori

  • Cc jpflori added

comment:15 Changed 7 years ago by jpflori

I endured a nightmare on Solaris because of LD/AS/CC troubles. I'll try to gather exactly what was problematic with my install and report back here.

comment:16 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:17 Changed 6 years ago by jdemeyer

Updated with some small changes, anybody care for a review?

comment:18 Changed 6 years ago by jpflori

  • Reviewers changed from Leif Leonhardy to Leif Leonhardy, Jean-Pierre Flori
  • Status changed from needs_review to positive_review

Looks good to me as well.

Changed 6 years ago by jdemeyer

comment:19 Changed 6 years ago by jdemeyer

  • Status changed from positive_review to needs_review

Adding unsetting AS and LD in case of upgrading.

comment:20 follow-up: Changed 6 years ago by jpflori

Can you give more details of why this last change is needed?

comment:21 in reply to: ↑ 20 Changed 6 years ago by jdemeyer

Replying to jpflori:

Can you give more details of why this last change is needed?

The changes to spkg/install require the changes to sage-env. If gcc doesn't use the default assembler as, then the new spkg/install requires AS to be set to the assembler used by gcc. When upgrading, the file spkg/install is upgraded before sage-env. Therefore, we disable this check when upgrading by unsetting AS and LD if they are set to the default value.

comment:22 Changed 6 years ago by jpflori

  • Status changed from needs_review to positive_review

Ok, sure. I didn't think of the fact that spkg/install and spkg-env were also upgraded...

comment:23 Changed 6 years ago by jdemeyer

  • Merged in set to sage-5.13.beta3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.