chore(lock): read the lock as written; drop the pre-release spellings

changes/unreleased/lock-canonical-only.removed.md

@@ -0,0 +1 @@
+- A `.project.lock.json` is read exactly as this tool writes it: a repository `resource` in a non-canonical spelling (`pkg:git/…`, a bare `<vcs>+<transport>://…`) or a `usage` given as a bare string is refused with the canonical form, instead of being translated as pre-release locks were.

docs/packages.md

@@ -130,6 +130,9 @@ closure from the lock alone. Two packages that would install under the same
 published into `lib/` only after every usage resolved and verified; a failing run leaves `lib/`
 and the lock as they were. A lock entry wins over the constraint in `.project.json` as long as it satisfies it;
 `install` then verifies the installed copy's digest and fetches only what is missing or changed.
+A lock is read as written: a repository `resource` in any spelling other than the canonical
+identity above, or a `usage` entry that is a bare string, is refused with the form this tool
+writes, not translated.
 
 ## Install layout
 

internal/install/install.go

@@ -116,7 +116,9 @@ func Run(ctx context.Context, opts Options) (*Result, error) {
 	if old == nil {
 		old = lock.New()
 	}
-	keyResources(old)
+	if err := keyResources(old); err != nil {
+		return nil, err
+	}
 	r := &resolver{ctx: ctx, opts: opts, proj: proj, old: old, fresh: lock.New(), chosen: map[string]*lock.Entry{}, res: &Result{}, libDir: filepath.Join(root, project.LibDir), slots: map[string]slot{}}
 	for _, u := range proj.Usage {
 		c, err := usageConstraint(u)
@@ -472,20 +474,21 @@ func (r *resolver) requeueLocked(e *lock.Entry, old *lock.Entry) error {
 	return nil
 }
 
-// keyResources rewrites each lock resource to its matching key, so a lock
-// written in an older or differently suffixed spelling still matches its usage.
-func keyResources(l *lock.Lock) {
-	for i := range l.Packages {
-		l.Packages[i].Resource = lockKey(l.Packages[i].Resource)
-	}
-}
-
-// lockKey is origin.Key for a repository resource and the resource itself otherwise.
-func lockKey(resource string) string {
-	if o, err := origin.Parse(resource); err == nil && o.Kind == origin.VCS {
-		return o.Key()
+// keyResources rewrites each repository resource to its key, so an entry and a
+// usage match however the .git suffix was spelled. Any other spelling is not an
+// identity this tool writes, and the lock is refused rather than guessed at.
+func keyResources(l *lock.Lock) error {
+	for i, e := range l.Packages {
+		o, err := origin.Parse(e.Resource)
+		if err != nil || o.Kind != origin.VCS {
+			continue
+		}
+		if id := o.Identity(); id != e.Resource {
+			return fmt.Errorf("%s: packages[%d] resource %s is not canonical (this tool writes %s); delete the lock and run syspi lock", project.LockFile, i, e.Resource, id)
+		}
+		l.Packages[i].Resource = o.Key()
 	}
-	return resource
+	return nil
 }
 
 func (r *resolver) download(url string, dst string) error {

internal/install/install_test.go

@@ -220,20 +220,15 @@ func TestGitInstallLockAndOffline(t *testing.T) {
 		t.Fatalf("re-resolved lock differs:\n%s\n%s", first, third)
 	}
 
-	// A lock whose resources carry an older, non-canonical spelling still
-	// matches the installed trees offline.
-	legacy := strings.ReplaceAll(string(first), `"`+ident(t, wres)+`"`, `"`+wres+`"`)
-	if legacy == string(first) {
+	// A lock whose resources carry a non-canonical spelling is not one this
+	// tool wrote and is refused, not guessed at.
+	stale := strings.ReplaceAll(string(first), `"`+ident(t, wres)+`"`, `"`+wres+`"`)
+	if stale == string(first) {
 		t.Fatal("lock did not contain the canonical widgets identity")
 	}
-	write(t, dir, project.LockFile, legacy)
-	res5, log5 := run(t, dir, Options{Offline: true})
-	if len(res5.Reused) != 2 || !res5.LockWritten {
-		t.Fatalf("legacy lock run %+v\n%s", res5, log5)
-	}
-	if again, _ := os.ReadFile(filepath.Join(dir, project.LockFile)); !bytes.Equal(again, first) {
-		t.Fatalf("legacy lock not rewritten canonically:\n%s", again)
-	}
+	write(t, dir, project.LockFile, stale)
+	runErr(t, dir, Options{Offline: true}, "resource "+wres+" is not canonical")
+	write(t, dir, project.LockFile, string(first))
 
 	// Offline with the repository gone: still satisfied.
 	os.RemoveAll(widgets)
@@ -253,27 +248,37 @@ func TestGitInstallLockAndOffline(t *testing.T) {
 
 func TestKeyResources(t *testing.T) {
 	l := lock.New()
-	l.Put(lock.Entry{Resource: "pkg:git/example.org/team/widgets", Kind: lock.KindVCS})
-	l.Put(lock.Entry{Resource: "git+ssh://example.org/team/gears.git#packages/gears", Kind: lock.KindVCS})
 	l.Put(lock.Entry{Resource: "https://example.org/dist/tools_1.0.0.kpar", Kind: lock.KindArchive})
 	l.Put(lock.Entry{Resource: "pkg:generic/rover?vcs_url=git%2Bhttps:%2F%2Fexample.org%2Fteam%2Frover.git", Kind: lock.KindVCS})
-	keyResources(l)
+	l.Put(lock.Entry{Resource: "pkg:generic/gears?vcs_url=git%2Bssh:%2F%2Fexample.org%2Fteam%2Fgears%23packages%2Fgears", Kind: lock.KindVCS})
+	if err := keyResources(l); err != nil {
+		t.Fatal(err)
+	}
 	want := []string{
-		"pkg:generic/widgets?vcs_url=git%2Bhttps:%2F%2Fexample.org%2Fteam%2Fwidgets",
-		"pkg:generic/gears?vcs_url=git%2Bssh:%2F%2Fexample.org%2Fteam%2Fgears%23packages%2Fgears",
 		"https://example.org/dist/tools_1.0.0.kpar",
 		"pkg:generic/rover?vcs_url=git%2Bhttps:%2F%2Fexample.org%2Fteam%2Frover",
+		"pkg:generic/gears?vcs_url=git%2Bssh:%2F%2Fexample.org%2Fteam%2Fgears%23packages%2Fgears",
 	}
 	for i, w := range want {
 		if l.Packages[i].Resource != w {
 			t.Errorf("entry %d = %s, want %s", i, l.Packages[i].Resource, w)
 		}
 	}
+	for _, res := range []string{
+		"pkg:git/example.org/team/widgets",
+		"git+ssh://example.org/team/gears.git#packages/gears",
+		"pkg:generic/widgets?vcs_url=git%2Bhttps:%2F%2Fexample.org%2Fteam%2Fwidgets%401.0.0",
+	} {
+		l := lock.New()
+		l.Put(lock.Entry{Resource: res, Kind: lock.KindVCS})
+		if err := keyResources(l); err == nil || !strings.Contains(err.Error(), "not canonical") {
+			t.Errorf("%s: want a non-canonical error, got %v", res, err)
+		}
+	}
 }
 
-// A lock written in any spelling of the repository, with or without .git,
-// matches a usage in any spelling of the same location, and the installed
-// tree is reused offline; a usage naming the other location (repo for
+// A lock written with or without .git matches a usage in any spelling of the
+// same location, and the installed tree is reused offline; a usage naming the other location (repo for
 // repo.git) is a different repository and is not served from that tree.
 func TestLockMatchesAcrossGitSuffixSpellings(t *testing.T) {
 	widgets, _ := fixture(t)
@@ -310,6 +315,9 @@ func TestLockMatchesAcrossGitSuffixSpellings(t *testing.T) {
 		t.Fatalf("older source not rewritten:\n%s", again)
 	}
 	for _, lk := range spellings {
+		if !strings.HasPrefix(lk.res, "pkg:") {
+			continue // the lock holds canonical identities only
+		}
 		for _, us := range spellings {
 			p := &project.Project{Name: "app", Publisher: "test", Version: "0.1.0", Usage: []project.Usage{usage(us.res)}}
 			if err := p.Write(dir); err != nil {

internal/lock/lock.go

@@ -56,23 +56,6 @@ type Usage struct {
 	VersionConstraint string `json:"versionConstraint,omitempty"`
 }
 
-// UnmarshalJSON also accepts the bare resource string earlier locks wrote
-// (its @<version>, if any, is the constraint, as in .project.json).
-func (u *Usage) UnmarshalJSON(data []byte) error {
-	var s string
-	if err := json.Unmarshal(data, &s); err == nil {
-		*u = Usage{Resource: s}
-		return nil
-	}
-	type plain Usage
-	var p plain
-	if err := json.Unmarshal(data, &p); err != nil {
-		return err
-	}
-	*u = Usage(p)
-	return nil
-}
-
 func sortUsages(u []Usage) {
 	sort.Slice(u, func(i, j int) bool {
 		if u[i].Resource != u[j].Resource {

internal/lock/lock_test.go

@@ -4,6 +4,8 @@ package lock
 
 import (
 	"bytes"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -58,3 +60,35 @@ func TestDeterministicRoundTrip(t *testing.T) {
 		}
 	}
 }
+
+func TestParseRejectsStringUsages(t *testing.T) {
+	_, err := Parse([]byte(`{"lockVersion":1,"packages":[{"resource":"pkg:generic/app?vcs_url=git%2Bhttps:%2F%2Fexample.org%2Fapp","kind":"vcs","name":"app","version":"1.0.0","usage":["pkg:generic/core?vcs_url=git%2Bhttps:%2F%2Fexample.org%2Fcore%401.2.0"]}]}`))
+	if err == nil || !strings.Contains(err.Error(), "lock:") {
+		t.Fatalf("a bare usage string must be refused, got %v", err)
+	}
+}
+
+func TestWriteThroughSymlink(t *testing.T) {
+	root := t.TempDir()
+	shared := filepath.Join(root, "shared.lock.json")
+	os.WriteFile(shared, []byte("{}"), 0o644)
+	dir := filepath.Join(root, "a")
+	os.Mkdir(dir, 0o755)
+	if err := os.Symlink(shared, filepath.Join(dir, "lock.json")); err != nil {
+		t.Skip(err)
+	}
+	l := New()
+	l.Packages = append(l.Packages, Entry{Resource: "pkg:git/example.org/core", Kind: KindVCS, Name: "core", Version: "1.0.0"})
+	if wrote, err := l.Write(dir, "lock.json"); err != nil || !wrote {
+		t.Fatal(wrote, err)
+	}
+	if fi, err := os.Lstat(filepath.Join(dir, "lock.json")); err != nil || fi.Mode()&os.ModeSymlink == 0 {
+		t.Fatal("the symlink must be kept")
+	}
+	if got, _ := os.ReadFile(shared); string(got) != string(l.Marshal()) {
+		t.Fatalf("target not updated:\n%s", got)
+	}
+	if left, _ := filepath.Glob(filepath.Join(root, ".*")); len(left) != 0 {
+		t.Fatalf("temporary files left: %v", left)
+	}
+}