Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Stardoc repository mapping aware #16775

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 2 additions & 9 deletions src/main/java/com/google/devtools/build/skydoc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,6 @@ filegroup(

java_binary(
name = "skydoc",
jvm_flags = [
# quiet warnings from com.google.protobuf.UnsafeUtil,
# see: https://github.com/google/protobuf/issues/3781
# and: https://github.com/bazelbuild/bazel/issues/5599
"--add-opens=java.base/java.nio=ALL-UNNAMED",
"--add-opens=java.base/java.lang=ALL-UNNAMED",
# ... but only on JDK >= 9
"-XX:+IgnoreUnrecognizedVMOptions",
],
main_class = "com.google.devtools.build.skydoc.SkydocMain",
visibility = ["//visibility:public"],
runtime_deps = [
Expand All @@ -59,5 +50,7 @@ java_library(
"//src/main/java/net/starlark/java/lib/json",
"//src/main/java/net/starlark/java/syntax",
"//third_party:guava",
"//tools/java/runfiles",
"//tools/java/runfiles:runfiles_for_stardoc",
],
)

This file was deleted.

83 changes: 34 additions & 49 deletions src/main/java/com/google/devtools/build/skydoc/SkydocMain.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,21 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Functions;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.Label.PackageContext;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.Depset;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.runfiles.Runfiles;
import com.google.devtools.build.runfiles.RunfilesForStardoc;
import com.google.devtools.build.skydoc.fakebuildapi.FakeApi;
import com.google.devtools.build.skydoc.fakebuildapi.FakeDeepStructure;
import com.google.devtools.build.skydoc.fakebuildapi.FakeProviderApi;
Expand All @@ -48,6 +50,7 @@
import java.io.BufferedOutputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.NoSuchFileException;
import java.nio.file.Path;
import java.nio.file.Paths;
Expand Down Expand Up @@ -105,21 +108,13 @@ public class SkydocMain {
private final EventHandler eventHandler = new SystemOutEventHandler();
private final LinkedHashSet<Path> pending = new LinkedHashSet<>();
private final Map<Path, Module> loaded = new HashMap<>();
private final StarlarkFileAccessor fileAccessor;
private final List<String> depRoots;
private final String workspaceName;
private final Runfiles.Preloaded runfiles;

public SkydocMain(
StarlarkFileAccessor fileAccessor, String workspaceName, List<String> depRoots) {
this.fileAccessor = fileAccessor;
String workspaceName, Runfiles.Preloaded runfiles) {
this.workspaceName = workspaceName;
if (depRoots.isEmpty()) {
// For backwards compatibility, if no dep_roots are specified, use the current
// directory as the only root.
this.depRoots = ImmutableList.of(".");
} else {
this.depRoots = depRoots;
}
this.runfiles = runfiles;
}

public static void main(String[] args)
Expand All @@ -135,7 +130,6 @@ public static void main(String[] args)

String targetFileLabelString;
String outputPath;
ImmutableList<String> depRoots;

if (Strings.isNullOrEmpty(skydocOptions.targetFileLabel)
|| Strings.isNullOrEmpty(skydocOptions.outputFilePath)) {
Expand All @@ -144,7 +138,6 @@ public static void main(String[] args)

targetFileLabelString = skydocOptions.targetFileLabel;
outputPath = skydocOptions.outputFilePath;
depRoots = ImmutableList.copyOf(skydocOptions.depRoots);

Label targetFileLabel = Label.parseCanonical(targetFileLabelString);

Expand All @@ -156,7 +149,7 @@ public static void main(String[] args)
Module module = null;
try {
module =
new SkydocMain(new FilesystemFileAccessor(), skydocOptions.workspaceName, depRoots)
new SkydocMain(skydocOptions.workspaceName, Runfiles.preload())
.eval(
semanticsOptions.toStarlarkSemantics(),
targetFileLabel,
Expand Down Expand Up @@ -293,7 +286,11 @@ public Module eval(

List<AspectInfoWrapper> aspectInfoList = new ArrayList<>();

Module module = recursiveEval(semantics, label, ruleInfoList, providerInfoList, aspectInfoList);
// Resolve the label provided on the command line with the main repository's repository mapping.
// The stardoc rules always pass in a canonical label, so in this case the repository mapping
// is not used.
Module module = recursiveEval(semantics, label, RepositoryName.MAIN.getName(), ruleInfoList,
providerInfoList, aspectInfoList);

Map<StarlarkCallable, RuleInfoWrapper> ruleFunctions =
ruleInfoList.stream()
Expand Down Expand Up @@ -386,23 +383,24 @@ private static void putStructFields(
* dependencies using a fake build API and collects information about all rule definitions made in
* those files.
*
* @param label the label of the Starlark file to evaluate
* @param ruleInfoList a collection of all rule definitions made so far (using rule()); this
* method will add to this list as it evaluates additional files
* @param label the label of the Starlark file to evaluate
* @param parentSourceRepository the canonical name of the Bazel repository that loads label
* @param ruleInfoList a collection of all rule definitions made so far (using rule());
* this method will add to this list as it evaluates additional
* files
* @throws InterruptedException if evaluation is interrupted
*/
private Module recursiveEval(
StarlarkSemantics semantics,
Label label,
String parentSourceRepository,
List<RuleInfoWrapper> ruleInfoList,
List<ProviderInfoWrapper> providerInfoList,
List<AspectInfoWrapper> aspectInfoList)
throws InterruptedException,
IOException,
LabelSyntaxException,
StarlarkEvaluationException,
EvalException {
Path path = pathOfLabel(label, semantics);
throws InterruptedException, IOException, LabelSyntaxException, StarlarkEvaluationException {
Path path = pathOfLabel(label, parentSourceRepository);
String sourceRepository = RunfilesForStardoc.getCanonicalRepositoryName(
runfiles.withSourceRepository(parentSourceRepository), label.getRepository().getName());

if (pending.contains(path)) {
throw new StarlarkEvaluationException("cycle with " + path);
Expand Down Expand Up @@ -436,7 +434,7 @@ private Module recursiveEval(
};

// parse & compile (and get doc)
ParserInput input = getInputSource(path.toString());
ParserInput input = ParserInput.fromLatin1(Files.readAllBytes(path), path.toString());
Program prog;
try {
StarlarkFile file = StarlarkFile.parse(input, FileOptions.DEFAULT);
Expand All @@ -449,19 +447,20 @@ private Module recursiveEval(
// process loads
Map<String, Module> imports = new HashMap<>();
for (String load : prog.getLoads()) {
Label relativeLabel =
Label apparentLabel =
Label.parseWithPackageContext(
load,
PackageContext.of(label.getPackageIdentifier(), RepositoryMapping.ALWAYS_FALLBACK));
try {
Module loadedModule =
recursiveEval(semantics, relativeLabel, ruleInfoList, providerInfoList, aspectInfoList);
recursiveEval(semantics, apparentLabel, sourceRepository, ruleInfoList,
providerInfoList, aspectInfoList);
imports.put(load, loadedModule);
} catch (NoSuchFileException noSuchFileException) {
throw new StarlarkEvaluationException(
String.format(
"File %s imported '%s', yet %s was not found, even at roots %s.",
path, load, pathOfLabel(relativeLabel, semantics), depRoots),
"File %s imported '%s', yet %s was not found.",
path, load, pathOfLabel(apparentLabel, sourceRepository)),
noSuchFileException);
}
}
Expand Down Expand Up @@ -491,25 +490,11 @@ path, load, pathOfLabel(relativeLabel, semantics), depRoots),
return module;
}

private Path pathOfLabel(Label label, StarlarkSemantics semantics) throws EvalException {
String workspacePrefix = "";
if (!label.getWorkspaceRootForStarlarkOnly(semantics).isEmpty()
&& !label.getWorkspaceName().equals(workspaceName)) {
workspacePrefix = label.getWorkspaceRootForStarlarkOnly(semantics) + "/";
}

return Paths.get(workspacePrefix + label.toPathFragment());
}

private ParserInput getInputSource(String bzlWorkspacePath) throws IOException {
for (String rootPath : depRoots) {
if (fileAccessor.fileExists(rootPath + "/" + bzlWorkspacePath)) {
return fileAccessor.inputSource(rootPath + "/" + bzlWorkspacePath);
}
}

// All depRoots attempted and no valid file was found.
throw new NoSuchFileException(bzlWorkspacePath);
private Path pathOfLabel(Label label, String sourceRepository) {
String targetRepositoryApparentName =
label.getRepository().isMain() ? workspaceName : label.getRepository().getName();
String rlocationPath = targetRepositoryApparentName + "/" + label.toPathFragment().toString();
return Paths.get(runfiles.withSourceRepository(sourceRepository).rlocation(rlocationPath));
}

private static void addMorePredeclared(ImmutableMap.Builder<String, Object> env) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,4 @@ public class SkydocOptions extends OptionsBase {
+ " is empty, then documentation for all exported rule definitions will be"
+ " generated.")
public List<String> symbolNames;

@Option(
name = "dep_roots",
allowMultiple = true,
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = OptionEffectTag.UNKNOWN,
help = "File path roots to search when resolving transitive bzl dependencies")
public List<String> depRoots;
}

This file was deleted.

4 changes: 1 addition & 3 deletions src/test/java/com/google/devtools/build/skydoc/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,16 @@ java_test(
deps = [
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skydoc:skydoc_lib",
"//src/main/java/com/google/devtools/build/skydoc/fakebuildapi",
"//src/main/java/com/google/devtools/build/skydoc/rendering",
"//src/main/java/com/google/devtools/build/skydoc/rendering/proto:stardoc_output_java_proto",
"//src/main/java/net/starlark/java/eval",
"//src/main/java/net/starlark/java/syntax",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax also seems unused

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used for Resolver.

"//src/test/java/com/google/devtools/build/lib/analysis/util",
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
"//tools/java/runfiles",
],
)

Expand Down