[PATCH 2/3] dim: Introduce dim_request_pull_tags

[Vivi, Rodrigo]

> On Aug 27, 2018, at 1:34 AM, Nikula, Jani <jani.nikula at intel.com> wrote:
> 
>> On Mon, 27 Aug 2018, Daniel Vetter <daniel at ffwll.ch> wrote:
>>> On Fri, Aug 24, 2018 at 02:43:35PM -0700, Rodrigo Vivi wrote:
>>> Besides drm-intel-next, sometimes it is used to re-use an
>>> already existent tag that you just created outside
>>> or in cases where mutt/smtp goof-up you don't want
>>> to generate another identical tag with another date
>>> or with "-1", "-2", etc
>>> 
>>> Suggested-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
>>> Cc: Jani Nikula <jani.nikula at intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>>> ---
>>> dim     | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>> dim.rst |  6 ++++++
>>> 2 files changed, 51 insertions(+)
>>> 
>>> diff --git a/dim b/dim
>>> index fec51f766e55..b4d7996a1072 100755
>>> --- a/dim
>>> +++ b/dim
>>> @@ -1882,6 +1882,51 @@ function dim_tag_next
>>>    dim_tag_branch "drm-intel-next"
>>> }
>>> 
>>> +function check_tags # tags
>>> +{
>>> +    local tag
>>> +
>>> +    for tag in "$@";
>>> +    do
>>> +        if ! git rev-parse "$tag^{tag}" > /dev/null 2>&1; then
>>> +            echoerr "Aborting: Tag ${tag} not found"
>>> +            exit 1
>>> +        else
>>> +            echo $tag
>>> +        fi
>>> +    done
>>> +}
>>> +
>>> +# dim_pull_request_tag branch upstream tag-list
>>> +function dim_pull_request_tags
>>> +{
>>> +    local branch upstream repo req_file url_list git_url tags tag
>>> +
>>> +    branch=${1:?$usage}
>>> +    shift
>>> +    upstream=${1:?$usage}
>>> +    shift
>>> +    tags="$*"
>>> +    repo=$(branch_to_repo $branch)
>>> +    req_file=$(mktemp)
>>> +
>>> +    git_fetch_helper ${upstream%%/*}
>>> +    echo "Using $upstream as the upstream"
>>> +
>>> +    # Sort with newest first
>>> +    tags=$(check_tags $tags | sort -r)
>>> +    tag=$(echo $tags | cut -d " " -f 1)
>>> +
>>> +    prep_pull_mail $req_file $tags
>>> +
>>> +    url_list=${drm_tip_repos[$repo]}
>>> +    git_url=$(pick_protocol_url git $url_list)
>>> +
>>> +    git request-pull $upstream $git_url $tag >> $req_file
>>> +    $DRY $DIM_MUA -s "[PULL] $branch" \
>>> +         -i $req_file "${dim_pull_request_recipients[@]}"
>>> +}
>> 
>> s/tags/tag/ everywhere, since it's just a single tag, not plural. That
>> also means you can simplify the code a lot. Or I'm missing why you want to
>> send a pull request for multiple tags at the same time?

Because drm-Intel-next flow....

Just making it generic so I can move specific stuff for drm-Intel-next into its own function...

>> 
>> Same for the help text below.
>> 
>> Aside: Autocomplete for this would be _very_ nifty. Should be possible to
>> wire up the git tag completion function into our own autocomplete.

Oh yeah, I forgot that, sorry!

>> 
>>> +
>>> # dim_pull_request branch upstream
>>> function dim_pull_request
>>> {
>>> diff --git a/dim.rst b/dim.rst
>>> index 6d7528ce497f..1a9bed464021 100644
>>> --- a/dim.rst
>>> +++ b/dim.rst
>>> @@ -292,6 +292,12 @@ be merged have been added, in order to help maintainers with deciding which tree
>>> is in need of a pull request. Commiters that want to check the status of their
>>> current branch should use normal **git status** commands.
>>> 
>>> +pull-request-tags *branch* *upstream* *tags*
>>> +--------------------------------------------
>>> +Fetches the *upstream* remote to make sure it's up-to-date. Based on the given
>>> +*tags*, it generates a pull request template with the specified *upstream*,
>>> +and finally is starts \$DIM_MUA with the template with subject and
>>> +recipients already set.
>> 
>> I'd elaborate here a bit more.
>> 
>> "This can be used if the final steps of *pull-request* failed, or together
>> with *tag-branch* to separate the pull request generation from tag
>> creation."
>> 
>> I'd also add a hint to the help text for tag-branch about this new
>> command.
> 
> So what's the purpose here, really?

Allow to re-use tags when you know they are good. Keep old tag regeneration as the main path.

> We're carefully generating a new tag
> specifically to avoid re-using existing tags

Why?
Really, what’s the reason of forcing it without allowing to re-use?!


> , and now you want to allow
> it again?

Yes, I do.

> What are these "already existent tag that you just created
> outside"?

I was going to ask Who told I created outside?! And then I noticed this came from the poor commit message. I never created any tag from outside. All of them were always created with dim.
So I probably need to change commit message to make it clear.

> 
> I've seen these -1 suffix tags from Rodrigo, but I don't recall from
> anyone else.

Apparently I’m the only Goofy maintainer around, and I lost a day to fix the tool flow for 100% of the cases I faced here, so, why just mock it at first sight without considering it?!

> What goes wrong? Should we focus on fixing that instead?

Another way to solve the -1 case is checking of tag exist and reuse or delete and recreate...
But this doesn’t cover 100% of my exclusive goof-ups.

Last week I did a pull request of drm-Intel-next-fixes and for me everything worked fine and mail “went” through... I just realized that pull didn’t really go through when Dave sent his to Linus without our stuff.

I really don’t see any reason why I need to regenerate another tag to send a pull request if I’m sure that I already have an already existent tag that is good.

This can be same day, this can be different day, whatever. I just want a reliable path where I don’t need to regenerate tag.

Some cases that apparently only I face here that can mess things up:
- Machine hang in the middle of pull request
- Receive a f2f hard ping and get abducted to a meeting and after returning don’t even know why mutt is opened there and closes it. Or after the meeting closes the laptop to go home and suspend/resume fails and it reboots.
- Realize that you need to do something else before (like checking CI first) and type ctrl-c immediately after hitting enter, but this tag command with *push* goes faster... 
- mutt/smtp/proxy simply don’t work and don’t give a feedback closing and making you to believe email went through.


> 
> BR,
> Jani.
> 
> 
> 
>> 
>> Aside from the minor polish lgtm.
>> -Daniel

Thanks

>> 
>>> 
>>> pull-request *branch* *upstream*
>>> --------------------------------
>>> -- 
>>> 2.17.1
>>> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center