-
Notifications
You must be signed in to change notification settings - Fork 7
eic-opticks: add Spack package recipe #824
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| # Copyright Spack Project Developers. See COPYRIGHT file for details. | ||
| # | ||
| # SPDX-License-Identifier: (Apache-2.0 OR MIT) | ||
|
|
||
| from spack_repo.builtin.build_systems.cmake import CMakePackage | ||
| from spack_repo.builtin.build_systems.cuda import CudaPackage | ||
|
|
||
| from spack.package import * | ||
|
|
||
|
|
||
| class EicOpticks(CMakePackage, CudaPackage): | ||
| """GPU-Accelerated Optical Photon Simulation using NVIDIA OptiX""" | ||
|
|
||
| homepage = "https://github.com/bnlnpps/eic-opticks" | ||
| git = "https://github.com/bnlnpps/eic-opticks.git" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs to be on the EIC organization on GitHub to allow/encourage co-development by EIC community. This is also problematic since it is a fork of another project with changes applied in the original source tree. That's not a sustainable situation. Changes should be upstreamed and we should package opticks, or we should be able to add the EIC specific aspects without needing to pin a specific upstream commit. |
||
|
|
||
| license("Apache-2.0") | ||
|
|
||
| maintainers("plexoos") | ||
|
|
||
| version("main", branch="main") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs released versions. No moving targets as the only version we can install.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The release will come, but not now. I suggested we pin by hash for now. |
||
|
|
||
| depends_on("cxx", type="build") | ||
| depends_on("cmake@3.10:", type="build") | ||
|
|
||
| depends_on("cuda") | ||
| depends_on("geant4") | ||
| depends_on("glew") | ||
| depends_on("glfw") | ||
| depends_on("glm") | ||
| depends_on("glu") | ||
| depends_on("nlohmann-json") | ||
| depends_on("mesa") | ||
| depends_on("optix-dev") | ||
| depends_on("openssl") | ||
| depends_on("plog") | ||
| depends_on("python") | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The import statements for CMakePackage and CudaPackage are using an unusual pattern. In this repository, packages that are not extending builtin packages should import these classes via the wildcard import
from spack.package import *rather than explicitly importing fromspack_repo.builtin.build_systems.The explicit imports from
spack_repo.builtinare only used when extending existing builtin packages (as seen in packages like geant4, dd4hep, acts, etc.). Since eic-opticks is a new package that doesn't extend a builtin, it should follow the standard pattern used by other new packages in this repository.Remove lines 5-6 and rely on the wildcard import on line 8 to provide CMakePackage and CudaPackage, as demonstrated in packages/jana2/package.py which also uses CudaPackage.