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

Separate build and run environments using dev variant #294

Merged
merged 10 commits into from
Feb 5, 2025

Conversation

andrewmogan
Copy link
Contributor

This PR depends on daq-release PR #119 which introduces the dev variant of the externals, coredaq, and fddaq umbrella packages. With these changes, dbt-create will create a build environment which includes cmake, gdb, and ninja, while dbt-setup-release will excludes those spack packages. This can be tested using the nightly NFDU_DEV_241218_A9, which was built using the daq-release branch amogan/issue134_build_vs_run_environments which installs both the +dev and ~dev variants. I've confirmed locally that which cmake returns the system-level cmake when using this version of dbt-setup-release, while creating a local area with dbt-create and sourcing the local env.sh will pull in the spack cmake used by externals.

@jcfreeman2
Copy link
Collaborator

Thanks for this. I've checked that it works as advertised with dbt-setup-release and dbt-create, but two comments:

  1. dbt-setup-release.sh has variant="~~dev" rather than variant="~dev", curious what the reason for this is?
  2. When we start cutting nightlies where fddaq has a +dev and a ~dev variant, we'll need to use an updated daq-buildtools which includes the changes on this branch. However, we'll also want it to be backwards compatible. I.e., we'll want this to work:
setup_dbt latest_v5   # Brings in a version of daq-buildtools which includes the changes on this branch
dbt-setup-release <some frozen release from the pre-separate-environment-era>

...which means daq-buildtools will need to account for cases where fddaq doesn't have a dev variant.

Use coredaq instead of $spack_pkgname to check for the existence of the
dev variant because $spack_pkgname might be "systems", which will never
have that variant.
@andrewmogan
Copy link
Contributor Author

  1. The original reasoning behind ~~dev was to propagate the ~dev variant to package dependencies, but since we have the dev variant defined explicitly in each package that needs it, that's not really necessary. There's also the possibility that ~ gets expanded to $HOME, but that's easy enough to work around (just use package~dev instead of package ~dev. I've switched to using ~dev since it's a little more readable and doesn't impact the intended functionality.
  2. I've added a check where, if coredaq has a dev variant, use it. Otherwise, don't use a variant. The choice coredaq is somewhat arbitrary in that it just needs to be a non-systems package, since systems doesn't have the dev variant. In particular, checking for the variant using $spack_pkgname didn't work.

jcfreeman2 pushed a commit to DUNE-DAQ/daq-release that referenced this pull request Feb 4, 2025
Copy link
Collaborator

@jcfreeman2 jcfreeman2 left a comment

Choose a reason for hiding this comment

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

I ran a regression test checking that using this feature branch of daq-buildtools, it still works fine with a "traditional" work area, i.e., one where fddaq doesn't have a dev variant. It did: https://github.com/DUNE-DAQ/daq-release/actions/runs/13141994553/workflow

Then yesterday I built a test nightly which used Andrew's feature branch in daq-release and which installs fddaq+dev and fddaq~dev. I ran the suite of daq-buildtools tests on that test nightly, and again things looked fine: https://github.com/DUNE-DAQ/daq-release/actions/runs/13160749541

daq-buildtools can handle the dev variants of fddaq, and is also backwards compatible. Approved.

@andrewmogan andrewmogan merged commit f34b889 into develop Feb 5, 2025
@andrewmogan andrewmogan deleted the amogan/daq_release_issue134 branch February 5, 2025 15:48
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.

2 participants