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 specification versioning- and release management instructions and checks #87

Merged
merged 3 commits into from
Mar 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
language: python

# Disable auto-cloning and ...
git:
clone: false

# ... instead manually fetch and checkout the pull request source branch, which
# is expected by check_release.py
# https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally
install:
- git clone --depth=50 https://github.com/${TRAVIS_REPO_SLUG}.git ${TRAVIS_REPO_SLUG}
- cd ${TRAVIS_REPO_SLUG}
- git fetch origin pull/${TRAVIS_PULL_REQUEST}/head:${TRAVIS_PULL_REQUEST_BRANCH}
- git checkout -qf ${TRAVIS_PULL_REQUEST_BRANCH}

script:
- python check_release.py
33 changes: 31 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
The Update Framework specification
----------------------------------

Latest: `version 1.0.0 <https://github.com/theupdateframework/specification/blob/master/tuf-spec.md>`_
- `latest stable <https://github.com/theupdateframework/specification/blob/master/tuf-spec.md>`_
- `current draft <https://github.com/theupdateframework/specification/blob/draft/tuf-spec.md>`_
- `new changes since latest stable <https://github.com/theupdateframework/specification/compare/master..draft>`_
- `release history <https://github.com/theupdateframework/specification/tags>`_


Contact
Expand Down Expand Up @@ -31,7 +34,33 @@ Versioning
----------

The TUF specification uses `Semantic Versioning 2.0.0 <https://semver.org/>`_
for its version numbers.
(semver) for its version numbers, and a gitflow-based release management:

- The 'master' branch of this repository always points to the latest stable
version of the specification.
- The 'draft' branch of this repository always points to the latest development
version of the specification and must always be based off of the latest
'master' branch.
- Contributors must submit changes as pull requests against these branches,
depending on the type of the change (see semver rules).
- For patch-type changes, pull requests may be submitted directly against the
'master' branch.
- For major- and minor-type changes, pull requests must be submitted against
the 'draft' branch.
Copy link
Member

Choose a reason for hiding this comment

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

It's slightly odd to me that the person sending a PR doesn't indicate minor vs major. Also, it sounds like those are getting intertwined in a way we may not want. What if we want to push a release with some backwards compat (minor version) changes but not others that are not backwards compatible? Do we disentangle these changes at that time?

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to have a model where there are major, minor, and patch branches and they interrelate in the way your master and draft branches do.

I like the overall approach though!

Copy link
Member Author

Choose a reason for hiding this comment

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

It's slightly odd to me that the person sending a PR doesn't indicate minor vs major.

I think the person who sends the PR should not be bothered with the question into what future release the submitted change goes, regardless of it being major, minor or patch. This is the task of the maintainer. Maybe I should add a note to this section that says maintainers may change the target of a pending PR?

Also, it sounds like those are getting intertwined in a way we may not want. What if we want to push a release with some backwards compat (minor version) changes but not others that are not backwards compatible? Do we disentangle these changes at that time?

Coordinating major and minor releases can still be done, either by holding back PRs from being merged into draft, or, if it's too late for that, by shuffling around the git history on draft. The consensus in #73 seemed to be that the operational simplicity of having one working branch, plus the option to directly patch master, outweighs the flexibility of multiple permanent draft branches.

I like the overall approach though!

Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree with @lukpueh, we want to make PRs as simple as possible. As long as we merge draft into master regularly, I don't think that we will need any additional branches.

- Maintainers may, from time to time, decide that the 'draft' branch is ready
for a new major or minor release, and submit a pull request from 'draft'
against 'master'.
- Before merging a branch with 'master' the 'last modified date' and 'version'
in the specification header must be bumped.
- Merges with 'master' that originate from the 'draft' branch must bump either
the major or minor version number.
- Merges with 'master' that originate from any other branch must bump the patch
version number.
- Merges with 'master' must be followed by a git tag for the new version
number.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is currently not verified by check_release.py, because the script is executed before the merge, and the tag should probably be created after the merge. It would be nice to automate tag or release creation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Any thoughts, anyone?

Copy link
Member

Choose a reason for hiding this comment

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

I agree this would be nice to automate.

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I can look into it.

- Merges with 'master' must be followed by a rebase of 'draft' onto 'master'.



Acknowledgements
----------------
Expand Down
160 changes: 160 additions & 0 deletions check_release.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
"""
<Program Name>
check_release.py

<Author>
Lukas Puehringer <[email protected]>

<Started>
Jan 3, 2020

<Copyright>
See LICENSE for licensing information.

<Purpose>
Check that specification updates are performed according to the versioning
requirements in README.rst.

Expects Travis environment variables:
- TRAVIS_BRANCH
- TRAVIS_PULL_REQUEST_BRANCH
(see https://docs.travis-ci.com/user/environment-variables/)

"""
import os
import re
import sys
import shlex
import datetime
import subprocess

SPEC_NAME = "tuf-spec.md"

LAST_MODIFIED_PATTERN = "Last modified: **%d %B %Y**\n"
LAST_MODIFIED_LINENO = 3

VERSION_PATTERN = r"^Version: \*\*(\d*)\.(\d*)\.(\d*)\*\*$"
VERSION_LINENO = 5

class SpecError(Exception):
"""Common error message part. """
def __init__(self, msg):
super().__init__(
msg + " please see 'Versioning' section in README.rst for details.")


def get_spec_head():
"""Return the lines (as list) at the head of the file that contain last date
modified and version. """
with open(SPEC_NAME) as spec_file:
spec_head = [next(spec_file)
for x in range(max(VERSION_LINENO, LAST_MODIFIED_LINENO))]

return spec_head


def get_date(spec_head):
"""Parse date from spec head and return datetime object. """
last_modified_line = spec_head[LAST_MODIFIED_LINENO - 1]

try:
date = datetime.datetime.strptime(last_modified_line,
LAST_MODIFIED_PATTERN)

except ValueError as e:
raise SpecError("expected to match '{}' (datetime format) in line {}, but"
" got '{}': {}.".format(LAST_MODIFIED_PATTERN, LAST_MODIFIED_LINENO,
last_modified_line, e))

return date


def get_version(spec_head):
"""Parse version from spec head and return (major, minor, patch) tuple. """
version_line = spec_head[VERSION_LINENO - 1]

version_match = re.search(VERSION_PATTERN, version_line)
if not version_match:
raise SpecError("expected to match '{}' (regex) in line {}, but got '{}'."
.format(VERSION_PATTERN, VERSION_LINENO, version_line))

return version_match.groups()


def main():
"""Check that the current branch is based off of the master branch and that
the last modified date and version number in the specification document
header are higher than in the master branch, i.e. were bumped. """

# Skip version and date comparison on push builds ...
# As per https://docs.travis-ci.com/user/environment-variables/
# if the current job is a push build, this [env] variable is empty ("")
if not os.environ.get("TRAVIS_PULL_REQUEST_BRANCH"):
print("skipping version and date check for non pr builds ...")
sys.exit(0)

# ... also skip on PRs that don't target the master branch
# As per https://docs.travis-ci.com/user/environment-variables/:
# for builds triggered by a pull request this [env variable] is the name of
# the branch targeted by the pull request
if not os.environ.get("TRAVIS_BRANCH") == "master":
print("skipping version and date for builds that don't target master ...")
sys.exit(0)

# Check that the current branch is based off of the master branch
try:
subprocess.run(
shlex.split("git merge-base --is-ancestor master {}".format(
os.environ["TRAVIS_PULL_REQUEST_BRANCH"])), check=True)

except subprocess.CalledProcessError as e:
raise SpecError("make sure the current branch is based off of master")

# Read the first few lines from the updated specification document and
# extract date and version from spec file header in the current branch
spec_head = get_spec_head()
date_new = get_date(spec_head)
version_new = get_version(spec_head)

# Checkout master branch
subprocess.run(shlex.split("git checkout -q master"), check=True)

# Read the first few lines from the previous specification document and
# extract date and version from spec file header in the master branch
spec_head = get_spec_head()
date_prev = get_date(spec_head)
version_prev = get_version(spec_head)

# Assert date update
if not date_new > date_prev:
raise SpecError("new 'last modified date' ({:%d %B %Y}) must be greater"
" than the previous one ({:%d %B %Y})".format(date_new, date_prev))

# Assert version bump type depending on the PR originating branch
# - if the originating branch is 'draft', it must be a major (x)or minor bump
# - otherwise, it must be a patch bump
if os.environ["TRAVIS_PULL_REQUEST_BRANCH"] == "draft":
if not (((version_new[0] > version_prev[0]) !=
(version_new[1] > version_prev[1])) and
(version_new[2] == version_prev[2])):
raise SpecError("new version ({}) must have exactly one of a greater"
" major or a greater minor version number than the previous one ({}),"
" if the PR originates from the 'draft' branch.".format(version_new,
version_prev))

else:
if not (version_new[:2] == version_prev[:2] and
version_new[2] > version_prev[2]):
raise SpecError("new version ({}) must have exactly a greater patch"
" version number than the previous one ({}), if the PR originates"
" from a feature branch (i.e. not 'draft')".format(version_new,
version_prev))

print("*"*68)
print("thanks for correctly bumping version and last modified date. :)")
print("don't forget to tag the release and to sync 'draft' with master!! :P")
print("*"*68)


if __name__ == '__main__':
main()