Problem/Motivation

Some PHP 8.1 test failures are coming from the QueryPath library (querypath/querypath). However, this library seems to be unmaintained for the past 5 years.

Proposed resolution

There looks to be a replacement for the library that supports PHP 8.1: arthurkushman/query-path.
Let's find out if libraries can be safely switched and fixes all PHP 8.1 issues related to it.

Remaining tasks

  • Switch library in composer.json
  • See if tests pass.
  • Manually test Ludwig integration
  • Commit?

User interface changes

None.

API changes

A library switch could introduce changes for custom parsers extending the QueryPath parsers.

Data model changes

None.

Issue fork feeds_ex-3271774

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

MegaChriz created an issue. See original summary.

megachriz’s picture

megachriz
Primary language Dutch
commented
Issue summary: View changes

Changed my mind. I'm trying "arthurkushman/query-path" first. "satisfactory-clips-archive/querypath" requires at least PHP 8.1 and we still want to support PHP 7.3 and above.

megachriz’s picture

megachriz
Primary language Dutch
commented
Issue summary: View changes

If switching libraries can be done safely, we'll have to verify that the Ludwig module integration still works too.

MegaChriz opened merge request !19

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Active » Needs review
devad’s picture

devad commented

Regarding the Ludwig integration:

.module file changes

Your .module file change is ok.

ludwig.json file changes

With all new library versions and latest Ludwig improvements the new ludwig.json file should look like this one:

{
    "require": {
        "mtdowling/jmespath.php": {
            "version": "2.6.1",
            "url": "https://github.com/jmespath/jmespath.php/archive/2.6.1.zip"
        },
        "softcreatr/jsonpath": {
            "version": "0.7.5",
            "url": "https://github.com/SoftCreatR/JSONPath/archive/0.7.5.zip"
        },
        "arthurkushman/query-path": {
            "version": "3.1.3",
            "url": "https://github.com/arthurkushman/querypath/archive/3.1.3.zip"
        }
    }
}

Merging is not in my skills yet... so @MegaChriz, if you can merge this new ludwig.json file in - that would be great.

megachriz’s picture

megachriz
Primary language Dutch
commented

As of now "arthurkushman/query-path" is not compatible with PHP 8.1, but "satisfactory-clips-archive/querypath" requires PHP 8.1. I've opened a pull request to get "arthurkushman/query-path" compatible with PHP 8.1.
https://github.com/arthurkushman/querypath/pull/9

In case that pull request does not get accepted (no changes to "arthurkushman/query-path" have been made in the past 3.5 years), I could create a wrapper composer package to require one or the other library depending on the PHP version. Instructions for that I found here: https://stackoverflow.com/questions/51333587/php-composer-require-depend...

@devad
Is it possible within Ludwig to download one or the other library depending on the PHP version being used? "arthurkushman/query-path" and "satisfactory-clips-archive/querypath" both use the same namespace, so they would likely conflict if they are downloaded both.

megachriz’s picture

megachriz
Primary language Dutch
commented

Is it possible within Ludwig to download one or the other library depending on the PHP version being used?

@devad
I see that there is a possibility to use a different library depending on the Drupal version:
https://www.drupal.org/docs/contributed-modules/ludwig/ludwig-integratio...
Maybe that is an acceptable workaround. Only people on PHP 8.1 and Drupal 9 would then get errors when they use the QueryPath library (assuming that Feeds Extensible Parsers would work with "satisfactory-clips-archive/querypath"), but people on Drupal 10 won't. I assume that the userbase for people using Ludwig + Drupal 9 + PHP 8.1 + QueryPath is very small. I think that I never got a question about the QueryPath parser so far while I got a lot about the XML and JSON parsers. The last real question about it seems to be from 6 years ago: #2631354: QueryPath XML parser proper format?

devad’s picture

devad commented

I

see that there is a possibility to use a different library depending on the Drupal version:

Yes, there is a possibility to use a different library depending on the Drupal version. As you said... this will solve the most use cases but not all.

The php version resolving does not exist inside Ludwig yet.

megachriz’s picture

megachriz
Primary language Dutch
commented

Thanks for your reply :). I see that the pull request has been accepted! So hopefully the library split is not necessary.

  • MegaChriz committed b58716d on 8.x-1.x 
    Issue #3271774 by MegaChriz, devad: Replaced QueryPath library with "...
megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Needs review » Fixed

I manually tested the Ludwig integration (just to be sure) and I found out a piece of code in the QueryPath parser was no longer needed. From the library there was no longer a need to manually load /src/qp.php from it (also because it no longer existed) so I removed that part from the code.

Merged!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.