Electron-packager: Warn instead of fail when missing Wine on non-Windows platforms
If I do a build for all architectures... but don't have wine installed (on Mac) it stops because of that...
it would be preferable to warn that this architecture will not be built but continue to build all the others.
Packaging app for platform linux ia32 using electron v0.37.8
Packaging app for platform win32 ia32 using electron v0.37.8
spawn wine ENOENT
Version 7.0.1
--platform=all --arch=all --overwrite
bigmonkeyboy
All 21 comments
Would this only be for --all, or would this behavior happen for --platform=all and/or --platform=win32?
malept
on 6 May 2016
I don't like it. If you specify all — you expect all. If something goes wrong — build should be failed. (Travis will not notify you if there is a warning in the output because it is just a warning). platform allows you to specify a comma-delimited string or array of strings. So, if you don't want to install wine, just explicitly set platform to supported platform list.
develar
on 6 May 2016
I'm inclined to agree with @develar on this one - I would prefer it if someone contributed a patch to show a better warning when Wine is not installed.
malept
on 6 May 2016
someone contributed a patch to show a better warning when Wine is not installed.
In the electron-builder we check wine version (must be 1.8+) — https://github.com/electron-userland/electron-builder/blob/master/src/packager.ts#L225 and show user-friendly error message (I didn't contribute to electron-packager because this check is not part of platform package step and originally was specific for Squirrel.Windows (wine < 1.8 is broken)).
develar
on 6 May 2016
I can kind of see @develar's point too, but at the same time, on the other hand, we skip packaging for Mac OS X on Windows in cases where we know that will cause problems due to Windows permissions and symlinks (see https://github.com/electron-userland/electron-packager/issues/71#issuecomment-113945576).
So while I can see the "you're telling it to build all these things, if one fails, it didn't do what you want" argument, I could also see a "well, it'll build all it _can_ anyway" argument.
kfranqueiro
on 6 May 2016
I think the correct approach might be to skip these builds (with warning messages) but still fail at the end (process.exit(1) or however you do it in Node) to make it obvious that it didn't fully complete. Then the user (or the script) can decide what to do. I'm less sure of a better way to handle that in the API case.
malept
on 6 May 2016
Doesn't cli.js exit with an error code based on the actual API resulting in an error anyway?
Currently skipped platforms resolve to an undefined value which ends up pruned from the appPaths array at the end of index.js; maybe for the case of skips caused by environment issues we keep track of that separately so we can decide whether to in fact call cb with an error at the end instead?
kfranqueiro
on 6 May 2016
I think the approach that @malept mentions makes most sense to me... overall fail return code... but only after doing all things possible. (especially if something similar already happens on Windows.
bigmonkeyboy
on 6 May 2016
I've added a better error message, but I couldn't figure out a good way to do the "keep going if wine isn't found but error at the end", given that wine is called so late in the win32 transform code.
Of course, others are welcome to try. There might end up being a refactor.
malept
on 3 Jul 2016
Maybe instead of failing or warning when wine is not present electron-packager could just skip calling rcedit if none of the win32 metadata (or icon) are specified. This way we keep best of both worlds: incomplete builds still fail (when metadata is specified but cannot be applied because wine is not installed), and it is possible to make win32 build without wine if you don't care about metadata.
matix2267
on 11 Dec 2016
That is an interesting idea, however we infer app-version from the Electron app's package.json, so there will very nearly always be some metadata to update the app's executable with.
malept
on 11 Dec 2016
Maybe try to run rcedit and if there's no wine print warning and continue build if no user-specified metadata is present, otherwise fail build. This feels a bit hacky though.
Alternatively it should be quite simple to add a command line switch that would disable this part of processing for Windows build.
matix2267
on 11 Dec 2016
@malept thanks for adding the message, it would be awesome to fail after building everything possible, but the message alone helps.
averissimo
on 12 Dec 2016
My opinion is that the way that Electron Packager is currently designed prevents us from implementing "warn instead of fail". If it was completely refactored to use Promises instead of the run-series module, then I think this would work.
malept
on 12 Dec 2016
@malept what's your opinion on having a flag like --no-rcedit like @matix2267 suggests? Currently hitting a wall w/ my ops team because they can't/won't get wine on our Jenkins servers, and we're not using any options that require rcedit other than the inferred app-version. It's a whole lot of pain for our small internal tool.
rahatarmanahmed
on 27 Jan 2017
@rahatarmanahmed I'll think about it. I'm currently considering this an edge case, because
- I've come around to the opinion that it's generally better to run tools like Packager (and Electron Forge, for that matter) where host platform == target platform
- there's a very, very small percentage of Packager users who wouldn't want their Windows Electron executable customized for their app
- I'm concerned about adding increased complexity
malept
on 27 Jan 2017
What I did as a workaround for my case (as @rahatarmanahmed) was to make a symlink bin/wine -> /bin/true and call packager with PATH=$PWD/bin:$PATH
matix2267
on 27 Jan 2017
@matix2267 oh wow that's a real nice way to handle this edge case. I put this in a package so anyone else who doesn't want to use wine can just have this in their package.json (if they use npm scripts).
rahatarmanahmed
on 27 Jan 2017
@rahatarmanahmed do you mind adding caveats to the readme for that package, like:
- The icon will not be set
- the application metadata will not be changed from the Electron defaults
Basically, I can just imagine tickets being filed here saying "I used electron-packager-dummy-wine and my icon isn't showing up"
malept
on 27 Jan 2017
@malept sure, done!
rahatarmanahmed
on 27 Jan 2017
I think I'm going to close this. There's a workaround by using electron-packager-dummy-wine.
malept
on 28 Jan 2018
Related issues
Bharwcb
·
5Comments
Write-Guy
·
3Comments
ghost
·
3Comments
corvinrok
·
4Comments
rafaelcastrocouto
·
3Comments
Most helpful comment
I think the correct approach might be to skip these builds (with warning messages) but still fail at the end (
process.exit(1)or however you do it in Node) to make it obvious that it didn't fully complete. Then the user (or the script) can decide what to do. I'm less sure of a better way to handle that in the API case.