CI: upgrade to ubuntu 24.04

[t0b3]

upgrade CI runner for qt5 build and tests to ubuntu-24.04 (latest LTS)

[github-actions]

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit 726213f)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit 726213f)

[github-actions]

Tests failed for Qt 5

One or more tests failed using the build from commit 726213f

symbol_layer

symbol_layer

Test failed at testSimpleLineWithOffset at tests/src/python/test_qgslinesymbollayers.py:72

Rendered image did not match tests/testdata/control_images/symbol_layer/expected_simpleline_offset/expected_simpleline_offset.png (found 342 pixels different)

The full test report (included comparison of rendered vs expected images) can be found here.

Further documentation on the QGIS test infrastructure can be found in the Developer's Guide.

[nyalldawson]

@t0b3 render test fixes are in #59814

The offset line test is failing because of a bug in recent GEOS versions -- libgeos/geos#1037

I have no idea what is happening with the Oracle test 😆

[nyalldawson]

@t0b3 another one fixed here: #59844

[t0b3]

@nyalldawson your results of the just triggered rerun https://cdash.orfeo-toolbox.org/viewTest.php?onlyfailed&buildid=27614

[t0b3]

We are getting really close now 😃 only a few test fail now

• 22 - ProcessingGrassAlgorithmsImageryTest (Failed)
• 571 - PyQgsLineSymbolLayers (Failed)
• 774 - PyQgsVectorLayerEditBuffer (Failed)
• 778 - PyQgsVectorLayerProfileGenerator (Failed)

Test results submitted to: https://tinyurl.com/28llwwqx

[nyalldawson]

@rouault

774 - PyQgsVectorLayerEditBuffer (Failed)

This is an odd one, which I'm struggling to fix. I can't work out if it's a regression in newer GDAL or QGIS is misusing GDAL API (which used to work, but now doesn't).

What happens is :

1. A call to QgsVectorLayer::addAttribute triggers a call to QgsVectorLayerEditPassthrough::addAttribute, QgsVectorLayerUndoPassthroughCommand, in turn resulting in a QgsVectorLayerUndoPassthroughCommandAddAttribute being constructed. All good so far.
2. As is normal for QUndoCommands, QgsVectorLayerUndoPassthroughCommandAddAttribute::redo is immediately called. This sets the transaction save point (via QgsTransaction::createSavepoint, which executes "SAVEPOINT XXX" on the Geopackage via QgsOgrTransaction::executeSql -> QgsOgrDataset::executeSQLNoReturn . Then it creates the new attribute through QgsOgrProvider::addAttributes, just like normal (in turn calling QgsOgrLayer::CreateField) At this stage the data source in QgsOgrLayer and now has the new attribute
3. Later, the add attribute command is undone. This triggers QgsVectorLayerUndoPassthroughCommandAddAttribute::undo. Which calls first rollBackToSavePoint successfully (via executing "ROLLBACK TO SAVEPOINT XXX" on the Geopackage). The field is removed from the Geopackage, but NOT YET FROM THE QGIS data provider. So then, as the comment in ::undo notes, a call is made to mBuffer->L->dataProvider()->deleteAttributes( QgsAttributeIds() << attr ); to force the QGIS data provider to recognise the field removal.
4. This is where things get broken -- in QgsOgrProvider::deleteAttributes we attempt to remove a no longer existing field from the OGR layer, and that correctly errors out. At the end of this function loadFields() is called, in an attempt to re-read the actual fields which still exist in the datasource. This gets the feature definition using QgsOgrFeatureDefn &fdef = mOgrLayer->GetLayerDefn(); -- but this is the ALREADY EXISTING oFDefn member of QgsOgrLayer -- which has the out of date fields from before the "ROLLBACK TO SAVEPOINT" command was executed.

In older GDAL versions it seems that the oFDefn member of QgsOgrLayer was updated to reflect the fields after the ROLLBACK command, but that's no longer the case.

So my question is -- is this all abuse of GDAL API? Should we NOT be using the existing feature definition from before the transaction commands and should we re-fetch it anew? Or is this a GDAL regression and the existing feature definition should always reflect the actual state after the ROLLBACK command?

[rouault]

In older GDAL versions it seems that the oFDefn member of QgsOgrLayer was updated to reflect the fields after the ROLLBACK command

That would be surprising. I'd say that no OGR driver works properly regarding rolling back changes of the layer definition. The underlying database will obviously rollback a ALTER TABLE ... ADD COLUMN, but the OGR driver itself doesn't pay attention to this. One could argue this is a bug of GDAL, but I'd be surprised this has worked in the past and no longer works. The BeginTransaction() / CommitTransaction() / RollbackTransaction() in OGR has been thought mostly about the INSERT / UPDATE / DELETE and not structural changes to the layer

[nyalldawson]

@rouault hmm... Could it be that the gdal delete attribute API used to update the feature definition even when the field didn't exist, and now it doesn't and aborts earlier?

[rouault]

Could it be that the gdal delete attribute API used to update the feature definition even when the field didn't exist, and now it doesn't and aborts earlier?

the implementation of OGRGeoPackageTableLayer::DeleteField() hasn't significantly changed since at least 3.4, but my assumption is that with the update to 24.04, we now take the code path sqlite >= 3.35.5 at https://github.com/OSGeo/gdal/blob/03bd0363fe1702c18051309af1c534e96b131236/ogr/ogrsf_frmts/gpkg/ogrgeopackagetablelayer.cpp#L6448 , and that code path will fail if the column doesn't exist at the SQLite code path, whereas the code path sqlite < 3.35.5 will be tolerant to the column not existing

[rouault]

I'm not sure how practical this is on QGIS side, but I'd suggest that the QGIS OGR provider sets a boolean if CreateField / AlterFieldDefn / DeleteFieldDefn have been called within a transaction, and at rollback time, it closes and reopen the OGR dataset

[nyalldawson]

@rouault ok, I tried that and failed. I'm just proposing we disable the test in #59910 , until someone wants to do the extensive refactoring required for a proper fix for this functionality.

[t0b3]

Is there any impact of #59901 ...?
I rebase on master to have all recent commits included...

EDIT: ok, obviously nothing changed 👍