[Qemu-devel] [PATCH 0/2] Minor makefile fixes

[Qemu-devel] [PATCH 0/2] Minor makefile fixes

[Alexandre Raymond]

Two simple patches for "make distclean"


Alexandre Raymond (2):
  Makefile: distclean should clean all possible targets
  Makefile: delete config.log in distclean

 Makefile  |    3 ++-
 configure |    1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

-- 
1.7.5

[Qemu-devel] [PATCH 1/2] Makefile: distclean should clean all possible targets

[Alexandre Raymond]

At the moment, "make distclean" relies on the TARGET_DIRS variable, set by
configure. The problem is that this variable does not always contain all
possible targets.

For example, the following will leave build data in the tree:

./configure && make && ./configure --target-list=i386-softmmu \
&& make distclean

as it will only remove the i386-softmmu build directory, although the
first build created additional directories.

Solution : pass the full list of targets from configure to make via
the DEFAULT_TARGET_LIST variable.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 Makefile  |    2 +-
 configure |    1 +
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/Makefile b/Makefile
index 723d41f..878402e 100644
--- a/Makefile
+++ b/Makefile
@@ -225,7 +225,7 @@ distclean: clean
 	rm -f qemu-doc.log qemu-doc.pdf qemu-doc.pg qemu-doc.toc qemu-doc.tp
 	rm -f qemu-doc.vr
 	rm -f qemu-tech.info qemu-tech.aux qemu-tech.cp qemu-tech.dvi qemu-tech.fn qemu-tech.info qemu-tech.ky qemu-tech.log qemu-tech.pdf qemu-tech.pg qemu-tech.toc qemu-tech.tp qemu-tech.vr
-	for d in $(TARGET_DIRS) $(QEMULIBS); do \
+	for d in $(DEFAULT_TARGET_LIST) $(QEMULIBS); do \
 	rm -rf $$d || exit 1 ; \
         done
 
diff --git a/configure b/configure
index 600da9b..83f980a 100755
--- a/configure
+++ b/configure
@@ -2810,6 +2810,7 @@ qemu_version=`head $source_path/VERSION`
 echo "VERSION=$qemu_version" >>$config_host_mak
 echo "PKGVERSION=$pkgversion" >>$config_host_mak
 echo "SRC_PATH=$source_path" >> $config_host_mak
+echo "DEFAULT_TARGET_LIST=$default_target_list" >> $config_host_mak
 echo "TARGET_DIRS=$target_list" >> $config_host_mak
 if [ "$docs" = "yes" ] ; then
   echo "BUILD_DOCS=yes" >> $config_host_mak
-- 
1.7.5

[Qemu-devel] [PATCH 2/2] Makefile: delete config.log in distclean

[Alexandre Raymond]

Distclean should remove anything created by the configure script.

Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
---
 Makefile |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Makefile b/Makefile
index 878402e..9f65d86 100644
--- a/Makefile
+++ b/Makefile
@@ -224,6 +224,7 @@ distclean: clean
 	rm -f qemu-doc.fn qemu-doc.fns qemu-doc.info qemu-doc.ky qemu-doc.kys
 	rm -f qemu-doc.log qemu-doc.pdf qemu-doc.pg qemu-doc.toc qemu-doc.tp
 	rm -f qemu-doc.vr
+	rm -f config.log
 	rm -f qemu-tech.info qemu-tech.aux qemu-tech.cp qemu-tech.dvi qemu-tech.fn qemu-tech.info qemu-tech.ky qemu-tech.log qemu-tech.pdf qemu-tech.pg qemu-tech.toc qemu-tech.tp qemu-tech.vr
 	for d in $(DEFAULT_TARGET_LIST) $(QEMULIBS); do \
 	rm -rf $$d || exit 1 ; \
-- 
1.7.5

Re: [Qemu-devel] [PATCH 1/2] Makefile: distclean should clean all possible targets

[Markus Armbruster]

Alexandre Raymond <cerbere@gmail.com> writes:

> At the moment, "make distclean" relies on the TARGET_DIRS variable, set by
> configure. The problem is that this variable does not always contain all
> possible targets.
>
> For example, the following will leave build data in the tree:
>
> ./configure && make && ./configure --target-list=i386-softmmu \
> && make distclean
>
> as it will only remove the i386-softmmu build directory, although the
> first build created additional directories.

Why is that a problem?

> Solution : pass the full list of targets from configure to make via
> the DEFAULT_TARGET_LIST variable.

Well, I'd expect distclean to remove exactly what *this* makefile can
build, and leave everything else alone.

Your patch adds a special case to that simple rule: also remove
not-configured target directories.  Other not-configured stuff is still
left behind.

Special cases need special justification, hence my question above.

Re: [Qemu-devel] [PATCH 1/2] Makefile: distclean should clean all possible targets

[Alexandre Raymond]

Hi Markus,

> Well, I'd expect distclean to remove exactly what *this* makefile can
> build, and leave everything else alone.

I was expecting "distclean" to bring back the source directory to
however it was after checkout, removing anything that might have been
created by the build/configure process.


> Your patch adds a special case to that simple rule: also remove
> not-configured target directories.  Other not-configured stuff is still
> left behind.
>
> Special cases need special justification, hence my question above.

This patch stems from the discussion in "Makefile: fix out-of-tree builds"
http://lists.gnu.org/archive/html/qemu-devel/2011-07/msg02707.html

What happens is that SRC_PATH is set in the VPATH variable in the
Makefile, which causes it to search for files inside the source
directory (and outside the current build directory).

Say you're building out-of-tree, and the Makefile happens to pick up
old files from your main source directory, it can lead to errors in
the build. If you are "unable" to delete old build data because you've
run a more restricted "configure" after an earlier build, this earlier
build data will not go away if you distclean.

For example:
---8<---
SUBDIR_DEVICES_MAK=$(patsubst %, %/config-devices.mak, $(TARGET_DIRS))
config-all-devices.mak: $(SUBDIR_DEVICES_MAK)
    $(call quiet-command,cat $(SUBDIR_DEVICES_MAK) | grep =y | sort -u
> $@,"  GEN   $@")

...

%/config-devices.mak: default-configs/%.mak
...
---8<---

Let's say my sources are in ~/src/qemu/ and I build in /tmp/foo/.

Those rules can match i386-softmmu/config-devices.mak inside
~/src/qemu/ from a previous build if it doesn't exist in the current
build directory. Since this file is up to date wrt
~/src/qemu/default-configs/i386-softmu.mak, it won't be rebuilt in the
build directory. Then, when the recipe for config-all-devices.mak is
run, it will try to cat i386-softmmu/config-devices.mak, but "cat"
doesn't know about VPATH, so it will fail.

Alexandre


PS: if you're interested, here is the trace of this problem :

 Considering target file `config-all-devices.mak'.
  File `config-all-devices.mak' does not exist.
   Considering target file `i386-softmmu/config-devices.mak'.
    Looking for an implicit rule for `i386-softmmu/config-devices.mak'.
    Trying pattern rule with stem `i386-softmmu'.
    Trying implicit prerequisite `default-configs/i386-softmmu.mak'.
    Found prerequisite `default-configs/i386-softmmu.mak' as VPATH
`/Users/myuser/src/qemu/default-configs/i386-softmmu.mak'
    Found an implicit rule for `i386-softmmu/config-devices.mak'.
     Considering target file `default-configs/i386-softmmu.mak'.
      Looking for an implicit rule for `default-configs/i386-softmmu.mak'.
      No implicit rule found for `default-configs/i386-softmmu.mak'.
      Finished prerequisites of target file `default-configs/i386-softmmu.mak'.
     No need to remake target `default-configs/i386-softmmu.mak';
using VPATH name
`/Users/myuser/src/qemu/default-configs/i386-softmmu.mak'.
    Finished prerequisites of target file `i386-softmmu/config-devices.mak'.
    Prerequisite
`/Users/myuser/src/qemu/default-configs/i386-softmmu.mak' is older
than target `i386-softmmu/config-devices.mak'.
   No need to remake target `i386-softmmu/config-devices.mak'; using
VPATH name `/Users/myuser/src/qemu/i386-softmmu/config-devices.mak'.
<---- ***this here is the problem***
  Finished prerequisites of target file `config-all-devices.mak'.
 Must remake target `config-all-devices.mak'.
Putting child 0x001278c0 (config-all-devices.mak) PID 1699 on the chain.
Live child 0x001278c0 (config-all-devices.mak) PID 1699
  GEN   config-all-devices.mak
cat: i386-softmmu/config-devices.mak: No such file or directory
<---- ***this is the error***

Re: [Qemu-devel] [PATCH 1/2] Makefile: distclean should clean all possible targets

[Markus Armbruster]

Alexandre Raymond <cerbere@gmail.com> writes:

> Hi Markus,
>
>> Well, I'd expect distclean to remove exactly what *this* makefile can
>> build, and leave everything else alone.
>
> I was expecting "distclean" to bring back the source directory to
> however it was after checkout, removing anything that might have been
> created by the build/configure process.
>
>
>> Your patch adds a special case to that simple rule: also remove
>> not-configured target directories.  Other not-configured stuff is still
>> left behind.
>>
>> Special cases need special justification, hence my question above.
>
> This patch stems from the discussion in "Makefile: fix out-of-tree builds"
> http://lists.gnu.org/archive/html/qemu-devel/2011-07/msg02707.html
>
> What happens is that SRC_PATH is set in the VPATH variable in the
> Makefile, which causes it to search for files inside the source
> directory (and outside the current build directory).
>
> Say you're building out-of-tree, and the Makefile happens to pick up
> old files from your main source directory, it can lead to errors in
> the build. If you are "unable" to delete old build data because you've
> run a more restricted "configure" after an earlier build, this earlier
> build data will not go away if you distclean.
>
> For example:
[...]

There are many more object files that are built conditionally.  Why is
it okay not to delete them?

If you unwisely messed up your source tree by building in it, a simple
and reliable way out is to git-clone yourself a new one.  Or if you
insist on recovering in-place, remove files outside .git that aren't in
git.

Re: [Qemu-devel] [PATCH 1/2] Makefile: distclean should clean all possible targets

[Alexandre Raymond]

> There are many more object files that are built conditionally.  Why is
> it okay not to delete them?

Perhaps they should be deleted too...

The GNU Make manual says the following about "distclean":
http://www.gnu.org/s/hello/manual/make/Standard-Targets.html

"Delete all files in the current directory (or created by this
makefile) that are created by configuring or building the program. If
you have unpacked the source and built the program without creating
any other files, ‘make distclean’ should leave only the files that
were in the distribution. However, there is no need to delete parent
directories that were created with ‘mkdir -p’, since they could have
existed anyway. "

Now, if everyone agrees that "distclean" is fine as it is, I won't
insist on anything.


>
> If you unwisely messed up your source tree by building in it, a simple
> and reliable way out is to git-clone yourself a new one.  Or if you
> insist on recovering in-place, remove files outside .git that aren't in
> git.
>

Indeed.

Alexandre

Re: [Qemu-devel] [Qemu-trivial] [PATCH 1/2] Makefile: distclean should clean all possible targets

[Michael Roth]

On 07/27/2011 08:55 AM, Alexandre Raymond wrote:
>> There are many more object files that are built conditionally.  Why is
>> it okay not to delete them?
>
> Perhaps they should be deleted too...
>
> The GNU Make manual says the following about "distclean":
> http://www.gnu.org/s/hello/manual/make/Standard-Targets.html
>
> "Delete all files in the current directory (or created by this
> makefile) that are created by configuring or building the program. If
> you have unpacked the source and built the program without creating
> any other files, ‘make distclean’ should leave only the files that
> were in the distribution. However, there is no need to delete parent
> directories that were created with ‘mkdir -p’, since they could have
> existed anyway. "
>
> Now, if everyone agrees that "distclean" is fine as it is, I won't
> insist on anything.
>
>

I'm with you in that distclean to me reads as "make clean for 
re-distribution". i.e. a pristine source tree.

But I do agree that if we want to implement it in that fashion there 
would be a bit more work to do.

>>
>> If you unwisely messed up your source tree by building in it, a simple
>> and reliable way out is to git-clone yourself a new one.  Or if you
>> insist on recovering in-place, remove files outside .git that aren't in
>> git.
>>
>
> Indeed.
>
> Alexandre
>

Re: [Qemu-devel] [PATCH 2/2] Makefile: delete config.log in distclean

[Stefan Hajnoczi]

On Mon, Jul 25, 2011 at 11:56:02PM -0400, Alexandre Raymond wrote:
> Distclean should remove anything created by the configure script.
> 
> Signed-off-by: Alexandre Raymond <cerbere@gmail.com>
> ---
>  Makefile |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Thanks, applied to the trivial patches tree:
http://repo.or.cz/w/qemu/stefanha.git/shortlog/refs/heads/trivial-patches-next

Stefan

Re: [Qemu-devel] [Qemu-trivial] [PATCH 1/2] Makefile: distclean should clean all possible targets

[Paolo Bonzini]

On 07/27/2011 04:42 PM, Michael Roth wrote:
>> "Delete all files in the current directory (or created by this
>> makefile) that are created by configuring or building the program. If
>> you have unpacked the source and built the program without creating
>> any other files, ‘make distclean’ should leave only the files that
>> were in the distribution. However, there is no need to delete parent
>> directories that were created with ‘mkdir -p’, since they could have
>> existed anyway. "
>>
>> Now, if everyone agrees that "distclean" is fine as it is, I won't
>> insist on anything.
>
> I'm with you in that distclean to me reads as "make clean for
> re-distribution". i.e. a pristine source tree.

It should be like that _as long as you rerun make distclean before every 
reconfiguration_.

Paolo