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

Proposed by Bryce Harrington
Status: Rejected
Rejected by: Bryce Harrington
Proposed branch: ~bryce/ubuntu/+source/php-parser:merge-v4.10.3-1-hirsute
Merge into: ubuntu/+source/php-parser:ubuntu/devel
Diff against target: 211 lines (+164/-1)
5 files modified
debian/changelog (+32/-0)
debian/control (+2/-1)
debian/patches/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch (+57/-0)
debian/patches/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch (+71/-0)
debian/patches/series (+2/-0)
Reviewer Review Type Date Requested Status
Sergio Durigan Junior (community) Needs Fixing
Ubuntu Server Dev import team Pending
Canonical Server Team Pending
Ubuntu Server Developers Pending
Review via email: mp+395546@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

I'm reviewing this one.

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

* 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
Revision history for this message
Bryce Harrington (bryce) wrote :
Download full text (3.4 KiB)

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: -------------------...

Read more...

Revision history for this message
Sergio Durigan Junior (sergiodj) wrote :

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.

Revision history for this message
Bryce Harrington (bryce) wrote :

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

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

98d6ac9... by Bryce Harrington

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

213fc35... by Bryce Harrington

changelog

07a8309... by Bryce Harrington

update-maintainer

01ce3b2... by Bryce Harrington

  * 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

  * 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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/debian/changelog b/debian/changelog
2index 15d5f15..db25f0e 100644
3--- a/debian/changelog
4+++ b/debian/changelog
5@@ -1,3 +1,17 @@
6+php-parser (4.10.3-1ubuntu1) hirsute; urgency=medium
7+
8+ * Merge with Debian unstable. Remaining changes:
9+ - d/p/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch:
10+ + Disable two new test cases in order to fix proposed migration
11+ blockage on armhf due to autopkgtest failures. Upstream indicates
12+ these new test cases have not been verified to work with 32-bit.
13+ (LP #1878102)
14+ - d/p/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch:
15+ + Disable another test case that is not yet 32-bit compatible, to fix
16+ proposed migration blockage on armhf.
17+
18+ -- Bryce Harrington <bryce@canonical.com> Tue, 15 Dec 2020 17:40:12 +0000
19+
20 php-parser (4.10.3-1) unstable; urgency=medium
21
22 [ Nikita Popov ]
23@@ -94,6 +108,24 @@ php-parser (4.5.0-1) unstable; urgency=medium
24
25 -- David Prévot <taffit@debian.org> Fri, 05 Jun 2020 17:25:11 -1000
26
27+php-parser (4.4.0-1ubuntu2) groovy; urgency=medium
28+
29+ * d/p/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch:
30+ - Disable another test case that is not yet 32-bit compatible, to fix
31+ proposed migration blockage on armhf.
32+
33+ -- Bryce Harrington <bryce@canonical.com> Thu, 14 May 2020 23:25:09 +0000
34+
35+php-parser (4.4.0-1ubuntu1) groovy; urgency=medium
36+
37+ * d/p/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch:
38+ - Disable two new test cases in order to fix proposed migration
39+ blockage on armhf due to autopkgtest failures. Upstream indicates
40+ these new test cases have not been verified to work with 32-bit.
41+ (LP: #1878102)
42+
43+ -- Bryce Harrington <bryce@canonical.com> Tue, 12 May 2020 21:59:29 +0000
44+
45 php-parser (4.4.0-1) unstable; urgency=medium
46
47 [ Nikita Popov ]
48diff --git a/debian/control b/debian/control
49index 715f004..07fe9d0 100644
50--- a/debian/control
51+++ b/debian/control
52@@ -1,7 +1,8 @@
53 Source: php-parser
54 Section: php
55 Priority: optional
56-Maintainer: Debian PHP PEAR Maintainers <pkg-php-pear@lists.alioth.debian.org>
57+Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
58+XSBC-Original-Maintainer: Debian PHP PEAR Maintainers <pkg-php-pear@lists.alioth.debian.org>
59 Uploaders: David Prévot <taffit@debian.org>,
60 Prach Pongpanich <prachpub@gmail.com>
61 Build-Depends: debhelper-compat (= 13),
62diff --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
63new file mode 100644
64index 0000000..759c633
65--- /dev/null
66+++ b/debian/patches/0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch
67@@ -0,0 +1,57 @@
68+From d8a2b2392cc65b493e375987cb593a4f641e7b64 Mon Sep 17 00:00:00 2001
69+From: Bryce Harrington <bryce@bryceharrington.org>
70+Date: Tue, 12 May 2020 20:37:14 +0000
71+Subject: [PATCH] Disable new test cases not yet 32-bit compatible
72+
73+Two test cases new in 4.4.0 fail on armhf:
74+
75+ - CodeParsingTest::testParse: Different integer syntaxes
76+ - Lexer\EmulativeTest::testLexNewFeatures: '0xCAFE_F00D'
77+
78+Upstream reports these may not be 32-bit compatible.
79+
80+For now, don't run these two new test cases, until they're expected to be
81+able to run on all platforms.
82+
83+Signed-off-by: Bryce Harrington <bryce@bryceharrington.org>
84+---
85+ test/PhpParser/Lexer/EmulativeTest.php | 12 ------------
86+ 1 file changed, 12 deletions(-)
87+
88+Origin: vendor
89+Bug: https://github.com/nikic/PHP-Parser/issues/662
90+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/php-parser/+bug/1878102
91+Last-Updated: 2020-12-18
92+
93+diff --git a/test/PhpParser/Lexer/EmulativeTest.php b/test/PhpParser/Lexer/EmulativeTest.php
94+index e141b13..9b74fd1 100644
95+--- a/test/PhpParser/Lexer/EmulativeTest.php
96++++ b/test/PhpParser/Lexer/EmulativeTest.php
97+@@ -105,15 +105,6 @@ class EmulativeTest extends LexerTest
98+ $this->assertSame($expectedTokens, $tokens);
99+ }
100+
101+- /**
102+- * @dataProvider provideTestLexNewFeatures
103+- */
104+- public function testLexNewFeatures($code, array $expectedTokens) {
105+- $lexer = $this->getLexer();
106+- $lexer->startLexing('<?php ' . $code);
107+- $this->assertSameTokens($expectedTokens, $lexer);
108+- }
109+-
110+ /**
111+ * @dataProvider provideTestLexNewFeatures
112+ */
113+@@ -241,9 +232,6 @@ class EmulativeTest extends LexerTest
114+ ['1_000', [
115+ [Tokens::T_LNUMBER, '1_000'],
116+ ]],
117+- ['0xCAFE_F00D', [
118+- [Tokens::T_LNUMBER, '0xCAFE_F00D'],
119+- ]],
120+ ['0b0101_1111', [
121+ [Tokens::T_LNUMBER, '0b0101_1111'],
122+ ]],
123+--
124+2.25.1
125diff --git a/debian/patches/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch b/debian/patches/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch
126new file mode 100644
127index 0000000..de04972
128--- /dev/null
129+++ b/debian/patches/0004-Disable-CodeParsingTest-due-to-integer-syntax.patch
130@@ -0,0 +1,71 @@
131+From a354b3bc4fcf1bfe22d84741ebd1d945c4fa50df Mon Sep 17 00:00:00 2001
132+From: Bryce Harrington <bryce@bryceharrington.org>
133+Date: Thu, 14 May 2020 23:11:18 +0000
134+Subject: [PATCH] Disable CodeParsingTest due to integer syntax inconsistency
135+ on armhf
136+
137+Workaround testsuite failure when run on armhf by disabling the test
138+case. The specific failing test case is from
139+PhpParser\CodeParsingTest::testParse with the
140+scalar/numberSeparators.test#0 data set:
141+
142+ 2: Stmt_Expression(
143+ - expr: Scalar_LNumber(
144+ - value: 3405705229
145+ + expr: Scalar_DNumber(
146+ + value: 0
147+ )
148+
149+Signed-off-by: Bryce Harrington <bryce@bryceharrington.org>
150+---
151+ test/PhpParser/CodeParsingTest.php | 29 -----------------------------
152+ 1 file changed, 29 deletions(-)
153+
154+Origin: vendor
155+Bug: https://github.com/nikic/PHP-Parser/issues/662
156+Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/php-parser/+bug/1878102
157+Last-Updated: 2020-12-18
158+
159+diff --git a/test/PhpParser/CodeParsingTest.php b/test/PhpParser/CodeParsingTest.php
160+index 24e93dd..1947932 100644
161+--- a/test/PhpParser/CodeParsingTest.php
162++++ b/test/PhpParser/CodeParsingTest.php
163+@@ -7,35 +7,6 @@ use PhpParser\Node\Stmt;
164+
165+ class CodeParsingTest extends CodeTestAbstract
166+ {
167+- /**
168+- * @dataProvider provideTestParse
169+- */
170+- public function testParse($name, $code, $expected, $modeLine) {
171+- if (null !== $modeLine) {
172+- $modes = array_fill_keys(explode(',', $modeLine), true);
173+- } else {
174+- $modes = [];
175+- }
176+-
177+- list($parser5, $parser7) = $this->createParsers($modes);
178+- list($stmts5, $output5) = $this->getParseOutput($parser5, $code, $modes);
179+- list($stmts7, $output7) = $this->getParseOutput($parser7, $code, $modes);
180+-
181+- if (isset($modes['php5'])) {
182+- $this->assertSame($expected, $output5, $name);
183+- $this->assertNotSame($expected, $output7, $name);
184+- } elseif (isset($modes['php7'])) {
185+- $this->assertNotSame($expected, $output5, $name);
186+- $this->assertSame($expected, $output7, $name);
187+- } else {
188+- $this->assertSame($expected, $output5, $name);
189+- $this->assertSame($expected, $output7, $name);
190+- }
191+-
192+- $this->checkAttributes($stmts5);
193+- $this->checkAttributes($stmts7);
194+- }
195+-
196+ public function createParsers(array $modes) {
197+ $lexer = new Lexer\Emulative(['usedAttributes' => [
198+ 'startLine', 'endLine',
199+--
200+2.25.1
201+
202diff --git a/debian/patches/series b/debian/patches/series
203index 629876c..47821f7 100644
204--- a/debian/patches/series
205+++ b/debian/patches/series
206@@ -1,2 +1,4 @@
207 0001-Allow-require-to-search-in-the-path.patch
208 0002-Adapt-shebang-for-PHP-script.patch
209+0003-Disable-new-test-cases-not-yet-32-bit-compatible.patch
210+0004-Disable-CodeParsingTest-due-to-integer-syntax.patch
211\ No newline at end of file

Subscribers

People subscribed via source and target branches