Re: [libvirt PATCH 000/351] port libvirt to Meson build system
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.