-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
fix(#758): add path.join to build os-dependent java binary path #759
base: master
Are you sure you want to change the base?
Conversation
.filter(isString) | ||
.join(' '); | ||
|
||
return this.isWin() ? `"${command}"` : command; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if command
contains "
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the generated commands already have "
inside and only run successfully with these quotes in the windows build and without it will not.
concurrently
expects it in this case if there are spaces in the paths, which is very common in windows. therefore i would generalize this.
i was also been able to reproduce the behavior with concurrently
alone.
i can send later some examples.
the most important part of the windows build plan already looks promising. there is only one error in the last step that i can't get locally.
Error: spawn ENAMETOOLONG
sounds really special to me. i think the command is too long. i try to figure out why this is. i mean foo
is not longer then bar
😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i got it. i removed the JAVA_HOME
env wrong. for now all tests are passed.
can you tell me how i can resolve the conflicts? why does this not behave like the first PR, where i was able to resolve the conflicts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you will need to cherry-pick 3e949d8, which was reverted by me last week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the generated commands already have " inside and only run successfully with these quotes in the windows build and without it will not.
did you test with something like --additional-properties option="test-double-quote"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i tried to cherry-pick but after resolving the conflicts there was nothing to commit. i commited with git commit --allow-empty
but it will not resolve the conflicts.
did you test with something like --additional-properties option="test-double-quote" ?
i try to implement something like this to the test.
on my local env (windows - git bash) the following succeeded:
$ npm run oa generate -- -g ruby -i https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o ./out/ruby --additional-properties option="test-double-quote"
> examples@1.0.0 oa
> openapi-generator-cli generate -g ruby -i https://raw.githubusercontent.com/OpenAPITools/openapi-generator/master/modules/openapi-generator/src/test/resources/3_0/petstore.yaml -o ./out/ruby --additional-properties option=test-double-quote
[main] INFO o.o.codegen.DefaultGenerator - Generating with dryRun=false
[main] INFO o.o.codegen.DefaultGenerator - OpenAPI Generator: ruby (client)
[main] INFO o.o.codegen.DefaultGenerator - Generator 'ruby' is considered stable.
...
$ echo $?
0
…penAPITools#761) This reverts commit 3e949d8.
Feature/add windows to build
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…Tools#748) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…APITools#773)" (OpenAPITools#777) This reverts commit f6dcb95.
…enAPITools#762) * chore(deps): install `https-proxy-agent` as dependency * fix(app): configure http agent with proxy url if present in environment Fixes #OpenAPITools#714
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…#789) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* update versions mentioned in README * update --------- Co-authored-by: Martin Gebert <2026226+martin-mfg@users.noreply.github.com>
Co-authored-by: William Cheng <wing328hk@gmail.com>
any idea whether this pr can be continued? |
yeah, this would be nice! i dont know if i can do anything. is there still a task open? github says there are conflicts, but i don't see any and can't really resolve them. that's why the actions don't run for the PR |
* support JAVA_HOME environment variable fixes OpenAPITools#661 * test(generator-cmd): add consideration of the JAVA_HOME variable * docs: add instructions for the use of JAVA_HOME * fix: add quotes to accept spaces in the JAVA_HOME path * fix: add quotation marks only for windows paths --------- Co-authored-by: Nils Braune <78608305+adnbrownie@users.noreply.github.com>
fixes #758