Skip to content

Add a release script #31

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

pusherofbrooms
Copy link

It appears that modern cloud foundry requires a simple release script in order to run.

@emk
Copy link
Owner

emk commented May 13, 2018

Thank you for this patch!

Normally, this buildpack is supposed to be used with an explicit Procfile, as described near the end of this section. This is done for several reasons:

  • I don't want to invoke cargo at application startup time. Ideally, we should run the pre-built Rust application directly, and never invoke any compiler machinery.
  • There's no easy way to predict the name of the Rust application (at least not that I know of). I might be possible to ask Cargo to dump project information as JSON and use that to create a shell script which launches the application.

Do you know if it's possible to work on Cloud Foundry without a release script if you provide a Procfile as described in the docs?

Thank you for suggesting this, and I appreciate any help you can provide! I would definitely like to support Cloud Foundry.

@pusherofbrooms
Copy link
Author

Buildpacks don't seem to work in cloud foundry without a release script these days. I tested with your heroku-rust-cargo-hello which includes a Procfile.

I don't see an elegant way not to use "cargo run", but I haven't dug deeply.

I looked to http://engineering.pivotal.io/post/creating-a-custom-buildpack/ for inspiration as well as the release script that comes with the go buildpack. Their release scripts are over-complex in my opinion given that "cargo run" might be all you ever need with rust.

@emk
Copy link
Owner

emk commented May 14, 2018

Honestly, I don't know why cargo run even works. In theory, we should never include cargo or rustc in the compiled slug, and so attempting to use it should simply fail. If we do include the development tools at runtime on Heroku, that's a bug, and possibly a regression, because including large binaries slows down spin-up times and quickly exceeds various quotas and limits, IIRC.

Maybe CloudFoundry is doing something really weird?

In any case, the more that I think about it, the more that I'm convinced that using cargo run would break badly on Heroku, and it would probably give confusing errors to users.

In order to accept this PR:

  1. We'll need something that works on Heroku.
  2. We can't include cargo or rustc into the built image.
  3. If the user doesn't supply a Procfile on Heroku, we don't want to give them an error message that's any more confusing than what we do now.

If CloudFoundry is fundamentally very different from Heroku under the hood, you might just want to fork this buildpack and make a CloudFoundry-specific version. Or we could figure out how to ask cargo for the actually executable name & location, and generate a script that can be called from a Procfile. I wish I had a better answer for you, but this might not be an easy problem. :-(

@emk emk changed the title Add a release script for cloud foundry Add a release script Dec 9, 2018
@arve0
Copy link

arve0 commented Jun 15, 2019

There's no easy way to predict the name of the Rust application (at least not that I know of).

According to cargo issue 1706 one can provide output file name with cargo rustc -- -o filename.

@arve0
Copy link

arve0 commented Jun 16, 2019

So I've checked cargo rustc, and one can control output filename somewhat, thought it adds a hash:

$ mkdir output-filename
$ cd output-filename/
$ cargo init
    Created binary (application) package
$ cargo rustc -v --release -- -o filename
  Compiling output-filename v0.1.0 (/private/tmp/output-filename)
    Running `rustc --edition=2018 --crate-name output_filename src/main.rs --color always --crate-type bin --emit=dep-info,link -C opt-level=3 -o filename -C metadata=c961d75f8bcf3c48 -C extra-filename=-c961d75f8bcf3c48 --out-dir /private/tmp/output-filename/target/release/deps -L dependency=/private/tmp/output-filename/target/release/deps`
warning: due to multiple output types requested, the explicitly specified output file name will be adapted for each output type

warning: ignoring --out-dir flag due to -o flag

warning: ignoring -C extra-filename flag due to -o flag

    Finished release [optimized] target(s) in 0.39s
$ ls
Cargo.lock          filename-c961d75f8bcf3c48   src
Cargo.toml          filename-c961d75f8bcf3c48.d target

Not sure if the hash is a bug, ref the warning telling -C extra-filename is ignored. Anyhow, one can control this by moving the output:

$ mv $(ls filename* | grep -E 'filename-\w+$') filename
$ ls
Cargo.lock			filename			src
Cargo.toml			filename-c961d75f8bcf3c48.d	target

If you are interested, I can create a PR using this technique when a Procfile is missing.

@emk
Copy link
Owner

emk commented Jan 26, 2020

I would be interested in PR using this technique, yes!

(Sorry for the slow replies; I'm maintaining too many open source projects and GitHub hasn't let me into Notifications Beta yet, so it's really easy for things to slip through the cracks.)

Copy link
Owner

@emk emk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Held pending changes described in issue. Thank you!

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.

3 participants