Prepare for wheel publishing (alpha patch version of 0.0.1) #283
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Yes, I somehow thought that that is the default, but it's not. Will amend + force push/rebase. |
The answer is probably no: https://hatch.pypa.io/latest/plugins/builder/sdist/#default-file-selection
|
|
I was just going to paste that. Yeah, cannot be removed from sdist, but it is not in the wheel anyway. |
mihaimaruseac
force-pushed
the
packaging
branch
4 times, most recently
from
378c640
to
f3b0acb
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
mihaimaruseac
force-pushed
the
packaging
branch
from
f3b0acb
to
2bfd2ee
Compare
mihaimaruseac
force-pushed
the
packaging
branch
9 times, most recently
from
081c2e0
to
931a896
Compare
This commits moves files around to get to a `src/` layout (https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/#src-layout-vs-flat-layout) for the code. There are more benefits for a `src/` layout than a flat layout, such as: - documentation is easier to generate - we won't accidentally test against code that is different than what users would see - we can include only the source in the wheel, reducing PyPI bandwidth - it makes it clearer which parts of the repo go into the wheel and which ones are ancillary. Incidentally, we might now want to move the SLSA parts to another repo and maybe rename this one? We can do this at a later time. For this commit I only moved the files around. The only change is in `README.md` where I replaced the pointer to `model_signing/README.md` with one to `README.model_signing.md`, as I'm creating this file now. Since we would need to change `README.model_signing.md` anyway once the API is ready, I have not updated it besides just fixing the image pointer. I have deleted old files from when this was a POC. Since we're working on releasing the library, we no longer need those. The alternative to not moving the files to the root of the folder would have been to duplicate LICENSE, etc. inside the `model-signing` directory, so that packaging tools would see it. I decided against that, since we still needed to move files around to make a `src/` directory and it did no longer make sense to also do file duplication. Signed-off-by: Mihai Maruseac <[email protected]>
Decided to use [`hatch`](https://hatch.pypa.io/latest/). Well, `hatchling` for the build backend and `hatch` as project manager. While not important for this step, using `hatch` allows us to have multiple environments for different usage scenarios. We can have one environment for tests, one for type checking, etc. For this commit, I just focused on making sure that `hatch build` builds the wheel and the tarball. As a bonus, `hatch shell` drops us into a virtual environment with the package locally installed, in an editable fashion: we can just edit the files and can immediately test the code without having to reinstall the wheel somewhere else. So, we get the benefits of testing exactly the wheel that is shipped to the users without having to have a separate virtual environment to install the wheel in, without having to manually manage that. The wheel that gets built is minimal. Here's the corresponding tarball's contents (output of `tar tf`): ``` model_signing-0.0.1a0/model_signing/__init__.py model_signing-0.0.1a0/model_signing/hashing/__init__.py model_signing-0.0.1a0/model_signing/hashing/file.py model_signing-0.0.1a0/model_signing/hashing/hashing.py model_signing-0.0.1a0/model_signing/hashing/memory.py model_signing-0.0.1a0/model_signing/manifest/__init__.py model_signing-0.0.1a0/model_signing/manifest/manifest.py model_signing-0.0.1a0/model_signing/serialization/__init__.py model_signing-0.0.1a0/model_signing/serialization/serialization.py model_signing-0.0.1a0/model_signing/serialization/serialize_by_file.py model_signing-0.0.1a0/model_signing/serialization/serialize_by_file_shard.py model_signing-0.0.1a0/model_signing/signature/__init__.py model_signing-0.0.1a0/model_signing/signature/encoding.py model_signing-0.0.1a0/model_signing/signature/fake.py model_signing-0.0.1a0/model_signing/signature/key.py model_signing-0.0.1a0/model_signing/signature/pki.py model_signing-0.0.1a0/model_signing/signature/signing.py model_signing-0.0.1a0/model_signing/signature/sigstore.py model_signing-0.0.1a0/model_signing/signature/verifying.py model_signing-0.0.1a0/model_signing/signing/__init__.py model_signing-0.0.1a0/model_signing/signing/as_bytes.py model_signing-0.0.1a0/model_signing/signing/empty_signing.py model_signing-0.0.1a0/model_signing/signing/in_toto.py model_signing-0.0.1a0/model_signing/signing/signing.py model_signing-0.0.1a0/model_signing/signing/sigstore.py model_signing-0.0.1a0/.gitignore model_signing-0.0.1a0/LICENSE model_signing-0.0.1a0/README.model_signing.md model_signing-0.0.1a0/pyproject.toml model_signing-0.0.1a0/PKG-INFO ``` Signed-off-by: Mihai Maruseac <[email protected]>
Tested by running `hatch shell` then `python` then ``` >>> import model_signing >>> import model_signing.hashing >>> import model_signing.hashing.hashing >>> import model_signing.hashing.file >>> import model_signing.hashing.memory >>> import model_signing.manifest >>> import model_signing.manifest.manifest >>> import model_signing.serialization >>> import model_signing.serialization.serialization >>> import model_signing.serialization.serialize_by_file >>> import model_signing.serialization.serialize_by_file_shard >>> import model_signing.signing >>> import model_signing.signing.as_bytes >>> import model_signing.signing.in_toto >>> import model_signing.signing.signing >>> import model_signing.signing.sigstore >>> import model_signing.signature >>> import model_signing.signature.fake >>> import model_signing.signature.key >>> import model_signing.signature.pki >>> import model_signing.signature.signing >>> import model_signing.signature.sigstore >>> import model_signing.signature.verifying >>> ^D ``` Tested the same with `hatch build`, copying the wheel to outside the soruce tree, creating a virtual env, and installing the wheel in the empty env, after which I ran the above imports again. Signed-off-by: Mihai Maruseac <[email protected]>
Enable random order testing to surface out cases where one test case depends on another. Enable parallel testing to speed up testing. Will add matrix support later. ``` plugins: xdist-3.6.1, mock-3.14.0, rerunfailures-14.0, randomly-3.15.0 12 workers [233 items] ......................................................................................................................................................................................................................................... [100%] ================================================================================================================================== 233 passed in 2.18s =================================================================================================================================== ``` Signed-off-by: Mihai Maruseac <[email protected]>
Did some minor fixes here and there, but left a bunch for future PRs. Signed-off-by: Mihai Maruseac <[email protected]>
This builds a custom environment in which we run `pytype` for just the library and the test. Unlike style linting, here we completely ignore `slsa_for_models`. Signed-off-by: Mihai Maruseac <[email protected]>
Signed-off-by: Mihai Maruseac <[email protected]>
mihaimaruseac
force-pushed
the
packaging
branch
from
931a896
to
8970b8e
Compare
We now use hatch to install and manage the dependencies. Signed-off-by: Mihai Maruseac <[email protected]>
We already have a Sigstore signer in the main library, this is no longer needed. If the CodeQL scanner would not have failed here, I would have kept it around for a future cleanup. Signed-off-by: Mihai Maruseac <[email protected]>
mihaimaruseac
force-pushed
the
packaging
branch
from
8970b8e
to
38fa5f9
Compare
Comment on lines
+41
to
+42
| - name: Set up Hatch | ||
| uses: pypa/hatch@a3c83ab3d481fbc2dc91dd0088628817488dd1d5 |
There was a problem hiding this comment.
I'm curious how dependabot will update this. I haven't seen the install branch approach they take for the action before.
There was a problem hiding this comment.
Also, not sure if we'll lose dependency caching but can always revisit in future.
There was a problem hiding this comment.
Yeah, we have to keep an eye on this and revisit in the future. There's an alternative path to install via pipx
Comment on lines
+88
to
+89
| [tool.ruff.lint] | ||
| select = ["B", "D", "E", "F", "I", "N", "PLC", "PLE", "PT", "SIM", "W"] |
There was a problem hiding this comment.
should this be extend-select?
https://hatch.pypa.io/1.9/config/static-analysis/#extending-config
Or should we be explicit through select?
There was a problem hiding this comment.
We'll switch to extend-select once we minimize the rules we exclude, but right now we have a superset of what's by default, so select seems better.
|
also can i selfishly request this patch so the VSCode pytest integration still works with the new layout? As an added bonus it restores the ability to do diff --git a/pyproject.toml b/pyproject.toml
index 7b02f97..7691c99 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -62,6 +62,9 @@ installer = "pip"
parallel = true
randomize = true
+[tool.pytest.ini_options]
+pythonpath = "src"
+
[tool.ruff]
line-length = 80
target-version = "py311" |
Signed-off-by: Mihai Maruseac <[email protected]>
|
Added the patch, but note that it only works if you have pytest installed in the env from outside of hatch's managed ones. |
|
(wrong click on the close button, sorry) |
mihaimaruseac
added a commit
to mihaimaruseac/sigstore-community
that referenced
this pull request
Aug 14, 2024
These are added and tested in sigstore/model-transparency#283. We only require Linux checks for the model signing library and linting/py-style. Signed-off-by: Mihai Maruseac <[email protected]>
spencerschrock
approved these changes
Aug 14, 2024
|
Ohh, good catch. Yes! |
haydentherapper
pushed a commit
to sigstore/community
that referenced
this pull request
Aug 14, 2024
These are added and tested in sigstore/model-transparency#283. We only require Linux checks for the model signing library and linting/py-style. Signed-off-by: Mihai Maruseac <[email protected]>
mihaimaruseac
added a commit
to mihaimaruseac/model-transparency
that referenced
this pull request
Aug 14, 2024
This file gets generated when testing with coverage data (`hatch test -c`). I missed adding it to `.gitignore` in sigstore#283 because I didn't run testing with coverage locally. Fixing the issue now. Signed-off-by: Mihai Maruseac <[email protected]>
mihaimaruseac
added a commit
that referenced
this pull request
Aug 14, 2024
This file gets generated when testing with coverage data (`hatch test -c`). I missed adding it to `.gitignore` in #283 because I didn't run testing with coverage locally. Fixing the issue now. Signed-off-by: Mihai Maruseac <[email protected]>
This was referenced Aug 15, 2024
susperius
pushed a commit
to susperius/model-transparency
that referenced
this pull request
Aug 21, 2024
…#283) * Reorganize code to enable wheel publishing This commits moves files around to get to a `src/` layout (https://packaging.python.org/en/latest/discussions/src-layout-vs-flat-layout/#src-layout-vs-flat-layout) for the code. There are more benefits for a `src/` layout than a flat layout, such as: - documentation is easier to generate - we won't accidentally test against code that is different than what users would see - we can include only the source in the wheel, reducing PyPI bandwidth - it makes it clearer which parts of the repo go into the wheel and which ones are ancillary. Incidentally, we might now want to move the SLSA parts to another repo and maybe rename this one? We can do this at a later time. For this commit I only moved the files around. The only change is in `README.md` where I replaced the pointer to `model_signing/README.md` with one to `README.model_signing.md`, as I'm creating this file now. Since we would need to change `README.model_signing.md` anyway once the API is ready, I have not updated it besides just fixing the image pointer. I have deleted old files from when this was a POC. Since we're working on releasing the library, we no longer need those. The alternative to not moving the files to the root of the folder would have been to duplicate LICENSE, etc. inside the `model-signing` directory, so that packaging tools would see it. I decided against that, since we still needed to move files around to make a `src/` directory and it did no longer make sense to also do file duplication. Signed-off-by: Mihai Maruseac <[email protected]> * Add publishing metadata to pyproject.toml Decided to use [`hatch`](https://hatch.pypa.io/latest/). Well, `hatchling` for the build backend and `hatch` as project manager. While not important for this step, using `hatch` allows us to have multiple environments for different usage scenarios. We can have one environment for tests, one for type checking, etc. For this commit, I just focused on making sure that `hatch build` builds the wheel and the tarball. As a bonus, `hatch shell` drops us into a virtual environment with the package locally installed, in an editable fashion: we can just edit the files and can immediately test the code without having to reinstall the wheel somewhere else. So, we get the benefits of testing exactly the wheel that is shipped to the users without having to have a separate virtual environment to install the wheel in, without having to manually manage that. The wheel that gets built is minimal. Here's the corresponding tarball's contents (output of `tar tf`): ``` model_signing-0.0.1a0/model_signing/__init__.py model_signing-0.0.1a0/model_signing/hashing/__init__.py model_signing-0.0.1a0/model_signing/hashing/file.py model_signing-0.0.1a0/model_signing/hashing/hashing.py model_signing-0.0.1a0/model_signing/hashing/memory.py model_signing-0.0.1a0/model_signing/manifest/__init__.py model_signing-0.0.1a0/model_signing/manifest/manifest.py model_signing-0.0.1a0/model_signing/serialization/__init__.py model_signing-0.0.1a0/model_signing/serialization/serialization.py model_signing-0.0.1a0/model_signing/serialization/serialize_by_file.py model_signing-0.0.1a0/model_signing/serialization/serialize_by_file_shard.py model_signing-0.0.1a0/model_signing/signature/__init__.py model_signing-0.0.1a0/model_signing/signature/encoding.py model_signing-0.0.1a0/model_signing/signature/fake.py model_signing-0.0.1a0/model_signing/signature/key.py model_signing-0.0.1a0/model_signing/signature/pki.py model_signing-0.0.1a0/model_signing/signature/signing.py model_signing-0.0.1a0/model_signing/signature/sigstore.py model_signing-0.0.1a0/model_signing/signature/verifying.py model_signing-0.0.1a0/model_signing/signing/__init__.py model_signing-0.0.1a0/model_signing/signing/as_bytes.py model_signing-0.0.1a0/model_signing/signing/empty_signing.py model_signing-0.0.1a0/model_signing/signing/in_toto.py model_signing-0.0.1a0/model_signing/signing/signing.py model_signing-0.0.1a0/model_signing/signing/sigstore.py model_signing-0.0.1a0/.gitignore model_signing-0.0.1a0/LICENSE model_signing-0.0.1a0/README.model_signing.md model_signing-0.0.1a0/pyproject.toml model_signing-0.0.1a0/PKG-INFO ``` Signed-off-by: Mihai Maruseac <[email protected]> * Add required deps to install/use the package. Tested by running `hatch shell` then `python` then ``` >>> import model_signing >>> import model_signing.hashing >>> import model_signing.hashing.hashing >>> import model_signing.hashing.file >>> import model_signing.hashing.memory >>> import model_signing.manifest >>> import model_signing.manifest.manifest >>> import model_signing.serialization >>> import model_signing.serialization.serialization >>> import model_signing.serialization.serialize_by_file >>> import model_signing.serialization.serialize_by_file_shard >>> import model_signing.signing >>> import model_signing.signing.as_bytes >>> import model_signing.signing.in_toto >>> import model_signing.signing.signing >>> import model_signing.signing.sigstore >>> import model_signing.signature >>> import model_signing.signature.fake >>> import model_signing.signature.key >>> import model_signing.signature.pki >>> import model_signing.signature.signing >>> import model_signing.signature.sigstore >>> import model_signing.signature.verifying >>> ^D ``` Tested the same with `hatch build`, copying the wheel to outside the soruce tree, creating a virtual env, and installing the wheel in the empty env, after which I ran the above imports again. Signed-off-by: Mihai Maruseac <[email protected]> * Add testing support via `hatch test`. Enable random order testing to surface out cases where one test case depends on another. Enable parallel testing to speed up testing. Will add matrix support later. ``` plugins: xdist-3.6.1, mock-3.14.0, rerunfailures-14.0, randomly-3.15.0 12 workers [233 items] ......................................................................................................................................................................................................................................... [100%] ================================================================================================================================== 233 passed in 2.18s =================================================================================================================================== ``` Signed-off-by: Mihai Maruseac <[email protected]> * Standardize formatting with `hatch fmt` Did some minor fixes here and there, but left a bunch for future PRs. Signed-off-by: Mihai Maruseac <[email protected]> * Add `pytype` support via `hatch run type:check` This builds a custom environment in which we run `pytype` for just the library and the test. Unlike style linting, here we completely ignore `slsa_for_models`. Signed-off-by: Mihai Maruseac <[email protected]> * Update `.gitignore` to exclude all cache artifacts Signed-off-by: Mihai Maruseac <[email protected]> * Update GHAs to use hatch for testing and linting The scripts are now much simpler and easier to update to newer versions of Python. I also added copyright notices to all files. Removed the workflows that are no longer relevant (slsa_for_ml needs to be pinned right now to first make sure it works -- probably we'll move it to a separate repo). Signed-off-by: Mihai Maruseac <[email protected]> * Remove old model signing requirements files. We now use hatch to install and manage the dependencies. Signed-off-by: Mihai Maruseac <[email protected]> * Fix codeQL scan: remove unused file. We already have a Sigstore signer in the main library, this is no longer needed. If the CodeQL scanner would not have failed here, I would have kept it around for a future cleanup. Signed-off-by: Mihai Maruseac <[email protected]> * Handle review comments Signed-off-by: Mihai Maruseac <[email protected]> --------- Signed-off-by: Mihai Maruseac <[email protected]>
susperius
pushed a commit
to susperius/model-transparency
that referenced
this pull request
Aug 21, 2024
This file gets generated when testing with coverage data (`hatch test -c`). I missed adding it to `.gitignore` in sigstore#283 because I didn't run testing with coverage locally. Fixing the issue now. Signed-off-by: Mihai Maruseac <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Prepare the project so that we can publish a wheel with the library (as part of #175). Attempt to make sure we can still proceed with both building the library and the surrounding CLI.
We are going to use
hatchfor all development work as that makes it very simple:hatch testhatch test -chatch test --all(-cstill supported)hatch fmthatch fmt --check.hatch run type:check. This is a custom environment and we can create others in the future (e.g., for documentation)hatch build. The wheel can then be installed in a separate virtual env if needed for future testing.hatch shellorhatch run $cmd.Testing now happens in parallel, and in randomized fashion. Fast and removes the possibility of a test case influencing the behavior of another.
Another benefit of using
hatchis that it allows us to build C/C++ extensions (via plugins), so when we need to get extra performance or drop to C/C++ (e.g., for #201) we can just do it.In order to make this buildable as a wheel, I needed to restructure the code. So, the first commit moves files around and only changes 2 markdown links. The files are moved to get to a
src/layout. There are more benefits for asrc/layout than a flatlayout, such as:
Since we would need to change
README.model_signing.mdanyway once the API is ready, I have not updated it besides just fixing the image pointer.I have deleted old files from when this was a POC. Since we're working on releasing the library, we no longer need those. The code in these files is already implemented in the library. What these files used to test is now in the test files.
The alternative to not moving the files to the root of the folder would have been to duplicate
LICENSE, etc. inside themodel-signingdirectory, so that packaging tools would see it. But we still needed to move files around to get to thesrc/layout and to make documentation work correctly. So, it did not make sense to also duplicateLICENSE.For the second commit of the PR I just focused on making sure that
hatch buildbuilds the wheel and the tarball. As a bonus,hatch shelldrops us into a virtual environment with the package locally installed, in an editable fashion: we can just edit the files and can immediately test the code without having to reinstall the wheel somewhere else. So, we get the benefits of testing exactly the wheel that is shipped to the users without having to have a separate virtual environment to install the wheel in, without having to manually manage that.The wheel that gets built is minimal. Here's the corresponding tarball's contents (output of
tar tf):I added a few more formatting settings and run
hatch fmtacross the entire code, affectingslsa_for_modelstoo. Added some excludes and we'll work on minimizing those in the future.Updated
.gitignoreto exclude all the artifacts generated during development. Updated GitHub Actions to run viahatch. Removed GHA workflows which became obsolete, as well as the requirements files formodel_signing, since now dependencies are handled viahatch.Since CodeQL was complaining about
signature/sigstore.pyand we already havesinging/sigstore.pywith the implementation of signing with Sigstore, I removed the file insignature. Incidentally, we might want to renamesignature/aspki/as it only has the parts needed for signing with own PKI. We would then offer this as a feature of the wheel (pip install model_signing[pki]) instead of having it be there by default. But these changes will happen in future PRs.This would require rebase on #279, #240 given that we have moved files around. It closes #192 given that all tests have been migrated. It closes #68 due to the same. Similarly, closes #182. Also, closes #231 since we have fixed
slsa_for_modelscode to passhatch fmtRelease Note
NONE (will add when ready to publish, also with CHANGELOG.md)
Documentation
NONE (will add when ready to publish, there's some TODO now to merge
README.mdfiles)