Preserve native import alias identity
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Import Discovery and Scheduling Review Findings
|
# Import Discovery and Scheduling Review Findings
|
||||||
|
|
||||||
Status: open; six independently reproduced findings require remediation.
|
Status: in progress; issue 1 complete, five findings remain.
|
||||||
|
|
||||||
Reviewed implementation: `aa93aed..15943e2`, ending at
|
Reviewed implementation: `aa93aed..15943e2`, ending at
|
||||||
`15943e233ca3cd1286c03c0e7085871db344cd4b`, against the accepted
|
`15943e233ca3cd1286c03c0e7085871db344cd4b`, against the accepted
|
||||||
@@ -14,7 +14,7 @@ and discovery edge cases, not a proposal to change the accepted architecture.
|
|||||||
Implement fixes with focused regressions, preserving the selected semantics
|
Implement fixes with focused regressions, preserving the selected semantics
|
||||||
and existing resource limits. This review made no implementation changes.
|
and existing resource limits. This review made no implementation changes.
|
||||||
|
|
||||||
## 1. [ ] P1 — Preserve native source identity and relative import bases
|
## 1. [x] P1 — Preserve native source identity and relative import bases
|
||||||
|
|
||||||
Owners: [directory selection](../../internal/engine/import_selection.go),
|
Owners: [directory selection](../../internal/engine/import_selection.go),
|
||||||
especially assignment of `target.identity` to a file candidate's `accessPath`
|
especially assignment of `target.identity` to a file candidate's `accessPath`
|
||||||
@@ -50,6 +50,13 @@ to native filesystems. Verify path, directory, and glob equivalence for a file
|
|||||||
alias and an alias directory, including nested imports, source context, origins,
|
alias and an alias directory, including nested imports, source context, origins,
|
||||||
and existing repeated-source behavior. Keep host canonicalization unchanged.
|
and existing repeated-source behavior. Keep host canonicalization unchanged.
|
||||||
|
|
||||||
|
Completed: native directory and glob discovery retain namespace admission paths,
|
||||||
|
while traversal cycle identities remain resolved. Added public regressions for
|
||||||
|
file and directory aliases across all three selectors, helper/resource origins,
|
||||||
|
source contexts, repeated paths, and distinct alias admission. The regression
|
||||||
|
failed before the fix. Focused discovery/resource tests and `./scripts/check.sh`
|
||||||
|
passed with Go 1.26.9 and offline dependency selection.
|
||||||
|
|
||||||
## 2. [ ] P1 — Scope main-version protection to the actual effective path
|
## 2. [ ] P1 — Scope main-version protection to the actual effective path
|
||||||
|
|
||||||
Owner: [composePendingItems](../../internal/engine/imports.go), around lines
|
Owner: [composePendingItems](../../internal/engine/imports.go), around lines
|
||||||
|
|||||||
99
facade_import_alias_test.go
Normal file
99
facade_import_alias_test.go
Normal file
@@ -0,0 +1,99 @@
|
|||||||
|
package configkit_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"io/fs"
|
||||||
|
"os"
|
||||||
|
"path"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
configkit "gitea.maximumdirect.net/eric/ConfigKit"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestFacadeNativeDiscoveryPreservesAliasIdentity(t *testing.T) {
|
||||||
|
root := t.TempDir()
|
||||||
|
for name, contents := range map[string]string{
|
||||||
|
"data/item.yml": "configkit:\n imports:\n - path: helper.yml\nvalue: item\n",
|
||||||
|
"data/helper.yml": "owner: target\nresource: resource.txt\n",
|
||||||
|
"aliases/helper.yml": "owner: alias\nresource: resource.txt\n",
|
||||||
|
"data/resource.txt": "target resource",
|
||||||
|
"aliases/resource.txt": "alias resource",
|
||||||
|
} {
|
||||||
|
filename := filepath.Join(root, filepath.FromSlash(name))
|
||||||
|
if err := os.MkdirAll(filepath.Dir(filename), 0o700); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filename, []byte(contents), 0o600); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
for name, target := range map[string]string{"aliases/item.yml": "../data/item.yml", "linked": "data"} {
|
||||||
|
if err := os.Symlink(target, filepath.Join(root, filepath.FromSlash(name))); err != nil {
|
||||||
|
t.Skipf("symlinks unavailable: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
filesystem := configkit.NewFilesystem("aliases", os.DirFS(root))
|
||||||
|
type configuration struct {
|
||||||
|
Value string `yaml:"value"`
|
||||||
|
Owner string `yaml:"owner"`
|
||||||
|
Resource string `yaml:"resource"`
|
||||||
|
}
|
||||||
|
for _, test := range []struct {
|
||||||
|
name, declaration, item, helper, owner, resource string
|
||||||
|
}{
|
||||||
|
{"file/path", "path: aliases/item.yml", "aliases/item.yml", "aliases/helper.yml", "alias", "alias resource"},
|
||||||
|
{"file/dir", "dir: aliases\n pattern: item.yml", "aliases/item.yml", "aliases/helper.yml", "alias", "alias resource"},
|
||||||
|
{"file/glob", "glob: 'aliases/ite*.yml'", "aliases/item.yml", "aliases/helper.yml", "alias", "alias resource"},
|
||||||
|
{"directory/path", "path: linked/item.yml", "linked/item.yml", "linked/helper.yml", "target", "target resource"},
|
||||||
|
{"directory/dir", "dir: linked\n pattern: item.yml", "linked/item.yml", "linked/helper.yml", "target", "target resource"},
|
||||||
|
{"directory/glob", "glob: 'lin*/ite*.yml'", "linked/item.yml", "linked/helper.yml", "target", "target resource"},
|
||||||
|
} {
|
||||||
|
t.Run(test.name, func(t *testing.T) {
|
||||||
|
result, err := configkit.Load(configkit.Bytes("main", []byte("configkit:\n imports:\n - "+test.declaration+"\n")), configkit.Options[configuration]{
|
||||||
|
Imports: true, Filesystem: &configkit.FilesystemContext{FS: &filesystem, Root: ".", BaseDir: "."},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if result.Config.Owner != test.owner || result.Config.Value != "item" {
|
||||||
|
t.Fatalf("configuration = %#v", result.Config)
|
||||||
|
}
|
||||||
|
sources := result.Sources()
|
||||||
|
if len(sources) != 3 || sources[1].Path != test.item || sources[2].Path != test.helper {
|
||||||
|
t.Fatalf("source identities = %#v", sources)
|
||||||
|
}
|
||||||
|
for field, source := range map[string]configkit.SourceInfo{"value": sources[1], "resource": sources[2]} {
|
||||||
|
info, ok := result.Metadata().Lookup(configkit.Keys(field))
|
||||||
|
if !ok || len(info.Origin.Locations) != 1 || info.Origin.Locations[0].SourceID != source.ID {
|
||||||
|
t.Fatalf("%s origin = %#v", field, info)
|
||||||
|
}
|
||||||
|
context, ok := result.SourceContext(source.ID)
|
||||||
|
if !ok || context.BaseDir != path.Dir(source.Path) {
|
||||||
|
t.Fatalf("%s context = %#v", field, context)
|
||||||
|
}
|
||||||
|
contents, err := fs.ReadFile(context.FS, path.Join(context.BaseDir, result.Config.Resource))
|
||||||
|
if err != nil || string(contents) != test.resource {
|
||||||
|
t.Fatalf("%s resource = %q, %v", field, contents, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
// The same native path is still a repeat when separately included. An alias
|
||||||
|
// to a leaf file remains a different native namespace path, as for path imports.
|
||||||
|
for _, selector := range []string{"path: aliases/item.yml", "dir: aliases\n pattern: item.yml", "glob: 'aliases/ite*.yml'"} {
|
||||||
|
_, err := configkit.Load(configkit.Bytes("main", []byte("configkit:\n imports:\n - "+selector+"\n - path: aliases/item.yml\n")), configkit.Options[configuration]{
|
||||||
|
Imports: true, Filesystem: &configkit.FilesystemContext{FS: &filesystem, Root: ".", BaseDir: "."},
|
||||||
|
})
|
||||||
|
var loadError *configkit.LoadError
|
||||||
|
if !errors.As(err, &loadError) || len(loadError.Diagnostics()) != 1 || loadError.Diagnostics()[0].Code != configkit.CodeImportRepeated {
|
||||||
|
t.Fatalf("repeated source: %v", err)
|
||||||
|
}
|
||||||
|
_, err = configkit.Load(configkit.Bytes("main", []byte("configkit:\n imports:\n - "+selector+"\n - path: data/item.yml\n")), configkit.Options[configuration]{
|
||||||
|
Imports: true, Filesystem: &configkit.FilesystemContext{FS: &filesystem, Root: ".", BaseDir: "."},
|
||||||
|
})
|
||||||
|
if !errors.As(err, &loadError) || len(loadError.Diagnostics()) == 0 || loadError.Diagnostics()[0].Code != configkit.CodeComposeConflict {
|
||||||
|
t.Fatalf("distinct native alias paths should reach composition: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -79,6 +79,11 @@ func selectGlobImportCandidates(access *filesystemAccess, basePath string, spec
|
|||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
physicalPath := target.identity
|
physicalPath := target.identity
|
||||||
|
// Native source identity and relative imports use the namespace
|
||||||
|
// spelling; resolved link identities belong only to discovery.
|
||||||
|
if access.native != nil {
|
||||||
|
physicalPath = accessPath
|
||||||
|
}
|
||||||
if physicalPath == "" {
|
if physicalPath == "" {
|
||||||
physicalPath, err = access.canonical(accessPath)
|
physicalPath, err = access.canonical(accessPath)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -115,7 +115,9 @@ func selectDirectoryImportCandidates(access *filesystemAccess, basePath string,
|
|||||||
if targetErr != nil {
|
if targetErr != nil {
|
||||||
return importSelection{}, []Diagnostic{*importDirectoryTargetDiagnostic(root.site, targetErr)}
|
return importSelection{}, []Diagnostic{*importDirectoryTargetDiagnostic(root.site, targetErr)}
|
||||||
}
|
}
|
||||||
root.accessPath = rootTarget.identity
|
if access.native == nil {
|
||||||
|
root.accessPath = rootTarget.identity
|
||||||
|
}
|
||||||
entries, missing, diagnostic := importDirectoryEntries(access, root.accessPath, root.site, limits, accounting)
|
entries, missing, diagnostic := importDirectoryEntries(access, root.accessPath, root.site, limits, accounting)
|
||||||
if diagnostic != nil {
|
if diagnostic != nil {
|
||||||
return importSelection{}, []Diagnostic{*diagnostic}
|
return importSelection{}, []Diagnostic{*diagnostic}
|
||||||
@@ -175,13 +177,15 @@ func selectDirectoryImportCandidates(access *filesystemAccess, basePath string,
|
|||||||
return importSelection{}, []Diagnostic{*childDiagnostic}
|
return importSelection{}, []Diagnostic{*childDiagnostic}
|
||||||
}
|
}
|
||||||
if !vanished {
|
if !vanished {
|
||||||
candidate.accessPath = target.identity
|
if access.native == nil {
|
||||||
|
candidate.accessPath = target.identity
|
||||||
|
}
|
||||||
activeDirectories[target.identity] = struct{}{}
|
activeDirectories[target.identity] = struct{}{}
|
||||||
stack = append(stack, importDirectoryFrame{directory: candidate, depth: frame.depth + 1, entries: childEntries, identity: target.identity})
|
stack = append(stack, importDirectoryFrame{directory: candidate, depth: frame.depth + 1, entries: childEntries, identity: target.identity})
|
||||||
}
|
}
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
if target.identity != "" {
|
if target.identity != "" && access.native == nil {
|
||||||
candidate.accessPath = target.identity
|
candidate.accessPath = target.identity
|
||||||
} else {
|
} else {
|
||||||
canonical, canonicalErr := access.canonical(accessPath)
|
canonical, canonicalErr := access.canonical(accessPath)
|
||||||
|
|||||||
Reference in New Issue
Block a user