fix(linux): continue update process even if AppImage is not available #8539
Conversation
… in APPIMAGE env path
|
✅ Deploy Preview for car-park-attendant-cleat-11576 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| try { | ||
| unlinkSync(appImageFile) | ||
| } catch(e: any) { | ||
| this._logger.warn("Previous version of AppImage is not available"); |
There was a problem hiding this comment.
Fixed to continue the update process even if the previous version of AppImage is not available in APPIMAGE env path.
I'm not sure I follow here, shouldn't the appImageFile always exist? We already check that appImageFile !== null to get to this logic.
There was a problem hiding this comment.
appImageFile is process.env.["APPIMAGE"]
so it doesn't check whether the actual AppImage file exists, but the path is set as an environment variable.
For example, I set the path like this in code.
if(platform == 'linux'){
if(process.env.APP_ENV === 'development'){
process.env.APPIMAGE = path.join(__dirname, `../../dist/***-${app.getVersion()}.AppImage`);
}
If we set process.env.APPIMAGE like above, then APPIMAGE env is not defined error is not coming out and starts download process.
After the next version of AppImage downloded, then it checks whether the AppImage exists in the path, if not, it emits ENOENT: no such file or directory, unlink '/home/***/Documents/***-1.4.1.AppImage'
I know myself where to put the previous AppImage (cause I did set the path),
but others who use this app don't know where to put it, they would just download AppImage somewhere in for example /Downloads folder and the update process will not work.
References:
APPIMAGE env is not defined (#3167)
https://stackoverflow.com/questions/52442650/electron-builder-linux-updates-appimage-env-is-not-defined
There was a problem hiding this comment.
Can you provide an example of your usecase to override process.env.APPIMAGE?
If you truly need to override the AppImage path and the file doesn't exist yet, maybe create an empty/stub file at that path first?
Your link refrences are issues due to user misconfiguration (such as Webpack's DefinePlugin) in the respective projects. We can easily add to the documentation site for specifying the potential side effects that can occur with bundler misconfiguration.
APPIMAGE env var is a dedicated var set by the system, it should always exist and I'd prefer this logic to retain that logic expecting the file it represents to exist.
https://docs.appimage.org/packaging-guide/environment-variables.html#id1
APPIMAGE shall be used every time the full path of the AppImage is needed, e.g., if you need to touch the AppImage file, for example when you want to update it or read some meta information.
There was a problem hiding this comment.
I mentioned References as just for the same error message. APPIMAGE env is not defined
It's not related to webpack actually.
I edited my comment, so could you please review again? Thank you.
There was a problem hiding this comment.
I don't think this is a reasonable PR change, but you still haven't explained your use case to custom set/override process.env.APPIMAGE. Can you please elaborate on that?
but others who use this app don't know where to put it, they would just download AppImage somewhere in for example /Downloads folder and the update process will not work.
Wouldn't running the AppImage from the Downloads folder result in that file in the Downloads folder being updated? (e.g. correctly) Are you relying on some specific user environment configured for custom paths to work?
There was a problem hiding this comment.
I set the custom path because it emits error not defined. No other reason actually..
Wouldn't running the AppImage from the Downloads folder result in that file in the Downloads folder being updated?
If I set the path to ${process.env.HOME}/Downloads/***-${app.getVersion()}.AppImage, then it would work.
The path should be defined in the code, so if someone changed the download path, update is not working..
Are you relying on some specific user environment configured for custom paths to work?
This could be great, so I did thought about it once that scan the user' file system first, if the AppImage exists, then set that path to be process.env.APPIMAGE
But this logic is little bit complicated I think, so I wanted to do process even if the user had downloaded previous version AppImage to anywhere, the next version would download successfully and mv to defined path automatically.
Then for the next update, the downloaded next version is in the right path, so the update is work as the code said.
In my thought, it is little bit awkward process to whom install the app for the first time.
So if I add try-catch to doInstall function to go on even the AppImage is not available, very first installer's update would work, and the next update will go on normally after the first update.
There was a problem hiding this comment.
So on thinking on this further, I can't accept this PR change. If the unlink fails for a prod user running in current configuration but the relaunch sequence is able to proceed via your try-catch, there will be an infinite auto-update loop because it can't replace the original file with the new update via mv. (unlink is required https://stackoverflow.com/questions/1712033/replacing-a-running-executable-in-linux/1712051#1712051)
If you'd still like to proceed with your unique use case, please try using patch-package to patch your changes so that you can build your app with it.
There was a problem hiding this comment.
I have tried this in my project, and the result was
(set path)
process.env.APPIMAGE = ${process.env.HOME}/Documents/*.AppImage
[No try-catch]
- If current version AppImage is in /Documents
- next version AppImage downloaded in ~/pending directory ➡️ unlink current version in /Documents ➡️ mv downloaded next version to /Documents ➡️ Update success
- If current version is in random directory
- next version AppImage downloaded in ~/pending directory ➡️ unlink execute but
no such file or directory➡️ Update process stops and restarts current version
- next version AppImage downloaded in ~/pending directory ➡️ unlink execute but
[try-catch]
- If current version AppImage is in /Documents
- same process
- If current version is in random directory
- next version AppImage downloaded in ~/pending directory ➡️ try to unlink AppImage in /Documents but fail (cause current version is not in env path) but emits log ("Previous version of AppImage is not available") and keep goes on next process ➡️ mv downloaded next version to /Documents ➡️ Update success
- after the first update, downloaded next version had successfully mv to /Documents, so the update process will be run normally from now on update.
These are the whole process, I have tried several times but infinite loop was not found.
Thank you for your valuable time reviewing these long comments.
Whether my PR is accepted or not is not important, but I think I found a little bit awkward process while developing, so I was just giving my opinion.
Thank you again!
There was a problem hiding this comment.
mv downloaded next version to /Documents ➡️ Update success. after the first update, downloaded next version had successfully mv to /Documents, so the update process will be run normally from now on update.
This seems like unexpected behavior to me from what the default AppImage ecosystem expects, and as with AppImage Launcher. Happy to be read resources that state otherwise though.
What'd I'd recommend though for your app's logic is on launch of the app, detect the current APPIMAGE path and if it's not in Documents, locally run mv the current file represented APPIMAGE path to /Documents, then reset the process.env.APPIMAGE=<new path in /Documents>, after which you can either unlink the old file (or leave it), and then just run quitAndInstall or restart the app yourself via execFileSync(process.env.APPIMAGE); app.quit();. If you choose not to unlink the old file, if the user already created a .desktop file or registered it with AppImageLauncher, they'll still launch the legacy version AFAIK.
So with all this occurring, I would also recommend presenting a notification to the user for confirmation that you're moving the application path and also determine if you need to update/re-register the app with the .desktop file or with AppImage Launcher (if it's possible, I'm personally not familiar with it)
Alternatively, you could patch-package the try-catch, however I'd still recommend the .desktop file check and update AppImage Launcher
Situation
To update the AppImage, current (now running) version of AppImage must had to be on path that declared in code. (process.env.APPIMAGE)
If not, downloaded new version of AppImage disappers and current version app restarts.
Fix
Fixed to continue the update process even if the previous version of AppImage is not available in APPIMAGE env path.