Details

Reviewers

Sybren A. Stüvel (sybren)
Brecht Van Lommel (brecht)
Bastien Montagne (mont29)

Summary

Run autopep8 as well as clang-format when calling "make format",
the PATHS argument is passed to both utilities which will only operate on files they support.

For example: make format PATHS=release/scripts formats Python scripts, make format PATHS=source/blender/blenlib would format C/C++.
If users really want they can format C/C++ & Python files at the same time since both formatting utilities filter on file extension.

make format PATHS="release/scripts/startup/nodeitems_builtins.py source/creator/creator.c"

A LIBDIR variable has been added to the GNUmakefile to simplify references to this directory which can be one of 3 possible values.

Notes:

This is a follow up to D14686 which added defaults so autopep8 could run on all of the Python code in Blender's main repository.

I wasn't able to use the Python binary in place (it gives an error that libcrypt.so.1 isn't found), and didn't think it necessary to use the pre-compiled Python. So PYTHON will always be used to execute autopep8.py found in our ../lib/ directory. Otherwise the autopep8 command will be used if none of the paths under ../lib/ are found.

The checks for ../lib/* are awkward, we could consider setting a single lib path and reusing that.

To check formatting does something, temporarily delete ./pyproject.toml and run make format.

Diff Detail

Repository

rB Blender

Branch

TEMP-AUTOPEP8-FMT (branched from master)

Build Status

Buildable 21860
Build 21860: arc lint + arc unit

Event Timeline

Campbell Barton (campbellbarton) requested review of this revision.Apr 28 2022, 7:29 AM

Campbell Barton (campbellbarton) created this revision.

Harbormaster completed remote builds in B21860: Diff 50931.Apr 28 2022, 7:29 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Apr 28 2022, 7:49 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Use the autopep8 in ../lib where possible

Harbormaster completed remote builds in B21861: Diff 50932.Apr 28 2022, 9:12 AM

Add AUTOPEP8 Environment variable to the help target.
Silance the command output.

Harbormaster completed remote builds in B21862: Diff 50933.Apr 28 2022, 9:18 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Apr 28 2022, 9:20 AM

Campbell Barton (campbellbarton) added reviewers: Bastien Montagne (mont29), Sybren A. Stüvel (sybren), Brecht Van Lommel (brecht).

Campbell Barton (campbellbarton) retitled this revision from Include autopep8 in "make format" convenience target to GNUmakefile: include autopep8 in the "make format" target.

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Apr 28 2022, 9:30 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Apr 28 2022, 9:35 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Apr 28 2022, 9:38 AM

Replace environment variable with command line argument

Harbormaster completed remote builds in B21865: Diff 50938.Apr 28 2022, 12:30 PM

We'll also need this in build_files/windows/format.cmd for Windows.

The checks for ../lib/* are awkward, we could consider setting a single lib path and reusing that.

It would be good to to deduplicate logic with @PATH for clang_format_paths.py and possibly DEPS_INSTALL_DIR, to just find the appropriate libdir once.

I wasn't able to use the Python binary in place (it gives an error that libcrypt.so.1 isn't found), and didn't think it necessary to use the pre-compiled Python. So PYTHON will always be used to execute autopep8.py found in our ../lib/ directory. Otherwise the autopep8 command will be used if none of the paths under ../lib/ are found.

I don't think we have to run this script with the bundled Python binary, but that it fails to run on your system is a problem, since the same would happen for users downloading Blender.

However I see the same dependency in the python3.9 executable so it would not be a new problem.

This revision now requires changes to proceed.Apr 28 2022, 3:23 PM

Define a LIBDIR and use this the clang-format & autopep8 path

Harbormaster completed remote builds in B21889: Diff 50986.Apr 29 2022, 4:23 AM

Included reformatting by accident.

Harbormaster completed remote builds in B21890: Diff 50987.Apr 29 2022, 4:24 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)Apr 29 2022, 4:26 AM

Campbell Barton (campbellbarton) edited the summary of this revision. (Show Details)