Re: [dim PATCH 2/6] dim: url_to_remote can't normally fail

Daniel Vetter
2017-10-05T17:05:18Z

On Tue, Oct 03, 2017 at 04:38:04PM +0300, Jani Nikula 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.
>
> While at it, change the exit to a return. set -e at the top takes care
> of aborting.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Sean Paul <seanpaul@chromium.org>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Jani Nikula <jani.nikula@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."

So this practically makes worktrees mandatory since we now need all the
remotes everywhere. I'm ok with that, worktree is the default (with no
easy way to intentionally fall back) anyway. Might be good to mention in
the commit message though.
-Daniel

>
> 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)
>
> --
> 2.11.0
>

--
Daniel Vetter
Software Engineer, Intel Corporation