Sync ext/hash installed headers

[petk]

This syncs ext/hash headers installation on *nix and Windows:

• php_hash_joaat.h and php_hash_fnv.h added also on Windows installation
• xxhash/xxhash.h added on both installations as it is included in php_hash_xxhash.h
• Include path for xxhash.h changed to relative so the php_hash_xxhash.h can be included outside of php-src
• Redundant include flags removed

This looks ok or have I misses something perhaps?

[devnexen]

Looks correct

[petk]

@devnexen just one question here. Do you think this might conflict somewhere with possible system xxhash.h file? There is for example /usr/src/linux-headers-6.5.0-14/include/linux/xxhash.h... Probably not, right?

Because we can even write the include here #include "ext/hash/xxhash/xxhash.h or even move to simply ext/hash/xxhash.h and do similar include to be sure. The root path include is used in php-config script anyway.

[devnexen]

ah good point. What can be done to counteract possible conflict is to set this, for example

#undef XXH_NAMESPACE
#define XXH_NAMESPACE PHP_NAMESPACE

[petk]

@devnexen the xxhash.h is included with the XXH_INLINE_ALL:

#define XXH_INLINE_ALL 1
#include "xxhash.h"

Isn't then XXH_NAMESPACE undefined in the xxhash.h file (meaning it wouldn't take effect)? Or would this work ok?

#define XXH_NAMESPACE PHP_NAMESPACE
#define XXH_INLINE_ALL 1
#include "xxhash.h"

[devnexen]

@devnexen the xxhash.h is included with the XXH_INLINE_ALL:

#define XXH_INLINE_ALL 1
#include "xxhash.h"

Isn't then XXH_NAMESPACE undefined in the xxhash.h file (meaning it wouldn't take effect)? Or would this work ok?

#define XXH_NAMESPACE PHP_NAMESPACE
#define XXH_INLINE_ALL 1
#include "xxhash.h"

it should work like this, it s just a "safety net" in case a theoretical case when XXH_NAMESPACE is defined by a php extension for example.

[petk]

Ah, I understand now. Ok...

[petk]

@devnexen Do you know maybe about the include order of system include directories and local php-src ones set to the compiler? Is there any chance that the include order is changed somehow in some case? For example, if the system's xxhash.h would be included from some system location prior to ext/hash? That shouldn't happen, right? Or can happen?

[devnexen]

xxhash is so customisable, some include an untouched version, some use one with couple of fixes/improvements backported from git commits (what we do here) that there is no "system" version of it ; that would be quite a conundrum. Each project include their own. In the field, a code can have xxhash then including a dependency having its own xxhash.

[petk]

Sorry for bumping the topic again, 😄 but I'm still trying to figure out how the XXH_NAMESPACE should work in PHP case. Because anything used with XXH_INLINE defined is the same from the compiler point of view...

#define XXH_INLINE_ALL 1
#include "xxhash/xxhash.h"

#include "some/other/xxhash.h"

is the same as:

#undef XXH_NAMESPACE
#define XXH_NAMESPACE PHP_NAMESPACE
#define XXH_INLINE_ALL 1
#include "xxhash/xxhash.h"

#include "some/other/xxhash.h"

And

#include "some/other/xxhash.h"

#define XXH_INLINE_ALL 1
#include "xxhash/xxhash.h"

is the same as:

#include "some/other/xxhash.h"

#undef XXH_NAMESPACE
#define XXH_NAMESPACE PHP_NAMESPACE
#define XXH_INLINE_ALL 1
#include "xxhash/xxhash.h"

Even:

#define XXH_NAMESPACE EXT_NAMESPACE
#include "some/other/xxhash.h"

#define XXH_INLINE_ALL 1
#include "xxhash/xxhash.h"

is the same as:

#define XXH_NAMESPACE EXT_NAMESPACE
#include "some/other/xxhash.h"

#undef XXH_NAMESPACE
#define XXH_NAMESPACE PHP_NAMESPACE
#define XXH_INLINE_ALL 1
#include "xxhash/xxhash.h"

I think it's not possible to properly cover the case when some other xxhash.h is used in the extension without knowing the exact case how it is used. If extension has perhaps included some xxhash.h the result will be different no matter what. I think it's such an edge case that we don't need to bother with this. So maybe we simply don't change the XXH_NAMESPACE and that's it?

[devnexen]

Fair point, you can always commit as is we can always adjust later on in case.