Skip to content

Commit

Permalink
8294368: Java incremental builds broken on Windows after JDK-8293116
Browse files Browse the repository at this point in the history
Reviewed-by: erikj, djelinski, jlahoda
  • Loading branch information
JornVernee committed Oct 5, 2022
1 parent 4bdd1c9 commit 8ebebbc
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 8 deletions.
11 changes: 9 additions & 2 deletions make/common/JavaCompilation.gmk
Expand Up @@ -403,6 +403,7 @@ define SetupJavaCompilationBody
$1_COMPILE_TARGET := $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1_batch
$1_FILELIST := $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1_batch.filelist
$1_MODFILELIST := $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1_batch.modfiles
$1_MODFILELIST_FIXED := $$($1_MODFILELIST).fixed

$1_API_TARGET := $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1_pubapi
$1_API_INTERNAL := $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$1_internalapi
Expand Down Expand Up @@ -469,15 +470,21 @@ define SetupJavaCompilationBody
$$(eval $1_MODFILES := $$?)
$$(eval $$(call ListPathsSafely, $1_MODFILES, $$($1_MODFILELIST)))

# Convert the paths in the MODFILELIST file to Windows-style paths
# on Windows. This is needed because javac operates on Windows-style paths
# when running on Windows. On other platforms this just copies the MODFILELIST file.
$$($1_MODFILELIST_FIXED): $$($1_MODFILELIST)
$$(call FixPathFile, $$($1_MODFILELIST), $$($1_MODFILELIST_FIXED))

# Do the actual compilation
$$($1_COMPILE_TARGET): $$($1_SRCS) $$($1_FILELIST) $$($1_DEPENDS) \
$$($1_VARDEPS_FILE) $$($1_EXTRA_DEPS) $$($1_JAVAC_SERVER_CONFIG) \
$$($1_MODFILELIST)
$$($1_MODFILELIST_FIXED)
$$(call MakeDir, $$(@D))
$$(call ExecuteWithLog, $$($1_BIN)$$($1_MODULE_SUBDIR)/_the.$$($1_SAFE_NAME)_batch, \
$$($1_JAVAC_CMD) $$($1_FLAGS) \
$$($1_API_DIGEST_FLAGS) \
-XDmodifiedInputs=$$($1_MODFILELIST) \
-XDmodifiedInputs=$$($1_MODFILELIST_FIXED) \
-d $$($1_BIN) $$($1_HEADERS_ARG) @$$($1_FILELIST)) && \
$(TOUCH) $$@

Expand Down
11 changes: 11 additions & 0 deletions make/common/MakeBase.gmk
Expand Up @@ -442,12 +442,23 @@ endif
# list.
# This is normally not needed since we use the FIXPATH prefix for command lines,
# but might be needed in certain circumstances.
#
# FixPathFile is the file version of FixPath. It instead takes a file with paths in $1
# and outputs the 'fixed' paths into the file in $2. If the file in $2 already exists
# it is overwritten.
# On non-Windows platforms this instead does a copy, so that $2 can still be used
# as a depenendency of a make rule, instead of having to conditionally depend on
# $1 instead, based on the target platform.
ifeq ($(call isTargetOs, windows), true)
FixPath = \
$(strip $(subst \,\\, $(shell $(FIXPATH_BASE) print $(patsubst $(FIXPATH), , $1))))
FixPathFile = \
$(shell $(FIXPATH_BASE) convert $1 $2)
else
FixPath = \
$1
FixPathFile = \
$(shell $(CP) $1 $2)
endif

################################################################################
Expand Down
16 changes: 10 additions & 6 deletions make/jdk/src/classes/build/tools/depend/Depend.java
Expand Up @@ -154,7 +154,9 @@ public void init(JavacTask jt, String... args) {
if (internalAPIPath == null) {
throw new IllegalStateException("Expected internalAPIPath to be set using -XDinternalAPIPath=<internal-API-path>");
}
Set<String> modified = new HashSet<>(Files.readAllLines(Paths.get(modifiedInputs)));
Set<Path> modified = Files.readAllLines(Paths.get(modifiedInputs)).stream()
.map(Paths::get)
.collect(Collectors.toSet());
Path internalAPIDigestFile = Paths.get(internalAPIPath);
JavaCompiler compiler = JavaCompiler.instance(context);
Class<?> initialFileParserIntf = Class.forName("com.sun.tools.javac.main.JavaCompiler$InitialFileParserIntf");
Expand Down Expand Up @@ -255,7 +257,7 @@ public ClassLoader getClassLoader(JavaFileManager.Location location) {
}

private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(
JavaCompiler compiler, Iterable<JavaFileObject> fileObjects, Set<String> modified,
JavaCompiler compiler, Iterable<JavaFileObject> fileObjects, Set<Path> modified,
Path internalAPIDigestFile, AtomicBoolean noApiChange,
boolean debug) {
Map<String, String> internalAPI = new LinkedHashMap<>();
Expand All @@ -272,10 +274,11 @@ private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(
}
Map<JavaFileObject, JCCompilationUnit> files2CUT = new IdentityHashMap<>();
boolean fullRecompile = modified.stream()
.map(Path::toString)
.anyMatch(f -> !StringUtils.toLowerCase(f).endsWith(".java"));
ListBuffer<JCCompilationUnit> result = new ListBuffer<>();
for (JavaFileObject jfo : fileObjects) {
if (modified.contains(jfo.getName())) {
if (modified.contains(Path.of(jfo.getName()))) {
JCCompilationUnit parsed = compiler.parse(jfo);
files2CUT.put(jfo, parsed);
String currentSignature = treeDigest(parsed);
Expand All @@ -289,7 +292,7 @@ private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(

if (fullRecompile) {
for (JavaFileObject jfo : fileObjects) {
if (!modified.contains(jfo.getName())) {
if (!modified.contains(Path.of(jfo.getName()))) {
JCCompilationUnit parsed = files2CUT.get(jfo);
if (parsed == null) {
parsed = compiler.parse(jfo);
Expand Down Expand Up @@ -320,6 +323,7 @@ private com.sun.tools.javac.util.List<JCCompilationUnit> doFilteredParse(
.findAny()
.orElseGet(() -> "unknown");
String nonJavaModifiedFiles = modified.stream()
.map(Path::toString)
.filter(f -> !StringUtils.toLowerCase(f)
.endsWith(".java"))
.collect(Collectors.joining(", "));
Expand Down Expand Up @@ -879,13 +883,13 @@ public Void visitModifiers(ModifiersTree node, Void p) {
private class FilteredInitialFileParser implements InvocationHandler {

private final JavaCompiler compiler;
private final Set<String> modified;
private final Set<Path> modified;
private final Path internalAPIDigestFile;
private final AtomicBoolean noApiChange;
private final boolean debug;

public FilteredInitialFileParser(JavaCompiler compiler,
Set<String> modified,
Set<Path> modified,
Path internalAPIDigestFile,
AtomicBoolean noApiChange,
boolean debug) {
Expand Down
17 changes: 17 additions & 0 deletions make/scripts/fixpath.sh
Expand Up @@ -352,6 +352,21 @@ function convert_path() {
fi
}
# Treat $1 as name of a file containing paths. Convert those paths to Windows style,
# and output them to the file specified by $2.
# If the output file already exists, it is overwritten.
function convert_file() {
infile="$1"
outfile="$2"
if [[ -e $outfile ]] ; then
rm $outfile
fi
while read line; do
convert_path "$line"
echo "$result" >> $outfile
done < $infile
}
# Treat $1 as name of a file containing paths. Convert those paths to Windows style,
# in a new temporary file, and return a string "@<temp file>" pointing to that
# new file.
Expand Down Expand Up @@ -498,6 +513,8 @@ if [[ "$ACTION" == "import" ]] ; then
elif [[ "$ACTION" == "print" ]] ; then
print_command_line "$@"
echo "$result"
elif [[ "$ACTION" == "convert" ]] ; then
convert_file "$@"
elif [[ "$ACTION" == "exec" ]] ; then
exec_command_line "$@"
# Propagate exit code
Expand Down

1 comment on commit 8ebebbc

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.