Re: PATCH: Add --size-check=[error|warning]

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

(H.J. Lu, did you drop me from the Cc: line?)

* Jan Beulich <JBeulich@novell.com> wrote:

> >> Please make it so that it'll be a warning by default, and an error
> >> upon programmer request. Otherwise, for the very purpose of
> > 
> > I disagree. It should be error by default since the input is bogus,
> > Otherwise, those assembly bugs, benign or not, may not get
> > fixed.
> > 
> >> bisection, it won't help much as you would have to override
> >> compiler/assembler flags during that process.
> >>
> > 
> > They can use a wrapper to pass --size-check=warning to
> > assembler.  I think it is a small price to pay for those mistakes.
> 
> "Small" being relative here - it could be dozens if not hundreds of
> people having to learn that this is necessary, many of them
> possibly rather unfamiliar with gas and its command line options.
>
> Also, using a wrapper gets further complicated by the fact that
> you may have to pass an extra -B to the compiler (not everyone
> has full control over the file system of all the machines used to
> do development), making sure this doesn't have any other
> unwanted side effects.

Correct. In reality if the kernel does not build or boot then most people just wont 
continue with the bisection. So this change actively degrades debuggability, for no 
good reason.

The thing is, it is absolutely, breath-takingy incompetent for the new binutils 
version to break the Linux kernel build for 4 years of Linux kernel history 
retroactively (130,000 commits), just to 'warn' about a size bug in a few debug 
symbols that has no functional effects whatsoever and which few people care about.

The correct solution is to turn it into a warning as me and others have suggested.

No argument was offered *why* the build must be aborted. A warning serves the 
purpose of informing the developer just as much - and does not inconvenience the 
tester.

Thanks,

	Ingo

Re: PATCH: Add --size-check=[error|warning]

[Alan Modra]

On Mon, Mar 14, 2011 at 10:55:34AM +0100, Ingo Molnar wrote:
> The thing is, it is absolutely, breath-takingy incompetent for

kernel developers to write such poor asm!  And not notice the error
for 4 years!  Oh, and the binutils developers to write such a poor
assembler in the first place.  ;-)

Seriously, you are complaining because something is fixed??

> The correct solution is to turn it into a warning as me and others have suggested.

I disagree.  The whole world is not the linux kernel.  I think HJ is
bending over backwards to even offer a switch that turns the error
into a warning.

-- 
Alan Modra
Australia Development Lab, IBM

Re: PATCH: Add --size-check=[error|warning]

[Pekka Enberg]

Hi Alan,

On Mon, Mar 14, 2011 at 12:41 PM, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Mar 14, 2011 at 10:55:34AM +0100, Ingo Molnar wrote:
>> The thing is, it is absolutely, breath-takingy incompetent for
>
> kernel developers to write such poor asm!  And not notice the error
> for 4 years!  Oh, and the binutils developers to write such a poor
> assembler in the first place.  ;-)
>
> Seriously, you are complaining because something is fixed??
>
>> The correct solution is to turn it into a warning as me and others have suggested.
>
> I disagree.  The whole world is not the linux kernel.  I think HJ is
> bending over backwards to even offer a switch that turns the error
> into a warning.

So what do you suggest that testers who want to, say, build old Linux
kernel versions with new binutils do?

                        Pekka

Re: PATCH: Add --size-check=[error|warning]

[Jan Beulich]

>>> On 14.03.11 at 11:41, Alan Modra <amodra@gmail.com> wrote:
> On Mon, Mar 14, 2011 at 10:55:34AM +0100, Ingo Molnar wrote:
>> The thing is, it is absolutely, breath-takingy incompetent for
> 
> kernel developers to write such poor asm!  And not notice the error
> for 4 years!  Oh, and the binutils developers to write such a poor
> assembler in the first place.  ;-)
> 
> Seriously, you are complaining because something is fixed??
> 
>> The correct solution is to turn it into a warning as me and others have 
> suggested.
> 
> I disagree.  The whole world is not the linux kernel.  I think HJ is
> bending over backwards to even offer a switch that turns the error
> into a warning.

Just to repeat what I said in a previous reply - an error should be
issued if it is impossible for the assembler to produce valid output.
Anything less severe should be a warning.

In the case given, as also stated before, simply not issuing anything
to the object file if .size has an invalid operand is quite feasible for
the assembler to do, and won't produce invalid output. The warning
tells the programmer that not everything (s)he intended to be in
the object file actually made it there.

Jan

Re: PATCH: Add --size-check=[error|warning]

[Andreas Schwab]

Pekka Enberg <penberg@kernel.org> writes:

> So what do you suggest that testers who want to, say, build old Linux
> kernel versions with new binutils do?

The same that testers have to do in order to build old Linux kernel
versions with current versions of make.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: PATCH: Add --size-check=[error|warning]

[Sedat Dilek]

[QUOTE]
(H.J. Lu, did you drop me from the Cc: line?)

* Jan Beulich <JBeulich@novell.com> wrote:

> >> Please make it so that it'll be a warning by default, and an error
> >> upon programmer request. Otherwise, for the very purpose of
> >
> > I disagree. It should be error by default since the input is bogus,
> > Otherwise, those assembly bugs, benign or not, may not get
> > fixed.
> >
> >> bisection, it won't help much as you would have to override
> >> compiler/assembler flags during that process.
> >>
> >
> > They can use a wrapper to pass --size-check=warning to
> > assembler.  I think it is a small price to pay for those mistakes.
>
> "Small" being relative here - it could be dozens if not hundreds of
> people having to learn that this is necessary, many of them
> possibly rather unfamiliar with gas and its command line options.
>
> Also, using a wrapper gets further complicated by the fact that
> you may have to pass an extra -B to the compiler (not everyone
> has full control over the file system of all the machines used to
> do development), making sure this doesn't have any other
> unwanted side effects.

Correct. In reality if the kernel does not build or boot then most
people just wont
continue with the bisection. So this change actively degrades
debuggability, for no
good reason.

The thing is, it is absolutely, breath-takingy incompetent for the new binutils
version to break the Linux kernel build for 4 years of Linux kernel history
retroactively (130,000 commits), just to 'warn' about a size bug in a few debug
symbols that has no functional effects whatsoever and which few people
care about.

The correct solution is to turn it into a warning as me and others
have suggested.

No argument was offered *why* the build must be aborted. A warning serves the
purpose of informing the developer just as much - and does not
inconvenience the
tester.

Thanks,

	Ingo
[/QUOTE]

Nice to see there is an offer for a fix from binutils-side.

The followers of this "issue" like me have read the arguments from
kernel-developers.
Unfortunately, the Open Source world is not the linux-kernel alone.
I have built in the meantime a lot of Xorg stuff from GIT, etc. with a
binutils 2.21-snapshot (plus additional backported patches from
upstream).

If the goal is to catch real BUGs in the kernel, the current behaviour
of binutils/as is IMHO correct.
That's why I am on linux-next to squash bugs, not to ignore "warnings"

BTW "warnings", did someone tried gcc-4.6?
I used a snapshot from mid February (from Debian/experimental):
My linux-next build-log and the amount of warnings doubled or even
more (unfortunately, I have thrown away that logs and binaries).
Are all of these warnings ignoreable?
Which of them are really severe?

So, H.J. was pro-active in the meantime by offering this patch.
>From kernel-side it's getting IMHO more and more some sort of "burning
of witches".

Thus some questions to the kernel folks:

[1] Jan, what do you mean by "side-effects". Can you explain that a
bit more precisely?

[2] Where can someone set a "global behaviour" (hardcoded options) for
his/her assembler in the kernel's build-system (speaking of
"--size-check=[error|warning]")?

[3] Can the kernel-buildsystem check for system's binutils/as version
and/or its features/options? If yes, where would that be and can you
offer a snippet for a solution?

Answering and/or offering solutions for my askings can help to handle
things from kernel-side.

My 0,02EUR.

- Sedat -

Re: PATCH: Add --size-check=[error|warning]

[Pekka Enberg]

Hi Andreas,

Pekka Enberg <penberg@kernel.org> writes:
>> So what do you suggest that testers who want to, say, build old Linux
>> kernel versions with new binutils do?

On Mon, Mar 14, 2011 at 1:02 PM, Andreas Schwab <schwab@redhat.com> wrote:
> The same that testers have to do in order to build old Linux kernel
> versions with current versions of make.

Are you saying it's OK to screw over binutils users because GNU Make
did that too?

                       Pekka

Re: PATCH: Add --size-check=[error|warning]

[Jan Beulich]

>>> On 14.03.11 at 12:02, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> [1] Jan, what do you mean by "side-effects". Can you explain that a
> bit more precisely?

The -B compiler option controls more than just finding "helper" binaries.

> [2] Where can someone set a "global behaviour" (hardcoded options) for
> his/her assembler in the kernel's build-system (speaking of
> "--size-check=[error|warning]")?

Nowhere, selecting behavior is possible only via the command line.

> [3] Can the kernel-buildsystem check for system's binutils/as version
> and/or its features/options? If yes, where would that be and can you
> offer a snippet for a solution?

Making the kernel build system check for certain newly introduced
gas options would again require changes to the kernel sources,
which is precisely what is impossible to do for past kernel releases
(and bisection in particular).

Jan

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

* Jan Beulich <JBeulich@novell.com> wrote:

> >>> On 14.03.11 at 12:02, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> > [1] Jan, what do you mean by "side-effects". Can you explain that a
> > bit more precisely?
> 
> The -B compiler option controls more than just finding "helper" binaries.
> 
> > [2] Where can someone set a "global behaviour" (hardcoded options) for
> > his/her assembler in the kernel's build-system (speaking of
> > "--size-check=[error|warning]")?
> 
> Nowhere, selecting behavior is possible only via the command line.
> 
> > [3] Can the kernel-buildsystem check for system's binutils/as version
> > and/or its features/options? If yes, where would that be and can you
> > offer a snippet for a solution?
> 
> Making the kernel build system check for certain newly introduced
> gas options would again require changes to the kernel sources,
> which is precisely what is impossible to do for past kernel releases
> (and bisection in particular).

Yes, and all the counter-arguments here continue to miss that very simple point. 
That point was made in the first post about this topic and it's still not 
acknowledged - let alone addressed.

This breakage is unnecessary and retroactively goes back 130,000 commits. A warning 
combined with not issuing the debug symbol would be just as fine and would still 
result in valid output.

Thanks,

	Ingo

Re: PATCH: Add --size-check=[error|warning]

[Sedat Dilek]

[ Changing Alan's Email to valid <amodra@gmail.com> in CC ]

On Mon, Mar 14, 2011 at 12:26 PM, Jan Beulich <JBeulich@novell.com> wrote:
>>>> On 14.03.11 at 12:02, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
[...]
>> [2] Where can someone set a "global behaviour" (hardcoded options) for
>> his/her assembler in the kernel's build-system (speaking of
>> "--size-check=[error|warning]")?
>
> Nowhere, selecting behavior is possible only via the command line.
>

Via command-line, something like this would do it?

     $ export AS="/usr/bin/as --size-check=warning
[...]

- Sedat -

Re: PATCH: Add --size-check=[error|warning]

[Sedat Dilek]

On Mon, Mar 14, 2011 at 12:38 PM, Sedat Dilek
<sedat.dilek@googlemail.com> wrote:
> [ Changing Alan's Email to valid <amodra@gmail.com> in CC ]
>
> On Mon, Mar 14, 2011 at 12:26 PM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>> On 14.03.11 at 12:02, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> [...]
>>> [2] Where can someone set a "global behaviour" (hardcoded options) for
>>> his/her assembler in the kernel's build-system (speaking of
>>> "--size-check=[error|warning]")?
>>
>> Nowhere, selecting behavior is possible only via the command line.
>>
>
> Via command-line, something like this would do it?
>
>     $ export AS="/usr/bin/as --size-check=warning
> [...]
>
> - Sedat -
>

By looking through the binutils source-code, I have seen ASFLAGS.

     $ ASFLAGS="--size-check=warning" ; export ASFLAGS

Would that work?

- Sedat -

Re: PATCH: Add --size-check=[error|warning]

[Andreas Schwab]

Pekka Enberg <penberg@kernel.org> writes:

> Hi Andreas,
>
> Pekka Enberg <penberg@kernel.org> writes:
>>> So what do you suggest that testers who want to, say, build old Linux
>>> kernel versions with new binutils do?
>
> On Mon, Mar 14, 2011 at 1:02 PM, Andreas Schwab <schwab@redhat.com> wrote:
>> The same that testers have to do in order to build old Linux kernel
>> versions with current versions of make.
>
> Are you saying it's OK to screw over binutils users because GNU Make
> did that too?

I'm just telling you the facts.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

* Pekka Enberg <penberg@kernel.org> wrote:

> Hi Andreas,
> 
> Pekka Enberg <penberg@kernel.org> writes:
> >> So what do you suggest that testers who want to, say, build old Linux
> >> kernel versions with new binutils do?
> 
> On Mon, Mar 14, 2011 at 1:02 PM, Andreas Schwab <schwab@redhat.com> wrote:
> > The same that testers have to do in order to build old Linux kernel
> > versions with current versions of make.
> 
> Are you saying it's OK to screw over binutils users because GNU Make
> did that too?

The claim is not even true.

While really old Linux versions wont build with fresh versions of Make, something 
like v2.6.30, which is a 2 years old kernel, will build just fine, using latest 
Make.

So lets stop coming up with all the wrong reasons to not fix this problem ...

Thanks,

	Ingo

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

* Andreas Schwab <schwab@redhat.com> wrote:

> Pekka Enberg <penberg@kernel.org> writes:
> 
> > Hi Andreas,
> >
> > Pekka Enberg <penberg@kernel.org> writes:
> >>> So what do you suggest that testers who want to, say, build old Linux
> >>> kernel versions with new binutils do?
> >
> > On Mon, Mar 14, 2011 at 1:02 PM, Andreas Schwab <schwab@redhat.com> wrote:
> >> The same that testers have to do in order to build old Linux kernel
> >> versions with current versions of make.
> >
> > Are you saying it's OK to screw over binutils users because GNU Make
> > did that too?
> 
> I'm just telling you the facts.

And you are wrong - latest Make does not break reasonably old kernel builds such as 
v2.6.30.

Latest binutils insta-breaks the build over 130,000 commits, including the latest 
released kernel.

Please change that .size build error to a build warning, to avoid this unnecessary 
collateral damage.

Thanks,

	Ingo

Re: PATCH: Add --size-check=[error|warning]

[Jan Beulich]

>>> On 14.03.11 at 12:52, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> On Mon, Mar 14, 2011 at 12:38 PM, Sedat Dilek
> <sedat.dilek@googlemail.com> wrote:
>> [ Changing Alan's Email to valid <amodra@gmail.com> in CC ]
>>
>> On Mon, Mar 14, 2011 at 12:26 PM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>>> On 14.03.11 at 12:02, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>> [...]
>>>> [2] Where can someone set a "global behaviour" (hardcoded options) for
>>>> his/her assembler in the kernel's build-system (speaking of
>>>> "--size-check=[error|warning]")?
>>>
>>> Nowhere, selecting behavior is possible only via the command line.
>>>
>>
>> Via command-line, something like this would do it?
>>
>>     $ export AS="/usr/bin/as --size-check=warning
>> [...]

No - $(AS) isn't being used to translate .S files, $(CC) is being used
instead. That's why I pointed at the necessary -B option (so that
the compiler would be able to locate the wrapper H.J. suggested).

> By looking through the binutils source-code, I have seen ASFLAGS.
> 
>      $ ASFLAGS="--size-check=warning" ; export ASFLAGS

Where did you find that? All places where I see references to this
variable are in the testsuite.

Jan

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

* Alan Modra <amodra@gmail.com> wrote:

> On Mon, Mar 14, 2011 at 10:55:34AM +0100, Ingo Molnar wrote:
> > The thing is, it is absolutely, breath-takingy incompetent for
> 
> kernel developers to write such poor asm!  And not notice the error
> for 4 years! [...]

It is not 'poor asm'.

The 'bug' is just a slight assymetry in ENTRY()/END() debug-symbols sequences, with 
lots of assembly code between the ENTRY() and the END(). Here's an example:
    
         ENTRY(xen_do_hypervisor_callback)   # do_hypervisor_callback(struct *pt_regs)
           ...
         END(do_hypervisor_callback)

Human reviewers almost never catch such small mismatches, and binutils never even 
warned about it either - for over a decade.

Now kernel bisections are insta-broken on latest binutils, and there's nothing to do 
about it on the kernel side as during bisection all later fixes are unfolded. The 
fix itself i already applied - but my argument was not about that:

> [...] Oh, and the binutils developers to write such a poor assembler in the first 
> place.  ;-)
> 
> Seriously, you are complaining because something is fixed??

No, i reported this bug because the kernel build gets broken going back 130,000 
commits, breaking bisection and causing other damage - while issuing a warning 
message would achieve the same effect of warning the developer about the mismatch.

> > The correct solution is to turn it into a warning as me and others have suggested.
> 
> I disagree.  The whole world is not the linux kernel.  I think HJ is
> bending over backwards to even offer a switch that turns the error
> into a warning.

It's not about a switch at all - it's to not break builds by default. I.e. the 
default behavior should be to issue a warning and ignore the directive.

This is a very simple concept of compatibility: the build environment should always 
be very permissive - stuff that build fine before should be allowed to build.

Also, i hope you are not suggesting to break projects just because they are not 
important to you personally? The fix is exceedingly simple to do for the binutils 
project - and impossible to do for the kernel project (because during bisection - 
which is a very powerful debugging tool - older versions of the source get checked 
out).

Thanks,

	Ingo

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

(resend, fixed the To line)

* Alan Modra <amodra@gmail.com> wrote:
 
> On Mon, Mar 14, 2011 at 10:55:34AM +0100, Ingo Molnar wrote:
> > The thing is, it is absolutely, breath-takingy incompetent for
> 
> kernel developers to write such poor asm!  And not notice the error
> for 4 years! [...]

It is not 'poor asm'.

The 'bug' is just a slight assymetry in ENTRY()/END() debug-symbols sequences, with 
lots of assembly code between the ENTRY() and the END(). Here's an example:
    
         ENTRY(xen_do_hypervisor_callback)   # do_hypervisor_callback(struct *pt_regs)
           ...
         END(do_hypervisor_callback)

Human reviewers almost never catch such small mismatches, and binutils never even 
warned about it either - for over a decade.

Now kernel bisections are insta-broken on latest binutils, and there's nothing to do 
about it on the kernel side as during bisection all later fixes are unfolded. The 
fix itself i already applied - but my argument was not about that:

> [...] Oh, and the binutils developers to write such a poor assembler in the first 
> place.  ;-)
> 
> Seriously, you are complaining because something is fixed??
 
No, i reported this bug because the kernel build gets broken going back 130,000 
commits, breaking bisection and causing other damage - while issuing a warning 
message would achieve the same effect of warning the developer about the mismatch.

> > The correct solution is to turn it into a warning as me and others have suggested.
> 
> I disagree.  The whole world is not the linux kernel.  I think HJ is
> bending over backwards to even offer a switch that turns the error
> into a warning.
 
It's not about a switch at all - it's to not break builds by default. I.e. the 
default behavior should be to issue a warning and ignore the directive.
 
This is a very simple concept of compatibility: the build environment should always 
be very permissive - stuff that build fine before should be allowed to build.

Also, i hope you are not suggesting to break projects just because they are not 
important to you personally? The fix is exceedingly simple to do for the binutils 
project - and impossible to do for the kernel project (because during bisection - 
which is a very powerful debugging tool - older versions of the source get checked 
out).

Thanks,

	Ingo

Re: PATCH: Add --size-check=[error|warning]

[Sedat Dilek]

Hi H.J.,

against which binutils source-code is your patch?
I tried binutils-from-git (last commit 83e680a: "daily update") and
your binutils-2.21.51.0.7.

None of them have a gas/testsuite/gas/i386/bad-size.d file, so...

diff --git a/gas/testsuite/gas/i386/bad-size.d
b/gas/testsuite/gas/i386/bad-size.d
index be9655e..0bcf381 100644
--- a/gas/testsuite/gas/i386/bad-size.d
+++ b/gas/testsuite/gas/i386/bad-size.d
@@ -1,7 +1,7 @@
 #as: --size-check=warning
 #objdump: -dw
 #name: Check bad size directive
-#error-output: bad-size.err
+#error-output: bad-size.warn

 .*: +file format .*

... cannot be applied.

Also, there are no gas/ChangeLog.x86 and gas/testsuite/ChangeLog.x86
files (but w/o *.x86).

Can you resend a proper patch against binutils-from-upstream?
Thanks.

Regards,
- Sedat -

Re: PATCH: Add --size-check=[error|warning]

[Sedat Dilek]

On Mon, Mar 14, 2011 at 1:21 PM, Jan Beulich <JBeulich@novell.com> wrote:
>>>> On 14.03.11 at 12:52, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>> On Mon, Mar 14, 2011 at 12:38 PM, Sedat Dilek
>> <sedat.dilek@googlemail.com> wrote:
>>> [ Changing Alan's Email to valid <amodra@gmail.com> in CC ]
>>>
>>> On Mon, Mar 14, 2011 at 12:26 PM, Jan Beulich <JBeulich@novell.com> wrote:
>>>>>>> On 14.03.11 at 12:02, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
>>> [...]
>>>>> [2] Where can someone set a "global behaviour" (hardcoded options) for
>>>>> his/her assembler in the kernel's build-system (speaking of
>>>>> "--size-check=[error|warning]")?
>>>>
>>>> Nowhere, selecting behavior is possible only via the command line.
>>>>
>>>
>>> Via command-line, something like this would do it?
>>>
>>>     $ export AS="/usr/bin/as --size-check=warning
>>> [...]
>
> No - $(AS) isn't being used to translate .S files, $(CC) is being used
> instead. That's why I pointed at the necessary -B option (so that
> the compiler would be able to locate the wrapper H.J. suggested).
>

OK, now I am enlightened in things of -B option.
Anyway, can I see the correct command line to use?
(OMG, I am glad this is not the $500.000 question @ "Who Wants to Be a
Millionaire?" quiz show. Jan wanna be my telephone joker :-)?)

>> By looking through the binutils source-code, I have seen ASFLAGS.
>>
>>      $ ASFLAGS="--size-check=warning" ; export ASFLAGS
>
> Where did you find that? All places where I see references to this
> variable are in the testsuite.
>
> Jan
>

WildWildWeb, IIRC I saw ASFLAGS used in a Gentoo package and AS in
Cross-LFS/binutils build instructions.

- Sedat -

Re: PATCH: Add --size-check=[error|warning]

[Andreas Schwab]

Ingo Molnar <mingo@elte.hu> writes:

> * Andreas Schwab <schwab@redhat.com> wrote:
>
>> Pekka Enberg <penberg@kernel.org> writes:
>> 
>> > Hi Andreas,
>> >
>> > Pekka Enberg <penberg@kernel.org> writes:
>> >>> So what do you suggest that testers who want to, say, build old Linux
>> >>> kernel versions with new binutils do?
>> >
>> > On Mon, Mar 14, 2011 at 1:02 PM, Andreas Schwab <schwab@redhat.com> wrote:
>> >> The same that testers have to do in order to build old Linux kernel
>> >> versions with current versions of make.
>> >
>> > Are you saying it's OK to screw over binutils users because GNU Make
>> > did that too?
>> 
>> I'm just telling you the facts.
>
> And you are wrong

This is just ridiculous.  You are defining away facts just because they
don't fit your view.

> latest Make does not break reasonably old kernel builds such as
> v2.6.30.

$ git tag --contains 3c955b4 | head -1
v2.6.36

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

* Sedat Dilek <sedat.dilek@googlemail.com> wrote:

> Nice to see there is an offer for a fix from binutils-side.

Agreed.

> That's why I am on linux-next to squash bugs, not to ignore "warnings"

We x86 arch maintainers definitely do not ignore warnings from the assembler. 
Assembler warnings were pretty good historically and seldom produce false positives.

> BTW "warnings", did someone tried gcc-4.6? I used a snapshot from mid February 
> (from Debian/experimental): My linux-next build-log and the amount of warnings 
> doubled or even more (unfortunately, I have thrown away that logs and binaries). 
> Are all of these warnings ignoreable? Which of them are really severe?

Most of those are -Wunused-but-set-variable warnings, right? I'm definitely not 
ignoring the ones that affect the code i maintain - so they are very much useful.

But if GCC broke the build unnecessarily, just to over-eagerly warn about something 
that worked fine before, that would be extremely counter-productive! In such a 
situation the Linux kernel project would likely be fed up enough to build its own 
sane compiler/assembler/linker combo and would aim to become entirely independent in 
terms of its build environment.

Thanks,

	Ingo

git bisect plus fixes (was: PATCH: Add --size-check=[error|warning])

[Ralf Wildenhues]

[ adding the git list; this is
  http://thread.gmane.org/gmane.comp.gnu.binutils/52601/focus=1112779 ]

Hello,

* Ingo Molnar wrote on Mon, Mar 14, 2011 at 01:23:42PM CET:
> Also, i hope you are not suggesting to break projects just because
> they are not important to you personally? The fix is exceedingly
> simple to do for the binutils project - and impossible to do for the
> kernel project (because during bisection - which is a very powerful
> debugging tool - older versions of the source get checked out).

FWIW, I don't have an opinion on this particular binutils issue, but
it would be very helpful if 'git bisect' made it easy to denote
"when going back, you might also need some of these changes".
(I'd just use a patch -p1 with a here-file in the bisect script, but
that might not be enough for all practical use cases.)

This issue has come up several times with high-profile issues.

Thanks,
Ralf

Re: PATCH: Add --size-check=[error|warning]

[Ingo Molnar]

* Andreas Schwab <schwab@redhat.com> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> 
> > * Andreas Schwab <schwab@redhat.com> wrote:
> >
> >> Pekka Enberg <penberg@kernel.org> writes:
> >> 
> >> > Hi Andreas,
> >> >
> >> > Pekka Enberg <penberg@kernel.org> writes:
> >> >>> So what do you suggest that testers who want to, say, build old Linux
> >> >>> kernel versions with new binutils do?
> >> >
> >> > On Mon, Mar 14, 2011 at 1:02 PM, Andreas Schwab <schwab@redhat.com> wrote:
> >> >> The same that testers have to do in order to build old Linux kernel
> >> >> versions with current versions of make.
> >> >
> >> > Are you saying it's OK to screw over binutils users because GNU Make
> >> > did that too?
> >> 
> >> I'm just telling you the facts.
> >
> > And you are wrong - latest Make does not break reasonably old kernel builds such 
> > as v2.6.30.
> 
> This is just ridiculous.
> You are defining away facts just because they don't fit your view.

No, i actually tried out the latest released Make version (3.82) and it still builds 
v2.6.30 fine:

    LD      arch/x86/boot/setup.elf
    OBJCOPY arch/x86/boot/setup.bin
    BUILD   arch/x86/boot/bzImage
  Root device is (8, 1)
  Setup is 12364 bytes (padded to 12800 bytes). 
  System is 3730 kB
  CRC 77bb7f2d
  Kernel: arch/x86/boot/bzImage is ready  (#105830)

The kernel build count is at 105830 because i build and test a lot of kernels on 
this box.

And the resulting kernel boots fine on a testbox:

  mercury:~> uname -a
  Linux mercury 2.6.30 #105830 SMP Mon Mar 14 12:29:06 CET 2011 x86_64 x86_64 x86_64 GNU/Linux

I have built and booted over half a million Linux kernels in the past 3-4 years so i 
generally have quite a bit of experience when it comes to build environment bugs and 
workflow problems. Breaking the build retroactively and unnecessarily like here is 
one of the worst things that can be done to testing quality and efficiency.

Thanks,

	Ingo

Re: PATCH: Add --size-check=[error|warning]

[Andreas Schwab]

Ingo Molnar <mingo@elte.hu> writes:

> * Andreas Schwab <schwab@redhat.com> wrote:
>
>> Ingo Molnar <mingo@elte.hu> writes:
>> 
>> > * Andreas Schwab <schwab@redhat.com> wrote:
>> >
>> >> Pekka Enberg <penberg@kernel.org> writes:
>> >> 
>> >> > Hi Andreas,
>> >> >
>> >> > Pekka Enberg <penberg@kernel.org> writes:
>> >> >>> So what do you suggest that testers who want to, say, build old Linux
>> >> >>> kernel versions with new binutils do?
>> >> >
>> >> > On Mon, Mar 14, 2011 at 1:02 PM, Andreas Schwab <schwab@redhat.com> wrote:
>> >> >> The same that testers have to do in order to build old Linux kernel
>> >> >> versions with current versions of make.
>> >> >
>> >> > Are you saying it's OK to screw over binutils users because GNU Make
>> >> > did that too?
>> >> 
>> >> I'm just telling you the facts.
>> >
>> > And you are wrong - latest Make does not break reasonably old kernel builds such 
>> > as v2.6.30.
>> 
>> This is just ridiculous.
>> You are defining away facts just because they don't fit your view.
>
> No, i actually tried out the latest released Make version (3.82) and it still builds 
> v2.6.30 fine:

That's a very convincing argument.

Andreas.

-- 
Andreas Schwab, schwab@redhat.com
GPG Key fingerprint = D4E8 DBE3 3813 BB5D FA84  5EC7 45C6 250E 6F00 984E
"And now for something completely different."

[PATCH] git-bisect.txt: example for bisecting with hotfix

[Michael J Gruber]

Give an example on how to bisect when older revisions need a hotfix to
build, run or test. Triggered by the binutils/kernel issue at

http://thread.gmane.org/gmane.comp.gnu.binutils/52601/focus=1112779

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
Maybe this doc fix would do. Just tag the hotfix and tell people to cherry-pick it
like this. I don't think "git bisect --with-fix=hotfix" would be much simpler.
(culling kernel list from cc - don't apply this to the wrong tree :)

 Documentation/git-bisect.txt |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index c39d957..25acf26 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -322,6 +322,17 @@ $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
 +
 Does the same as the previous example, but on a single line.
 
+* Bisect with compatibility hotfix:
++
+------------
+$ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
+$ git bisect run sh -c "git cherry-pick -n hotfix || exit 125; make || exit 125; ~/check_test_case.sh"
+------------
++
+Does the same as the previous example, but applies an additional patch
+before building. This is useful when your build or test environment changed so
+that older revisions may need a fix which newer ones have already.
+
 Author
 ------
 Written by Linus Torvalds <torvalds@osdl.org>
-- 
1.7.4.1.404.g62d316

Re: PATCH: Add --size-check=[error|warning]

[Ian Lance Taylor]

"Jan Beulich" <JBeulich@novell.com> writes:

> No - $(AS) isn't being used to translate .S files, $(CC) is being used
> instead. That's why I pointed at the necessary -B option (so that
> the compiler would be able to locate the wrapper H.J. suggested).

You could also just put a -Wa option in your $(CC).  That seems simpler
than a -B option.

(I don't really care about this issue one way or another.)

Ian

Re: PATCH: Add --size-check=[error|warning]

[H. Peter Anvin]

On 03/14/2011 04:26 AM, Jan Beulich wrote:
> 
> Making the kernel build system check for certain newly introduced
> gas options would again require changes to the kernel sources,
> which is precisely what is impossible to do for past kernel releases
> (and bisection in particular).
> 

But something like "make CC='gcc -Wa,--size-check=warning'" should work,
I believe (tweaking may be required, but that's the idea).  Passing an
option to the assembler is a helluva lot easier than redirecting to a
different assembler.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH] git-bisect.txt: example for bisecting with hotfix

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> +* Bisect with compatibility hotfix:
> ++
> +------------
> +$ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
> +$ git bisect run sh -c "git cherry-pick -n hotfix || exit 125; make || exit 125; ~/check_test_case.sh"
> +------------
> ++
> +Does the same as the previous example, but applies an additional patch
> +before building. This is useful when your build or test environment changed so
> +that older revisions may need a fix which newer ones have already.
> +

It is a good idea to add an example that shows it is perfectly Ok to muck
with the working tree before testing, but I don't see this patch as such.

First of all, doesn't bisect_checkout does its job with "checkout -q"
without "-f"?  How would that interact with running cherry-pick _every
time_ you check commit to be tested out?  In some situations it may work
OK by accident, but as an example that is likely to be cut&pasted I think
we should show a reasonably safer way than this.

There likely are more than one hot-fixes; it would make more sense to
illustrate merging a hot-fixes branch using "git merge --no-commit" than
using cherry-pick.

But the above are minor; the biggest issue I have with this patch is that
it breaks the train of thought for people who are reading from top to
bottom.

Look at what is there currently. It starts simple (single command run via
"run" interface), and demonstrates that anything complex can be easily
managed with a fully-spelled-out "one test wrapper that builds and then
runs test" example, to show the most generic way you can use. It then
introduces special exit codes in the script, as that form is easier to
read than a single command line.

It then shows, as a final aside, that it isn't strictly required to use a
wrapper script and you could use "sh -c" to wrap that in the command line,
which may be easier to use when (and only when IMO---if you will have
anything that needs debugging then you are better off with the first
approach) the command line you use for testing is trivial.

I think a new example you are adding would fit much better in the flow if
you replaced the example it refers to as "the previous example".  There
are that "previous example" that uses the "check_test_case.sh", and the
one before that one that uses "make test"; they are duplicates that do not
add much value and we would add value by dropping one of them.  It
probably is better to remove the "make test" one and keep the
"check_test_case.sh" one, as long as you explain "check_test_case.sh"
sufficiently well, because the latter is more generally applicable.

The new example would fit well as an illustration of what you _could_ have
in test.sh script when you need to do more elaborate set-up before testing
each revision, e.g., you tweak the working tree with hotfix before running
"make || exit 125", and clean that up after you tested. The core of the
new section would look like this:

	$ cat test.sh
	#!/bin/sh

	# tweak the working tree by merging the hot-fix branch
        # and then attempt a build
	if	git cherry-pick --no-commit hot-fix &&
        	make
	then
                # run project specific test and report its status
                ./test.sh
                status=$?
	else
		# tell the caller this is untestable
		status=125
	fi

	# undo the tweak to allow clean flipping to the next commit
        git reset --hard

	# return control
	exit $status

Re: PATCH: Add --size-check=[error|warning]

[H. Peter Anvin]

On 03/14/2011 08:56 AM, Ian Lance Taylor wrote:
> "Jan Beulich" <JBeulich@novell.com> writes:
> 
>> No - $(AS) isn't being used to translate .S files, $(CC) is being used
>> instead. That's why I pointed at the necessary -B option (so that
>> the compiler would be able to locate the wrapper H.J. suggested).
> 
> You could also just put a -Wa option in your $(CC).  That seems simpler
> than a -B option.
> 
> (I don't really care about this issue one way or another.)
> 

-Wa is definitely the sane way to do it.  The -B thing came in if you
have to run a patched version of as, which is extremely painful.

	-hpa

Re: PATCH: Add --size-check=[error|warning]

[Alan Cox]

> > I disagree.  The whole world is not the linux kernel.  I think HJ is
> > bending over backwards to even offer a switch that turns the error
> > into a warning.
> 
> So what do you suggest that testers who want to, say, build old Linux
> kernel versions with new binutils do?

Use an older binutils. It's already the case you can't build old kernels
with current gcc and binutils, there have been repeated such events over
time and nobody has had too much trouble.

Alan

Re: PATCH: Add --size-check=[error|warning]

[Steven Rostedt]

On Mon, Mar 14, 2011 at 09:32:56AM -0700, H. Peter Anvin wrote:
> On 03/14/2011 04:26 AM, Jan Beulich wrote:
> > 
> > Making the kernel build system check for certain newly introduced
> > gas options would again require changes to the kernel sources,
> > which is precisely what is impossible to do for past kernel releases
> > (and bisection in particular).
> > 
> 
> But something like "make CC='gcc -Wa,--size-check=warning'" should work,
> I believe (tweaking may be required, but that's the idea).  Passing an
> option to the assembler is a helluva lot easier than redirecting to a
> different assembler.
> 
> 	-hpa
> 

Even if the above does work, how do we go about educating users doing
bisects with latest binutils? This is a very common practice among
kernel developers with users that hit bugs. And right now there's just a
handful of people that know of this work-around.

It will become a huge burden to us and our users (which is everyone
using Linux), if we do not understand the reason a build breaks when
doing a bisect, just because some "bug" in asm which binutils use to
work with now errors on.

If it was a bug in asm, but binutils can cope with it, then it should be
a warning. If binutils can't cope, then error.

-- Steve

Re: PATCH: Add --size-check=[error|warning]

[Linus Torvalds]

On Mon, Mar 14, 2011 at 12:24 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>
> If it was a bug in asm, but binutils can cope with it, then it should be
> a warning. If binutils can't cope, then error.

I think this is the really fundamental issue: anybody who makes a hard
error out of something that is recoverable is a total moron.

We have that in the kernel too - I've berated people for using
BUG_ON() _much_ too eagerly. If you make a hard error out of
something, that just makes things much much harder to handle, and is
just a big inconvenience for everybody. Why do it?

In the kernel, a hard error (like BUG_ON()) tends to result in a
system that is unusable and makes logging things harder. And in
development tools, a hard error just means that you stop _everybody_,
whether the user is a developer or just a tester who can't
realistically fix it (or a developer who is not involved in that
area). And even for developers who _are_ directly involved, a hard
error stops the build, instead of just letting it continue and help
him see if there are perhaps _other_ cases that should also be fixed.

So anybody who makes something a hard error when it's not required is
just being a STUPID. It hurts everybody. Don't do it.

                             Linus

[PATCH] Document 'git bisect fix'.

[Ralf Wildenhues]

git bisect is sometimes less effective than it could be in projects
with long-lived but simple bugs (e.g., little-tested configurations).
Rather than skipping vast revision ranges, it might be easier to fix
them up from known bugfix branches.

'git bisect fix' teaches bisect about when some known bug was
introduced and when it was fixed, so that bisect can merge in
the fix when needed into new test candidates.
---

* Ralf Wildenhues wrote on Mon, Mar 14, 2011 at 02:16:23PM CET:
> it would be very helpful if 'git bisect' made it easy to denote
> "when going back, you might also need some of these changes".

Merging in a set of bugfix branches (branches with minimal fixes, based
right off of commits introducing some bug) before testing a particular
contender would be a good start.  Of course we don't want some bugfix
branch to be merged in if the known bug isn't yet in the current
contender, so as to not merge unrelated changes.

Cherry-pick things is another option, but the above seems a bit more
gittish to me, and works well with bugfix branches.  Also, data like
"bugzilla X was introduced by C1 and fixed by C2" is helpful (and
already available in some projects) anyway in a semi-automatic fashion.
You might even want to version it, or keep it in project meta-data.

If some bug was fixed by a merge only, the more general notation
"f_1 ^b_1 ^b_1' ..." could apply.

Here's a balloon doc patch to show what I mean.  Comments?
Is this too unlike how bisect works today?  Too dangerous?

Thanks, and please keep me in Cc:,
Ralf

 Documentation/git-bisect.txt |   20 ++++++++++++++++++++
 git-bisect.sh                |    4 +++-
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index c39d957..9074cb3 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -25,6 +25,7 @@ on the subcommand:
  git bisect replay <logfile>
  git bisect log
  git bisect run <cmd>...
+ git bisect fix [(<range>|<rev>)...]
 
 This command uses 'git rev-list --bisect' to help drive the
 binary search process to find which change introduced a bug, given an
@@ -198,6 +199,25 @@ $ git bisect skip v2.5 v2.5..v2.6
 This tells the bisect process that the commits between `v2.5` included
 and `v2.6` included should be skipped.
 
+Fixing up known bugs
+~~~~~~~~~~~~~~~~~~~~
+
+If many revisions are broken due to some unrelated but known issue that
+is easily fixed, you might want to prefer fixing it up temporarily.
+If `<commit1>` introduces a bug fixed by `<commit2>`, instruct bisect
+to merge the latter before testing a commit that contains the former:
+
+------------
+$ git bisect fix <commit1>..<commit2>
+------------
+
+A single `<commit>` acts as if `<commit>^..<commit>` was specified.
+Fix statements can be repeated for every known bug, and are valid until
+the bisection state is cleaned up with reset.
+
+Any bisect action that causes a new commit to be chosen will try to merge
+the needed fixes and fail if they do not merge cleanly.
+
 
 Cutting down bisection by giving more parameters to bisect start
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/git-bisect.sh b/git-bisect.sh
index c21e33c..2b137f0 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-USAGE='[help|start|bad|good|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|skip|fix|next|reset|visualize|replay|log|run]'
 LONG_USAGE='git bisect help
         print this long help message.
 git bisect start [<bad> [<good>...]] [--] [<pathspec>...]
@@ -11,6 +11,8 @@ git bisect good [<rev>...]
         mark <rev>... known-good revisions.
 git bisect skip [(<rev>|<range>)...]
         mark <rev>... untestable revisions.
+git bisect fix [(<c1>..<c2>|<rev>)...]
+        mark descendants of <c1>/<rev^> as needing fixes <c2>/<rev>.
 git bisect next
         find next bisection to test and check it out.
 git bisect reset [<commit>]
-- 
1.7.4.1.231.ge4ce

Re: [PATCH] Document 'git bisect fix'.

[Yann Dirson]

Hi Ralf,

Maybe you should have made it more obvious that this patch is a RFC
for a proposed feature (say, in subject line).

>'git bisect fix' teaches bisect about when some known bug was
>introduced and when it was fixed, so that bisect can merge in
>the fix when needed into new test candidates.

This sounds like a great idea.  I do have myself to do conditional
cherry-picks from bisect scripts, to deal with this problem.

>If some bug was fixed by a merge only, the more general notation
>"f_1 ^b_1 ^b_1' ..." could apply.

Some more precise example may make this case more clear.


+Fixing up known bugs
+~~~~~~~~~~~~~~~~~~~~
+
+If many revisions are broken due to some unrelated but known issue that
+is easily fixed, you might want to prefer fixing it up temporarily.

It seems natural for "bisect run" to use this mechanism.  What about
the non-automated process ?  We may want to get the fixes applied, or
to only list available fixes to the user so he can "git merge" them
manually, or maybe an interactive selection mode ?  Probably something
to be chosen via some config variable and flags, in a separate patch
of the would-be series.  But what happens initially will be a good thing
to document.

+If `<commit1>` introduces a bug fixed by `<commit2>`, instruct bisect
+to merge the latter before testing a commit that contains the former:
+
+------------
+$ git bisect fix <commit1>..<commit2>
+------------

Usually, a bug also gets fixed by an official commit which does not
fulfill the constraint of being branched at the faulty one.  In this
case you don't want to merge the fix if such a fix is already included,
and thus you will need a way to specify "alternate fixes" to control
this.

+A single `<commit>` acts as if `<commit>^..<commit>` was specified.
+Fix statements can be repeated for every known bug, and are valid until
+the bisection state is cleaned up with reset.

That is on the safe side, but we may at some point want some sort of
"repository of fixes", where this info gets stored for easy reuse on
subsequent bisections.

+Any bisect action that causes a new commit to be chosen will try to merge
+the needed fixes and fail if they do not merge cleanly.

maybe "... similar to what happens when a bisect-run script terminates with exit code
greated than 127." ?

-- 
Yann Dirson - Bertin Technologies

[PATCHv2 1/2] git-bisect.txt: streamline run presentation

[Michael J Gruber]

Streamline the presentation of "bisect run" by removing one example
which does not introduce new concepts.

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
 Documentation/git-bisect.txt |   34 ++++++++--------------------------
 1 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index c39d957..47e8b1e 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -274,53 +274,35 @@ $ git bisect start HEAD origin --    # HEAD is bad, origin is good
 $ git bisect run make test           # "make test" builds and tests
 ------------
 
-* Automatically bisect a broken test suite:
-+
-------------
-$ cat ~/test.sh
-#!/bin/sh
-make || exit 125                   # this skips broken builds
-make test                          # "make test" runs the test suite
-$ git bisect start v1.3 v1.1 --    # v1.3 is bad, v1.1 is good
-$ git bisect run ~/test.sh
-------------
-+
-Here we use a "test.sh" custom script. In this script, if "make"
-fails, we skip the current commit.
-+
-It is safer to use a custom script outside the repository to prevent
-interactions between the bisect, make and test processes and the
-script.
-+
-"make test" should "exit 0", if the test suite passes, and
-"exit 1" otherwise.
-
 * Automatically bisect a broken test case:
 +
 ------------
 $ cat ~/test.sh
 #!/bin/sh
 make || exit 125                     # this skips broken builds
-~/check_test_case.sh                 # does the test case passes ?
+~/check_test_case.sh                 # does the test case pass?
 $ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
 $ git bisect run ~/test.sh
 ------------
 +
-Here "check_test_case.sh" should "exit 0" if the test case passes,
+Here we use a "test.sh" custom script. In this script, if "make"
+fails, we skip the current commit.
+"check_test_case.sh" should "exit 0" if the test case passes,
 and "exit 1" otherwise.
 +
-It is safer if both "test.sh" and "check_test_case.sh" scripts are
+It is safer if both "test.sh" and "check_test_case.sh" are
 outside the repository to prevent interactions between the bisect,
 make and test processes and the scripts.
 
-* Automatically bisect a broken test suite:
+* Automatically bisect a broken test case:
 +
 ------------
 $ git bisect start HEAD HEAD~10 --   # culprit is among the last 10
 $ git bisect run sh -c "make || exit 125; ~/check_test_case.sh"
 ------------
 +
-Does the same as the previous example, but on a single line.
+This shows that you can do without a run script if you write the test
+on a single line.
 
 Author
 ------
-- 
1.7.4.1.404.g62d316

[PATCHv2 2/2] git-bisect.txt: example for bisecting with hot-fix

[Michael J Gruber]

Give an example on how to bisect when older revisions need a hot-fix to
build, run or test. Triggered by the binutils/kernel issue at

http://thread.gmane.org/gmane.comp.gnu.binutils/52601/focus=1112779

Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
---
The example script is basically Junio's, with merge rather than cherry-pick.

 Documentation/git-bisect.txt |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index 47e8b1e..989e223 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -294,6 +294,39 @@ It is safer if both "test.sh" and "check_test_case.sh" are
 outside the repository to prevent interactions between the bisect,
 make and test processes and the scripts.
 
+* Automatically bisect with temporary modifications (hot-fix):
++
+------------
+$ cat ~/test.sh
+#!/bin/sh
+
+# tweak the working tree by merging the hot-fix branch
+# and then attempt a build
+if	git merge --no-commit hot-fix &&
+	make
+then
+	# run project specific test and report its status
+	~/check_test_case.sh
+	status=$?
+else
+	# tell the caller this is untestable
+	status=125
+fi
+
+# undo the tweak to allow clean flipping to the next commit
+git reset --hard
+
+# return control
+exit $status
+------------
++
+This applies modifications from a hot-fix branch before each test run,
+e.g. in case your build or test environment changed so that older
+revisions may need a fix which newer ones have already. (Make sure the
+hot-fix branch is based off a commit which is contained in all revisions
+which you are bisecting, so that the merge does not pull in too much, or
+use `git cherry-pick` instead of `git merge`.)
+
 * Automatically bisect a broken test case:
 +
 ------------
-- 
1.7.4.1.404.g62d316

Re: [PATCH] Document 'git bisect fix'.

[Christian Couder]

Hi,

On Mon, Mar 14, 2011 at 10:00 PM, Ralf Wildenhues
<Ralf.Wildenhues@gmx.de> wrote:
> git bisect is sometimes less effective than it could be in projects
> with long-lived but simple bugs (e.g., little-tested configurations).
> Rather than skipping vast revision ranges, it might be easier to fix
> them up from known bugfix branches.

It's already possible to deal with this problem by creating a new
branch where the bug is fixed, and then using "git replace", so that
the new branch is used instead of the old one.
Please search for "git replace" in this doc:

http://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html

> 'git bisect fix' teaches bisect about when some known bug was
> introduced and when it was fixed, so that bisect can merge in
> the fix when needed into new test candidates.

Perhaps some people would find it easier to use what you suggest but
using git replace may be nicer because you have to create the new
branch once, so you need to fix merge or rebase problems only once.
And the new branch may be useful not only for bisecting, for example
to recreate old versions.

Thanks,
Christian.

Re: [PATCH] Document 'git bisect fix'.

[Michael J Gruber]

Christian Couder venit, vidit, dixit 16.03.2011 10:52:
> Hi,
> 
> On Mon, Mar 14, 2011 at 10:00 PM, Ralf Wildenhues
> <Ralf.Wildenhues@gmx.de> wrote:
>> git bisect is sometimes less effective than it could be in projects
>> with long-lived but simple bugs (e.g., little-tested configurations).
>> Rather than skipping vast revision ranges, it might be easier to fix
>> them up from known bugfix branches.
> 
> It's already possible to deal with this problem by creating a new
> branch where the bug is fixed, and then using "git replace", so that
> the new branch is used instead of the old one.
> Please search for "git replace" in this doc:
> 
> http://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html
> 
>> 'git bisect fix' teaches bisect about when some known bug was
>> introduced and when it was fixed, so that bisect can merge in
>> the fix when needed into new test candidates.
> 
> Perhaps some people would find it easier to use what you suggest but
> using git replace may be nicer because you have to create the new
> branch once, so you need to fix merge or rebase problems only once.
> And the new branch may be useful not only for bisecting, for example
> to recreate old versions.

I'd say the replace method is perfect for transporting an existing fix
"back in time" when the range of non-bisectable commits is limited. But
since you have to replace the right (most recent) commit in that range
it is less convenient when you have a fix due to a changed/exotic build
environment or such which you do not want in your mainline.

Also, you have to rebase the whole history back to the commit which
introduced the problem - and that could be the root commit if the bisect
problems arise from a changed toolchain, like here.

Michael
P.S.: Did you cull cc on purpose or did gmane mess up? Readding AM, LT, TG

Re: [PATCH] Document 'git bisect fix'.

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Christian Couder venit, vidit, dixit 16.03.2011 10:52:
> ...
>> It's already possible to deal with this problem by creating a new
>> branch where the bug is fixed,...
>
> I'd say the replace method is perfect for transporting an existing fix
> "back in time" when the range of non-bisectable commits is limited. But
> since you have to replace the right (most recent) commit in that range
> it is less convenient when you have a fix due to a changed/exotic build
> environment or such which you do not want in your mainline.

I totally agree with Michael.  If somebody has _already_ used "replace" to
make an alternate history in which nobody made any mistake by masking each
and every bug fixed in the past, your bisect would be easier, but that is
nothing more than a theoretical daydreaming. Who in the right mind would
do that?

If you need fixes applied for unrelated bug to even trigger the bug you
are chasing, "replace" is not a practical option. You might even be the
first to notice that these "known fixes" mattered in the part of the
history you happen to be bisecting, and nobody sane would have prepared
such "replace" in the past just in case.

Treat "replace" as nothing more than a reimplementation of "grafts" done
right (i.e. can be transferred using the usual git transfer protocols); I
don't want to see its use advocated for applications it is not suited.  It
just confuses people.