Skip to content

Commit

Permalink
New incompatible flag to disable PACKAGE_NAME and REPOSITORY_NAME.
Browse files Browse the repository at this point in the history
RELNOTES[INC]:
  Variables PACKAGE_NAME and REPOSITORY_NAME are deprecated in favor of
  functions `package_name()` and `repository_name()`.
  https://docs.bazel.build/versions/master/skylark/lib/native.html#package_name

PiperOrigin-RevId: 190657188
  • Loading branch information
laurentlb authored and Copybara-Service committed Mar 27, 2018
1 parent 44eb964 commit 1cbce0f
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1331,7 +1331,13 @@ public Package createPackageForTesting(
throws NoSuchPackageException, InterruptedException {
Package externalPkg = newExternalPackageBuilder(
buildFile.getRelative(Label.WORKSPACE_FILE_NAME), "TESTING").build();
return createPackageForTesting(packageId, externalPkg, buildFile, locator, eventHandler);
return createPackageForTesting(
packageId,
externalPkg,
buildFile,
locator,
eventHandler,
SkylarkSemantics.DEFAULT_SEMANTICS);
}

/**
Expand All @@ -1344,7 +1350,8 @@ public Package createPackageForTesting(
Package externalPkg,
Path buildFile,
CachingPackageLocator locator,
ExtendedEventHandler eventHandler)
ExtendedEventHandler eventHandler,
SkylarkSemantics semantics)
throws NoSuchPackageException, InterruptedException {
String error =
LabelValidator.validatePackageName(packageId.getPackageFragment().getPathString());
Expand Down Expand Up @@ -1372,7 +1379,7 @@ public Package createPackageForTesting(
/*imports=*/ ImmutableMap.<String, Extension>of(),
/*skylarkFileDependencies=*/ ImmutableList.<Label>of(),
/*defaultVisibility=*/ ConstantRuleVisibility.PUBLIC,
SkylarkSemantics.DEFAULT_SEMANTICS,
semantics,
globber)
.build();
for (Postable post : result.getPosts()) {
Expand Down Expand Up @@ -1544,8 +1551,10 @@ private void buildPkgEnv(
}

pkgEnv.setupDynamic(PKG_CONTEXT, context);
pkgEnv.setupDynamic(Runtime.PKG_NAME, packageId.getPackageFragment().getPathString());
pkgEnv.setupDynamic(Runtime.REPOSITORY_NAME, packageId.getRepository().toString());
if (!pkgEnv.getSemantics().incompatiblePackageNameIsAFunction()) {
pkgEnv.setupDynamic(Runtime.PKG_NAME, packageId.getPackageFragment().getPathString());
pkgEnv.setupDynamic(Runtime.REPOSITORY_NAME, packageId.getRepository().toString());
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public void serialize(
codedOut.writeBoolNoTag(semantics.incompatibleDisallowThreeArgVardef());
codedOut.writeBoolNoTag(semantics.incompatibleDisallowToplevelIfStatement());
codedOut.writeBoolNoTag(semantics.incompatibleNewActionsApi());
codedOut.writeBoolNoTag(semantics.incompatiblePackageNameIsAFunction());
codedOut.writeBoolNoTag(semantics.incompatibleRemoveNativeGitRepository());
codedOut.writeBoolNoTag(semantics.incompatibleRemoveNativeHttpArchive());
codedOut.writeBoolNoTag(semantics.incompatibleStringIsNotIterable());
Expand All @@ -73,6 +74,7 @@ public SkylarkSemantics deserialize(DeserializationContext context, CodedInputSt
builder.incompatibleDisallowThreeArgVardef(codedIn.readBool());
builder.incompatibleDisallowToplevelIfStatement(codedIn.readBool());
builder.incompatibleNewActionsApi(codedIn.readBool());
builder.incompatiblePackageNameIsAFunction(codedIn.readBool());
builder.incompatibleRemoveNativeGitRepository(codedIn.readBool());
builder.incompatibleRemoveNativeHttpArchive(codedIn.readBool());
builder.incompatibleStringIsNotIterable(codedIn.readBool());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,19 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
)
public boolean incompatibleNewActionsApi;

@Option(
name = "incompatible_package_name_is_a_function",
defaultValue = "false",
category = "incompatible changes",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
help =
"If set to true, the values PACKAGE_NAME and REPOSITORY_NAME are not available. "
+ "Use the package_name() or repository_name() functions instead."
)
public boolean incompatiblePackageNameIsAFunction;

@Option(
name = "incompatible_remove_native_git_repository",
defaultValue = "false",
Expand Down Expand Up @@ -229,6 +242,7 @@ public SkylarkSemantics toSkylarkSemantics() {
.incompatibleDisallowThreeArgVardef(incompatibleDisallowThreeArgVardef)
.incompatibleDisallowToplevelIfStatement(incompatibleDisallowToplevelIfStatement)
.incompatibleNewActionsApi(incompatibleNewActionsApi)
.incompatiblePackageNameIsAFunction(incompatiblePackageNameIsAFunction)
.incompatibleRemoveNativeGitRepository(incompatibleRemoveNativeGitRepository)
.incompatibleRemoveNativeHttpArchive(incompatibleRemoveNativeHttpArchive)
.incompatibleStringIsNotIterable(incompatibleStringIsNotIterable)
Expand Down
20 changes: 20 additions & 0 deletions src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,26 @@ EvalException createInvalidIdentifierException(Set<String> symbols) {
if (name.equals("$error$")) {
return new EvalException(getLocation(), "contains syntax error(s)", true);
}

if (name.equals("PACKAGE_NAME")) {
return new EvalException(
getLocation(),
"The value 'PACKAGE_NAME' has been removed in favor of 'package_name()', "
+ "please use the latter ("
+ "https://docs.bazel.build/versions/master/skylark/lib/native.html#package_name). "
+ "You can temporarily allow the old name "
+ "by using --incompatiblePackageNameIsAFunction=false");
}
if (name.equals("REPOSITORY_NAME")) {
return new EvalException(
getLocation(),
"The value 'REPOSITORY_NAME' has been removed in favor of 'repository_name()', "
+ "please use the latter ("
+ "https://docs.bazel.build/versions/master/skylark/lib/native.html#repository_name)."
+ " You can temporarily allow the old name "
+ "by using --incompatiblePackageNameIsAFunction=false");
}

String suggestion = SpellChecker.didYouMean(name, symbols);
return new EvalException(getLocation(), "name '" + name + "' is not defined" + suggestion);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,31 @@ public abstract class SkylarkSemantics {

// <== Add new options here in alphabetic order ==>
public abstract boolean incompatibleBzlDisallowLoadAfterStatement();

public abstract boolean incompatibleDepsetIsNotIterable();

public abstract boolean incompatibleDepsetUnion();

public abstract boolean incompatibleDisableGlobTracking();

public abstract boolean incompatibleDisableObjcProviderResources();

public abstract boolean incompatibleDisallowDictPlus();

public abstract boolean incompatibleDisallowThreeArgVardef();

public abstract boolean incompatibleDisallowToplevelIfStatement();

public abstract boolean incompatibleNewActionsApi();

public abstract boolean incompatiblePackageNameIsAFunction();

public abstract boolean incompatibleRemoveNativeGitRepository();

public abstract boolean incompatibleRemoveNativeHttpArchive();

public abstract boolean incompatibleStringIsNotIterable();

public abstract boolean internalSkylarkFlagTestCanary();

/** Returns a {@link Builder} initialized with the values of this instance. */
Expand All @@ -77,6 +91,7 @@ public static Builder builderWithDefaults() {
.incompatibleDisallowThreeArgVardef(false)
.incompatibleDisallowToplevelIfStatement(true)
.incompatibleNewActionsApi(false)
.incompatiblePackageNameIsAFunction(false)
.incompatibleRemoveNativeGitRepository(false)
.incompatibleRemoveNativeHttpArchive(false)
.incompatibleStringIsNotIterable(false)
Expand All @@ -89,17 +104,31 @@ public abstract static class Builder {

// <== Add new options here in alphabetic order ==>
public abstract Builder incompatibleBzlDisallowLoadAfterStatement(boolean value);

public abstract Builder incompatibleDepsetIsNotIterable(boolean value);

public abstract Builder incompatibleDepsetUnion(boolean value);

public abstract Builder incompatibleDisableGlobTracking(boolean value);

public abstract Builder incompatibleDisableObjcProviderResources(boolean value);

public abstract Builder incompatibleDisallowDictPlus(boolean value);

public abstract Builder incompatibleDisallowThreeArgVardef(boolean value);

public abstract Builder incompatibleDisallowToplevelIfStatement(boolean value);

public abstract Builder incompatibleNewActionsApi(boolean value);

public abstract Builder incompatiblePackageNameIsAFunction(boolean value);

public abstract Builder incompatibleRemoveNativeGitRepository(boolean value);

public abstract Builder incompatibleRemoveNativeHttpArchive(boolean value);

public abstract Builder incompatibleStringIsNotIterable(boolean value);

public abstract Builder internalSkylarkFlagTestCanary(boolean value);

public abstract SkylarkSemantics build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,14 @@ public void testPackageConstant() throws Exception {
assertThat(Sets.newHashSet(pkg.getTargets(Rule.class)).size()).isSameAs(1);
}

@Test
public void testPackageConstantIsForbidden() throws Exception {
events.setFailFast(false);
Path buildFile = scratch.file("/pina/BUILD", "cc_library(name=PACKAGE_NAME + '-colada')");
packages.createPackage("pina", buildFile, "--incompatible_package_name_is_a_function=true");
events.assertContainsError("The value 'PACKAGE_NAME' has been removed");
}

@Test
public void testPackageNameFunction() throws Exception {
Path buildFile = scratch.file("/pina/BUILD", "cc_library(name=package_name() + '-colada')");
Expand All @@ -271,6 +279,20 @@ public void testPackageConstantInExternalRepository() throws Exception {
assertThat(AggregatingAttributeMapper.of(c).get("cmd", Type.STRING)).isEqualTo("@a b");
}

@Test
public void testPackageConstantInExternalRepositoryIsForbidden() throws Exception {
events.setFailFast(false);
Path buildFile =
scratch.file(
"/external/a/b/BUILD", "genrule(name='c', srcs=[], outs=['ao'], cmd=REPOSITORY_NAME)");
packages.createPackage(
PackageIdentifier.create("@a", PathFragment.create("b")),
buildFile,
events.reporter(),
"--incompatible_package_name_is_a_function=true");
events.assertContainsError("The value 'REPOSITORY_NAME' has been removed");
}

@Test
public void testPackageFunctionInExternalRepository() throws Exception {
Path buildFile =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ private static SkylarkSemanticsOptions buildRandomOptions(Random rand) throws Ex
"--incompatible_disallow_three_arg_vardef=" + rand.nextBoolean(),
"--incompatible_disallow_toplevel_if_statement=" + rand.nextBoolean(),
"--incompatible_new_actions_api=" + rand.nextBoolean(),
"--incompatible_package_name_is_a_function=" + rand.nextBoolean(),
"--incompatible_remove_native_git_repository=" + rand.nextBoolean(),
"--incompatible_remove_native_http_archive=" + rand.nextBoolean(),
"--incompatible_string_is_not_iterable=" + rand.nextBoolean(),
Expand All @@ -150,6 +151,7 @@ private static SkylarkSemantics buildRandomSemantics(Random rand) {
.incompatibleDisallowThreeArgVardef(rand.nextBoolean())
.incompatibleDisallowToplevelIfStatement(rand.nextBoolean())
.incompatibleNewActionsApi(rand.nextBoolean())
.incompatiblePackageNameIsAFunction(rand.nextBoolean())
.incompatibleRemoveNativeGitRepository(rand.nextBoolean())
.incompatibleRemoveNativeHttpArchive(rand.nextBoolean())
.incompatibleStringIsNotIterable(rand.nextBoolean())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.PackageFactory.LegacyGlobber;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions;
import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.lib.syntax.Environment.Extension;
import com.google.devtools.build.lib.syntax.ParserInputSource;
Expand All @@ -39,6 +40,7 @@
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.common.options.OptionsParser;
import java.io.IOException;

/**
Expand Down Expand Up @@ -76,34 +78,57 @@ private CachingPackageLocator getPackageLocator() {
return createEmptyLocator();
}

/**
* Parses and evaluates {@code buildFile} and returns the resulting {@link Package} instance.
*/
/** Parses and evaluates {@code buildFile} and returns the resulting {@link Package} instance. */
public Package createPackage(String packageName, Path buildFile) throws Exception {
return createPackage(PackageIdentifier.createInMainRepo(packageName), buildFile,
eventHandler);
return createPackage(PackageIdentifier.createInMainRepo(packageName), buildFile, eventHandler);
}

public Package createPackage(String packageName, Path buildFile, String skylarkOption)
throws Exception {
return createPackage(
PackageIdentifier.createInMainRepo(packageName), buildFile, eventHandler, skylarkOption);
}

/**
* Parses and evaluates {@code buildFile} with custom {@code eventHandler} and returns the
* resulting {@link Package} instance.
*/
public Package createPackage(
PackageIdentifier packageIdentifier, Path buildFile, ExtendedEventHandler reporter)
PackageIdentifier packageIdentifier,
Path buildFile,
ExtendedEventHandler reporter,
String skylarkOption)
throws Exception {

OptionsParser parser = OptionsParser.newOptionsParser(SkylarkSemanticsOptions.class);
parser.parse(
skylarkOption == null
? ImmutableList.<String>of()
: ImmutableList.<String>of(skylarkOption));
SkylarkSemantics semantics =
parser.getOptions(SkylarkSemanticsOptions.class).toSkylarkSemantics();

try {
Package externalPkg =
factory
.newExternalPackageBuilder(
buildFile.getRelative(Label.WORKSPACE_FILE_NAME), "TESTING")
.build();
Package pkg =
factory.createPackageForTesting(
packageIdentifier,
buildFile,
getPackageLocator(),
reporter);
packageIdentifier, externalPkg, buildFile, getPackageLocator(), reporter, semantics);
return pkg;
} catch (InterruptedException e) {
throw new IllegalStateException(e);
}
}

public Package createPackage(
PackageIdentifier packageIdentifier, Path buildFile, ExtendedEventHandler reporter)
throws Exception {
return createPackage(packageIdentifier, buildFile, reporter, null);
}

/**
* Parses the {@code buildFile} into a {@link BuildFileAST}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import com.google.devtools.build.lib.packages.NoSuchTargetException;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.skyframe.TransitiveBaseTraversalFunction.TargetAndErrorIfAnyImpl;
import com.google.devtools.build.lib.syntax.SkylarkSemantics;
import com.google.devtools.build.lib.util.GroupedList;
import com.google.devtools.build.lib.util.GroupedList.GroupedListHelper;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -104,6 +105,11 @@ private Package scratchPackage(String workspaceName, PackageIdentifier packageId
Package.Builder.DefaultHelper.INSTANCE, buildFile.getRelative("WORKSPACE"), "TESTING");
externalPkg.setWorkspaceName(workspaceName);
return pkgFactory.createPackageForTesting(
packageId, externalPkg.build(), buildFile, packageIdentifier -> buildFile, reporter);
packageId,
externalPkg.build(),
buildFile,
packageIdentifier -> buildFile,
reporter,
SkylarkSemantics.DEFAULT_SEMANTICS);
}
}

0 comments on commit 1cbce0f

Please sign in to comment.