Skip to content

Conversation

@firewave
Copy link
Collaborator

No description provided.

@firewave
Copy link
Collaborator Author

It is impressive how often things coincide...

@firewave firewave force-pushed the install-xxx branch 5 times, most recently from cf667ed to 5a35c65 Compare October 18, 2024 15:28
@firewave
Copy link
Collaborator Author

@danmar What is the point of compiling misra.py and not distributing it?

It is included in the portable package but I guess that was not intentional. But that might not even work - see below.

But including this causes the installer to blow up in side because the binary requires the Python libraries to be included as well. And I am not sure if that affects something regarding licensing (beside adding the Python license).

@firewave firewave force-pushed the install-xxx branch 2 times, most recently from 19b31ee to 3e5f7ca Compare December 11, 2024 21:45
@firewave firewave marked this pull request as ready for review December 11, 2024 21:45
@danmar
Copy link
Owner

danmar commented Dec 12, 2024

What is the point of compiling misra.py and not distributing it?

I made a "half hearted" attempt to distribute a misra addon that does not require python. We had a premium customer that didn't want to install python.

I feel uncomfortable about distributing the compiled misra addon I see it as untested. If a customer really wants it and understands that I didn't test it properly but still wants to use it well I can let the customer test this.

I think that for 99% of the customers it is OK to install python.

If we remove the extra code from release-windows.yml that is fine for me.. but it would be nice if I had some way to build the compiled misra addon in some github action somewhere.

@firewave
Copy link
Collaborator Author

If we remove the extra code from release-windows.yml that is fine for me.. but it would be nice if I had some way to build the compiled misra addon in some github action somewhere.

I still included it in the portable package so it can be tested.

@firewave firewave merged commit e195159 into danmar:main Dec 12, 2024
60 checks passed
@firewave firewave deleted the install-xxx branch December 12, 2024 12:46
@rouault
Copy link
Contributor

rouault commented Dec 12, 2024

The commit in cli/CMakeLists.txt breaks some builds. See for example https://github.com/OSGeo/gdal/actions/runs/12298084801/job/34320720920 :

Install the project...
-- Install configuration: "Release"
-- Installing: /usr/local/bin/cppcheck
-- Installing: /usr/local/bin/cppcheck-htmlreport
CMake Error at cli/cmake_install.cmake:78 (file):
  file INSTALL cannot find
  "/home/runner/work/gdal/gdal/cppcheck/cli/RUNTIME": No such file or
  directory.
Call Stack (most recent call first):
  cmake_install.cmake:50 (include)

with the cppcheck build being done at https://github.com/OSGeo/gdal/blob/master/.github/workflows/code_checks.yml#L68

@firewave
Copy link
Collaborator Author

I had a feeling I still wanted to do something in this PR - add tests for the install to the CI.

I can reproduce it locally and I am on it.

@firewave
Copy link
Collaborator Author

I have a fix - PR will go up in a few minutes.

@firewave
Copy link
Collaborator Author

See #7100.

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

Successfully merging this pull request may close these issues.

3 participants