On Thu, Nov 18 2021, Hamza Mahfooz wrote:
> Since, git aspires to support many PCRE2 versions, it is only sensible to
> test changes to git against versions of PCRE2 that are deemed to be
> notable, to ensure those changes to git don't cause regressions when using
> the aforementioned PCRE2 versions. This is underscored by the fact that,
> commit ae39ba431a (grep/pcre2: fix an edge case concerning ascii patterns
> and UTF-8 data, 2021-10-15) was able to make it's way to master while
> causing an otherwise easy to catch regression when an older version of
> PCRE2 is used. So, to address this issue, add a job for PCRE2 to build all
> of the notable versions, compile all of them against git and only run the
> tests that can possibly be impacted by PCRE2.
>
> Suggested-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> Signed-off-by: Hamza Mahfooz <someguy@xxxxxxxxxxxxxxxxxxx>
> ---
> .github/workflows/main.yml | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> index 6ed6a9e807..ae96fc0251 100644
> --- a/.github/workflows/main.yml
> +++ b/.github/workflows/main.yml
> @@ -319,3 +319,29 @@ jobs:
> - uses: actions/checkout@v2
> - run: ci/install-dependencies.sh
> - run: ci/test-documentation.sh
> + pcre2:
> + needs: ci-config
> + if: needs.ci-config.outputs.enabled == 'yes'
> + strategy:
> + fail-fast: false
> + matrix:
> + jit: ['--enable-jit', '--disable-jit']
> + utf: ['--enable-utf', '--disable-utf']
> + version: [31, 34, 36, 39]
> + runs-on: ubuntu-latest
> + steps:
> + - uses: actions/checkout@v2
> + - uses: actions/checkout@v2
> + with:
> + repository: 'PhilipHazel/pcre2'
> + path: 'compat/pcre2-repo'
> + - run: ci/install-dependencies.sh
> + - run: cd compat/pcre2-repo
> + - run: git checkout pcre2-10.${{matrix.version}}
> + - run: ./autogen.sh
> + - run: ./configure ${{matrix.jit}} ${{matrix.utf}} --prefix="$PWD/inst"
Thanks a lot for following-up on this. Do you have a link to a sample
run of this to see how it looks?
I think that the --enable-utf here is a bug, my fault, I suggested using
that option.
But on closer inspection I should have said
--{enable,disable}-unicode. Eyeballing the configure.ac in pcre2.git now
and checking if/how it passes our tests I think it might be a noop
unless --enable-ebcdic is also in play, which we don't need to test.
Any reason for picking those specific versions? I think we do need to
test on older than 10.31 (released in early 2018).
On the other hand PCREv2 wasn't released at all until 2015, and got up
to 10.20 all in that one year, and I think git may have been one of the
first popular packages to use it.
I (optionally at first) us from PCRE v1 to v2, and IIRC something like
only 1-2 obscure packages in Debian depended on it at the time, with
hundreds depending on PCREv1.
We should also add a "latest" in there, and then just map that ourselves to:
git tag -l --sort=-version:refname | head -n 1
I.e. it's probably overkill to test the pcre2.git bleeding edge, and it
might produce undue noise, but testing the latest release in addition to
select older versions seems like a good thing to do.
> + - run: make
> + - run: sudo make install
I don't think "sudo" here is needed (and I'm surprised it works in CI,
presumably a noop or it's always root). Just a:
make install
Should do, it'll also take care of doing "make"
> + - run: cd ../..
> + - run: make USE_LIBPCRE=Y CFLAGS=-O3 LIBPCREDIR="$PWD/compat/pcre2-repo/inst"
I was going to say: "We should probably leave CFLAGS undefined and use
whatever the default is, or is there a good reason for -O3?".
But I see this is another thing copied from my throw-away one-liner in
[1], sorry. We can just skip that, I just happened to be testing with
CFLAGS=-O3 locally at the time & copied that into my E-Mail.
> + - run: cd t && make *{grep,log,pickaxe}*
1. https://lore.kernel.org/git/211117.867dd67spq.gmgdl@xxxxxxxxxxxxxxxxxxx/
