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

BLD: preserve library order #7699

Merged
merged 1 commit into from
Jul 28, 2016
Merged

BLD: preserve library order #7699

merged 1 commit into from
Jul 28, 2016

Conversation

nolta
Copy link
Contributor

@nolta nolta commented May 31, 2016

Before, the list of libraries was resorted to match the order of library_dirs. Now, the opposite occurs: library_dirs is resorted to match the library order.

Ran into this problem while trying to build numpy for a Bluegene/Q. The order static libraries are passed to the linker is important.

@rgommers
Copy link
Member

CI failed: https://travis-ci.org/numpy/numpy/jobs/134270630. The error looks related to this PR, somehow ATLAS went missing. It only failed because of compiling with -OO, but ATLAS should be found during the build.

@rgommers
Copy link
Member

@nolta it would also be useful to provide the relevant part of the build log for your failing build. Both for searchability for the next person that runs into the issue you found, and for us to understand better what is going wrong here.

@nolta
Copy link
Contributor Author

nolta commented May 31, 2016

Basically, i have 2 directories and 3 libraries, and i need to make a library sandwich:

library_dirs = X : Y
libs = a, b, c

Libraries a and c are in directory X while library b is in Y.

The library order a, b, c is important. However, numpy reorders the libraries to match the directories. So only

library_dirs = X : Y
libs = a, c, b

or

library_dirs = Y : X
libs = b, a, c

are currently allowed.

@nolta
Copy link
Contributor Author

nolta commented Jun 1, 2016

CI now green. Was missing a break statement.

@rgommers rgommers self-assigned this Jun 5, 2016
@rgommers rgommers added this to the 1.12.0 release milestone Jun 5, 2016
@charris
Copy link
Member

charris commented Jun 12, 2016

@rgommers LGTY?

@charris
Copy link
Member

charris commented Jun 17, 2016

Needs a comment in the code so it doesn't get "cleaned up" later. Also a comment in the release notes.

@charris
Copy link
Member

charris commented Jun 22, 2016

@rgommers This seems like a reasonable change to me.

Before, the list of libraries was resorted to match the order of
library_dirs. Now, the opposite occurs: library_dirs is resorted to
match the library order.
@nolta
Copy link
Contributor Author

nolta commented Jun 25, 2016

Added a comment & release note.

if p:
assert len(p) == 1
Copy link
Member

Choose a reason for hiding this comment

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

Why not leave this part? It is a bit confusing to do it after breaking from the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure i understand? This change is almost entirely whitespace: c84c875?w=1

@charris charris merged commit 8c808a6 into numpy:master Jul 28, 2016
@charris
Copy link
Member

charris commented Jul 28, 2016

Thanks @nolta .

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

Successfully merging this pull request may close these issues.

None yet

3 participants