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
Allow version to be set manually #5072
Conversation
When Caddy is built from a release tarball (as downloaded from GitHub), `caddy version` returns an empty string. This causes confusion for downstream packagers. With this commit, VersionString can be set with eg. go build (...) -ldflags '-X (...).VersionString=v1.2.3' Then the short form version will be "v1.2.3", and the full version string will begin with "v1.2.3 ".
See #5065 for some discussion on this. |
Thanks! I just pushed some revisions that prefer the embedded version if available, but still prepends the CustomVersion (renamed from VersionString) if set. Let me know if that's OK with you, and if so I'll merge this and maybe release 2.6.2 here pretty soon. |
If I'm reading it correctly, It looks like it is still possible for if full == "" {
full = CustomVersion
} else if CustomVersion != "" {
full = CustomVersion + " " + full
} If Perhaps instead, default an empty if simple == "" || simple == "(devel)" {
simple = "unknown"
if CustomVersion != "" {
simple = CustomVersion
} else {
simple = "unknown"
}
}
if full == "" {
if CustomVersion == "" {
full = simple
} else {
full = CustomVersion
}
} else {
full = CustomVersion + " " + full
} |
The first It may also be safe to assume that an empty if simple == "" || simple == "(devel)" {
if CustomVersion != "" {
simple = CustomVersion
} else {
simple = "unknown"
}
}
if full == "" {
full = simple
} else {
full = CustomVersion + " " + full
} |
(Can you do inline comments next time? That'll make code review easier.)
It is still prepended: https://github.com/caddyserver/caddy/pull/5072/files#diff-57caf2be698e920a2599b611efd1b33dea35fd79976cacdf3354ab3f55706515R932
Yes, but only if there is absolutely no version information embedded and CustomVersion is empty, then full will be empty. This is probably OK, rather than covering up the fact with a non-empty value? I dunno.
Are you sure that's true? Not in my testing. Let's sort out those understandings and then we can make more tweaks if needed. |
@assistcontrol Is there anything else before I go ahead and merge this in? Will probably do so in a day or two otherwise. |
Sorry, IRL has been busy.
All it takes for there to be absolutely no version information embedded is to download any of the tarballs on the GH releases page. I guess the question is, when someone downloads a tarball from the GH release page and doesn't know to set |
I guess we could print "unknown" for the full version too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI is stuck, so going ahead and merging. Thanks for your contribution @assistcontrol !
GitHub incident: https://www.githubstatus.com/incidents/smn1qtqvbsb6
When Caddy is built from a release tarball (as downloaded from GitHub),
caddy version
returns an empty string. This causes confusion for downstream packagers.With this commit, VersionString can be set with eg.
go build (...) -ldflags '-X (...).VersionString=v1.2.3'
Then the short form version will be "v1.2.3", and the full version string will begin with "v1.2.3 ".