From: Emily Shaffer <emilyshaffer@google.com> To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> Cc: git@vger.kernel.org, "Jonathan Nieder" <jrnieder@gmail.com>, "Johannes Schindelin" <Johannes.Schindelin@gmx.de>, "Jeff King" <peff@peff.net>, "brian m. carlson" <sandals@crustytoothpaste.net>, "Martin Ågren" <martin.agren@gmail.com>, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, "Derrick Stolee" <stolee@gmail.com>, "Derrick Stolee" <derrickstolee@github.com>, "Derrick Stolee" <dstolee@microsoft.com> Subject: Re: [PATCH v2 7/7] maintenance: use 'git config --fixed-value' Date: Mon, 23 Nov 2020 14:48:14 -0800 Message-ID: <20201123224814.GG499823@google.com> (raw) In-Reply-To: <5a3acf811998bb728ce94c69611c237861775142.1606147507.git.gitgitgadget@gmail.com> On Mon, Nov 23, 2020 at 04:05:07PM +0000, Derrick Stolee via GitGitGadget wrote: > > > When a repository's leading directories contain regex glob characters, Minor "well, actually" - I'm not sure 'glob' is the right word to use here. > the config calls for 'git maintenance register' and 'git maintenance > unregister' are not careful enough. Use the new --fixed-value option > to direct the config machinery to use exact string matches. This is a > more robust option than excaping these arguments in a piecemeal fashion. Typo on 'escaping'. > > For the test, require that we are not running on Windows since the '+' > character is not allowed on that filesystem. > > Reported-by: Emily Shaffer <emilyshaffer@google.com> > Reported-by: Jonathan Nieder <jrnieder@gmail.com> > Signed-off-by: Derrick Stolee <dstolee@microsoft.com> > --- > builtin/gc.c | 5 +++-- > t/t7900-maintenance.sh | 12 ++++++++++++ > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/builtin/gc.c b/builtin/gc.c > index e3098ef6a1..6dde3ce1bb 100644 > --- a/builtin/gc.c > +++ b/builtin/gc.c > @@ -1452,7 +1452,8 @@ static int maintenance_register(void) > git_config_set("maintenance.strategy", "incremental"); > > config_get.git_cmd = 1; > - strvec_pushl(&config_get.args, "config", "--global", "--get", "maintenance.repo", > + strvec_pushl(&config_get.args, "config", "--global", "--get", > + "--fixed-value", "maintenance.repo", > the_repository->worktree ? the_repository->worktree > : the_repository->gitdir, > NULL); > @@ -1483,7 +1484,7 @@ static int maintenance_unregister(void) > > config_unset.git_cmd = 1; > strvec_pushl(&config_unset.args, "config", "--global", "--unset", > - "maintenance.repo", > + "--fixed-value", "maintenance.repo", > the_repository->worktree ? the_repository->worktree > : the_repository->gitdir, > NULL); > diff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh > index 20184e96e1..c4e5564c31 100755 > --- a/t/t7900-maintenance.sh > +++ b/t/t7900-maintenance.sh > @@ -367,6 +367,18 @@ test_expect_success 'register and unregister' ' > test_cmp before actual > ' > > +test_expect_success !MINGW 'register and unregister with glob characters' ' > + GLOB="a+b*c" && > + git init "$GLOB" && > + git -C "$GLOB" maintenance register && > + git config --get-all --show-origin maintenance.repo && Hm, what's the reason for --show-origin when the output isn't captured or checked? Is this leftover? > + git config --get-all --global --fixed-value \ > + maintenance.repo "$(pwd)/$GLOB" && > + git -C "$GLOB" maintenance unregister && > + test_must_fail git config --get-all --global --fixed-value \ > + maintenance.repo "$(pwd)/$GLOB" > +' > + > test_expect_success 'start from empty cron table' ' > GIT_TEST_CRONTAB="test-tool crontab cron.txt" git maintenance start && > And after all that buildup, this patch is a very straightforward test fix: explain the problem, add the arg in question, add a regression test. Thanks. Other than the one extraneous(?) line, Reviewed-by: Emily Shaffer <emilyshaffer@google.com>
next prev parent reply other threads:[~2020-11-23 22:55 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-19 15:52 [PATCH 0/7] config: add --literal-value option Derrick Stolee via GitGitGadget 2020-11-19 15:52 ` [PATCH 1/7] t1300: test "set all" mode with value_regex Derrick Stolee via GitGitGadget 2020-11-19 22:24 ` Junio C Hamano 2020-11-20 2:09 ` brian m. carlson 2020-11-20 2:33 ` Junio C Hamano 2020-11-20 18:39 ` Jeff King 2020-11-20 22:35 ` Junio C Hamano 2020-11-21 22:27 ` brian m. carlson 2020-11-22 3:31 ` Junio C Hamano 2020-11-24 2:38 ` Jeff King 2020-11-24 19:43 ` Junio C Hamano 2020-11-19 15:52 ` [PATCH 2/7] t1300: add test for --replace-all " Derrick Stolee via GitGitGadget 2020-11-19 15:52 ` [PATCH 3/7] config: convert multi_replace to flags Derrick Stolee via GitGitGadget 2020-11-19 22:32 ` Junio C Hamano 2020-11-19 15:52 ` [PATCH 4/7] config: add --literal-value option, un-implemented Derrick Stolee via GitGitGadget 2020-11-19 22:42 ` Junio C Hamano 2020-11-20 6:35 ` Martin Ågren 2020-11-19 15:52 ` [PATCH 5/7] config: plumb --literal-value into config API Derrick Stolee via GitGitGadget 2020-11-19 22:45 ` Junio C Hamano 2020-11-19 15:52 ` [PATCH 6/7] config: implement --literal-value with --get* Derrick Stolee via GitGitGadget 2020-11-19 15:52 ` [PATCH 7/7] maintenance: use 'git config --literal-value' Derrick Stolee via GitGitGadget 2020-11-19 23:17 ` Junio C Hamano 2020-11-20 13:19 ` [PATCH 0/7] config: add --literal-value option Ævar Arnfjörð Bjarmason 2020-11-20 13:23 ` Derrick Stolee 2020-11-20 18:30 ` Junio C Hamano 2020-11-20 18:51 ` Derrick Stolee 2020-11-20 21:52 ` Junio C Hamano 2020-11-24 12:35 ` Ævar Arnfjörð Bjarmason 2020-11-23 16:05 ` [PATCH v2 0/7] config: add --fixed-value option Derrick Stolee via GitGitGadget 2020-11-23 16:05 ` [PATCH v2 1/7] t1300: test "set all" mode with value_regex Derrick Stolee via GitGitGadget 2020-11-23 19:37 ` Emily Shaffer 2020-11-23 16:05 ` [PATCH v2 2/7] t1300: add test for --replace-all " Derrick Stolee via GitGitGadget 2020-11-23 19:40 ` Emily Shaffer 2020-11-23 16:05 ` [PATCH v2 3/7] config: convert multi_replace to flags Derrick Stolee via GitGitGadget 2020-11-23 21:43 ` Emily Shaffer 2020-11-23 16:05 ` [PATCH v2 4/7] config: add --fixed-value option, un-implemented Derrick Stolee via GitGitGadget 2020-11-23 19:37 ` Junio C Hamano 2020-11-23 21:51 ` Emily Shaffer 2020-11-23 22:41 ` Junio C Hamano 2020-11-25 14:08 ` Derrick Stolee 2020-11-25 17:22 ` Derrick Stolee 2020-11-25 17:28 ` Eric Sunshine 2020-11-25 19:30 ` Junio C Hamano 2020-11-25 19:29 ` Junio C Hamano 2020-11-23 16:05 ` [PATCH v2 5/7] config: plumb --fixed-value into config API Derrick Stolee via GitGitGadget 2020-11-23 22:21 ` Emily Shaffer 2020-11-24 0:52 ` Eric Sunshine 2020-11-25 15:41 ` Derrick Stolee 2020-11-25 17:55 ` Eric Sunshine 2020-11-23 16:05 ` [PATCH v2 6/7] config: implement --fixed-value with --get* Derrick Stolee via GitGitGadget 2020-11-23 19:53 ` Junio C Hamano 2020-11-23 22:43 ` Emily Shaffer 2020-11-23 16:05 ` [PATCH v2 7/7] maintenance: use 'git config --fixed-value' Derrick Stolee via GitGitGadget 2020-11-23 21:39 ` Junio C Hamano 2020-11-23 22:48 ` Emily Shaffer [this message] 2020-11-23 23:27 ` Junio C Hamano 2020-11-23 19:33 ` [PATCH v2 0/7] config: add --fixed-value option Junio C Hamano 2020-11-25 22:12 ` [PATCH v3 0/8] " Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 1/8] config: convert multi_replace to flags Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 2/8] config: replace 'value_regex' with 'value_pattern' Derrick Stolee via GitGitGadget 2020-11-25 22:50 ` Eric Sunshine 2020-11-25 22:12 ` [PATCH v3 3/8] t1300: test "set all" mode with value-pattern Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 4/8] t1300: add test for --replace-all " Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 5/8] config: add --fixed-value option, un-implemented Derrick Stolee via GitGitGadget 2020-11-25 23:04 ` Eric Sunshine 2020-11-25 22:12 ` [PATCH v3 6/8] config: plumb --fixed-value into config API Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 7/8] config: implement --fixed-value with --get* Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 8/8] maintenance: use 'git config --fixed-value' Derrick Stolee via GitGitGadget 2020-11-25 23:09 ` Junio C Hamano 2020-11-25 23:00 ` [PATCH v3 0/8] config: add --fixed-value option Junio C Hamano 2020-11-26 11:17 ` Derrick Stolee 2020-12-01 4:45 ` 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=20201123224814.GG499823@google.com \ --to=emilyshaffer@google.com \ --cc=Johannes.Schindelin@gmx.de \ --cc=avarab@gmail.com \ --cc=derrickstolee@github.com \ --cc=dstolee@microsoft.com \ --cc=git@vger.kernel.org \ --cc=gitgitgadget@gmail.com \ --cc=jrnieder@gmail.com \ --cc=martin.agren@gmail.com \ --cc=peff@peff.net \ --cc=sandals@crustytoothpaste.net \ --cc=stolee@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
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git