The EOL fiasco

Discussion area about developing or extending OGRE, adding plugins for it or building applications on it. No newbie questions please, use the Help forum for that.
Post Reply

Strip or merge?

Strip (makes repo size smaller, but harder to sync)
8
50%
Merge (repo size is kept)
8
50%
 
Total votes: 16

User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

The EOL fiasco

Post 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.
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: The EOL fiasco

Post 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.
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 66
Contact:

Re: The EOL fiasco

Post 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.
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: The EOL fiasco

Post 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.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: The EOL fiasco

Post 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?
User avatar
sinbad
OGRE Retired Team Member
OGRE Retired Team Member
Posts: 19269
Joined: Sun Oct 06, 2002 11:19 pm
Location: Guernsey, Channel Islands
x 66
Contact:

Re: The EOL fiasco

Post 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
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: The EOL fiasco

Post 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.
Last edited by scrawl on Sat Feb 22, 2014 8:16 pm, edited 1 time in total.
TV4Fun
Gnoblar
Posts: 19
Joined: Tue Mar 16, 2004 8:43 am
Contact:

Re: The EOL fiasco

Post 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.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: The EOL fiasco

Post 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.
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: The EOL fiasco

Post 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.
TV4Fun
Gnoblar
Posts: 19
Joined: Tue Mar 16, 2004 8:43 am
Contact:

Re: The EOL fiasco

Post 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.
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: The EOL fiasco

Post 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.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: The EOL fiasco

Post 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.
Transporter
Minaton
Posts: 933
Joined: Mon Mar 05, 2012 11:37 am
Location: Germany
x 110

Re: The EOL fiasco

Post 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.
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: The EOL fiasco

Post 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!
scrawl
OGRE Expert User
OGRE Expert User
Posts: 1119
Joined: Sat Jan 01, 2011 7:57 pm
x 216

Re: The EOL fiasco

Post 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.
TV4Fun
Gnoblar
Posts: 19
Joined: Tue Mar 16, 2004 8:43 am
Contact:

Re: The EOL fiasco

Post 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?
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: The EOL fiasco

Post 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 5278 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 5278 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 5278 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 5278 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 5278 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 5278 times
Step06.png
Step06.png (31.7 KiB) Viewed 5278 times
7. If you've done it correctly; you've succeeded:
Step07.png
Step07.png (93.09 KiB) Viewed 5278 times
User avatar
dark_sylinc
OGRE Team Member
OGRE Team Member
Posts: 5296
Joined: Sat Jul 21, 2007 4:55 pm
Location: Buenos Aires, Argentina
x 1278
Contact:

Re: The EOL fiasco

Post 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 5276 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 5276 times
5. You can select multiple the patches at the same time. Then hit import
Step03.png
Step03.png (93.41 KiB) Viewed 5276 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!
Post Reply