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

Re: [PATCH 2/2] sh-setup: make require_clean_work_tree() work on orphan branches

From
Johannes Sixt <j6t@kdbg.org>
Date
Nov 25, 2015, 06:51 UTC
Message-ID
<56555A5B.20504@kdbg.org>
In-Reply-To
<1448376345-27339-2-git-send-email-szeder@ira.uka.de>
Am 24.11.2015 um 15:45 schrieb SZEDER Gábor:
Show 79 quoted lines
> git-sh-setup's require_clean_work_tree() always exits with error on an
> orphan branch, even when the index and worktree are both clean.  The
> reason is that require_clean_work_tree() starts off with verifying
> HEAD, to make sure that it can safely pass HEAD to 'git diff-index'
> later when it comes to checking the cleanness of the index, and errors
> out finding the invalid HEAD of the orphan branch.
>
> There are scripts requiring a clean worktree that should work on an
> orphan branch as well, and those should be able to use this function
> instead of duplicating its functionality and nice error reporting in a
> way that handles orphan branches.
>
> Fixing this is easy: the index should be compared to the empty tree
> while on an orphan branch, and to HEAD otherwise.
>
> However, just fixing require_clean_work_tree() this way is also
> dangerous, because scripts must take care to work properly on orphan
> branches.  Currently a script calling require_clean_work_tree() would
> exit on a clean orphan branch, but with the simple fix it would
> continue executing and who knows what the consequences might be if
> the script is not prepared for orphan branches.
>
> Let scripts tell git-sh-setup that they are capable of dealing with
> orphan branches by setting the ORPHAN_OK variable, similar to how the
> ability to run in a subdirectory must be signalled by setting
> SUBDIRECTORY_OK.  Only if ORPHAN_OK is set while on an orphan branch
> will require_clean_work_tree() go on and compare the index and
> worktree to the empty tree, otherwise it will exit with error even
> when the index and worktree are clean, i.e the default exit behavior
> doesn't change.
>
> Also provide an error message in the orphan branch/invalid HEAD case
> that is consistent in style with the function's other error messages
> instead of the error message coming straight from 'git rev-parse
> --verify'.
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
>   Documentation/git-sh-setup.txt     |  3 ++-
>   git-sh-setup.sh                    | 16 ++++++++++++++--
>   t/t2301-require-clean-work-tree.sh | 29 +++++++++++++++++++++++++++++
>   3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-sh-setup.txt b/Documentation/git-sh-setup.txt
> index 4f67c4cde6..bccaa2488f 100644
> --- a/Documentation/git-sh-setup.txt
> +++ b/Documentation/git-sh-setup.txt
> @@ -25,7 +25,8 @@ Before sourcing it, your script should set up a few variables;
>   `USAGE` (and `LONG_USAGE`, if any) is used to define message
>   given by `usage()` shell function.  `SUBDIRECTORY_OK` can be set
>   if the script can run from a subdirectory of the working tree
> -(some commands do not).
> +(some commands do not).  `ORPHAN_OK` can be set if the script can
> +work on orphan branches.
>
>   The scriptlet sets `GIT_DIR` and `GIT_OBJECT_DIRECTORY` shell
>   variables, but does *not* export them to the environment.
> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index 4691fbcb64..f45b69386e 100644
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -200,7 +200,19 @@ require_work_tree () {
>   }
>
>   require_clean_work_tree () {
> -	git rev-parse --verify HEAD >/dev/null || exit 1
> +	if git rev-parse --verify HEAD >/dev/null 2>/dev/null
> +	then
> +		compare_to=HEAD
> +	else
> +		if [ -z "$ORPHAN_OK" ]
> +		then
> +			echo >&2 "Cannot $1: Your current branch does not have any commits yet."
> +			exit 1
> +		else
> +			# SHA1 of an empty tree
> +			compare_to=4b825dc642cb6eb9a060e54bf8d69288fbee4904
> +		fi
> +	fi

It is worrysome that this now throws away any error message of rev-parse. A more conservative approach would be to test for -z "$ORPHAN_OK" first and entail new behavior only for the "$ORPHAN_OK" case.

Show 48 quoted lines
>   	git update-index -q --ignore-submodules --refresh
>   	err=0
>
> @@ -210,7 +222,7 @@ require_clean_work_tree () {
>   		err=1
>   	fi
>
> -	if ! git diff-index --cached --quiet --ignore-submodules HEAD --
> +	if ! git diff-index --cached --quiet --ignore-submodules $compare_to --
>   	then
>   		if [ $err = 0 ]
>   		then
> diff --git a/t/t2301-require-clean-work-tree.sh b/t/t2301-require-clean-work-tree.sh
> index 1bb41b59a5..6d0957981e 100755
> --- a/t/t2301-require-clean-work-tree.sh
> +++ b/t/t2301-require-clean-work-tree.sh
> @@ -60,4 +60,33 @@ test_expect_success 'error on dirty index and work tree while on orphan branch'
>   	test_must_fail run_require_clean_work_tree
>   '
>
> +test_expect_success 'ORPHAN_OK - success on clean index and worktree while on orphan branch' '
> +	test_when_finished "git checkout master" &&
> +	git checkout --orphan orphan &&
> +	git reset --hard &&
> +	(
> +		ORPHAN_OK=Yes &&
> +		run_require_clean_work_tree
> +	)
> +'
> +
> +test_expect_success 'ORPHAN_OK - error on dirty index while on orphan branch' '
> +	test_when_finished "git checkout master" &&
> +	git checkout --orphan orphan &&
> +	(
> +		ORPHAN_OK=Yes &&
> +		test_must_fail run_require_clean_work_tree
> +	)
> +'
> +
> +test_expect_success 'ORPHAN_OK - error on dirty index and worktree while on orphan branch' '
> +	test_when_finished "git checkout master" &&
> +	git checkout --orphan orphan &&
> +	echo dirty >file &&
> +	(
> +		ORPHAN_OK=Yes &&
> +		test_must_fail run_require_clean_work_tree
> +	)
> +'
Tests with ORPHAN_OK=Yes, but being on a non-orphaned branch are missing.
-- Hannes
Previous: Junio C HamanoNext: SZEDER Gábor
Message 6 of 11 in “Add tests for git-sh-setup's require_clean_work_tree()”
  1. 1/2 Add tests for git-sh-setup's require_clean_work_tree()SZEDER Gábor, Nov 24, 2015
  2. 2/2 sh-setup: make require_clean_work_tree() work on orphan branchesSZEDER Gábor, Nov 24, 2015
  3. Jeff KingNov 24, 2015
  4. SZEDER GáborNov 30, 2015
  5. Junio C HamanoDec 2, 2015
  6. Johannes SixtNov 25, 2015
  7. SZEDER GáborNov 30, 2015
  8. Junio C HamanoJan 21, 2016
  9. Junio C HamanoJan 20, 2016
  10. SZEDER GáborJan 21, 2016
  11. Junio C HamanoJan 21, 2016

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.