use clang-format in pre-commit

[pmeier]

Follow-up to pytorch/text#1546 (review).

[facebook-github-bot]

💊 CI failures summary and remediations

As of commit 7198db8 (more details on the Dr. CI page):

---

• 3/3 failures introduced in this PR

---

🕵️ 1 new failure recognized by patterns

The following CI failures do not appear to be due to upstream breakages:

CodeQL / build (1/1)

Step: "Build TorchVision" (full log | diagnosis details | 🔁 rerun)

2022-01-31T13:00:46.5845043Z ##[error]Process completed with exit code 1.

2022-01-31T13:00:46.4232117Z     self.finalize_options()
2022-01-31T13:00:46.4232651Z   File "/home/runner/.local/lib/python3.8/site-packages/setuptools/command/develop.py", line 52, in finalize_options
2022-01-31T13:00:46.4233131Z     easy_install.finalize_options(self)
2022-01-31T13:00:46.4233704Z   File "/home/runner/.local/lib/python3.8/site-packages/setuptools/command/easy_install.py", line 276, in finalize_options
2022-01-31T13:00:46.4234183Z     self._fix_install_dir_for_user_site()
2022-01-31T13:00:46.4234862Z   File "/home/runner/.local/lib/python3.8/site-packages/setuptools/command/easy_install.py", line 382, in _fix_install_dir_for_user_site
2022-01-31T13:00:46.4235523Z     self.create_home_path()
2022-01-31T13:00:46.4236262Z   File "/home/runner/.local/lib/python3.8/site-packages/setuptools/command/easy_install.py", line 1338, in create_home_path
2022-01-31T13:00:46.4236715Z     if path.startswith(home) and not os.path.isdir(path):
2022-01-31T13:00:46.4237229Z AttributeError: 'int' object has no attribute 'startswith'
2022-01-31T13:00:46.5845043Z ##[error]Process completed with exit code 1.
2022-01-31T13:00:46.5929601Z Post job cleanup.
2022-01-31T13:00:46.7300450Z [command]/usr/bin/git version
2022-01-31T13:00:46.7365743Z git version 2.34.1
2022-01-31T13:00:46.7413169Z [command]/usr/bin/git config --local --name-only --get-regexp core\.sshCommand
2022-01-31T13:00:46.7472200Z [command]/usr/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'core\.sshCommand' && git config --local --unset-all 'core.sshCommand' || :
2022-01-31T13:00:46.7911566Z [command]/usr/bin/git config --local --name-only --get-regexp http\.https\:\/\/github\.com\/\.extraheader
2022-01-31T13:00:46.7962897Z http.https://github.com/.extraheader
2022-01-31T13:00:46.7975306Z [command]/usr/bin/git config --local --unset-all http.https://github.com/.extraheader
2022-01-31T13:00:46.8033962Z [command]/usr/bin/git submodule foreach --recursive git config --local --name-only --get-regexp 'http\.https\:\/\/github\.com\/\.extraheader' && git config --local --unset-all 'http.https://github.com/.extraheader' || :
2022-01-31T13:00:46.8677689Z Cleaning up orphan processes

---

2 failures not recognized by patterns:

Job	Step	Action
format	Lint code format	🔁 rerun
cmake_macos_cpu	curl -o conda.sh https://repo.anaconda.com/miniconda/Miniconda3-latest-MacOSX-x86_64.sh
sh conda.sh -b
source $HOME/miniconda3/bin/activate
conda install -yq conda-build cmake
packaging/build_cmake.sh
	🔁 rerun

---

This comment was automatically generated by Dr. CI (expand for details).

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

[datumbox]

A couple of notes:

• I believe that PyTorch uses a specific version of clang and to keep it aligned with Core we have the specific binaries.
• I have concerns about the use of pre-commit if it forces a specific workflow to contributors. I am OK using it in the CI as long as we don't force users to use the specific paradigm but instead offer them to ability to run linters manually via commands or via IDE integrations.

[pmeier]

I believe that PyTorch uses a specific version of clang and to keep it aligned with Core we have the specific binaries.

Not sure I understand. There is difference in that we currently download the binary from https://oss-clang-format.s3.us-east-2.amazonaws.com/linux64/clang-format-linux64 whereas the hook downloads it from the official LLVM repo. I couldn't find any mention of this link outside of PyTorch. As you can see from the failing CI job, there seem to be differences. If that is intended, i.e. the binary we currently use is actually doing something different from the official one, we cannot use the hook anyway.

I have concerns about the use of pre-commit if it forces a specific workflow to contributors. I am OK using it in the CI as long as we don't force users to use the specific paradigm but instead offer them to ability to run linters manually via commands or via IDE integrations.

It doesn't force a specific workflow. You can still run everything manually if you like. In this specific case, It should actually help the user. Our contribution guide doesn't mention clang-format at all. If a contributor currently hits an issue with it, we don't have any instructions how to run it manually. With this hook, this will be done automatically.

[datumbox]

I couldn't find any mention of this link outside of PyTorch.

See references at #2872. It might no longer be the case, if the tests pass without modifications on the C++ files you should be OK. Worth checking.

You can still run everything manually if you like.

Could you explain how one runs the clang now without any use of pre-commit? For example, right now my IDE is setup to run ./.circleci/unittest/linux/scripts/run-clang-format.py -r torchvision/csrc.

[pmeier]

if the tests pass without modifications on the C++ files you should be OK.

That is not the case, thus we probably can't use the hook.