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

lightningd: fix up installs in subdirectories. #7618

Merged

Conversation

rustyrussell
Copy link
Contributor

Commit a1fdeee "Makefile: clean up install path handling." broke the ability to configure with one path and then run in a different path. Turns out people actually do this! So, we have to use relative paths, compared to our existing binary.

Fixes: #7595

⚠️ IMPORTANT:

Feature freeze date for v24.08 is Aug 5th.

RC1 is scheduled on Aug 10th, RC2 on Aug 15, ...

The final release is on Aug 22nd.

Pull Request Title

Description

Please provide a brief summary of the changes made in this PR. What does it do, and why is it necessary?

Related Issues

List any related issues, bugs, or feature requests that are being addressed by this PR. Link them using GitHub's closing syntax (e.g., Closes #123).

  • Closes #issue_number

Changes Made

  • Feature: Brief description of the new feature or functionality added.
  • Bug Fix: Brief description of the bug fixed and how it was resolved.
  • Refactor: Any code improvements or refactoring done without changing the functionality.

Checklist

Ensure the following tasks are completed before submitting the PR:

  • Changelog has been added in relevant commit/s.
  • Tests have been added or updated to cover the changes.
  • Documentation has been updated as needed.
  • Any relevant comments or TODOs have been addressed or removed.

Additional Notes

Any additional information or context about this PR that reviewers should know.

@rustyrussell rustyrussell added this to the v24.08 milestone Aug 27, 2024
rel = path_rel(NULL, BINDIR, PKGLIBEXECDIR);
ld->subdaemon_dir = path_join(ld, my_path, take(rel));

rel = path_rel(NULL, BINDIR, PLUGINDIR);
Copy link
Collaborator

@Lagrang3 Lagrang3 Aug 28, 2024

Choose a reason for hiding this comment

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

This works as long as the BINDIR and PLUGINDIR exist. Otherwise I get rel==NULL here.
Is realpath that checks if the path exists. Using path_simplify instead of path_canon inside path_rel would fix it, but that would not resolve symlinks, right?

Alternatively one could hardcode the relative path: ../libexec/c-lightning for subdaemons and ../libexec/c-lightning/plugins for plugins.

@rustyrussell rustyrussell force-pushed the handle-relative-installs branch 2 times, most recently from 895052d to d7eb717 Compare August 28, 2024 20:29
@ShahanaFarooqui ShahanaFarooqui force-pushed the handle-relative-installs branch from d7eb717 to e59d980 Compare August 28, 2024 20:45
Commit a1fdeee "Makefile: clean up install path handling."
broke the ability to configure with one path and then run in a
different path.  Turns out people actually do this!  So, we have
to use relative paths, compared to our existing binary.

And we can't use path_rel, because that requires that the path
exist (thanks @Lagrang3!).

Fixes: ElementsProject#7595
Signed-off-by: Rusty Russell <[email protected]>
@ShahanaFarooqui ShahanaFarooqui force-pushed the handle-relative-installs branch from e59d980 to a0f7225 Compare August 28, 2024 20:48
ShahanaFarooqui added a commit to ShahanaFarooqui/lightning that referenced this pull request Aug 28, 2024
With PR ElementsProject#7618, Core Lightning installation with relative paths has been fixed and can be used again.

Changelog-None.
@ShahanaFarooqui
Copy link
Collaborator

ACK a0f7225

@rustyrussell rustyrussell merged commit 624d8b7 into ElementsProject:master Aug 28, 2024
38 checks passed
ShahanaFarooqui added a commit to ShahanaFarooqui/lightning that referenced this pull request Aug 28, 2024
With PR ElementsProject#7618, Core Lightning installation with relative paths has been fixed and can be used again.

Changelog-None.
ShahanaFarooqui added a commit that referenced this pull request Aug 29, 2024
With PR #7618, Core Lightning installation with relative paths has been fixed and can be used again.

Changelog-None.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants