Skip to content
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

Buildfile for vspec2binary lives in vss #121

Closed
SebastianSchildt opened this issue Nov 19, 2021 · 4 comments
Closed

Buildfile for vspec2binary lives in vss #121

SebastianSchildt opened this issue Nov 19, 2021 · 4 comments

Comments

@SebastianSchildt
Copy link
Collaborator

To use the vspec2binary converter some C(pp) code needs to be compiled. According to the documentation that is done in the VSS (the spec) Makefile here

https://github.com/COVESA/vehicle_signal_specification/blob/c5ae76070fbef8e6f599b89a7e2de80d9da80fef/Makefile#L42

However, BUILD instructions for tools should not live in the spec repo.

(for vss CI it is still cleaner if a build script inside vss-tools is executed)

@gunnarx
Copy link

gunnarx commented Nov 30, 2021

Isn't this true for a lot of things... traditionally the Makefile has been kept in vehicle_signal_specification repo. And it refers to tools in vss-tools, so why can it not refer to binarytool.c ?

Or am I missing your exact point?

Solution? Do you want to change the Makefile, move the makefile, or change the documentation?

@SebastianSchildt
Copy link
Collaborator Author

I see the makefile in vehicle_signal_specification as a way to test vss contributions ("is VSS ok in this branch/PR valid?"), so it needs to execute the tools. While maybe it would be "cleaner" in a gh action file, or just running a container from somehwere, or real "installable" packages (pypi), for "historic reasons" it works fine with the makefile. But whatever mechanism is used to test vehicle_signal_specification, I think it should boil down to

  • install tool
  • run tool

Building tools, which might change faster/on a different level, and would possibly use even different build systems, I would rather see only in the tools repo (where we need them anyway).

So I would see it as more clean if the vss would just execute a makefile from vss-tools where necessary instead of containing a like this
https://github.com/COVESA/vehicle_signal_specification/blob/e4adabe9b66258b5632e85cc03e89cc4251fa2cb/Makefile#L43

I think it is not a pressing "fix it or VSS will break" kind of thing, but as we are doing it now is neither a good nor elegant solution.

@erikbosch
Copy link
Collaborator

The current setup at least makes it a bit cumbersome if you e.g. refactor a tool, as you then to get builds to pass needs to change both the tool itself and the Makefile in VSS. As of today VSS Makefile executes e.g. vspec2csv.py, as I see see it would be cleaner is VSS (from Makefile or buildcheck.yml) just triggered "make csv" in the vss-tools Makefile with vspecfile as parameter/variable.

By the way, is anyone using the install command in the vss Makefile? I assume it does not work since contrib was not added to ${TOOLSDIR}/vspec2c when we moved vspec2c.

install:
	git submodule init
	git submodule update
	(cd ${TOOLSDIR}/; python3 setup.py install --install-scripts=${DESTDIR}/bin)
	$(MAKE) DESTDIR=${DESTDIR} -C ${TOOLSDIR}/vspec2c install
	install -d ${DESTDIR}/share/vss
	(cd spec; cp -r * ${DESTDIR}/share/vss

erikbosch added a commit to boschglobal/vehicle_signal_specification that referenced this issue Feb 12, 2024
This is the last part of
COVESA/vss-tools#121

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
erikbosch added a commit to COVESA/vehicle_signal_specification that referenced this issue Feb 20, 2024
This is the last part of
COVESA/vss-tools#121

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
@erikbosch
Copy link
Collaborator

This is fixed now

erikbosch added a commit to boschglobal/vehicle_signal_specification that referenced this issue May 6, 2024
This is the last part of
COVESA/vss-tools#121

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants