734214 - autoland-try fails to apply patches that add new files, complaining that the files already exist

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Daniel Holbert [:dholbert]]

Quoting bug 640443 comment 41:
(In reply to Mozilla RelEng Bot from comment #41)
> Autoland Patchset:
> 	Patches: 604134
> 	Branch: mozilla-central => try
> Patch 604134 could not be applied to mozilla-central.
> file layout/reftests/box/flexbox-abspos-container-1-ref.html already exists
> 1 out of 1 hunks FAILED -- saving rejects to file
> layout/reftests/box/flexbox-abspos-container-1-ref.html.rej
> file layout/reftests/box/flexbox-abspos-container-1a.html already exists
> 1 out of 1 hunks FAILED -- saving rejects to file
> layout/reftests/box/flexbox-abspos-container-1a.html.rej
> file layout/reftests/box/flexbox-abspos-container-1b.html already exists
> 1 out of 1 hunks FAILED -- saving rejects to file
> layout/reftests/box/flexbox-abspos-container-1b.html.rej
> file layout/reftests/box/flexbox-abspos-container-1c.html already exists
> 1 out of 1 hunks FAILED -- saving rejects to file
> layout/reftests/box/flexbox-abspos-container-1c.html.rej
> file layout/reftests/box/flexbox-abspos-container-1d.html already exists
> 1 out of 1 hunks FAILED -- saving rejects to file
> layout/reftests/box/flexbox-abspos-container-1d.html.rej
> file layout/reftests/box/flexbox-abspos-container-2-ref.html already exists
> 1 out of 1 hunks FAILED -- saving rejects to file
> layout/reftests/box/flexbox-abspos-container-2-ref.html.rej
> file layout/reftests/box/flexbox-abspos-container-2.html already exists
> 1 out of 1 hunks FAILED -- saving rejects to file
> layout/reftests/box/flexbox-abspos-container-2.html.rej
> abort: patch failed to apply
> 
> Patchset could not be applied and pushed.

I think this is a bug in [autoland].

These files absolutely did _not_ exist in m-c, and I'd never pushed them to Try before, so they didn't exist in any state on Try, either.

The bug _did_ have 2 patches attached that added those files (labeled "fix v4" and "fix v4a"), but I'd marked the older version (v4) as obsolete 7 minutes before I added the [autoland] status.

I suspect [autoland] didn't pick up on that somehow and tried to apply both patches, somehow... not sure why.

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> The bug _did_ have 2 patches attached that added those files (labeled "fix
> v4" and "fix v4a"), but I'd marked the older version (v4) as obsolete 7
> minutes before I added the [autoland] status.

(Actually, I doubt the older patch was involved, because the Autoland error message indicates that it only tried to apply one patch. ("Patches: 604134") (that's the attachment ID of the newer, correct patch)

So that only leaves the "applying the same patch twice" explanation, I guess.  (I've done a sanity-check to be sure the patch applies just fine to a local up-to-date m-c tree, btw.)

Comment 2

[Marc Jessome[:mjessome]]

Looking into this it was not an issue with pulling obsolete patches, it's an issue with cleaning the repository when verifying patch application. Still looking into what exactly went wrong here.

Comment 3

[Marc Jessome[:mjessome]]

This problem is stemming from a change that was made recently in the way that autoland verifies the application of patches. We use 'hg import' to pull in patches, which will automatically commit for each patch.
Since, if a patch fails to apply, we can't easily clean the repository to re-attempt, we perform the following:
  1. `hg import --no-commit -f` on each patch to confirm that they all apply together.
  2. `hg update -C` to clean up those changes.
  3. `hg import -u ...` on each patch
Since "hg update -C" doesn't remove the files added by a patch and leaves them untracked in the repository, performing step 3 will have a conflict.

I'm surprised we haven't run into this sooner, actually.
I will put in a temporary fix -> using the hg extension 'hg purge' to clean up the directory, but leave this bug open until I've implemented a permanent solution.

Comment 4

[Daniel Holbert [:dholbert]]

For the permanent solution -- could we use mercurial queues?  Then step 1 would be "hg qimport" on each patch, and step 2 would be "hg qpop -a".

Comment 5

[Marc Jessome[:mjessome]]

Great suggestion, thanks Daniel!

Comment 6

[Marc Jessome[:mjessome]]

Fixed, using merc. queues.