Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: move lockfile ffi boundary #4629

Merged
merged 18 commits into from Apr 26, 2023
Merged

chore: move lockfile ffi boundary #4629

Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
8c99b57
populate workspace graph serially
chris-olszewski Apr 14, 2023
8d93b1e
move external deps population to method
chris-olszewski Apr 17, 2023
ab9f0b3
move lockfile parsing and traversal to final step
chris-olszewski Apr 17, 2023
f9a460c
calculate closures all at once
chris-olszewski Apr 17, 2023
4f9c395
calculate all lockfiles at once in rust
chris-olszewski Apr 18, 2023
31ef289
revert serial graph construction
chris-olszewski Apr 18, 2023
1c6d663
minor pr feedback changes
chris-olszewski Apr 21, 2023
92f0b58
switch to protobuf enum for package manager
chris-olszewski Apr 21, 2023
2efd867
remove clone in favor of borrow
chris-olszewski Apr 21, 2023
57be317
remove type hint
chris-olszewski Apr 21, 2023
0ff9dbc
move ffi conversion logic to standard traits
chris-olszewski Apr 21, 2023
1931953
Merge branch 'main' into olszewski/move_lockfile_ffi_boundary
chris-olszewski Apr 24, 2023
fbdeffd
Merge branch 'main' into olszewski/move_lockfile_ffi_boundary
chris-olszewski Apr 24, 2023
8c6c647
fix berry test
chris-olszewski Apr 24, 2023
b0f20e7
add tests for verifying external hash is empty string when no lockfil…
chris-olszewski Apr 24, 2023
d1c52da
add tests for ensuring empty sets are different than no lockfile
chris-olszewski Apr 24, 2023
fada352
fix go lint
chris-olszewski Apr 24, 2023
9049e5c
add go unit test on the other side of ffi
chris-olszewski Apr 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
64 changes: 23 additions & 41 deletions cli/internal/context/context.go
View file
Open in desktop
Expand Up @@ -17,7 +17,6 @@ import (
"github.com/vercel/turbo/cli/internal/workspace"

"github.com/Masterminds/semver"
mapset "github.com/deckarep/golang-set"
"github.com/pyr-sh/dag"
"golang.org/x/sync/errgroup"
)
Expand Down Expand Up @@ -179,12 +178,6 @@ func BuildPackageGraph(repoRoot turbopath.AbsoluteSystemPath, rootPackageJSON *f
}
c.PackageManager = packageManager

if lockfile, err := c.PackageManager.ReadLockfile(repoRoot, rootPackageJSON); err != nil {
warnings.append(err)
} else {
c.Lockfile = lockfile
}

if err := c.resolveWorkspaceRootDeps(rootPackageJSON, &warnings); err != nil {
// TODO(Gaspar) was this the intended return error?
return nil, fmt.Errorf("could not resolve workspaces: %w", err)
Expand Down Expand Up @@ -231,6 +224,29 @@ func BuildPackageGraph(repoRoot turbopath.AbsoluteSystemPath, rootPackageJSON *f
}
c.WorkspaceInfos.PackageJSONs[util.RootPkgName] = rootPackageJSON

if lockFile, err := c.PackageManager.ReadLockfile(repoRoot, rootPackageJSON); err != nil {
warnings.append(err)
rootPackageJSON.TransitiveDeps = []lockfile.Package{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work as just nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't done a full code trace, but it should. This was just copied from previous behavior.

Edit: Checked and nil works just fine here

rootPackageJSON.ExternalDepsHash = ""
} else {
c.Lockfile = lockFile
for _, pkg := range c.WorkspaceInfos.PackageJSONs {
depSet, err := lockfile.TransitiveClosure(
pkg.Dir.ToUnixPath(),
pkg.UnresolvedExternalDeps,
c.Lockfile,
)
if err != nil {
warnings.append(err)
continue
chris-olszewski marked this conversation as resolved.
Show resolved Hide resolved
}
if err := pkg.SetExternalDeps(depSet); err != nil {
return nil, err
}
}

}

return c, warnings.errorOrNil()
}

Expand All @@ -246,25 +262,6 @@ func (c *Context) resolveWorkspaceRootDeps(rootPackageJSON *fs.PackageJSON, warn
for dep, version := range pkg.Dependencies {
pkg.UnresolvedExternalDeps[dep] = version
}
if c.Lockfile != nil {
depSet, err := lockfile.TransitiveClosure(
pkg.Dir.ToUnixPath(),
pkg.UnresolvedExternalDeps,
c.Lockfile,
)
if err != nil {
warnings.append(err)
// Return early to skip using results of incomplete dep graph resolution
return nil
}
if err := pkg.SetExternalDeps(depSet); err != nil {
return err
}
} else {
pkg.TransitiveDeps = []lockfile.Package{}
pkg.ExternalDepsHash = ""
}

return nil
}

Expand Down Expand Up @@ -315,17 +312,6 @@ func (c *Context) populateWorkspaceGraphForPackageJSON(pkg *fs.PackageJSON, root
}
}

externalDeps, err := lockfile.TransitiveClosure(
pkg.Dir.ToUnixPath(),
pkg.UnresolvedExternalDeps,
c.Lockfile,
)
if err != nil {
warnings.append(err)
// reset external deps to original state
externalDeps = mapset.NewSet()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was this doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was here instead of proper control flow, if the closure calculation errored we would override the return value with an empty set. This would then get translated to pkg.TransitiveDeps = make([]lockfile.Package, 0) and nothing would get added.

}

// when there are no internal dependencies, we need to still add these leafs to the graph
if internalDepsSet.Len() == 0 {
c.WorkspaceGraph.Connect(dag.BasicEdge(pkg.Name, core.ROOT_NODE_NAME))
Expand All @@ -336,10 +322,6 @@ func (c *Context) populateWorkspaceGraphForPackageJSON(pkg *fs.PackageJSON, root
pkg.InternalDeps = append(pkg.InternalDeps, fmt.Sprintf("%v", v))
}

if err := pkg.SetExternalDeps(externalDeps); err != nil {
return err
}

sort.Strings(pkg.InternalDeps)

return nil
Expand Down