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

Building complex_double.pyx fails on Cygwin #13806

Closed
jpflori opened this issue Dec 6, 2012 · 36 comments
Closed

Building complex_double.pyx fails on Cygwin #13806

jpflori opened this issue Dec 6, 2012 · 36 comments

Comments

@jpflori
Copy link
Contributor

jpflori commented Dec 6, 2012

At some point a dependency on the mc library was added, but this is not available by default on Cygwin (this surely lives in a very common and useful package, but I've reinstalled Cygwin recently and it has not appeared yet) and is not needed anyway.
So the dependency should be removed thus avoiding overlinking and build failure where this lib is not available.

Apply attachment: trac_13806.2.patch.

CC: @kcrisman @dimpase

Component: porting: Cygwin

Keywords: cygwin cephes spkg

Author: Jean-Pierre Flori

Reviewer: Karl-Dieter Crisman

Merged: sage-5.6.beta3

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

@jpflori jpflori added this to the sage-5.6 milestone Dec 6, 2012
@kcrisman
Copy link
Member

kcrisman commented Dec 6, 2012

comment:1

??? I didn't have any troubles with this.

sage: CDF(1)
1.0

@jpflori
Copy link
Contributor Author

jpflori commented Dec 6, 2012

comment:2

I already had this problem the first time I tried building on Cygwin (see CygwinPort page) but it magically disappeared the second time so I did not really care.

As I wrote in the description I think the lib got installed by some Cygwin package between my two attempts.

But anyway, everything buids fine without linking to that lib, so its definitely useless and dangerous to leave 'mc' in these file dependencies (there is the same problem with the complex_... file generated by gen_interpreters.py).

@kcrisman
Copy link
Member

comment:4

JP, am I correct in seeing that this patch only affects Cygwin-only code in any case? I find the different syntax for the two files irksome, but they both make sense. I can't test this right now but will try to do so tomorrow. How do I check whether I even have mc? I guess I must since this didn't cause trouble on my machine...

@kcrisman
Copy link
Member

comment:5

Okay, I checked this an it's so. Positive review on the patch part - it should not affect any other systems.

As to whether it works... I have no way of checking this on Windows 7. I don't even have this problem on XP, though I will try to check that it's not evil. How do I see whether my Cygwin has "mc" somehow?

@kcrisman
Copy link
Member

Reviewer: Karl-Dieter Crisman

@kcrisman
Copy link
Member

Author: Jean-Pierre Flori

@dimpase
Copy link
Member

dimpase commented Dec 14, 2012

comment:7

Replying to @kcrisman:

Okay, I checked this an it's so. Positive review on the patch part - it should not affect any other systems.

As to whether it works... I have no way of checking this on Windows 7. I don't even have this problem on XP, though I will try to check that it's not evil. How do I see whether my Cygwin has "mc" somehow?

huh, mc ? Midnight Commander? Something else?
Medical Condition? ;-)

@kcrisman
Copy link
Member

comment:8

huh, mc ? Midnight Commander? Something else?
Medical Condition? ;-)

Well, I have no idea! Though I suspect not these... maybe Multiplication, Complex. Did you read the patch? It's some library I've never heard of (which describes most libraries, but still...)

@jpflori
Copy link
Contributor Author

jpflori commented Dec 14, 2012

comment:9

I don't think the problem is 7 specific.
As I mentioned before i think the problem that libmc is installed by a quite common package that I did not install myself, as I use a fresh and minimal Cygwin setup.

And I'm not sure what this is...
Maybe that's some Cygwin specific version of libm for complex? and md would be for double?

@jpflori
Copy link
Contributor Author

jpflori commented Dec 14, 2012

comment:10

Maybe see #9543 as well.

@kcrisman
Copy link
Member

comment:11

Replying to @jpflori:

I don't think the problem is 7 specific.
As I mentioned before i think the problem that libmc is installed by a quite common package that I did not install myself, as I use a fresh and minimal Cygwin setup.

And I'm not sure what this is...
Maybe that's some Cygwin specific version of libm for complex? and md would be for double?

Maybe. Do the files in question actually test ok on your Cygwin without this libmc?

@kcrisman
Copy link
Member

comment:12

As to whether it works... I have no way of checking this on Windows 7. I don't even have this problem on XP, though I will try to check that it's not evil. How do I see whether my Cygwin has "mc" somehow?

When I do this, I don't find it, or md, for that matter:

$ls /cygdrive/c/cygwin/lib/libm<tab>
libm.a
<irrelevant stuff>

and other directories with lib also have either nothing or at any rate nothing related, including not libm.

Anyway, this file passes some tests for me, though it gives a different answer for one of the algdep things and gives annoying errors about being unable to start pari because gp --emacs --quiet --stacksize 1000000 failed. Is it possible that this is not possible to do in this (our Cygwin Sage) setup? But it looks like the standard way for our pexpect interface, and doing this command by hand ./sage -gp ... works fine.

@jpflori
Copy link
Contributor Author

jpflori commented Dec 14, 2012

comment:13

I think the libm* problems are from the time the Cygwin's libm was not C99 compliant or something like that and we needed cephes at http://www.netlib.org/cephes/
See #9543 and #8780.

I think we should try to remove all the Cygwin specific mc and md things.

@kcrisman
Copy link
Member

comment:14

I think the libm* problems are from the time the Cygwin's libm was not C99 compliant or something like that and we needed cephes at http://www.netlib.org/cephes/

It's certainly possible.

I think we should try to remove all the Cygwin specific mc and md things.

Let's please try to do this after we get Sage to build on Cygwin reliably, though :)

@jpflori
Copy link
Contributor Author

jpflori commented Dec 17, 2012

comment:15

Replying to @kcrisman:

I think the libm* problems are from the time the Cygwin's libm was not C99 compliant or something like that and we needed cephes at http://www.netlib.org/cephes/

It's certainly possible.

I think we should try to remove all the Cygwin specific mc and md things.

Let's please try to do this after we get Sage to build on Cygwin reliably, though :)

So let's merge the ticket as is!
it needs review :)

@kcrisman
Copy link
Member

comment:16

So let's merge the ticket as is!
it needs review :)

Fair enough, but it's hard for me to check whether I even have these libraries.

@kcrisman
Copy link
Member

comment:17

Just got past complex_double.o in compiling with this patch (it had been blocked at the algebras/letterplace stuff, so cleanly), so presumably the lack of the library isn't a problem. I'll assume this is indeed necessary on Win 7, though if Dima could independently verify this it would be nice...

@jpflori
Copy link
Contributor Author

jpflori commented Dec 18, 2012

comment:18

Wooo, I think we have to get back on this one and think a little more.
Indeed I spotted that the cephes spkg failed to install (although that was not detected properly...).
So we have to check two things:

  • why the cephes spkg fails (if it succeeds, it will provide libmc and libmd),
  • if the cephes spkg is really needed, indeed as I mentioned before, I think that the libm provided by Cygwin is now C99 compliant (enough) and cephes is not needed anymore (and I don't think keeping it in case one wants to build Sage on a very old Cygwin with a poor libm is any kind of justification).

So I think we should first:

  • fix the cephes spkg.
    Check it is not needed on Cygwin anymore, and if so:
  • disable its installation on Cygwin,
  • removed all Cygwin specific code involving libmc and libmd.

@jpflori
Copy link
Contributor Author

jpflori commented Dec 18, 2012

Changed keywords from none to cygwin cephes spkg

@jpflori
Copy link
Contributor Author

jpflori commented Dec 18, 2012

Work Issues: cephes spkg

@jpflori
Copy link
Contributor Author

jpflori commented Dec 18, 2012

comment:19

In fact cephes is currently only used on Cygwin, so if it is not needed, it should be removed completely (and we could as well not waste time fixing it).

@kcrisman
Copy link
Member

comment:20

In fact cephes is currently only used on Cygwin, so if it is not needed, it should be removed completely (and we could as well not waste time fixing it).

That's not quite correct - see #9543. So I would be happy to disable it on Cygwin, but not in general.

I agree that we should only support the newest Cygwin, given that this would primarily be used in binary form.

@jpflori
Copy link
Contributor Author

jpflori commented Dec 18, 2012

comment:21

I remember of #9543, but I worte my previous post after reading the first lines of spkg-install which states that is only installed on Cygwin :)
I've not looked further or at #9543 again to gather more information.

@jpflori
Copy link
Contributor Author

jpflori commented Dec 18, 2012

comment:22

Indeed there is no error checking at all in the spkg-install script and it is only installed on Cygwin.

#9543 is two years old, potentially the FreeBSD libm has gotten better since then just as the Cygwin one?

@jpflori
Copy link
Contributor Author

jpflori commented Dec 18, 2012

@kcrisman
Copy link
Member

comment:24

No, see this FreeBSD port patch which is basically #9543's patch and which afaik is definitely still needed on FreeBSD. I'll put that link on #9543, though, in case it turns out to be useful.

@jpflori
Copy link
Contributor Author

jpflori commented Dec 19, 2012

New version completly discarding Cephes use on Cygwin.

@jpflori
Copy link
Contributor Author

jpflori commented Dec 19, 2012

comment:25

Attachment: trac_13806.2.patch.gz

Here comes a new patches discarding any Cephes reference on Cygwin.

@kcrisman
Copy link
Member

comment:26

This seems to work fine with ./sage -ba on Cygwin. Interesting that we never actually installed Cephes - do I understand you correctly?

@jpflori
Copy link
Contributor Author

jpflori commented Dec 19, 2012

comment:27

I think you did install it, or the installation would have failed as for me.
It is maybe 7 specific.
Can you check for "$SAGE_LOCAL/lib/libm[c|d].a" ?

But what you just pointed out and confirmed is that linking to these files (and so installing Cephes) is not needed anymore on Cygwin.

@kcrisman
Copy link
Member

comment:28

I think you did install it, or the installation would have failed as for me.
It is maybe 7 specific.
Can you check for "$SAGE_LOCAL/lib/libm[c|d].a" ?

Oh, that's where they lived! Yes, they are there.

But what you just pointed out and confirmed is that linking to these files (and so installing Cephes) is not needed anymore on Cygwin.

Agreed.

Patchbot, apply trac_13806.2.patch

@kcrisman

This comment has been minimized.

@kcrisman
Copy link
Member

comment:30

Dumb question: Even though this patch worked fine, there seems to be an extraneous comma.

'sage/ext/interpreters/interp_cdf.c'],), 

in the new code. I assume that as usual Python tuples can end with commas, so it doesn't affect anything, but it looks a little silly, like

sage: integrate(x^2,x,)
1/3*x^3

@kcrisman
Copy link
Member

kcrisman commented Jan 3, 2013

comment:31

Dumb question: Even though this patch worked fine, there seems to be an extraneous comma.

Well, who cares? It is valid syntax. I don't want to hold this up.

@jdemeyer
Copy link
Contributor

jdemeyer commented Jan 7, 2013

Changed work issues from cephes spkg to none

@jdemeyer
Copy link
Contributor

jdemeyer commented Jan 7, 2013

Merged: sage-5.6.beta3

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

4 participants