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

support JAVA_HOME environment variable #756

Merged
merged 6 commits into from
Mar 3, 2024

Conversation

mok-liee
Copy link
Contributor

fixes #661

@mok-liee
Copy link
Contributor Author

@wing328
this is a next-try of #677

unfortunately it was not possible to create a PR from the same branch, so i had to fork again. the props of course go to @adnbrownie

@mok-liee
Copy link
Contributor Author

is it possible to run a test locally? i am working on windows with the git-bash.
if i run the unit-test it fails on all because of wrong interpretation of path's slash direction.

image

i want to get rid of this errors to see exactly whether adjustments to the unit-test are necessary.
i suspect that the modification brings this with it.

@mok-liee
Copy link
Contributor Author

it looks to me like i can't do anything about the build problem, can i? a manual generate with the versions that throw errors in the log works locally for me.

@wing328
Copy link
Member

wing328 commented Feb 29, 2024

it looks to me like i can't do anything about the build problem, can i?

one way is to repeat the issue in your fork.

@mok-liee
Copy link
Contributor Author

I am completly new to Github actions. I will do my best.

@mok-liee
Copy link
Contributor Author

sorry for messing up this PR but i dont know how to test this build locally. i thought if i do this in my fork this PR is free of my debuggings.

i see that the agent is using /usr/lib/jvm/temurin-11-jdk-amd64 from $JAVA_HOME but the logs are printing out that the executable is not found. i try to figure out more.

@mok-liee
Copy link
Contributor Author

this build should be successful

@wing328
Copy link
Member

wing328 commented Mar 1, 2024

@mok-liee thanks. all tests passed.

one more minor request:

can you please update the workflow file to run the build on windows-latest as well: https://github.com/OpenAPITools/openapi-generator-cli/blob/master/.github/workflows/build.yml#L56 so as to ensure changes work in Windows moving forward?

ref: https://stackoverflow.com/a/64036382/677735

@mok-liee
Copy link
Contributor Author

mok-liee commented Mar 2, 2024

i will implement this. unfortunately, the weekend will be difficult. i will try next week!

@wing328
Copy link
Member

wing328 commented Mar 3, 2024

I tried but looks like it will be very difficult as the tests won't work on both windows and linux: #757

e.g.

Object {
        "after": Array [
    -     "Downloaded 4.2.0 to custom storage location /c/w/d/custom/dir",
    +     "Downloaded 4.2.0 to custom storage location D:\\c\\w\\d\\custom\\dir",
        ],
        "before": Array [
          "Download 4.2.0 ...",
        ],
      }

      4[43](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:44) |               await compile();
      [44](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:45)4 |               await fixture.download('4.2.0');
    > 4[45](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:46) |               expect(logMessages).toEqual({
          |                                   ^
      4[46](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:47) |                 before: [chalk.yellow(`Download 4.2.0 ...`)],
      4[47](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:48) |                 after: [
      4[48](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:49) |                   chalk.green(

      at src/app/services/version-manager.service.spec.ts:445:35
      at fulfilled (../../node_modules/tslib/tslib.js:166:[62](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:63))

  ● VersionManagerService › API › download() › the server responds a file › logging › there is a custom storage location › returns /custom/dir for /custom/dir

    expect(received).toEqual(expected) // deep equality

    - Expected  - 1
    + Received  + 1

      Object {
        "after": Array [
    -     "Downloaded 4.2.0 to custom storage location /custom/dir",
    +     "Downloaded 4.2.0 to custom storage location D:\\custom\\dir",
        ],
        "before": Array [
          "Download 4.2.0 ...",
        ],
      }

      443 |               await compile();
      444 |               await fixture.download('4.2.0');
    > 445 |               expect(logMessages).toEqual({
          |                                   ^
      446 |                 before: [chalk.yellow(`Download 4.2.0 ...`)],
      447 |                 after: [
      448 |                   chalk.green(

      at src/app/services/version-manager.service.spec.ts:445:35
      at fulfilled (../../node_modules/tslib/tslib.js:1[66](https://github.com/OpenAPITools/openapi-generator-cli/actions/runs/8128392726/job/22214232230?pr=757#step:6:67):62)

@wing328
Copy link
Member

wing328 commented Mar 3, 2024

we will get this merged and find another way to test it on windows later with another PR instead

@wing328 wing328 merged commit 3e949d8 into OpenAPITools:master Mar 3, 2024
3 checks passed
Copy link

github-actions bot commented Mar 3, 2024

🎉 This PR is included in version 2.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

wing328 added a commit that referenced this pull request Mar 6, 2024
wing328 added a commit that referenced this pull request Mar 6, 2024
mok-liee added a commit to mok-liee/openapi-generator-cli that referenced this pull request Mar 12, 2024
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] cli doesn't support JAVA_HOME, requires the java executable to be on the PATH
3 participants