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

Make project go get-able #88

Closed
radeksimko opened this issue May 8, 2020 · 5 comments · Fixed by #98
Closed

Make project go get-able #88

radeksimko opened this issue May 8, 2020 · 5 comments · Fixed by #98
Labels
ci Continuous integration/delivery related

Comments

@radeksimko
Copy link
Member

Sorry for chiming in here, but I'm not quite sure why this has been introduced.
This change prevents people from just installing the language server through go get:

$> go get github.com/hashicorp/terraform-ls
$> terraform-ls
panic: Please use 'make build' to compile and install

goroutine 1 [running]:
github.com/hashicorp/terraform-ls/version.init.0()
        /Users/ramon/Documents/go/pkg/mod/github.com/hashicorp/terraform-ls@v0.2.0/version/version.go:27 +0xc5

I don't see why a server version 0.0.0 should explicitly be avoided, could you elaborate?
It would be nice if this project was just go-gettable like pretty much every other Go project.

Originally posted by @tommyknows in #45 (comment)

@radeksimko radeksimko added the ci Continuous integration/delivery related label May 8, 2020
@radeksimko
Copy link
Member Author

radeksimko commented May 8, 2020

Hi @tommyknows
There was a few motivating factors behind that PR you commented on.

Rest assured that go geting and build-from-source installation is generally something we intend to support going forward. The PR was merely aiming to communicate this is not meant to be the default way of installation for end-users and I'll try to explain why below.

Installation Tracking

It reduces our chance to track installations and such data can be critical in decision making about the future of the project.

There are ways we can address that:

  • GitHub exposes some data for repository owners about number of go gets of their Go modules over time
  • we front our import paths with a proxy that can provide such data - which comes with its own challenges that may not outweigh the benefits
  • we reduce the number of people that will use that method of installation, which makes the skew in numbers negligible

These are not necessarily mutually exclusive options and I think we can look into combining them carefully to reduce the negative effects.

Prescribing build environment

It limits our ways of communicating how the server is supposed to be compiled (e.g. with what version of Go or what compiler flags).

This could lead to folks trying to compile the server with an older/newer version of Go or passing certain compilation flags, which could in turn lead to them experiencing issues that are not reproducible by anyone else. It happens quite rarely in Go, but I have experienced that myself and these kinds of issues are the most difficult to deal with.

I believe/hope this will eventually be addressed by Go itself as the tooling matures - it looks like go stanza in go.mod is heading in that direction.

Identifying dev builds

Identification of dev builds is often needed when other maintainers and contributors file issues. It is useful to know whether the build contains certain bug fixes or features, or ideally even know the git sha it was built from.

Some other HashiCorp projects encode real version into their codebases (e.g. terraform), which (partially) solves this problem, but also comes with more complex release process as the number has to be bumped with every release. Perhaps that is something we will have to do and accept the downside of more complex release process.


I understand the PR addressed these issues in less than ideal way and created some unintended inconvenience. I'm hoping we can look into some better/alternative solutions to the above problems. More ideas are also welcomed!

@tommyknows
Copy link

Thanks for the detailed explanation!

Few thoughts on these points:

installation tracking

Agreed. Even a proxy or numbers from Github would not be a clear identifier as they could also just be dependencies of other Go modules.

Prescribing build environment

I've never worked on a large OSS project, but I also feel like Go is already pretty good on that (and with the compatibility guarantees, it should be a small issue anyway).
Obviously, compilation flags are not working with go get (unless this'd be added to go.mod).

Identifying dev builds

This feels - to me - like a "non"-issue.

For example, in an issue template, mention terraform-ls version. If version == v0.0.0, please build & reproduce through Makefile.
Maintainers / Contributors have the source code checked out locally anyway, so requiring them to build through make is not as hard. Then, use the git commit ID as the version in the Makefile.

The current Makefile just fetches the last version from Github, which is more problematic I'd say, as you could check out [really-old-version] and use make build which sets it to a completely different version, right?

The points you mentioned are valid and I understand the decision (and I'm not trying to "force" a revert). Sadly, it seems like there's no silver bullet here.
Thanks for taking your time and thoroughly explaining your points!

@radeksimko
Copy link
Member Author

There are some Go proposals/issues which mention the identification of dev builds: golang/go#37475 (might come in Go 1.16)
golang/go#29814

as for prescribing build environment - we could add runtime.GOOS and runtime.GOARCH to the version output which can at least inform us of any diversions and the buildflags may become a non-problem if the above issues are addressed as we just won't need them at all.

Generally I don't have too strong feelings about this though - so I'm happy to do something along the lines of what you're proposing in the meantime. I just mentioned it for completeness and also because none of our other HashiCorp products suffer from this problem (at the cost of more complex release process).

Also Paul submitted #98 that should hopefully unblock most folks for now and we can keep looking for solutions in the long run.

@radeksimko
Copy link
Member Author

#98 was merged, which should enable native custom builds on Windows and #127 tracks the remaining work to help identifying custom builds.

@ghost
Copy link

ghost commented Jul 1, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the context necessary to investigate further.

@hashicorp hashicorp locked and limited conversation to collaborators Jul 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ci Continuous integration/delivery related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants