Fails to build with GDAL 3.8.0 about geo-gdal-ffi HOT 13 CLOSED

Fixed in 8eee520. The write method must return 1 if the write was succesful. The closure in FFI.pm now returns 1 also in the case the user side class does not return anything (as in the test).

from geo-gdal-ffi.

I can reproduce locally using GDAL 3.8.0RC1.

The error message is only associated with the geojson OGR driver in the GDAL code base.
https://github.com/search?q=repo%3AOSGeo%2Fgdal+%22Cannot+write+feature%22&type=code

Disabling the preceding test block gets the test to pass.

Moving the failing test block so it is run first also gets the tests to pass.

The above suggests that something is not being cleaned up in the first test block starting at L34. I'm not sure what.

@ajolma - any ideas?

from geo-gdal-ffi.

Possibly something is awry with the default callback after an override has been cleared.

from geo-gdal-ffi.

Possibly something is awry with the default callback after an override has been cleared.

That looks like a red herring as the callback is not called in the second block with the geotiff creation.

from geo-gdal-ffi.

I could reproduce it in a docker with clone of gdal 3.8.0. However, I don't understand why it fails. I asked Even - apparently the mail did not go to gdal-dev but to him only - and he has no recollection of anything done in that area. I'm wondering why there is not VSIStdoutUnsetRedirection to undo the effect of the previous test but the undo seems to be a bit strange.

from geo-gdal-ffi.

The error is somehow caused by the first test, which copies a memory layer to geojson stdout redirected to a variable. The test is ok as the geojson is ok in the variable but for some reason there's an error generated and left in the error stack. That error then is mistaken as an error in the geotiff creation. I can't yet even find where the error is generated in gdal and why.

from geo-gdal-ffi.

The reason for why I did not find where the error came from in GDAL was that I was looking at wrong branch in GDAL tree, not the master.

from geo-gdal-ffi.

Confirmed fixed with the changes from 8eee520.

from geo-gdal-ffi.

Confirmed fixed with the changes from 8eee520.

Unfortunately not. libgeo-gdal-ffi-perl (0.1-3) contains a patch with those changes, but the rebuild with GDAL 3.8.0 on the Debian buildds still fails (which it did not do when I tested it on my system):

Cannot write feature at /<<PKGBUILDDIR>>/blib/lib/Geo/GDAL/FFI/Driver.pm line 49.
	Geo::GDAL::FFI::Driver::Create(Geo::GDAL::FFI::Driver=SCALAR(0x562aed4103d8), "/vsimem/test.tiff", 10) called at t/vsistdout.t line 63
# Tests were run but no plan was declared and done_testing() was not seen.
# Looks like your test exited with 2 just after 1.
t/vsistdout.t .. 

https://buildd.debian.org/status/package.php?p=libgeo-gdal-ffi-perl

from geo-gdal-ffi.

This is a leftover error from the JSON write.

Ideally the source of the error would be fixed, but clearing the @Geo::GDAL::FFI::errors array at the end of the test block gets the test to pass.

The question is whether any errors should be cleared automatically when the VSI writer is closed. Perhaps the contents should be written to STDOUT while clearing.

from geo-gdal-ffi.

This looks like a broader problem with CopyLayer or maybe the GDAL JSON conversion process.

I updated the test to create three features in the source layer. However, the resulting JSON string only contains the first one.

Calling FlushCache on the data set has no effect.

diff --git a/t/vsistdout.t b/t/vsistdout.t
index 02be848..038ecbc 100644
--- a/t/vsistdout.t
+++ b/t/vsistdout.t
@@ -33,21 +33,32 @@ use JSON;
 # test vsistdout redirection
 if(1){
     # create a small layer and copy it to vsistdout with redirection
-    my $layer = GetDriver('Memory')->Create->CreateLayer({GeometryType => 'None'});
+    my $ds = GetDriver('Memory')->Create;
+    my $layer = $ds->CreateLayer({GeometryType => 'None'});
     $layer->CreateField(value => 'Integer');
     $layer->CreateGeomField(geom => 'Point');
+for my $i (5..7) {
     my $feature = Geo::GDAL::FFI::Feature->new($layer->GetDefn);
     $feature->SetField(value => 12);
-    $feature->SetGeomField(geom => [WKT => 'POINT(1 1)']);
+    $feature->SetGeomField(geom => [WKT => "POINT(1 $i)"]);
     $layer->CreateFeature($feature);
-
+}
+    $ds->FlushCache;
+diag 1;
+diag @Geo::GDAL::FFI::errors;
+diag "F count: " . $layer->GetFeatureCount;
     my $output = Output->new;
     my $gdal = Geo::GDAL::FFI->get_instance;
     $gdal->SetWriter($output);
     GetDriver('GeoJSON')->Create('/vsistdout')->CopyLayer($layer);
     $gdal->CloseWriter;

+diag 2;
+diag @Geo::GDAL::FFI::errors;
     my $ret = $output->output;
+diag 3;
+diag $ret;
+diag @Geo::GDAL::FFI::errors;
     ok($ret eq
        '{"type": "FeatureCollection",'.
        '"features": '.
@@ -56,8 +67,10 @@ if(1){
        '"coordinates": [ 1.0, 1.0 ] } }]}end',
     "Redirect vsistdout to write/close methods of a class.");

+@Geo::GDAL::FFI::errors = ();
 }

+
 # test Translate
 if(1){
     my $ds = GetDriver('GTiff')->Create('/vsimem/test.tiff', 10);

from geo-gdal-ffi.

Found it.

The write method of the override class needs to return 1 on success, not any true value.

This is checked for by the GDAL JSON driver:
https://github.com/OSGeo/gdal/blob/edf1ee72fccc8cb91519218a2c4e021ecd31d0f4/ogr/ogrsf_frmts/geojson/ogrgeojsonwritelayer.cpp#L338-L342

I'll work up a PR.

from geo-gdal-ffi.

For reference, the GDAL commit that changed the return behaviour is OSGeo/gdal@b25706e

from geo-gdal-ffi.