Skip to content

Conversation

Flamefire
Copy link
Contributor

In https://gist.github.com/boegelbot/1e87f59c5b2d74984a925e56a7adcb3d there is

Failed to determine dynamically linked libraries for {path}, so skipping it in RPATH sanity check

There was a missed f-string causing this.

Additionally the file call (external tool) is unnecessary for symbolic links where it returns

symbolic link to libxxx

This will then not do any checks, so it can be skipped in a much cheaper way by using islink, This change doesn't change existing behavior it just makes it obvious.

This also means that sanity_check_rpath can/should be enhanced as the is_parent_path check for outside symlinks doesn't make sense if no check is done for any symlinks yielding an additional "Failed to determine dynamically linked libraries for..." message.

Shall we do that too?

`file` will return "symbolic link to libxxx" which will then skip the
remaining checks.
So do not bother running `file` on it which is much slower than an `islink` check.
@boegel boegel added the bug fix label Aug 12, 2025
@boegel boegel added this to the next release (5.1.2) milestone Aug 12, 2025
@boegel boegel changed the title Fix f-string in rpath check and Skip check for linked libs on symlinks Fix f-string in rpath check and skip check for linked libs on symlinks Aug 12, 2025
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

@boegel boegel enabled auto-merge August 27, 2025 13:24
@boegel boegel merged commit 4c0a6ca into easybuilders:develop Aug 27, 2025
37 checks passed
@Flamefire
Copy link
Contributor Author

Small followup after the discussion: #4988

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants