Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Ján Tomko]

On a Tuesday in 2020, Daniel P. Berrangé wrote:

On Tue, Jul 28, 2020 at 12:24:56PM +0200, Ján Tomko wrote:

For unattended bisects, it would be nice to return 125 which is the
magic value meaning 'skip' to 'git bisect run'.


I don't think its possible to force the ninja/meson exit status
upon failure. I think at best uou could have a wrapper script
"mymeson" that looks for this error message and returns 125.



  meson test

returns 125 on a build failure, that might be helpful.

Jano


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Tue, Jul 28, 2020 at 12:15:25PM +0200, Peter Krempa wrote:
> On Tue, Jul 28, 2020 at 10:33:52 +0200, Pavel Hrdina wrote:
> > On Tue, Jul 28, 2020 at 10:09:42AM +0200, Peter Krempa wrote:
> 
> [...]
> 
> > 
> > Here I disagree, it also means that compilation should not fail. We do
> > comments all the time to patch series that every single commit should
> > compile correctly on it's own within the series.
> 
> Doing a 'return 0' is not compiling code correctly. The idea of
> requirements to bulild cleanly is that you can test the code.
> 
> In this case we need to observe it from a different angle though.
> 
> In reality an incomplete build is a failed build regardless of what the
> return value of the build system is. And that is important here. The
> main reason for the build system is to build everything so the only
> success is when everything is built.
> 
> If you don't have the resulting binary it's impossible to test the code
> or do anything else.
> 
> What would be even worse is to get a compiled binary that e.g. doesn't
> have the dependencies installed (such as RNG schemas, cpu XML docs and
> such) and fails in magic ways. At that point you can't be sure whether
> it's the bug you are trying to locate or just plainly broken build
> which the build system lied to you that it's complete.

Obviously this would be a big issue if it would randomly fail because of
some missing files that are not yet compiled. I have no objection here.

If your only use-case is to run binaries during git bisect then yes
having not-complete build when build system returns 0 is not correct.

I'm just saying there might be some other use-cases for git bisect
where having incomplete build is not an issue and we should not dismiss
these use-case.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Martin Kletzander]

On Tue, Jul 28, 2020 at 10:46:07AM +0100, Daniel P. Berrangé wrote:

On Fri, Jul 17, 2020 at 07:27:50PM +0200, Martin Kletzander wrote:

On Fri, Jul 17, 2020 at 03:12:00PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 17, 2020 at 04:10:01PM +0200, Pavel Hrdina wrote:
> > On Fri, Jul 17, 2020 at 03:02:10PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > > Personally I'd really like to avoid squashing them, because splitting
> > > > > up big patches is not merely to benefit the initial pre-merge review,
> > > > > but to also benefit people who need to debug stuff that's already
> > > > > merged and understand the scope of the intended change. So being able
> > > > > to look back at the changes in isolation after commit is still a big
> > > > > plus point.
> > > >
> > > > I would like to avoid squashing the patches as well and in most cases I
> > > > would object to it as well. I only suggested that to not break git
> > > > bisect.
> > > >
> > > > If we don't care about git bisect and the fact that we would not be able
> > > > to build libvirt correctly within these patches I'm OK with pushing it
> > > > without squashing.
> > >
> > > git bisect reliabity is key, so I reluctantly think we'll need to
> > > squash. I don't want to hit a pathc in this series with a bisect
> > > and be unable to continue the bisect due to inability to build the
> > > code.
> >
> > I can try to rearrange the patches to not break git bisect. It will
> > still require some script to be used for git bisect to detect if it
> > should run autotools or Meson.
>
> Maybe there's a reasonable tradeoff - instead of a 350 patch series,
> just a 10-20 patch series.
>

One other option would be a semi-linear merge. bisect would try the commit
before the rewrite and after, if both of them worked or both were broken then it
will not try the commits in the middle.  If it does, then you know it was
because of the autotools=>meson rewrite.  You will not break git-bisect and
you'll keep the history of the commits.


I'm not sure I follow exactly what you are saying here, so let me try to
illustrate what I think you mean and you can correct me.

Consider that

 - The current repo has commits A, B and C
 - The Meson series has commits R, S, T and U
 - After meson, we add commits H, I and J

IIUC, by "semi-linear merge" you mean a graph that looks
like this:

 ABC---HIJ
   |   |
   +-R-ST-U+


Now we notice a regression at J and know the last good commit was B.

IIUC you're saying that when "git bisect" runs it will stop on
commits 'C' and 'H', and only bisect into R, S, T, & U, if
the results of C & H show the problem is in the meson series.

That would certainly be nice, but I can't find any documentation
that clearly says this is what git bisect will do.

The docs I see merely say that bisect will look for a commit half way
between the current good & bad markers, but doesn't clarify what
"half way" means when there are two possible paths to follow in
the graph. You seem to be saying it will choose the shorter of the
two paths, but I don't see that mentioned anywhere. I could easily
see it deciding the "S" was the first half-way points to try, not
C or H.

If someone can point me to credible docs explaining what git does
wrt merges I'd be interested if I'm right or wrong.



Thank you for correcting me.  I genuinely believed that this was happening in
another project that I was bisecting, but I should've checked more.

I spent some more time looking at the workarounds and there are ways how to
achieve what I described.  Unfortunately it is not built in the bisect command
itself and there is nowhere to supply your own bisect helper or list of skipped
commits per-repo (which would be pretty easy and helpful, actually).  Anyway,
whatever does not work automatically with the unmodified `git bisect` command is
not going to be helpful, no matter how much we'd document it.

One other idea would be to have a Makefile (or some other mean) that just tells
you what to do if you got to any of the commits in the middle a bisect.  But
maybe I'm overthinking it.

Sorry, let's leave this, I guess.  I do not have any time to look at adding the
feature in git :)


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 :|


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Tue, Jul 28, 2020 at 12:24:56PM +0200, Ján Tomko wrote:
> On a Tuesday in 2020, Daniel P. Berrangé wrote:
> > On Tue, Jul 28, 2020 at 10:00:20AM +0200, Pavel Hrdina wrote:
> > > 3) Keep the patches as they are but error out in meson until the
> > >conversion is complete. The error can be used to detect if git
> > >bisect is withing the meson rewrite.
> > > 
> > >Pros: - full history of changes where each commit removes the
> > >relevant bits from autotools
> > > 
> > >  - git bisect is not broken if failed compilation is not an
> > >issue and marked as git bisect skip
> > 
> > 
> > > 
> > >Cons: - meson build will fail and there is no autogen.sh so no
> > >way how to compile libvirt even partially
> > > 
> > >  - script used for git bisect will have to skip failed
> > >compilation with an option to check for specific error
> > 
> > ..So explicitly failing the meson build is a significant improvement.
> > 
> > We could have
> > 
> > - Meson build is forced to fail out of the box
> > - An option "force_incomplete_build" to turn off the fail
> > - When failing prints an error message
> > 
> >   "This commit is part of the meson conversion and does not
> >build a complete libvirt. If bisecting, use "git bisect skip"
> >to continue, or "-Dforce_incomplete_build=true" to perform a
> >partial build"
> > 
> 
> For unattended bisects, it would be nice to return 125 which is the
> magic value meaning 'skip' to 'git bisect run'.

I don't think its possible to force the ninja/meson exit status
upon failure. I think at best uou could have a wrapper script
"mymeson" that looks for this error message and returns 125.




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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Ján Tomko]

On a Tuesday in 2020, Daniel P. Berrangé wrote:

On Tue, Jul 28, 2020 at 10:00:20AM +0200, Pavel Hrdina wrote:

3) Keep the patches as they are but error out in meson until the
   conversion is complete. The error can be used to detect if git
   bisect is withing the meson rewrite.

   Pros: - full history of changes where each commit removes the
   relevant bits from autotools

 - git bisect is not broken if failed compilation is not an
   issue and marked as git bisect skip





   Cons: - meson build will fail and there is no autogen.sh so no
   way how to compile libvirt even partially

 - script used for git bisect will have to skip failed
   compilation with an option to check for specific error


..So explicitly failing the meson build is a significant improvement.

We could have

- Meson build is forced to fail out of the box
- An option "force_incomplete_build" to turn off the fail
- When failing prints an error message

  "This commit is part of the meson conversion and does not
   build a complete libvirt. If bisecting, use "git bisect skip"
   to continue, or "-Dforce_incomplete_build=true" to perform a
   partial build"



For unattended bisects, it would be nice to return 125 which is the
magic value meaning 'skip' to 'git bisect run'.

Jano


So with that we would have full history, and git bisect would be able
to identify problems in ANY commit that is NOT part of the meson
series, except the single commit that is immediately either side of
the meson series. That should be viable I think.


4) Rework the series to have patches adding meson bits without
   removing anything from autotools and drop the autotools files in
   a single commit once the meson rewrite is complete

   Pros: - full history of changes

 - git bisect not broken because autogen.sh && make will
   work the whole time until meson build && ninja is ready

   Cons: - no reference of the meson changes to autotools code

 - additional work for me to redo the patches



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 :|



signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Tue, Jul 28, 2020 at 12:08:17 +0200, Pavel Hrdina wrote:
> On Tue, Jul 28, 2020 at 10:58:03AM +0100, Daniel P. Berrangé wrote:
> > On Tue, Jul 28, 2020 at 10:00:20AM +0200, Pavel Hrdina wrote:

[...]

> >  - Meson build is forced to fail out of the box
> >  - An option "force_incomplete_build" to turn off the fail
> >  - When failing prints an error message
> > 
> >"This commit is part of the meson conversion and does not
> > build a complete libvirt. If bisecting, use "git bisect skip"
> > to continue, or "-Dforce_incomplete_build=true" to perform a
> > partial build"
> > 
> > So with that we would have full history, and git bisect would be able
> > to identify problems in ANY commit that is NOT part of the meson
> > series, except the single commit that is immediately either side of
> > the meson series. That should be viable I think.
> 
> I like the extra option which would be removed once we remove the error
> message as well. It makes nice compromise between options 2) and 3).
> 
> Peter suggested using the error message in a private conversation that
> we had and this improves it a bit more so if we can agree on this
> I'll go with it.

I reckon that doing a build at any point partway in the series is
generally not useful, but sure option 3 is my minimum requirement, so if
that is what happens by default I don't mind if you add the extra
option.

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Tue, Jul 28, 2020 at 10:33:52 +0200, Pavel Hrdina wrote:
> On Tue, Jul 28, 2020 at 10:09:42AM +0200, Peter Krempa wrote:

[...]

> 
> Here I disagree, it also means that compilation should not fail. We do
> comments all the time to patch series that every single commit should
> compile correctly on it's own within the series.

Doing a 'return 0' is not compiling code correctly. The idea of
requirements to bulild cleanly is that you can test the code.

In this case we need to observe it from a different angle though.

In reality an incomplete build is a failed build regardless of what the
return value of the build system is. And that is important here. The
main reason for the build system is to build everything so the only
success is when everything is built.

If you don't have the resulting binary it's impossible to test the code
or do anything else.

What would be even worse is to get a compiled binary that e.g. doesn't
have the dependencies installed (such as RNG schemas, cpu XML docs and
such) and fails in magic ways. At that point you can't be sure whether
it's the bug you are trying to locate or just plainly broken build
which the build system lied to you that it's complete.

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Tue, Jul 28, 2020 at 10:58:03AM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 28, 2020 at 10:00:20AM +0200, Pavel Hrdina wrote:
> > On Mon, Jul 27, 2020 at 06:11:11PM +0200, Peter Krempa wrote:
> > > On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:
> > > > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > > > Personally I'd really like to avoid squashing them, because 
> > > > > > splitting
> > > > > > up big patches is not merely to benefit the initial pre-merge 
> > > > > > review,
> > > > > > but to also benefit people who need to debug stuff that's already
> > > > > > merged and understand the scope of the intended change. So being 
> > > > > > able
> > > > > > to look back at the changes in isolation after commit is still a big
> > > > > > plus point.
> > > > > 
> > > > > I would like to avoid squashing the patches as well and in most cases 
> > > > > I
> > > > > would object to it as well. I only suggested that to not break git
> > > > > bisect.
> > > > > 
> > > > > If we don't care about git bisect and the fact that we would not be 
> > > > > able
> > > > > to build libvirt correctly within these patches I'm OK with pushing it
> > > > > without squashing.
> > > > 
> > > > git bisect reliabity is key, so I reluctantly think we'll need to
> > > > squash. I don't want to hit a pathc in this series with a bisect
> > > > and be unable to continue the bisect due to inability to build the
> > > > code.
> > > 
> > > I agree. It's definitely necessary that the build is complete at any
> > > point in time.
> > > 
> > > I'm reluctantly willing to accept that the build fails with an
> > > appropriate error message until the build system is able to build
> > > everything if we opt for commiting a patchset for simplicity. What's
> > > off-limits is if build "succeeds", but is incomplete due to missing
> > > steps in the implementation. I'm not going to want to guess which part
> > > is already built or which isn't.
> > > 
> > > Given that the rewrite is a singularity anyways it doesn't really matter
> > > that we will not be able to bisect problems caused by the build system
> > > across the boundary.
> > 
> > So based on all the comments we have these options for pushing this
> > series:
> > 
> > 1) Squash it into single commit.
> > 
> >Pros: - no issues with git bisect
> > 
> >Cons: - we will not have the history of changes
> > 
> > 2) Keep the patches as they are and running meson build & ninja will
> >not fail.
> > 
> >Pros: - full history of changes where each commit removes the
> >relevant bits from autotools
> > 
> >  - git bisect is not broken as compilation will not fail
> > 
> >Cons: - meson build && ninja will not produce complete libvirt
> >binaries and there is no autogen.sh
> > 
> >  - script used for git bisect will have to detect if tested
> >binaries are compiled or use git bisect skip
> 
> So basically any time git bisect lands on one of the meson conversion
> patches, we'll need to use "git bisect skip". You might have todo that
> multiple times. It'll make bisect less efficient, but assuming the
> problem was not part of the meson series, git will eventually show
> you the broken commit.
> 
> The problem here is remembering which commits are ones which need
> to be skipped.
> 
> > 3) Keep the patches as they are but error out in meson until the
> >conversion is complete. The error can be used to detect if git
> >bisect is withing the meson rewrite.
> > 
> >Pros: - full history of changes where each commit removes the 
> >relevant bits from autotools
> > 
> >  - git bisect is not broken if failed compilation is not an
> >issue and marked as git bisect skip
> 
> 
> > 
> >Cons: - meson build will fail and there is no autogen.sh so no
> >way how to compile libvirt even partially
> > 
> >  - script used for git bisect will have to skip failed
> >compilation with an option to check for specific error
> 
> ..So explicitly failing the meson build is a significant improvement.
> 
> We could have
> 
>  - Meson build is forced to fail out of the box
>  - An option "force_incomplete_build" to turn off the fail
>  - When failing prints an error message
> 
>"This commit is part of the meson conversion and does not
> build a complete libvirt. If bisecting, use "git bisect skip"
> to continue, or "-Dforce_incomplete_build=true" to perform a
> partial build"
> 
> So with that we would have full history, and git bisect would be able
> to identify problems in ANY commit that is NOT part of the meson
> series, except the single commit that is immediately either side of
> the meson series. That should be viable I think.

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Tue, Jul 28, 2020 at 10:00:20AM +0200, Pavel Hrdina wrote:
> On Mon, Jul 27, 2020 at 06:11:11PM +0200, Peter Krempa wrote:
> > On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:
> > > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > > Personally I'd really like to avoid squashing them, because splitting
> > > > > up big patches is not merely to benefit the initial pre-merge review,
> > > > > but to also benefit people who need to debug stuff that's already
> > > > > merged and understand the scope of the intended change. So being able
> > > > > to look back at the changes in isolation after commit is still a big
> > > > > plus point.
> > > > 
> > > > I would like to avoid squashing the patches as well and in most cases I
> > > > would object to it as well. I only suggested that to not break git
> > > > bisect.
> > > > 
> > > > If we don't care about git bisect and the fact that we would not be able
> > > > to build libvirt correctly within these patches I'm OK with pushing it
> > > > without squashing.
> > > 
> > > git bisect reliabity is key, so I reluctantly think we'll need to
> > > squash. I don't want to hit a pathc in this series with a bisect
> > > and be unable to continue the bisect due to inability to build the
> > > code.
> > 
> > I agree. It's definitely necessary that the build is complete at any
> > point in time.
> > 
> > I'm reluctantly willing to accept that the build fails with an
> > appropriate error message until the build system is able to build
> > everything if we opt for commiting a patchset for simplicity. What's
> > off-limits is if build "succeeds", but is incomplete due to missing
> > steps in the implementation. I'm not going to want to guess which part
> > is already built or which isn't.
> > 
> > Given that the rewrite is a singularity anyways it doesn't really matter
> > that we will not be able to bisect problems caused by the build system
> > across the boundary.
> 
> So based on all the comments we have these options for pushing this
> series:
> 
> 1) Squash it into single commit.
> 
>Pros: - no issues with git bisect
> 
>Cons: - we will not have the history of changes
> 
> 2) Keep the patches as they are and running meson build & ninja will
>not fail.
> 
>Pros: - full history of changes where each commit removes the
>relevant bits from autotools
> 
>  - git bisect is not broken as compilation will not fail
> 
>Cons: - meson build && ninja will not produce complete libvirt
>binaries and there is no autogen.sh
> 
>  - script used for git bisect will have to detect if tested
>binaries are compiled or use git bisect skip

So basically any time git bisect lands on one of the meson conversion
patches, we'll need to use "git bisect skip". You might have todo that
multiple times. It'll make bisect less efficient, but assuming the
problem was not part of the meson series, git will eventually show
you the broken commit.

The problem here is remembering which commits are ones which need
to be skipped.

> 3) Keep the patches as they are but error out in meson until the
>conversion is complete. The error can be used to detect if git
>bisect is withing the meson rewrite.
> 
>Pros: - full history of changes where each commit removes the 
>relevant bits from autotools
> 
>  - git bisect is not broken if failed compilation is not an
>issue and marked as git bisect skip


> 
>Cons: - meson build will fail and there is no autogen.sh so no
>way how to compile libvirt even partially
> 
>  - script used for git bisect will have to skip failed
>compilation with an option to check for specific error

..So explicitly failing the meson build is a significant improvement.

We could have

 - Meson build is forced to fail out of the box
 - An option "force_incomplete_build" to turn off the fail
 - When failing prints an error message

   "This commit is part of the meson conversion and does not
build a complete libvirt. If bisecting, use "git bisect skip"
to continue, or "-Dforce_incomplete_build=true" to perform a
partial build"

So with that we would have full history, and git bisect would be able
to identify problems in ANY commit that is NOT part of the meson
series, except the single commit that is immediately either side of
the meson series. That should be viable I think.

> 4) Rework the series to have patches adding meson bits without
>removing anything from autotools and drop the autotools files in
>a single commit once the meson rewrite is complete
> 
>Pros: - full history of changes
> 
>  - git bisect not broken because autogen.sh && make will
>

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Andrea Bolognani]

On Tue, 2020-07-28 at 10:00 +0200, Pavel Hrdina wrote:
> So based on all the comments we have these options for pushing this
> series:
> 
> 1) Squash it into single commit.
> 
> 2) Keep the patches as they are and running meson build & ninja will
>not fail.
> 
> 3) Keep the patches as they are but error out in meson until the
>conversion is complete. The error can be used to detect if git
>bisect is withing the meson rewrite.
> 
> 4) Rework the series to have patches adding meson bits without
>removing anything from autotools and drop the autotools files in
>a single commit once the meson rewrite is complete

I suggest going for option 1, with the caveat that before pushing you
you should post a fully-reviewed and fixed vN to the mailing list and
include a link to it in the message for the squashed commit. This
will allow us to look back at the original reasoning behind a change
in the same way the reviewer could, without affecting bisectability.

Options 4 would be acceptable as well, but it requires more work on
your side and still requires hitting the mailing list archives to get
the full picture of the rewrite, so I don't think is worth it.

Options 2 and 3 result in a partially-built libvirt over a pretty big
range of commits, which I feel is actually worse than breaking
bisectability.

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Fri, Jul 17, 2020 at 07:27:50PM +0200, Martin Kletzander wrote:
> On Fri, Jul 17, 2020 at 03:12:00PM +0100, Daniel P. Berrangé wrote:
> > On Fri, Jul 17, 2020 at 04:10:01PM +0200, Pavel Hrdina wrote:
> > > On Fri, Jul 17, 2020 at 03:02:10PM +0100, Daniel P. Berrangé wrote:
> > > > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > > > Personally I'd really like to avoid squashing them, because 
> > > > > > splitting
> > > > > > up big patches is not merely to benefit the initial pre-merge 
> > > > > > review,
> > > > > > but to also benefit people who need to debug stuff that's already
> > > > > > merged and understand the scope of the intended change. So being 
> > > > > > able
> > > > > > to look back at the changes in isolation after commit is still a big
> > > > > > plus point.
> > > > >
> > > > > I would like to avoid squashing the patches as well and in most cases 
> > > > > I
> > > > > would object to it as well. I only suggested that to not break git
> > > > > bisect.
> > > > >
> > > > > If we don't care about git bisect and the fact that we would not be 
> > > > > able
> > > > > to build libvirt correctly within these patches I'm OK with pushing it
> > > > > without squashing.
> > > >
> > > > git bisect reliabity is key, so I reluctantly think we'll need to
> > > > squash. I don't want to hit a pathc in this series with a bisect
> > > > and be unable to continue the bisect due to inability to build the
> > > > code.
> > > 
> > > I can try to rearrange the patches to not break git bisect. It will
> > > still require some script to be used for git bisect to detect if it
> > > should run autotools or Meson.
> > 
> > Maybe there's a reasonable tradeoff - instead of a 350 patch series,
> > just a 10-20 patch series.
> > 
> 
> One other option would be a semi-linear merge. bisect would try the commit
> before the rewrite and after, if both of them worked or both were broken then 
> it
> will not try the commits in the middle.  If it does, then you know it was
> because of the autotools=>meson rewrite.  You will not break git-bisect and
> you'll keep the history of the commits.

I'm not sure I follow exactly what you are saying here, so let me try to
illustrate what I think you mean and you can correct me.

Consider that

  - The current repo has commits A, B and C
  - The Meson series has commits R, S, T and U
  - After meson, we add commits H, I and J

IIUC, by "semi-linear merge" you mean a graph that looks
like this:

  ABC---HIJ
|   |
+-R-ST-U+


Now we notice a regression at J and know the last good commit was B.

IIUC you're saying that when "git bisect" runs it will stop on
commits 'C' and 'H', and only bisect into R, S, T, & U, if
the results of C & H show the problem is in the meson series.

That would certainly be nice, but I can't find any documentation
that clearly says this is what git bisect will do.

The docs I see merely say that bisect will look for a commit half way
between the current good & bad markers, but doesn't clarify what
"half way" means when there are two possible paths to follow in
the graph. You seem to be saying it will choose the shorter of the
two paths, but I don't see that mentioned anywhere. I could easily
see it deciding the "S" was the first half-way points to try, not
C or H.

If someone can point me to credible docs explaining what git does
wrt merges I'd be interested if I'm right or wrong.

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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Tue, Jul 28, 2020 at 10:09:42AM +0200, Peter Krempa wrote:
> On Tue, Jul 28, 2020 at 10:00:20 +0200, Pavel Hrdina wrote:
> > On Mon, Jul 27, 2020 at 06:11:11PM +0200, Peter Krempa wrote:
> > > On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:
> > > > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > > > Personally I'd really like to avoid squashing them, because 
> > > > > > splitting
> > > > > > up big patches is not merely to benefit the initial pre-merge 
> > > > > > review,
> > > > > > but to also benefit people who need to debug stuff that's already
> > > > > > merged and understand the scope of the intended change. So being 
> > > > > > able
> > > > > > to look back at the changes in isolation after commit is still a big
> > > > > > plus point.
> > > > > 
> > > > > I would like to avoid squashing the patches as well and in most cases 
> > > > > I
> > > > > would object to it as well. I only suggested that to not break git
> > > > > bisect.
> > > > > 
> > > > > If we don't care about git bisect and the fact that we would not be 
> > > > > able
> > > > > to build libvirt correctly within these patches I'm OK with pushing it
> > > > > without squashing.
> > > > 
> > > > git bisect reliabity is key, so I reluctantly think we'll need to
> > > > squash. I don't want to hit a pathc in this series with a bisect
> > > > and be unable to continue the bisect due to inability to build the
> > > > code.
> > > 
> > > I agree. It's definitely necessary that the build is complete at any
> > > point in time.
> > > 
> > > I'm reluctantly willing to accept that the build fails with an
> > > appropriate error message until the build system is able to build
> > > everything if we opt for commiting a patchset for simplicity. What's
> > > off-limits is if build "succeeds", but is incomplete due to missing
> > > steps in the implementation. I'm not going to want to guess which part
> > > is already built or which isn't.
> > > 
> > > Given that the rewrite is a singularity anyways it doesn't really matter
> > > that we will not be able to bisect problems caused by the build system
> > > across the boundary.
> > 
> > So based on all the comments we have these options for pushing this
> > series:
> > 
> > 1) Squash it into single commit.
> > 
> >Pros: - no issues with git bisect
> > 
> >Cons: - we will not have the history of changes
> > 
> > 2) Keep the patches as they are and running meson build & ninja will
> >not fail.
> > 
> >Pros: - full history of changes where each commit removes the
> >relevant bits from autotools
> > 
> >  - git bisect is not broken as compilation will not fail
> 
> You can't claim that 'git bisect' is not broken if "compilation does not
> fail". That is plainly and utterly misleading ...

The wording should have been:

git bisect is not broken if successful compilation that may not build
all binaries is not an issue and marked as git bisect skip

to match the wording for option 3 which was:

git bisect is not broken if failed compilation is not an issue and
marked as git bisect skip

>
> >Cons: - meson build && ninja will not produce complete libvirt
> >binaries and there is no autogen.sh
> 
> ... as bisect is used to figure out when some code broke. If you don't
> compile the code, then bisect _IS_ broken.
> 
> Not breaking bisect means that everything must compile.

Here I disagree, it also means that compilation should not fail. We do
comments all the time to patch series that every single commit should
compile correctly on it's own within the series.

To me personally both option 2) and option 3) are similar. In both cases
you will have to end up with the workaround mentioned by Martin and
basically skip the whole meson rewrite when doing git bisect.

> >  - script used for git bisect will have to detect if tested
> >binaries are compiled or use git bisect skip
> 
> No. Just no.
> 
> As I've pointed out earlier, you won't use bisect to find when build
> system change broke things, because the complete rewrite invalidates
> everything anyways.
> 
> Build system can be bisected only prior to the change and only after the
> change but nothing between.
> 
> Option 4 is the winner usability wise and change-insight wise. 1 follows
> with just usability benefits. Option 3 is acceptable. Option 2 is not at
> all.


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Tue, Jul 28, 2020 at 10:00:20 +0200, Pavel Hrdina wrote:
> On Mon, Jul 27, 2020 at 06:11:11PM +0200, Peter Krempa wrote:
> > On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:
> > > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > > Personally I'd really like to avoid squashing them, because splitting
> > > > > up big patches is not merely to benefit the initial pre-merge review,
> > > > > but to also benefit people who need to debug stuff that's already
> > > > > merged and understand the scope of the intended change. So being able
> > > > > to look back at the changes in isolation after commit is still a big
> > > > > plus point.
> > > > 
> > > > I would like to avoid squashing the patches as well and in most cases I
> > > > would object to it as well. I only suggested that to not break git
> > > > bisect.
> > > > 
> > > > If we don't care about git bisect and the fact that we would not be able
> > > > to build libvirt correctly within these patches I'm OK with pushing it
> > > > without squashing.
> > > 
> > > git bisect reliabity is key, so I reluctantly think we'll need to
> > > squash. I don't want to hit a pathc in this series with a bisect
> > > and be unable to continue the bisect due to inability to build the
> > > code.
> > 
> > I agree. It's definitely necessary that the build is complete at any
> > point in time.
> > 
> > I'm reluctantly willing to accept that the build fails with an
> > appropriate error message until the build system is able to build
> > everything if we opt for commiting a patchset for simplicity. What's
> > off-limits is if build "succeeds", but is incomplete due to missing
> > steps in the implementation. I'm not going to want to guess which part
> > is already built or which isn't.
> > 
> > Given that the rewrite is a singularity anyways it doesn't really matter
> > that we will not be able to bisect problems caused by the build system
> > across the boundary.
> 
> So based on all the comments we have these options for pushing this
> series:
> 
> 1) Squash it into single commit.
> 
>Pros: - no issues with git bisect
> 
>Cons: - we will not have the history of changes
> 
> 2) Keep the patches as they are and running meson build & ninja will
>not fail.
> 
>Pros: - full history of changes where each commit removes the
>relevant bits from autotools
> 
>  - git bisect is not broken as compilation will not fail

You can't claim that 'git bisect' is not broken if "compilation does not
fail". That is plainly and utterly misleading ...


>Cons: - meson build && ninja will not produce complete libvirt
>binaries and there is no autogen.sh

... as bisect is used to figure out when some code broke. If you don't
compile the code, then bisect _IS_ broken.

Not breaking bisect means that everything must compile.

>  - script used for git bisect will have to detect if tested
>binaries are compiled or use git bisect skip

No. Just no.

As I've pointed out earlier, you won't use bisect to find when build
system change broke things, because the complete rewrite invalidates
everything anyways.

Build system can be bisected only prior to the change and only after the
change but nothing between.

Option 4 is the winner usability wise and change-insight wise. 1 follows
with just usability benefits. Option 3 is acceptable. Option 2 is not at
all.

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Mon, Jul 27, 2020 at 06:11:11PM +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:
> > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > Personally I'd really like to avoid squashing them, because splitting
> > > > up big patches is not merely to benefit the initial pre-merge review,
> > > > but to also benefit people who need to debug stuff that's already
> > > > merged and understand the scope of the intended change. So being able
> > > > to look back at the changes in isolation after commit is still a big
> > > > plus point.
> > > 
> > > I would like to avoid squashing the patches as well and in most cases I
> > > would object to it as well. I only suggested that to not break git
> > > bisect.
> > > 
> > > If we don't care about git bisect and the fact that we would not be able
> > > to build libvirt correctly within these patches I'm OK with pushing it
> > > without squashing.
> > 
> > git bisect reliabity is key, so I reluctantly think we'll need to
> > squash. I don't want to hit a pathc in this series with a bisect
> > and be unable to continue the bisect due to inability to build the
> > code.
> 
> I agree. It's definitely necessary that the build is complete at any
> point in time.
> 
> I'm reluctantly willing to accept that the build fails with an
> appropriate error message until the build system is able to build
> everything if we opt for commiting a patchset for simplicity. What's
> off-limits is if build "succeeds", but is incomplete due to missing
> steps in the implementation. I'm not going to want to guess which part
> is already built or which isn't.
> 
> Given that the rewrite is a singularity anyways it doesn't really matter
> that we will not be able to bisect problems caused by the build system
> across the boundary.

So based on all the comments we have these options for pushing this
series:

1) Squash it into single commit.

   Pros: - no issues with git bisect

   Cons: - we will not have the history of changes

2) Keep the patches as they are and running meson build & ninja will
   not fail.

   Pros: - full history of changes where each commit removes the
   relevant bits from autotools

 - git bisect is not broken as compilation will not fail

   Cons: - meson build && ninja will not produce complete libvirt
   binaries and there is no autogen.sh

 - script used for git bisect will have to detect if tested
   binaries are compiled or use git bisect skip

3) Keep the patches as they are but error out in meson until the
   conversion is complete. The error can be used to detect if git
   bisect is withing the meson rewrite.

   Pros: - full history of changes where each commit removes the 
   relevant bits from autotools

 - git bisect is not broken if failed compilation is not an
   issue and marked as git bisect skip

   Cons: - meson build will fail and there is no autogen.sh so no
   way how to compile libvirt even partially

 - script used for git bisect will have to skip failed
   compilation with an option to check for specific error

4) Rework the series to have patches adding meson bits without
   removing anything from autotools and drop the autotools files in
   a single commit once the meson rewrite is complete

   Pros: - full history of changes

 - git bisect not broken because autogen.sh && make will
   work the whole time until meson build && ninja is ready

   Cons: - no reference of the meson changes to autotools code

 - additional work for me to redo the patches


There is a possible workaround as Martin suggested that when running git
bisect the script/developer would test libvirt before meson rewrite and
right after it to figure out if the issue is caused by meson rewrite or
not and then continue purely with meson or autotools to look for the
broken commit. This would make sense for option 2) and 3).

Looking forward to your ideas and comments or suggestion with other
options.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Mon, Jul 27, 2020 at 20:43:05 -0400, Neal Gompa wrote:
> On Mon, Jul 27, 2020 at 12:11 PM Peter Krempa  wrote:
> > On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:

[...]

> > I agree. It's definitely necessary that the build is complete at any
> > point in time.
> >
> > I'm reluctantly willing to accept that the build fails with an
> > appropriate error message until the build system is able to build
> > everything if we opt for commiting a patchset for simplicity. What's
> > off-limits is if build "succeeds", but is incomplete due to missing
> > steps in the implementation. I'm not going to want to guess which part
> > is already built or which isn't.
> >
> > Given that the rewrite is a singularity anyways it doesn't really matter
> > that we will not be able to bisect problems caused by the build system
> > across the boundary.
> >
> 
> Unfortunately, this is where email based workflows completely fall
> apart. If this was represented as a merge request, it'd be
> straightforward to look at it from either the "changeset view" (the
> delta from upstream main branch and the branch containing the changes)
> or the "per-change view" (the delta across a commit). I literally

You can do the same once you apply the patches on your local repository.

In the end a the merge request is just that. A repo with the patches
applied and the "cover letter" is represented as the merge request
"justification".

The only difference is how you get those ...

> could not figure out how to review this entire change set (despite my
> best efforts) because pulling down this 351-patch changeset is quite
> difficult for me. At least, not until I realized the cover letter
> pointed to a GitLab repository with the commits present.

... and Pavel provided both views in case your e-mail client doesn't
enable you to extract a patchset quickly.

Please note that in my reply I was specifically refering only to the
state once it's commited to the main repository and not in any way
refering to review. Once the patchset is comitted it's same situation
for everybody.

> But email is the workflow we have, not the one we deserve, so I'd
> rather see this re-sent as a single patch. That patch will be too big
> to send as an email, though, so it will likely need to be sent as an
> attachment.

The resulting squashed mail is 880K. We had bigger ones e.g. when Daniel
changed the translation systems which had 1.0M and 1.2M emails and they
went through just fine.

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Neal Gompa]

On Mon, Jul 27, 2020 at 12:11 PM Peter Krempa  wrote:
>
> On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:
> > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > Personally I'd really like to avoid squashing them, because splitting
> > > > up big patches is not merely to benefit the initial pre-merge review,
> > > > but to also benefit people who need to debug stuff that's already
> > > > merged and understand the scope of the intended change. So being able
> > > > to look back at the changes in isolation after commit is still a big
> > > > plus point.
> > >
> > > I would like to avoid squashing the patches as well and in most cases I
> > > would object to it as well. I only suggested that to not break git
> > > bisect.
> > >
> > > If we don't care about git bisect and the fact that we would not be able
> > > to build libvirt correctly within these patches I'm OK with pushing it
> > > without squashing.
> >
> > git bisect reliabity is key, so I reluctantly think we'll need to
> > squash. I don't want to hit a pathc in this series with a bisect
> > and be unable to continue the bisect due to inability to build the
> > code.
>
> I agree. It's definitely necessary that the build is complete at any
> point in time.
>
> I'm reluctantly willing to accept that the build fails with an
> appropriate error message until the build system is able to build
> everything if we opt for commiting a patchset for simplicity. What's
> off-limits is if build "succeeds", but is incomplete due to missing
> steps in the implementation. I'm not going to want to guess which part
> is already built or which isn't.
>
> Given that the rewrite is a singularity anyways it doesn't really matter
> that we will not be able to bisect problems caused by the build system
> across the boundary.
>

Unfortunately, this is where email based workflows completely fall
apart. If this was represented as a merge request, it'd be
straightforward to look at it from either the "changeset view" (the
delta from upstream main branch and the branch containing the changes)
or the "per-change view" (the delta across a commit). I literally
could not figure out how to review this entire change set (despite my
best efforts) because pulling down this 351-patch changeset is quite
difficult for me. At least, not until I realized the cover letter
pointed to a GitLab repository with the commits present. 

But email is the workflow we have, not the one we deserve, so I'd
rather see this re-sent as a single patch. That patch will be too big
to send as an email, though, so it will likely need to be sent as an
attachment. Or maybe this could be the inaugural change merged in
through a merge request in GitLab.com?

One could only hope...


-- 
真実はいつも一つ！/ Always, there's only one truth!

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Fri, Jul 17, 2020 at 15:02:10 +0100, Daniel Berrange wrote:
> On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > Personally I'd really like to avoid squashing them, because splitting
> > > up big patches is not merely to benefit the initial pre-merge review,
> > > but to also benefit people who need to debug stuff that's already
> > > merged and understand the scope of the intended change. So being able
> > > to look back at the changes in isolation after commit is still a big
> > > plus point.
> > 
> > I would like to avoid squashing the patches as well and in most cases I
> > would object to it as well. I only suggested that to not break git
> > bisect.
> > 
> > If we don't care about git bisect and the fact that we would not be able
> > to build libvirt correctly within these patches I'm OK with pushing it
> > without squashing.
> 
> git bisect reliabity is key, so I reluctantly think we'll need to
> squash. I don't want to hit a pathc in this series with a bisect
> and be unable to continue the bisect due to inability to build the
> code.

I agree. It's definitely necessary that the build is complete at any
point in time.

I'm reluctantly willing to accept that the build fails with an
appropriate error message until the build system is able to build
everything if we opt for commiting a patchset for simplicity. What's
off-limits is if build "succeeds", but is incomplete due to missing
steps in the implementation. I'm not going to want to guess which part
is already built or which isn't.

Given that the rewrite is a singularity anyways it doesn't really matter
that we will not be able to bisect problems caused by the build system
across the boundary.

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Thu, Jul 23, 2020 at 11:15:49AM +0200, Peter Krempa wrote:
> On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> 
> With the most recent version I've fetched from your repository I'm
> seeing the following test failure when building with 'gcc'. 'clang'
> builds fine.

Dan already reported this issue. It happens with new meson 0.55.0. They
changed the name of private target directory where object file are
created which we need for the protocol tests.

Unfortunately there is no way how to get this directory using meson
functions so it is hard-coded in the check-remote-protocol.py.

I've already fixed it locally by changing the code to detect both
name of the private directories, but I'll create an RFE in meson to
provide a way to get the correct path to the private directory.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:

With the most recent version I've fetched from your repository I'm
seeing the following test failure when building with 'gcc'. 'clang'
builds fine.

The output from the failed tests:

 11/154 check-virnetprotocolFAIL   0.09s (exit status 1)

--- command ---
09:13:39 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /bin/python3 
/home/pipo/libvirt/scripts/check-remote-protocol.py virnetprotocol virt_net_rpc 
/home/pipo/build/libvirt/gcc/src/rpc /bin/pdwtags 
/home/pipo/build/libvirt/gcc/../../../libvirt/src/virnetprotocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 55, in 

objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 50, in 
get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_net_rpc@.*' in 
'/home/pipo/build/libvirt/gcc/src/rpc'
---

 12/154 check-virkeepaliveprotocol  FAIL   0.09s (exit status 1)

--- command ---
09:13:39 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /bin/python3 
/home/pipo/libvirt/scripts/check-remote-protocol.py virkeepaliveprotocol 
virt_net_rpc /home/pipo/build/libvirt/gcc/src/rpc /bin/pdwtags 
/home/pipo/build/libvirt/gcc/../../../libvirt/src/virkeepaliveprotocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 55, in 

objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 50, in 
get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_net_rpc@.*' in 
'/home/pipo/build/libvirt/gcc/src/rpc'
---

 13/154 check-remote_protocol   FAIL   0.09s (exit status 1)

--- command ---
09:13:39 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /bin/python3 
/home/pipo/libvirt/scripts/check-remote-protocol.py remote_protocol 
virt_remote_driver /home/pipo/build/libvirt/gcc/src/remote /bin/pdwtags 
/home/pipo/build/libvirt/gcc/../../../libvirt/src/remote_protocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 55, in 

objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 50, in 
get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_remote_driver@.*' in 
'/home/pipo/build/libvirt/gcc/src/remote'
---

 14/154 check-qemu_protocol FAIL   0.09s (exit status 1)

--- command ---
09:13:39 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /bin/python3 
/home/pipo/libvirt/scripts/check-remote-protocol.py qemu_protocol 
virt_remote_driver /home/pipo/build/libvirt/gcc/src/remote /bin/pdwtags 
/home/pipo/build/libvirt/gcc/../../../libvirt/src/qemu_protocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 55, in 

objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 50, in 
get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_remote_driver@.*' in 
'/home/pipo/build/libvirt/gcc/src/remote'
---

 15/154 check-lxc_protocol  FAIL   0.09s (exit status 1)

--- command ---
09:13:39 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /bin/python3 
/home/pipo/libvirt/scripts/check-remote-protocol.py lxc_protocol 
virt_remote_driver /home/pipo/build/libvirt/gcc/src/remote /bin/pdwtags 
/home/pipo/build/libvirt/gcc/../../../libvirt/src/lxc_protocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 55, in 

objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 50, in 
get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_remote_driver@.*' in 
'/home/pipo/build/libvirt/gcc/src/remote'
---

 16/154 check-admin_protocolFAIL   0.09s (exit status 1)

--- command ---
09:13:39 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /bin/python3 
/home/pipo/libvirt/scripts/check-remote-protocol.py admin_protocol 
virt_admin_driver /home/pipo/build/libvirt/gcc/src/admin /bin/pdwtags 
/home/pipo/build/libvirt/gcc/../../../libvirt/src/admin_protocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/pipo/libvirt/scripts/check-remote-protocol.py", line 55, in 

objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File 

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 05:16:00PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 17, 2020 at 03:51:21PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > > So I was finally able to produce the patches to port libvirt to Meson.
> > > Obviously, it is a lot of changes. It might look that some of the
> > > patches could be squashed together but I would rather have it as
> > > separated as possible to make the review not that difficult.
> > > 
> > > Once we are done with review I suggest to squash all patches to single
> > > patch as it doesn't make sense to keep them separated as it will not be
> > > possible to build complete libvirt code by any of the build systems.
> > > Trying to achieve that would be even more challenging and the review
> > > would me more difficult.
> > > 
> > > The reasoning behind taking this approach is to have 1:1 conversion from
> > > autotools to Meson where each patch removes that part from autotools. It
> > > serves as a check that nothing is skipped and to make sure that the
> > > conversion is complete.
> > > 
> > > As probably most of us know Meson is completely different build system
> > > and one of the most challenging things was to deal with the fact that
> > > meson doesn't allow user functions and that everything has to be defined
> > > before it is used.
> > > 
> > > Patches are available in my Gitlab repo as well:
> > > 
> > > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> > 
> > I compared the contents of config.h and meson-config.h for the
> > before and after state, looking at wha "#define" are present.
> > I couple of problems appear
> > 
> > HAVE_DECL_SEEK_HOLE, HAVE_LIBATTR, and HAVE_LIBUTIL, WITH_PM_UTILS
> > were not set in meson-config.h
> 
> I have also compared symbols in the libvirt.so file
> 
> Run:
> 
>   nm -a libvirt.so.0.6006.0  | cut -c18- | sort > a
> 
> and then diff the autotools vs meson build, and we get
> 
> 1a2,4
> > a admin_protocol.c
> > a admin_server.c
> > a admin_server_dispatch.c
> 
> Dunno why these file names are listed as symbols now

Related to the admin symbols, I incorrectly added libvirt_admin_driver.a
static library into libvirt.so

> 738a742,743
> > d adminNProcs
> > d adminProcs
> 1436a1442
> > d virLogSelf
> 
> Also dunno why we have a couple of new data symbols

All of them related to the admin issue.

> 6715a6763,6794
> > t adminClientClose
> > t adminClientGetInfo
> > t adminClientGetInfo.cold
> > t adminConnectListServers
> > t adminConnectLookupServer
> > t adminDispatchClientCloseHelper
> > t adminDispatchClientGetInfoHelper
> > t adminDispatchConnectCloseHelper
> > t adminDispatchConnectGetLibVersionHelper
> > t adminDispatchConnectGetLoggingFiltersHelper
> > t adminDispatchConnectGetLoggingOutputsHelper
> > t adminDispatchConnectListServersHelper
> > t adminDispatchConnectLookupServerHelper
> > t adminDispatchConnectOpenHelper
> > t adminDispatchConnectSetLoggingFiltersHelper
> > t adminDispatchConnectSetLoggingOutputsHelper
> > t adminDispatchServerGetClientLimitsHelper
> > t adminDispatchServerGetThreadpoolParametersHelper
> > t adminDispatchServerListClientsHelper
> > t adminDispatchServerLookupClientHelper
> > t adminDispatchServerSetClientLimitsHelper
> > t adminDispatchServerSetThreadpoolParametersHelper
> > t adminDispatchServerUpdateTlsFilesHelper
> > t adminServerGetClientLimits
> > t adminServerGetClientLimits.cold
> > t adminServerGetThreadPoolParameters
> > t adminServerGetThreadPoolParameters.cold
> > t adminServerListClients
> > t adminServerLookupClient
> > t adminServerSetClientLimits
> > t adminServerSetThreadPoolParameters
> > t adminServerUpdateTlsFiles
> 8392a8472
> > t make_nonnull_client
> 8525a8606,8609
> > t remoteAdmClientFree
> > t remoteAdmClientNew
> > t remoteAdmClientNewPostExecRestart
> > t remoteAdmClientPreExecRestart
> 
> These are strange, as the admin stuff should be in
> libvirt-admin.so instead. Did some files get built
> into the wrong binary ?

As explained above, incorrect static library was linked into libvirt.so.

I'll fix it and push into gitlab.

> 12218a12303
> > t virFileActivateDirOverrideForProg.cold
> 
> I guess this is new ?

No, my guess is that this is related to the rewrite by patch:

meson: src/util/virfile: rewrite virFileActivateDirOverrideForProg


> 14125,14126d14209
> < t virNodeSuspendSupportsTarget
> < t virNodeSuspendSupportsTarget.cold
> 
> I think this is might be because meson failed to detect
> pm-utils which I already reported.

Already solved in different thread.

[...]

> 17039a17155
> > U g_getenv
> 
> Seems due to a code change

Correct, patch changed the code of virFileActivateDirOverrideForProg:

meson: src/util/virfile: rewrite virFileActivateDirOverrideForProg

> 16961c17077
> < U fcntl@@GLIBC_2.2.5
> ---
> > U fcntl64@@GLIBC_2.28
> 16971c17087
> < U fopen@@GLIBC_2.2.5
> ---
> > U fopen64@@GLIBC_2.2.5
> 16980,16981c17096,17097
> < U ftruncate@@GLIBC_2.2.5
> < U 

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Fri, Jul 17, 2020 at 07:50:35PM +0200, Pavel Hrdina wrote:
> On Fri, Jul 17, 2020 at 03:51:21PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > > So I was finally able to produce the patches to port libvirt to Meson.
> > > Obviously, it is a lot of changes. It might look that some of the
> > > patches could be squashed together but I would rather have it as
> > > separated as possible to make the review not that difficult.
> > > 
> > > Once we are done with review I suggest to squash all patches to single
> > > patch as it doesn't make sense to keep them separated as it will not be
> > > possible to build complete libvirt code by any of the build systems.
> > > Trying to achieve that would be even more challenging and the review
> > > would me more difficult.
> > > 
> > > The reasoning behind taking this approach is to have 1:1 conversion from
> > > autotools to Meson where each patch removes that part from autotools. It
> > > serves as a check that nothing is skipped and to make sure that the
> > > conversion is complete.
> > > 
> > > As probably most of us know Meson is completely different build system
> > > and one of the most challenging things was to deal with the fact that
> > > meson doesn't allow user functions and that everything has to be defined
> > > before it is used.
> > > 
> > > Patches are available in my Gitlab repo as well:
> > > 
> > > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> > 
> > I compared the contents of config.h and meson-config.h for the
> > before and after state, looking at wha "#define" are present.
> > I couple of problems appear
> > 
> > HAVE_DECL_SEEK_HOLE, HAVE_LIBATTR, and HAVE_LIBUTIL, WITH_PM_UTILS
> > were not set in meson-config.h
> 
> Nice catch, I actually had a code to define HAVE_DECL_SEEK_HOLE but it
> requires -D_GNU_SOURCE to detect it.
> 
> For some reason I missed HAVE_LIBATTR.
> 
> We don't use HAVE_LIBUTIL anywhere in the code, only HAVE_LIBUTIL_H
> which is in meson-config.h.

We do AC_CHECK_LIB([util],[openpty],[]) which I think auto-adds
"-lutil" to the global $LIBS, only if needed. So yeah, the HAVE_LIBUTIL
in meson-config.h is not required, as long as we're adding -lutil when
needed in $LIBS.


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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 07:00:07PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 17, 2020 at 07:50:35PM +0200, Pavel Hrdina wrote:
> > On Fri, Jul 17, 2020 at 03:51:21PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > > > So I was finally able to produce the patches to port libvirt to Meson.
> > > > Obviously, it is a lot of changes. It might look that some of the
> > > > patches could be squashed together but I would rather have it as
> > > > separated as possible to make the review not that difficult.
> > > > 
> > > > Once we are done with review I suggest to squash all patches to single
> > > > patch as it doesn't make sense to keep them separated as it will not be
> > > > possible to build complete libvirt code by any of the build systems.
> > > > Trying to achieve that would be even more challenging and the review
> > > > would me more difficult.
> > > > 
> > > > The reasoning behind taking this approach is to have 1:1 conversion from
> > > > autotools to Meson where each patch removes that part from autotools. It
> > > > serves as a check that nothing is skipped and to make sure that the
> > > > conversion is complete.
> > > > 
> > > > As probably most of us know Meson is completely different build system
> > > > and one of the most challenging things was to deal with the fact that
> > > > meson doesn't allow user functions and that everything has to be defined
> > > > before it is used.
> > > > 
> > > > Patches are available in my Gitlab repo as well:
> > > > 
> > > > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> > > 
> > > I compared the contents of config.h and meson-config.h for the
> > > before and after state, looking at wha "#define" are present.
> > > I couple of problems appear
> > > 
> > > HAVE_DECL_SEEK_HOLE, HAVE_LIBATTR, and HAVE_LIBUTIL, WITH_PM_UTILS
> > > were not set in meson-config.h
> > 
> > Nice catch, I actually had a code to define HAVE_DECL_SEEK_HOLE but it
> > requires -D_GNU_SOURCE to detect it.
> > 
> > For some reason I missed HAVE_LIBATTR.
> > 
> > We don't use HAVE_LIBUTIL anywhere in the code, only HAVE_LIBUTIL_H
> > which is in meson-config.h.
> 
> We do AC_CHECK_LIB([util],[openpty],[]) which I think auto-adds
> "-lutil" to the global $LIBS, only if needed. So yeah, the HAVE_LIBUTIL
> in meson-config.h is not required, as long as we're adding -lutil when
> needed in $LIBS.

meson.build:1377 is where we check for -lutil and if the library exists
it's added to link arguments so it is covered.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 03:51:21PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > So I was finally able to produce the patches to port libvirt to Meson.
> > Obviously, it is a lot of changes. It might look that some of the
> > patches could be squashed together but I would rather have it as
> > separated as possible to make the review not that difficult.
> > 
> > Once we are done with review I suggest to squash all patches to single
> > patch as it doesn't make sense to keep them separated as it will not be
> > possible to build complete libvirt code by any of the build systems.
> > Trying to achieve that would be even more challenging and the review
> > would me more difficult.
> > 
> > The reasoning behind taking this approach is to have 1:1 conversion from
> > autotools to Meson where each patch removes that part from autotools. It
> > serves as a check that nothing is skipped and to make sure that the
> > conversion is complete.
> > 
> > As probably most of us know Meson is completely different build system
> > and one of the most challenging things was to deal with the fact that
> > meson doesn't allow user functions and that everything has to be defined
> > before it is used.
> > 
> > Patches are available in my Gitlab repo as well:
> > 
> > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> 
> I compared the contents of config.h and meson-config.h for the
> before and after state, looking at wha "#define" are present.
> I couple of problems appear
> 
> HAVE_DECL_SEEK_HOLE, HAVE_LIBATTR, and HAVE_LIBUTIL, WITH_PM_UTILS
> were not set in meson-config.h

Nice catch, I actually had a code to define HAVE_DECL_SEEK_HOLE but it
requires -D_GNU_SOURCE to detect it.

For some reason I missed HAVE_LIBATTR.

We don't use HAVE_LIBUTIL anywhere in the code, only HAVE_LIBUTIL_H
which is in meson-config.h.

WITH_PM_UTILS is actually broken in autotools and it should not be
there. If you check m4/virt-pm-utils.m4 we don't use pm-utils if
building on host with D-Bus and systemd.

Digging into the history commit 
removed init_systemd but forgot to update m4/virt-pm-utils.m4 to use 
"$with_init_script" = "systemd" instead of "$init_systemd" = "yes".

I'll fix the first two defines but the other two are correct unless I
should add the not-used HAVE_LIBUTIL.

Thanks for the review.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Martin Kletzander]

On Fri, Jul 17, 2020 at 03:12:00PM +0100, Daniel P. Berrangé wrote:

On Fri, Jul 17, 2020 at 04:10:01PM +0200, Pavel Hrdina wrote:

On Fri, Jul 17, 2020 at 03:02:10PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > Personally I'd really like to avoid squashing them, because splitting
> > > up big patches is not merely to benefit the initial pre-merge review,
> > > but to also benefit people who need to debug stuff that's already
> > > merged and understand the scope of the intended change. So being able
> > > to look back at the changes in isolation after commit is still a big
> > > plus point.
> >
> > I would like to avoid squashing the patches as well and in most cases I
> > would object to it as well. I only suggested that to not break git
> > bisect.
> >
> > If we don't care about git bisect and the fact that we would not be able
> > to build libvirt correctly within these patches I'm OK with pushing it
> > without squashing.
>
> git bisect reliabity is key, so I reluctantly think we'll need to
> squash. I don't want to hit a pathc in this series with a bisect
> and be unable to continue the bisect due to inability to build the
> code.

I can try to rearrange the patches to not break git bisect. It will
still require some script to be used for git bisect to detect if it
should run autotools or Meson.


Maybe there's a reasonable tradeoff - instead of a 350 patch series,
just a 10-20 patch series.



One other option would be a semi-linear merge. bisect would try the commit
before the rewrite and after, if both of them worked or both were broken then it
will not try the commits in the middle.  If it does, then you know it was
because of the autotools=>meson rewrite.  You will not break git-bisect and
you'll keep the history of the commits.


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 :|



signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 03:28:52PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > So I was finally able to produce the patches to port libvirt to Meson.
> > Obviously, it is a lot of changes. It might look that some of the
> > patches could be squashed together but I would rather have it as
> > separated as possible to make the review not that difficult.
> > 
> > Once we are done with review I suggest to squash all patches to single
> > patch as it doesn't make sense to keep them separated as it will not be
> > possible to build complete libvirt code by any of the build systems.
> > Trying to achieve that would be even more challenging and the review
> > would me more difficult.
> > 
> > The reasoning behind taking this approach is to have 1:1 conversion from
> > autotools to Meson where each patch removes that part from autotools. It
> > serves as a check that nothing is skipped and to make sure that the
> > conversion is complete.
> > 
> > As probably most of us know Meson is completely different build system
> > and one of the most challenging things was to deal with the fact that
> > meson doesn't allow user functions and that everything has to be defined
> > before it is used.
> > 
> > Patches are available in my Gitlab repo as well:
> > 
> > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> > 
> > and link to Giltab pipeline:
> > 
> > https://gitlab.com/phrdina/libvirt/-/pipelines/167276632
> 
> FWIW, some unit tests fail for me - Fedora 31, with pip installed meson 0.55
> 
>  11/154 check-virnetprotocolFAIL   0.09s (exit status 1)

Right, they changed the name of internal temporary directories where
the object files are stored. They are required for pdwtags so I need to
figure out a better way how to get the object file path.

I knew that it was not that robust, sigh.

Thanks

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 06:14:13PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 17, 2020 at 07:00:44PM +0200, Pavel Hrdina wrote:
> > On Fri, Jul 17, 2020 at 03:24:38PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > > > So I was finally able to produce the patches to port libvirt to Meson.
> > > > Obviously, it is a lot of changes. It might look that some of the
> > > > patches could be squashed together but I would rather have it as
> > > > separated as possible to make the review not that difficult.
> > > 
> > > Some things I noticed, building on Fedora 31, with meson 0.55 from pip
> > > 
> > > A bunch of warnings from meson:
> > > 
> > > WARNING: custom_target 'virtesxgen' has more than one output! Using the 
> > > first one.
> > > WARNING: custom_target 'virthypervgen' has more than one output! Using 
> > > the first one.
> > > WARNING: custom_target 'protocol.h' has more than one output! Using the 
> > > first one.
> > > WARNING: custom_target 'generate-api' has more than one output! Using the 
> > > first one.
> > > WARNING: custom_target 'index-api' has more than one output! Using the 
> > > first one.
> > > WARNING: custom_target 'index-admin-api' has more than one output! Using 
> > > the first one.
> > > WARNING: custom_target 'index-lxc-api' has more than one output! Using 
> > > the first one.
> > > WARNING: custom_target 'index-qemu-api' has more than one output! Using 
> > > the first one.
> > 
> > So it fails with meson 0.55.0 on my Fedora 32 as well. I bisected Meson
> > repository and this commit 
> > breaks the behavior so I believe it's a bug in Meson.
> > 
> > The warning is generated by any custom_target that has multiple output
> > files which is obviously incorrect as documentation [1] states that:
> > 
> > * output: list of output files
> > 
> > I created an issue on github for this warning [2].
> > 
> > > During build, a bunch of bogus automake style progress messages:
> > > 
> > > 
> > >  Generating virtesxgen with a custom command
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typedef
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typeenum
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typetostring
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typefromstring
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.h
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.c
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.h
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.c
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.macro
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.h
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.c
> > > 
> > > Generating virthypervgen with a custom command
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.typedef
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.h
> > >   GEN  
> > > /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.c
> > 
> > These are generated by scripts:
> > 
> > scripts/esx_vi_generator.py
> > scripts/hyperv_wmi_generator.py
> > 
> > I was ignoring these outputs but I agree that it would be probably nice
> > to remote it. I'll fix it and push fixed version into gitlab.
> > 
> > > And one problem I think is unrelated / pre-existing, but lost in the noise
> > > of automake:
> > > 
> > > ../tests/qemuxml2xmltest.c: In function ‘mymain’:
> > > ../tests/qemuxml2xmltest.c:132:1: note: variable tracking size limit 
> > > exceeded with ‘-fvar-tracking-assignments’, retrying without
> > >   132 | mymain(void)
> > >   | ^~
> > 
> > It is pre-existing so not addressing it right now as it's unrelated.
> > 
> > > The conversion has introduced 9 new shell scripts in scripts/.
> > > 
> > > IMHO, all of these need to be python scripts instead to follow out intent
> > > to standardize on python. I dream of a world with zero shell scripts.
> > 
> > Sure, works for me. My idea was that these scripts are super simple and
> > python would be overkill but I'm OK with using python. I'll rewrite it
> > and push into gitlab.
> 
> Even though they are simple, we always have the trapdoor of bash vs
> non-bash, people always mess up  == vs = for example. So using python
> gives us stronger portability.
> 
> It would in fact let us build natively on Windows (except for all the
> other stuff that would break in the actual code if we didn't use mingw :-)

Make sense, the only one 

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Fri, Jul 17, 2020 at 07:00:44PM +0200, Pavel Hrdina wrote:
> On Fri, Jul 17, 2020 at 03:24:38PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > > So I was finally able to produce the patches to port libvirt to Meson.
> > > Obviously, it is a lot of changes. It might look that some of the
> > > patches could be squashed together but I would rather have it as
> > > separated as possible to make the review not that difficult.
> > 
> > Some things I noticed, building on Fedora 31, with meson 0.55 from pip
> > 
> > A bunch of warnings from meson:
> > 
> > WARNING: custom_target 'virtesxgen' has more than one output! Using the 
> > first one.
> > WARNING: custom_target 'virthypervgen' has more than one output! Using the 
> > first one.
> > WARNING: custom_target 'protocol.h' has more than one output! Using the 
> > first one.
> > WARNING: custom_target 'generate-api' has more than one output! Using the 
> > first one.
> > WARNING: custom_target 'index-api' has more than one output! Using the 
> > first one.
> > WARNING: custom_target 'index-admin-api' has more than one output! Using 
> > the first one.
> > WARNING: custom_target 'index-lxc-api' has more than one output! Using the 
> > first one.
> > WARNING: custom_target 'index-qemu-api' has more than one output! Using the 
> > first one.
> 
> So it fails with meson 0.55.0 on my Fedora 32 as well. I bisected Meson
> repository and this commit 
> breaks the behavior so I believe it's a bug in Meson.
> 
> The warning is generated by any custom_target that has multiple output
> files which is obviously incorrect as documentation [1] states that:
> 
> * output: list of output files
> 
> I created an issue on github for this warning [2].
> 
> > During build, a bunch of bogus automake style progress messages:
> > 
> > 
> >  Generating virtesxgen with a custom command
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typedef
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typeenum
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typetostring
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typefromstring
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.h
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.c
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.h
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.c
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.macro
> >   GEN  /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.h
> >   GEN  /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.c
> > 
> > Generating virthypervgen with a custom command
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.typedef
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.h
> >   GEN  
> > /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.c
> 
> These are generated by scripts:
> 
> scripts/esx_vi_generator.py
> scripts/hyperv_wmi_generator.py
> 
> I was ignoring these outputs but I agree that it would be probably nice
> to remote it. I'll fix it and push fixed version into gitlab.
> 
> > And one problem I think is unrelated / pre-existing, but lost in the noise
> > of automake:
> > 
> > ../tests/qemuxml2xmltest.c: In function ‘mymain’:
> > ../tests/qemuxml2xmltest.c:132:1: note: variable tracking size limit 
> > exceeded with ‘-fvar-tracking-assignments’, retrying without
> >   132 | mymain(void)
> >   | ^~
> 
> It is pre-existing so not addressing it right now as it's unrelated.
> 
> > The conversion has introduced 9 new shell scripts in scripts/.
> > 
> > IMHO, all of these need to be python scripts instead to follow out intent
> > to standardize on python. I dream of a world with zero shell scripts.
> 
> Sure, works for me. My idea was that these scripts are super simple and
> python would be overkill but I'm OK with using python. I'll rewrite it
> and push into gitlab.

Even though they are simple, we always have the trapdoor of bash vs
non-bash, people always mess up  == vs = for example. So using python
gives us stronger portability.

It would in fact let us build natively on Windows (except for all the
other stuff that would break in the actual code if we didn't use mingw :-)

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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 05:03:31PM +0100, Daniel P. Berrangé wrote:
> On Fri, Jul 17, 2020 at 05:49:42PM +0200, Pavel Hrdina wrote:
> > On Fri, Jul 17, 2020 at 04:53:28PM +0200, Peter Krempa wrote:
> 
> > > usr/share/doc/libvirt-docs/README is a symlink with meson ... the
> > > automake version
> > > doesn't differ. Is it even worth having them?
> > 
> > The symlink is also in our repository, I was not able to find any rule
> > in autoconf that would do anything with it. The difference exists in
> > the tarballs so it's some autotools magic that I'm not willing to
> > investigate.
> 
> IIRC, the README -> README.md symlink just exists because automake
> really wants a file exactly called README. I say we just drop this
> symlink. Anyone is clever enough to find README.md.

Agreed, but I would like to do it in different series to not complicate
the Meson rewrite.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 03:24:38PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > So I was finally able to produce the patches to port libvirt to Meson.
> > Obviously, it is a lot of changes. It might look that some of the
> > patches could be squashed together but I would rather have it as
> > separated as possible to make the review not that difficult.
> 
> Some things I noticed, building on Fedora 31, with meson 0.55 from pip
> 
> A bunch of warnings from meson:
> 
> WARNING: custom_target 'virtesxgen' has more than one output! Using the first 
> one.
> WARNING: custom_target 'virthypervgen' has more than one output! Using the 
> first one.
> WARNING: custom_target 'protocol.h' has more than one output! Using the first 
> one.
> WARNING: custom_target 'generate-api' has more than one output! Using the 
> first one.
> WARNING: custom_target 'index-api' has more than one output! Using the first 
> one.
> WARNING: custom_target 'index-admin-api' has more than one output! Using the 
> first one.
> WARNING: custom_target 'index-lxc-api' has more than one output! Using the 
> first one.
> WARNING: custom_target 'index-qemu-api' has more than one output! Using the 
> first one.

So it fails with meson 0.55.0 on my Fedora 32 as well. I bisected Meson
repository and this commit 
breaks the behavior so I believe it's a bug in Meson.

The warning is generated by any custom_target that has multiple output
files which is obviously incorrect as documentation [1] states that:

* output: list of output files

I created an issue on github for this warning [2].

> During build, a bunch of bogus automake style progress messages:
> 
> 
>  Generating virtesxgen with a custom command
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typedef
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typeenum
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typetostring
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typefromstring
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.h
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.c
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.h
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.c
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.macro
>   GEN  /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.h
>   GEN  /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.c
> 
> Generating virthypervgen with a custom command
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.typedef
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.h
>   GEN  
> /home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.c

These are generated by scripts:

scripts/esx_vi_generator.py
scripts/hyperv_wmi_generator.py

I was ignoring these outputs but I agree that it would be probably nice
to remote it. I'll fix it and push fixed version into gitlab.

> And one problem I think is unrelated / pre-existing, but lost in the noise
> of automake:
> 
> ../tests/qemuxml2xmltest.c: In function ‘mymain’:
> ../tests/qemuxml2xmltest.c:132:1: note: variable tracking size limit exceeded 
> with ‘-fvar-tracking-assignments’, retrying without
>   132 | mymain(void)
>   | ^~

It is pre-existing so not addressing it right now as it's unrelated.

> The conversion has introduced 9 new shell scripts in scripts/.
> 
> IMHO, all of these need to be python scripts instead to follow out intent
> to standardize on python. I dream of a world with zero shell scripts.

Sure, works for me. My idea was that these scripts are super simple and
python would be overkill but I'm OK with using python. I'll rewrite it
and push into gitlab.

Thanks for the review.

Pavel

[1] 
[2] 


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Fri, Jul 17, 2020 at 03:51:21PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > So I was finally able to produce the patches to port libvirt to Meson.
> > Obviously, it is a lot of changes. It might look that some of the
> > patches could be squashed together but I would rather have it as
> > separated as possible to make the review not that difficult.
> > 
> > Once we are done with review I suggest to squash all patches to single
> > patch as it doesn't make sense to keep them separated as it will not be
> > possible to build complete libvirt code by any of the build systems.
> > Trying to achieve that would be even more challenging and the review
> > would me more difficult.
> > 
> > The reasoning behind taking this approach is to have 1:1 conversion from
> > autotools to Meson where each patch removes that part from autotools. It
> > serves as a check that nothing is skipped and to make sure that the
> > conversion is complete.
> > 
> > As probably most of us know Meson is completely different build system
> > and one of the most challenging things was to deal with the fact that
> > meson doesn't allow user functions and that everything has to be defined
> > before it is used.
> > 
> > Patches are available in my Gitlab repo as well:
> > 
> > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> 
> I compared the contents of config.h and meson-config.h for the
> before and after state, looking at wha "#define" are present.
> I couple of problems appear
> 
> HAVE_DECL_SEEK_HOLE, HAVE_LIBATTR, and HAVE_LIBUTIL, WITH_PM_UTILS
> were not set in meson-config.h

I have also compared symbols in the libvirt.so file

Run:

  nm -a libvirt.so.0.6006.0  | cut -c18- | sort > a

and then diff the autotools vs meson build, and we get

1a2,4
> a admin_protocol.c
> a admin_server.c
> a admin_server_dispatch.c

Dunno why these file names are listed as symbols now

738a742,743
> d adminNProcs
> d adminProcs
1436a1442
> d virLogSelf

Also dunno why we have a couple of new data symbols


6715a6763,6794
> t adminClientClose
> t adminClientGetInfo
> t adminClientGetInfo.cold
> t adminConnectListServers
> t adminConnectLookupServer
> t adminDispatchClientCloseHelper
> t adminDispatchClientGetInfoHelper
> t adminDispatchConnectCloseHelper
> t adminDispatchConnectGetLibVersionHelper
> t adminDispatchConnectGetLoggingFiltersHelper
> t adminDispatchConnectGetLoggingOutputsHelper
> t adminDispatchConnectListServersHelper
> t adminDispatchConnectLookupServerHelper
> t adminDispatchConnectOpenHelper
> t adminDispatchConnectSetLoggingFiltersHelper
> t adminDispatchConnectSetLoggingOutputsHelper
> t adminDispatchServerGetClientLimitsHelper
> t adminDispatchServerGetThreadpoolParametersHelper
> t adminDispatchServerListClientsHelper
> t adminDispatchServerLookupClientHelper
> t adminDispatchServerSetClientLimitsHelper
> t adminDispatchServerSetThreadpoolParametersHelper
> t adminDispatchServerUpdateTlsFilesHelper
> t adminServerGetClientLimits
> t adminServerGetClientLimits.cold
> t adminServerGetThreadPoolParameters
> t adminServerGetThreadPoolParameters.cold
> t adminServerListClients
> t adminServerLookupClient
> t adminServerSetClientLimits
> t adminServerSetThreadPoolParameters
> t adminServerUpdateTlsFiles
8392a8472
> t make_nonnull_client
8525a8606,8609
> t remoteAdmClientFree
> t remoteAdmClientNew
> t remoteAdmClientNewPostExecRestart
> t remoteAdmClientPreExecRestart

These are strange, as the admin stuff should be in
libvirt-admin.so instead. Did some files get built
into the wrong binary ?

12218a12303
> t virFileActivateDirOverrideForProg.cold

I guess this is new ?


14125,14126d14209
< t virNodeSuspendSupportsTarget
< t virNodeSuspendSupportsTarget.cold

I think this is might be because meson failed to detect
pm-utils which I already reported.

16108a16192,16224
> T xdr_admin_client_close_args
> T xdr_admin_client_get_info_args
> T xdr_admin_client_get_info_ret
> T xdr_admin_connect_get_lib_version_ret
> T xdr_admin_connect_get_logging_filters_args
> T xdr_admin_connect_get_logging_filters_ret
> T xdr_admin_connect_get_logging_outputs_args
> T xdr_admin_connect_get_logging_outputs_ret
> T xdr_admin_connect_list_servers_args
> T xdr_admin_connect_list_servers_ret
> T xdr_admin_connect_lookup_server_args
> T xdr_admin_connect_lookup_server_ret
> T xdr_admin_connect_open_args
> T xdr_admin_connect_set_logging_filters_args
> T xdr_admin_connect_set_logging_outputs_args
> T xdr_admin_nonnull_client
> T xdr_admin_nonnull_server
> T xdr_admin_nonnull_string
> T xdr_admin_procedure
> T xdr_admin_server_get_client_limits_args
> T xdr_admin_server_get_client_limits_ret
> T xdr_admin_server_get_threadpool_parameters_args
> T xdr_admin_server_get_threadpool_parameters_ret
> T xdr_admin_server_list_clients_args
> T xdr_admin_server_list_clients_ret
> T xdr_admin_server_lookup_client_args
> T xdr_admin_server_lookup_client_ret
> T 

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Fri, Jul 17, 2020 at 05:49:42PM +0200, Pavel Hrdina wrote:
> On Fri, Jul 17, 2020 at 04:53:28PM +0200, Peter Krempa wrote:

> > usr/share/doc/libvirt-docs/README is a symlink with meson ... the
> > automake version
> > doesn't differ. Is it even worth having them?
> 
> The symlink is also in our repository, I was not able to find any rule
> in autoconf that would do anything with it. The difference exists in
> the tarballs so it's some autotools magic that I'm not willing to
> investigate.

IIRC, the README -> README.md symlink just exists because automake
really wants a file exactly called README. I say we just drop this
symlink. Anyone is clever enough to find README.md.


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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 04:18:52PM +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > 
> > I've tried building RPMs both from the pre-patch tree state and after
> > your patchset. I've then extracted all the RPMs toghether and compared
> > the file lists (minus the 'build-id' directory which differs).
> > 
> > I'm not sure whether it's due to the RPM or regular build process
> > though.
> 
> I've also started to verify file differences:
> 
> config files which are installed in /etc/libvirt from said RPMs were not
> post-processed.
> 
> E.g. virtnetworkd.conf:
> 
> The meson-originated version has:
> 
> # Set the name of the directory in which sockets will be found/created.
> #
> # This setting is not required or honoured if using systemd socket
> # activation with systemd version >= 227
> #
> #unix_sock_dir = "@runstatedir@/libvirt"
> 
> while automake:
> 
> # Set the name of the directory in which sockets will be found/created.
> #
> # This setting is not required or honoured if using systemd socket
> # activation with systemd version >= 227
> #
> #unix_sock_dir = "/run/libvirt"

So it's actually the other way around. I just checked it and with meson
it is expanded correctly. With autotools we have @runstatedir@ and
others in the config files. I also checked the installed files in my
fedora system and they also contain not-expanded strings.

So the meson rewrite fixes the issue that we have with autotools.

So I guess no need to do anything here.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 04:53:28PM +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 16:37:45 +0200, Peter Krempa wrote:
> > On Fri, Jul 17, 2020 at 16:31:05 +0200, Peter Krempa wrote:
> > > On Fri, Jul 17, 2020 at 16:18:52 +0200, Peter Krempa wrote:
> > > > On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> > > > > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > > > > 
> > > > > I've tried building RPMs both from the pre-patch tree state and after
> > > > > your patchset. I've then extracted all the RPMs toghether and compared
> > > > > the file lists (minus the 'build-id' directory which differs).
> > > > > 
> > > > > I'm not sure whether it's due to the RPM or regular build process
> > > > > though.
> > > > 
> > > 
> > > Another difference is paths to shared objects:
> > > 
> > > e.g virsh:
> > > 
> > > automakerpm/unpack/usr/bin/virsh:
> > >   linux-vdso.so.1 (0x7ffccb1ea000)
> > >   libvirt-lxc.so.0 => /lib64/libvirt-lxc.so.0 (0x7f51f7082000)
> > >   libvirt-qemu.so.0 => /lib64/libvirt-qemu.so.0 (0x7f51f707d000)
> > >   libvirt.so.0 => /lib64/libvirt.so.0 (0x7f51f6bbd000)
> > >   libxml2.so.2 => /lib64/libxml2.so.2 (0x7f51f6a4d000)
> > >   libreadline.so.8 => /lib64/libreadline.so.8 (0x7f51f69f7000)
> > >   libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x7f51f68cc000)
> > >   libpthread.so.0 => /lib64/libpthread.so.0 (0x7f51f68a8000)
> > > 
> > > mesonrpm/unpack/usr/bin/virsh:
> > >   linux-vdso.so.1 (0x7ffcb4b8d000)
> > >   libvirt-lxc.so.0 => /usr/lib64/libvirt-lxc.so.0 (0x7fc2b9596000)
> > >   libvirt-qemu.so.0 => /usr/lib64/libvirt-qemu.so.0 (0x7fc2b9591000)
> > >   libvirt.so.0 => /usr/lib64/libvirt.so.0 (0x7fc2b90d1000)
> > >   libxml2.so.2 => /usr/lib64/libxml2.so.2 (0x7fc2b8f61000)
> > >   libglib-2.0.so.0 => /usr/lib64/libglib-2.0.so.0 (0x7fc2b8e36000)
> > >   libreadline.so.8 => /usr/lib64/libreadline.so.8 (0x7fc2b8de)
> > >   libgcc_s.so.1 => /usr/lib64/libgcc_s.so.1 (0x7fc2b8dc3000)
> > > 
> > > 
> > > This one is probably safe and also possibly considered modern, but I
> > > wanted to point it out.
> > > 
> > > One mistake though is that usr/bin/virt-xml-validate is not installed as
> > > executalbe with meson!
> > 
> > Also virt-pki-validate
> > 
> 
> And virt-sanlock-cleanup in usr/sbin/

Already fixed, thanks.

> usr/share/doc/libvirt-docs/README is a symlink with meson ... the
> automake version
> doesn't differ. Is it even worth having them?

The symlink is also in our repository, I was not able to find any rule
in autoconf that would do anything with it. The difference exists in
the tarballs so it's some autotools magic that I'm not willing to
investigate.

> then
> 
> usr/share/doc/libvirt-docs/AUTHORS '#contributorslist#' is not expanded
> with meson

Fixed, thanks, it has to be '@contributorslist@' to expand it with
meson.

> I obviously could not verify binaries, but the files are at least
> present :)

Thanks for the feedback.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 04:37:45PM +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 16:31:05 +0200, Peter Krempa wrote:
> > On Fri, Jul 17, 2020 at 16:18:52 +0200, Peter Krempa wrote:
> > > On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> > > > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > > > 
> > > > I've tried building RPMs both from the pre-patch tree state and after
> > > > your patchset. I've then extracted all the RPMs toghether and compared
> > > > the file lists (minus the 'build-id' directory which differs).
> > > > 
> > > > I'm not sure whether it's due to the RPM or regular build process
> > > > though.
> > > 
> > 
> > Another difference is paths to shared objects:
> > 
> > e.g virsh:
> > 
> > automakerpm/unpack/usr/bin/virsh:
> > linux-vdso.so.1 (0x7ffccb1ea000)
> > libvirt-lxc.so.0 => /lib64/libvirt-lxc.so.0 (0x7f51f7082000)
> > libvirt-qemu.so.0 => /lib64/libvirt-qemu.so.0 (0x7f51f707d000)
> > libvirt.so.0 => /lib64/libvirt.so.0 (0x7f51f6bbd000)
> > libxml2.so.2 => /lib64/libxml2.so.2 (0x7f51f6a4d000)
> > libreadline.so.8 => /lib64/libreadline.so.8 (0x7f51f69f7000)
> > libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x7f51f68cc000)
> > libpthread.so.0 => /lib64/libpthread.so.0 (0x7f51f68a8000)
> > 
> > mesonrpm/unpack/usr/bin/virsh:
> > linux-vdso.so.1 (0x7ffcb4b8d000)
> > libvirt-lxc.so.0 => /usr/lib64/libvirt-lxc.so.0 (0x7fc2b9596000)
> > libvirt-qemu.so.0 => /usr/lib64/libvirt-qemu.so.0 (0x7fc2b9591000)
> > libvirt.so.0 => /usr/lib64/libvirt.so.0 (0x7fc2b90d1000)
> > libxml2.so.2 => /usr/lib64/libxml2.so.2 (0x7fc2b8f61000)
> > libglib-2.0.so.0 => /usr/lib64/libglib-2.0.so.0 (0x7fc2b8e36000)
> > libreadline.so.8 => /usr/lib64/libreadline.so.8 (0x7fc2b8de)
> > libgcc_s.so.1 => /usr/lib64/libgcc_s.so.1 (0x7fc2b8dc3000)
> > 
> > 
> > This one is probably safe and also possibly considered modern, but I
> > wanted to point it out.
> > 
> > One mistake though is that usr/bin/virt-xml-validate is not installed as
> > executalbe with meson!
> 
> Also virt-pki-validate

And virt-sanlock-cleanup and libvirt-guests.sh. Fixing all of these as
I checked everything that's installed.

Thanks


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Fri, Jul 17, 2020 at 16:37:45 +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 16:31:05 +0200, Peter Krempa wrote:
> > On Fri, Jul 17, 2020 at 16:18:52 +0200, Peter Krempa wrote:
> > > On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> > > > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > > > 
> > > > I've tried building RPMs both from the pre-patch tree state and after
> > > > your patchset. I've then extracted all the RPMs toghether and compared
> > > > the file lists (minus the 'build-id' directory which differs).
> > > > 
> > > > I'm not sure whether it's due to the RPM or regular build process
> > > > though.
> > > 
> > 
> > Another difference is paths to shared objects:
> > 
> > e.g virsh:
> > 
> > automakerpm/unpack/usr/bin/virsh:
> > linux-vdso.so.1 (0x7ffccb1ea000)
> > libvirt-lxc.so.0 => /lib64/libvirt-lxc.so.0 (0x7f51f7082000)
> > libvirt-qemu.so.0 => /lib64/libvirt-qemu.so.0 (0x7f51f707d000)
> > libvirt.so.0 => /lib64/libvirt.so.0 (0x7f51f6bbd000)
> > libxml2.so.2 => /lib64/libxml2.so.2 (0x7f51f6a4d000)
> > libreadline.so.8 => /lib64/libreadline.so.8 (0x7f51f69f7000)
> > libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x7f51f68cc000)
> > libpthread.so.0 => /lib64/libpthread.so.0 (0x7f51f68a8000)
> > 
> > mesonrpm/unpack/usr/bin/virsh:
> > linux-vdso.so.1 (0x7ffcb4b8d000)
> > libvirt-lxc.so.0 => /usr/lib64/libvirt-lxc.so.0 (0x7fc2b9596000)
> > libvirt-qemu.so.0 => /usr/lib64/libvirt-qemu.so.0 (0x7fc2b9591000)
> > libvirt.so.0 => /usr/lib64/libvirt.so.0 (0x7fc2b90d1000)
> > libxml2.so.2 => /usr/lib64/libxml2.so.2 (0x7fc2b8f61000)
> > libglib-2.0.so.0 => /usr/lib64/libglib-2.0.so.0 (0x7fc2b8e36000)
> > libreadline.so.8 => /usr/lib64/libreadline.so.8 (0x7fc2b8de)
> > libgcc_s.so.1 => /usr/lib64/libgcc_s.so.1 (0x7fc2b8dc3000)
> > 
> > 
> > This one is probably safe and also possibly considered modern, but I
> > wanted to point it out.
> > 
> > One mistake though is that usr/bin/virt-xml-validate is not installed as
> > executalbe with meson!
> 
> Also virt-pki-validate
> 

And virt-sanlock-cleanup in usr/sbin/


usr/share/doc/libvirt-docs/README is a symlink with meson ... the
automake version
doesn't differ. Is it even worth having them?

then

usr/share/doc/libvirt-docs/AUTHORS '#contributorslist#' is not expanded
with meson
.

I obviously could not verify binaries, but the files are at least
present :)

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> So I was finally able to produce the patches to port libvirt to Meson.
> Obviously, it is a lot of changes. It might look that some of the
> patches could be squashed together but I would rather have it as
> separated as possible to make the review not that difficult.
> 
> Once we are done with review I suggest to squash all patches to single
> patch as it doesn't make sense to keep them separated as it will not be
> possible to build complete libvirt code by any of the build systems.
> Trying to achieve that would be even more challenging and the review
> would me more difficult.
> 
> The reasoning behind taking this approach is to have 1:1 conversion from
> autotools to Meson where each patch removes that part from autotools. It
> serves as a check that nothing is skipped and to make sure that the
> conversion is complete.
> 
> As probably most of us know Meson is completely different build system
> and one of the most challenging things was to deal with the fact that
> meson doesn't allow user functions and that everything has to be defined
> before it is used.
> 
> Patches are available in my Gitlab repo as well:
> 
> git clone -b meson https://gitlab.com/phrdina/libvirt.git

I compared the contents of config.h and meson-config.h for the
before and after state, looking at wha "#define" are present.
I couple of problems appear

HAVE_DECL_SEEK_HOLE, HAVE_LIBATTR, and HAVE_LIBUTIL, WITH_PM_UTILS
were not set in meson-config.h


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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Fri, Jul 17, 2020 at 16:31:05 +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 16:18:52 +0200, Peter Krempa wrote:
> > On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> > > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > > 
> > > I've tried building RPMs both from the pre-patch tree state and after
> > > your patchset. I've then extracted all the RPMs toghether and compared
> > > the file lists (minus the 'build-id' directory which differs).
> > > 
> > > I'm not sure whether it's due to the RPM or regular build process
> > > though.
> > 
> 
> Another difference is paths to shared objects:
> 
> e.g virsh:
> 
> automakerpm/unpack/usr/bin/virsh:
>   linux-vdso.so.1 (0x7ffccb1ea000)
>   libvirt-lxc.so.0 => /lib64/libvirt-lxc.so.0 (0x7f51f7082000)
>   libvirt-qemu.so.0 => /lib64/libvirt-qemu.so.0 (0x7f51f707d000)
>   libvirt.so.0 => /lib64/libvirt.so.0 (0x7f51f6bbd000)
>   libxml2.so.2 => /lib64/libxml2.so.2 (0x7f51f6a4d000)
>   libreadline.so.8 => /lib64/libreadline.so.8 (0x7f51f69f7000)
>   libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x7f51f68cc000)
>   libpthread.so.0 => /lib64/libpthread.so.0 (0x7f51f68a8000)
> 
> mesonrpm/unpack/usr/bin/virsh:
>   linux-vdso.so.1 (0x7ffcb4b8d000)
>   libvirt-lxc.so.0 => /usr/lib64/libvirt-lxc.so.0 (0x7fc2b9596000)
>   libvirt-qemu.so.0 => /usr/lib64/libvirt-qemu.so.0 (0x7fc2b9591000)
>   libvirt.so.0 => /usr/lib64/libvirt.so.0 (0x7fc2b90d1000)
>   libxml2.so.2 => /usr/lib64/libxml2.so.2 (0x7fc2b8f61000)
>   libglib-2.0.so.0 => /usr/lib64/libglib-2.0.so.0 (0x7fc2b8e36000)
>   libreadline.so.8 => /usr/lib64/libreadline.so.8 (0x7fc2b8de)
>   libgcc_s.so.1 => /usr/lib64/libgcc_s.so.1 (0x7fc2b8dc3000)
> 
> 
> This one is probably safe and also possibly considered modern, but I
> wanted to point it out.
> 
> One mistake though is that usr/bin/virt-xml-validate is not installed as
> executalbe with meson!

Also virt-pki-validate

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 04:31:05PM +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 16:18:52 +0200, Peter Krempa wrote:
> > On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> > > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > > 
> > > I've tried building RPMs both from the pre-patch tree state and after
> > > your patchset. I've then extracted all the RPMs toghether and compared
> > > the file lists (minus the 'build-id' directory which differs).
> > > 
> > > I'm not sure whether it's due to the RPM or regular build process
> > > though.
> > 
> 
> Another difference is paths to shared objects:
> 
> e.g virsh:
> 
> automakerpm/unpack/usr/bin/virsh:
>   linux-vdso.so.1 (0x7ffccb1ea000)
>   libvirt-lxc.so.0 => /lib64/libvirt-lxc.so.0 (0x7f51f7082000)
>   libvirt-qemu.so.0 => /lib64/libvirt-qemu.so.0 (0x7f51f707d000)
>   libvirt.so.0 => /lib64/libvirt.so.0 (0x7f51f6bbd000)
>   libxml2.so.2 => /lib64/libxml2.so.2 (0x7f51f6a4d000)
>   libreadline.so.8 => /lib64/libreadline.so.8 (0x7f51f69f7000)
>   libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x7f51f68cc000)
>   libpthread.so.0 => /lib64/libpthread.so.0 (0x7f51f68a8000)
> 
> mesonrpm/unpack/usr/bin/virsh:
>   linux-vdso.so.1 (0x7ffcb4b8d000)
>   libvirt-lxc.so.0 => /usr/lib64/libvirt-lxc.so.0 (0x7fc2b9596000)
>   libvirt-qemu.so.0 => /usr/lib64/libvirt-qemu.so.0 (0x7fc2b9591000)
>   libvirt.so.0 => /usr/lib64/libvirt.so.0 (0x7fc2b90d1000)
>   libxml2.so.2 => /usr/lib64/libxml2.so.2 (0x7fc2b8f61000)
>   libglib-2.0.so.0 => /usr/lib64/libglib-2.0.so.0 (0x7fc2b8e36000)
>   libreadline.so.8 => /usr/lib64/libreadline.so.8 (0x7fc2b8de)
>   libgcc_s.so.1 => /usr/lib64/libgcc_s.so.1 (0x7fc2b8dc3000)
> 
> 
> This one is probably safe and also possibly considered modern, but I
> wanted to point it out.

This should not be an issue.

> One mistake though is that usr/bin/virt-xml-validate is not installed as
> executalbe with meson!

I'll fix it, thanks.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Fri, Jul 17, 2020 at 16:18:52 +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > 
> > I've tried building RPMs both from the pre-patch tree state and after
> > your patchset. I've then extracted all the RPMs toghether and compared
> > the file lists (minus the 'build-id' directory which differs).
> > 
> > I'm not sure whether it's due to the RPM or regular build process
> > though.
> 

Another difference is paths to shared objects:

e.g virsh:

automakerpm/unpack/usr/bin/virsh:
linux-vdso.so.1 (0x7ffccb1ea000)
libvirt-lxc.so.0 => /lib64/libvirt-lxc.so.0 (0x7f51f7082000)
libvirt-qemu.so.0 => /lib64/libvirt-qemu.so.0 (0x7f51f707d000)
libvirt.so.0 => /lib64/libvirt.so.0 (0x7f51f6bbd000)
libxml2.so.2 => /lib64/libxml2.so.2 (0x7f51f6a4d000)
libreadline.so.8 => /lib64/libreadline.so.8 (0x7f51f69f7000)
libglib-2.0.so.0 => /lib64/libglib-2.0.so.0 (0x7f51f68cc000)
libpthread.so.0 => /lib64/libpthread.so.0 (0x7f51f68a8000)

mesonrpm/unpack/usr/bin/virsh:
linux-vdso.so.1 (0x7ffcb4b8d000)
libvirt-lxc.so.0 => /usr/lib64/libvirt-lxc.so.0 (0x7fc2b9596000)
libvirt-qemu.so.0 => /usr/lib64/libvirt-qemu.so.0 (0x7fc2b9591000)
libvirt.so.0 => /usr/lib64/libvirt.so.0 (0x7fc2b90d1000)
libxml2.so.2 => /usr/lib64/libxml2.so.2 (0x7fc2b8f61000)
libglib-2.0.so.0 => /usr/lib64/libglib-2.0.so.0 (0x7fc2b8e36000)
libreadline.so.8 => /usr/lib64/libreadline.so.8 (0x7fc2b8de)
libgcc_s.so.1 => /usr/lib64/libgcc_s.so.1 (0x7fc2b8dc3000)


This one is probably safe and also possibly considered modern, but I
wanted to point it out.

One mistake though is that usr/bin/virt-xml-validate is not installed as
executalbe with meson!

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> So I was finally able to produce the patches to port libvirt to Meson.
> Obviously, it is a lot of changes. It might look that some of the
> patches could be squashed together but I would rather have it as
> separated as possible to make the review not that difficult.
> 
> Once we are done with review I suggest to squash all patches to single
> patch as it doesn't make sense to keep them separated as it will not be
> possible to build complete libvirt code by any of the build systems.
> Trying to achieve that would be even more challenging and the review
> would me more difficult.
> 
> The reasoning behind taking this approach is to have 1:1 conversion from
> autotools to Meson where each patch removes that part from autotools. It
> serves as a check that nothing is skipped and to make sure that the
> conversion is complete.
> 
> As probably most of us know Meson is completely different build system
> and one of the most challenging things was to deal with the fact that
> meson doesn't allow user functions and that everything has to be defined
> before it is used.
> 
> Patches are available in my Gitlab repo as well:
> 
> git clone -b meson https://gitlab.com/phrdina/libvirt.git
> 
> and link to Giltab pipeline:
> 
> https://gitlab.com/phrdina/libvirt/-/pipelines/167276632

FWIW, some unit tests fail for me - Fedora 31, with pip installed meson 0.55

 11/154 check-virnetprotocolFAIL   0.09s (exit status 1)

--- command ---
14:26:24 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /usr/bin/python3 
/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py virnetprotocol 
virt_net_rpc /home/berrange/src/virt/libvirt/build/src/rpc /usr/bin/pdwtags 
/home/berrange/src/virt/libvirt/build/../src/virnetprotocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
55, in 
objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
50, in get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_net_rpc@.*' in 
'/home/berrange/src/virt/libvirt/build/src/rpc'
---

 12/154 check-virkeepaliveprotocol  FAIL   0.11s (exit status 1)

--- command ---
14:26:24 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /usr/bin/python3 
/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py 
virkeepaliveprotocol virt_net_rpc /home/berrange/src/virt/libvirt/build/src/rpc 
/usr/bin/pdwtags 
/home/berrange/src/virt/libvirt/build/../src/virkeepaliveprotocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
55, in 
objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
50, in get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_net_rpc@.*' in 
'/home/berrange/src/virt/libvirt/build/src/rpc'
---

 13/154 check-remote_protocol   FAIL   0.09s (exit status 1)

--- command ---
14:26:24 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /usr/bin/python3 
/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py 
remote_protocol virt_remote_driver 
/home/berrange/src/virt/libvirt/build/src/remote /usr/bin/pdwtags 
/home/berrange/src/virt/libvirt/build/../src/remote_protocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
55, in 
objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
50, in get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_remote_driver@.*' in 
'/home/berrange/src/virt/libvirt/build/src/remote'
---

 14/154 check-qemu_protocol FAIL   0.11s (exit status 1)

--- command ---
14:26:24 LC_CTYPE='en_US.UTF-8' LC_ALL='' LANG='C' /usr/bin/python3 
/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py qemu_protocol 
virt_remote_driver /home/berrange/src/virt/libvirt/build/src/remote 
/usr/bin/pdwtags 
/home/berrange/src/virt/libvirt/build/../src/qemu_protocol-structs
--- stderr ---
Traceback (most recent call last):
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
55, in 
objectdir = get_subdir(builddir, r'.*@{0}@.*'.format(libname))
  File "/home/berrange/src/virt/libvirt/scripts/check-remote-protocol.py", line 
50, in get_subdir
raise Exception("Failed to find '{0}' in '{1}'".format(subdir, dirname))
Exception: Failed to find '.*@virt_remote_driver@.*' in 

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> So I was finally able to produce the patches to port libvirt to Meson.
> Obviously, it is a lot of changes. It might look that some of the
> patches could be squashed together but I would rather have it as
> separated as possible to make the review not that difficult.

Some things I noticed, building on Fedora 31, with meson 0.55 from pip

A bunch of warnings from meson:

WARNING: custom_target 'virtesxgen' has more than one output! Using the first 
one.
WARNING: custom_target 'virthypervgen' has more than one output! Using the 
first one.
WARNING: custom_target 'protocol.h' has more than one output! Using the first 
one.
WARNING: custom_target 'generate-api' has more than one output! Using the first 
one.
WARNING: custom_target 'index-api' has more than one output! Using the first 
one.
WARNING: custom_target 'index-admin-api' has more than one output! Using the 
first one.
WARNING: custom_target 'index-lxc-api' has more than one output! Using the 
first one.
WARNING: custom_target 'index-qemu-api' has more than one output! Using the 
first one.


During build, a bunch of bogus automake style progress messages:


 Generating virtesxgen with a custom command
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typedef
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typeenum
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typetostring
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.typefromstring
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.h
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_types.generated.c
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.h
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.c
  GEN  
/home/berrange/src/virt/libvirt/build/src/esx/esx_vi_methods.generated.macro
  GEN  /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.h
  GEN  /home/berrange/src/virt/libvirt/build/src/esx/esx_vi.generated.c

Generating virthypervgen with a custom command
  GEN  
/home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.typedef
  GEN  
/home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.h
  GEN  
/home/berrange/src/virt/libvirt/build/src/hyperv/hyperv_wmi_classes.generated.c


And one problem I think is unrelated / pre-existing, but lost in the noise
of automake:

../tests/qemuxml2xmltest.c: In function ‘mymain’:
../tests/qemuxml2xmltest.c:132:1: note: variable tracking size limit exceeded 
with ‘-fvar-tracking-assignments’, retrying without
  132 | mymain(void)
  | ^~


The conversion has introduced 9 new shell scripts in scripts/.

IMHO, all of these need to be python scripts instead to follow out intent
to standardize on python. I dream of a world with zero shell scripts.

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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 04:04:16PM +0200, Peter Krempa wrote:
> On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> 
> I've tried building RPMs both from the pre-patch tree state and after
> your patchset. I've then extracted all the RPMs toghether and compared
> the file lists (minus the 'build-id' directory which differs).
> 
> I'm not sure whether it's due to the RPM or regular build process
> though.

It's because we don't track these files with autotools so there is no
conflict when rebasing this series. I tried to make sure nothing like
this happens by going through all the new commits since the last rebase
but it's easy to miss something. It the missing HTML docs files and XML
CPU files have to be fixed (I'll do it right now and update gitlab).

The qemu probes file location was moved because it is generated in qemu
directory with meson, I will check if we need to update our spec file.

Thanks for the comparison.

Pavel

> Here are differences which should be considered to be addressed:
> 
> Specifically lines with '-' are MISSING from the RPMs obtained by
> building the meson-generated tarball:
> 
> --- files-automake2020-07-17 15:44:29.602038223 +0200
> +++ files-meson   2020-07-17 15:44:38.204937457 +0200
> @@ -1531,8 +1571,6 @@
>  ..
>  backing_chains.html
>  domainstatecapture.html
> -incrementalbackupinternals.html
> -kvm-realtime.html
>  launch_security_sev.html
>  locking.html
>  locking-lockd.html
> @@ -1540,7 +1578,6 @@
>  migrationinternals.html
>  qemu-passthrough-security.html
>  rpm-deployment.html
> -s390_protected_virt.html
>  secureusage.html
>  virtiofs.html
> 
> @@ -1564,6 +1601,8 @@
>  logo-square-powered-96.png
>  logo-square-powered.svg
>  logo-square.svg
> +logo-sticker-hexagon.svg
> +logo-sticker-square.svg
> 
>  unpack/usr/share/doc/libvirt-docs/html/manpages:
>  .
> @@ -1636,7 +1675,6 @@
>  x86_Broadwell-noTSX-IBRS.xml
>  x86_Broadwell-noTSX.xml
>  x86_Broadwell.xml
> -x86_Cascadelake-Server-noTSX.xml
>  x86_Cascadelake-Server.xml
>  x86_Conroe.xml
>  x86_Cooperlake.xml
> @@ -1644,7 +1682,6 @@
>  x86_coreduo.xml
>  x86_cpu64-rhel5.xml
>  x86_cpu64-rhel6.xml
> -x86_Dhyana.xml
>  x86_EPYC-IBPB.xml
>  x86_EPYC.xml
>  x86_features.xml
> @@ -3274,10 +3311,10 @@
>  esx
>  hyperv
>  libvirt_probes.h
> -libvirt_qemu_probes.h
>  locking
>  logging
>  lxc
> +qemu
>  remote
>  rpc
>  util
> @@ -3340,6 +3377,11 @@
>  lxc_monitor_protocol.c
>  lxc_monitor_protocol.h
> 
> +unpack/usr/src/debug/libvirt-6.6.0-1.fc32.x86_64/x86_64-redhat-linux-gnu/src/qemu:
> +.
> +..
> +libvirt_qemu_probes.h
> +
>  
> unpack/usr/src/debug/libvirt-6.6.0-1.fc32.x86_64/x86_64-redhat-linux-gnu/src/remote:
>  .
>  ..
> 
> 
> I'm unsure about the 'probes' file location. The 'kbase' articles are
> unfortunate, but the cpu modes could pose real problems.
> 
> The CPU xml files are from usr/share/libvirt/cpu_map


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Fri, Jul 17, 2020 at 16:04:16 +0200, Peter Krempa wrote:
> On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> 
> I've tried building RPMs both from the pre-patch tree state and after
> your patchset. I've then extracted all the RPMs toghether and compared
> the file lists (minus the 'build-id' directory which differs).
> 
> I'm not sure whether it's due to the RPM or regular build process
> though.

I've also started to verify file differences:

config files which are installed in /etc/libvirt from said RPMs were not
post-processed.

E.g. virtnetworkd.conf:

The meson-originated version has:

# Set the name of the directory in which sockets will be found/created.
#
# This setting is not required or honoured if using systemd socket
# activation with systemd version >= 227
#
#unix_sock_dir = "@runstatedir@/libvirt"

while automake:

# Set the name of the directory in which sockets will be found/created.
#
# This setting is not required or honoured if using systemd socket
# activation with systemd version >= 227
#
#unix_sock_dir = "/run/libvirt"


A non-exhaustive list of differences that I can see:

automakerpm/unpack/etc/libvirt/virtinterfaced.conf
automakerpm/unpack/etc/libvirt/virtlxcd.conf
automakerpm/unpack/etc/libvirt/virtnetworkd.conf
automakerpm/unpack/etc/libvirt/virtnodedevd.conf
automakerpm/unpack/etc/libvirt/virtnwfilterd.conf
automakerpm/unpack/etc/libvirt/virtqemud.conf
automakerpm/unpack/etc/libvirt/virtsecretd.conf
automakerpm/unpack/etc/libvirt/virtstoraged.conf
automakerpm/unpack/etc/libvirt/virtvboxd.conf
automakerpm/unpack/etc/libvirt/virtxend.conf

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Fri, Jul 17, 2020 at 04:10:01PM +0200, Pavel Hrdina wrote:
> On Fri, Jul 17, 2020 at 03:02:10PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > > Personally I'd really like to avoid squashing them, because splitting
> > > > up big patches is not merely to benefit the initial pre-merge review,
> > > > but to also benefit people who need to debug stuff that's already
> > > > merged and understand the scope of the intended change. So being able
> > > > to look back at the changes in isolation after commit is still a big
> > > > plus point.
> > > 
> > > I would like to avoid squashing the patches as well and in most cases I
> > > would object to it as well. I only suggested that to not break git
> > > bisect.
> > > 
> > > If we don't care about git bisect and the fact that we would not be able
> > > to build libvirt correctly within these patches I'm OK with pushing it
> > > without squashing.
> > 
> > git bisect reliabity is key, so I reluctantly think we'll need to
> > squash. I don't want to hit a pathc in this series with a bisect
> > and be unable to continue the bisect due to inability to build the
> > code.
> 
> I can try to rearrange the patches to not break git bisect. It will
> still require some script to be used for git bisect to detect if it
> should run autotools or Meson.

Maybe there's a reasonable tradeoff - instead of a 350 patch series,
just a 10-20 patch series. 

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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 03:02:10PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> > On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > > Personally I'd really like to avoid squashing them, because splitting
> > > up big patches is not merely to benefit the initial pre-merge review,
> > > but to also benefit people who need to debug stuff that's already
> > > merged and understand the scope of the intended change. So being able
> > > to look back at the changes in isolation after commit is still a big
> > > plus point.
> > 
> > I would like to avoid squashing the patches as well and in most cases I
> > would object to it as well. I only suggested that to not break git
> > bisect.
> > 
> > If we don't care about git bisect and the fact that we would not be able
> > to build libvirt correctly within these patches I'm OK with pushing it
> > without squashing.
> 
> git bisect reliabity is key, so I reluctantly think we'll need to
> squash. I don't want to hit a pathc in this series with a bisect
> and be unable to continue the bisect due to inability to build the
> code.

I can try to rearrange the patches to not break git bisect. It will
still require some script to be used for git bisect to detect if it
should run autotools or Meson.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Thu, Jul 16, 2020 at 03:44:25PM +0200, Pavel Hrdina wrote:
> On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> > Personally I'd really like to avoid squashing them, because splitting
> > up big patches is not merely to benefit the initial pre-merge review,
> > but to also benefit people who need to debug stuff that's already
> > merged and understand the scope of the intended change. So being able
> > to look back at the changes in isolation after commit is still a big
> > plus point.
> 
> I would like to avoid squashing the patches as well and in most cases I
> would object to it as well. I only suggested that to not break git
> bisect.
> 
> If we don't care about git bisect and the fact that we would not be able
> to build libvirt correctly within these patches I'm OK with pushing it
> without squashing.

git bisect reliabity is key, so I reluctantly think we'll need to
squash. I don't want to hit a pathc in this series with a bisect
and be unable to continue the bisect due to inability to build the
code.


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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:

I've tried building RPMs both from the pre-patch tree state and after
your patchset. I've then extracted all the RPMs toghether and compared
the file lists (minus the 'build-id' directory which differs).

I'm not sure whether it's due to the RPM or regular build process
though.

Here are differences which should be considered to be addressed:

Specifically lines with '-' are MISSING from the RPMs obtained by
building the meson-generated tarball:

--- files-automake  2020-07-17 15:44:29.602038223 +0200
+++ files-meson 2020-07-17 15:44:38.204937457 +0200
@@ -1531,8 +1571,6 @@
 ..
 backing_chains.html
 domainstatecapture.html
-incrementalbackupinternals.html
-kvm-realtime.html
 launch_security_sev.html
 locking.html
 locking-lockd.html
@@ -1540,7 +1578,6 @@
 migrationinternals.html
 qemu-passthrough-security.html
 rpm-deployment.html
-s390_protected_virt.html
 secureusage.html
 virtiofs.html

@@ -1564,6 +1601,8 @@
 logo-square-powered-96.png
 logo-square-powered.svg
 logo-square.svg
+logo-sticker-hexagon.svg
+logo-sticker-square.svg

 unpack/usr/share/doc/libvirt-docs/html/manpages:
 .
@@ -1636,7 +1675,6 @@
 x86_Broadwell-noTSX-IBRS.xml
 x86_Broadwell-noTSX.xml
 x86_Broadwell.xml
-x86_Cascadelake-Server-noTSX.xml
 x86_Cascadelake-Server.xml
 x86_Conroe.xml
 x86_Cooperlake.xml
@@ -1644,7 +1682,6 @@
 x86_coreduo.xml
 x86_cpu64-rhel5.xml
 x86_cpu64-rhel6.xml
-x86_Dhyana.xml
 x86_EPYC-IBPB.xml
 x86_EPYC.xml
 x86_features.xml
@@ -3274,10 +3311,10 @@
 esx
 hyperv
 libvirt_probes.h
-libvirt_qemu_probes.h
 locking
 logging
 lxc
+qemu
 remote
 rpc
 util
@@ -3340,6 +3377,11 @@
 lxc_monitor_protocol.c
 lxc_monitor_protocol.h

+unpack/usr/src/debug/libvirt-6.6.0-1.fc32.x86_64/x86_64-redhat-linux-gnu/src/qemu:
+.
+..
+libvirt_qemu_probes.h
+
 
unpack/usr/src/debug/libvirt-6.6.0-1.fc32.x86_64/x86_64-redhat-linux-gnu/src/remote:
 .
 ..


I'm unsure about the 'probes' file location. The 'kbase' articles are
unfortunate, but the cpu modes could pose real problems.

The CPU xml files are from usr/share/libvirt/cpu_map

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 03:45:36PM +0200, Peter Krempa wrote:
> On Fri, Jul 17, 2020 at 11:39:15 +0200, Pavel Hrdina wrote:
> > On Fri, Jul 17, 2020 at 11:21:56AM +0200, Peter Krempa wrote:
> > > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > > 
> > > [...]
> > > 
> > > Tests fail when building with clang:
> > > 
> > > built as:
> > > 
> > >  CC=clang meson clang ~/libvirt
> > >  cd clang
> > >  ninja
> > >  ninja test
> > > 
> > > 
> > > Looking at the test output (pasted below) you didn't faithfully
> > > reproduce the skipping of symbol file tests which have different
> > > ordering when the code is compiled with clang.
> > 
> > Nice catch, thanks. I tried building it with clang but did not try
> > running tests. Should have tried running ninja dist.
> 
> The fixed version on your branch now doesn't execute the tests when
> building with clang.

With autoconf they were silently skipped so I decided to not run them at
all, but we can change it to show the as skipped.

> After some learning curve (e.g. using -Dsystem=true to do the equivalent
> of ./autogen.sh --system, and the .libs/ directory stopping to exist
> I've successfully built both my normal workflows.

Glad to hear that it works for someone else as well.

Thanks for testing and the feedback.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Fri, Jul 17, 2020 at 11:39:15 +0200, Pavel Hrdina wrote:
> On Fri, Jul 17, 2020 at 11:21:56AM +0200, Peter Krempa wrote:
> > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > 
> > [...]
> > 
> > Tests fail when building with clang:
> > 
> > built as:
> > 
> >  CC=clang meson clang ~/libvirt
> >  cd clang
> >  ninja
> >  ninja test
> > 
> > 
> > Looking at the test output (pasted below) you didn't faithfully
> > reproduce the skipping of symbol file tests which have different
> > ordering when the code is compiled with clang.
> 
> Nice catch, thanks. I tried building it with clang but did not try
> running tests. Should have tried running ninja dist.

The fixed version on your branch now doesn't execute the tests when
building with clang.

After some learning curve (e.g. using -Dsystem=true to do the equivalent
of ./autogen.sh --system, and the .libs/ directory stopping to exist
I've successfully built both my normal workflows.

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Fri, Jul 17, 2020 at 12:06:54PM +0200, Ján Tomko wrote:
> On a Friday in 2020, Pavel Hrdina wrote:
> > On Fri, Jul 17, 2020 at 11:21:56AM +0200, Peter Krempa wrote:
> > > On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> > > 
> > > [...]
> > > 
> > > Tests fail when building with clang:
> > > 
> > > built as:
> > > 
> > >  CC=clang meson clang ~/libvirt
> > >  cd clang
> > >  ninja
> > >  ninja test
> > > 
> > > 
> > > Looking at the test output (pasted below) you didn't faithfully
> > > reproduce the skipping of symbol file tests which have different
> > > ordering when the code is compiled with clang.
> > 
> > Nice catch, thanks. I tried building it with clang but did not try
> > running tests.
> 
> Umm, how did the pipeline pass then, do we not run tests?

We only use clang on FreeBSD & macOS and don't run unit tests
on either. 


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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Ján Tomko]

On a Friday in 2020, Pavel Hrdina wrote:

On Fri, Jul 17, 2020 at 11:21:56AM +0200, Peter Krempa wrote:

On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:

[...]

Tests fail when building with clang:

built as:

 CC=clang meson clang ~/libvirt
 cd clang
 ninja
 ninja test


Looking at the test output (pasted below) you didn't faithfully
reproduce the skipping of symbol file tests which have different
ordering when the code is compiled with clang.


Nice catch, thanks. I tried building it with clang but did not try
running tests.


Umm, how did the pipeline pass then, do we not run tests?

Jano


Should have tried running ninja dist.

I'll fix it.

Pavel





signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Fri, Jul 17, 2020 at 11:21:56AM +0200, Peter Krempa wrote:
> On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:
> 
> [...]
> 
> Tests fail when building with clang:
> 
> built as:
> 
>  CC=clang meson clang ~/libvirt
>  cd clang
>  ninja
>  ninja test
> 
> 
> Looking at the test output (pasted below) you didn't faithfully
> reproduce the skipping of symbol file tests which have different
> ordering when the code is compiled with clang.

Nice catch, thanks. I tried building it with clang but did not try
running tests. Should have tried running ninja dist.

I'll fix it.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Peter Krempa]

On Thu, Jul 16, 2020 at 11:53:56 +0200, Pavel Hrdina wrote:

[...]

Tests fail when building with clang:

built as:

 CC=clang meson clang ~/libvirt
 cd clang
 ninja
 ninja test


Looking at the test output (pasted below) you didn't faithfully
reproduce the skipping of symbol file tests which have different
ordering when the code is compiled with clang.

Test output:

[0/1] Running all tests.
  1/154 check-aclperms  OK 0.06s
  2/154 check-symfile   OK 0.08s
  3/154 check-admin-symfile OK 0.04s
  4/154 check-symsortingOK 0.04s
  5/154 check-admin-symsorting  OK 0.03s
  6/154 check-drivernameOK 0.04s
  7/154 check-admin-drivername  OK 0.06s
  8/154 check-driverimpls   OK 0.22s
  9/154 check-aclrules  OK 0.24s
 10/154 check-augeasOK 0.08s
 11/154 check-virnetprotocolFAIL   0.05s (exit status 1)
 12/154 check-virkeepaliveprotocol  OK 0.05s
 13/154 check-remote_protocol   FAIL   0.10s (exit status 1)
 14/154 check-qemu_protocol FAIL   0.05s (exit status 1)
 15/154 check-lxc_protocol  FAIL   0.05s (exit status 1)
 16/154 check-admin_protocolFAIL   0.05s (exit status 1)
 17/154 check-lock_protocol FAIL   0.04s (exit status 1)
 18/154 check-lxc_monitor_protocol  FAIL   0.04s (exit status 1)
 19/154 commandtest OK 0.42s
 20/154 cputest OK 0.77s
 21/154 domaincapstest  OK 0.32s
 22/154 domainconftest  OK 0.05s
 23/154 genericxml2xmltest  OK 0.11s
 24/154 interfacexml2xmltestOK 0.04s
 25/154 metadatatestOK 0.03s
 26/154 networkxml2xmlupdatetestOK 0.04s
 27/154 nodedevxml2xmltest  OK 0.02s
 28/154 nwfilterxml2xmltest OK 0.03s
 29/154 objecteventtest OK 0.04s
 30/154 seclabeltestOK 0.02s
 31/154 secretxml2xmltest   OK 0.02s
 32/154 shunloadtestOK 0.02s
 33/154 sockettest  OK 0.03s
 34/154 storagevolxml2xmltest   OK 0.02s
 35/154 sysinfotest OK 0.03s
 36/154 utiltestOK 0.05s
 37/154 viralloctestOK 0.02s
 38/154 virauthconfigtest   OK 0.02s
 39/154 virbitmaptest   OK 0.02s
 40/154 virbuftest  OK 0.02s
 41/154 vircapstest OK 0.02s
 42/154 vircgrouptest   OK 0.05s
 43/154 virconftest OK 0.04s
 44/154 vircryptotest   OK 0.03s
 45/154 virendiantest   OK 0.03s
 46/154 virerrortestOK 0.03s
 47/154 virfilecachetestOK 0.04s
 48/154 virfiletest OK 0.03s
 49/154 virfirewalltest OK 0.04s
 50/154 virhashtest OK 0.02s
 51/154 virhostcputest  OK 0.05s
 52/154 virhostdevtest  OK 0.11s
 53/154 viriscsitestOK 0.03s
 54/154 virkeycodetest  OK 0.02s
 55/154 virkmodtest OK 0.03s
 56/154 virlockspacetestOK 0.02s
 57/154 virlogtest  OK 0.03s
 58/154 virnetdevtest   OK 0.03s
 59/154 virnetworkportxml2xmltest   OK 0.03s
 60/154 virnwfilterbindingxml2xmltest   OK 0.02s
 61/154 virpcitest  OK 0.04s
 62/154 virportallocatortestOK 0.04s
 63/154 virrotatingfiletest OK 0.04s
 64/154 virschematest   OK 1.05s
 65/154 virshtest   OK 1.10s
 66/154 virstringtest   OK 0.02s
 67/154 virtimetest OK 0.02s
 68/154 virtypedparamtest   OK 0.03s
 69/154 viruritest  OK 0.03s
 70/154 vshtabletestOK 0.01s
 71/154 fchosttest  OK 0.04s
 72/154 scsihosttestOK 0.02s
 73/154 vircaps2xmltest OK 0.03s
 74/154 virnetdevbandwidthtest

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Thu, Jul 16, 2020 at 08:36:42PM +0200, Jiri Denemark wrote:
> On Thu, Jul 16, 2020 at 15:47:48 +0200, Pavel Hrdina wrote:
> > On Thu, Jul 16, 2020 at 02:01:34PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > > > Patches are available in my Gitlab repo as well:
> > > > 
> > > > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> > > > 
> > > > and link to Giltab pipeline:
> > > > 
> > > > https://gitlab.com/phrdina/libvirt/-/pipelines/167276632
> > > > 
> > > > The pipeline is not for the latest version is I tweaked some commit
> > > > messages.
> > > 
> > > BTW, assuming we do positively review this, we need to consider when
> > > is a good time to merge. Having positive CI builds is good, but that
> > > only tells us that a build succeeded, it doesn't tell us that all the
> > > right features are enabled. I think it is inevitable that we are going
> > > to break stuff and miss it in review.  So the prudent approach would
> > > be to merge this series immediately after a release, so that we have
> > > as much of a full month available as possible for debugging post-merge.
> > 
> > Completely agree here. I wanted to post the patches with some time
> > before next release so the review can catch the main and obvious issues
> > or suggest different approach to some parts of the new build system.
> > 
> > There were also privet suggestions to push it even without review right
> > after release is done and deal with the issues before next release.
> 
> This was more a joke than a serious suggestion. But I basically wanted
> to express what Daniel said already. We will face bugs that won't be
> caught by any review and it will take time to find and fix them. I agree
> with the plan to push this after the release.

You were not the only one :) obviously it was a joke. There should be
review at least of the main constructs and design of the patches to make
sure we are on the same page. But doing review of every single line
would take ages.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Jiri Denemark]

On Thu, Jul 16, 2020 at 15:47:48 +0200, Pavel Hrdina wrote:
> On Thu, Jul 16, 2020 at 02:01:34PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > > Patches are available in my Gitlab repo as well:
> > > 
> > > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> > > 
> > > and link to Giltab pipeline:
> > > 
> > > https://gitlab.com/phrdina/libvirt/-/pipelines/167276632
> > > 
> > > The pipeline is not for the latest version is I tweaked some commit
> > > messages.
> > 
> > BTW, assuming we do positively review this, we need to consider when
> > is a good time to merge. Having positive CI builds is good, but that
> > only tells us that a build succeeded, it doesn't tell us that all the
> > right features are enabled. I think it is inevitable that we are going
> > to break stuff and miss it in review.  So the prudent approach would
> > be to merge this series immediately after a release, so that we have
> > as much of a full month available as possible for debugging post-merge.
> 
> Completely agree here. I wanted to post the patches with some time
> before next release so the review can catch the main and obvious issues
> or suggest different approach to some parts of the new build system.
> 
> There were also privet suggestions to push it even without review right
> after release is done and deal with the issues before next release.

This was more a joke than a serious suggestion. But I basically wanted
to express what Daniel said already. We will face bugs that won't be
caught by any review and it will take time to find and fix them. I agree
with the plan to push this after the release.

Jirka

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Thu, Jul 16, 2020 at 02:01:34PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > Patches are available in my Gitlab repo as well:
> > 
> > git clone -b meson https://gitlab.com/phrdina/libvirt.git
> > 
> > and link to Giltab pipeline:
> > 
> > https://gitlab.com/phrdina/libvirt/-/pipelines/167276632
> > 
> > The pipeline is not for the latest version is I tweaked some commit
> > messages.
> 
> BTW, assuming we do positively review this, we need to consider when
> is a good time to merge. Having positive CI builds is good, but that
> only tells us that a build succeeded, it doesn't tell us that all the
> right features are enabled. I think it is inevitable that we are going
> to break stuff and miss it in review.  So the prudent approach would
> be to merge this series immediately after a release, so that we have
> as much of a full month available as possible for debugging post-merge.

Completely agree here. I wanted to post the patches with some time
before next release so the review can catch the main and obvious issues
or suggest different approach to some parts of the new build system.

There were also privet suggestions to push it even without review right
after release is done and deal with the issues before next release.

All of the above works for me and would be probably the most reasonable
thing to do.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

On Thu, Jul 16, 2020 at 01:59:00PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> > So I was finally able to produce the patches to port libvirt to Meson.
> > Obviously, it is a lot of changes. It might look that some of the
> > patches could be squashed together but I would rather have it as
> > separated as possible to make the review not that difficult.
> > 
> > Once we are done with review I suggest to squash all patches to single
> > patch as it doesn't make sense to keep them separated as it will not be
> > possible to build complete libvirt code by any of the build systems.
> > Trying to achieve that would be even more challenging and the review
> > would me more difficult.
> > 
> > The reasoning behind taking this approach is to have 1:1 conversion from
> > autotools to Meson where each patch removes that part from autotools. It
> > serves as a check that nothing is skipped and to make sure that the
> > conversion is complete.
> 
> Can you clarify a bit more what the expected behaviour is for the
> intermediate patches in the series ? eg if I was to "git bisect"
> across this series, how much will work vs break ?  I'm not fussed
> if stuff like "make dist" breaks, but does the basic "make" and
> "make check" (or meson equivalent) work ?  I'm also not fussed if
> the intermediate stages require running *both* make and meson
> as separate commands in order to full build.

The way how the patches are done will mean that autotools will not work
or randomly fail and things like that. I was not basically paying any
attention to not breaking autotools. Some of the bits removed from
autotools throughout the series will definitely break even simple make
invocation.

Running both to fully build libvirt would be IMHO insane and complicated
so I was not even considering that option.

I tried it and the current state is that make will break with patch

[PATCH 002/351] meson: remove automake specific directives

it complains a lot about incompatible endif reminder where deleted the
else branch of the if-else-endif structure.

Invoking meson will work since patch:

[PATCH 010/351] meson: introduce meson build files

But it will not do any actual build until patch:

[PATCH 129/351] meson: src: build dtrace files

I also tried running git rebase with:

--exec='git clean -dfx && meson build && ninja -C build'

and discovered that I would have to move patch

[PATCH 080/351] meson: add driver_remote build option

before patch that requires 'driver_remote' option:

[PATCH 058/351] meson: add libssh build dependency

otherwise running meson between patches 58 and 80 will fail.

> Personally I'd really like to avoid squashing them, because splitting
> up big patches is not merely to benefit the initial pre-merge review,
> but to also benefit people who need to debug stuff that's already
> merged and understand the scope of the intended change. So being able
> to look back at the changes in isolation after commit is still a big
> plus point.

I would like to avoid squashing the patches as well and in most cases I
would object to it as well. I only suggested that to not break git
bisect.

If we don't care about git bisect and the fact that we would not be able
to build libvirt correctly within these patches I'm OK with pushing it
without squashing.

Pavel


signature.asc
Description: PGP signature

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> Patches are available in my Gitlab repo as well:
> 
> git clone -b meson https://gitlab.com/phrdina/libvirt.git
> 
> and link to Giltab pipeline:
> 
> https://gitlab.com/phrdina/libvirt/-/pipelines/167276632
> 
> The pipeline is not for the latest version is I tweaked some commit
> messages.

BTW, assuming we do positively review this, we need to consider when
is a good time to merge. Having positive CI builds is good, but that
only tells us that a build succeeded, it doesn't tell us that all the
right features are enabled. I think it is inevitable that we are going
to break stuff and miss it in review.  So the prudent approach would
be to merge this series immediately after a release, so that we have
as much of a full month available as possible for debugging post-merge.

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 :|

Re: [libvirt PATCH 000/351] port libvirt to Meson build system

[Daniel P . Berrangé]

On Thu, Jul 16, 2020 at 11:53:56AM +0200, Pavel Hrdina wrote:
> So I was finally able to produce the patches to port libvirt to Meson.
> Obviously, it is a lot of changes. It might look that some of the
> patches could be squashed together but I would rather have it as
> separated as possible to make the review not that difficult.
> 
> Once we are done with review I suggest to squash all patches to single
> patch as it doesn't make sense to keep them separated as it will not be
> possible to build complete libvirt code by any of the build systems.
> Trying to achieve that would be even more challenging and the review
> would me more difficult.
> 
> The reasoning behind taking this approach is to have 1:1 conversion from
> autotools to Meson where each patch removes that part from autotools. It
> serves as a check that nothing is skipped and to make sure that the
> conversion is complete.

Can you clarify a bit more what the expected behaviour is for the
intermediate patches in the series ? eg if I was to "git bisect"
across this series, how much will work vs break ?  I'm not fussed
if stuff like "make dist" breaks, but does the basic "make" and
"make check" (or meson equivalent) work ?  I'm also not fussed if
the intermediate stages require running *both* make and meson
as separate commands in order to full build.

Personally I'd really like to avoid squashing them, because splitting
up big patches is not merely to benefit the initial pre-merge review,
but to also benefit people who need to debug stuff that's already
merged and understand the scope of the intended change. So being able
to look back at the changes in isolation after commit is still a big
plus point.

> 
> As probably most of us know Meson is completely different build system
> and one of the most challenging things was to deal with the fact that
> meson doesn't allow user functions and that everything has to be defined
> before it is used.
> 
> Patches are available in my Gitlab repo as well:
> 
> git clone -b meson https://gitlab.com/phrdina/libvirt.git
> 
> and link to Giltab pipeline:
> 
> https://gitlab.com/phrdina/libvirt/-/pipelines/167276632
> 
> The pipeline is not for the latest version is I tweaked some commit
> messages.
> 
> Pavel Hrdina (351):
>   meson: ci: increase git clone depth to 1000
>   meson: remove automake specific directives
>   meson: drop driver_module configure argument
>   meson: drop loader_nvram build option
>   meson: Makefile: drop cov target
>   meson: syntax-check: drop Makefile and m4 related checks
>   meson: m4: drop not relevant m4 files
>   meson: src/util/virfile: rewrite virFileActivateDirOverrideForProg
>   meson: tests: remove '.libs' from all relevant paths
>   meson: introduce meson build files
>   meson: build everything with PIE
>   meson: move content from config-post.h to config.h
>   meson: set windows variables for AI_ADDRCONFIG
>   meson: generate configmake.h
>   meson: add packager build options
>   meson: add test_suite build option
>   meson: add expensive_tests build option
>   meson: add test_coverage build option
>   meson: add static analysis detection
>   meson: add manywarnings
>   meson: add compiler warnings
>   meson: add linker checks
>   meson: add scripts directory
>   meson: add include directory
>   meson: add functions check
>   meson: add headers check
>   meson: add symbols check
>   meson: add types check
>   meson: add members check
>   meson: add sizeof check
>   meson: add programs checks
>   meson: add iscsiadm as optional program
>   meson: add acl build dependency
>   meson: add AppArmor build dependency
>   meson: add attr build option
>   meson: add audit build dependency
>   meson: add readline build option
>   meson: add bash_completion build options
>   meson: add blkid build dependency
>   meson: add capng build dependency
>   meson: add curl build dependency
>   meson: add dbus build dependency
>   meson: add devmapper build dependency
>   meson: add dlopen build dependency
>   meson: add firewalld build option
>   meson: add firewalld_zone build option
>   meson: add fuse build dependency
>   meson: add GLib dependency
>   meson: add glusterfs build dependency
>   meson: add GnuTLS build dependency
>   meson: add hal build dependency
>   meson: add kvm build dependency
>   meson: add libiscsi build dependency
>   meson: add macvtap build option
>   meson: add libnl build dependency
>   meson: add libparted dependency
>   meson: add libpcap build option
>   meson: add libssh build dependency
>   meson: add libssh2 build dependency
>   meson: add libxml build dependency
>   meson: add netcf build options
>   meson: add nls build dependency
>   meson: add numactl build dependency
>   meson: add openwsman build dependency
>   meson: add parallels-sdk build check
>   meson: add pciaccess build option
>   meson: add polkit build option
>   meson: add rbd build dependency
>   meson: add sanlock build option
>   meson: add sasl build dependency
>   

[libvirt PATCH 000/351] port libvirt to Meson build system

[Pavel Hrdina]

So I was finally able to produce the patches to port libvirt to Meson.
Obviously, it is a lot of changes. It might look that some of the
patches could be squashed together but I would rather have it as
separated as possible to make the review not that difficult.

Once we are done with review I suggest to squash all patches to single
patch as it doesn't make sense to keep them separated as it will not be
possible to build complete libvirt code by any of the build systems.
Trying to achieve that would be even more challenging and the review
would me more difficult.

The reasoning behind taking this approach is to have 1:1 conversion from
autotools to Meson where each patch removes that part from autotools. It
serves as a check that nothing is skipped and to make sure that the
conversion is complete.

As probably most of us know Meson is completely different build system
and one of the most challenging things was to deal with the fact that
meson doesn't allow user functions and that everything has to be defined
before it is used.

Patches are available in my Gitlab repo as well:

git clone -b meson https://gitlab.com/phrdina/libvirt.git

and link to Giltab pipeline:

https://gitlab.com/phrdina/libvirt/-/pipelines/167276632

The pipeline is not for the latest version is I tweaked some commit
messages.

Pavel Hrdina (351):
  meson: ci: increase git clone depth to 1000
  meson: remove automake specific directives
  meson: drop driver_module configure argument
  meson: drop loader_nvram build option
  meson: Makefile: drop cov target
  meson: syntax-check: drop Makefile and m4 related checks
  meson: m4: drop not relevant m4 files
  meson: src/util/virfile: rewrite virFileActivateDirOverrideForProg
  meson: tests: remove '.libs' from all relevant paths
  meson: introduce meson build files
  meson: build everything with PIE
  meson: move content from config-post.h to config.h
  meson: set windows variables for AI_ADDRCONFIG
  meson: generate configmake.h
  meson: add packager build options
  meson: add test_suite build option
  meson: add expensive_tests build option
  meson: add test_coverage build option
  meson: add static analysis detection
  meson: add manywarnings
  meson: add compiler warnings
  meson: add linker checks
  meson: add scripts directory
  meson: add include directory
  meson: add functions check
  meson: add headers check
  meson: add symbols check
  meson: add types check
  meson: add members check
  meson: add sizeof check
  meson: add programs checks
  meson: add iscsiadm as optional program
  meson: add acl build dependency
  meson: add AppArmor build dependency
  meson: add attr build option
  meson: add audit build dependency
  meson: add readline build option
  meson: add bash_completion build options
  meson: add blkid build dependency
  meson: add capng build dependency
  meson: add curl build dependency
  meson: add dbus build dependency
  meson: add devmapper build dependency
  meson: add dlopen build dependency
  meson: add firewalld build option
  meson: add firewalld_zone build option
  meson: add fuse build dependency
  meson: add GLib dependency
  meson: add glusterfs build dependency
  meson: add GnuTLS build dependency
  meson: add hal build dependency
  meson: add kvm build dependency
  meson: add libiscsi build dependency
  meson: add macvtap build option
  meson: add libnl build dependency
  meson: add libparted dependency
  meson: add libpcap build option
  meson: add libssh build dependency
  meson: add libssh2 build dependency
  meson: add libxml build dependency
  meson: add netcf build options
  meson: add nls build dependency
  meson: add numactl build dependency
  meson: add openwsman build dependency
  meson: add parallels-sdk build check
  meson: add pciaccess build option
  meson: add polkit build option
  meson: add rbd build dependency
  meson: add sanlock build option
  meson: add sasl build dependency
  meson: add SELinux build dependency
  meson: add thread build dependency
  meson: add udev build options
  meson: add util build dependency
  meson: add virtualport build dependency
  meson: add win32 build dependency
  meson: add wireshark build dependency
  meson: add xdr build dependency
  meson: add yajl build dependency
  meson: add driver_remote build option
  meson: add libvirtd driver build option
  meson: add BHyVe build option
  meson: add ESX driver build option
  meson: add Hyper-V driver build option
  meson: add libxl driver build option
  meson: add LXC driver build option
  meson: add OpenVZ driver build option
  meson: add qemu driver build options
  meson: add test driver build option
  meson: add vbox driver build options
  meson: add VMWare driver build option
  meson: add Virtuozzo driver build option
  meson: add secdriver build options
  meson: add network driver build option
  meson: add interface driver build option
  meson: add secrets driver build option
  meson: add node_device driver check
  meson: add storage build check