Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable setting the working dir for the go tool #365

Merged
merged 1 commit into from
Jun 10, 2021

Conversation

halvards
Copy link
Collaborator

@halvards halvards commented May 18, 2021

This change adds a WorkingDirectory field to options.BuildOptions, but doesn't
expose this as a CLI flag. The default zero value means the current
working directory. The value is used as the working directory for
executing go tool commands.

When embedding ko in other tools, it is sometimes necessary to set the
working directory for executing the go tool, instead of assuming the
current process working directory.

An example of where this is required from Skaffold:
https://github.com/GoogleContainerTools/skaffold/tree/master/examples/microservices

In this example, the working directory doesn't contain either go.mod
or any Go files. The skaffold.yaml configuration file specifies
a context field for each image, which is the directory where the go
tool can find package information.

I considered updating the buildContext interface to wrap go/build.Context.
That'd avoid duplicating the dir field in gobuild and buildContext.
Instead of add that to this pull request, I made that change in another branch,
mainly to explore what that could look like:
https://github.com/halvards/ko/blob/base-dir-buildcontext/pkg/build/context.go#L17
PLMKWYT (this could be a starting point for #305)

@google-cla google-cla bot added the cla: yes label May 18, 2021
@jonjohnsonjr
Copy link
Collaborator

Can you explain the QualifyImport addition? Do we need to expose this? Or can we teach IsSupportedReference and Build to do this automagically?

@halvards
Copy link
Collaborator Author

Can you explain the QualifyImport addition?

QualifyImport wraps this logic in a function:
https://github.com/google/ko/blob/522c37c4e0ec9537068f16e8f1652f969377b62e/pkg/commands/publisher.go#L47-L56

And to resolve local "import paths" correctly, the function needs to know the current directory, from the new dir field on gobuild:
https://github.com/google/ko/blob/c9883379f5e168368aaa3416c27531a18e0260ea/pkg/build/gobuild.go#L257

Do we need to expose this? Or can we teach IsSupportedReference and Build to do this automagically?

Not without changing the method signatures AFAICT. We could call a non-exported qualifyImport() from both IsSupportedReference and Build, but Publish also needs the resolved/qualified import path: https://github.com/google/ko/blob/c9883379f5e168368aaa3416c27531a18e0260ea/pkg/commands/publisher.go#L43


// Dir allows for setting the working directory for invocations of the `go` tool.
// Empty string means the current working directory.
Dir string
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we rename this to WorkingDir or WorkingDirectory? I think that would help with the name collision for #365

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Also squashed the commits.

This change adds a `WorkingDirectory` field to `options.BuildOptions`,
but doesn't expose this as a CLI flag. The default zero value means the
current working directory. The value is used as the directory for
executing `go` tool commands.

When embedding ko in other tools, it is sometimes necessary to set the
working directory for executing the `go` tool, instead of assuming the
current process working directory.

An example of where this is required from Skaffold:
https://github.com/GoogleContainerTools/skaffold/tree/master/examples/microservices

In this example, the working directory doesn't contain either `go.mod`
or any Go files. The `skaffold.yaml` configuration file specifies
a `context` field for each image, which is the directory where the `go`
tool can find package information.
@jonjohnsonjr jonjohnsonjr merged commit 2ba8bb2 into ko-build:main Jun 10, 2021
@halvards halvards deleted the base-dir branch August 24, 2021 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants