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

Add brew release command #10370

Merged
merged 4 commits into from Jan 23, 2021
Merged

Add brew release command #10370

merged 4 commits into from Jan 23, 2021

Conversation

Rylan12
Copy link
Member

@Rylan12 Rylan12 commented Jan 20, 2021

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?
  • Have you successfully run brew man locally and committed any changes?

This PR adds a new brew release developer command to automate the release process. The command, by default, will simply bump the patch version number. If --major or --minor is passed, the command will bump the major and minor version numbers respectively (and reset other numbers to 0 as needed).

This PR is still extremely rough (hence being marked as a draft). I'm going to immediately cancel CI because I know there are issues that I haven't yet fixed locally. However, I have a few general questions, so I figured I'd open this to track my progress.

I also plan on updating the Release docs in this PR in the future.

Additionally, there's some overlap between this command and brew release-notes that I plan on extracting to a new module.


Questions

  1. Should there be any interactivity in this command? At the moment, the whole thing runs in about 5 seconds so there isn't really any time for a frantic control-c if someone realizes they messed up. I wonder if we should make this require an additional confirmation before pushing the tags and creating the release. I know we generally like to avoid interactivity, but I don't think this command should even be used in a script so I'm not super worried about that. I'm open to other options here as well.
  2. What is the best way to determine the "current tag." At the moment, I'm simply running a git fetch origin (although I'm now realizing I probably want git fetch --tags origin) before the command starts and then simply taking the highest version number from git tag --list --sort=-version:refname *.*.*. Is there a way to determine the latest upstream tag rather than relying on local tags? I guess an alternative would be to query GitHub for the latest release and then use that as the base. An extension question would be: is it okay to assume that those who will run this command will be in a reasonable repository state?
  3. How should non-tip-of-the-master-branch releases be handled? I know that there's debate about this in Homebrew release policy #9331 so I don't need to duplicate that here. For the moment, I think I'm going to assume the following: the brew release command will be designed for creating releases only at the tip of the master branch. If a non-tip-of-the-master-branch release is desired, this command should not be used. Is that acceptable? If so, I'll probably either call brew update within the command or simply fail if the brew repo isn't up to date (or, to be more precise, I'll check that git rev-parse HEAD matched git rev-parse origin/HEAD).
  4. I think we should sign tags when possible. Should we run git tag --sign by default? If not, should we add a --sign flag to brew release to control this? I have tag.gpgSign enabled in my ~/.gitconfig so my tags would be signed either way, but I'm sure tha not all maintainers do.
  5. More specifically, how do I handle the scopes for the GitHub API call? I'm a little confused with what needs to be specified here. GitHub.open_api takes an optional scopes argument. In GitHub.create_pull_request, the GitHub.CREATE_ISSUE_FORK_OR_PR_SCOPES constant which is set to public_repo is passed. What (if any) scope needs to be used for creating a release? Obviously, some write access to the repo is needed. Just not totally sure what to do here and hoping someone can guide me through it :)

Quick Summary

Based on my answers to the questions above, here is what I'm picturing (at the moment) will happen when this command is run:

  1. git fetch --tags origin is run in the background.
  2. If git rev-parse HEAD doesn't match git rev-parse origin/HEAD, fail
  3. Find the most recent tag using git tag --list --sort=-version:refname *.*.*
  4. Generate the new tag based on args.major? and args.minor?
  5. If creating a major/minor release, check to make sure the latest release was less than a month ago. If so, fail unless --force is passed (which should change the failure into a warning that's still displayed)
  6. Create the new tag locally using git tag and maybe --sign
  7. Generate the release notes
  8. Asking for a final confirmation before pushing the tag and creating the release
  9. Run git push origin <tag> to push the tag
  10. Create a new release using the GitHub API
  11. Print the url to the release
  12. Open the url in a browser unless --no-browse is passed.

CC @MikeMcQuaid

@BrewTestBot
Copy link
Member

Review period will end on 2021-01-21 at 06:59:28 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Jan 20, 2021
@MikeMcQuaid
Copy link
Member

  1. Should there be any interactivity in this command?

No.

  1. I wonder if we should make this require an additional confirmation before pushing the tags and creating the release.

I would say it's not worth pushing the tags (which cannot be undone) but instead telling the maintainer what command to run to do so.

2. What is the best way to determine the "current tag." At the moment, I'm simply running a git fetch origin (although I'm now realizing I probably want git fetch --tags origin) before the command starts and then simply taking the highest version number from git tag --list --sort=-version:refname *.*.*.

Sounds good.

2. Is there a way to determine the latest upstream tag rather than relying on local tags? I guess an alternative would be to query GitHub for the latest release and then use that as the base.

Yes, query GitHub.

2. An extension question would be: is it okay to assume that those who will run this command will be in a reasonable repository state?

Yes.

3. For the moment, I think I'm going to assume the following: the brew release command will be designed for creating releases only at the tip of the master branch.

Sounds good (forever, in my opinion).

3. If a non-tip-of-the-master-branch release is desired, this command should not be used. Is that acceptable?

Yes.

4. I think we should sign tags when possible. Should we run git tag --sign by default?

Yes.

5. GitHub.open_api takes an optional scopes argument. In GitHub.create_pull_request, the GitHub.CREATE_ISSUE_FORK_OR_PR_SCOPES constant which is set to public_repo is passed. What (if any) scope needs to be used for creating a release?

public_repo should be sufficient. The GitHub API docs should be able to confirm.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Feel free to punt until PR further along but: would also be nice to reference this in the release docs when this PR is shipped.

Library/Homebrew/dev-cmd/release.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release.rb Outdated Show resolved Hide resolved
@Rylan12
Copy link
Member Author

Rylan12 commented Jan 20, 2021

Thanks, that's good feedback. I'm in agreement about not pushing the tags. Makes sense to not even risk it.

Can you create a draft release that isn't attached to a tag? I'll have to play around with it. It's certainly ideal if that's the case.

  1. GitHub.open_api takes an optional scopes argument. In GitHub.create_pull_request, the GitHub.CREATE_ISSUE_FORK_OR_PR_SCOPES constant which is set to public_repo is passed. What (if any) scope needs to be used for creating a release?

public_repo should be sufficient. The GitHub API docs should be able to confirm.

This is saying that the token being used must have access to all public_repos that the user it's associated with has access to, right?

@MikeMcQuaid
Copy link
Member

Can you create a draft release that isn't attached to a tag?

Yup! The tag isn't created until it's published. That's often the flow I do for creating a release.

This is saying that the token being used must have access to all public_repos that the user it's associated with has access to, right?

Yes, think so.

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Jan 21, 2021
@BrewTestBot
Copy link
Member

Review period ended.

@Rylan12 Rylan12 marked this pull request as ready for review January 21, 2021 22:53
@Rylan12
Copy link
Member Author

Rylan12 commented Jan 21, 2021

Okay, here's where this command currently stands.

Running brew release will create a draft PR with the correct release notes. It doesn't actually create the tag locally and push but relies on the tag being created when the new release is published. By default, the tag is created at the tip of the master branch. I believe this can be customized if we want, instead, to create the tag at the HEAD commit of the user running the release command. Since in almost all cases the release it at the tip (I know there's a debate about this), I don't think this is necessary.

The --major and --minor flags can be passed to create a new major or minor release, respectively. If neither is passed, a patch release will be made. The command will fail if a major/minor release is requested and the latest major/minor release was published less than one month ago (determined by querying the GitHub API).

In order to generate the appropriate release notes, this command needs to have the latest commits on origin/master fetched locally. I've added the auto-update functionality to the brew release command. To work around the case where HOMEBREW_NO_AUTO_UPDATE is set (which may be true for maintainers), I added an extra step that runs git fetch origin only if HOMEBREW_NO_AUTO_UPDATE is set. This will ensure that the latest master branch commits are available locally. Do we need to explicitly call git fetch origin master or is git fetch origin enough? Or another command?

Alternatively, another option is to query the GitHub API for the latest commit on the master branch. If it detects that the local origin/HEAD does not point to the same commit that the GitHub API reports, it will prompt the user to run brew update. Not sure which is preferred.

I've also refactored some of the shared logic between the brew release command and the brew release-notes command. I think the two commands are distinct enough that they should both be kept. However, the brew release-notes command is no longer used in our release process so could theoretically be removed.

Lastly, I've updated the release documentation.

Library/Homebrew/release_notes.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/release-notes.rb Outdated Show resolved Hide resolved
@MikeMcQuaid
Copy link
Member

Running brew release will create a draft PR with the correct release notes. It doesn't actually create the tag locally and push but relies on the tag being created when the new release is published. By default, the tag is created at the tip of the master branch. I believe this can be customized if we want, instead, to create the tag at the HEAD commit of the user running the release command. Since in almost all cases the release it at the tip (I know there's a debate about this), I don't think this is necessary.

👍🏻

In order to generate the appropriate release notes, this command needs to have the latest commits on origin/master fetched locally. I've added the auto-update functionality to the brew release command. To work around the case where HOMEBREW_NO_AUTO_UPDATE is set (which may be true for maintainers), I added an extra step that runs git fetch origin only if HOMEBREW_NO_AUTO_UPDATE is set. This will ensure that the latest master branch commits are available locally. Do we need to explicitly call git fetch origin master or is git fetch origin enough? Or another command?

Alternatively, another option is to query the GitHub API for the latest commit on the master branch. If it detects that the local origin/HEAD does not point to the same commit that the GitHub API reports, it will prompt the user to run brew update. Not sure which is preferred.

I am fine with any of these options. I'd probably suggest "whichever one requires the least code to handle this edge case".

I've also refactored some of the shared logic between the brew release command and the brew release-notes command. I think the two commands are distinct enough that they should both be kept. However, the brew release-notes command is no longer used in our release process so could theoretically be removed.

Good to know. Perhaps it's worth deprecating? Could add a commented-out odeprecated and I can remove on the next release?

Library/Homebrew/dev-cmd/release.rb Show resolved Hide resolved
Library/Homebrew/release_notes.rb Show resolved Hide resolved
@Rylan12
Copy link
Member Author

Rylan12 commented Jan 23, 2021

In order to generate the appropriate release notes, this command needs to have the latest commits on origin/master fetched locally. I've added the auto-update functionality to the brew release command. To work around the case where HOMEBREW_NO_AUTO_UPDATE is set (which may be true for maintainers), I added an extra step that runs git fetch origin only if HOMEBREW_NO_AUTO_UPDATE is set. This will ensure that the latest master branch commits are available locally. Do we need to explicitly call git fetch origin master or is git fetch origin enough? Or another command?
Alternatively, another option is to query the GitHub API for the latest commit on the master branch. If it detects that the local origin/HEAD does not point to the same commit that the GitHub API reports, it will prompt the user to run brew update. Not sure which is preferred.

I am fine with any of these options. I'd probably suggest "whichever one requires the least code to handle this edge case".

Sounds good. I went with the one-line git fetch solution.

@Rylan12
Copy link
Member Author

Rylan12 commented Jan 23, 2021

Okay, I'm going to go ahead and merge this. @MikeMcQuaid let me know how this works! (I'm assuming you'll be the one to make the next release).

@Rylan12 Rylan12 merged commit 2c83ea7 into Homebrew:master Jan 23, 2021
@Rylan12 Rylan12 deleted the brew-release branch January 23, 2021 20:35
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Feb 24, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Feb 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants