Php8 comp

[lifenglsf]

修复php8，stdint.h文件变化,php8.2使用系统的stdint.h

[sodabrew]

This is defined by the Microsoft C compiler version. Can you point to a document explaining why this is the right variable to use to switch which header to include?

[lifenglsf]

for this commit,php removed php_stdint.h file

[sodabrew]

Found it: https://github.com/php/php-src/blob/PHP-8.2/UPGRADING.INTERNALS#L26

But I still don't understand why the MSC_VER is the right way to determine whether to use the win32 header variant. Isn't the issue here that the PHP version needs to be checked for >= 80200?

[cmb69]

Yeah, it's probably best to check for the PHP version only; in theory, <stdint.h> might not be available (i.e. for very old MSVC/SDKs), but in practice this can be ignored.

[lifenglsf]

reference this pull request

[cmb69]

Oh, I just realize that this is actually about win32/php_stdint.h which is removed as of PHP 8.0.0 (I thought it is about main/php_stdint.h which is removed as of PHP 8.2.0 only). So, I don't understand why there should be any change at all. Apparently, standard Windows builds for PHP 8.0 and 8.1 are successful.

[lifenglsf]

I want to less if else condition

[cmb69]

Okay, simplification is nice, and compatibility with PHP 8.2.0 is important. But why not:

 php_memcached_private.h | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/php_memcached_private.h b/php_memcached_private.h
index b4b1115..2d67e36 100644
--- a/php_memcached_private.h
+++ b/php_memcached_private.h
@@ -48,11 +48,11 @@
 #include <ext/standard/basic_functions.h>
 
 #ifdef PHP_WIN32
-  #  if PHP_VERSION_ID >= 80000
-  #  include "php_stdint.h"
-#else
-# include "win32/php_stdint.h"
-#endif
+# if PHP_VERSION_ID >= 80000
+#  include <stdint.h>
+# else
+#  include "win32/php_stdint.h"
+# endif
 #else
 /* Used to store the size of the block */
 #  if defined(HAVE_INTTYPES_H)

This is basically the same as your suggestion, but checking for PHP_VERSION instead of _MSC_VER.

[lifenglsf]

ok,good suggestion.i'll modify it.

[sodabrew]

thank you, I'm keeping an eye out for the PR update!

[remicollet]

Perhaps @cmb69 may help on this ?

[sodabrew]

Sidenote: It occurs to me that GitHub Actions offers Windows builds, that might be helpful to put into the matrix.

Edit: I totally missed #532 submitted last week!

[sodabrew]

Just merged #532 adding Windows CI to the build matrix. Could you a) rebase, b) adjust per @cmb69 suggestions? I agree strongly that unless there's a specific compiler workaround required for MSVC, we should go with something like ifdef PHP_WIN32 and PHP_VERSION >= 8xxx to select the appropriate header file that comes with that version of PHP.

[sodabrew]

Should it be >= 80200 instead of 80000? There are still some uses of u_char in 8.0 - 8.1 that were fully removed before 8.2.0, e.g. php/php-src#8611 which was the purpose of the prior main/php_stdint.h header.

[cmb69]

php_stdint.h is removed as of PHP 8.2.0, but win32/php_stdint.h is already removed as of PHP 8.0.0. So unless you want to have three cases, >=80000 looks good to me.

[sodabrew]

Thank you @lifenglsf and everyone who helped me understand the background on this PR!