Skip to content

All lib installs target CMake install libdir - #20879

Merged
nil-is-all merged 1 commit into
pytorch:mainfrom
corey-cole:feature/portable-install-fix
Jul 13, 2026
Merged

All lib installs target CMake install libdir#20879
nil-is-all merged 1 commit into
pytorch:mainfrom
corey-cole:feature/portable-install-fix

Conversation

@corey-cole

@corey-cole corey-cole commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

Summary

4 libraries previously targeted the lib directory in the build directory. This change modifies target location to match the other 55 libraries in the project.

Fixes #20709

Test plan

# Configure, build, and install
cmake -B cmake-out --preset linux -DCMAKE_BUILD_TYPE=Release -DEXECUTORCH_BUILD_XNNPACK=ON -DCMAKE_INSTALL_PREFIX="et-install" -DCMAKE_POSITION_INDEPENDENT_CODE=ON -DEXECUTORCH_BUILD_EXTENSION_MODULE=ON DEXECUTORCH_BUILD_EXTENSION_DATA_LOADER=ON -DEXECUTORCH_BUILD_EXTENSION_TENSOR=ON
cmake --build cmake-out -j15
cmake --install cmake-out --prefix et-install
# Confirm source directory isn't baked into release .cmake file (exit code 1)
grep -n 'cmake-out' et-install/lib/cmake/ExecuTorch/ExecuTorchTargets-release.cmake
# Confirm modified targets appear in installation lib directory
ls -l et-install/lib/libextension_evalue_util.a # exists
ls -l et-install/lib/libflatccrt.a # exists

cc @larryliu0820 @GregoryComer

@pytorch-bot

pytorch-bot Bot commented Jul 11, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/20879

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Cancelled Job

As of commit 2d6bac4 with merge base a6d812a (image):

CANCELLED JOB - The following job was cancelled. Please retry:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 11, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: corey-cole / name: Corey Cole (2d6bac4)

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 11, 2026
@github-actions

Copy link
Copy Markdown

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@nil-is-all nil-is-all added the module: build/install Issues related to the cmake and buck2 builds, and to installing ExecuTorch label Jul 13, 2026
@psiddh

psiddh commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

@claude review

@claude

claude Bot commented Jul 13, 2026

Copy link
Copy Markdown

Claude finished @psiddh's task in 53s —— View job


Code Review

  • Analyze the PR diff
  • Verify CMAKE_INSTALL_LIBDIR availability and consistency with the rest of the project

LGTM ✅ — this is a clean, correct fix that resolves #20709.

What the change does

Four install(...) calls previously hardcoded DESTINATION ${CMAKE_BINARY_DIR}/lib, which pins the install location to the build tree rather than honoring CMAKE_INSTALL_PREFIX. This PR switches them to ${CMAKE_INSTALL_LIBDIR}, matching the other ~55 install targets across the project:

  • devtools/bundled_program/CMakeLists.txt:54bundled_program
  • devtools/etdump/CMakeLists.txt:66etdump, flatccrt
  • extension/evalue_util/CMakeLists.txt:27extension_evalue_util
  • third-party/CMakeLists.txt:190flatccrt

Correctness checks

  • CMAKE_INSTALL_LIBDIR is defined at these points. The root CMakeLists.txt:110 runs include(GNUInstallDirs) before add_subdirectory(third-party) (line 274), schema (line 520), and the devtools/extension subdirectories, so the variable is populated wherever these installs run. No ordering hazard.
  • Consistency confirmed. Every other install target in the repo (backends/*, extension/*, schema, configurations, etc.) already uses ${CMAKE_INSTALL_LIBDIR}. These four were the outliers.
  • Fixes the reported bug. The old ${CMAKE_BINARY_DIR}/lib destination baked the source/build path into the exported ExecuTorchTargets config, breaking relocatable installs — exactly what the test plan's grep for cmake-out verifies is now gone.

Minor / non-blocking notes

  1. Commit messageMove targets to is truncated/incomplete. Consider amending to something like Install libs to CMAKE_INSTALL_LIBDIR instead of build dir before merge. Per the repo's CLAUDE.md, please also disclose if this PR was authored with Claude.
  2. release notes: label — the bot is asking for one; release notes: none is likely appropriate for a build-config fix (though packagers/downstream consumers do benefit, so release notes: build could be argued).
  3. GNUInstallDirs defaultCMAKE_INSTALL_LIBDIR defaults to lib (or lib64 on some 64-bit distros). That's the intended, standards-compliant behavior and is what the rest of the project already relies on, so no action needed — just flagging that installs on multilib systems may land in lib64/, which is correct.

Nice, well-scoped fix with a clear reproducible test plan. 👍
feature/portable-install-fix

@psiddh
psiddh self-requested a review July 13, 2026 19:49
@nil-is-all
nil-is-all merged commit e5c55b3 into pytorch:main Jul 13, 2026
201 of 204 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: build/install Issues related to the cmake and buck2 builds, and to installing ExecuTorch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Some targets install to ${CMAKE_BINARY_DIR}/lib, breaking find_package(executorch) from an install prefix

3 participants