heroku / libcnb.rs

A framework for writing Cloud Native Buildpacks in Rust
BSD 3-Clause "New" or "Revised" License
36 stars 6 forks source link

Cannot write a basic web process with libcnb #49

Closed schneems closed 3 years ago

schneems commented 3 years ago

Reproduction

Description

This in rust:

let process = Process::new(
    "web",
    String::from("while true; do echo 'lol'; sleep 2; done"),
    vec![String::from("")],
    false,
)?;
let launch = Launch::default().process(process);

context
    .write_launch(launch).unwrap();

Should be the same as this in bash:

cat >> "${layers_dir}/launch.toml" <<EOL
[[processes]]
type = "web"
command = "while true; do echo 'lol'; sleep 2; done"
args = [""]
direct = false
EOL

But it's not. The bash version works, the rust version does not.

schneems commented 3 years ago

Closing this, Ed pointed out the examples are using different specs of CNB (0.2 vs 0.4). When I switch the bash version to 0.4 it fails too. Ed pointed out this change https://github.com/buildpacks/spec/releases/tag/buildpack%2Fv0.4 to arg parsing. If I boot with empty args (instead of a vector with an empty string) then it works.

While it looked like it was a bug in lifecycle, Go and Ruby both treat an empty arg the same:

irb(main):005:0> pid = Process.spawn("while true; do echo 'lol'; sleep 2; done")
=> 13083
irb(main):006:0> lol

^C
irb(main):006:0> pid = Process.spawn("while true; do echo 'lol'; sleep 2; done", "")
(irb):6:in `spawn': No such file or directory - while true; do echo 'lol'; sleep 2; done (Errno::ENOENT)

I think that while the lifecycle launcher is technically correct, the error is confusing and difficult to debug.

Also looking at the output:

$ pack inspect cutlass_image_basic_bash | grep -B2 web
Processes:
  TYPE                 SHELL        COMMAND        ARGS
  web (default)        bash         while true; do echo 'lol'; sleep 2; done

It's unclear that it's supplying the command with empty args (versus no args). If you put an actual arg the formatting changes:

Processes:
  TYPE                 SHELL        COMMAND                                         ARGS
  web (default)        bash         while true; do echo 'lol'; sleep 2; done        foo

Or if you supply no/empty args:

Processes:
  TYPE                 SHELL        COMMAND        ARGS
  web (default)        bash         while true; do echo 'lol'; sleep 2; done
  1. If args [] versus [""] provides different behavior, then the output of inspect should be different between the two. I think this is definitively a bug.

One idea is to include the arg count in the output (as well as align the args to not be on top of command:

Processes:
  TYPE                 SHELL        COMMAND                                         ARGS (count = 0)
  web (default)        bash         while true; do echo 'lol'; sleep 2; done        

That would tell us when there are no args versus some args, but still might make it tricky to understand that an empty string is being used:

Processes:
  TYPE                 SHELL        COMMAND                                         ARGS (count = 1)
  web (default)        bash         while true; do echo 'lol'; sleep 2; done        

We could special case empty strings to instead show an empty quote:

Processes:
  TYPE                 SHELL        COMMAND                                         ARGS (count = 1)
  web (default)        bash         while true; do echo 'lol'; sleep 2; done     ""
  1. There's also an opportunity to improve this failure case. I would like a warning (since this used to work before).
"WARNING You've specified to boot a process with an empty argument, which may have unexpected results"

If we can visibly show we're passing args "" to the command in the pack inspect output that would be "good", adding a warning/error would be "better/best".