Added merge helper script - #265
Conversation
arthurp
left a comment
There was a problem hiding this comment.
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:
- Create the "upstream reference fix" commit which just changes all image and repo references blindly.
- Perform
git merge - Revert the "upstream reference fix" changes, during the syntactic merge.
- Finish the syntactic merge and commit it.
- 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?
| 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 |
There was a problem hiding this comment.
I don't understand what's going on here.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
For context on the misc changes: |
076ea3d to
8aa7ffc
Compare
arthurp
left a comment
There was a problem hiding this comment.
A couple of small changes. Then go ahead and merge.
| @@ -0,0 +1,186 @@ | |||
| import argparse | |||
| # 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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Rename this to merge_upstream.py
|
Oh. And run |
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.