Fix git-env version for PRs

[bayandin]

Fixes #3619

Problem

github.sha for PRs it's a commit that doesn't belong to the repo.

Here's an example:

• a workflow https://github.com/neondatabase/neon/actions/runs/5465813018/jobs/9950280665#step:4:280
• a commit it shows b4d1824

Summary of changes

• Prefer GIT_VERSION env variable even if git information was accessible
• Use ${{ github.event.pull_request.head.sha || github.sha }} instead of ${{ github.sha }} for GIT_VERSION in workflows

So the builds will still happen from this phantom commit, but we will report the PR commit.

[github-actions]

1028 tests run: 976 passed, 0 failed, 52 skipped (full report)

---

_{The comment gets automatically updated with the latest test results
c1f06c2 at 2023-07-10T15:57:53.118Z :recycle:}

[koivunej]

Logs in the test report have

2023-07-05T16:56:09.452254Z  INFO version: git:7b39653c4ad27b8b705e8bc0369dee8bdb8001ec failpoints: true, features: ["testing"] launch_timestamp: 2023-07-05 16:56:09.448570616 UTC

7b39653c4ad27b8b705e8bc0369dee8bdb8001ec is the phantom merge?

[bayandin]

Logs in the test report have

2023-07-05T16:56:09.452254Z  INFO version: git:7b39653c4ad27b8b705e8bc0369dee8bdb8001ec failpoints: true, features: ["testing"] launch_timestamp: 2023-07-05 16:56:09.448570616 UTC

7b39653 is the phantom merge?

While building Neon, getting sha from git itself takes precedence over GIT_VERSION env var and by default actions/checkout checks out exactly this phantom merge commit.
But the actions is configurable enough https://github.com/actions/checkout#checkout-pull-request-head-commit-instead-of-merge-commit

[koivunej]

While building Neon, getting sha from git itself takes precedence over GIT_VERSION env var and by default actions/checkout checks out exactly this phantom merge commit.
But the actions is configurable enough https://github.com/actions/checkout#checkout-pull-request-head-commit-instead-of-merge-commit

True, but that would give us a chance of merge skew.

Hmm... I think we could just let the PR branches commit info be the phantom commits; at least that would be always real and not fake. Allure reports will have a path designating the PR so I didn't really see clearly yesterday what would be the benefit.

I don't really understand the docker changes either, do we really need them? As long as deployed versions (and docker versions pushed to hub) have proper git commits it is very much a good situation.

[bayandin]

True, but that would give us a chance of merge skew.

Do you mean some cases when we don't set it (forget to set it)?

Hmm... I think we could just let the PR branches commit info be the phantom commits; at least that would be always real and not fake. Allure reports will have a path designating the PR so I didn't really see clearly yesterday what would be the benefit.

I don't really understand the docker changes either, do we really need them? As long as deployed versions (and docker versions pushed to hub) have proper git commits it is very much a good situation.

The original problem described in #3619 doesn't exist anymore because we build and deploy staging and prod from branches only.
But I'd like to have this fixed for PRs also, just in case we're changing something and suddenly not getting the correct git-env version. Also, it's nice to have a consistent version across all the logs; it's pretty hard to find to a branch/PR which contains this phantom merge commit.

[koivunej]

We had a call, discussed if this phantom merge is really needed. I want to keep the phantom merge commit, because I think it will give some protection against merge skew, assuming our engineers are not trying to game the system by merging very old unconflicting but successful in the past PRs.

However I agree it's helpful to have the real commits in logs, because logs could be looked at much later. We landed on "win-win" solution where we'll use $GIT_VERSION when it is set, and otherwise use the git_version::git_version!. This will give us:

• on PR branches, one can correlate allure logs to actual commits, if there was ever any confusion
• on local dev boxes, git version will be used (as expected, because we expected no one is running out with GIT_VERSION set)

[koivunej]

Drafting while test failures, and I reverted one too many commit.

[koivunej]

Okay, latest logs have:

# pageserver
2023-07-07T09:24:10.660538Z  INFO version: git-env:d1738d0896b1a138a1fe0da6a7a04a2eb1626ae1 failpoints: true, features: ["testing"] launch_timestamp: 2023-07-07 09:24:10.657548402 UTC

# safekeeper, storage broker
2023-07-07T09:24:58.572133Z  INFO version: git-env:d1738d0896b1a138a1fe0da6a7a04a2eb1626ae1
2023-07-07T09:24:58.687562Z  INFO version: git-env:d1738d0896b1a138a1fe0da6a7a04a2eb1626ae1

Initially copied some previous run via flaky tests, commit d1738d0896b1a138a1fe0da6a7a04a2eb1626ae1 is as expected but now delivered via GIT_VERSION env var.

Noted that compute_ctl does not have this kind of logging, no version at all. I think it should be added. Well, it has some build_tag, unsure what that is, leaving this from this PR.

[koivunej]

After latest:

2023-07-07T09:52:55.902094Z  INFO version: git-env:4dc3643eeeff493ab978604c43367e5343154907 failpoints: true, features: ["testing"] launch_timestamp: 2023-07-07 09:52:55.896253737 UTC

Which is 4dc3643eeeff493ab978604c43367e5343154907 as in the last pushed commit. Great.

[koivunej]

Removed the automatically added reviewes because I worked away from all those changes.