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

From: Matheus Afonso Martins Moreira <matheus@matheusmoreira.com>
Date: May 3, 2026, 19:36 UTC
Message-ID: <6c0a1601cd379bcdc87b4fe3b854166a@matheusmoreira.com>
In-Reply-To: <20260503172838.GA22957@tb-raspi4>

> Reviewers comment: Nicely done.

Thank you!

Show 6 quoted lines

> 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