Join GitHub today
GitHub is home to over 28 million developers working together to host and review code, manage projects, and build software together.
Sign upRust Iron framework #1101
Conversation
msmith-techempower
reviewed
Oct 17, 2014
| RETCODE=$(fw_exists rust.installed) | ||
| [ ! "$RETCODE" == 0 ] || { return 0; } | ||
| curl -s https://static.rust-lang.org/rustup.sh | sudo sh |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Oct 17, 2014
Member
Is there any chance this could be changed to a permalink with a version?
msmith-techempower
Oct 17, 2014
Member
Is there any chance this could be changed to a permalink with a version?
hamiltont
reviewed
Oct 17, 2014
| [ ! "$RETCODE" == 0 ] || { return 0; } | ||
| curl -s https://static.rust-lang.org/rustup.sh | sudo sh | ||
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hamiltont
Oct 17, 2014
Contributor
Could you add the equivalent of rust --version here, so log output will have the version
hamiltont
Oct 17, 2014
Contributor
Could you add the equivalent of rust --version here, so log output will have the version
torhve
added some commits
Oct 17, 2014
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Oct 22, 2014
Member
@torhve Looks like it's still failing to complile: https://travis-ci.org/TechEmpower/FrameworkBenchmarks/jobs/38420515
CalledProcessError: Command 'cargo build --release' returned non-zero exit status 101
|
@torhve Looks like it's still failing to complile: https://travis-ci.org/TechEmpower/FrameworkBenchmarks/jobs/38420515
|
torhve
added some commits
Oct 24, 2014
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
torhve
Oct 26, 2014
Contributor
Looks like it tests OK now, but the build still fails.
It looks like it might be the verify is failing or setup script? Output from the actualy web testing looks OK.
|
Looks like it tests OK now, but the build still fails. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Oct 27, 2014
Member
@torhve I think this is our toolset having some growing pains - we will try and get this fixed up soon.
|
@torhve I think this is our toolset having some growing pains - we will try and get this fixed up soon. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hamiltont
Oct 27, 2014
Contributor
The Rust work all LGTM. My vote is to merge this PR, but create an issue
indicating that something is failing with our test framework here. It
shouldn't say everything passes and then return 1, that's a bug somewhere
|
The Rust work all LGTM. My vote is to merge this PR, but create an issue |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Oct 28, 2014
Member
@hamiltont Are you saying that you tested this with your travis instance and it worked?
|
@hamiltont Are you saying that you tested this with your travis instance and it worked? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hamiltont
Oct 28, 2014
Contributor
I am saying that the detailed output logs for the last travis build on this
pull request indicate that rust passed all verification tests successfully.
The travis build should have been green, but it looks like there is a bug
causing run tests to return exit code 1 when it should have returning exit
code 0
On Oct 28, 2014 2:37 PM, "Mike Smith" notifications@github.com wrote:
@hamiltont https://github.com/hamiltont Are you saying that you tested
this with your travis instance and it worked?—
Reply to this email directly or view it on GitHub
#1101 (comment)
.
|
I am saying that the detailed output logs for the last travis build on this
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hamiltont
Oct 28, 2014
Contributor
Although I did not look for an error message about the port not being
released by stop . Perhaps that is what is happening here , someone needs
to read the detailed log carefully
On Oct 28, 2014 2:49 PM, "Hamilton Turner" hamiltont@gmail.com wrote:
I am saying that the detailed output logs for the last travis build on
this pull request indicate that rust passed all verification tests
successfully. The travis build should have been green, but it looks like
there is a bug causing run tests to return exit code 1 when it should have
returning exit code 0
On Oct 28, 2014 2:37 PM, "Mike Smith" notifications@github.com wrote:@hamiltont https://github.com/hamiltont Are you saying that you tested
this with your travis instance and it worked?—
Reply to this email directly or view it on GitHub
#1101 (comment)
.
|
Although I did not look for an error message about the port not being
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Oct 28, 2014
Member
I see the exact error message and understand it. Basically, this was pulled before your reworked of our __forciblyEndPortBoundProcesses to have tuple out/err from the communicates. Essentially, we were trying to write to an instance of err that didn't exist because it overwrote the original.
Rant: this is why I hate python sometimes - why can I redeclare variables? Oh, because declaration and assignment are the same. /rant
I agree that this looks like Rust isn't probably stoping; we would have more useful information after a rebase and a rerun after we merge #1145
|
I see the exact error message and understand it. Basically, this was pulled before your reworked of our Rant: this is why I hate python sometimes - why can I redeclare variables? Oh, because declaration and assignment are the same. /rant I agree that this looks like Rust isn't probably |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Oct 31, 2014
Member
@torhve I pulled your latest and there is still an error:
INFO:root:Start exception:
Traceback (most recent call last):
File "/home/techempower/FrameworkBenchmarks/toolset/benchmark/framework_test.py", line 153, in start
retcode = self.setup_module.start(self, out, err)
File "/home/techempower/FrameworkBenchmarks/frameworks/Rust/Iron/setup.py", line 8, in start
subprocess.check_call("cargo build --release", shell=True, cwd="Iron", stderr=errfile, stdout=logfile)
File "/usr/lib/python2.7/subprocess.py", line 540, in check_call
raise CalledProcessError(retcode, cmd)
CalledProcessError: Command 'cargo build --release' returned non-zero exit status 101
|
@torhve I pulled your latest and there is still an error:
|
torhve
added some commits
Nov 4, 2014
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Nov 5, 2014
Member
Ugh, the bleeding edge has cut you again:
techempower@tfbapp3:~/FrameworkBenchmarks/frameworks/Rust/Iron$ cargo build --release
Compiling mysql v0.0.0 (https://github.com/blackbeam/rust-mysql-simple#7934b0ee)
Compiling http v0.1.0-pre (https://github.com/chris-morgan/rust-http.git#c8aef7e0)
src/value.rs:6:16: 6:27 error: unresolved import `std::i64::parse_bytes`. There is no `parse_bytes` in `std::i64`
src/value.rs:6 use std::i64::{parse_bytes};
^~~~~~~~~~~
error: aborting due to previous error
Build failed, waiting for other jobs to finish...
Could not compile `mysql`.
To learn more, run the command again with --verbose.
Additionally, you will need to rebase master into your branch to get it to work.
|
Ugh, the bleeding edge has cut you again:
Additionally, you will need to rebase master into your branch to get it to work. |
torhve
added some commits
Nov 6, 2014
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Nov 10, 2014
Member
I suggest that you clamp down the version being used. Maybe just use v0.12.0 for the time being?
https://static.rust-lang.org/dist/rust-0.12.0-x86_64-unknown-linux-gnu.tar.gz
|
I suggest that you clamp down the version being used. Maybe just use v0.12.0 for the time being? https://static.rust-lang.org/dist/rust-0.12.0-x86_64-unknown-linux-gnu.tar.gz |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
msmith-techempower
Nov 11, 2014
Member
There are still some warnings, and the process does not exit successfully, but we are able to start/run/stop... so I'm considering merging this.
My biggest concern is that if I merge this, the nightly build will break it two days from now.
|
There are still some warnings, and the process does not exit successfully, but we are able to start/run/stop... so I'm considering merging this. My biggest concern is that if I merge this, the nightly build will break it two days from now. |
msmith-techempower
added
PR: Please Update
and removed
PR: Please Update
labels
Jan 22, 2015
msmith-techempower
added
the
PR: Please Update
label
Jan 22, 2015
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Vikaton
commented
Feb 8, 2015
|
any updates on merging this? |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hamiltont
Feb 8, 2015
Contributor
@Ap0ph1s I think the main concern is this line curl -s https://static.rust-lang.org/rustup.sh | sudo sh, becuase it was regularly returning a different version of rust and breaking this PR. I think @torhve updated this multiple times because Rust was moving so quickly.
There's not much value in merging something that's only stable for a week, so IIRC the consensus is that this needs to be linked to one version, but it's unclear which version. If you're interested in seeing Rust results ASAP, feel free to pull this branch into your own repo, fix it to a single worthy version, and send in an updated PR
|
@Ap0ph1s I think the main concern is this line There's not much value in merging something that's only stable for a week, so IIRC the consensus is that this needs to be linked to one version, but it's unclear which version. If you're interested in seeing Rust results ASAP, feel free to pull this branch into your own repo, fix it to a single worthy version, and send in an updated PR |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hamiltont
Feb 8, 2015
Contributor
I see this timeline, perhaps this could be fixed to the beta when it's out?
Rust 1.0.0-alpha – Friday, Jan 9, 2015
Rust 1.0.0-beta1 – Week of Feb 16, 2015
Rust 1.0.0 – One or more six-week cycles later
|
I see this timeline, perhaps this could be fixed to the beta when it's out?
|
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
hamiltont
Feb 8, 2015
Contributor
Oh, I also see that the alpha candidate is supposedly feature complete, so it is not too unreasonable to fix this to the alpha if the beta is taking too long as the upgrade should be painless
|
Oh, I also see that the alpha candidate is supposedly feature complete, so it is not too unreasonable to fix this to the alpha if the beta is taking too long as the upgrade should be painless |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
Vikaton
Feb 8, 2015
Rust is improving every day, whether performance or features, or even both, so I think It can wait until beta1, or the actual 1.0, every version that comes out is more worthy than the last.
Vikaton
commented
Feb 8, 2015
|
Rust is improving every day, whether performance or features, or even both, so I think It can wait until beta1, or the actual 1.0, every version that comes out is more worthy than the last. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
fbernier
May 21, 2015
Rust 1.0 has now been released. I guess it would be worth putting some more work into this.
fbernier
commented
May 21, 2015
|
Rust 1.0 has now been released. I guess it would be worth putting some more work into this. |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
oh, i missed this PR, and wrote up #1636 . Time to compare! |
This comment has been minimized.
Show comment
Hide comment
This comment has been minimized.
|
Closing in favor of #1638 |
torhve commentedOct 17, 2014
No description provided.