-
Notifications
You must be signed in to change notification settings - Fork 56
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
Adds Rust deployment capabilities #774
Conversation
Adding @nplus11's work on Rust init
My debugging has proved fruitless... I thought it was because post_req.end was never called, but it is, so that's not it. I do know we reach the L175 block in index.js (Data Complete State) and it starts running the dataFill. Theories:
I don't understand why Regardless, I can't make any progress now. Any help is appreciated. |
You'll want to run Also, ping @johnnyman727 |
… only) Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
…ms (where unnecessary) Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
* t2 init: broad cleanup Signed-off-by: Rick Waldron <waldron.rick@gmail.com> * t2 init: more tests for --lang=javascript Signed-off-by: Rick Waldron <waldron.rick@gmail.com> * t2 init: per review: not necessary to explicitly coerce lang input to string Signed-off-by: Rick Waldron <waldron.rick@gmail.com> * t2 init: init.rs.createSampleProgram use fs.copy Signed-off-by: Rick Waldron <waldron.rick@gmail.com> * t2 init: special *Error classes are not exported. Signed-off-by: Rick Waldron <waldron.rick@gmail.com> * t2 init: reuse rust resource paths Signed-off-by: Rick Waldron <waldron.rick@gmail.com> * t2 init: cleanup all resource paths Signed-off-by: Rick Waldron <waldron.rick@gmail.com>
I've got grunt happy up to testing @johnnyman727 will need to take it from here. |
Reader({ | ||
path: opts.target, | ||
type: 'Directory' | ||
}) | ||
.on('error', reject) | ||
.pipe(outgoingPacker) |
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.
Are outgoingPacker and post_req the wrong way round here?
@cflewis I had a few hours to dig into the issue tonight. Ultimately, I just patched the cross compilation server to archive the binary on its end before passing back to the CLI which, I think, is the cleaner way to do it. I opened a PR on your branch with the fix but I'm going to keep adding to my PR to make the code more robust (you currently can't control+c to abort once the LEDs start blinking for some reason). |
@rwaldron Can you confirm this is rebased as you expect it to be? |
@johnnyman727 : I'm not seeing this work, although there may be some sort of user error :/
Something isn't right :/ |
@cflewis I'm not sure why you're hitting that. Here's was the output looks like for me:
It looks like your execution didn't even reach the cross compilation server before it failed. It's failing somewhere between running the tar command and the call to compile the bundle remotely. Are you sure you have a valid Also, when I look at the git history, it appears to be rebased but Github still shows a bunch of commits that are already in |
Why do you have |
I'm still seeing the same thing (the /hello/git/master/ is just a shell display thingy) with a fresh clone, using a binary of
|
Here's the problem: the path that is being sent to the
The TOML file is one level up. I'm going to try and see why this string is built wrongly. |
Even when I put the TOML there, something barfs in |
exportables.preBundle = function(opts) { | ||
return new Promise((resolve, reject) => { | ||
// Get details of the project | ||
var details = exportables.meta.checkConfiguration(opts.target); |
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 is where I get my failure, even if the TOML is copied.
opts.target
is set to "/Users/cflewis/Downloads/hello/src"
, when checkConfiguration
is expecting three arguments. When L31 is triggered, the path
package barfs as program
is undefined. Question is why opts
is only one rather than three things, and that I did not have time to work out before I had to stop.
My
I think the issue is actually here where we assume that the Cargo.toml is in the same directory that you pushed from. Is it possible that you are deploying from within YOUR_PROJECT/src instead of YOUR_PROJECT? Perhaps we need to write some logic to traverse up from the |
No, I'm not doing that :/ Here's the exact commands I'm running:
|
Ah I figured out the issue. You are likely using Node 5.x or Node 6.x which changed the The CLI officially only supports Node LTS (4.x for now) but I'll make a quick addition to resolve this. It doesn't matter too much because Rust doesn't use the |
Although, we don't have a version of the |
Aha! We will need to make this clearer in the README for Rust types, most of them I have seen use Mac OS X, and if you're using Homebrew, I have new weirdness instead :( My compilation server is up, and I can talk to my Tessel, but something isn't communicating correctly:
|
Try adding "http://" in front of the IP address. |
Awesome, adding HTTP does indeed make this work. I'll add a patch that'll force http:// if not provided. |
I'm happy for this to land into master now, pending the CI checks being made happy (which I do not think I have permission to see as I don't have write permission to t2-cli). I think I've blundered into most of the edge cases :) |
@@ -86,6 +86,10 @@ exportables.remoteRustCompilation = (opts) => { | |||
var buffers = []; | |||
|
|||
// Parse out the components of the remote rust compilation server | |||
if (!opts.rustCC.startsWith('http://')) { |
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.
You may want to also check that it starts with https://
in case someone deploys their own cross compilation server somewhere behind SSL (or if we do that...).
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.
Done
@cflewis after addressing my two comments, can you re- @rwaldron has requested that we merge #787 before this one. It cleans up some of the deploy process but shouldn't affect our work with this PR. After that, we'll need one more review before we can merge. @rwaldron are you up for that? |
And, as discussed in this issue, add yourself to the contributor list! |
For convenience:
|
I must be doing something wrong here with the rebasing (yay Git).
and GitHub still shows the commits in this PR :( Do you want to squash this PR into one commit? That might get us the right UI display at least. |
Yeah, sounds good. |
THIS CODE DOES NOT WORK YET
This PR contains code that fixes up some of jon-rust-deploy-2 so that it seems to do the right thing, but it hangs waiting for the rust cross-compliation server. The server itself seems to be getting request and returning the response just fine, so there's something here that's waiting and I don't know why. My Javascript is not great, and the asynchroncity here makes things worse for me.
I'm happy to take comments as to what to do next, or just have this PR pulled into the branch and @johnnyman727 continues with it.