GIS_H_VERSION: use git hash #325

metzm · 2020-02-02T21:51:14Z

Previously, GIS_H_VERSION was the svn revision number of gis.h and GIS_H_DATE was the svn revision date of gis.h.

Now they are the git hash and date of the last change to GRASS headers (and anything else in include)

In g.version, these have been and are still referred to as GIS library revision number (now git hash) and date which is wrong: GIS_H_VERSION and GIS_H_DATE refer to GIS headers, not the GIS library. This phrasing can not be changed without breaking backwards compatibility when parsing the output of g.version, because e.g. libgis_revision would need to be changed to gisheaders_version.

use current (target) location + mapset when calling GPJ_init_transform()

previously, GIS_H_VERSION was the svn revision number of gis.h and GIS_H_DATE was the svn revision date of gis.h now they are the git hash and date of the last change to GRASS headers (and anything else in include) in g.version, these have been and are still referred to as GIS library revision number and date which is wrong, these are GIS headers revision number and date, which is not the same

use $SRCDIR for git log

landam · 2020-02-03T22:49:38Z

@metzm , are r.in.gdal/r.proj/v.proj related commits relevant to this PR? Thanks for clarification.

metzm · 2020-02-04T09:10:21Z

@metzm , are r.in.gdal/r.proj/v.proj related commits relevant to this PR? Thanks for clarification.

No, I must have created this feature branch from another feature branch. I will remove these commits from this feature branch.

This reverts commit 0eb5ceb and 0624e6d.

This reverts commit 0624e6d.

wenzeslaus · 2020-02-04T19:39:36Z

Thanks for clarifying the headers versus library. I agree that having whole include/ there makes more sense. As for the non-header files, I think eventually we should have there only header files and move the Make directory elsewhere (or we will have CMake in cmake directory).

The new code looks like that it should work. I'm not sure if it is just a naming issue (see below), but can you please describe the reasons behind the following values (here or in the code)?

date is a fallback when Git is not available, right? That makes sense.

GRASS_HEADERS_GIT_HASH=`date`

But the triplet below looks like fallback as well, or is it to provide a more readable error message to users when the check in lib/gis/gisinit.c fails?

#define GRASS_HEADERS_GIT_HASH "@GRASS_VERSION_NUMBER@ (@GRASS_VERSION_DATE@) @GRASS_HEADERS_GIT_HASH@"

It seems to me the name should be different. In Git, hash is really the hexadecimal string. The point in code history Git calls commit or revision like in Subversion.

To keep in line with that terminology, what has "hash" in it should probably not contain anything else. So, what ends up being GIS_H_VERSION, and is used both for check and user message, should be something like GRASS_HEADERS_REVISION (it is not a version and commit sounds like version control system info only).

As for the renaming/backwards compatibility (grass --config and g.version keys and GIS_...) now is the best time to do right (so that lib does not stand for interface/headers, version for revision, and GIS for GRASS), but that's for a separate PR.

metzm · 2020-02-06T21:59:22Z

The new code looks like that it should work. I'm not sure if it is just a naming issue (see below), but can you please describe the reasons behind the following values (here or in the code)?

date is a fallback when Git is not available, right? That makes sense.

Yes, datewould work as a fallback to determine the date when GRASS was configured

But the triplet below looks like fallback as well, or is it to provide a more readable error message to users when the check in lib/gis/gisinit.c fails?
#define GRASS_HEADERS_GIT_HASH "@GRASS_VERSION_NUMBER@ (@GRASS_VERSION_DATE@) @GRASS_HEADERS_GIT_HASH@"

Yes, the purpose is to provide a more informative error message. A git commit hash is not really informative for users not compiling GRASS from a git clone soucre.

It seems to me the name should be different. In Git, hash is really the hexadecimal string. The point in code history Git calls commit or revision like in Subversion.

The code in configure calls $GIT log -1 --pretty=format:"%h". From git help log:

 ·   %h: abbreviated commit hash

To keep in line with that terminology, what has "hash" in it should probably not contain anything else. So, what ends up being GIS_H_VERSION, and is used both for check and user message, should be something like GRASS_HEADERS_REVISION (it is not a version and commit sounds like version control system info only).

Revision comes from svn, we are now on git. If you do not like GRASS_HEADERS_GIT_HASH, how about GRASS_HEADERS_GIT_VERSION in include/version.h.in:

#define GRASS_HEADERS_GIT_VERSION "@GRASS_VERSION_NUMBER@ (@GRASS_VERSION_DATE@) @GRASS_HEADERS_GIT_HASH@"

Maybe it is better to remove GIT from these macros, to be more generic. I am more interested in the content of GRASS_HEADERS_GIT_HASH and less interested in its name. *_GIT_HASH might be a bit restrictive.

remove git reference from macro names

neteler · 2020-02-15T12:28:48Z

I just applied the patch locally, it now looks like this:

GRASS 7.9.dev (nc_spm_08):~ > g.version -rge
version=7.9.dev
date=2020
revision=55cc84109
build_date=2020-01-15
build_platform=x86_64-pc-linux-gnu
build_off_t_size=8
libgis_revision=7.9.dev (2020) fda3b2441
libgis_date="Sun Jan 12 14:29:41 2020 +0100"
proj=5.2.0
gdal=2.3.2
geos=3.7.1
sqlite=3.30.0

To better understand:

revision=55cc84109

is the current master hash:

git log
commit 55cc841094158255a63510eddfdf3daee66807a3 (HEAD -> master, origin/master, origin/HEAD)
Author: ...

while

libgis_revision=7.9.dev (2020) fda3b2441

comes from where?

metzm · 2020-02-15T19:00:54Z

The git short hash fda3b24 comes from the latest change to include, see https://github.com/OSGeo/grass/commits/master/include (git log -1 --pretty=format:"%h" -- include).

Previously, GIS_H_VERSION was the svn revision number of gis.h and GIS_H_DATE was the svn revision date of gis.h.

Now they are the git hash and date of the last change to GRASS headers (and anything else in include).

As I mentioned in my initial commit, the name libgis_revision in the output of g.version was always misleading because it referred to the latest svn change of include/gis.h and not lib/gis.

neteler · 2020-02-15T21:05:49Z

For me it looks good, what do you think, @wenzeslaus ?

neteler · 2020-02-22T11:43:13Z

For me it looks good, what do you think, @wenzeslaus ?

Ping
Can we merge?

HuidaeCho · 2020-02-23T03:55:17Z

Looks good for me. Actually, I like this one better than #365. Closing mine.

As for naming, I prefer to use the more generic GRASS_HEADERS_VERSION. I think we can merge it.

general/g.version/main.c

rev_ver has spaces.

landam · 2020-03-12T14:55:38Z

@metzm What is reason for duplication (version and date are already defined by separated variables)?

libgis_revision="7.9.dev (2020) df76b070f"

Wouldn't be

libgis_revision=df76b070f

enough?

metzm · 2020-03-12T21:01:21Z

@landam The reason is that G__gisinit() and G__no_gisinit only check GIS_H_VERSION, but you are right, the git short hash should be sufficient to check for compatibility.

use only git short hash as GRASS_HEADERS_VERSION

landam · 2020-03-13T10:23:47Z

@metzm I would suggest to remove quotes from libgis_revision:

g.version -gre
...
revision=35f284289
...
libgis_revision="5872054b4"

general/g.version/main.c

change output of g.version -r to e.g. libgis revision: 5872054 libgis date: Thu Mar 12 22:06:12 2020 +0100

neteler · 2020-03-29T21:18:06Z

Seems we are fine to go now?

HuidaeCho · 2020-03-29T22:00:14Z

Seems we are fine to go now?

I think so.

landam · 2020-03-30T14:02:36Z

@metzm Can we merge?

neteler · 2020-03-30T17:08:26Z

Let me suggest, to:

merge it now into master
see how it "behaves"
backport once happy (but asap of course, for 7.8.3)

neteler · 2020-03-31T19:48:48Z

For reference, it now looks like this:

GRASS 7.9.dev (nc_spm_08):~/software/grass_master > g.version -rge
version=7.9.dev
date=2020
revision=f062bffc8
build_date=2020-03-31
build_platform=x86_64-pc-linux-gnu
build_off_t_size=8
libgis_revision=f062bffc8
libgis_date="Tue Mar 31 20:34:57 2020 +0200"
proj=5.2.0
gdal=2.3.2
geos=3.7.1
sqlite=3.30.0

veroandreo · 2020-04-01T22:01:05Z

Is there a reason why buid_date and libgis_date are displayed differently? My OCD could not avoid noticing that, sorry :) Is it too complex to display them with the same date format (ideally as in build_date)?

landam · 2020-04-01T22:22:09Z

Well libgis_date is truly to say a timestamp ;-) But this is beyond this PR. And sure build_date can be turned to a timestamp. Let's create a new PR for that.

HuidaeCho · 2020-04-01T22:22:59Z

Good catch! I agree it should use the same format (ideally, build_date).

…

On Wed, Apr 1, 2020 at 6:01 PM Veronica Andreo ***@***.***> wrote: Is there a reason why buid_date and libgis_date are displayed differently? My OCD could not avoid noticing that, sorry :) Is it too complex to display them with the same date format (ideally as in build_date)? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#325 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABY4K5OQ3FR34SXRDOD3FTLRKO2TBANCNFSM4KO4KLZA> .

-- Huidae Cho, Ph.D., GISP GRASS GIS Developer https://idea.isnew.info

HuidaeCho · 2020-04-02T00:23:14Z

One more potential improvement... because we're checking for only one character (space)

                if (strstr(rev_ver, " ") != NULL)

to

                if (strchr(rev_ver, ' ') != NULL)

would make more sense semantically and avoid function calls internally.

GCC strchr vs. strstr

landam · 2020-04-02T10:55:08Z

One more potential improvement... because we're checking for only one character (space)

Let's create a new PR for this purpose.

landam · 2020-04-02T10:56:19Z

I have tested merged PR also on Windows.

I will backport PR to releasebranch_7_8.

previously, GIS_H_VERSION was the svn revision number of gis.h and GIS_H_DATE was the svn revision date of gis.h now they are the git hash and date of the last change to GRASS headers (and anything else in include) in g.version, these have been and are still referred to as GIS library revision number and date which is wrong, these are GIS headers revision number and date, which is not the same Co-authored-by: Huidae Cho <grass4u@gmail.com> Co-authored-by: Martin Landa <landa.martin@gmail.com>

landam · 2020-04-02T11:17:59Z

One more potential improvement... because we're checking for only one character (space)

Let's create a new PR for this purpose.

See #478

landam · 2020-04-02T11:23:14Z

Good catch! I agree it should use the same format (ideally, build_date).

In any case libgis_revision should be a timestamp (if git is not available).

HuidaeCho · 2020-04-02T13:08:01Z

Good catch! I agree it should use the same format (ideally, build_date).

In any case libgis_revision should be a timestamp (if git is not available).

We can change date to date -I for both GRASS_HEADERS_GIT_HASH and GRASS_HEADERS_GIT_DATE in configure if we don't care about time (preferred because build_date is also only date).

2020-04-02

If we need time, date -Iseconds.

2020-04-02T09:05:08-04:00

Actually, it also addresses the space issue and we can remove the strchr.

What do you think?

landam · 2020-04-04T13:30:58Z

We can change date to date -I for both GRASS_HEADERS_GIT_HASH and GRASS_HEADERS_GIT_DATE in configure if we don't care about time (preferred because build_date is also only date).

date -I would be probably not enough for GRASS_HEADERS_GIT_HASH since it must be unique when git not available (from this point of view a timestamp is a better choice).

If we need time, date -Iseconds.
2020-04-02T09:05:08-04:00
Actually, it also addresses the space issue and we can remove the strchr.

What do you think?

Would be fine. But let's discuss it in a new PR.

* use space delimiter for GIS_H_VERSION * choose GRASS_VERSION_STRING list element * report header version and date * write GRASS_VERSION_GIT to grass.py * use GRASS_VERSION_GIT * write GRASS_VERSION_GIT to grass.py * fix tab indent * add date * add date * get date correctly after #325 * get date correctly after #325 * date: no indent, no quotes * Update lib/init/grass.py Co-Authored-By: Markus Neteler <neteler@gmail.com> Co-authored-by: Stefan Blumentrath <stefan@vm-srv-wallace.vm.ntnu.no> Co-authored-by: Martin Landa <landa.martin@gmail.com> Co-authored-by: Markus Neteler <neteler@gmail.com>

metzm added 3 commits January 16, 2020 17:44

switch environment: also change mapset

0624e6d

r.proj

0eb5ceb

use current (target) location + mapset when calling GPJ_init_transform()

metzm requested a review from wenzeslaus February 2, 2020 21:51

configure

55f5742

use $SRCDIR for git log

landam added the enhancement New feature or request label Feb 3, 2020

metzm added 2 commits February 4, 2020 10:12

Revert "r.proj" and related

86ec788

This reverts commit 0eb5ceb and 0624e6d.

Revert "switch environment: also change mapset"

25faf34

This reverts commit 0624e6d.

grass version

df76b07

remove git reference from macro names

neteler added this to the 7.8.3 milestone Feb 15, 2020

HuidaeCho mentioned this pull request Feb 23, 2020

g.version: Fix a bogus warning from the -r flag #365

Closed

HuidaeCho reviewed Feb 23, 2020

View reviewed changes

general/g.version/main.c Outdated Show resolved Hide resolved

g.version

21978b7

rev_ver has spaces.

ninsbl mentioned this pull request Mar 6, 2020

grass.py: used space as delimiter to get GRASS_VERSION_STRING #405

Merged

GRASS headers version

5872054

use only git short hash as GRASS_HEADERS_VERSION

landam reviewed Mar 16, 2020

View reviewed changes

general/g.version/main.c Outdated Show resolved Hide resolved

landam reviewed Mar 16, 2020

View reviewed changes

general/g.version/main.c Outdated Show resolved Hide resolved

g.version

492156a

change output of g.version -r to e.g. libgis revision: 5872054 libgis date: Thu Mar 12 22:06:12 2020 +0100

landam approved these changes Mar 29, 2020

View reviewed changes

HuidaeCho approved these changes Mar 29, 2020

View reviewed changes

landam merged commit f062bff into OSGeo:master Mar 31, 2020

neteler added the backport_needed label Mar 31, 2020

landam mentioned this pull request Apr 2, 2020

g.version: strstr -> strchr #478

Merged

landam mentioned this pull request Apr 4, 2020

GIS_H_DATE: switch to date -Iseconds format #485

Merged

wenzeslaus mentioned this pull request Apr 5, 2020

v.db.select: Add -e flag (escape newlines and backslashes) and -j flag (JSON output) #476

Merged

ninsbl added a commit to ninsbl/grass that referenced this pull request Apr 8, 2020

get date correctly after OSGeo#325

92ff788

ninsbl added a commit to ninsbl/grass that referenced this pull request Apr 8, 2020

get date correctly after OSGeo#325

a751af7

neteler removed the backport_needed label Aug 22, 2020

metzm deleted the grass_version branch September 9, 2020 20:35

neteler mentioned this pull request Oct 26, 2022

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GIS_H_VERSION: use git hash #325

GIS_H_VERSION: use git hash #325

metzm commented Feb 2, 2020

landam commented Feb 3, 2020

metzm commented Feb 4, 2020

wenzeslaus commented Feb 4, 2020

metzm commented Feb 6, 2020

neteler commented Feb 15, 2020

metzm commented Feb 15, 2020

neteler commented Feb 15, 2020

neteler commented Feb 22, 2020

HuidaeCho commented Feb 23, 2020

landam commented Mar 12, 2020

metzm commented Mar 12, 2020

landam commented Mar 13, 2020

neteler commented Mar 29, 2020

HuidaeCho commented Mar 29, 2020

landam commented Mar 30, 2020

neteler commented Mar 30, 2020

neteler commented Mar 31, 2020

veroandreo commented Apr 1, 2020

landam commented Apr 1, 2020

HuidaeCho commented Apr 1, 2020 via email

HuidaeCho commented Apr 2, 2020

landam commented Apr 2, 2020

landam commented Apr 2, 2020

landam commented Apr 2, 2020

landam commented Apr 2, 2020

HuidaeCho commented Apr 2, 2020

landam commented Apr 4, 2020

GIS_H_VERSION: use git hash #325

GIS_H_VERSION: use git hash #325

Conversation

metzm commented Feb 2, 2020

landam commented Feb 3, 2020

metzm commented Feb 4, 2020

wenzeslaus commented Feb 4, 2020

metzm commented Feb 6, 2020

neteler commented Feb 15, 2020

metzm commented Feb 15, 2020

neteler commented Feb 15, 2020

neteler commented Feb 22, 2020

HuidaeCho commented Feb 23, 2020

landam commented Mar 12, 2020

metzm commented Mar 12, 2020

landam commented Mar 13, 2020

neteler commented Mar 29, 2020

HuidaeCho commented Mar 29, 2020

landam commented Mar 30, 2020

neteler commented Mar 30, 2020

neteler commented Mar 31, 2020

veroandreo commented Apr 1, 2020

landam commented Apr 1, 2020

HuidaeCho commented Apr 1, 2020 via email

HuidaeCho commented Apr 2, 2020

landam commented Apr 2, 2020

landam commented Apr 2, 2020

landam commented Apr 2, 2020

landam commented Apr 2, 2020

HuidaeCho commented Apr 2, 2020

landam commented Apr 4, 2020