[Toaster] [review-request][PATCH] toaster: Fix import of custom layers from toasterjson.conf
sujith h
sujith.h at gmail.com
Mon Aug 24 19:16:34 PDT 2015
On 24 Aug 2015 6:49 pm, "sujith h" <sujith.h at gmail.com> wrote:
>
>
>
> On Fri, Aug 21, 2015 at 8:07 PM, Damian, Alexandru <
alexandru.damian at intel.com> wrote:
>>
>>
>>
>> On Fri, Aug 21, 2015 at 1:52 PM, sujith h <sujith.h at gmail.com> wrote:
>>>
>>>
>>>
>>> On Fri, Aug 21, 2015 at 4:45 PM, Damian, Alexandru <
alexandru.damian at intel.com> wrote:
>>>>
>>>> Hi,
>>>>
>>>> I think we need a better patch description. If I understand correctly,
the issue you were facing is:
>>>> - even if we have layers at HEAD that are not under the poky/ tree,
the code would always return the
>>>> checkout path in the poky/ tree; I recognize that this is a valid
issue.
>>>>
>>>> I feel that this issue can be handled a bit differently if we frame it
as such:
>>>> - if we have a HEAD branch specified for a layer, we need to find out
the current checkout directory for that layer, since no checkout would be
performed.
>>>>
>>>> As such, when we import a toasterconf.json file (they only way to get
HEAD branches for a layer) we need to store the actual layer checkout path
in the database (in the localpath, perhaps?), and reuse that value, instead
of employing special logic to search for it. What I mean, the core of the
problem is solved by replacing the whole gitrepos checkout logic with a
database value for all "HEAD" layers, and eliminate the magic in
getGitCloneDirectory().
>>>>
>>>> I have a couple more comments on the patch below.
>>>>
>>>> Cheers,
>>>> Alex
>>>>
>>>> On Fri, Aug 21, 2015 at 9:41 AM, Sujith H <sujith.h at gmail.com> wrote:
>>>>>
>>>>> This patch fixes import of custom layers from toasterjson.conf
>>>>> file. For example it was verified using layers like meta-mentor,
>>>>> meta-fsl-arm, meta-sourcery etc.
>>>>>
>>>>> Signed-off-by: Sujith Haridasan <Sujith_Haridasan at mentor.com>
>>>>> ---
>>>>> .../toaster/bldcontrol/localhostbecontroller.py | 49
++++++++++++++++++++--
>>>>> .../bldcontrol/management/commands/loadconf.py | 12 ++++--
>>>>> 2 files changed, 54 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/bitbake/lib/toaster/bldcontrol/localhostbecontroller.py
b/bitbake/lib/toaster/bldcontrol/localhostbecontroller.py
>>>>> index 231a7d3..c43f33f 100644
>>>>> --- a/bitbake/lib/toaster/bldcontrol/localhostbecontroller.py
>>>>> +++ b/bitbake/lib/toaster/bldcontrol/localhostbecontroller.py
>>>>> @@ -178,7 +178,7 @@ class
LocalhostBEController(BuildEnvironmentController):
>>>>> self.be.save()
>>>>> logger.debug("localhostbecontroller: Stopped bitbake server")
>>>>>
>>>>> - def getGitCloneDirectory(self, url, branch):
>>>>> + def
>>>>>
>>>>> getGitCloneDirectory(self, url, branch, cached_layers=None):
>>>>> """ Utility that returns the last component of a git path as
directory
>>>>> """
>>>>> import re
>>>>> @@ -191,6 +191,17 @@ class
LocalhostBEController(BuildEnvironmentController):
>>>>>
>>>>> # word of attention; this is a localhost-specific issue;
only on the localhost we expect to have "HEAD" releases
>>>>> # which _ALWAYS_ means the current poky checkout
>>>>> + if cached_layers:
>>>>
>>>>
>>>> Using cached_layers here works only by coincidence - if the desired
layer is not manually checked out under be.sourcedir, it will not be found
by cached_layers code, and will not appear here.
>>>
>>>
>>> Ah, so would it be good to read from the database? Because I was giving
full path of layers in the toasterconf.json file. I assume that layer
information from the toasterconf.json file would be updated in the
database?
>>
>>
>> Would be inserted into the database as the file is read.
>>
>>>>
>>>>
>>>> I think a completely new piece of code is needed, which will track
where any layer that has a HEAD branch is actually checked out. This path
would be discovered and stored by importing a toasterconf.json file (the
only place that can generate a HEAD branch), and verified to actually exist
at build time in this function, getGitCloneDirectory(). This patch touches
loadconf, but I think we might be able to get a better handling of that.
>>>>
>>>>
>>>>> + if not url.endswith('.git'):
>>>>
>>>>
>>>> I am not sure why the actual GIT url format is relevant for "HEAD"
branches; we don't need to figure out where the layer would get checked
out, but where it actually exists.
>>>>
>>>>
>>>>> + from os.path import dirname
>>>>> + extractDir = dirname(url)
>>>>> + for key, val in cached_layers.iteritems():
>>>>> + if val == extractDir:
>>>>> + local_checkout_path = key
>>>>> + return cached_layers[local_checkout_path]
>>>>> + else:
>>>>> + local_checkout_path = cached_layers[url]
>>>>> + return local_checkout_path
>>>>> from os.path import dirname as DN
>>>>> local_checkout_path =
DN(DN(DN(DN(DN(os.path.abspath(__file__))))))
>>>>> #logger.debug("localhostbecontroller: using HEAD checkout in
%s" % local_checkout_path)
>>>>> @@ -245,7 +256,14 @@ class
LocalhostBEController(BuildEnvironmentController):
>>>>>
>>>>> # 3. checkout the repositories
>>>>> for giturl, commit in gitrepos.keys():
>>>>> - localdirname = os.path.join(self.be.sourcedir,
self.getGitCloneDirectory(giturl, commit))
>>>>> + if not giturl.endswith('.git'):
>>>>> + for key, val in cached_layers.iteritems():
>>>>> + if os.path.dirname(giturl) == val:
>>>>> + tmpKey = key
>>>>> + tmpVal = gitrepos[(giturl, commit)]
>>>>> + gitrepos[(tmpKey, commit)] += tmpVal
>>>>> + giturl = tmpKey
>>>>> + localdirname = os.path.join(self.be.sourcedir,
self.getGitCloneDirectory(giturl, commit, cached_layers))
>>>>> logger.debug("localhostbecontroller: giturl %s:%s
checking out in current directory %s" % (giturl, commit, localdirname))
>>>>>
>>>>> # make sure our directory is a git repository
>>>>> @@ -280,13 +298,36 @@ class
LocalhostBEController(BuildEnvironmentController):
>>>>>
>>>>> # verify our repositories
>>>>> for name, dirpath in gitrepos[(giturl, commit)]:
>>>>> - localdirpath = os.path.join(localdirname, dirpath)
>>>>
>>>>
>>>> I am not sure what this change tries to do; dirpath is actually a
path in the git repo where the layer lives; it is not immediate that the
all the layers live in the top directory of a git repo. This change
invalidates the check below, where we verify that the layer path actually
exists after checkout.
>>>>
>>>>>
>>>>> + localdirpath = localdirname
>>>>>
>>>>> logger.debug("localhostbecontroller: localdirpath
expected '%s'" % localdirpath)
>>>>> if not os.path.exists(localdirpath):
>>>>> raise BuildSetupException("Cannot find layer git
path '%s' in checked out repository '%s:%s'. Aborting." % (localdirpath,
giturl, commit))
>>>>>
>>>>> + layerlisturl, layerlistdir = "", ""
>>>>> + layerlisturl = [localurl for localurl, localpath in
cached_layers.iteritems() if localpath == localdirpath]
>>>>
>>>>
>>>> I suspect this logic will go away if we're not going to use
cached_layers to discover the local checkout directories for HEAD urls.
>>>>
>>>>
>>>>>
>>>>> + if len(layerlisturl) != 0:
>>>>> + layerlisturl = layerlisturl[0]
>>>>> + if len(layerlisturl) == 0:
>>>>> + if os.path.basename(localdirpath).startswith("_"):
>>>>> + layerlisturl = localdirpath
>>>>> if name != "bitbake":
>>>>> - layerlist.append(localdirpath.rstrip("/"))
>>>>> + for gitlocal, localpath in gitrepos.iteritems():
>>>>> + if layerlisturl in gitlocal:
>>>>> + if len(localpath) > 2:
>>>>> + for paths in localpath:
>>>>> + if "bitbake" not in paths[1]:
>>>>> + layerlistdir = localdirpath
+"/" + str(paths[1])
>>>>> + if layerlistdir not in layerlist:
>>>>> +
layerlist.append(layerlistdir.rstrip("/"))
>>>>> + else:
>>>>> +
layerlist.append(localdirpath.rstrip("/"))
>>>>> + break
>>>>> + if len(layerlist) == 0:
>>>>> + for gitlocal, localpath in
gitrepos.iteritems():
>>>>> + for paths in localpath:
>>>>> + if "bitbake" not in paths:
>>>>> + layerlist.append(layerlisturl +
"/" + str(paths[1]))
>>>>> +
>>>>>
>>>>> logger.debug("localhostbecontroller: current layer list %s "
% pformat(layerlist))
>>>>>
>>>>> diff --git a/
>>>>>
>>>>> bitbake/lib/toaster/bldcontrol/management/commands/loadconf.py
b/bitbake/lib/toaster/bldcontrol/management/commands/loadconf.py
>>>>> index 5022b59..a22f744 100644
>>>>> --- a/bitbake/lib/toaster/bldcontrol/management/commands/loadconf.py
>>>>> +++ b/bitbake/lib/toaster/bldcontrol/management/commands/loadconf.py
>>>>> @@ -45,12 +45,14 @@ class Command(BaseCommand):
>>>>> for i in ['bitbake', 'releases', 'defaultrelease', 'config',
'layersources']:
>>>>> assert i in data
>>>>>
>>>>> - def _read_git_url_from_local_repository(address):
>>>>> + def _read_git_url_from_local_repository(address, path =
None):
>>>>
>>>>
>>>> If you want to pass in the path, please move the whole function
outside _import_layer_config, as it's no longer a closure.
>>>>
>>>>
>>>>>
>>>>> url = None
>>>>> + if not path:
>>>>> + path = os.path.dirname(filepath)
>>>>> # we detect the remote name at runtime
>>>>> import subprocess
>>>>> (remote, remote_name) = address.split(":", 1)
>>>>> - cmd = subprocess.Popen("git remote -v", shell=True, cwd
= os.path.dirname(filepath), stdout=subprocess.PIPE, stderr =
subprocess.PIPE)
>>>>> + cmd = subprocess.Popen("git remote -v", shell=True, cwd
= path, stdout=subprocess.PIPE, stderr = subprocess.PIPE)
>>>>> (out,err) = cmd.communicate()
>>>>> if cmd.returncode != 0:
>>>>> logging.warning("Error while importing layer
vcs_url: git error: %s" % err)
>>>>> @@ -121,7 +123,11 @@ class Command(BaseCommand):
>>>>> logger.error("Local layer path %s must
exists. Are you trying to import a layer that does not exist ? Check your
local toasterconf.json" % lo.local_path)
>>>>>
>>>>> if layerinfo['vcs_url'].startswith("remote:"):
>>>>> - lo.vcs_url =
_read_git_url_from_local_repository(layerinfo['vcs_url'])
>>>>> + if layerinfo['local_path'] not in ["meta",
"meta-yocto", "meta-yocto-bsp"]:
>>>>
>>>>
>>>> Please don't use hardcoded layer names anywhere in the code.
>>>>
>>>> We do not want special handling for some layers, all layers need to
be treated in the same, uniform, way.
>>>>
>>>>
>>>>> + repo_path = layerinfo['local_path']
>>>>> + else:
>>>>> + repo_path = None
>>>>> + lo.vcs_url =
_read_git_url_from_local_repository(layerinfo['vcs_url'], repo_path)
>>>>
>>>>
>>>> I think this code needs
>>>>
>>>>
>>>>>
>>>>> if lo.vcs_url is None:
>>>>> logger.error("The toaster config file
references the local git repo, but Toaster cannot detect it.\nYour local
configuration for layer %s is invalid. Make sure that the toasterconf.json
file is correct." % layerinfo['name'])
>>>
>>>
>>> A bit more explanation would help me move further to improvise the
patch. If you don't mind please? So if I understand correctly a separate
function should be there to handle HEAD branches provided by
toasterconf.json file, right?
>>
>>
>>
>> Yep, exactly my point.
>>
>> When inserting the Layer_Version objects for the "HEAD" branch, the
Layer_Version.local_path should
>>
>> be set to the layer path.
>>
>> When checking out the layers in localhost bbcontroller, the checkout
should be skipped at all times for "HEAD" branches; instead, the value in
Layer_Version.local_path should be used.
>
>
>
> Is there a way to access local_path of toasterconf.json file from
localhostbecontroller.py file?
I got the solution. I have modified local_path from "/" to the value in the
configuration file in the database.
>
>
>>
>>
>>
>>>>>
>>>>>
>>>>> --
>>>>> 1.9.1
>>>>>
>>>>> --
>>>>> _______________________________________________
>>>>> toaster mailing list
>>>>> toaster at yoctoproject.org
>>>>> https://lists.yoctoproject.org/listinfo/toaster
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Alex Damian
>>>> Yocto Project
>>>> SSG / OTC
>>>
>>>
>>>
>>>
>>> --
>>> സുജിത് ഹരിദാസന്
>>> Bangalore
>>> <Project>Contributor to KDE project
>>> http://fci.wikia.com/wiki/Anti-DRM-Campaign
>>> <Blog> http://sujithh.info
>>
>>
>>
>>
>> --
>> Alex Damian
>> Yocto Project
>> SSG / OTC
>
>
>
>
> --
> സുജിത് ഹരിദാസന്
> Bangalore
> <Project>Contributor to KDE project
> http://fci.wikia.com/wiki/Anti-DRM-Campaign
> <Blog> http://sujithh.info
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150825/61976996/attachment-0001.html>
More information about the toaster
mailing list