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

[vcpkg-scripts] Exclude .a files from patchelf #40048

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

LandryNorris
Copy link

@LandryNorris LandryNorris commented Jul 23, 2024

Exclude static libraries (.a) files from patchelf checks. This prevents systems that only build static libraries from requiring patchelf to be installed. Since a static library is not a valid ELF file, patchelf will fail when run on it anyways.

Fixes #40032

@LandryNorris
Copy link
Author

LandryNorris commented Jul 23, 2024

Given that CMake allows a dev to set the CMAKE_EXECUTABLE_SUFFIX, would it be good to use CMake's file(READ <filename> <variable> [OFFSET <offset>] [LIMIT <max-in>] [HEX]) call to check for an ELF header instead of relying on known extensions to exclude? This would add a more authoritative check that would only require patchelf to be installed and run if the file truly is an ELF. If this is a desired change, I could create a PR.

@Osyotr
Copy link
Contributor

Osyotr commented Jul 24, 2024

Skipping files based on extension is much cheaper than reading them.

@data-queue
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@WangWeiLin-MV WangWeiLin-MV self-assigned this Jul 24, 2024
@WangWeiLin-MV WangWeiLin-MV added the category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly label Jul 24, 2024
@WangWeiLin-MV WangWeiLin-MV added the info:reviewed Pull Request changes follow basic guidelines label Jul 24, 2024
@Cheney-W Cheney-W changed the title Exclude .a files from patchelf [vcpkg-scripts] Exclude .a files from patchelf Jul 26, 2024
@data-queue data-queue merged commit a2938d0 into microsoft:master Jul 26, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:tool-update The issue is with build tool or build script, which requires update or should be executed correctly info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installing a package as a static library for Linux/mac target should not require patchelf to be installed
4 participants