Skip to content

Commit

Permalink
Make the attr new_local_repository.build_file label-typed
Browse files Browse the repository at this point in the history 
The attribute is string-typed to support a legacy use case where a path instead of a label could be passed. This causes us to parse a passed label as a canonical label, forgoing repo mapping altogether.

Fixes #19963.

RELNOTES[INC]: The attribute `new_local_repository.build_file` no longer accepts a path; a label must be passed instead.
  • Loading branch information
@Wyverald
Wyverald committed Oct 30, 2023
1 parent 0d4739e commit 332c5e8
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 73 deletions.
5 changes: 3 additions & 2 deletions src/main/java/com/google/devtools/build/lib/rules/repository/NewLocalRepositoryRule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;

Expand Down Expand Up @@ -46,7 +47,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
named BUILD, but can be. (Something like BUILD.new-repo-name may work well for
distinguishing it from the repository's actual BUILD files.)</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("build_file", STRING))
.add(attr("build_file", BuildType.NODEP_LABEL))
/* <!-- #BLAZE_RULE(new_local_repository).ATTRIBUTE(build_file_content) -->
The content for the BUILD file for this repository.
Expand All @@ -62,7 +63,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi
named WORKSPACE, but can be. (Something like WORKSPACE.new-repo-name may work well for
distinguishing it from the repository's actual WORKSPACE files.)</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr("workspace_file", STRING))
.add(attr("workspace_file", BuildType.NODEP_LABEL))
/* <!-- #BLAZE_RULE(new_local_repository).ATTRIBUTE(workspace_file_content) -->
The content for the WORKSPACE file for this repository.
Expand Down
90 changes: 19 additions & 71 deletions src/main/java/com/google/devtools/build/lib/rules/repository/NewRepositoryFileHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@

import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.LabelValidator;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction.RepositoryFunctionException;
import com.google.devtools.build.lib.skyframe.PackageLookupValue;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
Expand Down Expand Up @@ -147,14 +145,7 @@ public void finishFile(Rule rule, Path outputDirectory, Map<String, String> mark
if (fileValue != null) {
// Link x/FILENAME to <build_root>/x.FILENAME.
symlinkFile(fileValue, filename, outputDirectory);
String fileAttribute = getFileAttributeValue(rule);
String fileKey;
if (LabelValidator.isAbsolute(fileAttribute)) {
fileKey = getFileAttributeAsLabel(rule).toString();
} else {
// TODO(pcloudy): Don't add absolute path into markerData once it's not supported
fileKey = fileValue.realRootedPath().asPath().getPathString();
}
String fileKey = getFileAttributeAsLabel(rule).toString();
try {
markerData.put("FILE:" + fileKey, RepositoryFunction.fileValueToMarkerValue(fileValue));
} catch (IOException e) {
Expand All @@ -167,75 +158,32 @@ public void finishFile(Rule rule, Path outputDirectory, Map<String, String> mark
}
}

private String getFileAttributeValue(Rule rule) throws RepositoryFunctionException {
WorkspaceAttributeMapper mapper = WorkspaceAttributeMapper.of(rule);
String fileAttribute;
private Label getFileAttributeAsLabel(Rule rule) throws RepositoryFunctionException {
try {
fileAttribute = mapper.get(getFileAttrName(), Type.STRING);
return WorkspaceAttributeMapper.of(rule).get(getFileAttrName(), BuildType.NODEP_LABEL);
} catch (EvalException e) {
throw new RepositoryFunctionException(e, Transience.PERSISTENT);
}
return fileAttribute;
}

private Label getFileAttributeAsLabel(Rule rule) throws RepositoryFunctionException {
Label label;
try {
// Parse a label
label = Label.parseCanonical(getFileAttributeValue(rule));
} catch (LabelSyntaxException ex) {
throw new RepositoryFunctionException(
Starlark.errorf(
"the '%s' attribute does not specify a valid label: %s",
getFileAttrName(), ex.getMessage()),
Transience.PERSISTENT);
}
return label;
}

@Nullable
private FileValue getFileValue(Rule rule, Environment env)
throws RepositoryFunctionException, InterruptedException {
String fileAttribute = getFileAttributeValue(rule);
RootedPath rootedFile;

if (LabelValidator.isAbsolute(fileAttribute)) {
Label label = getFileAttributeAsLabel(rule);
SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
if (pkgLookupValue == null) {
return null;
}
if (!pkgLookupValue.packageExists()) {
throw new RepositoryFunctionException(
Starlark.errorf("Unable to load package for %s: not found.", fileAttribute),
Transience.PERSISTENT);
}

// And now for the file
Root packageRoot = pkgLookupValue.getRoot();
rootedFile = RootedPath.toRootedPath(packageRoot, label.toPathFragment());
} else {
// TODO(dmarting): deprecate using a path for the workspace_file attribute.
PathFragment file = PathFragment.create(fileAttribute);
Path fileTarget = workspacePath.getRelative(file);
if (!fileTarget.exists()) {
throw new RepositoryFunctionException(
Starlark.errorf(
"the '%s' attribute does not specify an existing file (%s does not exist)",
getFileAttrName(), fileTarget),
Transience.PERSISTENT);
}

if (file.isAbsolute()) {
rootedFile =
RootedPath.toRootedPath(
Root.fromPath(fileTarget.getParentDirectory()),
PathFragment.create(fileTarget.getBaseName()));
} else {
rootedFile = RootedPath.toRootedPath(Root.fromPath(workspacePath), file);
}
Label label = getFileAttributeAsLabel(rule);
SkyKey pkgSkyKey = PackageLookupValue.key(label.getPackageIdentifier());
PackageLookupValue pkgLookupValue = (PackageLookupValue) env.getValue(pkgSkyKey);
if (pkgLookupValue == null) {
return null;
}
if (!pkgLookupValue.packageExists()) {
throw new RepositoryFunctionException(
Starlark.errorf("Unable to load package for %s: not found.", label),
Transience.PERSISTENT);
}

// And now for the file
Root packageRoot = pkgLookupValue.getRoot();
RootedPath rootedFile = RootedPath.toRootedPath(packageRoot, label.toPathFragment());
SkyKey fileKey = FileValue.key(rootedFile);
FileValue fileValue;
try {
Expand All @@ -251,7 +199,7 @@ private FileValue getFileValue(Rule rule, Environment env)
}
} catch (IOException e) {
throw new RepositoryFunctionException(
new IOException("Cannot lookup " + fileAttribute + ": " + e.getMessage()),
new IOException("Cannot lookup " + label + ": " + e.getMessage()),
Transience.TRANSIENT);
}

Expand Down

0 comments on commit 332c5e8

Please sign in to comment.