mjordan / islandora_bagger

Tool for generating Bags for Islandora 8 content.
MIT License
4 stars 12 forks source link

PHP version locked at 7.2 #78

Open alxp opened 2 years ago

alxp commented 2 years ago

I tried to run this on my desktop Mac which is running PHP 8 via Homebrew, and composer refused to install due to the hard-coded PHP version requirement in composer.json:

    "require": {
        "php": "^7.2",
        "ext-ctype": "*",
        "ext-iconv": "*",
        "guzzlehttp/guzzle": "~6.0",

Going to try to see if anything breaks if I remove this requirement.

alxp commented 2 years ago

The PHP 8 compatibility code sniffer plugin found no errors, and I was able to run the create_bag command with no issues on PHP 8.1.

e] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env ENV] [--no-debug] [--] <command>

iMac:bin aoneill$ ./console app:islandora_bagger:create_bag --node 96 --settings /Users/aoneill/dev/islandora_bagger/sample_config.yml  

 [OK] Bag created for http://localhost:8000/node/96 at /tmp/96.zip                                                      

iMac:bin aoneill$ composer sniffer:php8^C
iMac:bin aoneill$ cd ..
iMac:islandora_bagger aoneill$ composer sniffer:php8
> phpcs -p ./src --standard=vendor/phpcompatibility/php-compatibility/PHPCompatibility --runtime-set testVersion 8.0
...................... 22 / 22 (100%)

Time: 297ms; Memory: 10MB

iMac:islandora_bagger aoneill$ 
iMac:islandora_bagger aoneill$ 

@mjordan I can make a PR for this but it would be more straightforward to wait for #25 to be merged since there are composer.lock changes.

mjordan commented 2 years ago

Thanks very much. WRT #25, I can generate bags from the command line no problem but not from within the Drupal interface. No info is being logged anywhere that I can find. So I'm still trying to figure out what's going on there.

mjordan commented 2 years ago

Ok, I've merged #25 into main. I had to fix some merge conflicts. I still can't generate Bags from the UI but since you can I'm OK with merging now so we can move on.

If we bump "php": "^7.2" to 8.1, will that make 8.1 a new requirement to run the CLI?

alxp commented 2 years ago

You can specify ranges of required / compatible versions, so if it is something like

php:^7.2 || >= 8

Then it will still be compatible with 7.x.

mjordan commented 2 years ago

That would be my preference.

whikloj commented 2 years ago

The current composer.lock specifies a league/csv version of 9.8.0 which requires php: ^7.4 || ^8.0. So either you'll need to either:

  1. regenerate the composer.lock with PHP 7.2
  2. lock league/csv to version 9.6.2
  3. don't commit your composer.lock to the repository
  4. Let PHP 7.2 die and I can submit a PR with PHP 7.3 as the base.