diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 01411b00..b272a5e3 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -17,6 +17,10 @@ env: # Used by the `Version` class so it can identify the current fork PR_AUTHOR: ${{ github.event.pull_request.user.login || github.actor }} + # Used by `Version`. We want the PR author's latest commit hash if this is a PR + # but the default SHA env var reflects a new commit into the target repo. + SOURCE_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + jobs: test: runs-on: ubuntu-latest diff --git a/src/seedsigner/helpers/version.py b/src/seedsigner/helpers/version.py index cc65e4fa..17e75cc7 100644 --- a/src/seedsigner/helpers/version.py +++ b/src/seedsigner/helpers/version.py @@ -86,10 +86,10 @@ class Version(Singleton): In Github Actions CI: * version_name: read from GITHUB_REF_NAME env var. - * version_fork: read from GITHUB_REPOSITORY_OWNER env var. + * version_fork: read from PR_AUTHOR custom CI env var or GITHUB_REPOSITORY_OWNER. * version_timestamp: Shell `git` call to get the last commit time for the current branch/tag/commit. - * commit_hash: read from GITHUB_SHA env var. + * commit_hash: read from SOURCE_SHA custom CI env var or GITHUB_SHA. This class defines the limited methods that are meant to be publicly accessible across the SeedSigner codebase. @@ -195,6 +195,7 @@ class VersionUtils: ENV_VAR__GITHUB_ACTIONS__IS_CI = "CI" ENV_VAR__GITHUB_ACTIONS__HEAD_REF = "GITHUB_HEAD_REF" ENV_VAR__GITHUB_ACTIONS__REF_NAME = "GITHUB_REF_NAME" + ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA = "SOURCE_SHA" ENV_VAR__GITHUB_ACTIONS__SHA = "GITHUB_SHA" ENV_VAR__GITHUB_ACTIONS__PR_AUTHOR = "PR_AUTHOR" ENV_VAR__GITHUB_ACTIONS__REPOSITORY_OWNER = "GITHUB_REPOSITORY_OWNER" @@ -465,16 +466,19 @@ class VersionUtils: HEAD_REF: head ref or source branch. But only present for PRs. REF_NAME: branch or tag name. But it is "/merge" for unmerged PRs (not what we want) + SOURCE_SHA is the full commit hash of the commit that triggered the workflow (if + it's a PR). SHA is the full commit hash; not expecting to ever need this fallback. """ for env_var in [ cls.ENV_VAR__GITHUB_ACTIONS__HEAD_REF, cls.ENV_VAR__GITHUB_ACTIONS__REF_NAME, + cls.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA, cls.ENV_VAR__GITHUB_ACTIONS__SHA ]: version_name = os.getenv(env_var) if version_name: - if env_var == cls.ENV_VAR__GITHUB_ACTIONS__SHA: + if env_var in [cls.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA, cls.ENV_VAR__GITHUB_ACTIONS__SHA]: # Return the short version version_name = version_name[:7] return version_name @@ -497,7 +501,13 @@ class VersionUtils: @classmethod def _get_full_commit_hash_from_github_actions_env_vars(cls) -> str | None: - return os.getenv(cls.ENV_VAR__GITHUB_ACTIONS__SHA) + for env_var in [ + cls.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA, + cls.ENV_VAR__GITHUB_ACTIONS__SHA + ]: + commit_hash = os.getenv(env_var) + if commit_hash: + return commit_hash diff --git a/tests/test_version.py b/tests/test_version.py index e4e1ecf3..7530563e 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -296,12 +296,15 @@ class TestVersionUtils_GithubActions(VersionBaseTest): """ # CI uses some limited `git` shell calls; mock out the associated calls. with patch("seedsigner.helpers.version.VersionUtils._get_version_timestamp_from_git_shell", return_value=TEST__VERSION_TIMESTAMP): + random_sha = "abcd1234ef567890abcd1234ef567890abcd1234" + # When running CI on a branch with patch.dict(os.environ, { VersionUtils.ENV_VAR__GITHUB_ACTIONS__IS_CI: "true", VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: TEST__VERSION_BRANCH, VersionUtils.ENV_VAR__GITHUB_ACTIONS__PR_AUTHOR: TEST__VERSION_FORK, - VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: TEST__FULL_COMMIT_HASH, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: TEST__FULL_COMMIT_HASH, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: random_sha, }): assert VersionUtils.get_version_name() == TEST__VERSION_BRANCH assert VersionUtils.get_version_fork() == TEST__VERSION_FORK @@ -331,18 +334,20 @@ class TestVersionUtils_GithubActions(VersionBaseTest): VersionUtils.ENV_VAR__GITHUB_ACTIONS__IS_CI: "true", VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", - VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: TEST__FULL_COMMIT_HASH, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: TEST__FULL_COMMIT_HASH, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: random_sha, }): assert VersionUtils.get_version_name() == TEST__SHORT_COMMIT_HASH - # When running CI on a commit (detached HEAD) with no HEAD_REF orREF_NAME and - # no SHA, raise error. + # When running CI on a commit (detached HEAD) with no HEAD_REF or REF_NAME and + # no SOURCE_SHA or SHA, raise error. # Note: This scenario definitely would never happen. Just trying to get to # 100% test coverage. with patch.dict(os.environ, { VersionUtils.ENV_VAR__GITHUB_ACTIONS__IS_CI: "true", VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: "", }): with pytest.raises(Exception): @@ -375,12 +380,14 @@ class TestVersionUtils_GithubActions(VersionBaseTest): test_head_ref = "head_ref" test_ref_name = "ref_name" test_sha = TEST__FULL_COMMIT_HASH + random_sha = "abcd1234ef567890abcd1234ef567890abcd1234" # Need to signal that we're in a GitHub Actions CI environment with patch.dict(os.environ, { VersionUtils.ENV_VAR__GITHUB_ACTIONS__IS_CI: "true", VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: "", }): # With none of the env vars set, should return None @@ -390,7 +397,8 @@ class TestVersionUtils_GithubActions(VersionBaseTest): with patch.dict(os.environ, { VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: test_head_ref, VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: test_ref_name, - VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: test_sha, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: test_sha, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: random_sha, }): result = VersionUtils._get_version_name_from_github_actions_env_vars() assert result == test_head_ref @@ -399,17 +407,29 @@ class TestVersionUtils_GithubActions(VersionBaseTest): with patch.dict(os.environ, { VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: test_ref_name, - VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: test_sha, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: test_sha, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: random_sha, }): result = VersionUtils._get_version_name_from_github_actions_env_vars() assert result == test_ref_name - # Unlikely scenario: no HEAD_REF nor REF_NAME but SHA is set; should return - # short commit hash + # Unlikely scenario: no HEAD_REF nor REF_NAME but SOURCE_SHA is set; should return + # short commit hash. with patch.dict(os.environ, { VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", - VersionUtils.ENV_VAR__GITHUB_ACTIONS__SHA: test_sha, + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: test_sha, + }): + result = VersionUtils._get_version_name_from_github_actions_env_vars() + assert result == test_sha[:7] + + # Unlikely scenario 2: no HEAD_REF nor REF_NAME and no SOURCE_SHA, but SHA is + # set; should return short commit hash. + with patch.dict(os.environ, { + VersionUtils.ENV_VAR__GITHUB_ACTIONS__HEAD_REF: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__REF_NAME: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: "", + VersionUtils.ENV_VAR__GITHUB_ACTIONS__SOURCE_SHA: test_sha, }): result = VersionUtils._get_version_name_from_github_actions_env_vars() assert result == test_sha[:7]