From fe0eca6e7a5bbca4de176da9c340d137c5eff5df Mon Sep 17 00:00:00 2001 From: Matthew Huxtable Date: Tue, 28 Dec 2021 19:13:48 +0000 Subject: [PATCH] Correct path resolution to nested local dependencies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tool failed to properly resolve nested local dependencies to jsonnet bundles in different directory trees. This arises because the installation command resolves and installs nested jsonnet local dependencies relative to the root jsonnetfile, rather than track and evaluate the installation path relative to the nested library's jsonnetfile. Consider a repository with multiple local jsonnet bundles in various directory trees, organised as follows (lockfiles elided for brevity): /top/of/tree |- lib/module_A |- jsonnetfile.json |- lib/module_B |- jsonnetfile.json |- src/root_module |- jsonnetfile.json The modules depend on each other as follows: ┌───────────────────┐ ┌───────────────────┐ ┌───────────────────┐ │ │ │ │ │ │ │ src/root_module │──>│ lib/module_A │──>│ lib/module_B │ │ │ │ │ │ │ └───────────────────┘ └───────────────────┘ └───────────────────┘ where X ──> Y indicates bundle X depends on bundle Y, expressed by adding a dependency of type local in bundle X's jsonnetfile.json, whose path is the relative path from bundle X to bundle Y in the directory structure. For example, src/root_module will express a local dependency on path ../lib/module_A to depend on library module A. Invoking jb install in src/root_module will result in an error: jb: error: failed to install packages: downloading: symlink destination path does not exist: %w: stat /top/of/tree/src/module_B: no such file or directory This occurs because jsonnet-bundler improperly attempts to resolve the nested dependency on library module B relative to the root module path, i.e. src/root_module. The correct behaviour is to perform such resolution relative to the depending module's jsonnetfile.json, i.e. relative to lib/module_A. --- pkg/local.go | 8 ++++---- pkg/packages.go | 31 +++++++++++++++++++++++++------ 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/pkg/local.go b/pkg/local.go index 9dc839b..45ce8d3 100644 --- a/pkg/local.go +++ b/pkg/local.go @@ -38,7 +38,7 @@ func NewLocalPackage(source *deps.Local) Interface { func (p *LocalPackage) Install(ctx context.Context, name, dir, version string) (lockVersion string, err error) { wd, err := os.Getwd() if err != nil { - return "", errors.Wrap(err, "failed to get current working directory: %w") + return "", errors.Wrap(err, "failed to get current working directory") } oldname := filepath.Join(wd, p.Source.Directory) @@ -51,17 +51,17 @@ func (p *LocalPackage) Install(ctx context.Context, name, dir, version string) ( err = os.RemoveAll(newname) if err != nil { - return "", errors.Wrap(err, "failed to clean previous destination path: %w") + return "", errors.Wrap(err, "failed to clean previous destination path") } _, err = os.Stat(oldname) if os.IsNotExist(err) { - return "", errors.Wrap(err, "symlink destination path does not exist: %w") + return "", errors.Wrap(err, "symlink destination path does not exist") } err = os.Symlink(linkname, newname) if err != nil { - return "", errors.Wrap(err, "failed to create symlink for local dependency: %w") + return "", errors.Wrap(err, "failed to create symlink for local dependency") } color.Magenta("LOCAL %s -> %s", name, oldname) diff --git a/pkg/packages.go b/pkg/packages.go index 8f83965..ab14941 100644 --- a/pkg/packages.go +++ b/pkg/packages.go @@ -52,7 +52,7 @@ var ( func Ensure(direct v1.JsonnetFile, vendorDir string, oldLocks map[string]deps.Dependency) (map[string]deps.Dependency, error) { // ensure all required files are in vendor // This is the actual installation - locks, err := ensure(direct.Dependencies, vendorDir, oldLocks) + locks, err := ensure(direct.Dependencies, vendorDir, "", oldLocks) if err != nil { return nil, err } @@ -210,7 +210,7 @@ func known(deps map[string]deps.Dependency, p string) bool { return false } -func ensure(direct map[string]deps.Dependency, vendorDir string, locks map[string]deps.Dependency) (map[string]deps.Dependency, error) { +func ensure(direct map[string]deps.Dependency, vendorDir, pathToParentModule string, locks map[string]deps.Dependency) (map[string]deps.Dependency, error) { deps := make(map[string]deps.Dependency) for _, d := range direct { @@ -231,7 +231,7 @@ func ensure(direct map[string]deps.Dependency, vendorDir string, locks map[strin dir := filepath.Join(vendorDir, d.Name()) os.RemoveAll(dir) - locked, err := download(d, vendorDir) + locked, err := download(d, vendorDir, pathToParentModule) if err != nil { return nil, errors.Wrap(err, "downloading") } @@ -257,7 +257,12 @@ func ensure(direct map[string]deps.Dependency, vendorDir string, locks map[strin return nil, err } - nested, err := ensure(f.Dependencies, vendorDir, locks) + absolutePath, err := filepath.EvalSymlinks(filepath.Join(vendorDir, d.Name())) + if err != nil { + return nil, err + } + + nested, err := ensure(f.Dependencies, vendorDir, absolutePath, locks) if err != nil { return nil, err } @@ -274,13 +279,27 @@ func ensure(direct map[string]deps.Dependency, vendorDir string, locks map[strin // download retrieves a package from a remote upstream. The checksum of the // files is generated afterwards. -func download(d deps.Dependency, vendorDir string) (*deps.Dependency, error) { +func download(d deps.Dependency, vendorDir, pathToParentModule string) (*deps.Dependency, error) { var p Interface switch { case d.Source.GitSource != nil: p = NewGitPackage(d.Source.GitSource) case d.Source.LocalSource != nil: - p = NewLocalPackage(d.Source.LocalSource) + wd, err := os.Getwd() + if err != nil { + return nil, fmt.Errorf("failed to get current working directory: %w", err) + } + + // Resolve the relative path to the parent module. When a local + // dependency tree is resolved recursively, nested local dependencies + // with relative paths must be evaluated relative to their referencing + // jsonnetfile, rather than relative to the top-level jsonnetfile. + modulePath, err := filepath.Rel(wd, filepath.Join(pathToParentModule, d.Source.LocalSource.Directory)) + if err != nil { + modulePath = d.Source.LocalSource.Directory + } + + p = NewLocalPackage(&deps.Local{Directory: modulePath}) } if p == nil {