Robustify and speed up cros_mark_all_as_stable.

What's new?
 - cros_mark_all_as_stable is now in Python and has unit tests.
 - We now detect and report coding errors that can cause incorrect behavior.
   E.g., if 9999 ebuild or stable ebuild can't be found, we report a hard
   failure so that developers will know about the problem.
 - We now check that git hashes we report actually match up with the
   right repository.
 - Unified diff of changes are now printed to stdout, so that developers
   can see a list of the changes and debug any problems.
 - cros_mark_all_as_stable now takes 2.5 seconds to run.

BUG=chromium-os:7795
TEST=Manually examined diff output from run of cros_mark_all_as_stable
     on full repository.

Change-Id: I762597c9b94e5f8e8171b83c966ad54e21a65c1b

Review URL: http://codereview.chromium.org/3798003

bin/cbuildbot.py

@@ -184,19 +184,15 @@ def _UprevFromRevisionList(buildroot, revision_list):
     return
 
   package_str = ''
-  commit_str = ''
   for package, revision in revision_list:
     package_str += package + ' '
-    commit_str += revision + ' '
 
   package_str = package_str.strip()
-  commit_str = commit_str.strip()
 
   cwd = os.path.join(buildroot, 'src', 'scripts')
   RunCommand(['./cros_mark_as_stable',
               '--tracking_branch="cros/master"',
               '--packages="%s"' % package_str,
-              '--commit_ids="%s"' % commit_str,
               'commit'],
               cwd=cwd, enter_chroot=True)
 
@@ -204,8 +200,8 @@ def _UprevFromRevisionList(buildroot, revision_list):
 def _UprevAllPackages(buildroot):
   """Uprevs all packages that have been updated since last uprev."""
   cwd = os.path.join(buildroot, 'src', 'scripts')
-  RunCommand(['./cros_mark_all_as_stable',
+  RunCommand(['./cros_mark_as_stable', '--all',
-              '--tracking_branch="cros/master"'],
+              '--tracking_branch="cros/master"', 'commit'],
               cwd=cwd, enter_chroot=True)
 
 

bin/cbuildbot_unittest.py

@@ -113,8 +113,8 @@ class CBuildBotTest(mox.MoxTestBase):
     m_file.read().AndReturn(self._test_string)
     m_file.close()
 
-    cbuildbot.RunCommand(['./cros_mark_all_as_stable',
+    cbuildbot.RunCommand(['./cros_mark_as_stable', '--all',
-                     '--tracking_branch="cros/master"'],
+                     '--tracking_branch="cros/master"', 'commit'],
                      cwd='%s/src/scripts' % self._buildroot,
                      enter_chroot=True)
 
@@ -133,8 +133,8 @@ class CBuildBotTest(mox.MoxTestBase):
     m_file.read().AndReturn('None')
     m_file.close()
 
-    cbuildbot.RunCommand(['./cros_mark_all_as_stable',
+    cbuildbot.RunCommand(['./cros_mark_as_stable', '--all',
-                         '--tracking_branch="cros/master"'],
+                         '--tracking_branch="cros/master"', 'commit'],
                          cwd='%s/src/scripts' % self._buildroot,
                          enter_chroot=True)
 

cros_mark_all_as_stable

@@ -1,104 +0,0 @@
-#!/bin/bash
-
-# Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style license that can be
-# found in the LICENSE file.
-
-# Wrapper scripts around cros_mark_as_stable that marks all packages as stable
-# that have CROS_WORKON_COMMIT that is different than the current HEAD commit
-# of the corresponding git repository.
-
-# Load common constants.  This should be the first executable line.
-# The path to common.sh should be relative to your script's location.
-. "$(dirname "$0")/common.sh"
-
-# Load common functions for workon scripts.
-. "$(dirname "$0")/lib/cros_workon_common.sh"
-
-get_default_board
-
-DEFINE_string board "${DEFAULT_BOARD}" \
-  "The board to set package keywords for."
-DEFINE_string tracking_branch "origin" \
-  "Used with commit to specify branch to track against."
-
-FLAGS "$@" || exit 1
-eval set -- "${FLAGS_ARGV}"
-
-set -e
-
-BOARD_DIR=/build/"${FLAGS_board}"
-EQUERYCMD=equery-"${FLAGS_board}"
-EBUILDCMD=ebuild-"${FLAGS_board}"
-
-PACKAGES=$( show_workon_ebuilds )
-
-GRAB_HEAD_COMMIT_CMD="git show HEAD | head -1 | cut -f 2 -d ' '"
-
-# Packages to mark as stable.
-PACKAGE_LIST=""
-# List of commit ids corresponding to package list.
-COMMIT_ID_LIST=""
-# List of IFS-delimited ebuilds to ignore.
-PACKAGE_BLACKLIST=""
-# File containing the names of blacklisted packages.
-BLACKLIST_FILE=$(dirname "${0}")/cros_mark_as_stable_blacklist
-
-[ -f "${BLACKLIST_FILE}" ] && \
-  PACKAGE_BLACKLIST=$(cat "${BLACKLIST_FILE}")
-
-function package_is_blacklisted() {
-  # Makes a list that looks like "\|package1\|package2\|...packagen".
-  local blist_regex=$(for i in ${PACKAGE_BLACKLIST}; do echo -n "\\|${i}"; done)
-  expr "${1}" : "^\(${blist_regex/\\|/}\)$" &> /dev/null && return 0 || return 1
-}
-
-# For each package, compares the head commit id to the commit id in the ebuild.
-# If they do not match, add the package and its commit id into ${PACKAGE_LIST}
-# and ${COMMIT_ID_LIST}
-for package in ${PACKAGES}; do
-  if package_is_blacklisted ${package}; then
-    info "${package} blacklisted, skipping"
-    continue
-  fi
-  # We need to pick up any stable ebuilds for any platform.
-  ebuild_path=$(ACCEPT_KEYWORDS="arm x86 amd64" ${EQUERYCMD} which ${package})\
-      || continue
-  # Get 9999 ebuild path to see if it got changed.
-  ebuild_9999_path=$(ACCEPT_KEYWORDS="~*" ${EQUERYCMD} which ${package}) \
-      || continue
-  # Sets ${CROS_WORKON_SRCDIR} from the ebuild.
-  eval $(${EBUILDCMD} ${ebuild_path} info) &> /dev/null || continue
-  head_commit=$( cd "${CROS_WORKON_SRCDIR}" &&\
-    bash -c "${GRAB_HEAD_COMMIT_CMD}" ) || continue
-  egit_commit=$(\
-    eval echo $(grep CROS_WORKON_COMMIT ${ebuild_path} | cut -f 2 -d '=')) ||\
-      echo "No CROS_WORKON_COMMIT found in ${ebuild_path}"
-  if [[ ${head_commit} != ${egit_commit} ]] && \
-      [ -n "${head_commit}" ]; then
-    info\
-      "HEAD ${head_commit} != CROS_WORKON_COMMIT ${egit_commit} for ${package}"
-    PACKAGE_LIST="${PACKAGE_LIST} ${package}"
-    COMMIT_ID_LIST="${COMMIT_ID_LIST} ${head_commit}"
-  elif [[ ${head_commit}  = ${egit_commit} ]]; then
-    info "Commit id's match for ${package}, checking for 9999 ebuild change."
-    # egrep succeeds if there are important differences between the ebuilds.
-    if diff "${ebuild_path}" "${ebuild_9999_path}" | \
-        egrep -v "KEYWORDS|CROS_WORKON_COMMIT|^---|^[<>]\ *$|^[0-9]"; then
-      info "Detected 9999 ebuild change for ${package}."
-      PACKAGE_LIST="${PACKAGE_LIST} ${package}"
-      COMMIT_ID_LIST="${COMMIT_ID_LIST} ${egit_commit}"
-    fi
-  fi
-done
-
-if [ -n "${PACKAGE_LIST}" ] ; then
-  info "Candidate package list ${PACKAGE_LIST}"
-  info "With commit id list ${COMMIT_ID_LIST}"
-
-  ./cros_mark_as_stable --board ${FLAGS_board} -p "${PACKAGE_LIST}" \
-    -i "${COMMIT_ID_LIST}" -t ${FLAGS_tracking_branch} commit  || \
-    die "Could not mark all packages as stable"
-else
-  info "No candidate packages to be marked"
-fi

cros_mark_as_stable.py

@@ -14,9 +14,10 @@ import re
 import shutil
 import subprocess
 import sys
+from portage.versions import pkgsplit, pkgsplit, vercmp
 
 sys.path.append(os.path.join(os.path.dirname(__file__), 'lib'))
-from cros_build_lib import Info, Warning, Die
+from cros_build_lib import Info, RunCommand, Warning, Die
 
 
 gflags.DEFINE_string('board', 'x86-generic',
@@ -38,6 +39,8 @@ gflags.DEFINE_string('srcroot', '%s/trunk/src' % os.environ['HOME'],
 gflags.DEFINE_string('tracking_branch', 'cros/master',
                      'Used with commit to specify branch to track against.',
                      short_name='t')
+gflags.DEFINE_boolean('all', False,
+                      'Mark all packages as stable.')
 gflags.DEFINE_boolean('verbose', False,
                       'Prints out verbose information about what is going on.',
                       short_name='v')
@@ -69,22 +72,81 @@ def _Print(message):
     Info(message)
 
 
-def _BuildEBuildDictionary(overlays, package_list, commit_id_list):
+def _BestEBuild(ebuilds):
-  for index in range(len(package_list)):
+  """Returns the newest EBuild from a list of EBuild objects."""
-    package = package_list[index]
+  winner = ebuilds[0]
-    commit_id = ''
+  for ebuild in ebuilds[1:]:
-    if commit_id_list:
+    if vercmp(winner.version, ebuild.version) < 0:
-      commit_id = commit_id_list[index]
+      winner = ebuild
-    ebuild = _EBuild(package, commit_id)
+  return winner
-    if ebuild.ebuild_path:
+
-      for overlay in overlays:
+
-        if ebuild.ebuild_path.startswith(overlay):
+def _FindStableEBuilds(files):
-          overlays[overlay].append(ebuild)
+  """Return a list of stable ebuilds from specified list of files.
-          break
+
-      else:
+  Args:
-        Die('No overlay found for %s' % ebuild.ebuild_path)
+    files: List of files.
-    else:
+  """
-      Die('No ebuild found for %s' % package)
+  workon_dir = False
+  stable_ebuilds = []
+  unstable_ebuilds = []
+  for path in files:
+    if path.endswith('.ebuild') and not os.path.islink(path):
+      ebuild = _EBuild(path)
+      if ebuild.is_workon:
+        workon_dir = True
+        if ebuild.is_stable:
+          stable_ebuilds.append(ebuild)
+        else:
+          unstable_ebuilds.append(ebuild)
+
+  # If we found a workon ebuild in this directory, apply some sanity checks.
+  if workon_dir:
+    if len(unstable_ebuilds) > 1:
+      Die('Found multiple unstable ebuilds in %s' % root)
+    if len(stable_ebuilds) > 1:
+      stable_ebuilds = [_BestEBuild(stable_ebuilds)]
+
+      # Print a warning if multiple stable ebuilds are found in the same
+      # directory. Storing multiple stable ebuilds is error-prone because
+      # the older ebuilds will not get rev'd.
+      #
+      # We make a special exception for x11-drivers/xf86-video-msm for legacy
+      # reasons.
+      if stable_ebuilds[0].package != 'x11-drivers/xf86-video-msm':
+        Warning('Found multiple stable ebuilds in %s' % root)
+
+    if not unstable_ebuilds:
+      Die('Missing 9999 ebuild in %s' % root)
+    if not stable_ebuilds:
+      Die('Missing stable ebuild in %s' % root)
+
+  if stable_ebuilds:
+    return stable_ebuilds[0]
+  else:
+    return None
+
+
+def _BuildEBuildDictionary(overlays, all, packages):
+  """Build a dictionary of the ebuilds in the specified overlays.
+
+  overlays: A map which maps overlay directories to arrays of stable EBuilds
+    inside said directories.
+  all: Whether to include all ebuilds in the specified directories. If true,
+    then we gather all packages in the directories regardless of whether
+    they are in our set of packages.
+  packages: A set of the packages we want to gather.
+  """
+  for overlay in overlays:
+    for root_dir, dirs, files in os.walk(overlay):
+      # Add stable ebuilds to overlays[overlay].
+      paths = [os.path.join(root_dir, path) for path in files]
+      ebuild = _FindStableEBuilds(paths)
+
+      # If the --all option isn't used, we only want to update packages that
+      # are in packages.
+      if ebuild and (all or ebuild.package in packages):
+        overlays[overlay].append(ebuild)
 
 
 def _CheckOnStabilizingBranch():
@@ -93,19 +155,16 @@ def _CheckOnStabilizingBranch():
   return current_branch == _STABLE_BRANCH_NAME
 
 
-def _CheckSaneArguments(package_list, commit_id_list, command):
+def _CheckSaneArguments(package_list, command):
   """Checks to make sure the flags are sane.  Dies if arguments are not sane."""
   if not command in _COMMAND_DICTIONARY.keys():
     _PrintUsageAndDie('%s is not a valid command' % command)
-  if not gflags.FLAGS.packages and command == 'commit':
+  if not gflags.FLAGS.packages and command == 'commit' and not gflags.FLAGS.all:
     _PrintUsageAndDie('Please specify at least one package')
   if not gflags.FLAGS.board and command == 'commit':
     _PrintUsageAndDie('Please specify a board')
   if not os.path.isdir(gflags.FLAGS.srcroot):
     _PrintUsageAndDie('srcroot is not a valid path')
-  if commit_id_list and (len(package_list) != len(commit_id_list)):
-    _PrintUsageAndDie(
-        'Package list is not the same length as the commit id list')
 
 
 def _Clean():
@@ -214,30 +273,76 @@ class _GitBranch(object):
 class _EBuild(object):
   """Wrapper class for an ebuild."""
 
-  def __init__(self, package, commit_id=None):
+  def __init__(self, path):
     """Initializes all data about an ebuild.
 
     Uses equery to find the ebuild path and sets data about an ebuild for
     easy reference.
     """
-    self.package = package
+    self.ebuild_path = path
-    self.ebuild_path = self._FindEBuildPath(package)
     (self.ebuild_path_no_revision,
      self.ebuild_path_no_version,
      self.current_revision) = self._ParseEBuildPath(self.ebuild_path)
-    self.commit_id = commit_id
+    _, self.category, pkgpath, filename = path.rsplit('/', 3)
+    filename_no_suffix = os.path.join(filename.replace('.ebuild', ''))
+    self.pkgname, version_no_rev, rev = pkgsplit(filename_no_suffix)
+    self.version = '%s-%s' % (version_no_rev, rev)
+    self.package = '%s/%s' % (self.category, self.pkgname)
+    self.is_workon = False
+    self.is_stable = False
 
-  @classmethod
+    for line in fileinput.input(path):
-  def _FindEBuildPath(cls, package):
+      if line.startswith('inherit ') and 'cros-workon' in line:
-    """Static method that returns the full path of an ebuild."""
+        self.is_workon = True
-    _Print('Looking for unstable ebuild for %s' % package)
+      elif (line.startswith('KEYWORDS=') and '~' not in line and
-    equery_cmd = (
+            ('amd64' in line or 'x86' in line or 'arm' in line)):
-        'ACCEPT_KEYWORDS="x86 arm amd64" equery-%s which %s 2> /dev/null'
+        self.is_stable = True
-            % (gflags.FLAGS.board, package))
+    fileinput.close()
-    path = _SimpleRunCommand(equery_cmd)
+
-    if path:
+  def GetCommitId(self):
-      _Print('Unstable ebuild found at %s' % path)
+    """Get the commit id for this ebuild."""
-    return path.rstrip()
+
+    # Grab and evaluate CROS_WORKON variables from this ebuild.
+    unstable_ebuild = '%s-9999.ebuild' % self.ebuild_path_no_version
+    cmd = ('CROS_WORKON_LOCALNAME="%s" CROS_WORKON_PROJECT="%s" '
+           'eval $(grep -E "^CROS_WORKON" %s) && '
+           'echo $CROS_WORKON_PROJECT '
+           '$CROS_WORKON_LOCALNAME/$CROS_WORKON_SUBDIR'
+           % (self.pkgname, self.pkgname, unstable_ebuild))
+    project, subdir = _SimpleRunCommand(cmd).split()
+
+    # Calculate srcdir.
+    srcroot = gflags.FLAGS.srcroot
+    if self.category == 'chromeos-base':
+      dir = 'platform'
+    else:
+      dir = 'third_party'
+    srcdir = os.path.join(srcroot, dir, subdir)
+
+    # TODO(anush): This hack is only necessary because the kernel ebuild has
+    # 'if' statements, so we can't grab the CROS_WORKON_LOCALNAME properly.
+    # We should clean up the kernel ebuild and remove this hack.
+    if not os.path.exists(srcdir) and subdir == 'kernel/':
+      srcdir = os.path.join(srcroot, 'third_party/kernel/files')
+
+    if not os.path.exists(srcdir):
+      Die('Cannot find commit id for %s' % self.ebuild_path)
+
+    # Verify that we're grabbing the commit id from the right project name.
+    # NOTE: chromeos-kernel has the wrong project name, so it fails this
+    # check.
+    # TODO(davidjames): Fix the project name in the chromeos-kernel ebuild.
+    cmd = 'cd %s && git config --get remote.cros.projectname' % srcdir
+    actual_project =_SimpleRunCommand(cmd).rstrip()
+    if project not in (actual_project, 'chromeos-kernel'):
+      Die('Project name mismatch for %s (%s != %s)' % (unstable_ebuild, project,
+          actual_project))
+
+    # Get commit id.
+    output = _SimpleRunCommand('cd %s && git rev-parse HEAD' % srcdir)
+    if not output:
+      Die('Missing commit id for %s' % self.ebuild_path)
+    return output.rstrip()
 
   @classmethod
   def _ParseEBuildPath(cls, ebuild_path):
@@ -317,11 +422,21 @@ class EBuildStableMarker(object):
         redirect_file.write(line)
     fileinput.close()
 
-    _Print('Adding new stable ebuild to git')
+    # If the new ebuild is identical to the old ebuild, return False and
-    _SimpleRunCommand('git add %s' % new_ebuild_path)
+    # delete our changes.
+    old_ebuild_path = self._ebuild.ebuild_path
+    diff_cmd = ['diff', '-Bu', old_ebuild_path, new_ebuild_path]
+    if 0 == RunCommand(diff_cmd, exit_code=True,
+                       print_cmd=gflags.FLAGS.verbose):
+      os.unlink(new_ebuild_path)
+      return False
+    else:
+      _Print('Adding new stable ebuild to git')
+      _SimpleRunCommand('git add %s' % new_ebuild_path)
 
-    _Print('Removing old ebuild from git')
+      _Print('Removing old ebuild from git')
-    _SimpleRunCommand('git rm %s' % self._ebuild.ebuild_path)
+      _SimpleRunCommand('git rm %s' % old_ebuild_path)
+      return True
 
   def CommitChange(self, message):
     """Commits current changes in git locally.
@@ -352,17 +467,16 @@ def main(argv):
     _PrintUsageAndDie(str(e))
 
   package_list = gflags.FLAGS.packages.split()
-  if gflags.FLAGS.commit_ids:
+  _CheckSaneArguments(package_list, command)
-    commit_id_list = gflags.FLAGS.commit_ids.split()
-  else:
-    commit_id_list = None
-  _CheckSaneArguments(package_list, commit_id_list, command)
 
   overlays = {
     '%s/private-overlays/chromeos-overlay' % gflags.FLAGS.srcroot: [],
     '%s/third_party/chromiumos-overlay' % gflags.FLAGS.srcroot: []
   }
-  _BuildEBuildDictionary(overlays, package_list, commit_id_list)
+  all = gflags.FLAGS.all
+
+  if command == 'commit':
+    _BuildEBuildDictionary(overlays, all, package_list)
 
   for overlay, ebuilds in overlays.items():
     if not os.path.exists(overlay):
@@ -374,17 +488,19 @@ def main(argv):
     elif command == 'push':
       _PushChange()
     elif command == 'commit' and ebuilds:
-      work_branch = _GitBranch(_STABLE_BRANCH_NAME)
-      work_branch.CreateBranch()
-      if not work_branch.Exists():
-        Die('Unable to create stabilizing branch in %s' % overlay)
       for ebuild in ebuilds:
         try:
           _Print('Working on %s' % ebuild.package)
           worker = EBuildStableMarker(ebuild)
-          worker.RevEBuild(ebuild.commit_id)
+          commit_id = ebuild.GetCommitId()
-          message = _GIT_COMMIT_MESSAGE % (ebuild.package, ebuild.commit_id)
+          if worker.RevEBuild(commit_id):
-          worker.CommitChange(message)
+            if not _CheckOnStabilizingBranch():
+              work_branch = _GitBranch(_STABLE_BRANCH_NAME)
+              work_branch.CreateBranch()
+              if not work_branch.Exists():
+                Die('Unable to create stabilizing branch in %s' % overlay)
+            message = _GIT_COMMIT_MESSAGE % (ebuild.package, commit_id)
+            worker.CommitChange(message)
         except (OSError, IOError):
           Warning('Cannot rev %s\n' % ebuild.package,
                   'Note you will have to go into %s '

cros_mark_as_stable_unittest.py

@@ -89,46 +89,35 @@ class EBuildTest(mox.MoxTestBase):
 
   def setUp(self):
     mox.MoxTestBase.setUp(self)
-    self.package = 'test_package'
-    self.ebuild_path = '/path/test_package-0.0.1-r1.ebuild'
-    self.ebuild_path_no_rev = '/path/test_package-0.0.1.ebuild'
 
   def testInit(self):
-    self.mox.StubOutWithMock(cros_mark_as_stable._EBuild, '_FindEBuildPath')
     self.mox.StubOutWithMock(cros_mark_as_stable._EBuild, '_ParseEBuildPath')
 
-    cros_mark_as_stable._EBuild._FindEBuildPath(
+    ebuild_path = '/overlay/cat/test_package/test_package-0.0.1-r1.ebuild'
-        self.package).AndReturn(self.ebuild_path)
     cros_mark_as_stable._EBuild._ParseEBuildPath(
-        self.ebuild_path).AndReturn(['/path/test_package-0.0.1',
+        ebuild_path).AndReturn(['/overlay/cat/test_package-0.0.1',
-                                     '/path/test_package',
+                                '/overlay/cat/test_package',
-                                     1])
+                                1])
-    self.mox.ReplayAll()
+    self.mox.StubOutWithMock(cros_mark_as_stable.fileinput, 'input')
-    ebuild = cros_mark_as_stable._EBuild(self.package, 'my_id')
+    mock_file = ['EAPI=2', 'CROS_WORKON_COMMIT=old_id',
-    self.mox.VerifyAll()
+                 'KEYWORDS=\"~x86 ~arm\"', 'src_unpack(){}']
-    self.assertEquals(ebuild.package, self.package)
+    cros_mark_as_stable.fileinput.input(ebuild_path).AndReturn(mock_file)
-    self.assertEquals(ebuild.ebuild_path, self.ebuild_path)
-    self.assertEquals(ebuild.ebuild_path_no_revision,
-                      '/path/test_package-0.0.1')
-    self.assertEquals(ebuild.ebuild_path_no_version, '/path/test_package')
-    self.assertEquals(ebuild.current_revision, 1)
-    self.assertEquals(ebuild.commit_id, 'my_id')
 
-  def testFindEBuildPath(self):
-    self.mox.StubOutWithMock(cros_mark_as_stable, '_SimpleRunCommand')
-    cmd = ('ACCEPT_KEYWORDS="x86 arm amd64" '
-           'equery-x86-generic which %s 2> /dev/null')
-    cros_mark_as_stable._SimpleRunCommand(cmd % self.package).AndReturn(
-        self.ebuild_path)
     self.mox.ReplayAll()
-    path = cros_mark_as_stable._EBuild._FindEBuildPath(self.package)
+    ebuild = cros_mark_as_stable._EBuild(ebuild_path)
     self.mox.VerifyAll()
-    self.assertEquals(path, self.ebuild_path)
+    self.assertEquals(ebuild.package, 'cat/test_package')
+    self.assertEquals(ebuild.ebuild_path, ebuild_path)
+    self.assertEquals(ebuild.ebuild_path_no_revision,
+                      '/overlay/cat/test_package-0.0.1')
+    self.assertEquals(ebuild.ebuild_path_no_version,
+                      '/overlay/cat/test_package')
+    self.assertEquals(ebuild.current_revision, 1)
 
   def testParseEBuildPath(self):
     # Test with ebuild with revision number.
     no_rev, no_version, revision = cros_mark_as_stable._EBuild._ParseEBuildPath(
-        self.ebuild_path)
+        '/path/test_package-0.0.1-r1.ebuild')
     self.assertEquals(no_rev, '/path/test_package-0.0.1')
     self.assertEquals(no_version, '/path/test_package')
     self.assertEquals(revision, 1)
@@ -136,7 +125,7 @@ class EBuildTest(mox.MoxTestBase):
   def testParseEBuildPathNoRevisionNumber(self):
     # Test with ebuild without revision number.
     no_rev, no_version, revision = cros_mark_as_stable._EBuild._ParseEBuildPath(
-        self.ebuild_path_no_rev)
+        '/path/test_package-0.0.1.ebuild')
     self.assertEquals(no_rev, '/path/test_package-0.0.1')
     self.assertEquals(no_version, '/path/test_package')
     self.assertEquals(revision, 0)
@@ -147,6 +136,8 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
   def setUp(self):
     mox.MoxTestBase.setUp(self)
     self.mox.StubOutWithMock(cros_mark_as_stable, '_SimpleRunCommand')
+    self.mox.StubOutWithMock(cros_mark_as_stable, 'RunCommand')
+    self.mox.StubOutWithMock(os, 'unlink')
     self.m_ebuild = self.mox.CreateMock(cros_mark_as_stable._EBuild)
     self.m_ebuild.package = 'test_package'
     self.m_ebuild.current_revision = 1
@@ -175,6 +166,10 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
     m_file.write('CROS_WORKON_COMMIT="my_id"\n')
     m_file.write('KEYWORDS="x86 arm"')
     m_file.write('src_unpack(){}')
+    diff_cmd = ['diff', '-Bu', self.m_ebuild.ebuild_path,
+                self.revved_ebuild_path]
+    cros_mark_as_stable.RunCommand(diff_cmd, exit_code=True,
+                                   print_cmd=False).AndReturn(1)
     cros_mark_as_stable._SimpleRunCommand('git add ' + self.revved_ebuild_path)
     cros_mark_as_stable._SimpleRunCommand('git rm ' + self.m_ebuild.ebuild_path)
 
@@ -183,6 +178,37 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
     marker.RevEBuild('my_id', redirect_file=m_file)
     self.mox.VerifyAll()
 
+  def testRevUnchangedEBuild(self):
+    self.mox.StubOutWithMock(cros_mark_as_stable.fileinput, 'input')
+    self.mox.StubOutWithMock(cros_mark_as_stable.os.path, 'exists')
+    self.mox.StubOutWithMock(cros_mark_as_stable.shutil, 'copyfile')
+    m_file = self.mox.CreateMock(file)
+
+    # Prepare mock fileinput.  This tests to make sure both the commit id
+    # and keywords are changed correctly.
+    mock_file = ['EAPI=2', 'CROS_WORKON_COMMIT=old_id',
+                 'KEYWORDS=\"~x86 ~arm\"', 'src_unpack(){}']
+
+    ebuild_9999 = self.m_ebuild.ebuild_path_no_version + '-9999.ebuild'
+    cros_mark_as_stable.os.path.exists(ebuild_9999).AndReturn(True)
+    cros_mark_as_stable.shutil.copyfile(ebuild_9999, self.revved_ebuild_path)
+    cros_mark_as_stable.fileinput.input(self.revved_ebuild_path,
+                                        inplace=1).AndReturn(mock_file)
+    m_file.write('EAPI=2')
+    m_file.write('CROS_WORKON_COMMIT="my_id"\n')
+    m_file.write('KEYWORDS="x86 arm"')
+    m_file.write('src_unpack(){}')
+    diff_cmd = ['diff', '-Bu', self.m_ebuild.ebuild_path,
+                self.revved_ebuild_path]
+    cros_mark_as_stable.RunCommand(diff_cmd, exit_code=True,
+                                   print_cmd=False).AndReturn(0)
+    cros_mark_as_stable.os.unlink(self.revved_ebuild_path)
+
+    self.mox.ReplayAll()
+    marker = cros_mark_as_stable.EBuildStableMarker(self.m_ebuild)
+    marker.RevEBuild('my_id', redirect_file=m_file)
+    self.mox.VerifyAll()
+
   def testRevMissingEBuild(self):
     self.mox.StubOutWithMock(cros_mark_as_stable.fileinput, 'input')
     self.mox.StubOutWithMock(cros_mark_as_stable.os.path, 'exists')
@@ -205,6 +231,10 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
     m_file.write('CROS_WORKON_COMMIT="my_id"\n')
     m_file.write('KEYWORDS="x86 arm"')
     m_file.write('src_unpack(){}')
+    diff_cmd = ['diff', '-Bu', self.m_ebuild.ebuild_path,
+                self.revved_ebuild_path]
+    cros_mark_as_stable.RunCommand(diff_cmd, exit_code=True,
+                                   print_cmd=False).AndReturn(1)
     cros_mark_as_stable._SimpleRunCommand('git add ' + self.revved_ebuild_path)
     cros_mark_as_stable._SimpleRunCommand('git rm ' + self.m_ebuild.ebuild_path)
 
@@ -231,49 +261,44 @@ class EBuildStableMarkerTest(mox.MoxTestBase):
     #self.mox.VerifyAll()
     pass
 
+
+class _Package(object):
+  def __init__(self, package):
+    self.package = package
+
+
 class BuildEBuildDictionaryTest(mox.MoxTestBase):
 
   def setUp(self):
     mox.MoxTestBase.setUp(self)
-    self.mox.StubOutWithMock(cros_mark_as_stable, '_SimpleRunCommand')
+    self.mox.StubOutWithMock(cros_mark_as_stable.os, 'walk')
-    self.ebuild_path = '/path/test_package-0.0.1-r1.ebuild'
+    self.mox.StubOutWithMock(cros_mark_as_stable, 'RunCommand')
-    self.package = "test_package"
+    self.package = 'chromeos-base/test_package'
+    self.root = '/overlay/chromeos-base/test_package'
+    self.package_path = self.root + '/test_package-0.0.1.ebuild'
+    paths = [[self.root, [], []]]
+    cros_mark_as_stable.os.walk("/overlay").AndReturn(paths)
+    self.mox.StubOutWithMock(cros_mark_as_stable, '_FindStableEBuilds')
 
-  def testValidPackage(self):
+
-    overlays = {"/path": []}
+  def testWantedPackage(self):
-    cmd = ('ACCEPT_KEYWORDS="x86 arm amd64" '
+    overlays = {"/overlay": []}
-           'equery-x86-generic which %s 2> /dev/null' % self.package)
+    package = _Package(self.package)
-    cros_mark_as_stable._SimpleRunCommand(cmd).AndReturn(self.ebuild_path)
+    cros_mark_as_stable._FindStableEBuilds([]).AndReturn(package)
     self.mox.ReplayAll()
-    cros_mark_as_stable._BuildEBuildDictionary(overlays, [self.package], [])
+    cros_mark_as_stable._BuildEBuildDictionary(overlays, False, [self.package])
-    self.assertEquals(len(overlays), 1)
-    self.assertEquals(overlays["/path"][0].package, self.package)
     self.mox.VerifyAll()
-
-  def testPackageInDifferentOverlay(self):
-    self.mox.StubOutWithMock(cros_mark_as_stable, 'Die')
-    cros_mark_as_stable.Die("No overlay found for %s" % self.ebuild_path)
-    cmd = ('ACCEPT_KEYWORDS="x86 arm amd64" '
-           'equery-x86-generic which %s 2> /dev/null' % self.package)
-    cros_mark_as_stable._SimpleRunCommand(cmd).AndReturn(self.ebuild_path)
-    overlays = {"/newpath": []}
-    self.mox.ReplayAll()
-    cros_mark_as_stable._BuildEBuildDictionary(overlays, [self.package], [])
     self.assertEquals(len(overlays), 1)
-    self.assertEquals(overlays["/newpath"], [])
+    self.assertEquals(overlays["/overlay"], [package])
-    self.mox.VerifyAll()
 
-  def testMissingPackage(self):
+  def testUnwantedPackage(self):
-    self.mox.StubOutWithMock(cros_mark_as_stable, 'Die')
+    overlays = {"/overlay": []}
-    cros_mark_as_stable.Die("No ebuild found for %s" % self.package)
+    package = _Package(self.package)
-    cmd = ('ACCEPT_KEYWORDS="x86 arm amd64" '
+    cros_mark_as_stable._FindStableEBuilds([]).AndReturn(package)
-           'equery-x86-generic which %s 2> /dev/null' % self.package)
-    cros_mark_as_stable._SimpleRunCommand(cmd).AndReturn("")
     self.mox.ReplayAll()
-    overlays = {"/path": []}
+    cros_mark_as_stable._BuildEBuildDictionary(overlays, False, [])
-    cros_mark_as_stable._BuildEBuildDictionary(overlays, [self.package], [])
     self.assertEquals(len(overlays), 1)
-    self.assertEquals(overlays["/path"], [])
+    self.assertEquals(overlays["/overlay"], [])
     self.mox.VerifyAll()