encode development requirements in pyproject.toml #32616

trws · 2022-09-12T17:51:27Z

Add a project block to the toml config along with development and CI dependencies and a minimal build-system block, doing basically nothing, so that spack can be bootstrapped to a full development environment with:

$ hatch shell

or for a minimal environment without hatch:

$ python3 -m venv venv
$ source venv/bin/activate
$ python3 -m pip install --upgrade pip
$ python3 -m pip install -e '.[dev]'

This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in one place rather than several disparate ones. We may be stuck with a couple more temporarily to continue supporting python2.7, but aside from that it's less places to get out of sync and a couple new bootstrap options.

adamjstewart

Excited to some day be able to tell people to pip install spack.

adamjstewart · 2022-09-12T17:55:14Z

pyproject.toml

+requires = ["hatchling"]
+build-backend = "hatchling.build"


Why hatchling?

Honestly, because I couldn't figure out how to get setuptools to just let me do an editable install without erroring out or declaring explicit package parameters. Poetry, which is my usual weapon of choice, seems to require a good bit of extra boilerplate. This version made it "just work" so the dependencies will install with pip or hatch, or any pep-517/660 compliant tool presumably, out of the box. The hatch env stuff I just find really handy, honestly, but that isn't tied to using hatchling like this, if you know a better way to get pip install with and without -e working to set this up I'm happy to explore alternatives.

What version of setuptools were you using? I know there was a recent refactor in v64 that added support for editable installs using pyproject.toml. I have no personal preference on build backend, just curious about the decision since I'm never sure which backend I should be using in my own projects.

Maintainer here! Hatchling is also now used as the default in the official Python packaging tutorial.

@ofek do you know how the default was chosen? I haven't yet seen an official comparison between backends and PyPA refuses to admit that "default" == "recommended".

@pradyunsg it's a bit of a grey area IMO, it's things like package files, which are technically code but are most definitely not libraries or things meant to be run by users, configuration files, etc.

I'm pretty sure that you can remove the bin/spack file, since it's being replaced by the script that will be generated for project.scripts declarations. FWIW, those are what the console_scripts entry points from setuptools map to -- I'm quite pleased that we have a friendlier user-facing name for the concept.

I think it's a bit of a gray area. We would like to be able to run Spack just by cloning and running ./spack/bin/spack, but we would also like to be able to install with pip. I think the former use case will actually be more common that the latter, so we can't just get rid of the script.

@pradyunsg Yeah, it wouldn't be installed by pip, but we need it there for the "clone and go" use-case.

On installing, the shared-data option definitely works, but splitting our directories up breaks all the relative pathing stuff so things break, really badly. I did manage to get install to produce a console_script that works, and install all the directories under site-packages/spack_root though rather than splatting them all over the place, and the resulting install can install packages and generally kinda just works.

In thar case, I suggest removing it from the list of files being included in the wheel (python -m build && unzip -l dist/*.whl shouldn't list any sort of .scripts/spack since that would be overwritten by a launcher script that pip generates instead.)

pyproject.toml

trws · 2022-09-14T22:10:16Z

@adamjstewart, this update makes the pip installed spack actually work. It also reads the spack version from the canonical spack location, which unfortunately required a bit of reworking there because the pip mechanism for it uses a regex, and the original tuple isn't compatible with that.

Only the spack command is installed at the moment, but it's hooked in such that it will use the python managed by a venv or similar, and refactors the setup code so that both bin/spack and the pip generated script will use the same path setup logic. It's kinda strange, but it seems to work. I'm not sure I like using the externals like this, they really should just be pulled from packages for this probably, but this is more like how spack works when cloned so I went with it for now.

adamjstewart · 2022-09-14T22:24:34Z

This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in one place rather than several disparate ones.

Given that Spack's deps are still listed in all of the same places, this kinda feels like: https://xkcd.com/927/

trws · 2022-09-14T22:27:34Z

This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in one place rather than several disparate ones.

Given that Spack's deps are still listed in all of the same places, this kinda feels like: https://xkcd.com/927/

Sadly this is true, we can replace the pip commands in the workflows to reuse this at least, but it doesn't necessarily get all of them (because I'm not sure a new enough pip can run on python2.7).

adamjstewart · 2022-09-14T22:28:35Z

There's also var/spack/repos/builtin/packages/spack/package.py lol

trws · 2022-09-14T22:34:05Z

🤦

If someone were sufficiently motivated, we could write a shim build-system that actually uses spack to generate the dependency list for pip from that file, make wheels, etc. I'm not sure if it's worth it, but we could. Alternately, we could have that package reworked to have spack use pip to install spack... 🤔

At least this way we can easily generate a local venv or similar with the same deps that are used in CI, and not have to re-list the stuff in mac, and linux, and windows and so-forth. It's not everything, but it makes bootstrapping to style with pip less frustrating.

adamjstewart · 2022-09-14T22:36:32Z

Alternately, we could have that package reworked to have spack use pip to install spack

We would still need to list the deps in the package though.

trws · 2022-09-21T00:36:08Z

Ok, I've actually gone through and replaced all the pip install commands in our workflows with the exact same string, installing using the deps listed in pyproject.toml. With conditional dependencies that includes python2.7 and windows. Assuming this works, pretty happy with how many of those just went away.

adamjstewart · 2022-09-21T14:43:03Z

.github/workflows/audit.yaml

@@ -25,7 +25,8 @@ jobs:
        python-version: ${{inputs.python_version}}
    - name: Install Python packages
      run: |
-        pip install --upgrade pip six setuptools pytest codecov 'coverage[toml]<=6.2'
+        pip install --upgrade pip
+        pip install '.[dev,ci]' # not using -e because of python 2.7 support


Is there any reason to use -e even for Python 3? It's all CI so why install in editable mode?

coverage output is readable by default without rewrite config

Yeah, really it's to make sure that we don't accidentally run spack from the install location, for reasons like @ofek mentions. We point to the in-tree version anyway, so it should be fine, but it's one less source of confusion. If there were a way to only install the dependencies rather than the dependencies and spack that's what I would want to use, but it seems like that isn't a supported use-case in pip.

ofek · 2022-10-04T17:20:21Z

CI flake?

trws · 2022-11-01T16:10:24Z

So... I would really like to merge this, so it doesn't atrophy. Is there a remaining blocker or concern here @adamjstewart, @tgamblin? I'm rebasing to resolve the workflows right now.

Add a `project` block to the toml config along with development and CI dependencies and a minimal `build-system` block, doing basically nothing, so that spack can be bootstrapped to a full development environment with: ```shell $ hatch -e dev shell ``` or for a minimal environment without hatch: ```shell $ python3 -m venv venv $ source venv/bin/activate $ python3 -m pip install --upgrade pip $ python3 -m pip install -e '.[dev]' ``` This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in *one place* rather than several disparate ones. We may be stuck with a couple more temporarily to continue supporting python2.7, but aside from that it's less places to get out of sync and a couple new bootstrap options.

Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>

trws · 2022-11-05T22:37:04Z

Rebased again, from the pool. Any chance of a review? This is verified to work with hatch, pip, and with a bit of adaptor code nix as well. Could certainly do with more helper commands and some thought on actually using it as an installed package, but everything is functional.

Add a `project` block to the toml config along with development and CI dependencies and a minimal `build-system` block, doing basically nothing, so that spack can be bootstrapped to a full development environment with: ```shell $ hatch -e dev shell ``` or for a minimal environment without hatch: ```shell $ python3 -m venv venv $ source venv/bin/activate $ python3 -m pip install --upgrade pip $ python3 -m pip install -e '.[dev]' ``` This means we can re-use the requirements list throughout the workflow yaml files and otherwise maintain this list in *one place* rather than several disparate ones. We may be stuck with a couple more temporarily to continue supporting python2.7, but aside from that it's less places to get out of sync and a couple new bootstrap options. Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>

trws requested review from tgamblin and adamjstewart September 12, 2022 17:51

spackbot-app bot added the core PR affects Spack core functionality label Sep 12, 2022

adamjstewart reviewed Sep 12, 2022

View reviewed changes

tldahlgren assigned adamjstewart Sep 12, 2022

trws force-pushed the pyproject-project branch from 5e1ab13 to 11181ee Compare September 14, 2022 22:08

trws force-pushed the pyproject-project branch from 11181ee to 96356c7 Compare September 14, 2022 22:12

spackbot-app bot added the workflow label Sep 21, 2022

trws force-pushed the pyproject-project branch from ca07986 to 16b3194 Compare September 21, 2022 00:08

adamjstewart reviewed Sep 21, 2022

View reviewed changes

trws force-pushed the pyproject-project branch 2 times, most recently from a3a3d05 to afd0e88 Compare November 2, 2022 16:06

trws and others added 5 commits November 5, 2022 12:33

Update pyproject.toml version

33af64c

Co-authored-by: Adam J. Stewart <ajstewart426@gmail.com>

read spack version from __init__.py

70efe82

reorganize sys.path setup so pip installed spack can be run

28c6109

avoid detached env

c93a0ff

trws force-pushed the pyproject-project branch from afd0e88 to c93a0ff Compare November 5, 2022 22:33

tgamblin approved these changes Nov 7, 2022

View reviewed changes

tgamblin merged commit 6241cdb into spack:develop Nov 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

encode development requirements in pyproject.toml #32616

encode development requirements in pyproject.toml #32616

trws commented Sep 12, 2022 •

edited

Loading

adamjstewart left a comment

adamjstewart Sep 12, 2022

trws Sep 12, 2022

adamjstewart Sep 12, 2022

ofek Sep 13, 2022

adamjstewart Sep 13, 2022

trws Sep 21, 2022

pradyunsg Sep 21, 2022

adamjstewart Sep 21, 2022

trws Sep 21, 2022

pradyunsg Sep 21, 2022 •

edited

Loading

trws commented Sep 14, 2022

adamjstewart commented Sep 14, 2022

trws commented Sep 14, 2022

adamjstewart commented Sep 14, 2022

trws commented Sep 14, 2022

adamjstewart commented Sep 14, 2022

trws commented Sep 21, 2022

adamjstewart Sep 21, 2022

ofek Sep 21, 2022

trws Sep 21, 2022

ofek commented Oct 4, 2022

trws commented Nov 1, 2022

trws commented Nov 5, 2022

		requires = ["hatchling"]
		build-backend = "hatchling.build"

encode development requirements in pyproject.toml #32616

encode development requirements in pyproject.toml #32616

Conversation

trws commented Sep 12, 2022 • edited Loading

adamjstewart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pradyunsg Sep 21, 2022 • edited Loading

Choose a reason for hiding this comment

trws commented Sep 14, 2022

adamjstewart commented Sep 14, 2022

trws commented Sep 14, 2022

adamjstewart commented Sep 14, 2022

trws commented Sep 14, 2022

adamjstewart commented Sep 14, 2022

trws commented Sep 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ofek commented Oct 4, 2022

trws commented Nov 1, 2022

trws commented Nov 5, 2022

trws commented Sep 12, 2022 •

edited

Loading

pradyunsg Sep 21, 2022 •

edited

Loading