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.

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

Prefer output dir for hot deployment of resources #15312

Merged
merged 1 commit into from
Mar 16, 2021

Conversation

brunolmfg
Copy link
Contributor

The original resources without Maven properties resolution were provided by Static Handler for src/main/resources/META-INF/resources files when running in dev mode. The strategy used for hot deployment doesn't consider the resources filtering. So the interpolation of Maven properties wasn't processed.

Resolves #15303

@brunolmfg
Copy link
Contributor Author

That solution has a side effect in config editor of dev mode console. Failed tests in DevConsoleConfigEditorTest and DevConsoleConfigEditorBodyHandlerTest when editing config.

@brunolmfg
Copy link
Contributor Author

I've turned to include the first classesPath in RuntimeUpdatesProcessor#getClassesDir() instead resourcesPath. And StaticResourcesHotReplacementSetup will consider and prefer the HotReplacementContext#getClassesDir().

@gsmet
Copy link
Member

gsmet commented Feb 25, 2021

@stuartwdouglas probably something for you to look at.

@stuartwdouglas
Copy link
Member

This won't help if the resource is modified though, as dev mode will copy the file without interpolation. On the face of it it appears ok but it is only a partial solution to the issue.

@brunolmfg
Copy link
Contributor Author

It's true. Using a IDE the changes were copied with interpolation and dev mode doesn't detect any change. But using a usual text editor the dev mode detect changes and copy the source file without interpolation. To resolve the later situation will need to call ClassLoaderCompiler firing the "compilation" with a CompilationProvider for filtered resources. Is it correct?

Is ok merging that partial solution?

Copy link
Member

@stuartwdouglas stuartwdouglas left a comment

Choose a reason for hiding this comment

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

I think it is better than the current situation, so I am ok with merging.

@brunolmfg
Copy link
Contributor Author

@gsmet will it be necessary to rebase? Is it possible to be merged into 1.12 or only for the next minor? 😄

Base automatically changed from master to main March 12, 2021 15:55
The original resources without Maven properties resolution were provided by
Static Handler for `src/main/resources/META-INF/resources` files when running
in dev mode. The strategy used for hot deployment doesn't consider the resources
filtering. So the interpolation of Maven properties wasn't processed.

Resolves quarkusio#15303
@gsmet gsmet merged commit bc1480e into quarkusio:main Mar 16, 2021
@quarkus-bot quarkus-bot bot added this to the 1.13 - main milestone Mar 16, 2021
@gsmet
Copy link
Member

gsmet commented Mar 16, 2021

I think it's a bit too risky to be backported so it will be in 1.13.

@brunolmfg brunolmfg deleted the issue-15303 branch March 16, 2021 14:07
@brunolmfg
Copy link
Contributor Author

I totally agree. And I'm thinking about a set of changes over static resources. I've opened #15320 for provide customization of index page, but there are other improvements very useful. In a future moment I could invest a effort cataloging them.

Thank you @gsmet and @stuartwdouglas!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Static Resources in devmode doesn't provide filtered ones
3 participants