Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang-format all C code, enforce formatting in CI. #329

Merged
merged 5 commits into from
Jul 11, 2023

Conversation

cpu
Copy link
Member

@cpu cpu commented Jul 10, 2023

Description

This branch builds on the already checked-in .clang-format, making it easier to use locally and enforcing that C code in-repo is always formatted using it w/ a CI check.

I haven't attempted to extend the Windows Makefile with similar functionality on the assumption that all development is happening on Unix-like-systems and not Windows.

Resolves #327

Makefile: rm empty space.

Simple tidy commit.

Makefile: fix PHONY targets.

The all, clean, and test aren't traditional file targets: they don't produce a built artifact on disk from other files. If there were
to be a file with a matching name found in the workdir these targets wouldn't perform their jobs as expected, thinking the workdir was up-to-date.

This commit marks these targets as "PHONY" targets, telling Make that they're not associated with produced files and are meta-targets for running helpful commands.

Makefile: add format-check and format targets.

This commit adds two new PHONY targets:

  • make format-check for enforcing that all .c/.h files are correctly formatted. This will be run in CI in subsequent commits once it will pass.

  • make format for reformatting all .c/.h files according to the clang-format profile. This can be run by developers to format code.

Both targets use find to locate all suitable .h and .c files. Crucially we configure find to ignore the target build directory, as well as the src/rustls.h file that is generated by cbindgen.

tests: universally apply clang-format.

This commit uses the new make format target to re-format all of the .c/.h files using clang-format. Afterwards the make format-check passes, where previously it flagged the re-formatted files in this commit as needing formatting.

ci: enforce C formatting w/ make format-check.

This commit extends the existing format task in CI that was enforcing the use of cargo fmt to also call make format-check to enforce the use of clang-format for C code.

@cpu cpu self-assigned this Jul 10, 2023
cpu added 5 commits July 11, 2023 10:30
The `all`, `clean`, and `test` aren't traditional file targets: they
don't produce a built artifact on disk from other files. If there were
to be a file with a matching name found in the workdir these targets
wouldn't perform their jobs as expected, thinking the workdir was
up-to-date.

This commit marks these targets as "PHONY" targets, telling Make that
they're not associated with produced files and are meta-targets for
running helpful commands.
This commit adds two new PHONY targets:

* `make format-check` for enforcing that all .c/.h files are correctly
  formatted. This will be run in CI in subsequent commits once it will
  pass.

* `make format` for reformatting all .c/.h files according to the
  clang-format profile. This can be run by developers to format code.

Both targets use `find` to locate all suitable `.h` and `.c` files.
Crucially we configure `find` to ignore the `target` build directory, as
well as the `src/rustls.h` file that is generated by `cbindgen`.
This commit uses the new `make format` target to re-format all of the
.c/.h files using `clang-format`. Afterwards the `make format-check`
passes, where previously it flagged the re-formatted files in this
commit as needing formatting.
This commit extends the existing format task in CI that was enforcing
the use of `cargo fmt` to also call `make format-check` to enforce the
use of `clang-format` for C code.
@cpu cpu force-pushed the cpu-clang-format-all-the-things branch from 4d0f519 to 21e200e Compare July 11, 2023 14:33
@cpu
Copy link
Member Author

cpu commented Jul 11, 2023

cpu force-pushed the cpu-clang-format-all-the-things branch from 4d0f519 to 21e200e

Resolved conflicts.

Copy link
Collaborator

@jsha jsha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks for all the cleanups!

@jsha jsha merged commit 254e1d6 into rustls:main Jul 11, 2023
@cpu cpu deleted the cpu-clang-format-all-the-things branch July 11, 2023 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format C code with clang-format
2 participants