The Wayback Machine - https://web.archive.org/web/20220530091424/https://github.com/rclone/rclone/pull/5979
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

Jump to bottom

Configurable version suffix independent of version number #5979

Merged
merged 1 commit into from May 17, 2022
Merged

Configurable version suffix independent of version number #5979

merged 1 commit into from May 17, 2022

Conversation

albertony
Copy link
Contributor

@albertony albertony commented Feb 4, 2022 •
edited

What is the purpose of this change?

Introduces a way to quickly set a custom pre-release label / version suffix on my own builds of rclone.

As of now master version is "v1.58.0-DEV". I can override this easily with something like -ldflags "-X github.com/rclone/rclone/fs.Version=v9.9.9-CUSTOM", but then I loose the version number it was based on. It would be better if I could keep the number, and only set my own suffix. That could be achived, of course, with a little sed magic or similar. But with this PR I can simply just set fs.VersionSuffix instead. This makes the build command very easy on any platform, and easy configurable in IDEs like VSCode etc.

Nice to have? Definitely not a must have. And perhaps there are better ways to do this already? Perhaps this file needs to be single variable-only to not break some build pipeline I'm not aware of?

PS: I would like to leave out the - separator from the VersionSuffix, which makes it more "semver-safe", but then it would no longer be variables-only file, so I started with the simplest "proof of concept" suggestion...

var Version = "v" + VersionNumber
if VersionSuffix != "" {
	Version += "-" + VersionSuffix
}

PS2: For reference, .NET uses the name VersionPrefix for the number part ("1.58.0") and VersionSuffix for the pre-release label ("DEV"), and Version for the combination of the two separated with -. And it also has a InformationalVersion which typically is the combination of Version and a commit hash separated with + ("1.58.0-DEV+27d5f545fa0e2f09629d0bcbb0e571fc79341f2b").

PS3: Speaking of semver-safe... in cmd/version/version.go and bin/cross-compile.go we parse the string into a semver struct (after first trimming off the "v" prefix which is not strict semver) and use the version number components:

rclone/bin/cross-compile.go

Lines 202 to 227 in aa2d7f0

version := strings.TrimPrefix(versionTag, "v")
semanticVersion := semver.New(version)
// Build json input to goversioninfo utility
bs, err := json.Marshal(M{
"FixedFileInfo": M{
"FileVersion": M{
"Major": semanticVersion.Major,
"Minor": semanticVersion.Minor,
"Patch": semanticVersion.Patch,
},
"ProductVersion": M{
"Major": semanticVersion.Major,
"Minor": semanticVersion.Minor,
"Patch": semanticVersion.Patch,
},
},
"StringFileInfo": M{
"CompanyName": "https://rclone.org",
"ProductName": "Rclone",
"FileDescription": "Rsync for cloud storage",
"InternalName": "rclone",
"OriginalFilename": "rclone.exe",
"LegalCopyright": "The Rclone Authors",
"FileVersion": version,
"ProductVersion": version,

Was the change discussed in an issue or in the forum before?

Checklist

  • I have read the contribution guidelines.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • I'm done, this Pull Request is ready for review :-)

@albertony albertony added the build label Feb 4, 2022
@albertony albertony self-assigned this Feb 4, 2022
@ncw
Copy link
Member

@ncw ncw commented Feb 6, 2022

Nice to have? Definitely not a must have. And perhaps there are better ways to do this already? Perhaps this file needs to be single variable-only to not break some build pipeline I'm not aware of?

What it breaks is this...

rclone/Makefile

Lines 245 to 259 in aa2d7f0

startdev:
@echo "Version is $(VERSION)"
@echo "Next version is $(NEXT_VERSION)"
echo -e "package fs\n\n// Version of rclone\nvar Version = \"$(NEXT_VERSION)-DEV\"\n" | gofmt > fs/version.go
echo -n "$(NEXT_VERSION)" > docs/layouts/partials/version.html
echo "$(NEXT_VERSION)" > VERSION
git commit -m "Start $(NEXT_VERSION)-DEV development" fs/version.go VERSION docs/layouts/partials/version.html
startstable:
@echo "Version is $(VERSION)"
@echo "Next stable version is $(NEXT_PATCH_VERSION)"
echo -e "package fs\n\n// Version of rclone\nvar Version = \"$(NEXT_PATCH_VERSION)-DEV\"\n" | gofmt > fs/version.go
echo -n "$(NEXT_PATCH_VERSION)" > docs/layouts/partials/version.html
echo "$(NEXT_PATCH_VERSION)" > VERSION
git commit -m "Start $(NEXT_PATCH_VERSION)-DEV development" fs/version.go VERSION docs/layouts/partials/version.html

@albertony
Copy link
Contributor Author

@albertony albertony commented Feb 6, 2022

Ah yes, that is; The makefile would revert this pr's changes next time it's run.

Pushed an alternative implementation, what do you think?

Now any of the following will work, depending on what you want to change, assuming default is "v1.58.0-DEV":

  • -ldflags "-X github.com/rclone/rclone/fs.VersionTag=v9.9.9" --> "v9.9.9-DEV"
  • -ldflags "-X github.com/rclone/rclone/fs.VersionSuffix=CUSTOM" --> "v1.58.0-CUSTOM"
  • -ldflags "-X github.com/rclone/rclone/fs.Version=v9.9.9-CUSTOM" --> "v9.9.9-CUSTOM"

Script editing the file, like the makefile does, is easy for VersionTag and VersionSuffix individually, but not the complete Version.

ncw
ncw reviewed May 13, 2022
View changes
Copy link
Member

@ncw ncw left a comment

I'm happy to merge this now, but it needs updating since we did a release, so the version number needs changing.

I'm sure its going to break something, but I can't think what!

If you rebase and update I'll merge

Thank you

@albertony
Copy link
Contributor Author

@albertony albertony commented May 15, 2022

I'm happy to merge this now, but it needs updating since we did a release, so the version number needs changing.

Done: Rebased, and updated version number according to master.

I'm sure its going to break something, but I can't think what!

Sure, I may have overlooked something (but then at least you get to yell "I told you" or "I knew it"... 🙇‍♂️ )

@ncw
Copy link
Member

@ncw ncw commented May 17, 2022

I'm happy to merge this now, but it needs updating since we did a release, so the version number needs changing.

Done: Rebased, and updated version number according to master.

Thank you.

I'm sure its going to break something, but I can't think what!

Sure, I may have overlooked something (but then at least you get to yell "I told you" or "I knew it"... bowing_man )

I predict that this will come to bite at release time or point release time - we shall see :-)

And if it goes wrong, it will be all on me as I did the review, so no yelling!

@ncw ncw merged commit ce168ec into rclone:master May 17, 2022
9 checks passed
@albertony albertony deleted the build-version-config branch May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncw ncw

Assignees

@albertony albertony

Labels
build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@albertony @ncw