Skip to content

chore: avoid squashing patch files into a single file#12772

Closed
TomAFrench wants to merge 3 commits intonextfrom
tf/preserve-patch-files
Closed

chore: avoid squashing patch files into a single file#12772
TomAFrench wants to merge 3 commits intonextfrom
tf/preserve-patch-files

Conversation

@TomAFrench
Copy link
Member

Having a single monolithic patch file discourages us from breaking patches down into self-contained units which we can then easily send back to the main noir repo and then remove again.

This PR replaces noir-repo.patch with the patches directory. As a bonus I've completely removed the fixup script as after #12771 it can be replaced with a simple patch.

# to push the branch without the fixup commit. In that case we'd need to do a fresh
# checkout to re-apply the fixup and the patches.
if [ $have == true ] && has_fixup_and_patch; then
if [ $have == true ] && has_patch_commit; then
Copy link
Member Author

Choose a reason for hiding this comment

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

We can probably avoid the empty patch commit by just searching for the commit message in the last patch file.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't necessarily have to have a patch file, right? I think the empty commit is easy to understand, and easy to see that it finished doing it without having to look into the patches directory when you're just looking at the git log.

Copy link
Contributor

Choose a reason for hiding this comment

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

Another reason is that it makes it easy to see which commit is the first one that I added on top of the local patches, when I want to rebase my feature branch.

fixup=$(find_fixup_commit)
[ -z "$fixup" ] && return 1
git -C noir-repo rev-parse "$fixup~1"
git -C noir-repo rev-list -n 1 $(read_wanted_ref)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this command do exactly?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like if I invoke it with a branch name it gives me the last commit of that branch. What if I'm on a feature branch, like when I was working on af/msgpack-codegen, and have the following commits in my history: origin/af/msgpack-codegen; patch1; patch2; patch-marker; commit1; commit2. Wouldn't it return commit2?

Copy link
Member Author

@TomAFrench TomAFrench Apr 8, 2025

Choose a reason for hiding this comment

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

From git rev-list --help

List commits that are reachable by following the parent links from the given commit(s)

I'm limiting this command to only return 1 commit hash so it will always return the commit hash which the tag returned from $(read_wanted_ref)` resolves to.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using this as a method so that I can unify branches/tags/commit hashes/etc into a commit hash.

@TomAFrench TomAFrench changed the base branch from master to next June 5, 2025 15:16
@TomAFrench TomAFrench closed this Jan 27, 2026
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