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 support for installing directly with go modules #1470

Merged
merged 4 commits into from
Jan 17, 2024
Merged

Conversation

endigma
Copy link
Contributor

@endigma endigma commented Jan 16, 2024

This mimics the NPM/Cargo functionality for Go install.

Things you should look at:

  1. is that the best way to get Go into the build/test process?
  2. should I be only installing the binary into the rtx path or moving the whole gopath like I am currently? The whole GOPATH benefits sandboxing I guess but would almost definitely result in duplication of libraries across similar project installs. Could alternatively set GOBIN and do an mkdir of the /bin inside the version folder.

Things I noticed while contributing:

  • docs on running mise or tests without the devcontainer are sparse at best, couldn't figure out how to deactivate mise enough to satisfy the tests
  • perhaps an option to run tests in a container would be good? Maybe stepchowfun/toast or earthly or similar?
  • It is also pretty hard to wipe a messed up package install, or at least undocumented.
  • you mention something about a shim in CONTRIBUTING.md, but I couldn't figure out how this was meant to work it seems like not all the info required to set it up
  • pre commit hooks don't work without some md-magic thing I don't have installed

src/forge/mod.rs Show resolved Hide resolved
@jdx
Copy link
Owner

jdx commented Jan 16, 2024

is that the best way to get Go into the build/test process?

the e2e test? yeah lgtm

should I be only installing the binary into the rtx path or moving the whole gopath like I am currently? The whole GOPATH benefits sandboxing I guess but would almost definitely result in duplication of libraries across similar project installs. Could alternatively set GOBIN and do an mkdir of the /bin inside the version folder.

it's a good question and I'm not really sure. maybe what would be better is if we used a cache directory instead to put the source files in since we really don't need them in the installs directory. I think that better fits with how I build things in other plugins (build in cache, output to installs). So yeah, I think set GOPATH=tv.cache_path GOBIN=tv.install_path (probably slightly off with my naming there but you get the idea)

As far as all the local dev stuff—yeah it's awful. Sorry about that. It's my fault since I have everything basically setup for myself but haven't taken the time to make sure it works well for everyone. The ironic thing is we're literally building a tool that should be making local setup easier but just not using it. Part of that is I have been a bit hesitant to actually dogfood mise. It creates problems since the mise you use to setup and run the project often conflicts with the mise we're testing with. That's one of the main reasons I don't personally use mise activate at all.

Or maybe the e2e tests should just not be a part of the suite and instead in a docker container. The unit tests (which aren't something this project relies on quite as heavily) wouldn't be able to do that though so I'm not sure what to do about those.

So I'm still not sure how much I want to use mise for mise development. Some amount of dogfooding I think is good but maybe I should adopt something like pkgx just to avoid compatibility problems. This is an issue completely specific to this one project so it's not a use-case I am particularly concerned about supporting.

The only thing I would say though is I have noticed a lot of users checking out the mise.toml in the project using github's traffic tracking. So there is an argument that mise should be a well-architected mise project itself because people do look at it.

@jdx
Copy link
Owner

jdx commented Jan 16, 2024

btw md-magic would probably be fine to add:

mise x npm:markdown-magic -- md-magic

the places it is called in the pre-commit task could just be modified to go through mise x like that

@endigma
Copy link
Contributor Author

endigma commented Jan 16, 2024

Failing dogfooding, maybe just nix? Nix works, even if its a little complex for a new user, its documented and reliable.

@jdx
Copy link
Owner

jdx commented Jan 16, 2024

don't like nix, not at all

src/forge/go.rs Outdated Show resolved Hide resolved
@endigma
Copy link
Contributor Author

endigma commented Jan 17, 2024

I've updated it to function as per your suggestion, but I'm still not sure this is totally correct. Go already does build caching in ~/.cache/go-build, the stuff in $GOPATH which is now in ~/.cache/mise is literally the downloaded source for the version, which normally goes in your system GOPATH to avoid dupes.

Intuition and experience tells me this should function fine if I removed the GOPATH bit entirely, as that's how Go install normally works, but I'll defer to whatever you want for the project here.

Could also update GOCACHE if you wanted to make it totally portable, I suppose, although it couldn't share a directory with GOPATH, so it would have to be like ~/.cache/mise/<app>/<version>/{mod,cache} or something.

@jdx jdx enabled auto-merge (squash) January 17, 2024 03:13
@jdx jdx merged commit 05f76ec into jdx:main Jan 17, 2024
6 of 7 checks passed
@jdx
Copy link
Owner

jdx commented Jan 17, 2024

Go already does build caching in ~/.cache/go-build

Yeah I think that's fine to keep there, I don't think everything needs to be centralized.

the stuff in $GOPATH which is now in ~/.cache/mise is literally the downloaded source for the version, which normally goes in your system GOPATH to avoid dupes

Yes this is what I think is best. I wouldn't want my project sources to be commingled with stuff from CLIs I'm installing. The cache I'll probably never look at, but the source adds a lot of noise that I will see.

This is personal preference, and I'm open to changing it if people have strong feelings here but this is the way I would want it to behave.

@jdx
Copy link
Owner

jdx commented Feb 9, 2024

I've updated it to function as per your suggestion, but I'm still not sure this is totally correct. Go already does build caching in ~/.cache/go-build, the stuff in $GOPATH which is now in ~/.cache/mise is literally the downloaded source for the version, which normally goes in your system GOPATH to avoid dupes.
Intuition and experience tells me this should function fine if I removed the GOPATH bit entirely, as that's how Go install normally works, but I'll defer to whatever you want for the project here.

wanted to let you know that I think I was wrong and you were correct here. I did a lot of research into this in #1638 and came to the same conclusion. I'm going to remove GOPATH from the forge and just use whatever the system is using.

My concern is just that $HOME/go/src would be full of a bunch of "junk", but I think this mentality comes from my time as a go developer before modules which likely changes things since you're not needing to sort through everything in $HOME/go/src to find your repos and libraries you actively use.

Anyhow, sorry for not listening at the time but I'm glad you brought this up and we got this corrected while it's still experimental.

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