Skip to content

Commit 1dbcad2

Browse files
evslgcurtis
andauthored
nix/flake: installables exclude attribute path from URL (#1819)
I ran into an issue causing the generated `.devbox/gen/flake` to contain an invalid flake reference when adding some installables to `devbox.json`, specifically those which pointed to a remote resource. Local files were fine. After a quick hike around the repo, the fix seemed trivial enough to open up a PR rather than open up an issue. Interestingly, I didn't seem to find any issues around this even though this bug would've been around for some months. As it turns out, `Installable`s returned from `ParseInstallable` included the attribute path when provided a value with any scheme aside from `<empty string>`, `flake`, `path`, or `github`. This was because the `parseURLRef` method was not separating the fragment out of the refURL. More generically, the affected branches were any of those that set `parsed.URL` to the result of `refURL.String()`, since that specific method will re-add `Fragment` unless its empty. Additionally, the test for `ParseInstallable` with a flake was passing because the test case itself had an invalid flake reference. I corrected that, as well as adding a couple of relevant test cases for some other types as well. ## Repro steps 1. Add any non-local-path flake reference **with an Installable** to `devbox.json` e.g. `https://github.com/NixOS/patchelf/archive/master.tar.gz#patchelf` 2. Execute the following ```sh DEVBOX_DEBUG=1 devbox shell ``` 3. Observe the following `nix` error: ```sh Error: nix print-dev-env --json "path:/path/to/your/.devbox/gen/flake": exit status 1 2024/02/16 18:44:09 Command stderr: error: unexpected fragment 'patchelf' in flake reference 'tarball+https://github.com/NixOS/patchelf/archive/master.tar.gz#patchelf' ``` ## How was it tested? Updating the relevant test, then making adjustments to ensure they pass. --------- Co-authored-by: Greg Curtis <[email protected]>
1 parent caba63c commit 1dbcad2

File tree

2 files changed

+25
-7
lines changed

2 files changed

+25
-7
lines changed

nix/flake/flakeref.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ func ParseRef(ref string) (Ref, error) {
103103
parsed.Path = ref
104104
return parsed, nil
105105
}
106-
parsed, _, err := parseURLRef(ref)
106+
parsed, fragment, err := parseURLRef(ref)
107+
if fragment != "" {
108+
return Ref{}, redact.Errorf("flake reference %q contains a URL fragment", ref)
109+
}
107110
return parsed, err
108111
}
109112

@@ -116,6 +119,11 @@ func parseURLRef(ref string) (parsed Ref, fragment string, err error) {
116119
return Ref{}, "", redact.Errorf("parse flake reference as URL: %v", err)
117120
}
118121

122+
// ensure that the fragment is excluded from the parsed URL
123+
// since those are not valid in flake references.
124+
fragment = refURL.Fragment
125+
refURL.Fragment = ""
126+
119127
switch refURL.Scheme {
120128
case "", "flake":
121129
// [flake:]<flake-id>(/<rev-or-ref>(/rev)?)?
@@ -191,7 +199,7 @@ func parseURLRef(ref string) (parsed Ref, fragment string, err error) {
191199
default:
192200
return Ref{}, "", redact.Errorf("unsupported flake reference URL scheme: %s", redact.Safe(refURL.Scheme))
193201
}
194-
return parsed, refURL.Fragment, nil
202+
return parsed, fragment, nil
195203
}
196204

197205
func parseGitHubRef(refURL *url.URL, parsed *Ref) error {

nix/flake/flakeref_test.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,13 @@ func TestParseFlakeRefError(t *testing.T) {
189189
}
190190
}
191191
})
192+
t.Run("URLFragment", func(t *testing.T) {
193+
ref := "https://github.com/NixOS/patchelf/archive/master.tar.gz#patchelf"
194+
_, err := ParseRef(ref)
195+
if err == nil {
196+
t.Error("got nil error for flakeref with fragment:", ref)
197+
}
198+
})
192199
}
193200

194201
func TestFlakeRefString(t *testing.T) {
@@ -288,11 +295,14 @@ func TestParseFlakeInstallable(t *testing.T) {
288295
"nixpkgs#app^out,lib": {AttrPath: "app", Outputs: "lib,out", Ref: Ref{Type: TypeIndirect, ID: "nixpkgs"}},
289296
"nixpkgs^out": {Outputs: "out", Ref: Ref{Type: TypeIndirect, ID: "nixpkgs"}},
290297

291-
"%23#app": {AttrPath: "app", Ref: Ref{Type: TypeIndirect, ID: "#"}},
292-
"./%23#app": {AttrPath: "app", Ref: Ref{Type: TypePath, Path: "./#"}},
293-
"/%23#app": {AttrPath: "app", Ref: Ref{Type: TypePath, Path: "/#"}},
294-
"path:/%23#app": {AttrPath: "app", Ref: Ref{Type: TypePath, Path: "/#"}},
295-
"http://example.com/%23.tar#app": {AttrPath: "app", Ref: Ref{Type: TypeTarball, URL: "http://example.com/%23.tar#app"}},
298+
"%23#app": {AttrPath: "app", Ref: Ref{Type: TypeIndirect, ID: "#"}},
299+
"./%23#app": {AttrPath: "app", Ref: Ref{Type: TypePath, Path: "./#"}},
300+
"/%23#app": {AttrPath: "app", Ref: Ref{Type: TypePath, Path: "/#"}},
301+
"path:/%23#app": {AttrPath: "app", Ref: Ref{Type: TypePath, Path: "/#"}},
302+
303+
"http://example.com/%23.tar#app": {AttrPath: "app", Ref: Ref{Type: TypeTarball, URL: "http://example.com/%23.tar"}},
304+
"file:///flake#app": {AttrPath: "app", Ref: Ref{Type: TypeFile, URL: "file:///flake"}},
305+
"git://example.com/repo/flake#app": {AttrPath: "app", Ref: Ref{Type: TypeGit, URL: "git://example.com/repo/flake"}},
296306
}
297307

298308
for installable, want := range cases {

0 commit comments

Comments
 (0)