[RFC] [refactor] Consolidate profile update code under syncPackagesToProfile

[savil]

Summary

This PR refactors code to reduce duplication of logic around nix profile updates.

Specifically:

• the implementation under syncPackagesToProfile is refactored so that we first prepare profileDir, and installablePackages, and then remove extra packages from profile, and add missing packages to profile. Preparing these variables upfront reduces much code duplication.
• in devbox.Remove, we now rely on ensurePackagesAreInstalled calling syncPackagesToProfile to remove the package from the profile. We can eliminate the extra removePackagesFromProfile function.
• in ensurePackagesAreInstalled, we now always execute it if mode is install or uninstall. We only check the lockfile status for ensure mode.

How was it tested?

Did some basic adding and removing of packages

[savil]

Current dependencies on/for this PR:

• main
  • PR [RFC] [refactor] Consolidate profile update code under syncPackagesToProfile #1529 👈

This comment was auto-generated by Graphite.

[savil]

My proposal for the next PR is:
Drop ensurePackagesAreInstalled from add, rm, update. Print a message to users to run devbox install to ensure their devbox environment is up-to-date.

In the future, we can also augment add, rm, update to only update the devbox environment with changes to the specified packages. In practice, we have to evaluate how feasible this is, because certain operations are applied holistically. For example:

1. nix print-dev-env applies on the entire generated flake.nix, which is sourced from devbox.json. So, if there's an unapplied change to the packages in devbox.json, I don't see how we'd avoid having that be included.
2. nix profile install has a priority field that relies on the order of packages in devbox.json

[mikeland73]

Much nicer!

[mikeland73]

Is this strictly true? for example adding a package that is already in devbox.json. Doesn't feel like a blocker though, I think it's OK

[savil]

hmm you are correct, its not strictly needed if the same package is being added or removed. We do run it, but its fairly fast (1 second), because its essentially a no-op, but not instant (0 seconds).

[mikeland73]

nit, can we replace continue outer with a found boolean?

for _, item := range profileItems {
	found := false
	for _, pkg := range packages {
		if item.Matches(pkg, d.lockfile) {
			itemsToKeep = append(itemsToKeep, item)
			found = true
                        break
		}
	}
	if !found {
		extras = append(extras, item)
	}
}

[savil]

haha yes, lets do that.