phar update - Externals

16 years ago by Greg Beaver — view source

unread

Hi,

As PHP has the good fortune of being blessed with Scott MacVicar who
connected me with the ppc64 test machine provided by IBM and the sparc
machine provided by Sun through David Soria-Parra (thank you thank you
thank you), phar has now been verified as working in the following
configurations that were previously untested:

64-bit, big-endian, gcc linux
64-bit, big-endian, suncc SunOS

This means phar has been verified as working by me on the following
configurations:

32-bit, little-endian, gcc linux
64-bit, little-endian, gcc Darwin OS X
32-bit, little-endian, MS Windows XP
64-bit, little-endian, gcc linux
64-bit, big-endian, gcc linux
64-bit, big-endian, suncc, SunOS

When I say working, I mean 100% of non-skipped tests passing, no compile
warnings. The tests exert 80% code coverage, mostly leaving untestable
stuff like errors that are only likely to occur when the disk crashes.

In addition, I discovered potential problems with the zip
implementation, in that the code originally used a packed struct, taking
advantage of #pragma pack on windows and sgi, __attribute for gcc.
However, it did not work with sun's cc, and on further reading, I
discovered that using a packed struct without specific compiler flags
could lead to bus errors on sparc machines, and to page faults on
windows 64-bit. The specific compiler flags to fix this would result in
a slower PHP, which was unacceptable.

Thus, I undertook a refactoring of the way zip's structs are accessed,
taking advantage of the fact that structs of char arrays are never
padded, and use some simple math to convert char[4] to php_uint32 and
char[2] to php_uint16. The code is much simpler, works without #ifdef
on both big-endian and little-endian systems, and has no risk of failing
with any future compilers or OSes as it doesn't rely on
compiler-specific processing commands.

To be clear: this issue did not affect tar-based phars, or phar-based
phars, and only affected zip-based phars on SunOS with sun CC, or on any
Sparc machine or Windows 64-bit if and only if the specific zip file
would cause a read across a word boundary (whew). And also to be clear:
this problem is ancient history now.

I do need some help verifying phar as working with the following
configurations:

any endian any unix with intel CC
any Windows Vista
64-bit Windows XP
any other supported OS/compiler/processor combo I didn't list.

Verification is as simple as compiling PHP 5.3 from CVS or snap and
running the tests.

Also thanks to those raising concerns about phar, it has served to make
it stronger.

Thanks,
Greg

16 years ago by Felipe Pena — view source

unread

Em Sex, 2009-02-20 às 21:36 -0600, Greg Beaver escreveu:

Hi,

As PHP has the good fortune of being blessed with Scott MacVicar who
connected me with the ppc64 test machine provided by IBM and the sparc
machine provided by Sun through David Soria-Parra (thank you thank you
thank you), phar has now been verified as working in the following
configurations that were previously untested:

64-bit, big-endian, gcc linux
64-bit, big-endian, suncc SunOS

This means phar has been verified as working by me on the following
configurations:

32-bit, little-endian, gcc linux
64-bit, little-endian, gcc Darwin OS X
32-bit, little-endian, MS Windows XP
64-bit, little-endian, gcc linux
64-bit, big-endian, gcc linux
64-bit, big-endian, suncc, SunOS

When I say working, I mean 100% of non-skipped tests passing, no compile
warnings. The tests exert 80% code coverage, mostly leaving untestable
stuff like errors that are only likely to occur when the disk crashes.

Great work! :D

--
Regards,
Felipe Pena

16 years ago by Marcus Boerger — view source

unread

Hello Greg,

in one word: Awesome!

Saturday, February 21, 2009, 4:36:38 AM, you wrote:

Hi,

As PHP has the good fortune of being blessed with Scott MacVicar who
connected me with the ppc64 test machine provided by IBM and the sparc
machine provided by Sun through David Soria-Parra (thank you thank you
thank you), phar has now been verified as working in the following
configurations that were previously untested:

64-bit, big-endian, gcc linux
64-bit, big-endian, suncc SunOS

This means phar has been verified as working by me on the following
configurations:

32-bit, little-endian, gcc linux
64-bit, little-endian, gcc Darwin OS X
32-bit, little-endian, MS Windows XP
64-bit, little-endian, gcc linux
64-bit, big-endian, gcc linux
64-bit, big-endian, suncc, SunOS

When I say working, I mean 100% of non-skipped tests passing, no compile
warnings. The tests exert 80% code coverage, mostly leaving untestable
stuff like errors that are only likely to occur when the disk crashes.

In addition, I discovered potential problems with the zip
implementation, in that the code originally used a packed struct, taking
advantage of #pragma pack on windows and sgi, __attribute for gcc.
However, it did not work with sun's cc, and on further reading, I
discovered that using a packed struct without specific compiler flags
could lead to bus errors on sparc machines, and to page faults on
windows 64-bit. The specific compiler flags to fix this would result in
a slower PHP, which was unacceptable.

Thus, I undertook a refactoring of the way zip's structs are accessed,
taking advantage of the fact that structs of char arrays are never
padded, and use some simple math to convert char[4] to php_uint32 and
char[2] to php_uint16. The code is much simpler, works without #ifdef
on both big-endian and little-endian systems, and has no risk of failing
with any future compilers or OSes as it doesn't rely on
compiler-specific processing commands.

To be clear: this issue did not affect tar-based phars, or phar-based
phars, and only affected zip-based phars on SunOS with sun CC, or on any
Sparc machine or Windows 64-bit if and only if the specific zip file
would cause a read across a word boundary (whew). And also to be clear:
this problem is ancient history now.

I do need some help verifying phar as working with the following
configurations:

any endian any unix with intel CC
any Windows Vista
64-bit Windows XP
any other supported OS/compiler/processor combo I didn't list.

Verification is as simple as compiling PHP 5.3 from CVS or snap and
running the tests.

Also thanks to those raising concerns about phar, it has served to make
it stronger.

Thanks,
Greg

Best regards,
Marcus

16 years ago by David Coallier — view source

unread

When I say working, I mean 100% of non-skipped tests passing, no compile
warnings. The tests exert 80% code coverage, mostly leaving untestable
stuff like errors that are only likely to occur when the disk crashes.

Seriously well done! Kudos

--
Slan,
David

16 years ago by Kalle Sommer Nielsen — view source

unread

Hi Greg

2009/2/21 Greg Beaver greg@chiaraquartet.net:

Hi,

[snip]

Good work!

I do need some help verifying phar as working with the following
configurations:

any endian any unix with intel CC
any Windows Vista
64-bit Windows XP
any other supported OS/compiler/processor combo I didn't list.

I tested phar under 5.3-cvs and HEAD with Windows Vista 32bit little
endian and VC9.

Theres only one compilation warning which only exists in HEAD:
ext\phar\func_interceptors.c(101) : warning C4101: 'newlen' :
unreferenced local variable

However tests wise then the following tests fails in 5.3-cvs:
Phar front controller with mounted external file
[C:\php\src\ext\phar\tests\front.phar.phpt]
Phar::getSupportedSignatures()
[C:\php\src\ext\phar\tests\phar_get_supported_signatures_002.phpt]
Phar::setStub() (zip-based) [C:\php\src\ext\phar\tests\zip\phar_stub.phpt]

and in 124 tests fails for in HEAD, instead of writing an insanely
long list here, I have zipped both the test log and diffs for 5.3 and
HEAD which can be downloaded here:

Verification is as simple as compiling PHP 5.3 from CVS or snap and
running the tests.

Also thanks to those raising concerns about phar, it has served to make
it stronger.

Thanks,
Greg

--

--
Kalle Sommer Nielsen
kalle@php.net

16 years ago by Kalle Sommer Nielsen — view source

unread

Seems like gmail did not include my link for whatever reason, so here
it is again:
http://www.tuxxedo.net/php/phar-tests.zip

--
Kalle Sommer Nielsen
kalle@php.net

16 years ago by Steph Fox — view source

unread

Hi Kalle,

and in 124 tests fails for in HEAD, instead of writing an insanely
long list here, I have zipped both the test log and diffs for 5.3 and
HEAD which can be downloaded here:

Yeah, that's normal - most of Phar doesn't work yet in HEAD.

Thanks!

Steph

16 years ago by Greg Beaver — view source

unread

hi Kalle,

Thank you very much for your work on this, it is much appreciated.
Sorry for the delay in replying, I have been out of town for a while.

Kalle Sommer Nielsen wrote:

Hi Greg

2009/2/21 Greg Beaver greg@chiaraquartet.net:

Hi,

[snip]

Good work!

I do need some help verifying phar as working with the following
configurations:

any endian any unix with intel CC
any Windows Vista
64-bit Windows XP
any other supported OS/compiler/processor combo I didn't list.

I tested phar under 5.3-cvs and HEAD with Windows Vista 32bit little
endian and VC9.

Theres only one compilation warning which only exists in HEAD:
ext\phar\func_interceptors.c(101) : warning C4101: 'newlen' :
unreferenced local variable

However tests wise then the following tests fails in 5.3-cvs:
Phar front controller with mounted external file
[C:\php\src\ext\phar\tests\front.phar.phpt]

It appears that this test is not line-ending agnostic, and I'm about to
commit a fix, as the problem is simply the length of the string is 172
instead of 167 due to extra \r. This is very good news :)

Phar::getSupportedSignatures()
[C:\php\src\ext\phar\tests\phar_get_supported_signatures_002.phpt]

This one is perplexing - I wonder if you have built ext/hash statically
or as a .dll? The phar ext is not detected ext/hash, but the test is
not skipping because ext/hash is present in the SKIPIF

Phar::setStub() (zip-based) [C:\php\src\ext\phar\tests\zip\phar_stub.phpt]

This one is actually troubling, as it looks like a crash is terminating
the test at the second setStub()

I will try to investigate, but if others can confirm this failure on
their windows, that would be helpful.

Thanks,
Greg

16 years ago by Kalle Sommer Nielsen — view source

unread

Hi Greg

2009/3/1 Greg Beaver greg@chiaraquartet.net:

hi Kalle,

Thank you very much for your work on this, it is much appreciated.
Sorry for the delay in replying, I have been out of town for a while.

No problem :)

Kalle Sommer Nielsen wrote:

Hi Greg

2009/2/21 Greg Beaver greg@chiaraquartet.net:

Hi,

[snip]

Good work!

I do need some help verifying phar as working with the following
configurations:

any endian any unix with intel CC
any Windows Vista
64-bit Windows XP
any other supported OS/compiler/processor combo I didn't list.

I tested phar under 5.3-cvs and HEAD with Windows Vista 32bit little
endian and VC9.

Theres only one compilation warning which only exists in HEAD:
ext\phar\func_interceptors.c(101) : warning C4101: 'newlen' :
unreferenced local variable

However tests wise then the following tests fails in 5.3-cvs:
Phar front controller with mounted external file
[C:\php\src\ext\phar\tests\front.phar.phpt]

It appears that this test is not line-ending agnostic, and I'm about to
commit a fix, as the problem is simply the length of the string is 172
instead of 167 due to extra \r. This is very good news :)

Phar::getSupportedSignatures()
[C:\php\src\ext\phar\tests\phar_get_supported_signatures_002.phpt]

This one is perplexing - I wonder if you have built ext/hash statically
or as a .dll? The phar ext is not detected ext/hash, but the test is
not skipping because ext/hash is present in the SKIPIF

ext/hash was built staticlly (default)

I looked into the issue and it seems like PHAR_HASH_OK isn't getting
defined, so I assume its a missing AC_DEFINE call in config.w32, so
something along the lines of the following should do if ext/hash is
built staticlly:

if(PHP_HASH != "no" && !PHP_HASH_SHARED) {
AC_DEFINE("PHAR_HASH_OK", 1);
}

using the above code in config.w32 and then buildconf, configure,
nmake and run-tests on that test makes it pass but the test will still
fail if ext/hash is built as shared ofcourse.

Phar::setStub() (zip-based) [C:\php\src\ext\phar\tests\zip\phar_stub.phpt]

This one is actually troubling, as it looks like a crash is terminating
the test at the second setStub()

I did some more tests and a couple of fresh checkouts and builds and
this test does not fail anymore for me, if anyone can confirm it would
be good.

I will try to investigate, but if others can confirm this failure on
their windows, that would be helpful.

Thanks,
Greg

--
Kalle Sommer Nielsen
kalle@php.net

16 years ago by Greg Beaver — view source

unread

Kalle Sommer Nielsen wrote:

Phar::getSupportedSignatures()
[C:\php\src\ext\phar\tests\phar_get_supported_signatures_002.phpt]
This one is perplexing - I wonder if you have built ext/hash statically
or as a .dll? The phar ext is not detected ext/hash, but the test is
not skipping because ext/hash is present in the SKIPIF

ext/hash was built staticlly (default)

I looked into the issue and it seems like PHAR_HASH_OK isn't getting
defined, so I assume its a missing AC_DEFINE call in config.w32, so
something along the lines of the following should do if ext/hash is
built staticlly:

if(PHP_HASH != "no" && !PHP_HASH_SHARED) {
AC_DEFINE("PHAR_HASH_OK", 1);
}

using the above code in config.w32 and then buildconf, configure,
nmake and run-tests on that test makes it pass but the test will still
fail if ext/hash is built as shared ofcourse.

Phar::setStub() (zip-based) [C:\php\src\ext\phar\tests\zip\phar_stub.phpt]
This one is actually troubling, as it looks like a crash is terminating
the test at the second setStub()

I did some more tests and a couple of fresh checkouts and builds and
this test does not fail anymore for me, if anyone can confirm it would
be good.

I found the problem, and it's amazing that only windows had this
problem, and only intermittently. I found different tests would fail,
it's a miracle none failed on the first try for me. The problem was
that the splitting of a timestamp into date and time fields for zip
format was inadvertantly flipped when I did the most recent commit for
big-endian stuff. None of the tests confirm that timestamp is
unmodified on load, and so it slipped by. I'm going to add that to the
test in question shortly, to be certain this doesn't happen again.

Thanks for catching this Kalle, and for the config.w32 fix.

Greg

16 years ago by Igor Feghali — view source

unread

Hello Greg,

AIX 5.3.0.0, PPC 64bit:

297 passed
214 skipped

I am doing a second run right now to see if something goes different.
Please let me know if you want the test log as well.

Regards,
Igor Feghali.

16 years ago by Greg Beaver — view source

unread

Igor Feghali wrote:

Hello Greg,

AIX 5.3.0.0, PPC 64bit:

297 passed
214 skipped

I am doing a second run right now to see if something goes different.
Please let me know if you want the test log as well.

Hi Igor,

You can cut down on the skipped tests by building bz2, zlib, and openssl.

Thanks,
Greg

16 years ago by Igor Feghali — view source

unread

You can cut down on the skipped tests by building bz2, zlib, and openssl.

350 passed
161 skipped

By the way, phar complained Archive.php wasn't found (no PEAR
installed in this machine) in make time. Does it affects anything ?

16 years ago by Marcus Boerger — view source

unread

Hello Igor,

Monday, March 2, 2009, 5:31:51 PM, you wrote:

You can cut down on the skipped tests by building bz2, zlib, and openssl.

350 passed
161 skipped

By the way, phar complained Archive.php wasn't found (no PEAR
installed in this machine) in make time. Does it affects anything ?

No, it just limits the capabilities of the phar command line tool.

Best regards,
Marcus

16 years ago by Igor Feghali — view source

unread

Hello Greg,

I have an AIX PPC 64 bits, IBM XL C/C++ compiler, which I can run the
tests. Is it of any interest ?

I read you tested on a PPC 64 already but as far as i understand you
used a GNU compiler.

Regards,
Igor Feghali.

16 years ago by Igor Feghali — view source

unread

While trying to perform phar testing on the system previously
mentioned by me, I couldn't manage to get php5.3-200902261130 to
compile with IBM CC. First 3 lines of error:

"/tmp/php5.3-200902261130/ext/pcre/pcrelib/pcre_internal.h", line
976.3: 1506-046 (S) Syntax error.
"/tmp/php5.3-200902261130/ext/pcre/pcrelib/pcre_internal.h", line
1027.3: 1506-046 (S) Syntax error.
"/tmp/php5.3-200902261130/ext/pcre/pcrelib/pcre_internal.h", line
1137.8: 1506-166 (S) Definition of function BOOL requires parentheses.

It seems that BOOL isn't getting defined. Can someone please help me ?

Thank you.

16 years ago by Scott MacVicar — view source

unread

Igor Feghali wrote:

While trying to perform phar testing on the system previously
mentioned by me, I couldn't manage to get php5.3-200902261130 to
compile with IBM CC. First 3 lines of error:

"/tmp/php5.3-200902261130/ext/pcre/pcrelib/pcre_internal.h", line
976.3: 1506-046 (S) Syntax error.
"/tmp/php5.3-200902261130/ext/pcre/pcrelib/pcre_internal.h", line
1027.3: 1506-046 (S) Syntax error.
"/tmp/php5.3-200902261130/ext/pcre/pcrelib/pcre_internal.h", line
1137.8: 1506-166 (S) Definition of function BOOL requires parentheses.

It seems that BOOL isn't getting defined. Can someone please help me ?

It's a bug in PCRE that has been reported, can't find the bug report
just now though, you can fix it with a compiler define for using ANSI C.

Scott

16 years ago by Igor Feghali — view source

unread

It's a bug in PCRE that has been reported, can't find the bug report
just now though, you can fix it with a compiler define for using ANSI C.

Just for the record thats the patch to fix this issue:

http://bugs.exim.org/attachment.cgi?id=294

And here is the original bug:

http://bugs.php.net/bug.php?id=46040

Even though "make" finishes with success, cli is not made.

$ make cli
echo '

Target "cli" is up to date.

$ ls sapi/cli/php
ls: 0653-341 The file sapi/cli/php does not exist.

will dig into this next monday when i get to office again.