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

Allow uname calls in download mode + Fix handling of files without RPATH #37129

Merged

Conversation

BillyONeal
Copy link
Member

@BillyONeal BillyONeal commented Mar 4, 2024

... and don't fetch patchelf unless we actually need it. For example cmake helper ports have no need to fetch patchelf.

In #36056 , things were changed to unconditionally patchelf when targeting Linux. Unfortunately that broke --only-downloads a lot of the time. This fixes that.

Also contains @Osyotr 's fixes from #37190 to coalesce world rebuilds a bit, so repeating comments from there:

Fixes #37183
Towards #25668

…e actually need it.

In microsoft#36056 , things were changed to unconditionally patchelf when targeting Linux. Unfortunately that broke --only-downloads a lot of the time. This fixes that.
@BillyONeal BillyONeal added category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team. labels Mar 4, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Mar 5, 2024

Note that you must ensure that vcpkg_find_acquire_program(PATCHELF) is called in download mode in order to have it available in disconnected operation.

I wonder if the result of uname -m shouldn't be cached, early, for the current run. If not already known by the tool.

@BillyONeal BillyONeal changed the title Allow uname calls in download mode Allow uname calls in download mode + Fix handling of files without RPATH Mar 6, 2024
@Neumann-A
Copy link
Contributor

Looking at the logs it seems like the REMOVE_DUPLICATES call needs to be later in the fixup script.

@Osyotr
Copy link
Contributor

Osyotr commented Mar 6, 2024

Looking at the logs it seems like the REMOVE_DUPLICATES call needs to be later in the fixup script.

It's harmless but would be nice to fix.
Adjusted RPATH of '/mnt/vcpkg-ci/p/hdf5_x64-linux/tools/hdf5/gif2h5-shared' (From '' -> To '$ORIGIN:$ORIGIN/../../lib:$ORIGIN/../../lib:$ORIGIN/../lib:$ORIGIN')

@BillyONeal
Copy link
Member Author

It's harmless but would be nice to fix. Adjusted RPATH of '/mnt/vcpkg-ci/p/hdf5_x64-linux/tools/hdf5/gif2h5-shared' (From '' -> To '$ORIGIN:$ORIGIN/../../lib:$ORIGIN/../../lib:$ORIGIN/../lib:$ORIGIN')

I get $ORIGIN:$ORIGIN/../../lib but why $ORIGIN/../lib ?

@Neumann-A
Copy link
Contributor

I get $ORIGIN:$ORIGIN/../../lib but why $ORIGIN/../lib ?

Unix structure. If you are in /bin you need ../lib. The real question here is how does it go from "" to 4 entries since only two should be added.

@BillyONeal
Copy link
Member Author

Unix structure. If you are in /bin you need ../lib. The real question here is how does it go from "" to 4 entries since only two should be added.

Can you double check the test I added then since there's clearly something I'm misunderstanding here

@Neumann-A
Copy link
Contributor

-- Adjusted RPATH of '/mnt/vcpkg-ci/p/llvm_x64-linux/debug/lib/LLVMPolly.so' (From '' -> To '$ORIGIN')
-- Adjusted RPATH of '/mnt/vcpkg-ci/p/llvm_x64-linux/debug/lib/libLTO.so.17' (From '' -> To '$ORIGIN:$ORIGIN/../lib')

I feel like there is some variable being reused between runs.

@Neumann-A
Copy link
Contributor

Ah the message is wrong:
message(STATUS "Adjusted RPATH of '${elf_file}' (From '${org_rpath}' -> To '${new_rpath}')")
needs to be changed to:
message(STATUS "Adjusted RPATH of '${elf_file}' (From '${readelf_output}' -> To '${new_rpath}')")

the refactor broke it.

@BillyONeal
Copy link
Member Author

Ah the message is wrong: message(STATUS "Adjusted RPATH of '${elf_file}' (From '${org_rpath}' -> To '${new_rpath}')") needs to be changed to: message(STATUS "Adjusted RPATH of '${elf_file}' (From '${readelf_output}' -> To '${new_rpath}')")

the refactor broke it.

Fixed!

@Neumann-A
Copy link
Contributor

Adjusted RPATH of '/mnt/vcpkg-ci/p/hdf5_x64-linux/tools/hdf5/gif2h5' (From '/mnt/vcpkg-ci/installed/x64-linux/lib:$ORIGIN/../lib:$ORIGIN/' -> To '$ORIGIN:$ORIGIN/../../lib:$ORIGIN/../lib')

looks ok now. It is just keeping already existing rpath settings.

@BillyONeal BillyONeal merged commit e38b936 into microsoft:master Mar 7, 2024
8 of 16 checks passed
@BillyONeal BillyONeal deleted the allow-uname-in-download-mode branch March 7, 2024 23:19
@BillyONeal
Copy link
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-bug The issue is with the vcpkg system (including helper scripts in `scripts/cmake/`) info:internal This PR or Issue was filed by the vcpkg team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[vcpkg-tool] After 2ac6ba17 RUNPATH on built dynamic libraries is not set
5 participants