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? > > 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? > >> -- >> 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
-- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
