Page MenuHome

Solve clang-format problems with different versions
Closed, ResolvedPublicTO DO

Description

clang-format 6 and 7 are shown by make format as valid version, but give different results when run on code in master. Meanwhile clang-format in precompiled libraries is being upgraded to 9.

  • Decide and document supported versions.
  • Add /* clang-format off */ or other workarounds to make result same on all supported versions.
  • make format does not use clang-format from precompiled libraries on Linux, path is wrong.

clang-format version 6 to 9 should now give the same result, and that range has been marked as supported in make format. We can decide to narrow it later if needed.

Event Timeline

Dalai Felinto (dfelinto) changed the task status from Needs Triage to Confirmed.Wed, Feb 12, 11:23 AM
Dalai Felinto (dfelinto) created this task.

Not sure this is really needed to force that, make format already uses clang-format from make deps area if it exists...

Will tweak py script behind make format to try and use system clang-format in recommended version if possible too.

If this is already reinforced, how is @Brecht Van Lommel (brecht) still running into conflicting clang-format versions at his work stations?

rBDT0254e2baa88a57 checks for versions between 6 and 7 and warns in case of other version.

Clang-format in the precompiled libs on Windows is version 9 already, and other platforms will be soon as well. So, we need to decide to either upgade to 9 or revert it to 6 on Windows. Or support 6 through 9 and add more /* clang-format off */ to make it work.

make format does not use the clang-format from the precompiled libraries on Linux. The path needs to be fixed here:

PATH="../lib/${OS_NCASE}/llvm/bin/:$(PATH)" \
    $(PYTHON) source/tools/utils_maintenance/clang_format_paths.py $(PATHS)

For me on Ubuntu Linux, I have version 6.0.0-1ubuntu2 from the distribution and 6.0.1 in the precompiled libraries. Both give the same result, which has some differences compared to master.

intern/cycles/util/util_defines.h        | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------
intern/cycles/util/util_static_assert.h  |  16 ++++++++--------
intern/ghost/intern/GHOST_ContextCGL.mm  |   9 ++++++---
intern/ghost/intern/GHOST_WindowCocoa.mm |   3 ++-
source/blender/blenlib/BLI_map.h         |  19 +++++++++----------
5 files changed, 121 insertions(+), 118 deletions(-)

for me on debian testing, both 8.0 and 6.0 versions change current master.

  • 6.0.1:
intern/cycles/util/util_defines.h        | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------
intern/cycles/util/util_static_assert.h  |  16 ++++++++--------
intern/ghost/intern/GHOST_ContextCGL.mm  |   9 ++++++---
intern/ghost/intern/GHOST_WindowCocoa.mm |   3 ++-
source/blender/blenlib/BLI_map.h         |  19 +++++++++----------
5 files changed, 121 insertions(+), 118 deletions(-)
  • 8.0.1:
intern/cycles/util/util_defines.h       | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------------------------------------------------------
intern/cycles/util/util_static_assert.h |  16 ++++++++--------
2 files changed, 104 insertions(+), 104 deletions(-)

So we clearly do not have a clang-format-6 formatted master currently...

Note that I don't mind switching to 9.0, but we really, really have to stick to one version, otherwise working with the tools becomes somewhat painful and/or noisy.

Brecht Van Lommel (brecht) renamed this task from `make format` to use `make deps` installed clang-format to Solve clang-format problems with different versions.Sat, Feb 15, 9:02 AM
Brecht Van Lommel (brecht) updated the task description. (Show Details)
Brecht Van Lommel (brecht) updated the task description. (Show Details)