Re: [PATCH qa-frontpage] Apply incoming patches onto the correct feature branch
Christopher Baines writes: > Vivien Kraus writes: > >> It seems that the only available information of the feature branch is >> in the patch name. >> --- >> >> Hello guix! >> >> Thank you for making the QA tool. It seems to me that feature branches >> are ignored for patches. The patches seem to always be applied on top >> of master. I get that idea because the base-for-issue-* tag is always >> put on HEAD, and all my patches targetting gnome-team recently get >> applied to master. I understand that the latter could be a problem >> with me. >> >> If patches are indeed applied on top of master, then the question >> arises, what to do. The available information from patchwork is >> scarce: the base-commit is not available, and the optional feature >> branch only appears in the name of the patches of the series. >> >> I think it is possible to parse the patch names. This way, we get some >> useful information: the feature branch, the series revision, and the >> patch index. The patch index can be used to make sure the patches are >> in correct order (this can be the case if a server in the path >> re-orders the emails). >> >> What do you think? > > Hi Vivien, > > Thanks for trying to implement this and sending a patch, I think it's a > good feature to add but there's a few things needed to make this work. > > Firstly, you need to actually change which branch the patch is applied > to, and the place to do that is here [1] in the call to > with-git-worktree. You can probably use get-commit from > (guix-qa-frontpage git-repository) to find the commit hash for a > branch. You'll also probably need to move around some of the code in > create-branch-for-issue so that you have the data about the patches to > work out what the desired branch is before you call with-git-worktree. > > 1: > https://git.cbaines.net/guix/qa-frontpage/tree/guix-qa-frontpage/manage-patch-branches.scm#n250 > > The second thing is that there's other places in the codebase that > assume the patches have been applied to the master branch. In > start-manage-patch-branches-thread, there's > get-changes-compared-to-master which is used to decide if a branch needs > recreating if there's too many changes between the base revision and a > recent master branch revision. That'll need changing to at least skip > over any patches been applied to a non-master branch, or perform the > comparison against the tip of the relevant branch. > > The third thing that will need to change to allow this to happen is > adding clear messaging on to the issue page to indicate where patches > have been applied to non-master branches. That'll help to avoid people > merging these patches in to the master branch rather than the branch > they were intended for. > > Maybe what would be helpful is a procedure in (guix-qa-frontpage issue) > that takes the data for the issue from latest-patchwork-series-by-issue, > and gives you back the branch name that the patches should be applied > to. That can then be used to get the branch information for all the > above 3 use cases. > > As for trying to order the patches, do you know of a case where the > ordering from Patchwork is wrong? I think it's only worth changing the > ordering in the qa-frontpage if we know we're doing it for a reason. > > There's now a create-issue-branch command to the guix-qa-frontpage > script as well, which will be a good way of testing the code out for > applying patches to various branches. It'll fail when it comes to > pushing the branch, but it'll still be useful for testing the code up to > that point. > > Would you be able to look at sending some revised patches? I've now gone ahead and applied a slimmed down version of this patch, and made the other changes to get a basic form of applying patches to specific branches in place. There's probably going to be some problems and more error handling to add, but this should be a good starting point. Thanks, Chris signature.asc Description: PGP signature
Re: [PATCH qa-frontpage] Apply incoming patches onto the correct feature branch
Vivien Kraus writes: > It seems that the only available information of the feature branch is > in the patch name. > --- > > Hello guix! > > Thank you for making the QA tool. It seems to me that feature branches > are ignored for patches. The patches seem to always be applied on top > of master. I get that idea because the base-for-issue-* tag is always > put on HEAD, and all my patches targetting gnome-team recently get > applied to master. I understand that the latter could be a problem > with me. > > If patches are indeed applied on top of master, then the question > arises, what to do. The available information from patchwork is > scarce: the base-commit is not available, and the optional feature > branch only appears in the name of the patches of the series. > > I think it is possible to parse the patch names. This way, we get some > useful information: the feature branch, the series revision, and the > patch index. The patch index can be used to make sure the patches are > in correct order (this can be the case if a server in the path > re-orders the emails). > > What do you think? Hi Vivien, Thanks for trying to implement this and sending a patch, I think it's a good feature to add but there's a few things needed to make this work. Firstly, you need to actually change which branch the patch is applied to, and the place to do that is here [1] in the call to with-git-worktree. You can probably use get-commit from (guix-qa-frontpage git-repository) to find the commit hash for a branch. You'll also probably need to move around some of the code in create-branch-for-issue so that you have the data about the patches to work out what the desired branch is before you call with-git-worktree. 1: https://git.cbaines.net/guix/qa-frontpage/tree/guix-qa-frontpage/manage-patch-branches.scm#n250 The second thing is that there's other places in the codebase that assume the patches have been applied to the master branch. In start-manage-patch-branches-thread, there's get-changes-compared-to-master which is used to decide if a branch needs recreating if there's too many changes between the base revision and a recent master branch revision. That'll need changing to at least skip over any patches been applied to a non-master branch, or perform the comparison against the tip of the relevant branch. The third thing that will need to change to allow this to happen is adding clear messaging on to the issue page to indicate where patches have been applied to non-master branches. That'll help to avoid people merging these patches in to the master branch rather than the branch they were intended for. Maybe what would be helpful is a procedure in (guix-qa-frontpage issue) that takes the data for the issue from latest-patchwork-series-by-issue, and gives you back the branch name that the patches should be applied to. That can then be used to get the branch information for all the above 3 use cases. As for trying to order the patches, do you know of a case where the ordering from Patchwork is wrong? I think it's only worth changing the ordering in the qa-frontpage if we know we're doing it for a reason. There's now a create-issue-branch command to the guix-qa-frontpage script as well, which will be a good way of testing the code out for applying patches to various branches. It'll fail when it comes to pushing the branch, but it'll still be useful for testing the code up to that point. Would you be able to look at sending some revised patches? Thanks, Chris signature.asc Description: PGP signature
[PATCH qa-frontpage] Apply incoming patches onto the correct feature branch
It seems that the only available information of the feature branch is
in the patch name.
---
Hello guix!
Thank you for making the QA tool. It seems to me that feature branches
are ignored for patches. The patches seem to always be applied on top
of master. I get that idea because the base-for-issue-* tag is always
put on HEAD, and all my patches targetting gnome-team recently get
applied to master. I understand that the latter could be a problem
with me.
If patches are indeed applied on top of master, then the question
arises, what to do. The available information from patchwork is
scarce: the base-commit is not available, and the optional feature
branch only appears in the name of the patches of the series.
I think it is possible to parse the patch names. This way, we get some
useful information: the feature branch, the series revision, and the
patch index. The patch index can be used to make sure the patches are
in correct order (this can be the case if a server in the path
re-orders the emails).
What do you think?
Best regards,
Vivien
guix-qa-frontpage/manage-patch-branches.scm | 80 +++--
1 file changed, 76 insertions(+), 4 deletions(-)
diff --git a/guix-qa-frontpage/manage-patch-branches.scm
b/guix-qa-frontpage/manage-patch-branches.scm
index 7f3adb1..0512680 100644
--- a/guix-qa-frontpage/manage-patch-branches.scm
+++ b/guix-qa-frontpage/manage-patch-branches.scm
@@ -102,6 +102,55 @@
(close-pipe pipe)
result))
+(define (parse-patch-name name)
+ (let ((args
+ (and
+ (string-prefix? "[" name)
+ (let ((stop (string-index name #\])))
+(substring name 1 stop
+(as-bug-number
+ (lambda (arg)
+ (and (string-prefix? "bug#" arg)
+(string->number (substring arg (string-length "bug#"))
+(as-v2
+ (lambda (arg)
+ (and (string-prefix? "v" arg)
+(string->number (substring arg 1)
+(as-patch-number
+ (lambda (arg)
+ (match (string-split arg #\/)
+ (((= string->number index) (= string->number total))
+ (and index total (<= index total)
+ (cons index total)))
+ (else #f)
+(let analyze ((bug-number #f)
+ (branch "master")
+ (version 1)
+ (index 1)
+ (total 1)
+ (arguments
+ (if args
+ (string-split args #\,)
+ '(
+ (match arguments
+((or ("") ())
+ `((bug-number . ,bug-number)
+ (branch . ,branch)
+ (version . ,version)
+ (index . ,index)
+ (total . ,total)))
+(((= as-bug-number (? number? new-bug-number))
+ arguments ...)
+ (analyze new-bug-number branch version index total arguments))
+(((= as-v2 (? number? new-version))
+ arguments ...)
+ (analyze bug-number branch new-version index total arguments))
+(((= as-patch-number ((? number? new-index) . (? number? new-total)))
+ arguments ...)
+ (analyze bug-number branch version new-index new-total arguments))
+((feature-branch arguments ...)
+ (analyze bug-number feature-branch version index total arguments))
+
(define (create-branch-for-issue issue-number patchwork-series)
(define (apply-patches)
(let ((series-data
@@ -116,9 +165,32 @@
(json->scm body)
(if (assoc-ref series-data "received_all")
- (let* ((patch-data
- (vector->list
- (assoc-ref series-data "patches")))
+ (let* ((annotated-patch-data
+ (map
+ (lambda (patch)
+ (let ((name (assoc-ref patch "name")))
+ `((patch . ,patch)
+ (metadata . ,(parse-patch-name name)
+ (vector->list
+(assoc-ref series-data "patches"
+ (patch-data
+ (map
+ (lambda (annotated)
+ (assq-ref annotated 'patch))
+ (stable-sort
+annotated-patch-data
+(lambda (a b)
+ (let ((a-meta (assq-ref a 'metadata))
+(b-meta (assq-ref b 'metadata)))
+(< (assq-ref a-meta 'index)
+ (assq-ref b-meta 'index)))
+ (feature-branch
+ (match annotated-patch-data
+((p rest ...)
+ (let ((meta (assq-ref p 'metadata)))
+ (if meta
+ (assq-ref meta 'branch)
+ "master")
(branch-name
(simple-format #f "issue-~A" issue-number))
(base-tag
@@ -127,7
