Thread (40 messages) 40 messages, 5 authors, 2026-05-03

Re: [PATCH v3 0/8] builtin: implement, document and test url-parse

From: Matheus Afonso Martins Moreira <hidden>
Date: 2026-05-03 19:36:55

Reviewers comment: Nicely done.
Thank you!
More a question to myself, may be, about t9904 (and may be other parts)
I have in mind that the parser learned to handle

file://server/share/repo
correctly under Windows.
I don't know if this needs to be addressed here or in a follow-up commit ?
I'd be happy to revisit this in a follow-up. It's been a while
since I used MSYS but I do remember the fact it rewrites paths
internally. I wasn't sure how to handle it properly in the tests.

The problematic test case is:

    test_must_fail git url-parse "/abs/path" 2>err &&
      test_grep "is not a URL" err &&
      test_grep "file:///abs/path" err

MSYS bash rewrites /abs/path to C:/Program Files/Git/abs/path
before git even runs. This edge case caused the error message:

    fatal: 'C:/Program Files/Git/abs/path' is not a URL;
    if you meant a local repository, use a 'file://' URL
    with an absolute path

The test_grep "is not a URL" passed but test_grep "file:///abs/path"
failed because the suggestion did not contain the literal string
"file:///abs/path". The drive letter broke the tool's absolute
path recognition: it was printing the generic error message.

The fix was to use has_dos_drive_prefix() to recognize the edge case.
However, that led to the generation of error messages containing paths
that I wasn't sure if I could depend on in the test suite, such as:

    file:///C:/Program Files/Git/abs/path

So I decided to relax the test case just a little:

    test_must_fail git url-parse "/abs/path" 2>err &&
      test_grep "is not a URL" err &&
      test_grep "file:///" err

The "file:///" checks that the path was properly recognized
and that the friendlier error message was printed, all while
avoiding the hard coding of a "C:/Program Files/Git" prefix
that may or may not vary depending on testing environment.

In any case, the parser already handles it correctly.

It decomposes:

    file://server/share/repo

As:

  - scheme: file
  - host:   server
  - path:   /share/repo

Which is the correct interpretation.

On Windows, connect.c then takes that data and reconstructs
the UNC path \\server\share\repo for the filesystem.
So the UNC reconstruction happens downstream in connect.c,
not directly in the url-parse builtin or url_parse logic.

    Matheus
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help