On Fri, Aug 21, 2015 at 8:07 PM, Damian, Alexandru < [email protected]> wrote:
> > > 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. > Is there a way to access local_path of toasterconf.json file from localhostbecontroller.py file? > > > >> >>>> -- >>>> 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 > -- സുജിത് ഹരിദാസന് 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
