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

lib: Move to go modules and shed a few tears #674

Closed
wants to merge 1 commit into from
Closed

Conversation

purpleidea
Copy link
Owner

@purpleidea purpleidea commented Sep 29, 2021

The old system with vendor/ and git submodules worked great,
unfortunately FUD around git submodules seemed to scare people away and
golang moved to a go.mod system that adds a new lock file format instead
of using the built-in git version. It's now almost impossible to use
modern golang without this, so we've switched.

So much for the golang compatibility promise-- turns out it doesn't
apply to the useful parts that I actually care about like this.

Thanks to frebib for his incredibly valuable contributions to this
patch. This snide commit message is mine alone.

This patch also mixes in some changes due to legacy golang as we've also
bumped the minimum version to 1.16 in the docs and tests.

@purpleidea
Copy link
Owner Author

I can't seem to make this work. Locally if I do this:

$ go-mod-upgrade 
   • Discovering modules...   
   ⨯ upgrade failed            error=Error running go command to discover modules: exit status 1 stderr=go list -m: loading module retractions for github.com/coreos/go-systemd@v0.0.0-20190321100706-95778dfbb74e: no matching versions for query "latest"
go list -m: loading module retractions for gopkg.in/fsnotify.v1@v1.4.7: version "v1.5.1" invalid: go.mod has non-....v1 module path "github.com/fsnotify/fsnotify" at revision v1.5.1

I get these scary errors. I've tried different things... Not sure how to fix this. It does build though. And the build errors here are different, but possible related??

@frebib I tried your branch as well and I get the same error locally...

@purpleidea purpleidea force-pushed the bug/go-mod branch 2 times, most recently from 8979522 to 63f11a8 Compare September 29, 2021 06:05
Copy link
Contributor

@frebib frebib left a comment

Choose a reason for hiding this comment

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

@purpleidea I wonder if you have a stale cache or something. Pulling github.com/fsnotify/fsnotify in a blank new project works fine: https://gist.github.com/frebib/9944516ccbac6e55f593e6ed9a7d968b/raw

return fmt.Errorf("program was not compiled correctly, see Makefile")
}
if cliArgs.Copying == "" {
return fmt.Errorf("program copyrights we're removed, can't run")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/we're/were/

)

// CLIArgs is a struct of values that we pass to the main CLI function.
type CLIArgs struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed that we use Cli in gapi: https://github.com/purpleidea/mgmt/blob/master/gapi/gapi.go#L60
Should we change one or the other to be consistent?

@purpleidea purpleidea force-pushed the bug/go-mod branch 3 times, most recently from 12c1b7a to d35037d Compare October 5, 2021 11:34
The old system with vendor/ and git submodules worked great,
unfortunately FUD around git submodules seemed to scare people away and
golang moved to a go.mod system that adds a new lock file format instead
of using the built-in git version. It's now almost impossible to use
modern golang without this, so we've switched.

So much for the golang compatibility promise-- turns out it doesn't
apply to the useful parts that I actually care about like this.

Thanks to frebib for his incredibly valuable contributions to this
patch. This snide commit message is mine alone.

This patch also mixes in some changes due to legacy golang as we've also
bumped the minimum version to 1.16 in the docs and tests.
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.

2 participants