[NaCl SDK] The auto-updater should only mark one version as stable, everything (issue 11275081)

[bi...@chromium.org]

Reviewers: Sam Clegg,

Description:
[NaCl SDK] The auto-updater should only mark one version as stable,
everything
prior should be marked as post_stable.

BUG=156765
R=s...@chromium.org
NOTRY=true


Please review this at http://codereview.chromium.org/11275081/

SVN Base: svn://svn.chromium.org/chrome/trunk/src

Affected files:
M native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
M native_client_sdk/src/build_tools/update_nacl_manifest.py


Index: native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
diff --git
a/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
b/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
index
11b565dff33b7616455abc878c8262c39f6cda91..b5e931fc43fc3a075d4ea7f36717f3c7c601f956
100755
--- a/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
+++ b/native_client_sdk/src/build_tools/tests/test_update_nacl_manifest.py
@@ -31,6 +31,7 @@ OS_M = ('mac',)
OS_ML = ('mac', 'linux')
OS_MW = ('mac', 'win')
OS_MLW = ('mac', 'linux', 'win')
+POST_STABLE = 'post_stable'
STABLE = 'stable'
BETA = 'beta'
DEV = 'dev'
@@ -515,6 +516,20 @@ mac,canary,21.0.1156.0,2012-05-30 12:14:21.305090"""
self._AssertUploadedManifestHasBundle(bundle, BETA)
self.assertEqual(len(self.uploaded_manifest.GetBundles()), 1)

+ def testOnlyOneStableBundle(self):
+ self.manifest = MakeManifest(B18_R1_NONE, B19_R1_NONE)
+ self.history.Add(OS_MLW, STABLE, V18_0_1025_163)
+ self.history.Add(OS_MLW, STABLE, V19_0_1084_41)
+ self.files.Add(B18_0_1025_163_R1_MLW)
+ self.files.Add(B19_0_1084_41_R1_MLW)
+ self._MakeDelegate()
+ self._Run(OS_MLW)
+ self._ReadUploadedManifest()
+ p18_bundle = self.uploaded_manifest.GetBundle(B18_R1_NONE.name)
+ self.assertEqual(p18_bundle.stability, POST_STABLE)
+ p19_bundle = self.uploaded_manifest.GetBundle(B19_R1_NONE.name)
+ self.assertEqual(p19_bundle.stability, STABLE)
+

class TestUpdateVitals(unittest.TestCase):
def setUp(self):
@@ -538,7 +553,6 @@ class TestUpdateVitals(unittest.TestCase):
# (file:///C:\whatever)
path = '/' + path
archive.url = 'file://' + path
- print archive.url

bundle = MakeBundle(18)
bundle.AddArchive(archive)
Index: native_client_sdk/src/build_tools/update_nacl_manifest.py
diff --git a/native_client_sdk/src/build_tools/update_nacl_manifest.py
b/native_client_sdk/src/build_tools/update_nacl_manifest.py
index
4b05a000a90f60dcfe3c78dee5b242a128a28f19..c8fb442bf43d8278267f7f289b92919e6353ffda
100755
--- a/native_client_sdk/src/build_tools/update_nacl_manifest.py
+++ b/native_client_sdk/src/build_tools/update_nacl_manifest.py
@@ -479,6 +479,13 @@ class Updater(object):
Args:
manifest: The manifest used as a template for updating. Only pepper
bundles that contain no archives will be considered for
auto-updating."""
+ # Make sure there is only one stable branch: the one with the max
version.
+ # All others are post-stable.
+ stable_major_versions = [SplitVersion(version)[0] for _, version,
channel, _
+ in self.versions_to_update if channel
== 'stable']
+ # Add 0 in case there are no stable versions.
+ max_stable_version = max([0] + stable_major_versions)
+
for bundle_name, version, channel, archives in self.versions_to_update:
self.delegate.Print('Updating %s to %s...' % (bundle_name, version))
bundle = manifest.GetBundle(bundle_name)
@@ -489,7 +496,12 @@ class Updater(object):
# snippet.
platform_bundle.name = bundle_name
bundle.MergeWithBundle(platform_bundle)
- bundle.stability = channel
+
+ major_version = SplitVersion(version)[0]
+ if major_version < max_stable_version and channel == 'stable':
+ bundle.stability = 'post_stable'
+ else:
+ bundle.stability = channel
# We always recommend the stable version.
if channel == 'stable':
bundle.recommended = 'yes'

[s...@chromium.org]

On 2012/10/31 18:28:05, binji wrote:

lgtm.

I mildly perfer "legacy" or "old_stable" (this is what Debian call it)
to "post_stable". post_stable somehow tells me that its better than
stable.

http://codereview.chromium.org/11275081/

[bi...@chromium.org]

I agree, though we'll have to bump manifest versions if we want to make that
change -- the name "post_stable" is currently used.

http://codereview.chromium.org/11275081/

[noel...@chromium.org]

I'd keep post stable for historical reasons.
LGTM

http://codereview.chromium.org/11275081/

[commi...@chromium.org]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bi...@chromium.org/11275081/1


https://chromiumcodereview.appspot.com/11275081/

[commi...@chromium.org]

Change committed as 165856

https://chromiumcodereview.appspot.com/11275081/