-
Notifications
You must be signed in to change notification settings - Fork 160
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
Comments
@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 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.
But
things will be more complicated and still not contained in a single place... |
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. |
@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 As to |
@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. |
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. |
@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:
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. |
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 |
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 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 ;-). |
@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. |
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... |
@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? |
Suggest to move this to 4.8.2 - it's not needed for 2nd beta. |
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. |
@fingolfin I suggest to not to touch this now, and re-assign this to 4.10.0. |
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 |
We have taken care of this some time ago, by adding |
It would be useful if a suitable GAP header (included a set of defines like these (based on what GMP does):
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 headergap_version.h
currently generated bycnf/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.
The text was updated successfully, but these errors were encountered: