Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#1518 closed enhancement (wontfix)

Report SVN revision in PostGIS_Full_Version

Reported by: realityexists Owned by: pramsey
Priority: medium Milestone: PostGIS 2.0.0
Component: postgis Version: master
Keywords: Cc:

Description

It would be helpful if there was a way to find the exact PostGIS version, including the SVN revision it was built from, in SQL. PostGIS_Full_Version() reports "2.0.0SVN" or similar, but not the revision.

Sometimes our application relies on a particular bug fix, which we know was made in a particular revision. If an earlier version of PostGIS is used it may run for hours before failing, so I'd like to check the version at the start and fail immediately if it's not right. I currently do this by testing for the bug directly, but that kind of check doesn't really belong in production code. Checking the version would be simpler and cleaner.

Change History (20)

comment:1 by mcayland, 12 years ago

I'm not 100% convinced by your argument, since if you're running an SVN (i.e. non-release) version of PostGIS then you really are on your own at that point. If you build your own versions from trunk, you have automatically taken on the responsibility for tracking and managing these changes yourself, including distributing fixes to 3rd party clients to ensure compatibility with your application if required.

comment:2 by realityexists, 12 years ago

Oh, of course we're on our own as far as tracking and managing changes. It would just be helpful, in doing that, to know the exact version. We typically download the snapshot from the website, rather than directly from SVN, so even if you only put the revision number into those snapshots it would be helpful. Of course, if the build automatically checked the current revision when building from an SVN working copy, that would be even better.

comment:3 by mcayland, 12 years ago

I think this is a tarball-building issue rather an SVN issue. SVN only updates the revision Id for a file *if that file is modified as part of a commit*. Hence unless we get everyone to modify one particular file for every commit then we cannot use plain SVN Id keywords to do this automatically.

The only way I can see to achieve this is to ask the tarball maintainers to modify their build scripts to create some svn commands that extract the latest revision from the remote repository and drop it into a file which is then added to the tarball before upload. However this may/may not be possible on all platforms.

comment:4 by realityexists, 12 years ago

I think to actually get the SVN revision in a build script and put it into an environment variable or a file is quite easy: just run svnversion inside the working copy, ignoring any errors (in case it's not a working copy or SVN is not even installed). That returns the revision of the entire working copy (which might be mixed, but hopefully won't be in an official build) But this ticket is really about being able to retrieve it from an SQL client, so it would need to somehow get from the variable/file into the output of PostGIS_Full_Version (or some other SQL function).

comment:5 by mcayland, 12 years ago

Right, but then changing the output of postgis_full_version() is going to break a lot of existing scripts which already parse its output. And also, it doesn't answer the question as to how to get the revision number embedded within the file in the first place (it's a classic chicken-and-egg situation - you don't know the revision until you've done the checkout, and you can't automatically include the revision number within something you haven't checked out). The only solution I can see is to include it later as part of the packaging stage, which is outside the scope of SVN.

comment:6 by realityexists, 12 years ago

Yes, I meant getting the version as part of the build. The version returned by PostGIS_Full_Version() is read from Version.config during the build somehow, I'm guessing? I'm proposing a similar thing for the SVN revision. Run svnversion during the build (make), write its output to another file, say Revision.config (which is SVN-ignored). Then whatever reads Version.config can also read Revision.config and append its content to the micro version, so PostGIS_Full_Version() returns

    POSTGIS="2.0.0SVN9010" GEOS="...

or maybe this is more backwards compatible:

    POSTGIS="2.0.0SVN" POSTGIS_REVISION="9010" GEOS="...

If that's considered a breaking change then maybe a new SQL function?

comment:7 by mcayland, 12 years ago

I don't believe that adding svn binaries as a build dependency is a sensible thing here for two reasons:

1) a lot of people build PostGIS from source, and by doing this you're asking them to install a development tool which I see as being outside of pure build dependencies. Development tools should only be required by people who wish to develop on the project.

2) you're assuming that everyone who builds/develops on PostGIS is using SVN (hint: I'm not)

I'm afraid that I've yet to be persuaded that this is other than a packaging issue: if you are downloading arbitrary builds of PostGIS, it is up to you to track the svn revision of what you have installed, whether that's an svnversion on your own local checkout or even renaming the tarball to include the revision number before you distribute it.

It's a -1 from me I'm afraid.

comment:8 by strk, 12 years ago

Doesn't need to be a strong dependency, you could advertise the revision when available and not when not available. So snapshots won't have the revision encoded (unless the packaging tool includes it), but builds from repository would.

comment:9 by realityexists, 12 years ago

It would not be a dependency and I certainly wouldn't ask anyone to install SVN. Like strk said, just try to run it, but ignore any error if it fails. It should be included in the snapshots on the website, though, that's the whole point.

To take a step back, this isn't really about SVN for me - that's an implementation detail. The goal is to be able to tell the difference between a version before some bug fix and a version after it. So if you want to call it a "build number" that works just as well, as long as it somehow increments for every change. (Maybe an SVN trigger that automatically updates a file as part of every commit?) SVN revision just seemed like a natural choice for this, because that's what's already published on the download page.

comment:10 by pramsey, 12 years ago

Check out r9079, which I think ensures the existence of an svnrevision.h file that other build components can now pick up if they want to advertise the global revision of the code base. This could be used by postgis_full_version, the loader/dumpers, even the scripts versions I think.

comment:11 by pramsey, 12 years ago

With the changes in r9123 postgis_scripts_released should return the released revision number. Do we want to be more explicit?

comment:12 by pramsey, 12 years ago

Resolution: fixed
Status: newclosed

Done at rDone at r9149

comment:13 by pramsey, 12 years ago

Resolution: fixed
Status: closedreopened

comment:14 by pramsey, 12 years ago

I'm going to have to back this whole thing out. Marks' right, it's just too hard to square this against all the possible build scenarios there are out there (svn up, svn export, tarball, ..?)

comment:15 by pramsey, 12 years ago

Resolution: wontfix
Status: reopenedclosed

Backed out at r9151

comment:16 by pramsey, 12 years ago

Trying again in r9155. This time we only try to put in an SVN revision if we have SVN available or in the case of building with make_dist.sh. We let the scripts get their versions from the Id headers still to keep things simple. SVN revision is exposed in postgis_svn_version, and returns NULL is we don't know what it is. Suitable for users building from svn checkouts.

comment:17 by strk, 12 years ago

How it should work:

  1. Always try to get SVN revision (svn exists, you have an .svn)
  2. Compare the SVN revision got with what's in the existing postgis_svn_version.h
  3. At "make" time, only update postgis_svn_version.h if you detected something _and_ what you detected is different from what postgis_svn_version.h knows about
  4. Always distribute postgis_svn_version.h

Supposedly any case in which (1) would fail would have a postgis_svn_version.h in it.

comment:18 by strk, 12 years ago

I've added support for git-svn into r9158. IFF an .svn folder exists SVN is used, otherwise if .git folder exist GIT-SVN is used.

The conditional update-only-if-new-revision-detected logic is yet to be implemented. Once that is done we can encode dependency of targets on the new header file.

comment:19 by strk, 12 years ago

r9159 adds override prevention, to be make-friendly

comment:20 by strk, 12 years ago

r9161 encodes dependencies and r9162 has run_test report the revision on make check

Note: See TracTickets for help on using tickets.