From 364a4a0351055bd555c8871e5eecfc6d29886dee Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 01:58:13 +0000 Subject: [PATCH 1/3] fix(origin): a drive letter is only a drive on Windows fileURIPath stripped the leading slash from file:///C:/tools.kpar on every system, so on Linux and macOS a directory named C: resolved relative to the project. The drive check is now Windows-only (windowsPaths), the Windows file:// spellings are documented, and the lock-rollback test runs on Windows by making the lock file read-only instead of skipping. Co-Authored-By: jason.han --- README.md | 2 +- changes/unreleased/windows-file-uris.fixed.md | 1 + docs/packages.md | 5 ++- internal/install/install_test.go | 17 +++++---- internal/origin/origin.go | 15 +++++--- internal/origin/origin_test.go | 35 +++++++++++++++++++ 6 files changed, 62 insertions(+), 13 deletions(-) create mode 100644 changes/unreleased/windows-file-uris.fixed.md diff --git a/README.md b/README.md index fc2696f..4204ab2 100644 --- a/README.md +++ b/README.md @@ -18,7 +18,7 @@ what package managers install. plus `file` for a repository on this machine; `` is the repository's last path segment. Two shorter spellings are accepted as input and normalised to it: `pkg:git//[@][#]` (git over https) and the bare notation `git+https://host/path@1.2.0`, `svn://host/repo@1.2.0`, - `hg+https://…`, `git+file:///srv/repo@1.2.0`. Refused with a message naming those three: + `hg+https://…`, `git+file:///srv/repo@1.2.0` (`git+file:///C:/repo@1.2.0` on Windows). Refused with a message naming those three: `pkg:svn/…`, `pkg:hg/…`, `pkg:github/…`, plain clone URLs (`https://host/path.git`, `ssh://…`, `git@host:path`, `file:///path@1.2.0`) and a commit id as version. SysPI hand-parses this one purl shape and uses no purl library. A plain `http(s)` IRI is only ever read as a project diff --git a/changes/unreleased/windows-file-uris.fixed.md b/changes/unreleased/windows-file-uris.fixed.md new file mode 100644 index 0000000..cfd15d4 --- /dev/null +++ b/changes/unreleased/windows-file-uris.fixed.md @@ -0,0 +1 @@ +- A drive letter in a `file://` resource is only a drive on Windows; on Linux and macOS `file:///C:/tools.kpar` names the directory `C:` again. The Windows spellings are documented, and the lock-rollback test runs on Windows too. diff --git a/docs/packages.md b/docs/packages.md index 35786f2..09daf1a 100644 --- a/docs/packages.md +++ b/docs/packages.md @@ -33,7 +33,10 @@ Two input spellings normalise to it (and `syspi add` writes the canonical form i `.project.json`): `pkg:git//[@][#]`, meaning `git+https`; and the bare vcs_url notation `git+https://…`, `git+ssh://…`, `git://…`, `hg+https://…`, `hg+static-http://…`, `svn://…`, `svn+https://…`, `svn+ssh://…`, `+file:///…`, each with an -optional `@` and `#`. The version-free identity (`vcs_url` without `@`) +optional `@` and `#`. A repository on a Windows machine is +`+file:///C:/path` (RFC 8089); `+file://C:/path` is accepted as input and normalised to +it, and the same two forms name a local `.kpar` or project directory (`file:///C:/tools.kpar`). +On other systems `/C:/path` is an ordinary directory named `C:`. The version-free identity (`vcs_url` without `@`) is what the lock records as `resource`; what the VCS executable is given is kept as `source` (`git+ssh` stays SSH; the path is cloned exactly as written, so `…/repo` and `…/repo.git` are two locations and the one your server serves is the one to write). diff --git a/internal/install/install_test.go b/internal/install/install_test.go index 2e902f8..175e807 100644 --- a/internal/install/install_test.go +++ b/internal/install/install_test.go @@ -550,9 +550,6 @@ func TestPublishRollsBackWhenTheLockCannotBeWritten(t *testing.T) { if os.Geteuid() == 0 { t.Skip("root ignores directory permissions") } - if runtime.GOOS == "windows" { - t.Skip("a read-only directory does not refuse new files on Windows") - } widgets, _ := fixture(t) dir := consumer(t, project.Usage{Resource: "git+file://" + filepath.ToSlash(widgets) + "@1.0.0"}) run(t, dir, Options{}) @@ -560,14 +557,20 @@ func TestPublishRollsBackWhenTheLockCannotBeWritten(t *testing.T) { // Move to 1.1.0 (replaces the installed tree) with the project directory read-only, // so the lock cannot be written after lib/ was updated. write(t, dir, project.ProjectFile, `{"name":"app","version":"0.1.0","usage":[{"resource":"git+file://`+filepath.ToSlash(widgets)+`@1.1.0"}]}`) - if err := os.Chmod(dir, 0o555); err != nil { + // Windows ignores directory modes but refuses to replace a read-only file. + locked, mode := dir, os.FileMode(0o555) + if runtime.GOOS == "windows" { + locked, mode = filepath.Join(dir, project.LockFile), 0o444 + } + if err := os.Chmod(locked, mode); err != nil { t.Fatal(err) } - t.Cleanup(func() { os.Chmod(dir, 0o755) }) + t.Cleanup(func() { os.Chmod(locked, mode|0o200) }) opts := Options{Root: dir} _, err := Run(context.Background(), opts) - os.Chmod(dir, 0o755) - if err == nil || !strings.Contains(err.Error(), "permission denied") || !strings.Contains(err.Error(), project.LockFile) { + os.Chmod(locked, mode|0o200) + denied := err != nil && (strings.Contains(err.Error(), "permission denied") || strings.Contains(err.Error(), "Access is denied")) + if !denied || !strings.Contains(err.Error(), project.LockFile) { t.Fatalf("the lock write must be what failed: %v", err) } after, _ := os.ReadFile(filepath.Join(dir, project.LockFile)) diff --git a/internal/origin/origin.go b/internal/origin/origin.go index e6e9382..49c44e2 100644 --- a/internal/origin/origin.go +++ b/internal/origin/origin.go @@ -18,6 +18,7 @@ import ( "path" "path/filepath" "regexp" + "runtime" "strings" "github.com/Open-MBEE/SysPI/internal/version" @@ -303,7 +304,7 @@ func parseVCSURL(res, raw string) (*Origin, error) { } if transport == "file" { // file:///C:/path is the RFC 8089 form; file://C:/path is accepted as input. - if drive.MatchString(rest) { + if isDrivePath(rest) { rest = "/" + rest } } @@ -323,7 +324,7 @@ func parseVCSURL(res, raw string) (*Origin, error) { return nil, fmt.Errorf("resource %q: a repository on this machine is %s+file:///absolute/path", res, vcs) } o.Path = u.Path - if drive.MatchString(o.Path[1:]) { + if isDrivePath(o.Path[1:]) { o.Path = o.Path[1:] } o.Path = filepath.ToSlash(filepath.Clean(o.Path)) @@ -402,6 +403,12 @@ func parseHTTP(res string) (*Origin, error) { // drive matches the start of a Windows path, C:/ or C:\. var drive = regexp.MustCompile(`^[A-Za-z]:[/\\]`) +// windowsPaths is whether this machine spells absolute paths with a drive; on +// other systems /C:/dir is an ordinary directory named C:. +var windowsPaths = runtime.GOOS == "windows" + +func isDrivePath(p string) bool { return windowsPaths && drive.MatchString(p) } + // FileURIPath is the path part of a file:// URI naming abs: /C:/dir on Windows, /dir elsewhere. func FileURIPath(abs string) string { abs = filepath.ToSlash(abs) @@ -413,13 +420,13 @@ func FileURIPath(abs string) string { // fileURIPath turns what follows file:// into a path on this machine; a host is refused. func fileURIPath(rest string) (string, bool) { - if drive.MatchString(rest) { + if isDrivePath(rest) { return rest, true } if !strings.HasPrefix(rest, "/") { return "", false } - if drive.MatchString(rest[1:]) { + if isDrivePath(rest[1:]) { return rest[1:], true } return rest, true diff --git a/internal/origin/origin_test.go b/internal/origin/origin_test.go index ede937d..7318113 100644 --- a/internal/origin/origin_test.go +++ b/internal/origin/origin_test.go @@ -135,3 +135,38 @@ func TestKey(t *testing.T) { t.Errorf("archive key %s != identity %s", o.Key(), o.Identity()) } } + +// A drive letter is only a drive on Windows; elsewhere /C:/dir is a directory named C:. +func TestDriveLetterFileURIs(t *testing.T) { + defer func(w bool) { windowsPaths = w }(windowsPaths) + for _, tc := range []struct { + windows bool + in, path, src string + }{ + {true, "git+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", `C:\srv\widgets`}, + {true, "git+file://C:/srv/widgets@1.0.0", "C:/srv/widgets", `C:\srv\widgets`}, + {true, "svn+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", "file:///C:/srv/widgets"}, + {false, "git+file:///C:/srv/widgets@1.0.0", "/C:/srv/widgets", "/C:/srv/widgets"}, + {false, "svn+file:///C:/srv/widgets@1.0.0", "/C:/srv/widgets", "file:///C:/srv/widgets"}, + } { + windowsPaths = tc.windows + o, err := Parse(tc.in) + if err != nil { + t.Errorf("windows=%v %s: %v", tc.windows, tc.in, err) + continue + } + if o.Path != tc.path || o.Source != tc.src { + t.Errorf("windows=%v %s: path %q source %q, want %q %q", tc.windows, tc.in, o.Path, o.Source, tc.path, tc.src) + } + if _, err := Parse(o.Canonical()); err != nil { + t.Errorf("windows=%v canonical %s: %v", tc.windows, o.Canonical(), err) + } + } + windowsPaths = false + if _, err := Parse("file://C:/tools.kpar"); err == nil { + t.Error("file://C:/… must be refused where C: is a host, not a drive") + } + if o, err := Parse("file:///C:/tools.kpar"); err != nil || o.Source != "/C:/tools.kpar" { + t.Errorf("file:///C:/tools.kpar: %v %+v", err, o) + } +} From 4f89ff45947e4452ec8580ce573216730953ded1 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 02:04:19 +0000 Subject: [PATCH 2/3] test(origin): the git source of a drive path uses this machine's separator Co-Authored-By: jason.han --- internal/origin/origin_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/origin/origin_test.go b/internal/origin/origin_test.go index 7318113..f492d32 100644 --- a/internal/origin/origin_test.go +++ b/internal/origin/origin_test.go @@ -143,8 +143,8 @@ func TestDriveLetterFileURIs(t *testing.T) { windows bool in, path, src string }{ - {true, "git+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", `C:\srv\widgets`}, - {true, "git+file://C:/srv/widgets@1.0.0", "C:/srv/widgets", `C:\srv\widgets`}, + {true, "git+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", filepath.FromSlash("C:/srv/widgets")}, + {true, "git+file://C:/srv/widgets@1.0.0", "C:/srv/widgets", filepath.FromSlash("C:/srv/widgets")}, {true, "svn+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", "file:///C:/srv/widgets"}, {false, "git+file:///C:/srv/widgets@1.0.0", "/C:/srv/widgets", "/C:/srv/widgets"}, {false, "svn+file:///C:/srv/widgets@1.0.0", "/C:/srv/widgets", "file:///C:/srv/widgets"}, From 0d22cbd092586b349ac548544cad853482c4ebfb Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Fri, 30 Oct 2026 02:11:06 +0000 Subject: [PATCH 3/3] test(origin): the git source of a simulated drive path still uses the host's separators Co-Authored-By: jason.han --- internal/origin/origin_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/origin/origin_test.go b/internal/origin/origin_test.go index f492d32..a1842f0 100644 --- a/internal/origin/origin_test.go +++ b/internal/origin/origin_test.go @@ -143,12 +143,15 @@ func TestDriveLetterFileURIs(t *testing.T) { windows bool in, path, src string }{ - {true, "git+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", filepath.FromSlash("C:/srv/widgets")}, - {true, "git+file://C:/srv/widgets@1.0.0", "C:/srv/widgets", filepath.FromSlash("C:/srv/widgets")}, + {true, "git+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", ""}, + {true, "git+file://C:/srv/widgets@1.0.0", "C:/srv/widgets", ""}, {true, "svn+file:///C:/srv/widgets@1.0.0", "C:/srv/widgets", "file:///C:/srv/widgets"}, - {false, "git+file:///C:/srv/widgets@1.0.0", "/C:/srv/widgets", "/C:/srv/widgets"}, + {false, "git+file:///C:/srv/widgets@1.0.0", "/C:/srv/widgets", ""}, {false, "svn+file:///C:/srv/widgets@1.0.0", "/C:/srv/widgets", "file:///C:/srv/widgets"}, } { + if tc.src == "" { // git: the path in this machine's separators, whichever machine is simulated + tc.src = filepath.FromSlash(tc.path) + } windowsPaths = tc.windows o, err := Parse(tc.in) if err != nil {