Geert and I started discussing the BUILDING_FROM_VCS CMake variable after I suggested that the Debian packager remove a hack for it in his build script when I fixed bug 793900 [1]. As you’ll see the discussion has gotten rather broad and not really germane to that bug, so I’m moving it here.
First, a recap: > Geert Janssens [GnuCash developer] 2018-03-09 14:45:05 PST > (In reply to John Ralls from comment #11) > > > In rules, you should remove the > > BUILDING_FROM_VCS hack. There is no autogen.sh any more so it breaks > > building > > from git and the code in utils/gnc-vcs-info works fine. > > > I'm not too sure about this particular piece of advice. I seem to remember > the BUILDING_FROM_VCS hack was introduced because downstream packagers can > also store our tarball in git in which case gnc-vcs-info draws the wrong > conclusion. > > You're certainly right the autogen.sh test will no longer work. Perhaps it > should be replaced with an existence test on > libgnucash/core-utils/gnc-vcs-info.h. If that file exists, the build is from > a tarball, otherwise it's a source repo based build. > > [reply] [−]Comment 13John Ralls [GnuCash developer] 2018-03-09 15:07:05 PST > There's also the contrary issue of people trying to use Github-generated > tarballs, in which case gnc-vcs-info gives the wrong answer in the other > direction. > > Another consideration: The only reason for wrapping a tarball into a vcs repo > is because you want to change it, and in that case you may need to re-swig > it. Not only that, but once it's changed then the contents of gnc-vcs-info.h > should change too. It's also possible to use a vcs other than git, svn, or > svk, or to change the code without using a vcs at all. > > [reply] [−]Comment 14Geert Janssens [GnuCash developer] 2018-03-10 02:29:32 > PST > (In reply to John Ralls from comment #13) > > > There's also the contrary issue of people trying to use Github-generated > > tarballs, in which case gnc-vcs-info gives the wrong answer in the other > > direction. > > > In that case the build will fail because the extracted tarball isn't a git > repository so the build system assumes certain files to be there that are in > fact missing: gnc-version.h (which we really should eliminate as it's become > a static file), gnc-version-info.h, ChangeLog, gnucash manpage, gnucash.pot. > > > > Another consideration: The only reason for wrapping a tarball into a vcs > > repo is because you want to change it, and in that case you may need to > > re-swig it. > > > Not necessarily. Distributions also do this to guarantee reproducibility. The > store the exact source they have used to build a certain package. > It may be altered indeed, but that's usually with patches kept in a separate > tree. > > Regardless, I have recently altered the build system to separate swig > generation from the repository presence detection. There are now two > environment variables to control this: > GENERATE_SWIG_WRAPPERS can be used to override whether or not to generate the > wrappers. If not set it will fall back to the previous default being: only > generate the wrappers when building from vcs. > BUILDING_FROM_VCS controls generation of any file that depends on git history > to be available (ChangeLog, gnc-version-info.h) > So setting GENERATE_SWIG_WRAPPERS to yes will cover this situation. > > > > Not only that, but once it's changed then the contents of > > gnc-vcs-info.h should change too. It's also possible to use a vcs other than > > git, svn, or svk, or to change the code without using a vcs at all. > > > True. First off, anything other than git will not work properly any more. In > particular the ChangeLog generation is hard wired to query git history and > gnucash will report git as vcs to the user regardless of what the real vcs > was when generating the binaries. In that context I think it's probably time > to drop support for the other version control systems from > utils/gnc-vsc-info. It made sense in the transition period from svn to git, I > doubt it still makes sense now. > > Having said that I agree we would prefer the version info to be altered if > the sources are changed outside (a clone of) our git repository. > Unfortunately that's something we can only recommend, not enforce. There are > currently two ways to handle this: > 1. updating gnc-vcs-info.h in the source tree > 2. set GNUCASH_BUILD_ID to override the version information we otherwise > would extract from gnc-vcs-info.h. While writing this I believe we could > still polish the experience from this. > > Lastly, one could argue it would also be preferable to update the changelog > when making changes outside (a clone of) our git repository. Yes, I know that building from GitHub-generated tarballs is broken. That’s why I brought it up. If building from an svn/svk repo no longer works we should surely not set BUILDING_FROM_VCS if that’s the kind of VCS we’re in. But let’s back up a bit. I think we’ve pretty thoroughly established that making build decisions based on the presence or absence of a “.git” directory in the source directory is fragile. It’s also unusual; I don’t think I’ve seen any other build system that does that. Let’s catalog everything that we’re switching with BUILDING_WITH_VCS: 1. Generate gnc_vcs_info.h and GNC_VCS_REV 2. Generate Changelog 3. Set GENERATE_SWIG_WRAPPERS (can be manually overridden) 4. Generate gnucash.pot 5. Include (the mostly useless) design.info <http://design.info/> in a dist-generated tarball. 6. Find gnc_vcs_info.h since its location (srcdir vs. builddir) depends on whether it’s generated of packaged. 7. Configure where to get files (srcdir vs. builddir) when making a tarball. 6 and 7 can be done using cmake’s find_path function as I did yesterday in fbb172d09. ISTM 5 shouldn’t depend on whether one is building from vcs. IMO it shouldn’t be done at all, but that’s a separate issue. Deciding whether to generate swig wrappers and gnucash.pot rather depends on what one is doing. A distribution tarball contains swig wrappers and gnucash.pot, but those can be invalidated by changes to the source. It seems to me that it would be more robust if we check for the presence of gnucash.pot and one of the swig wrapper files in srcdir and set the default values of GENERATE_SWIG_WRAPPERS and GENERATE_POTFILE accordingly, which the builder could override from the cmake command line. ChangeLog is a bit harder: It’s not possible to generate a correct one unless the source is a clone of our git repository. It’s arguable that it’s obsolete: The complete history is easily obtained from the repo if someone is interested. We currently generate it as part of every build from vcs, but perhaps it should be moved to the dist target--or maybe it should be simply dropped. Which leaves setting the version. When building from a release tarball we want to use the plain release number as the version, e.g. GnuCash-2.7.5; when a build isn’t from a release tarball we want to indicate that. Ideally, of course, someone building from a modified release tarball would also do that but as Geert observed it’s not possible for us to enforce that. There are two other problems: First, as Geert pointed out when he began the discussion, it’s not unreasonable for someone to take a tarball and create a new repository from it. As discussed already that repo won’t necessarily be a git one, thus breaking the versioning code, but even if it is the retrieved commit will be confusing because it won’t be one from the canonical repository. One possible solution would be to test the value of `git remote -v 2> /dev/null | grep "origin.*(fetch)"`. If it’s either code or the Github mirror then we set the version from `git describe`, otherwise it’s the ${VERSION} as set in CMakeLists.txt. Regards, John Ralls [1] https://bugzilla.gnome.org/show_bug.cgi?id=793900 <https://bugzilla.gnome.org/show_bug.cgi?id=793900> _______________________________________________ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
