internal/devconfig: add PackageSpec for parsing package strings #1878
Conversation
Attempt to formalize raw Devbox package strings (hereby called package
specs) by introducing a new `PackageSpec` type.
This commit just adds the new parsing logic, it doesn't integrate it
with any other code yet. The `PackageSpec` type is the result of parsing
a raw string with `ParsePackageSpec`:
type PackageSpec struct {
Name, Version string
Installable flake.Installable
AttrPathInstallable flake.Installable
RunX types.PkgRef
}
func ParsePackageSpec(raw, nixpkgsCommit string) PackageSpec
When a package spec is ambiguous, `ParsePackageSpec` will populate the
`PackageSpec` fields with all possible interpretations. For example, the
spec `go` could be `go@latest`, a flake named `go`, or the attribute
path `nixpkgs#go`. It's up to a resolver to try them in some priority
order and pick one.
| // | Syntax | Example | Description | | ||
| // | ---------------- | --------------------------- | ------------------------------------ | | ||
| // | name@version | [email protected] | Resolved w/ search service | | ||
| // | name | go | Same as above; implies @latest | | ||
| // | flake | github:/nixos/nixpkgs#go | Matches Nix installable syntax | | ||
| // | attr path | go | Equivalent to flake:nixpkgs#go | | ||
| // | legacy attr path | go | Uses deprecated nixpkgs.commit field | | ||
| // | runx | runx:golangci/golangci-lint | Experimental | | ||
| // | ||
| // Most package specs are ambiguous, making it impossible to tell which exact | ||
| // syntax the user intended. PackageSpec parses as many as it can and leaves it | ||
| // up to the caller to prioritize and resolve them. |
There was a problem hiding this comment.
I think we should understand a string that has no version and is not a flake/runx to always mean attribute path. Thoughts?
There was a problem hiding this comment.
The tricky part is that almost any string is a valid flake reference due to indirect flakes. Still, I’d be onboard with saying we just don’t support scheme-less indirect flakes. So whereas Nix does:
foo → flake:foo
flake:foo → flake:foo
Devbox would do:
foo → flake:nixpkgs#foo
flake:foo → flake:foo
That still leaves ambiguity between Devbox packages and attribute paths though. We can't tell if nodePackages.@angular/cli is named nodePackages.@angular/cli or named nodePackages. with version angular/cli without querying the search service.
| // | Syntax | Example | Description | | ||
| // | ---------------- | --------------------------- | ------------------------------------ | | ||
| // | name@version | [email protected] | Resolved w/ search service | | ||
| // | name | go | Same as above; implies @latest | |
There was a problem hiding this comment.
This is not really valid. (or should not be). I was under impression that anything without a version is understood to mean:
nixpkgs/<hash>#attr
When someone goes devbox add go it adds go@latest to devbox.json.
If someone does devbox add stdenv.cc.cc.lib which is not in the index, then it should add stdenv.cc.cc.lib which represents nixpkgs/<hash>#stdenv.cc.cc.lib
If someone manually edits the devbox.json to add attribute paths without @version flag they get nixpkgs/<hash>#attr
There was a problem hiding this comment.
We also support nixpkgs attribute path via nixpkgs#hello
There was a problem hiding this comment.
Oh wow, I didn't realize that devbox add go and manually adding go to devbox.json did different things. I never would've guessed that {"packages": ["go"]} gives me an old version of Go, especially because there's no nixpkgs.commit field.
Could we align the CLI and config to both follow the CLI's behavior of checking for an @latest version? Having the same package string mean different things depending on if it's given on the command line vs. a config is pretty confusing IMO.
There was a problem hiding this comment.
I wonder how often that @latest user is accidentally mentioned on GitHub.
| // | name | go | Same as above; implies @latest | | ||
| // | flake | github:/nixos/nixpkgs#go | Matches Nix installable syntax | | ||
| // | attr path | go | Equivalent to flake:nixpkgs#go | | ||
| // | legacy attr path | go | Uses deprecated nixpkgs.commit field | |
There was a problem hiding this comment.
Is it deprecated? We removed it because we don't really want anyone to use it. I guess our preference would be for folks to use the hash in the package they want?
There was a problem hiding this comment.
Yeah, our docs say it's deprecated and will eventually be removed. I think pinning using a revision in a flake reference is clearer.
| // For example, the package spec "cachix" is a Devbox package (implied @latest), | ||
| // a flake ref (cachix), and an attribute path (nixpkgs#cachix). When resolving | ||
| // the package, Devbox must pick one (most likely cachix@latest). | ||
| type PackageSpec struct { |
There was a problem hiding this comment.
nit should this go in own file?
Attempt to formalize raw Devbox package strings (hereby called package specs) by introducing a new
PackageSpectype.This commit just adds the new parsing logic, it doesn't integrate it with any other code yet. The
PackageSpectype is the result of parsing a raw string withParsePackageSpec:When a package spec is ambiguous,
ParsePackageSpecwill populate thePackageSpecfields with all possible interpretations. For example, the specgocould bego@latest, a flake namedgo, or the attribute pathnixpkgs#go. It's up to a resolver to try them in some priority order and pick one.