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

Go: unable to build snap when using go mod and main package is not in the root #2962

Merged
merged 4 commits into from Mar 10, 2020

Conversation

pavel-d
Copy link
Contributor

@pavel-d pavel-d commented Mar 6, 2020

When using go modules, go build -o <path> will pick up main package only if it's in the root of repository.

Steps to reproduce:

  1. Given a go module, where main package is located in ./cli subdirectory
  2. Clone https://github.com/pavel-d/snaptest
  3. cd snaptest
  4. Run snapcraft build --use-lxd --debug

Result:

snapcraft --use-lxd  --debug
The LXD provider is offered as a technology preview for early adopters.
The command line interface, container names or lifecycle handling may change in upcoming releases.
Launching a container.
Waiting for container to be ready
......................
status: done
snapd 2.43.3 from Canonical✓ refreshed
Skipping pull snaptest (already ran)
Building snaptest 
go build -o /root/parts/snaptest/install/bin
go build: no main packages to build
Failed to run 'go build -o /root/parts/snaptest/install/bin' for 'snaptest': Exited with code 1.
Verify that the part is using the correct parameters and try again.
snapcraft-snaptest # snapcraft --version
snapcraft, version 3.10.1

The ./... pattern matches all the packages within the current module and go build will automatically build main packages in subdirectories.

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

When using go modules, `go build -o <path>` will pick up main package only if it's in the root of repository.

The `./...` pattern matches all the packages within the current module and `go build` will automatically build main packages in all subdirectories.
@pavel-d pavel-d marked this pull request as ready for review March 7, 2020 16:33
cjp256
cjp256 previously approved these changes Mar 9, 2020
Copy link
Contributor

@cjp256 cjp256 left a comment

Choose a reason for hiding this comment

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

@cjp256
Copy link
Contributor

cjp256 commented Mar 9, 2020

A couple of unit tests need to be updated:
https://travis-ci.org/snapcore/snapcraft/jobs/659245540#L7921

@pavel-d pavel-d force-pushed the pd-fix-go-main-in-subdirectory branch from edd8090 to 7eb996b Compare March 9, 2020 17:39
@sergiusens
Copy link
Collaborator

This is great, an integration test would be really good though, it can look exactly like the current go mod one but with a main package in a subdir. I can add it to this PR if you cannot do so.

@pavel-d
Copy link
Contributor Author

pavel-d commented Mar 10, 2020

@sergiusens, thanks, I've added an integration test.

Unfortunately, I haven't figured out how to run it locally, I hope I did it correctly :)

@cjp256
Copy link
Contributor

cjp256 commented Mar 10, 2020

Nice!

@pavel-d: If you have lxd installed, you can test it with:

# From inside snapcraft directory, build snapcraft snap to use for testing.
snapcraft snap

# Run spread test locally using LXD.
./runtests.sh spread lxd:tests/spread/plugins/go/run-go-mod-subdir

I am running it now myself...

cjp256
cjp256 previously approved these changes Mar 10, 2020
Copy link
Contributor

@cjp256 cjp256 left a comment

Choose a reason for hiding this comment

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

Tested, LGTM

@cjp256
Copy link
Contributor

cjp256 commented Mar 10, 2020

Unfortunately the CLA check is failing because one (or more) of the commits were made with @assembla.com address instead of the @travis-ci.com which is associated with the CLA.

@pavel-d
Copy link
Contributor Author

pavel-d commented Mar 10, 2020

@cjp256, oh, thanks. I've fixed my email address in commits.

@sergiusens sergiusens merged commit dd74311 into canonical:master Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants