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

Allow NodeTask to be extended and exec to be overridden. #307

Open
pwagland opened this issue Mar 8, 2024 · 2 comments
Open

Allow NodeTask to be extended and exec to be overridden. #307

pwagland opened this issue Mar 8, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@pwagland
Copy link

pwagland commented Mar 8, 2024

In https://github.com/node-gradle/gradle-node-plugin/blob/main/src/main/kotlin/com/github/gradle/node/task/NodeTask.kt#L90-L99 exec is not defined as an open method, which means that tasks that extend this cannot add functionality during the execution.

In our specific case, we are using node to run SCSS, and so want to clean the target directory before calling node to populate it. I can add a second TaskAction, but there is no guarantee that this will run first, which means that sometimes I delete everything after it has been generated. Which is also not the point ;-)

I can, of course, just delegate everything, that involves a lot of boilerplate, since the there quite a few properties that should be delegated across..

So, would it be possible to make exec an open method?

@deepy
Copy link
Member

deepy commented Mar 9, 2024

The original reason exec() is not open is that our intention was that for these tasks you shouldn't be extending them
If you needed to extend them, you'd pick whichever Runner was most suitable and then wire that up yourself

But things changed, designs changed, and setting the wiring up might be the harder part of this

We could consider changing this, but could you create an example project showing the issue?

In the short-term I think either depending on a Delete task or making use of doFirst() might work

@deepy deepy added the enhancement New feature or request label Mar 9, 2024
@pwagland
Copy link
Author

Hi @deepy,

Thanks for your comment, while I can't extract out everything, I can give a taste of what we are trying to do:

We have an ScssTask:

@CacheableTask
abstract class ScssTask extends NodeTask {
  @Input
  public List<String> getArguments() {
    var result = new ArrayList()
    result.add("--source-map")
    result.add("--style=compressed")
    argsList.each(v -> result.add(v instanceof Property ? v.get().toString() : v.toString()))
    result.add(inDir.getOrElse("").toString() + ":" + outDir.getOrElse("").toString())
    (project.configurations.uiScss + project.configurations.uiScssIn + project.configurations.uiScssIn.artifacts.files).each {
      var dir = it
      result.add("--load-path=${dir}")
    }
    result.each { logger.debug("--DEBUG: ${getName()} ${getClass()} argument list : " + it) }
    return result
  }

  @InputFiles
  @PathSensitive(PathSensitivity.RELATIVE)
  public Collection<File> getInputFiles() {
    // This code is calculating and returning all empty directories from sourceSet excluding src/
    // all subdirectories of src containing *.scss files
    HashSet<File> inputFiles = new HashSet<>()
    inputFiles = Util.findDirsIncluding(project, project.configurations.uiScssIn, ['**/*.scss'])
    inputFiles.addAll(project.configurations.uiScss.files)
    inputFiles.addAll(project.configurations.uiScssIn.artifacts.files)
    return inputFiles
  }
}

Now what we have discovered is that if the input files have changed, sometimes we need to remove the entire output, as otherwise we might end up with old, and now incorrect, files in our cached build results, so this is where we want to

We cannot use doFirst(), as it isn't guaranteed to run before our action. Related reading:

The workaround that we have adopted is to add the clean task as a dependency, since it is cached, rebuilding doesn't take long, but it seems wasteful to always delete, and then unpack the cache. If we could override exec then we could just add:

  @Override
  @TaskAction
  public void exec() {
    clean();
    super.exec();
  }

to ensure that the output was always cleaned, and would therefore always be correct.

@pwagland pwagland changed the title All NodeTask to be extended and exec to be overridden. Allow NodeTask to be extended and exec to be overridden. Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants