#17630: Further clean up of ATLAS (or equivalent BLAS) linking
-------------------------------------+-------------------------------------
Reporter: jpflori | Owner:
Type: defect | Status: needs_work
Priority: major | Milestone: sage-6.5
Component: packages: | Resolution:
standard | Merged in:
Keywords: | Reviewers:
Authors: Jean-Pierre Flori | Work issues:
Report Upstream: N/A | Commit:
Branch: u/fbissey/blas | eda2eb638d7c10f36053aa43474bb2fd21cd4db2
Dependencies: #18777, #18778 | Stopgaps:
-------------------------------------+-------------------------------------
Comment (by jpflori):
Replying to [comment:71 fbissey]:
> Replying to [comment:70 jdemeyer]:
> > Replying to [comment:69 fbissey]:
> > > I'll happily field comments on the new changes but I will otherwise
start moving libraries to the `.pxd` files.
> > I'd do that in a new ticket. There is already a lot of stuff on this
ticket and I wouldn't add anything more.
>
> That's a fair comment we touched things all over the place and some more
will need to be "aligned" eventually.
>
> >
> > I might not have much time these days to look at this ticket, so you
can just go ahead. I am still very suspicious about the change to
`LDFLAGS` in `sage-env`: it's a seemingly innocent change but with so much
potential to break stuff. For handling `CFLAGS`, `LDFLAGS`,... in Sage we
have always treated "unset" and "set to empty" equivalently. So such a
change shouldn't be made lightly.
> >
>
> I am a bit concerned that there is even a need for these tests and
setting/unsetting. It just feel wrong so I am probably not the best person
to pass a judgement on that particular change, my own reaction is to make
the whole thing disappear.
>
My only point is that now `LDFLAGS` just follows what `CFLAGS` does and
gave me a better behavior.
So I don't agree that we've always treated LDFLAGS and CFLAGS (and
CXXFLAGS) in the same way when they are unset and empty.
The problem with the current behavior of LDFLAGS, is that if `LDFLAGS` is
not set, then whe get `LDFLAGS=""`.
And some packages won't touch LDFLAGS if `LFDLAGS=""`.
If you feel worried about this, then just remove this change.
For the current behavior of CFLAGS, what you get is that if CFLAGS is
empty, then it's unset, that might not be the best behavior as well...
> > Two other small things I noticed now: there is some strange
indentation in `atlas/spkg-install`, be sure to use mutiples of 4 spaces
everywhere. And of course `get_libs_config()` needs a doctest.
>
> I will check the indent ASAP and I was afraid you would ask for that
doctest, but I know what to do.
>
> @Jean-Pierre, any comments on the last few commits? As can be surmised
by comments we'll eventually touch again some packages using
`blas/cblas/lapack` to enforce settings in the future. But as Jeroen says
there is enough here already.
Yeah I generally agree with going step by step even if the current
solution is not perfect.
I also spotted a few indentation issues in `atlas/spkg-install` and
`src/sage/env.py`
--
Ticket URL: <http://trac.sagemath.org/ticket/17630#comment:72>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/sage-trac.
For more options, visit https://groups.google.com/d/optout.
