Skip to content

Commit

Permalink
WIP: external_data: Hoist lint checking to Drake; simplify dependencies.
Browse files Browse the repository at this point in the history
  • Loading branch information
EricCousineau-TRI committed Jan 9, 2018
1 parent 0c94752 commit d4c5d86
Show file tree
Hide file tree
Showing 15 changed files with 111 additions and 133 deletions.
8 changes: 0 additions & 8 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,4 @@ install(
]),
)

# This provides the directories necessary for `external_data:workspace_test` to
# consume what it needs from `@drake//`.
filegroup(
name = "external_data_meta_testing_files",
srcs = glob(["*"]),
visibility = ["//tools/external_data:__pkg__"],
)

add_lint_tests()
16 changes: 13 additions & 3 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,13 @@ github_archive(
build_file = "tools/workspace/styleguide/styleguide.BUILD.bazel", # noqa
)

load("//tools/lint:lint_repositories.bzl", "lint_repositories")

lint_repositories()
github_archive(
name = "pycodestyle",
repository = "PyCQA/pycodestyle",
commit = "2.3.1",
sha256 = "e9fc1ca3fd85648f45c0d2e33591b608a17d8b9b78e22c5f898e831351bacb03", # noqa
build_file = "tools/workspace/pycodestyle/pycodestyle.BUILD.bazel",
)

bitbucket_archive(
name = "eigen",
Expand Down Expand Up @@ -319,6 +323,12 @@ github_archive(
build_file = "tools/workspace/yaml_cpp/yaml_cpp.BUILD.bazel",
)

load("//tools/workspace/buildifier:buildifier.bzl", "buildifier_repository")

buildifier_repository(
name = "buildifier",
)

load("//tools/workspace/gurobi:gurobi.bzl", "gurobi_repository")

gurobi_repository(
Expand Down
15 changes: 0 additions & 15 deletions tools/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -71,19 +71,4 @@ config_setting(
values = {"define": "NO_SCS=ON"},
)

# This provides the directories necessary for `external_data:workspace_test` to
# consume what it needs from `@drake//`.
filegroup(
name = "external_data_meta_testing_files",
# `glob(..., exclude_directories=0)` will include package directories under
# `bazel run`, but not `bazel test`.
srcs = glob(["*"]) + [
"install",
"lint",
"skylark",
"workspace",
],
visibility = ["//tools/external_data:__pkg__"],
)

add_lint_tests()
41 changes: 17 additions & 24 deletions tools/external_data/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,35 +1,28 @@
load("//tools/lint:lint.bzl", "add_lint_tests")
load(":external_data.bzl", "external_data_stub_test")

# Use a symlink forest, so `bazel run` of `workspace_test` will not pollute the
# source directory with `bazel-*` symlinks.
filegroup(
name = "workspace_symlink_forest",
srcs = glob(
["workspace/*"],
exclude_directories = 0,
),
load(
"@bazel_external_data_pkg//test:workspace_test.bzl",
"workspace_test",
)

# Run all tests in `workspace` as a separate package.
sh_test(
workspace_test(
name = "workspace_test",
srcs = ["test/workspace_test.sh"],
# We must use a file, and not just `workspace`, because Bazel will not give
# us the location unless it's in `data`. We do NOT want this, because
# we want a symlink forest. (See above.)
args = ["$(location workspace/WORKSPACE)"],
data = [
# Must explicitly list this file so that we can get its location.
"workspace/WORKSPACE",
"workspace_symlink_forest",
# Get necessary upstream files from Drake.
"//tools:external_data_meta_testing_files",
"//:external_data_meta_testing_files",
],
workspace = "workspace",
)

# Ensure that we can consume `external_data.bzl`.
external_data_stub_test()

add_lint_tests()
# Ensure that we add all relevant files under `workspace`.
# We should be able to glob them, as they are not a package.
add_lint_tests(
bazel_lint_extra_srcs = glob([
"workspace/**/BUILD.bazel",
"workspace/**/WORKSPACE",
"workspace/**/*.bzl",
]),
python_lint_extra_srcs = glob([
"workspace/**/*.py",
]),
)
4 changes: 0 additions & 4 deletions tools/external_data/workspace/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

package(default_visibility = ["//visibility:public"])

load("//tools:lint.bzl", "add_lint_tests")

# Expose this package's main bits to be consumed via a test.
filegroup(
name = "meta_testing_files",
Expand All @@ -15,5 +13,3 @@ filegroup(
],
visibility = ["//test:__pkg__"],
)

add_lint_tests()
15 changes: 0 additions & 15 deletions tools/external_data/workspace/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,3 @@ local_repository(
name = "bazel_external_data_test_ignore",
path = "test/workspaces",
)

# Add `drake` to access lint tools.
local_repository(
name = "drake",
path = dirname(__workspace_dir__, 3),
)

load("@drake//tools/lint:lint_repositories.bzl", "lint_repositories")

lint_repositories(
include = [
"buildifier",
"pycodestyle",
],
)
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
# -*- python -*-

load("//tools:lint.bzl", "add_lint_tests")

# For macro testing.
exports_files(["stub_test.py"])

Expand All @@ -11,5 +9,3 @@ py_test(
srcs = ["stub_test.py"],
visibility = ["//visibility:public"],
)

add_lint_tests()
22 changes: 9 additions & 13 deletions tools/external_data/workspace/test/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,23 +1,19 @@
# -*- python -*-
load("//tools:lint.bzl", "add_lint_tests")
load(":workspace_writeable_test.bzl", "workspace_writeable_test")
load(":workspace_test.bzl", "workspace_test")

root_data = ["//:meta_testing_files"]

workspace_writeable_test(
workspace_test(
name = "bazel_pkg_test",
data = root_data,
workspace = "workspaces/bazel_pkg_test",
writeable = 1,
)

# Ensure that we add all relevant files under `workspaces`.
# We should be able to glob them, as they are not a package.
add_lint_tests(
bazel_lint_extra_srcs = glob([
"workspaces/**/BUILD.bazel",
"workspaces/**/WORKSPACE",
]),
python_lint_extra_srcs = glob([
"workspaces/**/*.py",
]),
exports_files(
srcs = [
"workspace_test.sh",
"workspace_writeable_test.sh",
],
visibility = ["//visibility:public"],
)
66 changes: 66 additions & 0 deletions tools/external_data/workspace/test/workspace_test.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# -*- python -*-

# Default command. Do not run linting tests, so that the downstream test
# packages do not need to consume `drake`.
# Needs quotes, because `sh_test(args = [...])` just concatenates them???
_CMD_DEFAULT = "'bazel test //...'"

def _get_target(name):
return "@bazel_external_data_pkg//test:" + name

def workspace_test(
name,
workspace,
cmd = _CMD_DEFAULT,
data = [],
writeable = 0):
"""Provides a unittest access to a given workspace
contained in the current project.
@param workspace
Workspace directory relative to this package.
@param cmd
Command to run. By default is `bazel test //...`.
@param data
Additional data (e.g. other workspaces).
@param writeable
If the data should be copied such that modifications can be made. """

if writeable:
args = [cmd, "$(location {})".format(workspace)]
for datum in data:
args.append("$(locations {})".format(datum))
native.sh_test(
name = name,
srcs = [_get_target("workspace_writeable_test.sh")],
args = args,
data = [workspace] + data,
)
else:
# Use a symlink forest, so `bazel run` of `workspace_test` will not
# pollute the source directory with `bazel-*` symlinks.
workspace_filegroup = name + "_filegroup"
# We must use a file, and not just `workspace`, because Bazel will not
# give us the location unless it's in `data`. We do NOT want this,
# because we want a symlink forest. (See above.)
workspace_file = workspace + "/WORKSPACE"
native.filegroup(
name = workspace_filegroup,
srcs = native.glob(
[workspace + "/*"],
exclude_directories = 0,
),
)
native.sh_test(
name = name,
srcs = [_get_target("workspace_test.sh")],
args = [
cmd,
"$(location {})".format(workspace_file)],
data = [
# Must explicitly list this file so that we can get its
# location.
workspace_file,
workspace_filegroup,
] + data,
)
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
#!/bin/bash
set -e -u -x

workspace_file=${1}
cmd=${1}
workspace_file=${2}
cd $(dirname ${workspace_file})

# This message will only show up if symlinks are present and the user runs this
Expand All @@ -19,6 +20,4 @@ EOF
fi

# Run all the tests.
# Since this is pure Python + Bazel, we shouldn't care about propagating any
# configuration flags.
bazel test //...
eval ${cmd}
27 changes: 0 additions & 27 deletions tools/external_data/workspace/test/workspace_writeable_test.bzl

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,8 @@ done
# Change to the workspace directory.
cd ${mock_dir}/${pkg_reldir}

# Get rid of Bazel symlinks.
# Get rid of Bazel symlinks if they exist.
rm bazel-* || :
# Stub base workspace `add_lint_tests` so that each downstream project does not
# need Drake. `--test_tag_filters` won't work since Bazel needs to load the
# target to check tags.
cat > ${mock_dir}/tools/lint.bzl <<EOF
def add_lint_tests(*args, **kwargs): pass
EOF

# Execute command.
eval ${cmd}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*- python -*-

def dirname(p, remove=1):
def dirname(p, remove = 1):
pieces = p.split('/')
return '/'.join(pieces[0:-remove])
7 changes: 0 additions & 7 deletions tools/external_data/workspace/tools/lint.bzl

This file was deleted.

2 changes: 1 addition & 1 deletion tools/external_data/workspace/tools/path.bzl
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# -*- python -*-

def dirname(p, remove=1):
def dirname(p, remove = 1):
pieces = p.split('/')
return '/'.join(pieces[0:-remove])

0 comments on commit d4c5d86

Please sign in to comment.