[PATCH] eapply: Drop -s option for patch.

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[PATCH] eapply: Drop -s option for patch.

Ulrich Müller
We generally try to have verbose build logs, e.g., by calling
configure with --disable-silent-rules. Silencing patch contradicts
this, and will suppress reporting of fuzz factors.

Note that the eapply specification in PMS calls patch without -s:
https://projects.gentoo.org/pms/7/pms.html#x1-127001r1
Traditionally, the -s option wasn't used by epatch either.

Bug: https://bugs.gentoo.org/674562
Signed-off-by: Ulrich Müller <[hidden email]>
---
 bin/phase-helpers.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
index b53d39650..60f8d3243 100644
--- a/bin/phase-helpers.sh
+++ b/bin/phase-helpers.sh
@@ -993,10 +993,9 @@ if ___eapi_has_eapply; then
  ebegin "${prefix:-Applying }${f##*/}"
  # -p1 as a sane default
  # -f to avoid interactivity
- # -s to silence progress output
  # -g0 to guarantee no VCS interaction
  # --no-backup-if-mismatch not to pollute the sources
- ${patch_cmd} -p1 -f -s -g0 --no-backup-if-mismatch \
+ ${patch_cmd} -p1 -f -g0 --no-backup-if-mismatch \
  "${patch_options[@]}" < "${f}"
  failed=${?}
  if ! eend "${failed}"; then
--
2.24.0

signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Zac Medico-2
On 11/25/19 5:03 AM, Ulrich Müller wrote:

> We generally try to have verbose build logs, e.g., by calling
> configure with --disable-silent-rules. Silencing patch contradicts
> this, and will suppress reporting of fuzz factors.
>
> Note that the eapply specification in PMS calls patch without -s:
> https://projects.gentoo.org/pms/7/pms.html#x1-127001r1
> Traditionally, the -s option wasn't used by epatch either.
>
> Bug: https://bugs.gentoo.org/674562
> Signed-off-by: Ulrich Müller <[hidden email]>
> ---
>  bin/phase-helpers.sh | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
> index b53d39650..60f8d3243 100644
> --- a/bin/phase-helpers.sh
> +++ b/bin/phase-helpers.sh
> @@ -993,10 +993,9 @@ if ___eapi_has_eapply; then
>   ebegin "${prefix:-Applying }${f##*/}"
>   # -p1 as a sane default
>   # -f to avoid interactivity
> - # -s to silence progress output
>   # -g0 to guarantee no VCS interaction
>   # --no-backup-if-mismatch not to pollute the sources
> - ${patch_cmd} -p1 -f -s -g0 --no-backup-if-mismatch \
> + ${patch_cmd} -p1 -f -g0 --no-backup-if-mismatch \
>   "${patch_options[@]}" < "${f}"
>   failed=${?}
>   if ! eend "${failed}"; then
>
Looks good. Please merge.
--
Thanks,
Zac


signature.asc (1000 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Mike Gilbert-2
On Mon, Nov 25, 2019 at 11:53 AM Zac Medico <[hidden email]> wrote:

>
> On 11/25/19 5:03 AM, Ulrich Müller wrote:
> > We generally try to have verbose build logs, e.g., by calling
> > configure with --disable-silent-rules. Silencing patch contradicts
> > this, and will suppress reporting of fuzz factors.
> >
> > Note that the eapply specification in PMS calls patch without -s:
> > https://projects.gentoo.org/pms/7/pms.html#x1-127001r1
> > Traditionally, the -s option wasn't used by epatch either.
> >
> > Bug: https://bugs.gentoo.org/674562
> > Signed-off-by: Ulrich Müller <[hidden email]>
> > ---
> >  bin/phase-helpers.sh | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/bin/phase-helpers.sh b/bin/phase-helpers.sh
> > index b53d39650..60f8d3243 100644
> > --- a/bin/phase-helpers.sh
> > +++ b/bin/phase-helpers.sh
> > @@ -993,10 +993,9 @@ if ___eapi_has_eapply; then
> >                       ebegin "${prefix:-Applying }${f##*/}"
> >                       # -p1 as a sane default
> >                       # -f to avoid interactivity
> > -                     # -s to silence progress output
> >                       # -g0 to guarantee no VCS interaction
> >                       # --no-backup-if-mismatch not to pollute the sources
> > -                     ${patch_cmd} -p1 -f -s -g0 --no-backup-if-mismatch \
> > +                     ${patch_cmd} -p1 -f -g0 --no-backup-if-mismatch \
> >                               "${patch_options[@]}" < "${f}"
> >                       failed=${?}
> >                       if ! eend "${failed}"; then
> >
>
> Looks good. Please merge.

I think this should be reverted. It causes too much noise, and
"solves" a problem only very rarely.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Ulrich Müller
>>>>> On Thu, 12 Dec 2019, Mike Gilbert wrote:

> I think this should be reverted. It causes too much noise, and
> "solves" a problem only very rarely.

Now, how many lines of output does this typically produce, compared
to the total size of a typical build log? Especially with mgorny's
subsequent modification, which suppresses the output unless the patch
doesn't apply cleanly.

It was also suggested that we add -F0 in EAPI 8, but that would break
the build in those cases that are producing extra output now. I don't
think that would be preferable.

Ulrich

signature.asc (497 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michael Orlitzky
On 12/12/19 3:15 PM, Ulrich Mueller wrote:
>
> It was also suggested that we add -F0 in EAPI 8, but that would break
> the build in those cases that are producing extra output now. I don't
> think that would be preferable.

It would only break the build for the maintainer, who would then
presumably fix the patch.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Mike Gilbert-2
In reply to this post by Ulrich Müller
On Thu, Dec 12, 2019 at 3:15 PM Ulrich Mueller <[hidden email]> wrote:

>
> >>>>> On Thu, 12 Dec 2019, Mike Gilbert wrote:
>
> > I think this should be reverted. It causes too much noise, and
> > "solves" a problem only very rarely.
>
> Now, how many lines of output does this typically produce, compared
> to the total size of a typical build log? Especially with mgorny's
> subsequent modification, which suppresses the output unless the patch
> doesn't apply cleanly.

In most cases, I would be inclined to simply ignore the patch output
since there's really no need for me to take any action on it.

On the other hand, it makes it more difficult to quickly identify the
list of patches being applied if there is junk output in the middle of
the list.

> It was also suggested that we add -F0 in EAPI 8, but that would break
> the build in those cases that are producing extra output now. I don't
> think that would be preferable.

I am opposed to including such a change in EAPI 8. It would make
ebuild maintenance more difficult for everyone, and I don't think the
potential benefit is worth it.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michał Górny-5
On Fri, 2019-12-13 at 08:47 -0500, Mike Gilbert wrote:

> On Thu, Dec 12, 2019 at 3:15 PM Ulrich Mueller <[hidden email]> wrote:
> > > > > > > On Thu, 12 Dec 2019, Mike Gilbert wrote:
> > > I think this should be reverted. It causes too much noise, and
> > > "solves" a problem only very rarely.
> >
> > Now, how many lines of output does this typically produce, compared
> > to the total size of a typical build log? Especially with mgorny's
> > subsequent modification, which suppresses the output unless the patch
> > doesn't apply cleanly.
>
> In most cases, I would be inclined to simply ignore the patch output
> since there's really no need for me to take any action on it.
>
> On the other hand, it makes it more difficult to quickly identify the
> list of patches being applied if there is junk output in the middle of
> the list.
>
> > It was also suggested that we add -F0 in EAPI 8, but that would break
> > the build in those cases that are producing extra output now. I don't
> > think that would be preferable.
>
> I am opposed to including such a change in EAPI 8. It would make
> ebuild maintenance more difficult for everyone, and I don't think the
> potential benefit is worth it.
...and why do we consider it correct to apply patches when the context
doesn't match?  If our only goal is to make things 'easier' for
'everyone', then we could just pass -F9999 and ignore all the context.

Though I don't understand why include any context in the first place if
you don't care about it matching.  Sounds like a waste of space to me!

--
Best regards,
Michał Górny


signature.asc (631 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Mike Gilbert-2
On Fri, Dec 13, 2019 at 8:52 AM Michał Górny <[hidden email]> wrote:

>
> On Fri, 2019-12-13 at 08:47 -0500, Mike Gilbert wrote:
> > On Thu, Dec 12, 2019 at 3:15 PM Ulrich Mueller <[hidden email]> wrote:
> > > > > > > > On Thu, 12 Dec 2019, Mike Gilbert wrote:
> > > > I think this should be reverted. It causes too much noise, and
> > > > "solves" a problem only very rarely.
> > >
> > > Now, how many lines of output does this typically produce, compared
> > > to the total size of a typical build log? Especially with mgorny's
> > > subsequent modification, which suppresses the output unless the patch
> > > doesn't apply cleanly.
> >
> > In most cases, I would be inclined to simply ignore the patch output
> > since there's really no need for me to take any action on it.
> >
> > On the other hand, it makes it more difficult to quickly identify the
> > list of patches being applied if there is junk output in the middle of
> > the list.
> >
> > > It was also suggested that we add -F0 in EAPI 8, but that would break
> > > the build in those cases that are producing extra output now. I don't
> > > think that would be preferable.
> >
> > I am opposed to including such a change in EAPI 8. It would make
> > ebuild maintenance more difficult for everyone, and I don't think the
> > potential benefit is worth it.
>
> ...and why do we consider it correct to apply patches when the context
> doesn't match?  If our only goal is to make things 'easier' for
> 'everyone', then we could just pass -F9999 and ignore all the context.
>
> Though I don't understand why include any context in the first place if
> you don't care about it matching.  Sounds like a waste of space to me!

The patch command defaults to -F2. If that makes no sense, why is it
the upstream default?

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michał Górny-5
On Fri, 2019-12-13 at 09:06 -0500, Mike Gilbert wrote:

> On Fri, Dec 13, 2019 at 8:52 AM Michał Górny <[hidden email]> wrote:
> > On Fri, 2019-12-13 at 08:47 -0500, Mike Gilbert wrote:
> > > On Thu, Dec 12, 2019 at 3:15 PM Ulrich Mueller <[hidden email]> wrote:
> > > > > > > > > On Thu, 12 Dec 2019, Mike Gilbert wrote:
> > > > > I think this should be reverted. It causes too much noise, and
> > > > > "solves" a problem only very rarely.
> > > >
> > > > Now, how many lines of output does this typically produce, compared
> > > > to the total size of a typical build log? Especially with mgorny's
> > > > subsequent modification, which suppresses the output unless the patch
> > > > doesn't apply cleanly.
> > >
> > > In most cases, I would be inclined to simply ignore the patch output
> > > since there's really no need for me to take any action on it.
> > >
> > > On the other hand, it makes it more difficult to quickly identify the
> > > list of patches being applied if there is junk output in the middle of
> > > the list.
> > >
> > > > It was also suggested that we add -F0 in EAPI 8, but that would break
> > > > the build in those cases that are producing extra output now. I don't
> > > > think that would be preferable.
> > >
> > > I am opposed to including such a change in EAPI 8. It would make
> > > ebuild maintenance more difficult for everyone, and I don't think the
> > > potential benefit is worth it.
> >
> > ...and why do we consider it correct to apply patches when the context
> > doesn't match?  If our only goal is to make things 'easier' for
> > 'everyone', then we could just pass -F9999 and ignore all the context.
> >
> > Though I don't understand why include any context in the first place if
> > you don't care about it matching.  Sounds like a waste of space to me!
>
> The patch command defaults to -F2. If that makes no sense, why is it
> the upstream default?
>
You should ask upstream, not me.  But if I were to guess, the answer
would be because patch(1) is used by random people trying to apply
random patches they've found somewhere.  We on the other hand are
applying patches that *we* are supposed to provide.

--
Best regards,
Michał Górny


signature.asc (631 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Fabian Groffen-2
On 13-12-2019 15:24:40 +0100, Michał Górny wrote:

> > > ...and why do we consider it correct to apply patches when the context
> > > doesn't match?  If our only goal is to make things 'easier' for
> > > 'everyone', then we could just pass -F9999 and ignore all the context.
> > >
> > > Though I don't understand why include any context in the first place if
> > > you don't care about it matching.  Sounds like a waste of space to me!
> >
> > The patch command defaults to -F2. If that makes no sense, why is it
> > the upstream default?
> >
>
> You should ask upstream, not me.  But if I were to guess, the answer
> would be because patch(1) is used by random people trying to apply
> random patches they've found somewhere.  We on the other hand are
> applying patches that *we* are supposed to provide.
We are providing those patches, maybe.  In reality very often the
patches originate from somewhere else though.  And you don't want to
have to respin all of those just because.  At least that's what I feel.

Thanks,
Fabian

--
Fabian Groffen
Gentoo on a different level

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michael Orlitzky
On 12/13/19 9:28 AM, Fabian Groffen wrote:
>
> We are providing those patches, maybe.  In reality very often the
> patches originate from somewhere else though.  And you don't want to
> have to respin all of those just because.  At least that's what I feel.
>

Just because... the context changed? A new "!" in a line of context can
be the difference between letting someone log in with the right password
and letting them log in with the wrong password. You should at least
have to stop and verify that the patch does what you think it does when
it "gains" fuzz. And at that point, git diff will give you a clean
version of it.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Zac Medico-2
In reply to this post by Fabian Groffen-2
On 12/13/19 6:28 AM, Fabian Groffen wrote:

> On 13-12-2019 15:24:40 +0100, Michał Górny wrote:
>>>> ...and why do we consider it correct to apply patches when the context
>>>> doesn't match?  If our only goal is to make things 'easier' for
>>>> 'everyone', then we could just pass -F9999 and ignore all the context.
>>>>
>>>> Though I don't understand why include any context in the first place if
>>>> you don't care about it matching.  Sounds like a waste of space to me!
>>>
>>> The patch command defaults to -F2. If that makes no sense, why is it
>>> the upstream default?
>>>
>>
>> You should ask upstream, not me.  But if I were to guess, the answer
>> would be because patch(1) is used by random people trying to apply
>> random patches they've found somewhere.  We on the other hand are
>> applying patches that *we* are supposed to provide.
>
> We are providing those patches, maybe.  In reality very often the
> patches originate from somewhere else though.  And you don't want to
> have to respin all of those just because.  At least that's what I feel.
Yeah, the QA Notice is apparently intended for downstream patches that
require frequent rebase, while it doesn't make much sense for patches
that are cherry-picked from upstream.
--
Thanks,
Zac


signature.asc (1000 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Fabian Groffen-2
In reply to this post by Michael Orlitzky
On 13-12-2019 14:24:33 -0500, Michael Orlitzky wrote:

> On 12/13/19 9:28 AM, Fabian Groffen wrote:
> >
> > We are providing those patches, maybe.  In reality very often the
> > patches originate from somewhere else though.  And you don't want to
> > have to respin all of those just because.  At least that's what I feel.
> >
>
> Just because... the context changed? A new "!" in a line of context can
> be the difference between letting someone log in with the right password
> and letting them log in with the wrong password. You should at least
> have to stop and verify that the patch does what you think it does when
> it "gains" fuzz. And at that point, git diff will give you a clean
> version of it.
Counter argument is that we've been doing this for decades, and always
relied on maintainers to check the contents of their patches, without
problems.  We didn't introduce a predictable random number generator or
something.
Your very specific example just illustrates the niche this proposal is
targetting.

As with many of the proposals lately, they just seem to aim at more work
for individual maintainers, with a very low gain ratio.
Better, allow this to be a FEATURE, or whatever, that devs should enable,
but don't spit this in the user's face by default.

Thanks,
Fabian

--
Fabian Groffen
Gentoo on a different level

signature.asc (499 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michał Górny-5
On Fri, 2019-12-13 at 21:31 +0100, Fabian Groffen wrote:

> On 13-12-2019 14:24:33 -0500, Michael Orlitzky wrote:
> > On 12/13/19 9:28 AM, Fabian Groffen wrote:
> > > We are providing those patches, maybe.  In reality very often the
> > > patches originate from somewhere else though.  And you don't want to
> > > have to respin all of those just because.  At least that's what I feel.
> > >
> >
> > Just because... the context changed? A new "!" in a line of context can
> > be the difference between letting someone log in with the right password
> > and letting them log in with the wrong password. You should at least
> > have to stop and verify that the patch does what you think it does when
> > it "gains" fuzz. And at that point, git diff will give you a clean
> > version of it.
>
> Counter argument is that we've been doing this for decades, and always
> relied on maintainers to check the contents of their patches, without
> problems.  We didn't introduce a predictable random number generator or
> something.
Is this really an argument for or *against* it?  Developers are entirely
capable of keeping seds that do nothing for years, as well as patches
that -- while apparently applying correctly -- are entirely meaningless.
Should I remind you that epatch was entirely capable of creating
meaningless files by randomly applying the wrong patch level?

> Your very specific example just illustrates the niche this proposal is
> targetting.
>
> As with many of the proposals lately, they just seem to aim at more work
> for individual maintainers, with a very low gain ratio.
> Better, allow this to be a FEATURE, or whatever, that devs should enable,
> but don't spit this in the user's face by default.
>

Just like 'many of the proposals lately', developers are going to be
the ones disabling it (because they don't care), and users will be the
ones enabling it (because they do care), just to learn that developers
don't care and go complaining to the mailing lists that users dare
report issues they don't care about.


--
Best regards,
Michał Górny


signature.asc (631 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Mike Gilbert-2
On Fri, Dec 13, 2019 at 3:36 PM Michał Górny <[hidden email]> wrote:
> Just like 'many of the proposals lately', developers are going to be
> the ones disabling it (because they don't care), and users will be the
> ones enabling it (because they do care), just to learn that developers
> don't care and go complaining to the mailing lists that users dare
> report issues they don't care about.

I care if the patch is actually broken, which the warning doesn't
really tell me. It's just not a very reliable indicator, and will
produce false-positives frequently.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michał Górny-5
On Fri, 2019-12-13 at 16:37 -0500, Mike Gilbert wrote:

> On Fri, Dec 13, 2019 at 3:36 PM Michał Górny <[hidden email]> wrote:
> > Just like 'many of the proposals lately', developers are going to be
> > the ones disabling it (because they don't care), and users will be the
> > ones enabling it (because they do care), just to learn that developers
> > don't care and go complaining to the mailing lists that users dare
> > report issues they don't care about.
>
> I care if the patch is actually broken, which the warning doesn't
> really tell me. It's just not a very reliable indicator, and will
> produce false-positives frequently.
>
You can also take less context into the patch and use -F0.  Then you'll
have the same effect, no warnings to bother you and no pretending that
the patch applies when it doesn't.

--
Best regards,
Michał Górny


signature.asc (631 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michael 'veremitz' Everitt
On 13/12/19 21:42, Michał Górny wrote:

> On Fri, 2019-12-13 at 16:37 -0500, Mike Gilbert wrote:
>> On Fri, Dec 13, 2019 at 3:36 PM Michał Górny <[hidden email]> wrote:
>>> Just like 'many of the proposals lately', developers are going to be
>>> the ones disabling it (because they don't care), and users will be the
>>> ones enabling it (because they do care), just to learn that developers
>>> don't care and go complaining to the mailing lists that users dare
>>> report issues they don't care about.
>> I care if the patch is actually broken, which the warning doesn't
>> really tell me. It's just not a very reliable indicator, and will
>> produce false-positives frequently.
>>
> You can also take less context into the patch and use -F0.  Then you'll
> have the same effect, no warnings to bother you and no pretending that
> the patch applies when it doesn't.
>
Is there any mileage in having a similar scheme to which we already apply
'-p' increments to the -F variable?
eg.
1) attempt patch with -F0
2) if (1) fails, attempt with -F2/3 & display 'yellow' warning & QA notice
3) if (2) fails, attempt with, say, -F10 & display big fat 'red' warning
and QA notice
4) Fail and abort

Regards,
veremitz/Michael.


signature.asc (817 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michał Górny-5
On Fri, 2019-12-13 at 21:56 +0000, Michael 'veremitz' Everitt wrote:

> On 13/12/19 21:42, Michał Górny wrote:
> > On Fri, 2019-12-13 at 16:37 -0500, Mike Gilbert wrote:
> > > On Fri, Dec 13, 2019 at 3:36 PM Michał Górny <[hidden email]> wrote:
> > > > Just like 'many of the proposals lately', developers are going to be
> > > > the ones disabling it (because they don't care), and users will be the
> > > > ones enabling it (because they do care), just to learn that developers
> > > > don't care and go complaining to the mailing lists that users dare
> > > > report issues they don't care about.
> > > I care if the patch is actually broken, which the warning doesn't
> > > really tell me. It's just not a very reliable indicator, and will
> > > produce false-positives frequently.
> > >
> > You can also take less context into the patch and use -F0.  Then you'll
> > have the same effect, no warnings to bother you and no pretending that
> > the patch applies when it doesn't.
> >
> Is there any mileage in having a similar scheme to which we already apply
> '-p' increments to the -F variable?
> eg.
> 1) attempt patch with -F0
> 2) if (1) fails, attempt with -F2/3 & display 'yellow' warning & QA notice
That is precisely what my patch does and what people are complaining
about.

> 3) if (2) fails, attempt with, say, -F10 & display big fat 'red' warning
> and QA notice

That makes no sense as it exceeds context provided in most patches.

--
Best regards,
Michał Górny


signature.asc (631 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] eapply: Drop -s option for patch.

Michael 'veremitz' Everitt
On 13/12/19 21:59, Michał Górny wrote:

> On Fri, 2019-12-13 at 21:56 +0000, Michael 'veremitz' Everitt wrote:
>> On 13/12/19 21:42, Michał Górny wrote:
>>> On Fri, 2019-12-13 at 16:37 -0500, Mike Gilbert wrote:
>>>> On Fri, Dec 13, 2019 at 3:36 PM Michał Górny <[hidden email]> wrote:
>>>>> Just like 'many of the proposals lately', developers are going to be
>>>>> the ones disabling it (because they don't care), and users will be the
>>>>> ones enabling it (because they do care), just to learn that developers
>>>>> don't care and go complaining to the mailing lists that users dare
>>>>> report issues they don't care about.
>>>> I care if the patch is actually broken, which the warning doesn't
>>>> really tell me. It's just not a very reliable indicator, and will
>>>> produce false-positives frequently.
>>>>
>>> You can also take less context into the patch and use -F0.  Then you'll
>>> have the same effect, no warnings to bother you and no pretending that
>>> the patch applies when it doesn't.
>>>
>> Is there any mileage in having a similar scheme to which we already apply
>> '-p' increments to the -F variable?
>> eg.
>> 1) attempt patch with -F0
>> 2) if (1) fails, attempt with -F2/3 & display 'yellow' warning & QA notice
> That is precisely what my patch does and what people are complaining
> about.
Ah, apologies for the failure to grok.

Tangentially, but also brought up on the thread, I'm actually even
moderately concerned about the ghost seds that may never apply. Topic for
another thread though I feel.
>> 3) if (2) fails, attempt with, say, -F10 & display big fat 'red' warning
>> and QA notice
> That makes no sense as it exceeds context provided in most patches.
>
Fair .. hadn't thought of that - depends very much if you're using unified
diffs, which I believe we largely are.


signature.asc (817 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

RFC: [QA] notice with 'failed' seds [was PATCH: eapply drop -s option]

Michael 'veremitz' Everitt
In reply to this post by Michał Górny-5
On 13/12/19 20:36, Michał Górny wrote [excerpted]:
>
> Is this really an argument for or *against* it?  Developers are entirely
> capable of keeping seds that do nothing for years, as well as patches
> that -- while apparently applying correctly -- are entirely meaningless.
<snip>

I think there is some merit in some kind of feedback when sed's are doing
nothing, although how feasible it is to generate any useful feedback I
can't say. I wouldn't say it needs to explicitly fail or make lots of
noise, just an info message that could prompt some further investigation.


signature.asc (817 bytes) Download Attachment
12