-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[bug][maven] Fix custom windows classpaths in maven plugin #7587
Conversation
… paths Previous checks would cause logic in Windows to return early, for built-in templates only. This reorganizes and simplifies the ordering behavior.
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
This follows similar approach used in PMD and other plugins managed by maven. Unit tests simply verify we can load configuration as expected into the Mojo. Integration tests execute actual sample projects bound to the current build's Maven plugin. This uses maven-invoker-plugin, which also allows for specifying the maven options in invoker.properties to execute the test. It also provides a verification framework using groovy files with the required naming convention of "verify.groovy". This allows us to quickly and easily check that certain files are outputted by generation, and we may also spotcheck file contents. templateResourcePath option is skipped on windows. I've tested back to version 3.3.3 and this doesn't seem to have worked consistently with how the property works on non-Windows.
Previous commits on this branch failed, and it took me a while to figure out.
maven-invoker-plugin is resolving with groovy 2.4.8 in CI, but resolves locally for me to 3.0.5. I'm able to build in Java 8 and 11 locally because of the supported groovy version. I'm not sure why there's a difference in resolution between my local machine and CI, but I've verified that explicitly setting the groovy dependency on the plugin will affect the groovy version evaluating the scripts. This should allow CI to pass. |
* master: (66 commits) [Typescript][Angular] Fix generated README when using apiModulePrefix (#7725) remove outdated scala files (#7723) [FEAT][TYPESCRIPT-ANGULAR] Add configurationPrefix option to allow generating unique configuration token (#7731) [bug] Fix FILES sort and path provider issue (#7729) better csharp tests (#7727) [go] Improve examples generation (#7576) Fixes #7635: typescript-inversify generator wrongly handles array type parameters (#7636) [Java] Fix import mapping for arrays with reference items of type string (#7182) [Java][Native] Support oneOf/anyOf schemas (#7263) [BUG][Ada] Incorrect client Ada code generated (#7719) add cake, sbt integration (#7713) Use 3.0 spec in documentations, update docs (#7710) remove github.com/antihax/optional from go.sum (#7692) Update junit to newer version (4.13.1) (#7690) [Fix/Dart2] Resolve an exception with status 204 and no body. (#7647) [typescript-angular] pass array as a single JSON string to url query when queryParamObjectFormat=json (fix #7620) (#7649) Add back HttpSigningConfiguration.cs remove HTTPSigningConfiguration.cs add AnyType support to Swift generators (#7644) fix warning, remove trailing spaces (#7659) ...
I can't get some of our CI to work with the Loading from resource is potentially still an issue in Windows, but I couldn't find any release version which supported Maven's Once this is merged, I'll add the maven integration tests to the check supported versions script which runs only on master. |
And |
It looks good now and works for Windows. I confirmed it with the 5.0.0-beta3 release. |
Was introduced with OpenAPITools#7587 could be removed with OpenAPITools#10544
…18576) * Added support for <inputSpec/> arguments of JAR URLs. E.g., jar:jar-specific-uri!/spec.yml. * Resolve and search COMPILE dependencies for inputSpec resource. * Added test cases for openapi-generator-maven-plugin:generate input specifications: * URLs of the form jar:jar-specific-uri!/spec.yaml * Resources on the compilation classpath in addition to the existing FILE test case. * Check for inputSpecFile existence else it is a remote URL && url is not empty * replaced deprecated usage * use Unix separators when on win-os * example with jar inputSpec * Comment not required anymore Was introduced with #7587 could be removed with #10544 * referenced same maven version these artifacts are referenced by same ${project.version} in https://github.com/apache/maven/blob/master/pom.xml * updating maven dependencies to 3.9.6 --------- Co-authored-by: Allen D. Ball <ball@hcf.dev>
Fixes #4208
Thanks to @DenisKnoepfle for the discussion around this bug to help reproduce the underlying issue.
templateDirectory
propertytemplateDir
property of a codegen configThe Maven plugin follows a different flow for template directory versus resource loaded templates. I'd tried reproducing the reported bug, but it seems I was only testing
templateDirectory
under git bash so the problems didn't arise. I've testedtemplateDir
andtemplateDirectory
under both Mac and Windows, both locally and in a private git repo using workflows for cross-platform checks. This should be good to go, but I'd like to have another set of eyes on it before merge.cc @OpenAPITools/generator-core-team
PR checklist
./bin/generate-samples.sh
to update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example./bin/generate-samples.sh bin/configs/java*
. For Windows users, please run the script in Git BASH.master