Modernize python build setup, protoc dependency #783
Conversation
pmai
force-pushed
the
build/modernize-python
branch
7 times, most recently
from
2923784
to
6968c07
Compare
| {name = "ASAM Open Simulation Interface Project", email = "[email protected]"}, | ||
| ] | ||
| dependencies = [ | ||
| "protobuf>=4.24.4", |
There was a problem hiding this comment.
Conservatively == would likely be better, however there might be some leeway here, which is why I have for now left it at >=, but might change to == prior to finalizing the PR.
| requires = [ | ||
| "setuptools", | ||
| "wheel", | ||
| "protoc-wheel-0==24.4", |
There was a problem hiding this comment.
In the protobuf.yml, the protobuf version in configurable as an environment variable. Currently it is set to 3.20.1:
PROTOBUF_VERSION: 3.20.1
How does this fit to the hardcoded protobuf version here, and also in the dependencies below?
In my PR regarding this topic I tried to make the python protobuf version coherent with the installed protoc version on the system.
There was a problem hiding this comment.
The protobuf.yml is just our test build pipeline, i.e. how we test any changes, especially also the C++ side.
This does not hard-code any actual requirements, it just controls our build environment so that our build environment stays half-way stable.
The pyproject.toml will define how others build the python packages. Since this requires easy access to a suitable protoc (and a matching runtime), we build-depend the python package on a fixed version of protoc-wheel-0, which we then also have to use as a dependency for the protobuf runtime. Since none of this works very well with dynamic dependencies, this more or less has to be hard-coded. In the end we might converge on one version between CI and python package, but that is really independent.
This PR prepares the complete separation of the C++ and python build processes, which should have happened in the first place, and also - in a later PR - separates our CI from the package requirements.
We really, really do not want to have hard version requirements across projects, so osi-validator can just depend on the osi3 python package with a >= requirement, and not have any dependencies on protobuf at all.
There was a problem hiding this comment.
The protobuf.yml is just our test build pipeline.
Yes, but we now test the C++ build with 3.20.1 and the Python build with 24.4.
Just for my understanding: To build OSI for C++, I need to have the protoc compiler installed on my system, with any protobuf version. The python build is independent of that protoc compiler and it always uses version 24.4 (unless of course I change the toml file). Is that correct?
There was a problem hiding this comment.
The protobuf.yml is just our test build pipeline.
Yes, but we now test the C++ build with 3.20.1 and the Python build with 24.4.
Yes, and we might or might not switch to something newer on the C++ side, however that is now independent of the Python side (since for Python it means what we ship, not what we test, whereas for C++ it only means what we test). In any case the C++ changes would be handled in a separate PR.
Just for my understanding: To build OSI for C++, I need to have the protoc compiler installed on my system, with any protobuf version. The python build is independent of that protoc compiler and it always uses version 24.4 (unless of course I change the toml file). Is that correct?
Yes, though the python build scripts have a way of falling back to system- or user-supplied protoc, if you want that. Just by default they will build-depend on the protoc-wheel-0 package and use that for building.
|
Unit tests need to be fixed. There seem to be some issues due to the renaming of the format folder, see pipeline run. |
|
Proto 3.6.1 in use for OSI-Validation at BMW |
pmai
force-pushed
the
build/modernize-python
branch
from
6968c07
to
5efd7e7
Compare
|
@pmai In |
|
@pmai will this be ready to merge next CCB? |
pmai
added
the
ReadyForCCBReview
Yes, that was however already wrong prior to this PR, so I would suggest fixing in separate PR. |
|
CCB 2025-03-25: Merge as-is. |
pmai
added
ReadyToMerge
This moves most meta-data to pyproject.toml, automates the version handling and makes it available at runtime, depends on protoc via the protoc-wheel-0 package, makes sdist and bdist distributions work correctly, and removes dependencies that are not build dependencies, but rather used only in CI. Signed-off-by: Pierre R. Mai <[email protected]>
Signed-off-by: Pierre R. Mai <[email protected]>
Signed-off-by: Pierre R. Mai <[email protected]>
Also renames the module file to fit PEP-8 conventions. Signed-off-by: Pierre R. Mai <[email protected]>
pmai
force-pushed
the
build/modernize-python
branch
from
211ba12
to
2a89e61
Compare
|
I was a bit confused about the diff at first but this was a mix of moving files and linter. so OK Reviewed for v3.7.0 |
This moves most meta-data to pyproject.toml, automates the version handling and makes it available at runtime, depends on protoc via the protoc-wheel-0 package, makes sdist and bdist distributions work correctly, and removes dependencies that are not build dependencies, but rather used only in CI.