nigig-org/REVIEWS/PDF_PARITY_PHASE1_STATUS.md

Phase 1 status — fork sync + makepad_test enablement

Against NIGIG_PDF_FEATURE_PARITY_PLAN.md §1 Phase 1. Everything below was verified by running it, not by reading code.

Fork rev pinned at time of writing: 5efe6e24c.

Exit criteria

Criterion	State	Evidence
Workspace green on the new rev	✅	TEST_TARGET=pdf 606 passing
rev = bumped in pdf-makepad + map	✅	both already at 5efe6e24c
Baseline the gap list vs upstream libs/pdf_parse + pdf_view.rs	✅	§2 below
cargo test -p nigig-pdf-makepad --test ui on the desktop, no Studio	❌	blocked, see §3
Same suite on an Android device	❌	no device in this environment

Two of five criteria are outside what this sandbox can deliver. §3 states precisely where the wall is and what the fix looks like, so the fork work can be picked up without rediscovering it.

1. What changed since the last PDF session

The fork moved from a79f0dce to 5efe6e24c, and the six #[ignore = "needs a Makepad Studio hub"] markers were removed from pdf-makepad/tests/ui.rs. The module docs now claim the suite runs "without a Studio hub".

The markers were removed but the tests do not pass. They fail, which is a worse state than being ignored: TEST_TARGET=pdf-ui is red, so the gate that is supposed to protect the widget now fails for an environmental reason and will be tuned out.

That is not a criticism of the fork work — the failure mode genuinely did change, and it changed for the better (see §3). It just is not finished.

2. Baseline: upstream PDF code vs nigig-pdf

The plan asks for this so we can demonstrate nigig-pdf has superseded the upstream code.

	upstream libs/pdf_parse	nigig-pdf
Source	4,575 lines	25,845 lines
Writing a PDF	✗ none — no save, write or serialize entry point exists	✓ writer.rs, incremental.rs, save.rs
Encryption	✗	✓ RC4 40/128, AES-128, AES-256 (ADR 0005)
Signatures	✗	✓ read + byte-range integrity (ADR 0010)
Structure tree / PDF-UA	✗	✓ structure.rs (ADR 0011)
Transparency, ICC, functions	✗	✓ transparency.rs, icc.rs, function.rs (ADRs 0006, 0009)
Forms	partial	✓ edit, actions, validation (ADRs 0003, 0012)
Annotations	partial	✓ edit + save (ADR 0004)

widgets/src/pdf_view.rs is 1,161 lines and view-only: selection, zoom, page caching. pdf-makepad covers that plus form interaction, typed host actions and event routing.

Conclusion: nigig-pdf supersedes both. Nothing in libs/pdf_parse or pdf_view.rs is a capability we lack. This satisfies the plan's baseline requirement.

3. Why the UI suite cannot pass here — three layers, in order

Each was found by fixing the one in front of it. The first two are real bugs worth fixing in the fork; the third is a genuine environment limit.

Layer 1 — sh -lc loses cargo (exit 127)

studio/hub/src/build_manager.rs:398 takes a "direct stdio run" path that spawns the build through:

"/bin/sh".to_string(), vec!["-lc".to_string(), script]

-l makes it a login shell, which discards the inherited PATH and rebuilds it from /etc/profile. A rustup toolchain lives in ~/.cargo/bin, which is not on that path:

$ /bin/sh -c  'command -v cargo'   → /home/user/.cargo/bin/cargo
$ /bin/sh -lc 'command -v cargo'   → not found
$ /bin/sh -lc 'echo $PATH'         → /usr/local/bin:/usr/bin:/bin:...

So cargo is not found and the child exits 127 before any build starts — in 0.4 s, which is the tell.

This hits any CI or container using a rustup toolchain, not just this sandbox. Suggested fix in the fork: use sh -c rather than sh -lc, or resolve cargo to an absolute path (CARGO env var, which cargo sets for its children) instead of relying on the login shell's PATH.

Worked around here by symlinking cargo and rustc into /usr/local/bin.

Layer 2 — no headless backend exists on Linux (exit 101 → 139)

With cargo reachable the build runs (88 s) and the failure becomes 101.

libs/makepad_test/src/runtime.rs:112 sets MAKEPAD=headless for the child. But platform/src/os/linux/windowing_backend.rs only knows two backends:

pub enum WindowingProtocol { X11, Wayland }

There is no headless variant, and MAKEPAD=headless is not consulted in the selection. Running the app with it and no display:

Selected: X11 backend
Reason: Default fallback (no display variables set)
exit=139          ← SIGSEGV

It picks X11, finds no display, and segfaults. This is the substantive Phase 1 fork work: "terminal/standalone mode" as the plan calls it needs an actual headless backend in windowing_backend.rs, not just an env var the test harness sets.

Layer 3 — the sandbox has no display

Under xvfb-run the app starts properly and OpenGL initialises:

DISPLAY: :99 → Selected: X11 backend
Makepad GL: vendor="Mesa" renderer="llvmpipe (LLVM 19.1.7)" ...

The binary is fine. But the hub spawns its own child process outside the xvfb-run wrapper's inherited display in a way I could not thread through, so the suite still reports 101. Past layer 2 this stops mattering, because a real headless backend needs no display at all.

4. Verified state

TEST_TARGET=pdf     606 passing   ✅
TEST_TARGET=pdf-ui  6 failing     ❌  (environmental, layers 1–3)

One genuine improvement worth recording: pdf-makepad now builds in release inside the workspace. That was previously blocked by a Makepad os::linux feature-unification bug and is fixed on this rev (cargo build --release -p nigig-pdf-makepad succeeds in 1 m 54 s).

5. Recommendation

Phase 1 is not complete, and the remaining work is in the Makepad fork, not in nigig-org:

1. Fix sh -lc → sh -c in build_manager.rs (small, unblocks every rustup-based CI).
2. Add a real headless backend to windowing_backend.rs honouring MAKEPAD=headless.
3. Then Android mode, which the plan already scopes.

Until (2) exists, pdf-makepad/tests/ui.rs cannot pass anywhere without a display. The six tests should be #[ignore]d again with a reason pointing at this document rather than left failing — a red suite that everyone knows to disregard is worse than an explicitly skipped one, because it hides the next real regression.