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

Add a go.mod file #932

Closed

Conversation

natewernimont-wk
Copy link
Contributor

@natewernimont-wk natewernimont-wk commented Feb 24, 2021

A Pull Request should be associated with an Issue.

Related issue: #933

Description

This PR moves gh-ost to use go modules.

In case this PR introduced Go code changes:

  • contributed code is using same conventions as original code
  • script/cibuild returns with no formatting errors, build errors or unit test errors.

Copy link
Contributor

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

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

While I don't own this repo, I want to share my view, I'm unhappy with go.mod. I realize this is where most go projects are going, but personally I'm uncomfortable supporting a project with go.mod. This is not my decision here.

@natewernimont-wk
Copy link
Contributor Author

While I don't own this repo, I want to share my view, I'm unhappy with go.mod. I realize this is where most go projects are going, but personally I'm uncomfortable supporting a project with go.mod. This is not my decision here.

Would you mind sharing the issues with supporting a project with go.mod? It seems simpler than the current state in my eyes, so I am very curious why you prefer the current state.

@shlomi-noach
Copy link
Contributor

Heh, at the risk of making an utter fool of myself, here goes. But first:

  1. I understand which way the wind is blowing and that there's probably no escaping this.
  2. I speak for myself, not for my colleagues past and present.
  3. Nor do I speak for the owners of this repo. While I created it, I'm not an owner today. The owners will make their own decisions and I will cheer them from the crowd.
  4. I don't presume to be an expert golang engineer.
  5. I do not think the current gh-ost repo dependency setup is good. It is not, it's a lesser of evils. To clarify, I'm not saying it's necessarily the least evil.
  6. I'm a fan of the golang development team and applaud their work.

My reservations about go mod are as follows:

  1. The idea that a repo is not self contained is alarming to me. I'm not fanatic about that. Sure, C/C++ programs can require libaio or libssl etc., but these are very standard libraries and supported by your Linux distribution. But, depending on dozens or more of external golang repos that are just out there to build your repo seems over the top for me. Their availability and existence is not guaranteed. I'm uncomfortable with the thought that you'd checkout the repo, and it will not build because [insert name of external dependency] is no longer available.

  2. Again, I'm no golang expert, but go module proxy doesn't seem to be the solution to me. Once I set up my own personal laptop, yeah, I feel like i'm on top of things. But setting it up again, and again, and again on other computers?

  3. Specifically, I feel like this doesn't play well with Docker builds, where the Docker image always begins empty. I'm happy to wait for a long compilation, but I'm not so happy to waste time and bandwidth in minutes worth of downloads just to kick my build. When my repo contains its own dependencies, there is no such wasted time nor bandwidth.

  4. Really a personal observation here, I feel like this looks more like npm hell. Someone's gonna take their package off the internet and everything then breaks. I feel like some effort in ensuring your repo contains its dependencies is worth it.

  5. I foresee a negative effect on development culture. Previously, you'd think about what you're importing into your code. But with go mod it's just "too easy". That is to say, it's good that it's easy, I don't mean it should be hard. But importing libraries now comes as an afterthought, and, if I may extrapolate and make a prediction, this causes developers to not care about how many sub dependencies and sub-sub dependencies they're importing along the way, as it's so conveniently hidden away. I think there's a case to be conservative about the number of dependencies your project has. This is true IMHO security-wise, bug-wise, debug-wise, maintenance-wise.

Anecdotally I'm a maintainer for a large scale golang CNCF project, https://github.com/vitessio/vitess, I mention this because I'm not alone in the team who think the switch to go mod was not good for the project, so at least I think I must not be the only weird person to think so.

@timvaillancourt
Copy link
Collaborator

timvaillancourt commented Feb 25, 2021

@shlomi-noach thanks your your feedback on this!

I understand which way the wind is blowing and that there's probably no escaping this.

Yes, Golang 1.16 defaults to assuming you're running go mod. This makes me wonder how long alternatives will be supportable

I don't presume to be an expert golang engineer.

I'd argue you ARE a golang expert 😄

Someone's gonna take their package off the internet and everything then breaks. I feel like some effort in ensuring your repo contains its dependencies is worth it.

I'm curious if using go mod vendor would address some of your concerns here? This allows the repo to be handled by go mod but it still keeps copies of your dependencies in vendor/ (which must be checked-in)

To my knowledge a dependency disappearing off the internet is resolved by this approach and it also prevents go from re-downloading the dependencies each build, which I agree is totally annoying

We've been moving towards that approach for most database-related repos at GitHub and haven't had any complaints, but we probably don't spend enough time developing these repos (we mostly run databases) to know all the gotchas that approach might have created

At first we avoided go mod vendor but found we needed it for some CI to succeed when repos have private dependencies

@shlomi-noach
Copy link
Contributor

I'm curious if using go mod vendor

Do you see now why I said I'm not an expert? 😂 Let me look into it -- thanks for letting me know you're moving towards that approach!

@timvaillancourt
Copy link
Collaborator

timvaillancourt commented Feb 25, 2021

I'm curious if using go mod vendor

Do you see now why I said I'm not an expert? 😂 Let me look into it -- thanks for letting me know you're moving towards that approach!

@shlomi-noach sure, let me know what you think! I'm curious if you can spot any gotchas we didn't think of there

To be more clear, this is done in these steps:

  1. go mod tidy (as usual)
  2. go mod vendor (updates vendor/ dir and vendor/modules.txt)
  3. git add vendor/ --all
  4. We also add the -mod=vendor flag to go build to force the compiler to use vendored deps
    • I forget if that was required or if go build would have discovered vendor/ on it's own

@shlomi-noach
Copy link
Contributor

Some internal discussion in the Vitess team shows people are (obviously) more familiar than myself on this topic. For the Vitess project it looks like an overkill, but for gh-ost this looks to me a good solution, in my understanding.

@natewernimont-wk natewernimont-wk marked this pull request as ready for review March 2, 2021 21:33
This was referenced Mar 3, 2021
@natewernimont-wk
Copy link
Contributor Author

Closed in favor of #935

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.

3 participants