Rewrite in Rust

[jonas-schievink]

"You can build stuff"? Let's see!

Major TODOs:

• Get rid of all the unwraps (Iron does recover from panics, but it's obviously better to do this properly)
  • For the server, use Iron's itry! macro
  • For the bot, regular try, but this one is almost more important since the bot doesn't "survive" a panic while the server does
• Reimplement ASM/LLVM syntax highlighting
  • Probably by manually invoking pygmentize (needs to be installed separately!) - I don't have a nicer solution (we could probably use Ace when LLVM IR support is added, but I don't speak JS so someone else will have to do it)
• Cache executed programs like the python script does
• Make sure this is fast enough
• Rewrite the IRC bot
  • Basic functionality (running code, replying with output or compile errors, /msg and public chat)
  • Put long output on Pastebin bit.ly (and bail on "far too long" output)
  • Implement playbot-mini
  • Allow switching between nightly, beta and stable Maybe in a later PR.

More stuff:

• Non-separate output mode for evaluate-requests on playpen isn't (yet) implemented. I don't know what uses it has, but I can implement it if needed. Is now implemented
• Some unwraps are still questionable, but shouldn't lead to crashes in practice
• ...more I have forgotten, probably

[rust-highfive]

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

[jonas-schievink]

I rewrote (most of) playbot as well. It needs to be run from the project's root dir because it accesses the whitelist.

[alexcrichton]

Awesome! This seems great to me and a great way to help move towards making this runnable on more platforms perhaps?

I'd want to hold off on merging until we've reached feature parity, but I'm all for it!

[brson]

Cool!

Does this just work with the existing playpen sandbox, or does the sandbox need to change? Does this reuse the existing HTML?

[jonas-schievink]

It still uses playpen, I don't plan on changing that (at least in this PR). The HTML also stayed the same (the Rust server is compatible with the Python server, so nothing in the static/ directory should need changes).

[jonas-schievink]

The Playbot port however isn't yet compatible with the config (everything's just hardcoded for now). The IRC interface is different as well. I do plan on changing that, but it might be a good idea to incorporate some ideas from #142.

[jonas-schievink]

Okay, I did a bit more work, and this might now be ready for a first look!

[jonas-schievink]

Ping @alexcrichton, I want to know if I'm going in the right direction with this

[alexcrichton]

Looking pretty good @jonas-schievink! (sorry was a bit busy last week). What's left to implement?

[jonas-schievink]

No worries!

• There's no caching of executed programs
• The "non-separate-output" mode of evaluate.json is missing (I'm curious as to why this even exists - no other endpoint supports this)
• playbot-rs can't identify itself to NickServ
• playbot doesn't support channel keys
  • I don't think the IRC crate supports them, and I don't know what they do. Halp!
• playbot-rs has a hardcoded nickname (I guess this should be moved to the config - at least it shouldn't have the -rs suffix)
• playbot-rs is also unable to handle multiple servers (although I could make it work by passing the name of the config file, then you could run distinct instances)
• it also currently doesn't show warnings if the code still runs fine

Other than that, playbot-rs generally has a different interface.

[alexcrichton]

Thanks for the update! Can playbot-rs have the same interface as playbot? How come they're different?

Also can you be sure to make as much as configurable as possible through a TOML file on the side? I do think that supporting multiple servers will be a blocker for landing this.

I guess I'm not even sure what a channel key is, where're you seeing this again?

[jonas-schievink]

Can playbot-rs have the same interface as playbot? How come they're different?

It was in some way easier to implement (since I don't have to start another thread just for a different nickname), and is much more flexible (release channel selection via ~nightly, ~help, etc.). But if you want, I can implement it the other way.

Also can you be sure to make as much as configurable as possible through a TOML file on the side?

Sure, the only thing that bot.py supports but the new playbot doesn't is the port to connect to. I'll implement that as well.

I do think that supporting multiple servers will be a blocker for landing this.

Would it be sufficient if each server gets its own config file (like I mentioned above)?

I guess I'm not even sure what a channel key is, where're you seeing this again?

I saw this comment and bot.py mention something with an array of "keys" (presumably for each channel), but the irc crate doesn't support them directly (I'd have to tinker with IRC messages directly).

[alexcrichton]

Yeah for now I think we should keep the same interface, and we can look into expanding it later. Currently the python version spawns a bunch of threads for each server/username pair (I think), and it'd be fine for Rust to do the same. I'd ideally prefer to keep it all in one config file and one process as that'd just make it easier to manage.

Ah so the keys there are actually just channel passwords. Currently none of the channels this connects to have passwords, so that can probably be left out for now.

[alexcrichton]

So one thing that'd also be amazing is could you add some tests as part of this as well? We tend to have regressions from time to time and it'd be great to prevent them!

[jonas-schievink]

Hmm. Writing tests for the HTTP server is surprisingly hard, since hyperium/hyper#338 prevents me from closing the server, and Rust doesn't provide a way to do one-time setup before running tests :/

Anyways, everything else should be implemented now. I've added a few tests to make sure the core functionality works, but none specific to the IRC bot or the server.

[alexcrichton]

Yeah if running full integration tests are more difficult than I was thinking that this could have a more specialized set of unit tests (perhaps scattered around as well).

Could you also update .travis.yml to build/test all the Rust code?

[jonas-schievink]

The tests require that the root-* directories are set up, which seems very hard to do from Ubuntu. But I made Travis build all Rust code.

[alexcrichton]

So I guess if we want to rewrite everything we should probably do it to get some concrete benefits rather than "just because". The benefits of Rust I can think of are:

• We understand Rust more than Python
• We can perhaps benefit one day from Rust bindings for dealing with containers rather than forking out to an external process
• Rust code tends to lend it self well to unit testing and such.

I'd prefer to try to write as comprehensive a test suite as possible as part of a rewrite if we're going to do one, and it seems like the tests aren't really that useful if they have the same limitation as the rest of this, which is it only runs in a native Arch Linux environment. Can the tests maybe be reduced in scope, exercise different portions, or perhaps use some mocking to be runnable?

[jonas-schievink]

Can the tests maybe be reduced in scope, exercise different portions, or perhaps use some mocking to be runnable?

They already have a very narrow scope, sadly. By mocking playpen, the tests would become near-useless since they'd just test the .sh scripts and Rust installation on the host (which was already tested by the fact that this has been compiled successfully), and nothing more. The IRC crate seems to provide some sort of mock connection (presumably used for testing the IRC crate), but I don't know how much sense it would make to make use of that (given that we'd still have to deal with the playpen invocation at the end of the chain).

I was thinking of replacing the whole playpen stuff with docker or something else so it can be used on other systems, but that seems clearly out of scope for this PR (I also don't have any experience with Docker).

[alexcrichton]

Yeah we'd actually love to move to a more modern or up-to-date sandboxing solution. Docker may work out but has historically not been recommended for applications like this, we could probably start out by just running on Ubuntu rather than Arch :)

I'll try to give this a closer look over the coming days to ensure we're not losing functionality.

[jonas-schievink]

Ping @alexcrichton, this needs to be updated, since there were a few new features since I last worked on this, but I'd like to know if this is still wanted.

[alexcrichton]

Gah sorry about this @jonas-schievink! I know that I definitely want to still merge this, and I'm somewhat leaning moreso on the side of just doing so without a careful review as it seems that I unfortunately may not have a lot of time to do so. Do you think you'll be on hand to fix up bugs as they arise in the near future perhaps?

I'd also like to consider removing usage of playpen in favor of docker which is easier to run and more understood by us, but that's just facilitated by this PR and can be done elsewhere at all :). In general though that'd also probably break things so we may just want to batch things up to have only one period of breakage.

[jonas-schievink]

Do you think you'll be on hand to fix up bugs as they arise in the near future perhaps?

Sure, no problem!

we may just want to batch things up to have only one period of breakage.

Sounds like a good plan. I'll see if I can update this PR in the coming days so it works correctly with the current master.

[alexcrichton]

Ok, poking around here's some things I've found:

• The mirlexer.py script I believe was added after this PR, so MIR isn't colorized (not a blocker, just a thing)
• Looks like some dependencies (e.g. iron/hyper/staticfile) are a bit out of date
• Could the default features of irc be turned off to trim down dependencies?
• Could logging get enabled somehow as well? Perhaps log + env_logger, but basically logging all requests would be useful
• Can the serde_json dep be dropped? rustc_serialize is already pulled in and should suffice I believe

I've also got a rewrite to use docker instead of playpen as a sandboxing solution built on top of this, which should hopefully re-enable rustfmt again. I haven't tested out the IRC bot much yet but it looks pretty reasonable.

[jonas-schievink]

Okay, I think I've got everything. Regarding MIR highlighting, maybe @birkenfeld's pygments port is now ready enough to be used for that?

[alexcrichton]

Alright, I'm gonna merge this and look to deploying this soon as well, thanks so much again @jonas-schievink! I'll keep you posted with status updates as this gets out.

[alexcrichton]

Ok this is now deployed, both IRC and web. I've pushed a number of updates as well (migrating to docker at the same time)

If all goes well I'll leave it turned on tomorrow and I'll make a post on internals. The old server is still running it just has the services disabled currently.

[alexcrichton]

Thanks again @jonas-schievink! This is awesome :)