Skip to content

fix: obsolete comment and add test for double-escaping hostname in nix_eval_homebrew_attr #654

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Scott McMaster (scottmcmaster) merged 1 commit into from
Aug 27, 2026
Merged

fix: obsolete comment and add test for double-escaping hostname in nix_eval_homebrew_attr #654

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions apps/native/src-tauri/src/managed_edits/homebrew_adopt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,8 @@ const NIX_EVAL_HOMEBREW_APPLY: &str = r#"cfg: {
}"#;

fn nix_eval_homebrew_attr(hostname: &str) -> Result<String> {
// serde_json::to_string already wraps the hostname in quotes and escapes
// internals — do not also embed quote chars in the format string or we
// produce malformed attrs like .#darwinConfigurations.""host"".config…
// nix_string_literal returns a fully quoted Nix string literal, so the

@alex-sparus Alex Shabalin (alex-sparus) Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small nit on the wording: the attr path here isn't parsed as a Nix string literal. Nix's parseAttrPath (src/libexpr/attr-path.cc) understands "…" quoting but no backslash escapes:

} else if (*i == '"') {
    ++i;
    while (1) {
        if (i == s.end())
            throw ParseError("missing closing quote in selection path '%1%'", s);
        if (*i == '"')
            break;
        cur.push_back(*i++);
    }

So the \" cases in the test lock in output that nix rejects. Reproduced on nix 2.34.7 with a scratch flake that defines both attrs:

$ nix eval --json '.#darwinConfigurations."office\"mac".config.homebrew'
error: missing closing quote in selection path '...darwinConfigurations."office\"mac".config.homebrew'

$ nix eval --json '.#darwinConfigurations."\"quoted-host\"".config.homebrew'
error: flake ... does not provide attribute '...darwinConfigurations."\"quoted-host\"".config.homebrew'
       Did you mean "quoted-host"?

Nothing breaks in practice — scutil --get LocalHostName can't produce quotes or backslashes — so this is purely about the comment and test not asserting something nix can't parse. Would it work to say just "nix_string_literal already wraps the hostname in quotes, so the format string must not add another pair" and drop the two escape assertions?

// format string must not add another layer of quotes around the hostname.
let host_attr = nix_string_literal(hostname);
Ok(format!(
".#darwinConfigurations.{}.config.homebrew",
Expand Down Expand Up @@ -901,6 +900,12 @@ mod tests {
nix_eval_homebrew_attr(r#"office"mac"#).expect("attr should escape hostname"),
r#".#darwinConfigurations."office\"mac".config.homebrew"#
);

// Make sure we don't double-escape the hostname if it already has quotes.
assert_eq!(
nix_eval_homebrew_attr(r#""quoted-host""#).expect("attr should escape hostname"),
r#".#darwinConfigurations."\"quoted-host\"".config.homebrew"#
);
}

#[test]
Expand Down
Loading