CMake Pre-Build command

[ben.boeckel (Ben Boeckel (Kitware))]

@jwm

Can you explain why “format before build” is a bad idea? I’m not a CMake guru (yet)

It’s not a CMake-specific thing. I wouldn’t be happy with this in any build step. Editor? OK. Separate build target? OK. But not during the normal build. It upends your build graph because your source files are now outputs of some computation. Sure, that’s doable, but now they need to go to some other path (anything else is a circular dependency). Then your compiler is warning about copied source files and your editor which helpfully uses the compiler to drop you at warnings and errors is having you edit the reformatted file off in the hinterlands of your build tree instead of the one in your source tree.

@jwm

When is a better time to format the code than before building

On your code review/CI workflow part of the contribution pipeline. I’ve contributed to a number of projects without actually building code (typos, trivial fixes, etc.) and having the CI pipeline track my mistakes is much faster for me than faffing about getting dev environments up and running for trivial patches.

@jwm

You don’t want to wait for commit for a couple of reasons, not the least being that the newly formatted code would never have been built.

Eh, commits are fungible. (I don’t buy Mercurial or Fossil’s excuses; rebase is too useful.)

@jwm

Another is that git doesn’t encourage that type of modification before commit either,

Huh? It may not encourage, but it certainly doesn’t discourage.

@jwm

and there’s no way to enforce it server-side, so it relies upon developers configuring their repos properly (not reliable).

Configure it as part of your review/CI workflow. If you’re committing directly to master…don’t do that. Put server-side safeguards on what goes into master. That I can trust. I certainly wouldn’t trust developers setting up their local repos reliably. How many CMake contributors do you think actually run Utilities/SetupForDevelopment.sh? Probably the regular contributors and no one else.

CMake (and other Kitware projects) reject code that is not formatted properly. But, we also have tools that will fix the formatting for contributors and push the fixed branch back to their fork (upon request). If the robot allows it, it’s all good and one less thing I have to worry about as a reviewer. As a reviewer, I can focus on the content of the contribution rather than things like “is that a valid UTF-8 codepoint?”, “is this file too damn big?”, or “is the whitespace sane”.

@jwm

To me, formatting before build, like unit tests after build, should never be optional. It would be very nice if CMake supported these kinds of workflow actions.

CMake is a build system, not a workflow engine. And the problem with any of this is that since development is local, I can turn off any barriers you put up. make foo/fast skips any build steps you put up (assuming I even make a build tree), git commit -n ignores your git hooks, I have a different/wrong version of the formatter, the formatter mocked by a symlink to /usr/bin/true, etc. It just needs to be done server-side in some capacity because that’s the only place you can actually trust in your workflow.