Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of GCC_DEP variable in Makefile #25198

Closed
embray opened this issue Apr 17, 2018 · 16 comments
Closed

Improve handling of GCC_DEP variable in Makefile #25198

embray opened this issue Apr 17, 2018 · 16 comments

Comments

@embray
Copy link
Contributor

embray commented Apr 17, 2018

This is a followup to #25188 that I thought was worth doing while fixing that issue. However, since it's not strictly needed for the fix I'll make it a separate ticket.

Depends on #25188

CC: @vbraun

Component: build

Author: Erik Bray

Reviewer: Dima Pasechnik

Issue created by migration from https://trac.sagemath.org/ticket/25198

@embray embray added this to the sage-8.3 milestone Apr 17, 2018
@embray
Copy link
Contributor Author

embray commented Apr 17, 2018

Dependencies: #25188

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2018

Changed commit from 14b6f4d to b33c210

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 18, 2018

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

2965003add vim modeline for this file
f5f68daAdd a package that by all rights should be included in this list
15c7b88move this list into a TOOLCHAIN_DEPS variable we can use to inspect this list elsewhere
1954e5eAdd -k/--keep-existing to sage-spkg
f1411c5From -clean targets in the Makefile, just remove the package's
9976091Use sage-spkg --keep-existing when installing/re-installing packages in
22a1f34Revert to using sage-spkg-uninstall in -clean targets, but add a
d71c88fMerge branch 'u/embray/ticket-25857' into u/embray/build/ticket-25188
f062068fix accidentally removed blank line needed to make debugging output look better
b33c210Reduce some unnecessary text duplication in the Makefile.

@embray
Copy link
Contributor Author

embray commented Jul 18, 2018

comment:4

I believe this issue can reasonably be addressed for Sage 8.4.

@embray embray modified the milestones: sage-8.3, sage-8.4 Jul 18, 2018
@dimpase
Copy link
Member

dimpase commented Oct 15, 2018

comment:5

Toolchain packages are unconditionally built - it's also not addressed on #24919.
Anyhow this ought to be dealt with a follow-up to #24919.

@dimpase
Copy link
Member

dimpase commented Oct 15, 2018

Reviewer: Dima Pasechnik

@kiwifb
Copy link
Member

kiwifb commented Oct 28, 2018

comment:7

Needs rebasing.

@dimpase
Copy link
Member

dimpase commented Oct 28, 2018

Changed commit from b33c210 to 7647e1f

@dimpase
Copy link
Member

dimpase commented Oct 28, 2018

comment:8

(automatically) rebased.

@dimpase
Copy link
Member

dimpase commented Oct 28, 2018

Changed branch from u/embray/build/ticket-25188-2 to public/build/ticket-25188-2

@kiwifb
Copy link
Member

kiwifb commented Oct 28, 2018

comment:9

I was lead here because Volker selected this ticket for inclusion in his merging branch and when I looked on github to see the content of the commit, it came empty. vbraun@0c158ca

Then clicking here resulted in a failure to merge. So Volker will need to re-merge this one now.

Ping @vbraun the content of this ticket is currently missing in the merge.

@embray
Copy link
Contributor Author

embray commented Oct 29, 2018

Changed commit from 7647e1f to none

@embray
Copy link
Contributor Author

embray commented Oct 29, 2018

Changed branch from public/build/ticket-25188-2 to none

@embray
Copy link
Contributor Author

embray commented Oct 29, 2018

comment:10

(Edited, because I was mistaken slightly about the relation of this ticket to #24919.)

This doesn't seem right. This branch just looks identical to the one on #25857 now.
However, this branch should not actually be merged anyways.

These changes are already included implicitly in #24919 which is positively reviewed and should be merged.

The reason this is confusing is that I worked on this ticket at the same time as #24919, but #24919 redid a lot of configure stuff so it included those changes by necessity.

If we just merge #24919, this ticket can be closed as fixed.

@embray
Copy link
Contributor Author

embray commented Oct 29, 2018

comment:11

I take it back. Apparently I'm wrong. #24919 does not incorporate all elements of this ticket. Apparently at some point I decoupled them somewhat.

I'll create a new branch...

@embray
Copy link
Contributor Author

embray commented Oct 29, 2018

comment:12

Okay, apparently I ended up rolling this back into #25188, hence the empty merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants