Opened 11 years ago
Closed 11 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: |
Description (last modified by )
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)
Change History (53)
comment:1 Changed 11 years ago by
- Cc mhansen added
comment:2 Changed 11 years ago by
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 11 years ago by
- 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 11 years ago by
- Description modified (diff)
comment:5 Changed 11 years ago by
comment:6 Changed 11 years ago by
Okay, installing ncurses didn't help anyway. Trying with libncurses-devel, given the tickets mentioned above...
comment:7 Changed 11 years ago by
- 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: ↓ 9 Changed 11 years ago by
- 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 11 years ago by
- 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: ↓ 11 Changed 11 years ago by
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: ↓ 12 Changed 11 years ago by
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 11 years ago by
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: ↓ 15 Changed 11 years ago by
- 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 11 years ago by
I tried using #!diff
to format the diff, but it didn't work...
comment:15 in reply to: ↑ 13 Changed 11 years ago by
- 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: ↓ 17 Changed 11 years ago by
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 11 years ago by
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: ↓ 19 Changed 11 years ago by
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 11 years ago by
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: ↓ 21 Changed 11 years ago by
- 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: ↓ 22 Changed 11 years ago by
- 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: ↓ 23 Changed 11 years ago by
- 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: ↓ 24 Changed 11 years ago by
- 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 11 years ago by
- 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 11 years ago by
- Merged in set to sage-4.7.2.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
comment:26 Changed 11 years ago by
- 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 11 years ago by
- 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 11 years ago by
- Status changed from new to needs_review
comment:29 Changed 11 years ago by
- Status changed from needs_review to needs_work
comment:30 follow-up: ↓ 32 Changed 11 years ago by
- Status changed from needs_work to needs_info
I could also accommodate that in a p11, fixing #11645.
comment:31 Changed 11 years ago by
comment:32 in reply to: ↑ 30 ; follow-ups: ↓ 33 ↓ 35 Changed 11 years ago by
comment:33 in reply to: ↑ 32 ; follow-up: ↓ 34 Changed 11 years ago by
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: ↓ 36 Changed 11 years ago by
- 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: ↓ 37 Changed 11 years ago by
comment:36 in reply to: ↑ 34 Changed 11 years ago by
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 11 years ago by
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: ↓ 40 Changed 11 years ago by
- 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 11 years ago by
The file patches/Minor.h
inside the spkg has permission 0640, could you change this to 0644?
comment:40 in reply to: ↑ 38 Changed 11 years ago by
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.
comment:41 Changed 11 years ago by
- 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: ↓ 45 Changed 11 years ago by
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 11 years ago by
P.S.: Jeroen will know why (or how) this happened... ;-)
comment:44 Changed 11 years ago by
Ooops, and otherwise positive review from me; Jeroen should decide on that.
comment:45 in reply to: ↑ 42 ; follow-up: ↓ 46 Changed 11 years ago by
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: ↓ 47 Changed 11 years ago by
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: ↓ 48 Changed 11 years ago by
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 usehg 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: ↓ 49 Changed 11 years ago by
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: ↓ 50 Changed 11 years ago by
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, buthg 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.p12to 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 11 years ago by
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 11 years ago by
- 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 11 years ago by
- Merged in set to sage-4.7.2.alpha0
- Resolution set to fixed
- Status changed from positive_review to closed
Dima, do you know if this fix is already in upstream? Just curious.