Skip to content

Commit b6b196b

Browse files
savilmikeland73
savil
and
mikeland73
authored
[refactor] move nix.Package to devpkg.Package (#1239)
## Summary This PR moves `nix.Package` to `devpkg` package. `devpkg.Package` depends on `nix` and `lock`, which is okay. I needed to introduce a small package called `devpkgutil` to resolve some gnarly cycles. - `lock/lockfile.go` calls these utilities. - `nix/search.go` calls these utilities. This `devpkgutil` is definitely a workaround, and I spent a lot of time trying to make alternate paths work: - tried refactoring `lockfile.Resolve` to not depend on `devpkg.ParseVersionedPackage` - tried refactoring `nix/search.go` to a `nixsearch` package (similar to `nixprofile`), but that wouldn't help in this case. Open to hear alternative solutions. UPDATE: - moved 2 of the 3 `devpkgutil` functions to `nix` package in `nix/nixpkgs.go` - one function remains: `ParseVersionedPackage`. Will tackle in follow up to remove `devpkgsutil` ## How was it tested? --------- Co-authored-by: Mike Landau <[email protected]>
1 parent 52d9fbd commit b6b196b

File tree

23 files changed

+117
-117
lines changed

23 files changed

+117
-117
lines changed

‎internal/devpkg/pkg.go‎ renamed to ‎internal/devpkg/devpkgutil/pkg.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright 2023 Jetpack Technologies Inc and contributors. All rights reserved.
22
// Use of this source code is governed by the license in the LICENSE file.
33

4-
package devpkg
4+
package devpkgutil
55

66
import (
77
"strings"

‎internal/nix/input.go‎ renamed to ‎internal/devpkg/package.go‎

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright 2023 Jetpack Technologies Inc and contributors. All rights reserved.
22
// Use of this source code is governed by the license in the LICENSE file.
33

4-
package nix
4+
package devpkg
55

66
import (
77
"crypto/md5"
@@ -18,6 +18,7 @@ import (
1818
"go.jetpack.io/devbox/internal/boxcli/usererr"
1919
"go.jetpack.io/devbox/internal/cuecfg"
2020
"go.jetpack.io/devbox/internal/lock"
21+
"go.jetpack.io/devbox/internal/nix"
2122
)
2223

2324
// Package represents a "package" added to the devbox.json config.
@@ -112,8 +113,8 @@ func (p *Package) FlakeInputName() string {
112113
result = filepath.Base(p.Path) + "-" + p.Hash()
113114
} else if p.isGithub() {
114115
result = "gh-" + strings.Join(strings.Split(p.Opaque, "/"), "-")
115-
} else if url := p.URLForFlakeInput(); IsGithubNixpkgsURL(url) {
116-
commitHash := HashFromNixPkgsURL(url)
116+
} else if url := p.URLForFlakeInput(); nix.IsGithubNixpkgsURL(url) {
117+
commitHash := nix.HashFromNixPkgsURL(url)
117118
if len(commitHash) > 6 {
118119
commitHash = commitHash[0:6]
119120
}
@@ -176,7 +177,7 @@ func (p *Package) NormalizedDevboxPackageReference() (string, error) {
176177
}
177178

178179
if path != "" {
179-
s, err := System()
180+
s, err := nix.System()
180181
if err != nil {
181182
return "", err
182183
}
@@ -252,7 +253,7 @@ func (p *Package) normalizePackageAttributePath() (string, error) {
252253

253254
// We prefer search over just trying to parse the URL because search will
254255
// guarantee that the package exists for the current system.
255-
infos := search(query)
256+
infos := nix.Search(query)
256257

257258
if len(infos) == 1 {
258259
return lo.Keys(infos)[0], nil
@@ -286,7 +287,7 @@ func (p *Package) normalizePackageAttributePath() (string, error) {
286287
)
287288
}
288289

289-
if pkgExistsForAnySystem(query) {
290+
if nix.PkgExistsForAnySystem(query) {
290291
return "", usererr.New(
291292
"Package \"%s\" was found, but we're unable to build it for your system."+
292293
" You may need to choose another version or write a custom flake.",
@@ -381,7 +382,7 @@ func (p *Package) EnsureNixpkgsPrefetched(w io.Writer) error {
381382
if hash == "" {
382383
return nil
383384
}
384-
return EnsureNixpkgsPrefetched(w, hash)
385+
return nix.EnsureNixpkgsPrefetched(w, hash)
385386
}
386387

387388
// version returns the version of the package
@@ -399,7 +400,7 @@ func (p *Package) isVersioned() bool {
399400
}
400401

401402
func (p *Package) HashFromNixPkgsURL() string {
402-
return HashFromNixPkgsURL(p.URLForFlakeInput())
403+
return nix.HashFromNixPkgsURL(p.URLForFlakeInput())
403404
}
404405

405406
// BinaryCacheStore is the store from which to fetch this package's binaries.
@@ -416,7 +417,7 @@ func (p *Package) IsInBinaryStore() (bool, error) {
416417
return false, err
417418
}
418419

419-
userSystem, err := System()
420+
userSystem, err := nix.System()
420421
if err != nil {
421422
return false, err
422423
}
@@ -444,7 +445,7 @@ func (p *Package) PathInBinaryStore() (string, error) {
444445
return "", err
445446
}
446447

447-
userSystem, err := System()
448+
userSystem, err := nix.System()
448449
if err != nil {
449450
return "", err
450451
}
@@ -457,25 +458,3 @@ func (p *Package) PathInBinaryStore() (string, error) {
457458
storeDir := strings.Join(storeDirParts, "-")
458459
return filepath.Join("/nix/store", storeDir), nil
459460
}
460-
461-
// IsGithubNixpkgsURL returns true if the package is a flake of the form:
462-
// github:NixOS/nixpkgs/...
463-
//
464-
// While there are many ways to specify this input, devbox always uses
465-
// github:NixOS/nixpkgs/<hash> as the URL. If the user wishes to reference nixpkgs
466-
// themselves, this function may not return true.
467-
func IsGithubNixpkgsURL(url string) bool {
468-
return strings.HasPrefix(url, "github:NixOS/nixpkgs/")
469-
}
470-
471-
var hashFromNixPkgsRegex = regexp.MustCompile(`github:NixOS/nixpkgs/([^#]+).*`)
472-
473-
// HashFromNixPkgsURL will (for example) return 5233fd2ba76a3accb5aaa999c00509a11fd0793c
474-
// from github:nixos/nixpkgs/5233fd2ba76a3accb5aaa999c00509a11fd0793c#hello
475-
func HashFromNixPkgsURL(url string) string {
476-
matches := hashFromNixPkgsRegex.FindStringSubmatch(url)
477-
if len(matches) == 2 {
478-
return matches[1]
479-
}
480-
return ""
481-
}

‎internal/nix/input_test.go‎ renamed to ‎internal/devpkg/package_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright 2023 Jetpack Technologies Inc and contributors. All rights reserved.
22
// Use of this source code is governed by the license in the LICENSE file.
33

4-
package nix
4+
package devpkg
55

66
import (
77
"fmt"
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/samber/lo"
1313
"go.jetpack.io/devbox/internal/lock"
14+
"go.jetpack.io/devbox/internal/nix"
1415
)
1516

1617
const nixCommitHash = "hsdafkhsdafhas"
@@ -167,7 +168,7 @@ func TestHashFromNixPkgsURL(t *testing.T) {
167168
}
168169

169170
for _, test := range tests {
170-
result := HashFromNixPkgsURL(test.url)
171+
result := nix.HashFromNixPkgsURL(test.url)
171172
if result != test.expected {
172173
t.Errorf(
173174
"Expected hash '%s' for URL '%s', but got '%s'",

‎internal/impl/devbox.go‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/pkg/errors"
2121
"github.com/samber/lo"
2222
"go.jetpack.io/devbox/internal/boxcli/featureflag"
23+
"go.jetpack.io/devbox/internal/devpkg"
2324
"go.jetpack.io/devbox/internal/impl/generate"
2425
"go.jetpack.io/devbox/internal/shellgen"
2526
"go.jetpack.io/devbox/internal/telemetry"
@@ -137,7 +138,7 @@ func (d *Devbox) Config() *devconfig.Config {
137138
}
138139

139140
func (d *Devbox) ConfigHash() (string, error) {
140-
hashes := lo.Map(d.PackagesAsInputs(), func(i *nix.Package, _ int) string { return i.Hash() })
141+
hashes := lo.Map(d.PackagesAsInputs(), func(i *devpkg.Package, _ int) string { return i.Hash() })
141142
h, err := d.cfg.Hash()
142143
if err != nil {
143144
return "", err
@@ -334,11 +335,19 @@ func (d *Devbox) Info(ctx context.Context, pkg string, markdown bool) error {
334335
ctx, task := trace.NewTask(ctx, "devboxInfo")
335336
defer task.End()
336337

337-
info := nix.PkgInfo(pkg, d.lockfile)
338-
if info == nil {
338+
locked, err := d.lockfile.Resolve(pkg)
339+
if err != nil {
340+
return err
341+
}
342+
343+
results := nix.Search(locked.Resolved)
344+
if len(results) == 0 {
339345
_, err := fmt.Fprintf(d.writer, "Package %s not found\n", pkg)
340346
return errors.WithStack(err)
341347
}
348+
349+
// we should only have one result
350+
info := lo.Values(results)[0]
342351
if _, err := fmt.Fprintf(
343352
d.writer,
344353
"%s%s\n",
@@ -349,7 +358,7 @@ func (d *Devbox) Info(ctx context.Context, pkg string, markdown bool) error {
349358
}
350359
return plugin.PrintReadme(
351360
ctx,
352-
nix.PackageFromString(pkg, d.lockfile),
361+
devpkg.PackageFromString(pkg, d.lockfile),
353362
d.projectDir,
354363
d.writer,
355364
markdown,
@@ -917,8 +926,8 @@ func (d *Devbox) Packages() []string {
917926
return d.cfg.Packages
918927
}
919928

920-
func (d *Devbox) PackagesAsInputs() []*nix.Package {
921-
return nix.PackageFromStrings(d.Packages(), d.lockfile)
929+
func (d *Devbox) PackagesAsInputs() []*devpkg.Package {
930+
return devpkg.PackageFromStrings(d.Packages(), d.lockfile)
922931
}
923932

924933
func (d *Devbox) HasDeprecatedPackages() bool {
@@ -933,7 +942,7 @@ func (d *Devbox) HasDeprecatedPackages() bool {
933942
func (d *Devbox) findPackageByName(name string) (string, error) {
934943
results := map[string]bool{}
935944
for _, pkg := range d.cfg.Packages {
936-
i := nix.PackageFromString(pkg, d.lockfile)
945+
i := devpkg.PackageFromString(pkg, d.lockfile)
937946
if i.String() == name || i.CanonicalName() == name {
938947
results[i.String()] = true
939948
}

‎internal/impl/packages.go‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/pkg/errors"
1717
"github.com/samber/lo"
18+
"go.jetpack.io/devbox/internal/devpkg"
1819
"go.jetpack.io/devbox/internal/nix/nixprofile"
1920
"go.jetpack.io/devbox/internal/shellgen"
2021
"golang.org/x/exp/slices"
@@ -39,8 +40,8 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames ...string) error {
3940

4041
// Only add packages that are not already in config. If same canonical exists,
4142
// replace it.
42-
pkgs := []*nix.Package{}
43-
for _, pkg := range nix.PackageFromStrings(lo.Uniq(pkgsNames), d.lockfile) {
43+
pkgs := []*devpkg.Package{}
44+
for _, pkg := range devpkg.PackageFromStrings(lo.Uniq(pkgsNames), d.lockfile) {
4445
versioned := pkg.Versioned()
4546

4647
// If exact versioned package is already in the config, skip.
@@ -58,7 +59,7 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames ...string) error {
5859
}
5960
}
6061

61-
pkgs = append(pkgs, nix.PackageFromString(versioned, d.lockfile))
62+
pkgs = append(pkgs, devpkg.PackageFromString(versioned, d.lockfile))
6263
d.cfg.Packages = append(d.cfg.Packages, versioned)
6364
}
6465

@@ -93,7 +94,7 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames ...string) error {
9394
}
9495

9596
if err := d.lockfile.Add(
96-
lo.Map(pkgs, func(pkg *nix.Package, _ int) string { return pkg.Raw })...,
97+
lo.Map(pkgs, func(pkg *devpkg.Package, _ int) string { return pkg.Raw })...,
9798
); err != nil {
9899
return err
99100
}
@@ -299,7 +300,7 @@ func (d *Devbox) removePackagesFromProfile(ctx context.Context, pkgs []string) e
299300
return err
300301
}
301302

302-
for _, input := range nix.PackageFromStrings(pkgs, d.lockfile) {
303+
for _, input := range devpkg.PackageFromStrings(pkgs, d.lockfile) {
303304
index, err := nixprofile.ProfileListIndex(&nixprofile.ProfileListIndexArgs{
304305
Lockfile: d.lockfile,
305306
Writer: d.writer,

‎internal/impl/update.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"fmt"
99

1010
"go.jetpack.io/devbox/internal/devpkg"
11+
"go.jetpack.io/devbox/internal/devpkg/devpkgutil"
1112
"go.jetpack.io/devbox/internal/nix"
1213
"go.jetpack.io/devbox/internal/nix/nixprofile"
1314
"go.jetpack.io/devbox/internal/searcher"
@@ -22,7 +23,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
2223
return err
2324
}
2425

25-
pendingPackagesToUpdate := []*nix.Package{}
26+
pendingPackagesToUpdate := []*devpkg.Package{}
2627
for _, pkg := range inputs {
2728
if pkg.IsLegacy() {
2829
fmt.Fprintf(d.writer, "Updating %s -> %s\n", pkg.Raw, pkg.LegacyToVersioned())
@@ -43,7 +44,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
4344
}
4445

4546
for _, pkg := range pendingPackagesToUpdate {
46-
if _, _, isVersioned := devpkg.ParseVersionedPackage(pkg.Raw); !isVersioned {
47+
if _, _, isVersioned := devpkgutil.ParseVersionedPackage(pkg.Raw); !isVersioned {
4748
if err = d.attemptToUpgradeFlake(pkg); err != nil {
4849
return err
4950
}
@@ -65,7 +66,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
6566
return nix.FlakeUpdate(shellgen.FlakePath(d))
6667
}
6768

68-
func (d *Devbox) inputsToUpdate(pkgs ...string) ([]*nix.Package, error) {
69+
func (d *Devbox) inputsToUpdate(pkgs ...string) ([]*devpkg.Package, error) {
6970
var pkgsToUpdate []string
7071
for _, pkg := range pkgs {
7172
found, err := d.findPackageByName(pkg)
@@ -78,12 +79,12 @@ func (d *Devbox) inputsToUpdate(pkgs ...string) ([]*nix.Package, error) {
7879
pkgsToUpdate = d.Packages()
7980
}
8081

81-
return nix.PackageFromStrings(pkgsToUpdate, d.lockfile), nil
82+
return devpkg.PackageFromStrings(pkgsToUpdate, d.lockfile), nil
8283
}
8384

8485
func (d *Devbox) updateDevboxPackage(
8586
ctx context.Context,
86-
pkg *nix.Package,
87+
pkg *devpkg.Package,
8788
) error {
8889
existing := d.lockfile.Packages[pkg.Raw]
8990
newEntry, err := searcher.Client().Resolve(pkg.Raw)
@@ -109,7 +110,7 @@ func (d *Devbox) updateDevboxPackage(
109110

110111
// attemptToUpgradeFlake attempts to upgrade a flake using `nix profile upgrade`
111112
// and prints an error if it fails, but does not propagate upgrade errors.
112-
func (d *Devbox) attemptToUpgradeFlake(pkg *nix.Package) error {
113+
func (d *Devbox) attemptToUpgradeFlake(pkg *devpkg.Package) error {
113114
profilePath, err := d.profilePath()
114115
if err != nil {
115116
return err

‎internal/lock/lockfile.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ import (
1212
"github.com/pkg/errors"
1313
"github.com/samber/lo"
1414
"go.jetpack.io/devbox/internal/boxcli/featureflag"
15+
"go.jetpack.io/devbox/internal/devpkg/devpkgutil"
1516

1617
"go.jetpack.io/devbox/internal/cuecfg"
17-
"go.jetpack.io/devbox/internal/devpkg"
1818
)
1919

2020
const lockFileVersion = "1"
@@ -100,7 +100,7 @@ func (l *File) Resolve(pkg string) (*Package, error) {
100100
if !hasEntry || entry.Resolved == "" || needsSysInfo {
101101
locked := &Package{}
102102
var err error
103-
if _, _, versioned := devpkg.ParseVersionedPackage(pkg); versioned {
103+
if _, _, versioned := devpkgutil.ParseVersionedPackage(pkg); versioned {
104104
locked, err = l.resolver.Resolve(pkg)
105105
if err != nil {
106106
return nil, err
@@ -160,7 +160,7 @@ func (l *File) LegacyNixpkgsPath(pkg string) string {
160160
// This probably belongs in input.go but can't add it there because it will
161161
// create a circular dependency. We could move Input into own package.
162162
func IsLegacyPackage(pkg string) bool {
163-
_, _, versioned := devpkg.ParseVersionedPackage(pkg)
163+
_, _, versioned := devpkgutil.ParseVersionedPackage(pkg)
164164
return !versioned &&
165165
!strings.Contains(pkg, ":") &&
166166
// We don't support absolute paths without "path:" prefix, but adding here

‎internal/nix/nix.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func ExperimentalFlags() []string {
120120
var cachedSystem string
121121

122122
func System() (string, error) {
123-
// For Savil to debug "remove nixpkgs" feature. The search api lacks x86-darwin info.
123+
// For Savil to debug "remove nixpkgs" feature. The Search api lacks x86-darwin info.
124124
// So, I need to fake that I am x86-linux and inspect the output in generated devbox.lock
125125
// and flake.nix files.
126126
override := os.Getenv("__DEVBOX_NIX_SYSTEM")

0 commit comments

Comments
 (0)