Refactor buildpack output

[runesoerensen]

No description provided.

[schneems]

Seems like a bug in fun-run. I filed the first ever issue on the repo! schneems/fun_run#13

Also I think there be backticks around the command i.e.

Running `dotnet publish /workspace/foo.csproj --runtime {rid} -p:PublishDir=bin/publish --verbosity normal`

[runesoerensen]

Huh I thought it was intentional :) I believe it's the regex here that causes the -p:PublishDir=bin/publish argument to be quoted -- and more specifically the = character.

If this should be allowed, maybe the regex needs to be updated to something like ([^A-Za-z0-9_\-.,:=/@\n])? Not sure it's a bug though.

[runesoerensen]

Also, yes there should be backticks around the command - and there is (since 4cad9ea which included this change).

You're commenting on an old commit :)

[schneems]

We can close this conversation, it's more an FYI than a blocker

The regex was a "works for now" solution and as they say...there's no such thing as a temporary fix. I just hadn't hit any situations where it didn't give me what I wanted.

FWIW if you still want to use something like https://docs.rs/bullet_stream/0.5.0/bullet_stream/global/print/fn.sub_stream_cmd.html (or the CmdError enum), a core feature of fun_run is the ability to re-name commands, I would still like to investigate and fix this upstream, but if you ever need an escape valve like for just one function you can always BYO naming, via named or named_fn like:

sub_stream_cmd(cmd.named_fn(command_to_string));

[schneems]

The current output won't produce entirely valid copy/pasteable commands (which is the goal, so users can run them locally and debug/reproduce) in the event that one of the args has a space in it i.e. if someone's path to their cs proj has a space

dotnet publish /workspace/path to a/Foo.csproj

Will be interpreted as dotnet publish /workspace/path rather than dotnet publish " /workspace/path to a/Foo.csproj" or /workspace/path\ to\ a/Foo.csproj

[runesoerensen]

Good catch! Fixed in ed3b93e

[schneems]

Hmm...this commit added quotes back in:

dotnet publish /workspace/foo.csproj --runtime {rid} '-p:PublishDir=bin/publish' --verbosity normal

[schneems]

While you're poking at alternative interfaces, this is another one https://docs.rs/bullet_stream/0.5.0/bullet_stream/global/print/index.html

[runesoerensen]

While you're poking at alternative interfaces, this is another one https://docs.rs/bullet_stream/0.5.0/bullet_stream/global/print/index.html

@schneems I did see that change and it looks interesting for sure, but the interface itself isn't the reason I'm looking to refactor the output implementation here. The primary reason is that I want to be able print output in different styles (e.g. both our CNB style and the classic buildpack style) so I don't have to reformat the CNB output in the classic buildpack.

This PR currently uses the code from the JVM buildpack PR, but that has mostly been to help test that implementation, and verify the implementation of the bullet_stream output spec in a buildpack that already uses it. However, my plan is to copy the JVM buildpack implementation to this repository, and add the relevant logic to allow configuration of the "output style" (prefixes, header, warning and error styles etc) based on an environment variable.

I figured it's a bit out of scope for both the shared JVM output module and the bullet_stream crate to support different output styles, so that's why I'm looking to migrate the code to a somewhat more lean implementation (e.g. I can just copy the output.rs file and make my changes to that). The goal was really just to avoid increasing the complexity of other implementations to support my very specific use case -- that doesn't mean it can't be done (and actually the stateful implementation of bullet_stream::Print would likely allow for a relatively elegant way to configure different log formats), I'm just not sure it's worth the effort/maintenance burden :)

[schneems]

I would like to pave a path towards deprecating and ending maintenance of my classic buildpack, even if that horizon is 2+ years, even if Classic never goes away. From that angle, I don't think that supporting output for a classic buildpack is entirely out of scope.

I'm not sure where the boundry should be though, nor am I entirely aware of the UI/UX problems faced in running a CNB buildpack alongside of classic. I don't think there's much consistency, rhyme or reason with between the ---> and leaders. I think the bullet points look better than classic, but I'm also a bit biased, I guess I'm saying I would try to see what the minimum change to not make it look "off" would be, even if it stands out a bit from other buildpacks. I'm guessing indentation is an issue.

One way to hack around indentation could be by providing a writer that added indentation, something like:

if is_classic()  {
    let writer = libcnb::line_mapped(&mut std::io::stderr(), |line| format!("       {line}"))
    bullet_stream::global::set_writer(writer);
} 
let mut log = Print::global().h2("Heroku .NET Buildpack");