Don't add the wasm/wasi builtins to the global lexical scope

[edolstra]

Motivation

This wasn't intended anyway. Adding them to the global scope breaks Nixpkgs.

Context

Summary by CodeRabbit

• Chores
  • Renamed internal WASM/WASI operation identifiers to use double-underscore prefixes for naming consistency.
• Bug Fixes / Behavior
  • Provenance URL handling adjusted: some provenance entries now store raw URL strings (and constructors allow optional sanitization), affecting how URLs appear in provenance records.

[coderabbitai]

📝 Walkthrough

Walkthrough

Renames two WASM primitive registration identifiers ("wasm" → "__wasm", "wasi" → "__wasi") and changes FetchurlProvenance API and call sites: constructor signature now FetchurlProvenance(std::string url, bool sanitize = true), updated implementations and call sites (some now pass false), and several call sites switched from sanitized/parsed URIs to plain to_string().

Changes

Cohort / File(s)	Summary
WASM primitive registration src/libexpr/primops/wasm.cc	Registration names changed: primop_wasm now registers as "__wasm" and primop_wasi as "__wasi". No logic or binding changes.
FetchurlProvenance API & impl src/libfetchers/include/nix/fetchers/provenance.hh, src/libfetchers/provenance.cc	Constructor signature changed from FetchurlProvenance(const std::string&) to FetchurlProvenance(std::string url, bool sanitize = true); implementation added sanitization conditional and updated factory/registration call to pass false where appropriate.
Provenance call sites / URL representation src/libfetchers/tarball.cc, src/nix/prefetch.cc	Provenance construction now uses request.uri.to_string() or url.to_string() instead of parsed().renderSanitized(); provenance object is constructed with the updated constructor where provenance-collection code applies.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 I hopped through code with nimble paws,

renamed two friends with tiny flaws.
URLs now choose their plain attire,
provenance learns when to sanitize.
Two underscores, a quiet cheer — hop! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title describes the main change: renaming wasm/wasi builtins from 'wasm'/'wasi' to '__wasm'/'__wasi' to prevent them from being added to the global lexical scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch __wasm

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/libexpr/primops/wasm.cc (1)

571-578: ⚠️ Potential issue | 🟡 Minor

Align user-facing error text with the new builtin name.

The error string in prim_wasm still says builtins.wasm, which will be misleading after this rename. Update it to builtins.__wasm for consistency (Line 547).

🛠️ Proposed fix

-        std::string(state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument of `builtins.wasm`"));
+        std::string(state.forceStringNoCtx(*args[1], pos, "while evaluating the second argument of `builtins.__wasm`"));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/libexpr/primops/wasm.cc` around lines 571 - 578, The user-facing error
message inside the prim_wasm implementation still references "builtins.wasm";
update that error text to "builtins.__wasm" to match the renamed builtin
declared in the RegisterPrimOp (named "__wasm") so runtime errors report the
correct builtin name; locate prim_wasm in the same file
(src/libexpr/primops/wasm.cc) and change the error string literal accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/libexpr/primops/wasm.cc`:
- Around line 571-578: The user-facing error message inside the prim_wasm
implementation still references "builtins.wasm"; update that error text to
"builtins.__wasm" to match the renamed builtin declared in the RegisterPrimOp
(named "__wasm") so runtime errors report the correct builtin name; locate
prim_wasm in the same file (src/libexpr/primops/wasm.cc) and change the error
string literal accordingly.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 358fb43 and 3dd9d38.

📒 Files selected for processing (1)

• src/libexpr/primops/wasm.cc

[github-actions]

🚀 Deployed on https://699cd430b869fda56ecc9500--determinate-nix.netlify.app

[coderabbitai]

🧹 Nitpick comments (1)

src/libfetchers/provenance.cc (1)

37-41: Consider a debug-level log in the BadURL fallback path.

The silent catch (BadURL &) is intentional, but it makes it impossible to tell at runtime whether provenance stored a sanitized URL or a verbatim fallback. A debug(...) message (e.g., debug("FetchurlProvenance: could not sanitize URL '%s', storing verbatim", url)) inside the catch block would improve observability with no behavioral impact.

♻️ Suggested debug log in catch block

     if (sanitize) {
         try {
             url = parseURL(url, true).renderSanitized();
         } catch (BadURL &) {
+            debug("FetchurlProvenance: URL '%s' could not be sanitized, storing verbatim", url);
         }
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/libfetchers/provenance.cc` around lines 37 - 41, The silent catch of
BadURL in provenance.cc hides whether parseURL(url, true).renderSanitized()
failed; add a debug log inside the catch to record the failure and the original
URL for observability. Modify the catch (BadURL &) block next to parseURL and
renderSanitized to call the project logger's debug (e.g.,
debug("FetchurlProvenance: could not sanitize URL '%s', storing verbatim", url))
or the equivalent debug logging function used in this file; do not change
behavior or rethrow, only emit the debug message.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/libfetchers/provenance.cc`:
- Around line 37-41: The silent catch of BadURL in provenance.cc hides whether
parseURL(url, true).renderSanitized() failed; add a debug log inside the catch
to record the failure and the original URL for observability. Modify the catch
(BadURL &) block next to parseURL and renderSanitized to call the project
logger's debug (e.g., debug("FetchurlProvenance: could not sanitize URL '%s',
storing verbatim", url)) or the equivalent debug logging function used in this
file; do not change behavior or rethrow, only emit the debug message.

---

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3dd9d38 and d49a2ee.

📒 Files selected for processing (4)

• src/libfetchers/include/nix/fetchers/provenance.hh
• src/libfetchers/provenance.cc
• src/libfetchers/tarball.cc
• src/nix/prefetch.cc