Opened 2 years ago

Closed 2 years ago

#24961 closed defect (fixed)

Followup to #21524 -- upgrade broken when gcc spkg is installed

Reported by: embray Owned by:
Priority: blocker Milestone: sage-8.2
Component: build Keywords:
Cc: Merged in:
Authors: Erik Bray Reviewers: John Palmieri
Report Upstream: N/A Work issues:
Branch: 861a616 (Commits) Commit: 861a6169213b065fd38240e12e19147009b354c0
Dependencies: Stopgaps:

Description


Change History (9)

comment:1 Changed 2 years ago by embray

  • Status changed from new to needs_review

comment:2 Changed 2 years ago by jhpalmieri

There was a change in a recent beta to use clang instead of gcc when available. As a result, I have machines which used to build gcc, and indeed in which $SAGE_LOCAL/bin/gcc exists, but which now use clang according to Sage's configure:

configure:8250: result:     gcc-7.2.0 not installed (configure check)

So should you check whether $(SAGE_LOCAL)/bin/gcc exists or whether gcc is going to be installed?

Version 0, edited 2 years ago by jhpalmieri (next)

comment:3 follow-up: Changed 2 years ago by jhpalmieri

I also don't understand the spacing. I am guessing that the leading space in GCC_DEP was put there intentionally in #24703 (maybe so people could concatenate dependencies without worrying about the sort of issue that arose here), so why delete it? It doesn't harm anything, right?

comment:4 in reply to: ↑ 3 Changed 2 years ago by embray

Replying to jhpalmieri:

I also don't understand the spacing. I am guessing that the leading space in GCC_DEP was put there intentionally in #24703 (maybe so people could concatenate dependencies without worrying about the sort of issue that arose here), so why delete it? It doesn't harm anything, right?

That leading space was actually needed in the old code, but it's superfluous (albeit harmless, as you say) in the new code.

comment:5 Changed 2 years ago by jhpalmieri

  • Status changed from needs_review to positive_review

comment:6 Changed 2 years ago by jhpalmieri

  • Reviewers set to John Palmieri

comment:7 Changed 2 years ago by git

  • Commit changed from 4be4011aed91b29cf6be96f2e482bb6093a49679 to 861a6169213b065fd38240e12e19147009b354c0
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

861a616Fix missing space here

comment:8 Changed 2 years ago by embray

  • Status changed from needs_review to positive_review

While we're at it, since this wasn't merged yet here's a quick fix to another bug I found.

comment:9 Changed 2 years ago by vbraun

  • Branch changed from u/embray/build/makefile-in to 861a6169213b065fd38240e12e19147009b354c0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.