[Toaster] [review-request][PATCH] toaster: Fix import of custom layers from toasterjson.conf
Brian Avery
avery.brian at gmail.com
Thu Aug 27 09:06:29 PDT 2015
Hi Sujith,
I'd like to test this out. Is it possible to get a config file that
exercises the new features?
-b
On Mon, Aug 24, 2015 at 7:16 PM, sujith h <sujith.h at gmail.com> wrote:
> 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
>
>
> --
> _______________________________________________
> toaster mailing list
> toaster at yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>
More information about the toaster
mailing list