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

sage-env exports empty CXXFLAGS #16149

Closed
nexttime mannequin opened this issue Apr 12, 2014 · 12 comments
Closed

sage-env exports empty CXXFLAGS #16149

nexttime mannequin opened this issue Apr 12, 2014 · 12 comments

Comments

@nexttime
Copy link
Mannequin

nexttime mannequin commented Apr 12, 2014

That happens if CFLAGS are empty (or not exported at all, which presumably is the most frequent case) and CXXFLAGS aren't either:

if [ "$CXXFLAGS" = "" ]; then
    export CXXFLAGS="$CFLAGS"
fi

Unintentionally having exported but empty CXXFLAGS breaks at least MPIR, because it assumes the user had intentionally set them, so leaves them as they are (empty that is), not passing potentially important architecture and ABI flags to the C++ compiler (e.g. -m... flags to g++).

The solution is of course simple: Don't (set and) export CXXFLAGS if CFLAGS aren't exported either. Not sure if setting CXXFLAGS to CFLAGS in case the latter are non-empty (or empty but exported) is desirable at all.

CC: @jpflori

Component: scripts

Keywords: CFLAGS MPIR C++ ABI

Author: Frédéric Chapoton

Branch/Commit: 420fa5f

Reviewer: Volker Braun

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

@nexttime nexttime mannequin added this to the sage-6.2 milestone Apr 12, 2014
@nexttime nexttime mannequin added c: scripts labels Apr 12, 2014
@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 12, 2014

comment:1

I'd suggest to replace the above with at least

if [ "${CFLAGS+set}" = set ] && [ -z "$CXXFLAGS" ]; then
    export CXXFLAGS="$CFLAGS"
fi

which only sets (and exports) CXXFLAGS in case CFLAGS are set (but probably to "") and CXXFLAGS are empty.

Or only use CFLAGS if they're really non-empty by adding && [ -n "$CFLAGS" ] or replacing [ "${CFLAGS+set}" = set ] by the former.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 12, 2014

comment:2

It would probably make sense to even unset both in case they're just empty. (Same for FCFLAGS and CPPFLAGS etc.)

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin Author

nexttime mannequin commented Apr 12, 2014

comment:3

Typo.

@nexttime nexttime mannequin added the s: needs info label Apr 12, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@fchapoton
Copy link
Contributor

Branch: u/chapoton/16149

@fchapoton
Copy link
Contributor

New commits:

5c9bce6trac #16149 proposal

@fchapoton
Copy link
Contributor

Commit: 5c9bce6

@fchapoton
Copy link
Contributor

Author: Frédéric Chapoton

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2014

Branch pushed to git repo; I updated commit sha1. New commits:

420fa5ftrac #16149 typos

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 21, 2014

Changed commit from 5c9bce6 to 420fa5f

@vbraun
Copy link
Member

vbraun commented Dec 3, 2014

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Dec 3, 2014

Changed branch from u/chapoton/16149 to 420fa5f

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

2 participants