Quoting Michael Roth (2015-09-07 14:55:47) > Quoting Paolo Bonzini (2015-09-07 05:48:27) > > > > > > On 28/08/2015 01:55, Michael Roth wrote: > > > Now we can build qemu-ga MSI package with: > > > ./configure ... > > > make qemu-ga.exe > > > make msi > > > > > > or simply: > > > ./configure ... > > > make msi > > > > Shouldn't the latter have always worked? > > Hmm, at the time of the patch I'm not sure, since out-of-tree builds > were broken with `make msi` until the recent: > > decdfbd qemu-ga: Fixed paths issue with MSI build > > With that patch in place I noticed out-of-tree builds were still > broken: > > [mdroth@vm4 qemu-build-w64]$ ../w/qemu4.git/configure --enable-guest-agent > --target-list=x86_64-softmmu --extra-cflags=-Wall --enable-guest-agent-msi > --cross-prefix=x86_64-w64-mingw32- --with-vss-sdk=/home/mdroth/w/vss-win32/ > && make msi > ... > AR libqemustub.a > LINK qemu-ga.exe > CXX qga/vss-win32/requester.o > ... > CXX qga/vss-win32/provider.o > CXX qga/vss-win32/install.o > LINK qga/vss-win32/qga-vss.dll > WIXL qemu-ga-x86_64.msi > Couldn't find file /home/mdroth/qemu-build-w64/qga/vss-win32/qga-vss.tlb > make: *** [qemu-ga-x86_64.msi] Error 1 > > For out-of-tree, qga-vss.tlb dependency gets met by QEMU tools target, which > is what motivated this patch. > > But for in-tree builds, qga-vss.tlb is already present in working directory, > so it might have actually worked for that case. > > So I may have misdiagnosed the root issue here: that *.tlb (not just > *.dll) needed to be added to MSI dependency list instead of being > assumed (via in-tree build or full qemu build with tools). > > > > > I think that if someone does "make qemu-ga.exe" they should *not* get > > the VSS files. Perhaps we can add a Win32-specific phony qemu-ga target > > to build both qemu-ga.exe and the VSS files, but this patch's use of > > filter-out is a bit ugly. > > I think it might make sense to re-de-couple VSS from qemu-ga.exe, but > I'm not sure I like the idea of making MSI target responsible for > VSS files. MSI is relatively new, but VSS support has been around for a > while when documentated install procedure for qemu-ga.exe was manually > copying files. MSI isn't the source of the issue, presumably everybody > distributing qemu-ga.exe in this manner was relying on the full build > to get the VSS files. But moving VSS completely to MSI means that use > case would break (whereas moving them to qemu-ga.exe would still work, > since qemu-ga.exe is build as part of default/full build) > > I think the ideal solution is too keep things as they are with this > patch (previous working methods supported), but move to a new 'qemu-ga' > build target that just does the right thing on each platform: > > on posix: > 1) build 'qemu-ga' executable > > on mingw: > 1) build 'qemu-ga.exe' executable (no need to build .exe directly) > 2) build VSS if VSS supported/requested > 3) build MSI package if supported/requested (and include VSS files > if VSS supported) > > That would let us drop the wierd filtering, and bring the w32 build > process more inline with posix. > > I'm not even sure if that's possible atm, but if that's reasonable I can > look into it as a follow-up (this series is merged already).
diff --git a/Makefile b/Makefile index 9ce3972..d0ee41e 100644 --- a/Makefile +++ b/Makefile @@ -293,15 +293,15 @@ $(qga-obj-y) qemu-ga.o: $(QGALIB_GEN) # we require QGA_VSS_PROVIDER files to be built alongside qemu-ga # executable since they are shipped together, but we don't want to actually # link against them -qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a $(QGA_VSS_PROVIDER) - $(call LINK, $(filter-out $(QGA_VSS_PROVIDER), $^)) +qemu-ga$(EXESUF): $(qga-obj-y) libqemuutil.a libqemustub.a + $(call LINK, $^) ifdef QEMU_GA_MSI_ENABLED QEMU_GA_MSI=qemu-ga-$(ARCH).msi msi: $(QEMU_GA_MSI) -$(QEMU_GA_MSI): qemu-ga.exe +$(QEMU_GA_MSI): qemu-ga.exe $(QGA_VSS_PROVIDER) $(QEMU_GA_MSI): config-host.mak @@ -313,6 +313,8 @@ msi: @echo "MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)" endif +qemu-ga: qemu-ga$(EXESUF) $(QGA_VSS_PROVIDER) clean: # avoid old build problems by removing potentially incorrect old files rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h This kinda does it, although it introduces a circular dependency warning on posix (since qemu-ga$(EXESUF) == qemu-ga there, yet qemu-ga has a qemu-ga$(EXESUF) dependency. Could be fixed by making a new 'qemu-guest-agent' do-the-right-thing target, but that means new build process for posix and w32 instead of just w32. The alternative is to only define the 'qemu-ga' do-the-right-thing target for w32 (since posix already has such a target). That requires a Makefile ifdef MINGW or somesuch though, which isn't ideal. I think the latter is worth the simplified build process (same as we currently do on posix, but with some extra config params) > > > > > Paolo > > > >