Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 32 additions & 3 deletions py/private/py_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,45 @@ def _make_srcs_depset(ctx):
],
)

def _make_import_path(workspace, base, imp):
def _make_import_path(label, workspace, base, imp):
if imp.startswith("/"):
fail(
"Import path '{imp}' on target {target} is invalid. Absolute paths are not supported.".format(
imp = imp,
target = str(label),
),
)

base_segments = base.split("/")
path_segments = imp.split("/")

relative_segments = 0
for segment in path_segments:
if segment == "..":
relative_segments += 1
else:
break

# Check if the relative segments that the import path starts with match the number of segments in the base path
# that would break use out of the workspace root.
# The +1 is base_segments would take the path to the root, then one more to escape.
if relative_segments == (len(base_segments) + 1):
fail(
"Import path '{imp}' on target {target} is invalid. Import paths must not escape the workspace root".format(
imp = imp,
target = str(label),
),
)

if imp.startswith(".."):
return paths.normalize(paths.join(workspace, *base.split("/")[0:-len(imp.split("/"))]))
return paths.normalize(paths.join(workspace, *(base_segments[0:-relative_segments] + path_segments[relative_segments:])))
else:
return paths.normalize(paths.join(workspace, base, imp))

def _make_imports_depset(ctx):
base = paths.dirname(ctx.build_file_path)
import_paths = [
_make_import_path(ctx.workspace_name, base, im)
_make_import_path(ctx.label, ctx.workspace_name, base, im)
for im in ctx.attr.imports
]

Expand Down
10 changes: 10 additions & 0 deletions py/tests/import-pathing/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
load(":tests.bzl", "py_library_import_pathing_test_suite")

# This is used in the py_library import pathing tests
py_library(
name = "__native_rule_import_list_for_test",
imports = ["baz"],
tags = ["manual"],
)

py_library_import_pathing_test_suite()
117 changes: 117 additions & 0 deletions py/tests/import-pathing/tests.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,117 @@
load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts", "unittest")
load("//py/private:py_library.bzl", _py_library = "py_library", py_library = "py_library_utils")

def _ctx_with_imports(imports, deps = []):
return struct(
attr = struct(
deps = deps,
imports = imports,
),
build_file_path = "foo/bar/BUILD.bazel",
workspace_name = "aspect_rules_py",
label = Label("//foo/bar:baz"),
)

def _can_resolve_path_in_workspace_test_impl(ctx):
env = unittest.begin(ctx)

# The import path is the current package
fake_ctx = _ctx_with_imports(["."])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py/foo/bar", imports[0])

# The import path is the parent package
fake_ctx = _ctx_with_imports([".."])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py/foo", imports[0])

# The import path is both the current and parent package
fake_ctx = _ctx_with_imports([".", ".."])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py/foo/bar", imports[0])
asserts.equals(env, "aspect_rules_py/foo", imports[1])

# The import path is the current package, but with trailing /
fake_ctx = _ctx_with_imports(["./"])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py/foo/bar", imports[0])

# The import path has some other child path in it
fake_ctx = _ctx_with_imports(["./baz"])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py/foo/bar/baz", imports[0])

# The import path is at the root
fake_ctx = _ctx_with_imports(["../.."])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py", imports[0])

# The relative import path is longer than the path from the root
fake_ctx = _ctx_with_imports(["../some/python/library/imp/path"])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py/foo/some/python/library/imp/path", imports[0])

# Transitive imports are included in depset
fake_ctx = _ctx_with_imports([".."], [ctx.attr.import_dep])
imports = py_library.make_imports_depset(fake_ctx).to_list()
asserts.equals(env, "aspect_rules_py/py/tests/import-pathing/baz", imports[0])
asserts.equals(env, "aspect_rules_py/foo", imports[1])

return unittest.end(env)

_can_resolve_path_in_workspace_test = unittest.make(
_can_resolve_path_in_workspace_test_impl,
attrs = {
"import_dep": attr.label(
default = "//py/tests/import-pathing:__native_rule_import_list_for_test",
),
},
)

def _fails_on_imp_path_that_breaks_workspace_root_imp(ctx):
env = analysistest.begin(ctx)
asserts.expect_failure(env, "Import paths must not escape the workspace root")
return analysistest.end(env)

_fails_on_imp_path_that_breaks_workspace_root_test = analysistest.make(
_fails_on_imp_path_that_breaks_workspace_root_imp,
expect_failure = True,
)

def _fails_on_absolute_imp_impl(ctx):
env = analysistest.begin(ctx)
asserts.expect_failure(env, "Absolute paths are not supported")
return analysistest.end(env)

_fails_on_absolute_imp_test = analysistest.make(
_fails_on_absolute_imp_impl,
expect_failure = True,
)

def py_library_import_pathing_test_suite():
unittest.suite(
"py_library_import_pathing_test_suite",
_can_resolve_path_in_workspace_test,
)

_py_library(
name = "__imports_break_out_of_root_lib",
imports = ["../../../.."],
tags = ["manual"],
)

_fails_on_imp_path_that_breaks_workspace_root_test(
name = "imp_path_that_breaks_workspace_root",
target_under_test = ":__imports_break_out_of_root_lib",
)

_py_library(
name = "__imp_path_is_absolute_lib",
imports = ["/foo"],
tags = ["manual"],
)

_fails_on_absolute_imp_test(
name = "imp_path_can_not_be_absolute",
target_under_test = ":__imp_path_is_absolute_lib",
)