-
Notifications
You must be signed in to change notification settings - Fork 70
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
Build Cleanup - Using static classpaths rather than dynamic ones. #349
base: master
Are you sure you want to change the base?
Conversation
Anathema.java, and cleans up dependencies.
0th element, while JToolBar.getComponentAtIndex(0) will return null if there is no 0th element.
in using runtime configurations, then stores it in the manifest.
references to /plugins, and instead we use just the /dependencies folder
There were two reasons for the dynamic classpath, the update system and (replacing the) plugin system. The latter reason is obsolete. We now support loading custom data via the repository and there are no external contributions anyway. For that reason alone, we could drop it. I'm not too hot on having auto-updates, but some people (you among them?) were, and I don't see the need to take this away from them. And we could always include another updater: Maybe you can replace I'd love your opinion on this. |
Keeping Anathema Up To DateI don't think I was ever totally behind in-app updating. I certainly don't mind software that has an update feature, and the easier it is, the more likely I am to update the software. Do we have any metrics on number of people using the system vs normal downloads? That said, I personally have not used used Anathema's update feature outside of a few tests. When dealing with my "official" repository, I use a manually downloaded copy of Anathema, and I only update it a few times a year. As I recall, one of the reasons people were clamoring for an update system was for better notification of the fact that there is an update. Currently you can keep track of updates by subscribing to the Google+ account, subscribing to the anathema.github.io RSS feed, monitoring the forums daily for an update post, or using the in-app update system daily. To further expand our ability to let people know there's an update, I'd like to see a preferences option to check for updates on startup (checked by default), regardless of what update method we use. From personal experience, Anathema users I've dealt with just download Anathema once and almost never update it. They're new to Exalted, they endlessly play with Anathema during character generation, but after they export their final PDF it goes un-used for weeks. It was like pulling teeth to get them to update their copy of Anathema, even with the in-app update option. Often I'd find out they'd done several updates at my request, but had never run Anathema between updates. Even assuming someone's running Anathema weekly to track XP and story notes, that doesn't mean they're necessarily going to want to update twice a month. Updates introduce new features and fix bugs, but they also generate bugs occasionally. Personally, unless the changelog lists something that affects me in a major way, I'm not likely to update infrequently used software like Anathema. Updates and the Default ClassLoaderYou're definitely right, the update system would not work at all with this method, I hadn't fully read all the documentation on the default ClassLoader last night. Once you've loaded your jar files with the default ClassLoader, it doesn't offer a mechanism for unloading and reloading them. Removing UpdatesWe do have a large quantity of code in place to handle the custom ClassLoader and update system. It works currently, and I hesitate to remove something that works unless there are clear benefits. Benefits
As to another updater, there are always options. The easiest thing I can think of is if we shipped a separate executable that could update anathema. Doing it as part of the launch process will always involve a custom ClassLoader. JavaFXI'll eventually tackle using the javafx bundling tasks, which will clean up a lot of our build process. We'll easily be able to generate a a runnable jar like we do now, self-contained standalone application packages to replace our launch4j/nsis/appbundler tasks, and webstart files. I'm hesitant to start in on this immediately, as we have solid working build code already, but it may be worth doing before 3E. |
If the ruckus back then really was about expectations only, I never read our users worse. I was convinced they were clamoring for an automated update system because replacing Anathema was an issue - after all, you have to remove the existing install first, lest versions interfere with each other. Moving the RepoAnd that would still be an issue now, coming to think of it. I'd say we should move the repository out of the install directory in the same breath, or shortly after, if we go this way. Which brings up other issues, namely that an external Repo would be picked up by any version of Anathema, and thus makes it harder to have several installs in parallel. BenefitsI'd like to see hard numbers before I believe in 4) and I don't know a thing about 2) and 3) but otherwise, I agree with the benefits. Bundling via JavaFXI can't see the benefit of commiting to a JavaFX build before we even have the system converted, let alone know the timespan between conversion and 3E. Once the migration is done, sure, but until then I believe your effort better spent elsewhere. |
Two more things I forgot to answer:
|
Oh, there were definitely some people raising a ruckus over it, but there's always a few people like that in any community. Manual UpdatingAs for requiring removing the existing install first, this is a non-issue on OSX, as manually installing the app automatically replaces the old code code, and you're able to run parallel copies (using the same repository) by just changing the application's name. On Windows, doesn't the NSIS installer handle the update fine, or were we running into issues with it? For the zip installation, the entire thing is self contained, so it shouldn't be an issue. Moving the RepoFor reference: By default, Anathema is hardcoded to use Remember that once a user selects a custom repository location in Preferences, it means that any copy of Anathema that runs on that user account will now use that location. We store the custom location in the Java Preferences, and this shared across all versions of anathema. I do this, and I have copious backups of my repository for when I'm launching test copies of anathema. That said, I've rarely had an issue with a development copy of anathema screwing with my repository data, even when running 3E builds against my 2.5E repository. We could pretty easily alter our preferences storage system to key repository locations to the location net.sf.anathema.Anathema is run from. This would allow a separate set of preferences per installed copy of Anathema. JavaFXYou can use the JavaFX packaging tools even if you aren't actually using javaFX, here is oracle's argument for it. |
I added some thoughts about replacing the updater to my previous post while you were writing. Sorry for the confusion. Does the NSIS installer work around existing Repos? I tend to forget, I only use it for testing and usually launch Anathema from the IDE. What do you mean by "Zip installation is self contained"? That wouldn't solve repo transition issues, would it? Regarding the repo move, yes, it came to my mind while I was writing about it earlier that the issue already exists. |
By "self contained zip install", I meant that our The NSIS installer just creates a repository directory in the install location, and grants systemwide read/write/execute access to it. If the directory already exists, nothing happens. The NSIS uninstaller removes the directory. I'm pretty sure the installer will just overwrite any old install files during the install process. Quick BenchmarksBefore/After numbers for the current trunk vs my branch. Before
first repeat run of average launch time of Anathema (opening jar file, stopwatch, 5 trials) - 7.02 seconds After
first repeat run of average launch time of Anathema (opening jar file, stopwatch, 5 trials) - 6.20 seconds I think I know where the bottleneck is in my building, it has to do with the way I'm generating the explicit classpath. I'll clean it up later. The anathema launch times were slightly faster overall. update: |
Self-containedOkay, that's what I thought. Auto-UpdateI did a quick survey on G+, and apparently, those guys actually use it. BenchmarkActually, I removed the (memory) setting on purpose. I thought it was added by Sean because the build broke on his system, and since the gradle guys promised that they had optimized their memory behaviour, I wanted to give it a try. If there was anything else in there, it was an accident. Your numbers, do they mean that the build takes longer, but only due to the bottleneck you mention? Overall impressionI have a growing feeling that dropping the updater for the moment isn't worse than dropping any of the other features we kicked during our cleanup run, and that the advantages win out in the short and in the long run. |
Self-containedTo be clear, nobody is required to move the repository around when manually updating unless they explicitly choose the Zip distribution and execute it by "java -jar Anathema.jar" or by double clicking the jar file. And even this is alleviated by setting a new location for the repository in Preferences. Repository LocationI have two opposing thoughts on this:
I expect I'll develop further thoughts on the repository location once I finish collecting my thoughts on the repository file formats. Auto-UpdateWhy doesn't it surprise me that the Venn Diagram of people using G+ and people using our auto-update has a natural intersection? :) We should probably talk @davidhfe into making us an in-depth user survey at some point. There's lots of little bits of information that'd help us make decisions. For this question, I'd like to know how often people are updating anathema, and how often they're running it, how often they check for updates, how frequently they update after finding out an update is available, and why they update (features, stability, boredom). BenchmarkThe memory settings shave 10-20 seconds off assembleZip, for example, definitely worth keeping. The default memory they allocate for gradle is criminally small. Yes, the numbers are higher due to the bottleneck; I suspect its in the code I use to build the dynamic classpath. Basically its iterating over all subprojects, over all depencencies, over all jar files, then creating a giant set of all jar files, throwing out duplicates, stripping out the path info, prepnding "lib/", then creating a giant string out of it all. I'll rework it to something more sane to get the time down. I'll also fork off the trunk again and benchmark my copyDependencies code with that, to see if its any faster/slower than the current method. OverallI'm on the fence myself, but mostly because on one hand I hate to toss working code, but on the other, I worry that its going to need to get thrown out anyways in the future, due to packaging changes, app stores, code signing, etc. |
So, maybe it's just not the moment to make the decision? If somebody wants to change the bundling or go for an appstore or whatever, we can decide really fast now, since we have the facts now. |
"As long as the bootloader doesn't hinder us..."It does now. Apparently, the JavaFX Note: With this fact known, I think it's best to do as we suggested above: Drop updating for now, convert to a standard classpath/launch system (reaping the benefits as we go) and re-implement auto-updates at a later time. I've created a Trello card for the problem. |
I think using oracle's javafx packaging tools will alleviate this problem. I'll spend this week getting the build system working with this system. |
That's great. I'm sure you are aware of the Gradle JavaFX Plugin, in case you need a more gradly syntax. By 'alleviate', do you mean that it will work with the current mechanism, or that we'll switch over to the standard way of launching and building the classpath? As I said above, I'm fine with either way, I'm just curious. |
In case the splashscreen code gets in your way, deal with it in whatever way you like. I'll probably re-build that part later using FX's preloader mechanism. |
I read through the page you linked to, and some related blog you mentioned some months back. I am wondering whether there is a way around that by now. |
@curttasker: Ryan Gray, one of our more active followers on G+, just shared an interesting data point: He updated from 4.3.0 to 5.1.2 just yesterday. So maybe you're right and people don't update as frequently! |
@curttasker, did you already find time to look at this? |
@UrsKR I've been researching and poking at it a little, but haven't had the solid chunk of time I need to get it ready for roll-out yet. Hopefully early next week, my work schedule's clearing up finally. |
@UrsKR I'm making progress, but its slow going. I have the javafx gradle plugin building a app file and dmg release on the mac, though I need to tweak our launching and classloading code (similar to what I did in this pull request) to get it launching anathema from this automatically built binary properly. After that, I'll tackle the other platforms. One minor wrinkle: I'm not entirely sure that you can bundle the JRE using javaFX with Java7, we may have to jump to Java8 early. I'll update more on this as I discover it. |
That's good to hear. I am pretty sure bundling the JRE is already possible. If you look at the blog post I linked to above, it's explicitly mentioned. The official documentation gives that impression as well, and is written for JavaFX 2, i.e. the version bundled with Java 7. Going to Java 8 early would be unfortunate, but not impossible. There are some issues still unresolved in their current builds, and we'd have to work around them. |
I have made good progress updating Anathema to Java 8 over the past week, and it works flawlessly now. If not, I'll try to apply your work to the Java 8-branch, move the launcher to static classpaths and drop the auto-update. |
I haven't spent nearly as much time as I'd have liked; With the delays for Most of my work on the bootloader stuff was stymied by lots of minor issues On Sat, Apr 19, 2014 at 9:50 AM, Urs Reupke [email protected]:
|
We are on Java 8 now. |
Why I'm trying this
I've never been fully convinced that Anathema's dynamic classpath scheme using reflection was the way to go. I figured I'd try to do it "normally", and see what you thought of it.
There's still room for improvement, but I wanted you to see the general direction of my thoughts before I went any farther.
Build Cleanup
I started by getting rid of all the reflection code we use to build the classpath. This included deleting all of the files in
:Anathema
. I then took:Platform_Swing/Anathema.java
, moved it to:Anathema
, and gave it a static main method. This is now the main class of Anathema (which incidentally fixes the menu title issue in #348). I made:Anathema
depend upon:Platform_Swing
(it has to now).:Anathema/build.gradle
now uses gradle's runtime configurations to build a classpath, which is then put into the manifest.mf file for:Anathema:jar
. This replaces the old reflection code used to build the classpath dynamically. Note that it is impossible to simply say include all jar files from this folder in a manifest file; You must explicitly denote each file.I also threw in my version of the
copyAllDependencies
code while I was mucking around in here. It seems to work well, but @UrsKR's version worked equally well. (See my email comments on picking one over the other).JavaFX Exclusions
With Java 8 and newer, JavaFX classes (
jfxrt.jar
) will be on the default runtime classpath. With Java 1.7.0_06 and higher,jfxrt.jar
is included as part of the JRE, but you have to manually specify this file on the java runtime classpath.Currently Anathema bundles this jar inside the JRE for Win/Mac, and we explicitly exclude it being included as part of our runtime dependencies to avoid duplication.
I ran into a minor snag with this method and the new classpaths: There is no way to reference JAVA_HOME inside a manifest file, so you're unable to add
jfxrtr.jar
to the classpath.This called my attention to an edge case. While we can enforce a minimum JRE version of 1.7 for people running the Zip distribution, we can't enforce 1.7.0_21. Since JavaFX was only included on 1.7.0_06 and newer, its possible for users with older Java 7 installs to attempt to run Anathema through the jar file, and it will fail because there is no
jfxrt.jar
on their system.The new JavaFX bundling tasks should alleviate this, but until we switch over to it, we need a workaround. I decided on removing our exclusions for
jfxrt.jar
from our dependencies; I compensate by adding the same inclusion to our bundled JREs. Thus we're still only shipping one copy of this jar, its location has just moved.Possible problems
I didn't see any immediate issues with our update system, but you'd know better than I would. Because the classpath is now stored in the manifest file of
:Anathema/Anathema.jar
, if any new jar files get added to the project, then this jar file needs to be regenerated and pushed out with the update. This should happen automatically, as theAnathema.jar
file should be regenerated every time the classpath changes.Final Thoughts
Everything seems to be running just as it was before. I didn't run any benchmarks, but I suspect Anathema will launch slightly faster now, due to the lack of dynamic classpath code.
Anyways, let me know what your thoughts are on using this approach.