devpkg: allow multiple/non-default package outputs #1630
Conversation
There was a problem hiding this comment.
Aside from adding the new feature, this is a really well done refactor. Nicely done!
This is relatively high risk so a few areas to test:
- Performance (when you have ~50 packages)
- Ensure runx works
- Hopefully most flake and package permutations are tested by our CICD, but we should smoke test a bit as well.
Update `devpkg.Package` to use `FlakeInstallable` instead of a URL so that it can correctly handle the flake outputs syntax. For example, `flake:nixpkgs#prometheus^out,cli`. Untangling parts of `devpkg` has been tricky, so this commit focuses on changing only the pieces necessary to replace `Package.URL` with `Package.installable`. There's still a lot more cleanup that can be done. Tested by adding/removing various flakes with and without outputs specified: ``` $ devbox add prometheus $ devbox run -- promtool --version promtool: command not found Error: error running script "promtool" in Devbox: exit status 127 $ devbox add flake:nixpkgs#prometheus $ devbox run -- promtool --version promtool: command not found Error: error running script "promtool" in Devbox: exit status 127 $ devbox add flake:nixpkgs#prometheus^out,cli $ devbox run -- promtool --version promtool, version 2.47.2 (branch: unknown, revision: unknown) build user: nix@nixpkgs build date: unknown go version: go1.21.3 platform: darwin/arm64 tags: builtinassets $ devbox add path:$HOME/src/tailscale/tailscale ```
gcurtis
force-pushed
the
gcurtis/flake-outputs
branch
from
f040251 to
0d63bb2
Compare
|
I've been testing out runx and various package combinations. I'll try doing a large project next for performance. |
|
I ran |
| @@ -30,8 +30,36 @@ import ( | |||
| // This Package will be aggregated into a specific "flake input" (see shellgen.flakeInput). | |||
| type Package struct { | |||
| plugins.BuiltIn | |||
| url.URL | |||
|
|
||
| // We currently don't lock flake references in devbox.lock, so there's | ||
| // nothing to resolve. | ||
| pkg.resolve = sync.OnceValue(func() error { return nil }) |
There was a problem hiding this comment.
I don't understand the comment here. It makes sense in isolation, but why refer to flake references in devbox.lock in this function? this function is used for all kinds of packages.
There was a problem hiding this comment.
We handle Devbox packages above starting on line 126. If the raw package name doesn't parse as a flake reference (err != nil) or it's a flake reference that could also be a Devbox package, then we assume it's a Devbox package and return. The code after assumes that raw is a valid flakeref.
Depends on #1627 and #1629.
Update
devpkg.Packageto useFlakeInstallableinstead of a URL so that it can correctly handle the flake outputs syntax. For example,flake:nixpkgs#prometheus^out,cli.Untangling parts of
devpkghas been tricky, so this commit focuses on changing only the pieces necessary to replacePackage.URLwithPackage.installable. There's still a lot more cleanup that can be done.Tested by adding/removing various flakes with and without outputs specified:
Related to #1431.