1. Patchew
  2. Libvirt
  3. [libvirt] [PATCH 0/2] Support more container engines in Makefile.ci
  4. View patch

[libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am

Martin Kletzander posted 2 patches 6 years, 9 months ago
[libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
Posted by Martin Kletzander 6 years, 9 months ago 
The way it works now the Makefile needs to be both make valid and automake
valid.  That is fine for now, but if we want to use anything more advanced, like
conditionals, we cannot have it like that any more.

So instead forward all ci-* rules to that file.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 Makefile.am | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 0d8bb733e6d2..442bae511828 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -35,6 +35,7 @@ EXTRA_DIST = \
   libvirt-qemu.pc.in \
   libvirt-lxc.pc.in \
   libvirt-admin.pc.in \
+	Makefile.ci \
   Makefile.nonreentrant \
   autogen.sh \
   cfg.mk \
@@ -107,4 +108,5 @@ gen-AUTHORS:
 	  rm -f all.list maint.list contrib.list; \
 	fi
 
-include Makefile.ci
+ci-%:
+	$(MAKE) -f Makefile.ci $@
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
Posted by Daniel P. Berrangé 6 years, 9 months ago 
On Tue, May 07, 2019 at 05:45:30PM +0200, Martin Kletzander wrote:
> The way it works now the Makefile needs to be both make valid and automake
> valid.  That is fine for now, but if we want to use anything more advanced, like
> conditionals, we cannot have it like that any more.
> 
> So instead forward all ci-* rules to that file.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  Makefile.am | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 0d8bb733e6d2..442bae511828 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -35,6 +35,7 @@ EXTRA_DIST = \
>    libvirt-qemu.pc.in \
>    libvirt-lxc.pc.in \
>    libvirt-admin.pc.in \
> +	Makefile.ci \
>    Makefile.nonreentrant \
>    autogen.sh \
>    cfg.mk \

Indentation is not consistent here - tabs vs non-tabs.

> @@ -107,4 +108,5 @@ gen-AUTHORS:
>  	  rm -f all.list maint.list contrib.list; \
>  	fi
>  
> -include Makefile.ci
> +ci-%:
> +	$(MAKE) -f Makefile.ci $@

Will this cause all variables to be forwarded ?

eg will

  make ci-build@fedora-29 CI_IMAGE_TAG=:latest

result in

  make -f Makefile.ci ci-build@fedora-29 CI_IMAGE_TAG=:latest

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
Posted by Martin Kletzander 6 years, 9 months ago 
On Thu, May 09, 2019 at 01:54:42PM +0100, Daniel P. Berrangé wrote:
>On Tue, May 07, 2019 at 05:45:30PM +0200, Martin Kletzander wrote:
>> The way it works now the Makefile needs to be both make valid and automake
>> valid.  That is fine for now, but if we want to use anything more advanced, like
>> conditionals, we cannot have it like that any more.
>>
>> So instead forward all ci-* rules to that file.
>>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  Makefile.am | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/Makefile.am b/Makefile.am
>> index 0d8bb733e6d2..442bae511828 100644
>> --- a/Makefile.am
>> +++ b/Makefile.am
>> @@ -35,6 +35,7 @@ EXTRA_DIST = \
>>    libvirt-qemu.pc.in \
>>    libvirt-lxc.pc.in \
>>    libvirt-admin.pc.in \
>> +	Makefile.ci \
>>    Makefile.nonreentrant \
>>    autogen.sh \
>>    cfg.mk \
>
>Indentation is not consistent here - tabs vs non-tabs.
>
>> @@ -107,4 +108,5 @@ gen-AUTHORS:
>>  	  rm -f all.list maint.list contrib.list; \
>>  	fi
>>
>> -include Makefile.ci
>> +ci-%:
>> +	$(MAKE) -f Makefile.ci $@
>
>Will this cause all variables to be forwarded ?
>
>eg will
>
>  make ci-build@fedora-29 CI_IMAGE_TAG=:latest
>
>result in
>
>  make -f Makefile.ci ci-build@fedora-29 CI_IMAGE_TAG=:latest
>

It worked for me with CI_CENGINE, I thing this is forwarded thanks to $(MAKE).

>Regards,
>Daniel
>-- 
>|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
>|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] Don't include Makefile.ci in Makefile.am
Posted by Daniel P. Berrangé 6 years, 9 months ago 
On Thu, May 09, 2019 at 03:56:20PM +0200, Martin Kletzander wrote:
> On Thu, May 09, 2019 at 01:54:42PM +0100, Daniel P. Berrangé wrote:
> > On Tue, May 07, 2019 at 05:45:30PM +0200, Martin Kletzander wrote:
> > > The way it works now the Makefile needs to be both make valid and automake
> > > valid.  That is fine for now, but if we want to use anything more advanced, like
> > > conditionals, we cannot have it like that any more.
> > > 
> > > So instead forward all ci-* rules to that file.
> > > 
> > > Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> > > ---
> > >  Makefile.am | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index 0d8bb733e6d2..442bae511828 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -35,6 +35,7 @@ EXTRA_DIST = \
> > >    libvirt-qemu.pc.in \
> > >    libvirt-lxc.pc.in \
> > >    libvirt-admin.pc.in \
> > > +	Makefile.ci \
> > >    Makefile.nonreentrant \
> > >    autogen.sh \
> > >    cfg.mk \
> > 
> > Indentation is not consistent here - tabs vs non-tabs.
> > 
> > > @@ -107,4 +108,5 @@ gen-AUTHORS:
> > >  	  rm -f all.list maint.list contrib.list; \
> > >  	fi
> > > 
> > > -include Makefile.ci
> > > +ci-%:
> > > +	$(MAKE) -f Makefile.ci $@
> > 
> > Will this cause all variables to be forwarded ?
> > 
> > eg will
> > 
> >  make ci-build@fedora-29 CI_IMAGE_TAG=:latest
> > 
> > result in
> > 
> >  make -f Makefile.ci ci-build@fedora-29 CI_IMAGE_TAG=:latest
> > 
> 
> It worked for me with CI_CENGINE, I thing this is forwarded thanks to $(MAKE).

I don't think it is $(MAKE), as that's just a variable that expands to
the string "make". It looks like any variables in the makefile and turned
into environment variables that are inherited by the child make process.

I did a quick test and it all worked as desired.

$ cat bar.mak

FISH=food

fish:
	$(MAKE) -f foo.mak $@

$ cat foo.mak

FISH=cake

fish:
	@echo ">>$(FISH)<<"


$ make  -f bar.mak fish FISH=pond
make -f foo.mak fish
make[1]: Entering directory '/home/berrange/tmp'
>>pond<<
make[1]: Leaving directory '/home/berrange/tmp'

So on that basis, with the indent fix:

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.