git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/3] check-ref-format --branch: do not expand @{...} outside repository

From
Jonathan Nieder <jrnieder@gmail.com>
Date
Oct 17, 2017, 07:08 UTC
Message-ID
<20171017070808.plddffhzdobyekmo@aiede.mtv.corp.google.com>
In-Reply-To
<20171017070619.bdgvujmbw4uxvwle@aiede.mtv.corp.google.com>
From: Junio C Hamano <gitster@pobox.com>

Running "git check-ref-format --branch @{-1}" from outside any repository produces

	$ git check-ref-format --branch @{-1}
	BUG: environment.c:182: git environment hasn't been setup

This is because the expansion of @{-1} must come from the HEAD reflog, which involves opening the repository. @{u} and @{push} (which are more unusual because they typically would not expand to a local branch) trigger the same assertion.

This has been broken since day one. Before v2.13.0-rc0~48^2 (setup_git_env: avoid blind fall-back to ".git", 2016-10-02), the breakage was more subtle: Git would read reflogs from ".git" within the current directory even if it was not a valid repository. Usually that is harmless because Git is not being run from the root directory of an invalid repository, but in edge cases such accesses can be confusing or harmful. Since v2.13.0, the problem is easier to diagnose because Git aborts with a BUG message.

Erroring out is the right behavior: when asked to interpret a branch name like "@{-1}", there is no reasonable answer in this context. But we should print a message saying so instead of an assertion failure.

We do not forbid "check-ref-format --branch" from outside a repository altogether because it is ok for a script to pre-process branch arguments without @{...} in such a context. For example, with pre-2.13 Git, a script that does

	branch='master'; # default value
	parse_options
	branch=$(git check-ref-format --branch "$branch")

to normalize an optional branch name provided by the user would work both inside a repository (where the user could provide '@{-1}') and outside (where '@{-1}' should not be accepted).

So disable the "expand @{...}" half of the feature when run outside a repository, but keep the check of the syntax of a proposed branch name. This way, when run from outside a repository, "git check-ref-format --branch @{-1}" will gracefully fail:

	$ git check-ref-format --branch @{-1}
	fatal: '@{-1}' is not a valid branch name
and "git check-ref-format --branch master" will succeed as before:
	$ git check-ref-format --branch master
	master
restoring the usual pre-2.13 behavior.
[jn: split out from a larger patch; moved conditional to
 strbuf_check_branch_ref instead of its caller; fleshed out commit
 message; some style tweaks in tests]
Reported-by: Marko Kungla <marko.kungla@gmail.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 sha1_name.c                 |  5 ++++-
 t/t1402-check-ref-format.sh | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index c7c5ab376c..603e667faa 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1331,7 +1331,10 @@ void strbuf_branchname(struct strbuf *sb, const char *name, unsigned allowed)
 
 int strbuf_check_branch_ref(struct strbuf *sb, const char *name)
 {
-	strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+	if (startup_info->have_repository)
+		strbuf_branchname(sb, name, INTERPRET_BRANCH_LOCAL);
+	else
+		strbuf_addstr(sb, name);
 	if (name[0] == '-')
 		return -1;
 	strbuf_splice(sb, 0, 0, "refs/heads/", 11);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 0790edf60d..98e4a8613b 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -144,6 +144,11 @@ test_expect_success "check-ref-format --branch @{-1}" '
 	refname2=$(git check-ref-format --branch @{-2}) &&
 	test "$refname2" = master'
 
+test_expect_success 'check-ref-format --branch -naster' '
+	test_must_fail git check-ref-format --branch -naster >actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success 'check-ref-format --branch from subdir' '
 	mkdir subdir &&
 
@@ -161,6 +166,17 @@ test_expect_success 'check-ref-format --branch from subdir' '
 	test "$refname" = "$sha1"
 '
 
+test_expect_success 'check-ref-format --branch @{-1} from non-repo' '
+	nongit test_must_fail git check-ref-format --branch @{-1} >actual &&
+	test_must_be_empty actual
+'
+
+test_expect_success 'check-ref-format --branch master from non-repo' '
+	echo master >expect &&
+	nongit git check-ref-format --branch master >actual &&
+	test_cmp expect actual
+'
+
 valid_ref_normalized() {
 	prereq=
 	case $1 in
-- 
2.15.0.rc1.287.g2b38de12cc
Previous: Jonathan NiederNext: Jonathan Nieder
Message 20 of 36 in “bug with check-ref-format outside of repository”
  1. Marko KunglaJul 8, 2017
  2. Jeff KingJul 14, 2017
  3. check-ref-format: require a repository for --branchJeff King, Jul 14, 2017
  4. Jonathan NiederJul 17, 2017
  5. Marko KunglaJul 17, 2017
  6. Jeff KingAug 17, 2017
  7. Jeff KingAug 17, 2017
  8. Junio C HamanoAug 17, 2017
  9. Jeff KingAug 18, 2017
  10. Junio C HamanoAug 18, 2017
  11. Junio C HamanoOct 16, 2017
  12. Junio C HamanoOct 16, 2017
  13. Jeff KingOct 16, 2017
  14. Jonathan NiederOct 16, 2017
  15. Junio C HamanoOct 17, 2017
  16. Jeff KingOct 17, 2017
  17. Junio C HamanoOct 17, 2017
  18. Junio C HamanoOct 17, 2017
  19. 0/3 Re: [PATCH] check-ref-format: require a repository for --branchJonathan Nieder, Oct 17, 2017
  20. 1/3 check-ref-format --branch: do not expand @{...} outside repositoryJonathan Nieder, Oct 17, 2017
  21. 2/3 check-ref-format --branch: strip refs/heads/ using skip_prefixJonathan Nieder, Oct 17, 2017
  22. Junio C HamanoOct 17, 2017
  23. Jonathan NiederOct 17, 2017
  24. Junio C HamanoOct 17, 2017
  25. 3/3 check-ref-format doc: --branch validates and expands <branch>Jonathan Nieder, Oct 17, 2017
  26. Junio C HamanoOct 17, 2017
  27. Jonathan NiederOct 17, 2017
  28. Jeff KingOct 18, 2017
  29. Kevin DaudtOct 17, 2017
  30. Junio C HamanoOct 17, 2017
  31. Kevin DaudtOct 17, 2017
  32. Jeff KingOct 16, 2017
  33. Jonathan NiederOct 17, 2017
  34. Junio C HamanoOct 17, 2017
  35. Jonathan NiederOct 17, 2017
  36. Jonathan NiederOct 17, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.