All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] stubdom: fix make clean and distclean on a freshly cloned tree
@ 2015-03-02 11:09 Wei Liu
  2015-03-02 11:15 ` Wei Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2015-03-02 11:09 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Sander Eikelenboom, Wei Liu, Ian Campbell

Clean and distclean targets need not depend on existence of the mini-os
tree. Don't check for mini-os and don't try to blindly include
mini-os's Config.mk when doing clean and distclean.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 stubdom/Makefile      | 13 ++++++++-----
 stubdom/c/Makefile    |  2 ++
 stubdom/caml/Makefile |  2 ++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/stubdom/Makefile b/stubdom/Makefile
index 1a1f263..f339b20 100644
--- a/stubdom/Makefile
+++ b/stubdom/Makefile
@@ -1,15 +1,18 @@
 XEN_ROOT = $(CURDIR)/..
 MINI_OS = $(XEN_ROOT)/extras/mini-os
 
-ifeq ($(wildcard $(MINI_OS)/Config.mk),)
-$(error Please run `make mini-os-dir' in top-level directory)
-endif
-
 export XEN_OS=MiniOS
 
 export stubdom=y
 export debug=y
-include $(XEN_ROOT)/Config.mk
+
+ifeq (,$(findstring clean,$(MAKECMDGOALS)))
+  ifeq ($(wildcard $(MINI_OS)/Config.mk),)
+    $(error Please run `make mini-os-dir' in top-level directory)
+  endif
+  include $(XEN_ROOT)/Config.mk
+endif
+
 -include $(XEN_ROOT)/config/Stubdom.mk
 
 GNU_TARGET_ARCH:=$(XEN_TARGET_ARCH)
diff --git a/stubdom/c/Makefile b/stubdom/c/Makefile
index c646c26..b252dca 100644
--- a/stubdom/c/Makefile
+++ b/stubdom/c/Makefile
@@ -1,6 +1,8 @@
 XEN_ROOT = $(CURDIR)/../..
 
+ifeq (,$(findstring clean,$(MAKECMDGOALS)))
 include $(XEN_ROOT)/Config.mk
+endif
 
 all: main.a
 
diff --git a/stubdom/caml/Makefile b/stubdom/caml/Makefile
index e79c98d..f550de1 100644
--- a/stubdom/caml/Makefile
+++ b/stubdom/caml/Makefile
@@ -1,6 +1,8 @@
 XEN_ROOT = $(CURDIR)/../..
 
+ifeq (,$(findstring clean,$(MAKECMDGOALS)))
 include $(XEN_ROOT)/Config.mk
+endif
 
 CAMLLIB = $(shell $(OCAMLC_CROSS_PREFIX)ocamlc -where)
 DEF_CPPFLAGS += -I$(CAMLLIB)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] stubdom: fix make clean and distclean on a freshly cloned tree
  2015-03-02 11:09 [PATCH] stubdom: fix make clean and distclean on a freshly cloned tree Wei Liu
@ 2015-03-02 11:15 ` Wei Liu
  2015-03-02 14:15   ` Ian Campbell
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2015-03-02 11:15 UTC (permalink / raw)
  To: xen-devel
  Cc: Wei Liu, Ian Campbell, Stefano Stabellini, Ian Jackson,
	Sander Eikelenboom, samuel.thibault

CC Samuel and Stefano.

On Mon, Mar 02, 2015 at 11:09:10AM +0000, Wei Liu wrote:
> Clean and distclean targets need not depend on existence of the mini-os
> tree. Don't check for mini-os and don't try to blindly include
> mini-os's Config.mk when doing clean and distclean.
> 
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  stubdom/Makefile      | 13 ++++++++-----
>  stubdom/c/Makefile    |  2 ++
>  stubdom/caml/Makefile |  2 ++
>  3 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/stubdom/Makefile b/stubdom/Makefile
> index 1a1f263..f339b20 100644
> --- a/stubdom/Makefile
> +++ b/stubdom/Makefile
> @@ -1,15 +1,18 @@
>  XEN_ROOT = $(CURDIR)/..
>  MINI_OS = $(XEN_ROOT)/extras/mini-os
>  
> -ifeq ($(wildcard $(MINI_OS)/Config.mk),)
> -$(error Please run `make mini-os-dir' in top-level directory)
> -endif
> -
>  export XEN_OS=MiniOS
>  
>  export stubdom=y
>  export debug=y
> -include $(XEN_ROOT)/Config.mk
> +
> +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
> +  ifeq ($(wildcard $(MINI_OS)/Config.mk),)
> +    $(error Please run `make mini-os-dir' in top-level directory)
> +  endif
> +  include $(XEN_ROOT)/Config.mk
> +endif
> +
>  -include $(XEN_ROOT)/config/Stubdom.mk
>  
>  GNU_TARGET_ARCH:=$(XEN_TARGET_ARCH)
> diff --git a/stubdom/c/Makefile b/stubdom/c/Makefile
> index c646c26..b252dca 100644
> --- a/stubdom/c/Makefile
> +++ b/stubdom/c/Makefile
> @@ -1,6 +1,8 @@
>  XEN_ROOT = $(CURDIR)/../..
>  
> +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
>  include $(XEN_ROOT)/Config.mk
> +endif
>  
>  all: main.a
>  
> diff --git a/stubdom/caml/Makefile b/stubdom/caml/Makefile
> index e79c98d..f550de1 100644
> --- a/stubdom/caml/Makefile
> +++ b/stubdom/caml/Makefile
> @@ -1,6 +1,8 @@
>  XEN_ROOT = $(CURDIR)/../..
>  
> +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
>  include $(XEN_ROOT)/Config.mk
> +endif
>  
>  CAMLLIB = $(shell $(OCAMLC_CROSS_PREFIX)ocamlc -where)
>  DEF_CPPFLAGS += -I$(CAMLLIB)
> -- 
> 1.9.1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] stubdom: fix make clean and distclean on a freshly cloned tree
  2015-03-02 11:15 ` Wei Liu
@ 2015-03-02 14:15   ` Ian Campbell
  2015-03-02 14:25     ` Wei Liu
  0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2015-03-02 14:15 UTC (permalink / raw)
  To: Wei Liu
  Cc: Sander Eikelenboom, Stefano Stabellini, Ian Jackson,
	samuel.thibault, xen-devel

On Mon, 2015-03-02 at 11:15 +0000, Wei Liu wrote:
> > -include $(XEN_ROOT)/Config.mk
> > +
> > +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
> > +  ifeq ($(wildcard $(MINI_OS)/Config.mk),)

> > +    $(error Please run `make mini-os-dir' in top-level directory)
> > +  endif
> > +  include $(XEN_ROOT)/Config.mk

I'm not sure why the inclusion of this apparently unrelated file
($(MINI_OS) vs $(XEN_ROOT) is conditionalised here.

If that wasn't here then the two ifeq's could be combined into something
like:

ifeq(,$(findstring clean,$(MAKECMDGOALS)$(wildcard $(MINI_OS)/Config.mk))

or is that too subtle?

> > +endif
> > +
> >  -include $(XEN_ROOT)/config/Stubdom.mk
> >  
> >  GNU_TARGET_ARCH:=$(XEN_TARGET_ARCH)
> > diff --git a/stubdom/c/Makefile b/stubdom/c/Makefile
> > index c646c26..b252dca 100644
> > --- a/stubdom/c/Makefile
> > +++ b/stubdom/c/Makefile
> > @@ -1,6 +1,8 @@
> >  XEN_ROOT = $(CURDIR)/../..
> >  
> > +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
> >  include $(XEN_ROOT)/Config.mk

XEN_ROOT/Config.mk always exists. Did you misread it as MINI_OS?

(and in the next hunk too)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] stubdom: fix make clean and distclean on a freshly cloned tree
  2015-03-02 14:15   ` Ian Campbell
@ 2015-03-02 14:25     ` Wei Liu
  2015-03-02 14:34       ` Ian Campbell
  0 siblings, 1 reply; 5+ messages in thread
From: Wei Liu @ 2015-03-02 14:25 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Wei Liu, Stefano Stabellini, Ian Jackson, xen-devel,
	Sander Eikelenboom, samuel.thibault

On Mon, Mar 02, 2015 at 02:15:43PM +0000, Ian Campbell wrote:
> On Mon, 2015-03-02 at 11:15 +0000, Wei Liu wrote:
> > > -include $(XEN_ROOT)/Config.mk
> > > +
> > > +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
> > > +  ifeq ($(wildcard $(MINI_OS)/Config.mk),)
> 
> > > +    $(error Please run `make mini-os-dir' in top-level directory)
> > > +  endif
> > > +  include $(XEN_ROOT)/Config.mk
> 
> I'm not sure why the inclusion of this apparently unrelated file
> ($(MINI_OS) vs $(XEN_ROOT) is conditionalised here.
> 
> If that wasn't here then the two ifeq's could be combined into something
> like:
> 
> ifeq(,$(findstring clean,$(MAKECMDGOALS)$(wildcard $(MINI_OS)/Config.mk))
> 
> or is that too subtle?
> 

A bit subtle to me. I used the same line for all three locations so
it might be easier to understand the condition.

> > > +endif
> > > +
> > >  -include $(XEN_ROOT)/config/Stubdom.mk
> > >  
> > >  GNU_TARGET_ARCH:=$(XEN_TARGET_ARCH)
> > > diff --git a/stubdom/c/Makefile b/stubdom/c/Makefile
> > > index c646c26..b252dca 100644
> > > --- a/stubdom/c/Makefile
> > > +++ b/stubdom/c/Makefile
> > > @@ -1,6 +1,8 @@
> > >  XEN_ROOT = $(CURDIR)/../..
> > >  
> > > +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
> > >  include $(XEN_ROOT)/Config.mk
> 
> XEN_ROOT/Config.mk always exists. Did you misread it as MINI_OS?
> 

No, I don't think I misread.

The subtle thing is that XEN_ROOT/Config.mk has

   include $(XEN_ROOT)/config/$(XEN_OS).mk

Here XEN_OS=MiniOS. Then in MiniOS.mk

   include $(XEN_ROOT)/extras/mini-os/Config.mk

If you don't have extra/mini-os you get an error.

Wei.

> (and in the next hunk too)
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] stubdom: fix make clean and distclean on a freshly cloned tree
  2015-03-02 14:25     ` Wei Liu
@ 2015-03-02 14:34       ` Ian Campbell
  0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2015-03-02 14:34 UTC (permalink / raw)
  To: Wei Liu
  Cc: Sander Eikelenboom, Stefano Stabellini, Ian Jackson,
	samuel.thibault, xen-devel

On Mon, 2015-03-02 at 14:25 +0000, Wei Liu wrote:
> > > > +endif
> > > > +
> > > >  -include $(XEN_ROOT)/config/Stubdom.mk
> > > >  
> > > >  GNU_TARGET_ARCH:=$(XEN_TARGET_ARCH)
> > > > diff --git a/stubdom/c/Makefile b/stubdom/c/Makefile
> > > > index c646c26..b252dca 100644
> > > > --- a/stubdom/c/Makefile
> > > > +++ b/stubdom/c/Makefile
> > > > @@ -1,6 +1,8 @@
> > > >  XEN_ROOT = $(CURDIR)/../..
> > > >  
> > > > +ifeq (,$(findstring clean,$(MAKECMDGOALS)))
> > > >  include $(XEN_ROOT)/Config.mk
> > 
> > XEN_ROOT/Config.mk always exists. Did you misread it as MINI_OS?
> > 
> 
> No, I don't think I misread.
> 
> The subtle thing is that XEN_ROOT/Config.mk has
> 
>    include $(XEN_ROOT)/config/$(XEN_OS).mk
> 
> Here XEN_OS=MiniOS. Then in MiniOS.mk
> 
>    include $(XEN_ROOT)/extras/mini-os/Config.mk
> 
> If you don't have extra/mini-os you get an error.

Ah, that is definitely worth spelling out in the commit log.

(this is the underlying cause of the comments on the first hunk too)

> 
> Wei.
> 
> > (and in the next hunk too)
> > 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-03-02 14:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-02 11:09 [PATCH] stubdom: fix make clean and distclean on a freshly cloned tree Wei Liu
2015-03-02 11:15 ` Wei Liu
2015-03-02 14:15   ` Ian Campbell
2015-03-02 14:25     ` Wei Liu
2015-03-02 14:34       ` Ian Campbell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.