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

Non-standard installation of plugins to <prefix>/plugins may lead to conflicts #136

Open
wdconinc opened this issue Aug 7, 2022 · 7 comments

Comments

@wdconinc
Copy link
Member

wdconinc commented Aug 7, 2022

Currently JANA2 installs plugins into the <prefix>/plugins directory. This directory that could resolve (for system-wide installs) to e.g. /usr/plugins which is non-standard and can lead to conflicts when generic names such as dd4hep.so, podio.so or tutorial.so are used, where the full path does not include any reference to JANA2. (Naming libraries as <name>.so without the lib prefix appears to be acceptable in dedicated plugin directories.) This affects downstream projects such as EICrecon as well and prevents multiple independent downstream projects from installing identically-named plugins into the same prefix. This is particularly relevant for environments like eic-shell which uses spack views to install into /usr/local and will fail upon collisions.

A brief search indicates that the following locations are used by other packages:

  • <prefix>/lib/<packagename>/plugins/<pluginname>/*.so (vlc, qt5, qt6, rpm-utils, meshlab, krb5)
  • <prefix>/lib/<packagename>[/<version>]/plugins/*.so (thunderbird, uniquity)

There are a few other files which may lead to collisions as well:

  • <prefix>/include/external/catch.hpp (should not be installed)
  • <prefix>/include/regressiontest (should be under a <packagename> directory in include)
@faustus123
Copy link
Collaborator

We'll look into this. Historically, it was never a good idea to install this type of software in /usr or even /usr/local. However, with containerization, it seems much less problematic.

@wdconinc
Copy link
Member Author

wdconinc commented Aug 8, 2022

it was never a good idea

And why not, actually? Generally this has been avoided because it requires adhering to some standards, being somewhat organized, and figuring out some things in the interest of user experience. For as long as have been involved in software development at JLab I have found it very useful to make sure everything can be installed in /usr/local, and that users don't need to do anything beyond running the command that lives in /usr/local/bin. That includes making sure that that software finds resources in /usr/local/share etc, or in ~/.local or ./ if available. When we have been doing things that no other linux program does, I like to think about what the reasons are, and what we lose by doing what we do.

@faustus123
Copy link
Collaborator

The main issues are:

  1. A very large fraction of the work is done on central machines where users cannot write to /usr/ or /usr/local
  2. It is important to maintain multiple versions of the software and writing to /usr/local/bin does not support this

The /usr and /usr/local system is really designed for user-level software, not developer environments. The software in our field has been largely needed as a developer environment. Even with ROOT on my laptop, I have several versions simultaneously which simply would not work if they were all trying to install in /usr/local

Having said that, containers are a different story. It is effectively like having many different computers, each having to support only a single set of software versions. There, the single directory tree installation structure is much more reasonable.

@nathanwbrei
Copy link
Collaborator

I definitely don't recommend people do system installs of JANA, (or any scientific software for that matter, particularly any scientific software that has transitive dependencies on both Python and ROOT... In fact I prefer not linking against any system libraries in most cases.) If the user doesn't provide a CMAKE_INSTALL_PREFIX or a JANA_HOME environment variable, JANA's build system will default to installing it in CMAKE_BUILD_DIR/install rather than /usr/local.

That being said, we don't disallow users from installing in /usr/local if that's what they really want to do. However they have to say so explicitly. I think that installing plugins in <prefix>/plugins/JANA/ to avoid conflicts is a good idea. It's easy to add that to the default plugin search path. The main downside I see is that changing the default install location would be a breaking change, so we should hold off on it for a bit so as not to disrupt everyone's EICrecon work.

@wdconinc
Copy link
Member Author

wdconinc commented Aug 9, 2022

I install all scientific software into spack environment views equivalent to /usr/local (with maximal rpath linking and any linking against system libraries limited to libc). That approach avoids your concerns (and is used widely in HPC environments or e.g. CERN LCG views), yet also leads to conflicts here.

You have to stop thinking that JANA2 is something each user will install themselves; they won't (regardless of containers or cvmfs or central system install on a JLab group disk). There will be system administrators and software librarians who will have to roll this out for others. Installing stacks of software in a single prefix is very common, and /usr/local is just a shorthand for that because the prefix will usually be very different. But it behaves like /usr/local.

@faustus123
Copy link
Collaborator

I'm pretty sure we arguing completely different things at this point. Re-reading Wouter's original post, I believe his concern is regarding the directory structure under being compatible with other packages that are also installing under . The /usr/local business is just a red herring.

The main requests here are:

  1. plugins should install to <prefix>/lib/JANA/plugins instead of just <prefix>/plugins
  2. files like catch.hpp should not be installed, but more generally, any headers JANA2 installs should be under <prefix>/include/JANA. This means they are all kept in a JANA-specific part of the directory tree. This will, BTW affect the header files for all plugins which are currently installed in directories named after the plugins at the <prefix>/include level. The main drawback there is that a users incorporating headers from installed plugins, would need to do:
#include "JANA/<plugin_name>/file.h"

instead of

#include "<plugin_name>/file.h"

where the former is a little misleading since that file does not exist under the JANA directory in the source tree, Alternatively, we could install the directories for all plugin headers under <prefix>/include/JANA/plugins and just have user code add that to the search path in addition to <prefix>/include. Then they could do:

#include "JANA/JEventProcessor.h"
#include "<plugin_name>/file.h"

I'm not 100% sure the best way to handle this, but I do agree tightening thing up so JANA can play nicely with other packages installed in the same <prefix> will be a good thing.

github-merge-queue bot pushed a commit to eic/EICrecon that referenced this issue Sep 11, 2023
…942)

### Briefly, what does this PR introduce?
`JANA_PLUGIN_PATH` in eic-shell (and therefore CI environments) is
prepopulated with `/usr/local/lib/EICrecon/plugins:/usr/local/plugins`.
This means that removing (intentionally or otherwise) a plugin in a PR
will still cause it to be found in `/usr/local/lib/EICrecon/plugins`. We
need `/usr/local/plugins` though, because that contains the `janadot`
plugin to create callgraphs.

There's an issue reported to JANA2
(JeffersonLab/JANA2#136) that points out that
installing plugins in a shared namespace at `/usr/local/plugins` may
lead to issues and does not conform to the linux FHS. But at least
EICrecon plugins are not installed there, otherwise we would have
exactly this issue.

### What kind of change does this PR introduce?
- [x] Bug fix (issues, potentially)
- [ ] New feature (issue #__)
- [ ] Documentation update
- [ ] Other: __

### Please check if this PR fulfills the following:
- [ ] Tests for the changes have been added
- [ ] Documentation has been added / updated
- [ ] Changes have been communicated to collaborators

### Does this PR introduce breaking changes? What changes might users
need to make to their code?
No.

### Does this PR change default behavior?
No.
nathanwbrei added a commit that referenced this issue Aug 2, 2024
- Plugin libraries install to lib/JANA/plugins/
- Plugin headers install to include/JANA/plugins/$PLUGIN_NAME
Addresses issue #136
@nathanwbrei
Copy link
Collaborator

Pretty much all of the discussion here is incorporated into PR #330. Several loose ends to consider:

  • The pybind11 bindings are installed to $PREFIX/python which I'm guessing should be lib/JANA/python/ instead.
  • Example executable are installed to $PREFIX/bin and example plugins are installed to $PREFIX/lib/JANA/plugins, which doesn't quite feel right. Other projects, e.g. Geant4, install their examples to $PREFIX/share/$NAMESPACE/examples. This seems more correct, but will not quite play nicely with the JANA_PLUGIN_PATH. I'm mulling over the possibility of having src/examples be its own separate CMake project, for which the user usually wouldn't do a system install at all. Right now there is no full working example of an "external" JANA project inside the JANA source tree, although the tutorial and jana-generate.py walk the user through creating one.
  • I'm not 100% happy with installing libJANA headers to $PREFIX/JANA and plugin headers to $PREFIX/JANA/plugins. I feel like a cleaner approach would be to make the install namespace be JANA2 instead, such that libJANA headers to $PREFIX/JANA2/JANA and plugin headers to $PREFIX/JANA2/plugins. Thanks to CMake targets, we can make this change without affecting EICrecon. (This might require a tweak to the GlueX port, I haven't checked yet)
  • I wish we had named the CMake project JANA2 instead of simply JANA. We might be able to transition over without breaking anyone by generating both JANAConfig.cmake and JANA2Config.cmake, and having the former redirect to the latter, though I haven't tested it and am not sure if it is worth the hassle.

@faustus123 @DraTeots

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

No branches or pull requests

3 participants