Hi Masahiro-san, On Thu, Jun 4, 2015 at 11:54 PM, Masahiro Yamada <yamada.masah...@socionext.com> wrote: > Hi Joe, > > > 2015-05-30 6:23 GMT+09:00 Joe Hershberger <joe.hershber...@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.hershber...@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
