Skip to content

Added merge helper script - #265

Open
mitchell-fanger wants to merge 4 commits into
mainfrom
mootch/merge-helpering
Open

Added merge helper script#265
mitchell-fanger wants to merge 4 commits into
mainfrom
mootch/merge-helpering

Conversation

@mitchell-fanger

Copy link
Copy Markdown
Collaborator

I added a script to assist with the upstream merge process. The script creates a commit which replaces all ldosproject/asterinas docker image names with the upstream asterinas/asterinas names so they don't show up as merge conflicts.

@mitchell-fanger
mitchell-fanger requested a review from a team as a code owner July 22, 2026 01:00

@arthurp arthurp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm sorry to do this, but there are enough intermediates and complexities that I think this needs to be in python. To understand this, someone would need to know sed, awk, printf, IFS.

I'm also not sure why you need to parse the diff and then generate the fixup commit. You could just changes ALL image/repo names and then change them back, instead of dealing with the dill.

Overall, this doesn't work how I expected really. I expected the following steps in the merge:

  1. Create the "upstream reference fix" commit which just changes all image and repo references blindly.
  2. Perform git merge
  3. Revert the "upstream reference fix" changes, during the syntactic merge.
  4. Finish the syntactic merge and commit it.
  5. Somehow squash the commit from step 1 into the merge commit.

Step 5 is actually pretty hard I think. I think the easiest way to do it is actually to perform a new merge (without the "fix" commit from step 1) and then restore the merge state to the completed merge from step 4 and then commit that.

The script should be able to do steps 1-3 in one invocation, and then have another call which does step 5.

Does this all makes sense? Is it what you were thinking?

Comment thread tools/merge.sh Outdated
Comment on lines +29 to +35
elif [[ $line == *"ldosproject/asterinas:"* ]]; then
saw_ldos_tag="true"
ldos_tag=$(echo "$line" | sed -E 's^[[:space:]]*\+[[:space:]]*^^')
elif [[ $saw_ldos_tag == "true" && $line == *"asterinas/asterinas:"* ]]; then
saw_ast_tag="true"
ast_tag=$(echo "$line" | sed -E 's^[[:space:]]*\+[[:space:]]*^^')
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't understand what's going on here.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

So I was parsing the diff to grab only the image names that result in merge conflicts... I was thinking there could be image names that don't conflict, so we wouldn't want to overwrite those? Though I guess since we'll be reverting this commit anyway it wouldn't really matter... Also, I think I was originally thinking that we had to track what we changed so we could change it back to the right value later, but since we can just revert the commit I think we don't need to...

Also, this just grabs our ldos image name and then asterinas image name from the diff. This just is so we only pick up pairs, because I assume a single ldos image name without an asterinas image name isn't a conflict.

Also I think the steps you laid out make sense... Give me some time to think about it and I'll see what I can do... Just for clarification, what step would you be expected to manually resolve the real merge conflicts? Step 4 I think?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yea. Step 4 is the manual syntactic merge.

One notable thing is that I would prefer that this catches new references to the asterinas images and repos. Actually that implies that we might want to run a revert script instead of actually reverting the commit directly. That way any new references are also converted to the ldos equivalents.

@mitchell-fanger

mitchell-fanger commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator Author

For context on the misc changes:
devcontainer.template.json: seems like the image id update wasnt applied to this file, but now it should be good
kata.md: I think this had the wrong image id, unless there was a specific reason it doesn't match upstream. I changed this because it was getting replaced by the wrong image name and just caused another conflict. Let me know if you need me to change it back.
benchmark_x86.yml: There was a weird space here that was causing a merge conflict, so I removed it.

@mitchell-fanger
mitchell-fanger force-pushed the mootch/merge-helpering branch from 076ea3d to 8aa7ffc Compare July 29, 2026 20:09

@arthurp arthurp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A couple of small changes. Then go ahead and merge.

Comment thread tools/merge.py
@@ -0,0 +1,186 @@
import argparse

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add #! ...

Comment thread tools/merge.py
Comment on lines +5 to +17
# run to start a git merge:
# python3 tools/merge.py --start
# this will replace all ldos image id references with the asterinas version, along with a single github.com link!
# this avoids ~35 merge conflicts?

# after resolving all real merge conflicts and committing the changes, run:
# python3 tools/merge.py --finish
# this will squash all changes this script made into the merge commit

# to abort a merge, run this:
# python3 tools/merge.py --abort
# this will abort the git merge and reset your working tree to before
# the merge.py commit

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Make this appear in the argparse documentation. You can just copy it there, or you can put it in a doc string and reference it there. I think Yingqi did that in mariposa-cli. I think it's a pretty nice way to do it.

Comment thread tools/merge.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rename this to merge_upstream.py

@arthurp

arthurp commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Oh. And run black (the python formatter) on everything. I'm adding the requirement that it be used, so it would simplify things just to do it now.

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