Opened 10 years ago

Closed 10 years ago

#11550 closed defect (fixed)

Singular does not build on Cygwin on Windows 7

Reported by: kcrisman Owned by: kcrisman
Priority: major Milestone: sage-4.7.2
Component: porting: Cygwin Keywords: cygwin singular
Cc: dimpase, mhansen, leif Merged in: sage-4.7.2.alpha0
Authors: Mike Hansen, Karl-Dieter Crisman Reviewers: Karl-Dieter Crisman, Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by kcrisman)

Apparently the fix is to

Add "#include <time.h>" to src/Singular/Minor.h the in the Singular spkg 

I think this fix is due to Mike Hansen, cf. http://trac.sagemath.org/sage_trac/wiki/CygwinPort?version=56


New spkg at http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p13.spkg.

Attachments (1)

trac_11550.diff (37.3 KB) - added by kcrisman 10 years ago.
For review only

Download all attachments as: .zip

Change History (53)

comment:1 Changed 10 years ago by kcrisman

  • Cc mhansen added

Dima, do you know if this fix is already in upstream? Just curious.

comment:2 Changed 10 years ago by dimpase

Hmm, do you said I didn't get around to open a ticket on this? Well, maybe. I vaguely recall mentioning this somewhere...

I have no idea whether the upstream has moved on this.

Dima

comment:3 Changed 10 years ago by kcrisman

  • Owner changed from tbd to kcrisman

Now I get beyond this, but it fails at a different spot. Full log at my Sage directory.

In file included from ../kernel/ring.h:13,
                 from ../kernel/ideals.h:11,
                 from ipshell.h:12,
                 from tesths.cc:20:
../kernel/polys-impl.h:177:1: warning: "pPolyAssumeReturn" redefined
../kernel/polys-impl.h:176:1: warning: this is the location of the previous definition
mpsr_Tok.cc:1: warning: -fPIC ignored for target (all code is position independent)
mpsr_Tok.cc: In function ‘void mpsr_ttGen()’:
mpsr_Tok.cc:551: warning: deprecated conversion from string constant to ‘char*’
/usr/lib/gcc/i686-pc-cygwin/4.3.4/../../../../i686-pc-cygwin/bin/ld: cannot find -lncurses
collect2: ld returned 1 exit status
make[4]: *** [iparith.inc] Error 1

It's true that this Cygwin doesn't (yet) have ncurses, though it has libncurses. But I don't remember this ever being a prerequisite for Sage, and am hesitant to insert it without knowing whether this is actually an upstream bug. My Macs build Singular fine and don't even have this (one has ncurses-config). They do both have libncurses, but then again so does this Cygwin!

comment:4 Changed 10 years ago by dimpase

  • Description modified (diff)

comment:5 Changed 10 years ago by kcrisman

Now I see that at #6743 and #7005 apparently Singular did at one point need libncurses-devel. I don't know whether my XP has that or not.

Another possibly related ticket is #10235.

comment:6 Changed 10 years ago by kcrisman

Okay, installing ncurses didn't help anyway. Trying with libncurses-devel, given the tickets mentioned above...

comment:7 Changed 10 years ago by kcrisman

  • Authors set to Mike Hansen, Karl-Dieter Crisman
  • Reviewers set to Karl-Dieter Crisman

libncurses-devel takes care of it. I'll try to make a new spkg after lunch.

comment:8 follow-up: Changed 10 years ago by kcrisman

  • Description modified (diff)
  • Status changed from new to needs_review

New spkg at http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p10.spkg. Needs review.

We will in due course require linbcurses-devel, but this is addressed at #6743 and the wiki for Cygwin.

comment:9 in reply to: ↑ 8 Changed 10 years ago by dimpase

  • Status changed from needs_review to needs_info

Replying to kcrisman:

New spkg at http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p10.spkg. Needs review.

We will in due course require linbcurses-devel, but this is addressed at #6743 and the wiki for Cygwin.

Are you sure that unconditional

#include <time.h>

would work everywhere?

I would rather do

#ifdef CYGWIN
#include <time.h>
#endif

}}}

comment:10 follow-up: Changed 10 years ago by kcrisman

I am open to any of them.

By the way, the wiki page says gcc on CYGWIN defines a variable __CYGWIN__. So you can use it to #if(n)def stuff. So would I use #ifdef CYGWIN or #ifdef __CYGWIN__? I realize this is a total newbie question. But remember I didn't know anything about headers a few days ago :)

Anyway, the libncurses-devel is in some ways the more important piece of this.

comment:11 in reply to: ↑ 10 ; follow-up: Changed 10 years ago by dimpase

Replying to kcrisman:

I am open to any of them.

By the way, the wiki page says gcc on CYGWIN defines a variable __CYGWIN__. So you can use it to #if(n)def stuff. So would I use #ifdef CYGWIN or #ifdef __CYGWIN__? I realize this is a total newbie question. But remember I didn't know anything about headers a few days ago :)

Anyway, the libncurses-devel is in some ways the more important piece of this.

oops, yes, of course, CYGWIN, not CYGWIN.

comment:12 in reply to: ↑ 11 Changed 10 years ago by dimpase

Replying to dimpase:

Replying to kcrisman:

I am open to any of them.

By the way, the wiki page says gcc on CYGWIN defines a variable __CYGWIN__. So you can use it to #if(n)def stuff. So would I use #ifdef CYGWIN or #ifdef __CYGWIN__? I realize this is a total newbie question. But remember I didn't know anything about headers a few days ago :)

Anyway, the libncurses-devel is in some ways the more important piece of this.

should have done a preview....

oops, yes, of course, __CYGWIN__, not CYGWIN.

comment:13 follow-up: Changed 10 years ago by kcrisman

  • Status changed from needs_info to needs_review

I just realized this whole discussion about variables is a red herring. In the spkg-install, we have

    if [ "$UNAME" = "CYGWIN" ]; then
        # Fine to make this patch on any system, because it is code that is onl$
        cp patches/sing_win.cc src/Singular/
        cp patches/IntegerProgramming-Makefile.in src/IntegerProgramming/Makefi$
        cp patches/Minor.h src/Singular/
    fi

which includes our patch.

Compare the relevant part of the diff:

diff -r 0825382212aa spkg-install
a        b       
114     114             # Fine to make this patch on any system, because it is code that is only compiled on Windows. 
115     115             cp patches/sing_win.cc src/Singular/ 
116     116             cp patches/IntegerProgramming-Makefile.in src/IntegerProgramming/Makefile.in 
        117             cp patches/Minor.h src/Singular/ 
117     118         fi 
118     119      
119     120         # parallel build patches 

so I see why it was confusing! But even with the comment (legacy comment, not mine), this is still all just included on Cygwin.

comment:14 Changed 10 years ago by kcrisman

I tried using #!diff to format the diff, but it didn't work...

comment:15 in reply to: ↑ 13 Changed 10 years ago by dimpase

  • Status changed from needs_review to positive_review

Replying to kcrisman:

so I see why it was confusing! But even with the comment (legacy comment, not mine), this is still all just included on Cygwin.

OK, then it's OK. Positive review.

comment:16 follow-up: Changed 10 years ago by vbraun

I have the newest Singular version packaged at http://www.stp.dias.ie/~vbraun/Sage/spkg/singular-3-1-3-2.spkg in case anybody wants to see if the newer version no longer requires the patch. Attention: it does not compile with SAGE_DEBUG yet. The newer Minor.h still does not #include <time.h>, but then maybe it does not have to. This ticket doesn't say what it is required for. I don't have a windows machine to test this myself.

comment:17 in reply to: ↑ 16 Changed 10 years ago by kcrisman

Replying to vbraun:

I have the newest Singular version packaged at http://www.stp.dias.ie/~vbraun/Sage/spkg/singular-3-1-3-2.spkg in case anybody wants to see if the newer version no longer requires the patch. Attention: it does not compile with SAGE_DEBUG yet.

Hmm, just getting all these spkgs working properly has been quite a job, and I only have about another week before I will have to give back these machines.

The newer Minor.h still does not #include <time.h>, but then maybe it does not have to. This ticket doesn't say what it is required for. I don't have a windows machine to test this myself.

It's possible. The reason Minor.h required this is because the command time is used in Minor.cc

void MinorValue::SetRankingStrategy (const int rankingStrategy)
{
  g_rankingStrategy = rankingStrategy;
  if (g_rankingStrategy == 6)
  {
    /* initialize the random generator with system time */
    srand ( time(NULL) );
  }
}

or at least that seems to be what the error messages were indicating. I just got that code five seconds ago from the Singular SVN website so I assume it is pretty up-to-date.

comment:18 follow-up: Changed 10 years ago by vbraun

Yes its still there. I'll make sure to keep the patch around in the new singular spkg, then.

comment:19 in reply to: ↑ 18 Changed 10 years ago by kcrisman

Replying to vbraun:

Yes its still there. I'll make sure to keep the patch around in the new singular spkg, then.

Thanks.

comment:20 follow-up: Changed 10 years ago by jdemeyer

  • Status changed from positive_review to needs_work

I guess this should be reported upstream to Singular.

comment:21 in reply to: ↑ 20 ; follow-up: Changed 10 years ago by dimpase

  • Status changed from needs_work to needs_review

Replying to jdemeyer:

I guess this should be reported upstream to Singular.

by reading http://www.singular.uni-kl.de/WINDOWS/index_all.html one gets an impression that Singular 3.1.1 is running under Cygwin just fine, as it is a Cygwin package. So I don't see much need to report anything.

comment:22 in reply to: ↑ 21 ; follow-up: Changed 10 years ago by jdemeyer

  • Status changed from needs_review to needs_info

Replying to dimpase:

Replying to jdemeyer:

I guess this should be reported upstream to Singular.

by reading http://www.singular.uni-kl.de/WINDOWS/index_all.html one gets an impression that Singular 3.1.1 is running under Cygwin just fine, as it is a Cygwin package.

Then explain why the Sage-Singular does need a patch.

comment:23 in reply to: ↑ 22 ; follow-up: Changed 10 years ago by dimpase

  • Status changed from needs_info to needs_review

Replying to jdemeyer:

Then explain why the Sage-Singular does need a patch.

We have a witness, Your Honor... :-)

I don't know how to access the Cygwin sources 3.1.1 for the "native Cygwin" Singular; indeed, http://cygwin.com/packages/singular-base/ lists only 3.1.2 and 3.1.3. I think the point of talking to Singular people about 3.1.1 is moot.

When Sage is upgraded to the next Singular release, it's certainly makes sense to investigate, and perhaps even use the "native" Singular rather than the one supplied by Sage.

comment:24 in reply to: ↑ 23 Changed 10 years ago by kcrisman

  • Status changed from needs_review to positive_review

Then explain why the Sage-Singular does need a patch.

We have a witness, Your Honor... :-)

Several, in fact. And at any rate time is used in the relevant place.

When Sage is upgraded to the next Singular release, it's certainly makes sense to investigate, and perhaps even use the "native" Singular rather than the one supplied by Sage.

Yes, this probably belongs wherever Volker is attaching his spkg in earlier comments. Perhaps this isn't an issue with newer Singular, but no need to try to upgrade Singular just to fix this.

comment:25 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:26 Changed 10 years ago by leif

  • Cc leif added

(I only need to know when the ticket's status should change, as I'm perhaps going to provide a p11 addressing #11645. Please notify me directly in case the trac notifications should still be broken then.)

comment:27 Changed 10 years ago by jdemeyer

  • Merged in sage-4.7.2.alpha0 deleted
  • Resolution fixed deleted
  • Status changed from closed to new

The spkg contains files which are not world-readable. In order to avoid another #11660, this should be fixed by a simple

chmod -R "og+rX" .

comment:28 Changed 10 years ago by jdemeyer

  • Status changed from new to needs_review

comment:29 Changed 10 years ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:30 follow-up: Changed 10 years ago by leif

  • Status changed from needs_work to needs_info

I could also accommodate that in a p11, fixing #11645.

comment:31 Changed 10 years ago by leif

... and also install the missing help.cnf, fixing #11519 (and at least partially #5994).

comment:32 in reply to: ↑ 30 ; follow-ups: Changed 10 years ago by kcrisman

Replying to leif:

I could also accommodate that in a p11, fixing #11645.

That would be helpful, if you were going to do that anyway.

jdemeyer: are those files an upstream issue, or did I do something wrong in creating the spkg?

comment:33 in reply to: ↑ 32 ; follow-up: Changed 10 years ago by leif

Replying to kcrisman:

Replying to leif:

I could also accommodate that in a p11, fixing #11645.

That would be helpful, if you were going to do that anyway.

Note that Jeroen has meanwhile created a new, different p10 based on the p9, since #11663 was marked a blocker for 4.7.1 (and your p10 had previously been merged into some 4.7.2 alpha).

So you should create a new p11 for this ticket, based on Jeroen's p10.

comment:34 in reply to: ↑ 33 ; follow-up: Changed 10 years ago by kcrisman

  • Description modified (diff)
  • Status changed from needs_info to needs_review

Note that Jeroen has meanwhile created a new, different p10 based on the p9, since #11663 was marked a blocker for 4.7.1 (and your p10 had previously been merged into some 4.7.2 alpha).

So you should create a new p11 for this ticket, based on Jeroen's p10.

Okay, done. See http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p11.spkg for this.

This spkg also resolve #10235, should be an easy review there.

Is there anything else that needs to be done to review this one? The changes are exactly the same, except I added a comment about #10235.

comment:35 in reply to: ↑ 32 ; follow-up: Changed 10 years ago by jdemeyer

Replying to kcrisman:

jdemeyer: are those files an upstream issue, or did I do something wrong in creating the spkg?

It is upstream, see #11663.

comment:36 in reply to: ↑ 34 Changed 10 years ago by leif

Replying to kcrisman:

Note that Jeroen has meanwhile created a new, different p10 based on the p9, since #11663 was marked a blocker for 4.7.1 (and your p10 had previously been merged into some 4.7.2 alpha).

So you should create a new p11 for this ticket, based on Jeroen's p10.

Okay, done. See http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p11.spkg for this.

Unfortunately Jeroen's p10 needs work again, as it doesn't fix the issue as is.

This spkg also resolve #10235, should be an easy review there.

See comment there.

Is there anything else that needs to be done to review this one? The changes are exactly the same, except I added a comment about #10235.

Well, just make sure it doesn't revert anything else, e.g. changes made in the p10, but see above.

But as soon as Jeroen's spkg has positive review, you should be able to just apply your patch on top of Jeroen's p10 to create your p11.

comment:37 in reply to: ↑ 35 Changed 10 years ago by leif

Replying to jdemeyer:

Replying to kcrisman:

jdemeyer: are those files an upstream issue, or did I do something wrong in creating the spkg?

It is upstream, see #11663.

Not only, or not the real problem, since we "manually" copy two files without -p and that can mess up their permissions (cf. #11633).

comment:38 follow-up: Changed 10 years ago by kcrisman

  • Description modified (diff)

Okay, should be now a package at http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p13.spkg. Let's hope this is the last one, I'm getting carpal tunnel! :)

It's the same as it was, except now based on #11563 and #11645, in principle.

comment:39 Changed 10 years ago by jdemeyer

The file patches/Minor.h inside the spkg has permission 0640, could you change this to 0644?

comment:40 in reply to: ↑ 38 Changed 10 years ago by kcrisman

Replying to kcrisman:

Okay, should be now a package at http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p13.spkg. Let's hope this is the last one, I'm getting carpal tunnel! :)

It's the same as it was, except now based on #11563 and #11645, in principle.

I meant #11663 and #11645, of course.

The file patches/Minor.h inside the spkg has permission 0640, could you change this to 0644?

I think so. I'll post here when once I get a chance to do that.

Changed 10 years ago by kcrisman

For review only

comment:41 Changed 10 years ago by kcrisman

  • Reviewers changed from Karl-Dieter Crisman to Karl-Dieter Crisman, Jeroen Demeyer

Okay, the spkg is now based on the spkgs in rc2 and is ready for review that it has no weird files or weird permissions. I did run ls -a and ls -l on the various directories, so I sure hope I got everything!

Given that the fix itself got positive review a while ago, hopefully this is enough? I added Jeroen because of catching the permissions issue.

Still at http://sage.math.washington.edu/home/kcrisman/singular-3-1-1-4.p13.spkg.

comment:42 follow-up: Changed 10 years ago by leif

You won't like to hear that, but you've "deleted" the Mercurial tag for singular-3-1-1-4.p12...

But that's IMHO a minor issue.

comment:43 Changed 10 years ago by leif

P.S.: Jeroen will know why (or how) this happened... ;-)

comment:44 Changed 10 years ago by leif

Ooops, and otherwise positive review from me; Jeroen should decide on that.

comment:45 in reply to: ↑ 42 ; follow-up: Changed 10 years ago by kcrisman

Replying to leif:

You won't like to hear that, but you've "deleted" the Mercurial tag for singular-3-1-1-4.p12...

I saw that in the log. But this was already the case when I downloaded the latest p12 from #11645. I don't know exactly how to make those tags, and didn't want to screw anything up, so I just left it alone.

But that's IMHO a minor issue.

Considering that there is no p10 either, indeed!

comment:46 in reply to: ↑ 45 ; follow-up: Changed 10 years ago by leif

Replying to kcrisman:

I don't know exactly how to make those tags

.hgtags is a plain text file, i.e. you can simply edit it.

You usually just do hg tag <name> when you've finished a new patch level (revision), otherwise you can use hg tag -r <revision number> <name> to tag an older changeset / revision afterwards. hg help tag shows further options.

comment:47 in reply to: ↑ 46 ; follow-up: Changed 10 years ago by kcrisman

Replying to leif:

Replying to kcrisman:

I don't know exactly how to make those tags

.hgtags is a plain text file, i.e. you can simply edit it.

You usually just do hg tag <name> when you've finished a new patch level (revision), otherwise you can use hg tag -r <revision number> <name> to tag an older changeset / revision afterwards. hg help tag shows further options.

Hmm, I thought it was more mysterious than that. So it's just an arbitrary way to mark stopping points, not really anything else. Interesting that these have to be committed.

comment:48 in reply to: ↑ 47 ; follow-up: Changed 10 years ago by leif

Replying to kcrisman:

So it's just an arbitrary way to mark stopping points, not really anything else. Interesting that these have to be committed.

Well, .hgtags is (or should be) under revision control, but hg tag ... automatically commits.

The tags just create aliases for revision numbers or changesets, i.e. you can then for example do

hg diff -r singular-3-1-1-4.p9 -r singular-3-1-1-4.p12

to get the (cumulative) changes between these two spkgs / patch levels.

comment:49 in reply to: ↑ 48 ; follow-up: Changed 10 years ago by kcrisman

Replying to leif:

Replying to kcrisman:

So it's just an arbitrary way to mark stopping points, not really anything else. Interesting that these have to be committed.

Well, .hgtags is (or should be) under revision control, but hg tag ... automatically commits.

The tags just create aliases for revision numbers or changesets, i.e. you can then for example do

hg diff -r singular-3-1-1-4.p9 -r singular-3-1-1-4.p12

to get the (cumulative) changes between these two spkgs / patch levels.

Thanks. Before, I didn't know enough HG to care about this, but now it seems useful.

But perhaps this is OT for this ticket :)

comment:50 in reply to: ↑ 49 Changed 10 years ago by leif

Replying to kcrisman:

But perhaps this is OT for this ticket :)

Open another ticket and put it into the Sage Developer's Guide. ;-)

comment:51 Changed 10 years ago by jdemeyer

  • Status changed from needs_review to positive_review

Looks good to me (I have not tested this on Cygwin, but that was done before).

comment:52 Changed 10 years ago by jdemeyer

  • Merged in set to sage-4.7.2.alpha0
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.