[GRASS-dev] [GRASS GIS] #3880: update/drop revision check after git switch

GRASS GRASS developer
1

#3880: update/drop revision check after git switch
--------------------------------+---------------------------------
Reporter: hellik | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.6.2
Component: Compiling | Version: svn-releasebranch76
Keywords: svn, git, revision | CPU: All
Platform: All |
--------------------------------+---------------------------------
see [https://github.com/OSGeo/grass/pull/69#issuecomment-516579640 gh
comment]

{{{
Yeah, the revision check no longer works. I suppose it needs to be dropped
since git does not provide anything similar (AFAIK).
}}}

the issue popped up while adapting the precompiled winGRASS addon system.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880&gt;
GRASS GIS <https://grass.osgeo.org>

2

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: blocker | Milestone: 7.6.2
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------
Changes (by neteler):

* priority: normal => blocker

Comment:

The code in question is here:

{{{
# include/gis.h
41:#define GIS_H_VERSION "$Revision$"
44:#define G_gisinit(pgm) G__gisinit(GIS_H_VERSION, (pgm))
45:#define G_no_gisinit() G__no_gisinit(GIS_H_VERSION)

# lib/gis/gisinit.c, line 86ff
     if (strcmp(version, GIS_H_VERSION) != 0)
         G_fatal_error(_("Module built against version %s but "
                         "trying to use version %s. "
                         "You need to rebuild GRASS GIS or untangle
multiple installations."),
                         version, GIS_H_VERSION);
     gisinit();
}}}

This needs urgently to be addressed since SVN "$Revision$" doesn't exist
any more in git.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

3

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: blocker | Milestone: 7.6.2
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

See also #3856

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:2&gt;
GRASS GIS <https://grass.osgeo.org>

4

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: blocker | Milestone: 7.6.2
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

PR available at https://github.com/OSGeo/grass/pull/117

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:3&gt;
GRASS GIS <https://grass.osgeo.org>

5

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: blocker | Milestone: 7.6.2
Component: Compiling | Version: svn-releasebranch76
Resolution: fixed | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------
Changes (by martinl):

* status: new => closed
* resolution: => fixed

Comment:

Closed by https://github.com/OSGeo/grass/pull/117

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:4&gt;
GRASS GIS <https://grass.osgeo.org>

6

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------
Changes (by neteler):

* status: closed => reopened
* resolution: fixed =>
* milestone: 7.6.2 => 7.8.3

Comment:

The changes in https://github.com/OSGeo/grass/pull/117 were reverted
yesterday in

* master:
https://github.com/OSGeo/grass/commit/6617ffb4edd9ffcf1cd016958f70a93dc9229e15
* relbr78:
https://github.com/OSGeo/grass/commit/156cdb5c3b91db2dd4183247b36b724c419db574

However, I am not convinced that it should be reverted. Rationale: The old
SVN tag

https://github.com/OSGeo/grass/blob/8b9512b8b2fb0531df848fce58619308cbc29b3d/include/gis.h#L41

is no longer expanded after the migration to git and in the last many
months we didn't find a git equivalent (as it simply doesn't exist).

Reverting is fine once an alternative has been found. For now we have the
risk that addons are broken again.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:5&gt;
GRASS GIS <https://grass.osgeo.org>

7

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

Note: it lead to https://github.com/OSGeo/grass/pull/278

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:6&gt;
GRASS GIS <https://grass.osgeo.org>

8

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by mmetz):

Replying to [comment:6 neteler]:
> Note: it lead to https://github.com/OSGeo/grass/pull/278

The patch in PR278 for gisinit.c is masking a more serious problem with
ctypes generated for Python 3.8: the string "$Revision$" is truncated to
its first character "$", then "$" is compared to "$Revision$" which fails
of course. Such truncations of strings to their first character must occur
elsewhere too, which will only become apparent at a later stage when
executing a Python module that uses ctypes, probably causing fairly
cryptic errors.

The test in gisinit.c as it is now always compares "$Revision$" to
"$Revision$" (in GRASS C modules) which always succeeds which is bad. We
could instead use GRASS_VERSION_STRING from include/version.h to catch the
obvious cases: module x as been compiled against GRASS 7.y and is
attempted to be used with GRASS 7.z.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

9

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

Replying to [comment:7 mmetz]:
> We could instead use GRASS_VERSION_STRING from include/version.h to
catch the obvious cases: module x as been compiled against GRASS 7.y and
is attempted to be used with GRASS 7.z.

This sounds good to me.

(of course the "docker/alpine/alpine-py38-gisinit.patch" in
https://github.com/OSGeo/grass/pull/278 can then be removed)

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>

10

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by mmetz):

Replying to [comment:8 neteler]:
> Replying to [comment:7 mmetz]:
> > We could instead use GRASS_VERSION_STRING from include/version.h to
catch the obvious cases: module x as been compiled against GRASS 7.y and
is attempted to be used with GRASS 7.z.
>
> This sounds good to me.
OK
>
> (of course the "docker/alpine/alpine-py38-gisinit.patch" in
https://github.com/OSGeo/grass/pull/278 can then be removed)

This patch does not solve problems, it masks problems because no errors
occur in GRASS core, only when compiling certain addons.

The patch in PR278 for gisinit.c is masking a more serious problem with
ctypes generated for Python 3.8: the string "$Revision$" is truncated to
its first character "$", then "$" is compared to "$Revision$" which fails
of course. Such truncations of strings to their first character must occur
elsewhere too, which will only become apparent at a later stage when
executing a Python module that uses ctypes, probably causing fairly
cryptic errors.

Which parts of this description need to be elaborated?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:9&gt;
GRASS GIS <https://grass.osgeo.org>

11

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

Replying to [comment:9 mmetz]:
> The patch in PR278 for gisinit.c is masking a more serious problem with
ctypes

Keep in mind that this temporary patch is only in the Alpine build.

> Which parts of this description need to be elaborated?

All fine - the question (for me) is only how to change the gisinit.c code
to use GRASS_VERSION_STRING.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:10&gt;
GRASS GIS <https://grass.osgeo.org>

12

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by hellik):

the original report was opened because winGRASS addons were broken after
switch to git.

>The changes in ​https://github.com/OSGeo/grass/pull/117 were reverted
yesterday in

the addons will be broken again with the reverted changes until another
solution will be found

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:11&gt;
GRASS GIS <https://grass.osgeo.org>

13

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by mmetz):

Replying to [comment:11 hellik]:
> the original report was opened because winGRASS addons were broken after
switch to git.
>
> >The changes in ​https://github.com/OSGeo/grass/pull/117 were reverted
yesterday in
>
> the addons will be broken again with the reverted changes until another
solution will be found

I have replaced the SVN keyword $Revision$ with the generic
GRASS_VERSION_STRING in master
[https://github.com/OSGeo/grass/commit/ec9b6f24b3bfaf657ebf4ae67d95ffe197d10d9b
ec9b6f2].

The confusion of main GRASS checked out with git and addons checked out
with svn should be solved by now.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:12&gt;
GRASS GIS <https://grass.osgeo.org>

14

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

Replying to [comment:12 mmetz]:
>
> I have replaced the SVN keyword $Revision$ with the generic
GRASS_VERSION_STRING in master
[https://github.com/OSGeo/grass/commit/ec9b6f24b3bfaf657ebf4ae67d95ffe197d10d9b
ec9b6f2].

Thanks!

**Important:** `make distclean` is needed to compile GRASS GIS.

> The confusion of main GRASS checked out with git and addons checked out
with svn should be solved by now.

This needs to be backported, probably down to 7.4. I can do that.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:13&gt;
GRASS GIS <https://grass.osgeo.org>

15

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

All backports done and gisinit part of PR278 removed.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:14&gt;
GRASS GIS <https://grass.osgeo.org>

16

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: fixed | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------
Changes (by neteler):

* status: reopened => closed
* resolution: => fixed

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:15&gt;
GRASS GIS <https://grass.osgeo.org>

17

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------
Changes (by mmetz):

* status: closed => reopened
* resolution: fixed =>

Comment:

Re-opening the ticket because `g.version` needs to be updated as well to
avoid
{{{
WARNING: GRASS GIS libgis version and date number not available
}}}

Also, let's wait for confirmation from wingrass addon building that this
is still working.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:16&gt;
GRASS GIS <https://grass.osgeo.org>

18

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by mmetz):

Regarding version and date check as in
[https://github.com/OSGeo/grass/blob/master/lib/gis/gisinit.c#L52 lib/gis
G__gisinit()],
[https://github.com/OSGeo/grass/blob/master/lib/gis/gisinit.c#L86 lib/gis
G__no_gisinit()] and
[https://github.com/OSGeo/grass/blob/master/general/g.version/main.c#L142
g.version], the hash and date of the last git commit can be easily
obtained for any file/directory and used to define
[https://github.com/OSGeo/grass/blob/master/include/gis.h#L42
GIS_H_VERSION] and
[https://github.com/OSGeo/grass/blob/master/include/gis.h#L43 GIS_H_DATE].

The hash and date of the latest commit to the `include` folder can be
obtained with e.g.
{{{
git log -1 --pretty=format:"%H|%cd" -- include
}}}
catching any changes in the GRASS header files.

This git commit hash and date can be stored in a file and then used by
`configure` to update `include/version.h` after this header has been
generated (also by configure).

Obviously, `configure` must also work if git is not available or if the
source code is not a git clone.

The challenge is to automatically update the file with the hash and date
of the latest git commit to the `include` folder. Updating and uploading
this file will require another commit. We could provide a script to update
the file with the hash and date of the latest git commit to the `include`
folder and add a [https://git-scm.com/book/en/v2/Customizing-Git-Git-Hooks
git hook] to make sure this hash+date file has been updated if any changes
have been made to the `include` folder.

The challenge is to create an appropriate [https://git-scm.com/book/en/v2
/Customizing-Git-Git-Hooks git hook].

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:17&gt;
GRASS GIS <https://grass.osgeo.org>

19

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

For a fix, see PR325 (https://github.com/OSGeo/grass/pull/325)

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:18&gt;
GRASS GIS <https://grass.osgeo.org>

20

#3880: update/drop revision check after git switch
------------------------+---------------------------------
  Reporter: hellik | Owner: grass-dev@…
      Type: defect | Status: reopened
  Priority: blocker | Milestone: 7.8.3
Component: Compiling | Version: svn-releasebranch76
Resolution: | Keywords: svn, git, revision
       CPU: All | Platform: All
------------------------+---------------------------------

Comment (by neteler):

The updated PR325 (​https://github.com/OSGeo/grass/pull/325) has been
merged today, yet a backport is needed (or modification).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3880#comment:19&gt;
GRASS GIS <https://grass.osgeo.org>