Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PHP Deprecated: Calling get_class() without arguments #135

Merged
merged 2 commits into from
Mar 9, 2024

Conversation

alecpl
Copy link
Contributor

@alecpl alecpl commented Feb 11, 2024

@alecpl
Copy link
Contributor Author

alecpl commented Feb 11, 2024

Test failures unrelated. Tests on PHP 5.4 and 5.5 also fail in master.

@mpdude
Copy link

mpdude commented Feb 23, 2024

@ashnazg Can you advise what needs to be done about the failing tests?

@ashnazg
Copy link
Member

ashnazg commented Feb 23, 2024

@mpdude ,

The test failures are part of me trying to figure out why Github's Ubuntu images for older PHP5 seem to behave differently than the Travis-CI images do, particularly around XML stuff. I've blown two weekends fighting these test failures only to discover in each case (so far) that they boil down to different error message wordings from XML activity.

I've been hesitant to merge/release anything until I know these builds are good, since I cannot rely on Travis builds any longer since they dropped free builds for open source projects... $69/month out of my own pocket back in December did not prove to be worth it to me, since it only covered one build instance, and every code push ran 9 builds, each of which usually took 3min or more... thus one commit always took over 30min to fully build.

You're welcome to have a look at the failing tests on the 5.4/5.5 builds to see if you can make any headway with them. Since I don't have an available local env to step-debug with, it's been a very slow slog leveraging the Github builds and printed outputs.

@mpdude
Copy link

mpdude commented Feb 23, 2024

Ok, so it’s not flaky tests. I’ll see if I can fork the repo, enable actions and investigate.

@mpdude
Copy link

mpdude commented Feb 23, 2024

XML error messages probably come directly from libxml. I don’t know if that’s linked statically (baked into PHP) or dynamically linked. In the latter case, the version of libxml installed on the Linux system might be relevant.

@ashnazg
Copy link
Member

ashnazg commented Feb 23, 2024

@mpdude right, my hunch is that one of them (either Travis or Github) uses a base Ubuntu, and the second might use an Ubuntu with backported patches... and thus the same OS library giving different behaviors.

My "remedy" for this discrepancy on the other tests I've fixed so far has been to tweak the logic in those PHPT tests to do version checks and adjust expectations accordingly... but I haven't done this with a given test until I knew for sure that the differing library behavior was indeed the culprit.

@mpdude
Copy link

mpdude commented Feb 23, 2024

See https://github.com/mpdude/pear-core/actions/runs/8024927894/job/21924484015?pr=1

PHP 5.4 and 5.5 are compiled with or against libxml 2.9.1, PHP 5.6 and above against libxml 2.9.14.

@mpdude
Copy link

mpdude commented Feb 23, 2024

I am not 💯 sure about this, but when I look at the

libxml
libXML support => active
libXML Compiled Version => 2.9.1
libXML Loaded Version => 20913
libXML streams => enabled

lines I'd say all tests run on the same OS with libxml 2.9.13 being installed as a dynamic library; and the different PHP versions have been compiled against different versions (2.9.1 and 2.9.14) but use 2.9.13 at runtime.

Do the different error messages really come from libxml or some other library? Do you happen to have them at hand?

@ashnazg
Copy link
Member

ashnazg commented Feb 23, 2024

Once again, this has only been a hunch of mine, mostly based on the other tests I've had to tweak since migrating from Travis-CI to Github Action builds. You can see where I left off on the current failures in my fork PR (ashnazg#2), but the last thing I remember is that what I saw last made me more confident that the different behavior was something to do with XML behavior in 5.4 & 5.5 that differed from 5.6 and up.

@ashnazg
Copy link
Member

ashnazg commented Feb 23, 2024

Actually, I think in my last session, I had reached a point where I thought the multibyte character in Stig's name in the Archive_Tar tarball that's included for the testcases was the point of contention, in that the XML lib reading that was where the behavior acted differently... and that my next plan of attack was to remove that special character, recreate the tarball, and see how to tweak the tests to use that modified tarball. I might be able to resume this in the morning.

@mpdude
Copy link

mpdude commented Feb 23, 2024

There is so much error output that I do not see what to lookout for. So much noise dumped I don’t see a clear backtrace.

Would it help if you could ssh into the GHA runner to investigate, or would you need a full fledged IDE setup with xDebug?

@ashnazg
Copy link
Member

ashnazg commented Feb 23, 2024

Right, because it's PHPT output, and these are convoluted testcases, it's a mess.

It never occurred to me that one could SSH into one of the runner agents... is there an info URL describing that?

At any rate, I do want to pursue my adjust-the-tarball idea first, in case that's a quick fix (too late!) ...

@mpdude
Copy link

mpdude commented Feb 23, 2024

I use this:

https://github.com/luchihoratiu/debug-via-ssh

You need an ngrok token, which you can get for free. Does that help you or do you need more detailed explanations?

You can also ssh into the runner and tunnel xDebug connections back to your machine, so you get remote debugging with you local IDE… but that’s a tad more work to set up.

@ashnazg
Copy link
Member

ashnazg commented Feb 24, 2024

So I think my "backported" hunch was sort-of right... the Github builds for even old PHP5 are all using ubuntu-latest, thus LTS 22.04.03... unlike Travis-CI that used LTS 14.04.5 for 5.5 and LTS 12.04.5 for 5.4. So there's almost certainly OS-level/library differences between the Github workers and Travis workers. At least I now feel better about trusting my understanding of what I've been seeing.

I'm resuming my troubleshooting of the last testcase failures now.

@mpdude
Copy link

mpdude commented Feb 25, 2024

Have you been able to confirm that the failures are in fact related to XML processing?

@ashnazg
Copy link
Member

ashnazg commented Feb 26, 2024

@mpdude , I got three of the four failing tests fixed... root symptom seems to be a multibyte character in the Archive_Tar tarballs that are included in the testsuite. One to go, but ran out of time today.

@mpdude
Copy link

mpdude commented Feb 26, 2024

Regarding the failing test at tests/PEAR_Command_Install/upgrade-all/test.phpt, does this help?

(Don't ask me how I got there, I don't fully remember...)

003+     [_log] => Array
004+         (
005+             [0] => Array
006+                 (
007+                     [0] => 0
008+                     [1] => XML error: not well-formed (invalid token) at line 36
009+                 )
010+
011+             [1] => Array
012+                 (
013+                     [0] => 2
014+                     [1] => Cannot initialize '/root/pear/pear-core/tests/PEAR_Command_Install/upgrade-all/packages/File-1.1.0RC3.tgz', invalid or missing package file
015+                 )
016+
017+             [2] => Array
018+                 (
019+                     [0] => 2
020+                     [1] => Package "/root/pear/pear-core/tests/PEAR_Command_Install/upgrade-all/packages/File-1.1.0RC3.tgz" is not valid
021+                 )
022+
023+             [3] => Array
024+                 (
025+                     [0] => 0
026+                     [1] => XML error: not well-formed (invalid token) at line 20
027+                 )
028+
029+             [4] => Array
030+                 (
031+                     [0] => 2
032+                     [1] => Cannot initialize '/root/pear/pear-core/tests/PEAR_Command_Install/upgrade-all/packages/XML_Parser-1.2.4.tgz', invalid or missing package file
033+                 )
034+
035+             [5] => Array
036+                 (
037+                     [0] => 2
038+                     [1] => Package "/root/pear/pear-core/tests/PEAR_Command_Install/upgrade-all/packages/XML_Parser-1.2.4.tgz" is not valid
039+                 )
040+
041+             [6] => Array
042+                 (
043+                     [0] => 0
044+                     [1] => XML error: not well-formed (invalid token) at line 19
045+                 )
046+
047+             [7] => Array
048+                 (
049+                     [0] => 2
050+                     [1] => Cannot initialize '/root/pear/pear-core/tests/PEAR_Command_Install/upgrade-all/packages/Archive_Tar-1.2.tgz', invalid or missing package file
051+                 )
052+
053+             [8] => Array
054+                 (
055+                     [0] => 2
056+                     [1] => Package "/root/pear/pear-core/tests/PEAR_Command_Install/upgrade-all/packages/Archive_Tar-1.2.tgz" is not valid
057+                 )
058+

@mpdude
Copy link

mpdude commented Feb 26, 2024

Also:

/pear/pear-core/tests/PEAR_Command_Install/upgrade-all/packages# tar xzf File-1.1.0RC3.tgz

gzip: stdin: unexpected end of file
tar: A lone zero block at 124
tar: Child returned status 1
tar: Error is not recoverable: exiting now

@ashnazg
Copy link
Member

ashnazg commented Feb 27, 2024

@mpdude , that was exactly what I needed... the culprit was multibyte char in package.xml just like the other tests, but this time it was three tarballs instead of just one with the same issue.

@ashnazg
Copy link
Member

ashnazg commented Feb 27, 2024

@alecpl can you fetch/rebase ?

Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@mpdude
Copy link

mpdude commented Feb 27, 2024

Chuck,

sincere thanks for the effort and patience you put into this! This is not the first time your work makes my life easier. I still depend on PEAR and PEAR_DB in some applications and they process ten thousands of queries every day without complaining - now also with PHP 8.3. That’s also because of the work you put into it.

@mpdude
Copy link

mpdude commented Feb 27, 2024

Now what’s that - a new series of errors?

@ashnazg ashnazg self-assigned this Mar 9, 2024
@ashnazg ashnazg merged commit 23a81cd into pear:master Mar 9, 2024
10 checks passed
@ashnazg
Copy link
Member

ashnazg commented Mar 9, 2024

v1.10.15 is released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants