Workaround in ZipArchiver when posix_spawn_file_actions_addchdir_np is unavailable #7187
Conversation
yim-lee
requested review from
neonichu,
tomerd and
MaxDesiatov
as code owners
|
@swift-ci please test |
| completion: completion | ||
| ) | ||
| } else { | ||
| let process = Foundation.Process() |
There was a problem hiding this comment.
we should probably use TSC's process since it handles stdout and stderr better
There was a problem hiding this comment.
If we use TSC.Process we would hit the same blocker (i.e., the Glibc check) that's preventing publish from working.
There was a problem hiding this comment.
only if we set the working directory, no?
There was a problem hiding this comment.
in fact I believe if we set the working directory in Foundation process we will also run into the same issue because its also using posix_spawn_file_actions_addchdir
There was a problem hiding this comment.
Using posix_spawn_file_actions_addchdir is good, that's what we want right? The issue is just if that's not available, then we need to fail (and not just call chdir).
There was a problem hiding this comment.
We want to make this work when posix_spawn_file_actions_addchdir is not available instead of failing. And rather than having different code paths, we do the same for all scenarios (except on Windows) so we'd know sooner if it breaks.
There was a problem hiding this comment.
@yim-lee the only way to make this work on UNIX without posix_spawn_file_actions_addchdir is to reimplement with fork+exec.
Do you happen to know which systems lack posix_spawn_file_actions_addchdir?
There was a problem hiding this comment.
@weissi We are trying to make something works for Amazon Linux 2, which has an earlier version of glibc.
There was a problem hiding this comment.
okay, thanks. if that's still an important target platform then it would need to be fork+exec there
|
Do we actually need the check? We could just always |
My concern is the change might potentially change/break existing behavior, but we can always fix it if it happens I suppose. |
|
Right, the advantage would be that we learn about the breakage quicker since we would exercise the same code in more scenarios. |
yim-lee
force-pushed
the
al2-registry-publish
branch
from
4fff895 to
d698cdb
Compare
|
Updated per feedback. @swift-ci please test |
|
@swift-ci please test Windows platform |
|
@swift-ci test macos |
yim-lee
force-pushed
the
al2-registry-publish
branch
from
d698cdb to
2f30d19
Compare
|
Rebased. @swift-ci please test |
|
@swift-ci please test Windows |
1 similar comment
|
@swift-ci please test Windows |
|
@swift-ci please test Linux |
|
@swift-ci please test Windows |
|
@swift-ci smoke test |
|
@swift-ci test |
…s unavailable Motivation: `swift package-registry publish` tool doesn't work in Amazon Linux 2 because it has an older version of Glibc that doesn't support `posix_spawn_file_actions_addchdir_np`. Modifications: Add workaround in `ZipArchiver` that does `cd <working directory> && zip ...` when `posix_spawn_file_actions_addchdir_np` is not available.
yim-lee
force-pushed
the
al2-registry-publish
branch
from
2f30d19 to
27c24b9
Compare
|
@swift-ci please test |
|
@swift-ci please test Windows |
|
Build timed out. @swift-ci please test Windows |
|
@swift-ci please test Windows |
Motivation:
swift package-registry publishtool doesn't work in Amazon Linux 2 because it has an older version of Glibc that doesn't supportposix_spawn_file_actions_addchdir_np.Modifications:
Add workaround in
ZipArchiverthat doescd <working directory> && zip ...whenposix_spawn_file_actions_addchdir_npis not available.