NewLayerContributor does not log expected logs

[sambhav]

What happened?

NewLayerContributor does not accept a logger instance. As a result bard.Logger does not have a proper file descriptor to log to. As a result contributors created using this don't show proper logs during the build process around caching etc.

• What were you attempting to do?
  Use NewLayerContributor
• What did you expect to happen?
  See logs around it
• What was the actual behavior? Please provide log output, if possible.
  No logs

Build Configuration

• What platform (pack, kpack, tekton buildpacks plugin, etc.) are you
  using? Please include a version.
  pack and kpack
• What buildpacks are you using? Please include versions.
  Internal
• What builder are you using? If custom, can you provide the output from pack inspect-builder <builder>?

Checklist

• I have included log output.
• The log output includes an error message.
• I have included steps for reproduction.

[sambhav]

We should probably add an argument to the layer contributor with the logger or default it to some sane value like os.Stdout

[dmikusa]

The issue with changing the signature of NewLayerContributor is that it would break the API and we'd have to a major version bump of the library. It's something we could do, but I don't think it's something we want to do at this time.

It's a little annoying, but what we typically do when using this API is to just set the logger directly after.

	lc := NewLayerContributor(d.Name(), d.ExpectedMetadata, d.ExpectedTypes)
	lc.Logger = d.Logger

Probably the only thing I could see us addressing here in the short to medium-term would be adding a net new method, like NewLayerContributorWithLogger or NewLayerContributorWithDefaultLogger. If that would be of interest let me know. That would be easy to add.

[c0d1ngm0nk3y]

Defaulting it to os.Stdout sounds reasonable to me. That alone should not break anyone, right?

Having some possibility to set the logger like WithLogger() or NewLayerContributorWithLogger might be nice, but since Logger is already public, there would not be a real benefit.

[dmikusa]

My issue with defaulting this to os.Stdout is that it changes the behavior. I know there are times where we intentionally suppress logging. If we change the default, it could alter some of this behavior.

I actually almost closed this issue out the other day because in libcnb v2 we removed the concept of layer contributors entirely. I do want to add them back in libpak v2, because I think they are valuable and it'll help Paketo with upgrading to v2, but at that point we can change the API and ensure that loggers are being passed around correctly. That's ultimately why I didn't close this out, cause I wanted to keep it open as a reminder of this use case.

[dmikusa]

I believe this should be addressed in #284

If you have a moment to look @samj1912, feedback appreciated.