Solve clang-format problems with different versions #73747

New Issue

Dalai Felinto · 2020-02-12T11:23:30+01:00

Dalai Felinto commented

2020-02-12 11:23:30 +01:00

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.

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. - [x] Decide and document supported versions. - [x] Add `/* clang-format off */` or other workarounds to make result same on all supported versions. - [x] `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.

Dalai Felinto commented

2020-02-12 11:23:30 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Dalai Felinto commented

2020-02-12 11:23:30 +01:00

Added subscriber: @dfelinto

Bastien Montagne commented

2020-02-12 12:17:49 +01:00

Added subscriber: @mont29

Bastien Montagne commented

2020-02-12 12:17:49 +01:00

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.

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.

Bastien Montagne commented

2020-02-12 12:35:50 +01:00

See blender/blender-dev-tools@0254e2baa8

Dalai Felinto commented

2020-02-12 13:03:05 +01:00

Added subscriber: @brecht

Dalai Felinto commented

2020-02-12 13:03:05 +01:00

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

Brecht Van Lommel commented

2020-02-12 13:46:24 +01:00

blender/blender-dev-tools@0254e2baa8 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(-)

blender/blender-dev-tools@0254e2baa8 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(-) ```

Bastien Montagne commented

2020-02-12 14:40:51 +01:00

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.

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 changed title from ~~`make format` to use `make deps` installed clang-format~~ to Solve clang-format problems with different versions

2020-02-15 09:02:39 +01:00

blender-admin commented

2020-02-15 09:22:26 +01:00

This issue was referenced by 6669eca820

This issue was referenced by 6669eca820f5295204a9b0fe5cf19e1ce15ac792

blender-admin commented

2020-02-15 13:51:46 +01:00

This issue was referenced by blender/cycles@99a7da00e0

This issue was referenced by blender/cycles@99a7da00e0d37ae9eaeea5e8504be1425ac1d5e8

blender-admin commented

2020-02-15 13:51:46 +01:00

This issue was referenced by f0c991a380

This issue was referenced by f0c991a380e9d449ebc02cc91649ef3728848070

Brecht Van Lommel commented

2020-02-15 14:09:20 +01:00

Changed status from 'Confirmed' to: 'Resolved'

Brecht Van Lommel closed this issue

2020-02-15 14:09:20 +01:00

Sign in to join this conversation.

4 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#73747

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Solve clang-format problems with different versions #73747