From ed598bc180ff347f50ca70a04074f430185aa9c5 Mon Sep 17 00:00:00 2001 From: dannark Date: Tue, 7 Aug 2018 16:31:52 -0700 Subject: Put main repo remapping behind a flag. RELNOTES: None PiperOrigin-RevId: 207801155 --- .../lib/packages/SkylarkSemanticsOptions.java | 26 ++++++++++++++-------- .../build/lib/packages/WorkspaceFactory.java | 22 ++++++++++-------- .../build/lib/syntax/SkylarkSemantics.java | 5 +++++ .../packages/SkylarkSemanticsConsistencyTest.java | 2 ++ .../build/lib/packages/WorkspaceFactoryTest.java | 10 ++++++++- .../skyframe/RepositoryMappingFunctionTest.java | 17 +++++--------- src/test/shell/bazel/workspace_test.sh | 9 +++++--- 7 files changed, 58 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java index 3277c38440..5ef70d319b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java @@ -58,15 +58,6 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable // <== Add new options here in alphabetic order ==> - @Option( - name = "experimental_enable_repo_mapping", - defaultValue = "false", - documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, - effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS, - help = "If set to true, enables the use of the `repo_mapping` attribute in WORKSPACE files." - ) - public boolean experimentalEnableRepoMapping; - @Option( name = "experimental_cc_skylark_api_enabled_packages", converter = CommaSeparatedOptionListConverter.class, @@ -79,6 +70,22 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable + "we will be making breaking changes.") public List experimentalCcSkylarkApiEnabledPackages; + @Option( + name = "experimental_enable_repo_mapping", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS, + help = "If set to true, enables the use of the `repo_mapping` attribute in WORKSPACE files.") + public boolean experimentalEnableRepoMapping; + + @Option( + name = "experimental_remap_main_repo", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = OptionEffectTag.LOADING_AND_ANALYSIS, + help = "If set to true, will treat references to '@
' the same as '@'.") + public boolean experimentalRemapMainRepo; + @Option( name = "incompatible_bzl_disallow_load_after_statement", defaultValue = "false", @@ -363,6 +370,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable // <== Add new options here in alphabetic order ==> .experimentalCcSkylarkApiEnabledPackages(experimentalCcSkylarkApiEnabledPackages) .experimentalEnableRepoMapping(experimentalEnableRepoMapping) + .experimentalRemapMainRepo(experimentalRemapMainRepo) .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement) .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable) .incompatibleDepsetUnion(incompatibleDepsetUnion) diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 6cc7ddfffe..c412181506 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java @@ -346,10 +346,12 @@ public class WorkspaceFactory { } // Add entry in repository map from "@name" --> "@" to avoid issue where bazel // treats references to @name as a separate external repo - builder.addRepositoryMappingEntry( - RepositoryName.MAIN, - RepositoryName.createFromValidStrippedName(name), - RepositoryName.MAIN); + if (env.getSemantics().experimentalRemapMainRepo()) { + builder.addRepositoryMappingEntry( + RepositoryName.MAIN, + RepositoryName.createFromValidStrippedName(name), + RepositoryName.MAIN); + } return NONE; } }; @@ -498,11 +500,13 @@ public class WorkspaceFactory { // Add an entry in every repository from @ to "@" to avoid treating // @ as a separate repository. This will be overridden if the main // repository has a repo_mapping entry from to something. - if (!Strings.isNullOrEmpty(builder.pkg.getWorkspaceName())) { - builder.addRepositoryMappingEntry( - RepositoryName.createFromValidStrippedName(externalRepoName), - RepositoryName.createFromValidStrippedName(builder.pkg.getWorkspaceName()), - RepositoryName.MAIN); + if (env.getSemantics().experimentalRemapMainRepo()) { + if (!Strings.isNullOrEmpty(builder.pkg.getWorkspaceName())) { + builder.addRepositoryMappingEntry( + RepositoryName.createFromValidStrippedName(externalRepoName), + RepositoryName.createFromValidStrippedName(builder.pkg.getWorkspaceName()), + RepositoryName.MAIN); + } } if (env.getSemantics().experimentalEnableRepoMapping()) { if (kwargs.containsKey("repo_mapping")) { diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java index 5893ac49ab..85f6d4d212 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java @@ -45,6 +45,8 @@ public abstract class SkylarkSemantics { public abstract boolean experimentalEnableRepoMapping(); + public abstract boolean experimentalRemapMainRepo(); + public abstract boolean incompatibleBzlDisallowLoadAfterStatement(); public abstract boolean incompatibleDepsetIsNotIterable(); @@ -102,6 +104,7 @@ public abstract class SkylarkSemantics { // <== Add new options here in alphabetic order ==> .experimentalCcSkylarkApiEnabledPackages(ImmutableList.of()) .experimentalEnableRepoMapping(false) + .experimentalRemapMainRepo(false) .incompatibleBzlDisallowLoadAfterStatement(false) .incompatibleDepsetIsNotIterable(false) .incompatibleDepsetUnion(false) @@ -133,6 +136,8 @@ public abstract class SkylarkSemantics { public abstract Builder experimentalEnableRepoMapping(boolean value); + public abstract Builder experimentalRemapMainRepo(boolean value); + public abstract Builder incompatibleBzlDisallowLoadAfterStatement(boolean value); public abstract Builder incompatibleDepsetIsNotIterable(boolean value); diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java index 21328c8ff1..5bc110986c 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java @@ -123,6 +123,7 @@ public class SkylarkSemanticsConsistencyTest { + "," + rand.nextDouble(), "--experimental_enable_repo_mapping=" + rand.nextBoolean(), + "--experimental_remap_main_repo=" + rand.nextBoolean(), "--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(), "--incompatible_depset_is_not_iterable=" + rand.nextBoolean(), "--incompatible_depset_union=" + rand.nextBoolean(), @@ -155,6 +156,7 @@ public class SkylarkSemanticsConsistencyTest { .experimentalCcSkylarkApiEnabledPackages( ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble()))) .experimentalEnableRepoMapping(rand.nextBoolean()) + .experimentalRemapMainRepo(rand.nextBoolean()) .incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean()) .incompatibleDepsetIsNotIterable(rand.nextBoolean()) .incompatibleDepsetUnion(rand.nextBoolean()) diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java index de9b48e825..c964539e7a 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java @@ -129,7 +129,6 @@ public class WorkspaceFactoryTest { " repo_mapping = {'@x' : '@y'},", ")"); assertMapping(helper, "@foo", "@x", "@y"); - assertMapping(helper, "@foo", "@bar", "@"); } @Test @@ -200,10 +199,19 @@ public class WorkspaceFactoryTest { @Test public void testImplicitMainRepoRename() throws Exception { + helper.setSkylarkSemantics("--experimental_remap_main_repo"); helper.parse("workspace(name = 'foo')"); assertMapping(helper, "@", "@foo", "@"); } + @Test + public void testNoImplicitMainRepoRenameWithoutFlag() throws Exception { + helper.parse("workspace(name = 'foo')"); + RepositoryName foo = RepositoryName.create("@foo"); + assertThat(helper.getPackage().getRepositoryMapping("@")) + .doesNotContainEntry(foo, RepositoryName.MAIN); + } + @Test public void testEmptyRepositoryHasEmptyMap() throws Exception { helper.parse(""); diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java index 737fa5b51c..78a95305f6 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java @@ -67,9 +67,7 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase { .hasEntryThat(skyKey) .isEqualTo( RepositoryMappingValue.withMapping( - ImmutableMap.of( - RepositoryName.create("@good"), RepositoryName.MAIN, - RepositoryName.create("@a"), RepositoryName.create("@b")))); + ImmutableMap.of(RepositoryName.create("@a"), RepositoryName.create("@b")))); } @Test @@ -97,16 +95,12 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase { .hasEntryThat(skyKey1) .isEqualTo( RepositoryMappingValue.withMapping( - ImmutableMap.of( - RepositoryName.create("@good"), RepositoryName.MAIN, - RepositoryName.create("@a"), RepositoryName.create("@b")))); + ImmutableMap.of(RepositoryName.create("@a"), RepositoryName.create("@b")))); assertThatEvaluationResult(eval(skyKey2)) .hasEntryThat(skyKey2) .isEqualTo( RepositoryMappingValue.withMapping( - ImmutableMap.of( - RepositoryName.create("@good"), RepositoryName.MAIN, - RepositoryName.create("@x"), RepositoryName.create("@y")))); + ImmutableMap.of(RepositoryName.create("@x"), RepositoryName.create("@y")))); } @Test @@ -128,7 +122,6 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase { .isEqualTo( RepositoryMappingValue.withMapping( ImmutableMap.of( - RepositoryName.create("@good"), RepositoryName.MAIN, RepositoryName.create("@a"), RepositoryName.create("@b"), RepositoryName.create("@x"), RepositoryName.create("@y")))); } @@ -157,7 +150,8 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase { @Test public void testDefaultMainRepoNameInMapping() throws Exception { - setSkylarkSemanticsOptions("--experimental_enable_repo_mapping"); + setSkylarkSemanticsOptions( + "--experimental_remap_main_repo", "--experimental_enable_repo_mapping"); scratch.overwriteFile( "WORKSPACE", "local_repository(", @@ -179,6 +173,7 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase { @Test public void testExplicitMainRepoNameInMapping() throws Exception { + setSkylarkSemanticsOptions("--experimental_remap_main_repo"); scratch.overwriteFile( "WORKSPACE", "workspace(name = 'good')", diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh index 680d71153e..daed1375c4 100755 --- a/src/test/shell/bazel/workspace_test.sh +++ b/src/test/shell/bazel/workspace_test.sh @@ -574,7 +574,8 @@ a = 1 EOF cd mainrepo - bazel query //... &>"$TEST_log" || fail "Expected query to succeed" + bazel query --experimental_remap_main_repo //... &>"$TEST_log" \ + || fail "Expected query to succeed" expect_log "def.bzl loaded" expect_not_log "external" } @@ -601,7 +602,8 @@ EOF # the bzl file should be loaded from the main workspace and # not as an external repository cd mainrepo - bazel query @a//... &>"$TEST_log" || fail "Expected query to succeed" + bazel query --experimental_remap_main_repo @a//... &>"$TEST_log" \ + || fail "Expected query to succeed" expect_log "def.bzl loaded" expect_not_log "external" @@ -618,7 +620,8 @@ EOF # now that @mainrepo doesn't exist within workspace "a", # the query should fail cd mainrepo - bazel query --experimental_enable_repo_mapping @a//... &>"$TEST_log" \ + bazel query --experimental_remap_main_repo --experimental_enable_repo_mapping \ + @a//... &>"$TEST_log" \ && fail "Failure expected" || true } -- cgit v1.2.3