Skip to content

Commit 253d087

Browse files
fix(git): check destination emptiness without full readdir
Avoid reading an entire directory just to determine if clone destination is empty. Use ReadDir(1) to detect emptiness efficiently while preserving error behavior for non-empty and inaccessible paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: AdeshDeshmukh <adeshkd123@gmail.com>
1 parent 85bf690 commit 253d087

6 files changed

Lines changed: 44 additions & 12 deletions

File tree

pkg/cmd/inspect/inspect.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,14 @@ func getInspectInfo(ctx context.Context, repository oras.Target, ref string) (*i
7171
return nil, err
7272
}
7373
version := "unknown"
74-
if manifest.Annotations != nil && manifest.Annotations[constants.CliVersionAnnotation] != "" {
74+
if manifest.Annotations != nil {
7575
version = manifest.Annotations[constants.CliVersionAnnotation]
76+
if version == "" {
77+
version = manifest.Annotations[constants.LegacyCliVersionAnnotation]
78+
}
79+
if version == "" {
80+
version = "unknown"
81+
}
7682
}
7783
return &inspectInfo{
7884
Digest: desc.Digest,

pkg/lib/constants/consts.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,21 @@ const (
4949
DevModelsSubpath = "dev-models"
5050
CurrentDevModelSubpath = "current"
5151

52-
// Kitops-specific annotations for modelkit artifacts
53-
// TODO: update these to use the newer kitops.org domain
54-
CliVersionAnnotation = "ml.kitops.modelkit.cli-version"
55-
KitfileJsonAnnotation = "ml.kitops.modelkit.kitfile"
52+
// KitOps-specific annotations for ModelKit artifacts.
53+
// Canonical keys use the reverse-DNS form of kitops.org.
54+
CliVersionAnnotation = "org.kitops.modelkit.cli-version"
55+
KitfileJsonAnnotation = "org.kitops.modelkit.kitfile"
56+
57+
// Legacy annotation keys kept for backwards compatibility when reading/writing
58+
// manifests produced by older CLI versions.
59+
LegacyCliVersionAnnotation = "ml.kitops.modelkit.cli-version"
60+
LegacyKitfileJsonAnnotation = "ml.kitops.modelkit.kitfile"
5661

5762
// LayerSubtypeAnnotation stores additional type information for a given OCI manifest
5863
// layer within its annotations (e.g. storing prompts within code-type layers)
59-
LayerSubtypeAnnotation = "ml.kitops.modelkit.layer-subtype"
60-
LayerSubtypePrompt = "prompt"
64+
LayerSubtypeAnnotation = "org.kitops.modelkit.layer-subtype"
65+
LegacyLayerSubtypeAnnotation = "ml.kitops.modelkit.layer-subtype"
66+
LayerSubtypePrompt = "prompt"
6167

6268
// MaxModelRefChain is the maximum number of "parent" modelkits a modelkit may have
6369
// by e.g. referring to another modelkit in its .model.path

pkg/lib/external/git/clone.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package git
1919
import (
2020
"errors"
2121
"fmt"
22+
"io"
2223
"io/fs"
2324
"os"
2425
"os/exec"
@@ -113,13 +114,19 @@ func checkDestination(path string) error {
113114
if !stat.IsDir() {
114115
return fmt.Errorf("path %s exists and is not a directory", path)
115116
}
116-
// TODO: probably don't need to read the _whole_ directory
117-
contents, err := os.ReadDir(path)
117+
// Only check whether at least one entry exists; avoid reading full directory.
118+
dir, err := os.Open(path)
118119
if err != nil {
119120
return fmt.Errorf("failed to inspect directory %s: %w", path, err)
120121
}
121-
if len(contents) > 0 {
122+
defer dir.Close()
123+
124+
_, err = dir.ReadDir(1)
125+
if err == nil {
122126
return fmt.Errorf("cannot clone to a non-empty directory")
123127
}
128+
if !errors.Is(err, io.EOF) {
129+
return fmt.Errorf("failed to inspect directory %s: %w", path, err)
130+
}
124131
return nil
125132
}

pkg/lib/filesystem/local-storage.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ func SaveModel(ctx context.Context, localRepo local.LocalRepo, kitfile *artifact
7474
manifest.Annotations = map[string]string{}
7575
}
7676
manifest.Annotations[constants.KitfileJsonAnnotation] = string(kitfileBytes)
77+
manifest.Annotations[constants.LegacyKitfileJsonAnnotation] = string(kitfileBytes)
7778
}
7879

7980
manifestDesc, err := saveModelManifest(ctx, localRepo, manifest)
@@ -211,6 +212,7 @@ func saveKitfileLayers(ctx context.Context, localRepo local.LocalRepo, kitfile *
211212
layer.Annotations = map[string]string{}
212213
}
213214
layer.Annotations[constants.LayerSubtypeAnnotation] = constants.LayerSubtypePrompt
215+
layer.Annotations[constants.LegacyLayerSubtypeAnnotation] = constants.LayerSubtypePrompt
214216
layers = append(layers, layer)
215217
diffIDs = append(diffIDs, digest.FromString(layerInfo.DiffId))
216218
kitfile.Prompts[idx].LayerInfo = layerInfo
@@ -345,6 +347,7 @@ func createManifest(configDesc ocispec.Descriptor, layerDescs []ocispec.Descript
345347
manifest.Annotations = map[string]string{}
346348
}
347349
manifest.Annotations[constants.CliVersionAnnotation] = constants.Version
350+
manifest.Annotations[constants.LegacyCliVersionAnnotation] = constants.Version
348351

349352
return manifest, nil
350353
}

pkg/lib/filesystem/unpack/core.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,11 @@ func unpackRecursive(ctx context.Context, opts *UnpackOptions, visitedRefs []str
157157

158158
case mediatype.CodeBaseType:
159159
// Code-type layers may be either regular code or prompts
160-
if layerDesc.Annotations[constants.LayerSubtypeAnnotation] == constants.LayerSubtypePrompt {
160+
subtype := layerDesc.Annotations[constants.LayerSubtypeAnnotation]
161+
if subtype == "" {
162+
subtype = layerDesc.Annotations[constants.LegacyLayerSubtypeAnnotation]
163+
}
164+
if subtype == constants.LayerSubtypePrompt {
161165
entry := config.Prompts[promptIdx]
162166
promptIdx += 1
163167
if !kitfile.LayerMatchesAnyFilter(entry, opts.FilterConfs) {

pkg/lib/repo/util/reference.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,16 @@ func GetKitfileForManifest(ctx context.Context, store oras.ReadOnlyTarget, manif
8585
return GetConfig(ctx, store, manifest.Config)
8686
case mediatype.ModelPackFormat:
8787
// TODO: can we (try to) generate a Kitfile from a ModelPack manifest?
88-
if manifest.Annotations == nil || manifest.Annotations[constants.KitfileJsonAnnotation] == "" {
88+
if manifest.Annotations == nil {
8989
return nil, ErrNoKitfile
9090
}
9191
kfstring := manifest.Annotations[constants.KitfileJsonAnnotation]
92+
if kfstring == "" {
93+
kfstring = manifest.Annotations[constants.LegacyKitfileJsonAnnotation]
94+
}
95+
if kfstring == "" {
96+
return nil, ErrNoKitfile
97+
}
9298
kitfile := &artifact.KitFile{}
9399
if err := json.Unmarshal([]byte(kfstring), kitfile); err != nil {
94100
return nil, fmt.Errorf("failed to parse config: %w", err)

0 commit comments

Comments
 (0)