pkg-config: do not ever successfully detect Strawberry Perl. #9384
Conversation
eli-schwartz
force-pushed
the
strawberry-perl-has-a-broken-pkg-config
branch
2 times, most recently
from
6489bbc
to
4b9fa3a
Compare
Codecov Report
@@ Coverage Diff @@
## master #9384 +/- ##
==========================================
+ Coverage 66.53% 67.28% +0.75%
==========================================
Files 394 396 +2
Lines 85336 85410 +74
Branches 17457 17481 +24
==========================================
+ Hits 56776 57470 +694
+ Misses 23877 23250 -627
- Partials 4683 4690 +7
Continue to review full report at Codecov.
|
…string There are a bunch of cases in a single function where we would want to log the detected path of pkg-config. Formatting this is awkward. Define it once, then use f-strings everywhere. :D
eli-schwartz
force-pushed
the
strawberry-perl-has-a-broken-pkg-config
branch
from
4b9fa3a
to
ac83835
Compare
|
Checking for "Strawberry/" in the path doesn't strike me as particularly reliable - yes, that's the default, but e.g. I installed it under c:/perl/version.of.perl/ because I need to switch between multiple versions. A bit more complicated approach would be to check if (dirname pkg-config.bat)/../../DISTRIBUTIONS.txt starts with "Strawberry Perl". But maybe that's too complicated... |
mesonbuild/dependencies/pkgconfig.py
Outdated
| @@ -423,18 +423,20 @@ def check_pkgconfig(self, pkgbin: ExternalProgram) -> T.Optional[str]: | |||
| if not pkgbin.found(): | |||
| mlog.log(f'Did not find pkg-config by name {pkgbin.name!r}') | |||
| return None | |||
| command_as_string = ' '.join(pkgbin.get_command()) | |||
| if r'Strawberry\perl\bin' in pkgbin.get_path(): | |||
There was a problem hiding this comment.
I'm slightly worried that this check is too strict. Like get_path() could return POSIX path, or it could be in another subdir. I would rather go with 'strawberry' in pkgbin.get_path().lower(). But in any case, yes we should black list that crap, and probably do that for any binary such as gcc.exe, etc. I just now realise it's the cause of msvc env not being activated by default on GitHub Windows CI, meson find strawberry gcc...
There was a problem hiding this comment.
Strawberry GCC!!!
Oh dear, the rabbit hole just goes deeper.
Should we encode this test in mesonbuild.programs.ExternalProgram then?
There was a problem hiding this comment.
if seems_to_be_strawberry and self.name != 'perl':
return [None]dies
Because we do of course want to make sure that actual perl.exe is detected, people might want to actually use that. As a script interpreter at the very least.
|
FWIW I compiled a bunch of things successfully with strawberry perl's gcc. And used several binaries from it. Mostly because it made the CI process faster. So refusing anything from strawberry seems like a good of overkill?
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
I think that's the most reliable way indeed.
I agree, let's blacklist only pkg-config for now because that one cannot be useful in any possible way. |
|
oh wait, there is a better way: |
This is broken and terrible and thus completely unusable. Don't torture
users by finding pkg-config on Windows, thus permitting the pkg-config
lookup of several dependencies that do not actually work -- which then
fails at build time.
This also breaks CI for the wrapdb, because Strawberry Perl is provided
as part of the base image for the OS (yes, even though it is terribly
broken!!!) and anything that depends on e.g. zlib will "find" zlib
because of this broken disaster, even though it should use the wrapdb
subproject of zlib.
It is assumed no one actually wants to mix Strawberry Perl and meson. In
fact, some projects, such as gst-build, already unconditionally error
out if Strawberry Perl is detected in PATH:
error('You have Strawberry Perl in PATH which is known to cause build issues with gst-build. Please remove it from PATH or uninstall it.')
Other projects (postgresql) actually do want to build perl extensions,
and link to the perl dlls, but absolutely under no circumstances ever
want to use its pkg-config implementation. ;)
Let's solve this problem by just considering this to not be a valid
pkg-config, let the user find another or not have one at all.
This change "solves"
StrawberryPerl/Perl-Dist-Strawberry#11
eli-schwartz
force-pushed
the
strawberry-perl-has-a-broken-pkg-config
branch
from
ac83835
to
88d9646
Compare
|
LGTM, thanks! |
|
Should the test for "help message indicates strawberry perl" should come after the check that "pkg-config runs"? Perhaps the only difference is that if pkg-config is can't run, then testing for strawberry perl will mean that it is run twice. |
This is needed because Meson skips the Pure Perl `pkg-config.bat` provided by Strawberry Perl. Connects with: - <mesonbuild/meson#9384> - <PerlAlien/Alien-Build#227>
Strawberry Perl places a `pkg-config.bat` into PATH that is written in Perl and is not intended to be used by third parties as a MinGW distribution. This wouldn't matter, except that Strawberry Perl is also included in Github CI images out of the box, in `PATH`, and it breaks everyone's CI jobs. This is already done by Meson and CMake: mesonbuild/meson#9384 https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9375 Fixes rust-lang#164
Strawberry Perl places a `pkg-config.bat` into PATH that is written in Perl and is not intended to be used by third parties as a MinGW distribution. This wouldn't matter, except that Strawberry Perl is also included in Github CI images out of the box, in `PATH`, and it breaks everyone's CI jobs. This is already done by Meson and CMake: mesonbuild/meson#9384 https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9375 Fixes rust-lang#164
Strawberry Perl places a `pkg-config.bat` into PATH that is written in Perl and is not intended to be used by third parties as a MinGW distribution. This wouldn't matter, except that Strawberry Perl is also included in Github CI images out of the box, in `PATH`, and it breaks everyone's CI jobs. This is already done by Meson and CMake: mesonbuild/meson#9384 https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9375 Fixes rust-lang#164
Strawberry Perl places a `pkg-config.bat` into PATH that is written in Perl and is not intended to be used by third parties as a MinGW distribution. This wouldn't matter, except that Strawberry Perl is also included in Github CI images out of the box, in `PATH`, and it breaks everyone's CI jobs. This is already done by Meson and CMake: mesonbuild/meson#9384 https://gitlab.kitware.com/cmake/cmake/-/merge_requests/9375 Fixes rust-lang#164
This is broken and terrible and thus completely unusable. Don't torture users by finding pkg-config on Windows, thus permitting the pkg-config lookup of several dependencies that do not actually work -- which then fails at build time.
This also breaks CI for the wrapdb, because Strawberry Perl is provided as part of the base image for the OS (yes, even though it is terribly broken!!!) and anything that depends on e.g. zlib will "find" zlib because of this broken disaster, even though it should use the wrapdb subproject of zlib.
It is assumed no one actually wants to mix Strawberry Perl and meson. In fact, some projects, such as gst-build, already unconditionally error out if Strawberry Perl is detected in PATH:
Other projects (postgresql) actually do want to build perl extensions, and link to the perl dlls, but absolutely under no circumstances ever want to use its pkg-config implementation. ;)
Let's solve this problem by just considering this to not be a valid pkg-config, let the user find another or not have one at all.
This change "solves" StrawberryPerl/Perl-Dist-Strawberry#11