On Fri, Aug 21, 2015 at 1:52 PM, sujith h <[email protected]> wrote:
> > > On Fri, Aug 21, 2015 at 4:45 PM, Damian, Alexandru < > [email protected]> 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 <[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. >> > > 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. > >>> -- >>> 1.9.1 >>> >>> -- >>> _______________________________________________ >>> toaster mailing list >>> [email protected] >>> 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
-- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
