quarkus-quinoa icon indicating copy to clipboard operation
quarkus-quinoa copied to clipboard

Quinoa checks for "src/main" directory, even if not used

Open LouizFC opened this issue 2 years ago • 12 comments
trafficstars

Greetings.

I am using quinoa + jib to produce docker images for a web project. The project itself is using Quarkus as thin wrapper to serve the files produced by the Quinoa processor, so the only "java-related" files in the project is a config/application.properties and a pom.xml.

The project is structured like a typical vite web project, which means that the project is not structured like a maven project, the web related files are in ${project.root}, not containing any main folder (like a typical maven project would).

My application.properties:

quarkus.quinoa.package-manager-install=true
quarkus.quinoa.package-manager-install.node-version=16.17.0
quarkus.quinoa.dev-server.port=5173
quarkus.quinoa.build-dir=dist
quarkus.quinoa.enable-spa-routing=true
quarkus.quinoa.ui-dir=.

When I run ./mvnw quarkus:dev, quarkus shows the following warning:

[WARNING] [io.quarkiverse.quinoa.deployment.QuinoaProcessor] Quinoa directory not found 'quarkus.quinoa.ui-dir=.'. It is recommended to remove the quarkus-quinoa extension if not used.

I found it strange at first, so I tried running the following command: ./mvnw quarkus:dev -Dquarkus.quinoa.ui-dir=$PWD

Which resulted to the same warning (but with the $PWD path resolved).

I debugged the building process by doing mvnDebug clean package -Dquarkus.quinoa.ui-dir=$PWD and found the following check being done:

https://github.com/quarkiverse/quarkus-quinoa/blob/e90d83f5e9f160976f25908989a9c49db8ed3423/deployment/src/main/java/io/quarkiverse/quinoa/deployment/QuinoaProcessor.java#L287

I them added a mock file in src/main/testfile and everything worked.

So wanted to point out if the src/main check is really necessary? Maintaining a mock file just as a workaround seems like a minor inconvenience and adds the possibility of accidentally breaking the build if someone removes it, so I think a mechanism to work around the findMainSourcesRoot check should be created.

Thanks in advance.

LouizFC avatar Dec 02 '22 14:12 LouizFC

This is a very good point, TBH I haven't thought about the possibility to have the application.properties somewhere else than src/main/resources :). Did you configure that in the pom.xml?

Looking for src/main is the current way to find the project directory from the extension processor (independent of the buildtool), maybe there is another way to do so? @geoand any idea?

ia3andy avatar Dec 02 '22 15:12 ia3andy

Thank you for the fast reply :)

Did you configure that in the pom.xml?

No. Initially I was planning to use Env variables, but this page: https://quarkus.io/guides/config-reference#configuration-sources says that quarkus also looks for $PWD/config/application.properties

LouizFC avatar Dec 02 '22 15:12 LouizFC

No. Initially I was planning to use Env variables, but this page: quarkus.io/guides/config-reference#configuration-sources says that quarkus also looks for $PWD/config/application.properties

so maybe the solution is to look for src/main or config/

ia3andy avatar Dec 02 '22 15:12 ia3andy

Let's wait for @geoand, if there is a best option he will know :)

@LouizFC are you up for your first contrib to Quinoa?

ia3andy avatar Dec 02 '22 15:12 ia3andy

@ia3andy sure. Lets wait for some feedback on how to approach this, meanwhile I will take a look to see how the integration tests work.

LouizFC avatar Dec 02 '22 17:12 LouizFC

There isn't really any bulletproof way. We use src/main in Jib because in that case it's guantateed to be around. Here, you'll probable have to be a little more flexible.

geoand avatar Dec 05 '22 07:12 geoand

Thanks @geoand for the info!

@LouizFC I let you find the best solution and start the PR :)

ia3andy avatar Dec 05 '22 08:12 ia3andy

🙏🏼

geoand avatar Dec 05 '22 08:12 geoand

I think a "quick and dirty" way to (partially) solve this is by skipping computeUIDir if ui-dir is absolute, but I don't know if that would be an acceptable solution. Here is my reasoning:

As far as I checked, findMainSourcesRoot is used only in computeUIDir.

so maybe the solution is to look for src/main or config/

I understand that the reasoning for calling findMainSourcesRoot is because outputTarget could be anywhere in the project depending what tooling is calling it (and what config such tooling has), but checking for other folders (config/ in this case) could end up breaking other unusual setups like mine (for example, setups that rely only on env variables)

The ideal way would be rethinking findMainSourcesRoot, but as said above, there is no bulletproof way of doing so as of now.

So, to not introduce any regressions, I think skipping the findMainSourcesRoot problem for cases where ui-dir is absolute, while not ideal, would be enough for such edge cases. Maybe also add a disclaimer in the docs about this behavior.


I also wanted to point out that I am having difficulty coming up with an IT test, because to "ideally" check for this, the IT test folder would need to have no src/main present, the current project setup has all tests inside the same folder, which contains a src/main folder.

With that said, should I create another integration-tests-like folder to test for this?

LouizFC avatar Dec 05 '22 15:12 LouizFC

I think a "quick and dirty" way to (partially) solve this is by skipping computeUIDir if ui-dir is absolute, but I don't know if that would be an acceptable solution. Here is my reasoning:

As far as I checked, findMainSourcesRoot is used only in computeUIDir.

so maybe the solution is to look for src/main or config/

I understand that the reasoning for calling findMainSourcesRoot is because outputTarget could be anywhere in the project depending what tooling is calling it (and what config such tooling has), but checking for other folders (config/ in this case) could end up breaking other unusual setups like mine (for example, setups that rely only on env variables)

The ideal way would be rethinking findMainSourcesRoot, but as said above, there is no bulletproof way of doing so as of now.

So, to not introduce any regressions, I think skipping the findMainSourcesRoot problem for cases where ui-dir is absolute, while not ideal, would be enough for such edge cases. Maybe also add a disclaimer in the docs about this behavior.

Using absolute path is not possible for multi-user projects, so better not recommend this approach

I also wanted to point out that I am having difficulty coming up with an IT test, because to "ideally" check for this, the IT test folder would need to have no src/main present, the current project setup has all tests inside the same folder, which contains a src/main folder.

With that said, should I create another integration-tests-like folder to test for this?

Instead of creating a new integration-test module, I would make computeUIDir static and provide a UnitTest on it to make sure it works the way we want.

ia3andy avatar Dec 06 '22 08:12 ia3andy

Using absolute path is not possible for multi-user projects, so better not recommend this approach

I see, my mind was set on allowing ./mvnw quarkus:dev -Dquarkus.quinoa.ui-dir=$PWD (or similarly, CI dir variables) as a workaround while maintaining the current behavior, because honestly I currently don't see a solution that does not introduce other problems.

I was thinking about employing the same approach Quarkus itself does, which is checking for a config/application.properties file at System.getProperty("user.dir"), but seeing how one could call maven/gradle/quarkus-cli from anywhere, I think this could also introduce unintended behavior.

Overall after researching for this problem, it is better to be flexible as @geoand has pointed out. I solved this cleanly just by creating a server/pom.xml with a dummy src/main folder and calling ./mvnw quarkus:dev -f server/pom.xml. Another counter argument for this issue is configuration, which is pointed out here, rather than relying "on build" static configs for the web files, it is better to have a simple endpoint that serves it, which would require a src/main (unless it is configured in another way, but the point still stands).

That said, I think that pointing out in the docs that there is a known issue and presenting the workarounds should be enough to solve this issue as of now.

LouizFC avatar Dec 06 '22 13:12 LouizFC

I have a similar problem with the findMainSourcesRoot functionality. When using a custom nested quarkus.package.output-directory and its navigating to the parent, the parent does not exist. That means we receive an error.

Would it be an option to make the main sources root configurable with an absolute path? This would probably solve the problem without any regression.

stephan-strate avatar Dec 14 '22 09:12 stephan-strate

@stephan-strate what's the value in quarkus.package.output-directory I will give it a try

ia3andy avatar Dec 15 '22 10:12 ia3andy

Example usecase would be a centralized build pipeline for multiple different quarkus projects. The pipeline can control where to output the result.

stephan-strate avatar Dec 15 '22 10:12 stephan-strate

I think this problem is not a "Quinoa" problem but more general and should be fixed in core, we can't have all the extension providing a config for the same thing (src dir) cc @gsmet.

Let's just remove this need :) so here is my take on this, finding the project root is used as default dir to install node/npm, but we could also install it in the webui directory when it is set with an absolute path AND the src/main is not found ([webuidir]/.quinoa). Which would mean that using quarkus.package.output-directory would work by default.

ia3andy avatar Dec 15 '22 15:12 ia3andy