Skip to content

In conda-build produced binaries, use paths relative to $PREFIX#817

Merged
rapids-bot[bot] merged 14 commits intorapidsai:branch-25.06from
jakirkham:chng_gcc_prefix_pth
Apr 21, 2025
Merged

In conda-build produced binaries, use paths relative to $PREFIX#817
rapids-bot[bot] merged 14 commits intorapidsai:branch-25.06from
jakirkham:chng_gcc_prefix_pth

Conversation

@jakirkham
Copy link
Copy Markdown
Member

@jakirkham jakirkham commented Apr 8, 2025 •
edited
Loading

Description

Fixes #798

Add compiler flags in conda-build runs to remap binary embedded paths from absolute paths (with $PREFIX) to $PREFIX relative paths. This keeps the meaningful portion of the path while dropping the build time specific $PREFIX. Plus bypasses the need to do prefix replacement with these paths.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The cmake-format.json is up to date with these changes.
  • I have added new files under rapids-cmake/
    • I have added include guards (include_guard(GLOBAL))
    • I have added the associated docs/ rst file and update the api.rst

@jakirkham jakirkham requested a review from a team as a code owner April 8, 2025 01:17
@jakirkham jakirkham added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels Apr 8, 2025
@jakirkham jakirkham force-pushed the chng_gcc_prefix_pth branch 4 times, most recently from 16ec700 to 6b0988a Compare April 8, 2025 05:19
@jakirkham jakirkham force-pushed the chng_gcc_prefix_pth branch from 6b0988a to 9a899aa Compare April 8, 2025 06:18
@jakirkham
Copy link
Copy Markdown
Member Author

jakirkham commented Apr 9, 2025

Thanks Kyle! 🙏

@vyasr would you like to take a look as well?

@jakirkham jakirkham mentioned this pull request Apr 9, 2025
Closed
3 tasks
@bdice
Copy link
Copy Markdown
Contributor

bdice commented Apr 9, 2025

Is it possible to test this?

@jakirkham
Copy link
Copy Markdown
Member Author

jakirkham commented Apr 9, 2025

Is there a way to checkout this PR in a build of say RMM or cuDF? If so, we could use that to test this change in downstream CI

This was referenced Apr 16, 2025
Closed
Closed
@bdice
Copy link
Copy Markdown
Contributor

bdice commented Apr 16, 2025

Yes. Here are test PRs:

vyasr
vyasr approved these changes Apr 16, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@vyasr vyasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. One very minor suggestion but nothing blocking. I just merged latest so that I could rerun CI on the test PRs since this PR is now out of date w.r.t. the rmm layout changes, so the cudf test job is failing. Once it passes we should be good.

robertmaynard
robertmaynard requested changes Apr 17, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@robertmaynard robertmaynard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docs for rapids_cmake_support_conda_env need to be expanded to document this new behavior ( and what version it is added in ). See the paragraph about -O0 for an example

jakirkham
jakirkham commented Apr 17, 2025
View reviewed changes
Copy link
Copy Markdown
Member Author

@jakirkham jakirkham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks all for the feedback!

Also thanks for testing Bradley! 🙏

Included Vyas' suggestion. Thank for mentioning that simplification

Also included a doc/release note based on Rob's feedback

Please let me know if there is anything else 🙂

@jakirkham jakirkham requested a review from robertmaynard April 17, 2025 23:06
jakirkham
jakirkham commented Apr 19, 2025
View reviewed changes
rapids-cmake/cmake/support_conda_env.cmake
Comment on lines +44 to +50
.. versionadded:: v25.06.00

The flag `-ffile-prefix-map` is now passed to remap absolute paths starting
with `$ENV{PREFIX}` to paths relative to it in binaries generated via
compilation. This ensures paths baked into binaries are relative to the
environment prefix. This prevents Conda from rewriting these paths when the
package is installed.
Copy link
Copy Markdown
Member Author

@jakirkham jakirkham Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about this revision?

Copy link
Copy Markdown
Contributor

@robertmaynard robertmaynard Apr 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it

@jakirkham jakirkham changed the title Use relative paths for $PREFIX in conda-build In conda-build produced binaries, use paths relative to $PREFIX Apr 19, 2025
@jakirkham jakirkham requested a review from bdice April 19, 2025 07:08
@bdice
Copy link
Copy Markdown
Contributor

bdice commented Apr 21, 2025

/merge

@rapids-bot rapids-bot bot merged commit de1012f into rapidsai:branch-25.06 Apr 21, 2025
13 checks passed
@jakirkham jakirkham deleted the chng_gcc_prefix_pth branch April 21, 2025 18:18
@jakirkham
Copy link
Copy Markdown
Member Author

jakirkham commented Apr 21, 2025

Thanks all! 🙏

@jakirkham jakirkham mentioned this pull request May 20, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@robertmaynard robertmaynard robertmaynard approved these changes

@vyasr vyasr vyasr approved these changes

@bdice bdice bdice approved these changes

@KyleFromNVIDIA KyleFromNVIDIA KyleFromNVIDIA approved these changes

Assignees

No one assigned

Labels

improvement Improves an existing functionality non-breaking Introduces a non-breaking change

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEA] Set -ffile_prefix_map when in a conda-build

5 participants

@jakirkham @bdice @robertmaynard @vyasr @KyleFromNVIDIA