From 84ffb1eb0563a7dd4effb4b961409eaa874fa970 Mon Sep 17 00:00:00 2001 From: mitchell Date: Thu, 21 Aug 2025 11:04:23 -0400 Subject: [PATCH 1/5] Initial support for removing dynamically imported artifacts. --- internal/captain/values.go | 6 +-- pkg/runtime/depot.go | 28 ++++++++++--- pkg/runtime/ecosystem.go | 21 +++++----- pkg/runtime/ecosystem/dotnet.go | 13 +++++- pkg/runtime/ecosystem/golang.go | 62 ++++++++++++++++++++++++++--- pkg/runtime/ecosystem/java.go | 14 ++++++- pkg/runtime/ecosystem/javascript.go | 42 +++++++++++++------ pkg/runtime/ecosystem/rust.go | 13 +++++- pkg/runtime/setup.go | 40 ++++++++++++++----- 9 files changed, 183 insertions(+), 56 deletions(-) diff --git a/internal/captain/values.go b/internal/captain/values.go index 42c1acff0b..a58fe6e77f 100644 --- a/internal/captain/values.go +++ b/internal/captain/values.go @@ -155,9 +155,9 @@ func (p *PackageValue) Set(s string) error { } switch { case strings.Contains(s, ":"): - v := strings.Split(s, ":") - p.Namespace = strings.TrimSpace(strings.Join(v[0:len(v)-1], ":")) - p.Name = strings.TrimSpace(v[len(v)-1]) + namespace, name, _ := strings.Cut(s, ":") + p.Namespace = strings.TrimSpace(namespace) + p.Name = strings.TrimSpace(name) case strings.Contains(s, "/"): v := strings.Split(s, "/") p.Namespace = strings.TrimSpace(strings.Join(v[0:len(v)-1], "/")) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index a7804af918..7076c86750 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -21,6 +21,7 @@ import ( configMediator "github.com/ActiveState/cli/internal/mediators/config" "github.com/ActiveState/cli/internal/sliceutils" "github.com/ActiveState/cli/internal/smartlink" + "github.com/ActiveState/cli/pkg/buildplan" ) const ( @@ -52,6 +53,11 @@ type artifactInfo struct { Size int64 `json:"size"` LastAccessTime int64 `json:"lastAccessTime"` + // These fields are used by ecosystems during Add/Remove/Apply. + Namespace string `json:"namespace,omitempty"` + Name string `json:"name,omitempty"` + Version string `json:"version,omitempty"` + id strfmt.UUID // for convenience when removing stale artifacts; should NOT have json tag } @@ -218,7 +224,7 @@ func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) Path: absoluteDest, Files: files.RelativePaths(), RelativeSrc: relativeSrc, - }) + }, nil) if err != nil { return errs.Wrap(err, "Could not record artifact use") } @@ -275,7 +281,7 @@ func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) Path: absoluteDest, Files: files.RelativePaths(), RelativeSrc: relativeSrc, - }) + }, nil) if err != nil { return errs.Wrap(err, "Could not record artifact use") } @@ -286,7 +292,9 @@ func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) // Track will record an artifact deployment. // This is automatically called by `DeployVia*()` functions. // This should be called for ecosystems that handle installation of artifacts. -func (d *depot) Track(id strfmt.UUID, deploy *deployment) error { +// The artifact parameter is only necessary for tracking dynamically imported artifacts after being +// added by an ecosystem. +func (d *depot) Track(id strfmt.UUID, deploy *deployment, artifact *buildplan.Artifact) error { d.mapMutex.Lock() defer d.mapMutex.Unlock() @@ -308,6 +316,14 @@ func (d *depot) Track(id strfmt.UUID, deploy *deployment) error { } d.config.Cache[id].InUse = true d.config.Cache[id].LastAccessTime = time.Now().Unix() + + // For dynamically imported artifacts, also include artifact metadata. + if artifact != nil { + d.config.Cache[id].Namespace = artifact.Ingredients[0].Namespace + d.config.Cache[id].Name = artifact.Name() + d.config.Cache[id].Version = artifact.Version() + } + return nil } @@ -317,8 +333,8 @@ func (d *depot) Track(id strfmt.UUID, deploy *deployment) error { // This is automatically called by the `Undeploy()` function. // This should be called for ecosystems that handle uninstallation of artifacts. func (d *depot) Untrack(id strfmt.UUID, path string) { - if _, ok := d.config.Deployments[id]; ok { - d.config.Deployments[id] = sliceutils.Filter(d.config.Deployments[id], func(d deployment) bool { return d.Path != path }) + if deployments, ok := d.config.Deployments[id]; ok { + d.config.Deployments[id] = sliceutils.Filter(deployments, func(d deployment) bool { return d.Path != path }) } } @@ -445,7 +461,7 @@ func (d *depot) Save() error { for id := range d.artifacts { if deployments, ok := d.config.Deployments[id]; !ok || len(deployments) == 0 { if _, exists := d.config.Cache[id]; !exists { - err := d.Track(id, nil) // create cache entry for previously used artifact + err := d.Track(id, nil, nil) // create cache entry for previously used artifact if err != nil { return errs.Wrap(err, "Could not update depot cache with previously used artifact") } diff --git a/pkg/runtime/ecosystem.go b/pkg/runtime/ecosystem.go index 79ce43c5ee..29c0ab9a96 100644 --- a/pkg/runtime/ecosystem.go +++ b/pkg/runtime/ecosystem.go @@ -9,7 +9,7 @@ type ecosystem interface { Init(runtimePath string, buildplan *buildplan.BuildPlan) error Namespaces() []string Add(artifact *buildplan.Artifact, artifactSrcPath string) ([]string, error) - Remove(artifact *buildplan.Artifact) error + Remove(name, version string, installedFiles []string) error Apply() error } @@ -36,12 +36,10 @@ func artifactMatchesEcosystem(a *buildplan.Artifact, e ecosystem) bool { return false } -func namespacesMatchesEcosystem(namespaces []string, e ecosystem) bool { - for _, namespace := range e.Namespaces() { - for _, n := range namespaces { - if n == namespace { - return true - } +func namespaceMatchesEcosystem(namespace string, e ecosystem) bool { + for _, n := range e.Namespaces() { + if n == namespace { + return true } } return false @@ -56,12 +54,11 @@ func filterEcosystemMatchingArtifact(artifact *buildplan.Artifact, ecosystems [] return nil } -func filterEcosystemsMatchingNamespaces(ecosystems []ecosystem, namespaces []string) []ecosystem { - result := []ecosystem{} +func filterEcosystemMatchingNamespace(ecosystems []ecosystem, namespace string) ecosystem { for _, ecosystem := range ecosystems { - if namespacesMatchesEcosystem(namespaces, ecosystem) { - result = append(result, ecosystem) + if namespaceMatchesEcosystem(namespace, ecosystem) { + return ecosystem } } - return result + return nil } diff --git a/pkg/runtime/ecosystem/dotnet.go b/pkg/runtime/ecosystem/dotnet.go index ae4d017b58..a7f3499e41 100644 --- a/pkg/runtime/ecosystem/dotnet.go +++ b/pkg/runtime/ecosystem/dotnet.go @@ -105,8 +105,17 @@ func (e *DotNet) Add(artifact *buildplan.Artifact, artifactSrcPath string) ([]st return installedFiles, nil } -func (e *DotNet) Remove(artifact *buildplan.Artifact) error { - return nil // TODO: CP-956 +func (e *DotNet) Remove(name, version string, installedFiles []string) (rerr error) { + for _, dir := range installedFiles { + if !fileutils.DirExists(dir) { + continue + } + err := os.RemoveAll(dir) + if err != nil { + rerr = errs.Pack(rerr, errs.Wrap(err, "Unable to remove directory for '%s': %s", name, dir)) + } + } + return rerr } func (e *DotNet) Apply() error { diff --git a/pkg/runtime/ecosystem/golang.go b/pkg/runtime/ecosystem/golang.go index eff461a517..d6db88cc98 100644 --- a/pkg/runtime/ecosystem/golang.go +++ b/pkg/runtime/ecosystem/golang.go @@ -10,15 +10,17 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/fileutils" + "github.com/ActiveState/cli/internal/sliceutils" "github.com/ActiveState/cli/internal/smartlink" "github.com/ActiveState/cli/internal/unarchiver" "github.com/ActiveState/cli/pkg/buildplan" ) type Golang struct { - runtimeDir string - proxyDir string - addedModuleVersions map[string][]string + runtimeDir string + proxyDir string + addedModuleVersions map[string][]string + removedModuleVersions map[string][]string } func (e *Golang) Init(runtimePath string, buildplan *buildplan.BuildPlan) error { @@ -29,6 +31,7 @@ func (e *Golang) Init(runtimePath string, buildplan *buildplan.BuildPlan) error return errs.Wrap(err, "Unable to create Go proxy directory") } e.addedModuleVersions = make(map[string][]string) + e.removedModuleVersions = make(map[string][]string) return nil } @@ -135,8 +138,28 @@ func (e *Golang) Add(artifact *buildplan.Artifact, artifactSrcPath string) (_ [] return installedFiles, nil } -func (e *Golang) Remove(artifact *buildplan.Artifact) error { - return nil // TODO: CP-956 +// Remove a module's .mod and .zip files from the filesystem proxy. +func (e *Golang) Remove(name, version string, installedFiles []string) (rerr error) { + for _, proxyDir := range installedFiles { + modFile := filepath.Join(proxyDir, "@v", version+".mod") + if fileutils.TargetExists(modFile) { + err := os.Remove(modFile) + if err != nil { + rerr = errs.Pack(rerr, errs.Wrap(err, "Unable to remove mod file for '%s': %s", name, modFile)) + } + } + + zipFile := filepath.Join(proxyDir, "@v", version+".zip") + if fileutils.TargetExists(zipFile) { + err := os.Remove(zipFile) + if err != nil { + rerr = errs.Pack(rerr, errs.Wrap(err, "Unable to remove zip file for '%s': %s", name, zipFile)) + } + } + + e.removedModuleVersions[name] = append(e.removedModuleVersions[name], version) + } + return rerr } // Create/update each added module's version list file. @@ -165,5 +188,34 @@ func (e *Golang) Apply() error { return errs.Wrap(err, "Unable to write %s", listFile) } } + + for name, versions := range e.removedModuleVersions { + listFile := filepath.Join(e.runtimeDir, e.proxyDir, name, "@v", "list") + if !fileutils.FileExists(listFile) { + continue + } + + // Read known versions and remove the ones being removed. + contents, err := fileutils.ReadFile(listFile) + if err != nil { + return errs.Wrap(err, "Unable to read %s", listFile) + } + knownVersions := strings.Split(string(contents), "\n") + knownVersions = sliceutils.Filter(knownVersions, func(version string) bool { + for _, v := range versions { + if v == version { + return false + } + } + return true + }) + + // Write the remaining versions. + err = fileutils.WriteFile(listFile, []byte(strings.Join(knownVersions, "\n"))) + if err != nil { + return errs.Wrap(err, "Unable to write %s", listFile) + } + } + return nil } diff --git a/pkg/runtime/ecosystem/java.go b/pkg/runtime/ecosystem/java.go index 38e8e275f5..630062323e 100644 --- a/pkg/runtime/ecosystem/java.go +++ b/pkg/runtime/ecosystem/java.go @@ -2,6 +2,7 @@ package ecosystem import ( "encoding/json" + "os" "path/filepath" "strings" @@ -59,8 +60,17 @@ func (e *Java) Add(artifact *buildplan.Artifact, artifactSrcPath string) ([]stri return installedFiles, nil } -func (e *Java) Remove(artifact *buildplan.Artifact) error { - return nil // TODO: CP-956 +func (e *Java) Remove(name, version string, installedFiles []string) (rerr error) { + for _, file := range installedFiles { + if !fileutils.TargetExists(file) { + continue + } + err := os.Remove(file) + if err != nil { + rerr = errs.Pack(rerr, errs.Wrap(err, "Unable to remove installed file for '%s': %s", name, file)) + } + } + return rerr } func (e *Java) Apply() error { diff --git a/pkg/runtime/ecosystem/javascript.go b/pkg/runtime/ecosystem/javascript.go index fcd4883eaa..5d43a4fb0b 100644 --- a/pkg/runtime/ecosystem/javascript.go +++ b/pkg/runtime/ecosystem/javascript.go @@ -16,13 +16,14 @@ import ( const nodeModulesDir = "usr/lib/node_modules" type JavaScript struct { - runtimePath string - packages []string + runtimePath string + installPackages []string + uninstallPackages []string } func (e *JavaScript) Init(runtimePath string, buildplan *buildplan.BuildPlan) error { e.runtimePath = runtimePath - e.packages = []string{} + e.installPackages = []string{} return nil } @@ -56,33 +57,48 @@ func (e *JavaScript) Add(artifact *buildplan.Artifact, artifactSrcPath string) ( packageName = file.Name()[:i] } } - e.packages = append(e.packages, file.AbsolutePath()) + e.installPackages = append(e.installPackages, file.AbsolutePath()) } installedDir := filepath.Join(nodeModulesDir, packageName) // Apply() will install here return []string{installedDir}, nil } -func (e *JavaScript) Remove(artifact *buildplan.Artifact) error { - return nil // TODO: CP-956 +func (e *JavaScript) Remove(name, version string, installedFiles []string) error { + e.uninstallPackages = append(e.uninstallPackages, name) + return nil } func (e *JavaScript) Apply() error { - if len(e.packages) == 0 { + if len(e.installPackages) == 0 && len(e.uninstallPackages) == 0 { return nil // nothing to do } binDir := filepath.Join(e.runtimePath, "usr", "bin") - args := []string{"install", "-g", "--offline"} // do not install to current directory - for _, arg := range e.packages { - args = append(args, arg) + installArgs := []string{"install", "-g", "--offline"} // do not install to current directory + for _, arg := range e.installPackages { + installArgs = append(installArgs, arg) + } + uninstallArgs := []string{"uninstall", "-g", "--no-save"} // do not remove from current directory + for _, arg := range e.uninstallPackages { + uninstallArgs = append(uninstallArgs, arg) } env := []string{ fmt.Sprintf("PATH=%s%s%s", binDir, string(os.PathListSeparator), os.Getenv("PATH")), fmt.Sprintf("NPM_CONFIG_PREFIX=%s", filepath.Join(e.runtimePath, "usr")), } - _, stderr, err := osutils.ExecSimple(filepath.Join(binDir, "npm"), args, env) - if err != nil { - return errs.Wrap(err, "Error running npm: %s", stderr) + + if len(e.installPackages) > 0 { + _, stderr, err := osutils.ExecSimple(filepath.Join(binDir, "npm"), installArgs, env) + if err != nil { + return errs.Wrap(err, "Error running npm install: %s", stderr) + } + } + + if len(e.uninstallPackages) > 0 { + _, stderr, err := osutils.ExecSimple(filepath.Join(binDir, "npm"), uninstallArgs, env) + if err != nil { + return errs.Wrap(err, "Error running npm uninstall: %s", stderr) + } } return nil } diff --git a/pkg/runtime/ecosystem/rust.go b/pkg/runtime/ecosystem/rust.go index 73b1442ae2..bcd7ab37b3 100644 --- a/pkg/runtime/ecosystem/rust.go +++ b/pkg/runtime/ecosystem/rust.go @@ -92,8 +92,17 @@ func (e *Rust) Add(artifact *buildplan.Artifact, artifactSrcPath string) ([]stri return installedFiles, nil } -func (e *Rust) Remove(artifact *buildplan.Artifact) error { - return nil // TODO: CP-956 +func (e *Rust) Remove(name, version string, installedFiles []string) (rerr error) { + for _, dir := range installedFiles { + if !fileutils.DirExists(dir) { + continue + } + err := os.RemoveAll(dir) + if err != nil { + rerr = errs.Pack(rerr, errs.Wrap(err, "Unable to remove directory for '%s': %s", name, dir)) + } + } + return rerr } // configFileContents replaces the crates.io source with our vendored crates. diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index 98f32eb594..51919f4deb 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -484,11 +484,12 @@ func (s *setup) install(artifact *buildplan.Artifact) (rerr error) { if err != nil { return errs.Wrap(err, "Ecosystem unable to add artifact") } + s.depot.Track(id, &deployment{ Type: deploymentTypeEcosystem, Path: s.path, Files: files, - }) + }, artifact) return nil } @@ -534,16 +535,6 @@ func (s *setup) uninstall(id strfmt.UUID) (rerr error) { artifactDepotPath := s.depot.Path(id) - // TODO: CP-956 - //if ecosys := filterEcosystemMatchingArtifact(artifact, s.ecosystems); ecosys != nil { - // err := ecosys.Remove(artifact) - // if err != nil { - // return errs.Wrap(err, "Ecosystem unable to remove artifact") - // } - // s.depot.Untrack(id, filepath.Join(s.path, artifact.ArtifactID.String())) - // return nil - //} - envDef, err := s.env.Load(artifactDepotPath) if err != nil { return errs.Wrap(err, "Could not get env") @@ -553,6 +544,33 @@ func (s *setup) uninstall(id strfmt.UUID) (rerr error) { return errs.Wrap(err, "Could not unload artifact envdef") } + // If this is a dynamically imported artifact, tell the ecosystem to remove/undeploy it. + if artifact, exists := s.depot.config.Cache[id]; exists && artifact.Namespace != "" { + if ecosys := filterEcosystemMatchingNamespace(s.ecosystems, artifact.Namespace); ecosys != nil { + installedFiles := []string{} + // Find record of our deployment + if deployments, ok := s.depot.config.Deployments[id]; ok { + deployments = sliceutils.Filter(deployments, func(d deployment) bool { return d.Path == s.path }) + if len(deployments) > 0 { + installedFiles = deployments[0].Files + } + } + + // Convert relative install locations to absolute paths. + for i, file := range installedFiles { + installedFiles[i] = filepath.Join(s.path, file) + } + + // Remove/undeploy the artifact. + err := ecosys.Remove(artifact.Name, artifact.Version, installedFiles) + if err != nil { + return errs.Wrap(err, "Ecosystem unable to remove artifact") + } + s.depot.Untrack(id, filepath.Join(s.path, id.String())) + return nil + } + } + if err := s.depot.Undeploy(id, envDef.InstallDir, s.path); err != nil { return errs.Wrap(err, "Could not unlink artifact") } From 99c5bbbd54c38105ce88620e04a9d3f3a6152082 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 26 Aug 2025 15:39:30 -0400 Subject: [PATCH 2/5] Use public methods for depot access. --- pkg/runtime/depot.go | 25 +++++++++++++++++++------ pkg/runtime/setup.go | 13 ++++++------- 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index 7076c86750..df60c6c6d2 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -154,9 +154,15 @@ func (d *depot) SetCacheSize(mb int) { d.cacheSize = int64(mb) * MB } -func (d *depot) Exists(id strfmt.UUID) bool { - _, ok := d.artifacts[id] - return ok +// Note: the returned artifactInfo may be nil for depot artifacts that predate this functionality. +func (d *depot) Exists(id strfmt.UUID) (bool, *artifactInfo) { + if _, ok := d.artifacts[id]; ok { + if artifact, exists := d.config.Cache[id]; exists { + return true, artifact + } + return true, nil + } + return false, nil } func (d *depot) Path(id strfmt.UUID) string { @@ -184,7 +190,7 @@ func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) d.fsMutex.Lock() defer d.fsMutex.Unlock() - if !d.Exists(id) { + if exists, _ := d.Exists(id); !exists { return errs.New("artifact not found in depot") } @@ -237,7 +243,7 @@ func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) d.fsMutex.Lock() defer d.fsMutex.Unlock() - if !d.Exists(id) { + if exists, _ := d.Exists(id); !exists { return errs.New("artifact not found in depot") } @@ -327,6 +333,13 @@ func (d *depot) Track(id strfmt.UUID, deploy *deployment, artifact *buildplan.Ar return nil } +func (d *depot) Deployments(id strfmt.UUID) []deployment { + if deployments, ok := d.config.Deployments[id]; ok { + return deployments + } + return nil +} + // Untrack will remove an artifact deployment. // It does not actually delete files; it just tells the depot a previously tracked artifact should // no longer be tracked. @@ -342,7 +355,7 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error { d.fsMutex.Lock() defer d.fsMutex.Unlock() - if !d.Exists(id) { + if exists, _ := d.Exists(id); !exists { return errs.New("artifact not found in depot") } diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index 51919f4deb..a81591e9ed 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -139,7 +139,8 @@ func newSetup(path string, bp *buildplan.BuildPlan, env *envdef.Collection, depo // by definition we'll need to download it (unless we're setting up the runtime from an archive). // We also calculate which artifacts are immediately ready to be installed, as its the inverse condition of the above. artifactsToDownload := artifactsToInstall.Filter(func(a *buildplan.Artifact) bool { - return !depot.Exists(a.ArtifactID) + exists, _ := depot.Exists(a.ArtifactID) + return !exists }) artifactsToUnpack := artifactsToDownload if opts.FromArchive != nil { @@ -545,15 +546,13 @@ func (s *setup) uninstall(id strfmt.UUID) (rerr error) { } // If this is a dynamically imported artifact, tell the ecosystem to remove/undeploy it. - if artifact, exists := s.depot.config.Cache[id]; exists && artifact.Namespace != "" { + if exists, artifact := s.depot.Exists(id); exists && artifact != nil && artifact.Namespace != "" { if ecosys := filterEcosystemMatchingNamespace(s.ecosystems, artifact.Namespace); ecosys != nil { installedFiles := []string{} // Find record of our deployment - if deployments, ok := s.depot.config.Deployments[id]; ok { - deployments = sliceutils.Filter(deployments, func(d deployment) bool { return d.Path == s.path }) - if len(deployments) > 0 { - installedFiles = deployments[0].Files - } + deployments := sliceutils.Filter(s.depot.Deployments(id), func(d deployment) bool { return d.Path == s.path }) + if len(deployments) > 0 { + installedFiles = deployments[0].Files } // Convert relative install locations to absolute paths. From ae23900a3471c1f0ddf23854feb0184906a241e6 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 26 Aug 2025 16:28:19 -0400 Subject: [PATCH 3/5] Simplify depot Track function and make setup call it. --- pkg/runtime/depot.go | 66 ++++++++++++++++++-------------------------- pkg/runtime/setup.go | 18 +++++++++--- 2 files changed, 41 insertions(+), 43 deletions(-) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index df60c6c6d2..fc34fdee17 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -186,84 +186,78 @@ func (d *depot) Put(id strfmt.UUID) error { } // DeployViaLink will take an artifact from the depot and link it to the target path. -func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) error { +func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) (*deployment, error) { d.fsMutex.Lock() defer d.fsMutex.Unlock() if exists, _ := d.Exists(id); !exists { - return errs.New("artifact not found in depot") + return nil, errs.New("artifact not found in depot") } if err := d.validateVolume(absoluteDest); err != nil { - return errs.Wrap(err, "volume validation failed") + return nil, errs.Wrap(err, "volume validation failed") } // Collect artifact meta info var err error absoluteDest, err = fileutils.ResolvePath(absoluteDest) if err != nil { - return errs.Wrap(err, "failed to resolve path") + return nil, errs.Wrap(err, "failed to resolve path") } if err := fileutils.MkdirUnlessExists(absoluteDest); err != nil { - return errs.Wrap(err, "failed to create path") + return nil, errs.Wrap(err, "failed to create path") } absoluteSrc := filepath.Join(d.Path(id), relativeSrc) if !fileutils.DirExists(absoluteSrc) { - return errs.New("artifact src does not exist: %s", absoluteSrc) + return nil, errs.New("artifact src does not exist: %s", absoluteSrc) } // Copy or link the artifact files, depending on whether the artifact in question relies on file transformations if err := smartlink.LinkContents(absoluteSrc, absoluteDest); err != nil { - return errs.Wrap(err, "failed to link artifact") + return nil, errs.Wrap(err, "failed to link artifact") } files, err := fileutils.ListDir(absoluteSrc, false) if err != nil { - return errs.Wrap(err, "failed to list files") + return nil, errs.Wrap(err, "failed to list files") } - // Record deployment to config - err = d.Track(id, &deployment{ + return &deployment{ Type: deploymentTypeLink, Path: absoluteDest, Files: files.RelativePaths(), RelativeSrc: relativeSrc, - }, nil) - if err != nil { - return errs.Wrap(err, "Could not record artifact use") - } - - return nil + }, nil } // DeployViaCopy will take an artifact from the depot and copy it to the target path. -func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) error { +func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) (*deployment, error) { d.fsMutex.Lock() defer d.fsMutex.Unlock() if exists, _ := d.Exists(id); !exists { - return errs.New("artifact not found in depot") + return nil, errs.New("artifact not found in depot") } var err error absoluteDest, err = fileutils.ResolvePath(absoluteDest) if err != nil { - return errs.Wrap(err, "failed to resolve path") + return nil, errs.Wrap(err, "failed to resolve path") } if err := d.validateVolume(absoluteDest); err != nil { - return errs.Wrap(err, "volume validation failed") + return nil, errs.Wrap(err, "volume validation failed") } if err := fileutils.MkdirUnlessExists(absoluteDest); err != nil { - return errs.Wrap(err, "failed to create path") + return nil, errs.Wrap(err, "failed to create path") } absoluteSrc := filepath.Join(d.Path(id), relativeSrc) if !fileutils.DirExists(absoluteSrc) { - return errs.New("artifact src does not exist: %s", absoluteSrc) + return nil, errs.New("artifact src does not exist: %s", absoluteSrc) } // Copy or link the artifact files, depending on whether the artifact in question relies on file transformations @@ -272,38 +266,30 @@ func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) if errors.As(err, &errExist) { logging.Warning("Skipping files that already exist: " + errs.JoinMessage(errExist)) } else { - return errs.Wrap(err, "failed to copy artifact") + return nil, errs.Wrap(err, "failed to copy artifact") } } files, err := fileutils.ListDir(absoluteSrc, false) if err != nil { - return errs.Wrap(err, "failed to list files") + return nil, errs.Wrap(err, "failed to list files") } - // Record deployment to config - err = d.Track(id, &deployment{ + return &deployment{ Type: deploymentTypeCopy, Path: absoluteDest, Files: files.RelativePaths(), RelativeSrc: relativeSrc, - }, nil) - if err != nil { - return errs.Wrap(err, "Could not record artifact use") - } - - return nil + }, nil } // Track will record an artifact deployment. -// This is automatically called by `DeployVia*()` functions. -// This should be called for ecosystems that handle installation of artifacts. -// The artifact parameter is only necessary for tracking dynamically imported artifacts after being -// added by an ecosystem. -func (d *depot) Track(id strfmt.UUID, deploy *deployment, artifact *buildplan.Artifact) error { +func (d *depot) Track(artifact *buildplan.Artifact, deploy *deployment) error { d.mapMutex.Lock() defer d.mapMutex.Unlock() + id := artifact.ArtifactID + // Record deployment of this artifact. if _, ok := d.config.Deployments[id]; !ok { d.config.Deployments[id] = []deployment{} @@ -474,10 +460,12 @@ func (d *depot) Save() error { for id := range d.artifacts { if deployments, ok := d.config.Deployments[id]; !ok || len(deployments) == 0 { if _, exists := d.config.Cache[id]; !exists { - err := d.Track(id, nil, nil) // create cache entry for previously used artifact + // Create cache entry for previously used artifact. + size, err := fileutils.GetDirSize(d.Path(id)) if err != nil { - return errs.Wrap(err, "Could not update depot cache with previously used artifact") + return errs.Wrap(err, "Could not get artifact size on disk") } + d.config.Cache[id] = &artifactInfo{Size: size, id: id} } d.config.Cache[id].InUse = false logging.Debug("Artifact '%s' is no longer in use", id.String()) diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index a81591e9ed..dd7038ad2a 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -486,11 +486,14 @@ func (s *setup) install(artifact *buildplan.Artifact) (rerr error) { return errs.Wrap(err, "Ecosystem unable to add artifact") } - s.depot.Track(id, &deployment{ + err = s.depot.Track(artifact, &deployment{ Type: deploymentTypeEcosystem, Path: s.path, Files: files, - }, artifact) + }) + if err != nil { + return errs.Wrap(err, "Could not track deployment") + } return nil } @@ -499,8 +502,10 @@ func (s *setup) install(artifact *buildplan.Artifact) (rerr error) { return errs.Wrap(err, "Could not get env") } + var deploy *deployment if envDef.NeedsTransforms() || !s.supportsHardLinks || s.opts.Portable { - if err := s.depot.DeployViaCopy(id, envDef.InstallDir, s.path); err != nil { + deploy, err = s.depot.DeployViaCopy(id, envDef.InstallDir, s.path) + if err != nil { return errs.Wrap(err, "Could not deploy artifact via copy") } if envDef.NeedsTransforms() { @@ -509,10 +514,15 @@ func (s *setup) install(artifact *buildplan.Artifact) (rerr error) { } } } else { - if err := s.depot.DeployViaLink(id, envDef.InstallDir, s.path); err != nil { + deploy, err = s.depot.DeployViaLink(id, envDef.InstallDir, s.path) + if err != nil { return errs.Wrap(err, "Could not deploy artifact via link") } } + err = s.depot.Track(artifact, deploy) + if err != nil { + return errs.Wrap(err, "Could not track deployment") + } return nil } From 399873623215573c73af4988c6094778ddb34d25 Mon Sep 17 00:00:00 2001 From: mitchell Date: Tue, 26 Aug 2025 16:32:15 -0400 Subject: [PATCH 4/5] Use filesystem path equality, not string equality. --- pkg/runtime/depot.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index fc34fdee17..77cf158166 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -356,7 +356,10 @@ func (d *depot) Undeploy(id strfmt.UUID, relativeSrc, path string) error { if !ok { return errs.New("deployment for %s not found in depot", id) } - deployments = sliceutils.Filter(deployments, func(d deployment) bool { return d.Path == path }) + deployments = sliceutils.Filter(deployments, func(d deployment) bool { + equal, _ := fileutils.PathsEqual(d.Path, path) + return equal + }) if len(deployments) != 1 { return errs.New("no deployment found for %s in depot", path) } From 3e95cab7dd2725f250e7a2696def3f190bd73ed8 Mon Sep 17 00:00:00 2001 From: mitchell Date: Wed, 27 Aug 2025 14:06:09 -0400 Subject: [PATCH 5/5] Update comments. --- pkg/runtime/depot.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index 77cf158166..d32096d499 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -154,7 +154,11 @@ func (d *depot) SetCacheSize(mb int) { d.cacheSize = int64(mb) * MB } -// Note: the returned artifactInfo may be nil for depot artifacts that predate this functionality. +// Exists returns whether or not an artifact ID exists in the depot, along with any known metadata +// associated with that artifact ID. +// Existence is merely whether a directory with the name of the given ID exists on the filesystem. +// Artifact metadata comes from the depot's cache, and may not exist for installed artifacts that +// predate the cache. func (d *depot) Exists(id strfmt.UUID) (bool, *artifactInfo) { if _, ok := d.artifacts[id]; ok { if artifact, exists := d.config.Cache[id]; exists { @@ -186,6 +190,7 @@ func (d *depot) Put(id strfmt.UUID) error { } // DeployViaLink will take an artifact from the depot and link it to the target path. +// It should return deployment info to be used for tracking the artifact. func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) (*deployment, error) { d.fsMutex.Lock() defer d.fsMutex.Unlock() @@ -233,6 +238,7 @@ func (d *depot) DeployViaLink(id strfmt.UUID, relativeSrc, absoluteDest string) } // DeployViaCopy will take an artifact from the depot and copy it to the target path. +// It should return deployment info to be used for tracking the artifact. func (d *depot) DeployViaCopy(id strfmt.UUID, relativeSrc, absoluteDest string) (*deployment, error) { d.fsMutex.Lock() defer d.fsMutex.Unlock()