From a4e21fb4dc0ee1783b94be78fd91b73df18992f3 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 26 Feb 2016 18:26:32 -0500 Subject: [PATCH 1/2] t1515: add tests for rev-parse out-of-repo helpers The git-rev-parse command is a dumping ground for helpers that let scripts make various queries of git. Many of these are conceptually independent of being inside a git repository. With the exception of --parseopt, we do not directly test most of these features in our test suite. Let's give them some basic sanity checks, which reveals that some of them have been broken for some time when run from outside a repository. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/t1515-rev-parse-outside-repo.sh | 45 +++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100755 t/t1515-rev-parse-outside-repo.sh diff --git a/t/t1515-rev-parse-outside-repo.sh b/t/t1515-rev-parse-outside-repo.sh new file mode 100755 index 0000000000..ae330931b7 --- /dev/null +++ b/t/t1515-rev-parse-outside-repo.sh @@ -0,0 +1,45 @@ +#!/bin/sh + +test_description='check that certain rev-parse options work outside repo' +. ./test-lib.sh + +test_expect_success 'set up non-repo directory' ' + GIT_CEILING_DIRECTORIES=$(pwd) && + export GIT_CEILING_DIRECTORIES && + mkdir non-repo && + cd non-repo && + # confirm that git does not find a repo + test_must_fail git rev-parse --git-dir +' + +# Rather than directly test the output of sq-quote directly, +# make sure the shell can read back a tricky case, since +# that's what we really care about anyway. +tricky="really tricky with \\ and \" and '" +dump_args () { + for i in "$@"; do + echo "arg: $i" + done +} +test_expect_success 'rev-parse --sq-quote' ' + dump_args "$tricky" easy >expect && + eval "dump_args $(git rev-parse --sq-quote "$tricky" easy)" >actual && + test_cmp expect actual +' + +test_expect_failure 'rev-parse --local-env-vars' ' + git rev-parse --local-env-vars >actual && + # we do not want to depend on the complete list here, + # so just look for something plausible + grep ^GIT_DIR actual +' + +test_expect_failure 'rev-parse --resolve-git-dir' ' + git init --separate-git-dir repo dir && + test_must_fail git rev-parse --resolve-git-dir . && + echo "$(pwd)/repo" >expect && + git rev-parse --resolve-git-dir dir/.git >actual && + test_cmp expect actual +' + +test_done From fc7d47f0dd4f041af2ed2004700f18b635f3b204 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Mon, 29 Feb 2016 06:01:56 -0500 Subject: [PATCH 2/2] rev-parse: let some options run outside repository Once upon a time, you could use "--local-env-vars" and "--resolve-git-dir" outside of any git repository, but they had to come first on the command line. Commit 68889b4 (rev-parse: remove restrictions on some options, 2013-07-21) put them into the normal option-parsing loop, fixing the latter. But it inadvertently broke the former, as we call setup_git_directory() before starting that loop. We can note that those options don't care even conditionally about whether we are in a git repo. So it's fine if we simply wait to setup the repo until we see an option that needs it. However, there is one special exception we should make: historically, rev-parse will set up the repository and read config even if there are _no_ options. Some of the tests in t1300 rely on this to check "git -c $config" parsing. That's not mirroring real-world use, and we could tweak the test. But t0002 uses a bare "git rev-parse" to check "are we in a git repository?". It's plausible that real-world scripts are relying on this. So let's cover this case specially, and treat an option-less "rev-parse" as "see if we're in a repo". Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/rev-parse.c | 50 ++++++++++++++++++++----------- t/t1515-rev-parse-outside-repo.sh | 4 +-- 2 files changed, 34 insertions(+), 20 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 7e074aad40..3116d29d83 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -505,6 +505,7 @@ N_("git rev-parse --parseopt [] -- [...]\n" int cmd_rev_parse(int argc, const char **argv, const char *prefix) { int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0; + int did_repo_setup = 0; int has_dashdash = 0; int output_prefix = 0; unsigned char sha1[20]; @@ -528,11 +529,40 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) } } - prefix = setup_git_directory(); - git_config(git_default_config, NULL); + /* No options; just report on whether we're in a git repo or not. */ + if (argc == 1) { + setup_git_directory(); + git_config(git_default_config, NULL); + return 0; + } + for (i = 1; i < argc; i++) { const char *arg = argv[i]; + if (!strcmp(arg, "--local-env-vars")) { + int i; + for (i = 0; local_repo_env[i]; i++) + printf("%s\n", local_repo_env[i]); + continue; + } + if (!strcmp(arg, "--resolve-git-dir")) { + const char *gitdir = argv[++i]; + if (!gitdir) + die("--resolve-git-dir requires an argument"); + gitdir = resolve_gitdir(gitdir); + if (!gitdir) + die("not a gitdir '%s'", argv[i]); + puts(gitdir); + continue; + } + + /* The rest of the options require a git repository. */ + if (!did_repo_setup) { + prefix = setup_git_directory(); + git_config(git_default_config, NULL); + did_repo_setup = 1; + } + if (!strcmp(arg, "--git-path")) { if (!argv[i + 1]) die("--git-path requires an argument"); @@ -706,12 +736,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) add_ref_exclusion(&ref_excludes, arg + 10); continue; } - if (!strcmp(arg, "--local-env-vars")) { - int i; - for (i = 0; local_repo_env[i]; i++) - printf("%s\n", local_repo_env[i]); - continue; - } if (!strcmp(arg, "--show-toplevel")) { const char *work_tree = get_git_work_tree(); if (work_tree) @@ -766,16 +790,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) puts(get_git_common_dir()); continue; } - if (!strcmp(arg, "--resolve-git-dir")) { - const char *gitdir = argv[++i]; - if (!gitdir) - die("--resolve-git-dir requires an argument"); - gitdir = resolve_gitdir(gitdir); - if (!gitdir) - die("not a gitdir '%s'", argv[i]); - puts(gitdir); - continue; - } if (!strcmp(arg, "--is-inside-git-dir")) { printf("%s\n", is_inside_git_dir() ? "true" : "false"); diff --git a/t/t1515-rev-parse-outside-repo.sh b/t/t1515-rev-parse-outside-repo.sh index ae330931b7..3ec2971ee5 100755 --- a/t/t1515-rev-parse-outside-repo.sh +++ b/t/t1515-rev-parse-outside-repo.sh @@ -27,14 +27,14 @@ test_expect_success 'rev-parse --sq-quote' ' test_cmp expect actual ' -test_expect_failure 'rev-parse --local-env-vars' ' +test_expect_success 'rev-parse --local-env-vars' ' git rev-parse --local-env-vars >actual && # we do not want to depend on the complete list here, # so just look for something plausible grep ^GIT_DIR actual ' -test_expect_failure 'rev-parse --resolve-git-dir' ' +test_expect_success 'rev-parse --resolve-git-dir' ' git init --separate-git-dir repo dir && test_must_fail git rev-parse --resolve-git-dir . && echo "$(pwd)/repo" >expect &&