Thread (70 messages) 70 messages, 3 authors, 3d ago

Re: [PATCH v4 2/5] setup: extract path_allowlist_apply()

flat view

From: Christian Couder <hidden>
Date: 2026-10-02 09:00:35

On Tue, Sep 29, 2026 at 7:26 PM Junio C Hamano [off-list ref] wrote:
Christian Couder [off-list ref] writes:
quoted
+     /*
+      * A .gitconfig in $HOME may be shared across different
+      * machines and the config variable entries may or may not
+      * exist as paths on all of these machines.  In other words,
+      * it is not a warning worthy event when there is no such path
+      * on this machine---the entry may be useful elsewhere.
+      */
This might be a minor point (as not many people may be using the
safe.directory feature that this was moved from), and this dates
back two years, starting with dc0edbb01c (safe.directory: normalize
the configured path, 2024-07-30), but the above design decision cuts
both ways.  If you misspelled a pathname, you would never be told
about it.

I wonder if we want to allow users to explicitly mark that it is OK if
a path does not exist, in much the same way that a pathname-typed
configuration variable can be prefixed with :(optional) to tell the
system "if this path exists on the system, use it, but if not, instead
of warning, pretend that you did not see this specified".

That way, a user can first specify the value normally, and then when
they reuse the .gitconfig file somewhere else that does not have the
path, they see a warning message.  You would help them by giving a
hint, e.g.,

    Specified path foo/bar does not exist.  If you spelled the
    pathname correctly, and the path is allowed to be missing,
    mark it as optional, i.e., ":(optional)foo/bar".

or something along those lines in the warning message and the world
would be a much better place.

In any case, it is outside the scope of this series, beyond leaving
a NEEDSWORK comment here, and/or a #leftoverbits comment in the
review.
There is the following new NEEDSWORK comment in the v5 I just sent:

+        * NEEDSWORK: this also silently ignores misspelled paths. We
+        * may want to warn about a missing path unless it is marked
+        * as allowed to be missing, e.g., with an ":(optional)"
+        * prefix like pathname-typed configuration values, and hint
+        * about that prefix in the warning.

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