lcalc doesn't build on cygwin due to missing time.h include

[William Stein]

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

[William Stein]

Spkg here: ​http://sage.math.washington.edu/home/wstein/patches/lcalc-20100428-1.23.p4.spkg

[Mike Hansen]

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

[Leif Leonhardy]

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.

[Leif Leonhardy]

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)

[Leif Leonhardy]

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.

[Leif Leonhardy]

Diff between the p2 (#9592) and Mike's p4 spkg - for review/reference.

[Leif Leonhardy]

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.)

[Leif Leonhardy]

Also, the dist/ (Debian) directory should be removed, cf. #5903.

[Mitesh Patel]

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).

[Jeroen Demeyer]

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.

[Jeroen Demeyer]

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

[Jeroen Demeyer]

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

[Jeroen Demeyer]

ignore this file

[Jeroen Demeyer]

Diff between p4 and p5 - for review/reference.

[Leif Leonhardy]

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.

[Leif Leonhardy]

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.

[Leif Leonhardy]

Replying to leif:

The new p5 spkg successfully installed on Sage 4.6.alpha1 (with MAKE="make -j32") and passed ptestlong on Ubuntu 10.04 x86_64, [...]

Same on Ubuntu 9.04 x86 (with MAKE="make -j8").