-
Notifications
You must be signed in to change notification settings - Fork 174
CMakeLists.txt: allow disabling Python support #2051
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
base: main
Are you sure you want to change the base?
Conversation
In some cases, even if Swig is found and Python3 is found, it may not be desirable to build Python support in avrdude, so this commit adds an ENABLE_PYTHON_SUPPORT option to be able to explicitly disable using Python support (unfortunately CMake doesn't allow passing arguments that would prevent it from finding Swig/Python 3 if available). To preserve existing behavior, this option defaults to enabled (ON). Signed-off-by: Thomas Petazzoni <[email protected]>
|
Looks like a good improvement to me. |
|
White technically the implementation is correct, and preserves backward compatibility, it actually introduces a new confusing workflow which I've seen in similar context but in other project: cmake -B avrdude -DENABLE_PYTHON_SUPPORT=True ...And the CMake configuration will finish w/o errors, but in case if python/swig is missing, the python support will be disabled, and that part is a bit confusing considering the variable name. Alternatives:
|
|
@Youw Thanks for your considered review! @tpetazzoni I personally would prefer either one of the two
Also, the is an unsuccessful check, which if caused by this PR would need addressing before merging. |
|
Hmm, the macOS build failure does not seem to be related to this PR, but due to Homebrew cmake formula conflict. |
Looks like a transient issue, I just trigger the rebuild and everything is okay now. The warnings are okay, as we do not know if the runner will change in the future. |
Would you please address the comment from Stefan? Thanks. |
In some cases, even if Swig is found and Python3 is found, it may not be desirable to build Python support in avrdude, so this commit adds an ENABLE_PYTHON_SUPPORT option to be able to explicitly disable using Python support (unfortunately CMake doesn't allow passing arguments that would prevent it from finding Swig/Python 3 if available).
To preserve existing behavior, this option defaults to enabled (ON).