icecave/parity dependency concern

[wimleers]

In the master branch, #518 added a dependency on icecave/parity. That in turn has a dependency on icecave/repr. Per icecave/parity#15 (comment), Drupal core maintainers do not want to add icecave/* dependencies to Drupal core.

(The 5.2.x releases don't contain the icecave/* dependencies, but if Drupal needs to update to a release based that contains #518, it'd become a problem at that point.)

Would you be willing to use an alternative approach that does not depend on that library?

P.S.: this is related to #569, and thanks for your prompt response there!

[shmax]

@wimleers

Just so I understand, which part of his response was it that raised red flags? He said that most of his packages don't see active development, but he also seems open to fixes, so before I go off and find a replacement it might save me some time if I know exactly what it is I should be looking for...

[shmax]

To put it another way, if you're concerned about maintenance on icecave/parity, wouldn't it make more sense to look for a replacement if/when it actually becomes an issue?

[xjm]

Thanks @shmax for the prompt response!

For us it is about predictability, future compatibility, and risk mitigation. Let's say the next major version of json-schema is released, and then six months later Icecave EOLs their package as they've indicated is going to happen at some point. Then the choices are:

• Proceeding with an unsupported package that won't receive security coverage.
• Forking the package.
• A BC break.
• A new major version with whatever replacement.

All of the above are variously disruptive or risky for a package with a need for scheduled releases and LTS support (like ours).

[erayd]

@xjm That makes sense. Noting the way we use parity, the dependency could likely be removed.

[shmax]

Well, sure, we could probably factor it out or replace it, but if I spent all my time trying to second guess which projects are going to EOL in six months and preemptively replacing them I wouldn't get much done (this project isn't exactly a hotbed of activity, either). We use this package at one place in the code, and it hasn't been an issue since I put it in. So barring some unforeseen future complication, if they EOL it won't make any difference to us. And if there is some complication, then we can fix it then. We have 56 open issues now--many of which document real bugs--so it seems a little bizarre to me to focus on this as some kind of blocker. Hell, we could just revert the commit and go back to having 57 open issues instead of 56. Would that improve the situation? shrug

[DannyvdSluijs]

@wimleers in an attempt to cleanup this repo we are trying to filter the issues and see which ones might be closed. Is it safe to assume this is an issue which time has caught up on? The icecave/parity [link] repo seems to be inactive but the code is there and over time it even has grown to a version 3 whereas this repo still is set on version 1. I see your concerns and I think they are valid up to a point, but functionally there is currently no impact on the library.
So my remaining question is can this issue be closed? Feel free to close it yourself with some comments if helpful.

[alcalyn]

Hi, we had to use version 5.3 instead of 6 of this package because version 6 introduces icecave/parity =1.0.0, which requires icecave/repr ~1, which breaks on newer versions of php because of this syntax:

https://github.com/icecave/repr/blob/1.0.1/src/Generator.php#L229

(fixed in 2.0.1)

The commit of the tag v6 has been done 6 years ago so I understand about using version 1 at this time, but the tag v6 has been released 3 weeks ago (which was surprising to me).

[DannyvdSluijs]

Hi, we had to use version 5.3 instead of 6 of this package because version 6 introduces icecave/parity =1.0.0, which requires icecave/repr ~1, which breaks on newer versions of php because of this syntax:

https://github.com/icecave/repr/blob/1.0.1/src/Generator.php#L229

(fixed in 2.0.1)

The commit of the tag v6 has been done 6 years ago so I understand about using version 1 at this time, but the tag v6 has been released 3 weeks ago (which was surprising to me).

Hi @alcalyn

thanks for you comment. If you think this is something needed fixing you could best open a new issue. It would be a great timesaver if you can provide more details on how to reproduce.

Mainly the PHP version is important. Version 6.0 still supports PHP 5.3 (which is probably why it was pinned in such a way). Work is being done to get version 6.1 done with the main feature to support PHP 7.2 and up and fix any issues with PHP 8.0 and greater.

[shirshir]

Unfortunately I was also unable to upgrade to v6 because of icecave/parity:1.0.0. This dependency triggered a couple of vulnerability warnings for the twig/twig:v1.15 sub dependency:

CVE-2015-7809
CVE-2022-39261
CVE-2019-9942

[DannyvdSluijs]

@shirshir the same comment applies for you as well, if you believe this needs to be fixed please open an issue with some details, about the PHP version being used and the exact error messages you encounter.

You're pointing to the composer.lock of icecave/parity which isn't used in the context of your application. The composer.lock is used in the context of just that library. That is how composer works. It will only install dependencies from composer.json when used as a dependency.

A quick check shows the following (on icecave/parity:1.0.0)

/tmp/parity on #1.0.0: composer why twig/twig --locked --tree
twig/twig v1.15.0 Twig, the flexible, fast, and secure template language for PHP
└──sami/sami v1.1 (requires twig/twig 1.*)
   └──icecave/archer 1.1.1 (requires sami/sami ~1.1.0)
      └──icecave/parity 1.0.0 (requires (for development) icecave/archer ~1)

Where you can see the path to twig/twig is in require-dev which also isn't used when you install it as a dependecy.

I'm not saying there might not be an issue but more details are required as I strongly believe you've found a red herring.

❗ On a side note in the master branch (which we are updating to drop PHP < 7.2 and include support for >= PHP 8.0) the icecave/parity has been updated to ^3.0

[shirshir]

@DannyvdSluijs Thanks for the reply. Indeed, the code is not used. It's just that my application is scanned using a vulnerability scanner, and that trips over the lock file from icecave/parity:1.0.0. So I can't install v6.

❗ On a side note in the master branch (which we are updating to drop PHP < 7.2 and include support for >= PHP 8.0) the icecave/parity has been updated to ^3.0

That is good news, thanks!