sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.1k stars 1.28k forks source link

"hello world" spec fails to create ref for branch `${{ batch_change.name }}` if name does not comply with ref name spec #53119

Open courier-new opened 1 year ago

courier-new commented 1 year ago

The regex expression for a valid git ref name is kinda disgusting and based on a very complex spec.

In our starter hello-world.spec.yaml, we set the batch change name to be the branch name by default:

branch: ${{ batch_change.name }}

However, there's actually no guarantee that the name a user picked for their Batch Change is actually compliant with the spec for git branch ref names. For example, we allow .s in the batch change name. While . characters are allowed in certain settings, but they can't follow a / or another . according to the spec.

This yields a confusing error in the UI because gitserver fails to push the commit so there's no ref to create the changeset from:

Image

But you can see what actually went wrong from closer inspection in the gitserver logs:

ERROR gitserver.createCommitFromPatch server/patch.go:316 Failed to push {"repo": "github.com/sourcegraph-testing/batch-changes-test-repo", "baseCommit": "c5342f15f19494d9eaab66c9406495edf3ad73db", "targetRef": "refs/heads/bad-bc-name...", "commit": "f462a5fda37ead92c8b870ca79b6a4d7ee12fcf6", "output": "fatal: invalid refspec 'f462a5fda37ead92c8b870ca79b6a4d7ee12fcf6:refs/heads/bad-bc-name...'\n"}

Namely that fatal: invalid refspec part.

We ought to handle this a bit more gracefully, by doing one or multiple of a couple things:

github-actions[bot] commented 1 year ago

Hey, @sourcegraph/batchers (@eseliger @courier-new @adeola-ak @BolajiOlajide @st0nebraker @malomarrec @chrispine @danielmarquespt) - we have been mentioned. Let's take a look.