Opened 11 years ago
Closed 10 years ago
#9845 closed defect (fixed)
lcalc doesn't build on cygwin due to missing time.h include
Reported by: | was | Owned by: | GeorgSWeber |
---|---|---|---|
Priority: | major | Milestone: | sage-4.6 |
Component: | build | Keywords: | |
Cc: | rishi, leif, jdemeyer | Merged in: | sage-4.6.alpha2 |
Authors: | Jeroen Demeyer, Mike Hansen, William Stein | Reviewers: | Leif Leonhardy |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
One needs to add
#include <time.h>
to include/Lcommandline_numbertheory.h
to get lcalc to build on cygwin.
Final spkg: http://sage.math.washington.edu/home/jdemeyer/spkg/lcalc-20100428-1.23.p5.spkg
Attachments (3)
Change History (20)
comment:1 Changed 11 years ago by
- Status changed from new to needs_review
comment:2 Changed 11 years ago by
comment:3 Changed 11 years ago by
- Cc rishi leif jdemeyer added
I've posted a new spkg at http://sage.math.washington.edu/home/mhansen/lcalc-20100428-1.23.p4.spkg which contains some of the fixes pointed out at #9775
comment:4 Changed 11 years ago by
Thanks for fixing much of what I mentioned at #9775.
I'm not sure if we should drop the usual "sanity check" at the top of spkg-install
.
(Btw, you missed the "p2" in its top comment.)
The first comment regarding SAGE_DEBUG
and the message in the else
part aren't current.
("Unset SAGE_DEBUG ..." should read "Set SAGE_DEBUG to yes ...". Though I'd prefer adding -g
independently of that setting, since it doesn't influence performance, and most other spkgs do so.)
Replacing -lmpir
by -lgmp
is perhaps a minor issue; using $MAKE
instead of make
($(MAKE)
in Lcalc's Makefile itself, too) IMHO not. Unfortunately, the current spkg/install
also uses make
instead of $MAKE
unless SAGE_PARALLEL_SPKG_BUILD=yes
, which I consider a bug, since adding -j
is not the only case in which one would set MAKE
. One should never call make
inside a Makefile or a script that's executed through some "make", because it might be a different one.
comment:5 Changed 11 years ago by
P.S.:
all: # make print_vars make libLfunction.so make lcalc make examples # make find_L # make test
should simply (for our purposes) be
all: libLfunction.so lcalc examples
or even just
all: lcalc examples
but one has to add the proper dependency, i.e.:
examples: libLfunction.so $(CC) $(CCFLAGS) $(INCLUDEFILES) example_programs/example.cc libLfunction.so -o example_programs/example $(GMP_FLAGS)
comment:6 Changed 11 years ago by
Btw, the -lmpir
(or -lgmp
) is only needed if we build a static version of lcalc
, since only the also linked PARI library directly uses it.
comment:7 Changed 11 years ago by
I've attached a diff between the lcalc-20100428-1.23.p2 spkg from #9592 and Mike's / William's lcalc-20100428-1.23.p4 for easier reviewing.
Note that #9592 also still needs review! (We need this modified package for upgrading PARI / Sage 4.6.)
comment:8 Changed 11 years ago by
Also, the dist/
(Debian) directory should be removed, cf. #5903.
comment:9 Changed 11 years ago by
- Description modified (diff)
comment:10 follow-up: ↓ 11 Changed 10 years ago by
Mike, are there Windows virtual machines running Cygwin available to potential reviewers? (We could give access instructions off-trac.) It would be great to reduce the number of remaining build tickets at CygwinPort. And remote Cygwin access might help with #9914 (see comment 4ff).
comment:11 in reply to: ↑ 10 Changed 10 years ago by
Replying to mpatel:
Mike, are there Windows virtual machines running Cygwin available to potential reviewers? (We could give access instructions off-trac.)
I wouldn't mind testing this if needed.
comment:12 Changed 10 years ago by
- Description modified (diff)
I have made a trivial change to spkg-install (removed the comment "# since 'success' relies on an exit code, call set +e before running it."). I also removed the dist directory.
New spkg: http://sage.math.washington.edu/home/jdemeyer/spkg/lcalc-20100428-1.23.p4.spkg
comment:13 Changed 10 years ago by
- Description modified (diff)
- Reviewers set to Leif Leonhardy
I also made the other changes suggested by Leif. New spkg: http://sage.math.washington.edu/home/jdemeyer/spkg/lcalc-20100428-1.23.p5.spkg
comment:14 Changed 10 years ago by
Looks as if we no longer need patches/Lcommandline_elliptic.cc.cygwin
(and patches/Lcommandline_elliptic.cc.cygwin.diff
), since this patch isn't applied.
Mike (or Jeroen, if you can test this), is this Cygwin patch / addition obsolete:
// SAGE -- used below -- needed for Cygwin. #ifndef llrint inline long long int llrint (double x) { long long int llrintres; asm ("fistpll %0" : "=m" (llrintres) : "t" (x) : "st"); return llrintres; } #endif
(It's not included in the generic patch to Lcommandline_elliptic.cc
, nor upstream.)
Apart from old typos, an obsolete comment regarding SAGE_DEBUG
, and the recent changelog headings lacking the upstream (snapshot?) date (20100428), I'm quite happy with the new spkg (i.e., the Sage part; the patched Makefile is still suboptimal, but never mind). :)
If this spkg really contains an upstream snapshot, it is unclear from SPKG.txt when this was taken / actually put into the spkg.
comment:15 follow-up: ↓ 16 Changed 10 years ago by
- Status changed from needs_review to positive_review
The new p5 spkg successfully installed on Sage 4.6.alpha1 (with MAKE="make -j32"
) and passed ptestlong
on Ubuntu 10.04 x86_64, so I'm setting this to "positive review".
Feel free to revert this in case any errors occur on other systems.
comment:16 in reply to: ↑ 15 Changed 10 years ago by
Replying to leif:
The new p5 spkg successfully installed on Sage 4.6.alpha1 (with
MAKE="make -j32"
) and passedptestlong
on Ubuntu 10.04 x86_64, [...]
Same on Ubuntu 9.04 x86 (with MAKE="make -j8"
).
comment:17 Changed 10 years ago by
- Merged in set to sage-4.6.alpha2
- Resolution set to fixed
- Status changed from positive_review to closed
Spkg here: http://sage.math.washington.edu/home/wstein/patches/lcalc-20100428-1.23.p4.spkg