Page MenuHome

Clang-format migration: Investigate branch migration plan & document it
Closed, ResolvedPublic

Description

Before applying the big-bang migration, it's critical to investigate how hard it will be to merge branches that started before the big bang change.

This task:

  • Figure out the best approach for branches-- should they apply make format themselves? Does that work? Or ignore it?
  • Document this approach either in the wiki or the blog post.

Details

Type
To Do

Event Timeline

Also, just tried merging this into asset-engine branch, looks like -Xours is mandatory… Should not be an issue with following process though, I think:

  1. Merge temp-clang-format into asset-engine (about 6 conflicting files, essentially due to recent pre-clang-format edits in master).
  2. Apply clang-format on temp-clang-format and commit.
  3. Apply clang-format on asset-engine and commit.
  4. Merge clang-format into asset-engine using git merge -Xignore-all-space -Xours temp-clang-format command.

For the final merge, regular merge is catastrophic ( ALL edited files in asset-engine are conflicting, over 30). Using only -Xignore-all-space option, it’s a tad better (although I did not check the actual result), but am still getting 23 files conflicting, even the mere addition of a single function in creator.c is enough to create an issue!

After discussion, it would be nice to avoid applying clang-format on whole codebase in every branch. So I tried using -s ours instead of -Xours, following that process:

  1. Merge temp-clang-format into asset-engine.
  2. Apply clang-format on temp-clang-format and commit.
  3. Merge clang-format into asset-engine using git merge -Xignore-all-space -s ours temp-clang-format command.
  4. Apply clang-format on asset-engine and commit.

That does not really work well, it basically applies none of clang-format changes, i.e. after step 4 all files get modified, not only the branch-modified ones.

So instead, I reversed the step 3 and 4 of the first attempt, think this gives best results (smooth merge with much smaller commit when applying clang-format on the branch):

  1. Merge temp-clang-format into asset-engine (about 6 conflicting files, essentially due to recent pre-clang-format edits in master).
  2. Apply clang-format on temp-clang-format and commit.
  3. Merge clang-format into asset-engine using git merge -Xignore-all-space -Xours temp-clang-format command.
  4. Apply clang-format on asset-engine and commit.

Yes, I think it can be summarized as this once the formatting is done in master:

git merge <clang-format-commit>^1
... merge any conflicts and commit...

git merge -Xignore-all-space -Xours <clang-format-commit>
make format
git commit

git merge master
... merge any conflicts and commit...

I've added a script to rebase and merge master into branches: rBDTa64a4237d820: Add script to merge clang-format changes into branches.
https://wiki.blender.org/wiki/Tools/ClangFormat#Migrating_Branches_and_Patches

Ended up being more complicated than I hoped, wouldn't mind if someone else can verify the logic is sane.

Brecht Van Lommel (brecht) claimed this task.