Merge ~canonical-kernel-team/ubuntu-cve-tracker:kcve-merge-separate into ubuntu-cve-tracker:master

Proposed by Yuxuan Luo
Status: Merged
Merged at revision: 5243344f1f03c57e661ee0da0c4190790479caa3
Proposed branch: ~canonical-kernel-team/ubuntu-cve-tracker:kcve-merge-separate
Merge into: ubuntu-cve-tracker:master
Diff against target: 38 lines (+10/-14)
1 file modified
scripts/kcve-merge (+10/-14)
Reviewer Review Type Date Requested Status
Seth Arnold Approve
Rodrigo Figueiredo Zaiden Approve
Review via email: mp+460198@code.launchpad.net

Commit message

In the previous approach, kcve-merge format one single commit gathering all the changes if multiple CVE IDs are given, which is not friendly for reviewing. This patch change the behavior to always generating separate patches for each CVE.

To post a comment you must log in.
Revision history for this message
Rodrigo Figueiredo Zaiden (rodrigo-zaiden) wrote :

I haven't really used the script, but I have played with it before and also
again with this proposed change.
Anyway, I'm totally onboard for single commits per CVE for a better picture
(and control) of what is being changed.
It also helps me, personally, when reviewing what is being changed with the
autotriage.

Appreciate the changes!

review: Approve
Revision history for this message
Seth Arnold (seth-arnold) wrote :

This looks like a good idea to me, thanks.

review: Approve
Revision history for this message
Steve Beattie (sbeattie) wrote :

On Wed, Feb 07, 2024 at 05:30:33PM -0000, Yuxuan Luo wrote:
> Commit message:
> In the previous approach, kcve-merge format one single commit gathering all the changes if multiple CVE IDs are given, which is not friendly for reviewing. This patch change the behavior to always generating separate patches for each CVE.

LGTM, merged.

One minor point, not even a nit and not introduced by this merge
request:

> + try:
> + cve_lib.git_commit("kernel/" + cve_path.split(sep="/")[-1] + ": autotriage")

os.path.basename(cve_path) could be used instead of
cve_path.split(sep="/")[-1] to identify the CVE for the commit
message subject.

--
Steve Beattie
<email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/scripts/kcve-merge b/scripts/kcve-merge
index 9897b1d..3fd3de7 100755
--- a/scripts/kcve-merge
+++ b/scripts/kcve-merge
@@ -284,23 +284,19 @@ def main():
284 continue284 continue
285 print(line, end='')285 print(line, end='')
286286
287 # Prepare for git push287 # Prepare for git push
288 git_status = subprocess.run(["git", "status", "--short"], capture_output=True).stdout.decode()288 git_status = subprocess.run(["git", "status", "--short"], capture_output=True).stdout.decode()
289 if len(git_status) == 0:289 if len(git_status) == 0:
290 warn('No changes being made')290 warn('No changes being made')
291 else:291 else:
292 for cve_num in cve_path_list:
293 try:292 try:
294 cve_lib.git_add(cve_num)293 cve_lib.git_add(cve_path)
295 except ValueError as e:294 except ValueError as e:
296 error(e)295 error(e)
297 try:296 try:
298 if len(cve_path_list) == 1:297 cve_lib.git_commit("kernel/" + cve_path.split(sep="/")[-1] + ": autotriage")
299 cve_lib.git_commit("kernel/" + cve_path_list[0].split(sep="/")[-1] + ": autotriage")298 except ValueError as e:
300 else:299 error(e.args[0])
301 cve_lib.git_commit("kernel: autotriage")
302 except ValueError as e:
303 error(e.args[0])
304300
305301
306if __name__ == '__main__':302if __name__ == '__main__':

Subscribers

People subscribed via source and target branches