Re: [Qemu-devel] [PATCH] build: Cleanup qga make output

[Luiz Capitulino]

On Tue, 13 Dec 2011 09:22:39 -0600
Michael Roth <address@hidden> wrote:

> On 12/13/2011 08:33 AM, Adam Litke wrote:
> > On Tue, Dec 13, 2011 at 10:23:18AM -0200, Luiz Capitulino wrote:
> >> On Mon, 12 Dec 2011 17:38:36 -0600
> >> Michael Roth<address@hidden>  wrote:
> >>
> >>> On 12/12/2011 05:03 PM, Anthony Liguori wrote:
> >>>> On 12/07/2011 10:33 AM, Adam Litke wrote:
> >>>>> Currently the make variable qapi-dir refers to the qapi-generated
> >>>>> directory in
> >>>>> absolute terms. This causes the harmless but ugly make output below. By
> >>>>> changing this variable to the relative path the output conforms to the
> >>>>> norm and
> >>>>> the build works fine.
> >>>>>
> >>>>> Before patch:
> >>>>> CC /home/aglitke/src/qemu/qapi-generated/qga-qapi-types.o
> >>>>> CC /home/aglitke/src/qemu/qapi-generated/qga-qapi-visit.o
> >>>>> CC /home/aglitke/src/qemu/qapi-generated/qga-qmp-marshal.o
> >>>>> After patch:
> >>>>> CC qapi-generated/qga-qapi-types.o
> >>>>> CC qapi-generated/qga-qapi-visit.o
> >>>>> CC qapi-generated/qga-qmp-marshal.o
> >>>>
> >>>> This was supposedly to fix a build issue that I was never able to
> >>>> reproduce. I think Luiz could reproduce it though. Luiz, could you try
> >>>> out Adam's patch and confirm it breaks for you?
> >>
> >> It doesn't :( I also tried to reproduce again with -rc4 (which was the
> >> version I was getting it) but it doesn't happen anymore.
> >>
> >>> I think that was Stefano:
> >>>
> >>> http://lists.gnu.org/archive/html/qemu-devel/2011-11/msg02752.html
> >>>
> >>> Stefano had a patch that fixed a build breakage he saw with dirty
> >>> directories by setting:
> >>>
> >>> qapi-dir := $(SRC_DIR)/qapi-generated
> >>>
> >>> That patch ended up breaking the build for others, however, and was
> >>> reverted.
> >>>
> >>> This patch was an improvement on the original, but we'd all agreed that
> >>> it wasn't necessary since we don't support working around issues related
> >>> to dirty directories.
> >
> > I am lacking some context here.  I am confused how a dirty directory could
> > change the output in this way.
> 
> I guess I'm lacking context as well, I wasn't aware that Luiz had also 
> hit an issue related to this patch, was just going off what I'd read in 
> the thread where I posted this patch...

Well, it happened a few days before the final release. While I was discussing
it with Anthony over IRC I was reading qemu-devel and I saw your patches. I
guessed they would fix the issue and they did, so Anthony decided to apply them.

> In Stefano's case, he'd noticed his build issue went away if
> 
> a) this (rather, Stefano's original) patch was applied, or
> b) if he did a `make distclean` in his source directory beforehand

I don't exactly remember if I did 'make distclean'. I probably did, but
what I did do was to remove my build dir and build again.

> So that's why we assumed dirty directories were the problem.
> 
> I'm not sure of the circumstances behind the breakage Luiz saw, so I'd 
> err on the side of caution and keep this patch until that's clarified.
> 
> >
> >> What kind of issue? I'm 100% certain that my tree was clean, the only
> >> thing I did not do was to clone it again.
> >>
> >> We have had races when building the qapi in the past, maybe that's the
> >> problem and commit 9b12940 makes it less likely.
> >>
> >> We could revert it and wait for the problem to happen again...
> >
> > I would be fine with doing whatever is least likely to cause anyone else
> > problems.  I wouldn't want to break the actual build just to make the output
> > look nice.
> >
> >>
> >>>
> >>> So if it's ugly, we can safely drop it.
> >>>
> >>>>
> >>>> Regards,
> >>>>
> >>>> Anthony Liguori
> >>>>
> >>>>>
> >>>>> Signed-off-by: Adam Litke<address@hidden>
> >>>>> ---
> >>>>> Makefile | 2 +-
> >>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
> >>>>>
> >>>>> diff --git a/Makefile b/Makefile
> >>>>> index 301c75e..7c93739 100644
> >>>>> --- a/Makefile
> >>>>> +++ b/Makefile
> >>>>> @@ -168,7 +168,7 @@ check-qjson: check-qjson.o $(qobject-obj-y)
> >>>>> $(tools-obj-y)
> >>>>> test-coroutine: test-coroutine.o qemu-timer-common.o async.o
> >>>>> $(coroutine-obj-y) $(tools-obj-y)
> >>>>>
> >>>>> $(qapi-obj-y): $(GENERATED_HEADERS)
> >>>>> -qapi-dir := $(BUILD_DIR)/qapi-generated
> >>>>> +qapi-dir := qapi-generated
> >>>>> test-visitor.o test-qmp-commands.o qemu-ga$(EXESUF): QEMU_CFLAGS += -I
> >>>>> $(qapi-dir)
> >>>>> qemu-ga$(EXESUF): LIBS = $(LIBS_QGA)
> >>>>>
> >>>>
> >>>>
> >>>
> >>
> >
>