CI: Update Ubuntu to 24.04

[ninsbl]

Main changes are rather trivial, just replacing version numbers.
Only change in dependencies is that PDAL was not available in 24.04 and I had to add UbuntuGIS repo to get it.

The build-scripts are copied to names with the new version numbers but did not have to be changed at all. So it would be probably more efficient to remove the version from the name instead?!

Also, I noticed there are several slightly different configurations and dependencies for Ubuntu to be maintained. Not sure how deliberate /necessary these differences are.

Finally, I had to add Ubuntugis to all places where dependencies are installed. With a helper script for installing dependencies that step could be simplified / unified (possibly with a script arg for the packages that are listed in the respective actions)...

For some reason, CodeQL for C-CPP fails in my fork because PDAL is not found during compilation. It is installed though av code seems the same as for other workflows. Here I am a bit confused. Is it due to some caching?

[ninsbl]

BTW, would it be of interest to test with a matrix of the two most recent Ubuntu versions?

[echoix]

BTW, would it be of interest to test with a matrix of the two most recent Ubuntu versions?

I don’t know if we would want to add even more CI time. At least not on all PRs. If we had a mechanism to add more, like if a certain label is added maybe, plus periodically.

[github-advanced-security]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[echoix]

What would be nice, is if the split wasn’t between with/without gui, but more like a full config and min config.

That way, the code paths where all fallbacks are used could be tested. It’s a bit where we were getting to near the end, but it makes more sense like that.

[echoix]

I’m curious to see how the numpy doctests will behave.

[echoix]

What is the impact of adding ubuntugis? Does it bring incompatible dependencies? At one point it wasn’t ready for 24.04 when I tried.

[echoix]

And in less than half a year, we’ll need to think about the next Ubuntu LTS

[ninsbl]

What is the impact of adding ubuntugis? Does it bring incompatible dependencies? At one point it wasn’t ready for 24.04 when I tried.

It does bring more recent packages for a bunch of dependencies (PROJ, GDAL, in addition to PDAL ...) and is more close to what people on Ubuntu would use as GIS packages (despite the name "unstable"). That said, it adds a layer of complexity / uncertainty... Since only PDAL is missing, the PDAL library could be compiled from source (e.g. in a dependency installation script)...

Any idea what makes the CodeQL / Analyze (c-cpp) check fail? I am not too familiar with the depth of GH action where this gets off tracks...

[wenzeslaus]

...the split wasn’t between with/without gui, but more like a full config and min config.

That's good to for testing and maybe also for deployment (small minimal image for some processing which happens to be in min config), but practically, Docker users will most often not use GUI I'm guessing, so if anything, it would be three: min config, full without GUI, and full with GUI.

[nilason]

It does bring more recent packages for a bunch of dependencies (PROJ, GDAL, in addition to PDAL ...) and is more close to what people on Ubuntu would use as GIS packages (despite the name "unstable"). That said, it adds a layer of complexity / uncertainty... Since only PDAL is missing, the PDAL library could be compiled from source (e.g. in a dependency installation script)...

The macOS runner is currently testing with GDAL 3.10.3, PDAL 2.8.4 and PROJ 9.6.2. Which are all fairly recent.

[nilason]

The newer version of GCC issues new warnings.

main.c: In function ‘fatalError’:
main.c:54:6: error: infinite recursion detected [-Werror=infinite-recursion]
   54 | void fatalError(char *errorMsg)
      |      ^~~~~~~~~~
main.c:59:13: note: recursive call
   59 |             fatalError(_("Unable to close 3D raster map"));
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

Here:

grass/raster3d/r3.out.ascii/main.c

Lines 54 to 63 in 34f31bd

	void fatalError(char *errorMsg)
	{
	if (map != NULL) {
	/* should unopen map here! */
	if (!Rast3d_close(map))
	fatalError(_("Unable to close 3D raster map"));
	}
	Rast3d_fatal_error("%s", errorMsg);
	}

and

grass/raster3d/r3.out.v5d/main.c

Lines 46 to 63 in 34f31bd

	void fatalError(char *errorMsg)
	{
	if (map != NULL) {
	/* should unopen map here! */
	if (!Rast3d_close(map))
	fatalError(_("Unable to close 3D raster map"));
	}
	Rast3d_fatal_error("%s", errorMsg);
	}
	/*---------------------------------------------------------------------------*/
	/* Convenient way to set up the arguments we are expecting
	*/
	void setParams(void)
	{
	param.input = G_define_option();
	param.input->key = "input";

[ninsbl]

The newer version of GCC issues new warnings.

Is this: ninsbl@da77836 an acceptable fix for the warnings (I am no C developer))?
It makes the test pass, but I dont know if it is a proper fix...

[nilason]

The newer version of GCC issues new warnings.

Is this: ninsbl@da77836 an acceptable fix for the warnings (I am no C developer))? It makes the test pass, but I dont know if it is a proper fix...

The original error message errorMsg should be added too. Otherwise it seems good to me.

[nilason]

The newer version of GCC issues new warnings.

Is this: ninsbl@da77836 an acceptable fix for the warnings (I am no C developer))? It makes the test pass, but I dont know if it is a proper fix...

The original error message errorMsg should be added too. Otherwise it seems good to me.

It should go to a separate PR.

[ninsbl]

@echoix Any idea why compilation in CodeQL does not find PDAL, it is installed as a dependency like in all other tests (https://github.com/OSGeo/grass/actions/runs/18097090686/job/51490527208?pr=6430#step:4:1154) and in the other tests compilation has no issues. Is it a caching thing?

[echoix]

@echoix Any idea why compilation in CodeQL does not find PDAL, it is installed as a dependency like in all other tests (https://github.com/OSGeo/grass/actions/runs/18097090686/job/51490527208?pr=6430#step:4:1154) and in the other tests compilation has no issues. Is it a caching thing?

There’s one way to know: trying it. So I cleared the caches of codeql trap, from main branch and all others I could find, and relaunched the job. If no other run of codeql in main runs before that, it should not have any cache.

I’m not convinced it is this, but knowing the answer by trying it is easier than to investigate if I’m right.

[echoix]

Doesn't seem like a caching issue

[ninsbl]

Doesn't seem like a caching issue

Any idea what else may cause this? I am a bit lost on this one...