Be more selective about what packages we remove so that we can detect conflicts.

cros_mark_as_stable.py

 #!/usr/bin/python
 
 # 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.
 
 """This module uprevs a given package's ebuild to the next revision."""
 
 
 import fileinput
@@ skipping 301 matching lines @@
     _SimpleRunCommand(delete_cmd)
 
 
 class EBuild(object):
   """Wrapper class for information about an ebuild."""
 
   def __init__(self, path):
     """Sets up data about an ebuild from its path."""
     from portage.versions import pkgsplit
     unused_path, self.category, self.pkgname, filename = path.rsplit('/', 3)
-    unused_pkgname, version_no_rev, rev = pkgsplit(
+    unused_pkgname, self.version_no_rev, rev = pkgsplit(
         filename.replace('.ebuild', ''))
 
     self.ebuild_path_no_version = os.path.join(
         os.path.dirname(path), self.pkgname)
-    self.ebuild_path_no_revision = '%s-%s' % (self.ebuild_path_no_version,

[sosa, 2010/12/09 19:09:04]

No.  This is used by cros_mark_chrome_as_stable.

This change also isn't mentioned in your issue description

-                                              version_no_rev)
     self.current_revision = int(rev.replace('r', ''))
-    self.version = '%s-%s' % (version_no_rev, rev)
+    self.version = '%s-%s' % (self.version_no_rev, rev)
     self.package = '%s/%s' % (self.category, self.pkgname)
     self.ebuild_path = path
 
     self.is_workon = False
     self.is_stable = False
 
     for line in fileinput.input(path):
       if line.startswith('inherit ') and 'cros-workon' in line:
         self.is_workon = True
       elif (line.startswith('KEYWORDS=') and '~' not in line and
@@ skipping 101 matching lines @@
         commit_id: String corresponding to the commit hash of the developer
           package to rev.
         redirect_file: Optional file to write the new ebuild.  By default
           it is written using the standard rev'ing logic.  This file must be
           opened and closed by the caller.
 
     Raises:
         OSError: Error occurred while creating a new ebuild.
         IOError: Error occurred while writing to the new revved ebuild file.
     Returns:
-      True if the revved package is different than the old ebuild.
+      If the revved package is different than the old ebuild, return the full
+      revved package name, including the version number. Otherwise, return None.
     """
     if self._ebuild.is_stable:
-      new_stable_ebuild_path = '%s-r%d.ebuild' % (
-          self._ebuild.ebuild_path_no_revision,
-          self._ebuild.current_revision + 1)
+      stable_version_no_rev = self._ebuild.version_no_rev
     else:
       # If given unstable ebuild, use 0.0.1 rather than 9999.
-      new_stable_ebuild_path = '%s-0.0.1-r%d.ebuild' % (
-          self._ebuild.ebuild_path_no_version,
-          self._ebuild.current_revision + 1)
+      stable_version_no_rev = '0.0.1'

[sosa, 2010/12/09 19:09:04]

extra line

[davidjames, 2010/12/21 19:48:29]

Looks like an artifact of rietveld -- there's no extra line here.

+    new_version = '%s-r%d' % (stable_version_no_rev,
+                              self._ebuild.current_revision + 1)
+    new_stable_ebuild_path = '%s-%s.ebuild' % (
+        self._ebuild.ebuild_path_no_version, new_version)
 
     _Print('Creating new stable ebuild %s' % new_stable_ebuild_path)
     unstable_ebuild_path = ('%s-9999.ebuild' %
                             self._ebuild.ebuild_path_no_version)
     if not os.path.exists(unstable_ebuild_path):
       Die('Missing unstable ebuild: %s' % unstable_ebuild_path)
 
     self.MarkAsStable(unstable_ebuild_path, new_stable_ebuild_path,
                       'CROS_WORKON_COMMIT', commit_id, redirect_file)
 
     old_ebuild_path = self._ebuild.ebuild_path
     diff_cmd = ['diff', '-Bu', old_ebuild_path, new_stable_ebuild_path]
     if 0 == RunCommand(diff_cmd, exit_code=True, redirect_stdout=True,
                        redirect_stderr=True, print_cmd=gflags.FLAGS.verbose):
       os.unlink(new_stable_ebuild_path)

[diandersAtChromium, 2010/12/10 23:58:54]

Leave in explicit return of None?

[davidjames, 2010/12/21 19:48:29]

Done.

-      return False
     else:
       _Print('Adding new stable ebuild to git')
       _SimpleRunCommand('git add %s' % new_stable_ebuild_path)
 
       if self._ebuild.is_stable:
         _Print('Removing old ebuild from git')
         _SimpleRunCommand('git rm %s' % old_ebuild_path)
 
-      return True
+      return '%s-%s' % (self._ebuild.package, new_version)
 
   @classmethod
   def CommitChange(cls, message):
     """Commits current changes in git locally with given commit message.
 
     Args:
         message: the commit string to write when committing to git.
 
     Raises:
         OSError: Error occurred while committing.
@@ skipping 51 matching lines @@
       if not work_branch.Exists():
         Die('Unable to create stabilizing branch in %s' % overlay)
 
       # Contains the array of packages we actually revved.
       revved_packages = []
       for ebuild in ebuilds:
         try:
           _Print('Working on %s' % ebuild.package)
           worker = EBuildStableMarker(ebuild)
           commit_id = ebuild.GetCommitId()
-          if worker.RevWorkOnEBuild(commit_id):
+          new_package = worker.RevWorkOnEBuild(commit_id)
+          if new_package:
             message = _GIT_COMMIT_MESSAGE % (ebuild.package, commit_id)
             worker.CommitChange(message)
-            revved_packages.append(ebuild.package)
+            revved_packages.append('=%s' % new_package)
 
         except (OSError, IOError):
           Warning('Cannot rev %s\n' % ebuild.package,
                   'Note you will have to go into %s '
                   'and reset the git repo yourself.' % overlay)
           raise
 
       if revved_packages:
         _CleanStalePackages(gflags.FLAGS.board, revved_packages)
         if gflags.FLAGS.drop_file:
           fh = open(gflags.FLAGS.drop_file, 'w')
           fh.write(' '.join(revved_packages))

[diandersAtChromium, 2010/12/10 23:58:54]

Won't this file now contain "=" before each package?  Is that a problem?  ...it
sure seems like it.  It looks like cbuildbot uses this, which then gets used in
cros_run_unit_tests.

[davidjames, 2010/12/21 19:48:29]

Great catch. Fixed -- I changed my patch so that revved_packages was left alone.

           fh.close()
       else:
         work_branch.Delete()
 
 
 if __name__ == '__main__':
   main(sys.argv)