#11447: python spkg build fails on various Debian systems
---------------------------------------------------------+------------------
Reporter: pipedream | Owner:
jdemeyer
Type: defect | Status:
positive_review
Priority: blocker | Milestone:
sage-4.7.1
Component: packages | Keywords:
python spkg crypt library
Work_issues: | Upstream: N/A
Reviewer: David Kirkby, Bill Odefey, Jan Groenewald | Author: Jan
Medlock
Merged: | Dependencies:
---------------------------------------------------------+------------------
Comment(by leif):
Replying to [comment:41 jdemeyer]:
> Replying to [comment:40 leif]:
> > Replying to [comment:39 jdemeyer]:
> > Details, please. ;-)
>
{{{
if command -v dpkg && ! command -v dpkg-architecture >/dev/null; then
}}}
> should be
{{{
if command -v dpkg >/dev/null && ! command -v dpkg-architecture
>/dev/null; then
}}}
Yes, I mentioned on the ticket that I forgot it; minor.
> Mixed usage of `cp` of `patch`, better always use `patch`.
Agreed, but didn't want to change too much, since it is a blocker and
needs quick review.
> On SunOS, you first patch `setup.py` and then copy a different version
over it.
No idea. Ask Dave. (The real patch is only necessary on Debian Linuces, so
at least that shouldn't break anything.)
> Patching is better done using one `for` loop to prevent an error-prone
laundry list of patches to be applied.
Hmmm, depends on the situation. Order might matter, and not all patches
are applied on all systems. You'd also have to delete (or rename) patches
that [currently] don't get applied anymore. Having a bit of redundancy in
`spkg-install` and `SPKG.txt` also isn't that bad, as it should reduce the
risk of unintentional changes.
But we still have the `cp`s in there anyway, so this should be changed
when all patches get real patches.
[[BR]]
> Why call `./configure` with arguments `CC="$CC $CFLAGS" CXX="$CXX
$CXXFLAGS"`? Any why only on certain systems?
Guess that's some libtool issue, which sometimes drops e.g. `-m64`, but
apparently not on all systems (or it doesn't matter on all).
>
{{{
MAKE=make; export MAKE
make -i install
}}}
should be
{{{
$MAKE -j1 -i install
}}}
I know. I always get beaten when I change such, as some people consider it
to be a "dangerous change"... (I put a comment on that in.)
> Remove this:
{{{
# Do not exit script if there is an error, but instead print an
# informative error message. This is helps in determining why the
# configuration, compilation or installation failed. So put this before
the
# build() function.
set +e # This is redundant here, but doesn't hurt to keep it... ;-)
}}}
Orrr.
> Why this:
{{{
echo "Sleeping for three seconds before testing python"
sleep 3
}}}
See above ("dangerous change"). I added a comment that this is certainly
no longer necessary, on the other hand it doesn't hurt much.
> I guess `EXTRAFLAGS` is only used inside the `spkg-install` script, so
why export it?
Typical stupidity. Only one of N instances of that.
> On SunOS, do we need the configure flag `--with-gcc="gcc -m64"` given
that `-m64` is already inside `$OPT`? Or, if `$OPT` does nothing, why
specify `$OPT`?
That's again related to libtool I think.
----
If you come across such things and don't want to fix them immediately,
it's IMHO best to simply put comments into the files or `SPKG.txt`.
Comments don't hurt and don't have to be tested, so no reviewer should
complain about them (as opposed to too many or "difficult" functional
changes).
Also, the next one touching the spkg (or files in general) hopefully will
take them into account, and doesn't have to search trac for older tickets
with open issues.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/11447#comment:42>
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 post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sage-trac?hl=en.
