On Tue, May 10, 2022 at 11:20:41AM -0700, Carlo Arenas wrote:

On Mon, May 9, 2022 at 8:38 AM Christian Couder
[off-list ref] wrote:

quoted

diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index f92c79c132..4a8dbb7eee 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh

@@ -567,4 +567,11 @@ test_expect_success 'client falls back from v2 to v0 to match server' '
        grep symref=HEAD:refs/heads/ trace
 '

+test_expect_success 'passing hostname resolution information works' '
+       BOGUS_HOST=gitbogusexamplehost.com &&
+       BOGUS_HTTPD_URL=$HTTPD_PROTO://$BOGUS_HOST:$LIB_HTTPD_PORT &&
+       test_must_fail git ls-remote "$BOGUS_HTTPD_URL/smart/repo.git" >/dev/null &&
+       git -c "http.curloptResolve=$BOGUS_HOST:$LIB_HTTPD_PORT:127.0.0.1" ls-remote "$BOGUS_HTTPD_URL/smart/repo.git" >/dev/null
+'

Is setting it up as a command line config option the way you expect to
use this, and if so why not make it a full blown command line option
with the previous caveats that were discussed before?

If you did this as a command-line option, you'd now be forced to add it
to every single command you want to support this: git-fetch, git-pull,
git-remote, git-ls-remote and maybe others I forgot about. On the other
hand, by having this as a configuration variable in `http.c` all of
those commands benefit the same.

Furthermore, using a config option is a lot more flexible: you can
persist it at different levels of your gitconfig, can easily inject it
in a script via the use of environment variables, or directly override
it when spawning a command with `-c`.

Overall, I think it is preferable to keep this as an option as opposed
to adding such an obscure parameter to all of the commands.

I also think it might be a little confusing (and probably warranted of
an advice message) if git will decide based on a configuration
somewhere in its resolution tree that the IP I am connecting is
different than the one I expect it to use through the system
configured resolution mechanism for such a thing.

That's true already though, isn't it? A user may set `url.*.insteadOf`
and be surprised at a later point that their URLs are getting redirected
somewhere else. And there's probably a lot more examples where a user
may be confused when forgetting about certain configuration variables
that change the way Git behaves.

I also don't think that using an advise here would be ideal. The main
use case of this configuration variable is going to be servers, and
there is a high chance that they might actually be parsing output of any
such commands. Forcing them to always disable this advise doesn't feel
like the right thing to do.

Patrick

I assume that if you want to use this frequently, having that advice
disabled in your global config wouldn't be a hassle, but it might be
useful to know that I am interacting with a potentially different IP
when referring to some host by name in my local repo, maybe because I
forgot to change that setting after some debugging.

I am sure all those folks that forget to edit their /etc/hosts after
they are done with their local site versions might instead use this
and then be happy to be warned about it later.

Carlo