[sssd PR#5593][opened] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: opened

PR body:
"""
:relnote:This release deprecates pcre1 support. This support will be
removed completely in following releases.
"""

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][synchronized] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
Under valgrind:
```
ERROR: test_ipa_subdom_server
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-823040655

[sssd PR#5593][+Changes requested] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Changes requested

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
Under valgrind:
```
ERROR: dyndns-tests
ERROR: nestedgroups-tests
ERROR: dp_opt_tests
ERROR: test_ipa_subdom_server
```
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-823040655

[sssd PR#5593][synchronized] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][synchronized] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][synchronized] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
I fixed issues in `nestedgroups-tests` and `dp_opt_tests`.
Two other still fail.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-823831521

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
Ok, it seems the issue with `dyndns-tests` is the following:
 - this test wraps `execv()`
 - thus child helpers doing fork()+execv() keep all memory content of original process
 - at exit Valgrind doesn't "understand" that `_sss_regexp_t::pcre2_code*`
is enough to free all memory allocated for `re` and reports it as `possibly lost`

As Sumit pointed out, there is [suppression pattern for
pcre](https://github.com/SSSD/sssd/blob/29abf94e3aec2c54c76adf61318099097....
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-824233050

[sssd PR#5593][synchronized] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][+Waiting for review] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Waiting for review

[sssd PR#5593][-Changes requested] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: -Changes requested

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

sumit-bose commented:
"""
Hi,

it looks like only `PCRE_LIBS` and `PCRE_CLAGS` are used in `Makefile.am` but the
configure check defines only  `PCRE2_LIBS` and `PCRE2_CFLAGS` if version 2 is found.

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-824993325

[sssd PR#5593][+Changes requested] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Changes requested

[sssd PR#5593][-Waiting for review] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: -Waiting for review

[sssd PR#5593][synchronized] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
>  it looks like only `PCRE_LIBS` and `PCRE_CLAGS` are used in
> `Makefile.am` but the configure check defines only `PCRE2_LIBS` and `PCRE2_CFLAGS` if
> version 2 is found.
>  
Hmm... indeed. Looks like an oversight in original introduction of pcre2... Thanks for the
catch.

Interesting that it was compiling anyway somehow.

Anyway, I updated first patch to include `PCRE2_*` in `Makefile.am`
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-825106727

[sssd PR#5593][-Changes requested] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: -Changes requested

[sssd PR#5593][+Waiting for review] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Waiting for review

[sssd PR#5593][+Bugzilla] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Bugzilla

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
>  it looks like only `PCRE_LIBS` and `PCRE_CLAGS` are used in
> `Makefile.am` but the configure check defines only `PCRE2_LIBS` and `PCRE2_CFLAGS` if
> version 2 is found.
>  
Hmm... indeed. Looks like an oversight in original introduction of pcre2... Thanks for the
catch.

Interesting that it was compiling and linking anyway somehow.

Anyway, I updated first patch to include `PCRE2_*` in `Makefile.am`
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-825106727

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

sumit-bose commented:
"""
>  > it looks like only `PCRE_LIBS` and `PCRE_CLAGS` are used in
> `Makefile.am` but the configure check defines only `PCRE2_LIBS` and `PCRE2_CFLAGS` if
> version 2 is found.
>  
>  Hmm... indeed. Looks like an oversight in original introduction of pcre2... Thanks for
> the catch.
>  
>  Interesting that it was compiling and linking anyway somehow.
>  
>  Anyway, I updated first patch to include `PCRE2_*` in `Makefile.am`
>  
Hi,

thank you, this seem to work, now e.g. `-lpcre2-8` can be seen in the build logs. Since we
cannot use both libraries at once I wonder if it would be more clear to only use
`PCRE_LIBS` and `PCRE_CLAGS` and define them in the case libpcre2 is found as well?

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-825454223

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
>  Since we cannot use both libraries at once I wonder if it would be
> more clear to only use `PCRE_LIBS` and `PCRE_CLAGS` and define them in the case libpcre2
> is found as well?
>  
That definitely would be more clean.
But those flags are set by `PKG_CHECK_MODULES`. Do you propose to do something like
`PCRE_LIBS = PCRE2_LIBS` "manually" in this case?
And btw I don't know what is the purpose of:
https://github.com/SSSD/sssd/blob/f5bbff2502b69dc5f37ba049025683fbd67d7bb...
  -- is the same required for `PCRE2_LIBS`..?

But in general, I wouldn't bother much, because:
 - currently either `PCRE_*` or `PCRE2_*` has value, and empty vars in makefile
doesn't seem to hurt
 - we will remove support of PCRE-1 in next release entirely anyway
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-825507678

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[sumit-bose]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

sumit-bose commented:
"""
Hi,

I was thinking of

    -    [PCRE2],
    +    [PCRE],

bye,
Sumit
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-825518878

[sssd PR#5593][synchronized] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: synchronized

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[alexey-tikhonov]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

alexey-tikhonov commented:
"""
Ok, I should read a doc on `PKG_CHECK_MODULES`, of course. Thanks. Updated.
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-825574238

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[thalman]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

thalman commented:
"""
I tested the patch, looks good to me. Works as expected.
Thanks
ACK
"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-825735412

[sssd PR#5593][-Waiting for review] BUILD: prefer PCRE2 over PCRE

[thalman]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: -Waiting for review

[sssd PR#5593][+Accepted] BUILD: prefer PCRE2 over PCRE

[thalman]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Accepted

[sssd PR#5593][+Ready to push] BUILD: prefer PCRE2 over PCRE

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Ready to push

[sssd PR#5593][comment] BUILD: prefer PCRE2 over PCRE

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

pbrezina commented:
"""
Pushed PR: https://github.com/SSSD/sssd/pull/5593

* `master`
    * f2bcf74c43a682897e586eeb775c4bfedd95bafa - sssd.supp: suppress false positive
valgrind warning about 'pcre2_code' ptr
    * 0fbe5af1f1e0b82cd36a8178e58d79b7dc357ab6 - util/regexp: regular talloc d-tor
shouldn't fail
    * 9aa6fb34bea891e0385a9fc77f0181d01984c212 - tests/test_nested_groups: mem leak fixed
    * 31bcb6f032c326d62fb7ac5efcf2ff55c9acbe04 - tests/test_dp_opts: mem leak fixed
    * 519d943424ee744ecf7418df6bf6f0688a7d9099 - util/regexp: local functions shall be
static
    * cbfccb173d1cfa631350778abbee82bca1fbc296 - BUILD: prefer PCRE2 over PCRE

"""

See the full comment at https://github.com/SSSD/sssd/pull/5593#issuecomment-826676666

[sssd PR#5593][+Pushed] BUILD: prefer PCRE2 over PCRE

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: +Pushed

[sssd PR#5593][-Accepted] BUILD: prefer PCRE2 over PCRE

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: -Accepted

[sssd PR#5593][-Ready to push] BUILD: prefer PCRE2 over PCRE

[pbrezina]

  URL: https://github.com/SSSD/sssd/pull/5593
Title: #5593: BUILD: prefer PCRE2 over PCRE

Label: -Ready to push

[sssd PR#5593][closed] BUILD: prefer PCRE2 over PCRE

[pbrezina]

   URL: https://github.com/SSSD/sssd/pull/5593
Author: alexey-tikhonov
 Title: #5593: BUILD: prefer PCRE2 over PCRE
Action: closed

To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5593/head:pr5593
git checkout pr5593