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

update M4RI to version 20110901 or later #11757

Closed
malb opened this issue Aug 29, 2011 · 22 comments
Closed

update M4RI to version 20110901 or later #11757

malb opened this issue Aug 29, 2011 · 22 comments

Comments

@malb
Copy link
Member

malb commented Aug 29, 2011

In #11574 it was noticed that M4RI does not provide sufficient information to allow third party software to choose the appropriate compiler flags to link against it. While a work around is provided in #11574, it is not as clean as it could be. That is, M4RI should export the flags that were used to when building it, which can then be re-used by third party code.

Install http://sage.math.washington.edu/home/malb/spkgs/libm4ri-20111004.spkg

Apply either attachment: trac_11757_m4ri_sse2.patch or attachment: trac_11757_m4ri_sse2_rebased10903.patch (on top of #10903).

Depends on #11574

CC: @nexttime @alexanderdreyer

Component: packages: standard

Author: Martin Albrecht

Reviewer: Alexander Dreyer

Merged: sage-4.7.2.alpha4

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

@malb malb added this to the sage-4.7.2 milestone Aug 29, 2011
@malb
Copy link
Member Author

malb commented Aug 29, 2011

comment:1

Here's how the output looks like on bsd.math:

#define __M4RI_CFLAGS                   " -mmmx -msse -msse2 -msse3   -I/Users/malb/sage-4.7.2.alpha2/local/include -g -fPIC -Wall -pedantic -O2"

I'm not sure we want the "-I/Users/..." part in there though. But we set CFLAGS in spkg-install so I'm not sure how to avoid it.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 29, 2011

comment:2

Replying to @malb:

Here's how the output looks like on bsd.math:

#define __M4RI_CFLAGS                   " -mmmx -msse -msse2 -msse3   -I/Users/malb/sage-4.7.2.alpha2/local/include -g -fPIC -Wall -pedantic -O2"

I'm not sure we want the "-I/Users/..." part in there though. But we set CFLAGS in spkg-install so I'm not sure how to avoid it.

This would introduce new potential hardcoding issues. (It shouldn't hurt if the directory later -- after moving Sage -- does no longer exist, but it could break the installation if it does, and the headers there are meanwhile incompatible.)

You could post-process the file in M4RI's spkg-install.

Do you also add __M4RI_CC? (Haven't looked at the spkg yet.)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 29, 2011

comment:3

Since you have -I... already in CPPFLAGS, not adding it to CFLAGS in spkg-install should also work.

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 30, 2011

comment:4

Replying to @nexttime:

Since you have -I... already in CPPFLAGS, not adding it to CFLAGS in spkg-install should also work.

Oh, I see M4RI doesn't honor CPPFLAGS, but it also doesn't use any of Sage's headers AFAIK, so you could simply remove -I... from CFLAGS (unless configure does weird things).

@malb
Copy link
Member Author

malb commented Aug 30, 2011

comment:5

I've revised spkg-install accordingly and replaced the SPKG. Now we get:

#define __M4RI_CC                       "gcc -std=gnu99"
#define __M4RI_CFLAGS                   "    -g -fPIC -Wall -pedantic -O2"

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 31, 2011

comment:6

Added #11574 as a dependency because of attachment: ticket:11574:m4ri_20110601.patch (API change, corrected extension module dependencies in module_list.py).

(Martin, correct me if I'm wrong...)

@nexttime
Copy link
Mannequin

nexttime mannequin commented Aug 31, 2011

Dependencies: #11574

@malb
Copy link
Member Author

malb commented Aug 31, 2011

comment:7

Uh, yep, that's correct. Thanks!

@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 23, 2011

comment:8

Why does this still need review?

And isn't "exporting" the compiler flags meanwhile fully fixed by #11574? (Except for perhaps a .pc file.)

The description should get updated...

Or is this spkg / ticket meanwhile obsolete, superseded by the one from #11574?

I actually first thought I had merged the wrong spkg.

@malb

This comment has been minimized.

@malb
Copy link
Member Author

malb commented Sep 23, 2011

comment:10

In #11574 we only work around this short coming of M4RI, in this ticket it's fixed. It's actually needs work because the Sage patch should be updated to use the exported flags (instead of setting its own).

@malb
Copy link
Member Author

malb commented Oct 3, 2011

Attachment: trac_11757_m4ri_sse2.patch.gz

@malb
Copy link
Member Author

malb commented Oct 4, 2011

comment:11

The updated SPKG & patch work fine for me on sage.math, bsd.math and cicero (where we had issues before).

@malb

This comment has been minimized.

@malb

This comment has been minimized.

@alexanderdreyer
Copy link
Mannequin

alexanderdreyer mannequin commented Oct 5, 2011

comment:13

I installed and tested spkg and patch successfully on a SLES 11 amd64 and Mac OS X 10.5 ppc (32bit). Also the patch and the spkg look sane, so positive review.

@alexanderdreyer
Copy link
Mannequin

alexanderdreyer mannequin commented Oct 5, 2011

Reviewer: Alexander Dreyer

@malb
Copy link
Member Author

malb commented Oct 5, 2011

comment:14

yay!

@jdemeyer
Copy link

jdemeyer commented Oct 5, 2011

Rebased on top of #10903

@jdemeyer
Copy link

jdemeyer commented Oct 5, 2011

comment:15

Attachment: trac_11757_m4ri_sse2_rebased10903.patch.gz

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

jdemeyer commented Oct 6, 2011

Merged: sage-4.7.2.alpha4

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

2 participants