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

build: replace deploy.sh with goreleaser #65

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Aug 14, 2022

Summary

  1. Replace bespoke deploy.sh script with the goreleaser library

  2. Replace outdated custom dependency management with standard, built-in Go modules

Details

  • deploy.sh was a custom bespoke script for building, cross-compiling, and releasing to GitHub

  • goreleaser is a popular Go library for doing the same thing and more!

  • replace deployment.yaml with .goreleaser.yaml with the same targets

    • note that darwin_arm64 is also here to support get_docopts.sh does not support macOS ARM #58
    • add various comments with links to help Sylvain and others get familiar with goreleaser's various configurations
    • configure archives and checksum to match previous output from deploy.sh
    • add some defaults for release and changelog -- to be changed by Sylvain as preferred
    • replace govvv and get_ldflags.sh by configuring ldflags
  • modify CHANGELOG.md to have the actual release notes (since deployment.yaml no longer exists)

    • use an awk script and goreleaser's --release-notes flag to insert the latest version's notes
  • gitignore dist instead of build as that's goreleaser's default output directory

  • change Makefile to use goreleaser instead of deploy.sh and go build

    • note that goreleaser build is intended to replace go build
      • the example in the docs specifically mentions the use-case of a Makefile and ldflags, i.e. the exact use-case here
    • add a release command -- to be changed by Sylvain as preferred
  • add go.mod and go.sum to use Go modules instead of a custom Makefile script

    • since this is built into Go nowadays
    • add tools.go to contain dev dependencies, i.e. goreleaser
    • using standard Go modules should also make it easier for contributors to get started and for users to build from source
      • made it easier for me!
      • simplify README.md's "Compiling" section accordingly
        • and remove reference to Go "Workspaces", as modules are now the preferred standard
      • should also make it easier to add this to homebrew-core as the build is a standard Go build now for Create Homebrew tap? #59 (comment) (search "Brew", step "4.")
  • add .go-version file for standardizing which Go version to use with goenv

Less hackiness and custom / bespoke scripts now! 🙂

@agilgur5 agilgur5 mentioned this pull request Aug 14, 2022
Makefile Show resolved Hide resolved
Copy link
Collaborator

@Sylvain303 Sylvain303 left a comment

Choose a reason for hiding this comment

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

So I'm training myself with github review. So you also received individual comment on this review. 😉

So lets move forward. I had to take this branch on my computer an try try goreleaser myself on want you did.

You did an amazing conversion work on my hacky code! 👍
goreleaser seems to have takeover all I was doing by external tool at the time I did start this project.

docopts.go Show resolved Hide resolved
github.com/charmbracelet/lipgloss v0.5.1-0.20220615005615-2e17a8a06096 // indirect
github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect
github.com/dghubble/go-twitter v0.0.0-20211115160449-93a8679adecb // indirect
github.com/dghubble/oauth1 v0.7.1 // indirect
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be a lot of stuff from other test you did?
I don't know well the go.mod behavior.
May it exists a clean stuff or rebuild dependencies?

Copy link
Collaborator

@Sylvain303 Sylvain303 Aug 24, 2022

Choose a reason for hiding this comment

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

ah, was awful (I'm investigating) I think all is coming from goreleaser as a go dependency?

sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ make install_builddep
go mod tidy
go: downloading github.com/goreleaser/goreleaser v1.10.3
go: downloading github.com/caarlos0/log v0.1.1
go: downloading github.com/caarlos0/ctrlc v1.1.0
go: downloading github.com/charmbracelet/lipgloss v0.5.1-0.20220615005615-2e17a8a06096
go: downloading github.com/invopop/jsonschema v0.5.0
go: downloading github.com/muesli/mango-cobra v1.2.0
go: downloading github.com/muesli/roff v0.1.0
go: downloading github.com/spf13/cobra v1.5.0
go: downloading github.com/muesli/termenv v0.12.1-0.20220615005108-4e9068de9898
go: downloading golang.org/x/sync v0.0.0-20220513210516-0976fa681c29
go: downloading github.com/stretchr/testify v1.8.0
go: downloading github.com/matryer/is v1.4.0
go: downloading github.com/goreleaser/nfpm/v2 v2.16.0
go: downloading github.com/lucasb-eyer/go-colorful v1.2.0
go: downloading github.com/mattn/go-runewidth v0.0.13
go: downloading github.com/muesli/reflow v0.2.1-0.20210115123740-9e1d0d53df68
go: downloading github.com/iancoleman/orderedmap v0.2.0
go: downloading github.com/muesli/mango v0.1.0
go: downloading github.com/muesli/mango-pflag v0.1.0
go: downloading github.com/cpuguy83/go-md2man/v2 v2.0.2
go: downloading github.com/inconshreveable/mousetrap v1.0.0
go: downloading github.com/aymanbagabas/go-osc52 v1.0.3
go: downloading golang.org/x/sys v0.0.0-20220209214540-3681064d5158
go: downloading code.gitea.io/sdk/gitea v0.15.1
go: downloading github.com/google/go-github/v45 v45.2.0
go: downloading github.com/xanzy/go-gitlab v0.68.2
go: downloading golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5
go: downloading github.com/jarcoal/httpmock v1.2.0
go: downloading github.com/Masterminds/semver/v3 v3.1.1
go: downloading golang.org/x/crypto v0.0.0-20220307211146-efcb8507fb70
go: downloading github.com/Masterminds/semver v1.5.0
go: downloading github.com/charmbracelet/keygen v0.3.0
go: downloading github.com/caarlos0/go-shellwords v1.0.12
go: downloading github.com/mitchellh/go-homedir v1.1.0
go: downloading github.com/imdario/mergo v0.3.13
go: downloading gopkg.in/yaml.v3 v3.0.1
go: downloading github.com/rivo/uniseg v0.2.0
go: downloading github.com/russross/blackfriday/v2 v2.1.0
go: downloading gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c
go: downloading github.com/hashicorp/go-version v1.2.1
go: downloading github.com/google/go-querystring v1.1.0
go: downloading github.com/google/go-cmp v0.5.8
go: downloading github.com/hashicorp/go-cleanhttp v0.5.2
go: downloading github.com/hashicorp/go-retryablehttp v0.7.1
go: downloading golang.org/x/time v0.0.0-20220411224347-583f2d630306
go: downloading gocloud.dev v0.24.0
go: downloading github.com/DisgoOrg/disgohook v1.4.4
go: downloading github.com/caarlos0/env/v6 v6.9.3
go: downloading github.com/caarlos0/go-reddit/v3 v3.0.1
go: downloading github.com/slack-go/slack v0.11.0
go: downloading gopkg.in/mail.v2 v2.3.1
go: downloading github.com/atc0005/go-teams-notify/v2 v2.6.1
go: downloading github.com/go-telegram-bot-api/telegram-bot-api v4.6.4+incompatible
go: downloading github.com/dghubble/go-twitter v0.0.0-20211115160449-93a8679adecb
go: downloading github.com/dghubble/oauth1 v0.7.1
go: downloading github.com/google/uuid v1.3.0
go: downloading github.com/goreleaser/fileglob v1.3.0
go: downloading github.com/caarlos0/sshmarshal v0.0.0-20220308164159-9ddb9f83c6b3
go: downloading github.com/AlekSi/pointer v1.2.0
go: downloading github.com/goreleaser/chglog v0.1.2
go: downloading github.com/klauspost/pgzip v1.2.5
go: downloading github.com/blakesmith/ar v0.0.0-20190502131153-809d4375e1fb
go: downloading github.com/ulikunitz/xz v0.5.10
go: downloading github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8
go: downloading github.com/google/rpmpack v0.0.0-20220314092521-38642b5e571e
go: downloading github.com/ProtonMail/go-crypto v0.0.0-20210512092938-c05353c2d58c
go: downloading github.com/caarlos0/go-rpmutils v0.2.1-0.20211112020245-2cd62ff89b11
go: downloading github.com/kr/pretty v0.2.1
go: downloading github.com/hashicorp/go-hclog v0.9.2
go: downloading github.com/Azure/azure-pipeline-go v0.2.3
go: downloading github.com/Azure/azure-storage-blob-go v0.14.0
go: downloading github.com/Azure/go-autorest/autorest/adal v0.9.15
go: downloading github.com/Azure/go-autorest/autorest v0.11.20
go: downloading github.com/Azure/go-autorest v14.2.0+incompatible
go: downloading github.com/google/wire v0.5.0
go: downloading github.com/Azure/go-autorest/autorest/azure/auth v0.5.8
go: downloading go.opencensus.io v0.23.0
go: downloading github.com/aws/aws-sdk-go v1.40.34
go: downloading cloud.google.com/go v0.94.0
go: downloading cloud.google.com/go/storage v1.16.1
go: downloading github.com/googleapis/gax-go/v2 v2.1.0
go: downloading google.golang.org/api v0.56.0
^Cmake: *** [Makefile:14: install_builddep] Interrupt

Rebuilding go.mod doesn't help:
https://stackoverflow.com/questions/65512353/how-to-undo-go-mod-init

rm go.mod
go mod init
go mod tidy
[...] plenty of stuff bloating my filesystem

So lets try it (173 dep downloaded) 🤔

nope a step is missing about installing goreleaser

sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ make clean
rm -f docopts-* docopts README.tmp dist/*
sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ make 
GOVERSION=$(go version) goreleaser build --rm-dist --snapshot --single-target -o docopts
/bin/sh: 1: goreleaser: not found
make: *** [Makefile:11: docopts] Error 127

neither (eating more disk storage)

go install github.com/goreleaser/goreleaser@latest

I lost track of how go get moving to go install at some point, changed the way it was installing binaries.

So I'm stopping here for today. I have to:

  • figure out how to clean the my go env install of all those lib
  • learn (again) about go get (deprecated) to go install
  • probably install goreleaser in alternative way
  • play with it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry I thought you'd be familiar with Go modules so I didn't explain that.

Yea so these are all dependencies of goreleaser (or dependencies of its dependencies, and so on).
The go.mod file doesn't just contain direct dependencies, but also indirect dependencies.

Note that in the go.mod file, there's actually a comment saying // indirect and the indirect deps are split into a separate require block as well.

That's all automatically generated too by go mod tidy. The tidy part will remove any dependencies that aren't necessary as well, so there's nothing extra.

goreleaser can do a ton of things, so unfortunately with that comes a bunch of deps. The functionality here is only a small part of what it can do, so a lot of those deps probably aren't used too.

Just to be clear -- these are all development dependencies just for using goreleaser (that's what tools.go is, a list of dev deps basically), so none of these make it into the actual docopts binary, as they're not used at compile-time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

none of the hundred of downloaded lib make it into the actual docopts binary

Yes, that's what I think too. But are they mandatory to be downloaded just for the purpose of running goreleaser?

Oh sorry I thought you'd be familiar with Go modules so I didn't explain that.

In fact, I started coding Go as a casual coder transposing my experience from other language. But at the time I started Go module was not made that way or not at all. As you figure out probably, docopts is a small code with very low external lib.

So yes I'm not so familiar with module. I will learn.

But I suppose, that they're probably not needed to be listed as part of the project?
I suppose goreleaser can be installed outside. As a tool, does it have to be registered as to be like "part" of the project? It's an helper, right?

I mean, I don't list the lib used to build make as part of the project.

building dev env instruction

It was section in the README for that purpose. It was to detail the command to run for building it. (Including an external Golang link you removed I think, to setup the Go workspace).
In fact, at the time I built doctops Go was really easy to install with not so bloated stuff to install all around.

And I gave instruction to do it, even for non-Go developer because of the very low dependencies of the project.

  1. Install Go (obviously) and described by Golang page directly, so not repeated in our doc. (tgz extract in /usr/local) + GOPATH + PATH GOBIN
  2. install our external tool, was mostly Go govvv and such, etc. make install_builddep
  3. make
  4. nothing more

the docopts binary was here.

There's also a small Dockerfile for experimenting: ./docker/debian-docopts.Dockerfile

For playing around. (only building docopts)

releasing

Releasing was a much more complex step as you lived yourself.

It's a separated process from the build. And it should be reserved (dedicated) to people wanting to publish to github release (unless I finally work on #22, that's another learning curve on my plan).
Basically only me actually.

Because my hacker and automation mindset, I did automate boring repetitive tasks.
So I finally created deploy.sh and moved it to its own repository.

The releasing process is described here: docs/release.md

what are the required action for building

So as you did an amazing job porting my hack to goreleaser 👍 I want to learn goreleaser how it replaces deploy.sh but also the version building (at build stage).

So let's focus first on the build. I will do it you from your branch and produce some updated instructions based on what I will learn.

Then I will try to deploy, but it looks like very similar of what I was doing (+ over than 170 dependencies 😋 )

Copy link
Contributor Author

@agilgur5 agilgur5 Aug 28, 2022

Choose a reason for hiding this comment

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

goreleaser deps

But are they mandatory to be downloaded just for the purpose of running goreleaser?

Yes, as you found out by running go get github.com/goreleaser/goreleaser@latest (or go mod tidy etc), they're all necessary for goreleaser and only goreleaser.

Go modules

In fact, I started coding Go as a casual coder transposing my experience from other language. But at the time I started Go module was not made that way or not at all.

Yea Go modules didn't exist I think until beta releases of Go 1.11 IIRC. Before that, there was no single dependency management system in Go. Now Go modules standard and built-in, thankfully.

I don't write that much Go myself, so it seemed like you might know the ecosystem better than me at first haha 😅
In particular, Go workspaces themselves were more complicated than Go modules. So if you were familiar with workspaces, I thought you might have seen modules by now.

So yes I'm not so familiar with module. I will learn.

No worries though! Go modules are pretty straightforward and simpler than workspaces, so it seems like you've mostly figured it out already!

tool dependency

But I suppose, that they're probably not needed to be listed as part of the project? I suppose goreleaser can be installed outside. As a tool, does it have to be registered as to be like "part" of the project? It's an helper, right?

I mean, I don't list the lib used to build make as part of the project.

So it is a tool dependency, that's why I put it in tools.go.

In other languages, such as JavaScript, devDependencies can be listed separately in the package manifest. In JS, devDependencies are only installed when you're working on the package itself. If you're just using the package as a dependency, e.g. import jsreleaser from "jsreleaser";, the devDependencies won't be installed, only dependencies will be.

Go modules do not seem to have a "formal" separation of these yet. Per the Go issue I linked above, tools.go is a current convention to list tool dependencies that aren't needed when importing, similar to devDependencies in JS.

JS's devDependencies also make it into JS's package-lock.json lockfile, similar to the go.sum.
So having goreleaser in go.mod and go.sum follows Go's current conventions. This ensures that anyone developing docopts -- in this case you and I -- has the same version of all dependencies, including indirect development dependencies.

go.mod nuances

As you found out, if you run go get github.com/goreleaser/goreleaser@latest, it'll automatically be added to the go.mod as an // indirect dependency. So it's meant to be listed there. Even if you remove it, it'll get added back as soon as it's installed.

Building vs. Releasing

In fact, at the time I built doctops Go was really easy to install with not so bloated stuff to install all around.

Agreed I'd like to keep that simple as well. I wasn't sure how to do so when I first made this PR, but I think I may have figured it out now (see next section).

  1. Install Go (obviously) and described by Golang page directly, so not repeated in our doc. (tgz extract in /usr/local) + GOPATH + PATH GOBIN
  2. install our external tool, was mostly Go govvv and such, etc. make install_builddep
  3. make
  4. nothing more

I should say, the installation actually simpler now, just a bunch of deps get added that aren't used:

  1. Install Go
  2. install external tools, go mod tidy
  3. make
  4. nothing more

The steps are simpler, but it is bloated unfortunately

Releasing

And it should be reserved (dedicated) to people wanting to publish to github release (unless I finally work on #22, that's another learning curve on my plan).

Agreed.

The releasing process is described here: docs/release.md

Ah, I need to modify this doc too with the new process with goreleaser. I'll add that change to this PR.

Building only

what are the required action for building

I need to test this a bit more, but apparently Go modules are a lot more intelligent than I first thought.
As I was building the Homebrew package, I realized you can actually just run go build without running go mod tidy first. And then it only detects that docopt-go is a dependency (as it's the only dep in docopts.go) and only installs it.

So I think I can remove the go mod tidy statement from the README.md for individual builds and leave it only for releases.

That will make it easy to build from source without needing goreleaser and all of its bloated deps! The only caveat is that the ldflags won't be set, but they weren't in the original directions either (just go build, with no flags).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realized you can actually just run go build without running go mod tidy first. And then it only detects that docopt-go is a dependency (as it's the only dep in docopts.go) and only installs it.

Yep, this works!

❯ go clean -modcache
❯ go build
go: downloading github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815

Only downloads the one necessary dep!

So I think I can remove the go mod tidy statement from the README.md for individual builds and leave it only for releases.

Will proceed with this then 🙂

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hello, I'm back after a while... (refresh in progress...) ⏳

Building only

So effectively it works but won't feed the versioning variables.

go build

I did some progress, I installed goreleaser finally (from their doc)

go install github.com/goreleaser/goreleaser@latest

I did update my branch:

git branch --set-upstream-to=agilgur5/build-with-goreleaser  agilgur5-build-with-goreleaser

(with some conflict may be the one we saw earlier)

So yes I confirm it builds (without variable version updated) which is not what I'm expecting while running make.

$ git diff HEAD master Makefile

+# govvv define main.Version with the contents of ./VERSION file, if exists
+BUILD_FLAGS=$(shell ./get_ldflags.sh)
 docopts: docopts.go Makefile
-   go build -o $@
+ go build -o $@ -ldflags "${BUILD_FLAGS} ${LDFLAGS}"

make should build the local docopts binary with all its flags, without complaining about uncommitted changes, and only one arch local target.

I didn't learn enough about goreleaser to ride it my way, it seems it would look like:

GOVERSION=$(go version) goreleaser build --single-target --snapshot --rm-dist -o docopts

but it still compile all targets and don't left a valid docopts in the top folder.
I will seek for that later.

So it means that Makefile first target doctops, should figure out to force gorelaser to replicate the same as go build -o $@ -ldflags "${BUILD_FLAGS} ${LDFLAGS}"

Either garbing built ldflags from some magical goreleaser --extract-ldflags
Or multiple action make moving the target file at the correct position.

No more time for today, talk later.

Copy link
Contributor Author

@agilgur5 agilgur5 Sep 10, 2022

Choose a reason for hiding this comment

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

So effectively it works but won't feed the versioning variables.

Yea, I realize that.

But I think that's a fine trade-off for quick builds and builds from source.
In fact, you already have that in the README: go build docopts.go doesn't provide any versioning information.
Since it was already in the README, that's why I thought it was ok to make that change.

As this does not change the approach in the README, I think this change is fine.

GOVERSION=$(go version) goreleaser build --single-target --snapshot --rm-dist -o docopts

And yes, if you want all the versioning you can run this. That's what I previously had.

but it still compile all targets

make snapshot that I have in this PR does so. But make snapshot doesn't use --single-target. We can add another command that does only --single-target etc.

So it means that Makefile first target doctops

I can change the default make if you want, but you might as well use make all at that point -- which is what the README already had as well.
It depends on who the audience for some of these are. If you're mentioning the Makefile in the README for building from source, then I think making that not require goreleaser would be best. Whereas contributors or publishers (i.e. me and you) can install goreleaser and use make all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

what are the required action for building

README seems a bit outdated at some point. Thanks for bringing me the eyes on that. As I said, it was simple long time ago... 😉

So lets focus, as being the main builder, and a vim user, and a make command user. Just make should be smart enough to accomplish complex work including knowing what to do.

It comes back from good old C compiling only requires changed dependencies, etc. As you may probably know. So make is very smart. And despite go build figures out itself a lot of thing if may be difficult to tell him to take care of VERSION file correctly.

Was done by govvv initially

BUILD_FLAGS=$(shell ./get_ldflags.sh)
docopts: docopts.go Makefile
	go build -o $@ -ldflags "${BUILD_FLAGS} ${LDFLAGS}"

So I need a way to get it back. go build without argument could be OK for casual building. I checked, there's was no functional testing for docopts --version. See below some internal history famous details about why --version. It could be tested will full variable expansion during development stage too or when I'm crafting the --version message too. So the default Makefile rule should be much more tricky than just a trivial go build.

docopts --version gory details

You may have noticed there's a hack on --version=<msg> switch vs --version without argument also triggering a "normal" software version output and exit(0). This was kept from backward compatibility of the original author of docopts.

I rarely myself use versioning in my own cli but that's part of the cli API of docopts itself for now.

So some future unit test may be introduced that the --version with or without argument may pass or fail.

I'm also disagree with the --help=<msg> too, may I decide to break it as some version to keep the code more easy to understand without going to exception handler, or such...

--single-target still build all target

For some unknown reason goreleaser still build all targets:

sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ make
GOVERSION=$(go version) goreleaser build --single-target --rm-dist --snapshot -o docopts
  • starting build...
  • loading config file                              file=.goreleaser.yaml
  • building only for linux/amd64                    reason=single target is enabled
  • loading environment variables
  • getting and validating git state
    • building...                                    commit=34b0c0ba4c696662087eeb1ef489f96558899951 latest tag=v0.6.4-with-no-mangle-double-dash
    • pipe skipped                                   reason=disabled during snapshot mode
  • parsing tag
  • setting defaults
  • running before hooks
    • running                                        hook=go mod tidy
  • snapshotting
    • building snapshot...                           version=0.6.4-next
  • checking distribution directory
  • loading go mod information
  • build prerequisites
  • writing effective config file
    • writing                                        config=dist/config.yaml
  • building binaries
    • building                                       binary=dist/docopts_linux_amd64_v1/docopts
    • building                                       binary=dist/docopts_darwin_arm64/docopts
    • building                                       binary=dist/docopts_freebsd_amd64_v1/docopts
    • building                                       binary=dist/docopts_linux_386/docopts
    • building                                       binary=dist/docopts_linux_arm_6/docopts
    • building                                       binary=dist/docopts_windows_amd64_v1/docopts.exe
    • building                                       binary=dist/docopts_darwin_amd64_v1/docopts
    • took: 1s
  • storing release metadata
    • writing                                        file=dist/artifacts.json
    • writing                                        file=dist/metadata.json
  • copying binary to "docopts"
  • build succeeded after 1s
sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ tree dist
dist
├── artifacts.json
├── config.yaml
├── docopts_darwin_amd64_v1
│   └── docopts
├── docopts_darwin_arm64
│   └── docopts
├── docopts_freebsd_amd64_v1
│   └── docopts
├── docopts_linux_386
│   └── docopts
├── docopts_linux_amd64_v1
│   └── docopts
├── docopts_linux_arm_6
│   └── docopts
├── docopts_windows_amd64_v1
│   └── docopts.exe
└── metadata.json

7 directories, 10 files

at least -o is honored and a docopts amd64 binary is present

./docopts --version
docopts 0.6.4-next commit 34b0c0b built at 2022-09-11T08:02:49Z
built from: go version go1.19.1 linux/amd64
Copyleft (Ɔ)  2022 Sylvain Viart (golang version).
Copyright (C) 2013 Vladimir Keleshev, Lari Rasku.
License MIT <http://opensource.org/licenses/MIT>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

may with multiple build target in the .goreleaser.yaml and a --id on the --single-target?

# .goreleaser.yaml
builds:
  - main: ./dont/know/the/path/here
    id: "dev"
    binary: docopts

I will probably ask on https://github.com/goreleaser/goreleaser/discussions
But I've to read much more about goreleaser.

I've the feeling that goreleaser, may be needed to be kept in a sub-folder to not interfere with basic build new requesting release. So moving tools.go to a sub-folder etc.

Makefile change

Do you want me to push that change to your branch or do you prefer me not to mess your code base? Because, as a maintainer, I've right to push on PR branches to collaborate.

diff --git a/Makefile b/Makefile
index 8cacfec..f45ad29 100644
--- a/Makefile
+++ b/Makefile
@@ -9,7 +9,7 @@ RELEASE_NOTES := $$(awk -v RS='\#\# *|\#\# ' 'NR==2 { print }' CHANGELOG.md)
 # keep docopts: as first target for development
 
 docopts: docopts.go Makefile
-   go build -o $@
+ GOVERSION=$(GOVERSION) goreleaser build --single-target --rm-dist --snapshot -o docopts
 
 install_builddep:
        go mod tidy
@@ -34,7 +34,9 @@ README.md: examples/legacy_bash/rock_hello_world.sh examples/legacy_bash/rock_he
        mv README.tmp README.md
 
 clean:
-   rm -f docopts-* docopts README.tmp dist/*
+ # remove goreleaser dist/ difrectrory recursively
+ rm -rf dist
+ rm -f docopts-* docopts README.tmp
 
 test_release_notes:
        echo "\n## $(RELEASE_NOTES)"

Copy link
Contributor Author

@agilgur5 agilgur5 Sep 18, 2022

Choose a reason for hiding this comment

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

go build without argument could be OK for casual building. I checked, there's was no functional testing for docopts --version

So some future unit test may be introduced that the --version with or without argument may pass or fail.

I think we can leave it as such for casual building. If needed, the VERSION file is still there, so cat VERSION can probably be added to the Makefile too. Could probably do the same for the other ldflags too.

You may have noticed there's a hack on --version=<msg> switch vs --version without argument also triggering a "normal" software version output and exit(0). This was kept from backward compatibility of the original author of docopts.

I'm also disagree with the --help=<msg> too, may I decide to break it as some version to keep the code more easy to understand without going to exception handler, or such...

Yea I did notice that and read about it. Agreed that it's not ideal but seems like an ok workaround for now

For some unknown reason goreleaser still build all targets:

Think we've figured this out in goreleaser/goreleaser#3371 (reply in thread) / goreleaser/goreleaser#3381 -- we found a bug!

Do you want me to push that change to your branch or do you prefer me not to mess your code base? Because, as a maintainer, I've right to push on PR branches to collaborate.

Feel free to push to my branch! For reference, the PR author actually has a checkbox to allow or deny those permissions to maintainers. By default it is allowed, and as a maintainer myself, it seems to be generally accepted to do so -- no need to ask!

I noticed make clean needed changing too recently, and with the fixes to --single-target and your requests for the default make, those changes would be needed too. So all the changes make sense to me 👍

I've the feeling that goreleaser, may be needed to be kept in a sub-folder to not interfere with basic build new requesting release. So moving tools.go to a sub-folder etc.

I hadn't thought about using a subfolder. That could make sense too, though might be more complicated than necessary

@agilgur5
Copy link
Contributor Author

agilgur5 commented Aug 25, 2022

So I'm training myself with github review. So you also received individual comment on this review. 😉

No worries, you seemed to have figured out how to use GH reviews already! 🙂

You did an amazing conversion work on my hacky code! 👍

Thank you! It took a bit of effort but seemed well worth it! 😄

goreleaser seems to have takeover all I was doing by external tool at the time I did start this project.

Yea and it's got lots of other features too -- like it can generate .deb packages for #22, Brew formulae, etc etc etc. I haven't used all of the features yet either.

So lets move forward.

I responded in-line to your comments / questions; let me know what changes you'd like to see from there.

At the very least, I'll have to update the pre-built-binaries.md doc with the new process using goreleaser and Go modules before this can be merged, as I didn't notice that file before.

@Sylvain303
Copy link
Collaborator

😆
from CI:

/bin/sh: 1: goreleaser: not found

Wheel and circle!

Copy link
Collaborator

@Sylvain303 Sylvain303 left a comment

Choose a reason for hiding this comment

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

Ho, button here.

image

But not on the other window. 🤔

Let's see if it sends the review comment may in separate notification. 🤷‍♂️

docopts.go Show resolved Hide resolved
Makefile Show resolved Hide resolved
Changelog.md Show resolved Hide resolved
@Sylvain303
Copy link
Collaborator

Sylvain303 commented Aug 26, 2022

Hello,
I will be in vacation for a week. So I will pause my brain, including Opensource Software. 😉

You wont see any more input from me until September 5th.
Read you later.

agilgur5 added a commit to agilgur5/homebrew-tap that referenced this pull request Aug 28, 2022
- on the root fork on `master`, there is no `go.mod` yet, so need to create one and add deps
  - I have a PR out that adds this (and more), but it is still under review (and maybe for a good bit): docopt/docopts#65
@agilgur5 agilgur5 force-pushed the build-with-goreleaser branch 2 times, most recently from ee571a6 to 95ca853 Compare August 28, 2022 16:43
@agilgur5
Copy link
Contributor Author

😆 from CI:

/bin/sh: 1: goreleaser: not found

I fixed that! I wrote this PR and #62 simultaneously, so that was a bit of a "merge conflict" of sorts as I wasn't able to use Go 1.17 until a go.mod / go.sum existed. CI passes now 🙂

agilgur5 and others added 5 commits August 28, 2022 13:13
- `deploy.sh` was a custom bespoke script for building, cross-compiling, and releasing to GitHub
  - it was eventually moved out of this repo and into https://github.com/opensource-expert/deploy.sh
- `goreleaser` is a popular Go library for doing the same thing and more!
  - https://github.com/goreleaser/goreleaser

- replace `deployment.yaml` with `.goreleaser.yaml` with the same targets
  - configure `archives` and `checksum` to match previous output from `deploy.sh`
  - add some defaults for `release` and `changelog` (to be changed by Sylvain as preferred)
  - replace `govvv` and `get_ldflags.sh` by configuring `ldflags`
- modify `Changelog.md` to point to GitHub Releases (since `deployment.yaml` no longer exists)
- `gitignore` `dist` instead of `build` as that's `goreleaser`'s default

- change `Makefile` to use `goreleaser` instead of `deploy.sh` and `go build`
  - add a `release` command (to be changed by Sylvain as preferred)

- add `go.mod` and `go.sum` to use Go modules instead of a custom `Makefile` script
  - since this is built into Go nowadays
  - add `tools.go` to contain dev dependencies, i.e. `goreleaser`
  - using standard Go modules should also make it easier for contributors to get started and for users to build from source
    - made it easier for me!
    - simplify `README.md`'s "Compiling" section accordingly
      - and remove reference to Go "Workspaces", as modules are now the preferred standard (old docs at: https://go.dev/doc/gopath_code#Workspaces)
    - should also make it easier to add this to `homebrew-core` as the build is a standard Go build now

- add `.go-version` file for standardizing which Go version to use with `goenv` (https://github.com/syndbg/goenv)
- `go build docopts.go` will automatically install any dependencies that are needed at compile-time
  - this also means it will _only_ install `docopt-go`, and not `goreleaser` or its many deps

- for releasing, it's still necessary to install `goreleaser`, but not if you're just building from source
- this should fix the failing CI on this PR
  - this PR was made before my Travis -> GH Actions migration PR was merged
  - now that it's merged, I've rebased this PR on top and made tiny adjustments
- this way the default target does not need `goreleaser`
- hadn't seen these docs before, so updating them to match all the `goreleaser` changes
  - no need to install various deps, just `goreleaser` with `go mod tidy`
  - no need to install and run `deploy.sh`, just `make snapshot` and `make release`

- update `make snapshot` to remove `--single-target` so that it will build all binaries as a dry-run
  - since it's no longer needed for the default target build, can make it output all binaries

- TODO: modify changelog / release notes / versioning to match Sylvain's current style
  - Didn't change some references to `VERSION` and `deployment.yml` as I will likely make those work with `goreleaser`

- also fix some typos:
  - "developper" -> "developer"
  - "dependancies" -> "dependencies"
  - "gitub" -> "github"
  - "repos" -> "repo"
  - "you" -> "your"
  - "priviledges" -> "privileges"
  - etc
- move notes from earlier `deployment.yml` into `CHANGELOG.md`
  - this is actual Markdown that will highlight so in the editor and when reading on GitHub
  - also fix various spelling, grammar, and formatting issues from `deployment.yml`'s release notes

- add a `RELEASE_NOTES` variable in the `Makefile` that uses `awk` to parse out the latest release's notes from `CHANGELOG.md`
- add a `make test_release_notes` command for double-checking

- update release docs with the `CHANGELOG.md` changes
@agilgur5
Copy link
Contributor Author

agilgur5 commented Sep 10, 2022

Added release notes generation now, so I think that addresses all remaining review comments.

@agilgur5
Copy link
Contributor Author

agilgur5 commented Sep 10, 2022

@Sylvain303 do you think you'll be ready to merge this soon?

If not, I might split up the Go modules piece into a separate PR to be merged first (and leave this as just goreleaser stuff), as it's currently blocking #59 (comment) .
That one would be much simpler; then you can take your time with this one as it wouldn't be blocking anything (for now).

I also made a few other tiny, mostly docs PRs 2 weeks ago in case you hadn't seen those.

@Sylvain303 Sylvain303 mentioned this pull request Sep 11, 2022
3 tasks
@Sylvain303
Copy link
Collaborator

Global note of the PR (may be more easy to search for, let's try)

For me to validate this PR

  • make (without argument build a single local docopts with correct VERSION embedded)
  • validate docs/release.md steps from testing and copy pasting command and complete missing stuf if any
  • publish release to my personal github account for testing the whole process
  • test CHANGELOG.md behavior ` --release-notes flag
  • go.mod huge list, hundred of megabyte download only for local build is (tools.go the way to go?)

VERSION vs goreleaser main.Version={{.Version}}

Our discussion here:
#65 (comment)

release notes

According to
https://goreleaser.com/customization/release/#custom-release-notes it can be a markdown file.

tools.go

according to cmd/go: clarify best practice for tool dependencies there's some behavior to keep in mind here.

go build only vs make

our conversation #65 (comment)

@Sylvain303
Copy link
Collaborator

@agilgur5

do you think you'll be ready to merge this soon?

I made a comment on the PR roadmap steps in my comment above.

Unfortunately, I'm near, or already, at burnout. I'm brain behaving like a colander
image

Leaving some information out from the previous day. And progressing in mud disorganized fashion.
So I may stop again from being active to rest. 😴 😟

I will progress on my decision today. Let's see how I organize myself...

I'm lost in PR too, obviously 😉

I'm waking up and coding, for fun, as usual, let's see that Makefile target with my colander. 😋

@Sylvain303
Copy link
Collaborator

So, I'm in a resting phase no more than 2h of computer per day. I did my day.

extract ldflags only à la govvv? (#3371)

@Sylvain303
Copy link
Collaborator

Hello @agilgur5

I'm trying to work 1h on the PR. I can't push my changes to your PR branch:

sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ git push agilgur5 build-with-goreleaser 
To github.com:agilgur5/docopts.git
 ! [rejected]        build-with-goreleaser -> build-with-goreleaser (non-fast-forward)
error: failed to push some refs to 'github.com:agilgur5/docopts.git'
hint: Updates were rejected because a pushed branch tip is behind its remote
hint: counterpart. Check out this branch and integrate the remote changes
hint: (e.g. 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

May some rebase did happen related to parallel PR on CI or something I don't know.

I add a local branch based on you changes named agilgur5-build-with-goreleaser

I made a new one from your branching history:

git switch master 
git checkout -b 2agilgur5-build-with-goreleaser 94799c5c834be4bd1e96723f3c269604e4c417ae 
git pull agilgur5 build-with-goreleaser 

I really dislike the bloated module thing introduced by goreleaser dependency as from being part of tools.go I think:

tooks > 1m to build because of the go mod tidy or something the builder think he has to download the world for building 🤔 quite strange I already downloaded plenty of time the 133 dependencies of goreleaser

sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ make clean 
# remove goreleaser dist/ difrectrory recursively
rm -rf dist
rm -f docopts-* docopts README.tmp
sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ make
GOVERSION=$(go version) goreleaser build --rm-dist --snapshot --single-target -o docopts
  • starting build...
  • loading config file                              file=.goreleaser.yaml
  • building only for linux/amd64                    reason=single target is enabled
  • loading environment variables
  • getting and validating git state
    • ignoring errors because this is a snapshot     error=couldn't get remote URL: fatal: No remote configured to list refs from.
    • building...                                    commit=none latest tag=v0.0.0
    • pipe skipped                                   reason=disabled during snapshot mode
  • parsing tag
  • setting defaults
  • running before hooks
    • running                                        hook=go mod tidy
    • took: 1m21s
  • snapshotting
    • building snapshot...                           version=0.0.1-next
  • checking distribution directory
  • loading go mod information
  • build prerequisites
  • writing effective config file
    • writing                                        config=dist/config.yaml
  • building binaries
    • building                                       binary=dist/docopts_linux_amd64_v1/docopts
  • storing release metadata
    • writing                                        file=dist/artifacts.json
    • writing                                        file=dist/metadata.json
  • copying binary to "docopts"
  • build succeeded after 1m21s

I think we missed some step here about tooling and goreleaser invasive behavior on the module side.

I was looking a the make default target and CHANGELOG.md new feature in the Makefile, but the git glitch took all my time. 😞

I still can't push 😞

sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ git pull agilgur5 build-with-goreleaser 
From github.com:agilgur5/docopts
 * branch            build-with-goreleaser -> FETCH_HEAD
Current branch 2agilgur5-build-with-goreleaser is up to date.
sylvain@lap43: ~/code/go/src/github.com/docopt/docopts$ git push agilgur5 build-with-goreleaser 
To github.com:agilgur5/docopts.git
 ! [rejected]        build-with-goreleaser -> build-with-goreleaser (non-fast-forward)
error: failed to push some refs to 'github.com:agilgur5/docopts.git'
hint: Updates were rejected because a pushed branch tip is behind its remote
hint: counterpart. Check out this branch and integrate the remote changes
hint: (e.g. 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

My changes:

diff --git a/Makefile b/Makefile
index 8cacfec..fa9240e 100644
--- a/Makefile
+++ b/Makefile
@@ -8,8 +8,8 @@ RELEASE_NOTES := $$(awk -v RS='\#\# *|\#\# ' 'NR==2 { print }' CHANGELOG.md)
 
 # keep docopts: as first target for development
 
-docopts: docopts.go Makefile
-	go build -o $@
+docopts: docopts.go Makefile VERSION
+	GOVERSION=$(GOVERSION) goreleaser build --single-target --rm-dist --snapshot -o $@
 
 install_builddep:
 	go mod tidy
@@ -34,7 +34,9 @@ README.md: examples/legacy_bash/rock_hello_world.sh examples/legacy_bash/rock_he
 	mv README.tmp README.md
 
 clean:
-	rm -f docopts-* docopts README.tmp dist/*
+	# remove goreleaser dist/ difrectrory recursively
+	rm -rf dist
+	rm -f docopts-* docopts README.tmp
 
 test_release_notes:
 	echo "\n## $(RELEASE_NOTES)"
diff --git a/go.mod b/go.mod
index f096bb9..c6acfc7 100644
--- a/go.mod
+++ b/go.mod
@@ -4,6 +4,7 @@ go 1.17
 
 require (
 	github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815
+	github.com/gookit/color v1.5.2
 	github.com/goreleaser/goreleaser v1.10.3
 )
 
@@ -120,6 +121,7 @@ require (
 	github.com/ulikunitz/xz v0.5.10 // indirect
 	github.com/xanzy/go-gitlab v0.68.2 // indirect
 	github.com/xanzy/ssh-agent v0.3.1 // indirect
+	github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778 // indirect
 	go.opencensus.io v0.23.0 // indirect
 	gocloud.dev v0.24.0 // indirect
 	golang.org/x/crypto v0.0.0-20220307211146-efcb8507fb70 // indirect
diff --git a/go.sum b/go.sum
index df532e0..42058be 100644
--- a/go.sum
+++ b/go.sum
@@ -424,6 +424,8 @@ github.com/googleapis/gax-go/v2 v2.0.4/go.mod h1:0Wqv26UfaUD9n4G6kQubkQ+KchISgw+
 github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
 github.com/googleapis/gax-go/v2 v2.1.0 h1:6DWmvNpomjL1+3liNSZbVns3zsYzzCjm6pRBO1tLeso=
 github.com/googleapis/gax-go/v2 v2.1.0/go.mod h1:Q3nei7sK6ybPYH7twZdmQpAd1MKb7pfu6SK+H1/DsU0=
+github.com/gookit/color v1.5.2 h1:uLnfXcaFjlrDnQDT+NCBcfhrXqYTx/rcCa6xn01Y8yI=
+github.com/gookit/color v1.5.2/go.mod h1:w8h4bGiHeeBpvQVePTutdbERIUf3oJE5lZ8HM0UgXyg=
 github.com/gopherjs/gopherjs v0.0.0-20181017120253-0766667cb4d1/go.mod h1:wJfORRmW1u3UXTncJ5qlYoELFm8eSnnEO6hX4iZ3EWY=
 github.com/goreleaser/chglog v0.1.2 h1:tdzAb/ILeMnphzI9zQ7Nkq+T8R9qyXli8GydD8plFRY=
 github.com/goreleaser/chglog v0.1.2/go.mod h1:tTZsFuSZK4epDXfjMkxzcGbrIOXprf0JFp47BjIr3B8=
@@ -682,6 +684,8 @@ github.com/xanzy/ssh-agent v0.3.1/go.mod h1:QIE4lCeL7nkC25x+yA3LBIYfwCc1TFziCtG7
 github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8 h1:nIPpBwaJSVYIxUFsDv3M8ofmx9yWTog9BfvIu0q41lo=
 github.com/xi2/xz v0.0.0-20171230120015-48954b6210f8/go.mod h1:HUYIGzjTL3rfEspMxjDjgmT5uz5wzYJKVo23qUhYTos=
 github.com/xiang90/probing v0.0.0-20190116061207-43a291ad63a2/go.mod h1:UETIi67q53MR2AWcXfiuqkDkRtnGDLqkBTpCHuJHxtU=
+github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778 h1:QldyIu/L63oPpyvQmHgvgickp1Yw510KJOqX7H24mg8=
+github.com/xo/terminfo v0.0.0-20210125001918-ca9a967f8778/go.mod h1:2MuV+tbUrU1zIOPMxZ5EncGwgmMJsa+9ucAQZXxsObs=
 github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=

I will see later.

The goal was to share my progress on the Makefile default target (requires goreleaser v1.11.3+)

To be continued.

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