Skip to content
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

Find workaround for symlinks on windows platform ... #128

Open
apendua opened this issue May 5, 2015 · 17 comments
Open

Find workaround for symlinks on windows platform ... #128

apendua opened this issue May 5, 2015 · 17 comments

Comments

@apendua
Copy link
Member

apendua commented May 5, 2015

... because we don't want to force users to run within the administrator console.

A detailed explanation of the problem can be found here:

#126 (comment)

@smeijer
Copy link
Contributor

smeijer commented May 5, 2015

I've taken a quick look at the issue, and am willing to investigate further in a while.

The first results are unclear, I can just comment out the body of createSymlink and the whole thing works. I guess because you're spawning the correct node binary? (meteor's own?).

So, I can't say for sure why it works without the symlink. Will come back to this in a couple of days, unless someone beats me to it.

I can say, the correct solution for each platform shoud be spawning node, with extra node_modules paths. Then there is no need to create symlinks at all.

@apendua
Copy link
Member Author

apendua commented May 7, 2015

@smeijer Interesting. If we could avoid creating those symlinks it would be awesome.

@apendua
Copy link
Member Author

apendua commented May 23, 2015

@smeijer Any status update on this one?

@rlgreen91
Copy link

I'm following this as well, since I seem to be running into the same issue. I think I mostly understood what you guys discussed in the previous issue, so I'm willing to take a look at the problem if time is an issue.

@smeijer
Copy link
Contributor

smeijer commented Dec 5, 2015

Sorry guys. I totally forgot about this one. I can take a look at it tomorrow.

@apendua, do you prefer me to use a specific branch for this issue?

@apendua
Copy link
Member Author

apendua commented Dec 6, 2015

@smeijer Yes please. Create a new branch from develop.

@rlgreen91 Thanks for being ready to help. Can you please coordinate your effrots with @smeijer? Whould be great if you could solve this guys!

@rlgreen91
Copy link

Sounds good. @smeijer just let me know when you create the branch, so I can fork with confidence.

@smeijer
Copy link
Contributor

smeijer commented Dec 6, 2015

@apendua, I lost the entire day of making this thing work. I was pretty sure it had to work as I expected, and at the end it turned out it did indeed work. I just cannot get the current develop branch to work stable on Windows.

That's why I have committed the changes against master. You are of course free to rebase against develop, or just postpone the merge for somewhere in the future. The change itself is really small.

@rlgreen91, If you are willing to validate that this is indeed the required change, then please do.
npm install -g smeijer/gagarin

Admin rights are no longer required for either installation or for a test run. I'll wait a while with the PR for you to be able to confirm that it's working.

@apendua
Copy link
Member Author

apendua commented Dec 6, 2015

@smeijer Thanks for spending your time on this one.

Maybe it's a good idea to do the PR against master after all. It will be definitely more safe to publish a new version starting from there rather than going from develop too early. Speaking of which, I am a little bit concerned about the problems you experienced related to the stability drop on Windows. Can you share some feedback with me? e.g. logs produced in --verbose mode, or error stacks?

@rlgreen91 When you confirm this update work for you, we should be ready to publish the new version.

@apendua
Copy link
Member Author

apendua commented Dec 6, 2015

@pociej Do you think the problem @smeijer is describing may be related to your recent updates? Can you make sure it works properly on Windows?

@smeijer
Copy link
Contributor

smeijer commented Dec 6, 2015

@apendua, I'll see what I can do. The last commit of @pociej had a little to do with it. But that errors I already fixed. There was a lot more going on, exceptions happening but not being reported, while the same tests succeeded after a reinstall of gagarin. There was also a bug in the cleanup part. But I don't have the logs with me a.t.m. So I'll come back to this with more detailed information.

For your info, this commit added a bug regarding to using an undefined appPath.

@apendua
Copy link
Member Author

apendua commented Dec 20, 2015

@rlgreen91 Can you help us by confirming that the solution implemented by @smeijer works for you?

@Madsn
Copy link

Madsn commented May 14, 2016

Even running gagarin in an elevated prompt is not working for me. Windows 10 64bit, node 6.1.0, npm 3.8.9, gagarin 0.4.12.

 system  searching for test files, using glob pattern `D:/repo/meteor-react-ci-seed/tests/gagarin/**/*.{js,coffee}`                                                                             
  --- added => D:\repo\meteor-react-ci-seed\tests\gagarin\demoTestSuite.js

  added 1 test files ...


  --- building app => D:\repo\meteor-react-ci-seed ---

 system  running meteor build at D:\repo\meteor-react-ci-seed                                                                                                                                   
 system  meteor add anti:gagarin@=0.4.12                                                                                                                                                        
                                                       C:\Users\ace\AppData\Roaming\npm\node_modules\gagarin\lib\mocha\gagarin.js:251
      throw err;
      ^

Error: spawn meteor ENOENT
  at exports._errnoException (util.js:949:11)
  at Process.ChildProcess._handle.onexit (internal/child_process.js:182:32)
  at onErrorNT (internal/child_process.js:348:16)
  at _combinedTickCallback (internal/process/next_tick.js:74:11)
  at process._tickCallback (internal/process/next_tick.js:98:9)

@apendua
Copy link
Member Author

apendua commented May 14, 2016

@Madsn This particular error means that meteor executable can not be found by the gagarin process. Are you sure it's available on your PATH?

@Madsn
Copy link

Madsn commented May 14, 2016

@apendua everything else is working as expected, so I would expect so. I'm able to start up the application by simply typing meteor in cmd.

@Madsn
Copy link

Madsn commented May 14, 2016

@apendua tried manually adding c:\Users\<username>\AppData\Local\.meteor\ to my path, where meteor.bat is located. Hasn't helped unfortunately. Even tried installing meteor as an npm package, but that didn't work either. I'm on meteor 1.3.2.4.

@rlgreen91
Copy link

Sorry I've been out of the loop. I'll start with validating the changes
from December tonight - I'm still running Meteor 1.2 if that helps. If
you'd prefer for me to do something else instead, just let me know.

On Sat, May 14, 2016 at 8:59 AM Mikkel Madsen notifications@github.com
wrote:

@apendua https://github.com/apendua tried manually adding
c:\Users\AppData\Local.meteor\ to my path, where meteor.bat
is located. Hasn't helped unfortunately. Even tried installing meteor as an
npm package, but that didn't work either. I'm on meteor 1.3.2.4.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#128 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants