jhthorsen / json-validator

:cop: Validate data against a JSON schema
https://metacpan.org/release/JSON-Validator
56 stars 57 forks source link

Installing before YAML::XS is available doesn't allow YAML::XS to be installed as a prerequisite. #259

Closed briandfoy closed 2 years ago

briandfoy commented 2 years ago

Steps to reproduce the behavior

Install JSON::Validator without a prior installation of YAML::XS.

Expected behavior

The code does what it says it does, but I'd like to use the appropriate version of YAML::XS even if it isn't installed yet. Makefile.PL, line 9:

my @PREREQ_YAML = eval 'use YAML::XS 0.67;1' ? ('YAML::XS' => '0.67') : ('YAML::PP' => '0.020');

I'd like some other way to choose what I want there. Perhaps something like JSON_VALIDATOR_YAML that lets me choose. I could set that in the process and get what I wanted despite what is currently installed.

Actual behavior

If YAML::XS 0.67 is not immediately available, then it will not be a prerequisite and not installed.

My particular problem shows up while building a fresh Docker container with Carton and a cpanfile, using a Pinto repository as the source. YAML::XS is in the mirror but YAML::PP is not. While building things, if YAML::XS is not installed already, JSON::Validator's Makefile.PL will choose YAML::PP. Since the cpanfile mechanism has no way to order installations (https://github.com/miyagawa/cpanfile/issues/42), it's not enough to specify JSON::XS in the cpanfile. Even listed, it might not be installed before JSON::Validator tries to do its work.

jhthorsen commented 2 years ago

Which version of YAML::XS do you want to use? The reason why 0.67 is required is because it has to support $YAML::XS::Boolean.

briandfoy commented 2 years ago

0.67 or any later version would be fine. It's not about the version. It's about installation sequencing.

jhthorsen commented 2 years ago

Do you want to prioritize YAML::XS instead of YAML::PP?

briandfoy commented 2 years ago

Yes, I want to choose to have YAML::XS as a dependency even if it is not yet installed.

jhthorsen commented 2 years ago

Thanks for clearing that up!

I'll take a PR for this, but I don't have time to change this myself I'm afraid.

jhthorsen commented 2 years ago

I've added a change where YAML::XS is preferred, but if someone needs to, they can depend on YAML::PP instead using JSON_VALIDATOR_PREFER_YAML_PP=1.