[2.1.0] Up-to-date checks are worse than before

[boris-petrov]

After updating to 2.1.0, pretty much every time the yarnSetup and yarn tasks are run. On 2.0.0 that wasn't the case. I see in the changelog that some work was done on that... but can you revisit it please? Can I help in some way?

[deepy]

This is my bad, I have a cold that just refuses to go away so I haven't yet looked at all the changes for 2.1.1, I'm going to do so today or tonight and hopefully we can release 2.1.1 at that point.

If you're comfortable with Gradle it'd be great help if you could look at the Yarn section of the repository and validate it!
Or contribute/validate tests for the Yarn part, since most of the feedback received is for Node/NPM

[boris-petrov]

Well, the inputs and outputs of the yarn install task seem fine to me... not sure why it's not working right on 2.1.0...

As for tests - couldn't you just copy this test from the npm tests to the yarn install tests? I see that a lot of the other tests are already copy-pasted (which is good).

P.S. Get well first, the release can wait a bit more! :)

[bsautel]

I improved the up-to-date checking in the 2.1.0 release. Before this version, the task configuration (command, environment, working directory, ...) was not taken into account in the up-to-date check and that was not correct. Indeed, if you change the npm ou yarn command for instance, we cannot assume the result will be the same (even if it could be the case).

When upgrading the plugin in the Java project where I use it, I realized that the tasks where very often executed again whereas they should be considered as up-to-date.
The --info option explains why a task is not up-to-date and I discovered it was because the environment changed. It appears that the way I detected the task configuration changes was too much sensitive. It considered the whole environment instead of only the overridden environment variables. As far as one environment variable changed since the last build, the task was considered as not up-to-date. I fixed this issue and the fix should be released soon by @deepy.

Since the 2.1.0 release, I also refined the up-to-date detection of the YarnTask command (as I did for NpmTask, NodeTask and NpxTask before). I also added integration tests for the YarnTask that ensure the up-do-date detection works as expected.

Could you confirm that your up-to-date detection issues are related to the environment (using the --info Gradle option)? And could you test that my fix correctly solves your issue?
Doing that is quite simple: you have to clone the repository and include it to your build by adding the following line to your settings.gradle file (change the path to this project if needed).

includeBuild "../gradle-node-plugin"

Gradle will compile the plugin code and use it instead of the plugin downloaded in the repository.

[boris-petrov]

@bsautel - I'm seeing the following as an example:

Task ':frontend:yarnSetup' is not up-to-date because:
  Value of input property 'runner.environment' has changed for task ':frontend:yarnSetup'

Task ':frontend:yarn' is not up-to-date because:
  Output property 'nodeModulesDir' file /home/boris/project/frontend/node_modules/.yarn-integrity has changed.
  Output property 'nodeModulesDir' file /home/boris/project/frontend/node_modules/bootstrap-sass/assets/stylesheets/_bootstrap.scss has changed.
  Output property 'nodeModulesDir' file /home/boris/project/frontend/node_modules/d3-color/README.md has changed.

As for the second point about trying master - do I have to keep id 'com.github.node-gradle.node' version '2.1.0' in plugins and apply plugin: 'com.github.node-gradle.node' in allprojects (I guess not)? How do I "enable" the plugin as commenting-out those lines causes Could not find method node() for arguments ... even with that includeBuild directive?

[bsautel]

This confirms the fact that the issue regarding the yarnSetup task comes from an environment change.
I assume that the fact the yarnSetup runs makes some changes in the node_modules directory and that explains why it is also considered as not up-to-date.

Yes, you do not have to change the plugin declaration. You just have to include the build in the settings.gradle file. I just come to test again and it works.

[boris-petrov]

@bsautel - yes, your changes seem to work. My yarn task is still being rerun but this is because a file is being changed in node_modules... not sure why yet. I'm wondering whether it's actually a good idea to make the output depend on the node_modules directory because of cases like mine...

[deepy]

There's a 2.1.1 available now wit the improvements.

But in regards to node_modules we probably need that as an output for more reasons, otherwise something else removing node_modules would break the build since the install task would consider itself up-to-date.

Though I'm kinda curious, is there something apart from the install task that changes your node_modules?

[boris-petrov]

Yes. We use Ember.js and one of the plugins for its build system modifies a file. I’ve opened an issue but I don’t think they’ll fox it. So some kind of option to ignore certain files/directories would be nice as otherwise in my case the ‘yarn’ task will rebuild every time.

[bsautel]

I'm not sure I understand what you mean @boris-petrov . In which task does this happen? The yarn task that is responsible of resolving dependencies in the node_modules directory or a task that executes the Ember.js build?

The Angular build uses some cache files in the node_modules/.cache directory (don't know whether this is a standard cache directory or not). When you run an Angular build, if you have the node_modules as an input of the task (and you should), it modifies the cache file. If you launch it again, it will not be up-to-date because of the cache change. The third time will be up-to-date because the cache file does not change during the second run.
To fix that you can simply exclude some files from the node_modules directory:

inputs.dir(fileTree("node_modules").exclude(".cache"))

But this works only for a custom task, not for the yarn task because the declaration is done by the plugin and as far as I know we cannot remove some input entries.

According to which task causes a node_modules change, you can fix the issue with that or not. If not, it would indeed be necessary to add some configuration options to the plugin to exclude some files.

[bsautel]

And thanks @deepy for the 2.1.1 release!

[boris-petrov]

Not the yarn task itself but another one which runs the build system is the one that modifies a file. This happens every time (that’s the way it is written), so the next time I run again the build system (which depends on the ‘yarn’ task) - the ‘yarn’ task is rerun. So that’s why an option for the ‘yarn’ task to ignore certain files would be useful...

[bsautel]

Ok, I see what you mean. The yarn task (of type YarnInstallTask) has the node_module directory declared as output (and not as input).

I don't know exactly how Gradle behaves when the output changes. I believe that Gradle does not run the task again when the content of an output directory changes but it does if the directory no longer exists. But I am not sure of that, so I did a little test:

1. I execute the yarn task.
2. I manually add a file in the node_modules directory to check how it behaves when a file is added.
3. I execute the yarn task again (up-to-date).
4. I manually update the file previously added at step 3 to check how it behaves when a file changes.
5. I execute the yarn task again (still up-to-date).

I don't know exactly why you encounter this problem but, unless I did an error, this does not seem to come from the yarn task. Did you change its inputs?

[boris-petrov]

@bsautel - thanks for the support!

I am seeing this when I run gradle with --info - Output property 'nodeModulesDir' file /home/boris/project/frontend/node_modules/bootstrap-sass/assets/stylesheets/_bootstrap.scss has changed.. So actually you should modify an already-existing file in order to reproduce the problem. This is very unfortunate... not sure how to fix this without changes in gradle-node-plugin. :(

[bsautel]

Indeed, I was not lucky in my attempt to reproduce it. The task is considered as out-of-date when a file inside the output directory is modified or deleted but not when a file is added.

It explains why I don't have the same issue with the Angular build that adds some files in the .cache directory. The reason is that it only adds some files.

Modifying the node_modules directory, or more precisely the contents of a package, during a build does not seem to be very clean. I think that would be nice whether they would stop doing that, as you requested them.

I don't see any way to fix this issue without modifying the plugin. To fix (we should rather tell workaround) this issue, you would need to be able to configure the task output. Probably we should use a file tree instead of a directory on which we can add some exclude rules and add a configuration closure that would configure it?

Note that you would have the same issue with the NpmInstallTask, it is not related to yarn.

And you reported this issue as a regression in the 2.1.0 version. The one regarding the environment is fixed in the 2.1.1 version which is now released. But can you confirm you already had this issue (regarding the modified files in the node_modules directory) with the 2.0.0 version?

[boris-petrov]

Probably we should use a file tree instead of a directory on which we can add some exclude rules and add a configuration closure that would configure it?

Yes, that sounds reasonable.

And you reported this issue as a regression in the 2.1.0 version. The one regarding the environment is fixed in the 2.1.1 version which is now released. But can you confirm you already had this issue (regarding the modified files in the node_modules directory) with the 2.0.0 version?

Yes, the issue in 2.1.0 is resolved and perhaps now it works the same way as on 2.0.0. I can't confirm exactly because it never was consistent... but it definitely did rerun the task when I've made no changes to package.json/yarn.lock before, and it still does now. It's not the biggest of deals but if there is something you can do (like making the output depend on something configurable), it would be nice.

Thanks a bunch!

[bsautel]

Thanks @boris-petrov for your explanations.

@deepy, what do you think about this solution to fix the issue?

[deepy]

@bsautel the solution being adding a way to provide an exclude for node_modules?

It sounds good to me, especially if the convention for many tools is treating node_modules like the way we treat the build folder in gradle

[bsautel]

Yes, I was speaking about adding a way to exclude some paths from the default node_modules task output.

I don't think that it is a convention, I would rather say that this is an exception. But in this case, we don't have any solution to do that correctly.

I will fix it soon for the NpmInstall and YarnInstall task because both of them can lead to this situation.

[bsautel]

@boris-petrov, I pushed a commit that should help you fix your issue.
You are now able to write that:

yarn {
    nodeModulesOutputFilter = { it.exclude("bootstrap-sass/assets/stylesheets/_bootstrap.scss") }
}

We can write exactly the same thing for the NpmInstallTask.

Is it good for you?

@deepy, are you okay with that? Is the naming satisfying?

[boris-petrov]

@bsautel - looks perfect, thank you for the great support!

[deepy]

@bsautel it's a mouthful but I can't think of a better name :-)