A way to make them undoable would be nice. Right now the only option is to revert the landmine change really quickly but that means that depending on when you sync you either get no clobbers or two clobbers.

I guess an undo would be list of lines that should be ignored when doing the diff?


I think that the problem with landmines is more severe than just the rebuilding. That might be needed anyway (although landmines make it worse). There's also the problem of directory deletions frequently failing on Windows for all sorts of reasons (command prompts open to those directories, Visual Studio debuggers holding locks on files or directories), and clobbering deletes generated .sln and .vcxproj files, and occasionally due to flakiness it deletes args.gn files.

To be clear: landmines *should* be a last-ditch effort. If we're using them
regularly, then doesn't it imply that something it wrong with gn/ninja
and/or how they're used?

Yes, you are correct that using a landmine suggests that there's something wrong with how gn/ninja is working. The linked bug was from a situation where it looked like gn was broken, for example, so using a landmine wasn't a bad guess. It was, however, not the right guess :(.

We're not using them regularly; a quick look at the change history of get_landmines.py suggests we change things once every couple of months.

https://chromium-review.googlesource.com/c/544867/ didn't get owner approval, and due to the TBR line the tooling didn't notice.

TBR= should only assume that people listed after TBR have lg'd, and if it isn't TBR'd against an owner the tooling should still request owner approval -- do we have a bug for that already?

It would also be nice to have the landmines script check an environment variable and skip clobbering if it is set. I would set that and it would be very helpful when traversing history (although it won't help with history prior to adding this change, but long-future me likes the idea).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46ac6b742672a82001cd3d8ef0c7525be72a9097

commit 46ac6b742672a82001cd3d8ef0c7525be72a9097
Author: brucedawson <brucedawson@chromium.org>
Date: Tue Jun 27 16:04:46 2017

Add comment warning about modifying landmines

Modifying landmines has greater consequences than is sometimes realized
for developers, especially on Windows. Add a comment to warn editors of
these consequences.

This was inspired by https://chromium-review.googlesource.com/c/544867/

R=dpranke@chromium.org
BUG= 736939 

Review-Url: https://codereview.chromium.org/2959743004
Cr-Commit-Position: refs/heads/master@{#482639}

[modify] https://crrev.com/46ac6b742672a82001cd3d8ef0c7525be72a9097/build/landmines.py

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1ed2e02283cf828822dd24dd0e8f5e22767b4bc

commit a1ed2e02283cf828822dd24dd0e8f5e22767b4bc
Author: brucedawson <brucedawson@chromium.org>
Date: Fri Jun 30 01:25:11 2017

Add comment warning about modifying landmines

Modifying landmines has greater consequences than is sometimes realized
for developers, especially on Windows. Add a comment to warn editors of
these consequences.

R=dpranke@chromium.org
BUG= 736939 

Review-Url: https://codereview.chromium.org/2960013002
Cr-Commit-Position: refs/heads/master@{#483579}

[modify] https://crrev.com/a1ed2e02283cf828822dd24dd0e8f5e22767b4bc/build/get_landmines.py

Friendly ping. This is a blocking bug on cit-pm-53. Please update need and priority accordingly. 

Robbie and M-A, please revisit this issue. I don't know enough about landmines, but you two know enough about named caches in swarming.

I think dpranke might be the best to comment at this point.

I don't think this has anything to do with named caches (as this bug was written), but the modern thing to do would, indeed, be to have a button to dump all of a given named cache (and remove landmines entirely). 

Landmines were originally added to provide a 'cache dump' functionality which was baked into the source history; when someone goofed a gyp file in a way that left garbage on the bots, we wanted to clobber any time a bot's checkout flipped between pre/post goof.

I believe (but can't say for sure) that gn/ninja are now at the point where all dependencies are correctly expressed and we shouldn't need landmines any more and catastrophic failures in bot pollution would be better handled with a cache dump lever in e.g. swarming.

It looks like we have used two landmines in the last year. If those were avoidable then that suggests that we could get rid of landmines entirely, which would be great.

landmines aren't completely unnecessary, but they are rare. The dependency graph in GN isn't perfect and probably never will be.

At this point, I wouldn't bother doing anything with this.