scikit-hep / pyhf

pure-Python HistFactory implementation with tensors and autodiff
https://pyhf.readthedocs.io/
Apache License 2.0
276 stars 82 forks source link

Fix deployment to TestPyPI on tagged commits #887

Closed matthewfeickert closed 4 years ago

matthewfeickert commented 4 years ago

Description

Deployment to TestPyPI on tagged commits (as opposed to untagged reelases headed only to TestPyPI) is failing (c.f. 2b0a1e2ba41a2aa1233511524e3aa78e696ead34 attempt https://github.com/scikit-hep/pyhf/runs/725740838).

Checks for dev in the wheel title should only be for untagged commits. The check should ensure dev only on untagged commits trying to release to TestPyPI, and ensure non-dev on all tagged commits, regardless if pushing to PyPI or TestPyPI.

Expected Behavior

Tag Creator makes a new release that is published to TestPyPI.

Actual Behavior

2b0a1e2ba41a2aa1233511524e3aa78e696ead34

Steps to Reproduce

Initiate a new release with the Tag Creator workflow

matthewfeickert commented 4 years ago

So the problem here is when in Tag Creator when bumpversion is run it creates both the tag and the commit with the bumped version numbers, and then pushes both the commit and the tag to master (all of this is of course expected and needed). However, as the (Test)PyPI publishing CI gets run on both pushes to master and tags

https://github.com/scikit-hep/pyhf/blob/2b0a1e2ba41a2aa1233511524e3aa78e696ead34/.github/workflows/publish-package.yml#L2-L7

this sets of two different publishing workflows (again as expected). The problem is that we normally want them to do different things, but here we're trying to publish to PyPI on the tag

https://github.com/scikit-hep/pyhf/blob/2b0a1e2ba41a2aa1233511524e3aa78e696ead34/.github/workflows/publish-package.yml#L62-L66

TestPyPI on the master commit

https://github.com/scikit-hep/pyhf/blob/2b0a1e2ba41a2aa1233511524e3aa78e696ead34/.github/workflows/publish-package.yml#L55-L61

On a non-release PR only one of these can ever be run, but on a release PR both of them get run at the same time by the push even kicked off from Tag Creator.

This might be able to be overcome though, as the only commits that are authored by "GitHub Action" are the tag commits. So we should be able to just check if the Author is not GitHub Action for the "Verify untagged commits have dev versions" step.

A subtle note is that the the publish-package workflow gets run 3 times for each release PR that gets merged in:

  1. First, when the PR gets merged to master this causes a push event on master which runs.
  2. When Tag Creator pushes the commit from bumpversion to origin this causes a push event on master which runs.
  3. When Tag Creator pushes the tag from bumpversion to origin this causes a tag event which runs.

2 and 3 happen simultaneously from the same push. So the confusion is in disentangling type 1 events from type 3 events.

matthewfeickert commented 4 years ago

This is actually able to be done in a more elegant way using Git. We can get the name of the latest tag that has been created (the tag that was just pushed) with

latest_tag=$(git describe --tags)

and then with that get the latest commit on the tag's rev-list

latest_tag_revlist_SHA=$(git rev-list -n 1 ${latest_tag})

As git rev-list ${latest_tag} will give the list of commits accessible from the ${latest tag}, then ${latest_tag_revlist_SHA} will point to the most recent commit accessible — the last commit on origin/master which is the one bumpversion created when making ${latest tag}. Note that we have to do it this way, as the actual SHA (from git rev-parse --verfiy) for the ${latest tag} and origin/master will be different. Then we can simply get the SHA from origin/master

master_SHA="$(git rev-parse --verify origin/master)"

and check if they're different

[[ "${latest_tag_revlist_SHA}" != "${master_SHA}" ]]

If they are, then the HEAD of origin/master is not accessible from ${latest tag} (it has advanced further ahead in the commit history), so the commit on the push event is coming from a commit on a PR and so should be checked if it is a dev version. If they're the same, then that's only possible if a tag was created by bumpversion and pushed to master on the push event that triggered the distribution so the check should be skipped and the distribution should be published to TestPyPI.