From be3b1ca0e03bdc6628b8975f386e02dbe0402106 Mon Sep 17 00:00:00 2001 From: Joseph Atkins-Turkish Date: Wed, 27 Jul 2016 10:22:03 -0700 Subject: [PATCH 1/3] Find the most shallow project. --- ide/tests/test_find_project_root.py | 9 +++++++++ ide/utils/project.py | 10 ++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/ide/tests/test_find_project_root.py b/ide/tests/test_find_project_root.py index 95164a12..1a41a15a 100644 --- a/ide/tests/test_find_project_root.py +++ b/ide/tests/test_find_project_root.py @@ -112,3 +112,12 @@ def test_PR_317(self): "src/" "src/main.c", ], "", "package.json") + + def test_find_most_shallow_project(self): + """ Check that the most shallow possible valid-looking project is chosen. """ + self.run_test([ + "build/appinfo.json", + "build/src/main.c", + "package.json", + "src/main.c", + ], "", "package.json") diff --git a/ide/utils/project.py b/ide/utils/project.py index 42879506..fdf331c9 100644 --- a/ide/utils/project.py +++ b/ide/utils/project.py @@ -1,6 +1,6 @@ -import logging import abc import json +import os.path from django.utils.translation import ugettext as _ @@ -50,7 +50,9 @@ def find_project_root_and_manifest(project_items): """ SRC_DIR = 'src/' - for item in project_items: + # Sort the paths by the number of path separators they have, + sorted_items = sorted(project_items, key=lambda x: os.path.normpath(x.path).count('/')) + for i, item in enumerate(sorted_items): base_dir = item.path # Check if the file is one of the kinds of manifest file @@ -70,8 +72,8 @@ def find_project_root_and_manifest(project_items): # The base dir is the location of the manifest file without the manifest filename. base_dir = base_dir[:dir_end] - # Now check that there is a a source directory containing at least one source file. - for source_item in project_items: + # Now check the rest of the items for a source directory containing at least one source file. + for source_item in sorted_items[i+1:]: source_dir = source_item.path if source_dir[:dir_end] != base_dir: continue From 60aec9766f1a8db939c1eeb0501c733a9c12b5a7 Mon Sep 17 00:00:00 2001 From: Joseph Atkins-Turkish Date: Wed, 27 Jul 2016 12:46:40 -0700 Subject: [PATCH 2/3] Rewrote find_project_root_and_manifest from scratch. --- ide/tests/test_find_project_root.py | 16 ++++++- ide/utils/project.py | 73 +++++++++++------------------ 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/ide/tests/test_find_project_root.py b/ide/tests/test_find_project_root.py index 1a41a15a..dd2c9c6a 100644 --- a/ide/tests/test_find_project_root.py +++ b/ide/tests/test_find_project_root.py @@ -99,7 +99,7 @@ def test_ignore_invalid_package_file(self): """ If a project has an invalid package.json and an appinfo.json, select the latter """ self.run_test([ FakeProjectItem("package.json", "{}"), - "src/" + "src/", "src/main.c", "appinfo.json" ], "", "appinfo.json") @@ -109,7 +109,7 @@ def test_PR_317(self): self.run_test([ "MAINTAINERS", "package.json", - "src/" + "src/", "src/main.c", ], "", "package.json") @@ -121,3 +121,15 @@ def test_find_most_shallow_project(self): "package.json", "src/main.c", ], "", "package.json") + + def test_malformed_names_bug(self): + """ Test for a bug where characters could be prepended to manifest names. """ + with self.assertRaises(InvalidProjectArchiveException): + self.run_test(["project/rrrpackage.json", "project/rrrsrc/", "project/rrrsrc/main.c"]) + + def test_no_source_at_root(self): + """ Check that projects still import if all sources are in subdirectories """ + self.run_test([ + "project/package.json", + "project/src/c/main.c", + ], "", "project/package.json") diff --git a/ide/utils/project.py b/ide/utils/project.py index fdf331c9..a7350659 100644 --- a/ide/utils/project.py +++ b/ide/utils/project.py @@ -1,12 +1,12 @@ import abc import json -import os.path +import os from django.utils.translation import ugettext as _ __author__ = 'katharine' - +SRC_DIR = 'src' PACKAGE_MANIFEST = 'package.json' APPINFO_MANIFEST = 'appinfo.json' MANIFEST_KINDS = [PACKAGE_MANIFEST, APPINFO_MANIFEST] @@ -33,14 +33,9 @@ def path(self): return None -def is_manifest(kind, contents): - """ A potentially valid manifest is a package.json file with a "pebble" object, or an appinfo.json file. """ - if kind == PACKAGE_MANIFEST: - return 'pebble' in json.loads(contents) - elif kind == APPINFO_MANIFEST: - return True - else: - return False +def rank_manifest_path(dirname, kind): + """ Sort key for manifest files. Sort first by depth and add a penalty for being an appinfo.json """ + return os.path.normpath(dirname).count('/') + (0.5 if kind == APPINFO_MANIFEST else 0) def find_project_root_and_manifest(project_items): @@ -48,42 +43,28 @@ def find_project_root_and_manifest(project_items): :param project_items: A list of BaseProjectItems :return: A tuple of (path_to_project, manifest BaseProjectItem) """ - SRC_DIR = 'src/' - - # Sort the paths by the number of path separators they have, - sorted_items = sorted(project_items, key=lambda x: os.path.normpath(x.path).count('/')) - for i, item in enumerate(sorted_items): - base_dir = item.path - - # Check if the file is one of the kinds of manifest file - for name in MANIFEST_KINDS: - dir_end = base_dir.rfind(name) - if dir_end == -1: - continue - # Ensure that the file is actually a manifest file - if dir_end + len(name) == len(base_dir): - if is_manifest(name, item.read()): - manifest_item = item - break - else: - # If the file is not a manifest file, continue looking for the manfiest. - continue + found_manifests = set() + found_src_files = set() - # The base dir is the location of the manifest file without the manifest filename. - base_dir = base_dir[:dir_end] - - # Now check the rest of the items for a source directory containing at least one source file. - for source_item in sorted_items[i+1:]: - source_dir = source_item.path - if source_dir[:dir_end] != base_dir: - continue - if not source_dir.endswith('.c') and not source_dir.endswith('.js'): - continue - if source_dir[dir_end:dir_end + len(SRC_DIR)] != SRC_DIR: - continue - break - else: - # If there was no source directory with a source file, keep looking for manifest files. + for item in project_items: + item_path = item.path + + # If the item looks like a manifest, add it to a set of potential manifests + item_dirname, item_basename = os.path.split(item_path) + if (item_basename == PACKAGE_MANIFEST and 'pebble' in json.loads(item.read())) or item_basename == APPINFO_MANIFEST: + found_manifests.add(((item_dirname, item_basename), item)) continue - return base_dir, manifest_item + + # Otherwise, add it to a set of source files if it is one. + if item_path.endswith(('.c', '.js', '.h')): + found_src_files.add(item_path) + + # Choose the most shallow manifest file which has a non-empty source directory. + sorted_manifests = sorted(found_manifests, key=lambda x: rank_manifest_path(*x[0])) + for (base, kind), item in sorted_manifests: + src_dir = os.path.join(base, SRC_DIR) + os.sep + for src_path in found_src_files: + if src_path.startswith(src_dir): + base = base + os.sep if base else "" + return base, item raise InvalidProjectArchiveException(_("No project root found.")) From 2458400ac79f3ebd28eeab97f32ce45611729694 Mon Sep 17 00:00:00 2001 From: Joseph Atkins-Turkish Date: Wed, 31 Aug 2016 12:02:35 -0700 Subject: [PATCH 3/3] Allow importing sourceless package.json projects. --- ide/utils/project.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/ide/utils/project.py b/ide/utils/project.py index a7350659..806b6e68 100644 --- a/ide/utils/project.py +++ b/ide/utils/project.py @@ -39,7 +39,11 @@ def rank_manifest_path(dirname, kind): def find_project_root_and_manifest(project_items): - """ Given the contents of an archive, find a valid Pebble project. + """ Given the contents of an archive, find a (potentially) valid Pebble project. + A potentially valid Pebble project is either: + - An appinfo.json file next to an 'src/' directory which contains app sources + - A package.json file which has a 'pebble' key. + The function chooses the most shallow possible project and prefers package.json to appinfo.json if both are present. :param project_items: A list of BaseProjectItems :return: A tuple of (path_to_project, manifest BaseProjectItem) """ @@ -62,9 +66,10 @@ def find_project_root_and_manifest(project_items): # Choose the most shallow manifest file which has a non-empty source directory. sorted_manifests = sorted(found_manifests, key=lambda x: rank_manifest_path(*x[0])) for (base, kind), item in sorted_manifests: - src_dir = os.path.join(base, SRC_DIR) + os.sep + if kind == "package.json": + return os.path.join(base, ''), item + src_dir = os.path.join(base, SRC_DIR, '') for src_path in found_src_files: if src_path.startswith(src_dir): - base = base + os.sep if base else "" - return base, item + return os.path.join(base, ''), item raise InvalidProjectArchiveException(_("No project root found."))