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

Update ns-3-dev to latest version and remove WAF build system #34084

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

alalazo
Copy link
Member

@alalazo alalazo commented Nov 23, 2022

Since #33898 has been merged we have only a single package in the entire builtin repository using the WAF build system, and I don't think we foresee getting more soon.

Sometimes ago we talked with @adamjstewart about removing it, so here's the PR.

The ns-3-dev package has been ported to CMake and the latest version builds, at least on:

  • Spack: 0.20.0.dev0 (a3b6f9b)
  • Python: 3.8.10
  • Platform: linux-ubuntu20.04-icelake
  • Concretizer: clingo

Pinging @adamjstewart and @chissg as people who contributed to the build system, and @yee29 as the maintainer of the ns-3-dev package. If need be I can have a look at recovering old versions as generic packages.

Modifications:

  • Port ns-3-dev to CMake and update to latest version
  • Remove all previous versions to ns-3-dev
  • Remove the WAF build system

There is only a single package that was using WAF, and that package
moved to CMake in newer versions. Since WAF is not widely used, we can
remove it from core Spack for the time being.
@spackbot-app spackbot-app bot added commands core PR affects Spack core functionality dependencies documentation new-version tests General test capability(ies) update-package labels Nov 23, 2022
@spackbot-app
Copy link

spackbot-app bot commented Nov 23, 2022

@yee29 can you review this PR?

This PR modifies the following package(s), for which you are listed as a maintainer:

  • ns-3-dev

@gartung
Copy link
Member

gartung commented Nov 23, 2022

@brettviren You were the primary author of WafPackage. What do you think?

@greenc-FNAL
Copy link
Member

greenc-FNAL commented Nov 23, 2022

@alalazo @tgamblin @adamjstewart @gartung @brettviren @drbenmorgan I have no objection to replacing use of WafPackage in the builtin recipes. However I do know that an HEP collaborator who has been (maybe still is, I don't know) that collaboration's release manager is also the author of the Waf build system, and that there may be many packages in DUNE-specific repositories that not only use WafPackage in their Spack recipes, but actually have Waf as their native build system.

Since Spack is not AFAIK yet modular enough to enable plug-in build systems in the same way it offers plug-in commands, may I suggest that you contact @brettviren about being the maintainer of lib/spack/spack/build_systems/waf.py going forward, and leave it at that?

@brettviren
Copy link
Member

Hi everyone. I don't think I ever touched WafPackage but fwiw, I'm fine with it being removed. Thanks for asking!

I do use Spack to build a waf-built package but that actually calls waf (and actually a custom waf bundle) via Spack's bash support. For reference, that's here https://github.com/WireCell/wire-cell-spack/blob/master/packages/wire-cell-toolkit/package.py

@gartung
Copy link
Member

gartung commented Nov 24, 2022

Thanks for responding @brettviren . Now that I look at the file history I see that @adamjstewart is the primary author.

@adamjstewart
Copy link
Member

Yes, I wrote most of the build systems we support. I have no preference on keeping or dropping it, so if there is any desire or intention of adding new packages that use Waf to build, speak now or forever hold your peace.

@greenc-FNAL
Copy link
Member

greenc-FNAL commented Nov 28, 2022

Given the existence in the wild of software using Waf natively (even if their recipes don't currently use WafPackage), I'd be more comfortable if WafPackage continued to exist rather than contend with the possibility of future packages reimplementing the wheel via the base Package class. @adamjstewart if you wish, I can be responsible for the maintenance of waf.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants