Subject: Re: [GSoC] [PATCH v2 2/4] submodule: prefix die messages with 'fatal'

From: Đoàn Trần Công Danh <[email-masked]>
Date: Fri, 9 Jul 2021 21:52:29 +0700

On 2021-07-08 15:25:31+0530, Atharva Raykar <[email-masked]> wrote:
> The standard `die()` function that is used in C code prefixes all the
> messages passed to it with 'fatal: '. This does not happen with the
> `die` used in 'git-submodule.sh'.
> 
> Let's prefix each of the shell die messages with 'fatal: ' so that when
> they are converted to C code, the error messages stay the same as before
> the conversion.

That sounds good.

> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -147,7 +147,7 @@ cmd_add()
>  
>  	if ! git submodule--helper config --check-writeable >/dev/null 2>&1
>  	then
> -		 die "$(eval_gettext "please make sure that the .gitmodules file is in the working tree")"
> +		 die "$(eval_gettext "fatal: please make sure that the .gitmodules file is in the working tree")"

Except that, "fatal: " isn't subjected to translation. And this will
create new translatable item for translator. Perhaps:

-		 die "$(eval_gettext "please make sure that the .gitmodules file is in the working tree")"
+		 die "fatal: $(eval_gettext "please make sure that the .gitmodules file is in the working tree")"

-- Danh

>  	fi
>  
>  	if test -n "$reference_path"
> @@ -176,7 +176,7 @@ cmd_add()
>  	case "$repo" in
>  	./*|../*)
>  		test -z "$wt_prefix" ||
> -		die "$(gettext "Relative path can only be used from the toplevel of the working tree")"
> +		die "$(gettext "fatal: Relative path can only be used from the toplevel of the working tree")"
>  
>  		# dereference source url relative to parent's url
>  		realrepo=$(git submodule--helper resolve-relative-url "$repo") || exit
> @@ -186,7 +186,7 @@ cmd_add()
>  		realrepo=$repo
>  		;;
>  	*)
> -		die "$(eval_gettext "repo URL: '\$repo' must be absolute or begin with ./|../")"
> +		die "$(eval_gettext "fatal: repo URL: '\$repo' must be absolute or begin with ./|../")"
>  	;;
>  	esac
>  
> @@ -205,17 +205,17 @@ cmd_add()
>  	if test -z "$force"
>  	then
>  		git ls-files --error-unmatch "$sm_path" > /dev/null 2>&1 &&
> -		die "$(eval_gettext "'\$sm_path' already exists in the index")"
> +		die "$(eval_gettext "fatal: '\$sm_path' already exists in the index")"
>  	else
>  		git ls-files -s "$sm_path" | sane_grep -v "^160000" > /dev/null 2>&1 &&
> -		die "$(eval_gettext "'\$sm_path' already exists in the index and is not a submodule")"
> +		die "$(eval_gettext "fatal: '\$sm_path' already exists in the index and is not a submodule")"
>  	fi
>  
>  	if test -d "$sm_path" &&
>  		test -z $(git -C "$sm_path" rev-parse --show-cdup 2>/dev/null)
>  	then
>  	    git -C "$sm_path" rev-parse --verify -q HEAD >/dev/null ||
> -	    die "$(eval_gettext "'\$sm_path' does not have a commit checked out")"
> +	    die "$(eval_gettext "fatal: '\$sm_path' does not have a commit checked out")"
>  	fi
>  
>  	if test -z "$force"
> @@ -238,7 +238,7 @@ cmd_add()
>  
>  	if ! git submodule--helper check-name "$sm_name"
>  	then
> -		die "$(eval_gettext "'$sm_name' is not a valid submodule name")"
> +		die "$(eval_gettext "fatal: '$sm_name' is not a valid submodule name")"
>  	fi
>  
>  	# perhaps the path exists and is already a git repo, else clone it
> @@ -281,7 +281,7 @@ or you are unsure what this means choose another name with the '--name' option."
>  	git config submodule."$sm_name".url "$realrepo"
>  
>  	git add --no-warn-embedded-repo $force "$sm_path" ||
> -	die "$(eval_gettext "Failed to add submodule '\$sm_path'")"
> +	die "$(eval_gettext "fatal: Failed to add submodule '\$sm_path'")"
>  
>  	git submodule--helper config submodule."$sm_name".path "$sm_path" &&
>  	git submodule--helper config submodule."$sm_name".url "$repo" &&
> @@ -290,7 +290,7 @@ or you are unsure what this means choose another name with the '--name' option."
>  		git submodule--helper config submodule."$sm_name".branch "$branch"
>  	fi &&
>  	git add --force .gitmodules ||
> -	die "$(eval_gettext "Failed to register submodule '\$sm_path'")"
> +	die "$(eval_gettext "fatal: Failed to register submodule '\$sm_path'")"
>  
>  	# NEEDSWORK: In a multi-working-tree world, this needs to be
>  	# set in the per-worktree config.
> @@ -565,7 +565,7 @@ cmd_update()
>  		else
>  			subsha1=$(sanitize_submodule_env; cd "$sm_path" &&
>  				git rev-parse --verify HEAD) ||
> -			die "$(eval_gettext "Unable to find current revision in submodule path '\$displaypath'")"
> +			die "$(eval_gettext "fatal: Unable to find current revision in submodule path '\$displaypath'")"
>  		fi
>  
>  		if test -n "$remote"
> @@ -575,12 +575,12 @@ cmd_update()
>  			then
>  				# Fetch remote before determining tracking $sha1
>  				fetch_in_submodule "$sm_path" $depth ||
> -				die "$(eval_gettext "Unable to fetch in submodule path '\$sm_path'")"
> +				die "$(eval_gettext "fatal: Unable to fetch in submodule path '\$sm_path'")"
>  			fi
>  			remote_name=$(sanitize_submodule_env; cd "$sm_path" && git submodule--helper print-default-remote)
>  			sha1=$(sanitize_submodule_env; cd "$sm_path" &&
>  				git rev-parse --verify "${remote_name}/${branch}") ||
> -			die "$(eval_gettext "Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
> +			die "$(eval_gettext "fatal: Unable to find current \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
>  		fi
>  
>  		if test "$subsha1" != "$sha1" || test -n "$force"
> @@ -604,36 +604,36 @@ cmd_update()
>  				# not be reachable from any of the refs
>  				is_tip_reachable "$sm_path" "$sha1" ||
>  				fetch_in_submodule "$sm_path" "$depth" "$sha1" ||
> -				die "$(eval_gettext "Fetched in submodule path '\$displaypath', but it did not contain \$sha1. Direct fetching of that commit failed.")"
> +				die "$(eval_gettext "fatal: Fetched in submodule path '\$displaypath', but it did not contain \$sha1. Direct fetching of that commit failed.")"
>  			fi
>  
>  			must_die_on_failure=
>  			case "$update_module" in
>  			checkout)
>  				command="git checkout $subforce -q"
> -				die_msg="$(eval_gettext "Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
> +				die_msg="$(eval_gettext "fatal: Unable to checkout '\$sha1' in submodule path '\$displaypath'")"
>  				say_msg="$(eval_gettext "Submodule path '\$displaypath': checked out '\$sha1'")"
>  				;;
>  			rebase)
>  				command="git rebase ${GIT_QUIET:+--quiet}"
> -				die_msg="$(eval_gettext "Unable to rebase '\$sha1' in submodule path '\$displaypath'")"
> +				die_msg="$(eval_gettext "fatal: Unable to rebase '\$sha1' in submodule path '\$displaypath'")"
>  				say_msg="$(eval_gettext "Submodule path '\$displaypath': rebased into '\$sha1'")"
>  				must_die_on_failure=yes
>  				;;
>  			merge)
>  				command="git merge ${GIT_QUIET:+--quiet}"
> -				die_msg="$(eval_gettext "Unable to merge '\$sha1' in submodule path '\$displaypath'")"
> +				die_msg="$(eval_gettext "fatal: Unable to merge '\$sha1' in submodule path '\$displaypath'")"
>  				say_msg="$(eval_gettext "Submodule path '\$displaypath': merged in '\$sha1'")"
>  				must_die_on_failure=yes
>  				;;
>  			!*)
>  				command="${update_module#!}"
> -				die_msg="$(eval_gettext "Execution of '\$command \$sha1' failed in submodule path '\$displaypath'")"
> +				die_msg="$(eval_gettext "fatal: Execution of '\$command \$sha1' failed in submodule path '\$displaypath'")"
>  				say_msg="$(eval_gettext "Submodule path '\$displaypath': '\$command \$sha1'")"
>  				must_die_on_failure=yes
>  				;;
>  			*)
> -				die "$(eval_gettext "Invalid update mode '$update_module' for submodule path '$path'")"
> +				die "$(eval_gettext "fatal: Invalid update mode '$update_module' for submodule path '$path'")"
>  			esac
>  
>  			if (sanitize_submodule_env; cd "$sm_path" && $command "$sha1")
> @@ -660,7 +660,7 @@ cmd_update()
>  			res=$?
>  			if test $res -gt 0
>  			then
> -				die_msg="$(eval_gettext "Failed to recurse into submodule path '\$displaypath'")"
> +				die_msg="$(eval_gettext "fatal: Failed to recurse into submodule path '\$displaypath'")"
>  				if test $res -ne 2
>  				then
>  					err="${err};$die_msg"
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 7aa7fefdfa..cb1b8e35db 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -51,7 +51,7 @@ test_expect_success 'submodule update aborts on missing gitmodules url' '
>  
>  test_expect_success 'add aborts on repository with no commits' '
>  	cat >expect <<-\EOF &&
> -	'"'repo-no-commits'"' does not have a commit checked out
> +	fatal: '"'repo-no-commits'"' does not have a commit checked out
>  	EOF
>  	git init repo-no-commits &&
>  	test_must_fail git submodule add ../a ./repo-no-commits 2>actual &&
> @@ -199,7 +199,7 @@ test_expect_success 'submodule add to .gitignored path with --force' '
>  test_expect_success 'submodule add to path with tracked content fails' '
>  	(
>  		cd addtest &&
> -		echo "'\''dir-tracked'\'' already exists in the index" >expect &&
> +		echo "fatal: '\''dir-tracked'\'' already exists in the index" >expect &&
>  		mkdir dir-tracked &&
>  		test_commit foo dir-tracked/bar &&
>  		test_must_fail git submodule add "$submodurl" dir-tracked >actual 2>&1 &&
> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
> index f4f61fe554..11cccbb333 100755
> --- a/t/t7406-submodule-update.sh
> +++ b/t/t7406-submodule-update.sh
> @@ -448,7 +448,7 @@ test_expect_success 'fsck detects command in .gitmodules' '
>  '
>  
>  cat << EOF >expect
> -Execution of 'false $submodulesha1' failed in submodule path 'submodule'
> +fatal: Execution of 'false $submodulesha1' failed in submodule path 'submodule'
>  EOF
>  
>  test_expect_success 'submodule update - command in .git/config catches failure' '
> @@ -465,7 +465,7 @@ test_expect_success 'submodule update - command in .git/config catches failure'
>  '
>  
>  cat << EOF >expect
> -Execution of 'false $submodulesha1' failed in submodule path '../submodule'
> +fatal: Execution of 'false $submodulesha1' failed in submodule path '../submodule'
>  EOF
>  
>  test_expect_success 'submodule update - command in .git/config catches failure -- subdirectory' '
> @@ -484,7 +484,7 @@ test_expect_success 'submodule update - command in .git/config catches failure -
>  
>  test_expect_success 'submodule update - command run for initial population of submodule' '
>  	cat >expect <<-EOF &&
> -	Execution of '\''false $submodulesha1'\'' failed in submodule path '\''submodule'\''
> +	fatal: Execution of '\''false $submodulesha1'\'' failed in submodule path '\''submodule'\''
>  	EOF
>  	rm -rf super/submodule &&
>  	test_must_fail git -C super submodule update 2>actual &&
> @@ -493,8 +493,8 @@ test_expect_success 'submodule update - command run for initial population of su
>  '
>  
>  cat << EOF >expect
> -Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
> -Failed to recurse into submodule path '../super'
> +fatal: Execution of 'false $submodulesha1' failed in submodule path '../super/submodule'
> +fatal: Failed to recurse into submodule path '../super'
>  EOF
>  
>  test_expect_success 'recursive submodule update - command in .git/config catches failure -- subdirectory' '
> -- 
> 2.32.0
> 

-- 
Danh