[dim PATCH 4/7] dim: abstract helper for listing unmerged tags

[Jani Nikula]

On Thu, 06 Dec 2018, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Wed, Dec 05, 2018 at 05:02:56PM +0200, Jani Nikula wrote:
>> Make the pull request code a bit easier to grasp. Use git log pretty
>> format for easier extraction of the tags.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>  dim | 18 +++++++++++++++++-
>>  1 file changed, 17 insertions(+), 1 deletion(-)
>> 
>> diff --git a/dim b/dim
>> index b227f0db99dc..e0f96f78c858 100755
>> --- a/dim
>> +++ b/dim
>> @@ -345,6 +345,22 @@ function git_branch_exists # branch
>>  	fi
>>  }
>>  
>> +# $1: branch
>> +# $2: upstream
>> +function git_unmerged_tags
>> +{
>> +	local branch upstream
>> +
>> +	branch=$1
>> +	upstream=$2
>> +
>> +	# assume branch based tag names
>> +	git log --decorate --pretty=%D "$branch@{upstream}" ^$upstream |\
>> +		grep -o "tag: $branch-[0-9-]\+" |\
>
> This drops the () which contain the branches, slightly increasing the risk
> we misparse. I'm still baffled that there's no better way to get a list of
> tags for a commit range.

Note how I use --pretty=%D so I can be sure the git log output only
contains ref names, and nothing else. I think this *decreases* the
chances of misparsing.

>> +		sed -e "s/^tag: //" |\
>> +		tr "[:space:]" " "
>
> Not clear to me why we need to do space translations with your pipeline.

Yeah, *sigh*.

It's not strictly about this pipeline, it's about robustness in general.

Assigning the output to a variable would include the newlines... but
$drm_intel_next_tags references would have the newlines expanded to
spaces while quoted "$drm_intel_next_tags" would retain the newlines. As
the follow-up patches need to be able to pick the first from the list, I
opted to this so I can use ${x%% *} to do that, without fear that
quoting would mess that up.

This was another one of those cases where I paused to contemplate the
relative merits of converting the entire thing to python.

> I'd go with the old pipeline exactly. Not because it's better, but because
> we know it works. At least until we have a real way to list tags ...

Alas the old one doesn't work as-is for the general case. The
drm-intel-next- grep&sed works only because the drm-intel-next branch
does not contain tags such as drm-intel-next-fixes- which would also
match.

BR,
Jani.

> -Daniel
>
>> +}
>> +
>>  function git_committer_email
>>  {
>>  	if ! committer_email=$(git config --get user.email) ; then
>> @@ -1930,7 +1946,7 @@ function dim_pull_request
>>  
>>  	if [ "$branch" = "drm-intel-next" ]; then
>>  		# drm-intel-next pulls have been tagged using dim update-next
>> -		drm_intel_next_tags=$(git log "$branch@{upstream}" ^$upstream --decorate | grep "(.*tag: drm-intel-next-" | sed -e "s/^.*(.*tag: \(drm-intel-next-[^ ,]*\).*)$/\1/")
>> +		drm_intel_next_tags=$(git_unmerged_tags "$branch" "$upstream")
>>  		prep_pull_mail $req_file $drm_intel_next_tags
>>  		tag=$(git describe --all --exact "$branch@{upstream}")
>>  
>> -- 
>> 2.11.0
>> 
>> _______________________________________________
>> dim-tools mailing list
>> dim-tools at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dim-tools

-- 
Jani Nikula, Intel Open Source Graphics Center