Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Version check in G__gisinit & G__no_gisinit is too strict for non-git builds #2610

Closed
sebastic opened this issue Oct 25, 2022 · 5 comments
Labels
bug Something isn't working
Milestone

Comments

@sebastic
Copy link
Contributor

sebastic commented Oct 25, 2022

Describe the bug

Every time GRASS is rebuilt, even when there are no changes, the timestamp recorded in GRASS_HEADERS_VERSION changes which breaks the gdal-grass plugins:

$ ogrinfo -so -al ~/grassdata/nc_basic_spm_grass7/PERMANENT/vector/zipcodes/head
Warning 1: GRASS warning: GISBASE environment variable was not set, using:
/usr/lib/grass82
ERROR: Module built against version 2022-06-11T10:48:36+00:00 but trying to
       use version 2022-10-25T05:34:10+00:00. You need to rebuild GRASS GIS
       or untangle multiple installations.

The timestamp is not used for git builds, the commit hash for the last commit affecting the include directory is used.

The error should not be triggered when there were no changes to the GRASS source tree that actually require rebuilding the gdal-grass plugins.

The GDAL version check only requires that the gdal-grass plugins are rebuilt when the major and or minor version changes which suggest ABI breakages.

Non-git builds of GRASS should use something like GRASS_VERSION_NUMBER for GRASS_HEADERS_VERSION which doesn't change when GRASS is rebuilt.

To Reproduce

Steps to reproduce the behavior:

  1. Rebuild GRASS from source tarball
  2. Execute ogrinfo on a GRASS dataset

Expected behavior

No error is trigggered.

System description

  • Operating System: Debian unstable
  • GRASS GIS version: 8.2.0

Additional context

Maintaining gdal-grass packages is very painful, because they need to be rebuilt every time GRASS is.

@sebastic sebastic added the bug Something isn't working label Oct 25, 2022
@sebastic
Copy link
Contributor Author

Something like this may suffice:

--- a/configure.ac
+++ b/configure.ac
@@ -139,7 +139,7 @@ changequote([,])
 GRASS_VERSION_GIT="exported"
 # get git short hash + date of last change in GRASS headers
 # (and anything else in include)
-GRASS_HEADERS_GIT_HASH=`date -u +%FT%T%z | sed 's/\(..\)$/:\1/'`
+GRASS_HEADERS_GIT_HASH="${GRASS_VERSION_NUMBER}"
 GRASS_HEADERS_GIT_DATE=`date -u +%FT%T%z | sed 's/\(..\)$/:\1/'`
 AC_PATH_PROG(GIT, git, no)
 if test "$GIT" != "no" ; then

Which results in a version.h like this:

#define GRASS_VERSION_STRING   "8.2.0 (2022)"
#define GRASS_VERSION_MAJOR    8
#define GRASS_VERSION_MINOR    2
#define GRASS_VERSION_RELEASE  "0"
#define GRASS_HEADERS_VERSION  "8.2.0"
#define GRASS_HEADERS_DATE     "2022-10-25T16:50:03+00:00"

Instead of:

#define GRASS_VERSION_STRING   "8.2.0 (2022)"
#define GRASS_VERSION_MAJOR    8
#define GRASS_VERSION_MINOR    2
#define GRASS_VERSION_RELEASE  "0"
#define GRASS_HEADERS_VERSION  "2022-10-25T04:21:31+00:00"
#define GRASS_HEADERS_DATE     "2022-10-25T04:21:31+00:00"

Or you could just use the version in gis.h to achieve the same for git builds:

--- a/include/grass/gis.h
+++ b/include/grass/gis.h
@@ -44,7 +44,7 @@ static const char *GRASS_copyright __attribute__ ((unused))
 /* GRASS version, GRASS date, git short hash of last change in GRASS headers
  * (and anything else in include)
  */
-#define GIS_H_VERSION GRASS_HEADERS_VERSION
+#define GIS_H_VERSION GRASS_VERSION_STRING
 /* git date of last change in GRASS headers
  * (and anything else in include)
  */

@neteler
Copy link
Member

neteler commented Oct 26, 2022

Every time GRASS is rebuilt, even when there are no changes, the timestamp recorded in GRASS_HEADERS_VERSION changes which breaks the gdal-grass plugins:

For some background discussion see #325 in which GRASS_HEADERS_VERSION has been introduced.
Perhaps @metzm can comment here if above change request may have drawbacks or not?

@sebastic
Copy link
Contributor Author

If you really need to detect changes to the headers, using a checksum of the sources may be an option, e.g.:

$ find include/grass/ -type f | sort | xargs cat | sha1sum - | cut -d' ' -f1
38bf7f86a1c488548e9cb8902d014a41ae855266

@metzm
Copy link
Contributor

metzm commented Nov 7, 2022

Changes to headers should trigger an increase in the minor version number. I prefer your suggested change to configure.ac and can't see any drawbacks, only advantages for package maintainers.

sebastic added a commit to sebastic/grass that referenced this issue Nov 8, 2022
Value is used in version check which fails when the value changes.

Fixes: OSGeo#2610
neteler pushed a commit that referenced this issue Nov 12, 2022
As reported in #2610, rebuilding the GRASS sources (from the release tarballs) with no changes break the gdal-grass plugins because the `GIS_H_VERSION` value changes.

Non-git builds use the current timestamp in `GRASS_HEADERS_GIT_HASH` which is also used for `GRASS_HEADERS_DATE`.

This PR uses `GRASS_VERSION_NUMBER` for `GRASS_HEADERS_GIT_HASH` as a better value for non-git builds, that value won't change for simple rebuilds of the source tree.
neteler pushed a commit to neteler/grass that referenced this issue Nov 12, 2022
As reported in OSGeo#2610, rebuilding the GRASS sources (from the release tarballs) with no changes break the gdal-grass plugins because the `GIS_H_VERSION` value changes.

Non-git builds use the current timestamp in `GRASS_HEADERS_GIT_HASH` which is also used for `GRASS_HEADERS_DATE`.

This PR uses `GRASS_VERSION_NUMBER` for `GRASS_HEADERS_GIT_HASH` as a better value for non-git builds, that value won't change for simple rebuilds of the source tree.
@wenzeslaus wenzeslaus added this to the 8.3.0 milestone Feb 17, 2023
ninsbl pushed a commit to ninsbl/grass that referenced this issue Feb 17, 2023
As reported in OSGeo#2610, rebuilding the GRASS sources (from the release tarballs) with no changes break the gdal-grass plugins because the `GIS_H_VERSION` value changes.

Non-git builds use the current timestamp in `GRASS_HEADERS_GIT_HASH` which is also used for `GRASS_HEADERS_DATE`.

This PR uses `GRASS_VERSION_NUMBER` for `GRASS_HEADERS_GIT_HASH` as a better value for non-git builds, that value won't change for simple rebuilds of the source tree.
@wenzeslaus
Copy link
Member

Implemented in #2636. Thanks @sebastic again!

marisn pushed a commit to marisn/grass that referenced this issue Jun 2, 2023
As reported in OSGeo#2610, rebuilding the GRASS sources (from the release tarballs) with no changes break the gdal-grass plugins because the `GIS_H_VERSION` value changes.

Non-git builds use the current timestamp in `GRASS_HEADERS_GIT_HASH` which is also used for `GRASS_HEADERS_DATE`.

This PR uses `GRASS_VERSION_NUMBER` for `GRASS_HEADERS_GIT_HASH` as a better value for non-git builds, that value won't change for simple rebuilds of the source tree.
neteler pushed a commit to nilason/grass that referenced this issue Nov 7, 2023
As reported in OSGeo#2610, rebuilding the GRASS sources (from the release tarballs) with no changes break the gdal-grass plugins because the `GIS_H_VERSION` value changes.

Non-git builds use the current timestamp in `GRASS_HEADERS_GIT_HASH` which is also used for `GRASS_HEADERS_DATE`.

This PR uses `GRASS_VERSION_NUMBER` for `GRASS_HEADERS_GIT_HASH` as a better value for non-git builds, that value won't change for simple rebuilds of the source tree.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants