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

Jani Nikula
2017-10-06T08:54:51Z

On Thu, 05 Oct 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> 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.

Added a paragraph about that in the log. Worktrees still aren't strictly
required, but indeed practically mandatory. I still have that one box
with a non-worktree setup even if the git has long since been upgraded
to support worktrees.

Pushed all 7, many thanks for the review.

BR,
Jani.

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

--
Jani Nikula, Intel Open Source Technology Center