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

Switch to using go modules #161

Merged
merged 1 commit into from
Oct 25, 2020
Merged

Switch to using go modules #161

merged 1 commit into from
Oct 25, 2020

Conversation

techknowlogick
Copy link
Contributor

Description

Switches from dep to use go modules.

Motivation and Context

By switching to go modules it would bring dependency management to align with go ecosystem.
Fixes #160

  • I have raised an issue to propose this change (required)

How Has This Been Tested?

make build test succeeded.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@techknowlogick
Copy link
Contributor Author

Sorry for the multiple commits.

I've been trying to debug, and travis is doing weird things with the vendor/modules.txt even before the docker build has started. In my fork I have a test branch where I've added ls vendor as the first command run, and modules.txt is missing, hence the errors in docker build.

I've reached my quota for debugging tonight, but hopefully I'll be recharged later and can revisit this. Another option is to remove the vendor dir as a whole, but I'm trying to solve this with making as few changes from existing workflow as possible.

Fix #160

Signed-off-by: Matti R <matti@mdranta.net>
@techknowlogick
Copy link
Contributor Author

Ah, I shouldn't debug so late at night. I wasn't actually adding in the modules.txt file which was what was missing. This PR is now good to go.

go.mod Show resolved Hide resolved
@alexellis
Copy link
Owner

Why did so many files change for a dep - mod update?

@alexellis
Copy link
Owner

@techknowlogick

@techknowlogick
Copy link
Contributor Author

Why did so many files change for a dep - mod update?

Primary cause is github.com/sirupsen/logrus this is due to the reference having the username uppercase, but due to conflicts I had to lowercase it as well as update the module. This had a knock on effect of updating other modules that it depends upon. As well, yaml import was not defined in the gopkg.toml, and only the lock, and go mod prefers to use semver references as much as possible.

Note: I updated as few things as possible, for example github.com/google/go-github is still at v17 whereas it has reached v32 recently.

@techknowlogick
Copy link
Contributor Author

This PR doesn't adhere to the Update to Go 1.13 (no higher) requirement from https://trello.com/c/duCUKQPe/161-migration-to-go-modules-for-all-projects so I will update this PR with that in mind.

@alexellis
Copy link
Owner

Thank you - this PR is wanted. @LucasRoesler bumped into something similar with logrus and faas/faas-cli.

Copy link
Owner

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Approved

@alexellis alexellis merged commit 9b0f631 into alexellis:master Oct 25, 2020
@alexellis
Copy link
Owner

FYI this didn't build for me after merging into master:

go: extracting github.com/Sirupsen/logrus v1.7.0
go: github.com/alexellis/derek/build/derek/handler imports
	github.com/Sirupsen/logrus: github.com/Sirupsen/logrus@v1.7.0: parsing go.mod:
	module declares its path as: github.com/sirupsen/logrus
	        but was required as: github.com/Sirupsen/logrus

What should I do?

@alexellis
Copy link
Owner

github.com/alexellis/derek/build/derek/handler <- interesting

The ./build folder is generated by OpenFaaS, and the go toolchain is picking it up, when it shouldn't.

I couldn't find a way to exclude a folder from go mod tidy.

@LucasRoesler
Copy link

could it be a file system case-sensitivity issue?

i don't understand the statement about handler why would you need to run go mod tidy at that point? are you talking about local builds and then committing?

@alexellis
Copy link
Owner

An older Derek version was in ./build/ from running faas-cli build earlier. Go modules can't seem to figure out what code to ignore and what to build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Switch to using go modules
3 participants