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.

Sign up for GitHub

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

Jump to bottom

Add mpfr #133

Merged
merged 5 commits into from
Mar 27, 2016
Merged

Add mpfr #133

merged 5 commits into from
Mar 27, 2016

Conversation

jakirkham
Copy link
Member

Requires: #132

Builds mpfr on Mac and Linux. Borrows the recipe from conda recipes. Makes some tweaks to fix in conda-forge.

@jakirkham
Copy link
Member Author

@asmeurer @frol, any interest in help to maintain this recipe?

@jakirkham jakirkham mentioned this pull request Mar 17, 2016
Merged
@jakirkham jakirkham changed the title Add mpfr WIP: Add mpfr Mar 17, 2016
@jakirkham
Copy link
Member Author

Restarting now that gmp has been released.

@jakirkham
Copy link
Member Author

@isuruf, do you have any interest in mpfr? Would happily add you as a maintainer.

@isuruf
Copy link
Member

isuruf commented Mar 27, 2016

Yes.
We'll need to add MPIR first to enable Windows builds

@pelson
Copy link
Member

pelson commented Mar 27, 2016

Grrr make check 😢

@jakirkham
Copy link
Member Author

Grrr make check 😢

I know, right? Was hoping it would go smoothly. 😞
Will take a closer look now.

@jakirkham
Copy link
Member Author

Funny, I cannot reproduce this locally. Will take a look at the linkages here to see what is going on. Maybe something else is missing.

@jakirkham jakirkham force-pushed the add_mpfr branch 3 times, most recently from 8be8afc to 962039a Compare March 27, 2016 19:00
jakirkham
jakirkham reviewed Mar 27, 2016
View reviewed changes
recipes/mpfr/run_test.sh
@@ -0,0 +1,2 @@
cc -L$PREFIX/lib -I$PREFIX/include -lmpfr -lgmp -Wl,-rpath,$PREFIX/lib $RECIPE_DIR/test.c -o test.o
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied this tweak to fix the test on Mac.

@jakirkham jakirkham force-pushed the add_mpfr branch 3 times, most recently from e1aca6a to 1cf0ff9 Compare March 27, 2016 19:27
jakirkham
jakirkham reviewed Mar 27, 2016
View reviewed changes
recipes/mpfr/build.sh
@@ -0,0 +1,12 @@
./configure --prefix=$PREFIX \
--with-gmp-include=$PREFIX/include \
--with-gmp-lib=$PREFIX/lib \
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried specifying using --with-gmp. However, there was a very nasty message about a mismatch between the include and library versions. So, trying this instead as this was suggested. Not expecting this will make a difference, but should rule it out anyways. Besides it always could work. 🍀

@jakirkham jakirkham force-pushed the add_mpfr branch 4 times, most recently from 2e8bdab to 7b1915c Compare March 27, 2016 19:44
@isuruf
Copy link
Member

isuruf commented Mar 27, 2016

Got it working on Travis-CI. See, isuruf@1e5cf19

@jakirkham jakirkham mentioned this pull request Mar 27, 2016
Merged
@jakirkham
Copy link
Member Author

Thanks. Just merged your changes here, @isuruf.

I suspect that brew was installing gmp too, which might have been giving us problems. Looks like your change would fix this.

jakirkham
jakirkham reviewed Mar 27, 2016
View reviewed changes
recipes/mpfr/build.sh
@@ -0,0 +1,10 @@
#!/bin/bash

export LD_LIBRARY_PATH=$PREFIX/lib:$LD_LIBRARY_PATH
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appears that brew may install its own copy of gmp, which was causing issue for this build. The change proposed here by @isuruf should fix this. Already it is apparent that the configuration issues that were seen on Travis CI are going away after this change. Thanks @isuruf.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, in gmp build also we saw homebrew installed gmp interfering with conda version

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could uninstall these things from our CI, but these are real problems that could occur when building locally on a user's system. So, it isn't bad that we come up with a fix.

@jakirkham jakirkham changed the title WIP: Add mpfr Add mpfr Mar 27, 2016
@jakirkham
Copy link
Member Author

CI passes. Any final feedback before merging this?

@isuruf
Copy link
Member

isuruf commented Mar 27, 2016

Looks good to me

@jakirkham
Copy link
Member Author

Going to go ahead and merge so that mpc can be finished up. More feedback can be provided in the feedstock.

@jakirkham jakirkham merged commit aa52654 into conda-forge:master Mar 27, 2016
@jakirkham jakirkham deleted the add_mpfr branch March 27, 2016 20:07
@jakirkham
Copy link
Member Author

Binaries are released and available. Also the feedstock has been created.

@jakirkham jakirkham mentioned this pull request Mar 28, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jakirkham @isuruf @pelson