diff mbox

[v2] libxl: fix gentypes call in Makefile

Message ID 20161110164600.29851-1-cbosdonnat@suse.com (mailing list archive)
State New, archived
Headers show

Commit Message

Cedric Bosdonnat Nov. 10, 2016, 4:46 p.m. UTC
From the make documentation:

"$* [...] If the target is `dir/a.foo.b' and the target pattern is
`a.%.b' then the stem is `dir/foo'. In a static pattern rule, the
stem is part of the file name that matched the `%' in the target
pattern."

The rule generating the c types files from the idl ones is not
a static pattern rule, but rather an implicit rule. Thus the value
of $* is preceded by the file path, instead of only what matches %.

In order to get this fixed, drop the path using a $(notdir $*).

Signed-off-by: Cédric Bosdonnat <cbosdonnat@suse.com>
---
  v2: use $(notdir $*) instead of $(shell basename $*)
---
 tools/libxl/Makefile | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

Comments

Wei Liu Nov. 11, 2016, 2:22 a.m. UTC | #1
On Thu, Nov 10, 2016 at 05:46:00PM +0100, Cédric Bosdonnat wrote:
> From the make documentation:
> 
> "$* [...] If the target is `dir/a.foo.b' and the target pattern is
> `a.%.b' then the stem is `dir/foo'. In a static pattern rule, the
> stem is part of the file name that matched the `%' in the target
> pattern."
> 
> The rule generating the c types files from the idl ones is not
> a static pattern rule, but rather an implicit rule. Thus the value
> of $* is preceded by the file path, instead of only what matches %.
> 
> In order to get this fixed, drop the path using a $(notdir $*).
> 
> Signed-off-by: Cédric Bosdonnat <cbosdonnat@suse.com>

OOI how did you discover this? Should we consider backporting this to
some older versions?

> ---
>   v2: use $(notdir $*) instead of $(shell basename $*)
> ---
>  tools/libxl/Makefile | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index af0a3ad..4dbaa98 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -250,12 +250,13 @@ $(LIBXL_OBJS) $(LIBXL_TEST_OBJS) $(LIBXLU_OBJS) \
>  $(LIBXL_OBJS) $(LIBXL_TEST_OBJS): libxl_internal.h
>  
>  _libxl_type%.h _libxl_type%_json.h _libxl_type%_private.h _libxl_type%.c: libxl_type%.idl gentypes.py idl.py
> -	$(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*_private.h \
> -		__libxl_type$*_json.h  __libxl_type$*.c
> -	$(call move-if-changed,__libxl_type$*.h,_libxl_type$*.h)
> -	$(call move-if-changed,__libxl_type$*_private.h,_libxl_type$*_private.h)
> -	$(call move-if-changed,__libxl_type$*_json.h,_libxl_type$*_json.h)
> -	$(call move-if-changed,__libxl_type$*.c,_libxl_type$*.c)
> +	$(eval stem = $(notdir $*))
> +	$(PYTHON) gentypes.py libxl_type$(stem).idl __libxl_type$(stem).h __libxl_type$(stem)_private.h \
> +		__libxl_type$(stem)_json.h  __libxl_type$(stem).c
> +	$(call move-if-changed,__libxl_type$(stem).h,_libxl_type$(stem).h)
> +	$(call move-if-changed,__libxl_type$(stem)_private.h,_libxl_type$(stem)_private.h)
> +	$(call move-if-changed,__libxl_type$(stem)_json.h,_libxl_type$(stem)_json.h)
> +	$(call move-if-changed,__libxl_type$(stem).c,_libxl_type$(stem).c)
>  
>  libxenlight.so: libxenlight.so.$(MAJOR)
>  	$(SYMLINK_SHLIB) $< $@
> -- 
> 2.10.1
>
Cedric Bosdonnat Nov. 14, 2016, 8:17 a.m. UTC | #2
On Fri, 2016-11-11 at 02:22 +0000, Wei Liu wrote:
> On Thu, Nov 10, 2016 at 05:46:00PM +0100, Cédric Bosdonnat wrote:
> > From the make documentation:
> > 
> > "$* [...] If the target is `dir/a.foo.b' and the target pattern is
> > `a.%.b' then the stem is `dir/foo'. In a static pattern rule, the
> > stem is part of the file name that matched the `%' in the target
> > pattern."
> > 
> > The rule generating the c types files from the idl ones is not
> > a static pattern rule, but rather an implicit rule. Thus the value
> > of $* is preceded by the file path, instead of only what matches %.
> > 
> > In order to get this fixed, drop the path using a $(notdir $*).
> > 
> > Signed-off-by: Cédric Bosdonnat <cbosdonnat@suse.com>
> 
> OOI how did you discover this? Should we consider backporting this to
> some older versions?

I discovered it by trying to add QED disks support, and thus changing the
libxl/types.idl file. I don't think it's worth backporting to stable releases
since the API shouldn't change in those.

--
Cedric

> > ---
> >   v2: use $(notdir $*) instead of $(shell basename $*)
> > ---
> >  tools/libxl/Makefile | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> > index af0a3ad..4dbaa98 100644
> > --- a/tools/libxl/Makefile
> > +++ b/tools/libxl/Makefile
> > @@ -250,12 +250,13 @@ $(LIBXL_OBJS) $(LIBXL_TEST_OBJS) $(LIBXLU_OBJS) \
> >  $(LIBXL_OBJS) $(LIBXL_TEST_OBJS): libxl_internal.h
> >  
> >  _libxl_type%.h _libxl_type%_json.h _libxl_type%_private.h _libxl_type%.c: libxl_type%.idl gentypes.py idl.py
> > -	$(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*_private.h \
> > -		__libxl_type$*_json.h  __libxl_type$*.c
> > -	$(call move-if-changed,__libxl_type$*.h,_libxl_type$*.h)
> > -	$(call move-if-changed,__libxl_type$*_private.h,_libxl_type$*_private.h)
> > -	$(call move-if-changed,__libxl_type$*_json.h,_libxl_type$*_json.h)
> > -	$(call move-if-changed,__libxl_type$*.c,_libxl_type$*.c)
> > +	$(eval stem = $(notdir $*))
> > +	$(PYTHON) gentypes.py libxl_type$(stem).idl __libxl_type$(stem).h __libxl_type$(stem)_private.h \
> > +		__libxl_type$(stem)_json.h  __libxl_type$(stem).c
> > +	$(call move-if-changed,__libxl_type$(stem).h,_libxl_type$(stem).h)
> > +	$(call move-if-changed,__libxl_type$(stem)_private.h,_libxl_type$(stem)_private.h)
> > +	$(call move-if-changed,__libxl_type$(stem)_json.h,_libxl_type$(stem)_json.h)
> > +	$(call move-if-changed,__libxl_type$(stem).c,_libxl_type$(stem).c)
> >  
> >  libxenlight.so: libxenlight.so.$(MAJOR)
> >  	$(SYMLINK_SHLIB) $< $@
> > -- 
> > 2.10.1
> > 
> 
>
Wei Liu Dec. 12, 2016, 9:46 a.m. UTC | #3
On Thu, Nov 10, 2016 at 05:46:00PM +0100, Cédric Bosdonnat wrote:
> From the make documentation:
> 
> "$* [...] If the target is `dir/a.foo.b' and the target pattern is
> `a.%.b' then the stem is `dir/foo'. In a static pattern rule, the
> stem is part of the file name that matched the `%' in the target
> pattern."
> 
> The rule generating the c types files from the idl ones is not
> a static pattern rule, but rather an implicit rule. Thus the value
> of $* is preceded by the file path, instead of only what matches %.
> 
> In order to get this fixed, drop the path using a $(notdir $*).
> 
> Signed-off-by: Cédric Bosdonnat <cbosdonnat@suse.com>

Acked-by: Wei Liu <wei.liu2@citrix.com>
diff mbox

Patch

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index af0a3ad..4dbaa98 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -250,12 +250,13 @@  $(LIBXL_OBJS) $(LIBXL_TEST_OBJS) $(LIBXLU_OBJS) \
 $(LIBXL_OBJS) $(LIBXL_TEST_OBJS): libxl_internal.h
 
 _libxl_type%.h _libxl_type%_json.h _libxl_type%_private.h _libxl_type%.c: libxl_type%.idl gentypes.py idl.py
-	$(PYTHON) gentypes.py libxl_type$*.idl __libxl_type$*.h __libxl_type$*_private.h \
-		__libxl_type$*_json.h  __libxl_type$*.c
-	$(call move-if-changed,__libxl_type$*.h,_libxl_type$*.h)
-	$(call move-if-changed,__libxl_type$*_private.h,_libxl_type$*_private.h)
-	$(call move-if-changed,__libxl_type$*_json.h,_libxl_type$*_json.h)
-	$(call move-if-changed,__libxl_type$*.c,_libxl_type$*.c)
+	$(eval stem = $(notdir $*))
+	$(PYTHON) gentypes.py libxl_type$(stem).idl __libxl_type$(stem).h __libxl_type$(stem)_private.h \
+		__libxl_type$(stem)_json.h  __libxl_type$(stem).c
+	$(call move-if-changed,__libxl_type$(stem).h,_libxl_type$(stem).h)
+	$(call move-if-changed,__libxl_type$(stem)_private.h,_libxl_type$(stem)_private.h)
+	$(call move-if-changed,__libxl_type$(stem)_json.h,_libxl_type$(stem)_json.h)
+	$(call move-if-changed,__libxl_type$(stem).c,_libxl_type$(stem).c)
 
 libxenlight.so: libxenlight.so.$(MAJOR)
 	$(SYMLINK_SHLIB) $< $@