Problem/Motivation

Related to #1952058: [META] Retesting stale RTBC core patches.

https://drupal.org/patch/reroll
has instructions that a machine could do.
if the merge has no conflict, that rerolled patch could be posted by the system to the issue.

Good background reading

Learn from

Proposed resolution

Have machines do things for us, and let us spend out time on complicated rerolls that have conflicts and need humans to look at.

Remaining tasks

  • Find the issue this is a duplicate of
  • look at other issues which have tried to automate parts of the patch workflow to learn from history
  • have a bot outside d.o try this on a *single* test issue, with a fake patch that needs rerolling

User interface changes

?

API changes

No.

Comments

catch’s picture

Mixologic’s picture

When a new commit happens in core, every CNR issue with a patch has the potential to be broken, and no longer apply.

1. How do we programatically determine which patches to reroll

We ought to be able to "reroll everything", however,

The following situations make it more difficult to programatically determine which patches should be rerolled (there may be more):

  1. Followup patches: issues that were RTBC, committed, Need's Worked, and a new patch is added.
  2. A Patch, which passes tests, which ought to be set to needs work due to issues like whitespace errors or coding standards problems, yet are still CNR
  3. A Patch, which have some overreach, which still pass tests, but the previous patch was the one we wanted.
  4. More than one patch, one for the tests, and one for the code.
  5. Multiple patches being discussed in an issue, all pass tests, but have architectural differences

If a script or bot were to automatically attempt re-rolling, what should be the rules for when it attempts?

Some ideas (obviously not all of them):

  1. Reroll any non-hidden patch that was passing tests when posted
  2. Only reroll patches for which there is *one* current patch file that passes tests, and all others are hidden
  3. Only reroll patches for which there is either just one current patch, or two current patches, one with 'test or testing' in the title of the patchname

If we go with option 1, what should the status be if some patches re-apply cleanly and some do not?

2. If an automated process is updating the queues, how should this impact issue queue users

This is probably a bigger question than just rerolls, but say an issue like this gets in: (https://drupal.org/node/7269)
And there is a bot that goes through and rerolls tons of patches. How big of an impact would it be to suddenly have hundreds of issues marked with new comments and status changes?
In an ideal world, should automated interactions trigger things like emails to users subscribed to issues? Should it mark them as "new or updated" in issue queue lists etc?

neclimdul’s picture

I wrote a script to do this because I was tired of doing it manually. Got pointed here.

Its a pretty naive implementation but it gets the job done.
https://gist.github.com/neclimdul/d611b5033c6d302fab7e

neclimdul’s picture

Wow, people where interested. opened a project for a PR https://github.com/neclimdul/drupal_reroll More are welcome.

YesCT’s picture

Maybe also see https://gist.github.com/benjifisher/7337143 And I thought Gaelan had one too. I'll follow up.

donquixote’s picture

I discussed this with someone on IRC a while ago. A shame I don't remember who it was.

It is not useful to have a new comment for every reroll.
Instead, if we would internally treat a patch as if it was a git pull request, our life becomes so much easier.

Work flow

  1. Person uploads a patch.
    drupal.org checks out the most recent commit from 8.x in "detached head", applies + commits the patch, creates a tag based on the issue nid and comment id and possibly the filename.
    If the patch does not apply a comment is posted immediately.
    Testbot runs tests on the tag.
    Test result shows up on the issue comment.
  2. Several new commits go into to 8.x.
  3. Automatic reroll time.
    drupal.org checks out the most recent commit from 8.x in "detached head", and merges the tag.
    If the merge fails and this would change the issue status, a comment will be posted.
    If the merge is successful, testbot runs tests on the tag.
    Test result shows up on the issue comment.
    If the test fails and this would change the issue status, testbot will post a new comment (same as now).

Issue comment display

Unlike now, the patch in the issue comment will have a field formatter which provides two download links:

  1. The original patch file.
  2. A download link to a dynamically generated patch file, where the tag is merged with the latest commit from 8.x - if applicable.
  3. (optionally, we could also provide the git tag for checkout somewhere)
donquixote’s picture

#7 could even be extended with some git bisect magic.
A cheap version of that would only bisect to identify the first commit to 8.x where a merge failed.
A more expensive version would bisect to find the first commit to 8.x where the test fails. This would be quite expensive on the testbot so we should only do this some day in the future.

donquixote’s picture

Another question is whether we have one big repository where we create all the tags for patches in issue comments, or if we should rather have e.g. one repository per issue. So locally you no longer need to download patches. Instead, you can clone the issue repository and merge a tag representing a patch.

git clone http://git.drupal.org/project-issue/drupal/issue-2252787.git
cd drupal
git fetch origin
git checkout 8.x
# create a local development branch, starting from 8.x
git checkout -b localdev
# show all tags, each representing a patch (or a release tag)
git tag
# merge a specific tag from a patch in issue comment #2252787-9
git merge 2252787-9-playing-with-bisect
isntall’s picture

Project: Drupal.org Testbots » DrupalCI: Drupal.org Integration (Modernizing Testbot Initiative)
catch’s picture

drumm’s picture

Status: Active » Closed (won't fix)

Now that we have merge requests, GitLab keeps track of whether they are mergeable. We should not be building any bespoke tooling around patches.