[remove nixpkgs] part 2: generate flake.nix using builtins.fetchClosure #1209

savil · 2023-06-23T04:29:45Z

Summary

See #1208 for motivation.
That previous PR handled the "writing the system info to the lockfile". In this PR:

Generate the flake.nix from flake_remove_nixpkgs.nix.tmpl template.
- this new template uses builtins.fetchClosure to download the package's binary directly from the binary-cache-store (like cache.nixos.org). This lets us bypass nixpkgs for nix packages in the binary store.
The template data is shellgen.flakePlan.
In the flakePlan, we:
a. filter flakeInputs to be those not in the binary store (like remote or local flakes)
b. add a new field Packages []*nix.Package.

In nix.Package, we have a few new methods specific to the BinaryStore, like IsInBinaryStore and PathInBinaryStore.

Testing note:
I was stymied in extensively testing this by:

search index lacking x86_64-darwin system info (and I have an intel mac)
devbox cloud having limitations (will share details elsewhere)
the simple test plan below took me many hours to complete.

cc @mikeland73

How was it tested?

Tested with examples/development/go/hello-world.

For a more complex example, with a flake, the examples/flakes/remote produces this flake.nix:

{
   description = "A devbox shell";

   inputs = {
     nixpkgs.url = "http://lax.devbox-nixed.internal/nixos/nixpkgs/archive/f80ac848e3d6f0c12c52758c0f25c10c97ca3b62.tar.gz";
     gh-nixos-nixpkgs-5233fd2ba76a3accb5aaa999c00509a11fd0793c.url = "github:nixos/nixpkgs/5233fd2ba76a3accb5aaa999c00509a11fd0793c";
     gh-F1bonacc1-process-compose.url = "github:F1bonacc1/process-compose";
   };

   outputs = {
     self,
     nixpkgs,
     gh-nixos-nixpkgs-5233fd2ba76a3accb5aaa999c00509a11fd0793c,
     gh-F1bonacc1-process-compose,
   }:
       let
          pkgs = nixpkgs.legacyPackages.x86_64-linux;
       in
       {
         devShells.x86_64-linux.default = pkgs.mkShell {
           buildInputs = [
             gh-nixos-nixpkgs-5233fd2ba76a3accb5aaa999c00509a11fd0793c.legacyPackages.x86_64-linux.hello
             gh-nixos-nixpkgs-5233fd2ba76a3accb5aaa999c00509a11fd0793c.legacyPackages.x86_64-linux.cowsay
             gh-F1bonacc1-process-compose.packages.x86_64-linux.process-compose
           ];
         };
       };
 }

savil · 2023-06-23T04:29:56Z

Current dependencies on/for this PR:

main
- PR [remove nixpkgs] part 1: write info for nix store paths in lockfile #1208
  - PR [remove nixpkgs] part 2: generate flake.nix using builtins.fetchClosure #1209 👈
    - PR [remove nixpkgs] part 2.1: gauging progress by seeing how many testscripts pass/fail #1218

This comment was auto-generated by Graphite.

gcurtis · 2023-06-28T05:03:56Z

Sorry for the delay - I just remembered that I never reviewed this. I'll take a look tomorrow morning.

gcurtis

I had some comments, but approving to keep the train moving.

internal/lock/lockfile.go

gcurtis · 2023-06-28T15:55:10Z

internal/shellgen/flake_input.go

+	allInputs = lo.Filter(allInputs, func(item *nix.Package, _ int) bool {
+		// This is temporary, so that we can support the existing flake.nix.tmpl
+		// as well as the new flake_remove_nixpkgs.nix.tmpl.
+		return !featureflag.RemoveNixpkgs.Enabled() || item.SystemInfo() == nil


Won't this just remove everything when the feature is enabled? In which case allInputs could just be nil.

This will continue to include packages (like local or remote flakes) that don't have system info

internal/shellgen/flake_package.go

internal/shellgen/tmpl/flake_remove_nixpkgs.nix.tmpl

internal/shellgen/flake_plan.go

internal/shellgen/flake_package.go

gcurtis · 2023-06-28T16:46:45Z

internal/shellgen/flake_package.go

+		Name:      attributePath,
+		FromStore: nixosCacheURL,
+		FromPath:  filepath.Join("/nix/store", storeDir),
+		// TODO add ToPath:


Have you looked at the builtins.storePath function? If we're running in impure mode, then it might make things a little easier. It doesn't require a content-addressed hash and it will automatically use the Nix-configured binary caches (aka substituters). That'll make this work automatically with nixed in Devbox Cloud.

I haven't yet. Thanks for the pointer. Will look into it.

mikeland73

Not blocking, but I would really want to avoid adding a new Package struct. I also think system info should be part of resolve. Ideally system info can be abstracted away so that the whatever generates the flake can just get the data if it's present.

btw, you should assume that we will not be able to avoid nixpkgs for certain packages for the short and medium term, so we need to structure this in a clean way where both types can coexist.

internal/lock/lockfile.go

internal/nix/input.go

internal/shellgen/flake_package.go

internal/shellgen/flake_plan.go

internal/shellgen/tmpl/flake_remove_nixpkgs.nix.tmpl

savil · 2023-06-28T23:46:04Z

@gcurtis addressed comments and made updates

@mikeland73 Folded flakePackages into nix.Package. Addressed the concern around calling lockfile.Resolve in the getter for FromPath in nix.Package.

mikeland73

This is great! I really like how this ended up.

mikeland73 · 2023-06-29T18:14:11Z

internal/nix/input.go

+	pkg := &Package{
+		URL:                                 *pkgURL,
+		lockfile:                            locker,
+		Raw:                                 raw,
+		normalizedPackageAttributePathCache: "",
+	}
+
+	return pkg


nit, you can revert this if you want. You can also omit normalizedPackageAttributePathCache if you used name fields.

mikeland73 · 2023-06-29T18:15:13Z

internal/nix/input.go

+func (p *Package) IsInBinaryStore() bool {
+	return p.isVersioned()
+}


Is there a better way to determine this? For example only true if lock file data has system info?

🤔 yeah, we could add that check too. In my previous iteraiton of this PR, I was checking both p.isVersioned() and "has system info".

I can add that here. It feels weird to possibly make network-calls in an if-statement but I understand the architecture better now so lets roll with it.

mikeland73 · 2023-06-29T18:17:20Z

internal/shellgen/flake_plan.go

-	var err error
-	shellPlan.FlakeInputs, err = flakeInputs(ctx, devbox)
+	userPackages := devbox.PackagesAsInputs()
+	pluginPackages, err := devbox.PluginManager().PluginInputs(userPackages)


I'll rename PluginInputs and PackagesAsInputs in a follow up.

Also, I'm thinking of changing plugins to encapsulate it more. Having plugins show up in shellgen package is not idael

savil mentioned this pull request Jun 23, 2023

[remove nixpkgs] part 1: write info for nix store paths in lockfile #1208

Merged

savil force-pushed the savil/rm-nixpkgs-2-flake-dot-nix branch 4 times, most recently from eed5ad6 to 6f26857 Compare June 23, 2023 13:22

Base automatically changed from savil/rm-nixpkgs-1-lockfile to main June 23, 2023 14:30

savil force-pushed the savil/rm-nixpkgs-2-flake-dot-nix branch 5 times, most recently from ecbc74f to 163ddb4 Compare June 23, 2023 23:01

savil requested review from gcurtis and loreto June 23, 2023 23:22

savil marked this pull request as ready for review June 23, 2023 23:22

savil requested a review from mikeland73 June 23, 2023 23:35

savil mentioned this pull request Jun 26, 2023

[remove nixpkgs] part 2.1: gauging progress by seeing how many testscripts pass/fail #1218

Closed

savil force-pushed the savil/rm-nixpkgs-2-flake-dot-nix branch from 163ddb4 to 3df3c5c Compare June 28, 2023 13:41

gcurtis approved these changes Jun 28, 2023

View reviewed changes

mikeland73 reviewed Jun 28, 2023

View reviewed changes

savil added 2 commits June 28, 2023 15:02

[remove nixpkgs] part 2: generate flake.nix using builtins.fetchClosure

44e7ba2

Call resolve from Package FromPath getter; use nix.Package in template

755eff3

savil force-pushed the savil/rm-nixpkgs-2-flake-dot-nix branch from 3df3c5c to 755eff3 Compare June 28, 2023 22:48

savil added 4 commits June 28, 2023 23:08

minor fixes

ecf6a4f

flake template cleanup

d8024b0

cleanup

eb1ca3c

add comment

e434481

remove comment

fe3a4d1

savil added 2 commits June 28, 2023 23:52

use all packages in flake plan

cf68455

lint fix

e86c1f4

savil requested review from mikeland73 and gcurtis June 29, 2023 00:46

mikeland73 approved these changes Jun 29, 2023

View reviewed changes

savil added 2 commits June 29, 2023 18:49

Check lockfile.SystemInfo in IsInBinaryStore; minor cleanup

ac7be75

lint fix

ef0cd47

savil merged commit 18bbfe9 into main Jun 29, 2023

savil deleted the savil/rm-nixpkgs-2-flake-dot-nix branch June 29, 2023 19:30

[remove nixpkgs] part 2: generate flake.nix using builtins.fetchClosure #1209

[remove nixpkgs] part 2: generate flake.nix using builtins.fetchClosure #1209

Uh oh!

Conversation

savil commented Jun 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

How was it tested?

Uh oh!

savil commented Jun 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gcurtis commented Jun 28, 2023

Uh oh!

gcurtis left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikeland73 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

savil commented Jun 28, 2023

Uh oh!

mikeland73 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

savil commented Jun 23, 2023 •

edited

Loading

savil commented Jun 23, 2023 •

edited

Loading