Skip to content

Commit 1e14e0d

Browse files
Fix bug in PathGroup that resulted in copying "libapplauncher.so" in wrong location
1 parent f6e9f44 commit 1e14e0d

File tree

2 files changed

+414
-34
lines changed
  • src/jdk.jpackage/share/classes/jdk/jpackage/internal/util
  • test/jdk/tools/jpackage/junit/share/jdk.jpackage/jdk/jpackage/internal/util

2 files changed

+414
-34
lines changed

src/jdk.jpackage/share/classes/jdk/jpackage/internal/util/PathGroup.java

Lines changed: 84 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@
2424
*/
2525
package jdk.jpackage.internal.util;
2626

27+
import static java.util.stream.Collectors.collectingAndThen;
28+
import static java.util.stream.Collectors.groupingBy;
2729
import static java.util.stream.Collectors.toMap;
30+
import static java.util.stream.Collectors.toSet;
2831

2932
import java.io.IOException;
3033
import java.io.UncheckedIOException;
@@ -34,6 +37,7 @@
3437
import java.nio.file.LinkOption;
3538
import java.nio.file.Path;
3639
import java.util.ArrayList;
40+
import java.util.Collection;
3741
import java.util.Comparator;
3842
import java.util.HashMap;
3943
import java.util.List;
@@ -53,6 +57,8 @@ public final class PathGroup {
5357
* @param paths the initial paths
5458
*/
5559
public PathGroup(Map<Object, Path> paths) {
60+
paths.keySet().forEach(Objects::requireNonNull);
61+
paths.values().forEach(Objects::requireNonNull);
5662
entries = new HashMap<>(paths);
5763
}
5864

@@ -179,14 +185,32 @@ public List<Path> roots() {
179185
*/
180186
public long sizeInBytes() throws IOException {
181187
long reply = 0;
182-
for (Path dir : roots().stream().filter(Files::isDirectory).toList()) {
183-
try (Stream<Path> stream = Files.walk(dir)) {
184-
reply += stream.filter(Files::isRegularFile).mapToLong(f -> f.toFile().length()).sum();
188+
final var roots = roots();
189+
try {
190+
for (Path dir : roots.stream().filter(Files::isDirectory).toList()) {
191+
try (Stream<Path> stream = Files.walk(dir)) {
192+
reply += stream.mapToLong(PathGroup::sizeInBytes).sum();
193+
}
185194
}
195+
reply += roots.stream().mapToLong(PathGroup::sizeInBytes).sum();
196+
} catch (UncheckedIOException ex) {
197+
throw ex.getCause();
186198
}
187199
return reply;
188200
}
189201

202+
private static long sizeInBytes(Path path) throws UncheckedIOException {
203+
if (Files.isRegularFile(path)) {
204+
try {
205+
return Files.size(path);
206+
} catch (IOException ex) {
207+
throw new UncheckedIOException(ex);
208+
}
209+
} else {
210+
return 0;
211+
}
212+
}
213+
190214
/**
191215
* Creates a copy of this path group with all paths resolved against the given
192216
* root. Taken action is equivalent to creating a copy of this path group and
@@ -300,16 +324,42 @@ private void deleteEntries() throws IOException {
300324
}
301325
}
302326

303-
private record CopySpec(Path from, Path to, Path fromNormalized, Path toNormalized) {
327+
private record CopySpec(Path basepath, Path from, Path to) {
304328
CopySpec {
305-
Objects.requireNonNull(from);
306-
Objects.requireNonNull(fromNormalized);
329+
Objects.requireNonNull(basepath);
307330
Objects.requireNonNull(to);
308-
Objects.requireNonNull(toNormalized);
331+
if (!from.startsWith(basepath)) {
332+
throw new IllegalArgumentException();
333+
}
334+
}
335+
336+
@Override
337+
public int hashCode() {
338+
return Objects.hash(from, to);
339+
}
340+
341+
@Override
342+
public boolean equals(Object obj) {
343+
if (this == obj) {
344+
return true;
345+
}
346+
if ((obj == null) || (getClass() != obj.getClass())) {
347+
return false;
348+
}
349+
CopySpec other = (CopySpec) obj;
350+
return Objects.equals(from, other.from) && Objects.equals(to, other.to);
351+
}
352+
353+
Path fromNormalized() {
354+
return from().normalize();
355+
}
356+
357+
Path toNormalized() {
358+
return to().normalize();
309359
}
310360

311361
CopySpec(Path from, Path to) {
312-
this(from, to, from.normalize(), to.normalize());
362+
this(from, from, to);
313363
}
314364
}
315365

@@ -361,47 +411,55 @@ private static void copy(List<CopySpec> copySpecs, List<Path> excludePaths,
361411
Objects.requireNonNull(excludePaths);
362412
Objects.requireNonNull(handler);
363413

364-
final var copySpecMap = copySpecs.stream().<CopySpec>mapMulti((copySpec, consumer) -> {
365-
final var src = copySpec.from;
414+
415+
final var filteredCopySpecs = copySpecs.stream().<CopySpec>mapMulti((copySpec, consumer) -> {
416+
final var src = copySpec.from();
366417

367418
if (!Files.exists(src) || match(src, excludePaths)) {
368419
return;
369420
}
370421

371-
if (Files.isDirectory(copySpec.from)) {
422+
if (Files.isDirectory(copySpec.from())) {
372423
final var dst = copySpec.to;
373424
final var walkMode = followSymlinks ? new FileVisitOption[] { FileVisitOption.FOLLOW_LINKS } : new FileVisitOption[0];
374425
try (final var files = Files.walk(src, walkMode)) {
375426
files.filter(file -> {
376427
return !match(file, excludePaths);
377428
}).map(file -> {
378-
return new CopySpec(file, dst.resolve(src.relativize(file)));
429+
return new CopySpec(src, file, dst.resolve(src.relativize(file)));
379430
}).toList().forEach(consumer::accept);
380431
} catch (IOException ex) {
381432
throw new UncheckedIOException(ex);
382433
}
383434
} else {
384435
consumer.accept(copySpec);
385436
}
386-
}).collect(toMap(CopySpec::toNormalized, x -> x, (x, y) -> {
387-
if (x.fromNormalized.equals(y.fromNormalized)) {
388-
// Duplicated copy specs, accept.
389-
return x;
390-
} else {
391-
throw new IllegalStateException(String.format(
392-
"Duplicate source files [%s] and [%s] for [%s] destination file", x.from, y.from, x.to));
393-
}
437+
}).collect(groupingBy(CopySpec::fromNormalized, collectingAndThen(toSet(), copySpecGroup -> {
438+
return copySpecGroup.stream().filter(copySpec -> {
439+
for (final var otherCopySpec : copySpecGroup) {
440+
if (otherCopySpec != copySpec && !otherCopySpec.basepath().equals(copySpec.basepath())
441+
&& otherCopySpec.basepath().startsWith(copySpec.basepath())) {
442+
return false;
443+
}
444+
}
445+
return true;
446+
}).toList();
447+
}))).values().stream().flatMap(Collection::stream).toList();
448+
449+
filteredCopySpecs.stream().collect(toMap(CopySpec::toNormalized, x -> x, (x, y) -> {
450+
throw new IllegalStateException(String.format(
451+
"Duplicate source files [%s] and [%s] for [%s] destination file", x.from(), y.from(), x.to()));
394452
}));
395453

396454
try {
397-
copySpecMap.values().stream().forEach(copySpec -> {
455+
filteredCopySpecs.stream().forEach(copySpec -> {
398456
try {
399-
if (Files.isSymbolicLink(copySpec.from)) {
400-
handler.copySymbolicLink(copySpec.from, copySpec.to);
401-
} else if (Files.isDirectory(copySpec.from)) {
402-
handler.createDirectory(copySpec.to);
457+
if (Files.isSymbolicLink(copySpec.from())) {
458+
handler.copySymbolicLink(copySpec.from(), copySpec.to());
459+
} else if (Files.isDirectory(copySpec.from())) {
460+
handler.createDirectory(copySpec.to());
403461
} else {
404-
handler.copyFile(copySpec.from, copySpec.to);
462+
handler.copyFile(copySpec.from(), copySpec.to());
405463
}
406464
} catch (IOException ex) {
407465
throw new UncheckedIOException(ex);

0 commit comments

Comments
 (0)