_project: don't _find_project_dir if a junctionaevri/direct_load_junction_projects

If we're creating a project for a junction, then don't go looking for
the project directory - the path has already been given to us.

This means that we can now detect when junctions are missing a
'project.conf', and importantly we don't accidentally start resolving
elements in the enclosing project.

Add tests to cover workspaced, local, and git repo cases. Note that
this is also the first test coverage for the INVALID_JUNCTION path.

In later work we might extract the _find_project_dir magic out of the
Project class, so that there are no surprises when instantiating it.

3 files changed, 82 insertions, 4 deletions

diff --git a/buildstream/_loader/loader.py b/buildstream/_loader/loader.py
index 90cc213cb..bf6775d35 100644
--- a/buildstream/_loader/loader.py
+++ b/buildstream/_loader/loader.py
@@ -567,7 +567,7 @@ 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 for {}.".format(element)
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/tests/format/junctions.py b/tests/format/junctions.py
index 5c6ebd0bd..43d556a65 100644
--- a/tests/format/junctions.py
+++ b/tests/format/junctions.py
@@ -49,6 +49,55 @@ 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
+
+    os.remove(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()
+
+    os.remove(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 +349,31 @@ 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.
+    os.remove(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, os.path.join(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')