Merge branch 'aevri/direct_load_junction_projects' into 'master'

project: don't do _find_project_dir if we're a junction

See merge request BuildStream/buildstream!1231

4 files changed, 96 insertions, 7 deletions

diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py
index 9b91e91fe..804dc5f7e 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -567,13 +567,17 @@ class Loader():
         try:
             from .._project import Project
             project = Project(project_dir, self._context, junction=element,
-                              parent_loader=self)
+                              parent_loader=self, search_for_project=False)
         except LoadError as e:
             if e.reason == LoadErrorReason.MISSING_PROJECT_CONF:
+                message = (
+                    "Could not find the project.conf file in the project "
+                    "referred to by junction element '{}'.".format(element.name)
+                )
+                if element.path:
+                    message += " Was expecting it at path '{}' in the junction's source.".format(element.path)
                 raise LoadError(reason=LoadErrorReason.INVALID_JUNCTION,
-                                message="Could not find the project.conf file for {}. "
-                                        "Expecting a project at path '{}'"
-                                .format(element, element.path or '.')) from e
+                                message=message) from e
             else:
                 raise
 
diff --git a/buildstream/_project.py b/buildstream/_project.py
index 21ea91683..b6a8727f5 100644
--- a/buildstream/_project.py
+++ b/buildstream/_project.py
@@ -96,15 +96,19 @@ class ProjectConfig:
 class Project():
 
     def __init__(self, directory, context, *, junction=None, cli_options=None,
-                 default_mirror=None, parent_loader=None):
+                 default_mirror=None, parent_loader=None,
+                 search_for_project=True):
 
         # The project name
         self.name = None
 
         self._context = context  # The invocation Context, a private member
 
-        # The project directory, and whether the element whose workspace it was invoked from
-        self.directory, self._invoked_from_workspace_element = self._find_project_dir(directory)
+        if search_for_project:
+            self.directory, self._invoked_from_workspace_element = self._find_project_dir(directory)
+        else:
+            self.directory = directory
+            self._invoked_from_workspace_element = None
 
         # Absolute path to where elements are loaded from within the project
         self.element_path = None
diff --git a/buildstream/plugintestutils/runcli.py b/buildstream/plugintestutils/runcli.py
index 5dbecdd60..c08dd0ff3 100644
--- a/buildstream/plugintestutils/runcli.py
+++ b/buildstream/plugintestutils/runcli.py
@@ -310,6 +310,10 @@ class Cli():
         if options is None:
             options = []
 
+        # We may have been passed e.g. pathlib.Path or py.path
+        args = [str(x) for x in args]
+        project = str(project)
+
         options = self.default_options + options
 
         with ExitStack() as stack:
diff --git a/tests/format/junctions.py b/tests/format/junctions.py
index 5c6ebd0bd..ab369b906 100644
--- a/tests/format/junctions.py
+++ b/tests/format/junctions.py
@@ -49,6 +49,57 @@ def test_simple_build(cli, tmpdir, datafiles):
 
 
 @pytest.mark.datafiles(DATA_DIR)
+def test_junction_missing_project_conf(cli, datafiles):
+    project = datafiles / 'foo'
+    copy_subprojects(project, datafiles, ['base'])
+
+    # TODO: see if datafiles can tidy this concat up
+
+    # py3.5 requires this str conversion.
+    os.remove(str(project / 'base' / 'project.conf'))
+
+    # Note that both 'foo' and 'base' projects have a 'target.bst'. The
+    # 'app.bst' in 'foo' depends on the 'target.bst' in 'base', i.e.:
+    #
+    #   foo/base/target.bst
+    #   foo/app.bst -> foo/base/target.bst
+    #   foo/target.bst -> foo/app.bst, foor/base/target.bst
+    #
+    # In a previous bug (issue #960) if the 'project.conf' was not found in the
+    # junction's dir then we were continuing the search in the parent dirs.
+    #
+    # This would mean that the dep on 'target.bst' would resolve to
+    # 'foo/target.bst' instead of 'foo/base/target.bst'.
+    #
+    # That would lead to a 'circular dependency error' in this setup, when we
+    # expect an 'invalid junction'.
+    #
+    result = cli.run(project=project, args=['build', 'app.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_JUNCTION)
+
+
+@pytest.mark.datafiles(DATA_DIR)
+def test_workspaced_junction_missing_project_conf(cli, datafiles):
+    # See test_junction_missing_project_conf for some more background.
+
+    project = datafiles / 'foo'
+    workspace_dir = project / 'base_workspace'
+    copy_subprojects(project, datafiles, ['base'])
+
+    result = cli.run(
+        project=project,
+        args=['workspace', 'open', 'base.bst', '--directory', workspace_dir])
+    print(result)
+    result.assert_success()
+
+    # py3.5 requires this str conversion.
+    os.remove(str(workspace_dir / 'project.conf'))
+
+    result = cli.run(project=project, args=['build', 'app.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_JUNCTION)
+
+
+@pytest.mark.datafiles(DATA_DIR)
 def test_build_of_same_junction_used_twice(cli, datafiles):
     project = os.path.join(str(datafiles), 'inconsistent-names')
 
@@ -300,6 +351,32 @@ def test_git_build(cli, tmpdir, datafiles):
     assert(os.path.exists(os.path.join(checkoutdir, 'foo.txt')))
 
 
+@pytest.mark.skipif(HAVE_GIT is False, reason="git is not available")
+@pytest.mark.datafiles(DATA_DIR)
+def test_git_missing_project_conf(cli, tmpdir, datafiles):
+    project = datafiles / 'foo'
+
+    # See test_junction_missing_project_conf for some more background.
+    # py3.5 requires this str conversion.
+    os.remove(str(datafiles / 'base' / 'project.conf'))
+
+    # Create the repo from 'base' subdir
+    repo = create_repo('git', str(tmpdir))
+    ref = repo.create(os.path.join(str(datafiles), 'base'))
+
+    # Write out junction element with git source
+    element = {
+        'kind': 'junction',
+        'sources': [
+            repo.source_config(ref=ref)
+        ]
+    }
+    _yaml.dump(element, str(project / 'base.bst'))
+
+    result = cli.run(project=project, args=['build', 'app.bst'])
+    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_JUNCTION)
+
+
 @pytest.mark.datafiles(DATA_DIR)
 def test_cross_junction_names(cli, datafiles):
     project = os.path.join(str(datafiles), 'foo')