nus-oss / AB3-J17

:ab::three: Address Book sample application (Level 3)
https://se-education.org/addressbook-level3
MIT License
0 stars 16 forks source link

Update build.gradle for Mac Silicon Compatibility #20

Closed hjungwoo01 closed 1 month ago

hjungwoo01 commented 2 months ago

Summary

This pull request introduces changes to the build.gradle file to ensure compatibility with JavaFX on Mac Silicon (ARM architecture). These changes are necessary to pass all tests and ensure the application runs correctly using the IntelliJ run button on Mac Silicon devices.

Changes Made

  1. Environment Variable for JavaFX Path:

    • Added a line to retrieve the JavaFX path from the environment variable PATH_TO_FX.
  2. Test Configuration:

    • Updated the test block in build.gradle to include the JavaFX module path and required modules.
      
      def pathToFx = System.getenv('PATH_TO_FX')

    test { useJUnitPlatform() finalizedBy jacocoTestReport jvmArgs = [ '--module-path', "${pathToFx}", '--add-modules', 'javafx.controls,javafx.fxml' ] }

  3. Application Configuration:

    • Updated the application block to set default JVM arguments for running the application with JavaFX modules.
      application {
      applicationDefaultJvmArgs = [
             '--module-path', "${pathToFx}",
             '--add-modules', 'javafx.controls,javafx.fxml'
      ]
      }

Justification

Request for Feedback

Instructions for Reviewers

  1. Set Up Alias in ~/.zshrc:

    • Ensure the JavaFX path is correctly set in your environment variables.
      export PATH_TO_FX=~/path/to/javafx-sdk-17.0.7/lib
    • Apply changes with:
      source ~/.zshrc
  2. Run the Build:

    • Verify if the build and tests pass when running the build.gradle file using the IntelliJ run button.
  3. Provide Feedback:

    • Share your experiences and any issues encountered.
    • Confirm if this change resolves the problem and if it should be merged.

Please review the changes and provide your feedback on whether these updates resolve the issue and improve compatibility on Mac Silicon.

damithc commented 2 months ago

@hjungwoo01 confirmed you are using the prescribed Azul JDK17+JavaFX version and used SDKman to set the default Java version to the Azul version?

Mac Silicon users, for your comments please ...

@baskargopinath @gongg21, @hjungwoo01, @jyue487, @itstrueitstrueitsrealitsreal, @brein62, @ljy0422, @ryanozx, @Carlintyj

hjungwoo01 commented 2 months ago

Yes I am using the prescribed Azul JDK17+JavaFX version and used SDKman to set the default Java version to the Azul version.

Carlintyj commented 2 months ago

I am on MacOS (Silicon - M1) with Azul zulu-17 version 17.0.11 (JDK FX). I am running on IntelliJ IDEA 2022.3.1 (Community Edition)

I am unable to replicate the errors in the first place. Without any changes, I could run the build.gradle file using the IntelliJ run button with pass all tests.

Screenshot 2024-07-17 at 4 49 11 AM

Running the test on gradle from the side bar also provides me all tests passed.

Screenshot 2024-07-17 at 4 51 44 AM

And lastly, using CLI ./gradlew clean build and ./gradlew test both resulted in all tests passed as well.

Screenshot 2024-07-17 at 4 52 18 AM
baskargopinath commented 2 months ago

im able to run build.gradle without any changes as well, able to do run gradlew clean build as well, im using the prescribed Azul JDK17+JavaFX

m2 macbook pro, is there any way i can replicate this error?

hjungwoo01 commented 2 months ago

Upon testing again, I found that the issue no longer occurs, and everything works as expected without these changes. I’m not sure why the issue doesn’t occur anymore, but there is no need to modify the build.gradle file as previously suggested.

damithc commented 2 months ago

Upon testing again, I found that the issue no longer occurs, and everything works as expected without these changes. I’m not sure why the issue doesn’t occur anymore, but there is no need to modify the build.gradle file as previously suggested

Nice. That was the outcome I was hoping for :-) Thanks @hjungwoo01 @Carlintyj @baskargopinath

itstrueitstrueitsrealitsreal commented 2 months ago

Similarly to @baskargopinath @Carlintyj , I am unable to replicate the error, and I have also run ./gradlew clean build without any issues.

I am using an M1 Pro chip running MacOS Sonoma 14.2.1.

Screenshot 2024-07-17 at 1 33 21 PM
damithc commented 1 month ago

As it looks like this fix is not needed, let's close this PR.