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

Bugfix for MacOS installs from pip #1

Merged
merged 7 commits into from Oct 25, 2019

Conversation

@mfherbst
Copy link
Member

mfherbst commented Oct 24, 2019

There was an issue properly replacing the linux binaries shipped with with the downloaded MacOS binaries. This should be fixed by this PR. I have made a test upload with the proposed fix at https://get.adc-connect.org/adcc-0.13.1.tar.gz.

@maxscheurer Could you test that?

Update: If you got here searching for a workaround for installing adcc 0.13.1 on mac, try

pip install https://get.adc-connect.org/adcc-0.13.1.tar.gz
@mfherbst mfherbst force-pushed the fix-macos-install branch 2 times, most recently from b18888f to 29963ae Oct 24, 2019
@mfherbst mfherbst added the bug label Oct 24, 2019
@maxscheurer

This comment has been minimized.

Copy link
Member

maxscheurer commented Oct 24, 2019

The problem still exists when installing via

pip3 install --no-cache-dir https://get.adc-connect.org/adcc-0.13.1.tar.gz

I have no idea what's going wrong here...

@mfherbst

This comment has been minimized.

Copy link
Member Author

mfherbst commented Oct 24, 2019

But you can checkout this github repository and successfully build and test adcc, right? So it's only an issue with the pip installation?

@mfherbst mfherbst force-pushed the fix-macos-install branch from 29963ae to 51f094c Oct 24, 2019
@maxscheurer

This comment has been minimized.

Copy link
Member

maxscheurer commented Oct 24, 2019

Let me check this quickly...

@maxscheurer

This comment has been minimized.

Copy link
Member

maxscheurer commented Oct 24, 2019

Yes, only the pip installation is broken. Checking out adcc and installing 'locally' works fine.

@maxscheurer

This comment has been minimized.

Copy link
Member

maxscheurer commented Oct 24, 2019

Okay I found the bug, but I don't know how to fix it...

The difference between the two installations is that in the adcc/lib directory installed by pip, I find libadccore.so, whereas in the case of my locally, working installation libadccore.0.13.1.dylib and libadccore.dylib.

@mfherbst

This comment has been minimized.

Copy link
Member Author

mfherbst commented Oct 24, 2019

Ok ... that's the error! Because libadccore.so is a linux elf binary. That really surprises me as it should be removed with the fix I introduced here. Also I just checked ... the fix is definitely deployed in the tarball I have linked above.

Could you double-check that you do not use some weird cache from pip? I'll try to cook up a way to enable a more "verbose" version of the installation, such that we can check what happens on your machine.

@maxscheurer

This comment has been minimized.

Copy link
Member

maxscheurer commented Oct 24, 2019

I did triple-check the installation in three new virtualenvs.
What surprises me is that there is no dylib at all...

@mfherbst

This comment has been minimized.

Copy link
Member Author

mfherbst commented Oct 24, 2019

Oh ... that's weird, too ... and definitely not the way it's supposed to be.

@mfherbst

This comment has been minimized.

Copy link
Member Author

mfherbst commented Oct 24, 2019

I think I'm starting to understand what the problem is.

When pip installs a package, then it does the download and the compilation of the extension in a temporary directory first and only later copies the resulting files over to the site-packages folder. It could be, that the download and replacement is only done inside this temporary folder, but (apart from the extension so file) the copying to the site-packages is done from the unchanged tarball anew. That way the installation does as it should, it just does not operate on the "right" folder, or put another way it should just copy the files from the temporary folder.

I guess this might be different for installations from a directory. We'll have to investigate this further.

@maxscheurer

This comment has been minimized.

Copy link
Member

maxscheurer commented Oct 24, 2019

Yes, because the site-packages folder is created at the last instance of the installation...
So annoying...

@maxscheurer

This comment has been minimized.

Copy link
Member

maxscheurer commented Oct 24, 2019

Probably this information helps:

In the temporary directory that pip builds adcc, the *.so files live in adcc/lib and build, whereas the dylib files live in extension/adccore/lib.

See the following output run in the temporary pip build directory for clarification:

find . -name "*.so"
./adcc/lib/libadccore.so
./build/lib.macosx-10.14-x86_64-3.7/adcc/lib/libadccore.so

vs.

find . -name "*.dylib"
./extension/adccore/lib/libadccore.dylib
./extension/adccore/lib/libadccore.0.13.1.dylib
@mfherbst

This comment has been minimized.

Copy link
Member Author

mfherbst commented Oct 24, 2019

Yes, that's it. This does not matter for source builds, because there adcc/lib is symlinked to extension/adccore/lib, so it's the same thing, but for pip builds, that's of course not true.

I'll think about something for that.

@mfherbst

This comment has been minimized.

Copy link
Member Author

mfherbst commented Oct 24, 2019

Ok, I now made the paths relative to the top of the repository and use the adcc/lib path explicitly in the globbing expressions. This makes the tarballs incompatible with 0.13.1, so we will have to version-bump.

I'll build the new adccore tarballs using our Travis setup now and then we can first verify that everthing is working on Mac.

@mfherbst mfherbst requested a review from maxscheurer Oct 25, 2019
@mfherbst

This comment has been minimized.

Copy link
Member Author

mfherbst commented Oct 25, 2019

@maxscheurer Could you extend the documentation on the MacOS installation if necessary? If not, just remove the TODO note 😄.

Copy link
Member

maxscheurer left a comment

G2G!

@mfherbst mfherbst merged commit a574e17 into master Oct 25, 2019
@mfherbst mfherbst deleted the fix-macos-install branch Oct 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.