Skip to content
This repository has been archived by the owner on Oct 2, 2024. It is now read-only.

rewrite embedded paths at install time #1724

Merged
merged 8 commits into from
Sep 14, 2023
Merged

rewrite embedded paths at install time #1724

merged 8 commits into from
Sep 14, 2023

Conversation

reidpr
Copy link
Collaborator

@reidpr reidpr commented Sep 8, 2023

Closes #683.

This PR changes library paths in sh and Python code to be valid in the source directory and removes the lib/charliecloud symlink. It then re-writes these paths to be valid for the install location at install time.

@reidpr reidpr force-pushed the hardcoded-dirs_683 branch from a7c0441 to d1464b0 Compare September 8, 2023 22:50
@reidpr reidpr requested a review from lucaudill September 11, 2023 22:14
@reidpr
Copy link
Collaborator Author

reidpr commented Sep 11, 2023

@ana @olifre @lnussbaum @wiene : I'd love opinions on how distributions feel about this approach.

@olifre
Copy link
Contributor

olifre commented Sep 12, 2023

Since the critical part for distros is valid for the install location at install time, I don't see any problem with this approach. To confirm this, I also tried to run this branch through Gentoo's packaging and the package manager was happy (i.e. not automated flagging), so that seems good for Gentoo 👍 .

Copy link
Collaborator

@lucaudill lucaudill left a comment

Choose a reason for hiding this comment

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

LGTM

@wiene
Copy link
Contributor

wiene commented Sep 12, 2023

At present the situation for Debian is the following:
A Debian specific patch is applied to modify the path to the tests in ch-test (CHTEST_DIR).

I glanced through the changes. Unless I overlooked something, it seems that a patch is still needed after the modifications of PR have been applied. Removing the above mentioned patch and applying this PR, I get

CHTEST_DIR=/usr/libexec/installed-tests/charliecloud/test

in ch-test. This is closer than before, but not precisely what I am looking for:

CHTEST_DIR=/usr/libexec/installed-tests/charliecloud

Of course, it is nice to get rid of distribution specific patches. On the other hand, if this only affects Debian, it is also fine to keep the patch.

@e4t
Copy link

e4t commented Sep 13, 2023

@ana @olifre @lnussbaum @wiene : I'd love opinions on how distributions feel about this approach.

@ana asked me to respond for her:
SUSE/openSUSE uses libexec on the rolling release (openSUSE Tumbleweed) and any upcoming product (ALP etc), however, SLE and Leap still adhere to the somewhat older FHS 2.3 standard.
For this RPM-based Linux distros provide a set of RPM predefined macros (%_libexecdir) which are set to the path elements of the underlying system. These will be specified during configure (using a predefined %_configure macro for convenience). Thus, allowing these paths / path elements to be configurable allows distributions to set them to their needs without extensive patching.

@reidpr
Copy link
Collaborator Author

reidpr commented Sep 13, 2023

Thanks folks.

@wiene I believe the latest version of this PR will let you eliminate your patch.

@reidpr reidpr requested a review from lucaudill September 14, 2023 16:03
Copy link
Collaborator

@lucaudill lucaudill left a comment

Choose a reason for hiding this comment

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

LGTM

@reidpr reidpr merged commit e560f0b into master Sep 14, 2023
6 checks passed
@reidpr reidpr deleted the hardcoded-dirs_683 branch September 14, 2023 23:18
@wiene
Copy link
Contributor

wiene commented Sep 24, 2023

@wiene I believe the latest version of this PR will let you eliminate your patch.

I can confirm that the patch can be removed thanks to this PR. Many thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

don't hard-code path to Python and sh libraries
5 participants