Sambigeara / fuzzynote

Terminal-based, hyper-fast, CRDT-backed, collaborative note-taking tool
GNU Affero General Public License v3.0
371 stars 8 forks source link

Please make version available from the commandline #72

Closed kseistrup closed 1 year ago

kseistrup commented 3 years ago

Problem

The version of the currently installed fzn binary is unknown:

$ fzn --version
2021/06/25 12:11:15 main : Parsing Root Config : version wanted
[1] $

Solution

It would be nice with a --version/-v switch that displays the version and exits successfully, e.g.:

$ fzn --version
fuzzynote/0.18.0 (2021-06-25)  # please use ISO-8601 date format
$ 

Cheers.

Sambigeara commented 3 years ago

Yeah, this should totally be a thing. Will get it done this eve 👍

Sambigeara commented 3 years ago

This was introduced in this change, available in v0.19.0

kseistrup commented 3 years ago

The Makefile has:

VERSION := $(shell git describe --abbrev=0 --tags)

This only works when building from a git clone. When building from the release source package you get:

$ bin/fzn --version
fuzzynote  (2021-06-25)

also (but perhaps that belongs in another issue):

build:
        go build -ldflags="-X main.version=$(VERSION)" -o bin/fzn ./cmd/term

that -ldflags assignment will shadow any -ldflags already found in $GOFLAGS. E.g., I have:

export GOFLAGS='-buildmode=pie -trimpath -ldflags=-linkmode=external -mod=readonly -modcacherw'

in my shell initialization file.

Sambigeara commented 3 years ago

... aaand reopened. Appreciate the thoroughness of review! Will rethink how to deal with this in a more agnostic way.

kseistrup commented 3 years ago

It's not an easy one. Instead of the usual make build I ended up using the following in the PKGBUILD file, following the guidelines of ArchLinux:

build() {
  cd "$pkgname-$pkgver" || exit 1

  # https://wiki.archlinux.org/title/Go_package_guidelines
  export CGO_CPPFLAGS="$CPPFLAGS"
  export CGO_CFLAGS="$CFLAGS"
  export CGO_CXXFLAGS="$CXXFLAGS"
  export CGO_LDFLAGS="$LDFLAGS"

  #make build

  go build \
    -buildmode=pie \
    -trimpath \
    -ldflags="-linkmode=external -X main.version=$pkgver" \
    -mod=readonly \
    -modcacherw \
    -o bin/fzn ./cmd/term
}
Sambigeara commented 3 years ago

Thanks! I'll work on tackling the git dependency in the first instance methinks.

Not sure I fully understand the implications of the appended -ldflags assignments after $GOFLAGS, could you elaborate?

Cheers

kseistrup commented 3 years ago

I don't know much about about building and linking go applications, but ArchLinux favours PIE executables and published some guidelines for building such PIE executables. I was merely appending fuzzynote's -ldflags to those from the guidelines (and I do indeed get a PIE executable).

Sambigeara commented 3 years ago

So this took me down a bit of a rabbithole.

I think this PR gets us part of the way there. It assumes a git source of truth, and I'm really not a fan of the VERSION file pattern, but at least it's auto-generated, and it seems to work for:

Your PKGBUILD conumdrum is a separate issue though, I think. For now, would it be possible to just add the additional flag in -ldflags? E.g.

-ldflags="-linkmode=external -X main.version=$pkgver" -X main.date=$foo \

Instead of

-ldflags="-linkmode=external -X main.version=$pkgver" \
kseistrup commented 3 years ago

Yeah, that's fine. Should the date reflect the release date ot the compile date?

Sambigeara commented 3 years ago

I'm not aware of best practice (I'm not super familiar with this stuff), but I figured that consistency was key here. The date will reflect the date of the specific tagged commit, across the board. In general, this will almost certainly be consistent with release date.

kseistrup commented 3 years ago

I agree. After reading the PR you showed me it makes sense.

Sambigeara commented 3 years ago

Nice! I'll roll it out.

I'll leave this issue open in the meantime as a prompt for me to look into making the build process more consistent (ref your PKGBUILD with explicit build command). Won't be imminent, but sometime in the future for sure.

Thanks for all your help!

Edit: hmm this doesn't quite work. Minor change requires to make sure VERSION is checked in with release commit

Sambigeara commented 3 years ago

Will wrap up in #78

Sambigeara commented 3 years ago

Released in https://github.com/Sambigeara/fuzzynote/releases/tag/v0.20.0

Good enough for now.