Skip to content

fix(build): do not store a failed rev-parse in safe.directory - #374

Open
o-mid wants to merge 1 commit into
UNAWatch:mainfrom
o-mid:fix/una-version-safe-directory
Open

o-mid wants to merge 1 commit into
UNAWatch:mainfrom
o-mid:fix/una-version-safe-directory

Conversation

@o-mid

@o-mid o-mid commented Oct 31, 2026 •

Copy link
Copy Markdown

Summary

una-version.sh treated every failed git rev-parse as dubious ownership, including fatal: not a git repository. Configuring a copied example (the path in Docs/sdk-setup.md) then wrote that error text into the global safe.directory.

Only an ownership failure (dubious ownership or unsafe repository) touches safe.directory now. If git rev-parse --show-toplevel still fails after that, the error text is not stored. Version parsing is unchanged.

Root cause

Utilities/Scripts/build-cube/una-version.sh matched fatal: in the same branch as the ownership errors, then ran git config --global safe.directory with the failed --show-toplevel output. A copied app directory is not a git repository, so that was the normal result, not an ownership problem.

Test plan

  • bash Utilities/Scripts/build-cube/test_una_version.sh before the change exited 1. With GIT_CONFIG_GLOBAL pointed at a temp file, safe.directory was set to fatal: not a git repository (or any of the parent directories): .git followed by fatal: git rev-parse failed.
  • The same command after the change exited 0. safe.directory was unset, and stdout contained BUILD_VERSION=0.0.0-dev.
  • The same script, against this checkout, with an isolated GIT_CONFIG_GLOBAL, printed BUILD_VERSION=1.6.2 (git describe --exact-match is sdk-v1.6.2 on this commit's parent) and did not set safe.directory.
  • ctest --test-dir build-host --output-on-failure — una-sdk-host-tests passed. Those tests do not run this script.
  • The dubious-ownership path was not executed here. There is no container run in this check.
  • The apps CI prepare job runs bash Utilities/Scripts/build-cube/test_una_version.sh.

una-version.sh treated every fatal git rev-parse as dubious ownership.
A copied example is not a repository, so the script wrote the error
text into the global safe.directory. Only an ownership failure touches
that setting, and a failed toplevel is not stored.
Copilot AI balanced review requested due to automatic review settings October 31, 2026 10:25
@coderabbitai

coderabbitai Bot commented Oct 31, 2026

Copy link
Copy Markdown

Warning

Review limit reached

This review ran on the open-source allowance, not this organization's plan, because the pull request author doesn't have an assigned seat. Waiting won't change this — ask an organization admin to assign them a seat, or add seats in Billing if every seat is already assigned, then retry.

Next included review available in 51 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 6bd43f0 and 4c6a4c6.

Files selected for processing: 3
  • .github/workflows/apps-ci.yml
  • Utilities/Scripts/build-cube/test_una_version.sh
  • Utilities/Scripts/build-cube/una-version.sh
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants