<div dir="ltr"><div>Dear <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Paul and all,</span></div><div dir="auto"><br></div><div dir="auto">Thanks for your reply.</div><div>1) OK. I will follow the "Commit Patch Message Guidelines" and revise my patch.</div><div>2) I am sorry that I forgot to consider the licensing issue.</div><div> I will remove these codes in my revised
<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">patch</span>.</div><div>3) I totally agree with you. It's awesome to use<span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"> <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">"/usr/bin/env <command> <options>"<span> right here.</span></span></span></div><br><div class="gmail_quote"><div dir="ltr">Paul Eggleton <<a href="mailto:paul.eggleton@linux.intel.com" target="_blank">paul.eggleton@linux.intel.com</a><wbr>> 於 2018年5月21日週一 12:13 寫道：<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi there,<br>
<br>
Thanks for the patch. A few comments:<br>
<br>
1) All patches need to have Signed-off-by and a proper commit message. Please <br>
refer to: <a href="http://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines" rel="noreferrer noreferrer" target="_blank">http://www.openembedded.org/<wbr>wiki/Commit_Patch_Message_<wbr>Guidelines</a><br>
<br>
2) We shouldn't be copying in functions verbatim from Python, that's <br>
potentially problematic for licensing reasons if nothing else.<br>
<br>
3) I wonder, is this even necessary? It should be searching PATH already, thus <br>
we should be able to just put the name of the executable we want to run <br>
without needing to find the full path. Looking at the other fetcher modules we <br>
typically use "/usr/bin/env <command> <options>" so I think that should work <br>
here as well.<br>
<br>
Cheers,<br>
Paul<br>
<br>
On Monday, 21 May 2018 2:51:52 AM NZST Tzu Hsiang Lin wrote:<br>
> ---<br>
> lib/bb/fetch2/clearcase.py | 28 ++++++++++++++++++++++++++--<br>
> lib/bb/fetch2/npm.py | 1 -<br>
> 2 files changed, 26 insertions(+), 3 deletions(-)<br>
> <br>
> diff --git a/lib/bb/fetch2/clearcase.py b/lib/bb/fetch2/clearcase.py<br>
> index 36beab6a..28589dbc 100644<br>
> --- a/lib/bb/fetch2/clearcase.py<br>
> +++ b/lib/bb/fetch2/clearcase.py<br>
> @@ -69,7 +69,6 @@ from bb.fetch2 import FetchMethod<br>
> from bb.fetch2 import FetchError<br>
> from bb.fetch2 import runfetchcmd<br>
> from bb.fetch2 import logger<br>
> -from distutils import spawn<br>
> <br>
> class ClearCase(FetchMethod):<br>
> """Class to fetch urls via 'clearcase'"""<br>
> @@ -107,7 +106,7 @@ class ClearCase(FetchMethod):<br>
> else:<br>
> ud.module = ""<br>
> <br>
> - ud.basecmd = d.getVar("FETCHCMD_ccrc") or <br>
spawn.find_executable("<wbr>cleartool") or spawn.find_executable("<wbr>rcleartool")<br>
> + ud.basecmd = d.getVar("FETCHCMD_ccrc") or <br>
_find_executable("cleartool") or _find_executable("rcleartool")<br>
> <br>
> if d.getVar("SRCREV") == "INVALID":<br>
> raise FetchError("Set a valid SRCREV for the clearcase fetcher in <br>
your recipe, e.g. SRCREV = \"/main/LATEST\" or any other label of your <br>
choice.")<br>
> @@ -207,6 +206,31 @@ class ClearCase(FetchMethod):<br>
> output = runfetchcmd(cmd, d, workdir=ud.ccasedir)<br>
> <a href="http://logger.info" rel="noreferrer noreferrer" target="_blank">logger.info</a>("rmview output: %s", output)<br>
> <br>
> + def _find_executable(executable, path=None):<br>
> + """Tries to find 'executable' in the directories listed in 'path'.<br>
> +<br>
> + A string listing directories separated by 'os.pathsep'; defaults to<br>
> + os.environ['PATH']. Returns the complete filename or None if not <br>
found.<br>
> + """<br>
> + if path is None:<br>
> + path = os.environ['PATH']<br>
> +<br>
> + paths = path.split(os.pathsep)<br>
> + base, ext = os.path.splitext(executable)<br>
> +<br>
> + if (sys.platform == 'win32') and (ext != '.exe'):<br>
> + executable = executable + '.exe'<br>
> +<br>
> + if not os.path.isfile(executable):<br>
> + for p in paths:<br>
> + f = os.path.join(p, executable)<br>
> + if os.path.isfile(f):<br>
> + # the file exists, we have a shot at spawn working<br>
> + return f<br>
> + return None<br>
> + else:<br>
> + return executable<br>
> +<br>
> def need_update(self, ud, d):<br>
> if ("LATEST" in ud.label) or (ud.customspec and "LATEST" in <br>
ud.customspec):<br>
> ud.identifier += "-%s" % d.getVar("DATETIME",d, True)<br>
> diff --git a/lib/bb/fetch2/npm.py b/lib/bb/fetch2/npm.py<br>
> index 730c346a..408dfc3d 100644<br>
> --- a/lib/bb/fetch2/npm.py<br>
> +++ b/lib/bb/fetch2/npm.py<br>
> @@ -32,7 +32,6 @@ from bb.fetch2 import runfetchcmd<br>
> from bb.fetch2 import logger<br>
> from bb.fetch2 import UnpackError<br>
> from bb.fetch2 import ParameterError<br>
> -from distutils import spawn<br>
> <br>
> def subprocess_setup():<br>
> # Python installs a SIGPIPE handler by default. This is usually not <br>
what<br>
> <br>
<br>
<br>
-- <br>
<br>
Paul Eggleton<br>
Intel Open Source Technology Centre<br>
<br>
<br>
</blockquote></div></div>