Page 1 of 1

The EOL fiasco

Posted: Sat Feb 22, 2014 7:17 pm
by dark_sylinc
Some of you have noticed a commit where I've changed all tabs to spaces.

Unfortunately, I did not notice Hg failed it's job in converting CR/LF to LF line endings and as result, CR/LF EOLs were pushed.

What's worse is that this issue was discovered too late, and we have two problems:
  • Default -> 2.0 merge was veeeery difficult. I'm not doing this twice. Not a chance.
  • There are already commits being pushed on top of default's branch with the wrong EOL; sometimes CR/LF & LF are being mixed now.
Because there's a merge, unfortunately the diff idea and close the branch that was suggested is not viable.

Alternatives I can see:
  • Push an .hgeol file to the repo so that everyone commits using CR/LF line endings from now on. (AFAIK the user still manually need to enable the eol extension)
  • Perform another mass commit to go back to LF. Then merge with branches but keep all files "mine". But it may create more fiasco.
Other suggestions are appreciated.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 7:32 pm
by scrawl
Perform another mass commit to go back to LF. Then merge with branches but keep all files "mine". But it may create more fiasco.
I think we should strip those commits and not just undo them because they increase the size of the repository a lot. Not suprising since basically every line of code in the repo was recommitted..
Default -> 2.0 merge was veeeery difficult. I'm not doing this twice. Not a chance.
You can save the results of your merge (e.g. a backup of the whole file tree). Then we can strip the bad commits and re-do them cleanly. Run merge again (with the internal:merge tool, afaik the default), paste the backup, mark all conflicts as resolved, done? Just make sure not to commit the line endings again :)

So, to sum up:
1. rebase children of the bad commits onto the parents
2. strip bad commits
3. redo tabs -> spaces commits cleanly
4. apply the merge with the method explained above.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 7:55 pm
by sinbad
Matias asked me to comment here :)

So in general for future reference, resist the urge to do tidy up commits, it's just never worth it in merge time. Think of every line of code you change as a line you might have to merge later, so don't make work for yourself.

I would concur that returning to the earlier commit before the tidy up is best, but you can't use strip if you've pushed without causing a headache for every person that has pulled that commit already. Instead, update locally back to the commit before the one which changed all the white space, then diverge from there, either merging it into wherever it needs to go or merging other stuff into it, doing follow-on commits etc. This obviously diverges the branch into 2 different heads which you have to resolve eventually. The way to do this is to merge one head into the other, but to prefer all the changes from your correct head (the one without all the white space changes). You can do that by doing the merge on the command line using these options: http://mercurial.selenic.com/wiki/TipsA ... ng_a_merge

Once you've done this it's the equivalent of the strip & start again approach but without actually destroying the commit and unifying the streams back into one. Repo size won't shrink this way though, but it avoids having to tell everyone to strip their own repos if they've pulled your commit. If you'd prefer to strip you can, but be prepared to tell people how to work with it on their next pull.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 7:58 pm
by scrawl
Hi Sinbad :)
Repo size won't shrink this way though
Yeah, pity. Doesn't really make it much better than reverting the commits.
but it avoids having to tell everyone to strip their own repos if they've pulled your commit. If you'd prefer to strip you can, but be prepared to tell people how to work with it on their next pull.
I don't see that as a big issue. If someone hasn't cleaned his repo, it will immediately show up in the pull request diff view by the huge number of files.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:07 pm
by dark_sylinc
mmmmm....... I've forgotten that I can override the merge results with the ones that I already have!

That is great!!!

Stripping sounds nice, but I agree it is a pain to synchronize. Although the team with Write access is low, pull requests also need to have it stripped, and then there's the build servers.
scrawl wrote:I think we should strip those commits and not just undo them because they increase the size of the repository a lot. Not suprising since basically every line of code in the repo was recommitted..
You said it took you a lot of time to download the changes.
But that sounds strange. I measured the repo size before and after and it was only ~8MBs bigger. It took only a few minutes to push, and I've got a 512kbit up connection (Real I get 45-60 kb/s).
May be something happened to your connection while you were pulling?

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:11 pm
by sinbad
I'm assuming you're talking about stripping on Bitbucket. Anyone who has already pulled that commit, when they pull next they'll end up with 2 heads on that branch, and will have to pick the right one to continue with if they decide to do dev work. It's not hard to resolve if you know what you're doing but you might have some Qs to answer. And beware of people trying to resolve it by merging those 2 heads together before submitting a PR ;)

[edit]that was a reply to scrawl BTW - if you go the update/re-merge route you won't have this problem, at the expense of having all content still in the repo

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:15 pm
by scrawl
For just text, 8mb is still a whole lot.

No, I said the push time was large, not the pull time. I normally have a comparable up speed but it took 30 minutes to push my updated repo. Whatever!
and then there's the build servers
All build servers I know start with a fresh clone for each build (usually a shallow clone so that it doesn't take ages to download). Travis CI, for example. Which build servers are you talking about?

We could do a vote regarding the strip vs alternate head decision. But it would be good to get to a conclusion rather sooner than later.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:15 pm
by TV4Fun
There haven't been that many commits to default since then. Using LF endings is the preferred style, and if you fix it now, the difficulties will be minimal. The longer you wait, the harder it will be to fix the line endings. You should push a mass commit now changing all the endings back to LF and enable the EOL extension to keep it that way.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:17 pm
by dark_sylinc
Obviously the best option here is to include a virus in Ogre source code that will strip the unwanted branch in all repos it can find.

**just kidding**

Thank you very much boss, err Sinbad! And you too scrawl.

I'm already starting w/ Sinbad's method (anyway scrawl's and sinbad's first steps are the same) as time is of the essence here.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:18 pm
by scrawl
There haven't been that many commits to default since then. Using LF endings is the preferred style, and if you fix it now, the difficulties will be minimal. The longer you wait, the harder it will be to fix the line endings. You should push a mass commit now changing all the endings back to LF and enable the EOL extension to keep it that way.
Whatever you do, don't push a revert commit. As far as I understand, that'll just add another 8 mb of junk to repo.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:24 pm
by TV4Fun
scrawl wrote:
There haven't been that many commits to default since then. Using LF endings is the preferred style, and if you fix it now, the difficulties will be minimal. The longer you wait, the harder it will be to fix the line endings. You should push a mass commit now changing all the endings back to LF and enable the EOL extension to keep it that way.
Whatever you do, don't push a revert commit. As far as I understand, that'll just add another 8 mb of junk to repo.
Out of the 287 MB already in the history, I don't see it as being that big of an addition. It would be better than continuing on with a broken source tree.

Re: The EOL fiasco

Posted: Sat Feb 22, 2014 8:58 pm
by scrawl
Ah, you created a poll.
I've voted strip. My reasoning is that it may be a bit of (one-time) manual work for contributors who've already pulled, but the smaller repo size should pay off for everyone in the long run.

Re: The EOL fiasco

Posted: Sun Feb 23, 2014 3:39 am
by dark_sylinc
There is a strong tie between merge and strip, even among the official devs.
But I can't reach them all at this hour.

I'm going to push a merge. If we end up stripping, we just include the merge in the stripping.

Re: The EOL fiasco

Posted: Sun Feb 23, 2014 4:23 pm
by Transporter
dark_sylinc wrote:Push an .hgeol file to the repo so that everyone commits using CR/LF line endings from now on. (AFAIK the user still manually need to enable the eol extension)
I think this is a good idea. The OGRE Developer Guide (Win) says:
By enabling the win32text extension, you cause all text files to be converted to Unix line endings before commit, and converted to Windows line endings when your working copy is updated.
So, why not using .hgeol with LF? I think all compiler on Windows can handle LF.

Re: The EOL fiasco

Posted: Sun Feb 23, 2014 6:59 pm
by dark_sylinc
STRIPPING HAS BEEN PERFORMED. Strip the following commits from your local repositories:
dad316a675428a66c24e5dc6f0e6b7abb6d5d7d1
81746916438203c5dfc06177d8eb91e84d4f50f3


Or alternatively, do a clean clone.
We've agreed in the team to sync ourselves with the stripping, as it was a clean solution.

Cheers!

Re: The EOL fiasco

Posted: Sun Feb 23, 2014 8:54 pm
by scrawl
Thanks. It should also be noted:
- If you've already pushed, you need to strip on your bitbucket fork as well (through the webinterface, "Strip changesets")
- If you have done any commits on top of the commits that you're stripping they will be stripped as well! To save your work, rebase them to the previous changeset (for default, b381289 "Misc warning and build fixes") first.

Now we need a PSA with a step-by-step noob friendly tutorial.

Re: The EOL fiasco

Posted: Mon Feb 24, 2014 3:12 pm
by TV4Fun
Since I am not intimately familiar with the finer points of mercurial, and I am running on online fork with several other developers, is there a way to strip these commits without either losing all the code we have written in the last 3 weeks or forcing everyone to do a complicated hg operation?

Re: The EOL fiasco

Posted: Tue Feb 25, 2014 2:13 am
by dark_sylinc
GUIDE - METHOD 1
This guide will help you to transplant all your fork changes to the parallel branch. It needs TortoiseHg, which works on both Windows & Linux (IIRC, OS X too):

1. Before you do anything, MAKE A BACKUP. Just copy the repo to another folder, in case you screw up.

1. First you have to be sure that the strip extension is enabled:
Step00.png
Step00.png (62.61 KiB) Viewed 5287 times
If it wasn't, tick the checkbox and restart Tortoise Workbench.

2. Your repo will look like this. In the picture, you want to move the commits that say "Commit to move 0N" to the new branch.
Step01.png
Step01.png (166.5 KiB) Viewed 5287 times
3. Make sure the current active commit is the "good" branch. The active commit will become the parent of the changes you will be moving. If you're unsure, you should probably probably select the first common cloned parent; otherwise you could get merge conflicts during the process.
Now, select all the commits you want to transplant, then right click, then "Graft Selected to Local". Note that the order in which you select the commits seems to be important, as I got a merge conflicts in one of my attempts. I simply restarted the Graft and that did the trick.
Step02.png
Step02.png (125.29 KiB) Viewed 5287 times
4. You will see a window like this one. Hit the Graft button and wait for it to finish.
Step03.png
Step03.png (50.13 KiB) Viewed 5287 times
5. If there were no merge conflicts (if you chose the right ancestor, there shouldn't), it should look like this:
Step04.png
Step04.png (85.74 KiB) Viewed 5287 times
6. Now choose the bad commit that needs to be stripped. Right click, Modify History->Strip.
Step05.png
Step05.png (101.91 KiB) Viewed 5287 times
Step06.png
Step06.png (31.7 KiB) Viewed 5287 times
7. If you've done it correctly; you've succeeded:
Step07.png
Step07.png (93.09 KiB) Viewed 5287 times

Re: The EOL fiasco

Posted: Tue Feb 25, 2014 2:30 am
by dark_sylinc
GUIDE - Method 2
Method 2 uses patch files instead. If method 1 is giving you too many non-trivial conflicts, they have the advantage that patches usually won't.
However patch files don't preserve the following information:
  • They break if trying to add an empty file in a commit (just edit by hand the patch file to remove the empty file line). The file will be added in the next import when it actually has content in it.
  • They don't work with merges in the middle: If there were no merge conflicts in that merge, you're in luck, just bypass that commit as if it never existed. Otherwise you're screwed with this method.
  • They lose "files moved" information. If you moved files, the patch will destroy the file in the old location, and recreate it in the new location. However Mercurial won't know that it was actually moved. If there is a commit where you've moved files, recreate that commit by hand, then continue exporting/importing.
1. First step is the same as Guide 1. MAKE A BACKUP

2. See in Guide 1 how to enable Strip.

3. Select the commits you want to transplant. Right click, "Export Selected..."; and choose a folder location.
Step01.png
Step01.png (30.94 KiB) Viewed 5285 times
4. Now make sure you're in the right commit (the one you want to be the parent). Then click on Repository->Import...
Step02.png
Step02.png (46.56 KiB) Viewed 5285 times
5. You can select multiple the patches at the same time. Then hit import
Step03.png
Step03.png (93.41 KiB) Viewed 5285 times
6. You should now see both branches with the same commits (unless you skipped merges or commits that only add empty files). Now strip the bad commit. See step 6 from Guide 1.

7. You're done!