Use plain path.join to fix path handling #22
Conversation
develar
force-pushed
the
use-path-join
branch
3 times, most recently
from
2528652 to
ca08067
Compare
PR is required to fix windows-installer — blocked on electron/windows-installer#22
PR is required to fix windows-installer — blocked on electron/windows-installer#22
| await jetpack.copyAsync( | ||
| p`${__dirname}/../vendor/Update.exe`, | ||
| p`${appDirectory}/Update.exe`); | ||
| const vendorPath = path.join(__dirname, "..", "vendor"); |
There was a problem hiding this comment.
path.resolve('../vendor') can be used instead.
There was a problem hiding this comment.
No. No. See docs — "the current working directory is used as well".
|
This probably fixes #13. You can also use jetpack more for path resolution. |
|
@lukeapage Ci failed due to unrelated error. Could you please restart AppVeyor build? Otherwise I have to push some dummy change to trigger rebuild. |
|
@lukeapage I want to get rid of jetpack because jetpack uses q instead of bluebird. Bluebird ships with the best cross-platform long stack traces. fs-extra + Promise.promisifyAll is enough. No need yet another fs lib (electron-builder and electron-packager uses fs-extra). |
|
@develar I'm not a admin on this repo, I just fixed a few things recently and took a look at the PR - so I can't restart appveyor - the same thing is happening on my PR's where I've just changed documentation. You can't see who is an admin on electronjs, the only person I know for sure is admin is @kevinsawicki but he is usually quite fast at looking at PR's re: jetpack - fine by me |
|
I don't think this is related to path.join, I think this is a jetpack issue. I had to replace several methods with |
|
@paulcbetts path helper does |
|
@paulcbetts replacing path-helper with path.join solves the problem, tested it |
|
@ridersx +1 electron-builder was published (2.8.5) with my changes and user reported that issue is gone. |
|
And yes — don't use windows-installer yet, use only previous version (not es6). A lot of errors — at least electron-userland/electron-builder#208 electron-builder 2.8.6 will continue to use my own fixed version. |
|
I can confirm that still have errors during building on windows. Even after ths PR applying. |
|
@kevinsawicki could you please take a look? |
develar
force-pushed
the
use-path-join
branch
2 times, most recently
from
6306ea6 to
a0e13a9
Compare
|
PR updated — path-helper.js completely removed. |
|
Confirmed by @havenchyk that |
|
Minor style comment, but it looks like both single quotes and double quotes are used in the new It would be nice to be consistent and maybe just standardize on single quotes. |
|
@kevinsawicki Fixed, eslint rule added to avoid such error in the future. |
Awesome, thanks 👍 |
Use plain path.join to fix path handling
Fix error "Path to copy doesn't exist D:\dev\projects\privateName\src\electron\dev\projects\ privateName\src\electron\node_modules\electron-winstaller\vendor\Update.exe"
use plain path.join — path helper doesn't add any value