[libc++] Add a merge driver that can apply clang-format

[ldionne]

In preparation for the moment when we'll clang-format the whole code base, this patch adds a script that can be used to rebase patches across clang-format changes mechanically, without requiring manual intervention.

See https://discourse.llvm.org/t/rfc-clang-formatting-all-of-libc-once-and-for-all.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

In preparation for the moment when we'll clang-format the whole code base, this patch adds a script that can be used to rebase patches across clang-format changes mechanically, without requiring manual intervention.

See https://discourse.llvm.org/t/rfc-clang-formatting-all-of-libc-once-and-for-all.

---

Full diff: https://github.com/llvm/llvm-project/pull/73712.diff

1 Files Affected:

• (added) libcxx/utils/clang-format-merge-driver.sh (+38)

diff --git a/libcxx/utils/clang-format-merge-driver.sh b/libcxx/utils/clang-format-merge-driver.sh
new file mode 100755
index 000000000000000..6b1a339720d76a4
--- /dev/null
+++ b/libcxx/utils/clang-format-merge-driver.sh
@@ -0,0 +1,38 @@
+#!/usr/bin/env bash
+
+# This script can be installed in .git/config to allow rebasing old patches across
+# libc++'s clang-format of the whole tree. Most contributors should not require that
+# since they don't have many pre-clang-format patches lying around. This script is to
+# make it easier for contributors that do have such patches.
+#
+# The script is installed by running the following from the root of your repository:
+#
+#   $ git config merge.libcxx-reformat.name "Run clang-format when rebasing libc++ patches"
+#   $ git config merge.libcxx-reformat.driver "libcxx/utils/clang-format-merge-driver.sh %O %A %B %P"
+#   $ git config merge.libcxx-reformat.recursive binary
+#
+# This is based on https://github.com/nico/hack/blob/main/notes/auto_git_rebase_across_mechanical_changes.md.
+# Many thanks to Nico Weber for paving the way here.
+
+# Path to the file's contents at the ancestor's version.
+base=$1
+
+# Path to the file's contents at the current version.
+current=$2
+
+# Path to the file's contents at the other branch's version (for nonlinear histories, there might be multiple other branches).
+other=$3
+
+# The path of the file in the repository.
+path=$4
+
+clang-format --style=file --assume-filename=$path < $base > $base.tmp
+mv $base.tmp $base
+
+clang-format --style=file --assume-filename=$path < $current > $current.tmp
+mv $current.tmp $current
+
+clang-format --style=file --assume-filename=$path < $other > $other.tmp
+mv $other.tmp $other
+
+git merge-file $current $base $other

[ldionne]

This PR was tested with the following script:

#!/usr/bin/env bash

#
# This test sets up artificial branches with artificial changes
# to test that the clang-format merge driver works as intended.
#

# First, checkout a WIP branch with some random changes on it.
git checkout wip/format/to-rebase
git reset --hard review/clang-format-merge-driver

# Apply a change to a non-formatted file
cat <<EOF | git apply -
diff --git a/libcxx/include/__chrono/calendar.h b/libcxx/include/__chrono/calendar.h
index 91aaf6325389..5345e2c6027f 100644
--- a/libcxx/include/__chrono/calendar.h
+++ b/libcxx/include/__chrono/calendar.h
@@ -26,8 +26,8 @@ namespace chrono
 {

 struct local_t {};
-template<class _Duration>
-using local_time  = time_point<local_t, _Duration>;
+template<class _DurationType>
+using local_time  = time_point<local_t, _DurationType>;
 using local_seconds = local_time<seconds>;
 using local_days    = local_time<days>;

EOF
git add libcxx/include/__chrono/calendar.h
git commit -m "[chrono] Rename DurationType"

# Apply a change to an already-formatted file
cat <<EOF | git apply -
diff --git a/libcxx/include/__chrono/concepts.h b/libcxx/include/__chrono/concepts.h
index 61ec256b23ab..33cad4d0ef38 100644
--- a/libcxx/include/__chrono/concepts.h
+++ b/libcxx/include/__chrono/concepts.h
@@ -23,8 +23,8 @@ _LIBCPP_BEGIN_NAMESPACE_STD

 #if _LIBCPP_STD_VER >= 20

-template <class _Tp>
-concept __is_hh_mm_ss = __is_specialization_v<_Tp, chrono::hh_mm_ss>;
+template <class _Type>
+concept __is_hh_mm_ss = __is_specialization_v<_Type, chrono::hh_mm_ss>;

 template <class _Tp>
 concept __is_time_point = __is_specialization_v<_Tp, chrono::time_point>;

EOF
git add libcxx/include/__chrono/concepts.h
git commit -m "[concepts] Rename template parameter"

# Apply a change outside of libc++ (this one should not be resolved by the merge driver)
cat <<EOF | git apply -
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 1f61bb02c6ae..4c334d9a7a28 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -2423,7 +2423,7 @@ void DarwinClang::AddClangSystemIncludeArgs(const llvm::opt::ArgList &DriverArgs

 bool DarwinClang::AddGnuCPlusPlusIncludePaths(const llvm::opt::ArgList &DriverArgs,
                                               llvm::opt::ArgStringList &CC1Args,
-                                              llvm::SmallString<128> Base,
+                                              llvm::SmallString<256> Base,
                                               llvm::StringRef Version,
                                               llvm::StringRef ArchDir,
                                               llvm::StringRef BitDir) const {

EOF
git add clang/lib/Driver/ToolChains/Darwin.cpp
git commit -m "[clang] Use larger SmallString"

# Create a formatted branch on top of main
git checkout wip/format/formatted-main
git reset --hard review/clang-format-merge-driver
find libcxx/include clang/lib/Driver/ToolChains -type f | xargs clang-format -i
cat <<EOF > .gitattributes
libcxx/**/*.cpp merge=libcxx-reformat
libcxx/**/*.h   merge=libcxx-reformat
EOF
git add libcxx clang .gitattributes
git commit -m "Reformat the whole codebase and setup optional merge driver for formatted files"

# Try rebasing the changes on top of the formatted main
#
# There should be no conflicts due to formatting in libcxx/.
# There should be one conflict due to formatting in clang/, since the
# merge driver is not supposed to be in effect there (via .gitattributes).
git rebase wip/format/formatted-main wip/format/to-rebase

[cjdb]

What's the difference between this and git clang-format?

[brunodf-snps]

General remark: in a bash script, I would always quote all the string substitutions containing file names, just to avoid any trouble ever with spaces in file names. Also, I don't know if you care about clang-format failures?

Chromium seems to have ended up with a Python script for this?

[ldionne]

What's the difference between this and git clang-format?

When you install it, this script will be run when you git merge or when you git rebase, and it will apply these custom rules for resolving merge conflicts. So for example, if you modify a file in a branch before the formatting and then try to rebase across the clang-formatting of the whole codebase, this merge driver will get called by git during the merge process and will mechanically resolve the merge conflict by basically clang-formatting the changes on your branch and then performing the merge as normal.

That way you end up not even noticing that you had a "conflict" to resolve, and the rebase just works.

[ldionne]

General remark: in a bash script, I would always quote all the string substitutions containing file names, just to avoid any trouble ever with spaces in file names.

Yeah, I almost always do this and I don't know why this time around I got lazy. I fixed that now, thanks for the heads up.

Also, I don't know if you care about clang-format failures?

Chromium seems to have ended up with a Python script for this?

I wonder if that's a bit overkill for us. I expect this to be mostly a one-shot deal, I wouldn't really expect this script to stay around after the original clang-format run. I also expect that very few people will actually want to install this merge driver -- basically only contributors who have a lot of patches to rebase across the change, and maybe some downstream integrators will install it as a one-shot thing to deal with the large initial reformatting. But if you think it's useful for this to be more robust / cross-platform, I can easily migrate to Python.

[brunodf-snps]

I wonder if that's a bit overkill for us. I expect this to be mostly a one-shot deal, I wouldn't really expect this script to stay around after the original clang-format run. I also expect that very few people will actually want to install this merge driver -- basically only contributors who have a lot of patches to rebase across the change, and maybe some downstream integrators will install it as a one-shot thing to deal with the large initial reformatting. But if you think it's useful for this to be more robust / cross-platform, I can easily migrate to Python.

Bash is fine for me. I agree this merge driver will only occasionally be needed. In fact, it would not be bad if you could enable it for individual git merge or git rebase invocations with some command-line option. Perhaps that can be done with git -c on those invocations, but I don't immediately see how.

[brunodf-snps]

LGTM

[ldionne]

I'll merge this now to make it easier for contributors to play around with the script and see if they encounter issues with it.