conda-forge / libsolv-feedstock

A conda-smithy repository for libsolv.
BSD 3-Clause "New" or "Revised" License
4 stars 17 forks source link

Add PCRE2 compatibility patch #60

Open jaimergp opened 1 year ago

jaimergp commented 1 year ago

Checklist

Adds patch from https://github.com/openSUSE/libsolv/pull/506.

This enables supports for properly merged glob strings in conda-libmamba-solver, as implemented in https://github.com/conda/conda/pull/11612

Please review the patch, this is a bit beyond my comfort zone, both CMake and C 😬

conda-forge-linter commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

jaimergp commented 1 year ago

@conda-forge-admin, please rerender

wolfv commented 1 year ago

nice!

There are two additional considerations -- we should figure out if we can have a pcre2-static on conda-forge (for micromamba) and also add a recipe to boa-forge :)

jaimergp commented 1 year ago

PCRE2 PR submitted here: https://github.com/conda-forge/pcre2-feedstock/pull/31

jezdez commented 1 year ago

For defaults the patch is in https://github.com/AnacondaRecipes/libsolv-feedstock/pull/2

jaimergp commented 1 year ago

@wolfv, PCRE2 10.40 is now available with a static lib too. Let me know if you need something else!

wolfv commented 1 year ago

nice!

jaimergp commented 1 year ago

I think we need a pcre2 migrator to push 10.37 to 10.40 if you want the static libs now.

jaimergp commented 1 year ago

Also note this warning:

The install/build script(s) for libsolv deleted the following files (from dependencies) from the prefix:
['lib/libbz2.a', 'lib/libz.a']
This will cause the post-link checks to mis-report. Please try not to delete and files (DSOs in particular) from the prefix
WARNING:conda_build.build:The install/build script(s) for libsolv deleted the following files (from dependencies) from the prefix:
['lib/libbz2.a', 'lib/libz.a']

So it looks like the build scripts do not want static libs at all, and hence we shouldn't depend on pcre2-static? Wouldn't the libsolv compilation create the static lib on its own from the shared libs anyway?

jaimergp commented 1 year ago

@wolfv ping :D

wolfv commented 1 year ago

I am about to release mamba / libmamba / micromamba 1.0 -- if it's OK let's wait after the release? I am also fixing some libarchive / curl issues in feedstocks specific to micromamba right now...

I really want to get this in, btw. sorry if it appears as if I was stalling this.

jaimergp commented 1 year ago

1.0 👀 That's exciting!!

No worries, I had forgotten myself 😬 I was just going through my GH notifications and found it again, fortunately. It's not super urgent, but it's blocking https://github.com/conda/conda/pull/11612

jaimergp commented 1 year ago

@wolfv gentle reminder :D

jaimergp commented 1 year ago

@conda-forge-admin, please rerender

jaimergp commented 10 months ago

@conda-forge-admin, please rerender

conda-forge-webservices[bot] commented 10 months ago

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

AntoinePrv commented 10 months ago

@jaimergp is this for libmamba?

jaimergp commented 10 months ago

This is required for compatibility with https://github.com/conda/conda/pull/11612, which requires advanced regexes internally. But if libmamba is going to be parsing things outside libsolv now maybe we don't need it anymore?

AntoinePrv commented 10 months ago

That would be our plan, but it is still unclear how we can plug a matchspec callback into libsolv. But isn't matching not trusted regex from repodata.json a DOS security risk?

jaimergp commented 10 months ago

We can still sanitize user input and avoid the DOS (which is already a problem today in current conda, given it allows pkg[build=/^some[-]regex$/] specs. Those advanced regexes are generated internally to accommodate the merge of several *build_string* globs à la *_gpu* or _*gpu_cuda11 (these two are compatible but conda would reject them now).

AntoinePrv commented 10 months ago

I was thinking of only allowing them in user input, not package input. The globs are different from the regex aren't they? They are already implemented libsolv AFAIK.

jaimergp commented 10 months ago

The globs are converted internally to regexes after they pass the merge step (which in current conda is only checking whether they are the same string or not). What the PR I linked above proposed is using lookahead regexes to handle the merge (instead of strict equals). The discussion there contains all the details you need!

AntoinePrv commented 10 months ago

On Mamba the globs are not converted to regex, and I think also not on libsolv.

jaimergp commented 10 months ago

With conda-libmamba-solver, we take the MatchSpec objects as generated by conda. If that PR was to be merged, then we could receive MatchSpec objects with a build string that has a lookahead regex, and then libsolv would crash because that kind of regex is not supported by the current regex engine. It does accept simpler regexes, just not lookaheads and similar.