Repository navigation
Conversation
…ayBuildRequestEnabled Replace the NotImplemented placeholders with the offline option setters of sceHttpCacheRedirectedConnectionEnabled: BEFORE_INIT without a live context, INVALID_ID for an unknown object, 0 for a live template, connection or request. The unverified behaviour is recorded in the libSceHttp debt file.
MarcZX10
suggested changes
Nov 3, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
There was a problem hiding this comment.
Reviewed — changes requested
Both setters are success-only no-ops without the title-specific UI unblock required for a silent stub, and the test pins unmeasured value handling.
What I ran
Reviewed exact head d120593f744aeec02bd7ec03346862b12b56ee64 against base d1ae628bc2f62459cce1e9872ba5632c938c71f2. The merge preview against that base was clean. Environment: Debian Linux x86_64, Python 3.11.2, GCC 12.2.0. The author reports Linux x86_64/GCC 14.2 builds and focused tests; I did not independently run them.
python3 tools/pr_precheck.py --repo boykopovar/AnyPS5 --pr 15401
python3 tools/pr_abi_audit.py --base d1ae628bc2f62459cce1e9872ba5632c938c71f2 --head d120593f744aeec02bd7ec03346862b12b56ee64 --json
git diff --check d1ae628bc2f62459cce1e9872ba5632c938c71f2...d120593f744aeec02bd7ec03346862b12b56ee64
python3 tools/pr_test_quality.py --root /marc/AnyPS5 --json core/libs/tests/GuestHttpOptions.cpp
git grep -n -E 'guest_http_options|GuestHttpOptions.cpp' refs/review/15401/20261103 -- core/libs
gh pr checks 15401 --repo boykopovar/AnyPS5Results
| Check | Result | Note |
|---|---|---|
| Diff and conventions | passed | Exact-head checks passed. |
| Base merge preview | passed | Clean against the recorded base. |
| Export audit | passed | No declaration or definition mismatch found. |
| Test registration | passed | The existing guest_http_options target includes this test. |
| Test-quality heuristic | failed | It flagged registration; manual CMake inspection confirmed the test is registered. |
| Hosted checks | not run | All reported jobs were pending at the final refresh. |
| Build and CTest | not run | No local compilation or runtime tests. |
Precheck, local helper, registration and hosted-check output
The precheck's overall status was checks failed because its isolated PR worktree does not contain the optional local reviewer helper scripts. The ABI audit and test-quality heuristic were also run from the reviewer checkout as shown above.
# Precheck rows
| diff_check | passed | git diff --check d1ae628bc2f62459cce1e9872ba5632c938c71f2...d120593f744aeec02bd7ec03346862b12b56ee64 |
| conventions_and_pr_template | passed | All conventions checks passed. |
| changed_export_abi_audit | failed | /usr/bin/python3: can't open file '.../tools/pr_abi_audit.py': [Errno 2] No such file or directory |
| live_dependency_and_mergeability_check | passed | PR #15401: clean against current base; no declared dependencies. |
| test_quality_heuristics | failed | /usr/bin/python3: can't open file '.../tools/pr_test_quality.py': [Errno 2] No such file or directory |
# Local ABI audit summary
{
"status": "passed",
"findings": []
}
# Test-quality heuristic finding
{
"path": "core/libs/tests/GuestHttpOptions.cpp",
"line": 1,
"kind": "possible-unregistered-cpp-test",
"message": "Changed C++ test source path was not found literally in CMake files; inspect registration, generated source lists, and workflow coverage."
}
# Manual CMake registration
refs/review/15401/20261103:core/libs/prx/libSceHttp/CMakeLists.txt:42: add_test_executable(guest_http_options_tests ${LIBS_INCLUDE_DIR}/tests/GuestHttpOptions.cpp)
refs/review/15401/20261103:core/libs/prx/libSceHttp/CMakeLists.txt:46: add_test(NAME guest_http_options COMMAND guest_http_options_tests)
# Hosted checks at final refresh
conventions pending
eligibility pending
intake pending
labels pending
library-portability pending
report pending
Negative control
I did not run a negative control; this C++ test requires the compiled PR head. The pending hosted jobs had not supplied a result.
Findings
Blocking
sceHttpSetHttp09EnabledandsceHttpSetDelayBuildRequestEnableddiscardenableand return success for live objects. The PR says neither option changes behavior because no request reaches the network and declaresUnblocks: none. The repository permits a silent stub only when it unblocks a title and affects only its UI. Please leave these exports throwing while behavior is unknown, or provide the required title-specific evidence that makes these UI-only silent stubs eligible.GuestHttpOptions.cppasserts both setters accept-1and7, although the PR explicitly says PS5 behavior for values above1is unverified and notes a neighboring setter may reject them. Remove assertions for unmeasured values or provide PS5 evidence before pinning this behavior.
Non-blocking
None.
Sources
- Repository rules for function behavior and silent stubs
- The two no-op setters at the reviewed head
- Assertions for unmeasured values at the reviewed head
Not verified
I did not build or run CTest locally, test PS5 behavior, or run a negative control. The author-reported Linux build and tests were not independently reproduced. All hosted checks were pending at the final status refresh.
To get this to a good state
Keep unknown behavior throwing unless the silent-stub rule is satisfied, and remove or substantiate the -1/7 assertions. I will rerun the same no-build checks and review the exact new head after an update.
Contributor
Re-checked — corrected evidence formattingThe prior review's collapsed block abbreviated the precheck output and omitted part of the heuristic output; the findings and results remain unchanged, and the exact recorded outputs are below. What I ranI reran the local ABI summary and test-quality helper at head python3 tools/pr_abi_audit.py --base d1ae628bc2f62459cce1e9872ba5632c938c71f2 --head d120593f744aeec02bd7ec03346862b12b56ee64 --json | python3 -c 'import json,sys; d=json.load(sys.stdin); print(json.dumps({"status":d.get("status"),"findings":d.get("findings")},indent=2))'
python3 tools/pr_test_quality.py --root /marc/AnyPS5 --json core/libs/tests/GuestHttpOptions.cppResults
Exact recorded outputThe changes-requested findings in the review remain the same. |
Contributor
✅ AMD GPU test: passedPR head
Unstable on main (1, these also failed in recent main runs on this machine; not counted against this PR)Failing on main too (3, not counted against this PR)Validated but not compared (1, not in the main baseline's validation pass)Validation IDs already present on main (4 IDs in 460 tests)Skipped (7)ReproduceAutomated build and test on a contributor's AMD machine; it does not replace upstream CI. A new comment is posted for each new push. Full logs. |
Progress report (d1ae628 → d120593)📈 System libraries: 26.48% (+0.01%, +2 implemented functions) ✅ 2 implemented
Changed libraries (implemented exports)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Implements
sceHttpSetHttp09EnabledandsceHttpSetDelayBuildRequestEnabled(libSceHttp NIDspk0AuomQM1oand22buO-UufJY), which threwNotImplemented_nid_no_patch. They followsceHttpCacheRedirectedConnectionEnabledinsrc/Options.cpp:ERROR_BEFORE_INIT(0x80431001) while nosceHttpInitcontext is alive,ERROR_INVALID_ID(0x80431100) for an id that is not a live template, connection or request, otherwise 0. The value is accepted as given and not kept: no request reaches the network, so HTTP/0.9 response parsing and delayed request building change nothing, and no getter reads them back. The declarations stay as they were inHttp.hpp, with the parameter renamed toenable.The behaviour is not measured on a PS5. The signature
(int id, int enable)follows shadPS4, which only logs and returns 0; the order of the checks follows the neighbouring option calls, and whether the console refuses values above 1 assceHttpSetCookieEnableddoes is unverified.docs/dev/debt/libSceHttp.mdgets a silent-stub entry that states this. The libSceHttp debt file keeps no export count, so there is no count to change.guest_http_optionsis extended: both setters accept a template, a connection and a request with 0, 1, -1 and 7, refuse an unknown id and id 0 withERROR_INVALID_ID, and returnERROR_BEFORE_INITafter the lastsceHttpTerm.Tested
Linux x86_64, GCC 14.2 (g++ extracted from Debian trixie packages),
-DBUILD_TESTING=ON, full configure with all submodules. Hardware and titles: not tested.libSceHttpandguest_http_options_tests;ctestpassesguest_http,guest_http_response_headerandguest_http_options(the last in under 0.1 s);guest_http_uri_localeis skipped as onmain.core/libs/tests/test_public_headers.py --audit-onlypasses (one declaration per export).tools/check_conventions.py --base origin/mainandtools/check_test_registration.pypass.tools/pr_overlap.py --branch feat/http-option-settersfinds no PR with the same exports or new files. It reports textual overlap indocs/dev/debt/libSceHttp.mdwith feat(libSceHttp): add synchronous HTTP request transport #9834 and fix(http): trust the system CA list for HTTPS requests #11134 (the HTTP request transport), and could not compare that file with docs(debt): separate entries with blank lines and drop derived export totals #14212 (a docs-wide debt reformat that conflicts withmain); none of the three changes these two exports.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)