[U-Boot] buildman: suspect issue with patman tags

[Steve Rae <srae@broadcom.com>]

On 14-08-28 05:59 PM, Simon Glass wrote:
> Hi Steve,
>
>
> On 28 August 2014 10:21, Steve Rae <srae@broadcom.com> wrote:
>>
>>
>>
>> On 14-08-28 07:45 AM, Simon Glass wrote:
>>>
>>> Hi Steve,
>>>
>>> On 27 August 2014 15:22, Steve Rae <srae@broadcom.com> wrote:
>>
>>
>> [... snip ...]
>>
>>
>>>>
>>>> *From:*Steve Rae
>>>> *Sent:* Monday, August 25, 2014 09:06
>>>> *To:* Simon Glass
>>>> *Subject:* buildman: issue with patman tags
>>>>
>>>> Simon,
>>>>
>>>> When I combine multiple patch-series, each with their own patman tags in the
>>>> commit messages, into a single branch...
>>>>
>>>> Then:
>>>>
>>>> $ tools/buildman/buildman -b combined bcm28155_ap
>>>> No section: 'make-flags'
>>>> Traceback (most recent call last):
>>>>      File "tools/buildman/buildman", line 139, in <module>
>>>>        control.DoBuildman(options, args)
>>>>      File "/home/srae/work/to_denx/tools/buildman/control.py", line 161,
>>>> in DoBuildman
>>>>        options.git_dir, count)
>>>>      File
>>>> "/home/srae/work/to_denx/tools/buildman/../patman/patchstream.py", line
>>>> 387, in GetMetaDataForList
>>>>        ps.ProcessLine(line)
>>>>      File
>>>> "/home/srae/work/to_denx/tools/buildman/../patman/patchstream.py", line
>>>> 248, in ProcessLine
>>>>        self.AddToSeries(line, name, value)
>>>>      File
>>>> "/home/srae/work/to_denx/tools/buildman/../patman/patchstream.py", line
>>>> 94, in AddToSeries
>>>>        self.series.AddTag(self.commit, line, name, value)
>>>>      File "/home/srae/work/to_denx/tools/buildman/../patman/series.py",
>>>> line 67, in AddTag
>>>>        (commit.hash, line, values, self[name]))
>>>> ValueError: In 442a87cd: line 'Series-version: 1': Cannot add another
>>>> value '['1']' to series '4'
>>>> $
>>>>
>>>> Note:  each patch-series individually build successfully with buildman,
>>>> it is only after I combine that there are issues....
>>>
>>>
>>> This is because buildman doesn't need these tags. For patman you need
>>> to work with one series at a time. If you have multiple series, then
>>> some might be at one version and some at another, and it wouldn't know
>>> what version to put on the patch. Also there might be multiple cover
>>> letters, etc.
>>>
>>> If you want to keep several series in a branch, you need to use -s and
>>> -c to control what portion of the branch represents each series.
>>
>>
>> Yes, I am successfully using these flags in patman; but I want to be able to build all the combined patches...
>>
>>>
>>> I have thought about adding support for multiple series in a branch -
>>> e.g. a marker on the bottom commit, giving each series a name. But I
>>> haven't convinced myself it is desirable.
>>
>>
>> That might be too complicated...
>> Maybe just add a flag (in buildman) to simply ignore all of the commit messages (or ignore the patman tags in the commit messages) just so that the build would execute.
>> Actually, the only think that I really need is to be able to build the last one (but I think that "--step=0" is close enough)
>
> You can build just the top commit by leaving off the -b argument.

OK, awesome!

>
> Re ignoring duplicate tags, there is logic in there but it doesn't go
> far enough for all cases. Please try this patch on top of what I sent
> earlier today (branch buildman6 at u-boot-x86.git)
>

By the time I fetched "buildman6" this morning, it seems that you 
already added this patch....
So with this:
	commit 6aa2c7fa621d888dd0d225af2be5dfe46409f942
	Author: Simon Glass <sjg@chromium.org>
	Date:   Thu Aug 28 17:54:40 2014 -0700

	    buildman: Ignore conflicting tags
running:
	/tools/buildman/buildman bcm28155_ap
it now builds successfully!

Thanks, Steve

> diff --git a/tools/buildman/control.py b/tools/buildman/control.py
> index c473ab9..3628ac1 100644
> --- a/tools/buildman/control.py
> +++ b/tools/buildman/control.py
> @@ -168,6 +168,10 @@ def DoBuildman(options, args, toolchains=None,
> make_func=None, boards=None,
>       # upstream/master~..branch but that isn't possible if upstream/master is
>       # a merge commit (it will list all the commits that form part of the
>       # merge)
> +    # Conflicting tags are not a problem for buildman, since it does not use
> +    # them. For example, Series-version is not useful for buildman. On the
> +    # other hand conflicting tags will cause an error. So allow later tags
> +    # to overwrite earlier ones by setting allow_overwrite=True
>       if options.branch:
>           if count == -1:
>               range_expr = gitutil.GetRangeInBranch(options.git_dir,
> @@ -175,19 +179,14 @@ def DoBuildman(options, args, toolchains=None,
> make_func=None, boards=None,
>               upstream_commit = gitutil.GetUpstream(options.git_dir,
>                                                     options.branch)
>               series = patchstream.GetMetaDataForList(upstream_commit,
> -                options.git_dir, 1)
> +                options.git_dir, 1, series=None, allow_overwrite=True)
>
> -            # Conflicting tags are not a problem for buildman, since it does
> -            # not use them. For example, Series-version is not useful for
> -            # buildman. On the other hand conflicting tags will cause an
> -            # error. So allow later tags to overwrite earlier ones.
> -            series.allow_overwrite = True
>               series = patchstream.GetMetaDataForList(range_expr,
> -                                              options.git_dir, None, series)
> +                    options.git_dir, None, series, allow_overwrite=True)
>           else:
>               # Honour the count
>               series = patchstream.GetMetaDataForList(options.branch,
> -                                                    options.git_dir, count)
> +                    options.git_dir, count, series=None, allow_overwrite=True)
>       else:
>           series = None
>           options.verbose = True
> diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
> index 2cb5cf0..7144b44 100644
> --- a/tools/buildman/func_test.py
> +++ b/tools/buildman/func_test.py
> @@ -79,6 +79,7 @@ Date:   Thu Aug 14 16:48:25 2014 -0600
>       Series-changes: 7
>       - Add new patch to fix the 'reverse' bug
>
> +    Series-version: 8
>
>       Change-Id: I79078f792e8b390b8a1272a8023537821d45feda
>       Reported-by: York Sun <yorksun@freescale.com>
> @@ -156,6 +157,8 @@ Date:   Fri Aug 22 15:57:39 2014 -0600
>       Series-changes: 9
>       - Add new patch to avoid changing the order of tags
>
> +    Series-version: 9
> +
>       Suggested-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
>       Change-Id: Ib1518588c1a189ad5c3198aae76f8654aed8d0db
>   """]
> diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
> index b3e66c3..d630157 100644
> --- a/tools/patman/patchstream.py
> +++ b/tools/patman/patchstream.py
> @@ -355,7 +355,7 @@ class PatchStream:
>
>
>   def GetMetaDataForList(commit_range, git_dir=None, count=None,
> -                       series = None):
> +                       series = None, allow_overwrite=False):
>       """Reads out patch series metadata from the commits
>
>       This does a 'git log' on the relevant commits and pulls out the tags we
> @@ -367,11 +367,13 @@ def GetMetaDataForList(commit_range,
> git_dir=None, count=None,
>           count: Number of commits to list, or None for no limit
>           series: Series object to add information into. By default a new series
>               is started.
> +        allow_overwrite: Allow tags to overwrite an existing tag
>       Returns:
>           A Series object containing information about the commits.
>       """
>       if not series:
>           series = Series()
> +    series.allow_overwrite = allow_overwrite
>       params = gitutil.LogCmd(commit_range,reverse=True, count=count,
>                               git_dir=git_dir)
>       stdout = command.RunPipe([params], capture=True).stdout
>