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

treat plugins as code dependencies #399

Merged
merged 1 commit into from
Jul 26, 2024
Merged

Conversation

rbberger
Copy link
Collaborator

@rbberger rbberger commented Jul 25, 2024

PR Summary

This simplifies the plugin spack situation by just treating plugins like any other code dependencies.

Each plugin just has to install its source code into the spack prefix and the singularity-eos plugin needs to be registered using its name, spack package name, and the relative path to the plugins source code.

PR Checklist

  • Adds a test for any bugs fixed. Adds tests for new features.
  • Format your changes by using the make format command after configuring with cmake.
  • Document any new features, update documentation for changes made.
  • Make sure the copyright notice on any files you modified is up to date.
  • After creating a pull request, note it in the CHANGELOG.md file.
  • LANL employees: make sure tests pass both on the github CI and on the Darwin CI

If preparing for a new release, in addition please check the following:

  • Update the version in cmake.
  • Move the changes in the CHANGELOG.md file under a new header for the new release, and reset the categories.

@jhp-lanl
Copy link
Collaborator

@Yurlungur what are you thoughts? I think this will simplify things, but I admit I'll need to see a full release process of a plugin to really understand the details. But based off of our other conversations, I think this is a good route to go in.

@Yurlungur
Copy link
Collaborator

I think I'm supportive of this design. @rbberger when resolving a spack spec, how does this work? Does the ordering get swapped so the plugin is a dependency of singularity?

@rbberger
Copy link
Collaborator Author

Right. Plugin spackages need to be made build dependencies and registered as plugins.

@rbberger rbberger force-pushed the rbberger_simpler_plugins branch from e8000ec to dab5840 Compare July 26, 2024 02:55
@rbberger rbberger changed the title treat plugins just like header-only dependencies treat plugins as code dependencies Jul 26, 2024
@rbberger
Copy link
Collaborator Author

rbberger commented Jul 26, 2024

@Yurlungur found one more issue due to the variant, which was a directory into the source tree. Since it now no longer needs to be in one folder, it'll convert plugin/some/path to {plugin.prefix}/some/path.

@Yurlungur
Copy link
Collaborator

Right. Plugin spackages need to be made build dependencies and registered as plugins.

Does that mean we need to specify all available plugins in the spackage? Or can this happen "on the fly"? I.e., does this make the plugin infrastructure only useful for us? Or can others leverage it still?

@rbberger
Copy link
Collaborator Author

It means that plugins can be added by manipulating the singularity-eos package since they need to become a build dependencies. Either directly or via inheritance.

@rbberger
Copy link
Collaborator Author

So if there are other plugins out there, in order to use it'll have to be registered in our spackage.

@Yurlungur
Copy link
Collaborator

Ok I think this is fine. But it is a downside since it means using a plugin with spack requires more "manual" intervention. But maybe that's unavoidable.

Copy link
Collaborator

@Yurlungur Yurlungur left a comment

Choose a reason for hiding this comment

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

Merge when ready.

@rbberger rbberger merged commit 864c674 into main Jul 26, 2024
5 checks passed
@rbberger rbberger deleted the rbberger_simpler_plugins branch July 26, 2024 17:22
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.

3 participants