Repository navigation
fix(origin): accept drive-letter file URIs and run the suite on Windows #5
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| # Go sources and scripts are committed with LF; a Windows checkout must not get CRLF (gofmt, sh). | ||
| * text=auto eol=lf |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - On Windows, a repository on this machine is accepted as `git+file:///C:/path` (and `git+file://C:/path`), `file://` resources with a drive letter resolve, and the suite runs on Windows; CI gates on Windows with hg and svn installed. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -189,7 +189,8 @@ func Parse(resource string) (*Origin, error) { | |
| return parseHTTP(res) | ||
| } | ||
| if rest, ok := strings.CutPrefix(res, "file://"); ok { | ||
| rest, ok = fileURIPath(rest) | ||
| if !ok { | ||
| return nil, fmt.Errorf("resource %q: file URIs must name a path on this machine (file:///path)", res) | ||
| } | ||
| return parseLocal(res, rest) | ||
|
|
@@ -270,7 +271,8 @@ func parseGeneric(res, tail string) (*Origin, error) { | |
| // git+https://<host>/<path>. | ||
| func parseGitAlias(res, tail string) (*Origin, error) { | ||
| if strings.HasPrefix(tail, "/") { | ||
| abs, _ := filepath.Abs(filepath.FromSlash("/" + strings.TrimLeft(tail, "/"))) | ||
| return nil, fmt.Errorf("resource %q: pkg:git needs a host; a repository on this machine is git+file://%s", res, FileURIPath(abs)) | ||
| } | ||
| if tail == "" || !strings.Contains(tail, "/") { | ||
| return nil, fmt.Errorf("resource %q: expected pkg:git/<host>/<path>[@<version>][#<subpath>]", res) | ||
|
|
@@ -299,6 +301,12 @@ func parseVCSURL(res, raw string) (*Origin, error) { | |
| if !transports[vcs][transport] { | ||
| return nil, fmt.Errorf("resource %q: %s+%s is not a vcs_url scheme for %s (ECMA-427 Annex B.3.5 allows %s)", res, vcs, transport, vcs, allowed(vcs)) | ||
| } | ||
| if transport == "file" { | ||
| // file:///C:/path is the RFC 8089 form; file://C:/path is accepted as input. | ||
| if drive.MatchString(rest) { | ||
| rest = "/" + rest | ||
| } | ||
|
Comment on lines
+304
to
+308
Contributor
Author
There was a problem hiding this comment. 🟡 Windows file URI contract remains undocumented
Learn moreExample: A Windows user reading the resource grammar finds Recommended fix: Document the accepted Windows Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Done in #6: |
||
| } | ||
| u, err := url.Parse(transport + "://" + rest) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("resource %q: not a valid URL: %v", res, err) | ||
|
|
@@ -314,11 +322,15 @@ func parseVCSURL(res, raw string) (*Origin, error) { | |
| if u.Host != "" || !strings.HasPrefix(u.Path, "/") { | ||
| 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:]) { | ||
| o.Path = o.Path[1:] | ||
| } | ||
| o.Path = filepath.ToSlash(filepath.Clean(o.Path)) | ||
| o.Name = strings.TrimSuffix(path.Base(o.Path), ".git") | ||
| switch vcs { | ||
| case "svn": | ||
| o.Source = "file://" + FileURIPath(o.Path) | ||
| default: | ||
| o.Source = filepath.FromSlash(o.Path) | ||
| } | ||
|
|
@@ -387,6 +399,32 @@ func parseHTTP(res string) (*Origin, error) { | |
| return &Origin{Kind: kind, Source: spec, Version: ver, Resource: res}, nil | ||
| } | ||
|
|
||
| // drive matches the start of a Windows path, C:/ or C:\. | ||
| var drive = regexp.MustCompile(`^[A-Za-z]:[/\\]`) | ||
|
|
||
| // 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) | ||
| if !strings.HasPrefix(abs, "/") { | ||
| abs = "/" + abs | ||
| } | ||
| return abs | ||
| } | ||
|
|
||
| // 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) { | ||
| return rest, true | ||
| } | ||
| if !strings.HasPrefix(rest, "/") { | ||
| return "", false | ||
| } | ||
| if drive.MatchString(rest[1:]) { | ||
| return rest[1:], true | ||
|
Comment on lines
+422
to
+423
Contributor
Author
There was a problem hiding this comment. 🟡 Absolute local paths become relative On POSIX, Learn moreA file URI with a first directory named Example: On Linux, a real archive at Recommended fix: Apply the drive normalization only on Windows in both Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
Author
There was a problem hiding this comment. Confirmed: on POSIX a directory named |
||
| } | ||
| return rest, true | ||
| } | ||
|
|
||
| // parseLocal handles file:// URIs and plain paths: a .kpar is a local | ||
| // archive, anything else an editable project directory. | ||
| func parseLocal(res, p string) (*Origin, error) { | ||
|
|
@@ -406,7 +444,7 @@ func parseLocal(res, p string) (*Origin, error) { | |
| } | ||
| if ver != "" { | ||
| abs, _ := filepath.Abs(p) | ||
| return nil, fmt.Errorf("resource %q: a path with @<version> is not an identity; a repository on this machine is git+file://%s@%s (or svn+file, hg+file)", res, FileURIPath(abs), ver) | ||
| } | ||
| if sub != "" { | ||
| p = path.Join(p, sub) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Windows rollback loses its test
On Windows,
TestPublishRollsBackWhenTheLockCannotBeWrittenskips instead of testing rollback. The Windows gate cannot catch a failed lock write leavinglib/inconsistent with the lock.Learn more
The rollback test simulates a lock write failure by changing directory permissions. Windows does not enforce that permission change in the same way, so the new skip removes rollback coverage precisely on the newly gated platform. Run publishes packages before writing the lock and invokes rollback if writing fails.
Example: On Windows, a regression in lock-write failure handling can leave
lib/containing version 1.1.0 while the on-disk lock still names 1.0.0; the CI test exits as skipped.Recommended fix: Replace the permissions-based failure setup on Windows with a deterministic lock-write failure that still lets
lib/be updated, then assert the old lock and installed tree remain intact. Retain the existing POSIX setup if needed.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. #6 removes the skip: Windows ignores a read-only directory but refuses to replace a read-only file, so there the test makes
.project.lock.jsonitself0444;lock.Write's finalos.Renamethen fails afterlib/was published and the same rollback path is asserted (message "Access is denied" instead of "permission denied"). The Windows gate on #6 is the proof.