On Sat, Aug 27, 2016 at 8:53 AM, Gregory Szorc <gsz...@mozilla.com> wrote:
> > > > On Aug 27, 2016, at 07:09, Kan-Ru Chen <kc...@mozilla.com> wrote: > > > >> On Sat, Aug 27, 2016, at 11:35 AM, Gregory Szorc wrote: > >>> On Fri, Aug 26, 2016 at 8:27 PM, Steve Fink <sf...@mozilla.com> wrote: > >>> > >>>> On 08/26/2016 08:16 PM, Gregory Szorc wrote: > >>>> > >>>> > >>>>> On Aug 26, 2016, at 19:54, Kan-Ru Chen <kc...@mozilla.com> wrote: > >>>>> > >>>>> Hello, > >>>>> > >>>>> In Bug 1297276 I landed a patch to rename mozilla/unused.h to > >>>>> mozilla/Unused.h to make it more consistent with our other MFBT > headers. > >>>>> Normally rename a header shouldn't cause too much trouble, however > this > >>>>> rename is only changing the case so you might experience some > problems > >>>>> on case insensitive filesystem. > >>>>> > >>>>> As pointed out by Tim in > >>>>> https://bugzilla.mozilla.org/show_bug.cgi?id=1297276#c19 if you use > >>>>> |git pull -f| to update local copy of gecko and git refuse to, you > can > >>>>> rm mfbt/unused.* first to make git happy. > >>>> Case only renames cause a lot of havoc. Somewhere there is an open > bug to > >>>> implement a server side hook to reject them. > >>>> > >>>> What I'm trying to say is thank you for reminding me to implement the > >>>> hook. And congratulations on likely being the last person to perform > a case > >>>> only rename on the repo. > >>> > >>> For the record, is there a better way to accomplish this? In this > >>> particular case, it seems like we really do want the rename. Would it > work > >>> better to do two commits, one from mozilla/unused.h -> > >>> mozilla/LucyTheDancingFerret.h, then another doing > >>> mozilla/LucyTheDancingFerret.h -> mozilla/Unused.h? > >> > >> > >> No. Because if you perform an update/checkout across the rename, you may > >> encounter problems. This can happen when bisecting, for example. > >> > >> Now, this isn't as bad as a case folding collision where you have both > >> unused.h and Unused.h: some filesystems just flat out refuse that. > >> > >> At least with a rename your VCS has the opportunity to delete the old > >> file > >> before creating the different cased one. But even then, build systems > and > >> other tools can be confused because some filesystems are case > insensitive > >> but case preserving and tools may make inappropriate decisions about > >> whether "unused.h" and "Unused.h" are the same file. There's a lot that > >> can > >> go wrong. So the whole scenario is best avoided. > > > > So given the fact people can make mistakes, what would you suggest if > > one really wants to fix the filename? I hope the hook will display such > > suggestions instead of just say "don't do this." > > The hook will likely link to a web page explaining in more detail what to > do. > > While there is always an option to override hooks, sadly in this case I > don't think we'll allow it too often. This also means - rather > unfortunately - that if someone makes a casing mistake it will forever be > enshrined in the repo. I suppose we can deal with these issues as they come up, but I don't think it's at all obvious that build/version control issues required to make the transition work outweigh the engineering ergonomics issues of having consistency in the code, so consider this a note that there's not really consensus that leaving these mistakes enshrined forever is good policy. -Ekr > The best way to avoid this is linting tests and code review. With the > autoland repo, we'll soon drop bad commits instead of backing them out. So > if something in automation finds case problems, the commit effectively > never existed and there won't be a problem (beyond automation). > > One could whip up a filename auditing TaskCluster task pretty easily. > Essentially copy https://hg.mozilla.org/mozilla-central/rev/f5e13a9a2e36. > I'll happily review it. > > Also, I'm sorry you had to fall into this mostly unmarked trap. Others > have likely done similar things unknowingly. You did the right thing and > sent an email to notify people. That also happened to remind me that we > need to fix things :) Please don't feel bad about what you did: it wasn't > your fault. Blame the lack of tools that didn't catch this. And blame me > for having a bug (assigned to me even!) to implement one of those tools. > > > > >> The bug tracking the server hook is 797962 btw. > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
