git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 4/6] config: return configset value for current_config_ functions
Date: Wed, 18 May 2016 20:08:21 -0400	[thread overview]
Message-ID: <20160519000821.GA22543@sigill.intra.peff.net> (raw)
In-Reply-To: <20160518224323.GD22443@sigill.intra.peff.net>

On Wed, May 18, 2016 at 06:43:23PM -0400, Jeff King wrote:

>  cache.h                |  1 +
>  config.c               | 51 +++++++++++++++++++++++++++++++++++++++++---------
>  t/helper/test-config.c | 20 ++++++++++++++++++++
>  t/t1308-config-set.sh  | 23 +++++++++++++++++++++++
> [...]
> +test_expect_success 'iteration shows correct origins' '
> +	echo "[alias]test-config = !test-config" >.gitconfig &&
> [...]
> +	git -c foo.bar=from-cmdline test-config iterate >actual &&

While writing and testing this, I got bit by e6e7530 (test helpers: move
test-* to t/helper/ subdirectory, 2016-04-13). I had an old test-config
binary leftover in the root of my repository, and the new one was
correctly built in t/helper/. Running "test-config" is fine, but inside
the git alias, it sticks the repository root at the front of $PATH
(because it's the exec-path). And so it ran the old version of
test-config, which did not understand my new "iterate" option.

Now I'll admit what I'm doing here is pretty funny (running test-* from
an alias). I'm doing it because I want to see how the program operates
with the "-c" config, and it's nicer to spell it as a user would,
instead of munging $GIT_CONFIG_PARAMETERS directly.

So I'm not sure if it's worth working around or not. The single tree
state produced by this commit is fine, but it does behave badly if
there's leftover cruft from a pre-e6e7530 build. A more robust version
would look more like:

  sq=\' ;# to ease quoting later
  ...
  GIT_CONFIG_PARAMETERS=${sq}foo.bar=from-cmdline${sq} test-config ...

Which is ugly, but it's probably worth it to avoid the flakiness.

The other option is to somehow make bin-wrappers more robust. E.g., it
would be nice if we didn't actually point into the repository root
directly, but rather somehow linked all of the git-* entries that
_would_ be installed into the exec-path into a fake exec-path (or
alternatively, actually build them directly into that fake exec-path).

That's a much bigger change, though. Given how unlikely the sequence of
steps in my test is, maybe it's better to just work around it in this
one case.

-Peff

  reply	other threads:[~2016-05-19  0:08 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-18 22:37 [PATCH/RFC 0/6] pack-objects hook for upload-pack Jeff King
2016-05-18 22:39 ` [PATCH 1/6] git_config_with_options: drop "found" counting Jeff King
2016-05-18 22:39 ` [PATCH 2/6] git_config_parse_parameter: refactor cleanup code Jeff King
2016-05-18 22:41 ` [PATCH 3/6] config: set up config_source for command-line config Jeff King
2016-05-18 22:43 ` [PATCH 4/6] config: return configset value for current_config_ functions Jeff King
2016-05-19  0:08   ` Jeff King [this message]
2016-05-26  7:47     ` Duy Nguyen
2016-05-26 16:42       ` Junio C Hamano
2016-05-26 16:50         ` Jeff King
2016-05-26 17:36           ` Junio C Hamano
2016-05-27  0:41             ` Jeff King
2016-05-27  2:11               ` Junio C Hamano
2016-05-27  0:32           ` Jeff King
2016-05-18 22:44 ` [PATCH 5/6] config: add a notion of "scope" Jeff King
2016-05-18 22:45 ` [PATCH 6/6] upload-pack: provide a hook for running pack-objects Jeff King
2016-05-19  0:14   ` Jeff King
2016-05-19 10:12   ` Ævar Arnfjörð Bjarmason
2016-05-19 12:08     ` Jeff King
2016-05-19 14:54       ` Ævar Arnfjörð Bjarmason
2016-05-26  5:37         ` Jeff King
2016-05-25  0:59 ` [PATCH/RFC 0/6] pack-objects hook for upload-pack Junio C Hamano
2016-05-26  5:44   ` Jeff King
2016-05-26 16:44     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160519000821.GA22543@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).