Bug#827339: Please revert patch for cmd, and fix default pattern

[Stephan Sürken]

On Di, 2016-11-22 at 11:33 +0100, Patrick Matthäi wrote:

(...)

> Thanks for your effort! I have just uploaded 1.0.3-3.
> 
> Would you (Stephan) be so kindly and upload it to jessie-bpo, if
> there
> are no new problems with it? I will be on vacation before it may ent

fwiw, I did just port this.

Fwiw2 ;): I have also created an upstream ticket for this:

https://github.com/DE-IBH/apt-dater/issues/124

Hth!

S

Bug#827339: Please revert patch for cmd, and fix default pattern

[Evgeni Golov]

Hi,

On Wed, Nov 23, 2016 at 11:21:02AM +0100, Stephan Sürken wrote:
> @evgeni: Would be nice if you could retest this.

tbh, I stopped using apt-dater, so I'd rather not :)

Bug#827339: Please revert patch for cmd, and fix default pattern

[Stephan Sürken]

Hi Patrick,

On Di, 2016-11-22 at 11:33 +0100, Patrick Matthäi wrote:

(...)

> > S
> Thanks for your effort! I have just uploaded 1.0.3-3.

ok, thx!

> Would you (Stephan) be so kindly and upload it to jessie-bpo, if
> there
> are no new problems with it? I will be on vacation before it may
> enter
> testing.

ok, I will do that if I don't forget ;).

@evgeni: Would be nice if you could retest this.

Fwiw, to properly test matches, I emulated some "error logs" on hosts
using s.th. like this

---
DPkg::Pre-Install-Pkgs {"/usr/bin/printf 'E: apt-dater\n'";};
DPkg::Pre-Install-Pkgs {"/usr/bin/printf 'apt-dater ERROR\n'";};
DPkg::Pre-Install-Pkgs {"/usr/bin/printf 'W: apt-dater\n'";};
---

as extra apt.conf (this still needs an actual package upgrade to take
place to see these logs).

Hth!

S

Bug#827339: Please revert patch for cmd, and fix default pattern

[Patrick Matthäi]

Am 21.11.2016 um 11:49 schrieb Stephan Sürken:
> Hi Patrick,
>
> On Mo, 2016-11-21 at 10:12 +0100, Patrick Matthäi wrote:
>
> (...)
>
>>> S
>> Ok so adding patch
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=827339#34 and drop
>> current patch 01-grep-syntax-error ?
> sure, that would be my suggestion.
>
> For your Debian patch, you should maybe only use the "entity fixes" of
> my patch (not the additions to the regex of my personal liking which
> are also in there ;).
>
> @Thomas:
>
> Of course upstream should fix this too, eventually.
>
> Thx!
>
> S

Thanks for your effort! I have just uploaded 1.0.3-3.

Would you (Stephan) be so kindly and upload it to jessie-bpo, if there
are no new problems with it? I will be on vacation before it may enter
testing.

-- 
/*
Mit freundlichem Gruß / With kind regards,
 Patrick Matthäi
 GNU/Linux Debian Developer

  Blog: http://www.linux-dev.org/
E-Mail: pmatth...@debian.org
patr...@linux-dev.org
*/

Bug#827339: Please revert patch for cmd, and fix default pattern

[Stephan Sürken]

Hi Patrick,

On Mo, 2016-11-21 at 10:12 +0100, Patrick Matthäi wrote:

(...)

> > S
> Ok so adding patch
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=827339#34 and drop
> current patch 01-grep-syntax-error ?

sure, that would be my suggestion.

For your Debian patch, you should maybe only use the "entity fixes" of
my patch (not the additions to the regex of my personal liking which
are also in there ;).

@Thomas:

Of course upstream should fix this too, eventually.

Thx!

S

Bug#827339: Please revert patch for cmd, and fix default pattern

[Patrick Matthäi]

Am 18.11.2016 um 17:51 schrieb Stephan Sürken:
> Hi,
>
> On Fr, 2016-11-18 at 17:01 +0100, Patrick Matthäi wrote:
>> Am 17.11.2016 um 18:52 schrieb Stephan Sürken:
>>> Hi Patrick,
>>>
>>> On Do, 2016-11-17 at 15:40 +0100, Patrick Matthäi wrote:
>>> (...)
 Then the question is, why it does not work on Jessies grep?
>>> did you overlook my comment? Or are my findings wrong?
>>>
>>> Thx!
>>>
>>> S
>> Ah now I get it. You have adjusted the C code of apt-dater, to detect
>> errors without using HTML.
> not quite. I rather fixed the default pattern being used when the user
> does not configure one.
>
>> But we have got still the problem, that the default grep syntax (also
>> your fixed grep syntax) from "typescript" produces a more or less
>> silent
>> "syntax error", like from the beginning of this report.
> afaict, my patch just fixes each and every aspect of this bug report
> ;).
>
> Hth,
>
> S

Ok so adding patch
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=827339#34 and drop
current patch 01-grep-syntax-error ?

-- 
/*
Mit freundlichem Gruß / With kind regards,
 Patrick Matthäi
 GNU/Linux Debian Developer

  Blog: http://www.linux-dev.org/
E-Mail: pmatth...@debian.org
patr...@linux-dev.org
*/

Bug#827339: Please revert patch for cmd, and fix default pattern

[Stephan Sürken]

Hi,

On Fr, 2016-11-18 at 17:01 +0100, Patrick Matthäi wrote:
> Am 17.11.2016 um 18:52 schrieb Stephan Sürken:
> > 
> > Hi Patrick,
> > 
> > On Do, 2016-11-17 at 15:40 +0100, Patrick Matthäi wrote:
> > (...)
> > > 
> > > Then the question is, why it does not work on Jessies grep?
> > did you overlook my comment? Or are my findings wrong?
> > 
> > Thx!
> > 
> > S
> Ah now I get it. You have adjusted the C code of apt-dater, to detect
> errors without using HTML.

not quite. I rather fixed the default pattern being used when the user
does not configure one.

> But we have got still the problem, that the default grep syntax (also
> your fixed grep syntax) from "typescript" produces a more or less
> silent
> "syntax error", like from the beginning of this report.

afaict, my patch just fixes each and every aspect of this bug report
;).

Hth,

S

Bug#827339: Please revert patch for cmd, and fix default pattern

[Patrick Matthäi]

Am 17.11.2016 um 18:52 schrieb Stephan Sürken:
> Hi Patrick,
>
> On Do, 2016-11-17 at 15:40 +0100, Patrick Matthäi wrote:
> (...)
>> Then the question is, why it does not work on Jessies grep?
> did you overlook my comment? Or are my findings wrong?
>
> Thx!
>
> S
Ah now I get it. You have adjusted the C code of apt-dater, to detect
errors without using HTML.
But we have got still the problem, that the default grep syntax (also
your fixed grep syntax) from "typescript" produces a more or less silent
"syntax error", like from the beginning of this report.

-- 
/*
Mit freundlichem Gruß / With kind regards,
 Patrick Matthäi
 GNU/Linux Debian Developer

  Blog: http://www.linux-dev.org/
E-Mail: pmatth...@debian.org
patr...@linux-dev.org
*/

Bug#827339: Please revert patch for cmd, and fix default pattern

[Stephan Sürken]

Hi Patrick,

On Do, 2016-11-17 at 15:40 +0100, Patrick Matthäi wrote:
(...)
> Then the question is, why it does not work on Jessies grep?

did you overlook my comment? Or are my findings wrong?

Thx!

S

Bug#827339: Please revert patch for cmd, and fix default pattern

[Patrick Matthäi]

Am 10.11.2016 um 22:09 schrieb Thomas Liske:
> Hi,
>
> this is a negative lookbehind assertion - quoting from perlre(1):
>
>"(?A zero-width negative lookbehind assertion.  For example
>"/(?follow "bar".  Works only for fixed-width lookbehind.
>
> So it should match 'error' (if it does not follow 'no ') and it should
> match 'warning', 'fail' and 'failed' (if it does not follow 'insserv: ').

Then the question is, why it does not work on Jessies grep?
>
>
> HTH,
> Thomas
>
>
> Patrick Matthäi  writes:
>
>> Am 30.10.2016 um 12:31 schrieb Stephan Sürken:
>>> Hi Evgeni, Patrick,
>>>
>>> fwiw (probably this is already worked on), i have fixed up my system by
>>>
>>> * reverting 01-grep-syntax-error.diff
>>>
>>> This actually totally breaks things, as this now uses "P" as pattern,
>>> practically matching always:
>>>
>>> --
>>> grep -aiqseP "$AD_HIST_ERRPATTERN" "$AD_HIST_PATH/typescript"
>>> --
>>>
>>> I would actually like to see call grep (in the cmd script) called like so
>>>
>>> --
>>> grep -a -q -i -s -P -e "$AD_HIST_ERRPATTERN" "$AD_HIST_PATH/typescript"
>>> --
>>>
>>> (however, '-aiqsPe' should work as well).
>>>
>>> * Use custom err-pattern
>>>
>>> I don't really know what's wrong with the default perl regex (should
>>> the '' stuff be actually be in the final string?), I am just using
>>> a simpler one now that works. On a shell you can see the error via
>>>
>>> --
>>> grep -P -e '((?!no )error|(?!insserv: )warning|fail(ed)?)'
>>> --
>>>
>>> (That's the same string you also see in the "meta" debug file, using
>>> all defaults for err-pattern).
>>>
>>> So for me it seems:
>>>
>>> * With the default pattern in place (and pre-patch), the grep would
>>> always fail, meaning it would never detect an actual error in
>>> typescript.
>>> * With the broken patch, it always wrongly finds errors (while showing
>>> an empty log via less).
>>>
>>> So please remove 01-grep-syntax-error.diff, and somehow fix the default
>>> pattern ;).
>>>
>>> Hth!
>>>
>>> S
>> I already noticed, that my patch always matches and removed it in my
>> vcs. On my cmd it worked..
>> Anyway with "grep -a -q -i -s -P -e" and "grep -P -e '((?!no
>> )error|(?!insserv: )warning|fail(ed)?)'" I get again a syntax error
>> on jessie. Maybe Thomas could say us, what he wanted to match and how we
>> should fix it now.
>>
>> -- 
>> /*
>> Mit freundlichem Gruß / With kind regards,
>>  Patrick Matthäi
>>  GNU/Linux Debian Developer
>>
>>   Blog: http://www.linux-dev.org/
>> E-Mail: pmatth...@debian.org
>> patr...@linux-dev.org
>> */
>>

-- 
/*
Mit freundlichem Gruß / With kind regards,
 Patrick Matthäi
 GNU/Linux Debian Developer

  Blog: http://www.linux-dev.org/
E-Mail: pmatth...@debian.org
patr...@linux-dev.org
*/

Bug#827339: Please revert patch for cmd, and fix default pattern

[Stephan Sürken]

Hi all,

On Do, 2016-11-10 at 22:09 +0100, Thomas Liske wrote:

(...)

> this is a negative lookbehind assertion - quoting from perlre(1):

(...)

Afaics, the hardcoded default value for err-pattern falsely uses HTML
entities. These are needed in the XML config obviously, but not in
getXPropStr() default value argument.

With this patch, the default "err-pattern" works for me:

---
$ absurd? cat debian/patches/02-fix-default-err-pattern.patch 
diff --git a/src/keyfiles.c b/src/keyfiles.c
index affdb2b..2e04a6b 100644
--- a/src/keyfiles.c
+++ b/src/keyfiles.c
@@ -401,7 +401,8 @@ gboolean loadConfig(const gchar *filename, CfgFile *lcfg) {
 
 #ifdef FEAT_HISTORY
 lcfg->record_history = getXPropBool(s_history, "record", TRUE);
-lcfg->history_errpattern = getXPropStr(s_history, "err-pattern", 
"((?!no )error|(?!insserv: )warning|fail(ed)?)");
+/* lcfg->history_errpattern = getXPropStr(s_history, "err-pattern", 
"((?!no )error|(?!insserv: )warning|fail(ed)?)"); */
+lcfg->history_errpattern = getXPropStr(s_history, "err-pattern", "((?history_dir = getXPropStr(s_path, "history-dir", 
g_strdup_printf("%s/%s/history", g_get_user_data_dir(), PACKAGE));
 #endif
---

This patch also adds patterns for "E: foo" "W: bar"-types of logs
(which I think are useful).

Hth!

S

Bug#827339: Please revert patch for cmd, and fix default pattern

[Thomas Liske]

Hi,

this is a negative lookbehind assertion - quoting from perlre(1):

   "(? writes:

> Am 30.10.2016 um 12:31 schrieb Stephan Sürken:
>> Hi Evgeni, Patrick,
>>
>> fwiw (probably this is already worked on), i have fixed up my system by
>>
>> * reverting 01-grep-syntax-error.diff
>>
>> This actually totally breaks things, as this now uses "P" as pattern,
>> practically matching always:
>>
>> --
>> grep -aiqseP "$AD_HIST_ERRPATTERN" "$AD_HIST_PATH/typescript"
>> --
>>
>> I would actually like to see call grep (in the cmd script) called like so
>>
>> --
>> grep -a -q -i -s -P -e "$AD_HIST_ERRPATTERN" "$AD_HIST_PATH/typescript"
>> --
>>
>> (however, '-aiqsPe' should work as well).
>>
>> * Use custom err-pattern
>>
>> I don't really know what's wrong with the default perl regex (should
>> the '' stuff be actually be in the final string?), I am just using
>> a simpler one now that works. On a shell you can see the error via
>>
>> --
>> grep -P -e '((?!no )error|(?!insserv: )warning|fail(ed)?)'
>> --
>>
>> (That's the same string you also see in the "meta" debug file, using
>> all defaults for err-pattern).
>>
>> So for me it seems:
>>
>> * With the default pattern in place (and pre-patch), the grep would
>> always fail, meaning it would never detect an actual error in
>> typescript.
>> * With the broken patch, it always wrongly finds errors (while showing
>> an empty log via less).
>>
>> So please remove 01-grep-syntax-error.diff, and somehow fix the default
>> pattern ;).
>>
>> Hth!
>>
>> S
> I already noticed, that my patch always matches and removed it in my
> vcs. On my cmd it worked..
> Anyway with "grep -a -q -i -s -P -e" and "grep -P -e '((?!no
> )error|(?!insserv: )warning|fail(ed)?)'" I get again a syntax error
> on jessie. Maybe Thomas could say us, what he wanted to match and how we
> should fix it now.
>
> -- 
> /*
> Mit freundlichem Gruß / With kind regards,
>  Patrick Matthäi
>  GNU/Linux Debian Developer
>
>   Blog: http://www.linux-dev.org/
> E-Mail: pmatth...@debian.org
> patr...@linux-dev.org
> */
>

-- 

::  WWW:https://fiasko-nw.net/~thomas/  ::
   :::  Jabber:   xmpp:tho...@jabber.fiasko-nw.net  :::
::  flickr: https://www.flickr.com/photos/laugufe/  ::

Bug#827339: Please revert patch for cmd, and fix default pattern

[Stephan Sürken]

Hi Evgeni, Patrick,

fwiw (probably this is already worked on), i have fixed up my system by

* reverting 01-grep-syntax-error.diff

This actually totally breaks things, as this now uses "P" as pattern,
practically matching always:

--
grep -aiqseP "$AD_HIST_ERRPATTERN" "$AD_HIST_PATH/typescript"
--

I would actually like to see call grep (in the cmd script) called like so

--
grep -a -q -i -s -P -e "$AD_HIST_ERRPATTERN" "$AD_HIST_PATH/typescript"
--

(however, '-aiqsPe' should work as well).

* Use custom err-pattern

I don't really know what's wrong with the default perl regex (should
the '' stuff be actually be in the final string?), I am just using
a simpler one now that works. On a shell you can see the error via

--
grep -P -e '((?!no )error|(?!insserv: )warning|fail(ed)?)'
--

(That's the same string you also see in the "meta" debug file, using
all defaults for err-pattern).

So for me it seems:

* With the default pattern in place (and pre-patch), the grep would
always fail, meaning it would never detect an actual error in
typescript.
* With the broken patch, it always wrongly finds errors (while showing
an empty log via less).

So please remove 01-grep-syntax-error.diff, and somehow fix the default
pattern ;).

Hth!

S