Repository navigation
feat: keep the diagram on a baseline branch of its own #142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feat/ancestor-base-seed
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -143,9 +143,13 @@ inputs: | |
| required: false | ||
| default: ${{ github.token }} | ||
| sync_strategy: | ||
| description: 'Sync delivery method: push or pull_request.' | ||
| description: 'Sync delivery method: push, pull_request, or branch (an orphan branch of its own, see baseline_branch).' | ||
| required: false | ||
| default: 'push' | ||
| baseline_branch: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe the word baseline is not very intuitive for our users as we call this syncing, so I suppose maybe we should name it syncing_branch |
||
| description: 'Branch that sync_strategy branch keeps the analysis on. Reviews read it too.' | ||
| required: false | ||
| default: 'codeboarding/baseline' | ||
| target_branch: | ||
| description: 'Branch updated by sync mode. Defaults to the event branch.' | ||
| required: false | ||
|
|
@@ -223,6 +227,7 @@ runs: | |
| HEAD_AUTHOR_EMAIL: ${{ github.event.head_commit.author.email }} | ||
| TARGET_BRANCH_INPUT: ${{ inputs.target_branch }} | ||
| SYNC_STRATEGY: ${{ inputs.sync_strategy }} | ||
| BASELINE_BRANCH: ${{ inputs.baseline_branch }} | ||
| COMMENT_BODY: ${{ github.event.comment.body }} | ||
| AUTHOR_ASSOCIATION: ${{ github.event.comment.author_association }} | ||
| ISSUE_PR_URL: ${{ github.event.issue.pull_request.url }} | ||
|
|
@@ -451,6 +456,8 @@ runs: | |
| CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target | ||
| STAGE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/out | ||
| FORCE_FULL: ${{ inputs.force_full }} | ||
| SYNC_STRATEGY: ${{ inputs.sync_strategy }} | ||
| BASELINE_BRANCH: ${{ inputs.baseline_branch }} | ||
| CFG_HASH: ${{ steps.state.outputs.cfg_hash }} | ||
| # Lets a branch without a usable committed baseline catch up from a saved analysis. | ||
| ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }} | ||
|
|
@@ -475,6 +482,8 @@ runs: | |
| TARGET_BRANCH: ${{ steps.guard.outputs.target_branch }} | ||
| SYNC_BRANCH_START_SHA: ${{ steps.guard.outputs.sync_branch_start_sha }} | ||
| SYNC_STRATEGY: ${{ inputs.sync_strategy }} | ||
| BASELINE_BRANCH: ${{ inputs.baseline_branch }} | ||
| ENGINE_VERSION: ${{ steps.state.outputs.engine_version }} | ||
| GITHUB_TOKEN: ${{ inputs.github_token }} | ||
| GH_TOKEN: ${{ inputs.github_token }} | ||
| GH_ENTERPRISE_TOKEN: ${{ inputs.github_token }} | ||
|
|
@@ -550,6 +559,7 @@ runs: | |
| ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }} | ||
| # For rewriting the progress comment while a base is built from scratch. | ||
| PROGRESS_HEADER: ${{ steps.guard.outputs.comment_id }} | ||
| BASELINE_BRANCH: ${{ inputs.baseline_branch }} | ||
| BASE_REF: ${{ steps.guard.outputs.base_ref }} | ||
| REPOSITORY: ${{ github.repository }} | ||
| GH_HOST: ${{ github.server_url }} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| { | ||
| "name": "CodeBoarding baseline branch", | ||
| "target": "branch", | ||
| "enforcement": "active", | ||
| "conditions": { | ||
| "ref_name": { | ||
| "include": ["refs/heads/codeboarding/baseline"], | ||
| "exclude": [] | ||
| } | ||
| }, | ||
| "rules": [ | ||
| { "type": "deletion" }, | ||
| { "type": "non_fast_forward" } | ||
|
Comment on lines
+11
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In repositories where contributors may push branches but the main branch requires reviewed PRs, this ruleset still lets any contributor create or fast-forward Useful? React with 👍 / 👎. |
||
| ], | ||
| "bypass_actors": [ | ||
| { | ||
| "actor_id": 4021464, | ||
| "actor_type": "Integration", | ||
| "bypass_mode": "always" | ||
| } | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,6 +126,16 @@ analyze_sync() { | |
|
|
||
| REQUIRES_FULL=true | ||
| if [ "$(printf '%s' "${FORCE_FULL:-false}" | tr '[:upper:]' '[:lower:]')" != true ]; then | ||
| # The baseline branch's tip is this branch's last analysis. Without one, the | ||
| # run below seeds from a saved ancestor or analyzes in full, and delivery | ||
| # creates the branch again. | ||
| if [ "${SYNC_STRATEGY:-}" = branch ]; then | ||
| local tip_entry | ||
| tip_entry="$(baseline_index "${REPOSITORY:-}" | awk '{print $1; exit}')" | ||
| if [ -z "$tip_entry" ] || ! restore_baseline "${REPOSITORY:-}" "$tip_entry" "$state"; then | ||
| echo "::notice::$BASELINE_BRANCH has no analysis to continue from; this sync creates it." | ||
| fi | ||
| fi | ||
| if [ "$(depth_cap_from "$state/analysis.json")" = "$DEPTH_CAP" ]; then | ||
| incremental "$CHECKOUT_DIR" "$state" | ||
| fi | ||
|
|
@@ -264,6 +274,54 @@ keep_user_config() { | |
| done | ||
| } | ||
|
|
||
| # sync_strategy: branch keeps one commit per sync on BASELINE_BRANCH, each with a | ||
| # CodeBoarding-Source trailer naming the commit it analysed. Lists them as | ||
| # "<branch commit> <source sha>", newest first, at most BASELINE_DEPTH of them. | ||
| # Fetched without blobs into a scratch repository: the lookup needs messages, and | ||
| # a hundred pickles would cost more than it saves. | ||
| BASELINE_DEPTH="${BASELINE_DEPTH:-100}" | ||
| baseline_index() { | ||
| local repository="$1" scratch="$RUNNER_TEMP/codeboarding-baseline-index.git" auth | ||
| [ -n "${BASELINE_BRANCH:-}" ] || return 0 | ||
| rm -rf "$scratch" | ||
| git init -q --bare "$scratch" | ||
| auth="$(printf 'x-access-token:%s' "${GIT_TOKEN:-}" | base64 -w0)" | ||
| git -C "$scratch" -c "http.extraheader=AUTHORIZATION: basic $auth" fetch -q --filter=blob:none \ | ||
| --depth="$BASELINE_DEPTH" "${GITHUB_SERVER_URL%/}/${repository}.git" "refs/heads/$BASELINE_BRANCH" 2>/dev/null || | ||
| return 0 | ||
| git -C "$scratch" log --format='%H %(trailers:key=CodeBoarding-Source,valueonly,separator=%x20)' FETCH_HEAD | | ||
| awk 'NF == 2' | ||
| } | ||
| # Lays a baseline-branch commit's analysis over $3, which keeps the configuration | ||
| # seeded from the checkout. source.json is provenance, not engine state. | ||
| restore_baseline() { | ||
| local repository="$1" commit="$2" state="$3" scratch="$RUNNER_TEMP/codeboarding-baseline-restore" | ||
| fetch_commit "$repository" "$commit" || return 1 | ||
| rm -rf "$scratch" | ||
| mkdir -p "$scratch" "$state" | ||
| git -C "$CHECKOUT_DIR" archive "$commit" .codeboarding | tar -x -C "$scratch" || return 1 | ||
| rm -f "$scratch/.codeboarding/source.json" | ||
| cp -a "$scratch/.codeboarding/." "$state/" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a repository switches from Useful? React with 👍 / 👎. |
||
| } | ||
| # Seeds $3 from the baseline branch's entry for $2, or for its nearest first-parent | ||
| # ancestor that has one. Sets BRANCH_SOURCE to the commit it describes and | ||
| # BRANCH_DISTANCE to how far below $2 that is. | ||
| BRANCH_SOURCE="" BRANCH_DISTANCE="" | ||
| seed_from_baseline_branch() { | ||
| local repository="$1" tip="$2" state="$3" index commit entry="" distance=0 | ||
| BRANCH_SOURCE="" BRANCH_DISTANCE="" | ||
| index="$(baseline_index "$repository")" | ||
| [ -n "$index" ] || return 1 | ||
| fetch_commit "$repository" "$tip" "$(( CATCHUP_BOUND + 1 ))" || true | ||
| for commit in $(git -C "$CHECKOUT_DIR" rev-list --first-parent --max-count=$(( CATCHUP_BOUND + 1 )) "$tip" 2>/dev/null); do | ||
| entry="$(awk -v source="$commit" '$2 == source {print $1; exit}' <<< "$index")" | ||
| [ -z "$entry" ] || break | ||
| distance=$(( distance + 1 )) | ||
| done | ||
| [ -n "$entry" ] && restore_baseline "$repository" "$entry" "$state" || return 1 | ||
| BRANCH_SOURCE="$commit" BRANCH_DISTANCE="$distance" | ||
| } | ||
|
|
||
| # Rewrites the sticky progress comment while the base is built from scratch. A | ||
| # fork's read-only token makes every call fail, which costs nothing. | ||
| PROGRESS_PID="" | ||
|
|
@@ -318,11 +376,12 @@ analyze_review() { | |
| # needs no engine run at all. Without one, the merge base is checked out and | ||
| # analyzed from whatever baseline the repository committed there. Each path | ||
| # records how the base was obtained, for the comment and the review artifact. | ||
| local base_started base_source=saved base_reason="" base_from_sha="" catchup_commits="" | ||
| local base_started base_source=saved base_reason="" base_from_sha="" catchup_commits="" base_was_published=false | ||
| base_started="$(date +%s)" | ||
| if [ "$(depth_cap_from "${BASE_DIR:-}/analysis.json")" = "$DEPTH_CAP" ]; then | ||
| mkdir -p "$base_state" | ||
| cp -a "$BASE_DIR/." "$base_state/" | ||
| base_was_published=true | ||
| base_from_sha="$REVIEW_BASE_SHA" catchup_commits=0 | ||
| else | ||
| # A bundle under this exact name that the run cannot use was made with another cap. | ||
|
|
@@ -344,6 +403,23 @@ analyze_review() { | |
| elif [ -f "$base_state/analysis.json" ]; then | ||
| base_reason=incompatible | ||
| fi | ||
| # The baseline branch: its entry for the merge base is that commit's own | ||
| # analysis, and an entry for an ancestor is caught up like a committed one. | ||
| if [ "$REQUIRES_FULL" = true ] && seed_from_baseline_branch "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" "$base_state"; then | ||
| if [ "$(depth_cap_from "$base_state/analysis.json")" != "$DEPTH_CAP" ]; then | ||
| base_reason=incompatible | ||
| elif [ "$BRANCH_DISTANCE" -eq 0 ]; then | ||
| REQUIRES_FULL=false base_source=saved base_from_sha="$BRANCH_SOURCE" catchup_commits=0 | ||
|
Comment on lines
+411
to
+412
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the baseline branch contains an entry for the merge base, this path accepts it after checking only Useful? React with 👍 / 👎. |
||
| else | ||
| incremental "$base_checkout" "$base_state" | ||
| if [ "$REQUIRES_FULL" = true ]; then | ||
| base_reason=incompatible | ||
| else | ||
| base_source=ancestor base_from_sha="$BRANCH_SOURCE" | ||
| catchup_commits="$(catchup_count "$BRANCH_SOURCE" "$REVIEW_BASE_SHA")" | ||
| fi | ||
| fi | ||
| fi | ||
| # Nothing at the merge base to grow from: catch up from the nearest saved | ||
| # ancestor, and publish the result under the merge base's own name below. | ||
| if [ "$REQUIRES_FULL" = true ] && seed_from_ancestor "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" "$base_state" false "$base_checkout"; then | ||
|
|
@@ -403,9 +479,10 @@ analyze_review() { | |
| # under the same name every run, so normally only a run that produced one | ||
| # publishes it. The exception is lifetime: a review artifact references a base | ||
| # by id for its whole retention, so one about to expire is renewed rather than | ||
| # left dangling under a review that outlives it. | ||
| # left dangling under a review that outlives it. A base read from the baseline | ||
| # branch is published too: no artifact holds it yet. | ||
| local publish_base=false | ||
| if [ "$base_source" != saved ] || [ "${RENEW_BASE:-false}" = true ]; then | ||
| if [ "$base_was_published" != true ] || [ "${RENEW_BASE:-false}" = true ]; then | ||
| stage "$base_state" base | ||
| publish_base=true | ||
| fi | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,72 @@ classify_push_failure() { | |
| exit 1 | ||
| } | ||
|
|
||
| # sync_strategy: branch keeps the analysis on an orphan branch of its own, one | ||
| # fast-forward commit per sync, and never writes to the target branch. | ||
| deliver_to_baseline_branch() { | ||
| local branch="$BASELINE_BRANCH" tree="$RUNNER_TEMP/codeboarding-baseline-tree" | ||
| local index="$RUNNER_TEMP/codeboarding-baseline-index" git_dir files new_tree tip parent commit now | ||
| git_dir="$(git rev-parse --absolute-git-dir)" | ||
| rm -rf "$tree" "$index" | ||
| mkdir -p "$tree" | ||
| CHECKOUT_DIR="$tree" "$ACTION_PATH/scripts/action/install-sync.sh" > /dev/null | ||
| files="$(find "$tree/.codeboarding" -maxdepth 1 -type f | wc -l | tr -d ' ')" | ||
| # Engine output is never edited; which commit it describes lives here only. | ||
| python3 -c 'import datetime,json,os,sys | ||
| json.dump({ | ||
| "schema": 1, | ||
| "source_branch": os.environ["TARGET_BRANCH"], | ||
| "source_sha": sys.argv[2], | ||
| "generated_at": datetime.datetime.now(datetime.timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), | ||
| "engine_version": os.environ.get("ENGINE_VERSION", ""), | ||
| }, open(sys.argv[1], "w"), indent=2)' "$tree/.codeboarding/source.json" "$BASE_SHA" | ||
| GIT_INDEX_FILE="$index" git --git-dir="$git_dir" --work-tree="$tree" -C "$tree" add -A -f .codeboarding | ||
| new_tree="$(GIT_INDEX_FILE="$index" git --git-dir="$git_dir" write-tree)" | ||
| git config user.name 'codeboarding-review[bot]' | ||
| git config user.email 'codeboarding-review[bot]@users.noreply.github.com' | ||
|
|
||
| # Two tries: a concurrent sync that moved the branch for an older commit is | ||
| # built on top of once. A second move means a newer run is handling it. | ||
| for _ in 1 2; do | ||
| git fetch -q "$REMOTE" "$TARGET_BRANCH" | ||
| if [ "$(git rev-parse FETCH_HEAD)" != "$BASE_SHA" ]; then | ||
| emit_result "$files" false "$BASE_SHA" | ||
| echo "::notice::$TARGET_BRANCH advanced during analysis; a newer run should update $branch." | ||
| exit 0 | ||
| fi | ||
| tip="$(git ls-remote "$REMOTE" "refs/heads/$branch" | awk '{print $1; exit}')" | ||
| parent=() | ||
| if [ -n "$tip" ]; then | ||
| git fetch -q --depth=1 "$REMOTE" "refs/heads/$branch" | ||
|
Comment on lines
+99
to
+102
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If another sync advances the baseline branch after Useful? React with 👍 / 👎. |
||
| parent=(-p "$tip") | ||
|
Comment on lines
+101
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If Useful? React with 👍 / 👎. |
||
| if [ "$(git log -1 --format='%(trailers:key=CodeBoarding-Source,valueonly)' "$tip" | tr -d '[:space:]')" = "$BASE_SHA" ] && | ||
| git diff --quiet -I '"generated_at"' -I '"timestamp"' "$tip" "$new_tree"; then | ||
| emit_result "$files" false "$BASE_SHA" | ||
| echo "::notice::$branch already holds this analysis of $TARGET_BRANCH @${BASE_SHA:0:7}." | ||
| exit 0 | ||
| fi | ||
| fi | ||
| commit="$(git commit-tree "$new_tree" ${parent[@]+"${parent[@]}"} \ | ||
| -m "chore(codeboarding): diagram of $TARGET_BRANCH @${BASE_SHA:0:7}" \ | ||
| -m "CodeBoarding-Source: $BASE_SHA")" | ||
| # Never forced: the parent is the tip just read, so this only ever fast-forwards. | ||
| if git push -q "$REMOTE" "$commit:refs/heads/$branch"; then | ||
|
Comment on lines
+114
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The target branch is checked by a separate fetch at the start of the loop, but this push updates only the baseline ref. If Useful? React with 👍 / 👎. |
||
| emit_result "$files" true "$BASE_SHA" | ||
| echo "baseline_branch_sha=$commit" >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| now="$(git ls-remote "$REMOTE" "refs/heads/$branch" | awk '{print $1; exit}')" | ||
| if [ "$now" = "$tip" ]; then | ||
| echo "::error::GitHub refused the push to $branch, most likely because a branch rule protects it. Add the CodeBoarding app as a bypass actor for $branch in the repository's rulesets, or set sync_strategy: push." | ||
| exit 1 | ||
| fi | ||
| done | ||
| emit_result "$files" false "$BASE_SHA" | ||
| echo "::notice::Another sync keeps updating $branch; leaving it to that run." | ||
| exit 0 | ||
| } | ||
| [ "$SYNC_STRATEGY" != branch ] || deliver_to_baseline_branch | ||
|
|
||
| "$ACTION_PATH/scripts/action/install-sync.sh" > "$GENERATED_PATHS" | ||
| stage_paths=() | ||
| while IFS= read -r path; do | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
target_branch, is this the target in tehms of where the sync will happen or in terms of which branch we will sync with, unsure that wording is clear again.
i think that most of these things will be read by ppl or even more by their agents so proly descriptive and somewhat clear names are worth investing in.