Tool for importing webrev patches?

Erik Helin erik.helin at oracle.com
Wed Jul 3 15:12:14 UTC 2019


On 7/3/19 2:19 PM, Jorn Vernee wrote:
> The git workflow I've figured out so far is to:
>    1. download the attached .patch file
>    2. create a new local branch based on the one the patch is targeting
>    3. switch to newly created branch
>    4. use `git apply the_patch.patch` to apply the changes
>    5. make a commit for the applied changes
>    6. run the tests and/or play with the code before submitting a review.
>    7. delete the local branch.
> 
> I think steps 2-5 could be merged into 1 using some tool (maybe even 
> step 1. as well). Then, after reviewing, I just have to delete the newly 
> created local branch.
> 
> Does such a tool exist in Skara already? (couldn't find it on the wiki 
> [1]) Or, is there another recommended way for doing this? Is there 
> interest in adding such a tool to Skara?

We would be happy to accept such a contribution :)

I think it is possible to automate quite a bit of what you are 
describing here, this is probably how I would approach it:

1. Name the tool, perhaps: git-webrev-apply? A little verbose maybe, but
    it gets the job done and won't conflict with other git commands.
2. Take the URL for a webrev as input. Since webrev does not use a
    standardized name for the patch file, we must parse the path to the
    .patch file from the index.html file. Fortunately this is fairly
    easy, see [0]. The pattern works with webrevs generated by both
    git-webrev and webrev.ksh.
3. For applying the patch you can see the code in GitPr.java [1].

The three steps above would result in something like:

$ git webrev-apply \
   http://cr.openjdk.java.net/~stefank/8227085/webrev.01/

which just applies the patch on top of HEAD.

I'm not sure about creating a branch unless you also intend to commit 
the applied patch? Otherwise the working tree will be dirty and if the 
user switch back to master it will remain dirty. It is fairly easy to 
use the above tool as part of an alias:

[alias]
webrev-fetch = !git checkout -b review && git webrev-apply $1 && git 
commit -m 'Webrev'

But I have an hard time coming with a clean way of doing the above from 
a tool. For example, what would the commit message be? A webrev is much 
more like a patch than a commit...

One drawback of the tool will be when the webrev has become stale (i.e. 
master has moved on and the patch in the webrev no longer applies 
cleanly). Since webrev does not include the base commit hash there is 
not much we can do here, besides fail :/

What do you think?

Thanks,
Erik

[0]: curl -s https://openjdk.github.io/cr/skara/26/webrev.00/ |\
      grep -P '[ ]*(<td>)?<a href\=\".*\">.*\.patch</a></td>(</tr>)?$'
[1]: 
http://git.openjdk.java.net/skara/blob/master/cli/src/main/java/org/openjdk/skara/cli/GitPr.java#L150


More information about the skara-dev mailing list