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

Version File Plugin #2107

Merged
merged 16 commits into from May 20, 2022
Merged

Version File Plugin #2107

merged 16 commits into from May 20, 2022

Conversation

KetanReddy
Copy link
Collaborator

@KetanReddy KetanReddy commented Nov 12, 2021

What Changed

Adding a plugin to add support for repositories that use a flat file to maintain version. The plugin itself is very minimal as it delegates the release responsibilities to an optional script present in the repository to do the heavy lifting for doing any builds/releases. The plugin is primarily for handling the versioning and orchestrating when to call the release script.

Why

No current plugins support this requirement.

Todo:

  • Add tests
  • Add docs

Change Type

Indicate the type of change your pull request is:

  • documentation
  • patch
  • minor
  • major

🐤 Download canary assets:

auto-linux--canary.2107.25360.gz
auto-macos--canary.2107.25360.gz
auto-win.exe--canary.2107.25360.gz

📦 Published PR as canary version: under canary scope @auto-canary@10.33.0--canary.2107.25360.0

✨ Test out this PR locally via:

npm install @auto-canary/bot-list@10.33.0--canary.2107.25360.0
npm install @auto-canary/auto@10.33.0--canary.2107.25360.0
npm install @auto-canary/core@10.33.0--canary.2107.25360.0
npm install @auto-canary/package-json-utils@10.33.0--canary.2107.25360.0
npm install @auto-canary/all-contributors@10.33.0--canary.2107.25360.0
npm install @auto-canary/brew@10.33.0--canary.2107.25360.0
npm install @auto-canary/chrome@10.33.0--canary.2107.25360.0
npm install @auto-canary/cocoapods@10.33.0--canary.2107.25360.0
npm install @auto-canary/conventional-commits@10.33.0--canary.2107.25360.0
npm install @auto-canary/crates@10.33.0--canary.2107.25360.0
npm install @auto-canary/docker@10.33.0--canary.2107.25360.0
npm install @auto-canary/exec@10.33.0--canary.2107.25360.0
npm install @auto-canary/first-time-contributor@10.33.0--canary.2107.25360.0
npm install @auto-canary/gem@10.33.0--canary.2107.25360.0
npm install @auto-canary/gh-pages@10.33.0--canary.2107.25360.0
npm install @auto-canary/git-tag@10.33.0--canary.2107.25360.0
npm install @auto-canary/gradle@10.33.0--canary.2107.25360.0
npm install @auto-canary/jira@10.33.0--canary.2107.25360.0
npm install @auto-canary/magic-zero@10.33.0--canary.2107.25360.0
npm install @auto-canary/maven@10.33.0--canary.2107.25360.0
npm install @auto-canary/microsoft-teams@10.33.0--canary.2107.25360.0
npm install @auto-canary/npm@10.33.0--canary.2107.25360.0
npm install @auto-canary/omit-commits@10.33.0--canary.2107.25360.0
npm install @auto-canary/omit-release-notes@10.33.0--canary.2107.25360.0
npm install @auto-canary/pr-body-labels@10.33.0--canary.2107.25360.0
npm install @auto-canary/released@10.33.0--canary.2107.25360.0
npm install @auto-canary/s3@10.33.0--canary.2107.25360.0
npm install @auto-canary/sbt@10.33.0--canary.2107.25360.0
npm install @auto-canary/slack@10.33.0--canary.2107.25360.0
npm install @auto-canary/twitter@10.33.0--canary.2107.25360.0
npm install @auto-canary/upload-assets@10.33.0--canary.2107.25360.0
npm install @auto-canary/version-file@10.33.0--canary.2107.25360.0
npm install @auto-canary/vscode@10.33.0--canary.2107.25360.0
# or 
yarn add @auto-canary/bot-list@10.33.0--canary.2107.25360.0
yarn add @auto-canary/auto@10.33.0--canary.2107.25360.0
yarn add @auto-canary/core@10.33.0--canary.2107.25360.0
yarn add @auto-canary/package-json-utils@10.33.0--canary.2107.25360.0
yarn add @auto-canary/all-contributors@10.33.0--canary.2107.25360.0
yarn add @auto-canary/brew@10.33.0--canary.2107.25360.0
yarn add @auto-canary/chrome@10.33.0--canary.2107.25360.0
yarn add @auto-canary/cocoapods@10.33.0--canary.2107.25360.0
yarn add @auto-canary/conventional-commits@10.33.0--canary.2107.25360.0
yarn add @auto-canary/crates@10.33.0--canary.2107.25360.0
yarn add @auto-canary/docker@10.33.0--canary.2107.25360.0
yarn add @auto-canary/exec@10.33.0--canary.2107.25360.0
yarn add @auto-canary/first-time-contributor@10.33.0--canary.2107.25360.0
yarn add @auto-canary/gem@10.33.0--canary.2107.25360.0
yarn add @auto-canary/gh-pages@10.33.0--canary.2107.25360.0
yarn add @auto-canary/git-tag@10.33.0--canary.2107.25360.0
yarn add @auto-canary/gradle@10.33.0--canary.2107.25360.0
yarn add @auto-canary/jira@10.33.0--canary.2107.25360.0
yarn add @auto-canary/magic-zero@10.33.0--canary.2107.25360.0
yarn add @auto-canary/maven@10.33.0--canary.2107.25360.0
yarn add @auto-canary/microsoft-teams@10.33.0--canary.2107.25360.0
yarn add @auto-canary/npm@10.33.0--canary.2107.25360.0
yarn add @auto-canary/omit-commits@10.33.0--canary.2107.25360.0
yarn add @auto-canary/omit-release-notes@10.33.0--canary.2107.25360.0
yarn add @auto-canary/pr-body-labels@10.33.0--canary.2107.25360.0
yarn add @auto-canary/released@10.33.0--canary.2107.25360.0
yarn add @auto-canary/s3@10.33.0--canary.2107.25360.0
yarn add @auto-canary/sbt@10.33.0--canary.2107.25360.0
yarn add @auto-canary/slack@10.33.0--canary.2107.25360.0
yarn add @auto-canary/twitter@10.33.0--canary.2107.25360.0
yarn add @auto-canary/upload-assets@10.33.0--canary.2107.25360.0
yarn add @auto-canary/version-file@10.33.0--canary.2107.25360.0
yarn add @auto-canary/vscode@10.33.0--canary.2107.25360.0

@adierkens adierkens added the minor Increment the minor version when merged label Nov 12, 2021
@codecov
Copy link

codecov bot commented Nov 15, 2021

Codecov Report

Merging #2107 (a9cac5d) into main (1d1ba5c) will increase coverage by 0.12%.
The diff coverage is 88.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2107      +/-   ##
==========================================
+ Coverage   80.20%   80.32%   +0.12%     
==========================================
  Files          66       67       +1     
  Lines        5415     5495      +80     
  Branches     1263     1276      +13     
==========================================
+ Hits         4343     4414      +71     
- Misses        709      713       +4     
- Partials      363      368       +5     
Impacted Files Coverage Δ
plugins/version-file/src/index.ts 88.75% <88.75%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fe51a2...a9cac5d. Read the comment docs.


## Installation

This plugin is not included with the `auto` CLI installed via NPM. To install:
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be worth bundling this into the binary version so that projects that don't have any NPM deps can still consume it easily.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Gotcha! I think that was included/set by default when the plugin was scaffolded. I will look into changing that.

@@ -0,0 +1,24 @@
# Bazel Plugin
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looking at the code, I'm wondering if this just makes sense to be a version-file-plugin instead of a Bazel one, since there isn't anything Bazel specific in here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fair point. Is there an easy way to rename a plugin and its references?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed, a Bazel plugin will be interesting though. If that is the purpose of this PR, maybe we should consider how Auto could stamp package.json that are output to a bazel-bin folder? At the end of the day, Bazel shouldn't look at a version file and have that impact the build, Auto should take the outputs of Bazel and release them appropriately, thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

With auto would we even need the VERSION file?

Could we just do something like

#!/usr/bin/env bash
echo STABLE_GIT_COMMIT $(git rev-parse HEAD)
echo STABLE_AUTO_VERSION $(auto version)

as the workspace-status-script? Then all of the build-in mechanisms for stamping a release would carry over and not be tooling dependent.

We'd still need a source of truth (and maybe that's where the VERSION file comes into play), but could easily also be done using the existing git tag plugin.

auto shipit would be interesting though since it'd have to know that bazel command to call

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah that could work, I think we'll just need to make sure auto creates a GitHub release with that still so that the next SemVer calculation will have appropriate context.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Bazel shouldn't look at a version file and have that impact the build

Just to note, the version file approach was heavily influenced by how the publishing Bazel libraries that I've used require the version to be stored:

https://github.com/vaticle/bazel-distribution

So this idea is somewhat interesting. The version file would be needed by the publish targets (if using those rules, which we most likely will be since I already patched those rules to work for our Maven releases), but that wouldn't cause a complete rebuild. Additionally, the idea with this is that whatever publish script you include here would stamp the Bazel build while actually publishing if the version is needed in the actual source.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's a fair callout, assuming that distribution via version file is the expectation within Bazel (admittedly my understanding of non JS/Go publishes is limited by comparison). I have seen recommendations from some rule maintainers to use scripts to correctly query for releasable assets and then substitute the version in via workspace status as @adierkens mentioned. In that case, each rule would just need to inject STABLE_AUTO_VERSION appropriately for the version.

Copy link
Collaborator

Choose a reason for hiding this comment

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

assuming that distribution via version file is the expectation within Bazel

Not really the expectation within Bazel, just the distribution rules that I linked above, which include rules for NPM distribution as well. Even if not using these rules, it might make sense to be able to standardize the repo such that it can be easy to version without Auto, but still work with rules of this style. In addition, it is very easy to stamp with a VERSION file:

#!/usr/bin/env bash
echo STABLE_VERSION $(cat VERSION)
echo STABLE_GIT_COMMIT $(git rev-parse HEAD)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This does touch on an issue I did come across while writing this PR. If in a bazel repo you have a script that does the build/stamp/release you don't know what version you need to stamp the built packages with until after auto figures it out as part of shipit. So when auto runs the release script as part of whatever kind of release it is (regular, canary, next) the release script needs to restamp the packages then run release. By having a version file and having auto change it, bazel automatically knows it just needs to rerun the stamping step before running its release targets. This flow would prevent bazel from having to rebuild everything from scratch.

@KetanReddy
Copy link
Collaborator Author

@sugarmanz Comments have been addressed. Bazel -> Version File. Release script is optional. Tests have been updated. Is now included in the cli.

@KetanReddy KetanReddy changed the title [WIP] Bazel plugin Version File Plugin Nov 16, 2021
@KetanReddy KetanReddy marked this pull request as ready for review November 16, 2021 00:55
## Options

- versionFile (optional, default="VERSION"): Path to where the version is stored in the repository. It should be a file containing just the semver.
- releaseScript: (optional, default=None): Path to script that runs the publish actions in your repository. If not supplied nothing will be called. If supplied will be called during the `publish`,`canary` and `next` hooks. For the `publish` hook the first parameter passed to the script will be `release` to indicate that a regular release is being called. For `canary` and `next` hooks the first parameter will be `snapshot` to indicate a prerelease version.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Now that you've called it out, should probably keep naming consistent here. If this is only called in the publish hook, then it'd be more appropriately named publishScript or something like that.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I will make it more explicit. Changes are pushed and rebuilding.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@KetanReddy looks like you did update the naming in the actual plugin. Can you update the docs to reflect those changes?

@KetanReddy
Copy link
Collaborator Author

Any update on this @sugarmanz @adierkens

@hipstersmoothie
Copy link
Collaborator

@KetanReddy @adierkens this pr good to go?

@hipstersmoothie hipstersmoothie merged commit 233ebdc into intuit:main May 20, 2022
@intuit-svc
Copy link

🚀 PR was released in v10.37.0 🚀

@intuit-svc intuit-svc added the released This issue/pull request has been released. label May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor Increment the minor version when merged released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants