Repository navigation
test(http): ensure URI locale regression runs in Linux CI - #11887
Conversation
|
Thanks, the test reads right. One CI problem: the new Could you either provision a private |
MarcZX10
left a comment
There was a problem hiding this comment.
The focused locale test design and its local evidence are detailed, but there is a blocking CI integration gap. On the Linux environment described in the PR (C/C.UTF-8/POSIX, with no locale that treats 0xE9 as alphabetic), guest_http_uri_locale returns 77 and skips. The PR itself notes that the CI skip audit rejects this unless the runner provides a Latin-1 locale, and the workflow does not currently provision one.
I checked the current main version of tools/check_skips.py as well as the version at the PR base. The current checker still rejects this skip: its device-limitation patterns do not recognize the emitted SKIP: URI locale regression... message. A representative CTest JUnit result with SKIP_RETURN_CODE=77 and that diagnostic returns error: guest_http_uri_locale skipped from the checker. The PR's automatic checks skip the full Linux build, so they do not cover this integration.
Please make the Linux test environment provide an effective private Latin-1 locale (as the PR's own validation does), or update the CI skip policy deliberately and document why this coverage skip is acceptable. Then run the relevant Linux CI path. I do not recommend approving while the full Linux job can fail on this test.
|
Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
Dismissing a stale review: the requested changes were addressed after it (new head) and the merge queue blocks on it. Please request changes again if anything is still open.
adriadam10
left a comment
There was a problem hiding this comment.
The full Build of this head (run 40733382859) fails in the linux job at python3 tools/check_skips.py build/ctest.xml: error: guest_http_uri_locale skipped.
- The job's step list has no "Prepare a private Latin-1 locale for the HTTP regression" step. The queue dispatches
build.ymlfrommainwithpr=N, so the workflow edit in.github/workflows/build.yml(line 522) never runs for this PR;ANYPS5_HTTP_TEST_LOCALE_DIRstays empty and the test skips with 77, which the skip audit rejects. - This means the provisioning can't be validated before merge, and the PR can't go green through the queue as written.
Suggested direction: do the provisioning from the PR's tree instead of the workflow, e.g. a script under tools/ that core/libs/prx/libSceHttp/CMakeLists.txt runs at configure time on Linux (generate en_US.ISO8859-1 into the build dir with localedef when the sources are installed, and set LOCPATH for guest_http_uri_locale only). Keep the skip-77 path for machines without localedef, and keep check_skips.py unchanged. The test and CMake registration changes themselves read fine.
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed 384d3436deddce0c9995eeeebd1c47a292653af6 against 8ff4baf53489033dae79090640143e82d159ee4d. The URI locale regression is separately registered with skip code 77. The Windows run actually selects .1252 and reports that 0xE9 is alphabetic, so its escape/parse checks exercise the intended locale condition. Source review covers Linux private-locale creation/LOCPATH handling; that Linux localedef branch was not executed locally. No actionable finding in this scoped test review.
Windows/GCC 15.3 Release: configured and built the actual pinned target; registered focused CTest: 2 passed, 0 skipped. Baseline was also built/run: 1 passed, 0 skipped. Matching cached dependency source pins and the matching existing FFmpeg package are used. No game, physical-console or GPU validation is claimed.
|
Checked the new head against the previous request: the locale is now generated during CMake configure from the PR tree (private |
What
On Linux installations with only C/C.UTF-8 locales, the alternate-locale URI escape/parser assertions added in #8017 could silently exercise no effective case and still report success. This follows up on the contributor's reproduction.
Register those existing assertions as
guest_http_uri_locale. It reports the tested locale and explicitly skips with code 77 when no locale classifies Latin-10xE9as alphabetic. Windows requires.1252. The ordinary HTTP test retains its C-locale assertions; production sources and every existing assertion expression remain unchanged.CMake now prepares a private
en_US.ISO8859-1in the HTTP build directory on Linux using hostlocaledefand installed locale sources. It runs from the checked-out PR tree, so a queue dispatch using the workflow frommainexecutes it before registering the test. This addresses review 5687342073: the previous workflow-only setup never ran in full Build 40733382859, and the Linux skip audit correctly rejected the missing test coverage.ANYPS5_HTTP_TEST_LOCALE_DIRremains an explicit override. Onlyguest_http_uri_localereceivesLOCPATH. Missinglocaledef, unavailable source data or generation failure is reported during configuration; the test still probes available locales and skips with 77 if none is effective. No package downloads, installations or system-locale changes occur. The ineffective workflow step and its CMake argument are removed;build.ymlnow matches current main.check_skips.pyis unchanged and still rejects lost coverage. BUILD.md documents the prerequisites and override.The current overlap snapshot reports six base-conflicted peers. #9834 adds distinct transport/linkage/lifecycle work in HTTP CMake and GuestHttp; #674, #1835 and #5021 document input/native ARM64 work; #10194 and #11351 document GPU memory accounting and shader probe capture. Complete shared-file patches were retained and reviewed, using identical earlier scoped baselines for unchanged heads. Preserve all test registrations, the original Windows PATH block and documentation sections during integration. No locale duplicate, unmerged import or open prerequisite was found.
Tested
Exact committed head
384d3436deddce0c9995eeeebd1c47a292653af6, a fast-forward follow-up on3eae29619679ce7fa3ef4f49f4f681fe396b02f7, with original base8ff4baf53489033dae79090640143e82d159ee4d. The focused Release harness uses complete real libc and libSceHttp built at a41c and reused after reconfiguring this exact commit. Production and runtime fixture sources are byte-identical; both incremental builds report no work.LOCPATH,I18NPATH, workflow locale variable or explicit CMake locale directory, creates the private locale and passes 4/4 HTTP tests (0.58 s) through actualtools/select_tests.py ctest. The test reports alphabetic0xE9; both the branch skip gate and current-main gate accept the JUnit with zero skips. The source-data fixture temporarily exposes the cached distrolocalespackage's source files at/usr/share/i18n, then restores that absent path; no package is installed or system locale generated. A separate run sourcing those same files throughI18NPATHpasses 4/4 (0.55 s).LOCPATH, and retain test skip 77. Both branch and current-main skip gates exit 1. The missing-command control runs after successful generation and proves stale generated files do not mask unavailability. An explicit locale-directory override works even withlocaledefabsent; repeat automatic configuration succeeds. CTest JSON verifies exactly four HTTP tests andLOCPATHon only the locale test.locale -aremains C/C.utf8/POSIX..1252. The external CTest manifest preserves arguments/skip metadata. No write-watch switch or runtime workaround is added.f21ef7c99c5af648c5a41bddb18ce118b8e60c46. Local merge previews pass on later main82dfc54c7bba408caecaaf3ebc277b614779ef8aand preserve every owned patch payload; these previews are not later-main runtime validation. The initialfind_programform failed the system-dependency convention; it was replaced with direct host-command invocation and the final commit was revalidated.For old head
3eae, the maintainer-dispatched full Build above merged into7fea87746bde8e14a1d3341af85377c5b53f89ad: Linux built and ran CTest but failed the skip audit; the Windows job's build, runtime CTest and skip-audit steps succeeded. This is historical CI evidence for that head/merge. The new head needs a fresh maintainer dispatch. No local native Windows, full-project build, packaged runtime, title or PS5 validation is claimed. No current-head approval is inferred.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)