fix #35

[garethbosch]

I hit issue #35 and was able to use @DvdGiessen's code to make a fix. Without this PR I was unable to run in non-admin mode. Ember CLI give no helpful errors because it dies before it can.

The test I added fails without this fix on machines where the user has permission to symlink files but not symlink directories.

[ef4]

Looks like the tests are failing in old node versions. It turns out process was not a module there.

But it also looks like process isn't actually being used here, so you can just delete this line to fix the problem.

[ntwb]

Thanks for the PR @garethbosch, I've just tested this PR and works as expected 👍

• WordPress/packages#79

Any chance of a merge and cutting a new release soon?

[ef4]

Hi @stefanpenner and @joliss, this PR fixes an issue that's causing pain for windows newbie users. If we can get a release out it will be appreciated. I can also do that for you if you are willing to delegate.

[stefanpenner]

this test isn't idempotent, re-running the tests causes the second test run to fail.

[stefanpenner]

fixed cc7cff2

[stefanpenner]

released as v1.2.0 🎉, sorry for the delay and @ef4 thanks for raising it to my attention.

[stefanpenner]

I think this can likely be improved, the scenarios where files can by symlinks but folders cannot should likely use junctions for the folders.

[stefanpenner]

@garethbosch / @ef4 / @ntwb what do you guys think?

[ef4]

Sounds plausible. I would need to go find some authoritative docs on the permissions model that's in play here.

Mostly my priority here was to make things not blow up for long enough that users can get to the ember-cli warning message that tells them it's better to run with Administrator permissions. That is the common use case once people are more familiar, which is why I don't think any contributors found and fixed this bug sooner -- it only struck poor newbies who didn't know to run as administrator, and it failed with a cryptic stack trace before they got the warning message.

[stefanpenner]

@ef4 interestingly, looks like I already handle the "use junction if no symlinks are available case" https://github.com/broccolijs/node-symlink-or-copy/blob/master/index.js#L142-L143.

Now I am curious if we should make this slightly more intelligent, as symlink of a file should be faster then a copy of said file.

So even if directories can't be symlinked, but files can we should still link files.

[stefanpenner]

optimization re my above concern: #37

[joliss]

Cc @DvdGiessen

I'd like to extract this into the can-symlink package, since we're using this logic in several places, so I updated the can-symlink code in ember-cli/can-symlink#5. Once that PR is merged, we should be able to use can-symlink in this package.

It's not really clear to me that we need to test for both files and directories -- so I in my patch we're only testing directories.

So even if directories can't be symlinked, but files can we should still link files.

Related to this, I would really like to understand under what circumstances it can happen that we can symlink files but not directories. I haven't been able to reproduce it on Node 9.5.0 on Windows 10.

[DvdGiessen]

I haven't yet seen a case where directory symlinks work but file symlinks don't, but given that this bug isn't documented anywhere, it might be good to check anyway. Not that expensive a call.

Since I am traveling, I do not have access to the system where I made the report back in December, but on my laptop right now I was only able to reproduce it in a older version of Node (8.8.1):

I cannot find anything in the release notes of Node about any relevant changes to symlinking though, so it might still be a bug or related to something else than the Node version.