Repository navigation
fix: ignore a token file in a jar that was written for another project - #25682
totally-not-ai[bot] wants to merge 9 commits into
Conversation
When no flow-build-info.json is found outside a jar, the lookup falls back to a copy inside one. A copy packaged into a dependency by mistake is written by prepare-frontend, so it carries the folders and the Node version of the machine that built the dependency, and the application fails to start with a folder it has never heard of. Adds a failing test for that case and one that keeps the packaged application case, where the token file of the application itself is inside a jar, working.
The flow-build-info.json lookup skips copies inside jars and falls back to one only when the application has none of its own, which is the case for a packaged application. A copy that a dependency packages by mistake was then used as well, and as prepare-frontend writes it, it brought the project folders and the Node version of the machine that built the dependency, failing the startup with a folder that does not exist. A file from a jar is now used only when it declares production mode, which a packaged application always does and a file left over from a development build never does. The ignored file is named in a warning so that the mistake in the dependency can be fixed.
The warning about not being able to tell which flow-build-info.json is the right one was left out whenever the application looked packaged into a jar, even when the file was not the one the rule for a packaged application points at, which is when knowing about the other candidates helps the most. The warning is now left out only when the file is the one in the outermost jar, and the cases with more than one file inside a jar are covered by tests.
Type of change
How to test
Test coverageIn
The tests that read the file from the class path now use a plain class API changesNone. Note #25681 touches the same method, and makes the counting of nested |
…oken-file-from-jar # Conflicts: # flow-server/src/main/java/com/vaadin/flow/server/startup/DefaultApplicationConfigurationFactory.java # flow-server/src/test/java/com/vaadin/flow/server/startup/DefaultApplicationConfigurationFactoryTest.java
|
To be verified against use cases on Vaadin Forum where the JARs are build in dev mode, could be valid uses cases (before making this ready for review). |
Refusing every token file inside a jar that is not from a production build also refused the file of an application that is packaged into a jar in development mode and run on the machine it was built on, where the project it points to is still there and the settings in it are the right ones. Only a plain or shaded jar is affected, as the file of an application packaged with Spring Boot is under BOOT-INF/classes and is never looked for inside a jar to begin with. A file that is not from a production build is now used when the project it names is on this machine, and ignored when it is not, which is the case for a file packaged into a dependency built somewhere else. The warning names the project folder that is missing.
|
@mshabarov Good call — there is a valid case, and the rule is now narrower. What I checked first is which packagings even reach this code. The fallback only runs when every
So an application packaged with Spring Boot, in development mode or not, never gets here: its own file is under The rule is now:
The warning now names the missing project folder, so the dependency at fault can be found. Added |
|
Instead of doing it this way, could we do a refactoring so that development time info and production mode info would be written to separate files and the production mode info would be read from a jar? So instead of |
|
@Artur- I agree that is the better model, and it would remove a whole family of workarounds rather than just this one. I wrote it up as #25856 instead of turning it into this pull request, because it reaches further than it looks. What made me keep the two apart:
What the write-up also collects is how much the single file already costs us, which is the strongest argument for doing it: the runtime guesses which jar owns a copy by counting So my suggestion is to land this as the fix for the broken startups, and do the split on its own in #25856. If you would rather have the split first and let this wait, that is fine too — say so and I will leave this pull request as is until then. |
|
Runtime (Copilot preparing frontend) and compile time |
In development mode the project folder of the application is found from the class path or the working directory, but a token file taken from a jar overrides it together with the Node version. A token file that an add-on built on this machine packages names a folder that exists, so it was used: the application then ran the frontend build with the Node version and, worse, the project folder of the add-on, while running the same build through the Maven plugin found the right ones. A development mode token file from a jar is now used only when the project it names is the project the application is run from, whenever that can be told. When it cannot, the file is used if its project is on this machine, so an application packaged in development mode keeps working. The check of the working directory moves to FileIOUtils so the configuration and the lookup share it.
…n a jar The working directory was taken as the project folder of the application as firmly as the class path, so an application packaged in development mode and started from the root of a multi-module build, as in java -jar app/target/app.jar, had its own token file ignored: the root has a pom.xml but is not the module the file was written for. The class path still has to name exactly the project of the file, as the application then runs from the output folder of that project. The working directory is only a hint, so a project inside it is accepted. The lookups are now tested through the real working directory as well.
totally-not-ai
Bot
changed the title
|
@samie Thanks, that case was not covered yet, and it is fixed now. Why the two disagree: the Maven plugin takes the Node version from its own configuration and never reads a token file, while the runtime in development mode reads The previous rule on this pull request only ignored such a file when the project folder it names did not exist. An add-on built on the same machine names a folder that does exist, so it slipped through — which matches what you saw. Changed now: a development mode token file from a jar is used only when it was written for the application being run. When the application runs from the output folder of its project ( The warning names both folders, for example: Covered by |
|
This branch has not been deployed
3 participants
Summary
Sometimes a dependency ships a
flow-build-info.jsonfrom a development build by mistake. The application then picked up that file's project folder and Node version, and startup either failed or ran the frontend build in the wrong project. Now a token file from a jar is used only when it is from a production build, or when it was written for the project the application runs from.What changed
Behavior change: these rules apply only when the lookup falls back to a
flow-build-info.jsoninside a jar. This happens when the application has no token file of its own, as with a packaged application.java -jar app/target/app.jarstarted from the root of a multi-module build.AbstractConfigurationtoFileIOUtils, so both places share it.API Changes
com.vaadin.flow.internal.FileIOUtils
Test summary
Token file lookup inside a jar:
Packaged application: