cloudflare / cfssl

CFSSL: Cloudflare's PKI and TLS toolkit
https://cfssl.org/
BSD 2-Clause "Simplified" License
8.71k stars 1.11k forks source link

build issues for package managers #1062

Open williamh opened 4 years ago

williamh commented 4 years ago

Hello,

I maintain the cfssl package on Gentoo Linux and I am running into a couple of small issues I am patching around to get our package manager to build cfssl.

The first is the way VERSION is set in the makefile. This doesn't work if you try to download the archive and build from there, which is what our package manager does. For example:

$ cd /tmp
$ wget https://github.com/cloudflare/cfssl/archive/v1.4.1.tar.gz
$ tar -xf v1.4.1.tar.gz
$ cd cfssl-1.4.1
$ make
fatal: not a git repository (or any of the parent directories): .git
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/cfssl ./cmd/cfssl
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/cfssl-bundle ./cmd/cfssl-bundle
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/cfssl-certinfo ./cmd/cfssl-certinfo
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/cfssl-newkey ./cmd/cfssl-newkey
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/cfssl-scan ./cmd/cfssl-scan
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/cfssljson ./cmd/cfssljson
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/mkbundle ./cmd/mkbundle
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=" -o bin/multirootca ./cmd/multirootca
$

I can pass VERSION= into the makefile, but that doesn't eliminat the attempt to run git and get a version from there:

$ make VERSION=1.4.1
fatal: not a git repository (or any of the parent directories): .git
go build -ldflags "-s -w -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/cfssl ./cmd/cfssl
...
$

The second issue is the use of -s and -w in LDFLAGS.

Normally stripping is handled by the package manager itself, so these options should not be the default.

I have a patch which definitely cannot be submitted to you because it is a hack,, but I was wondering if we can come up with a way of fixing these issues for everyone.

Thanks,

William

cbroglie commented 4 years ago

Thanks for the feedback, I think we can address with the following patch:

diff --git a/Makefile b/Makefile
index 10a52fc..163fbb8 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,6 @@
-VERSION := $(shell git describe --tags --abbrev=0 | tr -d '[:alpha:]')
-LDFLAGS := "-s -w -X github.com/cloudflare/cfssl/cli/version.version=$(VERSION)"
+VERSION ?= $(shell git describe --tags --abbrev=0 | tr -d '[:alpha:]')
+SRCFLAGS ?= -s -w
+LDFLAGS := "$(SRCFLAGS) -X github.com/cloudflare/cfssl/cli/version.version=$(VERSION)"

 export GOFLAGS := -mod=vendor
 export GOPROXY := off

Then you can run:

➜  cfssl-1.4.1 VERSION=1.4.1 SRCFLAGS="" make
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/cfssl ./cmd/cfssl
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/cfssl-bundle ./cmd/cfssl-bundle
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/cfssl-certinfo ./cmd/cfssl-certinfo
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/cfssl-newkey ./cmd/cfssl-newkey
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/cfssl-scan ./cmd/cfssl-scan
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/cfssljson ./cmd/cfssljson
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/mkbundle ./cmd/mkbundle
go build -ldflags " -X github.com/cloudflare/cfssl/cli/version.version=1.4.1" -o bin/multirootca ./cmd/multirootca
➜  cfssl-1.4.1

Does that work for you?

williamh commented 4 years ago

That would be fine.

I didn't write this as a p/r because I wasn't sure what the ramifications would be for changing from := to ?=.

Thanks,

William