Skip to content

Commit

Permalink
Be explicit about alowing empty globs
Browse files Browse the repository at this point in the history
There are several globs that are empty and this prevents building
with the incompatible flag bazelbuild#8195.
This commit just makes it explicit that empty is allowed.

Closes bazelbuild#15330.

PiperOrigin-RevId: 470706653
Change-Id: I4ca3323aa5651315b56d2316fa8341deaf2bbb4a
  • Loading branch information
limdor authored and copybara-github committed Aug 29, 2022
1 parent db64e7e commit 6ad7054
Show file tree
Hide file tree
Showing 11 changed files with 29 additions and 23 deletions.
1 change: 1 addition & 0 deletions BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ filegroup(
name = "git",
srcs = glob(
[".git/**"],
allow_empty = True,
exclude = [".git/**/*[*"], # gitk creates temp files with []
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ public void testInvalidVisibilityWithSelect() throws Exception {
" hdrs = select({",
" ':fastbuild': glob([",
" '*.h',",
" ]),",
" ], allow_empty = True),",
" }),",
")");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1076,15 +1076,16 @@ public void selectsWithGlobs() throws Exception {
@Test
public void selectsWithGlobsWrongType() throws Exception {
writeConfigRules();
scratch.file("foo/BUILD",
scratch.file(
"foo/BUILD",
"genrule(",
" name = 'gen',",
" srcs = [],",
" outs = ['gen.out'],",
" cmd = 'echo' + select({",
" '//conditions:a': 'a',",
" '//conditions:b': 'b',",
" }) + glob(['globbed.java']))");
" }) + glob(['globbed.java'], allow_empty = True))");

reporter.removeHandler(failFastHandler);
assertThrows(NoSuchTargetException.class, () -> getTarget("//foo:binary"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
"java_runtime_alias(name = 'current_java_runtime')",
"java_host_runtime_alias(name = 'current_host_java_runtime')",
"filegroup(name='bootclasspath', srcs=['jdk/jre/lib/rt.jar'])",
"filegroup(name='extdir', srcs=glob(['jdk/jre/lib/ext/*']))",
"filegroup(name='extdir', srcs=glob(['jdk/jre/lib/ext/*'], allow_empty = True))",
"filegroup(name='java', srcs = ['jdk/jre/bin/java'])",
"filegroup(name='JacocoCoverage', srcs = ['JacocoCoverage_deploy.jar'])",
"exports_files([",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1143,7 +1143,7 @@ public void testGlobPatternExtractor() {
List<String> globsWithDirs = new ArrayList<>();
List<String> subpackages = new ArrayList<>();
PackageFactory.checkBuildSyntax(
file, globs, globsWithDirs, subpackages, new HashMap<>(), /*eventHandler=*/ null);
file, globs, globsWithDirs, subpackages, new HashMap<>(), /* errors= */ null);
assertThat(globs).containsExactly("ab", "a", "**/*");
assertThat(globsWithDirs).containsExactly("c");
assertThat(subpackages).isEmpty();
Expand Down Expand Up @@ -1346,7 +1346,7 @@ private Package evaluateGlob(
scratch.file(
"globs/BUILD",
String.format(
"result = glob(%s, exclude=%s, exclude_directories=%d)",
"result = glob(%s, exclude=%s, exclude_directories=%d, allow_empty = True)",
Starlark.repr(includes), Starlark.repr(excludes), excludeDirs ? 1 : 0),
resultAssertion);
return loadPackage("globs");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public void testNonMultidexBuildStructure() throws Exception {
" name = 'nomultidex',",
" srcs = ['a.java'],",
" manifest = 'AndroidManifest.xml',",
" resource_files = glob(['res/**']),",
" resource_files = glob(['res/**'], allow_empty = True),",
")");
internalTestNonMultidexBuildStructure("//java/foo:nomultidex");
}
Expand All @@ -74,7 +74,7 @@ public void testDefaultBuildStructure() throws Exception {
" name = 'default',",
" srcs = ['a.java'],",
" manifest = 'AndroidManifest.xml',",
" resource_files = glob(['res/**']))");
" resource_files = glob(['res/**'], allow_empty = True))");
internalTestNonMultidexBuildStructure("//java/foo:default");
}

Expand All @@ -87,7 +87,7 @@ public void testManualMainDexMode() throws Exception {
" name = 'manual_main_dex',",
" srcs = ['a.java'],",
" manifest = 'AndroidManifest.xml',",
" resource_files = glob(['res/**']),",
" resource_files = glob(['res/**'], allow_empty = True),",
" multidex = 'manual_main_dex',",
" main_dex_list = 'main_dex_list.txt')");
internalTestMultidexBuildStructure("//java/foo:manual_main_dex", MultidexMode.MANUAL_MAIN_DEX);
Expand All @@ -107,7 +107,7 @@ public void testLegacyMultidexBuildStructure() throws Exception {
" name = 'legacy',",
" srcs = ['a.java'],",
" manifest = 'AndroidManifest.xml',",
" resource_files = glob(['res/**']),",
" resource_files = glob(['res/**'], allow_empty = True),",
" multidex = 'legacy')");
internalTestMultidexBuildStructure("//java/foo:legacy", MultidexMode.LEGACY);
}
Expand All @@ -126,7 +126,7 @@ public void testNativeMultidexBuildStructure() throws Exception {
" name = 'native',",
" srcs = ['a.java'],",
" manifest = 'AndroidManifest.xml',",
" resource_files = glob(['res/**']),",
" resource_files = glob(['res/**'], allow_empty = True),",
" multidex = 'native')");
internalTestMultidexBuildStructure("//java/foo:native", MultidexMode.NATIVE);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ private void writeDataBindingFilesWithNoResourcesDep() throws Exception {
" enable_data_binding = 1,",
" manifest = 'AndroidManifest.xml',",
" srcs = ['LibWithResourceFiles.java'],",
" resource_files = glob(['res/**']),",
" resource_files = glob(['res/**'], allow_empty = True),",
")");
scratch.file(
"java/android/lib_with_resource_files/LibWithResourceFiles.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -571,8 +571,8 @@ private static Iterable<Label> getSrcs(Package pkg, String targetName)
public void testOneNewElementInMultipleGlob() throws Exception {
scratch.file(
"foo/BUILD",
"sh_library(name = 'foo', srcs = glob(['*.sh']))",
"sh_library(name = 'bar', srcs = glob(['*.sh', '*.txt']))");
"sh_library(name = 'foo', srcs = glob(['*.sh'], allow_empty = True))",
"sh_library(name = 'bar', srcs = glob(['*.sh', '*.txt'], allow_empty = True))");
preparePackageLoading(rootDirectory);
SkyKey skyKey = PackageValue.key(PackageIdentifier.createInMainRepo("foo"));
Package pkg = validPackageWithoutErrors(skyKey);
Expand All @@ -589,8 +589,8 @@ public void testOneNewElementInMultipleGlob() throws Exception {
public void testNoNewElementInMultipleGlob() throws Exception {
scratch.file(
"foo/BUILD",
"sh_library(name = 'foo', srcs = glob(['*.sh', '*.txt']))",
"sh_library(name = 'bar', srcs = glob(['*.sh', '*.txt']))");
"sh_library(name = 'foo', srcs = glob(['*.sh', '*.txt'], allow_empty = True))",
"sh_library(name = 'bar', srcs = glob(['*.sh', '*.txt'], allow_empty = True))");
preparePackageLoading(rootDirectory);
SkyKey skyKey = PackageValue.key(PackageIdentifier.createInMainRepo("foo"));
Package pkg = validPackageWithoutErrors(skyKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,8 +300,10 @@ public void testChangeDirectory() throws Exception {
scratch.file("python/hello/foo.txt", "foo");

// A dummy directory that is not changed.
scratch.file("misc/BUILD",
"py_binary(name = 'misc', srcs = ['other.py'], data = glob(['*.txt']))");
scratch.file(
"misc/BUILD",
"py_binary(name = 'misc', srcs = ['other.py'], data = glob(['*.txt'], allow_empty ="
+ " True))");

sync("//python/hello:hello", "//misc:misc");

Expand Down Expand Up @@ -651,8 +653,10 @@ private void sync(String... labelStrings) throws Exception {
@Test
public void testInterruptLoadedTarget() throws Exception {
analysisMock.pySupport().setup(mockToolsConfig);
scratch.file("python/hello/BUILD",
"py_binary(name = 'hello', srcs = ['hello.py'], data = glob(['*.txt']))");
scratch.file(
"python/hello/BUILD",
"py_binary(name = 'hello', srcs = ['hello.py'], data = glob(['*.txt'], allow_empty ="
+ " True))");
Thread.currentThread().interrupt();
LoadedPackageProvider packageProvider =
new LoadedPackageProvider(skyframeExecutor.getPackageManager(), reporter);
Expand Down
4 changes: 2 additions & 2 deletions src/test/shell/integration/loading_phase_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -240,13 +240,13 @@ function test_glob_with_subpackage() {
assert_equals "3" $(wc -l "$TEST_log")

# glob returns an empty list, because t3.txt is outside the package
echo "exports_files(glob(['subpkg/t3.txt']))" >$pkg/p/BUILD
echo "exports_files(glob(['subpkg/t3.txt'], allow_empty = True))" >$pkg/p/BUILD
bazel query "$pkg/p:*" -k >$TEST_log || fail "Expected success"
expect_log "//$pkg/p:BUILD"
assert_equals "1" $(wc -l "$TEST_log")

# same test, with a nonexisting file
echo "exports_files(glob(['subpkg/no_glob.txt']))" >$pkg/p/BUILD
echo "exports_files(glob(['subpkg/no_glob.txt'], allow_empty = True))" >$pkg/p/BUILD
bazel query "$pkg/p:*" -k >$TEST_log || fail "Expected success"
expect_log "//$pkg/p:BUILD"
assert_equals "1" $(wc -l "$TEST_log")
Expand Down
2 changes: 1 addition & 1 deletion src/test/shell/integration/run_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ EOF
cat > cc/BUILD <<EOF
cc_binary(name='kitty',
srcs=['kitty.cc'],
data=glob(['*.txt']))
data=glob(['*.txt'], allow_empty = True))
EOF
}

Expand Down

0 comments on commit 6ad7054

Please sign in to comment.