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

Make (certain) environment variables available at build time #25

Open LukasKalbertodt opened 7 years ago

LukasKalbertodt commented 7 years ago

Right now, this buildpack always builds with --release. But there are situations in which one might want to build in debug mode... or at least with debug symbols enabled.

Maybe this could be added as a configuration in RustConfig?

emk commented 7 years ago

Thank you for trying out the build pack!

Rust performance can be shockingly terrible in debug mode. It's sufficiently bad that I don't want to support debug mode, because then people will turn it on, and then they'll file bugs that I have to look at (or they'll be disappointed with Rust).

Much better to turn on symbols in release mode, so that things perform acceptably. So I think the right solution here is to use:

[profile.release]
debug = true

...in your project's Cargo.toml. Please try this and let me know if it works for you.

LukasKalbertodt commented 7 years ago

Well, thanks for providing the build pack :wink:

For my application, I have a build script which compiles assets (e.g. compiling .less files to .css). Currently I test env::var("PROFILE") in my build script to decide whether or not to minify the resulting CSS. I know that this is far from optimal, but it seemed like an easy way to achieve what I want. And yes, I also want to have a development-heroku service with non minified css.

In the script I also check the ROCKET_ENV variable (which would make a lot more sense than using PROFILE), but those are not set for the build script on heroku. I only set those in Procfile: this means my application can read them, but not the build script. I could set the ROCKET_ENV variable on the Heroku website, but as I understand it, this only applies to the running application, not the build environment. That's the reason you reexport DATABASE_URL, right?

What do you think about reexporting ROCKET_ vars, too?

So: I can completely understand your reasoning and I think you're right. I asked for a slightly different reason, as you can see.

emk commented 7 years ago

Hmm. Unfortunately, I don't understand the ROCKET_ENV stuff well enough to know whether this would be the right approach or not. It feels gross to explicitly mention a specific framework in the build pack. Is there maybe another way to tackle this issue?

Personally, I would consider leaving CSS unminified only in the local dev environment, and always minifying it on Heroku. But that might not work for all workflows, I suppose.

LukasKalbertodt commented 7 years ago

It feels gross to explicitly mention a specific framework in the build pack. Is there maybe another way to tackle this issue?

I agree. Maybe the user could specify a list of var-names in the RustConfig and those vars are then being re-exported for the build step?

If you want to read about ROCKET_ENV, you can do so here. But I think there is no need to gain framework specific knowledge. I just guess that in the Rust world (where we have a strong focus on compile time), it's not uncommon that the build step needs access to environment variables. So maybe the buildpack should really offer a possibility to reexport certain env-vars?

emk commented 7 years ago

I just guess that in the Rust world (where we have a strong focus on compile time), it's not uncommon that the build step needs access to environment variables. So maybe the buildpack should really offer a possibility to reexport certain env-vars?

Yeah, that seems like a pretty reasonable argument. Rather than add a configuration knob (I hate configuration knobs), maybe we could just export variables using the export_env_dir script from this page? That seems pretty reasonable and it shouldn't break anything.

Would you by any chance have time to work on a PR implementing this? I have a big stack of other high-priority open source issues I need to look at soon, and a new buildpack feature like this might take weeks to bubble to the top of my list.

Thank you for looking into this!

LukasKalbertodt commented 7 years ago

we could just export variables using the export_env_dir script

But who decides what to export? In other words: who sets the whitelist and blacklist variables in the script?

  1. Do you think the user should provide the whole script?
  2. Would you just re-export all env-variables except for maybe a few ones?
  3. Would you let the user set env-variables like BUILD_STEP_REEXPORT_BLACKLIST which are used by the script?
emk commented 7 years ago

But who decides what to export? In other words: who sets the whitelist and blacklist variables in the script?

The default appears to export everything that isn't on the blacklist. I'd be OK with that. I suspect we could just add that script to the buildpack unchanged, and call it at the appropriate time, and that would work out OK. We might need to tweak the blacklist for some Rust-specific stuff. But as I mentioned above, I strongly dislike adding configuration options, and I would prefer to find a one-size-fits-all solution that Just Works™ without the user doing anything.

jrc-dev3 commented 2 years ago

Opened a PR #63 for this issue - hoping it suffices, @emk.