Repository navigation
fix(hrr): do not disable git ownership for the version stamp - #14342
Merged
danielbolsson merged 3 commits intoOct 28, 2026
Merged
Conversation
The stamp wrote a git config that trusted every directory, so a tree the build user does not own could run that repository's commands. Leave the ownership check on and report unknown when git refuses. Signed-off-by: Daniel Olsson <Daniel.Olsson@amd.com>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🚫 Please fix the failed policies before requesting reviews. The following policy checks failed:
The |
andraghetti
reviewed
Oct 27, 2026
andraghetti
left a comment
andraghetti
left a comment
Contributor
There was a problem hiding this comment.
Checked on gbt350: a parent repo with core.fsmonitor runs its command through develop's stamp and not through this one. The MI350 gate matches develop apart from the generated-file drift that #14241 fixed after this branch's base.
…p check by behaviour AIRUNTIME-3091. _out_dir and file(MAKE_DIRECTORY) only served the removed gitconfig; file(WRITE) creates the missing parent itself. The test now runs the stamp with GIT_TEST_ASSUME_DIFFERENT_OWNER=1 and expects unknown, which fails on develop's stamp, and drops the assertion on the header that passed there too.
andraghetti
approved these changes
Oct 28, 2026
danielbolsson
deleted the
users/danielbolsson/hrr-version-stamp-ownership
branch
October 28, 2026 12:23
1 task done
ywang103-amd
pushed a commit
that referenced
this pull request
Oct 28, 2026
JIRA ID : AIRUNTIME-3091 ## Summary - The version stamp wrote a temporary git config with `safe.directory = *` and pointed `GIT_CONFIG_GLOBAL` at it. - Git then trusts every directory, including a repository planted above a source tree the build user does not own, and will run that repository's commands as the builder. - The stamp no longer disables the ownership check. A checkout the builder does not own reports `unknown`. ## Test plan - [x] `python3 projects/hrr/tests/scripts/test_version_stamp.py`: an owned checkout still stamps a revision; with `GIT_TEST_ASSUME_DIFFERENT_OWNER=1` the stamp reports `unknown` (fails on develop's stamp) ## Issue Tracking JIRA ID : AIRUNTIME-3091 --------- Signed-off-by: Daniel Olsson <Daniel.Olsson@amd.com>
1 task done
Contributor
TheRock Submodule Bump ActivityNewest first
|
danielbolsson
added a commit
that referenced
this pull request
Oct 29, 2026
…heck (#14458) JIRA ID : AIRUNTIME-3091 ## Motivation `test_checkout_owned_by_someone_else_reports_unknown` (added by #14342) fails in the `HRR script tests` job on #13256 (run 40687541194): the stamp reports a revision, not `unknown`. That job is not a container job; it runs on a hosted ubuntu-24.04 runner with git 2.56.0 as user `runner`. Git there trusts every directory (a `safe.directory = *` in its system/global config), so `GIT_TEST_ASSUME_DIFFERENT_OWNER=1` has no effect and reading the revision is correct. The test only means something where git enforces the ownership check. ## Technical Details `projects/hrr/tests/scripts/test_version_stamp.py`: probe git once at import, in the checkout the stamp reads (not a throwaway repo, so a checkout trusted through `safe.directory` in the user's git config also skips), with the same environment the test uses. If git does not refuse the repository, `unittest.skipUnless` skips the ownership test. Where git does enforce the check the test is unchanged and still fails against a stamp that bypasses it. The env setup moves into a shared `git_env()` helper. ## Issue Tracking JIRA ID : AIRUNTIME-3091 ## Test Plan - `python3 -m unittest test_version_stamp` with a normal git (test runs, passes). - Same with `HOME` pointing at a `~/.gitconfig` holding `safe.directory = *` (test skipped, owned-repo test passes). - Negative control: add `-c safe.directory=*` to the stamp's `git rev-parse` and run with a normal git (test fails). ## Test Result - Normal git 2.43.0: 2 run, OK. `unittest discover` over `projects/hrr/tests/scripts`: 152 tests OK. - `safe.directory = *` in HOME gitconfig: OK (skipped=1). - Negative control: FAILED (failures=1); change reverted. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#provide-pull-request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA ID : AIRUNTIME-3091
Summary
safe.directory = *and pointedGIT_CONFIG_GLOBALat it.unknown.Test plan
python3 projects/hrr/tests/scripts/test_version_stamp.py: an owned checkout still stamps a revision; withGIT_TEST_ASSUME_DIFFERENT_OWNER=1the stamp reportsunknown(fails on develop's stamp)Issue Tracking
JIRA ID : AIRUNTIME-3091