Skip to content

Commit

Permalink
Create --incompatible_remap_main_repo flag.
Browse files Browse the repository at this point in the history
This flag fixes a bug in bazel where within a repository called "foo", the labels @foo//some/path:bar.bzl and //some/path:bar.bzl were treated as different labels. It is unlikely that this flag will affect any projects but it is technically a breaking change and so it is guarded behind a flag.

Towards #7130

RELNOTES: None
PiperOrigin-RevId: 229813128
  • Loading branch information
dkelmer authored and Copybara-Service committed Jan 17, 2019
1 parent accdde2 commit ed87463
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 24 deletions.
Expand Up @@ -117,14 +117,6 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
help = "Passes list of packages that can use the java_common.create_provider Starlark API.")
public List<String> experimentalJavaCommonCreateProviderEnabledPackages;

@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 '@<main repo name>' the same as '@'.")
public boolean experimentalRemapMainRepo;

@Option(
name = "experimental_platforms_api",
defaultValue = "false",
Expand Down Expand Up @@ -448,6 +440,19 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
+ "symbols introduced by load are not implicitly re-exported.")
public boolean incompatibleNoTransitiveLoads;

@Option(
name = "incompatible_remap_main_repo",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
effectTags = OptionEffectTag.LOADING_AND_ANALYSIS,
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
oldName = "experimental_remap_main_repo",
help = "If set to true, will treat references to '@<main repo name>' the same as '@'.")
public boolean incompatibleRemapMainRepo;

@Option(
name = "incompatible_remove_native_maven_jar",
defaultValue = "false",
Expand Down Expand Up @@ -508,7 +513,6 @@ public SkylarkSemantics toSkylarkSemantics() {
.experimentalCcSkylarkApiEnabledPackages(experimentalCcSkylarkApiEnabledPackages)
.experimentalEnableAndroidMigrationApis(experimentalEnableAndroidMigrationApis)
.experimentalEnableRepoMapping(experimentalEnableRepoMapping)
.experimentalRemapMainRepo(experimentalRemapMainRepo)
.experimentalJavaCommonCreateProviderEnabledPackages(
experimentalJavaCommonCreateProviderEnabledPackages)
.experimentalPlatformsApi(experimentalPlatformsApi)
Expand All @@ -535,6 +539,7 @@ public SkylarkSemantics toSkylarkSemantics() {
.incompatibleNoSupportToolsInActionInputs(incompatibleNoSupportToolsInActionInputs)
.incompatibleNoTargetOutputGroup(incompatibleNoTargetOutputGroup)
.incompatibleNoTransitiveLoads(incompatibleNoTransitiveLoads)
.incompatibleRemapMainRepo(incompatibleRemapMainRepo)
.incompatibleRemoveNativeMavenJar(incompatibleRemoveNativeMavenJar)
.incompatibleRequireFeatureConfigurationForPic(requireFeatureConfigurationForPic)
.incompatibleStricArgumentOrdering(incompatibleStricArgumentOrdering)
Expand Down
Expand Up @@ -342,7 +342,7 @@ public Object invoke(String name, FuncallExpression ast, Environment env)
}
// Add entry in repository map from "@name" --> "@" to avoid issue where bazel
// treats references to @name as a separate external repo
if (env.getSemantics().experimentalRemapMainRepo()) {
if (env.getSemantics().incompatibleRemapMainRepo()) {
builder.addRepositoryMappingEntry(
RepositoryName.MAIN,
RepositoryName.createFromValidStrippedName(name),
Expand Down
Expand Up @@ -99,7 +99,7 @@ public static Map<String, Object> getFinalKwargs(Map<String, Object> kwargs) {
*/
public static void addMainRepoEntry(
Package.Builder builder, String externalRepoName, SkylarkSemantics semantics) {
if (semantics.experimentalRemapMainRepo()) {
if (semantics.incompatibleRemapMainRepo()) {
if (!Strings.isNullOrEmpty(builder.getPackageWorkspaceName())) {
builder.addRepositoryMappingEntry(
RepositoryName.createFromValidStrippedName(externalRepoName),
Expand Down
Expand Up @@ -126,8 +126,6 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {

public abstract ImmutableList<String> experimentalJavaCommonCreateProviderEnabledPackages();

public abstract boolean experimentalRemapMainRepo();

public abstract boolean experimentalPlatformsApi();

public abstract boolean experimentalStarlarkConfigTransitions();
Expand Down Expand Up @@ -174,6 +172,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {

public abstract boolean incompatibleNoTransitiveLoads();

public abstract boolean incompatibleRemapMainRepo();

public abstract boolean incompatibleRemoveNativeMavenJar();

public abstract boolean incompatibleRequireFeatureConfigurationForPic();
Expand Down Expand Up @@ -204,7 +204,6 @@ public static Builder builderWithDefaults() {
.experimentalEnableAndroidMigrationApis(false)
.experimentalEnableRepoMapping(false)
.experimentalJavaCommonCreateProviderEnabledPackages(ImmutableList.of())
.experimentalRemapMainRepo(false)
.experimentalPlatformsApi(false)
.experimentalStarlarkConfigTransitions(false)
.experimentalTransitionWhitelistLocation("")
Expand All @@ -228,6 +227,7 @@ public static Builder builderWithDefaults() {
.incompatibleNoSupportToolsInActionInputs(false)
.incompatibleNoTargetOutputGroup(false)
.incompatibleNoTransitiveLoads(false)
.incompatibleRemapMainRepo(false)
.incompatibleRemoveNativeMavenJar(false)
.incompatibleRequireFeatureConfigurationForPic(true)
.incompatibleStricArgumentOrdering(false)
Expand All @@ -248,8 +248,6 @@ public abstract static class Builder {

public abstract Builder experimentalEnableRepoMapping(boolean value);

public abstract Builder experimentalRemapMainRepo(boolean value);

public abstract Builder experimentalJavaCommonCreateProviderEnabledPackages(List<String> value);

public abstract Builder experimentalPlatformsApi(boolean value);
Expand Down Expand Up @@ -300,6 +298,8 @@ public abstract static class Builder {

public abstract Builder incompatibleNoTransitiveLoads(boolean value);

public abstract Builder incompatibleRemapMainRepo(boolean value);

public abstract Builder incompatibleRemoveNativeMavenJar(boolean value);

public abstract Builder incompatibleStricArgumentOrdering(boolean value);
Expand Down
Expand Up @@ -130,7 +130,6 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
+ ","
+ rand.nextDouble(),
"--experimental_platforms_api=" + rand.nextBoolean(),
"--experimental_remap_main_repo=" + rand.nextBoolean(),
"--experimental_starlark_config_transitions=" + rand.nextBoolean(),
"--experimental_transition_whitelist_location=" + rand.nextDouble(),
"--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(),
Expand All @@ -153,6 +152,7 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
"--incompatible_no_support_tools_in_action_inputs=" + rand.nextBoolean(),
"--incompatible_no_target_output_group=" + rand.nextBoolean(),
"--incompatible_no_transitive_loads=" + rand.nextBoolean(),
"--incompatible_remap_main_repo=" + rand.nextBoolean(),
"--incompatible_remove_native_maven_jar=" + rand.nextBoolean(),
"--incompatible_require_feature_configuration_for_pic=" + rand.nextBoolean(),
"--incompatible_strict_argument_ordering=" + rand.nextBoolean(),
Expand All @@ -175,7 +175,6 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
.experimentalJavaCommonCreateProviderEnabledPackages(
ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble())))
.experimentalPlatformsApi(rand.nextBoolean())
.experimentalRemapMainRepo(rand.nextBoolean())
.experimentalStarlarkConfigTransitions(rand.nextBoolean())
.experimentalTransitionWhitelistLocation(String.valueOf(rand.nextDouble()))
.incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean())
Expand All @@ -198,6 +197,7 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
.incompatibleNoSupportToolsInActionInputs(rand.nextBoolean())
.incompatibleNoTargetOutputGroup(rand.nextBoolean())
.incompatibleNoTransitiveLoads(rand.nextBoolean())
.incompatibleRemapMainRepo(rand.nextBoolean())
.incompatibleRemoveNativeMavenJar(rand.nextBoolean())
.incompatibleRequireFeatureConfigurationForPic(rand.nextBoolean())
.incompatibleStricArgumentOrdering(rand.nextBoolean())
Expand Down
Expand Up @@ -204,7 +204,7 @@ public void testMappingsNotAMap() throws Exception {

@Test
public void testImplicitMainRepoRename() throws Exception {
helper.setSkylarkSemantics("--experimental_remap_main_repo");
helper.setSkylarkSemantics("--incompatible_remap_main_repo");
helper.parse("workspace(name = 'foo')");
assertMapping(helper, "@", "@foo", "@");
}
Expand Down
Expand Up @@ -146,7 +146,7 @@ public void testErrorWithMapping() throws Exception {

@Test
public void testDefaultMainRepoNameInMapping() throws Exception {
setSkylarkSemanticsOptions("--experimental_remap_main_repo");
setSkylarkSemanticsOptions("--incompatible_remap_main_repo");
scratch.overwriteFile(
"WORKSPACE",
"local_repository(",
Expand All @@ -168,7 +168,7 @@ public void testDefaultMainRepoNameInMapping() throws Exception {

@Test
public void testExplicitMainRepoNameInMapping() throws Exception {
setSkylarkSemanticsOptions("--experimental_remap_main_repo");
setSkylarkSemanticsOptions("--incompatible_remap_main_repo");
scratch.overwriteFile(
"WORKSPACE",
"workspace(name = 'good')",
Expand Down
6 changes: 3 additions & 3 deletions src/test/shell/bazel/workspace_test.sh
Expand Up @@ -726,7 +726,7 @@ a = 1
EOF

cd mainrepo
bazel query --experimental_remap_main_repo //... &>"$TEST_log" \
bazel query --incompatible_remap_main_repo //... &>"$TEST_log" \
|| fail "Expected query to succeed"
expect_log "def.bzl loaded"
expect_not_log "external"
Expand Down Expand Up @@ -754,7 +754,7 @@ EOF
# the bzl file should be loaded from the main workspace and
# not as an external repository
cd mainrepo
bazel query --experimental_remap_main_repo @a//... &>"$TEST_log" \
bazel query --incompatible_remap_main_repo @a//... &>"$TEST_log" \
|| fail "Expected query to succeed"
expect_log "def.bzl loaded"
expect_not_log "external"
Expand All @@ -772,7 +772,7 @@ EOF
# now that @mainrepo doesn't exist within workspace "a",
# the query should fail
cd mainrepo
bazel query --experimental_remap_main_repo \
bazel query --incompatible_remap_main_repo \
@a//... &>"$TEST_log" \
&& fail "Failure expected" || true
}
Expand Down

0 comments on commit ed87463

Please sign in to comment.