Distribute micromamba executable for plugins #1707
Conversation
|
@dcdenu4 I think the ReadTheDocs failure is unrelated |
There was a problem hiding this comment.
Hey @emlys, this will be great. The install size and time it takes to install InVEST is drastically better with this approach. I had a few questions, but mostly the Windows micromamba executable path still isn't quite right. I left a suggestion for what I think should do it. In the electron config when moving that file, making sure the exe extension remains in the filename.
.github/workflows/build-and-test.yml
Outdated
| run: | | ||
| curl -Ls https://micro.mamba.pm/api/micromamba/osx-64/latest | tar -xvj bin/micromamba | ||
| mv bin/micromamba dist/ | ||
| ./dist/micromamba --help |
There was a problem hiding this comment.
Is this just to make sure the command works, and if it doesn't we'll fail / exit here? If so, it might be nice to have a short comment here.
There was a problem hiding this comment.
Yes, just testing the executable. Which might not be necessary any more, but I think it's fine to leave in. Added a comment
| logger.info(`env info:\n${envInfo}`); | ||
| const regex = new RegExp(String.raw`^${envName} +(.+)$`, 'm'); | ||
| const regex = /env location : (.+)/; |
There was a problem hiding this comment.
I don't think I've seen this syntax before. Is the /.../ syntax for a regex, without needing it to be wrapped in a string?
There was a problem hiding this comment.
Yes, it's a regex literal. My linter recommended this over a string.
workbench/electron-builder-config.js
Outdated
| }, | ||
| { | ||
| from: '../dist/micromamba.exe', // windows | ||
| to: 'micromamba', |
There was a problem hiding this comment.
| to: 'micromamba', | |
| to: 'micromamba.exe', |
There was a problem hiding this comment.
Added this in another commit before I saw this
Description
Fixes #
Checklist