Repository navigation
fix(tools): report JobManager dependency audit failures - #11812
Conversation
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed head 5fa2c30 against current main. read_host_imports raises the dedicated AuditError for malformed/truncated ELF and PE data; adding that exception to the runner's existing CLI handler turns those cases into exit 2 while leaving unrelated exceptions uncaught. The catch still surrounds staging before the relinker is launched, and no label/comment-like side effect is involved. I extracted the exact PR blobs and their fixture dependencies, then ran the focused test suite under Python 3.11: 5 tests passed, 0 failures. The current-main merge tree is clean. Conventions, eligibility, intake, labels, ASan and Ubuntu relinker pass; product jobs are skipped and Windows/macOS relinker jobs are pending.
|
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.
|
MarcZX10
left a comment
MarcZX10
left a comment
There was a problem hiding this comment.
Follow-up after refreshing CI: the Windows relinker job fails the new job_reference_errors test (4/5 cases). Each failure is the assertion at test_job_reference_errors.py:54: str(artifact) uses an 8.3 temporary path such as C:\\Users\\RUNNER~1\\..., while the captured output contains the equivalent long path C:\\Users\\runneradmin\\.... The Linux suite passes, but this Windows path representation makes the required cross-platform test fail. Please compare the reported artifact path with the expected directory using filesystem identity (Path.samefile) or an equivalent Windows-aware normalization, then rerun the Windows relinker suite. This changes my prior approval; the production exception handling itself is not what this CI failure implicates.
5fa2c30 to
92b7921
Compare
|
Windows run of this head
AI-assisted: yes (Claude Code) |
THEMANnt
left a comment
THEMANnt
left a comment
There was a problem hiding this comment.
Reviewed 92b79217e12a3f2b417452b5024485057932b07e — approve. JobManager reference staging catches the existing import-audit exception and returns its normal diagnostic failure instead of exposing a traceback.
Windows/Python 3.11: 5/5 focused cases passed, including valid transitive staging and malformed PE/ELF inputs. The same current tests against old production produce two uncaught AuditError exceptions from the real import parser. No title execution is claimed.
The hosted Windows relinker job merged this exact head and completed successfully. Product jobs were skipped; no full product build is claimed.
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.
|
Checked that |
What
Treat
import_audit.AuditErroras a known dependency-audit failure in the JobManager reference runner. Malformed native dependencies follow the existing diagnostic/exit-2 path instead of escaping as an uncaught traceback. The explicitly named exception preserves unexpected programming errors.The Windows CI follow-up fixes the artifact-path assertion: the runner resolves the output path, while Windows temporary directories can use an 8.3 short spelling. Compare against
artifact.resolve()so the assertion checks the same canonical path. This follow-up changes only that assertion; production behavior and the other regression assertions are retained.Tested
Exact tested head:
92b79217e12a3f2b417452b5024485057932b07e, rebased onto upstream main801d5b9bc11049e6274daca09d70690bd5b8d0a9.Windows: Python 3.14.3, MinGW GCC 15.2.0, CMake/CTest 4.3.1, Ninja. WSL Ubuntu: Python 3.14.4, GCC 15.2.0, CMake/CTest 4.2.3, Ninja.
The original failure was Windows job 122091750463: four tests compared a
RUNNER~1temporary path against the canonicalrunneradminspelling printed by the runner.Real Windows short-path control: create temporary directories on the local Windows volume, obtain their actual 8.3 aliases through
GetShortPathNameW, and verify each alias refers to the same directory. Run the existing five tests with those aliases supplied as their temporary paths. The old assertion produces four failures and no errors; the final assertion passes all five tests with no skips. This uses actual filesystem aliases, not a mockedPath.resolve()result.Configured with
-G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=ON -DANYPS5_RELINKER_ONLY=ON, then ranctest --test-dir <build> -R '^(job_reference_errors|import_audit)$' --parallel 4 -V --output-junit <report>.import_audit: 154 unittest cases, 3 skips, zero failures; 12.55 s.job_reference_errors: 5 unittest cases, 0 skips, zero failures; 0.37 s.import_audit: 154 unittest cases, 2 skips, zero failures; 0.86 s.job_reference_errors: 5 unittest cases, 0 skips, zero failures; 0.19 s.Both registered CTest groups pass on both hosts. Platform-specific skips in the broader import-audit suite are not counted as executed passes; the JobManager suite has no skips. XML skip-policy checks pass. Each group finishes below 30 seconds.
The five JobManager tests exercise real
main -> stage_libraries -> read_host_importswith ELF/PE fixtures: transitive staging, truncated ELF, truncated PE, missing runtime and valid runtime. They still check diagnostics, exit statuses and the artifact directory. Only the native-launchrun()boundary is replaced by local staging; parsing and staging implementations are not copied or replaced. No original JobManager module is launched.Native relinker-only builds and
relinker --helppass on Windows and Linux. Current-main conventions, whitespace, test-script registration audit (203 scripts) and Python 3.9 grammar parsing pass. Grammar parsing is not execution on an older Python runtime.The original publication overlap scan and ten-PR integrations were recorded for the earlier head; they are not presented as final-head integration runs. This follow-up changes one assertion on the rebased PR and adds no test registration or production change beyond the original topic. No prerequisite PR is required.
New hosted CI is triggered for this head; the previous head's results do not validate it. Not run locally: native macOS, an older Python runtime, full system-library builds, game/PS5 or GPU execution. This establishes host dependency/error handling rather than JobManager scheduling semantics or crash durability. No performance improvement is claimed.
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)