Make socket path shorter for socket_cmsg_{rights|credentials}.phpt

[andypost]

When running in CI it fails when path/address is longer 108 with following message

Fatal error: Uncaught ValueError: socket_bind(): Argument #2 ($address) must be less than 108 in ...

[cmb69]

Oh, indeed, these paths may be too long. I wonder whether we want to go with relative paths, though, or whether we want to use sys_get_temp_dir() as prefix. I'm not too happy with either.

[andypost]

@cmb69 tmp dir also can be longer 108 chars, the great explanation I found is https://unix.stackexchange.com/a/367158

[andypost]

At least inside tests dir the test runner must be able to create files, so socksts too, as I get we are not supposed to move any files while tests are running, maybe just tune socket names according test to allow parallel

[cmb69]

I'm not concerned that relative filenames won't work as socket names, but rather that we would create the files in a folder relative to the CWD (so not necessarily besides the test files as usual), so the names might conflict. Not a big problem in this case, and your argument that the temporary directory might be too long makes sense.

[arnaud-lb]

We could chdir() to __DIR__ in the test so that we are in a known dir

[andypost]

@arnaud-lb I think this is more readable

[andypost]

@arnaud-lb I think this is more readable

[arnaud-lb]

@cmb69 I think that this addresses your concerns

[cmb69]

Thank you!

[cmb69]

Ugh, the new tests apparently fail for ZTS builds; I've reverted for now.

[cmb69]

Hmm, I think that chdir() approach can't work due to the different CWD PHP and the OS see on ZTS builds. So either drop chdir() and stick with the relative filenames, or reconsider using the temporary directory (and skip the test if the path is too long).

[andypost]

So I changed to chdir(sys_get_temp_dir()); and moved the CLEAN block to the same place as other file

[andypost]

It also fails on ZTS so only relative path works

[andypost]

Not clear why CI failed this time

[cmb69]

On NTS builds, chdir() calls chrdir(2), so the system knows where we are. On ZTS builds, chdir() only modifies PHP internal state, but doesn't call chdir(2). However, PHP resolves the path to the socket (file) in any case, so if the OS has a different notion of the CWD, that won't work.

[github-actions]

There has not been any recent activity in this PR. It will automatically be closed in 7 days if no further action is taken.

[andypost]

@cmb69 I have no idea how to fix path for zts build

[bukka]

@andypost Maybe try to not use chdir and do sys_get_temp_dir() . '/socket_cmsg_credentials.sock' instead...

[cmb69]

@bukka, see #8617 (comment). :(

[bukka]

Well it's quite unlikely to happen in our pipeline and it's already used elsewhere and working fine:

php-src/sapi/fpm/tests/tester.inc

Lines 1040 to 1042 in da94baf

	return sys_get_temp_dir().'/'.
	hash('crc32', dirname($address)).'-'.
	basename($address);

. It's just a test so we don't need to account for every system setup out there IMHO...

[bukka]

@andypost What's the status of this? Are you happy to do the suggested change in #8617 (comment) ? As I mentioned it should be fine because we are already using that for FPM tests.

[andypost]

@bukka I can change it but already faced with it in #8617 (comment)

And @cmb69 also mentioned it #8617 (comment)

[bukka]

@andypost Are you actually working / running test on the platform where sys_get_temp_dir() is longer than 108? What I try to say is that on such platform all our tests won't already run (specifically the FPM ones). The sys_get_temp_dir() has been there for years and no one complained about it so I assume there are not many users that would have any issue with it. So unless there's a better solution, I would just use it here as well.

[andypost]

@bukka I checked both builders and CI returns /tmp for sys_get_temp_dir()

[cmb69]

Then let's go with sys_get_temp_dir(). However, I think the --CLEAN-- section should come after the --FILE-- section, not the other way round.

[andypost]

Fixed! hope tests will be green)

[bukka]

Just to clarify, where is this currently failing? Just considering what branch to merge this to. I guess if this is failing for someone already and they need to fix it, we could potentially target 8.0 but if it's just more theoretical, I might just merge it to master...

[andypost]

@bukka it fails in CI when there's "home/whateber/subdir..hash" used and ext/sockets/tests/socket_cmsg_credentials.phpt itself is 46 chars

[bukka]

Yeah I understand when this happens. I was more asking in which pipeline this is failing as it is fine in ours... Anyway I guess it might be failing in yours pipelines so I just merged it to 8.0 ( c58241a ) as it's not a big deal to get it there anyway.

[andypost]

Thank you a lot, I'm using it in Alpinelinux (it adds extra depth in builders and CI)

[andypost]

@bukka I faced it only on PHP 8.2 build, others works fine