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

Several shell scripts are installed into $PREFIX #150

Closed
yurivict opened this issue Nov 7, 2023 · 5 comments
Closed

Several shell scripts are installed into $PREFIX #150

yurivict opened this issue Nov 7, 2023 · 5 comments

Comments

@yurivict
Copy link

yurivict commented Nov 7, 2023

These files are installed into $PREFIX:

crystal_setup.sh
ncrystal_unsetup.sh
setup.sh
unsetup.sh

They should be installed either into $PREFIX/bin or into $PREFIX/share/NCrystal.

@yurivict yurivict added the bug label Nov 7, 2023
@tkittel
Copy link
Member

tkittel commented Nov 8, 2023

Thanks for the report. To better understand your issue, it would be helpful if you could explain a bit about the context in which you are using and trying to install NCrystal (from your github account name I am guessing... FreeBSD packaging?). Also, I am interested if you have some sort of reference explaining why installation of source-able activation scripts into $PREFIX/bin should be preferred over $PREFIX (I am not disagreeing, I am just curious if this is just based on a gut feeling or if there is actually some sort of formal linux/unix/cmake/... document with words on this subject?

In any case, note that if you set the CMake option -DNCRYSTAL_ENABLE_SETUPSH=OFF, these files will not be produced at all, which we for instance use when packaging NCrystal (as we do for e.g. conda and PyPI - I am not 100% sure about the ). So depending on the context of your usage of NCrystal, you might want to specifiy this option as well. These activation scripts exists primarily for historical reasons, to let users of the (then young) project activate a custom installation into a non-system environment, and are needed less and less now that in recent years NCrystal is available on PyPI and conda-forge. However, since many expert users now expect these scripts, the default value of NCRYSTAL_ENABLE_SETUPSH remains at ON for now. Recently we also realised that the names setup.sh and unsetup.sh are not great names, as they will have a high likelihood of clashing with files for other projects, so we decided to migrate to ncrystal_setup.sh and ncrystal_unsetup.sh. However, the plan was to still produce the setup.sh/unsetup.sh files for a while, to provide a more comfortable migration path for downstream users.

Cheers,
Thomas

@yurivict
Copy link
Author

yurivict commented Nov 8, 2023

Hi @tkittel ,

Yes, I am working in the context of the FreeBSD port/package.
Shell files shouldn't be installed into $PREFIX because of (1) convention and (2) potential conflicts that such files might cause. It's just not common in the linux/BSD world to put files directly into $PREFIX.

Yuri

@tkittel
Copy link
Member

tkittel commented Nov 8, 2023

Ok, that seems reasonable enough, and I am happy to eventually move to a situation where we only install these activation scripts into $PREFIX/bin (and only files named ncrystal_[un]setup.sh no clashy [un]setup.sh). I am also happy to eventually have NCRYSTAL_ENABLE_SETUPSH default to OFF.

However, to play nice with current users, I want to apply these changes slowly over a longer period of time, initially adding the files in the new location, but not removing the old ones by default. So I suggest you simply hardwire -DNCRYSTAL_ENABLE_SETUPSH=OFF in your packaging recipe - such activation scripts are anyway not intended for usage when distributing NCrystal via a proper packaging mechanism.

@tkittel
Copy link
Member

tkittel commented Dec 3, 2023

In upcoming v3.8.0 I am setting NCRYSTAL_ENABLE_SETUPSH=OFF by default. Additionally, the <prefix>/setup.sh file is removed in any case, leaving only <prefix>/ncrystal_setup.sh (same for unsetup.sh). I am considering this enough to resolve the current issue, since only very few expert users are expected to still use the NCRYSTAL_ENABLE_SETUP option.

@tkittel
Copy link
Member

tkittel commented Jan 18, 2024

v3.8 was released in December, closing this.

@tkittel tkittel closed this as completed Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants