diff options
Diffstat (limited to 'src')
4 files changed, 65 insertions, 38 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java index 0898b3617f..2b1f76c2d4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java @@ -113,13 +113,6 @@ public class SkylarkImportLookupFunction implements SkyFunction { throws InconsistentFilesystemException, SkylarkImportFailedException, InterruptedException { PathFragment filePath = fileLabel.toPathFragment(); - boolean loadingFromDefaultlRepo = fileLabel.getPackageIdentifier().getRepository().isDefault(); - if (inWorkspace && !loadingFromDefaultlRepo) { - // Loads of files in external repos are currently prohibited in a WORKSPACE file to prevent - // circular skyframe dependencies. - throw SkylarkImportFailedException.noExternalLoadsFromWorkspace(fileLabel); - } - // Load the AST corresponding to this file. ASTFileLookupValue astLookupValue; try { diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java index 82d0b12780..86890628c7 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.skyframe; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -22,7 +23,6 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.packages.ConstantRuleVisibility; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; -import com.google.devtools.build.lib.skyframe.SkylarkImportLookupFunction.SkylarkImportFailedException; import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.ErrorInfo; @@ -216,7 +216,7 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { } @Test - public void testLoadFromExternalRepoInWorkspaceFileDisallowed() throws Exception { + public void testLoadFromExternalRepoInWorkspaceFileAllowed() throws Exception { scratch.deleteFile("tools/build_rules/prelude_blaze"); scratch.overwriteFile("WORKSPACE", "local_repository(", @@ -234,11 +234,6 @@ public class SkylarkImportLookupFunctionTest extends BuildViewTestCase { SkyframeExecutorTestUtils.evaluate( getSkyframeExecutor(), skylarkImportLookupKey, /*keepGoing=*/ false, reporter); - assertTrue(result.hasError()); - ErrorInfo errorInfo = result.getError(skylarkImportLookupKey); - String errorMessage = errorInfo.getException().getMessage(); - String expectedMsgTemplate = SkylarkImportFailedException.NO_EXT_WORKSPACE_LOAD_MSG_TEMPLATE; - assertEquals(String.format(expectedMsgTemplate, "@a_remote_repo//remote_pkg:ext.bzl"), - errorMessage); + assertFalse(result.hasError()); } } diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index a75c93ff57..7f9e9e1713 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java @@ -721,4 +721,38 @@ public class SkylarkRuleContextTest extends SkylarkTestCase { Label depLabel = (Label) evalRuleContextCode(context, "ruleContext.attr.internal_dep.label"); assertThat(depLabel).isEqualTo(Label.parseAbsolute("@r//:dep")); } + + @Test + public void testExternalWorkspaceLoad() throws Exception { + scratch.file( + "/r1/BUILD", + "filegroup(name = 'test',", + " srcs = ['test.txt'],", + " visibility = ['//visibility:public'],", + ")"); + scratch.file("/r1/WORKSPACE"); + scratch.file("/r2/BUILD", "exports_files(['test.bzl'])"); + scratch.file( + "/r2/test.bzl", + "def macro(name, path):", + " native.local_repository(name = name, path = path)" + ); + scratch.file( + "/r2/other_test.bzl", + "def other_macro(name, path):", + " print(name + ': ' + path)" + ); + scratch.file("BUILD"); + scratch.overwriteFile( + "WORKSPACE", + "local_repository(name='r2', path='/r2')", + "load('@r2//:test.bzl', 'macro')", + "macro('r1', '/r1')", + "NEXT_NAME = 'r3'", + "load('@r2//:other_test.bzl', 'other_macro')", // We can still refer to r2 in other chunks. + "macro(NEXT_NAME, '/r2')" // and we can still use macro outside of its chunk. + ); + invalidatePackages(); + assertThat(getConfiguredTarget("@r1//:test")).isNotNull(); + } } diff --git a/src/test/shell/bazel/skylark_repository_test.sh b/src/test/shell/bazel/skylark_repository_test.sh index 3d59bda4be..3c20c9c321 100755 --- a/src/test/shell/bazel/skylark_repository_test.sh +++ b/src/test/shell/bazel/skylark_repository_test.sh @@ -117,7 +117,6 @@ EOF expect_not_log "Tra-la-la!" # No invalidation cat bazel-genfiles/zoo/ball-pit2.txt >$TEST_log expect_log "Tra-la-la!" - } function test_load_from_symlink_to_outside_of_workspace() { @@ -140,38 +139,44 @@ EOF rm -fr $TEST_TMPDIR/other } -# Loading a skylark file located in an external repo from a WORKSPACE file -# is disallowed. -function test_external_load_from_workspace_file_invalid() { +# Test load from repository. +function test_external_load_from_workspace() { create_new_workspace - external_repo=${new_workspace_dir} - - cat > ${WORKSPACE_DIR}/WORKSPACE <<EOF -local_repository(name = "external_repo", path = "${external_repo}") -load("@external_repo//external_pkg:ext.bzl", "CONST") -EOF + repo2=$new_workspace_dir - mkdir ${WORKSPACE_DIR}/local_pkg - cat > ${WORKSPACE_DIR}/local_pkg/BUILD <<EOF + mkdir -p carnivore + cat > carnivore/BUILD <<'EOF' genrule( - name = "shouldnt_be_built", - cmd = "echo echo" + name = "mongoose", + cmd = "echo 'Tra-la-la!' | tee $@", + outs = ["moogoose.txt"], + visibility = ["//visibility:public"], ) EOF - mkdir ${external_repo}/external_pkg - touch ${external_repo}/external_pkg/BUILD - cat > ${external_repo}/external_pkg/ext.bzl <<EOF -CONST = 17 + create_new_workspace + repo3=$new_workspace_dir + # Our macro + cat >WORKSPACE + cat >test.bzl <<EOF +def macro(path): + print('bleh') + native.local_repository(name='endangered', path=path) +EOF + cat >BUILD <<'EOF' +exports_files(["test.bzl"]) EOF cd ${WORKSPACE_DIR} - bazel build local_pkg:shouldnt_be_built >& $TEST_log && \ - fail "Expected build to fail" || true + cat > WORKSPACE <<EOF +local_repository(name='proxy', path='$repo3') +load('@proxy//:test.bzl', 'macro') +macro('$repo2') +EOF - expect_log "Extension file '@external_repo//external_pkg:ext.bzl' may not be \ -loaded from a WORKSPACE file since the extension file is located in an \ -external repository." + bazel build @endangered//carnivore:mongoose >& $TEST_log \ + || fail "Failed to build" + expect_log "bleh." } function tear_down() { |