build operates differently if source is a git repo

[Dan Streetman]

[impact]

the 'configure' file checks for the existence of a '.git' directory, and if present it enables special build options that aren't used for 'normal' builds, some of which may cause the build to fail. This doesn't affect builds in PPAs or in the archive, since those do not happen in a git repo checkout, but this does cause very confusing failures and/or alternate behavior when building locally from a 'git ubuntu' checkout of the package.

[test case]

pull-lp-source qemu and build; assuming the build-deps are installed, it will build correctly.

git ubuntu clone qemu and build; it will likely fail to build even if build-deps are installed, and even if it does build, the result will be different than a non-git build.

[Where problems could occur]

A regression would likely cause a failed build, or incorrectly built package. We need to differ here for "packaging build" and "local (manual) build". For the "local (manual) build" the regression is net to zero as it currently does not work (or only with many many hurdles). The "packaging build" is the more interesting one for this category. Here the change should essentially turn out as no-op, but in case the most likely regression is that this turns out to "change" the build result in unexpected ways.

[scope]

this 'feature' comes from upstream, so this is needed in all releases.

[other info]

submitted patch upstream:
https://lists.nongnu.org/archive/html/qemu-trivial/2020-07/msg00180.html

and to Debian:
https://salsa.debian.org/qemu-team/qemu/-/merge_requests/14

this is related to bug 1887823, since building for bionic from a git repo fails due to that bug also.

[Christian Ehrhardt  (paelzer)]

As mentioned on IRC, I'm not opposed to a patch to avoid people trying to build the package falling into the trap. But that clearly is not Ubuntu-special nor urgent so I'd appreciate if you'd ask MJT with a PR to Debian directly.

[Dan Streetman (ddstreet)]

@mjt I'd really like to make progress on this; I've submitted upstream 3 times now with no response, and there's no response in the Debian PR:
https://salsa.debian.org/qemu-team/qemu/-/merge_requests/14

can you provide your opinion?

@paelzer even if upstream doesn't respond, and @mjt doesn't want this in Debian, we need to merge this in Ubuntu. I wasted hours figuring this out, and other members of my team have also wasted hours - days - because of this. I wonder how many community members have similarly wasted time due to this bug.

[Christian Ehrhardt  (paelzer)]

On Fri, Sep 25, 2020 at 6:16 PM Dan Streetman
<email address hidden> wrote:
>
> @mjt I'd really like to make progress on this; I've submitted upstream 3 times now with no response, and there's no response in the Debian PR:
> https://salsa.debian.org/qemu-team/qemu/-/merge_requests/14
>
> can you provide your opinion?
>
> @paelzer even if upstream doesn't respond, and @mjt doesn't want this in
> Debian, we need to merge this in Ubuntu. I wasted hours figuring this
> out, and other members of my team have also wasted hours - days -
> because of this. I wonder how many community members have similarly
> wasted time due to this bug.

I'm ok taking this change going forward even if not accepted upstream
as it isn't a functional but a packaging decision at this point.
I tagged the bug to pull it in on the next cycles merges at whatever
stage it is. Nevertheless we should continue the upstream discussion.

I reviewed and acked on salsa and I also pinged mjt, just in case he
has missed the MR.

On the qemu-trivial list I saw that Michael asked you to resend to
qemu-devel as it isn't "exactly trivial and it needs some review from
the developers".
And on that resend thread Daniel denied the patch as-is in:
  https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg07966.html
I think once you continue to drive that to a resolution this will get
upstream merged eventually and can be applied from there.

But I'd have two more things you won't like that much since the bug
has SRU-tasks:
1. The "time wasted" argument is hard and I know from other cases how
that feels, but the answer should be that one should never try to
build a package locally out of the dir anyway (and it isn't a problem
for sbuild, launchpad-builders, ...).
2. Due to #1 and for forcing millions of downloads to qemu users not
having a change I'm tempted to say that we should not consider SRUing
it IMHO.
Maybe we can bundle it with another upload that we'd need to do anyway.

[Dan Streetman (ddstreet)]

> And on that resend thread Daniel denied the patch as-is in:
> https://lists.nongnu.org/archive/html/qemu-devel/2020-09/msg07966.html

ah thanks, I hadn't made it down to that email yet; I just followed up and hopefully something can be worked out upstream. I asked for Michael and your opinion also, in case you have anything you want to suggest/add there.

Thanks!

> one should never try to build a package locally out of the dir anyway

using sbuild/pbuilder etc may be recommended, but many people (myself included) build locally with debuild and dpkg-buildpackage, especially during debugging/testing. I find sbuild/pbuilder excessively cumbersome and antiquated. But anyway, we don't need to devolve this bug into a build tool argument :)

> Maybe we can bundle it with another upload that we'd need to do anyway.

yes absolutely, I didn't mean we should release with just this fix, I just wanted to get it queued up to include in the next bugfix.

[Christian Ehrhardt  (paelzer)]

FYI - for Hirsute I guess I'll merge 1:5.2+dfsg-5 instead which contains:

commit ee3df39decb75d2c5e820870aec3df6bfec4a77d
Author: Dan Streetman <email address hidden>
Date: Fri Feb 12 04:43:13 2021 -0500

    Backport configure param --with-git-submodules and set to 'ignore'

And from there we can upload it along other changes (as discussed bug 1913421) that qualify for an SRU.

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu - 1:5.2+dfsg-6ubuntu2

---------------
qemu (1:5.2+dfsg-6ubuntu2) hirsute; urgency=medium

  * d/p/u/lp-1916705-disas-Fix-build-with-glib2.0-2.67.3.patch: Fix FTFBS due
    to glib2.0 >=2.67.3 (LP: #1916705)

qemu (1:5.2+dfsg-6ubuntu1) hirsute; urgency=medium

  * Merge with Debian unstable, includes fixes for
    - build operates differently if source is a git repo (LP: #1887535)
    Remaining changes:
    - qemu-kvm to systemd unit
      - d/qemu-kvm-init: script for QEMU KVM preparation modules, ksm,
        hugepages and architecture specifics
      - d/qemu-system-common.qemu-kvm.service: systemd unit to call
        qemu-kvm-init
      - d/qemu-system-common.install: install helper script
      - d/qemu-system-common.qemu-kvm.default: defaults for
        /etc/default/qemu-kvm
      - d/rules: call dh_installinit and dh_installsystemd for qemu-kvm
    - Distribution specific machine type (LP: 1304107 1621042)
      - d/p/ubuntu/define-ubuntu-machine-types.patch: distro machine types
      - d/qemu-system-x86.NEWS Info on fixed machine type definitions
        for host-phys-bits=true (LP: 1776189)
      - add an info about -hpb machine type in debian/qemu-system-x86.NEWS
      - provide pseries-bionic-2.11-sxxm type as convenience with all
        meltdown/spectre workarounds enabled by default. (LP: 1761372).
      - ubuntu-q35 alias added to auto-select the most recent q35 ubuntu type
    - Enable nesting by default
      - d/p/ubuntu/enable-svm-by-default.patch: Enable nested svm by default
        in qemu64 on amd
        [ No more strictly needed, but required for backward compatibility ]
    - improved dependencies
      - Make qemu-system-common depend on qemu-block-extra
      - Make qemu-utils depend on qemu-block-extra
      - let qemu-utils recommend sharutils
    - tolerate ipxe size change on migrations to >=18.04 (LP: 1713490)
      - d/p/ubuntu/pre-bionic-256k-ipxe-efi-roms.patch: old machine types
        reference 256k path
      - d/control-in: depend on ipxe-qemu-256k-compat-efi-roms to be able to
        handle incoming migrations from former releases.
    - d/control-in: Disable capstone disassembler library support (universe)
    - d/qemu-system-x86.README.Debian: add info about updated nesting changes
    - d/control*, d/rules: disable xen by default, but provide universe
      package qemu-system-x86-xen as alternative
      [includes compat links changes of 5.0-5ubuntu4]
    - allow qemu to load old modules post upgrade (LP 1847361)
      - Drop d/qemu-block-extra.*.in, d/qemu-system-gui.*.in
      - d/rules: Drop generating package version into maintainer scripts
      - d/qemu-system-gui.prerm: add no-op prerm to overcome upgrade issues on
        the bad old prerm (LP 1906245 1905377)
    - d/p/ubuntu/lp-1907789-build-no-pie-is-no-functional-liker-flag.patch: fix
      ld usage of -no-pie (LP 1907789)
  * Added changes
    - d/p/u/lp-1916230-hw-s390x-fix-build-for-virtio-9p-ccw.patch: fix
      virtio-9p-ccw being missing (LP: #1916230)

qemu (1:5.2+dfsg-6) unstable; urgency=medium

  * deprecate qemu-debootstrap. It is not needed anymore with
    binfmt F flag, since eve...

[Christian Ehrhardt  (paelzer)]

As usual for changes that are "good, but not worth an upload on their own" these fixes for Focal&Groovy will be tucked onto an SRU that is happening anyway. Preparing that atm.

[Christian Ehrhardt  (paelzer)]

FYI - uploaded to the -unapproved queue yesterday. Now on the SRU team to evaluate.

[Robie Basak (racb)]

Hello Dan, or anyone else affected,

Accepted qemu into groovy-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:5.0-5ubuntu9.7 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-groovy to verification-done-groovy. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-groovy. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Robie Basak (racb)]

Hello Dan, or anyone else affected,

Accepted qemu into focal-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/qemu/1:4.2-3ubuntu6.15 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-focal to verification-done-focal. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-focal. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Christian Ehrhardt  (paelzer)]

Two entry points:
a)
$ pull-lp-source qemu focal; cd qemu-4.2
b)
$ git ubuntu clone qemu; cd qemu; git checkout pkg/ubuntu/focal-proposed; dquilt push -a --fuzz=0

And the very same but for groovy.
Running a local build like ./debian/rules build and checking the behavior of submodules.

pull:
Source path /tmp/qemu-4.2
GIT binary git
GIT submodules
C compiler cc
...
sh /tmp/qemu-4.2/scripts/create_config < config-host.mak > config-host.h-timestamp
(GIT="git" "/tmp/qemu-4.2/scripts/git-submodule.sh" ignore )

git:
...
_FORTIFY_SOURCE=2" --extra-ldflags="-Wl,-Bsymbolic-functions -Wl,-z,relro -Wl,--as-needed" --prefix=/usr --sysconfdir=/etc --libdir=/usr/lib/x86_64-linux-gnu --libexecdir=/usr/lib/qemu --firmwarepath=/usr/share/qemu:/usr/share/seabios:/usr/lib/ipxe/qemu --localstatedir=/var --disable-blobs --disable-strip --interp-prefix=/etc/qemu-binfmt/%M --localstatedir=/var --with-git-submodules=ignore \
...
GIT binary git
GIT submodules ui/keycodemapdb tests/fp/berkeley-testfloat-3 tests/fp/berkeley-softfloat-3

Cross verify, without the patch it fails with the new d/rules:
ERROR: unknown option --with-git-submodules=ignore
Try '../../configure --help' for more information

Things are in place as planned and the PKG build in launchpad and such still works fine.
Maybe my testcase is moot :-/
@DDstreet I still see the submodules listed in the configure output, since it is your upstream change and your initial testcase could you have a look at this for SRU verification?

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted qemu (1:4.2-3ubuntu6.15) for focal have finished running.
The following regressions have been reported in tests triggered by the package:

casper/1.445.1 (amd64, ppc64el)
systemd/245.4-4ubuntu3.6 (amd64)
ubuntu-image/1.11+20.04ubuntu1 (armhf, amd64, s390x)
livecd-rootfs/2.664.19 (ppc64el)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/focal/update_excuses.html#qemu

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Ubuntu SRU Bot (ubuntu-sru-bot)]

All autopkgtests for the newly accepted qemu (1:5.0-5ubuntu9.7) for groovy have finished running.
The following regressions have been reported in tests triggered by the package:

systemd/246.6-1ubuntu1.3 (ppc64el)
cloud-utils/0.31-29-ge0792e3d-0ubuntu1 (s390x)
open-iscsi/2.1.1-1ubuntu2 (amd64)
ubuntu-image/1.11+20.10ubuntu1 (armhf)

Please visit the excuses page listed below and investigate the failures, proceeding afterwards as per the StableReleaseUpdates policy regarding autopkgtest regressions [1].

https://people.canonical.com/~ubuntu-archive/proposed-migration/groovy/update_excuses.html#qemu

[1] https://wiki.ubuntu.com/StableReleaseUpdates#Autopkgtest_Regressions

Thank you!

[Dan Streetman (ddstreet)]

for verifications:

checking the source build from a git checkout, which is where the problem was, I verified i could reproduce the issue with the -updates source versions:

$ ls -ld .git
drwxrwxr-x 7 ubuntu ubuntu 4096 Apr 7 19:06 .git
$ dpkg-parsechangelog -S Version
1:4.2-3ubuntu6.14
$ dpkg-buildpackage
...

after the 'configure' step has finished, while the build is running (or after it completes), check the build dir for the setting of GIT_UPDATE
$ grep -r GIT_UPDATE b
b/s390xfw/config-host.mak:GIT_UPDATE=yes
b/qemu-xen/config-host.mak:GIT_UPDATE=yes
b/qemu-microvm/config-host.mak:GIT_UPDATE=yes
b/qemu/config-host.mak:GIT_UPDATE=yes
b/user-static/config-host.mak:GIT_UPDATE=yes

since we *do not* want the build to try to automatically update the git submodules, this setting is wrong.

$ ls -ld .git
drwxrwxr-x 7 ubuntu ubuntu 4096 Apr 7 19:33 .git
$ dpkg-parsechangelog -S Version
1:4.2-3ubuntu6.15
$ dpkg-buildpackage
...

for the -proposed version, GIT_UPDATES has been replaced with GIT_SUBMODULES_ACTION, which should be set to 'ignore'
$ grep -r GIT_UPDATE b
$ grep -r GIT_SUBMODULES_ACTION b
b/s390xfw/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu-xen/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu-microvm/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/user-static/config-host.mak:GIT_SUBMODULES_ACTION=ignore

for the source build from a non-git checkout, this has not been a problem, so verification only requires checking there are no regressions:

$ dpkg-source -x qemu_4.2-3ubuntu6.14.dsc
...
$ cd qemu-4.2/
$ ls -ld .git
ls: cannot access '.git': No such file or directory
$ dpkg-parsechangelog -S Version
1:4.2-3ubuntu6.14
$ dpkg-buildpackage
...

$ grep -r GIT_UPDATE b
b/s390xfw/config-host.mak:GIT_UPDATE=no
b/qemu-xen/config-host.mak:GIT_UPDATE=no
b/qemu-microvm/config-host.mak:GIT_UPDATE=no
b/qemu/config-host.mak:GIT_UPDATE=no
b/user-static/config-host.mak:GIT_UPDATE=no
$ grep -r GIT_SUBMODULES_ACTION b
$

$ dpkg-source -x qemu_4.2-3ubuntu6.15.dsc
...
$ cd qemu-4.2/
$ ls -ld .git
ls: cannot access '.git': No such file or directory
$ dpkg-parsechangelog -S Version
1:4.2-3ubuntu6.15
$ dpkg-buildpackage
...

$ grep -r GIT_UPDATE b
$ grep -r GIT_SUBMODULES_ACTION b
b/s390xfw/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu-xen/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu-microvm/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/user-static/config-host.mak:GIT_SUBMODULES_ACTION=ignore

additionally, the build is successful with the version from -proposed for both a non-git source and git source.

[Dan Streetman (ddstreet)]

verification for groovy:

for git source:

$ ls -ld .git
drwxrwxr-x 7 ubuntu ubuntu 4096 Apr 7 19:58 .git
$ dpkg-parsechangelog -S Version
1:5.0-5ubuntu9.6
$ dpkg-buildpackage
...

$ grep -r GIT_UPDATE b
b/qemu-xen/config-host.mak:GIT_UPDATE=yes
b/qemu-microvm/config-host.mak:GIT_UPDATE=yes
b/qemu/config-host.mak:GIT_UPDATE=yes
b/user-static/config-host.mak:GIT_UPDATE=yes
$ grep -r GIT_SUBMODULES_ACTION b
$

$ ls -ld .git
drwxrwxr-x 7 ubuntu ubuntu 4096 Apr 7 20:02 .git
$ dpkg-parsechangelog -S Version
1:5.0-5ubuntu9.7
$ dpkg-buildpackage
...

$ grep -r GIT_UPDATE b
$ grep -r GIT_SUBMODULES_ACTION b
b/qemu-xen/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu-microvm/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/user-static/config-host.mak:GIT_SUBMODULES_ACTION=ignore

for non-git source:

$ dpkg-source -x qemu_5.0-5ubuntu9.6.dsc
...
$ cd qemu-5.0/
$ ls -ld .git
ls: cannot access '.git': No such file or directory
$ dpkg-parsechangelog -S Version
1:5.0-5ubuntu9.6
$ dpkg-buildpackage
...

$ grep -r GIT_UPDATE b
b/qemu-xen/config-host.mak:GIT_UPDATE=no
b/qemu-microvm/config-host.mak:GIT_UPDATE=no
b/qemu/config-host.mak:GIT_UPDATE=no
b/user-static/config-host.mak:GIT_UPDATE=no
$ grep -r GIT_SUBMODULES_ACTION b
$

$ dpkg-source -x qemu_5.0-5ubuntu9.7.dsc
...
$ cd qemu-5.0/
$ ls -ld .git
ls: cannot access '.git': No such file or directory
$ dpkg-parsechangelog -S Version
1:5.0-5ubuntu9.7
$ dpkg-buildpackage
...

$ grep -r GIT_UPDATE b
$ grep -r GIT_SUBMODULES_ACTION b
b/qemu-xen/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu-microvm/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/qemu/config-host.mak:GIT_SUBMODULES_ACTION=ignore
b/user-static/config-host.mak:GIT_SUBMODULES_ACTION=ignore

[Christian Ehrhardt  (paelzer)]

Ah now I see which way around you run your build, makes more sense now :-)
Thanks for the verifications!

[Christian Ehrhardt  (paelzer)]

FYI - autopkgtest issues resolved as well now (as assumed it was due to flaky tests)

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu - 1:5.0-5ubuntu9.7

---------------
qemu (1:5.0-5ubuntu9.7) groovy; urgency=medium

  * d/p/u/lp-1921468-*: fix issues handling boot menu index on s390x
    (LP: #1921468)
  * d/p/u/lp-1887535-configure-replace-enable-disable-git-update-with-wit.patch,
    d/rules: Backport --with-git-submodules param so building from git repo
    doesn't fail (LP: #1887535)
  * Fix byte aligned writes when writing to image stored on NFS
    server, as they aren't required to be 4kib aligned. (LP: #1921665)
    - d/p/u/lp-1921665-1-block-Require-aligned-image-size-to-avoid-assert.patch
    - d/p/u/lp-1921665-2-file-posix-Allow-byte-aligned-O_DIRECT-with-NFS.patch

 -- Christian Ehrhardt <email address hidden> Fri, 26 Mar 2021 10:36:31 +0100

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for qemu has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package qemu - 1:4.2-3ubuntu6.15

---------------
qemu (1:4.2-3ubuntu6.15) focal; urgency=medium

  * d/p/u/lp-1921468-*: fix issues handling boot menu index on s390x
    (LP: #1921468)
  * d/p/u/lp-1887535-configure-replace-enable-disable-git-update-with-wit.patch,
    d/rules: Backport --with-git-submodules param so building from git repo
    doesn't fail (LP: #1887535)
  * Fix byte aligned writes when writing to image stored on NFS
    server, as they aren't required to be 4kib aligned. (LP: #1921665)
    - d/p/u/lp-1921665-1-block-Require-aligned-image-size-to-avoid-assert.patch
    - d/p/u/lp-1921665-2-file-posix-Allow-byte-aligned-O_DIRECT-with-NFS.patch

 -- Christian Ehrhardt <email address hidden> Fri, 26 Mar 2021 10:38:47 +0100