-
Notifications
You must be signed in to change notification settings - Fork 29
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
Prepare for wheel publishing (alpha patch version of 0.0.1) #283
Conversation
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.
Just took a skim, looking forward to the changes. A few quick questions:
Can we exclude the .gitignore
?
The wheel that gets built is minimal. Here's the corresponding tarball's contents (output of tar tf):
... model_signing-0.0.1a0/.gitignore ...
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. |
378c640
to
f3b0acb
Compare
f3b0acb
to
2bfd2ee
Compare
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 <mihaimaruseac@google.com>
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 <mihaimaruseac@google.com>
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 <mihaimaruseac@google.com>
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 <mihaimaruseac@google.com>
Did some minor fixes here and there, but left a bunch for future PRs. Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>
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 <mihaimaruseac@google.com>
Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>
931a896
to
8970b8e
Compare
We now use hatch to install and manage the dependencies. Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>
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 <mihaimaruseac@google.com>
8970b8e
to
38fa5f9
Compare
- name: Set up Hatch | ||
uses: pypa/hatch@a3c83ab3d481fbc2dc91dd0088628817488dd1d5 |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, not sure if we'll lose dependency caching but can always revisit in future.
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.
Yeah, we have to keep an eye on this and revisit in the future. There's an alternative path to install via pipx
[tool.ruff.lint] | ||
select = ["B", "D", "E", "F", "I", "N", "PLC", "PLE", "PT", "SIM", "W"] |
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 <mihaimaruseac@google.com>
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) |
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 <mihaimaruseac@google.com>
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.
Similarly this should be able to close #231 ?
Ohh, good catch. Yes! |
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 <mihaimaruseac@google.com>
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 <mihaimaruseac@google.com>
…#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 <mihaimaruseac@google.com> * 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 <mihaimaruseac@google.com> * 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 <mihaimaruseac@google.com> * 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 <mihaimaruseac@google.com> * Standardize formatting with `hatch fmt` Did some minor fixes here and there, but left a bunch for future PRs. Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com> * 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 <mihaimaruseac@google.com> * Update `.gitignore` to exclude all cache artifacts Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com> * 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 <mihaimaruseac@google.com> * Remove old model signing requirements files. We now use hatch to install and manage the dependencies. Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com> * 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 <mihaimaruseac@google.com> * Handle review comments Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com> --------- Signed-off-by: Mihai Maruseac <mihaimaruseac@google.com>
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 <mihaimaruseac@google.com>
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
hatch
for all development work as that makes it very simple:hatch test
hatch test -c
hatch test --all
(-c
still supported)hatch fmt
hatch 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 shell
orhatch 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
hatch
is 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.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 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-signing
directory, 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 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
):I added a few more formatting settings and run
hatch fmt
across the entire code, affectingslsa_for_models
too. Added some excludes and we'll work on minimizing those in the future.Updated
.gitignore
to 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.py
and we already havesinging/sigstore.py
with 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_models
code to passhatch fmt
Release 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.md
files)