Consider not adding -trimpath flag by default when disabling compiler optimizations

[halvards]

ko adds the -trimpath flag if no build config is specified: /~https://github.com/google/ko/blob/5617d1ebf8560df778aa884fa328fe05f62bdb5e/pkg/build/gobuild.go#L562

This removes information necessary for remote debugging, see: GoogleContainerTools/skaffold#6843 (comment)

Users who want to debug would typically also use the --disable-optimizations flag.

Should we default to not add -trimpath if disableOptimizations is true?

In other words, change this conditional from !ok to !ok && !g.disableOptimizations?

A workaround is to ask users to specify a build config if they want to control whether the -trimpath flag is added.

[halvards]

(By the way, I'd be happy to implement it.)

[imjasonh]

Not making a judgement either way for now, but this issue explains why we added -trimpath in the first place: #71

[halvards]

Thanks for the context, I ❤️ reproducible builds too.

But when interactively debugging it's not as important IMHO. Or is --disable-optimizations used for other purposes too?

[imjasonh]

I don't know enough about Skaffold, but does it have some "run this in debug mode" option that could disable -trimpath for ko builds? Or is it always "run this code; I may want to debug it later"?

If Skaffold's use case is iterative (local?) development and not releasing to prod, I care a lot less about the reproducibility of Skaffold's ko builds, you could maybe even just have Skaffold always unset -trimpath and get everything you want. I'm not sure this even needs changes in ko to enable that.

[halvards]

Thanks for the reply, I'm definitely happy to discuss other solutions!

To your questions:

does it have some "run this in debug mode"

Yes, that's exactly the use case. Users run skaffold debug and attach a debugger, or perform an equivalent action using the Cloud Code IDE extensions. Reproducibility is not important for this action.

you could maybe even just have Skaffold always unset -trimpath

(warning: long answer!)

The integration between Skaffold and ko includes goals of:

1. Making it easy for existing Skaffold users to adopt ko - e.g., just change docker to ko in your Skaffold config and you're good to go (for simple scenarios); and
2. Making it easy for existing ko users to adopt Skaffold. Users who have a .ko.yaml file specifying base image overrides and/or build configs do not have to duplicate that configuration in Skaffold. Instead, they can create a minimal Skaffold config file saying they want to use ko (literally ko: {}), and the build picks up the configuration from .ko.yaml.

The integration achieves the second point by programmatically setting the ko BaseImage and BuildConfigs only if the Skaffold config includes those fields. If they're unset, ko reads from .ko.yaml instead:
/~https://github.com/google/ko/blob/5617d1ebf8560df778aa884fa328fe05f62bdb5e/pkg/commands/options/build.go#L100
/~https://github.com/google/ko/blob/5617d1ebf8560df778aa884fa328fe05f62bdb5e/pkg/commands/options/build.go#L120

The challenge right now is that the only way to prevent ko from adding -trimpath is to have a build config for the image being built:
/~https://github.com/google/ko/blob/5617d1ebf8560df778aa884fa328fe05f62bdb5e/pkg/build/gobuild.go#L559-L563

Users who don't specify a build config in either .ko.yaml or skaffold.yaml will find it challenging to interactively debug their images. We could instruct users to just create a build config in either file to control flags, however users may want it for debug but not for other actions, and they'd need a way to toggle the behavior.

As you suggest, we could always pass a build config to ko when debugging, regardless of whether the user has specified a build config or not. However, if we do this, then we don't get the behavior of falling back to build configs in .ko.yaml. In other words, we'd have to tell users that "when you're debugging, build configuration in .ko.yaml isn't used".

An alternative solution is to add a NoTrimpath BuildOption to ko. Tools like Skaffold could set this when running in debug mode. This would work well. Other tools that want to implement similar behavior would have to ensure they set both NoTrimpath and DisableOptimizations when users want to debug.

Thanks for reading all this! I hope it makes sense, and I'd be happy to clarify.

[jonjohnsonjr]

If -trimpath makes it harder to debug things, and --disable-optimizations is primarily for debugging, it seems pretty reasonable to me to change this behavior...

We could also maybe introduce a --debug flag to better express the intent?

[halvards]

Do you mean rename the existing --disable-optimizations flag to --debug (and keeping the old one as a hidden flag), or do you mean two flags?

• --disable-optimizations adds -gcflags="all=-N -l", but still adds -trimpath by default.
• --debug adds -gcflags="all=-N -l" and does not add -trimpath by default.

If we go with two flags, --debug is a reasonable name. But if we only alter the behavior of the existing --disable-optimizations flag, I'd be happy to keep the name as is.

We're using distroless as the default base image, and this flag doesn't change the base image to a debug image. Do we think that users would expect this? In other words, could users interpret --debug as "add a shell so I can poke around" rather than "I want to attach a debugger"? (I'm not suggesting that we change base image behavior.)

[imjasonh]

+1 to separate flags, and maybe considering deprecating/hiding --disable-optimizations eventually in favor of --debug. I don't consider reproducibility an "optimization", so I don't think I'd like to have -trimpath affected by that path.

We're using distroless as the default base image, and this flag doesn't change the base image to a debug image. Do we think that users would expect this? In other words, could users interpret --debug as "add a shell so I can poke around" rather than "I want to attach a debugger"? (I'm not suggesting that we change base image behavior.)

I think we should just solve this with clear documentation about what --debug does, even just your points above verbatim in the README.

Thanks for driving this and for being so through and considerate of all the options and user expectations. 👍

[jonjohnsonjr]

Skaffold doesn't use these flags, right? I'd be ok with exposing the -trimpath functionality as a build.Option.

I don't know if we want to drop DisableOptimizations and just lump this and trimpath into a single option that appends to config.Flags or just add a TrimPath option that defaults to true...

[halvards]

That's right, Skaffold would just use options.BuildOptions, not the CLI flags.

So how about this:

• Add a Trimpath field to the options.BuildOptions struct, and set a default value of true in LoadConfig().
• Do not expose it as a CLI flag.
• Plumb the value through to the gobuild struct.
• If Trimpath is true, we'll append the -trimpath flag. (Change this conditional to if g.trimpath)

As a result of this, if a user specifies -trimpath in their .ko.yaml build config, then Trimpath has no effect. Trimpath being false doesn't remove an explicitly added -trimpath flag.

This way we don't change the behavior of --disable-optimizations, and we don't introduce a new CLI flag.

Does this sound ok? Thanks both of you for your feedback and your help in shaping this. 👍

[jonjohnsonjr]

SGTM