Re: The upcoming C/C++ coding style change
On Sun, Dec 16, 2018 at 9:08 PM Makoto Kato wrote: > Hi, > > Is Objective-C++ (*.mm) still old format? Or Should I file a bug for it? > We didn't include Objective-C++ in the initial reformatting of the tree mostly to avoid scope creep, but clang-format is well capable of formatting it. Please file a bug for it now, it should be a very simple change (Adding ".mm" here: < https://searchfox.org/mozilla-central/rev/1a4a7745fd4a14402a0de0e9abc040724d4fcf8f/python/mozbuild/mozbuild/mach_commands.py#1650>) to include it. Thanks, Ehsan > > On Thu, Nov 29, 2018 at 9:38 PM Sylvestre Ledru > wrote: > > > Hello, > > > > As Ehsan announced recently, we are going to reformat Friday 30 November. > > In order to mitigate the impact on the uplifts process, we choose this > > date before the merge day. > > > > In term of execution, we will close the trees Friday around 8am > > Paris/Berlin time (11pm PST). > > We will then merge autoland and inbound into mozilla-central. > > We are planning to land the big patch around 12am (3am PST). > > > > The experimentation with dom/media highlighted that we need an efficient > > way to automatically rebase patches before the merge. > > To achieve this, we updated the bootstrap process to include an extension > > called hg formatsource. > > This extension will automatically rebase the local changes to avoid > > conflicts. > > Please note that it is important that the extension is installed before > > the pulling a revision including the reformatted sources. > > > > More information on: > > > > > https://docs.google.com/document/d/13AwAsvKMhH0mflDlfatBqn6LmZHiQih76oxM4zfrPl4/edit > > https://bugzilla.mozilla.org/show_bug.cgi?id=1507711 > > > > Sylvestre, Ehsan and Andi > > > > ___ > > 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 > -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
Hi, Is Objective-C++ (*.mm) still old format? Or Should I file a bug for it? -- Makoto On Thu, Nov 29, 2018 at 9:38 PM Sylvestre Ledru wrote: > Hello, > > As Ehsan announced recently, we are going to reformat Friday 30 November. > In order to mitigate the impact on the uplifts process, we choose this > date before the merge day. > > In term of execution, we will close the trees Friday around 8am > Paris/Berlin time (11pm PST). > We will then merge autoland and inbound into mozilla-central. > We are planning to land the big patch around 12am (3am PST). > > The experimentation with dom/media highlighted that we need an efficient > way to automatically rebase patches before the merge. > To achieve this, we updated the bootstrap process to include an extension > called hg formatsource. > This extension will automatically rebase the local changes to avoid > conflicts. > Please note that it is important that the extension is installed before > the pulling a revision including the reformatted sources. > > More information on: > > https://docs.google.com/document/d/13AwAsvKMhH0mflDlfatBqn6LmZHiQih76oxM4zfrPl4/edit > https://bugzilla.mozilla.org/show_bug.cgi?id=1507711 > > Sylvestre, Ehsan and Andi > > ___ > 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
Re: The upcoming C/C++ coding style change
>>If I'm understanding your situation correctly, I believe you can use rebase >>to update a whole queue of dependent patches, provided you have the >>format-source extension enabled. Ok. For anyone with mq patch queues, here's a sequence that should work. I hit a problem, and will file a bug on clang-format - the sequence below side-steps the bug. I found when I reviewed my queues in more detail that I only have maybe 6 active queues (mostly for specific projects) with perhaps 50-100 active patches; a number of other queues for areas I'm not actively working on at the moment (like my webrtc datachannels queue), and quite a few WIP queues for work I abandoned or decided to mothball, or for completed work (like webrtc import patch queues). Most of those date back to 2016 or earlier. At times in the past I've had multi-hundred patches for a few dozen active queues. That would have made this issue more problematic, time-wise. (Note: this sequence below could probably be automated. It's not worth my time to do so, however.) # First, ensure the tools are set up: # do these once; no need to do them again ./mach bootstrap ./mach clang-format # that forces clang-tidy/etc to be downloaded and setup! (bug) # Now here's how to reformat your patches: hg pull -u hg tip # record rev of tip of tree Now for each set of patches that can be applied together (assuming they're at 0-N of the current queue; if not adjust accordingly): hg qshow 0 | head 10 # Use this to find the parent rev this sequence was applied on. # In some cases that rev is gone (a patch that has since been pushed), # If it's not found, use PRE_TREEWIDE_CLANG_FORMAT instead. hg update -r hg qpush # resolve conflicts if any and hg qref # repeat hg qpush/resolve for all patches in the sequence (N) hg qser >/tmp/patch_names hg qfin -a # if not already on PRE_TREEWIDE_CLANG_FORMAT: hg rebase -d PRE_TREEWIDE_CLANG_FORMAT # resolve any conflicts before we reformat hg rebase -d # resolve conflicts # repeat N times: hg qimport -r tip -n # -n name is optional but recommended hg qpop Poof! that *should* be all. Thanks to jkew, pehrsons, ehsan for suggestions! -- Randell Jesup, Mozilla Corp remove "news" for personal email ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Dec 13, 2018 at 6:05 PM Randell Jesup wrote: > >> tl;dr: I need to figure out how I'm going to dig out of the rebasing > hole > >> I'm now in, and could use advice/help, and I think the above sequence > >> doesn't work for queues of dependent patches. > > > >If I'm understanding your situation correctly, I believe you can use > rebase > >to update a whole queue of dependent patches, provided you have the > >format-source extension enabled. > > [snip] > > Thanks - that was the sequence I thought might work, after hearing from > pehrsons that the rebase would reformat all the pushed patches. hg qfin -a > is critical I think. > Indeed it is. It converts the "mq" patches to "real changesets", allowing the Mercurial rebase to correctly reformat the patches through the formatsource extension. > If I wanted to optimize this, perhaps for each independent sequence: > hg update -r ; hg qpush (*N); > You probably need `hg qfin -a` here. I don't think you can rebase mq patches as long as they're still mq patches (but I could be wrong about that.) > hg rebase -d PRE_TREEWIDE_CLANG_FORMAT (assuming > this does what I expect); resolve any bitrots; > If by what you expect, you mean rebase the patches on top of the changeset before the tree-wide reformat, then yes. This should simplify resolving conflicts somewhat, I think, by splitting the conflict resolution to two phases (before and after the reformat) which is I think your goal here. > hg rebase -d ; > (hg qimport -r tip; hg qpop) (*N) > > Almost automatable (probably automatable; may not be worth it). > > Thanks! I'll let people know if it works > Good luck! -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
>> tl;dr: I need to figure out how I'm going to dig out of the rebasing hole >> I'm now in, and could use advice/help, and I think the above sequence >> doesn't work for queues of dependent patches. > >If I'm understanding your situation correctly, I believe you can use rebase >to update a whole queue of dependent patches, provided you have the >format-source extension enabled. [snip] Thanks - that was the sequence I thought might work, after hearing from pehrsons that the rebase would reformat all the pushed patches. hg qfin -a is critical I think. If I wanted to optimize this, perhaps for each independent sequence: hg update -r ; hg qpush (*N); hg rebase -d PRE_TREEWIDE_CLANG_FORMAT (assuming this does what I expect); resolve any bitrots; hg rebase -d ; (hg qimport -r tip; hg qpop) (*N) Almost automatable (probably automatable; may not be worth it). Thanks! I'll let people know if it works -- Randell Jesup, Mozilla Corp remove "news" for personal email ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 13/12/2018 14:52, Randell Jesup wrote: But still all is not lost here. When you do decide to do the manual merging when you needed those patches, you would need to: * Update your working tree to the parent of the commit that did the reformat. * Apply your patch to that tree and reformat the tree. * Diff the reformat commit and your current working directory. That would give the reformatted diff. tl;dr: I need to figure out how I'm going to dig out of the rebasing hole I'm now in, and could use advice/help, and I think the above sequence doesn't work for queues of dependent patches. If I'm understanding your situation correctly, I believe you can use rebase to update a whole queue of dependent patches, provided you have the format-source extension enabled. Starting with all patches popped: $ hg pull -u # get latest tree and make a note of the current tip $ hg log # changeset that you'll want to rebase your queue onto $ hg up PRE_TREEWIDE_CLANG_FORMAT # go to the pre-format changeset $ hg qpush $ hg qfin -a# turn your applied patches into local commits $ hg rebase -d DEST # where DEST is current m-c tip (or wherever you # want to be working) $ hg qimport -r tip # pull top local commit into mq $ hg qpop # and pop to unapply it Repeat last two steps for each commit that you want to pull back into mq. Now you should have a queue that is based on the updated tree. The rebase step may require some manual conflict resolution, but should be able to manage a lot of it automatically for you. You lose the original patch names in this process, as "hg qimport -r tip" names them automatically (or you could use -n to assign your preferred names when re-importing them). You can repeatedly go back to PRE_TREEWIDE_CLANG_FORMAT and run this sequence for as many separate sets of dependent patches as you care about. HTH, JK ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
>But still all is not lost here. When you do decide to do the manual >merging when you needed those patches, you would need to: > > * Update your working tree to the parent of the commit that did the >reformat. > * Apply your patch to that tree and reformat the tree. > * Diff the reformat commit and your current working directory. That >would give the reformatted diff. tl;dr: I need to figure out how I'm going to dig out of the rebasing hole I'm now in, and could use advice/help, and I think the above sequence doesn't work for queues of dependent patches. So in my 6 (current) repos on my main dev machine, I have ~150 mq patch queues with a total of ~3000-3500 patches (though perhaps only 2500 unique ones) - some ancient/abandoned, but most have (unfinished) work on some subject (not generally single bugs, but something like "datachannel", with some related patches and some independent patches, and usually with a tail of some temporary/abandoned/replaced patches). Perhaps not an optimal flow, but it has worked for me for many years. Some of these are under revision control (hg mq --init), and are synced with (shared) patch repos on hg.mozilla.org (for example webrtc.org update import patch queues, which typically got to 100-150 patches before squashing for landing). I also used that to share queues with my windows dev machine. Some queues I could drop, but probably the majority of queues and majority of patches within each queue I'll want to save, if I can. The "saving" could be done on an as-needed basis, they don't need to all be converted (and that helps with the issue of having to decide if they're still relevant). But I need a process that won't take crazy amounts of manual labor to save my patches. I'd guess I have about 1000 patches I may need to save, eventually, and several hundred in the short term. An example is a queue of work to redo Dispatch() to require an already_AddRefed<>; that has about 30 directory specific patches, hitting ~250 files (plus some cruft patches after those). Another is my profiler work queue, which has 42 patches, mostly for single issues (or 2 or 3 related patches for an issue), and perhaps 1/2 of them are still relevant. Dealing with normal bitrot when going back to one of these I'm used to, but the reformatting will make pretty much every patch be 100% conflicts I assume. So what are reasonable options here? Are any of the options an improvement on "deal with every patch totally manually with 100% conflicts"? (Which, over time as I go back to various queues, will use a lot of my time doing manual rebasing.) Note that the line above "Apply your patch to that tree" implies doing a manual un-bitrot if needed (like updating today), which is ok (as I said, it's the same as before the reformat; most will not need major surgery unless they're ancient). >From the discussion here, it sounds like some manual fixups might be needed after the tool runs as well. But doing this sequence for every patch sounds time-consuming. And a much bigger issue: I think that breaks down for a queue of patches that depend on each other - if I reformat the first patch, I can't go back to before the big reformat and just apply the second patch, as it may depend on the first. I'd have to (I guess) squash all the patches, which in most cases would be horribly worse than doing it 100% manually. So what are usuable options for a (long) queue of patches, which may be dependent? (Some aren't and so perhaps I could do the sequence above for each one, like the Dispatch queue with per-directory patches, but that's not the common case.) Can we come up with a way to partially script this, if there's a workable sequence? Or do I just have to do a ton of manual rebasing? Thanks for any help/ideas. -- Randell Jesup, Mozilla Corp remove "news" for personal email ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 10:47 PM Emilio Cobos Álvarez wrote: > On 11/30/18 2:06 AM, Ehsan Akhgari wrote: > > On Thu, Nov 29, 2018 at 9:43 AM Emilio Cobos Álvarez > > wrote: > > > >> On 11/29/18 1:38 PM, Sylvestre Ledru wrote: > >>> This extension will automatically rebase the local changes to avoid > >>> conflicts. > >> > >> Is there a way to do the same for cinnabar users? > >> > > > > Yes! Sorry for the delay... > > NP! > > > Please check out this script: > > https://github.com/ehsan/clang-format-reformat-branch. This does > something > > similar to the format-source extension for Mercurial but done as a > one-time > > tool, borrowing from the tool that the MongoDB project developed for the > > same use case. It takes a local branch based on a clone of > mozilla-central > > that doesn't yet have the reformat commit and rebases it on top of the > > reformat commit, reformatting your local modifications in the process. I > > hope it proves to be helpful for the git users out there! > > Nice! I haven't tried it yet (actually was going to report back when I > found the reply). > > I hacked up something today as well while looking into this. It's not > really sophisticated, and you need to tweak the git repo config, so your > script probably works best for most people. > > Just in case it's useful for somebody, while looking into a way to do > this (I basically followed[1]), I wrote a little merge driver which > seems to work fine (with a caveat, see below). I just uploaded it here: > >https://github.com/emilio/clang-format-merge > This actually looks quite decent, and a cleaner approach. It also taught me about merge drivers. :-) I suspect this would work equally well for everyone (but I haven't tested it myself, just based on reading the source.) Thanks for making it happen. -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 11/30/18 2:06 AM, Ehsan Akhgari wrote: On Thu, Nov 29, 2018 at 9:43 AM Emilio Cobos Álvarez wrote: On 11/29/18 1:38 PM, Sylvestre Ledru wrote: This extension will automatically rebase the local changes to avoid conflicts. Is there a way to do the same for cinnabar users? Yes! Sorry for the delay... NP! Please check out this script: https://github.com/ehsan/clang-format-reformat-branch. This does something similar to the format-source extension for Mercurial but done as a one-time tool, borrowing from the tool that the MongoDB project developed for the same use case. It takes a local branch based on a clone of mozilla-central that doesn't yet have the reformat commit and rebases it on top of the reformat commit, reformatting your local modifications in the process. I hope it proves to be helpful for the git users out there! Nice! I haven't tried it yet (actually was going to report back when I found the reply). I hacked up something today as well while looking into this. It's not really sophisticated, and you need to tweak the git repo config, so your script probably works best for most people. Just in case it's useful for somebody, while looking into a way to do this (I basically followed[1]), I wrote a little merge driver which seems to work fine (with a caveat, see below). I just uploaded it here: https://github.com/emilio/clang-format-merge The nice thing is that I can forget about this and all the regular commands (cherry-pick / rebase / am...) will "just work", which is nice for me since I don't need to think on all the patches I need to rebase before-hand. The caveat is that right now it needs a patch: https://phabricator.services.mozilla.com/D13505 Because the current './mach clang-format -a' used for 'hg formatsource' makes assumptions about the path it's formatting, and git loves paths like .merge_file_lwMX2O. So if it doesn't make it in it'll be quite useless :-) The repository has full instructions on how to use it, but please let me know if you have any questions or hit any issues. Thanks, I'll give that a shot for some of my checkouts and report back :) -- Emilio [1]: https://bugs.chromium.org/p/chromium/issues/detail?id=574611 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 9:43 AM Emilio Cobos Álvarez wrote: > On 11/29/18 1:38 PM, Sylvestre Ledru wrote: > > This extension will automatically rebase the local changes to avoid > > conflicts. > > Is there a way to do the same for cinnabar users? > Yes! Sorry for the delay... Please check out this script: https://github.com/ehsan/clang-format-reformat-branch. This does something similar to the format-source extension for Mercurial but done as a one-time tool, borrowing from the tool that the MongoDB project developed for the same use case. It takes a local branch based on a clone of mozilla-central that doesn't yet have the reformat commit and rebases it on top of the reformat commit, reformatting your local modifications in the process. I hope it proves to be helpful for the git users out there! The repository has full instructions on how to use it, but please let me know if you have any questions or hit any issues. Thanks, -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 12:20 PM Boris Zbarsky wrote: > On 11/29/18 11:40 AM, Ehsan Akhgari wrote: > > Yes, that is true sadly. But to be fair here, old mq patches that do not > > apply due to age are already beyond saving with any kind of automated > > tooling, and they require manual work to get them applied first. :-/ > > Sure. > > > That's not true. clang-format can happily format diffs. When formatting > > diffs though it tries to format your changes, not the context around the > > diffs > > Ah, OK. That makes sense. > > >* Update your working tree to the parent of the commit that did the > > reformat. > > This also makes sense. > > Have we considered adding an easy-to-remember tag for that commit to > make that easier in the future? > Yes, please check out https://bugzilla.mozilla.org/show_bug.cgi?id=1508324. (For git users, its sha1 will be recorded in .git-blame-ignore-revs for the benefit of the git hyper-blame command[1].) [1] https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/git-hyper-blame.html -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 11/29/2018 04:38 AM, Sylvestre Ledru wrote: The experimentation with dom/media highlighted that we need an efficient way to automatically rebase patches before the merge. To achieve this, we updated the bootstrap process to include an extension called hg formatsource. This extension will automatically rebase the local changes to avoid conflicts. Please note that it is important that the extension is installed before the pulling a revision including the reformatted sources. I attempted to experiment with this, and ran into some issues and confusion, which I wrote down in the form of https://bugzilla.mozilla.org/show_bug.cgi?id=1511093 ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 11/29/18 11:40 AM, Ehsan Akhgari wrote: Yes, that is true sadly. But to be fair here, old mq patches that do not apply due to age are already beyond saving with any kind of automated tooling, and they require manual work to get them applied first. :-/ Sure. That's not true. clang-format can happily format diffs. When formatting diffs though it tries to format your changes, not the context around the diffs Ah, OK. That makes sense. * Update your working tree to the parent of the commit that did the reformat. This also makes sense. Have we considered adding an easy-to-remember tag for that commit to make that easier in the future? -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 11/29/18 11:40 AM, Ehsan Akhgari wrote: Yes, that is true sadly. But to be fair here, old mq patches that do not apply due to age are already beyond saving with any kind of automated tooling, and they require manual work to get them applied first. :-/ Sure. That's not true. clang-format can happily format diffs. When formatting diffs though it tries to format your changes, not the context around the diffs Ah, OK. That makes sense. * Update your working tree to the parent of the commit that did the reformat. This also makes sense. Have we considered adding an easy-to-remember tag for that commit to make that easier in the future? -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
This short guide should be helpful for Mercurial users: https://docs.google.com/document/d/13AwAsvKMhH0mflDlfatBqn6LmZHiQih76oxM4zfrPl4 On Thu, Nov 29, 2018 at 11:49 AM Ehsan Akhgari wrote: > On Thu, Nov 29, 2018 at 11:44 AM Honza Bambas wrote: > >> On 2018-11-29 17:12, Ehsan Akhgari wrote: >> > On Thu, Nov 29, 2018 at 8:00 AM Boris Zbarsky wrote: >> > >> >> On 11/29/18 7:38 AM, Sylvestre Ledru wrote: >> >>> This extension will automatically rebase the local changes to avoid >> >>> conflicts. >> >> Sylvestre, >> >> >> >> Will it also update mq patches, or just in-repo commits? >> >> >> > I don't think this extension is aware of mq patches (since they don't >> > really get rebased), but that being said from my foggy memory of using >> mq >> > ages ago, I seem to remember that it used to be possible to use the >> qfinish >> > command to convert them into regular hg commits and then later on to use >> > qimport -r (IIRC) to convert them back into mq patches again. If that >> is >> > still a workflow that works, then it should be possible to: >> > >> >* Pull from a pre-reformat of the tree, run ./mach bootstrap to >> install >> > hg-formatsource. >> >* Do a one-time translation of your mq queue to a set of commits. >> >* Pull from hg.mozilla.org to pick up the reformat commit. >> >> And you forget that a merge will be needed here, because the formatting >> changes will likely collide with the code one's patches are touching. >> > > No, I didn't. :-) That's exactly what the hg-formatsource extension does > for you, it reformats your local changes on the fly as the rebase is in > progress, so you will get no collisions. > > >> When we were mass-converting from PRUint32 to uint32_t etc, there was a >> tool capable of converting your patches based on the pre-formated code >> to be apply-able on the formatted code. >> >> This is what we are missing. So some of us may expect a huge merge pain >> w/o something like that. >> > > No, those are old days and long gone, my friend. We are living in a new > world with better tools these days (for Mercurial users). > > >> OTOH, if the changes are only whitespace changes, maybe there is some >> way `patch --ignore-whitespace --fuzz N` could apply the patches. Then >> just re-format and your patches are OK. >> >> >> >* Do a one-time translation of your mq queue back to a series of >> patches. >> >> That doesn't make much sense, because the commit history will look >> something like (newest to oldest): >> - merge of my patches with the formatted changes, having two parents >> (formatted code default + my mq tip) >> - formatted code `tip` (or `default`) >> - my mq committed [ ] >> - pre-formated parent >> - ... >> >> You can't just recreate your mq from such changeset tree and you also >> can't avoid the likely quite complicated merge anyway. >> > > Again, no merge commits. Please do note that I was suggesting there that > you should use the rebase command, not merge. I think that would be |hg > pull --rebase|. > > -- > Ehsan > -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 2018-11-29 17:49, Ehsan Akhgari wrote: > No, I didn't. :-) That's exactly what the hg-formatsource extension does for you, it reformats your local changes on the fly as the rebase is in progress, so you will get no collisions. \o/ No, those are old days and long gone, my friend. We are living in a new world with better tools these days (for Mercurial users). \o/ \o/ Again, no merge commits. Please do note that I was suggesting there that you should use the rebase command, not merge. I think that would be |hg pull --rebase|. I missed that! This is awesome, thanks. Hopefully it will work well :) Thanks! -hb- -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 8:44 AM Honza Bambas wrote:
> OTOH, if the changes are only whitespace changes, maybe there is some
> way `patch --ignore-whitespace --fuzz N` could apply the patches. Then
> just re-format and your patches are OK.
>
The changes that clang-format makes include things like moving a { to a
different line, which I believe ignore-whitespace doesn't recognize as a
whitespace only change, because it is across multiple lines, and things
like reflowing comment blocks to reduce their width to 80 characters, which
of course will also not be counted as whitespace only.
Andrew
>
> >* Do a one-time translation of your mq queue back to a series of
> patches.
>
> That doesn't make much sense, because the commit history will look
> something like (newest to oldest):
> - merge of my patches with the formatted changes, having two parents
> (formatted code default + my mq tip)
> - formatted code `tip` (or `default`)
> - my mq committed [ ]
> - pre-formated parent
> - ...
>
> You can't just recreate your mq from such changeset tree and you also
> can't avoid the likely quite complicated merge anyway.
>
> -hb-
>
> >
> > Cheers,
>
> ___
> 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
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 11:44 AM Honza Bambas wrote: > On 2018-11-29 17:12, Ehsan Akhgari wrote: > > On Thu, Nov 29, 2018 at 8:00 AM Boris Zbarsky wrote: > > > >> On 11/29/18 7:38 AM, Sylvestre Ledru wrote: > >>> This extension will automatically rebase the local changes to avoid > >>> conflicts. > >> Sylvestre, > >> > >> Will it also update mq patches, or just in-repo commits? > >> > > I don't think this extension is aware of mq patches (since they don't > > really get rebased), but that being said from my foggy memory of using mq > > ages ago, I seem to remember that it used to be possible to use the > qfinish > > command to convert them into regular hg commits and then later on to use > > qimport -r (IIRC) to convert them back into mq patches again. If that is > > still a workflow that works, then it should be possible to: > > > >* Pull from a pre-reformat of the tree, run ./mach bootstrap to > install > > hg-formatsource. > >* Do a one-time translation of your mq queue to a set of commits. > >* Pull from hg.mozilla.org to pick up the reformat commit. > > And you forget that a merge will be needed here, because the formatting > changes will likely collide with the code one's patches are touching. > No, I didn't. :-) That's exactly what the hg-formatsource extension does for you, it reformats your local changes on the fly as the rebase is in progress, so you will get no collisions. > When we were mass-converting from PRUint32 to uint32_t etc, there was a > tool capable of converting your patches based on the pre-formated code > to be apply-able on the formatted code. > > This is what we are missing. So some of us may expect a huge merge pain > w/o something like that. > No, those are old days and long gone, my friend. We are living in a new world with better tools these days (for Mercurial users). > OTOH, if the changes are only whitespace changes, maybe there is some > way `patch --ignore-whitespace --fuzz N` could apply the patches. Then > just re-format and your patches are OK. > > > >* Do a one-time translation of your mq queue back to a series of > patches. > > That doesn't make much sense, because the commit history will look > something like (newest to oldest): > - merge of my patches with the formatted changes, having two parents > (formatted code default + my mq tip) > - formatted code `tip` (or `default`) > - my mq committed [ ] > - pre-formated parent > - ... > > You can't just recreate your mq from such changeset tree and you also > can't avoid the likely quite complicated merge anyway. > Again, no merge commits. Please do note that I was suggesting there that you should use the rebase command, not merge. I think that would be |hg pull --rebase|. -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 2018-11-29 17:12, Ehsan Akhgari wrote: On Thu, Nov 29, 2018 at 8:00 AM Boris Zbarsky wrote: On 11/29/18 7:38 AM, Sylvestre Ledru wrote: This extension will automatically rebase the local changes to avoid conflicts. Sylvestre, Will it also update mq patches, or just in-repo commits? I don't think this extension is aware of mq patches (since they don't really get rebased), but that being said from my foggy memory of using mq ages ago, I seem to remember that it used to be possible to use the qfinish command to convert them into regular hg commits and then later on to use qimport -r (IIRC) to convert them back into mq patches again. If that is still a workflow that works, then it should be possible to: * Pull from a pre-reformat of the tree, run ./mach bootstrap to install hg-formatsource. * Do a one-time translation of your mq queue to a set of commits. * Pull from hg.mozilla.org to pick up the reformat commit. And you forget that a merge will be needed here, because the formatting changes will likely collide with the code one's patches are touching. When we were mass-converting from PRUint32 to uint32_t etc, there was a tool capable of converting your patches based on the pre-formated code to be apply-able on the formatted code. This is what we are missing. So some of us may expect a huge merge pain w/o something like that. OTOH, if the changes are only whitespace changes, maybe there is some way `patch --ignore-whitespace --fuzz N` could apply the patches. Then just re-format and your patches are OK. * Do a one-time translation of your mq queue back to a series of patches. That doesn't make much sense, because the commit history will look something like (newest to oldest): - merge of my patches with the formatted changes, having two parents (formatted code default + my mq tip) - formatted code `tip` (or `default`) - my mq committed [ ] - pre-formated parent - ... You can't just recreate your mq from such changeset tree and you also can't avoid the likely quite complicated merge anyway. -hb- Cheers, ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 11:30 AM Boris Zbarsky wrote: > On 11/29/18 11:12 AM, Ehsan Akhgari wrote: > >* Pull from a pre-reformat of the tree, run ./mach bootstrap to > install > > hg-formatsource. > >* Do a one-time translation of your mq queue to a set of commits. > >* Pull from hg.mozilla.org to pick up the reformat commit. > >* Do a one-time translation of your mq queue back to a series of > patches. > > So just to be clear, I have a dozen or so queues with hundreds of > patches in them, not all of which will immediately apply due to age. So > this is not a simple operation, unfortunately... > Yes, that is true sadly. But to be fair here, old mq patches that do not apply due to age are already beyond saving with any kind of automated tooling, and they require manual work to get them applied first. :-/ > I guess there's no real way to clang-format diffs, so maybe the answer > is manual merging when I actually need those patches. > That's not true. clang-format can happily format diffs. When formatting diffs though it tries to format your changes, not the context around the diffs, as the use case it has been designed for is for example quickly reformatting your changes in a pre-commit hook. But still all is not lost here. When you do decide to do the manual merging when you needed those patches, you would need to: * Update your working tree to the parent of the commit that did the reformat. * Apply your patch to that tree and reformat the tree. * Diff the reformat commit and your current working directory. That would give the reformatted diff. -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 11/29/18 11:12 AM, Ehsan Akhgari wrote: * Pull from a pre-reformat of the tree, run ./mach bootstrap to install hg-formatsource. * Do a one-time translation of your mq queue to a set of commits. * Pull from hg.mozilla.org to pick up the reformat commit. * Do a one-time translation of your mq queue back to a series of patches. So just to be clear, I have a dozen or so queues with hundreds of patches in them, not all of which will immediately apply due to age. So this is not a simple operation, unfortunately... I guess there's no real way to clang-format diffs, so maybe the answer is manual merging when I actually need those patches. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 4:38 AM Sylvestre Ledru wrote: > The experimentation with dom/media highlighted that we need an efficient > way to automatically rebase patches before the merge. > To achieve this, we updated the bootstrap process to include an extension > called hg formatsource. > This extension will automatically rebase the local changes to avoid > conflicts. > Please note that it is important that the extension is installed before > the pulling a revision including the reformatted sources. > > More information on: > > https://docs.google.com/document/d/13AwAsvKMhH0mflDlfatBqn6LmZHiQih76oxM4zfrPl4/edit > https://bugzilla.mozilla.org/show_bug.cgi?id=1507711 This is great, I was wondering how badly the changes would mess up my in-progress patches. However I've read the doc and it isn't entirely clear to me what I actually need to do. It talks about needing to patch the local version-control-tools until some bugs are fixed, some of which are and some aren't currently. Do I just need to run './mach bootstrap' or is there something more? ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On Thu, Nov 29, 2018 at 8:00 AM Boris Zbarsky wrote: > On 11/29/18 7:38 AM, Sylvestre Ledru wrote: > > This extension will automatically rebase the local changes to avoid > > conflicts. > > Sylvestre, > > Will it also update mq patches, or just in-repo commits? > I don't think this extension is aware of mq patches (since they don't really get rebased), but that being said from my foggy memory of using mq ages ago, I seem to remember that it used to be possible to use the qfinish command to convert them into regular hg commits and then later on to use qimport -r (IIRC) to convert them back into mq patches again. If that is still a workflow that works, then it should be possible to: * Pull from a pre-reformat of the tree, run ./mach bootstrap to install hg-formatsource. * Do a one-time translation of your mq queue to a set of commits. * Pull from hg.mozilla.org to pick up the reformat commit. * Do a one-time translation of your mq queue back to a series of patches. Cheers, -- Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 11/29/18 1:38 PM, Sylvestre Ledru wrote: This extension will automatically rebase the local changes to avoid conflicts. Is there a way to do the same for cinnabar users? -- Emilio ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: The upcoming C/C++ coding style change
On 11/29/18 7:38 AM, Sylvestre Ledru wrote: This extension will automatically rebase the local changes to avoid conflicts. Sylvestre, Will it also update mq patches, or just in-repo commits? -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
