From 260752f501a3815ac924b57b129b33b6b03a2d0b Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Fri, 3 Jan 2020 18:49:27 -0700 Subject: [PATCH 1/2] when the root project name is the same as a dependency project name, raise an error --- core/dbt/deps/resolver.py | 18 +++++++--- core/dbt/parser/manifest.py | 11 +++++- core/dbt/task/deps.py | 5 ++- .../duplicate_dependency/dbt_project.yml | 4 +++ .../test_local_dependency.py | 34 +++++++++++++++++++ 5 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 test/integration/006_simple_dependency_test/duplicate_dependency/dbt_project.yml diff --git a/core/dbt/deps/resolver.py b/core/dbt/deps/resolver.py index d966ee6f703..8237463a48c 100644 --- a/core/dbt/deps/resolver.py +++ b/core/dbt/deps/resolver.py @@ -3,6 +3,7 @@ from dbt.exceptions import raise_dependency_error, InternalException +from dbt.config import Project from dbt.deps.base import BasePackage, PinnedPackage, UnpinnedPackage from dbt.deps.local import LocalUnpinnedPackage from dbt.deps.git import GitUnpinnedPackage @@ -95,21 +96,28 @@ def __iter__(self) -> Iterator[UnpinnedPackage]: def _check_for_duplicate_project_names( - final_deps: List[PinnedPackage], config + final_deps: List[PinnedPackage], config: Project ): seen = set() for package in final_deps: project_name = package.get_project_name(config) if project_name in seen: raise_dependency_error( - 'Found duplicate project {}. This occurs when a dependency' - ' has the same project name as some other dependency.' - .format(project_name)) + f'Found duplicate project "{project_name}". This occurs when ' + 'a dependency has the same project name as some other ' + 'dependency.' + ) + elif project_name == config.project_name: + raise_dependency_error( + 'Found a dependency with the same name as the root project ' + f'"{project_name}". Package names must be unique in a project.' + ' Please rename one of these packages.' + ) seen.add(project_name) def resolve_packages( - packages: List[PackageContract], config + packages: List[PackageContract], config: Project ) -> List[PinnedPackage]: pending = PackageListing.from_contracts(packages) final = PackageListing() diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index fc4fecbe35d..5bb857b7a5d 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -14,6 +14,7 @@ from dbt.config import Project, RuntimeConfig from dbt.contracts.graph.compiled import CompileResultNode from dbt.contracts.graph.manifest import Manifest, FilePath, FileHash +from dbt.exceptions import raise_compiler_error from dbt.parser.base import BaseParser from dbt.parser.analysis import AnalysisParser from dbt.parser.data_test import DataTestParser @@ -421,7 +422,15 @@ def load_all_projects(config) -> Mapping[str, Project]: internal_project_names(), _project_directories(config) ) - all_projects.update(_load_projects(config, project_paths)) + for project_name, project in _load_projects(config, project_paths): + if project_name in all_projects: + raise_compiler_error( + f'dbt found more than one package with the name ' + f'"{project_name}" included in this project. Package names ' + f'must be unique in a project. Please rename one of these ' + f'packages.' + ) + all_projects[project_name] = project return all_projects diff --git a/core/dbt/task/deps.py b/core/dbt/task/deps.py index 80d57a3414a..17e33e58fdc 100644 --- a/core/dbt/task/deps.py +++ b/core/dbt/task/deps.py @@ -1,7 +1,10 @@ +from typing import Optional + import dbt.utils import dbt.deprecations import dbt.exceptions +from dbt.config import Project from dbt.deps.base import downloads_directory from dbt.deps.resolver import resolve_packages @@ -12,7 +15,7 @@ class DepsTask(ProjectOnlyTask): - def __init__(self, args, config=None): + def __init__(self, args, config: Optional[Project] = None): super().__init__(args=args, config=config) def track_package_install(self, package_name, source_type, version): diff --git a/test/integration/006_simple_dependency_test/duplicate_dependency/dbt_project.yml b/test/integration/006_simple_dependency_test/duplicate_dependency/dbt_project.yml new file mode 100644 index 00000000000..254db1ecad2 --- /dev/null +++ b/test/integration/006_simple_dependency_test/duplicate_dependency/dbt_project.yml @@ -0,0 +1,4 @@ +name: 'test' +version: '1.0' + +profile: 'default' diff --git a/test/integration/006_simple_dependency_test/test_local_dependency.py b/test/integration/006_simple_dependency_test/test_local_dependency.py index a1133bac293..8ef59fe824a 100644 --- a/test/integration/006_simple_dependency_test/test_local_dependency.py +++ b/test/integration/006_simple_dependency_test/test_local_dependency.py @@ -1,5 +1,7 @@ from test.integration.base import DBTIntegrationTest, use_profile +import os import json +import shutil from unittest import mock import dbt.semver @@ -171,3 +173,35 @@ def test_postgres_hook_dependency(self): results = self.run_dbt(["run", '--vars', cli_vars]) self.assertEqual(len(results), 2) self.assertTablesEqual('actual', 'expected') + + +class TestSimpleDependencyDuplicateName(DBTIntegrationTest): + @property + def schema(self): + return "local_dependency_006" + + @property + def models(self): + return "local_models" + + @property + def packages_config(self): + return { + "packages": [ + { + 'local': 'duplicate_dependency' + } + ] + } + + @use_profile('postgres') + def test_postgres_local_dependency_same_name(self): + with self.assertRaises(dbt.exceptions.DependencyException): + self.run_dbt(['deps'], expect_pass=False) + + @use_profile('postgres') + def test_postgres_local_dependency_same_name_sneaky(self): + os.makedirs('dbt_modules') + shutil.copytree('./duplicate_dependency', './dbt_modules/duplicate_dependency') + with self.assertRaises(dbt.exceptions.CompilationException): + self.run_dbt(['compile'], expect_pass=False) From f1ef68c1665ad32e7a1313bc340e8428267c577b Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Sun, 5 Jan 2020 10:32:38 -0700 Subject: [PATCH 2/2] fix the unit tests --- test/unit/test_deps.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/test_deps.py b/test/unit/test_deps.py index 43161d0af26..3e3eacc499d 100644 --- a/test/unit/test_deps.py +++ b/test/unit/test_deps.py @@ -392,7 +392,7 @@ def test_dependency_resolution(self): {'package': 'fishtown-analytics-test/b', 'version': '0.2.1'}, ], }) - resolved = resolve_packages(package_config.packages, None) + resolved = resolve_packages(package_config.packages, mock.MagicMock(project_name='test')) self.assertEqual(len(resolved), 2) self.assertEqual(resolved[0].name, 'fishtown-analytics-test/a') self.assertEqual(resolved[0].version, '0.1.3')