You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jo...@apache.org on 2017/07/18 00:31:55 UTC
[1/4] mesos git commit: Linted support/verify-reviews.py.
Repository: mesos
Updated Branches:
refs/heads/master 3c264284e -> 92896871b
Linted support/verify-reviews.py.
This will allow us to use PyLint on the
entire support directory in the future.
Review: https://reviews.apache.org/r/60236/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/92896871
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/92896871
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/92896871
Branch: refs/heads/master
Commit: 92896871bdb9f02066a403da789cdb70e6e496e4
Parents: a536d10
Author: Armand Grillet <ag...@mesosphere.io>
Authored: Mon Jul 17 16:33:56 2017 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Mon Jul 17 17:18:12 2017 -0700
----------------------------------------------------------------------
support/verify-reviews.py | 127 ++++++++++++++++++++++++++++-------------
1 file changed, 88 insertions(+), 39 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/92896871/support/verify-reviews.py
----------------------------------------------------------------------
diff --git a/support/verify-reviews.py b/support/verify-reviews.py
index eb8f76e..1077b08 100755
--- a/support/verify-reviews.py
+++ b/support/verify-reviews.py
@@ -1,7 +1,37 @@
-#!/usr/bin/env python2.7
-
-# Provides a tool to verify Mesos reviews that are submitted to
-# Review Board.
+#!/usr/bin/env python
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This script is used to build and test (verify) reviews that are posted
+to ReviewBoard. The script is intended for use by automated "ReviewBots"
+that are run on ASF infrastructure (or by anyone that wishes to donate
+some compute power). For example, see 'support/jenkins/reviewbot.sh'.
+
+The script performs the following sequence:
+* A query grabs review IDs from Reviewboard.
+* In reverse order (most recent first), the script determines if the
+ review needs verification (if the review has been updated or changed
+ since the last run through this script).
+* For each review that needs verification:
+ * The review is applied (via 'support/apply-reviews.py').
+ * Mesos is built and unit tests are run.
+ * The result is posted to ReviewBoard.
+"""
import atexit
import json
@@ -13,22 +43,23 @@ import urllib
import urllib2
import urlparse
-from datetime import datetime, timedelta
+from datetime import datetime
REVIEWBOARD_URL = "https://reviews.apache.org"
-REVIEW_SIZE = 1000000 # 1 MB in bytes.
+REVIEW_SIZE = 1000000 # 1 MB in bytes.
# TODO(vinod): Use 'argparse' module.
# Get the user and password from command line.
if len(sys.argv) < 3:
- print "Usage: ./verify-reviews.py <user> <password> [num-reviews] [query-params]"
+ print("Usage: ./verify-reviews.py <user>"
+ "<password> [num-reviews] [query-params]")
sys.exit(1)
USER = sys.argv[1]
PASSWORD = sys.argv[2]
# Number of reviews to verify.
-NUM_REVIEWS = -1 # All possible reviews.
+NUM_REVIEWS = -1 # All possible reviews.
if len(sys.argv) >= 4:
NUM_REVIEWS = int(sys.argv[3])
@@ -43,10 +74,12 @@ if len(sys.argv) >= 5:
class ReviewError(Exception):
- pass
+ """Exception returned by post_review()."""
+ pass
def shell(command):
+ """Run a shell command."""
print command
return subprocess.check_output(
command, stderr=subprocess.STDOUT, shell=True)
@@ -56,6 +89,7 @@ HEAD = shell("git rev-parse HEAD")
def api(url, data=None):
+ """Call the ReviewBoard API."""
try:
auth_handler = urllib2.HTTPBasicAuthHandler()
auth_handler.add_password(
@@ -68,20 +102,22 @@ def api(url, data=None):
urllib2.install_opener(opener)
return json.loads(urllib2.urlopen(url, data=data).read())
- except urllib2.HTTPError as e:
- print "Error handling URL %s: %s (%s)" % (url, e.reason, e.read())
+ except urllib2.HTTPError as err:
+ print "Error handling URL %s: %s (%s)" % (url, err.reason, err.read())
exit(1)
- except urllib2.URLError as e:
- print "Error handling URL %s: %s" % (url, e.reason)
+ except urllib2.URLError as err:
+ print "Error handling URL %s: %s" % (url, err.reason)
exit(1)
def apply_review(review_id):
+ """Apply a review using the script apply-reviews.py."""
print "Applying review %s" % review_id
shell("python support/apply-reviews.py -n -r %s" % review_id)
def apply_reviews(review_request, reviews):
+ """Apply multiple reviews at once."""
# If there are no reviewers specified throw an error.
if not review_request["target_people"]:
raise ReviewError("No reviewers specified. Please find a reviewer by"
@@ -107,23 +143,26 @@ def apply_reviews(review_request, reviews):
def post_review(review_request, message):
+ """Post a review on the review board."""
print "Posting review: %s" % message
review_url = review_request["links"]["reviews"]["href"]
- data = urllib.urlencode({'body_top' : message, 'public' : 'true'})
+ data = urllib.urlencode({'body_top': message, 'public': 'true'})
api(review_url, data)
@atexit.register
def cleanup():
+ """Clean the git repository."""
try:
shell("git clean -fd")
shell("git reset --hard %s" % HEAD)
- except subprocess.CalledProcessError as e:
- print "Failed command: %s\n\nError: %s" % (e.cmd, e.output)
+ except subprocess.CalledProcessError as err:
+ print "Failed command: %s\n\nError: %s" % (err.cmd, err.output)
def verify_review(review_request):
+ """Verify a review."""
print "Verifying review %s" % review_request["id"]
build_output = "build_" + str(review_request["id"])
@@ -132,7 +171,7 @@ def verify_review(review_request):
reviews = []
apply_reviews(review_request, reviews)
- reviews.reverse() # Reviews are applied in the reverse order.
+ reviews.reverse() # Reviews are applied in the reverse order.
command = ""
if platform.system() == 'Windows':
@@ -155,11 +194,11 @@ def verify_review(review_request):
command = "%s; ./support/docker-build.sh" % configuration
-
- # `tee` the output so that the console can log the whole build output.
- # `pipefail` ensures that the exit status of the build command is
- # preserved even after tee'ing.
- subprocess.check_call(['bash', '-c', 'set -o pipefail; %s 2>&1 | tee %s'
+ # `tee` the output so that the console can log the whole build
+ # output. `pipefail` ensures that the exit status of the build
+ # command ispreserved even after tee'ing.
+ subprocess.check_call(['bash', '-c',
+ ('set -o pipefail; %s 2>&1 | tee %s')
% (command, build_output)])
# Success!
@@ -168,11 +207,14 @@ def verify_review(review_request):
"Patch looks great!\n\n" \
"Reviews applied: %s\n\n" \
"Passed command: %s" % (reviews, command))
- except subprocess.CalledProcessError as e:
+ except subprocess.CalledProcessError as err:
# If we are here because the docker build command failed, read the
# output from `build_output` file. For all other command failures read
# the output from `e.output`.
- output = open(build_output).read() if os.path.exists(build_output) else e.output
+ if os.path.exists(build_output):
+ output = open(build_output).read()
+ else:
+ output = err.output
if platform.system() == 'Windows':
# We didn't output anything during the build (because `tee`
@@ -180,28 +222,31 @@ def verify_review(review_request):
print output
# Truncate the output when posting the review as it can be very large.
- output = output if len(output) <= REVIEW_SIZE else "...<truncated>...\n" + output[-REVIEW_SIZE:]
- output = output + "\nFull log: " + urlparse.urljoin(os.environ['BUILD_URL'], 'console')
+ if len(output) > REVIEW_SIZE:
+ output = "...<truncated>...\n" + output[-REVIEW_SIZE:]
+
+ output += "\nFull log: "
+ output += urlparse.urljoin(os.environ['BUILD_URL'], 'console')
post_review(
review_request,
"Bad patch!\n\n" \
"Reviews applied: %s\n\n" \
"Failed command: %s\n\n" \
- "Error:\n%s" % (reviews, e.cmd, output))
- except ReviewError as e:
+ "Error:\n%s" % (reviews, err.cmd, output))
+ except ReviewError as err:
post_review(
review_request,
"Bad review!\n\n" \
"Reviews applied: %s\n\n" \
- "Error:\n%s" % (reviews, e.args[0]))
+ "Error:\n%s" % (reviews, err.args[0]))
# Clean up.
cleanup()
-# Returns true if this review request needs to be verified.
def needs_verification(review_request):
+ """Return True if this review request needs to be verified."""
print "Checking if review: %s needs verification" % review_request["id"]
# Skip if the review blocks another review.
@@ -212,15 +257,14 @@ def needs_verification(review_request):
diffs_url = review_request["links"]["diffs"]["href"]
diffs = api(diffs_url)
- if len(diffs["diffs"]) == 0: # No diffs attached!
+ if len(diffs["diffs"]) == 0: # No diffs attached!
print "Skipping review %s as it has no diffs" % review_request["id"]
return False
- RB_DATE_FORMAT = "%Y-%m-%dT%H:%M:%SZ"
-
# Get the timestamp of the latest diff.
timestamp = diffs["diffs"][-1]["timestamp"]
- diff_time = datetime.strptime(timestamp, RB_DATE_FORMAT)
+ rb_date_format = "%Y-%m-%dT%H:%M:%SZ"
+ diff_time = datetime.strptime(timestamp, rb_date_format)
print "Latest diff timestamp: %s" % diff_time
# Get the timestamp of the latest review from this script.
@@ -230,7 +274,7 @@ def needs_verification(review_request):
for review in reversed(reviews["reviews"]):
if review["links"]["user"]["title"] == USER:
timestamp = review["timestamp"]
- review_time = datetime.strptime(timestamp, RB_DATE_FORMAT)
+ review_time = datetime.strptime(timestamp, rb_date_format)
print "Latest review timestamp: %s" % review_time
break
@@ -241,7 +285,7 @@ def needs_verification(review_request):
for change in changes["changes"]:
if "depends_on" in change["fields_changed"]:
timestamp = change["timestamp"]
- dependency_time = datetime.strptime(timestamp, RB_DATE_FORMAT)
+ dependency_time = datetime.strptime(timestamp, rb_date_format)
print "Latest dependency change timestamp: %s" % dependency_time
break
@@ -251,13 +295,18 @@ def needs_verification(review_request):
(dependency_time and review_time < dependency_time)
-if __name__=="__main__":
- review_requests_url = "%s/api/review-requests/%s" % (REVIEWBOARD_URL, QUERY_PARAMS)
+def main():
+ """Main function to verify the submitted reviews."""
+ review_requests_url = \
+ "%s/api/review-requests/%s" % (REVIEWBOARD_URL, QUERY_PARAMS)
review_requests = api(review_requests_url)
num_reviews = 0
for review_request in reversed(review_requests["review_requests"]):
if (NUM_REVIEWS == -1 or num_reviews < NUM_REVIEWS) and \
- needs_verification(review_request):
+ needs_verification(review_request):
verify_review(review_request)
num_reviews += 1
+
+if __name__ == '__main__':
+ main()
[3/4] mesos git commit: Linted support/push-commits.py.
Posted by jo...@apache.org.
Linted support/push-commits.py.
This will allow us to use PyLint on the
entire support directory in the future.
Review: https://reviews.apache.org/r/60234/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/a536d101
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/a536d101
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/a536d101
Branch: refs/heads/master
Commit: a536d101a52e21b32cb584d4df9d468e8b76b6a2
Parents: c0e51a8
Author: Armand Grillet <ag...@mesosphere.io>
Authored: Mon Jul 17 11:41:55 2017 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Mon Jul 17 17:18:12 2017 -0700
----------------------------------------------------------------------
support/push-commits.py | 69 +++++++++++++++++++++++++++++---------------
1 file changed, 45 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/a536d101/support/push-commits.py
----------------------------------------------------------------------
diff --git a/support/push-commits.py b/support/push-commits.py
index 4835d67..210c88e 100755
--- a/support/push-commits.py
+++ b/support/push-commits.py
@@ -1,16 +1,34 @@
#!/usr/bin/env python
-
-# This script is typically used by Mesos committers to push a locally applied
-# review chain to ASF git repo and mark the reviews as submitted on ASF
-# ReviewBoard.
#
-# Example Usage:
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
#
-# > git checkout master
-# > git pull origin
-# > ./support/apply-reviews.py -c -r 1234
-# > ./support/push-commits.py
+# http://www.apache.org/licenses/LICENSE-2.0
#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+This script is typically used by Mesos committers to push a locally applied
+review chain to ASF git repo and mark the reviews as submitted on ASF
+ReviewBoard.
+
+Example Usage:
+
+> git checkout master
+> git pull origin
+> ./support/apply-reviews.py -c -r 1234
+> ./support/push-commits.py
+"""
+
# TODO(vinod): Also post the commit message to the corresponding ASF JIRA
# tickets and resolve them if necessary.
@@ -21,12 +39,11 @@ import sys
from subprocess import check_output
-REVIEWBOARD_URL = 'https://reviews.apache.org'
+REVIEWBOARD_URL = 'https://reviews.apache.org'
def get_reviews(revision_range):
- """Returns the list of reviews found in the commits in the revision range.
- """
+ """Return the list of reviews found in the commits in the revision range."""
log = check_output(['git',
'--no-pager',
'log',
@@ -52,17 +69,16 @@ def get_reviews(revision_range):
def close_reviews(reviews, options):
- """ Marks the given reviews as submitted on ReviewBoard."""
- # Close the reviews on ReviewBoard.
+ """Mark the given reviews as submitted on ReviewBoard."""
for review_id in reviews:
- print 'Closing review', review_id
- if not options['dry_run']:
- # TODO(vinod): Include the commit message as '--description'.
- check_output(['rbt', 'close', review_id])
+ print 'Closing review', review_id
+ if not options['dry_run']:
+ # TODO(vinod): Include the commit message as '--description'.
+ check_output(['rbt', 'close', review_id])
def parse_options():
- """Returns a dictionary of options parsed from command line arguments."""
+ """Return a dictionary of options parsed from command line arguments."""
parser = argparse.ArgumentParser()
parser.add_argument('-n',
@@ -78,7 +94,8 @@ def parse_options():
return options
-if __name__ == '__main__':
+def main():
+ """Main function to push the commits in this branch as review requests."""
options = parse_options()
current_branch_ref = check_output(['git', 'symbolic-ref', 'HEAD']).strip()
@@ -93,10 +110,11 @@ if __name__ == '__main__':
'--abbrev-ref',
'master@{upstream}']).strip()
- merge_base = check_output(['git',
- 'merge-base',
- remote_tracking_branch,
- 'master']).strip()
+ merge_base = check_output([
+ 'git',
+ 'merge-base',
+ remote_tracking_branch,
+ 'master']).strip()
if merge_base == current_branch_ref:
print 'No new commits found to push'
@@ -122,3 +140,6 @@ if __name__ == '__main__':
# Now mark the reviews as submitted.
close_reviews(reviews, options)
+
+if __name__ == '__main__':
+ main()
[4/4] mesos git commit: CMake: Defined GIT_SHA, GIT_BRANCH,
and GIT_TAG variables.
Posted by jo...@apache.org.
CMake: Defined GIT_SHA, GIT_BRANCH, and GIT_TAG variables.
This emits some extra build information into a generated header
file (much like BUILD_DATE, BUILD_TIME, and BUILD_USER).
This only has an effect when building from a git clone and results
in some extra fields showing up in the Master/Agent state results.
Review: https://reviews.apache.org/r/60482/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0df6892e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0df6892e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0df6892e
Branch: refs/heads/master
Commit: 0df6892e0792e94c6624b5a86f99260b2a59a43a
Parents: 3c26428
Author: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Authored: Mon Jul 17 10:07:35 2017 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Mon Jul 17 17:18:12 2017 -0700
----------------------------------------------------------------------
cmake/CompilationConfigure.cmake | 26 ++++++++++++++++++++++++++
src/common/build_config.hpp.in | 4 +++-
2 files changed, 29 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/0df6892e/cmake/CompilationConfigure.cmake
----------------------------------------------------------------------
diff --git a/cmake/CompilationConfigure.cmake b/cmake/CompilationConfigure.cmake
index 437300f..6fedcd2 100644
--- a/cmake/CompilationConfigure.cmake
+++ b/cmake/CompilationConfigure.cmake
@@ -344,7 +344,33 @@ else ()
set(BUILD_USER "$ENV{USER}")
endif ()
+# When building from source, from a git clone, emit some extra build info.
+if (IS_DIRECTORY "${CMAKE_SOURCE_DIR}/.git")
+ execute_process(
+ COMMAND git rev-parse HEAD
+ OUTPUT_VARIABLE BUILD_GIT_SHA
+ WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
+ ERROR_QUIET
+ OUTPUT_STRIP_TRAILING_WHITESPACE)
+
+ execute_process(
+ COMMAND git symbolic-ref HEAD
+ OUTPUT_VARIABLE BUILD_GIT_BRANCH
+ WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
+ ERROR_QUIET
+ OUTPUT_STRIP_TRAILING_WHITESPACE)
+
+ execute_process(
+ COMMAND git describe --exact --tags
+ OUTPUT_VARIABLE BUILD_GIT_TAG
+ WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
+ ERROR_QUIET
+ OUTPUT_STRIP_TRAILING_WHITESPACE)
+endif ()
+
# Emit the BUILD_DATE, BUILD_TIME, and BUILD_USER variables into a file.
+# When building from a git clone, the variables BUILD_GIT_SHA,
+# BUILD_GIT_BRANCH, and BUILD_GIT_TAG will also be emitted.
# This will be updated each time `cmake` is run.
configure_file(
"${CMAKE_SOURCE_DIR}/src/common/build_config.hpp.in"
http://git-wip-us.apache.org/repos/asf/mesos/blob/0df6892e/src/common/build_config.hpp.in
----------------------------------------------------------------------
diff --git a/src/common/build_config.hpp.in b/src/common/build_config.hpp.in
index 174d8aa..ac7059a 100644
--- a/src/common/build_config.hpp.in
+++ b/src/common/build_config.hpp.in
@@ -21,6 +21,8 @@
#cmakedefine BUILD_TIME "@BUILD_TIME@"
#cmakedefine BUILD_USER "@BUILD_USER@"
-// TODO(andschwa): Define GIT_SHA etc. for parity with autotools.
+#cmakedefine BUILD_GIT_SHA "@BUILD_GIT_SHA@"
+#cmakedefine BUILD_GIT_BRANCH "@BUILD_GIT_BRANCH@"
+#cmakedefine BUILD_GIT_TAG "@BUILD_GIT_TAG@"
#endif // __COMMON_BUILD_CONFIG_HPP__
[2/4] mesos git commit: Linted support/post-reviews.py.
Posted by jo...@apache.org.
Linted support/post-reviews.py.
This will allow us to use PyLint on the
entire support directory in the future.
Review: https://reviews.apache.org/r/60233/
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/c0e51a8d
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/c0e51a8d
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/c0e51a8d
Branch: refs/heads/master
Commit: c0e51a8d729aa6b06d70fa18259354536bc59b43
Parents: 0df6892
Author: Armand Grillet <ag...@mesosphere.io>
Authored: Mon Jul 17 11:16:23 2017 -0700
Committer: Joseph Wu <jo...@apache.org>
Committed: Mon Jul 17 17:18:12 2017 -0700
----------------------------------------------------------------------
support/post-reviews.py | 649 +++++++++++++++++++++++--------------------
1 file changed, 342 insertions(+), 307 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mesos/blob/c0e51a8d/support/post-reviews.py
----------------------------------------------------------------------
diff --git a/support/post-reviews.py b/support/post-reviews.py
index 410fb33..5ecde40 100755
--- a/support/post-reviews.py
+++ b/support/post-reviews.py
@@ -1,26 +1,41 @@
#!/usr/bin/env python
-# This is a wrapper around the 'post-review'/'rbt' tool provided by
-# Review Board. This is currently used by Apache Mesos development.
#
-# What does this do?
-# It provides the ability to send a review for each commit on the
-# current branch.
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
#
-# Why is that useful?
-# No one likes a 5000 line review request. Using this tool forces one
-# to create logical commits which can be reviewed independently.
+# http://www.apache.org/licenses/LICENSE-2.0
#
-# How do I use it?
-# First install 'RBTools' from Review Board.
-# http://www.reviewboard.org/downloads/rbtools/
-#
-# $ cd /path/to/mesos
-# $ [ do some work on your branch off of master, make commit(s) ]
-# $ ./support/post-reviews.py --server=https://reviews.apache.org \
-# --tracking-branch=origin/master --target-groups=mesos --open
-#
-# NOTE: post-reviews is currently specific to Mesos development,
-# but can easily be adapted for other projects.
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Wrapper around the post-review/rbt tool provided by Review Board.
+
+This script provides the ability to send one review for each
+commit on the current branch, instead of squashing all changes
+into a single large review. We encourage contributors to create
+logical commits which can be reviewed independently.
+
+Options to pass onto 'rbt' can be placed in a '.reviewboardrc'
+file at the top of the Mesos source directory. A default
+'.reviewboardrc' can be found at 'support/reviewboardrc'.
+Running './bootstrap' will populate this file for you.
+
+To use this script, first install 'RBTools' from Review Board:
+http://www.reviewboard.org/downloads/rbtools/
+
+$ cd /path/to/mesos
+$ [ do some work on your branch off of master, make commit(s) ]
+$ ./support/post-reviews.py
+"""
import argparse
import atexit
@@ -35,325 +50,345 @@ from distutils.version import LooseVersion
from subprocess import check_output, Popen, PIPE, STDOUT
+
def execute(command, ignore_errors=False):
+ """Execute a process and leave."""
process = None
try:
process = Popen(command,
- stdin=PIPE,
- stdout=PIPE,
- stderr=STDOUT,
- shell=False)
- except:
+ stdin=PIPE,
+ stdout=PIPE,
+ stderr=STDOUT,
+ shell=False)
+ except Exception:
if not ignore_errors:
raise
return None
- data, error = process.communicate()
+ data, _ = process.communicate()
status = process.wait()
if status != 0 and not ignore_errors:
cmdline = ' '.join(command) if isinstance(command, list) else command
- need_login = \
- 'Please log in to the Review Board server at reviews.apache.org.'
+ need_login = 'Please log in to the Review Board' \
+ ' server at reviews.apache.org.'
if need_login in data:
- print need_login, '\n'
- print "You can either:"
- print " (1) Run 'rbt login', or"
- print " (2) Set the default USERNAME/PASSWORD in '.reviewboardrc'"
+ print need_login, '\n'
+ print "You can either:"
+ print " (1) Run 'rbt login', or"
+ print " (2) Set the default USERNAME/PASSWORD in '.reviewboardrc'"
else:
- print 'Failed to execute: \'' + cmdline + '\':'
- print data
+ print 'Failed to execute: \'' + cmdline + '\':'
+ print data
sys.exit(1)
elif status != 0:
return None
return data
-# TODO(benh): Make sure this is a git repository, apologize if not.
-
-# Choose 'rbt' if available, otherwise choose 'post-review'.
-post_review = None
-
-rbt_command = 'rbt'
-# Windows command name must have `cmd` extension.
-if platform.system() == 'Windows':
- rbt_command = 'rbt.cmd'
-
-rbt_version = execute([rbt_command, '--version'], ignore_errors=True)
-if rbt_version:
- rbt_version = LooseVersion(rbt_version)
- post_review = [rbt_command, 'post']
-elif execute(['post-review', '--version'], ignore_errors=True):
- post_review = ['post-review']
-else:
- print 'Please install RBTools before proceeding'
- sys.exit(1)
-
-# Don't do anything if people have unstaged changes.
-diff_stat = execute(['git', 'diff', '--shortstat']).strip()
-
-if diff_stat:
- print 'Please commit or stash any changes before using post-reviews!'
- sys.exit(1)
-
-# Don't do anything if people have uncommitted changes.
-diff_stat = execute(['git', 'diff', '--shortstat', '--staged']).strip()
-
-if diff_stat:
- print 'Please commit staged changes before using post-reviews!'
- sys.exit(1)
-
-# Grab a reference to the repo's git directory. Usually this is simply .git in
-# the repo's top level directory. However, when submodules are used, it may
-# appear elsewhere. The most up-to-date way of finding this directory is to use
-# `git rev-parse --git-common-dir`. This is necessary to support things like
-# git worktree in addition to git submodules. However, as of January 2016,
-# support for the '--git-common-dir' flag is fairly new, forcing us to fall
-# back to the older '--git-dir' flag if '--git-common-dir' is not supported. We
-# do this by checking the output of `git rev-parse --git-common-dir` and seeing
-# if it gives us a valid directory back. If not, we set the git directory using
-# the '--git-dir' flag instead.
-git_dir = execute(['git', 'rev-parse', '--git-common-dir']).strip()
-if not os.path.isdir(git_dir):
- git_dir = execute(['git', 'rev-parse', '--git-dir']).strip()
-
-# Grab a reference to the top level directory of this repo.
-top_level_dir = execute(['git', 'rev-parse', '--show-toplevel']).strip()
-
-# Use the tracking_branch specified by the user if exists.
-parser = argparse.ArgumentParser(add_help=False)
-parser.add_argument('--server')
-parser.add_argument('--tracking-branch')
-args, _ = parser.parse_known_args()
-
-# Try to read the .reviewboardrc in the top-level directory.
-reviewboardrc_filepath = os.path.join(top_level_dir, '.reviewboardrc')
-if os.path.exists(reviewboardrc_filepath):
- sys.dont_write_bytecode = True # Prevent generation of '.reviewboardrcc'.
- reviewboardrc = imp.load_source('reviewboardrc', reviewboardrc_filepath)
-
-reviewboard_url = (
- args.server if args.server else
- reviewboardrc.REVIEWBOARD_URL if 'REVIEWBOARD_URL' in dir(reviewboardrc) else
- 'https://reviews.apache.org')
-
-tracking_branch = (
- args.tracking_branch if args.tracking_branch else
- reviewboardrc.TRACKING_BRANCH if 'TRACKING_BRANCH' in dir(reviewboardrc) else
- 'master')
-
-branch_ref = execute(['git', 'symbolic-ref', 'HEAD']).strip()
-branch = branch_ref.replace('refs/heads/', '', 1)
-
-# do not work on master branch
-if branch == "master":
- print "We're expecting you to be working on another branch from master!"
- sys.exit(1)
-
-temporary_branch = '_post-reviews_' + branch
-
-# Always delete the temporary branch.
-atexit.register(lambda: execute(['git', 'branch', '-D', temporary_branch], True))
-
-# Always put us back on the original branch.
-atexit.register(lambda: execute(['git', 'checkout', branch]))
-
-merge_base = execute(['git', 'merge-base', tracking_branch, branch_ref]).strip()
-
-
-
-output = check_output([
- 'git',
- '--no-pager',
- 'log',
- '--pretty=format:%Cred%H%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr)%Creset',
- merge_base + '..HEAD'])
-print 'Running \'%s\' across all of ...' % " ".join(post_review)
-print output
-
-log = execute(['git',
- '--no-pager',
- 'log',
- '--no-color',
- '--pretty=oneline',
- '--reverse',
- merge_base + '..HEAD']).strip()
-
-if len(log) <= 0:
- print "No new changes compared with master branch!"
- sys.exit(1)
-
-shas = []
-
-for line in log.split('\n'):
- sha = line.split()[0]
- shas.append(sha)
-
-
-previous = tracking_branch
-parent_review_request_id = None
-for i in range(len(shas)):
- sha = shas[i]
-
- execute(['git', 'branch', '-D', temporary_branch], True)
-
- message = execute(['git',
- '--no-pager',
- 'log',
- '--pretty=format:%s%n%n%b',
- previous + '..' + sha])
-
- review_request_id = None
-
- pos = message.find('Review:')
- if pos != -1:
- regex = 'Review: ({url})$'.format(
- url=urlparse.urljoin(reviewboard_url, 'r/[0-9]+'))
- pattern = re.compile(regex)
- match = pattern.search(message[pos:].strip().strip('/'))
- if match is None:
- print "\nInvalid ReviewBoard URL: '{}'".format(message[pos:])
- sys.exit(1)
+def main():
+ """Main function, post commits added to this branch as review requests."""
+ # TODO(benh): Make sure this is a git repository, apologize if not.
- url = match.group(1)
- review_request_id = url.split('/')[-1]
+ # Choose 'rbt' if available, otherwise choose 'post-review'.
+ post_review = None
- # Show the commit.
- if review_request_id is None:
- output = check_output([
- 'git',
- '--no-pager',
- 'log',
- '--pretty=format:%Cred%H%Creset -%C(yellow)%d%Creset %s',
- previous + '..' + sha])
- print '\nCreating diff of:'
- print output
+ rbt_command = 'rbt'
+ # Windows command name must have `cmd` extension.
+ if platform.system() == 'Windows':
+ rbt_command = 'rbt.cmd'
+
+ rbt_version = execute([rbt_command, '--version'], ignore_errors=True)
+ if rbt_version:
+ rbt_version = LooseVersion(rbt_version)
+ post_review = [rbt_command, 'post']
+ elif execute(['post-review', '--version'], ignore_errors=True):
+ post_review = ['post-review']
else:
- output = check_output([
- 'git',
- '--no-pager',
- 'log',
- '--pretty=format:%Cred%H%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr)%Creset',
- previous + '..' + sha])
- print '\nUpdating diff of:'
- print output
+ print 'Please install RBTools before proceeding'
+ sys.exit(1)
+
+ # Don't do anything if people have unstaged changes.
+ diff_stat = execute(['git', 'diff', '--shortstat']).strip()
+
+ if diff_stat:
+ print 'Please commit or stash any changes before using post-reviews!'
+ sys.exit(1)
+
+ # Don't do anything if people have uncommitted changes.
+ diff_stat = execute(['git', 'diff', '--shortstat', '--staged']).strip()
+
+ if diff_stat:
+ print 'Please commit staged changes before using post-reviews!'
+ sys.exit(1)
+
+ # Grab a reference to the repo's git directory. Usually this is simply
+ # .git in the repo's top level directory. However, when submodules are
+ # used, it may appear elsewhere. The most up-to-date way of finding this
+ # directory is to use `git rev-parse --git-common-dir`. This is necessary
+ # to support things like git worktree in addition to git submodules.
+ # However, as of January 2016, support for the '--git-common-dir' flag is
+ # fairly new, forcing us to fall back to the '--git-dir' flag if
+ # '--git-common-dir' is not supported. We do this by checking the output of
+ # git rev-parse --git-common-dir` and check if it gives a valid directory.
+ # If not, we set the git directory using the '--git-dir' flag instead.
+ git_dir = execute(['git', 'rev-parse', '--git-common-dir']).strip()
+ if not os.path.isdir(git_dir):
+ git_dir = execute(['git', 'rev-parse', '--git-dir']).strip()
+
+ # Grab a reference to the top level directory of this repo.
+ top_level_dir = execute(['git', 'rev-parse', '--show-toplevel']).strip()
+
+ # Use the tracking_branch specified by the user if exists.
+ parser = argparse.ArgumentParser(add_help=False)
+ parser.add_argument('--server')
+ parser.add_argument('--tracking-branch')
+ args, _ = parser.parse_known_args()
+
+ # Try to read the .reviewboardrc in the top-level directory.
+ reviewboardrc_filepath = os.path.join(top_level_dir, '.reviewboardrc')
+ if os.path.exists(reviewboardrc_filepath):
+ # Prevent generation of '.reviewboardrcc'.
+ sys.dont_write_bytecode = True
+ reviewboardrc = imp.load_source('reviewboardrc', reviewboardrc_filepath)
+
+ if args.server:
+ reviewboard_url = args.server
+ elif 'REVIEWBOARD_URL' in dir(reviewboardrc):
+ reviewboard_url = reviewboardrc.REVIEWBOARD_URL
+ else:
+ reviewboard_url = 'https://reviews.apache.org'
+
+ if args.tracking_branch:
+ tracking_branch = args.tracking_branch
+ elif 'TRACKING_BRANCH' in dir(reviewboardrc):
+ tracking_branch = reviewboardrc.TRACKING_BRANCH
+ else:
+ tracking_branch = 'master'
+
+ branch_ref = execute(['git', 'symbolic-ref', 'HEAD']).strip()
+ branch = branch_ref.replace('refs/heads/', '', 1)
+
+ # do not work on master branch
+ if branch == "master":
+ print "We're expecting you to be working on another branch from master!"
+ sys.exit(1)
+
+ temporary_branch = '_post-reviews_' + branch
+
+ # Always delete the temporary branch.
+ atexit.register(
+ lambda: execute(['git', 'branch', '-D', temporary_branch], True))
+
+ # Always put us back on the original branch.
+ atexit.register(lambda: execute(['git', 'checkout', branch]))
+
+ merge_base = execute(
+ ['git', 'merge-base', tracking_branch, branch_ref]).strip()
- # Show the "parent" commit(s).
output = check_output([
'git',
'--no-pager',
'log',
- '--pretty=format:%Cred%H%Creset -%C(yellow)%d%Creset %s %Cgreen(%cr)%Creset',
- tracking_branch + '..' + previous])
+ '--pretty=format:%Cred%H%Creset -%C'
+ '(yellow)%d%Creset %s %Cgreen(%cr)%Creset',
+ merge_base + '..HEAD'])
+ print 'Running \'%s\' across all of ...' % " ".join(post_review)
+ print output
- if output:
- print '\n... with parent diff created from:'
- print output
+ log = execute(['git',
+ '--no-pager',
+ 'log',
+ '--no-color',
+ '--pretty=oneline',
+ '--reverse',
+ merge_base + '..HEAD']).strip()
- try:
- raw_input('\nPress enter to continue or \'Ctrl-C\' to skip.\n')
- except KeyboardInterrupt:
- i = i + 1
- previous = sha
- parent_review_request_id = review_request_id
- continue
-
- # Strip the review url from the commit message, so that it is not included
- # in the summary message when GUESS_FIELDS is set in .reviewboardc. Update
- # the sha appropriately.
- if review_request_id:
- stripped_message = message[:pos]
- execute(['git', 'checkout', sha])
- execute(['git', 'commit', '--amend', '-m', stripped_message])
- sha = execute(['git', 'rev-parse', 'HEAD']).strip()
- execute(['git', 'checkout', branch])
-
- revision_range = previous + ':' + sha
-
- # Build the post-review/rbt command up to the point where they are common.
- command = post_review
-
- if args.tracking_branch is None:
- command = command + ['--tracking-branch=' + tracking_branch]
-
- if review_request_id:
- command = command + ['--review-request-id=' + review_request_id]
-
- # Determine how to specify the revision range.
- if rbt_command in post_review and rbt_version >= LooseVersion('RBTools 0.6'):
- # rbt >= 0.6.1 supports '--depends-on' argument.
- # Only set the "depends on" if this is not the first review in the chain.
- if rbt_version >= LooseVersion('RBTools 0.6.1') and parent_review_request_id:
- command = command + ['--depends-on=' + parent_review_request_id]
-
- # rbt >= 0.6 revisions are passed in as args.
- command = command + sys.argv[1:] + [previous, sha]
- else:
- # post-review and rbt < 0.6 revisions are passed in using the revision
- # range option.
- command = command + \
- ['--revision-range=' + revision_range] + \
- sys.argv[1:]
+ if len(log) <= 0:
+ print "No new changes compared with master branch!"
+ sys.exit(1)
+
+ shas = []
+
+ for line in log.split('\n'):
+ sha = line.split()[0]
+ shas.append(sha)
+
+ previous = tracking_branch
+ parent_review_request_id = None
+ for i, sha in enumerate(shas):
+ execute(['git', 'branch', '-D', temporary_branch], True)
+
+ message = execute(['git',
+ '--no-pager',
+ 'log',
+ '--pretty=format:%s%n%n%b',
+ previous + '..' + sha])
+
+ review_request_id = None
+
+ pos = message.find('Review:')
+ if pos != -1:
+ regex = 'Review: ({url})$'.format(
+ url=urlparse.urljoin(reviewboard_url, 'r/[0-9]+'))
+ pattern = re.compile(regex)
+ match = pattern.search(message[pos:].strip().strip('/'))
+ if match is None:
+ print "\nInvalid ReviewBoard URL: '{}'".format(message[pos:])
+ sys.exit(1)
+
+ url = match.group(1)
+ review_request_id = url.split('/')[-1]
+
+ # Show the commit.
+ if review_request_id is None:
+ output = check_output([
+ 'git',
+ '--no-pager',
+ 'log',
+ '--pretty=format:%Cred%H%Creset -%C(yellow)%d%Creset %s',
+ previous + '..' + sha])
+ print '\nCreating diff of:'
+ print output
+ else:
+ output = check_output([
+ 'git',
+ '--no-pager',
+ 'log',
+ '--pretty=format:%Cred%H%Creset -%C'
+ '(yellow)%d%Creset %s %Cgreen(%cr)%Creset',
+ previous + '..' + sha])
+ print '\nUpdating diff of:'
+ print output
+
+ # Show the "parent" commit(s).
+ output = check_output([
+ 'git',
+ '--no-pager',
+ 'log',
+ '--pretty=format:%Cred%H%Creset -%C'
+ '(yellow)%d%Creset %s %Cgreen(%cr)%Creset',
+ tracking_branch + '..' + previous])
+
+ if output:
+ print '\n... with parent diff created from:'
+ print output
+
+ try:
+ raw_input('\nPress enter to continue or \'Ctrl-C\' to skip.\n')
+ except KeyboardInterrupt:
+ i = i + 1
+ previous = sha
+ parent_review_request_id = review_request_id
+ continue
+
+ # Strip the review url from the commit message, so that
+ # it is not included in the summary message when GUESS_FIELDS
+ # is set in .reviewboardc. Update the SHA appropriately.
+ if review_request_id:
+ stripped_message = message[:pos]
+ execute(['git', 'checkout', sha])
+ execute(['git', 'commit', '--amend', '-m', stripped_message])
+ sha = execute(['git', 'rev-parse', 'HEAD']).strip()
+ execute(['git', 'checkout', branch])
+
+ revision_range = previous + ':' + sha
+
+ # Build the post-review/rbt command up
+ # to the point where they are common.
+ command = post_review
+
+ if args.tracking_branch is None:
+ command = command + ['--tracking-branch=' + tracking_branch]
+
+ if review_request_id:
+ command = command + ['--review-request-id=' + review_request_id]
+
+ # Determine how to specify the revision range.
+ if rbt_command in post_review and \
+ rbt_version >= LooseVersion('RBTools 0.6'):
+ # rbt >= 0.6.1 supports '--depends-on' argument.
+ # Only set the "depends on" if this
+ # is not the first review in the chain.
+ if rbt_version >= LooseVersion('RBTools 0.6.1') and \
+ parent_review_request_id:
+ command = command + ['--depends-on=' + parent_review_request_id]
+
+ # rbt >= 0.6 revisions are passed in as args.
+ command = command + sys.argv[1:] + [previous, sha]
+ else:
+ # post-review and rbt < 0.6 revisions are
+ # passed in using the revision range option.
+ command = command + \
+ ['--revision-range=' + revision_range] + \
+ sys.argv[1:]
- output = execute(command).strip()
+ output = execute(command).strip()
- print output
+ print output
+ # If we already have a request_id, continue on to the next commit in the
+ # chain. We update 'previous' from the shas[] array because we have
+ # overwritten the temporary sha variable above.
+ if review_request_id is not None:
+ previous = shas[i]
+ parent_review_request_id = review_request_id
+ i = i + 1
+ continue
+
+ # Otherwise, get the request_id from the output of post-review, append
+ # it to the commit message and rebase all other commits on top of it.
+ lines = output.split('\n')
+
+ # The last line of output in post-review is the review url.
+ # The second to the last line of output in rbt is the review url.
+ url = lines[len(lines) - 2] if rbt_command in post_review \
+ else lines[len(lines) - 1]
+
+ # Using rbt >= 0.6.3 on Linux prints out two URLs where the second
+ # one has /diff/ at the end. We want to remove this so that a
+ # subsequent call to post-reviews does not fail when looking up
+ # the reviewboard entry to edit.
+ url = url.replace('diff/', '')
+ url = url.strip('/')
+ review_request_id = os.path.basename(url)
+
+ # Construct new commit message.
+ message = message + '\n' + 'Review: ' + url + '\n'
+
+ execute(['git', 'checkout', '-b', temporary_branch])
+ execute(['git', 'reset', '--hard', sha])
+ execute(['git', 'commit', '--amend', '-m', message])
+
+ # Now rebase all remaining shas on top of this amended commit.
+ j = i + 1
+ old_sha = execute(
+ ['git', 'rev-parse', '--verify', temporary_branch]).strip()
+ previous = old_sha
+ while j < len(shas):
+ execute(['git', 'checkout', shas[j]])
+ execute(['git', 'rebase', temporary_branch])
+ # Get the sha for our detached HEAD.
+ new_sha = execute([
+ 'git',
+ '--no-pager',
+ 'log',
+ '--pretty=format:%H', '-n', '1', 'HEAD']).strip()
+ execute(['git',
+ 'update-ref',
+ 'refs/heads/' + temporary_branch,
+ new_sha,
+ old_sha])
+ old_sha = new_sha
+ shas[j] = new_sha
+ j = j + 1
+
+ # Okay, now update the actual branch to our temporary branch.
+ new_sha = old_sha
+ old_sha = execute(['git', 'rev-parse', '--verify', branch]).strip()
+ execute(['git', 'update-ref', 'refs/heads/' + branch, new_sha, old_sha])
- # If we already have a request_id, continue on to the next commit in the
- # chain. We update 'previous' from the shas[] array because we have
- # overwritten the temporary sha variable above.
- if review_request_id is not None:
- previous = shas[i]
- parent_review_request_id = review_request_id
i = i + 1
- continue
-
- # Otherwise, get the request_id from the output of post-review, append it
- # to the commit message and rebase all other commits on top of it.
- lines = output.split('\n')
-
- # The last line of output in post-review is the review url.
- # The second to the last line of output in rbt is the review url.
- url = lines[len(lines) - 2] if rbt_command in post_review \
- else lines[len(lines) - 1]
-
- # Using rbt >= 0.6.3 on Linux prints out two URLs where the second
- # one has /diff/ at the end. We want to remove this so that a
- # subsequent call to post-reviews does not fail when looking up
- # the reviewboard entry to edit.
- url = url.replace('diff/','')
- url = url.strip('/')
- review_request_id = os.path.basename(url)
-
- # Construct new commit message.
- message = message + '\n' + 'Review: ' + url + '\n'
-
- execute(['git', 'checkout', '-b', temporary_branch])
- execute(['git', 'reset', '--hard', sha])
- execute(['git', 'commit', '--amend', '-m', message])
-
- # Now rebase all remaining shas on top of this amended commit.
- j = i + 1
- old_sha = execute(['git', 'rev-parse', '--verify', temporary_branch]).strip()
- previous = old_sha
- while j < len(shas):
- execute(['git', 'checkout', shas[j]])
- execute(['git', 'rebase', temporary_branch])
- # Get the sha for our detached HEAD.
- new_sha = execute(['git', '--no-pager', 'log', '--pretty=format:%H', '-n', '1', 'HEAD']).strip()
- execute(['git',
- 'update-ref',
- 'refs/heads/' + temporary_branch,
- new_sha,
- old_sha])
- old_sha = new_sha
- shas[j] = new_sha
- j = j + 1
-
- # Okay, now update the actual branch to our temporary branch.
- new_sha = old_sha
- old_sha = execute(['git', 'rev-parse', '--verify', branch]).strip()
- execute(['git', 'update-ref', 'refs/heads/' + branch, new_sha, old_sha])
-
- i = i + 1
- parent_review_request_id = review_request_id
+ parent_review_request_id = review_request_id
+
+if __name__ == '__main__':
+ main()