Fix addMainModuleInfo erroneously considered UP-TO-DATE#17
Open
Marcono1234 wants to merge 1 commit intomoditect:masterfrom
Open
Fix addMainModuleInfo erroneously considered UP-TO-DATE#17Marcono1234 wants to merge 1 commit intomoditect:masterfrom
addMainModuleInfo erroneously considered UP-TO-DATE#17Marcono1234 wants to merge 1 commit intomoditect:masterfrom
Conversation
The addMainModuleInfo task replaces the original JAR with a modifies JAR. However, previously it did not properly specify the task inputs, therefore it was erroneously considered UP-TO-DATE even if the JAR changed. Note that now unfortunately the `jar` task is always run and never considered UP-TO-DATE because addMainModuleInfo modified the original JAR.
Marcono1234
commented
Aug 23, 2022
| @Input | ||
| abstract String getShortName() | ||
| @InputFile | ||
| abstract File getInputJar() |
Contributor
Author
There was a problem hiding this comment.
Have not specified @PathSensitive(PathSensitivity.NONE) here because for AddMainModuleInfoTask the path matters since the file is overwritten.
I hope this does not cause issues for ModuleConfiguration which overrides this method and adds its own annotations.
| primaryArtifact.file | ||
| } | ||
|
|
||
| @Input |
Contributor
Author
There was a problem hiding this comment.
I assume this is redundant because the overridden method already defines this (but have not tested this!).
Comment on lines
+23
to
+25
| @Input | ||
| String group | ||
| @Input |
Contributor
Author
There was a problem hiding this comment.
I think it makes sense to define these as inputs, but I have not tested this.
| abstract File getInputJar() | ||
| @Input | ||
| abstract String getVersion() | ||
| @Nested |
Contributor
Author
There was a problem hiding this comment.
Have not tested this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
addMainModuleInfotask replaces the original JAR with a modifies JAR. However, previously it did not properly specify the task inputs, therefore it was erroneously considered UP-TO-DATE even if the JAR changed.Note that now unfortunately the
jartask is always run and never considered UP-TO-DATE becauseaddMainModuleInfomodified the original JAR. Not sure if that can be solved in a cleaner way. Though interestingly manually specifying the JAR in thebuild.gradle.ktswithinputs.file(mainModule.get().inputJar)does not cause thejartask to be re-run (though maybe that also depends on the Gradle version being used).I am not that familiar with Gradle so any feedback is appreciated!
Note that there are some parts which I have not tested; if you want I can omit those and only add the annotations necessary for
getInputJar().