Make a change that tolerates php 7

[DonRichards]

make starter_dev fails to build completely. This allows composer to determine a solution that fits the current setup (allows for php 7)

To test

make clean
make starter_dev

This should trigger the following error.

Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires maennchen/zipstream-php == 2.4.0.0 -> satisfiable by maennchen/zipstream-php[v2.4.0].
    - maennchen/zipstream-php v2.4.0 requires php ^8.0 -> your php version (7.4.33) does not satisfy that requirement.
  Problem 2
    - Root composer.json requires symfony/string == 6.2.2.0 -> satisfiable by symfony/string[v6.2.2].
    - symfony/string v6.2.2 requires php >=8.1 -> your php version (7.4.33) does not satisfy that requirement.
  Problem 3
    - maennchen/zipstream-php v2.4.0 requires php ^8.0 -> your php version (7.4.33) does not satisfy that requirement.
    - drupal/search_api_solr 4.2.10 requires maennchen/zipstream-php ^2.2.1 -> satisfiable by maennchen/zipstream-php[v2.4.0].
    - Root composer.json requires drupal/search_api_solr == 4.2.10.0 -> satisfiable by drupal/search_api_solr[4.2.10].

Then switch to this PR and run the above steps and it should completely.

[DonRichards]

@rosiel Have you experienced this error?

[adam-vessey]

Not sure that this makes sense as-is, especially without specific testing of it. Like, if we want to ensure 7.4 compatibility of some sense, we should probably have it in the CI harness, or risk it being broken again and again.

That said, thinking we might be better suited by addressing compatibility with PHP versions somewhere... 8.1 is recommended, but 7.4 works with this slightly different instructions, and dealing with the slightly different instructions, which should effectively just be: Avoid trying to use the composer.lock that binds to 8.1 when running 7.4 (or really, any different minor version). Instead, run a

composer upgrade

which should hopefully build out a compatible list of packages for the given environment from the composer.json, assuming one can be built (and attempting, was successful on a PHP 7.4 envs just now (and via https://github.com/Islandora-Devops/islandora-starter-site/actions/runs/4068675242/jobs/7007509235#step:6:276)).

Could possibly make our CI try both, with both versions? Like: To try a composer install, but if it fails, to try a composer upgrade, only failing the run should both fail? Something like the workflow change in https://github.com/Islandora-Devops/islandora-starter-site/compare/fix/74-compat #54

... probably similarly have to such a fallback implemented wherever things are used? Might go so far as to suggest against the use of composer create-project and/or providing it the --no-install flag for those instances where a composer upgrade has to be run instead?

... or, could get into how other projects might support things across PHP version boundaries? Polyfills and the like?

---

Late addition, but in terms of isle-dc stuff, might be able to roll something like this: https://github.com/Islandora-Devops/isle-dc/compare/fix/starter-install-fallback-to-upgrade Islandora-Devops/isle-dc#325

... completely untested, and my Makefile-fu isn't terribly strong, but, that kind of thing (could probably break out to a separate script file, if necessary? Multi-line things in Makefiles seems to get weird).

[DonRichards]

@adam-vessey composer upgrade makes me nervous. It could quickly introduce all kinds of irregularities since it could be fetching untested versions of the dependencies. Besides, the build process needs to be seamless for isle-dc. Otherwise, we would be adding unnecessary complexity to what is supposed to be a platform used for deployment as well as learning. The point of publishing the lock file in any repo is that at this exact version configuration, we know that our software will build correctly.

You should commit the composer. lock file to your project repo so that all people working on the project are locked to the same versions of dependencies - getcomposer.org

Suggesting people run a stack with dependency versions that are potentially untested is going to be problematic at best as those dependencies are constantly being updated.

Although people could upgrade to PHP 8, the fact remains that Islandora's primary build method (isle-dc) doesn't support that yet and we should take this into heavy consideration.

I think the title of this PR is probably off-topic for the scope of this. As it sits right now, if the user tries to run make starter_dev it fails. The failure is from one of these dependencies relied on another dependency that relied on php8 as a minimal version. To open those dependencies up enough to allow composer to figure out what dependency versions play nice together I set search_api_solr to any version and the 2 dependencies that were causing the issue to a specific version only because in this configuration it allows composer to complete the install.

[adam-vessey]

@DonRichards : This being more of a "template" project, those recommendations for including the composer.lock I understand as being for something of a more "concrete" project, with a controlled environment regarding the PHP version and whatever extensions available and so on are less relevant... like, the issue here is that composer was run with some version of PHP 8.1, so it allowed things to "float" out of compatibility with PHP 7.4. This would be less of an issue if the starter site itself had a Dockerfile to have it build out a "Drupal"/"Islandora"/whatever image, which could then be used wherever, but would serve to bind the expected version of runtimes.

By changing the version of PHP, you're already making compromises with respect to the integrity of the composer.lock, yeah? As in, instead of touching the composer.json at all, to fix this, you could just run a composer upgrade in a PHP 7.4 env, and it should build out the compatible list of packages, instead of starting to "lock" things in the composer.json, such that what are supposed to be semantic version specs are no longer quite semantic?

... the alternative, if we want to support multiple versions of PHP, is to essentially roll separate branches (or forks?) for each version? Again: Might bear investigation into how other projects handle this issue.

Something of an aside, but: not sure the assertion that isle-dc is the "primary build method" is really valid? Maybe what some have bought into, but yeah... still thinking that ddev or lando configs included with the starter site might be the way to go, but the complexity of the stack of having crayfish and fedora as primary requirements complicates things... ddev at least supports docker compose stuff, so could probably go grab buildkit images, or something of the like.

[adam-vessey]

@DonRichards : Another note: I'm not suggesting doing the composer upgrade first, but only in the event the the composer install has already been shown to fail.

Hypothetically, given what composer specs are possible, things could eventually get such that a composer.lock from PHP 7.4 might not be resolvable on PHP 8.1, as packages can specify both lower and upper bounds of compatibility. What would you suggest should such a situation arise?

[DonRichards]

@adam-vessey Sorry about the comment about isle-dc being the "primary". I was trying to be playfully biased on the topic, but I'm really bad at jokes, and my humor is pretty dry at times.

There could be a compromise within the composer file. I just tested a slightly different modification that still works with isle that doesn't lock the dependencies to a specific version. I'll push that change to this.

The simplest way I know of to regenerate the lock file without downloading or upgrading anything is to run composer update --lock instead of relying on upgrade.

[adam-vessey]

@DonRichards : composer update and composer upgrade are two aliases of the same command (in Composer 2.5.1, anyway).

--lock isn't applicable to this situation, as we do want to change the actual versions of things that are locked, while the --lock flag just (as per composer upgrade --lock):

Overwrites the lock file hash to suppress warning about the lock file being out of date without updating package versions. Package metadata like mirrors and URLs are updated if they changed.

Presently, the diff in the lock file isn't terribly complex... and presumably it would be tested after such a change, anyway? If really desired, could pass --with stuff to artificially keep some things back, like your present changes to the composer.json here would do:

      --with=WITH                                      Temporary version constraint to add, e.g. foo/bar:1.0.0 or foo/bar=1.0.0 (multiple values allowed)

... but this is essentially just to keep the diff of the composer.lock file cleaner?... of stuff that we probably want to update, anyway, such as Drupal stuff going from 9.5.2 to 9.5.3? Shrug?

Still doesn't address the situation of composer.lock offering no real mechanism for supporting more than one version of PHP... could possibly get into things such as: Telling composer to require compatibility with whatever particular version?: https://getcomposer.org/doc/06-config.md#platform ? Unsure if it's really worthwhile pursuing.

[DonRichards]

@adam-vessey Isn't the point of specifying the range in the composer.json file?

"require": {
        "php": "^7.4 || ^8",

[DonRichards]

In isle it's simple enough to rebuild the composer.lock file if it's needed. Suppose the playbook uses this, it essentially needs to work without modification. In that case, we could have someone pull this update down onto a system that is running php8 and push the lock file to this PR, and I could add a PR to isle-dc to regenerate the lock file before running the composer install command. If that makes sense.

[adam-vessey]

@adam-vessey Isn't the point of specifying the range in the composer.json file?

"require": {
        "php": "^7.4 || ^8",

It's indicating what's required/sufficient for installing the project, yes; however, the composer.lock binds to the particular environment in which the composer upgrade is run as a result of being run directly or via a composer require (or whatever other command that might do one under the hood). This disjunction in the composer.json doesn't cause composer to build out a singular dependency graph/lock file satisfying all possible that is compatible with all the versions. All it's saying is that "this project should be installable if you've got a version of PHP less than 8 and greater than 7.4 or greater-than-or-equal-to 8 and less than 9. It doesn't resolve for all environments, but just the one environment on which it is running.

To able to build out for all versions, the lock file would need to potentially account essentially all version boundaries for all packages, as any package can include PHP constraints (as happened here). In my mind, the "safety" of using composer upgrade(/update/whatever) gets down to trusting those making releases to appropriately follow semantic versioning, as it's ultimately within the semantic versioning scheme in our specifications in our composer.json that indicate what is required to run things. The composer.lock just indicates a set of packages that satisfied the specs of the composer.json at one point in time, in one particular environment. In another environment with a different version of PHP or different PHP extensions available and so on, the composer.lock should not necessarily be expected to be compatible.

[rosiel]

This looks like some trickery leftover from fixing the dependency knot? It probably should go back to ^4.2.

[rosiel]

I'm fine moving this back down to be PHP 7 compatible until ISLE is on PHP 8.

I did a bad. This was a breaking change and should have been managed through Semantic Versioning. We don't yet use it between ISLE/Playbook and the starter site. We don't even cut versions of the Starter Site. We don't do that because versioning seems to imply that you can do in-place upgrades (you can't).

But we kind of need to version:

• for when the PHP version is implicated in the composer files (especially the lock file) like in this case
• for when the site config refers to environment things that need to be available (e.g. if we add or remove a matomo-like service)

So I'm proposing:

• I start tagging Starter Site commits with v1.x.x
• I put a big flag on the README that version numbers are for environment compatibility only, does not imply you will be upgrading live starter site
• We change Playbook (starter) and ISLE (make starter) to use the latest 1.x tag
• We keep Playbook (starter_dev) and ISLE (make starter_dev) to use the latest commit on the main branch.
• When PHP 8 is ready on ISLE, we can recompile the lock file, cut a 2.0.0 tag, and change ISLE and the Playbook to use the latest 2.x tag. (we could do this earlier, or start two branches, but i'm not sure about the wisdom of maintaining or seeming to maintain old branches)
• We merge this PR (with my suggestion above re. the search_api_solr version)

[DonRichards]

@rosiel I can give that a try.

[DonRichards]

OK, ready to test.

[DonRichards]

Should I adjust the ci test?

[DonRichards]

I went ahead and switched it.

[adam-vessey]

Feel like I should point out (again) that Composer has the platform/php config, that would still allow the project to be used with PHP 8, so long as all the packages supporting whatever indicated version would still work in the latest... so you could add:

diff --git a/composer.json b/composer.json
index 37dbac2..65411bd 100644
--- a/composer.json
+++ b/composer.json
@@ -70,7 +70,10 @@
             "drupal/core-composer-scaffold": true,
             "dealerdirect/phpcodesniffer-composer-installer": true
         },
-        "sort-packages": true
+        "sort-packages": true,
+        "platform": {
+            "php": "7.4"
+        }
     },
     "extra": {
         "drupal-scaffold": {

And this would tell Composer to always evaluate as though running with PHP 7.4, independent of the PHP runtime being used, but still allow installation on PHP 8 (assuming there's no conflicts preventing it)... would still be able to have the multiple versions under test, methinks?

[adam-vessey]

Yeah, just gave it a spin: #53

... something of an alternative, that tries to maintain compatibility?

[rosiel]

would this be a useful step for people running PHP 8 to be able to contribute to the starter site? I like that.
I'd want to leave a note in the README (since there's no commenting in .json) that this constraint should either:

• be removed, or
• get updated regularly to the lowest-common-denominator PHP version between ISLE and Playbook.

[rosiel]

Thanks for having this discussion! Closing, as it's solved with #53 .