pantsbuild / scie-pants

Protects your Pants from the elements.
https://www.pantsbuild.org/docs/installation
Apache License 2.0
18 stars 17 forks source link

Silence stdout/stderr during Pants setup. #375

Closed kaos closed 3 months ago

kaos commented 3 months ago

Closes pantsbuild/pants#20315 Broken out from #351

kaos commented 3 months ago

is this something we can add a test for so it doesn't regress?

I'm sure we could.. do you think it's worth looking into?

I assume it's the part about not leaking output to stdout during bootstrapping.. checking the presence of the pants-install.log file could also be done, but perhaps trickier as we'd need to find out the path.

In that case, it feels like we might prefer looking into adding support for something to query the lift runtime for the various resulting paths etc. perhaps something like SCIE=lift.expand scie-pants scie.bindings.

huonw commented 3 months ago

I'm sure we could.. do you think it's worth looking into?

I assume it's the part about not leaking output to stdout during bootstrapping..

Yeah, just keeping stdout clean. My preference would be yes, but if it's a tonne of effort, maybe not worth it? 🤷

kaos commented 3 months ago

I'm sure we could.. do you think it's worth looking into? I assume it's the part about not leaking output to stdout during bootstrapping..

Yeah, just keeping stdout clean. My preference would be yes, but if it's a tonne of effort, maybe not worth it? 🤷

I had a long reply that started out in favour of not testing this, but which in the end concluded that a test is worthwhile, as we want to ensure stdout is clean, regardless of what is being done during bootstrap, so it would protect not only against this fix in particular, but against all future changes that introduces new stuff during bootstrapping as well. I'll look into it :)

kaos commented 3 months ago

@huonw OK, I've added a test verifying bootstrap doesn't pollute stdout. PTAL :)