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