Merge ~bryce/ubuntu/+source/php-parser:merge-v4.10.3-1-hirsute into ubuntu/+source/php-parser:ubuntu/devel

[Bryce Harrington]

Straightforward merge of php-parser carrying the delta forward.

Upstream did a bunch of work on their tests, including the test cases we had disabled, so I tried a sync hoping they had fixed the issues but just hadn't updated the bug report we sent them. Unfortunately, the exact same armhf issues cropped up, so no dice.

Anyway, because of that sync attempt, I merged manually rather than through the traditional git ubuntu merge process, so there aren't the usual tags.

I took some time looking through upstream's changes to tests, and also into why the tests are failing. But I didn't want to devote too much time to that this cycle so timeboxed it when I didn't uncover any insights. The patches are a bit brute force and could be better constrained (e.g. only take affect for armhf), however I think when time is more available it would be worth figuring out exactly what's failing, or perhaps work closer with upstream so they can understand/reproduce it. Work for another day...

PPA: https://launchpad.net/~bryce/+archive/ubuntu/php-parser-merge-v4.10.3-1

[Sergio Durigan Junior (sergiodj)]

I'm reviewing this one.

[Sergio Durigan Junior (sergiodj)]

* Changelog:
  - [-] old content and logical tag match as expected
  - [√] changelog entry correct version and targeted codename
  - [√] changelog entries correct
  - [√] update-maintainer has been run

* Actual changes:
  - [√] no upstream changes to consider
  - [√] no further upstream version to consider
  - [√] debian changes look safe

* Old Delta:
  - [-] dropped changes are ok to be dropped
  - [√] nothing else to drop
  - [√] changes forwarded upstream/debian (if appropriate)

* New Delta:
  - [-] no new patches added
  - [-] patches match what was proposed upstream
  - [√] patches correctly included in debian/patches/series
  - [√] patches have correct DEP3 metadata

* Build/Test:
  - [√] build is ok
  - [√] verified PPA package installs/uninstalls
  - [X] autopkgtest against the PPA package passes
  - [√] sanity checks test fine

I've successfully installed/purged the package in a pristine hirsute container.

However, I ran the dep8 tests manually here and noticed they're failing:

autopkgtest [16:58:51]: test command1: -----------------------]
autopkgtest [16:58:52]: test command1: - - - - - - - - - - results - - - - - - - - - -
command1 FAIL non-zero exit status 2
autopkgtest [16:58:52]: @@@@@@@@@@@@@@@@@@@@ summary
command1 FAIL non-zero exit status 2

The log is too big to post here, but I'm seeing 21 failures in the test.

I also noticed that you created two extra commits on top of the branch in order to refresh the patches. Maybe it'd be better to squash them into their original commits? I'm not entirely sure this will make any difference next time the package is merged, but it's something I always try to do.

review: Needs Fixing

[Bryce Harrington (bryce)]

Huh, weird. I do not see autopkgtests running in my local lxc environment, just one warning (for a test we're intentionally disabling):

autopkgtest [19:03:43]: testing package php-parser version 4.10.3-1ubuntu1~hirsute1
autopkgtest [19:03:43]: build not needed
autopkgtest [19:03:43]: test command1: preparing testbed
Reading package lists... Done
Building dependency tree
Reading state information... Done
Starting pkgProblemResolver with broken count: 0
Starting 2 pkgProblemResolver with broken count: 0
Done
0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
1 not fully installed or removed.
After this operation, 0 B of additional disk space will be used.
Setting up autopkgtest-satdep (0) ...
(Reading database ... 114486 files and directories currently installed.)
Removing autopkgtest-satdep (0) ...
autopkgtest [19:03:46]: test command1: mkdir --parents vendor && phpab -o vendor/autoload.php -t debian/autoload.tests.php.tpl test/PhpParser && phpunit
autopkgtest [19:03:46]: test command1: [-----------------------
phpab %development% - Copyright (C) 2009 - 2021 by Arne Blankerts and Contributors

Scanning directory test/PhpParser

Autoload file vendor/autoload.php generated.

PHPUnit 8.5.9 by Sebastian Bergmann and contributors.

............................................................. 61 / 1241 ( 4%)
............................................................W 122 / 1241 ( 9%)
............................................................. 183 / 1241 ( 14%)
............................................................. 244 / 1241 ( 19%)
............................................................. 305 / 1241 ( 24%)
............................................................. 366 / 1241 ( 29%)
............................................................. 427 / 1241 ( 34%)
............................................................. 488 / 1241 ( 39%)
............................................................. 549 / 1241 ( 44%)
............................................................. 610 / 1241 ( 49%)
............................................................. 671 / 1241 ( 54%)
............................................................. 732 / 1241 ( 58%)
............................................................. 793 / 1241 ( 63%)
............................................................. 854 / 1241 ( 68%)
............................................................. 915 / 1241 ( 73%)
............................................................. 976 / 1241 ( 78%)
............................................................. 1037 / 1241 ( 83%)
............................................................. 1098 / 1241 ( 88%)
............................................................. 1159 / 1241 ( 93%)
............................................................. 1220 / 1241 ( 98%)
..................... 1241 / 1241 (100%)

Time: 618 ms, Memory: 72.00 MB

There was 1 warning:

1) Warning
No tests found in class "PhpParser\CodeParsingTest".

WARNINGS!
Tests: 1241, Assertions: 2273, Warnings: 1.
autopkgtest [19:03:47]: test command1: -------------------...

[Sergio Durigan Junior (sergiodj)]

I'm rerunning the tests now, but I've just noticed that php-parser was sync'ed from Debian in the meantime, with version 4.10.4-1, which is greater than the version you're proposing on this MP. I've also noticed that the package currently FTBFS:

https://launchpad.net/ubuntu/+source/php-parser/4.10.4-1
https://launchpadlibrarian.net/512528935/buildlog_ubuntu-hirsute-amd64.php-parser_4.10.4-1_BUILDING.txt.gz

so I think we'll need to abandon this MP and try to fix the build failure :-).

BTW, the autopktest has just finished with an error again:

https://paste.ubuntu.com/p/pdqHnTXn9P/

However, I ran autopkgtest again using the package from your PPA this time, and the tests are passing. This is kind of puzzling, but I won't spend more time on this since there's the FTBFS to fix now.

[Bryce Harrington (bryce)]

There is a new version 4.10.4 available, so I'm going to close out this MP and start a new one.

phpunit 9.5 still hasn't migrated but it's built for -proposed so I don't expect we'll hit these same issues.

Unmerged commits

0743c00... by Bryce Harrington on 2020-12-18

Refresh patch 0004-Disable-CodeParsingTest-due-to-integer-syntax.patch

98d6ac9... by Bryce Harrington on 2020-12-18

Refresh patch 0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch

213fc35... by Bryce Harrington on 2020-12-15

changelog

07a8309... by Bryce Harrington on 2020-12-15

update-maintainer

01ce3b2... by Bryce Harrington on 2020-12-15

  * d/p/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch:
    - Disable another test case that is not yet 32-bit compatible, to fix
      proposed migration blockage on armhf.

a8f82a2... by Bryce Harrington on 2020-12-15

  * d/p/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch:
    - Disable two new test cases in order to fix proposed migration
      blockage on armhf due to autopkgtest failures. Upstream indicates
      these new test cases have not been verified to work with 32-bit.
      (LP: #1878102)

diff --git a/debian/changelog b/debian/changelog
index 15d5f15..db25f0e 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,17 @@
+php-parser (4.10.3-1ubuntu1) hirsute; urgency=medium
+
+  * Merge with Debian unstable.  Remaining changes:
+    - d/p/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch:
+      + Disable two new test cases in order to fix proposed migration
+        blockage on armhf due to autopkgtest failures.  Upstream indicates
+        these new test cases have not been verified to work with 32-bit.
+        (LP #1878102)
+    - d/p/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch:
+      + Disable another test case that is not yet 32-bit compatible, to fix
+        proposed migration blockage on armhf.
+
+ -- Bryce Harrington <[e-mail]>  Tue, 15 Dec 2020 17:40:12 +0000
+
 php-parser (4.10.3-1) unstable; urgency=medium
 
   [ Nikita Popov ]
@@ -94,6 +108,24 @@ php-parser (4.5.0-1) unstable; urgency=medium
 
  -- David Prévot <[e-mail]>  Fri, 05 Jun 2020 17:25:11 -1000
 
+php-parser (4.4.0-1ubuntu2) groovy; urgency=medium
+
+  * d/p/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch:
+    - Disable another test case that is not yet 32-bit compatible, to fix
+      proposed migration blockage on armhf.
+
+ -- Bryce Harrington <[e-mail]>  Thu, 14 May 2020 23:25:09 +0000
+
+php-parser (4.4.0-1ubuntu1) groovy; urgency=medium
+
+  * d/p/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch:
+    - Disable two new test cases in order to fix proposed migration
+      blockage on armhf due to autopkgtest failures.  Upstream indicates
+      these new test cases have not been verified to work with 32-bit.
+      (LP: #1878102)
+
+ -- Bryce Harrington <[e-mail]>  Tue, 12 May 2020 21:59:29 +0000
+
 php-parser (4.4.0-1) unstable; urgency=medium
 
   [ Nikita Popov ]
diff --git a/debian/control b/debian/control
index 715f004..07fe9d0 100644
--- a/debian/control
+++ b/debian/control
@@ -1,7 +1,8 @@
 Source: php-parser
 Section: php
 Priority: optional
-Maintainer: Debian PHP PEAR Maintainers <[e-mail]>
+Maintainer: Ubuntu Developers <[e-mail]>
+XSBC-Original-Maintainer: Debian PHP PEAR Maintainers <[e-mail]>
 Uploaders: David Prévot <[e-mail]>,
            Prach Pongpanich <[e-mail]>
 Build-Depends: debhelper-compat (= 13),
diff --git a/debian/patches/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch b/debian/patches/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch
 new file mode 100644
index 0000000..759c633
--- /dev/null
+++ b/debian/patches/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch
@@ -0,0 +1,57 @@
+From d8a2b2392cc65b493e375987cb593a4f641e7b64 Mon Sep 17 00:00:00 2001
+From: Bryce Harrington <[e-mail]>
+Date: Tue, 12 May 2020 20:37:14 +0000
+Subject: [PATCH] Disable new test cases not yet 32-bit compatible
+
+Two test cases new in 4.4.0 fail on armhf:
+
+  - CodeParsingTest::testParse: Different integer syntaxes
+  - Lexer\EmulativeTest::testLexNewFeatures: '0xCAFE_F00D'
+
+Upstream reports these may not be 32-bit compatible.
+
+For now, don't run these two new test cases, until they're expected to be
+able to run on all platforms.
+
+Signed-off-by: Bryce Harrington <[e-mail]>
+---
+ test/PhpParser/Lexer/EmulativeTest.php | 12 ------------
+ 1 file changed, 12 deletions(-)
+
+Origin: vendor
+Bug: https://github.com/nikic/PHP-Parser/issues/662
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/php-parser/+bug/1878102
+Last-Updated: 2020-12-18
+
+diff --git a/test/PhpParser/Lexer/EmulativeTest.php b/test/PhpParser/Lexer/EmulativeTest.php
+index e141b13..9b74fd1 100644
+--- a/test/PhpParser/Lexer/EmulativeTest.php
++++ b/test/PhpParser/Lexer/EmulativeTest.php
+@@ -105,15 +105,6 @@ class EmulativeTest extends LexerTest
+         $this->assertSame($expectedTokens, $tokens);
+     }
+ 
+-    /**
+-     * @dataProvider provideTestLexNewFeatures
+-     */
+-    public function testLexNewFeatures($code, array $expectedTokens) {
+-        $lexer = $this->getLexer();
+-        $lexer->startLexing('<?php ' . $code);
+-        $this->assertSameTokens($expectedTokens, $lexer);
+-    }
+-
+     /**
+      * @dataProvider provideTestLexNewFeatures
+      */
+@@ -241,9 +232,6 @@ class EmulativeTest extends LexerTest
+             ['1_000', [
+                 [Tokens::T_LNUMBER, '1_000'],
+             ]],
+-            ['0xCAFE_F00D', [
+-                [Tokens::T_LNUMBER, '0xCAFE_F00D'],
+-            ]],
+             ['0b0101_1111', [
+                 [Tokens::T_LNUMBER, '0b0101_1111'],
+             ]],
+-- 
+2.25.1
diff --git a/debian/patches/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch b/debian/patches/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch
 new file mode 100644
index 0000000..de04972
--- /dev/null
+++ b/debian/patches/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch
@@ -0,0 +1,71 @@
+From a354b3bc4fcf1bfe22d84741ebd1d945c4fa50df Mon Sep 17 00:00:00 2001
+From: Bryce Harrington <[e-mail]>
+Date: Thu, 14 May 2020 23:11:18 +0000
+Subject: [PATCH] Disable CodeParsingTest due to integer syntax inconsistency
+ on armhf
+
+Workaround testsuite failure when run on armhf by disabling the test
+case.  The specific failing test case is from
+PhpParser\CodeParsingTest::testParse with the
+scalar/numberSeparators.test#0 data set:
+
+      2: Stmt_Expression(
+ -        expr: Scalar_LNumber(
+ -            value: 3405705229
+ +        expr: Scalar_DNumber(
+ +            value: 0
+          )
+
+Signed-off-by: Bryce Harrington <[e-mail]>
+---
+ test/PhpParser/CodeParsingTest.php | 29 -----------------------------
+ 1 file changed, 29 deletions(-)
+
+Origin: vendor
+Bug: https://github.com/nikic/PHP-Parser/issues/662
+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/php-parser/+bug/1878102
+Last-Updated: 2020-12-18
+
+diff --git a/test/PhpParser/CodeParsingTest.php b/test/PhpParser/CodeParsingTest.php
+index 24e93dd..1947932 100644
+--- a/test/PhpParser/CodeParsingTest.php
++++ b/test/PhpParser/CodeParsingTest.php
+@@ -7,35 +7,6 @@ use PhpParser\Node\Stmt;
+ 
+ class CodeParsingTest extends CodeTestAbstract
+ {
+-    /**
+-     * @dataProvider provideTestParse
+-     */
+-    public function testParse($name, $code, $expected, $modeLine) {
+-        if (null !== $modeLine) {
+-            $modes = array_fill_keys(explode(',', $modeLine), true);
+-        } else {
+-            $modes = [];
+-        }
+-
+-        list($parser5, $parser7) = $this->createParsers($modes);
+-        list($stmts5, $output5) = $this->getParseOutput($parser5, $code, $modes);
+-        list($stmts7, $output7) = $this->getParseOutput($parser7, $code, $modes);
+-
+-        if (isset($modes['php5'])) {
+-            $this->assertSame($expected, $output5, $name);
+-            $this->assertNotSame($expected, $output7, $name);
+-        } elseif (isset($modes['php7'])) {
+-            $this->assertNotSame($expected, $output5, $name);
+-            $this->assertSame($expected, $output7, $name);
+-        } else {
+-            $this->assertSame($expected, $output5, $name);
+-            $this->assertSame($expected, $output7, $name);
+-        }
+-
+-        $this->checkAttributes($stmts5);
+-        $this->checkAttributes($stmts7);
+-    }
+-
+     public function createParsers(array $modes) {
+         $lexer = new Lexer\Emulative(['usedAttributes' => [
+             'startLine', 'endLine',
+-- 
+2.25.1
+
diff --git a/debian/patches/series b/debian/patches/series
index 629876c..47821f7 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -1,2 +1,4 @@
 0001-Allow-require-to-search-in-the-path.patch
 0002-Adapt-shebang-for-PHP-script.patch
+0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch
+0004-Disable-CodeParsingTest-due-to-integer-syntax.patch
 \ No newline at end of file