[Intel-gfx] [dim PATCH 2/6] dim: url_to_remote can't normally fail

[Jani Nikula]

On Tue, 03 Oct 2017, Jani Nikula <jani.nikula at intel.com> wrote:
> Since commit cad37e1910f9 ("dim: auto-add remotes") url_to_remote can't
> really fail. Same for repo_to_remote when the repo exists. Redirecting
> their output when the remote isn't there leads to url_to_remote waiting
> for user input without prompting, giving an appearance of a hang.

Wah. I've been going back and forth with this, but I'm now leaning
towards reverting cad37e1910f9 ("dim: auto-add remotes") and going back
to the drawing board with it.

The problem is, url_to_remote, repo_to_remote and friends are low-level
helpers in the script. That kind of things are better off returning
true/false status instead of going out of their way to interactively fix
stuff up. The *caller*, where applicable, should auto-add remotes,
depending on the case.

I hit the problem first on one machine which doesn't use worktrees. I
conceded we could make worktrees a requirement. However, I have a strict
separation between maintainer and developer repos, with no push access
at all to the remotes from the developer repo. I have no desire to give
up on that safety measure. However, dim is helpful in the developer repo
too.

Also, there's been requests to move dim more towards working in the CWD
instead of cd to predefined repo. I'm not sure it's reasonable to
require that all of them have all of the remotes available, for no
particularly good reason. With worktrees the remotes are shared, but if
you don't use a common tree for all of them, that benefit is lost.

BR,
Jani.



>
> While at it, change the exit to a return. set -e at the top takes care
> of aborting.
>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>  dim | 24 ++++++++----------------
>  1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/dim b/dim
> index 7832ddca692c..ae8f30b8db83 100755
> --- a/dim
> +++ b/dim
> @@ -282,7 +282,7 @@ function url_to_remote # url [url ...]
>  		echoerr "Please set it up yourself using:"
>  		echoerr "    $ git remote add <name> $url"
>  		echoerr "with a name of your choice."
> -		exit 1
> +		return 1
>  	fi
>  
>  	git remote add $remote $url
> @@ -1749,10 +1749,7 @@ function dim_list_upstreams
>  {
>  	local dim_drm_upstream_remote
>  
> -	# Handle failures gracefully
> -	if ! dim_drm_upstream_remote=$(url_to_remote $drm_upstream_git 2>/dev/null); then
> -		return 0
> -	fi
> +	dim_drm_upstream_remote=$(url_to_remote $drm_upstream_git)
>  
>  	echo origin/master
>  	echo $dim_drm_upstream_remote/drm-next
> @@ -1772,17 +1769,14 @@ function dim_update_branches
>  
>  	cd $DIM_PREFIX/$DIM_DRM_INTEL
>  
> -	if remote=$(url_to_remote $linux_upstream_git 2>/dev/null); then
> -		echo -n "Fetching linux (local remote $remote)... "
> -		git_fetch_helper $remote
> -		echo "Done."
> -	fi
> +	remote=$(url_to_remote $linux_upstream_git)
> +	echo -n "Fetching linux (local remote $remote)... "
> +	git_fetch_helper $remote
> +	echo "Done."
>  
>  	for repo in "${!drm_tip_repos[@]}"; do
>  		url_list=${drm_tip_repos[$repo]}
> -		if ! remote=$(url_to_remote $url_list 2>/dev/null); then
> -			continue
> -		fi
> +		remote=$(url_to_remote $url_list)
>  		echo -n "Fetching $repo (local remote $remote)... "
>  		git_fetch_helper $remote
>  		echo "Done."
> @@ -1826,9 +1820,7 @@ function dim_status
>  
>  	for branch in $dim_branches ; do
>  		repo=$(branch_to_repo $branch)
> -		if ! remote=$(repo_to_remote $repo) ; then
> -			continue
> -		fi
> +		remote=$(repo_to_remote $repo)
>  
>  		patches=$(git log --oneline $remote/$branch ^origin/master ^$drm_remote/drm-next ^$drm_remote/drm-fixes | wc -l)

-- 
Jani Nikula, Intel Open Source Technology Center