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

buildcache install should fail if the tarball was created with a different directory layout #6536

Closed
wants to merge 27 commits into from

Conversation

gartung
Copy link
Member

@gartung gartung commented Dec 1, 2017

I found that tarballs created with a non-default directory layout cannot be relocated to a default directory layout. In this case replacing the old layout root with the new layout root does not work. The rpaths with still refer to the non-default relative prefixes.

Any replacement or rpaths happens in a workdir before moving the files to the install prefix. This prevents having a broken install prefix left over since I did not clean up on exceptions.

This PR includes
#6512
and
#6344

JavierCVilla and others added 24 commits November 17, 2017 09:24
Conflicts:
	lib/spack/spack/relocate.py
Conflicts:
	lib/spack/spack/relocate.py
The previous test skipped dynamically linked in filetype and never tested for ELF.

Add test for relocate_text to make sure problems are found in testing.

Do not include the prefix directory name in tarball. This causes problem when installing tarballs to a spack directory with a diffrent directory layout.
…le might end up in the text to relocate list is filter_file() is called during install
The previous test skipped dynamically linked in filetype and never tested for ELF.

Add test for relocate_text to make sure problems are found in testing.

Do not include the prefix directory name in tarball. This will aid later when installing to a different directory layout.
@scheibelp
Copy link
Member

I assume I should look at this vs. #6512

I'll say I think more-significant changes are needed than 54340e2 as I mentioned in #6344 (review). I think it would be fastest to remove those particular changes for the time being and keep it to a separate PR.

@gartung
Copy link
Member Author

gartung commented Dec 1, 2017

You should review and hopefully merge #6512 first. That has fixes for reported bugs. So far I am the only person who found this bug.

@gartung
Copy link
Member Author

gartung commented Dec 1, 2017

resubmitting

@gartung gartung closed this Dec 1, 2017
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