dynamically add supported Python versions to tox matrix, drawing from package metadata (requires-python)#326
Conversation
11af006 to
35089ae
Compare
|
EDIT: never mind it was a dumb issue with conditionals, ignore this for now! @Cadair if you have time, could you take a look at why it's failing here: https://github.com/OpenAstronomy/github-actions-workflows/actions/runs/19116707014/job/54627509255?pr=326#step:5:13 it looks to me like it's trying to install the script as a package, though I'm not sure why |
67770c8 to
dfa841f
Compare
e824962 to
602c646
Compare
python-requiresrequires-python
requires-pythonrequires-python
requires-pythonrequires-python)
928d17a to
5af0ed7
Compare
ad72251 to
339dabd
Compare
8b954b5 to
1ea502c
Compare
1ea502c to
e1a7215
Compare
| SUPPORTED_PYTHONS_SCRIPT: IyAvLy8gc2NyaXB0CiMgcmVxdWlyZXMtcHl0aG9uID0gIj49My4xMiIKIyBkZXBlbmRlbmNpZXMgPSBbCiMgICAgICJjbGljaz09OC4yLjEiLAojICAgICAicGVwcHlwcm9qZWN0PT0xLjAuMiIsCiMgICAgICJyZXF1ZXN0cz09Mi4zMi41IiwKIyAgICAgInBhY2thZ2luZz09MjUuMCIsCiMgXQojIC8vLwppbXBvcnQgb3MKaW1wb3J0IHdhcm5pbmdzCmZyb20gcGF0aGxpYiBpbXBvcnQgUGF0aAoKaW1wb3J0IGNsaWNrCmltcG9ydCByZXF1ZXN0cwpmcm9tIHBhY2thZ2luZy5zcGVjaWZpZXJzIGltcG9ydCBTcGVjaWZpZXJTZXQKZnJvbSBwYWNrYWdpbmcudmVyc2lvbiBpbXBvcnQgVmVyc2lvbgpmcm9tIHBlcHB5cHJvamVjdCBpbXBvcnQgUHlQcm9qZWN0Q29uZmlndXJhdGlvbgoKCkBjbGljay5jb21tYW5kKCkKQGNsaWNrLm9wdGlvbigiLS1wYWNrYWdlLXNvdXJjZSIsIGRlZmF1bHQ9Tm9uZSkKQGNsaWNrLm9wdGlvbigiLS1mYWN0b3JzIiwgZGVmYXVsdD1Ob25lKQpAY2xpY2sub3B0aW9uKCItLW5vLWVvYXMiLCBpc19mbGFnPVRydWUsIGRlZmF1bHQ9RmFsc2UpCkBjbGljay5vcHRpb24oIi0tcGxhdGZvcm1zIiwgZGVmYXVsdD1Ob25lKQpkZWYgc3VwcG9ydGVkX3B5dGhvbl9lbnZzX2Jsb2NrKAogICAgcGFja2FnZV9zb3VyY2U6IFBhdGggPSBOb25lLAogICAgZmFjdG9yczogbGlzdFtzdHJdID0gTm9uZSwKICAgIG5vX2VvYXM6IGJvb2wgPSBGYWxzZSwKICAgIHBsYXRmb3JtczogbGlzdFtzdHJdID0gTm9uZSwKKToKICAgICIiImVudW1lcmF0ZSB0b3hlbnZzIGZvciBlYWNoIFB5dGhvbiB2ZXJzaW9uIHN1cHBvcnRlZCBieSBwYWNrYWdlIiIiCgogICAgaWYgcGxhdGZvcm1zIGlzIE5vbmU6CiAgICAgICAgcGxhdGZvcm1zID0gWyJsaW51eCJdCiAgICBlbGlmIGlzaW5zdGFuY2UocGxhdGZvcm1zLCBzdHIpOgogICAgICAgIHBsYXRmb3JtcyA9IHBsYXRmb3Jtcy5zcGxpdCgiLCIpCgogICAgdG94ZW52cyA9IHN1cHBvcnRlZF9weXRob25fdG94ZW52cyhwYWNrYWdlX3NvdXJjZSwgZmFjdG9ycywgbm9fZW9hcykKICAgIGVudnNfYmxvY2sgPSAiXFxuIi5qb2luKAogICAgICAgIGYiLSB7cGxhdGZvcm19OiB7dG94ZW52fSIgZm9yIHBsYXRmb3JtIGluIHBsYXRmb3JtcyBmb3IgdG94ZW52IGluIHRveGVudnMKICAgICkKCiAgICBwcmludChlbnZzX2Jsb2NrKQogICAgd2l0aCBvcGVuKG9zLmVudmlyb25bIkdJVEhVQl9PVVRQVVQiXSwgImEiKSBhcyBmOgogICAgICAgIGYud3JpdGUoZiJlbnZzPXtlbnZzX2Jsb2NrfVxuIikKCgpkZWYgc3VwcG9ydGVkX3B5dGhvbl90b3hlbnZzKAogICAgcGFja2FnZV9zb3VyY2U6IFBhdGggPSBOb25lLAogICAgZmFjdG9yczogbGlzdFtzdHJdID0gTm9uZSwKICAgIG5vX2VvYXM6IGJvb2wgPSBGYWxzZSwKKSAtPiBsaXN0W3N0cl06CiAgICBpZiBpc2luc3RhbmNlKGZhY3RvcnMsIHN0cik6CiAgICAgICAgZmFjdG9ycyA9IGZhY3RvcnMuc3BsaXQoIiwiKQoKICAgIHJldHVybiBbCiAgICAgICAgZiJweXtzdHIocHl0aG9uX3ZlcnNpb24pLnJlcGxhY2UoJy4nLCAnJyl9eyctJyArICctJy5qb2luKGZhY3RvcnMpIGlmIGZhY3RvcnMgaXMgbm90IE5vbmUgYW5kIGxlbihmYWN0b3JzKSA+IDAgZWxzZSAnJ30iCiAgICAgICAgZm9yIHB5dGhvbl92ZXJzaW9uIGluIHN1cHBvcnRlZF9weXRob25zKHBhY2thZ2Vfc291cmNlLCBub19lb2FzPW5vX2VvYXMpCiAgICBdCgoKZGVmIHN1cHBvcnRlZF9weXRob25zKAogICAgcGFja2FnZV9zb3VyY2U6IFBhdGggPSBOb25lLAogICAgbm9fZW9hczogYm9vbCA9IEZhbHNlLAopIC0+IGxpc3RbVmVyc2lvbl06CiAgICBjdXJyZW50X3B5dGhvbl92ZXJzaW9ucyA9IGN1cnJlbnRfcHl0aG9ucyhub19lb2FzPW5vX2VvYXMpCgogICAgaWYgbm90IHBhY2thZ2Vfc291cmNlOgogICAgICAgIHN1cHBvcnRlZF92ZXJzaW9ucyA9IGN1cnJlbnRfcHl0aG9uX3ZlcnNpb25zCiAgICBlbHNlOgogICAgICAgIGNvbmZpZ3VyYXRpb24gPSBQeVByb2plY3RDb25maWd1cmF0aW9uLmZyb21fZGlyZWN0b3J5KHBhY2thZ2Vfc291cmNlKQogICAgICAgIHRyeToKICAgICAgICAgICAgcHl0aG9uX3ZlcnNpb25fcmVxdWlyZW1lbnRzID0gU3BlY2lmaWVyU2V0KGNvbmZpZ3VyYXRpb25bInByb2plY3QiXVsicmVxdWlyZXMtcHl0aG9uIl0pCgogICAgICAgICAgICBzdXBwb3J0ZWRfdmVyc2lvbnMgPSBbCiAgICAgICAgICAgICAgICBweXRob25fdmVyc2lvbgogICAgICAgICAgICAgICAgZm9yIHB5dGhvbl92ZXJzaW9uIGluIGN1cnJlbnRfcHl0aG9uX3ZlcnNpb25zCiAgICAgICAgICAgICAgICBpZiBweXRob25fdmVyc2lvbiBpbiBweXRob25fdmVyc2lvbl9yZXF1aXJlbWVudHMKICAgICAgICAgICAgXQogICAgICAgIGV4Y2VwdCAoS2V5RXJyb3IsIFR5cGVFcnJvcik6CiAgICAgICAgICAgIHdhcm5pbmdzLndhcm4oCiAgICAgICAgICAgICAgICAiY291bGQgbm90IGZpbmQgYHJlcXVpcmVzLXB5dGhvbmAgaW4gbWV0YWRhdGE7IGZhbGxpbmcgYmFjayB0byBjdXJyZW50IFB5dGhvbiB2ZXJzaW9ucy4uLiIKICAgICAgICAgICAgKQogICAgICAgICAgICBzdXBwb3J0ZWRfdmVyc2lvbnMgPSBjdXJyZW50X3B5dGhvbl92ZXJzaW9ucwoKICAgIHJldHVybiBzdXBwb3J0ZWRfdmVyc2lvbnMKCgpkZWYgY3VycmVudF9weXRob25zKG5vX2VvYXM6IGJvb2wgPSBGYWxzZSkgLT4gbGlzdFtWZXJzaW9uXToKICAgIHVybCA9ICJodHRwczovL2VuZG9mbGlmZS5kYXRlL2FwaS92MS9wcm9kdWN0cy9weXRob24iCiAgICByZXNwb25zZSA9IHJlcXVlc3RzLmdldCgiaHR0cHM6Ly9lbmRvZmxpZmUuZGF0ZS9hcGkvdjEvcHJvZHVjdHMvcHl0aG9uIikKICAgIGlmIHJlc3BvbnNlLnN0YXR1c19jb2RlID09IDIwMDoKICAgICAgICByZXR1cm4gWwogICAgICAgICAgICBWZXJzaW9uKHB5dGhvbl92ZXJzaW9uWyJuYW1lIl0pCiAgICAgICAgICAgIGZvciBweXRob25fdmVyc2lvbiBpbiByZXNwb25zZS5qc29uKClbInJlc3VsdCJdWyJyZWxlYXNlcyJdCiAgICAgICAgICAgIGlmIG5vdCBweXRob25fdmVyc2lvblsiaXNFb2FzIiBpZiBub19lb2FzIGVsc2UgImlzRW9sIl0KICAgICAgICBdCiAgICBlbHNlOgogICAgICAgIHJhaXNlIFZhbHVlRXJyb3IoZiJyZXF1ZXN0IHRvIHt1cmx9IHJldHVybmVkIHN0YXR1cyBjb2RlIHtyZXNwb25zZS5zdGF0dXNfY29kZX0iKQoKCmlmIF9fbmFtZV9fID09ICJfX21haW5fXyI6CiAgICBzdXBwb3J0ZWRfcHl0aG9uX2VudnNfYmxvY2soKQo= | ||
| - if: inputs.fill | ||
| id: supported-pythons | ||
| run: uv run supported_pythons.py --package-source . ${{ inputs.fill_platforms != '' && format('--platforms {0}', inputs.fill_platforms) || '' }} ${{ inputs.fill_factors != '' && format('--factors {0}', inputs.fill_factors) || '' }} |
There was a problem hiding this comment.
zimzor says this opens a security vulnerability in which an attacker could pass arbitrary shell expressions to fill_platforms or fill_factors:
https://results.pre-commit.ci/run/github/463175679/1773164253.bP-bx2P8T5SnpGjTr9AzmA
may expand into attacker-controllable code
Is this something we should worry about? Anyone using this workflow already has arbitrary code execution privileges via tox
There was a problem hiding this comment.
I set that step to be ignored by zimzor, let me know if we want to change it instead!
There was a problem hiding this comment.
If it's possible to it would be good to not have to ignore this. I don't think we are relying on shell expansion here, so running these variables through env should work?
There was a problem hiding this comment.
What do you mean by running them through env?
There was a problem hiding this comment.
It looks like we're also ignoring zizmor 's warning for the set-outputs call to tox_matrix.py as well
… package metadata (`requires-python`)
Co-authored-by: Stuart Mumford <stuart@cadair.com>
15cdff9 to
ad6e64f
Compare
|
I'd like to cut a release soon, so let's get this in and then I'll cut one. |
4905cd7 to
085e729
Compare
closes #325
adds
fill,fill_platforms, andfill_factorsparameters to thetox.ymlworkflow call.fill: truewill append toxenvs to the existingenvs:list, namely the currently supported Python versions, with factors fromfill_factorsand repeated for each platform infill_platforms.Python versions are first drawn from the current public Python releases that are not yet end-of-life (pulled from
endoflife.dateAPI), and then filtered based on therequires-pythonpins in the package metadata, if it exists.For instance, setting
for
jwst(which currently specifiesrequires-python = ">=3.11,<3.14") adds the following toxenvs toenvs:To read the metadata, I imported
peppyproject, a Python project metadata abstraction library I wrote to convertsetup.cfgtopyproject.toml(but it also works well in this use case, as it will input metadata frompyproject.toml,setup.cfg, orsetup.py).