diff --git a/.github/workflows/cron-conda.yml b/.github/workflows/cron-conda.yml index 08030b80a..b358ea104 100644 --- a/.github/workflows/cron-conda.yml +++ b/.github/workflows/cron-conda.yml @@ -558,11 +558,15 @@ jobs: # #1634: the upload below never makes the release. ncipollo/release-action # v1.21.0 creates a missing one, and its tag, at ``commit`` - # (src/Action.ts:72-92), and publishes a draft, since an unset ``draft`` - # is false on update (src/Inputs.ts:210-213). A bump made while Create - # Release waits on approval would then tag the pending version at main's - # tip, and Create Release's tag check would fail. Only the GitHub upload - # is skipped; the build is on anaconda.org regardless. + # (src/Action.ts:72-92), so a bump made while Create Release waits on + # approval would tag the pending version at main's tip, and Create + # Release's tag check would fail. It also retargets a draft at ``commit`` + # (:85-92,107-120), which ``omitDraftDuringUpdate`` keeps a draft, but + # Create Release's ``workflow_run`` run finds the draft and resets its + # target to the run's sha (softprops/action-gh-release v3.0.2, + # src/github.ts:636-646,832-866). Skipping a draft is a precaution + # against retargeting a release Create Release owns. Only the GitHub + # upload is skipped; the build is on anaconda.org regardless. - name: Check the GitHub release exists id: release if: ${{ matrix.os == 'ubuntu-latest' }} @@ -579,15 +583,15 @@ jobs: exit 0 fi if [ "$status" -eq 0 ] && [ "$draft" = true ]; then - why="is a draft, which the upload would publish" + why="is a draft, which the upload would retarget at main" elif [ "$(cat "$errors")" = 'release not found' ]; then - why="does not exist, and the upload would create it" + why="does not exist, and the upload would create it, tagging main's tip" else cat "$errors" >&2 echo "::error title=Could not read ${PCAPKIT_RELEASE_TAG}::gh release view exited ${status}." exit 1 fi - echo "::notice title=No GitHub upload for ${PCAPKIT_RELEASE_TAG}::${PCAPKIT_RELEASE_TAG} ${why}, tagging main's tip. Create Release makes it, at the commit its run was created for (#1634); this build is on anaconda.org only." + echo "::notice title=No GitHub upload for ${PCAPKIT_RELEASE_TAG}::${PCAPKIT_RELEASE_TAG} ${why}. Create Release makes it, at the commit its run was created for (#1634); this build is on anaconda.org only." echo "published=false" >> "$GITHUB_OUTPUT" # ``commit`` is sent to Update a release as ``target_commitish`` @@ -596,6 +600,14 @@ jobs: # GitHub's workflow-file rule checks main, not the commit Create Release # made the release at. The tag stays: the target is "Unused if the Git # tag already exists" (REST docs, Update a release). + # + # #1657: that update also sends ``name``, ``body``, ``prerelease`` and + # ``draft``, which unset inputs make the tag, "" and false + # (src/Inputs.ts:70-92,128-135,210-240), so each bump wiped the + # release's name and notes. Each ``omit*DuringUpdate`` makes its field + # undefined (:210-252), which Octokit drops from the request + # (dist/index.js:38369-38388), so the release keeps what Create Release + # wrote. - name: Upload package to GitHub Release if: ${{ matrix.os == 'ubuntu-latest' && steps.release.outputs.published == 'true' }} uses: ncipollo/release-action@v1.21.0 @@ -603,5 +615,9 @@ jobs: allowUpdates: true artifacts: ${{ steps.conda_build.outputs.packages_csv }} commit: ${{ github.ref_name }} + omitBodyDuringUpdate: true + omitDraftDuringUpdate: true + omitNameDuringUpdate: true + omitPrereleaseDuringUpdate: true tag: "v${{ needs.conda-update.outputs.PCAPKIT_VERSION }}" token: "${{ secrets.GITHUB_TOKEN }}" diff --git a/docs/source/contributing/releasing.rst b/docs/source/contributing/releasing.rst index 4de9bb7f0..6b81985c4 100644 --- a/docs/source/contributing/releasing.rst +++ b/docs/source/contributing/releasing.rst @@ -203,13 +203,22 @@ names the default branch -- ``github.ref_name`` on both its triggers -- as stays put, as Update a release calls the target "Unused if the Git tag already exists". It also uploads only to a published release, read first with ``gh release view``. With none, the action would create one, and its tag, at -``commit`` (``src/Action.ts:72-92``), and it would publish a draft, since an -unset ``draft`` is ``false`` on update (``src/Inputs.ts:210-213``). Conda -Update bumped 1.5.0b9's build three times while that run waited on approval; -once ``conda-dist`` runs on every bump -(`#1632 `__), either path -would tag the pending version at ``main``'s tip and fail Create Release's tag -check. That build then goes to anaconda.org only, with a ``::notice``. +``commit`` (``src/Action.ts:72-92``). Conda Update bumped 1.5.0b9's build +three times while that run waited on approval; once ``conda-dist`` runs on +every bump (`#1632 `__), +that would tag the pending version at ``main``'s tip and fail Create Release's +tag check. The action would also retarget a draft at ``commit`` +(``src/Action.ts:85-92,107-120``), which ``omitDraftDuringUpdate`` keeps a +draft, but Create Release's ``workflow_run`` run finds the draft and resets +its target to the run's sha (``action-gh-release`` 3.0.2's +``src/github.ts:636-646,832-866``), so skipping a draft is a precaution +against retargeting a release Create Release owns. +Either way the build goes to anaconda.org only, with a ``::notice``. The +update also sends the release's name, notes and ``prerelease`` and ``draft`` +flags, which unset inputs make the tag, ``""`` and ``false`` +(``src/Inputs.ts:70-92,128-135,210-240``), so the step sets the four +``omit*DuringUpdate`` inputs that leave them out (`#1657 +`__). The Release App --------------- diff --git a/tests/project/test_conda_dist_keeps_release_notes_1657.py b/tests/project/test_conda_dist_keeps_release_notes_1657.py new file mode 100644 index 000000000..296023502 --- /dev/null +++ b/tests/project/test_conda_dist_keeps_release_notes_1657.py @@ -0,0 +1,80 @@ +# -*- coding: utf-8 -*- +"""Conda Update's GitHub upload keeps the release's name and notes (#1657). + +``conda-dist`` in :file:`.github/workflows/cron-conda.yml` attaches each build +bump's packages with ``ncipollo/release-action@v1.21.0`` and ``allowUpdates: +true``, which calls Update a release before uploading (``src/Action.ts:107-120``). +That request sends ``name``, ``body``, ``prerelease`` and ``draft`` whether or not +the step sets them, and unset they are the tag, ``""`` and ``false`` +(``src/Inputs.ts:70-92,128-135,210-240``). ``v1.3.5.post42``, ``v1.4.1``, +``v1.4.1.post2`` and ``v1.5.0a1`` read name-is-tag and a 0-character body that +way. Each ``omit*DuringUpdate`` input makes its field ``undefined`` +(``:210-252``), which Octokit drops from the request body +(``dist/index.js:38369-38388``), so the release keeps what Create Release wrote. + +The step is read with the scanner of ``test_release_tags_the_run_sha_1634.py``, +which runs without PyYAML; :meth:`TestTheUploadKeepsTheRelease.test_yaml_reads_the_same` +checks it against :func:`yaml.safe_load` where PyYAML is installed. + +""" + +from __future__ import annotations + +import unittest + +from tests.project.test_release_tags_the_run_sha_1634 import (CONDA_WORKFLOW, UPLOAD_STEP, + CondaDistMixin) + +#: The pinned action. The input names and the citations above are for this tag. +ACTION = 'ncipollo/release-action@v1.21.0' + +#: The inputs that leave a field out of Update a release, and the field each keeps. +OMITTED = { + 'omitBodyDuringUpdate': 'body', + 'omitDraftDuringUpdate': 'draft', + 'omitNameDuringUpdate': 'name', + 'omitPrereleaseDuringUpdate': 'prerelease', +} + +#: Every input the step sets. Another one changes what the update sends -- a +#: ``makeLatest``, say, whose ``legacy`` default is already sent as +#: ``make_latest`` (``action.yml:58-61``, ``src/Releases.ts:168``) -- so adding +#: one means reading the action again before adding it here. +INPUTS = {'allowUpdates', 'artifacts', 'commit', 'tag', 'token', *OMITTED} + + +class TestTheUploadKeepsTheRelease(CondaDistMixin, unittest.TestCase): + """Update a release leaves the name, body and flags as Create Release wrote them.""" + + def test_the_action_is_the_one_read(self) -> None: + self.assertEqual(self.upload().get('uses'), ACTION) + self.assertEqual(self.upload()['with'].get('allowUpdates'), 'true') + + def test_each_field_the_update_would_reset_is_omitted(self) -> None: + inputs = self.upload()['with'] + for flag, field in OMITTED.items(): + with self.subTest(flag=flag): + self.assertEqual(inputs.get(flag), 'true', + f'without {flag}, every bump resets the release\'s {field}') + + def test_every_input_is_accounted_for(self) -> None: + self.assertEqual(sorted(self.upload()['with']), sorted(INPUTS)) + + def test_yaml_reads_the_same(self) -> None: + """A key the scanner reads but YAML places elsewhere would pass vacuously.""" + try: + import yaml # pylint: disable=import-outside-toplevel + except ImportError: + self.skipTest('PyYAML is not installed (the `test` extra)') + steps = yaml.safe_load(CONDA_WORKFLOW.read_text(encoding='utf-8'))['jobs']['conda-dist']['steps'] + found = [step for step in steps if step.get('name') == UPLOAD_STEP] + self.assertEqual(len(found), 1) + inputs = found[0]['with'] + self.assertEqual(sorted(inputs), sorted(INPUTS)) + for flag in OMITTED: + with self.subTest(flag=flag): + self.assertIs(inputs[flag], True) + + +if __name__ == '__main__': + unittest.main() diff --git a/tests/project/test_release_tags_the_run_sha_1634.py b/tests/project/test_release_tags_the_run_sha_1634.py index 8e730f477..b457a7cf2 100644 --- a/tests/project/test_release_tags_the_run_sha_1634.py +++ b/tests/project/test_release_tags_the_run_sha_1634.py @@ -1234,14 +1234,19 @@ class TestCondaUpdateNeverMakesTheRelease(CondaDistMixin, unittest.TestCase): ``conda-dist`` attaches its packages with ``ncipollo/release-action@v1.21.0`` and ``allowUpdates: true``, kept so #1632's ``test_cron_conda_really_does_publish`` still finds it. That action's ``createOrUpdateRelease`` (``src/Action.ts:53-70``) - reads the release by tag; on a 404 it updates a draft of that tag or else - calls ``createRelease`` (``:72-92``) with ``target_commitish: commit`` - (``src/Releases.ts:76-99``), and its update sends ``draft: false`` unless - ``omitDraftDuringUpdate`` is set (``src/Inputs.ts:210-213``), publishing the - draft. Either way a bump made while Create Release waits on its approval -- - 1.5.0b9 waited three days, and cron-conda bumped three times meanwhile -- - would put ``v`` on ``main``'s tip, and Create Release's tag check - would then fail. On an existing release it calls Update a release with + reads the release by tag, which GitHub's REST docs give as "a published + release"; on a 404 it updates a draft of that tag or else calls + ``createRelease`` (``:72-92``), both with ``target_commitish: commit`` + (``:107-120``, ``src/Releases.ts:76-99,150-174``). With no release, a bump + made while Create Release waits on its approval -- 1.5.0b9 waited three + days, and cron-conda bumped three times meanwhile -- would put + ``v`` on ``main``'s tip, and Create Release's tag check would then + fail. A draft stays a draft (``omitDraftDuringUpdate``, + ``src/Inputs.ts:210-213``) but is retargeted at ``commit``; Create Release's + ``workflow_run`` run finds the draft and resets its target to the run's sha + (``action-gh-release`` 3.0.2's ``src/github.ts:636-646,832-866``), so + skipping a draft is a precaution against retargeting a release Create + Release owns. On an existing release it calls Update a release with ``target_commitish: commit`` (``src/Action.ts:107-120``, ``src/Releases.ts:150-174``), and GitHub's REST docs check "the new value of target_commitish if you are changing it, otherwise the existing target" @@ -1336,7 +1341,8 @@ def test_no_release_skips_the_github_upload(self) -> None: self.assertEqual(result.returncode, 0, result.stdout + result.stderr) self.assertEqual(outputs, {'published': 'false'}) self.assertFalse(self.runs(outputs), 'the action would run, and create the release at main') - self.assertIn(f'::notice title=No GitHub upload for {self.TAG}::{self.TAG} does not exist', result.stdout) + self.assertIn(f'::notice title=No GitHub upload for {self.TAG}::{self.TAG} does not exist, and the ' + "upload would create it, tagging main's tip. Create Release makes it", result.stdout) self.assert_viewed_once() def test_a_draft_skips_the_github_upload(self) -> None: @@ -1345,8 +1351,9 @@ def test_a_draft_skips_the_github_upload(self) -> None: result, outputs = self.check(GH_DRAFT='true') self.assertEqual(result.returncode, 0, result.stdout + result.stderr) self.assertEqual(outputs, {'published': 'false'}) - self.assertFalse(self.runs(outputs), 'the action would run, and publish the draft') - self.assertIn(f'{self.TAG} is a draft, which the upload would publish', result.stdout) + self.assertFalse(self.runs(outputs), 'the action would run, and retarget the draft at main') + self.assertIn(f'::notice title=No GitHub upload for {self.TAG}::{self.TAG} is a draft, which the ' + 'upload would retarget at main. Create Release makes it', result.stdout) def test_a_published_release_is_uploaded_to(self) -> None: """With ``commit`` the default branch, so Update a release resolves ``main``."""