7903840: apidiff: optionally show expansion of --jdk-build

jonathan-gibbons · jonathan-gibbons · commit 04f70c7ad165 · 2024-10-01T22:05:28.000Z
Reviewed-by: iris
diff --git a/src/share/classes/jdk/codetools/apidiff/JDKBuildOption.java b/src/share/classes/jdk/codetools/apidiff/JDKBuildOption.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020,2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -61,22 +61,44 @@ public class JDKBuildOption {
     }
 
     void expand(Options options, Options.APIOptions apiOptions, Log log) {
-        apiOptions.addFileManagerOpt("--system", getSystem().toString());
+
+        boolean verbose = options.isVerbose(Options.VerboseKind.OPTIONS);
+        if (verbose) {
+            log.err.println("Expanding --jdk-build for API " + apiOptions.name);
+            log.err.println("    --jdk-build: " + buildDir);
+        }
+
+        Path system = getSystem();
+        if (verbose) {
+            log.err.println("  --system " + system);
+        }
+        apiOptions.addFileManagerOpt("--system", system.toString());
 
         // proactively get api dir if available, in case we want to subsequently
         // set compareAPIDescriptions by default
-        apiOptions.apiDir = getAPIDirectory(options, log);
+        Path apiDir = getAPIDirectory(options, log);
+        if (verbose && apiDir != null) {
+            log.err.println("  --api-directory " + apiDir);
+        }
+        apiOptions.apiDir = apiDir;
 
         if (options.compareDocComments == Boolean.TRUE) {
             Set<String> modules = new LinkedHashSet<>();
             Path tmpDir = unpackSource(options, log, modules);
             for (String m : modules) {
-                apiOptions.addFileManagerOpt("--patch-module",
-                        m + "=" + tmpDir.resolve(m));
+                String patchModule = m + "=" + tmpDir.resolve(m);
+                if (verbose) {
+                    log.err.println("  --patch-module " + patchModule);
+                }
+                apiOptions.addFileManagerOpt("--patch-module", patchModule);
             }
             // since we're also setting the --system option,
             // just set the --source option here
-            apiOptions.source = getRelease(log);
+            String release = getRelease(log);
+            if (verbose) {
+                log.err.println("  --source " + release);
+            }
+            apiOptions.source = release;
         }
     }
 
diff --git a/src/share/classes/jdk/codetools/apidiff/Main.java b/src/share/classes/jdk/codetools/apidiff/Main.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018,2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -170,10 +170,15 @@ private Result run(List<String> args, Log log) {
         Selector s = new Selector(options.includes, options.excludes);
         AccessKind ak = options.getAccessKind();
 
+        boolean verboseOptions = options.isVerbose(VerboseKind.OPTIONS);
+        if (verboseOptions) {
+            options.allAPIOptions.values().forEach(a -> a.showVerboseSummary(log));
+        }
+
         // TODO: when APIDiff moves to JDK 21, thia can trivially become SequencedSet,
         //       which would be useful in varoius places, such as PageReporter.getResultGlyph
         Set<API> apis = options.allAPIOptions.values().stream()
-                .map(a -> API.of(a, s, ak, log))
+                .map(a -> API.of(a, s, ak, log, verboseOptions))
                 .collect(Collectors.toCollection(LinkedHashSet::new));
 
         List<Reporter> rList = new ArrayList<>();
diff --git a/src/share/classes/jdk/codetools/apidiff/Options.java b/src/share/classes/jdk/codetools/apidiff/Options.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -119,6 +119,33 @@ void addFileManagerOpt(String opt, String arg) {
             fileManagerOpts.computeIfAbsent(opt, _o -> new ArrayList<>()).add(arg);
         }
 
+        public void showVerboseSummary(Log log) {
+            log.err.println("Summary of options for API " + name);
+            if (label != null && !label.equals(name)) {
+                log.err.println("  label: " + label);
+            }
+            if (jdkBuildDir != null) {
+                log.err.println("  [--jdk-build " + jdkBuildDir + "]");
+            }
+            if (!fileManagerOpts.isEmpty()) {
+                fileManagerOpts.forEach((opt, list) -> {
+                    log.err.println("  " + opt + " " + String.join(" ", list));
+                });
+            }
+            if (release != null) {
+                log.err.println("  --release " + release);
+            }
+            if (source != null) {
+                log.err.println("  --source " + source);
+            }
+            if (enablePreview) {
+                log.err.println("  --enable-preview");
+            }
+            if (apiDir != null) {
+                log.err.println("  --api-directory " + apiDir);
+            }
+        }
+
         public String toString() {
             return "APIOptions[name:" + name
                     + ",label:" + label
@@ -202,15 +229,16 @@ public enum VerboseKind {
         /** Generate messages about missing items. */
         MISSING,
         /** Generate messages about the time taken. */
-        TIME
+        TIME,
+        /** Generate messages about handling the command-line options. */
+        OPTIONS
     }
 
     private Set<VerboseKind> verboseKinds = EnumSet.noneOf(VerboseKind.class);
 
     // meta options
     boolean help;
     boolean version;
-    private boolean verbose;
 
     // hidden options
     private Map<String, String> hidden = new HashMap<>();
@@ -437,7 +465,7 @@ void process(String opt, String arg, Options options) throws BadOption {
          */
         JDK_DOCS("--jdk-docs", "opt.arg.jdk-docs") {
             @Override
-            void process(String opt, String arg, Options options) throws BadOption {
+            void process(String opt, String arg, Options options) {
                 options.jdkDocs = arg;
             }
         },
diff --git a/src/share/classes/jdk/codetools/apidiff/model/API.java b/src/share/classes/jdk/codetools/apidiff/model/API.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -107,11 +107,12 @@ public abstract class API {
      * @param ak    the access kind, to filter the set of elements to be compared according
      *              their declared access
      * @param log   a log, to which any problems will be reported
+     * @param verboseOptions whether to be verbose about internal opton details
      *
      * @return the API
      */
-    public static API of(APIOptions opts, Selector s, AccessKind ak, Log log) {
-        return new JavacAPI(opts, s, ak, log);
+    public static API of(APIOptions opts, Selector s, AccessKind ak, Log log, boolean verboseOptions) {
+        return new JavacAPI(opts, s, ak, log, verboseOptions);
     }
 
     /**
@@ -145,20 +146,26 @@ public static API of(APIOptions opts, Selector s, AccessKind ak, Log log) {
      */
     protected final StandardJavaFileManager fileManager;
 
+    /**
+     * Whether to be verbose about internal option details.
+     */
+    protected final boolean verboseOptions;
+
     /**
      * Creates an instance of an API.
      *
      * @param opts  the options for the API
      * @param s     the selector for the elements to be compared
      * @param ak    the access kind for the elements to be compared
-     * @param log   the log, to which any any problems will be reported
+     * @param log   the log, to which any problems will be reported
      */
-    protected API(APIOptions opts, Selector s, AccessKind ak, Log log) {
+    protected API(APIOptions opts, Selector s, AccessKind ak, Log log, boolean verboseOptions) {
         this.name = opts.name;
         this.label = opts.label;
         this.selector = s;
         this.accessKind = ak;
         this.log = log;
+        this.verboseOptions = verboseOptions;
 
         fileManager = compiler.getStandardFileManager(null, null, null);
     }
@@ -339,6 +346,7 @@ public abstract List<JavaFileObject> listFiles(LocationKind kind, Element e, Str
 
     static class JavacAPI extends API {
         private List<String> javacOpts;
+        private List<String> fmOpts; // just for verbose reporting
         private int platformVersion;
         private Elements elements;
         private Types types;
@@ -352,7 +360,7 @@ static class JavacAPI extends API {
         /**
          * A tuple containing a location and the kinds of files that may be read from that location.
          */
-        private class LocationAndKinds {
+        private static class LocationAndKinds {
             final Location locn;
             final Set<JavaFileObject.Kind> kinds;
             LocationAndKinds(Location locn, Set<JavaFileObject.Kind> kinds) {
@@ -381,13 +389,16 @@ protected boolean removeEldestEntry(Map.Entry<Path,APIDocs> eldest) {
             }
         };
 
-        JavacAPI(APIOptions opts, Selector s, AccessKind ak, Log log) {
-            super(opts, s, ak, log);
+        JavacAPI(APIOptions opts, Selector s, AccessKind ak, Log log, boolean verboseOptions) {
+            super(opts, s, ak, log, verboseOptions);
 
+            fmOpts = new ArrayList<>();
             for (Map.Entry<String, List<String>> e : opts.fileManagerOpts.entrySet()) {
                 String opt = e.getKey();
+                fmOpts.add(opt);
                 List<String> args = e.getValue();
                 for (String arg : args) {
+                    fmOpts.add(arg);
                     Iterator<String> argIter = arg == null
                             ? Collections.emptyIterator()
                             : Collections.singleton(arg).iterator();
@@ -441,6 +452,9 @@ void initJavac(Set<String> selectedModules) {
                 javacOpts.add(String.join(",", selectedModules));
             }
             javacOpts.add("-proc:only");
+            if (verboseOptions) {
+                showJavacOptions();
+            }
             JavacTask javacTask = (JavacTask) compiler.getTask(log.err, fileManager, this::reportDiagnostic, javacOpts, null, null);
             elements = javacTask.getElements();
             elements.getModuleElement("java.base"); // forces module graph to be instantiated, etc
@@ -465,6 +479,32 @@ public SerializedFormDocs getSerializedFormDocs(TypeElement te) {
             };
         }
 
+        private void showJavacOptions() {
+            log.err.println("Effective javac options for API " + name);
+            boolean needNewline = false;
+            // The following is a convenient fiction: to report all the javac options as "equivalent".
+            // In reality, the file manager options have already been handled separately and are
+            // now stashed in the file manager, without easy access (except via Locations).
+            List<String> allOpts = new ArrayList<>();
+            allOpts.addAll(fmOpts);
+            allOpts.addAll(javacOpts);
+            for (String opt : allOpts) {
+                if (opt.startsWith("-")) {
+                    if (needNewline) {
+                        log.err.println();
+                    }
+                    log.err.print("  ");
+                } else {
+                    log.err.print(" ");
+                }
+                log.err.print(opt);
+                needNewline = true;
+            }
+            if (needNewline) {
+                log.err.println();
+            }
+        }
+
         @Override
         public Set<PackageElement> getPackageElements() {
             if (packages == null) {
@@ -566,7 +606,7 @@ public Set<PackageElement> getExportedPackageElements(ModuleElement m) {
                     .filter(d -> d.getKind() == ModuleElement.DirectiveKind.EXPORTS)
                     .map(d -> (ModuleElement.ExportsDirective) d)
                     .filter(d -> d.getTargetModules() == null)
-                    .map(d -> d.getPackage())
+                    .map(ModuleElement.ExportsDirective::getPackage)
                     .collect(Collectors.toSet());
 
             return getPackageElements(m).stream()
@@ -1070,7 +1110,7 @@ public Void visitPrimitive(PrimitiveType t, StringBuilder sb) {
 
             @Override
             public Void visitNull(NullType t, StringBuilder sb) {
-                throw new IllegalArgumentException(t.getKind() + " " + t.toString());
+                throw new IllegalArgumentException(t.getKind() + " " + t);
             }
 
             @Override
diff --git a/src/share/doc/apidiff.md b/src/share/doc/apidiff.md
@@ -233,11 +233,11 @@ pairwise, with each of the older instances being compared against the newest ins
 <a id="option-verbose">`--verbose` _flag_[`,`_flag_]*</a>
 :   Specifies the kinds of verbose output. _flag_ may be one of
     `all`, `none`, or one of the following, optionally preceded by `-`:
-    `module`, `package`, `type`, `time`.
+    `module`, `package`, `type`, `time`, `options`.
 
 <a id="option-at">`@`*filename*</a>
 :   Reads options from a file. To shorten or simplify the `apidiff` command, you can specify
-    one or more files that contain arguments for the `apidiff` command. This lets you to create
+    one or more files that contain arguments for the `apidiff` command. This lets you create
     `apidiff` commands of any length on any operating system.
 
 
@@ -420,6 +420,9 @@ you can combine the recommended options for the two modes, specifying both
 `--patch-module` options for the source files and `--api-directory` for the
 generated documentation.
 
+If you want to see or understand how the options are used internally, you
+can use `--verbose options`, possibly implicitly as part of `--verbose all`.
+
 ### Comparing different releases of JDK
 
 When comparing any generated API documentation, the comparison is sensitive to any
diff --git a/test/junit/apitest/TestAPI.java b/test/junit/apitest/TestAPI.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -49,7 +49,7 @@
  */
 public class TestAPI extends API {
     TestAPI(String name) {
-        super(new Options.APIOptions(name), null, null, null);
+        super(new Options.APIOptions(name), null, null, null, false);
     }
 
     @Override

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+ * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`*`
`5`	`5`	`* This code is free software; you can redistribute it and/or modify it`
`@@ -49,7 +49,7 @@`
`49`	`49`	`*/`
`50`	`50`	`public class TestAPI extends API {`
`51`	`51`	`TestAPI(String name) {`
`52`		`- super(new Options.APIOptions(name), null, null, null);`
	`52`	`+ super(new Options.APIOptions(name), null, null, null, false);`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`@Override`