flow icon indicating copy to clipboard operation
flow copied to clipboard

The `vaadinPrepareFrontendTask` gradle task is not cached

Open steve-todorov opened this issue 1 year ago • 51 comments

Description of the bug

We have upgrade to Vaadin 24.2.0 with the hopes to benefit from the newly introduced incremental Gradle task caching for vaadinPrepareFrontendTask (related PR is https://github.com/vaadin/flow/pull/17150).

Unfortunately our local and CI builds seem to be unable to benefit from this feature. Upon closer investigation we noticed Gradle scans show the following notice:

Overlapping outputs: Gradle does not know how file 'package.json' was created (output property 'taskOutputProperties.packageJson'). 
Task output caching requires exclusive access to output paths to guarantee correctness

image

Expected behavior

The vaadinPrepareFrontendTask should show UP-TO-DATE or FROM CACHE in the gradle build scans and the task should not be computing again.

Minimal reproducible example

Not sure how to reproduce yet, but we don't have any special setup. We are using the gradle vaadin plugin and run gradle build.

Versions

  • Vaadin / Flow version: 24.2.0
  • Java version: 17
  • OS version: ubuntu-22.04 (GH CI)
  • Browser version (if applicable):
  • Application Server (if applicable):
  • IDE (if applicable):

steve-todorov avatar Oct 27 '23 17:10 steve-todorov

Thanks for the issue. We will try to reproduce with our Gradle-based starters and get back to you soon.

mshabarov avatar Oct 31 '23 11:10 mshabarov

Thanks for looking into this @mshabarov!

Let me know if you need me to give you additional context or if you need anything more.

steve-todorov avatar Oct 31 '23 12:10 steve-todorov

Tested this out with Vaadin 24.2.0 and the base starter. There the caching seems to work fine. This is the output I get:

> Task :vaadinPrepareFrontend FROM-CACHE
Build cache key for task ':vaadinPrepareFrontend' is 31bd25e9d107d53a30be48c593072e24
Task ':vaadinPrepareFrontend' is not up-to-date because:
  Output property 'taskOutputProperties.resourceOutputDirectory' file /Users/tepi/Downloads/base-starter-spring-gradle-24/build/vaadin-generated has been removed.
  Output property 'taskOutputProperties.resourceOutputDirectory' file /Users/tepi/Downloads/base-starter-spring-gradle-24/build/vaadin-generated/META-INF has been removed.
  Output property 'taskOutputProperties.resourceOutputDirectory' file /Users/tepi/Downloads/base-starter-spring-gradle-24/build/vaadin-generated/META-INF/VAADIN has been removed.
Loaded cache entry for task ':vaadinPrepareFrontend' with cache key 31bd25e9d107d53a30be48c593072e24
Resolve mutations for :processResources (Thread[Execution worker Thread 5,5,main]) started.
:processResources (Thread[Execution worker Thread 5,5,main]) started.

Any chance you could share a minimal project that reproduces the issue?

tepi avatar Nov 01 '23 12:11 tepi

Hey @tepi,

The base starter project does not appear to have a package.json and package-lock.json files. In our project we have both of them. My current working theory is the vaadinBuildFrontend task might be corrupting the cache, because sometimes it updates packages in the package.json which were not committed into the git.

image

So what appears to be happening is that the vaadinBuildFrontend is overriding the package.json file and thus changing the cache key that was initially generated by the vaadinPrepareFrontend task. I have been able to reproduce the problem with the base starter project.

You can have a look in my fork located here: https://github.com/steve-todorov/base-starter-gradle/tree/issue-17941

Also checkout the GH actions here: https://github.com/steve-todorov/base-starter-gradle/actions/runs/6724319472

The Gradle build scan shows the same problem:

image

The way to reproduce it is by doing the following steps:

  1. rm -rf ~/.gradle/caches/build-cache-1/
  2. gradle clean
  3. Have your package.json and package-lock.json committed in VCS
  4. Your package.json has one package that is outdated (check the package.json --- the @vaadin/upload": "24.1.0 which is then auto updated to 24.2.1 during the vaadinBuildFrontend)
  5. gradle build

steve-todorov avatar Nov 01 '23 19:11 steve-todorov

Most likely without knowing all th relevant context here, the caching seems to work like it should. vaadinPrepareFrontend will update dev dependencies only and vaadinBuildFrontend will update frontend dependencies to the package.json file. If there is an outdated frontend dependency or other discrepancy between package.json and package-lock.json it should be updated by vaadinBuildFrontend task - and at this point the cache is no longer valid. Once the files are up-to-date, the updates should be committed to VCS, after which caching should work on subsequent builds.

If it is Vaadin that causes the outdated dependency to exist after a successfull build, that is probably a separate issue.

tepi avatar Nov 06 '23 06:11 tepi

It's maybe fine if vaadinBuildFrontend overrides package.json within first build or after changes in the project. However, it sounds incorrect If vaadinBuildFrontend overrides package.json within subsequent builds, when no changes are applied to the project.

I propose to focus on why vaadinBuildFrontend updates it.

We can test and see what is changed by manually running vaadinPrepareFrontend, saving package.json and then running vaadinBuildFrontend and seeing what it changes.

mshabarov avatar Nov 06 '23 07:11 mshabarov

With the provided example run locally, vaadinBuildFrontend updates package.json once (the outdated frontend dependency) and after that no more updates.

tepi avatar Nov 06 '23 07:11 tepi

@steve-todorov you mentioned that "@vaadin/upload": "24.1.0" is updated to 24.2.1 during the vaadinBuildFrontend. This is for the first run, right? What is updated in package.json by vaadinBuildFrontend in subsequent runs, can you tell?

mshabarov avatar Nov 06 '23 14:11 mshabarov

@mshabarov You know how sometimes the plugin's version is sometimes behind Vaadin Platform's. At least I remember this in the Maven plugin. Both the plugin and startup code use shared code in vaadin-server that reads com/vaadin/flow/server/frontend/dependencies/default/package.json on the classpath. If the plugin's version is behind the dependency's, prepare frontend will produce a different package.json then after running the app in dev mode. Because running the plugin uses the plugins classpath and app uses app classpath. Then, if you ran build frontend, it would change again, being closer to a run's package.json, but may be different because build frontend does more stuff (I forget everything at the moment, but doesn't it do more intensive scanning?)

Thus, we can not depend on the result of vaadinPrepareFrontend for caching.

Perhaps something related to this is the problem.

oliveryasuna avatar Nov 06 '23 19:11 oliveryasuna

Additional thoughts... Not a solution but definitely part of a stable one.

Disclaimer: I am not a Gradle expert. Could you leverage the power of Gradle to make the vaadinPrepareFrontend and vaadinBuildFrontend tasks always priority your application's classpath, thus ensuring that /flow/server/frontend/dependencies/default/package.json from the Vaadin Platform version you have as a dependency is considered? @steve-todorov?

There is a serious drawback to this approach. Gradle configurations can be complex. I have seen situations where an addon uses a different Vaadin Platform version and ends up taking priority of the Vaadin Platform version used during build. But for some weird reason, when the app runs in dev mode, it uses the right /flow/server/frontend/dependencies/default/package.json. I have also seen this reversed.

oliveryasuna avatar Nov 06 '23 19:11 oliveryasuna

You know how sometimes the plugin's version is sometimes behind Vaadin Platform's.

That sounds like a bug in your build. Both should always be the same.

knoobie avatar Nov 06 '23 20:11 knoobie

You know how sometimes the plugin's version is sometimes behind Vaadin Platform's.

That sounds like a bug in your build. Both should always be the same.

I was not clear. They used to release the Maven plugin a few days or sometimes weeks after Platform. Not sure if they still do this and it doesn't matter for us since switching it Gradle it seems they release the Gradle plugin at the same time.

oliveryasuna avatar Nov 06 '23 20:11 oliveryasuna

Since 2x where the Gradle plugin was officially adopted, this shouldn't be a problem anymore. Now the release process looks simplified like this:

  1. Flow including Spring, Maven and Gradle ("base") (and others)
  2. Platform release with anything Vaadin related is build and tested together including the flow version of 1 with the official build Plugins

knoobie avatar Nov 06 '23 20:11 knoobie

Additional thoughts... Not a solution but definitely part of a stable one.

Disclaimer: I am not a Gradle expert. Could you leverage the power of Gradle to make the vaadinPrepareFrontend and vaadinBuildFrontend tasks always priority your application's classpath, thus ensuring that /flow/server/frontend/dependencies/default/package.json from the Vaadin Platform version you have as a dependency is considered? @steve-todorov?

There is a serious drawback to this approach. Gradle configurations can be complex. I have seen situations where an addon uses a different Vaadin Platform version and ends up taking priority of the Vaadin Platform version used during build. But for some weird reason, when the app runs in dev mode, it uses the right /flow/server/frontend/dependencies/default/package.json. I have also seen this reversed.

I'm not sure I fully understand your idea. Yet this sounds unrelated to the original problem - could you please create a new ticket for this, so that we can discuss this in deeper detail?

mvysny avatar Nov 08 '23 12:11 mvysny

@oliveryasuna it's very important to always use the same version of the Vaadin Gradle Plugin and the Vaadin itself, otherwise the Plugin will use slightly different version of flow-server.jar on its classpath, which could cause random bugs to appear.

Also, it is possible for vaadinBuildFrontend to modify package.json once, after a new dependency has been added or Vaadin has been upgraded. If that happens, vaadinPrepareFrontend cache will be considered expired and it's normal to see it being run fully. If vaadinBuildFrontend constantly keeps overwriting/modifying the package.json, then that's definitely a bug which needs to be reported as a separate issue and resolved by Vaadin. You can try to reproduce such a bug by running full build vaadinBuildFrontend in production mode once on your dev machine, then commit package.json to git, then run the same build on other machines - the package.json should not be modified by consequent builds, and ultimately the vaadinPrepareFrontend task should be cached properly.

mvysny avatar Nov 08 '23 12:11 mvysny

If I understand this correctly: the change to the package.json file is causing vaadinPrepareFrontend task cache to become stale, to which Gradle correctly responds by re-running vaadinPrepareFrontend in full. This is a correct behavior. The excessive modification of package.json file by vaadinBuildFrontend sounds like a bug, but I believe that to be a bug different from this one, and a new bug report should be opened.

Therefore, if everyone agrees, I propose to close this bug as "works as intended". @mshabarov @oliveryasuna @knoobie

mvysny avatar Nov 08 '23 12:11 mvysny

Thanks @mvysny @oliveryasuna @knoobie for you analysis! I like the idea to open a new one, but before I would like to understand what changes in package.json are made by vaadinBuildFrontend for @steve-todorov .

mshabarov avatar Nov 08 '23 12:11 mshabarov

Hey guys,

Apologies for the late reply.

@mvysny If vaadinBuildFrontend constantly keeps overwriting/modifying the package.json, then that's definitely a bug which needs to be reported as a separate issue and resolved by Vaadin. You can try to reproduce such a bug by running full build vaadinBuildFrontend in production mode once on your dev machine, then commit package.json to git, then run the same build on other machines - the package.json should not be modified by consequent builds, and ultimately the vaadinPrepareFrontend task should be cached properly.

What you've described is exactly what's happening. The original problem is that the package.json is overwritten during the CI build and then for some reason it's never able to be cached. Even if the package.json and package-lock.json are synced and committed to VCS the vaadinPrepareFrontend task is still not cached. We also tried clearing the GH Actions cache and running a few runs on the main but the result remains the same.

I am not entirely sure where this is coming from exactly, but the closest I could get to reproducing the problem was by introducing out-of-sync in the package.json as shown in https://github.com/steve-todorov/base-starter-gradle/tree/issue-17941. This leads me to believe something additional is happening in the vaadinBuildFrontend task that might be changing the package.json or package-lock.json causing the problem.

I'll record the package.json and package-lock.json during the CI and diff them in hopes to get more clarity and report back.

steve-todorov avatar Nov 08 '23 13:11 steve-todorov

Hey @mshabarov, @mvysny, @tepi

After some additional investigation we found out we had a few @NpmPackage annotations in our code which were using slightly older versions of @vaadin scoped packages. This was causing the issues with the package.json and package-lock.json.

When we removed those annotations and versions are now in sync (and in VCS). This fixed our local cache, but it still isn't working in GH Actions.

Here's an example: https://github.com/steve-todorov/base-starter-gradle/actions/runs/6830298961

Build scan: https://scans.gradle.com/s/2ya5bzgbshtms/timeline?details=afz2wjxxnx4ms&hide-timeline

Maybe we need to have some additional files cached?

steve-todorov avatar Nov 10 '23 23:11 steve-todorov

Thanks for the additional info @steve-todorov. It is at least progress that the local cache works now. I checked out your branch and can confirm that. So issue could be something in GH Actions runner or config as well.

Do you know if there's any chance of getting a diff on what actually changes in package.json during the GH action run?

Also, could you try out running clean vaadinClean build instead of clean build in GHA and see if that makes any difference?

tepi avatar Nov 13 '23 08:11 tepi

In our project nothing really changes in the package.json nor package-lock.json - the md5 checksums stay the same so they should be binary identical.

I've updated my base-starter-gradle branch to include your suggestion to have vaadinClean in the mix and record the package.json and package-lock.json. Unfortunately this only removes the package-lock.json:

https://github.com/steve-todorov/base-starter-gradle/actions/runs/6849622432

Build scan (continues to show Overlapping outputs: Gradle does not know how file 'package.json' was created): https://scans.gradle.com/s/vj756oofsmo7s/timeline?details=afz2wjxxnx4ms&hide-timeline

P.S. We did remove all GH actions cache across all branches to be sure it's not cache corruption.

steve-todorov avatar Nov 13 '23 12:11 steve-todorov

This is really strange. Only the vaadinPrepareFrontend task declares its input/output parameters (the taskOutputProperties.packageJson output parameters); vaadinBuildFrontend hasn't been converted to take advantage of the Gradle caching mechanism yet, and therefore it doesn't declare its inputs/outputs. Despite that, Gradle is able to detect the "overlapping outputs": that something else than vaadinPrepareFrontend modified the package.json file.

My hunch/wild guess is as following:

  1. vaadinBuildFrontend overwrites package.json with the same contents, which modifies the last-access timestamp.
  2. This last-access timestamp is taken into consideration by Gradle; for some reason only in the GHA environment.
  3. Gradle detects a 'change' in package.json file, disables the build caching and warns about "overlapping outputs".

If we could somehow disable vaadinBuildFrontend touching/overwriting package.json completely, then we could put this hypothesis into test. @tepi would you happen to know whether it's possible to do this?

mvysny avatar Nov 13 '23 12:11 mvysny

Can confirm point 1 from @mvysny above; the timestamp does update even though the contents do not change. This might be the underlying reason for this behavior. Unfortunately could not find any way to disable this update - rather I consider it a bug that needs to be fixed, since it makes no sense to write the file if the contents are identical.

tepi avatar Nov 13 '23 13:11 tepi

Sounds good! In such case a new bug ticket should be opened in this repository, for the Gradle plugin. @steve-todorov could you take care of it please?

mvysny avatar Nov 13 '23 14:11 mvysny

@mvysny, @tepi I'm having a hard time understanding how points 1,2,3 would not work only in GH Actions though? It seems to be working fine when I'm doing local builds. If this were true I would expect the same behavior to appear locally?

steve-todorov avatar Nov 13 '23 14:11 steve-todorov

@steve-todorov maybe your local FS has disabled the tracking of last-update timestamp while it's enabled in the GHA environment... Or maybe the filesystem is different, and Gradle FS monitoring hooks are only triggered on GHA... It's a bit far-fetched I agree, but that's the only hypothesis I have.

mvysny avatar Nov 13 '23 14:11 mvysny

@mvysny I'm using Ubuntu 23.04 with ext4 which is similar to what the default GH Actions runner are using with docker. I did a small experiment to validate your theory. Locally the package.json and package-lock.json do not change as can be seen below:

Local build, Cold cache (meaning no cache)
    steve@steve:base-starter-gradle$ stat package.json 
      File: package.json
      Size: 9926      	Blocks: 24         IO Block: 4096   regular file
    Device: 253,0	Inode: 831172      Links: 1
    Access: (0664/-rw-rw-r--)  Uid: ( 1000/   steve)   Gid: ( 1000/   steve)
    Access: 2023-11-13 16:39:47.348239724 +0200
    Modify: 2023-11-06 17:49:47.152887023 +0200
    Change: 2023-11-06 17:49:47.152887023 +0200
     Birth: 2023-11-01 21:42:01.779461450 +0200
    
    steve@steve:base-starter-gradle$ stat package-lock.json 
      File: package-lock.json
      Size: 576103    	Blocks: 1128       IO Block: 4096   regular file
    Device: 253,0	Inode: 794593      Links: 1
    Access: (0664/-rw-rw-r--)  Uid: ( 1000/   steve)   Gid: ( 1000/   steve)
    Access: 2023-11-13 16:39:47.430239538 +0200
    Modify: 2023-11-06 17:49:51.054877668 +0200
    Change: 2023-11-06 17:49:51.054877668 +0200
     Birth: 2023-11-06 17:48:47.668030020 +0200
Local build, Warm cache
steve@steve:base-starter-gradle$ stat package.json
  File: package.json
  Size: 9926      	Blocks: 24         IO Block: 4096   regular file
Device: 253,0	Inode: 831172      Links: 1
Access: (0664/-rw-rw-r--)  Uid: ( 1000/   steve)   Gid: ( 1000/   steve)
Access: 2023-11-13 16:39:47.348239724 +0200
Modify: 2023-11-06 17:49:47.152887023 +0200
Change: 2023-11-06 17:49:47.152887023 +0200
 Birth: 2023-11-01 21:42:01.779461450 +0200

steve@steve:base-starter-gradle$ stat package-lock.json
  File: package-lock.json
  Size: 576103    	Blocks: 1128       IO Block: 4096   regular file
Device: 253,0	Inode: 794593      Links: 1
Access: (0664/-rw-rw-r--)  Uid: ( 1000/   steve)   Gid: ( 1000/   steve)
Access: 2023-11-13 16:39:47.430239538 +0200
Modify: 2023-11-06 17:49:51.054877668 +0200
Change: 2023-11-06 17:49:51.054877668 +0200
 Birth: 2023-11-06 17:48:47.668030020 +0200

However, the build in GH does indeed modifies the package-lock.json which might validate your theory:

https://github.com/steve-todorov/base-starter-gradle/actions/runs/6852381698

steve-todorov avatar Nov 13 '23 15:11 steve-todorov

@mvysny I am a bit confused. Are you waiting on @steve-todorov to open a new issue? Why?

oliveryasuna avatar Nov 15 '23 17:11 oliveryasuna

I'm looking into the issue currently, no need to create a new issue, I will create one when needed. Right now I can't reliably reproduce the timestamp update of package.json file. With identical setup it sometimes updates and sometimes does not update for my local build. Will keep you posted when I figure out what is going on.

tepi avatar Nov 15 '23 18:11 tepi

I'm looking into the issue currently, no need to create a new issue, I will create one when needed. Right now I can't reliably reproduce the timestamp update of package.json file. With identical setup it sometimes updates and sometimes does not update for my local build. Will keep you posted when I figure out what is going on.

Did you try @steve-todorov's project? I think the code just needs to compare the hash of the current and new, and only write if they are different.

oliveryasuna avatar Nov 15 '23 18:11 oliveryasuna