Skip to content

fix: Include swiftshader directory when creating installer for Electron 10+ - #375

Merged
malept merged 12 commits into
electron:masterfrom
niik:conditionally-include-swiftshader
Feb 10, 2021
Merged

malept merged 12 commits into
electron:masterfrom
niik:conditionally-include-swiftshader

Conversation

@niik

@niik niik commented Jan 20, 2021 •

Copy link
Copy Markdown
Contributor

This is a continuation of #367 with the intention of including the swiftshader directory (present in Electron 10+) as well as the vk_swiftshader_icd.json file. These paths are required in order for electron to be able to run on systems where GPU acceleration is unavailable.

See #367 (comment):

👋 We (the GitHub Desktop team) is looking into upgrading to Electron 11 and came upon this issue which made us a tad bit nervous as it seems like it would affect Electon 11 as well (right?).

We'd be happy to help in any way we can to get this over the finish line but it's not immediately obvious how.

I haven't looked in detail, but I suspect that this change will fail with older versions of Electron that do not contain the swiftshader files. This module doesn't currently specify the versions of Electron that it supports, so it effectively supports pretty much all versions. If we restrict that, this will require a major version bump. Otherwise the change will need to be fixed so that it works for versions without these files.

@malept Do you have a sense of what your preferred approach would be?

In case the approach would be to special case versions prior to Electron 10 do you envision having two different nuspecttemplate files and conditionally including one or the other?

let templateData = await fs.readFile(path.join(__dirname, '..', 'template.nuspectemplate'), 'utf8');

Or would you rather attempt to add the entries to the existing template at runtime?

And last question (for now), what method would you prefer for detecting whether these files need to be included? Should we just check whether they exist in the appDirectory or would you like to specifically check the bundled electron version?

@malept Is the approach taken here in line with what you'd expect?

Closes #367.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants