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

Update Rust version (was: Remove Rust) #23

Closed
wants to merge 1 commit into from

Conversation

steveklabnik
Copy link

The Rust being used is incredibly out of date. I took a stab at updating it, but there have been significant changes in the meantime, and so I think that it's best to just simply remove it, so that it's not giving the wrong impression.

steveklabnik added a commit to steveklabnik/hookrace-web that referenced this pull request Mar 4, 2015
To go along with kanaka/mal#23, I don't think that the Rust example here is representative, and so is better to just remove.
@Ryman
Copy link

Ryman commented Mar 4, 2015

@steveklabnik this is adding a Cargo.lock?

@steveklabnik
Copy link
Author

@Ryman whoops!

@steveklabnik
Copy link
Author

Fixed!

@kanaka
Copy link
Owner

kanaka commented Mar 4, 2015

I remembering now that I have gone to update this a few times in past but ran into blockers so I put it aside each time.

I started working on it again just now with rust 1.0.0.alpha.2, but it looks like I still don't have a regex solution that works. The cadencemarseille-pcre module I was using no longer builds (no surprise since upstream hasn't been updated since rust 0.12). And one of the two bugs I filed against the standard regex crate way back is still unresolved: rust-lang/regex#28 And I don't seem to be able to build the regex crate anyways (this is probably something I'm doing wrong, but I'm at a bit of a loss):

$ cargo build
    Updating registry `https://github.com/rust-lang/crates.io-index`
 Downloading regex v0.1.17
   Compiling regex v0.1.17
/home/joelm/.cargo/registry/src/github.com-1ecc6299db9ec823/regex-0.1.17/src/re.rs:15:16: 15:23 error: unresolved import `std::str::Pattern`. There is no `Pattern` in `std::str`
/home/joelm/.cargo/registry/src/github.com-1ecc6299db9ec823/regex-0.1.17/src/re.rs:15 use std::str::{Pattern, Searcher, SearchStep};
...

Any suggestions for how I should address the regex issues? I would strongly prefer pcre (since most implementations use basically the same regex string to tokenize), however, I'm willing to use any regex that works correctly.

@Ryman
Copy link

Ryman commented Mar 4, 2015

@kanaka that regex version tracks nightly, you'll need to use a nightly version of rust not alpha2, you may find https://github.com/brson/multirust helpful.

@kanaka
Copy link
Owner

kanaka commented Mar 4, 2015

@Ryman ah, awesome. That's a good one to know about.

@steveklabnik
Copy link
Author

Yeah, I worked for a while on just updating it, but it's not easy. Wish I could have just said "here's Rust HEAD working" :/

I'm not sure of an up-to-date PCRE library, but @BurntSushi might have thoughts about fixing the blocking bug.

@runvnc
Copy link

runvnc commented Mar 4, 2015

@steveklabnik So what you are saying is that your programming language is so hard to use and changes so frequently that you, a core contributor, was unable to update another Rust programmer's code and had to give up? Its too hard for you?

Maybe you guys could put a team together, and within a week or two crank out an updated a Make a Lisp.

But don't put it all on kanaka.

What a joke.

@Manishearth
Copy link

As a pre-alpha language, I see nothing wrong with that.

Yes, now Rust is alpha and soon to be beta (~1 month), and it can be used without getting broken (much), but it wasn't at the time of that code being written.

I don't see him putting anything on kanaka.

@kanaka
Copy link
Owner

kanaka commented Mar 4, 2015

@runvnc well, to be honest, there are a couple aspects to the current mal implementation that make updating more than just a few transformations. First it's using FFI to call readline which is an API which has changes a fair bit and isn't stable yet. But the bigger issue and probably the thing that made @steveklabnik throw his hands in the air was that I pull in cadencemarseille-pcre in a bit of an odd way (I was trying to work around some odd cargo behavior back then). That and that pcre lib is completely out of date (even more than the mal impl). So I can understand the frustration on his part.

Some context: I think is that Rust often gets unfairly criticized for not having the performance that they claim and this was partly what @steveklabnik was reacting to. The author of the Nim implementation was talking with me about performance and I gave him a scratch pad of microbenchmarks (really bad benchmarks it must be said) to allow some level of comparison for Nim. Unfortunately, I should have made clear that they were mostly bogus and only a really rough guide and they ended up in a blog post. And I think that's when the rust guys got wind of it. I can understand not wanting an old version of Rust as any sort of baseline for performance comparisons. But performance comparison is not a goal for mal (not yet anyways). So I'm not going to remove it, but I would of course like to update it. I've just been blocked by regex issues.

@kmcallister
Copy link

you, a core contributor, was unable to update another Rust programmer's code and had to give up

Maybe he has better things to do? Rust updates are not impossible, I've been doing them on Servo (100+ KLoC) since July 2013, when the language was changing much more rapidly than now. But they aren't a trivial amount of effort, either. Nobody is claiming otherwise, pre-1.0.

But don't put it all on kanaka.

I didn't see where he did that. @steveklabnik is trying to be helpful given limited resources, and you are being massively entitled and whiny about it.

@BurntSushi
Copy link
Contributor

@kanaka I didn't realize that bug in regex was blocking you. I had considered it pretty low priority otherwise. I'll move it to the top. :-)

@kanaka
Copy link
Owner

kanaka commented Mar 4, 2015

@BurntSushi cool, that would be awesome! And it wasn't blocking me back at rust 0.13 because I could use the cadencemarseille-pcre module. But the priority has escalated a bit as you've probably noticed ;-)

@Manishearth
Copy link

If the regex library gets updated I could try rustupping this over the weekend if I get time.

@kanaka
Copy link
Owner

kanaka commented Mar 4, 2015

@Manishearth "rustupping"...nice. That would be awesome. I have a big class project due in a PhD class this weekend so I was going to have to try and fit this in somewhere. BTW, I hang out in #mal (freenode) with a couple of other mal implementors if you want to discuss. I'm afk for the night though.

@kanaka
Copy link
Owner

kanaka commented Mar 4, 2015

@Manishearth also, pcwalton left a few comments about improving performance on the HN thread: https://news.ycombinator.com/item?id=9145360

@Manishearth
Copy link

Yeah, I saw those. Couple of those are things that clippy can pick up if I add the functionality, so once we rustup I can add some lints for performancy things and run mal through it.

@kmcallister
Copy link

Oh what a surprise, @runvnc is trolling against Rust on HN too.

I like how you're lecturing about "the reality of Rust development and Rust code" to people who work on Rust and Servo and other hundred-KLoC codebases.

@runvnc
Copy link

runvnc commented Mar 5, 2015

I'm not trolling. I just think you people live in a different reality. I will leave you to it.

@kanaka kanaka changed the title Remove Rust. Update Rust version (was: Remove Rust) Mar 5, 2015
@steveklabnik
Copy link
Author

Superceded by #26

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.

None yet

7 participants