Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Even more minor Cygwin fixes for FLINT #11727

Closed
kcrisman opened this issue Aug 24, 2011 · 23 comments
Closed

Even more minor Cygwin fixes for FLINT #11727

kcrisman opened this issue Aug 24, 2011 · 23 comments

Comments

@kcrisman
Copy link
Member

I'm not quite sure how this happened, but apparently in newer Cygwins patch won't accept certain things.

FLINT now doesn't actually work on one of my Cygwins, even after #11547, because of an extra line that crept into one of the patches.

--- src/mpn_extras.h        2009-09-23 18:03:27.000000000 +0800
+++ patches/mpn_extras.h        2011-04-25 22:36:33.000000000 +0800
@@ -22,7 +22,6 @@
 #ifndef MPN_EXTRAS_H
 #define MPN_EXTRAS_H
 
-#include "flint.h"
 #include "ZmodF_poly.h"
 
 #include "longlong_wrapper.h"

Note the newline at the end; removing this solves the problem. I guess this didn't cause trouble on other systems, since these patches are applied universally. I can't even figure out where this change happened.

See also #9858 and #10328 for other updates to FLINT needed.


Spkg at http://sage.math.washington.edu/home/kcrisman/flint-1.5.0.p9.spkg.

CC: @mwhansen @dimpase @nexttime @jdemeyer

Component: porting: Cygwin

Author: Karl-Dieter Crisman, Dmitrii Pasechnik

Reviewer: Dmitrii Pasechnik, Karl-Dieter Crisman

Merged: sage-4.7.2.alpha3

Issue created by migration from https://trac.sagemath.org/ticket/11727

@dimpase
Copy link
Member

dimpase commented Aug 24, 2011

comment:1

I suppose here hg did not see the file being opened in Unix mode, and saved in DOS mode. If I open it in vim I see

--- src/mpn_extras.h    2009-09-23 18:03:27.000000000 +0800
+++ patches/mpn_extras.h        2011-04-25 22:36:33.000000000 +0800
@@ -22,7 +22,6 @@
 #ifndef MPN_EXTRAS_H^M
 #define MPN_EXTRAS_H^M
 ^M
-#include "flint.h"^M
 #include "ZmodF_poly.h"^M
 ^M
 #include "longlong_wrapper.h"^M

these ^M are DOS LF/CR symbols. You can go over the spkg leading to this one, to track where exactly this got in.
I can only say that in p6 spkg I created back in April it was still OK.

What you need to do is to save this file in Unix mode and re-package...

@kini
Copy link
Contributor

kini commented Aug 24, 2011

comment:2

The CR characters were introduced to this file in p7, as you can see by doing hg log -p patches/mpn_extras.h.patch in the spkg directory.

@dimpase
Copy link
Member

dimpase commented Aug 24, 2011

comment:3

Replying to @kini:

The CR characters were introduced to this file in p7, as you can see by doing hg log -p patches/mpn_extras.h.patch in the spkg directory.

in fact hg log -p mpn_extras.h.patch shows this change, done by Jeroen, so this came in p7. I cc to him. He didn't notice, apparently. The whole src/ directory is in DOS mode there...

@kcrisman
Copy link
Member Author

comment:4

Okay, see http://sage.math.washington.edu/home/kcrisman/flint-1.5.0.p9.spkg for a fix. Can someone very quickly just check that this has fixed it? (I can't see this in pico/nano.)

I think that if it works for me, and you review it, then that would be sufficient for positive review.

@kcrisman
Copy link
Member Author

comment:5

This fixes the problem. I don't see any crud in the spkg, either.

cc:ing Jeroen as apparently he did it as well. I'd appreciate a quick review for folks, if possible :)

@kcrisman
Copy link
Member Author

Author: Karl-Dieter Crisman

@kcrisman

This comment has been minimized.

@dimpase
Copy link
Member

dimpase commented Aug 25, 2011

comment:7

Replying to @kcrisman:

Okay, see http://sage.math.washington.edu/home/kcrisman/flint-1.5.0.p9.spkg for a fix. Can someone very quickly just check that this has fixed it? (I can't see this in pico/nano.)

I think that if it works for me, and you review it, then that would be sufficient for positive review.

patches/mpn_extras.h.patch
is in DOS mode. As well, some files in src/are in DOS mode, too.
Please convert all the files in the spkg into Unix mode.
This is in fact just on-liner (assuming dos2unix installed):

flint-1.5.0.p9$ find . -type f -exec dos2unix '{}' \;

New package, with this done, at http://boxen.math.washington.edu/home/dima/packages/flint-1.5.0.p9.spkg
I checked in on Linux and MacOSX only - please check on Cygwin.

@kcrisman
Copy link
Member Author

comment:8

Thanks. I have no idea what is going on with this - I only use Mac for developing (don't create patches/spkgs on Cygwin) so I am very unfamiliar with all this. My editors don't show these things.

(I'll try to check this on Cygwin soon - having some problems getting new builds started post 4.7.1, something might have changed in prereq or spkg/standard/deps.)

@kcrisman
Copy link
Member Author

Reviewer: Dima Pasechnik

@kcrisman
Copy link
Member Author

Changed author from Karl-Dieter Crisman to Karl-Dieter Crisman, Dima Pasechnik

@kcrisman
Copy link
Member Author

comment:9

Okay, your version did not change SPKG.txt (maybe mine didn't either?) so I did that, and then even added a tag for the p9 (thanks, Leif!) Still at http://sage.math.washington.edu/home/kcrisman/flint-1.5.0.p9.spkg.

@dimpase
Copy link
Member

dimpase commented Aug 25, 2011

comment:10

Replying to @kcrisman:

Thanks. I have no idea what is going on with this - I only use Mac for developing (don't create patches/spkgs on Cygwin) so I am very unfamiliar with all this. My editors don't show these things.

well, on every Mac there is vim, which you can run in terminal window - and it will show you the encoding...

@kcrisman
Copy link
Member Author

comment:11

This still fixes the patch problem on Cygwin, builds ok. Positive review. Thanks, Dima, for catching the precise cause of this.

@kcrisman
Copy link
Member Author

Changed reviewer from Dima Pasechnik to Dima Pasechnik, Karl-Dieter Crisman

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 28, 2011

comment:12

How's the storm? Did all machines survive?


If you can tell me what DOS file endings are... ;) (DOS actually used ^Z to indicate the end of a text file.)

Hopefully this will be the last spkg in the FLINT 1.5.0 series, so I can finally rebase my 1.5.2 one on it.

@dimpase
Copy link
Member

dimpase commented Aug 28, 2011

comment:13

Replying to @nexttime:

How's the storm? Did all machines survive?


If you can tell me what DOS file endings are... ;) (DOS actually used ^Z to indicate the end of a text file.)

yeah, "line endings" meant, indeed.

@kcrisman
Copy link
Member Author

comment:14

How's the storm? Did all machines survive?

Turned out to be more of a non-event here, fortunately. Elsewhere on the US East Coast, not so much.

Hopefully this will be the last spkg in the FLINT 1.5.0 series, so I can finally rebase my 1.5.2 one on it.

Yes, let's hope!

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 8, 2011

comment:15

Please don't put tabs into the ticket's description, as trac (at least our current version) doesn't properly handle these (i.e., doesn't escape them when exporting).

@nexttime

This comment has been minimized.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 12, 2011

Merged: sage-4.7.2.alpha3

@nexttime nexttime mannequin removed the s: positive review label Sep 12, 2011
@nexttime nexttime mannequin closed this as completed Sep 12, 2011
@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 23, 2011

Changed reviewer from Dima Pasechnik, Karl-Dieter Crisman to Dmitrii Pasechnik, Karl-Dieter Crisman

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 23, 2011

Changed author from Karl-Dieter Crisman, Dima Pasechnik to Karl-Dieter Crisman, Dmitrii Pasechnik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants