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

Don't use absolute dir for mkbootimg symlink #137

Merged
merged 2 commits into from
Jul 6, 2024

Conversation

Thaodan
Copy link
Contributor

@Thaodan Thaodan commented Mar 14, 2024

I noticed that the absolute symlink did not work on my system.
While investigating I noticed that on SUSE the symlink gets relinked
to be relative to be relative to the install location of the symlink
after which the link works.
Using a relative link seems safer to me.

@anatol
Copy link
Collaborator

anatol commented Mar 20, 2024

Your change contains 2 set of modifications - cmake file cleanup, and the relative symlink change itself.

I noticed that the absolute symlink did not work on my system.

Once you apply the cmake cleanup part, what is the target's absolute path for that symlink? I wonder if your cleanup part is enough to fix the issue.

@Thaodan
Copy link
Contributor Author

Thaodan commented Mar 22, 2024

Your change contains 2 set of modifications - cmake file cleanup, and the relative symlink change itself.

I can separate those changes.

I noticed that the absolute symlink did not work on my system.

Once you apply the cmake cleanup part, what is the target's absolute path for that symlink? I wonder if your cleanup part is enough to fix the issue.

RPM does like not like relative symlinks as they relative to the host environment and not to the builder which breaks for example the execution inside the built process.

To check if the script still works we start it once which doesn't work with an absolute link.

rpm-software-management/rpm#668

@fredldotme
Copy link

Something like this would be welcomed for the Snap release. Right now I'm replacing the symlink in a post-installation step during packaging.

@anatol
Copy link
Collaborator

anatol commented Jun 18, 2024

@Thaodan could you please split your change into multiple commit per its functionality: one commit for the symlink change, and another commit for the cleanup?

@anatol
Copy link
Collaborator

anatol commented Jul 4, 2024

@Biswa96 the PR is ready for merge. Could you please take a look?

…ative bin

Now the bin directory can be changed using standard cmake means.

Signed-off-by: Björn Bidar <[email protected]>
I noticed that the absolute symlink did not work on my system when
trying to execute the script inside it's installation directory before
the package is installed.
While investigating I noticed that on SUSE the symlink gets relinked
to be relative to be relative to the install location of the symlink
after which the link works.
Using a relative link seems safer to me.

Signed-off-by: Björn Bidar <[email protected]>
@Biswa96 Biswa96 merged commit 44c9dc3 into nmeum:master Jul 6, 2024
14 checks passed
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