search

emk / heroku-buildpack-rust

A buildpack for Rust applications on Heroku, with full support for Rustup, cargo and build caching.
522 stars 186 forks source link

Use rust-toolchain instead of RustConfig #29

Closed anp closed 6 years ago

anp commented 6 years ago

Something pretty similar to this is currently powering rusty-dash.com after I moved it over to Heroku last night/this morning. I think it should be backwards compatible with RustConfig's VERSION setup as well.

Fixes https://github.com/emk/heroku-buildpack-rust/issues/11

emk commented 6 years ago

This looks great! Unfortunately, I'm in meetings for the next couple of days and won't have time to test this myself, so I might need one more favor before merging this.

I think it should be backwards compatible with RustConfig's VERSION setup as well.

Could you please test the backwards-compatibility support, and confirm that it still works? I mean, obviously the code looks correct, but I don't want to risk people in the field by merging a cool patch without testing it. :-)

anp commented 6 years ago

Makes sense! (reminds me how nice it would be for Heroku to let you pin a custom buildpack to a commit/tag!)

In my fork, I created a RustConfig file with VERSION=1.20.0 as the contents, and ran ./test_buildpack:

 ~/c/heroku-buildpack-rust   master  heroku-rust-cargo-hello  nano RustConfig
 ~/c/heroku-buildpack-rust   master  heroku-rust-cargo-hello  cd ..
 ~/c/heroku-buildpack-rust   master  ./test_buildpack                     Wed Nov 15 23:55:54 2017
WARNING: The GROUPS variable is not set. Defaulting to a blank string.
Starting herokubuildpackrust_testbuild_1 ...
Starting herokubuildpackrust_testbuild_1 ... done
Attaching to herokubuildpackrust_testbuild_1
testbuild_1  | -----> Checking for new releases of Rust 1.20.0 channel
testbuild_1  | info: checking for self-updates
testbuild_1  | info: syncing channel updates for '1.20.0-x86_64-unknown-linux-gnu'
testbuild_1  | info: latest update on 2017-08-31, rust version 1.20.0 (f3d6973f4 2017-08-27)

Looks like the correct setting got picked up, since the latest stable on my machine (and what I've built with this container before) is 1.21.0.

anp commented 6 years ago

Sorry, forgot to comment here after I pushed a new commit. Addressed the requested changes (I think).

anp commented 6 years ago

Ping! Hope you're well. Would be nice to unfork :).

emk commented 6 years ago

CC @Floppy

The only thing blocking this PR is a one-line fix mentioned here. If anybody wants to fix it, I'd be happy to merge the change. (I'd do it myself, but I'm on deadline for work stuff.)

anp commented 6 years ago

Hey! Sorry about that, not sure what happened to those notifications. Just pushed a fix!

emk commented 6 years ago

Thank you! Merging. Let me know if you have any problems.