Distributing AddressBook-Level4 in a JDK11+ world #951
Comments
pyokagan
added
c.ResearchProject
|
@pyokagan Is it ok for the implementation to be only available for java 11(10 is not tested yet) but not work for java 9 (9.0.2 is not ok) anymore? Because Gradle has a plugin of javafx but require higher version of java. |
Yes, it's expected that we'll only use this project with JDK/JRE 11 and above. (i.e. we'll bump |
|
@pyokagan You have mentioned that a report of my research is needed? I currently find a way to solve the problem. The trick is to use another Main class to be the entry of our application. The reason can be referred from |
Yes. The report is to ensure that you understand the problem space. It doesn't need to be very long, though. Something like fae8700 and 8e4d306 combined , which gives a technical overview of the problem and goes though the possible solutions one by one, along with analysis.
Does the same JAR work on all platforms we support (Windows, Mac, Linux)?
Yes, please do. |
I have tested on Windows and it works. I am not sure about Mac and Linux because I do not have such a system. But I will try to figure out a way to test the jar. The file can be get from below: |
Yes, please do. I'm much more concerned about what happens when we run a JAR built on one system (e.g. Windows) on another (e.g. macOS). From what I understand JavaFX dependency artifacts are platform-dependent. If you see here you can see that there is a "linux" version, a "win" version and a "mac" version. |
|
Update my progress:
So, I will just leave this way here and try it later. For the testing of jar file of mac and linux, I have borrowed a mac and a ubuntu on Friday, the result can be get by then. |
OK. Not sure if you have seen the work done in #908 but it might help you.
OK. To speed up development and testing, since support for Linux can be easily tested in a VM, you may wish to prioritize getting a JAR that is compiled on Windows to work on Linux first. Once that works, it would be more likely that a JAR that is compiled on Windows would also work on a Mac as well. |
|
So we are only allowed to use jdk11 right? So letting them download another jar file is not OK right? |
Do you mean asking the user to download JavaFX separately? If so, that's undesirable and should only be done as a last resort. |
|
Breakthrough update: |
|
The jar generated in Linux is able to run in Win, too |
This problem is fixed by adding OS specified order of dependency |
The problem is partly solve by this |
|
Progress update: The checkStyle failure in JDK11 are fixed. |
|
Progress update: Gradle build is able to run correctly locally, however, the headless task seems to fail. Will looking into it. |
|
Headless task were fixed refering to: |
|
@pyokagan All the gradle task are able to run locally right now. However, the coverall task fails on Travis CI (Other tasks are OK either). I will keep looking into the reason of it. |
|
A reference may solve the problem: |
Great effort! I won't be able to take a look until Thursday unfortunately. |
I believe I am able to finish all the tasks by Thursday. |
It needs to be polished enough to be merged into master though :-) When you are done, make sure you understand the full technical details of your approach. We'll be doing a full technical evaluation. |
|
It seems that I am not able to configure netlify so I will leave it here. |
???? It's probably not a good idea to leave Netlify broken... |
Currently, Assert implements our custom assertThrows using try/catch. This implementation was done when we used JUnit 4, and had no proper support for assertThrows. As we migrate to JUnit 5[1], we can make use of JUnit 5's Assertions.assertThrows. However, our current assertThrows supports checking the correctness of error's message, whcih JUnit 5 does not support. Let's migrate Assert.java to use JUnit 5's assertThrows. In addition, let's create a custom assertThrowsWithMessage to support checking the correctness of error messages. [1] se-edu#951
Currently, Assert implements our custom assertThrows using try/catch. This implementation was done when we used JUnit 4, and had no proper support for assertThrows. As we migrate to JUnit 5[1], we can make use of JUnit 5's Assertions.assertThrows. However, our current assertThrows supports checking the correctness of error's message, whcih JUnit 5 does not support. Let's migrate Assert.java to use JUnit 5's assertThrows. In addition, let's create a custom assertThrowsWithMessage to support checking the correctness of error messages. The old assertThrows is kept for backward compatability, as many existing classes depend on it. It has been marked as deprecated, and will be removed in a future commit after migrating all tests. [1] se-edu#951
I am not so sure about how the netlify build works for our repo. It seems that there is no local config file under ab4. So, I believe this may due to the build script are on the netlify. However, I am not able to configure the netlify online, so that is the problem. Can you also help looking into it？ |
|
@fzdy1914 Here are the current settings: AB4 documentation has instructions on how to set up Netlify. |
|
@damithc Thanks for your reply, I will look into it. |
|
@pyokagan I will rebase my pr and add some commit message to it. In my point, I would like to answer all your question first. After that, I do a rebase for it. Is it ok for you? |
Yes, it is OK. Plugin upgrades need to be done in a separate commit though, unless they are closely relevant to the commit at hand.
Yes, authors are required to rebase their PRs as needed. You can see the contribution guide for more details. |
|
After some thinking, I do strongly suspect that the Netlify issues might be due to it only supporting JDK 8. |
|
From this repo, It seems that Netlify may only support JDK8. |
|
@pyokagan The netlify issue should be fixed. I have removed the plugin entirely. |
|
There are some minor problems and I will fix it tomorrow. |
|
@pyokagan Is all your question being answered except for the |
|
@fzdy1914 You can (and should) start rebasing now. We treat individual commits as individual changes to be discussed independently. As mentioned in the PR:
|
Currently, Assert implements our custom assertThrows using try/catch. This implementation was done when we used JUnit 4, and had no proper support for assertThrows. As we migrate to JUnit 5[1], we can make use of JUnit 5's Assertions.assertThrows. However, our current assertThrows supports checking the correctness of error's message, whcih JUnit 5 does not support. Let's migrate Assert.java to use JUnit 5's assertThrows. In addition, let's create a custom assertThrowsWithMessage to support checking the correctness of error messages. The old assertThrows is kept for backward compatability, as many existing classes depend on it. It has been marked as deprecated, and will be removed in a future commit after migrating all tests. [1] se-edu#951
Currently, Assert implements our custom assertThrows using try/catch. This implementation was done when we used JUnit 4, and had no proper support for assertThrows. As we migrate to JUnit 5[1], we can make use of JUnit 5's Assertions.assertThrows. However, our current assertThrows supports checking the correctness of error's message, whcih JUnit 5 does not support. Let's migrate Assert.java to use JUnit 5's assertThrows. In addition, let's create a custom assertThrowsWithMessage to support checking the correctness of error messages. The old assertThrows is kept for backward compatability, as many existing classes depend on it. It has been marked as deprecated, and will be removed in a future commit after migrating all tests. [1] se-edu#951
Currently, Assert implements our custom assertThrows using try/catch. This implementation was done when we used JUnit 4 which has no proper support for assertThrows. As we migrate to JUnit 5[1], we can make use of JUnit 5's Assertions.assertThrows. However, our current assertThrows supports checking the correctness of error's message, whcih JUnit 5 does not support. Let's * migrate Assert#assertThrows to use JUnit 5's assertThrows. * Remove VoidCallable interface within Assert * create an overloaded assertThrows(Class<? extends Throwable>, String, Executable) to support checking the correctness of error messages. * standardise to use only seedu.address.testutil.Assert#assertThrows instead of org.junit.jupiter.api.Assertions.assertThrows By creating an overloaded assertThrows method, we can extract the common routine of checking error message into a single utility function within Assert.java. This encourages reusability and follows the DRY principle. In addition, by using org.junit.jupiter.api.function.Executable that already accepts lambda expressions, we no longer need the VoidCallable interface previously. Hence, we can remove the unused VoidCallable interface. [1] se-edu#951
Currently, Assert implements our custom assertThrows using try/catch. This implementation was done when we used JUnit 4 which has no proper support for assertThrows. As we migrate to JUnit 5[1], we can make use of JUnit 5's Assertions.assertThrows. However, our current assertThrows supports checking the correctness of error's message, whcih JUnit 5 does not support. Let's * migrate Assert#assertThrows to use JUnit 5's assertThrows. * Remove VoidCallable interface within Assert * create an overloaded assertThrows(Class<? extends Throwable>, String, Executable) to support checking the correctness of error messages. * standardise to use only seedu.address.testutil.Assert#assertThrows instead of org.junit.jupiter.api.Assertions.assertThrows By creating an overloaded assertThrows method, we can extract the common routine of checking error message into a single utility function within Assert.java. This encourages reusability and follows the DRY principle. In addition, by using org.junit.jupiter.api.function.Executable that already accepts lambda expressions, we no longer need the VoidCallable interface previously. Hence, we can remove the unused VoidCallable interface. [1] se-edu#951
Currently, Assert implements our custom assertThrows using try/catch. This implementation was done when we used JUnit 4 which has no proper support for assertThrows. As we migrate to JUnit 5[1], we can make use of JUnit 5's Assertions.assertThrows. However, our current assertThrows supports checking the correctness of error's message, whcih JUnit 5 does not support. Let's * migrate Assert#assertThrows to use JUnit 5's assertThrows. * Remove VoidCallable interface within Assert * create an overloaded assertThrows(Class<? extends Throwable>, String, Executable) to support checking the correctness of error messages. * standardise to use only seedu.address.testutil.Assert#assertThrows instead of org.junit.jupiter.api.Assertions.assertThrows By creating an overloaded assertThrows method, we can extract the common routine of checking error message into a single utility function within Assert.java. This encourages reusability and follows the DRY principle. In addition, by using org.junit.jupiter.api.function.Executable that already accepts lambda expressions, we no longer need the VoidCallable interface previously. Hence, we can remove the unused VoidCallable interface. [1] se-edu#951
|
Fixed in #961 |
Background:
AB-4 is currently distributed as a single "Fat JAR". This JAR is packaged using the shadowjar plugin, which helps bundles dependencies with our application code. This works well when our dependencies do not contain any native libraries.
However, starting with JDK11+ JavaFX is not distributed with Oracle JDK any more. This is problematic for us because (1) JavaFX contains platform-specific native libraries, and (2) JavaFX specifically searches for its modules via the module system, so we probably can't use JARs anymore (since it will just be loaded as an unnamed module)
Problem Statement:
We need to find a distribution method for AB-4 that works for JDK11+.
Keep in mind that:
Expected deliverables:
Opportunities and benefits:
Mentor: @pyokagan
Assigned to: @fzdy1914
The text was updated successfully, but these errors were encountered: