New issue
Advanced search Search tips

Issue 764294 link

Starred by 1 user

Issue metadata

Status: Fixed
Merged: issue 666669
Owner: ----
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

checkout.py does not work when the patch level is greater than one.

Project Member Reported by ehmaldonado@chromium.org, Sep 12 2017

Issue description

checkout.apply_patch [1] fails for patchlevels greater than one, when deleting files or adding binary files. See [2] for example.

The reason is that the logic used to delete or add binary files refers to p.filename, which is a different file that the one git-cl apply would use.


So for example in [3] we can see:

webrtc/modules/audio_device/BUILD.gn
  Created missing directory webrtc/modules/audio_device.  // from checkout.py, referring to p.filename
  Checking patch modules/audio_device/BUILD.gn...         // from git-apply, using patchlevel=2
  Applied patch modules/audio_device/BUILD.gn cleanly.   

p.filename and git-apply end up modifying different files.


[1] https://cs.chromium.org/chromium/tools/depot_tools/checkout.py?l=238 
[2] https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin7_chromium_rel_ng%2F1903%2F%2B%2Frecipes%2Fsteps%2Fbot_update%2F0%2Fstdout
[3] https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Ftryserver.chromium.win%2Fwin10_chromium_x64_rel_ng%2F1291%2F%2B%2Frecipes%2Fsteps%2Fbot_update%2F0%2Fstdout
 
Mergedinto: 666669
Status: Duplicate (was: Untriaged)
This is a known issue but luckily it will go away as we move our code one level up and move to Gerrit.

Both me and Mirko tried fixing it but it's hard to write a proper test for it and it's also very risky to alter this part of depot tools since you risk breaking all bots.
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/5908f9906dc71c0229c2e359fd64b325605b8370

commit 5908f9906dc71c0229c2e359fd64b325605b8370
Author: Edward Lemur <ehmaldonado@chromium.org>
Date: Tue Sep 12 22:54:33 2017

Fix checkout.py issues when p.patchlevel > 1.

When p.patchlevel > 1, p.filename does not correspond to the files that
git-apply would modify.

See bug for details

Bug:  764294 
Change-Id: Icdb803056e306edb25238b2d9cdabd3ff175d8ed
Reviewed-on: https://chromium-review.googlesource.com/663357
Reviewed-by: Aaron Gable <agable@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/5908f9906dc71c0229c2e359fd64b325605b8370/checkout.py
[modify] https://crrev.com/5908f9906dc71c0229c2e359fd64b325605b8370/patch.py
[modify] https://crrev.com/5908f9906dc71c0229c2e359fd64b325605b8370/tests/checkout_test.py

Status: Fixed (was: Duplicate)
Edward decided to fix this anyway. Thanks for that!
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/depot_tools/+/f8792079d2f78d401aebc7df16e9a251454b28a1

commit f8792079d2f78d401aebc7df16e9a251454b28a1
Author: Edward Lesmes <ehmaldonado@chromium.org>
Date: Wed Sep 13 08:06:26 2017

Revert "Fix checkout.py issues when p.patchlevel > 1."

This reverts commit 5908f9906dc71c0229c2e359fd64b325605b8370.

Reason for revert:
Introduces bugs when deleting files.
The reason is that 
  patchlevel = patchlevel or self.patchlevel
will evaluate to self.patchlevel also when patchlevel is 0, which is wrong.

Original change's description:
> Fix checkout.py issues when p.patchlevel > 1.
> 
> When p.patchlevel > 1, p.filename does not correspond to the files that
> git-apply would modify.
> 
> See bug for details
> 
> Bug:  764294 
> Change-Id: Icdb803056e306edb25238b2d9cdabd3ff175d8ed
> Reviewed-on: https://chromium-review.googlesource.com/663357
> Reviewed-by: Aaron Gable <agable@chromium.org>
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

TBR=kjellander@chromium.org,agable@chromium.org,ehmaldonado@chromium.org

Change-Id: Ifa1f94602a023228cb32e5fe3fa07586b466981a
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  764294 
Reviewed-on: https://chromium-review.googlesource.com/663266
Reviewed-by: Edward Lesmes <ehmaldonado@chromium.org>
Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>

[modify] https://crrev.com/f8792079d2f78d401aebc7df16e9a251454b28a1/checkout.py
[modify] https://crrev.com/f8792079d2f78d401aebc7df16e9a251454b28a1/patch.py
[modify] https://crrev.com/f8792079d2f78d401aebc7df16e9a251454b28a1/tests/checkout_test.py

Sign in to add a comment