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

Normalize rpath entries to guard against missing default solib dir #14924

Closed
wants to merge 1 commit into from

Conversation

fmeum
Copy link
Collaborator

@fmeum fmeum commented Mar 1, 2022

When all dynamic deps in a build are built in transitioned configurations, the default solib dir is not created. However, while
resolving paths, the dynamic linker stops at the first directory that does not exist, even when followed by ../.

Before this commit, all rpath entries would consist of the relative path to the default solib dir followed by the relative path to the
particular library's solib dir. Thus, if the default solib dir was missing, the dynamic linker wouldn't resolve any of these paths.

This commit ensures that the relative path entries are normalized and thus contain no references to non-existing directories assuming the normalized path itself exists.

Work towards #13819.

Closes #14660.

PiperOrigin-RevId: 431671888

When all dynamic deps in a build are built in transitioned configurations, the default solib dir is not created. However, while
resolving paths, the dynamic linker stops at the first directory that does not exist, even when followed by `../`.

Before this commit, all rpath entries would consist of the relative path to the default solib dir followed by the relative path to the
particular library's solib dir. Thus, if the default solib dir was missing, the dynamic linker wouldn't resolve any of these paths.

This commit ensures that the relative path entries are normalized and thus contain no references to non-existing directories assuming the normalized path itself exists.

Work towards bazelbuild#13819.

Closes bazelbuild#14660.

PiperOrigin-RevId: 431671888
@fmeum
Copy link
Collaborator Author

fmeum commented Mar 1, 2022

@Wyverald Not sure whether this is too late to get into 5.1.0. What do you think?

@oquenchil oquenchil removed the request for review from lberki March 1, 2022 14:41
@oquenchil
Copy link
Contributor

This was merged 7 minutes ago: #14660

Why this PR?

@fmeum
Copy link
Collaborator Author

fmeum commented Mar 1, 2022

@oquenchil Sorry, you should not have gotten pinged about this: This is not a PR against master but rather against the branch intended for cherry picks into 5.1.0rc1. Unfortunately, CODEOWNERS is the same as on master for this branch, which means that the same reviewers will get pinged again even though only the release manager, in this case @Wyverald, should have to be notified. I discussed this with him over at #14899 (comment) and we found a workaround, so there shouldn't be any redundant pings starting with the next release.

Sorry again for the redundant ping and thank you very much for reviewing the original PR!

@brentleyjones
Copy link
Contributor

brentleyjones commented Mar 1, 2022

@Wyverald @fmeum The latest instructions says to cherry-pick into release-5.1.0 not release-5.1.0rc1. Which is correct? For example, I did release-5.1.0 for #14923.

@Wyverald
Copy link
Member

Wyverald commented Mar 2, 2022

Yeah please send a PR against release-5.1.0. The rcX branches will be used as tags (easier to create on the GitHub UI). Sorry for the confusion!

@Wyverald Wyverald removed the request for review from oquenchil March 2, 2022 10:36
@fmeum
Copy link
Collaborator Author

fmeum commented Mar 2, 2022

Closed in favor of #14929

@fmeum fmeum closed this Mar 2, 2022
@fmeum fmeum deleted the release-5.1.0rc1 branch March 2, 2022 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants