On Tue, May 10, 2022 at 11:20:41AM -0700, Carlo Arenas wrote:
> On Mon, May 9, 2022 at 8:38 AM Christian Couder
> <christian.couder@gmail.com> wrote:
> > 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