Bug 562743
|
Description
Aleksey Avdeev
2010-02-08 09:21:42 UTC
Created attachment 401399 [details]
libvirt-0.7.7-alt-offline-bootstrap.patch
git diff v0.7.7 ALT/fix/bootstrap
Comment on attachment 401399 [details] libvirt-0.7.7-alt-offline-bootstrap.patch Реализация возможности офлайнового bootstrap для libvitr v0.7.7. Патчу соответствует бранч ALT/fix/bootstrap моего git repo (see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=shortlog;h=refs/heads/ALT/fix/bootstrap). Example of use in my spec (see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=blob;f=libvirt.spec;h=c72a672964e44c936f861a009abfdb7f3de5505a;hb=master): %build export GNULIB_SRCDIR=gnulib-%name-%version export NO_GIT=t export SKIP_PO=t ./autogen.sh Comment on attachment 401399 [details] libvirt-0.7.7-alt-offline-bootstrap.patch Implementation capacity offline bootstrap for libvitr v0.7.7. Patch against the brunch ALT/fix/bootstrap my git repo (see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=shortlog;h=refs/heads/ALT/fix/bootstrap). Example of use in my spec (see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=blob;f=libvirt.spec;h=c72a672964e44c936f861a009abfdb7f3de5505a;hb=master): %build export GNULIB_SRCDIR=gnulib-%name-%version export NO_GIT=t export SKIP_PO=t ./autogen.sh I'm not sure why you insist on rerunning autogen.sh. If you are building an rpm of libvirt, you should start from an existing tarball, which should already contain everything pre-boostrapped. Therefore, building the rpm should start with './configure', not './autogen.sh'. autogen.sh is intended for developers, not distributors, and it can be assumed that developers have git handy. To run autogen.sh no. He runs one time: only when building the package. (Only from the spec when I run rpmbuild -bi). So for me it is important that all he needed was already inside the src.rpm sent to the assembly. All source code in case src.rpm contains the results of such things as libtoolize, aclocal, autoconf, autoheader, automake and the like: all this is done already in the spec. These are the traditions of my distribution. (And it is easy: what to do by hand what you can instruct the robot?) PS: I'm preparing a simplified patch. Comment on attachment 401399 [details] libvirt-0.7.7-alt-offline-bootstrap.patch Branch ALT/fix/bootstrap updated, see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=shortlog;h=refs/heads/ALT/fix/bootstrap Created attachment 401505 [details] 0001-bootstrap.conf-Do-not-add-git-in-buildreq-if-the-var.patch bootstrap.conf: Do not add git in buildreq, if the variable is defined GNULIB_SRCDIR. see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=commit;h=1406a33820048789fee0e33b4e2165954446e4ee Created attachment 401507 [details] 0002-Fix-status-function-in-initscript.patch autogen.sh: Always call the bootstrap, if you can not determine the hash submodule see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=commit;h=5497f7a05b61e925796d23c0ceaed15b61315924 Comment on attachment 401507 [details]
0002-Fix-status-function-in-initscript.patch
Этот патч добавлен по ошибочно.
Created attachment 401508 [details] 0002-autogen.sh-Always-call-the-bootstrap.patch autogen.sh: Always call the bootstrap, if you can not determine the hash submodule see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=commit;h=5497f7a05b61e925796d23c0ceaed15b61315924 Created attachment 401509 [details] 0003-bootstrap.conf-Do-not-check-rsync-if-the-variable-is.patch bootstrap.conf: Do not check rsync, if the variable is defined SKIP_PO see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=commit;h=a82acee5d2b33d59219632d7c04d51ff4c2580b6 Created attachment 401510 [details] 0004-README-hacking-If-you-do-not-need-to-download-po-fil.patch README-hacking: If you do not need to download po files, define the variable SKIP_PO see http://git.altlinux.org/people/solo/packages/?p=libvirt.git;a=commit;h=c1add6605849be4c88b80bd412355e280dbc783c Eric, does current libvirt GIT include these patches for bootstrap now ? I've lost track of what changes we made there I still disagree with patches 1, 2, and 3 - you should NOT be running ./autogen.sh nor ./bootstrap if you are building from a tarball; so it should not matter what bootstrap.conf nor autogen.sh contains if you will not be running them. Besides, what happens if your choice of $GNULIB_SRCDIR does not include the commit that libvirt is expecting, but you short-circuited things to not allow a git checkout to update to a newer GNULIB_SRCDIR? ./autogen.sh is intended solely for developers, not distro generation. If you want to rerun the autotools in your spec file, run 'autoreconf -f', not './autogen.sh'. But even then, I claim that you can get away without rebuilding the autotool files unless you are patching configure.ac or Makefile.am as one of your spec file patches. At any rate, I claim that the bug is in the %build of your spec file, not in libvirt. (If you can manage to convince me that patches 1 and 3 are still necessary, I'd rather see the style: buildreq=" a " if cond; then buildreq="$buildreq b " fi buildreq="$buildreq c " than the proposed buildreq1=" a " buildreq2= if cond; then buildreq2=" b " buildreq="$buildreq1$buildreq2" but that's stylistic, not a show-stopper) Patch 4 is documentation only, and seems useful. But it would be worth proposing that upstream to the bug-coreutils mailing list (libvirt borrows it's README-hacking from coreutils). Aleksey, can we close this out as NOTABUG? Created attachment 433627 [details] 0001-bootstrap.conf-Do-not-add-git-in-buildreq-if-the-var.patch bootstrap.conf: Do not add git in buildreq, if the variable is defined GNULIB_SRCDIR. see http://git.altlinux.org/people/solo/packages/libvirt.git?p=libvirt.git;a=commit;h=cf117ebf0f91a4ac31d5c3eab616e5a9279facc5 Created attachment 433628 [details] 0002-bootstrap.conf-Do-not-check-rsync-if-the-variable-is.patch bootstrap.conf: Do not check rsync, if the variable is defined SKIP_PO see http://git.altlinux.org/people/solo/packages/libvirt.git?p=libvirt.git;a=commit;h=987949fc2075ab8037daca509d2abd32c344e06c Created attachment 433629 [details] 0003-README-hacking-If-you-do-not-need-to-download-po-fil.patch README-hacking: If you do not need to download po files, define the variable SKIP_PO see http://git.altlinux.org/people/solo/packages/libvirt.git?p=libvirt.git;a=commit;h=800beb0e1ecd2855edd044304bc297b9926028fe Comment on attachment 401508 [details] 0002-autogen.sh-Always-call-the-bootstrap.patch Obsolete in new ALT/fix/bootstrap see http://git.altlinux.org/people/solo/packages/libvirt.git?p=libvirt.git;a=shortlog;h=refs/heads/ALT/fix/bootstrap 1. I build the package is not from the official tarball, but directly from the git repository transferred to the build system. All the necessary tarballs, sent directly to the assembly, build system creates itself, from these I commit. 2. Accessing external resources in the process of assembly (and / or at the preparatory stage tarballs build system), we considered a serious bug: all downloads must be fulfilled by man (in preparation for repository transferred to the assembly) and not a robot. Such is the policy of our distribution. 2. In any case, I want to replace files autotool (autoconf, automake, libtool, etc.) distributive -- are with us are often patched. By virtue of paragraph 1 (source git and not the official tarball) in any case, I want to form the desired gnulib (=> run the bootstrap). It is logical to assign this work the robot (build system). Robot (build system) has no right to do git submodule update (Clause 2), but can generate tarball from a given commit and deploy it in the specified directory. If you transfer this directory through $ GNULIB_SRCDIR bootstrap works off without causing a git. (With me and you want: no need to put in git to build chroot.) If $GNULIB_SRCDIR used during assembly does not contain the right - build fails, you may need to correct his hands: build system should not perform such things as a git submodule update in Clause 2. Common sense patches bootstrap.conf - an exception to that requirement in the given conditions bootstrap does not actually use: git is not necessary when you specify $GNULIB_SRCDIR, but rsync is not necessary if you set $SKIP_PO. (If the original ootstrap.conf git and rsync always required.) I do not see that these patches can break. Important for me as functional, they add. PS: I changed the style (patch for autogen.sh threw out - he no longer needed). See the new patches. PPS: In my opinion this is an issue requiring correction. Personally, I think your point 1 is a bit sadistic (in my opinion, a distro should be focused on stability, not bleeding edge - at which point, why not go with the stable tarball as the starting point), but I am not in a position to tell you that you can't choose to inflict this extra pain upon yourself (this is free software, after all). So, given your point 1, that you would rather build from a git snapshot than from a release tarball, yes, you are now responsible for providing all the bits that would have been provided automatically by the release process - so you are now in the position of a developer, that must have all the development tools installed. But I agree with point 2, where it logically follows that if you have already externally downloaded all the bits in place, it should be possible to run ./bootstrap without having to download additional bits. However, I still don't see why you don't want to have git installed. For example, it is more than just ./bootstrap. In my opinion, any sane distro will distribute the project's ChangeLog as part of the documentation of that package; but libvirt's ChangeLog is a generated file via build-aux/gitlog-to-changelog. Are you accounting for that, and why not just have git installed to generate that changelog from your local git checkout of libvirt? A git snapshot alone will not provide a correct ChangeLog - you would need the entire libvirt.git history. Meanwhile, I think it would be possible to already have a copy of the upstream gnulib.git repository downloaded, then use some git config tricks, like adding this to your git config: [url "path/to/local/gnulib"] insteadof = git://git.sv.gnu.org/gnulib.git as well as having GNULIB_SRCDIR=path/to/local/gnulib in your environment, such that the action of 'git submodule init' will not cause any network traffic, even though git is still installed and run during ./bootstrap. At which point, we didn't have to make any modifications to bootstrap, so I'm still not convinced that we need to remove git from bootstrap.conf. And since it causes no network traffic, this would be no different than the fact that you require automake and autoconf to be installed, which are also prerequisites of ./bootstrap. Now, on to rsync. The latest bootstrap.conf _already_ includes SKIP_PO=true and omits a dependency on rsync, because we don't use the GNU translation project (according to git log, this was in 0.8.0). So your patches are out of date - you shouldn't have to do anything to cripple rsync, because there is nothing to cripple. A couple more points: If we end up agreeing that bootstrap should not require git under some circumstances, that decision must be based on something _other_ than GNULIB_SRCDIR. As a developer, I _always_ have GNULIB_SRCDIR set, even though I still expect git to query the network, because I rely on the reduction in download time and disk space used by having a local reference repository to pre-seed my download. In other words, GNULIB_SRCDIR is an optimization hint (as of 0.7.7), rather than an alternate upstream location; so to claim that you don't need git, you would want to be telling ./bootstrap that you have an alternate upstream location that exists with all the bits downloaded from the exact commit that you would otherwise be downloading from upstream gnulib.git. Also, ./bootstrap is maintained by gnulib. Maybe it would make sense asking bug-gnulib to add an upstream feature SKIP_GIT (comparable to SKIP_PO), as a way of removing the git dependency without relying on writing shell conditionals in ./bootstrap.conf. Comment on attachment 433628 [details]
0002-bootstrap.conf-Do-not-check-rsync-if-the-variable-is.patch
Obsolete with v0.8.0
Comment on attachment 433629 [details]
0003-README-hacking-If-you-do-not-need-to-download-po-fil.patch
Obsolete with v0.8.0
Created attachment 433717 [details] 0001-bootstrap.conf-Do-not-add-git-in-buildreq-if-the-var.patch bootstrap.conf: Do not add git in buildreq, if the variable GNULIB_SRCDIR is defined and dir $GNULIB_SRCDIR/.git exist. see http://git.altlinux.org/people/solo/packages/libvirt.git?p=libvirt.git;a=commit;h=bc91ac62006cb7dc33d88ff0333a4cb36b4518de Comment on attachment 433627 [details] 0001-bootstrap.conf-Do-not-add-git-in-buildreq-if-the-var.patch Obsolete, see http://git.altlinux.org/people/solo/packages/libvirt.git?p=libvirt.git;a=commit;h=bc91ac62006cb7dc33d88ff0333a4cb36b4518de I offered to bug-gnulib patch that adds a build-aux/bootstrap handling variable $NO_GIT and parameter --no-git (see http://lists.gnu.org/archive/html/bug-gnulib/2010-03/msg00229.html). Then it died down. I can propose it again, replacing the variable on $SKIP_GIT, and the option to --scip-git, but I doubt that this proposal will be accepted - all functionality is already there: 1. In comments to the parameter - gnulib-srcdir said that work without recourse to external resources is possible. Quote build-aux/bootstrap (./bootstrap): --gnulib-srcdir=DIRNAME Specify the local directory where gnulib sources reside. Use this if you already have gnulib sources on your machine, and do not want to waste your bandwidth downloading them again. Defaults to \$GNULIB_SRCDIR. 2. Quote build-aux/bootstrap (./bootstrap): for option do case $option in ... --gnulib-srcdir=*) GNULIB_SRCDIR=`expr "X$option" : 'X--gnulib-srcdir=\(.*\)'`;; ... # Get gnulib files. case ${GNULIB_SRCDIR--} in ... *) # Use GNULIB_SRCDIR as a reference. if test -d "$GNULIB_SRCDIR"/.git && \ git_modules_config submodule.gnulib.url >/dev/null; then echo "$0: getting gnulib files..." ... fi ;; esac gnulib_tool=$GNULIB_SRCDIR/gnulib-tool <$gnulib_tool || exit That is, if $GNULIB_SRCDIR determined and he did not git repository - git to update the contents of $GNULIB_SRCDIR not used and the variable itself does not change. In cases of libvirt this idyll is disturbed only by a direct indication git variable buildreq file ./bootstrap.conf. What I propose to fix their patch. PS: The condition does not add git clarified. I'd like to add bug-gnulib to the cc of this bug, but bugzilla won't allow that - so you'll have to repeat the key points of this bug to the upstream gnulib list as part of (re-)proposing your patch from March. You've made a few additional points in this bug that you didn't make back in March, so there is a good chance that you can still get an upstream patch to gnulib's ./bootstrap to support a --skip-git or --no-git option that is then smart enough to override any bootstrap.conf dependency on git. But I do insist that it be an explicit option (to make it clear to the person using a static checkout that the onus is on them, now that git is no longer able to use submodules to guarantee the correct checkout), and not just rely on whether $GNULIB_SRCDIR points to a git repository. Furthermore, I still insist that any changes should be in bootstrap, so that all projects sharing that file reap the benefits, rather than adding logic to bootstrap.conf which would have to be duplicated to each client project. For that matter, the code you quoted has a bug - a bare git repository does not have $GNULIB_SRCDIR/.git, but can (and should) still be usable as a submodule seed. If upstream gnulib accepts a patch that is sufficient for your needs, then this BZ serves as the justification for me to immediately port it to libvirt (by updating the gnulib submodule to an appropriate point). But right now, this bug is looking like it is conditional on upstream gnulib reaction (last time around, I was the only person who replied on the gnulib list, and your answers back then were not as detailed as what is in this bug report, so I didn't see any reason to take action back then). Patch to gnulib proposed, see http://lists.gnu.org/archive/html/bug-gnulib/2010-08/msg00047.html I guess I dragged my feet on this long enough that Benjamin Lindner stepped up and did it instead: http://lists.gnu.org/archive/html/bug-gnulib/2011-01/msg00284.html At any rate, here's the proposed libvirt patch that reflects that: https://www.redhat.com/archives/libvir-list/2011-January/msg00873.html Patch upstream now: http://libvirt.org/git/?p=libvirt.git;a=commit;h=125978fe3baba34959185f94c14a2a5e05c4a28c Sounds like bug can be closed? Please reopen if I'm wrong |