edvin / fxlauncher

Auto updating launcher for JavaFX Applications
Apache License 2.0
715 stars 107 forks source link

Unable to pass remote location to --app option #87

Open mordechaim opened 7 years ago

mordechaim commented 7 years ago

https://github.com/edvin/fxlauncher/blob/f1eae017036124a1183cc9facf435ee6230a5ab9/src/main/java/fxlauncher/AbstractLauncher.java#L205

This will always cause a FileNotFoundException if a valid url is passed, leaving the only other option to use the --uri option.

This conflicts the README.MD description that you can pass the remote location into that option.

In my opinion the manifest loading is flawed. Instead of the --app and --uri params it should have --remote and --local params. We first try loading the remote manifest, if it succeeds sync the local manifest with the downloaded (if --local is used, of course). If it fails (no internet connection or --remote options wasn't used) try loading the local manifest. If this fails too (deleted etc. or --local option wasn't used) try loading the embedded manifest. If that fails (was never included) show error message to user.

As far as I understand, the project lacks a robust fallback design. This should solve it.

edvin commented 7 years ago

I totally agree, this sounds like a good solution. @ronsmits I guess this was your baby.. Do you have any comments or should we implement this pattern?

mordechaim commented 7 years ago

I'm a bit worried it will break existing user code. I want to remove these 2 params.

Maybe it's a small change and a small community. We can just explain changes and let them adapt to it?

edvin commented 7 years ago

We could keep the old ones for a while, but deprecate them.

ravenp1992 commented 6 years ago

Hi @edvin

I'm trying not to embed the app.xml file to the launcher instead I want the fxlauncher to read the app.xml to a remote location. I think I'm missing something here if my pom.

I tried to put the argument --app=http://localhost/test `

embed-manifest-in-launcher package exec jar ${app.dir} uf fxlauncher.jar fxlauncher.HeadlessMainLauncher --app=${app.url} ` what I really want to achieve is to dynamically point where to get the update. I hope you can help me.
edvin commented 6 years ago

The --app parameter is just a command line parameter to be able to override the url in the embedded app.xml manifest. You can always supply a new url in the remote manifest if you want to move the hosted artifacts later on.

ravenp1992 commented 6 years ago

Thanks for the fast reply @edvin

I already did that but when I turn off the server that holds the previous data and runs the application it gives me an error on update it still points to the old server. I will double check my pom and try it again.

edvin commented 6 years ago

Yeah, the old server would have to stay available until all clients have done one update. But if you anticipate the need to change url down the line, just fix it once and for all by using a custom hostname for your application. https://appname.yourdomain.com - if you ever need to change it, just point it to a new ip and you don't need to mess around with FXLauncher at all.

ravenp1992 commented 6 years ago

so the domain must always the same. if the previous domain is https://appname.yourdomain.com the new domain should also name with https://appname.yourdomain.com ?.

edvin commented 6 years ago

There is no such requirement, merely a suggestion if you think you need to move your app later, and don't want to redeploy the application to do so. It's no big deal using the aforementioned approach either though.

ravenp1992 commented 6 years ago

I did what you said earlier. I change the url to point the new server in the app.xml then run the application to get the update. then I close the app again then shutdown the old server then run again the application. but it didn't get the update from the new server. it gives me this error.

this is the old server java.io.FileNotFoundException: http://localhost/commtest/app.xml

anyways thank @edvin appreciate your help.

mordechaim commented 6 years ago

As i've long proved the --app param always fails for remote locations, so this might be the commentator's issue.

In my fork i completely redesigned the artifact resolution (and yes, it's digitally signed, as i promised), but i don't want to ask for pull request as it breaks too many things.

edvin commented 6 years ago

@mordechaim Can you align the API, or maybe we should even consider a major version upgrade with a PR from your fork?

mordechaim commented 6 years ago

Aligning would be very hard as i added quite alot new features. It would also be tedious as my Eclipse formatter uses tabs instead of spaces and git would consider everything as my change. If you used Google format I might straighten it out.

In any event you should test it since I never tried all corner cases (only made sure it works for me...)

One of the major changes is that you can set the location of each artifact seperately instead of a base location. This way you can host them on google drive or dropbox (the latter, which I used).

Btw Java 9's new class loaders completely breaks this whole project, maybe i should open an issue for that.

ravenp1992 commented 6 years ago

@edvin please check. This is where the updater puts the library upon update, what do you think? don't you think I'm missing something here like app.xml. I tried to install and uninstall the application check the pom and still not working when I change the server application

ravenp1992 commented 6 years ago

@edvin I created a test application then recreate my step on changing server and it works lol, don't know what's the problem with my previous app, cheers :clinking_glasses:

edvin commented 6 years ago

Hmm! OK, please post when you figure it out :)

mordechaim commented 6 years ago

@edvin Of course you could just fix that one-liner quoted in the first comment of mine the take a url instead of a file, and everything will be just fine.

It's just that this issue inspired me to make changes, but you don't have to go this path.

On 11/9/17, Edvin Syse notifications@github.com wrote:

Hmm! OK, please post when you figure it out :)

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/edvin/fxlauncher/issues/87#issuecomment-343064659

edvin commented 6 years ago

I think the easiest solution is to just change Line 205 in AbstractLauncher for now. Let's do the rest in a redesigned version. Java 9 is forcing the need for that anyways. Do you have time to submit a small PR? I'm on a mini cruise until Monday, so I won't be able to do something with this until next week.

ign4z commented 6 years ago

@ravenp1992 , please consider that on linux the installation directory is /opt/ and when you run your app you don't have the permission to write! Try to change the permission or use a different cache dir

ravenp1992 commented 6 years ago

Hi, thanks, @ignagnu,

I used a cache dir, I already saw the problem why it is happening but can't figure it out how it occurs. When I change the url inside the pom and push it to the current server the updater should create an xml file inside the cache dir so that the updater read that xml file and knows where to get the update, the file that will be created should be like this but in the previous app that is not happening.

`<?xml version="1.0" encoding="UTF-8" standalone="yes"?>

Updating... -fx-font-weight: bold; -fx-pref-width: 200; -fx-spacing: 10; -fx-padding: 25; --myOption=myValue --myOtherOption=myOtherValue USERLIB/Communicator false `
mordechaim commented 6 years ago

I've finally designed a complete new framework which was highly influenced by this project. It is fully Java 9 compatible. In fact you must use Java 9 to use it.

You can find it here.

Thanks edvin, your project really inspired me in the right direction.

mordechaim commented 6 years ago

I've put much thought on strong encapsulation and configuration. Also tried to make it as extensible as possible.

Although it was designed with JavaFX in mind, I went out of my way to make it framework neutral, so it works and JavaFX and in command-line out of the box.