-
Notifications
You must be signed in to change notification settings - Fork 17
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
Simplify plugin configuration #179
base: master
Are you sure you want to change the base?
Conversation
I don't think the build failure on GitHub Actions is caused by my changes. The error actually looks quite familiar to me. I think I once fixed the same issue at JetBrains/gradle-changelog-plugin#170. 😅 |
FYI, I also asked at gradle/gradle#9021 (comment) whether it is possible to tell Gradle to clean the output directory for you. Unfortunately, it doesn't seem to be possible right now. |
Introduce GenerateLexerTask.targetRootOutputDir as a replacement of targetOutputDir. When using the new property, the task creates the java sources in the directory matching the package name. The package name is detected using some heuristics. The commit also introduces GenerateLexerTask.packageName to manually specify the package name.
The two properties of GenerateParserTask are only used when purgeOldFiles is explicitly set to true, but they still had to be specified. As of this commit, if purgeOldFiles is set to true but none of the two properties are specified, the task will delete the whole content of targetRootOutputDir. I am not aware of any reason not to delete the whole directory of targetRootOutputDir unless the directory is overlapping with other files than the files generated by this task. Since overlapping task outputs are explicitly discouraged by Gradle, I deprecated the two properties.
Stale files should be avoided because they can cause build failures at subsequent tasks in incremental builds. For example removing a grammar rule from a *.bnf file may cause a compilation error when the stale AST note tries to call its corresponding method on the visitor. In combination with the build cache, a stale file may even be persisted in the cache, making :clean ineffective for fixing such issue.
The default should be sufficient in most use cases and makes the configuration of the plugin a bit easier. Note that the default is only available for the tasks :generateLexer and :generateParser. Custom tasks of type GenerateLexerTask or GenerateParserTask do not have a default for this property.
I rebased this pull request on the current master after #187 has been merged to fix the build. |
Pull Request Details
Sorry, but I got carried away over the weekend. This PR greatly simplifies the configuration of the plugin in a backward compatible manner (although I deprecated some properties). You may pick individual changes as you which, they are separated into semantically coherent commits.
The new minimal configuration looks like this:
This configuration is mostly equivalent to the following configuration, as it would have to be written manually before my changes:
Description
The new, much shorter configuration is possible due to the following individual changes:
GenerateLexerTask
to create a subdirectory for the package. This allows us to give the output directory toSourceDirectorySet.srcDir
directly. The new behavior is applied if the new propertytargetRootOutputDir
is used, which replacestargetOutputDir
. (362e9fc)GenerateParserTask.pathToParser
andpathToPsiRoot
optional. These properties are not needed in most cases, as they are only restrictingpurgeOldFiles
to specific sub-paths of the output directory. (05af194):clean
may not be able to fix such issues. For backwards compatibility, the new behavior is only applied when the previously mentioned properties (which were mandatory) are no-longer used. (87ac815)targetRootOutputDir
in both tasks. This simplifies the configuration and encourages people to use separate output directories for each task. Overlapping output directories are discouraged by Gradle and may cause issues with the build cache. (a8c60bd)lexerSource
andparserSource
in the plugin extension. These properties make it possible to specify the source files without having to configure the individual tasks directly. When this property is used, the corresponding tasks are also added as sources to the main source set. (86022b4)I also revised some existing tests in commit a204e86 and 5228085, but this doesn't have much impact on the other changes. I updated the changelog in commit 1a13907.
Related Issue
pathToParser
andpathToPsiRoot
optional #178Motivation and Context
This change makes using the plugin much easier.
How Has This Been Tested
I created various “unit” tests to test my changes. (I tripled the total amount of unit tests in this repository.) Furthermore, I tried out the new version with, and without simplifying the configuration on the nix-idea plugin.
Types of changes
Checklist