[PATCH] tools/firmware: update OVMF Makefile

[PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

OVMF has become dependent on OpenSSL, which it is included as a submodule.
Initialise submodules before building.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
This should fix the build breakage for OVMF branch in OSSTEST.

Cc: Anthony PERARD <anthony.perard@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
---
 tools/firmware/ovmf-makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
index 2838744461..3de2fc0300 100644
--- a/tools/firmware/ovmf-makefile
+++ b/tools/firmware/ovmf-makefile
@@ -16,6 +16,7 @@ all: build
 
 .PHONY: build
 build:
+	$(GIT) submodule update --init --recursive
 	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
 	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Wed, Nov 28, 2018 at 05:43:33PM +0000, Wei Liu wrote:
> OVMF has become dependent on OpenSSL, which it is included as a submodule.

s/it//

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Wed, Nov 28, 2018 at 05:43:33PM +0000, Wei Liu wrote:
> OVMF has become dependent on OpenSSL, which it is included as a submodule.
> Initialise submodules before building.
> 
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> This should fix the build breakage for OVMF branch in OSSTEST.
> 
> Cc: Anthony PERARD <anthony.perard@citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> ---
>  tools/firmware/ovmf-makefile | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> index 2838744461..3de2fc0300 100644
> --- a/tools/firmware/ovmf-makefile
> +++ b/tools/firmware/ovmf-makefile
> @@ -16,6 +16,7 @@ all: build
>  
>  .PHONY: build
>  build:
> +	$(GIT) submodule update --init --recursive
>  	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
>  	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
>  

What about the release tarball? Do we includes OVMF in it?

Also, doesn't osstest needs some updates? I forgot if there is something
to do when projects have submodules.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Thu, Nov 29, 2018 at 11:31:41AM +0000, Anthony PERARD wrote:
> On Wed, Nov 28, 2018 at 05:43:33PM +0000, Wei Liu wrote:
> > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > Initialise submodules before building.
> > 
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > ---
> > This should fix the build breakage for OVMF branch in OSSTEST.
> > 
> > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > ---
> >  tools/firmware/ovmf-makefile | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > index 2838744461..3de2fc0300 100644
> > --- a/tools/firmware/ovmf-makefile
> > +++ b/tools/firmware/ovmf-makefile
> > @@ -16,6 +16,7 @@ all: build
> >  
> >  .PHONY: build
> >  build:
> > +	$(GIT) submodule update --init --recursive
> >  	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
> >  	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
> >  
> 
> What about the release tarball? Do we includes OVMF in it?

Yes we do. But this should work because the Makefile is also shipped.
What does qemu-xen do regarding its submodules? OVMF should just follow
suite.

> 
> Also, doesn't osstest needs some updates? I forgot if there is something
> to do when projects have submodules.

Yes there is some special arrangement for libvirt. Not sure what needs
to be done for OVMF since it is part of xen.git.

Ian?

Wei.

> 
> -- 
> Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Thu, Nov 29, 2018 at 11:39:54AM +0000, Wei Liu wrote:
> On Thu, Nov 29, 2018 at 11:31:41AM +0000, Anthony PERARD wrote:
> > What about the release tarball? Do we includes OVMF in it?
> 
> Yes we do. But this should work because the Makefile is also shipped.

The fact that the Makefile is shipped doesn't matter... Also of course
it would work, as long as the machine that build xen have access to
internet.

If we ship OVMF sources code, we need to include its submodules, so we
need to ship openssl source code. I think I've look into removing this
build dependency, but I don't think that was possible with simple build
options.

> What does qemu-xen do regarding its submodules? OVMF should just follow
> suite.

Nothing, qemu-xen isn't a project. If you meant "what does xen.git do
regarding qemu-xen's submodules" then the answer is, we call a script
from qemu to have it export whatever it needs to build (the fact that
there are submodules are transparent to users building qemu, like xen
do).

OVMF/edk2 is a bit special, its build system doesn't know about external
source code and teaching it to do something about it is probably not
going to be simple.

So we can't treat OVMF like QEMU, regarding the build, and releases.

We probably need a script to export git-submodules in xen.git.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Thu, Nov 29, 2018 at 12:02:16PM +0000, Anthony PERARD wrote:
> On Thu, Nov 29, 2018 at 11:39:54AM +0000, Wei Liu wrote:
> > On Thu, Nov 29, 2018 at 11:31:41AM +0000, Anthony PERARD wrote:
> > > What about the release tarball? Do we includes OVMF in it?
> > 
> > Yes we do. But this should work because the Makefile is also shipped.
> 
> The fact that the Makefile is shipped doesn't matter... Also of course
> it would work, as long as the machine that build xen have access to
> internet.
> 

I had assumed that QEMU would clone submodule on the fly. But I was
wrong. They had a script to extract those modules.

> If we ship OVMF sources code, we need to include its submodules, so we
> need to ship openssl source code. I think I've look into removing this
> build dependency, but I don't think that was possible with simple build
> options.
> 

Yes so we should ship those submodules as well.

> > What does qemu-xen do regarding its submodules? OVMF should just follow
> > suite.
> 
> Nothing, qemu-xen isn't a project. If you meant "what does xen.git do
> regarding qemu-xen's submodules" then the answer is, we call a script
> from qemu to have it export whatever it needs to build (the fact that
> there are submodules are transparent to users building qemu, like xen
> do).
> 
> OVMF/edk2 is a bit special, its build system doesn't know about external
> source code and teaching it to do something about it is probably not
> going to be simple.
> 

Maybe we should ask upstream's opinion on this matter?

Their github page has a release tarball but it doesn't build because of
the same problem.

If upstream doesn't want to do anything we would need special
arrangement for xen.git build and tarball build separately.

> So we can't treat OVMF like QEMU, regarding the build, and releases.
> 
> We probably need a script to export git-submodules in xen.git.
> 

We should.

Wei.

> -- 
> Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Thu, Nov 29, 2018 at 11:39:54AM +0000, Wei Liu wrote:
> On Thu, Nov 29, 2018 at 11:31:41AM +0000, Anthony PERARD wrote:
> > On Wed, Nov 28, 2018 at 05:43:33PM +0000, Wei Liu wrote:
> > > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > > Initialise submodules before building.
> > > 
> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > ---
> > > This should fix the build breakage for OVMF branch in OSSTEST.
> > > 
> > > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > ---
> > >  tools/firmware/ovmf-makefile | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > > index 2838744461..3de2fc0300 100644
> > > --- a/tools/firmware/ovmf-makefile
> > > +++ b/tools/firmware/ovmf-makefile
> > > @@ -16,6 +16,7 @@ all: build
> > >  
> > >  .PHONY: build
> > >  build:
> > > +	$(GIT) submodule update --init --recursive
> > >  	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
> > >  	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
> > >  
> > 
> > What about the release tarball? Do we includes OVMF in it?
> 
> Yes we do. But this should work because the Makefile is also shipped.
> What does qemu-xen do regarding its submodules? OVMF should just follow
> suite.

I just found out that the answer to my question was incorrect, we don't
ship OVMF sources code within the Xen release tarball. So the patch is
fine as it is, we don't have to fix `make src-tarball-release` at all.

> > Also, doesn't osstest needs some updates? I forgot if there is something
> > to do when projects have submodules.
> 
> Yes there is some special arrangement for libvirt. Not sure what needs
> to be done for OVMF since it is part of xen.git.

For osstest, it doesn't seems that anything needs to be done. osstest
doesn't know that QEMU is going to clone submodules, and yet osstest
doesn't complain. Your patch have the build system takes care of
submodules, so that's should be similair to qemu, and osstest isn't
going to complain.

I've tested and run a flight with this patch and upstream ovmf, and it
works fine, the openssl repo gets cloned thanks to the patch, and the
xen build succeed.

So for the patch:
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Wed, Jan 09, 2019 at 10:58:21AM +0000, Anthony PERARD wrote:
> On Thu, Nov 29, 2018 at 11:39:54AM +0000, Wei Liu wrote:
> > On Thu, Nov 29, 2018 at 11:31:41AM +0000, Anthony PERARD wrote:
> > > On Wed, Nov 28, 2018 at 05:43:33PM +0000, Wei Liu wrote:
> > > > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > > > Initialise submodules before building.
> > > > 
> > > > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> > > > ---
> > > > This should fix the build breakage for OVMF branch in OSSTEST.
> > > > 
> > > > Cc: Anthony PERARD <anthony.perard@citrix.com>
> > > > Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> > > > ---
> > > >  tools/firmware/ovmf-makefile | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> > > > index 2838744461..3de2fc0300 100644
> > > > --- a/tools/firmware/ovmf-makefile
> > > > +++ b/tools/firmware/ovmf-makefile
> > > > @@ -16,6 +16,7 @@ all: build
> > > >  
> > > >  .PHONY: build
> > > >  build:
> > > > +	$(GIT) submodule update --init --recursive
> > > >  	OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
> > > >  	cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
> > > >  
> > > 
> > > What about the release tarball? Do we includes OVMF in it?
> > 
> > Yes we do. But this should work because the Makefile is also shipped.
> > What does qemu-xen do regarding its submodules? OVMF should just follow
> > suite.
> 
> I just found out that the answer to my question was incorrect, we don't
> ship OVMF sources code within the Xen release tarball. So the patch is
> fine as it is, we don't have to fix `make src-tarball-release` at all.
> 
> > > Also, doesn't osstest needs some updates? I forgot if there is something
> > > to do when projects have submodules.
> > 
> > Yes there is some special arrangement for libvirt. Not sure what needs
> > to be done for OVMF since it is part of xen.git.
> 
> For osstest, it doesn't seems that anything needs to be done. osstest
> doesn't know that QEMU is going to clone submodules, and yet osstest
> doesn't complain. Your patch have the build system takes care of
> submodules, so that's should be similair to qemu, and osstest isn't
> going to complain.
> 
> I've tested and run a flight with this patch and upstream ovmf, and it
> works fine, the openssl repo gets cloned thanks to the patch, and the
> xen build succeed.

Thank you very much for doing the hard work!

> 
> So for the patch:
> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks. I plan to push this fix with your Rb to unblock ovmf branch.

Wei.

> 
> -- 
> Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Tamas K Lengyel]

On Wed, Nov 28, 2018 at 10:44 AM Wei Liu <wei.liu2@citrix.com> wrote:
>
> OVMF has become dependent on OpenSSL, which it is included as a submodule.
> Initialise submodules before building.

If you are updating the ovmf makefile, could you by any chance also
make the debug build of it more useful on Xen by making it print to
the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
following one-liner to change it to correct port:

sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Tamas K Lengyel]

On Wed, Jan 9, 2019 at 7:56 AM Tamas K Lengyel
<tamas.k.lengyel@gmail.com> wrote:
>
> On Wed, Nov 28, 2018 at 10:44 AM Wei Liu <wei.liu2@citrix.com> wrote:
> >
> > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > Initialise submodules before building.
>
> If you are updating the ovmf makefile, could you by any chance also
> make the debug build of it more useful on Xen by making it print to
> the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
> following one-liner to change it to correct port:
>
> sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec
>

Whops, actually the other way around for sed:

sed -i 's/PcdDebugIoPort|0x402/PcdDebugIoPort|0xe9/g' OvmfPkg/OvmfPkg.dec

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Wed, Jan 09, 2019 at 07:56:59AM -0700, Tamas K Lengyel wrote:
> On Wed, Jan 9, 2019 at 7:56 AM Tamas K Lengyel
> <tamas.k.lengyel@gmail.com> wrote:
> >
> > On Wed, Nov 28, 2018 at 10:44 AM Wei Liu <wei.liu2@citrix.com> wrote:
> > >
> > > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > > Initialise submodules before building.
> >
> > If you are updating the ovmf makefile, could you by any chance also
> > make the debug build of it more useful on Xen by making it print to
> > the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
> > following one-liner to change it to correct port:
> >
> > sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec
> >
> 
> Whops, actually the other way around for sed:
> 
> sed -i 's/PcdDebugIoPort|0x402/PcdDebugIoPort|0xe9/g' OvmfPkg/OvmfPkg.dec

You can actually have OVMF debug output without rebuilding it, add this
to our VM config:
device_model_args_hvm = [
  # Debug OVMF
  '-chardev', 'file,id=debugcon,mux=on,path=/var/log/xen/qemu-dm-ovmf.log.debugcon,',
  '-device', 'isa-debugcon,iobase=0x402,chardev=debugcon',
]

That way OVMF boot isn't slow down by writing to an ioport if there
isn't someone to care.

The way you suggest will have OVMF write to an ioport that the Xen
hypervisor will then write to it's console (or xl dmesg) and with the
amount of debug that ovmf write, this is quite slow. And worse, that
isn't going to work anymore with upstream OVMF as it now check if there
is something to listen on the other side of the ioport, Xen isn't going
answer the right things and OVMF will stay silence. (That paragraph is
about the change of PcdDebugIoPort.)

I don't know if DEBUG_ON_SERIAL_PORT is going to work, and how it works,
but it certainly don't use PcdDebugIoPort. Using a serial port instead
of an IO port is even going to be slower. So I don't know if it would be
useful to have.

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Tamas K Lengyel]

On Wed, Jan 9, 2019 at 8:46 AM Anthony PERARD <anthony.perard@citrix.com> wrote:
>
> On Wed, Jan 09, 2019 at 07:56:59AM -0700, Tamas K Lengyel wrote:
> > On Wed, Jan 9, 2019 at 7:56 AM Tamas K Lengyel
> > <tamas.k.lengyel@gmail.com> wrote:
> > >
> > > On Wed, Nov 28, 2018 at 10:44 AM Wei Liu <wei.liu2@citrix.com> wrote:
> > > >
> > > > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > > > Initialise submodules before building.
> > >
> > > If you are updating the ovmf makefile, could you by any chance also
> > > make the debug build of it more useful on Xen by making it print to
> > > the Xen console? Needs the -D DEBUG_ON_SERIAL_PORT flag added and the
> > > following one-liner to change it to correct port:
> > >
> > > sed -i 's/PcdDebugIoPort|0xe9/PcdDebugIoPort|0x402/g' OvmfPkg/OvmfPkg.dec
> > >
> >
> > Whops, actually the other way around for sed:
> >
> > sed -i 's/PcdDebugIoPort|0x402/PcdDebugIoPort|0xe9/g' OvmfPkg/OvmfPkg.dec
>
> You can actually have OVMF debug output without rebuilding it, add this
> to our VM config:
> device_model_args_hvm = [
>   # Debug OVMF
>   '-chardev', 'file,id=debugcon,mux=on,path=/var/log/xen/qemu-dm-ovmf.log.debugcon,',
>   '-device', 'isa-debugcon,iobase=0x402,chardev=debugcon',
> ]
>
> That way OVMF boot isn't slow down by writing to an ioport if there
> isn't someone to care.
>
> The way you suggest will have OVMF write to an ioport that the Xen
> hypervisor will then write to it's console (or xl dmesg) and with the
> amount of debug that ovmf write, this is quite slow. And worse, that
> isn't going to work anymore with upstream OVMF as it now check if there
> is something to listen on the other side of the ioport, Xen isn't going
> answer the right things and OVMF will stay silence. (That paragraph is
> about the change of PcdDebugIoPort.)
>
> I don't know if DEBUG_ON_SERIAL_PORT is going to work, and how it works,
> but it certainly don't use PcdDebugIoPort. Using a serial port instead
> of an IO port is even going to be slower. So I don't know if it would be
> useful to have.

Ah, thanks for that info! We should probably put that on the wiki too ;)

Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Wed, Jan 09, 2019 at 09:03:25AM -0700, Tamas K Lengyel wrote:
> Ah, thanks for that info! We should probably put that on the wiki too ;)

Done:
https://wiki.xenproject.org/index.php?title=OVMF&diff=19075&oldid=19074

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 731 bytes --]

On Wed, Nov 28, Wei Liu wrote:

> OVMF has become dependent on OpenSSL, which it is included as a submodule.
> Initialise submodules before building.

> +++ b/tools/firmware/ovmf-makefile
>  build:
> +	$(GIT) submodule update --init --recursive

At least ef529e6ab7c31290a33045bb1f1837447cc0eb56 seems to have no
submodules. Also the referenced qemu-xen.git has submodules, but
building xen.git works fine if none of the download-on-demand extra
sources are actually git trees. I have not checked how qemu-xen.git
would get its submodules, but today there is most likely no
"$(GIT) submodule" call in the "build" make target.

I think to fix the build breakage introduced by b16281870 this call must
be moved somewhere else.

Olaf

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Sun, Jan 13, 2019 at 09:17:44PM +0100, Olaf Hering wrote:
> On Wed, Nov 28, Wei Liu wrote:
> 
> > OVMF has become dependent on OpenSSL, which it is included as a submodule.
> > Initialise submodules before building.
> 
> > +++ b/tools/firmware/ovmf-makefile
> >  build:
> > +	$(GIT) submodule update --init --recursive
> 
> At least ef529e6ab7c31290a33045bb1f1837447cc0eb56 seems to have no
> submodules. Also the referenced qemu-xen.git has submodules, but
> building xen.git works fine if none of the download-on-demand extra
> sources are actually git trees. I have not checked how qemu-xen.git
> would get its submodules, but today there is most likely no
> "$(GIT) submodule" call in the "build" make target.

QEMU's Makefile calls a script which calls git submodule commands.

Calling `git submodule update` in a git repo without submodules is a
nop.

> 
> I think to fix the build breakage introduced by b16281870 this call must
> be moved somewhere else.

Can you tell me what build breakage are you referring to? The latest
osstest flight (131927) didn't show any breakage in building.

Are you saying that the breakage is shown when you put a snapshot of
ovmf under xen.git? How does ovmf distribute their snapshot? How is that
generated? Does it contain snapshots of submodules it needs already?

Wei.

> 
> Olaf



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 864 bytes --]

Am Mon, 14 Jan 2019 11:28:57 +0000
schrieb Wei Liu <wei.liu2@citrix.com>:

> Are you saying that the breakage is shown when you put a snapshot of
> ovmf under xen.git? How does ovmf distribute their snapshot? How is that
> generated? Does it contain snapshots of submodules it needs already?

I export all required sources recursively and store them in the appropriate directories.
qemu itself requires only the keycodemapdb, it does no download-on-demand.

And now that I had a chance to look at the sources, qemu-xen-dir-find calls a custom script that will do nothing if the <dir>-remote already exists.

I think instead of changing the custom Makefile, the submodules have to be handled in git-checkout.sh which is invoked by ovmf-dir.
If I remember correctly, when keycodemapdb became a hard requirement nothing in xen.git was adjusted.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Mon, Jan 14, 2019 at 12:48:52PM +0100, Olaf Hering wrote:
> Am Mon, 14 Jan 2019 11:28:57 +0000
> schrieb Wei Liu <wei.liu2@citrix.com>:
> 
> > Are you saying that the breakage is shown when you put a snapshot of
> > ovmf under xen.git? How does ovmf distribute their snapshot? How is that
> > generated? Does it contain snapshots of submodules it needs already?
> 
> I export all required sources recursively and store them in the appropriate directories.
> qemu itself requires only the keycodemapdb, it does no download-on-demand.
> 
> And now that I had a chance to look at the sources, qemu-xen-dir-find calls a custom script that will do nothing if the <dir>-remote already exists.
> 
> I think instead of changing the custom Makefile, the submodules have to be handled in git-checkout.sh which is invoked by ovmf-dir.

I'm not sure if that's a good idea, we would end up downloading a lot of
data for just one submodules that xen.git needs. (QEMU as quite a few
submodules that are never needed).

Also, git-checkout.sh isn't the only place where `git` is called. All
the *-force-update targets calls git without the script, `mktarball`
is another script that calls git.

I think it's fine to keep the current `submodule update` call where it
is. We could just make it check that it's an actual git worktree by
checking for the presence of ".git" (file or directory) before executing
git.

Would that be good enough?

> If I remember correctly, when keycodemapdb became a hard requirement nothing in xen.git was adjusted.

That's not exactly true, we had to adjust the `src-tarball-release` make
target to dl QEMU's submodules (or rather call QEMU's own release
tarball script).

And qemu.git and edk2.git are different. QEMU's build
system takes care of its own submodules, like xen.git does,
dl-on-demand. edk2.git doesn't do anything and let the developper
download openssl on its own, either via git submodule or by dl a
specific release of openssl[1].

[1] https://github.com/tianocore/tianocore.github.io/wiki/UDK2018-How-to-Build

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 438 bytes --]

Am Mon, 14 Jan 2019 17:11:56 +0000
schrieb Anthony PERARD <anthony.perard@citrix.com>:

> I think it's fine to keep the current `submodule update` call where it
> is. We could just make it check that it's an actual git worktree by
> checking for the presence of ".git" (file or directory) before executing
> git.
> 
> Would that be good enough?

Maybe. Whatever works with env WGET/GIT=/bin/false ./configure $options.

Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Mon, Jan 14, 2019 at 06:27:44PM +0100, Olaf Hering wrote:
> Am Mon, 14 Jan 2019 17:11:56 +0000
> schrieb Anthony PERARD <anthony.perard@citrix.com>:
> 
> > I think it's fine to keep the current `submodule update` call where it
> > is. We could just make it check that it's an actual git worktree by
> > checking for the presence of ".git" (file or directory) before executing
> > git.
> > 
> > Would that be good enough?
> 
> Maybe. Whatever works with env WGET/GIT=/bin/false ./configure $options.
> 

Can you try this?

diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
index 3de2fc0300..649482bca8 100644
--- a/tools/firmware/ovmf-makefile
+++ b/tools/firmware/ovmf-makefile
@@ -16,7 +16,7 @@ all: build

 .PHONY: build
  build:
  -       $(GIT) submodule update --init --recursive
  +       [ -d .git ] && $(GIT) submodule update --init --recursive
          OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
	          cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Anthony PERARD]

On Mon, Jan 14, 2019 at 05:44:57PM +0000, Wei Liu wrote:
> On Mon, Jan 14, 2019 at 06:27:44PM +0100, Olaf Hering wrote:
> > Am Mon, 14 Jan 2019 17:11:56 +0000
> > schrieb Anthony PERARD <anthony.perard@citrix.com>:
> > 
> > > I think it's fine to keep the current `submodule update` call where it
> > > is. We could just make it check that it's an actual git worktree by
> > > checking for the presence of ".git" (file or directory) before executing
> > > git.
> > > 
> > > Would that be good enough?
> > 
> > Maybe. Whatever works with env WGET/GIT=/bin/false ./configure $options.
> > 
> 
> Can you try this?
> 
> diff --git a/tools/firmware/ovmf-makefile b/tools/firmware/ovmf-makefile
> index 3de2fc0300..649482bca8 100644
> --- a/tools/firmware/ovmf-makefile
> +++ b/tools/firmware/ovmf-makefile
> @@ -16,7 +16,7 @@ all: build
> 
>  .PHONY: build
>   build:
>   -       $(GIT) submodule update --init --recursive
>   +       [ -d .git ] && $(GIT) submodule update --init --recursive

Please, use -e or -r, it is perfectly reasonable to have .git been a
file. (someone could use `git worktree`, or even a submodule)

Also, I think we should use `if`, As make must not fail if it isn't a
git worktree. (or "|| true" or "||:", but that would hide git failures.)

>           OvmfPkg/build.sh -a X64 -b $(TARGET) -n 4
> 	          cp Build/OvmfX64/$(TARGET)_GCC*/FV/OVMF.fd ovmf.bin
> 

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Olaf Hering]

[-- Attachment #1.1: Type: text/plain, Size: 335 bytes --]

Am Mon, 14 Jan 2019 17:44:57 +0000
schrieb Wei Liu <wei.liu2@citrix.com>:

>   -       $(GIT) submodule update --init --recursive
>   +       [ -d .git ] && $(GIT) submodule update --init --recursive

This syntax fails, but this works for me:

	if test -d .git ; then $(GIT) submodule update --init --recursive ; fi


Olaf

[-- Attachment #1.2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] tools/firmware: update OVMF Makefile

[Wei Liu]

On Mon, Jan 14, 2019 at 05:11:56PM +0000, Anthony PERARD wrote:
[...]
> 
> I think it's fine to keep the current `submodule update` call where it
> is. We could just make it check that it's an actual git worktree by
> checking for the presence of ".git" (file or directory) before executing
> git.

I concur.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel