Skip to content

Conversation

wenzeslaus
Copy link
Member

Remove build vars from setup and use the existing dependency on grass.app.runtime to get the information. Additionally, move some of the parametrization to grass.app.runtime because now, both the functions executing the setup steps and the variable are there, so there is no reason to supply these from the outside.

Config dir does not require version, but I kept a general function there which is used in g.extension and in GUI because these use the runtime-obtained version as opposed to the one from grass.app.runtime (difference between running system and setup of the env).

I removed the build variables from the documentation which makes it a little simpler to edit, although less explicit assuming that the version usage there was actually correct. The example needs more unrelated edits, so I'm keeping it as is.

Remove build vars from setup and use the existing dependency on grass.app.runtime to get the information. Additionally, move some of the parametrization to grass.app.runtime because now, both the functions executing the setup steps and the variable are there, so there is no reason to supply these from the outside.

Config dir does not require version, but I kept a general function there which is used in g.extension and in GUI because these use the runtime-obtained version as opposed to the one from grass.app.runtime (difference between running system and setup of the env).

I removed the build variables from the documentation which makes it a little simpler to edit, although less explicit assuming that the version usage there was actually correct. The example needs more unrelated edits, so I'm keeping it as is.
@github-actions github-actions bot added GUI wxGUI related Python Related code is in Python libraries module general labels Sep 22, 2025
@wenzeslaus
Copy link
Member Author

Unrelated but cool: Pylint in CI uniquely identified a parameter problems other code checks did not, at least not the local ones (I didn't let the actual tests to run in CI).

@wenzeslaus
Copy link
Member Author

I did not touch CMake build here, but it seems that the corresponding CMakeList.txt is trying to do something special with setup.py, but I don't get what it is. (Configure but do not replace? ...which means to do what?)

configure_file(script/setup.py ${OUTDIR}/${PYDIR_GRASS}/script/setup.py
               COPYONLY)

@wenzeslaus
Copy link
Member Author

I removed the special line for setup.py from CMakeList.txt. Unrelated, but i.smap test failed again here.

@nilason
Copy link
Contributor

nilason commented Sep 23, 2025

I did not touch CMake build here, but it seems that the corresponding CMakeList.txt is trying to do something special with setup.py, but I don't get what it is. (Configure but do not replace? ...which means to do what?)

It should have been something similar to resource_paths.py, but this has obviously not been the case.

I removed the special line for setup.py from CMakeList.txt.

If you removed string replace tags in that file, that is fine.

@github-actions github-actions bot added the CMake label Sep 23, 2025
Copy link
Contributor

@petrasovaa petrasovaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I would be able to spot a problem here, but it looks ok to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CMake general GUI wxGUI related libraries module Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants