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.

Sign up for GitHub

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

Jump to bottom

Qute: ultimate fix for the problem with registering NativeImageResourceBuildItem correctly on Windows #40158

Merged
merged 3 commits into from
Apr 22, 2024
Merged

Qute: ultimate fix for the problem with registering NativeImageResourceBuildItem correctly on Windows #40158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cb39e07
Qute: add qute integration-test module to native Windows CI category
mkouba Apr 19, 2024
6014d51
Refactor Qute template scanning
aloubyansky Apr 16, 2024
202ea6e
Qute: use the relative resource path for NativeImageResourceBuildItem
mkouba Apr 19, 2024
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
6 changes: 3 additions & 3 deletions .github/native-tests.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@
"os-name": "ubuntu-latest"
},
{
"category": "Windows - RESTEasy Jackson",
"timeout": 25,
"test-modules": "resteasy-jackson",
"category": "Windows support",
"timeout": 50,
"test-modules": "resteasy-jackson, qute",
"os-name": "windows-latest"
},
{
Expand Down
186 changes: 49 additions & 137 deletions extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import jakarta.inject.Singleton;

Expand Down Expand Up @@ -93,12 +92,15 @@
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
import io.quarkus.deployment.pkg.NativeConfig;
import io.quarkus.deployment.pkg.builditem.CurateOutcomeBuildItem;
import io.quarkus.fs.util.ZipUtils;
import io.quarkus.gizmo.ClassOutput;
import io.quarkus.gizmo.MethodDescriptor;
import io.quarkus.maven.dependency.Dependency;
import io.quarkus.maven.dependency.ArtifactKey;
import io.quarkus.maven.dependency.DependencyFlags;
import io.quarkus.maven.dependency.ResolvedDependency;
import io.quarkus.panache.common.deployment.PanacheEntityClassesBuildItem;
import io.quarkus.paths.FilteredPathTree;
import io.quarkus.paths.PathFilter;
import io.quarkus.paths.PathTree;
import io.quarkus.qute.CheckedTemplate;
import io.quarkus.qute.Engine;
import io.quarkus.qute.EngineBuilder;
Expand Down Expand Up @@ -2124,9 +2126,6 @@ void collectTemplates(ApplicationArchivesBuildItem applicationArchives,
QuteConfig config,
TemplateRootsBuildItem templateRoots)
throws IOException {
Set<ApplicationArchive> allApplicationArchives = applicationArchives.getAllApplicationArchives();
List<ResolvedDependency> extensionArtifacts = curateOutcome.getApplicationModel().getDependencies().stream()
.filter(Dependency::isRuntimeExtensionArtifact).collect(Collectors.toList());

// Make sure the new templates are watched as well
watchedPaths.produce(HotDeploymentWatchedFileBuildItem.builder().setLocationPredicate(new Predicate<String>() {
Expand All @@ -2141,80 +2140,39 @@ public boolean test(String path) {
}
}).build());

for (ResolvedDependency artifact : extensionArtifacts) {
if (isApplicationArchive(artifact, allApplicationArchives)) {
// Skip extension archives that are also application archives
continue;
}
for (Path resolvedPath : artifact.getResolvedPaths()) {
if (Files.isDirectory(resolvedPath)) {
scanRootPath(resolvedPath, config, templateRoots, watchedPaths, templatePaths,
nativeImageResources);
} else {
try (FileSystem artifactFs = ZipUtils.newFileSystem(resolvedPath)) {
// Iterate over template roots, such as "templates", and collect the included templates
for (String templateRoot : templateRoots) {
Path artifactBasePath = artifactFs.getPath(templateRoot);
if (Files.exists(artifactBasePath)) {
LOGGER.debugf("Found template root in extension artifact: %s", resolvedPath);
scanDirectory(artifactBasePath, artifactBasePath, templateRoot + "/", watchedPaths,
templatePaths,
nativeImageResources,
config);
}
}
} catch (IOException e) {
LOGGER.warnf(e, "Unable to create the file system from the path: %s", resolvedPath);
}
}
final Set<ApplicationArchive> allApplicationArchives = applicationArchives.getAllApplicationArchives();
final Set<ArtifactKey> appArtifactKeys = new HashSet<>(allApplicationArchives.size());
for (var archive : allApplicationArchives) {
appArtifactKeys.add(archive.getKey());
}
for (ResolvedDependency artifact : curateOutcome.getApplicationModel()
.getDependencies(DependencyFlags.RUNTIME_EXTENSION_ARTIFACT)) {
// Skip extension archives that are also application archives
if (!appArtifactKeys.contains(artifact.getKey())) {
scanPathTree(artifact.getContentTree(), templateRoots, watchedPaths, templatePaths, nativeImageResources,
config);
}
}
for (ApplicationArchive archive : allApplicationArchives) {
archive.accept(tree -> {
for (Path root : tree.getRoots()) {
// Note that we cannot use ApplicationArchive.getChildPath(String) here because we would not be able to detect
// a wrong directory name on case-insensitive file systems
scanRootPath(root, config, templateRoots, watchedPaths, templatePaths, nativeImageResources);
}
});
archive.accept(
tree -> scanPathTree(tree, templateRoots, watchedPaths, templatePaths, nativeImageResources, config));
}
}

private void scanRootPath(Path rootPath, QuteConfig config, TemplateRootsBuildItem templateRoots,
private void scanPathTree(PathTree pathTree, TemplateRootsBuildItem templateRoots,
BuildProducer<HotDeploymentWatchedFileBuildItem> watchedPaths,
BuildProducer<TemplatePathBuildItem> templatePaths,
BuildProducer<NativeImageResourceBuildItem> nativeImageResources) {
scanRootPath(rootPath, rootPath, config, templateRoots, watchedPaths, templatePaths, nativeImageResources);
}

private void scanRootPath(Path rootPath, Path path, QuteConfig config, TemplateRootsBuildItem templateRoots,
BuildProducer<HotDeploymentWatchedFileBuildItem> watchedPaths,
BuildProducer<TemplatePathBuildItem> templatePaths,
BuildProducer<NativeImageResourceBuildItem> nativeImageResources) {
if (!Files.isDirectory(path)) {
return;
}
try (Stream<Path> paths = Files.list(path)) {
for (Path file : paths.collect(Collectors.toList())) {
if (Files.isDirectory(file)) {
// Iterate over the directories in the root
// "/io", "/META-INF", "/templates", "/web", etc.
Path relativePath = rootPath.relativize(file);
if (templateRoots.isRoot(relativePath)) {
LOGGER.debugf("Found templates root dir: %s", file);
// The base path is an OS-specific template root path relative to the scanned root path
String basePath = relativePath.toString() + relativePath.getFileSystem().getSeparator();
scanDirectory(file, file, basePath, watchedPaths, templatePaths,
nativeImageResources,
config);
} else if (templateRoots.maybeRoot(relativePath)) {
// Scan the path recursively because the template root may be nested, for example "/web/public"
scanRootPath(rootPath, file, config, templateRoots, watchedPaths, templatePaths, nativeImageResources);
}
BuildProducer<NativeImageResourceBuildItem> nativeImageResources,
QuteConfig config) {
for (String templateRoot : templateRoots) {
pathTree.accept(templateRoot, visit -> {
if (visit != null) {
// if template root is found in this tree then walk over its subtree
scanTemplateRootSubtree(
new FilteredPathTree(pathTree, PathFilter.forIncludes(List.of(templateRoot + "/**"))),
visit.getPath(), watchedPaths, templatePaths, nativeImageResources, config);
}
}
} catch (IOException e) {
throw new UncheckedIOException(e);
});
}
}

Expand Down Expand Up @@ -3397,85 +3355,52 @@ public static String getName(InjectionPointInfo injectionPoint) {
* @param templatePaths
* @param watchedPaths
* @param nativeImageResources
* @param osSpecificResourcePath The OS-specific resource path, i.e. templates\nested\foo.html
* @param resourcePath The relative resource path, including the template root
* @param templatePath The path relative to the template root; using the {@code /} path separator
* @param originalPath
* @param config
*/
private static void produceTemplateBuildItems(BuildProducer<TemplatePathBuildItem> templatePaths,
BuildProducer<HotDeploymentWatchedFileBuildItem> watchedPaths,
BuildProducer<NativeImageResourceBuildItem> nativeImageResources, String osSpecificResourcePath,
BuildProducer<NativeImageResourceBuildItem> nativeImageResources, String resourcePath,
String templatePath,
Path originalPath, QuteConfig config) {
if (templatePath.isEmpty()) {
return;
}
// OS-agnostic full path, i.e. templates/foo.html
String osAgnosticResourcePath = toOsAgnosticPath(osSpecificResourcePath, originalPath.getFileSystem());
LOGGER.debugf("Produce template build items [templatePath: %s, osSpecificResourcePath: %s, originalPath: %s",
templatePath,
osSpecificResourcePath,
resourcePath,
originalPath);
boolean restartNeeded = true;
if (config.devMode.noRestartTemplates.isPresent()) {
restartNeeded = !config.devMode.noRestartTemplates.get().matcher(osAgnosticResourcePath).matches();
restartNeeded = !config.devMode.noRestartTemplates.get().matcher(resourcePath).matches();
}
watchedPaths.produce(new HotDeploymentWatchedFileBuildItem(osAgnosticResourcePath, restartNeeded));
nativeImageResources.produce(new NativeImageResourceBuildItem(osSpecificResourcePath));
watchedPaths.produce(new HotDeploymentWatchedFileBuildItem(resourcePath, restartNeeded));
nativeImageResources.produce(new NativeImageResourceBuildItem(resourcePath));
Copy link
Member

Choose a reason for hiding this comment

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

It's good enough, probably but we still have the issue that this is gonna be interpreted like a regexp, right? So . will mean any char for instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but that's another issue that should be addressed in a separate pull request.

templatePaths.produce(
new TemplatePathBuildItem(templatePath, originalPath,
readTemplateContent(originalPath, config.defaultCharset)));
}

/**
*
* @param root
* @param directory
* @param basePath OS-specific template root path relative to the scanned root path, e.g. {@code templates/}
* @param watchedPaths
* @param templatePaths
* @param nativeImageResources
* @param config
* @throws IOException
*/
private void scanDirectory(Path root, Path directory, String basePath,
private void scanTemplateRootSubtree(PathTree pathTree, Path templateRoot,
BuildProducer<HotDeploymentWatchedFileBuildItem> watchedPaths,
BuildProducer<TemplatePathBuildItem> templatePaths,
BuildProducer<NativeImageResourceBuildItem> nativeImageResources,
QuteConfig config)
throws IOException {
try (Stream<Path> files = Files.list(directory)) {
Iterator<Path> iter = files.iterator();
while (iter.hasNext()) {
Path filePath = iter.next();
/*
* Fix for https://github.com/quarkusio/quarkus/issues/25751 where running tests in Eclipse
* sometimes produces `/templates/tags` (absolute) files listed for `templates` (relative)
* directories, so we work around this
*/
if (!directory.isAbsolute()
&& filePath.isAbsolute()
&& filePath.getRoot() != null) {
filePath = filePath.getRoot().relativize(filePath);
}
if (Files.isRegularFile(filePath)) {
LOGGER.debugf("Found template: %s", filePath);
Path relativePath = root.relativize(filePath);
String templatePath = toOsAgnosticPath(relativePath);
if (config.templatePathExclude.matcher(templatePath).matches()) {
LOGGER.debugf("Template file excluded: %s", filePath);
continue;
}
produceTemplateBuildItems(templatePaths, watchedPaths, nativeImageResources,
basePath + relativePath.toString(),
templatePath,
filePath, config);
} else if (Files.isDirectory(filePath)) {
LOGGER.debugf("Scan directory: %s", filePath);
scanDirectory(root, filePath, basePath, watchedPaths, templatePaths, nativeImageResources, config);
}
QuteConfig config) {
pathTree.walk(visit -> {
if (Files.isRegularFile(visit.getPath())) {
LOGGER.debugf("Found template: %s", visit.getPath());
String templatePath = toOsAgnosticPath(templateRoot.relativize(visit.getPath()));
if (config.templatePathExclude.matcher(templatePath).matches()) {
LOGGER.debugf("Template file excluded: %s", visit.getPath());
return;
}
produceTemplateBuildItems(templatePaths, watchedPaths, nativeImageResources,
visit.getRelativePath("/"),
templatePath, visit.getPath(), config);
}
}
});
}

private static String toOsAgnosticPath(String path, FileSystem fs) {
Expand Down Expand Up @@ -3518,19 +3443,6 @@ private void checkDuplicatePaths(List<TemplatePathBuildItem> templatePaths) {
}
}

private boolean isApplicationArchive(ResolvedDependency dependency, Set<ApplicationArchive> applicationArchives) {
for (ApplicationArchive archive : applicationArchives) {
if (archive.getKey() == null) {
continue;
}
if (dependency.getGroupId().equals(archive.getKey().getGroupId())
&& dependency.getArtifactId().equals(archive.getKey().getArtifactId())) {
return true;
}
}
return false;
}

static String readTemplateContent(Path path, Charset defaultCharset) {
try {
return Files.readString(path, defaultCharset);
Expand Down
5 changes: 1 addition & 4 deletions ...ent/src/test/java/io/quarkus/qute/deployment/templateroot/AdditionalTemplateRootTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,8 @@ public void execute(BuildContext context) {
List<NativeImageResourceBuildItem> items = context.consumeMulti(NativeImageResourceBuildItem.class);
for (NativeImageResourceBuildItem item : items) {
if (item.getResources().contains("web/public/hello.txt")
|| item.getResources().contains("web\\public\\hello.txt")
|| item.getResources().contains("templates/hi.txt")
|| item.getResources().contains("templates\\hi.txt")
|| item.getResources().contains("templates/nested/hoho.txt")
|| item.getResources().contains("templates\\nested\\hoho.txt")) {
|| item.getResources().contains("templates/nested/hoho.txt")) {
found++;
}
}
Expand Down