Opened 8 years ago
Closed 8 years ago
#6431 closed defect (fixed)
mkgdaldist.sh fails with Perl bindings
Reported by: | Even Rouault | Owned by: | Ari Jolma |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | PerlBindings | Version: | unspecified |
Severity: | normal | Keywords: | |
Cc: |
Change History (13)
comment:1 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
comment:2 by , 8 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
r33855 breaks the build of the various targets. For example https://s3.amazonaws.com/archive.travis-ci.org/jobs/120240267/log.txt :
t/00.t .............. 1/? TypeError in method 'SetCacheMax', argument 1 of type 'GIntBig' # Looks like your test exited with 255 just after 33. t/00.t .............. Dubious, test returned 255 (wstat 65280, 0xff00) All 33 subtests passed t/alg.t ............. 1/7 # Failed test 'polygonize made 2 features' # at t/alg.t line 19. # Looks like you failed 1 test of 7. t/alg.t ............. Dubious, test returned 1 (wstat 256, 0x100) t/feature.t ......... 1/? # Failed test 'Set and get integer64 field.' # at t/feature.t line 98. # Looks like you failed 1 test of 25. t/feature.t ......... Dubious, test returned 1 (wstat 256, 0x100) Failed 1/25 subtests
Do I understand well that the intent of r33855 is not to require cpl_config.h at bindings .cpp generation time (so mkgdaldist.sh will not need to run ./configure), but only at compilation time ?
follow-up: 4 comment:3 by , 8 years ago
auts
Nevermind the r33855, which is obviously a wrong solution to the problem, which is even more subtle than I (too quickly) assumed.
The issue is the inclusion of cpl_config.h, which gives the correct defines, so that swig(?) knows what GIntBig and GUIntBig really are (in the current compiler).
comment:4 by , 8 years ago
The issue is the inclusion of cpl_config.h, which gives the correct defines, so that swig(?) knows what GIntBig and GUIntBig really are (in the current compiler).
If the Perl bindings need cpl_config.h to generate the .cpp files, then it means they are dependant of the host target and that we can't ship them in the source archive. Which is not necessarily a huge problem ? Where I'm a bit surprised is that the other bindings don't need SWIG to include cpl_config.h to be able to handle GIntBig. For example in swig/include/python/typemaps_python.i there's a "%typemap(in) GIntBig" that maps to Python C type PY_LONG_LONG
comment:5 by , 8 years ago
Yes. I see that too. But GIntBig itself depends on the host system. It can be long or long long for example. I've built that logic into the typemaps. Python seems to assume it is always long long?
comment:6 by , 8 years ago
Maybe it is not needed, but if you have a pointer to long long it can't be used as a pointer to long?
comment:7 by , 8 years ago
I think Python PY_LONG_LONG might expend to "long long" or "int64", but that's Python headers business (and done in C code). So basically ideally, if doable, for the Perl bindings GIntBig should be handled as an opaque/non-standard type. Or perhaps there's a standard way of dealing with 64bit integers in SWIG / Perl SWIG ?
comment:8 by , 8 years ago
Yes. I think that is correct. Swig has built-in typemaps for 64bit integers for Perl and I'm just letting it use them.
follow-up: 11 comment:9 by , 8 years ago
follow-up: 12 comment:10 by , 8 years ago
I have now replicated the bigint typemaps in the Perl bindings. The tests are ok and the config.h is not needed any more. Let's try this one.
comment:11 by , 8 years ago
I would be inclided to leave the wrappers out of the tarball.
I'm fine with that. Would you mind modifying mkgdaldist.sh for that then ? And make sure ./configure --with-perl; make actually works from the generated tarball (not sure why I got an error yesterday)
comment:12 by , 8 years ago
Replying to ajolma:
I have now replicated the bigint typemaps in the Perl bindings. The tests are ok and the config.h is not needed any more. Let's try this one.
Oh ok, so let forget my comment about modifying mkgdaldist.sh. Just one question, seeing r33859 and the use of "%lld" and strtoull (might work, but this isn't in the C89 standard according to the man page), I 'm not sure this will be portable on Windows. Why not using CPL_FRMT_GIB instead of "%lld" ? And CPLScanUIntBig() instead of strtoull()
comment:13 by , 8 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
cpl_config.h is needed because the bindings need to have correct GIntBig and GUIntBig typedefs.
The problem is that that part needs to be put into %{ .. %} so that swig will just transfer it into the bindings and not look into it more.
Regarding undefined $ARGV{"--gdal-source-tree"} it is a silly hack to test it against '0'. The idea was, I think, to test downloading the GDAL source (extreme bootstrapping the Perl bindings that build robots do). It probably got into the trunk by mistake.
Fixed in r33855.