compare built tarball hash instead of buildid

buildids just change too damn much.  oh no, you changed a comment in
random which is transitively used by templating which processes .html, I
guess you need to bump all the versions of all packages with .html?
that sucks.  how about only changing it if the EFFECT of the build is
changed?

this does have some subtleties around platform-specific versions but we
think we have a good compromise

tools/catalog.js

@@ -841,6 +841,20 @@ _.extend(Catalog.prototype, {
       return buildsToUse;
     // We couldn't satisfy it!
     return null;
+  },
+
+  // Unlike the previous, this looks for a build which *precisely* matches the
+  // given architectures string (joined with +).
+  getBuildWithArchesString: function (name, version, archesString) {
+    var self = this;
+    self._requireInitialized();
+
+    var versionInfo = self.getVersion(name, version);
+    if (! versionInfo)
+      return null;
+
+    return _.where(self.builds, { versionId: versionInfo._id,
+                                  architecture: archesString } ) || null;
   }
 });
 

tools/commands.js

@@ -1757,7 +1757,7 @@ main.registerCommand({
     packageClient.handlePackageServerConnectionError(err);
     return 1;
   }
-  packageClient.createAndPublishBuiltPackage(conn, unipackage, packageDir);
+  packageClient.createAndPublishBuiltPackage(conn, unipackage);
 
   return 0;
 });
@@ -1893,7 +1893,8 @@ main.registerCommand({
                 // it doesn't we should fail. Hopefully, of course, we have
                 // tested our stuff before deciding to publish it to the package
                 // server, but we need to be careful.
-                var compileResult = compiler.compile(packageSource, { officialBuild: true });
+                var compileResult = compiler.compile(packageSource,
+                                                     { officialBuild: true });
                 if (buildmessage.jobHasMessages()) {
                   process.stderr.write("Error compiling unipackage:" + item + "\n");
                   return;
@@ -1909,31 +1910,45 @@ main.registerCommand({
 
                 // If there is no old version, then we need to publish this package.
                 if (!oldVersion) {
-                  toPublish[item] = {source: packageSource, unipackage: compileResult};
+                  toPublish[item] = {source: packageSource,
+                                     compileResult: compileResult};
                   return;
                 } else {
-                  // Now we need to check if the compiler inputs hash matches up. If
+                  var existingBuild = catalog.getBuildWithArchesString(
-                  // it doesn't, we need to update the version *and* republish. This
+                    item, oldVersion,
-                  // will constitute an error -- log a message, and mark the error
+                    compileResult.unipackage.architecturesString());
-                  // so we exit before trying to publish the release.
+                  // If the version number mentioned in package.js exists, but
-                  var myCompilerInputsHash = compileResult.unipackage.getBuildIdentifier({
+                  // there's no build of this architecture, then either the old
-                    relativeTo: packageSource.sourceRoot
+                  // version was only semi-published, or you've added some
-                  });
+                  // platform-specific dependencies but haven't bumped the
-                  if (myCompilerInputsHash === oldVersion.compilerInputsHash) {
+                  // version number yet; either way, you should probably bump
-                    // Cool, everything is exactly the same. We are done here.
+                  // the version number.
-                    return;
+                  var somethingChanged = !existingBuild;
+
+                  if (!somethingChanged) {
+                    // Bundle the build, just to get its hash.
+                    // XXX this is redundant with the bundle build step that
+                    // publishPackage will do later
+                    var bundleBuildResult = packageClient.bundleBuild(
+                      compileResult.unipackage);
+                    if (bundleBuildResult.tarballHash !==
+                        existingBuild.build.hash) {
+                      somethingChanged = true;
+                    }
                   }
 
-                  // The build ID of the old server record is not the same as
+                  if (somethingChanged) {
-                  // the buildID that we have on disk. This means something has
+                    // The build ID of the old server record is not the same as
-                  // changed -- maybe our source files, or a buildId of one of
+                    // the buildID that we have on disk. This means something
-                  // our build-time dependencies. There might be a false
+                    // has changed -- maybe our source files, or a buildId of
-                  // positive here (for example, we added some comments to a
+                    // one of our build-time dependencies. There might be a
-                  // package.js file somewhere), but, for now, we would rather
+                    // false positive here (for example, we added some comments
-                  // err on the side of catching this issue and forcing a more
+                    // to a package.js file somewhere), but, for now, we would
-                  // thorough check.
+                    // rather err on the side of catching this issue and forcing
-                  buildmessage.error("Something changed in package " + item + "." +
+                    // a more thorough check.
-                                     " Please upgrade version number. \n");
+                    buildmessage.error("Something changed in package " + item
+                                       + ". Please upgrade version number. \n");
+                  }
                 }
               });
           }
@@ -1960,7 +1975,7 @@ main.registerCommand({
      // checks around this.
      var pub = packageClient.publishPackage(
        prebuilt.source,
-       prebuilt.unipackage,
+       prebuilt.compileResult,
        conn,
        opts);
 

tools/package-client.js

@@ -386,13 +386,13 @@ var uploadTarball = function (putUrl, tarball) {
 
 exports.uploadTarball = uploadTarball;
 
-var bundleBuild = function (unipackage, packageDir) {
+var bundleBuild = function (unipackage) {
   var tempDir = files.mkdtemp('build-package-');
   var packageTarName = unipackage.name + '-' + unipackage.version + '-' +
-        unipackage.architectures().join('+');
+        unipackage.architecturesString();
   var tarInputDir = path.join(tempDir, packageTarName);
 
-  files.cp_r(path.join(packageDir, '.build.' + unipackage.name), tarInputDir);
+  unipackage.saveToPath(tarInputDir);
 
   // Don't upload buildinfo.json. It's only of interest locally (for
   // example, it contains a watchset with local paths).
@@ -413,7 +413,7 @@ var bundleBuild = function (unipackage, packageDir) {
 
 exports.bundleBuild = bundleBuild;
 
-var createAndPublishBuiltPackage = function (conn, unipackage, packageDir) {
+var createAndPublishBuiltPackage = function (conn, unipackage) {
   process.stdout.write('Creating package build...\n');
   var uploadInfo = conn.call('createPackageBuild', {
     packageName: unipackage.name,
@@ -421,7 +421,7 @@ var createAndPublishBuiltPackage = function (conn, unipackage, packageDir) {
     architecture: unipackage.architectures().join('+')
   });
 
-  var bundleResult = bundleBuild(unipackage, packageDir);
+  var bundleResult = bundleBuild(unipackage);
 
   process.stdout.write('Uploading build...\n');
   uploadTarball(uploadInfo.uploadUrl,
@@ -538,10 +538,6 @@ exports.publishPackage = function (packageSource, compileResult, conn, options)
                                                 sources,
                                                 packageSource.sourceRoot);
 
-  var compilerInputsHash = compileResult.unipackage.getBuildIdentifier({
-    relativeTo: packageSource.sourceRoot
-  });
-
   // Create the package. Check that the metadata exists.
   if (options.new) {
     process.stdout.write('Creating package...\n');
@@ -557,8 +553,7 @@ exports.publishPackage = function (packageSource, compileResult, conn, options)
     description: packageSource.metadata.summary,
     earliestCompatibleVersion: packageSource.earliestCompatibleVersion,
     containsPlugins: packageSource.containsPlugins(),
-    dependencies: packageSource.getDependencyMetadata(),
+    dependencies: packageSource.getDependencyMetadata()
-    compilerInputsHash: compilerInputsHash
   };
   var uploadInfo = conn.call('createPackageVersion', uploadRec);
 
@@ -574,8 +569,6 @@ exports.publishPackage = function (packageSource, compileResult, conn, options)
   conn.call('publishPackageVersion',
             uploadInfo.uploadToken, bundleResult.tarballHash);
 
-  createAndPublishBuiltPackage(conn, compileResult.unipackage,
+  createAndPublishBuiltPackage(conn, compileResult.unipackage);
-                               packageSource.sourceRoot);
   return 0;
-
 };

tools/unipackage.js

@@ -302,6 +302,11 @@ _.extend(Unipackage.prototype, {
     return _.uniq(_.pluck(self.builds, 'arch').concat(self._toolArchitectures())).sort();
   },
 
+  architecturesString: function () {
+    var self = this;
+    return self.architectures().join('+');
+  },
+
   _toolArchitectures: function () {
     var self = this;
     var toolArches = _.pluck(self.toolsOnDisk, 'arch');