[ci] Should git-subrepo commits be squashed/in side-history?

[ludamad]

Git subrepo requires tracking commits whenever you push to the other repo (in our case, mirror repos). For barretenberg and docs, a new git subrepo commit is made in master currently. This is straight forward, but perhaps not ideal in that 1. it introduces history noise 2. it commits to master, bypassing branch rules

Options:

• Do nothing. Accept the bot having this privilege, the fact that commits are made automatically, and look at history in a way that doesn't have the noise.
• Use a side history. A side branch e.g. docs-git-subrepo could have a .gitrepo file which is used if it is newer. This would be clean, but perhaps hiding complexity in not the best way if things do go wrong. At least you just have to commit a better .gitrepo to master still?

Pros are it could be cleanest, could roll back aztec bot bypass permissions, and cons it is the most complex/surprising.

• Squash the commit into the commit that caused it to kick off, but this involves some history rewriting at that point and still has the branch protection bypass rules issue. It would be clean but the downside is rewriting history kicks off CI again in a strange way that might confuse tools (unless there's a way I don't know about).

[spalladino]

Sorry, I'm still trying to wrap my head around how this works. So I understand that, whenever we make a change in aztec-packages to the docs folder in master (example here), then the CI automatically pushes the changes to the aztec/docs repo (here), and then pushes a new commit in aztec-packages that modifies the subrepo file (this one) to reflect the new commit on the docs repo.

Is this correct? And the commit we're trying to eliminate is the third one, ie the one that bumps the revision in the git-subrepo file in aztec-packages?

I'm personally not too annoyed by this extra commit, but to be honest I don't understand the side-history approach. As for the squash option, that seems pretty clean. While I'm usually against having the CI rewriting commits (it sucks to push a commit and then immediately see it diverge), this would only affect master, which we should never be pushing to directly. Though I'm not sure how to avoid the duplicate CI run, maybe we can detect it was a force-push with that change and skip the run..?

[ludamad]

Is this correct?

Yes, the commit updates some bookkeeping so it knows what commits to replay in the future. commit is part of the subrepo's corresponding repo to consider part of our local git subrepo tree, and parent is the commit to start replaying changes from (technically, the parent of that commit).

And the commit we're trying to eliminate is the third one, ie the one that bumps the revision in the git-subrepo file in aztec-packages?

Yes. If we had a few of these, it could add up, too (I guess at least those could be squashed together somehow? would need to be one mirror action).

[ludamad]

The side history approach: There's some branch we reference with this file updated, and we merge those locally basically only if they're newer. It works (I have it prototyped) but it feels finnicky, especially it complicates a history rewinding workflow

[ludamad]

Though I'm not sure how to avoid the duplicate CI run, maybe we can detect it was a force-push with that change and skip the run..?

It's a bit icky - we want the CI associated with the new commit hash. I'm also not sure what happens if multiple commits happen in succession (although a merge queue would protect against this)

[spalladino]

I guess it'd be ideal if we could intercept a PR immediately before it gets merged, so we run these actions then. But failing that, I'd vote for option 1 (doing nothing) as it seems to be the less magical one. No strong feelings though, you have the best context here and I trust your judgement if you think 2 or 3 is better.

[ludamad]

I've come to like the explicit commits showing me subrepo action. At least from my POV it keeps me confident the mechanisms are working. Closing until complaints.