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

Use "GoReleaser" #395

Merged
merged 24 commits into from
Oct 25, 2021
Merged

Use "GoReleaser" #395

merged 24 commits into from
Oct 25, 2021

Conversation

brandonbloom
Copy link
Member

No description provided.

@brandonbloom
Copy link
Member Author

@BenElgar I need to study the release machinery that @kendru put together to make sure I don't screw anything up switching over to this. Then I'll get the Homebrew package working. Once that's good, I'll ask you to investigate the Linux packaging story.

No ETA yet, this is stacked up behind a few things on my queue, so let me know if you get spare cycles for the Linux stuff before I get to the Mac stuff.

Thanks!

@brandonbloom
Copy link
Member Author

@BenElgar will take over this work & can do with this specific PR as he likes :)

@BenElgar
Copy link
Contributor

@brandonbloom okay I think that now this should, combined with https://github.com/deref/exo-install/pull/4/files, do the right thing. That being said, I haven't done a great deal of testing since it's kind of a nuisance to do it locally. Assuming you're happy, my plan is to test in prod by pushing a version number that precedes our current release.

The auto upgrade functionality that we've already released will stop working if we choose an initial version number less than 3.0. Alternatively I think we could stick with CalVer.

@BenElgar BenElgar marked this pull request as ready for review October 20, 2021 12:53

# Needed to create a Github release with archives.
#permissions:
# contents: write
Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably you need to uncomment this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah I was going to see if it worked without it before I set that flag.

.goreleaser.yml Outdated
- make codegen
#- make -C gui
env:
- CGO_ENABLED=0
Copy link
Member Author

Choose a reason for hiding this comment

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

If CGO_ENABLED is not 1, the build will succeed, but as soon as you try to open a Sqlite database, you'll encounter an error saying that CGO must be enabled.

.goreleaser.yml Outdated
hooks:
- go mod tidy
- make codegen
#- make -C gui
Copy link
Member Author

Choose a reason for hiding this comment

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

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, need to uncomment that — I just disabled it for testing.

.goreleaser.yml Outdated
- CGO_ENABLED=0
builds:
# This is the build for use in the install script.
- id: autoinstalled
Copy link
Member Author

Choose a reason for hiding this comment

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

nit picky: seems like an odd name for this. Does this name show up anywhere else? Like in the built files or whatever?

Copy link
Member Author

Choose a reason for hiding this comment

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

What about "standalone"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I was using the names from the files in ./internal/upgrade. Happy to change it to standalone though.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think "standalone" is much clearer to me. Please change. Thanks!

.goreleaser.yml Outdated
# This is the build for use by package managers.
- id: managed
main: .
goos:
Copy link
Member Author

@brandonbloom brandonbloom Oct 20, 2021

Choose a reason for hiding this comment

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

In addition to goos, we also need to build for goarch set to both amd64 and arm64, at least on darwin

Copy link
Contributor

Choose a reason for hiding this comment

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

The default is to build for arm64, amd64 and i386: https://goreleaser.com/customization/build/

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we need i386 support? Seems like that's extremely unlikely for any developer machine these days.

I'd suggest dropping that and making this configuration explicit.

Copy link
Member Author

@brandonbloom brandonbloom left a comment

Choose a reason for hiding this comment

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

I've identified a bunch of potential issues, but I realize that this is hard to test locally.

Is it possible to test this on a fork?

@BenElgar
Copy link
Contributor

Okay I tested this on a fork here: https://github.com/BenElgar/exo/releases

It seems to do the right thing and the homebrew install works on Linux at least. I'm going to test on macOS now.

@BenElgar
Copy link
Contributor

Ah no, it turns out we have a limit of at most zero mac instances in our AWS account. I've requested a limit increase but that usually takes a day or so.

@BenElgar
Copy link
Contributor

Okay I've tested and fixed the release process on the aforementioned fork and I've checked that it all works on macOS. Going to merge this.

@BenElgar BenElgar merged commit fa7b57e into main Oct 25, 2021
@BenElgar BenElgar deleted the releaser branch October 25, 2021 12:37
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