ciq-cherry-pick.sh: Some improvements #53
+368
−108
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
roxanan1996
force-pushed
the
{rnicolescu}_cherry_pick_fixes
branch
from
3324b9c to
b252a60
Compare
…are committed If the commit that needs to be cherry picked has "Fixes:" references in the commit body, there is now a check in place that verifies if those commits are present in the current branch. At the moment, the script returns an Exception because the developer must check why the commit has to be cherry picked for a bug fix or cve fix if the actual commit that introduced the bug/cve was not commited. If the commit does not reference any Fixes:, an warning is shown to make the developer aware that they have to double check if it makes sense to cherry pick this commit. The script continues as this can be reviewed after. This is common in the linux kernel community. Not all fixes have a Fixes: reference. Checking if a commit is part of the branch has now improved. It checks if either the commit was backported by our team, or if the commit came from upstream. Note: The implementation reuses some of the logic in the check_kernel_commits.py. Those have been moved to ciq_helper.py. This commit address the small refactor in check_kernel_commits.py as well. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
It now automatically cherry picks the Fixes: dependencies. To accomodate this, CIQ_find_mainline_fixes was moved to ciq_helpers. And an extra argument for upstream-ref was introduced, the default being origin/kernel-mainline, as the dependencies are looked up there. To simplify things and keep main cleaner, separate functions were used. If one of the commits (the original and its dependencies) cannot be applied, the return code will be 1. This is useful when ciq-cherry-pick.py is called from other script. This also removed redundant prints. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
…f conflict If the conflict is solved manually, then developer use ``` git commit ``` which will use the MERGE_MSG file for the commit message by default. Equivalent of ``` git commit -F MERGE_MSG ``` Therefore it makes sense to include "upstream-diff |" in the MERGE_MSG. In case the conflict is solved by cherry picking other commits, ciq-cherry-pick.py will be called again and MERGE_MSG will be rewritten, including the "upstream-diff" part. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
All exceptions are propapagated to main and handled there, except for CherryPickException that is raised only if cherry pick failed due to conflicts. This way user is not overwhelmed with a stacktrace when the error is obvious. For unexpected errors, the full stacktrace is shown. Also added some comments to full_cherry_pick function to explain that if one of the cherry pick fails, the previous successful ones are left intact. This can be improved in the future by making it interactive. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
…s are not in the tree This can be useful in cases where there are multiple commits this one is trying to fix, and only one if of interest for us. Signed-off-by: Roxana Nicolescu <rnicolescu@ciq.com>
roxanan1996
force-pushed
the
{rnicolescu}_cherry_pick_fixes
branch
from
b252a60 to
320c7d2
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
It adds extra fixes to the way we cherry pick commits.
check if the commit fixes another commit(s) and if so, it check if those commits were actually commited beforehand. By either a ciq backport or by rebasing from upstream.
If those commits are not in the tree, the script will stop execution. There is an option to bypass this in case we still want the commit we want to cherry pick is still needed.
As seen when check_kernel_commits is run, some commits require extra fixes themselves. We tag them as cve-bf.
Those are now automatically cherry-picked.
In case of conflicts, upstream-diff is added automatically to the merge commit in MERGE_MSG. If the commit is solved manually, the developer can just add their comments. If the commit is solved by cherry-picking some other commits, the old commit message in MERGE_MSG will be overridden and the developer will call the ciq-cherry-pick.sh again, so it does not matter.
Notes
The code is not in perfect state as I would use gitpython in the future and some other things. But it works well and it removed duplicates between ciq-cherry-pick.sh and check_kernel_commits.sh
In case one of the cherry picks fail, the scripts returns right away, without reverting the previous successful cherry pick (if any). This is intentional at the moment since it makes the logic more complicated. And in case we do this in a batch, it's quickly fixed by using an intermediate branch.
To avoid these intermediate states in the future, an interactive approach will be implemented, but this will be done in a latter merge request.
This is a small change but I feel it had a big impact on my workflow. Before this, I had to run the check_kernel_commits script to see if there's extra commits, then cherry pick then realize they may not apply cleanly.
Now, it's automatically done here.
I also noticed that people realize the missing dependencies after the pull request is created and they have to spend extra time updating the merge request.
More changing are coming, this is just an improvement.