[U-Boot] [PATCH] moveconfig: Add a new --git-ref option

[Joe Hershberger]

Hi Masahiro-san,

On Thu, Jun 4, 2015 at 11:54 PM, Masahiro Yamada
<yamada.masahiro at socionext.com> wrote:
> Hi Joe,
>
>
> 2015-05-30 6:23 GMT+09:00 Joe Hershberger <joe.hershberger at ni.com>:
>> This option allows the 'make *_defconfig' step to run against a former
>> repo state, while the savedefconfig step runs against the current repo
>> state. This is convenient for the case where something in the Kconfig
>> has changed such that the defconfig is no longer complete with the new
>> Kconfigs. This feature allows the .config to be built assuming those old
>> Kconfigs, but then savedefconfig based on the new state of the Kconfigs.
>>
>> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
>
>
> This idea seems nice, but I have some comments about the implementation.
>
>
>
>> +    defconfig_src_dir = ''
>> +
>> +    if options.git_ref:
>> +        work_dir = WorkDir()
>> +        defconfig_src_dir = work_dir.get()
>> +        cwd = os.getcwd()
>> +        print 'Cloning git repo for \'make *_defconfig\' step...'
>
> You can use signle quotes without escape sequences inside "...", and vice versa.

OK

>> diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
>> index 9e739d8..138f989 100644
>> --- a/tools/patman/gitutil.py
>> +++ b/tools/patman/gitutil.py
>> @@ -61,6 +61,21 @@ def CountCommitsToBranch():
>>      patch_count = int(stdout)
>>      return patch_count
>>
>> +def CommitHash(commit_ref):
>> +    """Gets the hash for a commit
>> +
>> +    Args:
>> +        commit_ref: Commit ref to look up
>> +
>> +    Return:
>> +        Hash of revision, if any, else None
>> +    """
>> +    pipe = ['git', 'rev-parse', '--short', commit_ref]
>> +    stdout = command.RunPipe([pipe], capture=True, oneline=True).stdout
>> +
>> +    hash = stdout.strip()
>> +    return hash
>> +
>>  def NameRevision(commit_hash):
>>      """Gets the revision name for a commit
>
>
>
> Finally, this tool is going to depend on patman.  I am afraid this
> tool is getting messy.
>
> gitutils.py depends on command.py, and then command.py depends on
> cros_subprocess.py.
>
> Do you really need to invoke such a chain of libraries
> to run a sub-process?

Why does that matter?

> For example, you can get a hash in a single line like this:
>
> subprocess.check_output(['git', 'rev-parse', '--short', 'HEAD'])

I certainly can do that, but it seems better to reuse the common
functionality. This tool doesn't seem so useful elsewhere that a
person would want it to work easily outside of u-boot. In u-boot,
patman exists. Simon did the same thing for buildman. I can change it
to call the git commands directly sine what I'm doing is simple, but I
don't see any value in doing it.

Simon, what do you think?

Thanks,
-Joe