-
Notifications
You must be signed in to change notification settings - Fork 204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixes #114 - add windows abstraction to handle file linking #136
Conversation
def command(options={}) | ||
watch = options[:watch] ? "--watch" : "" | ||
|
||
"\"#{ember_path}.cmd\" build #{watch} --environment #{environment} --output-path \"#{dist_path}\" #{log_pipe}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can't be right. The code is designed to blow up when ember_path
is not an executable file. So, it's not gonna return something sensible that you can just add .cmd
to, I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I changed it to check exist?
is because on windows executable?
will always return false here. However I totally forgot that npm will create a .cmd
for it which might work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@swelham is right. Windows doesn't understand hashbang (#!
) executables, so npm creates a .cmd
wrapper that basically calls node.exe <script>
. So if an npm creates a binstub on windows, then it also creates a .cmd
file so that it can be used as an executable. Ruby does the same thing, except as batch scripts.
We're sorry that this issue has sat for so long. The project is now maintained by thoughtbot, and we're declaring an issue bankruptcy of sorts. This looks promising, and we're really interested fixing this problem. However, since this project lacks a test suite (and CI in general), we're going to hold off on merging PRs until we've added some coverage. Thanks for your patience. |
@swelham sorry that this issue has sat for so long! Luckily, we've restructured the way shell interaction is implemented: We now have a If it's possible, could you list out the differences between generated strings from a *NIX / Windows perspective? Even better, could you update this PR to introduce a |
Unfortunately, I don't have access to a Windows machine to verify this works. There is a branch that introduces AppVeyor for CI testing: Would you mind helping to create windows versions of:
Once we have setup parity across platforms, we can start to ensure the Windows version behaves the same as other Operating Systems versions. |
I have added a layer on top of the existing
EmberCLI::App
class that handles the linking when running on windows.While the solution isn't pretty (and is a bit of a hack) I have found this to work reliably on windows.