twitchyliquid64 / subnet

Simple, auditable & elegant VPN, built with TLS mutual authentication and TUN.
MIT License
1.06k stars 79 forks source link

Package structure updates #3

Closed sethgrid closed 6 years ago

sethgrid commented 6 years ago

Cheers on a neat package, and one that has made it to hackernews :)

Your project structure is fighting the Go ecosystem. You should modify the structure so that you are not checking in src. This makes your package non go get-able.

The contents of your subnet directory should become the contents of your root project available at github.com/twitchyliquid64/subnet and then (with your README still in the project root), change it from:

git clone https://github.com/twitchyliquid64/subnet
cd subnet
export GOPATH=`pwd`
go build -o subnet *.go

to

go get github.com/twitchyliquid64/subnet

go get does two things, first, it pulls the files down into the user's GOPATH, and, second, it runs go install in that directory. This should make your binary available on their system without them altering their GOPATH. Folks should not have to alter their GOPATH to work with your package :)

This will automatically pull in the required dependencies. If you are worried that the wrong version of the dependencies would be pulled in, then you need to vendor them. To vendor your dependencies, after you have moved https://github.com/twitchyliquid64/subnet/tree/master/src/subnet to https://github.com/twitchyliquid64/subnet, then you can run the soon-to-be-official vendoring tool dep (https://github.com/golang/dep) or any other vendoring tool (govend, govendor, glide, ...). This will ensure that the expected versions of dependencies are used when the binary is installed.

Cheers!

twitchyliquid64 commented 6 years ago

Thanks seth, I've followed this approach of committing my entire GOPATH since before vendoring existed, back then it was a necessary evil to avoid exposure to dependency risk and software supply chain attacks. The dep tool looks like the future here :) As long as I can continue to pin my dependencies (I think I can), I'll happy.

akavel commented 6 years ago

For now, I believe changing the repo using the following command should be enough to already make the project compatible with pure go get, while keeping the dependencies pinned:

$ mv src vendor

:)

(NOTE: not sure especially about the 'subnet' package, whether it will keep working in such case. But maybe yes?)

twitchyliquid64 commented 6 years ago

One more thing I'm not sure about. I get how arranging the code in this fashion works for a library, but where should I put the files that make up the binary (ie package main) go?

Should I make a new folder ./subnet and put all such files in there (ie go build github.com/twitchyliquid64/subnet/subnet)? Can I mix package main and package subnet in the repo root?

albertorestifo commented 6 years ago

I recommend you to read the Code Organization section of the official "How to write Go code" guide.

In your specific case:

albertorestifo commented 6 years ago

An example of such organization is the spf13/cobra CLI tool:

sethgrid commented 6 years ago

I'm on my mobile and don't have a link handy. I believe the officially recommended structure would be ./cmd/subnet/main.go if you want the root to be library code and want the binary creation separate. The total path would then be github.com/$user/$package/cmd/$binary-name/main.go.

On Oct 7, 2017 12:43 AM, "Alberto Restifo" notifications@github.com wrote:

An example of such organization is the (cobra)[https://github.com/ spf13/cobra] CLI tool:

  • The library files are in the root
  • The CLI is in a cobra subdir

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/twitchyliquid64/subnet/issues/3#issuecomment-334917168, or mute the thread https://github.com/notifications/unsubscribe-auth/AA8afA3NKyz0PlROa7OA84xU_hZPs9JWks5spyuZgaJpZM4PvHfw .

twitchyliquid64 commented 6 years ago

Thanks everyone, and most notably thanks jpillora@ for doing the restructure. This work item should now be complete.

Ive updated the README to reflect new instructions for building, it looks good to me. Works for everyone?