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

Windows support & race condition fixes #11

Merged
merged 8 commits into from
Jun 3, 2015
Merged

Windows support & race condition fixes #11

merged 8 commits into from
Jun 3, 2015

Conversation

ondreian
Copy link
Contributor

Checks environment for Windows and if it is Windows uses a COMM socket instead of a UNIX socket for setup.

Uses the which module to enforce that the various necessary virtualbox executables are in the $PATH before startup

fixes that mDNS wasn't configured to start on the image I pulled from Tessel.

Also fixes the following race conditions:

  • the VM hasn't started but setup commands are being issued
  • the VM shuts down too quickly for all of the setup to have completed
  • the VM shuts down before the Tessel ID is written to the ~/.tessel folder

I verified it works on my Linux box as well, but I've always been a fan of a double check.

@tcr tcr self-assigned this May 27, 2015
@tcr
Copy link
Member

tcr commented May 29, 2015

Thanks for this pull request @ondreian! This will help immensely here and in t2-compiler.

Can you try pulling this branch and seeing if it works on your machines? https://github.com/tessel/t2-vm/tree/ondreian-master I just want to test the race condition assumptions:

  • The VM hasn't started but setup commands are being issued — This should be obviated by the hostname check (once the shell lists the hostname, you have a shell) but the added wait time is still useful.
  • The VM shuts down too quickly for all of the setup to have completed — If all the commands are written at once over serial, poweroff should be the last command run, triggering the exit event. So writing them in bulk should succeed or fail without need for reading the output.

In any case, I restored the old newline-polling-for-hostname check because it was failing on OS X and confusingly, also my Windows box.

@ondreian
Copy link
Contributor Author

I'm travelling over the next couple of days, and I will see if I can get access to a Linux machine to test here. If not, it may take me around a week to get back to this, just so you are aware I haven't dropped it.

I was unable to explain why the race conditions were occurring, but I could consistently recreate them, and subsequently created remote-exec.js for both debugging (since the ssh problem was occuring, but I could still fake an SSH connection via the serial port), and insuring the integrity of the execution order, however your solution is much more elegant now that we have figured a lot of those oddities out.

I'll update you as soon as I am have the resources to verify that branch again. Thanks!

@tcr
Copy link
Member

tcr commented Jun 2, 2015

@ondreian Thanks for the response! What I'm going to do tomorrow is merge the branch, and create a branch off of that commit that adds your diffs—both for debugging and comparison. I'll leave that open as a separate PR and issue to help development, but merges baseline Windows support asap.

@tcr tcr merged commit 4dcb3a5 into tessel:master Jun 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants