jorisroovers / gitlint

Linting for your git commit messages
http://jorisroovers.github.io/gitlint
MIT License
791 stars 99 forks source link

Investigate smoke testing of sdist package for downstream packaging #468

Closed jorisroovers closed 1 year ago

jorisroovers commented 1 year ago

From https://github.com/jorisroovers/gitlint/issues/460#issuecomment-1462121899

Additionally, you can make use of https://github.com/re-actors/checkout-python-sdist to run testing from sdist and not Git. This takes smoke-testing availability for the downstream packaging since they often use sdist as the source of truth.

Also from https://github.com/jorisroovers/gitlint/issues/460#issuecomment-1462187098:

There is also this which I've heard good things about (and of course it's Hynek) https://github.com/hynek/build-and-inspect-python-package

Reply https://github.com/jorisroovers/gitlint/issues/460#issuecomment-1462207361:

@ofek yes, I know about that action, but don't use it myself — it's too coupled for my taste.

CC: @ofek, @webknjaz

jorisroovers commented 1 year ago

So I looked into re-actors/checkout-python-sdist.

I found the code in the cherrypy repo (1, 2, 3) a bit too complex for my liking.

I think we could just use actions/upload-artifacts to upload whatever sdist file is created by python -m build sdist.

Something akin to (pseudo-code):

jobs:
    build:
        runs-on: "ubuntu-latest"
        steps:
            - run: python -m build
            - uses: actions/upload-artifact@v3
                with:
                    name: sdist-tarball
                    path: dist/*.tar.gz

    tests:
        runs-on: "ubuntu-latest"
        needs:
            - build
        steps:
            - uses: re-actors/checkout-python-sdist@release/v1
              with:
                  source-tarball-name: "*.tar.gz"  # default value, remove in implementation
                  workflow-artifact-name: sdist-tarball

The actual implementation would be a bit more complicated to add matrixing for different python versions. Actually trying this out is my next step here.

edit: I just realized that our sdist does not actually contain the test files, those are excluded at build time. I reckon I'd need to include them for this to work? Is that best practice? Perhaps just doing seeing if gitlint runs (i.e. a smoke test) is good enough here.

CC: @webknjaz


Reminder to self: While we currently do a deep clone in CI for hatch to determine the correct version, #470 added support for hatch to do this from sdist as well. https://github.com/jorisroovers/gitlint/blob/84f9218251eba531dbc4e964a29a86401658d84f/.github/workflows/ci.yml#L25-L28

jorisroovers commented 1 year ago

Implemented in #509.

I ended up not using the re-actors/checkout-python-sdist because I had to download tarballs for both gitlint and gitlint-core and re-actors/checkout-python-sdist does not support changing the target path or working directory. Also, it's really only 2 simple steps (download and extract) that are performed in that action which were easily duplicated as steps in our worfklow - also avoids adding an external dependency.


Reminder to self: While we currently do a deep clone in CI for hatch to determine the correct version, #470 added support for hatch to do this from sdist as well.

This is incorrect, the version is stored in the PKG-INFO that comes with the sdist tarball.

webknjaz commented 1 year ago

Interesting, it doesn't look like what I suggested (at least, looking from phone). I'll try to check the PR next week since I'm on vacation now.