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

Add GAP_VERSION to a suitable header #333

Closed
fingolfin opened this issue Nov 2, 2015 · 16 comments
Closed

Add GAP_VERSION to a suitable header #333

fingolfin opened this issue Nov 2, 2015 · 16 comments
Assignees
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: build system

Comments

@fingolfin
Copy link
Member

fingolfin commented Nov 2, 2015

It would be useful if a suitable GAP header (included a set of defines like these (based on what GMP does):

#define GAP_VERSION            4
#define GAP_VERSION_MINOR      7
#define GAP_VERSION_PATCHLEVEL 8
#define GAP_RELEASE (GAP_VERSION * 10000 + GAP_VERSION_MINOR * 100 + GAP_VERSION_PATCHLEVEL)

The above example is for GAP 4.7.8.

That header could be either a new one, say src/version.h or we could augment the header gap_version.h currently generated by cnf/mkversionheader.sh.

This would then be allow adding version checks to C code in packages. It could also be used by the GAP kernel itself, to verify a match between kernel and library version. We might add another #define / flag which indicate whether this is really that release, or an intermediate git version.

Of course that should also be tied suitably into the release process, so that this does not cause extra work, and so that there is no risk of this information becoming out-of-sync with other places carrying this information. A typical way to ensure that would be to encode the version in a single place (say, the configure.in file, or in a global VERSION file, or...) and then use that single source to generate resp. populate all other occurrence of the version.

@fingolfin fingolfin added the kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements label Nov 2, 2015
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.8.1 milestone Nov 2, 2015
@olexandr-konovalov
Copy link
Member

@fingolfin looks like the last paragraph is incomplete.

I support revising what we're doing here. I did a small fix in updateversioninfo from release wrapping tools to inject version number into mkversionheader.sh for GAP 4.8 beta, but was also keeping in mind revising that before the public release.

However, one should be careful: what will you keep under version control and in which branch, and how to ensure that this is not accidentally broken while merging one branch into another. At the moment, we inject version numbers only at the stage of wrapping the release, while builds from the clone of the repository have a well-defined header like e.g.

GAP v4.7.8-455-gedddca1 of 2015-10-26 21:55:20 (GMT)

But GAPInfo.Version is still equal to "4.dev" which ensures that version comparison considers it higher than any other version. I am slightly concerned that with

#define GAP_VERSION            4
#define GAP_VERSION_MINOR      7
#define GAP_VERSION_PATCHLEVEL 8

things will be more complicated and still not contained in a single place...

@stevelinton
Copy link
Contributor

Way back when we had in mind the possibility that some minor releases might not touch the kernel at all so that you might have a 4.1.2 kernel and a 4.1.3 library perfectly correctly. I'm guessing we can give that up now, but it is technically a change of policy.

@fingolfin
Copy link
Member Author

@alex-konovalov All good points, of course. Regarding the merging issue: If the version is in a single file, say VERSION, then it is quite easy to spot for it being merged incorrectly. Moreover, if one does things correctly, it should not be possible to merge it "by accident". To this end, I would envison that the master branch always carries the "next" major version. So, right now master would have a VERSION file containing, say 4.8.0; when branch stable-4.8 is created, the VERSION file on master is changed to 4.9.0, and that on stable-4.8 is not changed at all, initially. Then later, the VERSION file on the stable branch gets incremented to 4.8.1, 4.8.2, etc. -- during the next merge of stable into master, one would now get a conflict, which, however, is easy to resolve. Indeed, one could do such a merge immediately after incrementing the VERSION file, immediately resolving it; then any further merges of stable-4.8 into master would not conflict in VERSION (until VERSION is changed the next time on the stable branch, that is).

As to 4.dev being higher than any other version, i.e. effectively being infinity: I must say I never quite liked that. In the above scenario approach, one could have packages under development requiring "GAP >= 4.9" right after the branch is made. So, if your package requires features from GAP's master branch, you now have an obvious solution how to mark that. And this way, your PackageInfo.g will even stay valid once the 4.9.x series actually gets declared stable and hence released.

@fingolfin
Copy link
Member Author

@stevelinton I see your point, and I would indeed prefer to not have separate versions of kernel and library, even should the hypothetical situation occur in which we update the library but not the kernel. The gains from having two versions are small (actually: I can't think of any), but the cost is extra complication.

But of course if this is a policy change, it should be marked explicitly as such.

@stevelinton
Copy link
Contributor

The original gain was that people didn't need to compile, which was relatively tedious when you had a typical early 90s mixed bag of UNIX systems and not too much spare disk space.

@olexandr-konovalov
Copy link
Member

@fingolfin we had an idea of similar workflow in St Andrews, but haven't implemented it. Perhaps one of the stopping factors was that there were several places to update it, while with the current release tools I need only to bump up the version number like in gap-system/gap-distribution@ddb6a62. With your scenario, all changes will be in one place, but there will be more work on merging and/or on watching for hopefully rare accidental incorrect merges. I can agree that this may be worth efforts, because of the benefits like:

  • mentioned in your comment ability to distinguish between different branches in the development versions of packages
  • ability to mark a revision as release in GitHub and assign a DOI to the release.

One more thing: when version numbers will be hardcoded in the repository, downloading a shapshot of it as a zip file will result in an archive that may be mistaken for the source distribution of the core GAP system, but in fact that will be a release candidate.

@olexandr-konovalov
Copy link
Member

Accidentally posted the previous comment unfinished - so my suggestion is to make clear in the repository that this is a release candidate. Maybe by adding -rc in the version number, for example, 4.8.1-rc. Release tools would then remove -rc instead of injecting version numbers, and that would be a proper GAP distribution.

@fingolfin
Copy link
Member Author

But an arbitrary snapshot is not a release candidate. I'd argue that anybody who manages to download a snapshot is doing so at their own peril... Still, of course we could also set the VERSION to something like 4.9.0dev on the master branch (note: I realize that the current version compare code does not know how to deal with that. But that's easy to take care of. Moreover, the VERSION file is only used to generate the actual version string in other places, and nothing forces us to copy it verbatim, we can transform it any way we like).

In the end, I think a PR implementing this would be the best explanation of what I have in mind, and that would the allow discussing concrete issues with it, instead of hypothetical issues ;-).

@fingolfin
Copy link
Member Author

@stevelinton Yeah, disk use and network bandwith where things that came to mind, but I quickly discarded them ... as it is, the disk space used by packages in a GAP distro is dwarving the kernel and library.

@markuspf
Copy link
Member

Would it be possible (and advisable) to generate the version from a git tag? This certainly does away with merge problems. We just have to make sure to have some fallback in case we cannot parse the tag...

@olexandr-konovalov
Copy link
Member

@markuspf: do you mean using the algorithm based on the name of the current branch and the most recent tag in the ancestors of the current revision?

@olexandr-konovalov
Copy link
Member

Suggest to move this to 4.8.2 - it's not needed for 2nd beta.

@olexandr-konovalov
Copy link
Member

I've rescheduled this for GAP 4.9 - setting version numbers works well in stable-4.8 and it's refactoring there could be problematic.

@olexandr-konovalov
Copy link
Member

@fingolfin I suggest to not to touch this now, and re-assign this to 4.10.0.

@fingolfin fingolfin modified the milestones: GAP 4.9.0, GAP 4.10.0 Nov 3, 2017
@fingolfin fingolfin modified the milestones: GAP 4.10.0, GAP 4.11 Sep 28, 2018
@fingolfin
Copy link
Member Author

fingolfin commented Sep 28, 2018

This might also be useful for libGAP; but it certainly won't happen for GAP 4.10, so I am updating the milestone now.

I think the right way to go about this might actually be similar to what @ChrisJefferson is doing in PR #2873, i.e., the version #define should be in the generated config.h. Then the version data itself can either be in configure.ac, or in another file which is read by configure.ac. But of course this also interacts with how GAP distributions are made... Perhaps @alex-konovalov and me can discuss it some more in St Andrews in a few weeks.

@dimpase dimpase removed topic: libgap things related to libgap labels Oct 31, 2018
@fingolfin fingolfin removed this from the GAP 4.11 milestone Mar 24, 2019
@fingolfin
Copy link
Member Author

We have taken care of this some time ago, by adding GAP_KERNEL_MAJOR_VERSION and GAP_KERNEL_MINOR_VERSION

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: enhancement Label for issues suggesting enhancements; and for pull requests implementing enhancements topic: build system
Projects
None yet
Development

No branches or pull requests

5 participants