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 <[email protected]> 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 <[email protected]> > --- > .../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. 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']) > > -- > 1.9.1 > > -- > _______________________________________________ > toaster mailing list > [email protected] > https://lists.yoctoproject.org/listinfo/toaster > -- Alex Damian Yocto Project SSG / OTC
-- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
