#general
- a
ancient-stone-50795
04/08/2016, 7:43 PMI don't like that. - a
ancient-stone-50795
04/08/2016, 7:43 PMYes we need this option because if you run pants with the new release, then switch to an older branch, the older branch is permanently locked out - w
witty-crayon-22786
04/08/2016, 7:43 PMi mean, that's just not really an option - w
witty-crayon-22786
04/08/2016, 7:44 PMis there harm in just removing the file by default? - w
witty-crayon-22786
04/08/2016, 7:45 PM@ancient-stone-50795: "Should this be turned on by default, but be deprecated immediately?" - w
witty-crayon-22786
04/08/2016, 7:45 PM@ancient-stone-50795: no warning in that case, except if you disable it. - w
witty-crayon-22786
04/08/2016, 7:46 PMbut, really, it just seems like not having an option and deleting it would be fine. - w
witty-crayon-22786
04/08/2016, 7:46 PMi don't care strongly either way. just don't want an option in there to bitrot on an indefinite timeframe - b
bored-art-40741
04/08/2016, 7:49 PMI don't think this should be the default - b
bored-art-40741
04/08/2016, 7:50 PMIt adds complexity and, as noted in the code, a very slight race - l
limited-country-20626
04/08/2016, 7:52 PMJust use a TODO and track with an issue - maybe with a 1.0 tag? - a
ancient-stone-50795
04/08/2016, 7:55 PM@bored-art-40741: I don't think it really causes a race. - a
ancient-stone-50795
04/08/2016, 7:55 PMdoes it? - a
ancient-stone-50795
04/08/2016, 7:56 PMAnyway, if we are ambivalent on whether it is safe, let's just do that @limited-country-20626 says and just put in an issue that we don't really need this option and it can be deleted after... time passes. - b
bored-art-40741
04/08/2016, 7:57 PMThere's another issue, which is that no one should rely on this behavior - a
ancient-stone-50795
04/08/2016, 7:57 PMWe are going to run with it turned on.Maybe we should just run it for a while, and eventually not make it an option and leave it as the default behavior if we have no issues with it. - b
bored-art-40741
04/08/2016, 7:57 PMThe lock file itself definitely can and will be leaked - a
ancient-stone-50795
04/08/2016, 7:57 PMSure, but that happened before with the pid lock as well. It gets cleaned up on the next run. No one depended on that behavior. - a
ancient-stone-50795
04/08/2016, 7:58 PM(FWIW, We are blocked on pants releases until we accept a patch something like this. ) - b
bored-art-40741
04/08/2016, 7:58 PMhmmm I still think it's incorrect - b
bored-art-40741
04/08/2016, 7:59 PMIn fact, the more I think about this, the more hesitant I am to push it at all - b
bored-art-40741
04/08/2016, 7:59 PMI need to double check the implementation, but I think that someone waiting on the lock object in another process already has an open file handle to the file being deleted - a
ancient-stone-50795
04/08/2016, 8:00 PMhmm, that's something important to check. I would also be happy with the workaround we discussed to just change the name of the lock file, the way we handled the ivy global lock issue - b
bored-art-40741
04/08/2016, 8:01 PMI think I'd prefer that. In particular, it seems very implausible to the point of being nearly impossible that two different versions of pants would be trying to run in the same workspace at the same time - b
bored-art-40741
04/08/2016, 8:02 PMYeah looking at the implementation of the underlying lock, I'm not convinced at all this is safe - b
bored-art-40741
04/08/2016, 8:02 PMWe should just make the location of the global lock an option - b
bored-art-40741
04/08/2016, 8:02 PMThat's a permanent option I'm happy with - a
ancient-stone-50795
04/08/2016, 8:31 PM@bored-art-40741: are you planning to submit a PR or should I - b
bored-art-40741
04/08/2016, 8:31 PMI'm pretty slammed today on top of doing the release; if you have the time I'd appreciate it - a
ancient-stone-50795
04/08/2016, 8:40 PM@witty-crayon-22786: @bored-art-40741 https://rbcommons.com/s/twitter/r/3668/