more cache-busting, keep install directory clean

direct/src/p3d/HostInfo.py

@@ -1,6 +1,7 @@
-from pandac.PandaModules import TiXmlDocument, HashVal, Filename, PandaSystem, URLSpec, Ramfile
+from pandac.PandaModules import TiXmlDocument, HashVal, Filename, PandaSystem, DocumentSpec, Ramfile
 from direct.p3d.PackageInfo import PackageInfo
 from direct.p3d.FileSpec import FileSpec
+import time
 
 class HostInfo:
     """ This class represents a particular download host serving up
@@ -40,11 +41,22 @@ class HostInfo:
             # We've already got one.
             return True
 
-        url = URLSpec(self.hostUrlPrefix + 'contents.xml')
+        url = self.hostUrlPrefix + 'contents.xml'
-        print "Downloading %s" % (url)
+        # Append a uniquifying query string to the URL to force the
+        # download to go all the way through any caches.  We use the
+        # time in seconds; that's unique enough.
+        url += '?' + str(int(time.time()))
+
+        # We might as well explicitly request the cache to be disabled
+        # too, since we have an interface for that via HTTPChannel.
+        request = DocumentSpec(url)
+        request.setCacheControl(DocumentSpec.CCNoCache)
+
+        print "Downloading %s" % (request)
 
         rf = Ramfile()
-        channel = http.getDocument(url)
+        channel = http.makeChannel(False)
+        channel.getDocument(request)
         if not channel.downloadToRam(rf):
             print "Unable to download %s" % (url)
 
@@ -145,6 +157,24 @@ class HostInfo:
 
         return package
 
+    def getPackages(self, name, platform = None):
+        """ Returns a list of PackageInfo objects that match the
+        indicated name and/or platform, with no particular regards to
+        version. """
+
+        assert self.hasContentsFile
+
+        packages = []
+        for (pn, version), platforms in self.packages.items():
+            if pn != name:
+                continue
+
+            package = self.getPackage(name, version, platform = platform)
+            if package:
+                packages.append(package)
+
+        return packages
+
     def __determineHostDir(self, appRunner):
         """ Hashes the host URL into a (mostly) unique directory
         string, which will be the root of the host's install tree.

direct/src/p3d/PackageInfo.py

@@ -141,10 +141,14 @@ class PackageInfo:
 
         filename = Filename(self.packageDir, self.descFileBasename)
         filename.makeDir()
+        filename.unlink()
         f = open(filename.toOsSpecific(), 'wb')
         f.write(rf.getData())
         f.close()
 
+        # Now that we've written the desc file, make it read-only.
+        os.chmod(filename.toOsSpecific(), 0444)
+
         try:
             self.readDescFile()
         except ValueError:
@@ -275,11 +279,51 @@ class PackageInfo:
             # plan B as the only plan.
             self.installPlans = [planB]
 
+    def __scanDirectoryRecursively(self, dirname):
+        """ Generates a list of Filename objects: all of the files
+        (not directories) within and below the indicated dirname. """
+        
+        contents = []
+        for dirpath, dirnames, filenames in os.walk(dirname.toOsSpecific()):
+            dirpath = Filename.fromOsSpecific(dirpath)
+            if dirpath == dirname:
+                dirpath = Filename('')
+            else:
+                dirpath.makeRelativeTo(dirname)
+            for filename in filenames:
+                contents.append(Filename(dirpath, filename))
+        return contents
+
+    def __removeFileFromList(self, contents, filename):
+        """ Removes the indicated filename from the given list, if it is
+        present.  """
+        try:
+            contents.remove(Filename(filename))
+        except ValueError:
+            pass
 
     def __checkArchiveStatus(self):
         """ Returns true if the archive and all extractable files are
         already correct on disk, false otherwise. """
 
+        # Get a list of all of the files in the directory, so we can
+        # remove files that don't belong.
+        contents = self.__scanDirectoryRecursively(self.packageDir)
+        self.__removeFileFromList(contents, self.uncompressedArchive.filename)
+        self.__removeFileFromList(contents, self.descFileBasename)
+        for file in self.extracts:
+            self.__removeFileFromList(contents, file.filename)
+
+        # Now, any files that are still in the contents list don't
+        # belong.  It's important to remove these files before we
+        # start verifying the files that we expect to find here, in
+        # case there is a problem with ambiguous filenames or
+        # something (e.g. case insensitivity).
+        for filename in contents:
+            print "Removing %s" % (filename)
+            pathname = Filename(self.packageDir, filename)
+            pathname.unlink()
+            
         allExtractsOk = True
         if not self.uncompressedArchive.quickVerify(self.packageDir):
             #print "File is incorrect: %s" % (self.uncompressedArchive.filename)
@@ -292,6 +336,10 @@ class PackageInfo:
                     allExtractsOk = False
                     break
 
+        if allExtractsOk:
+            print "All %s extracts of %s seem good." % (
+                len(self.extracts), self.packageName)
+
         return allExtractsOk
 
     def __updateStepProgress(self, step):
@@ -451,6 +499,7 @@ class PackageInfo:
 
         sourcePathname = Filename(self.packageDir, self.compressedArchive.filename)
         targetPathname = Filename(self.packageDir, self.uncompressedArchive.filename)
+        targetPathname.unlink()
         print "Uncompressing %s to %s" % (sourcePathname, targetPathname)
         decompressor = Decompressor()
         decompressor.initiate(sourcePathname, targetPathname)
@@ -473,6 +522,9 @@ class PackageInfo:
                 self.uncompressedArchive.filename)
             return False
 
+        # Now that we've verified the archive, make it read-only.
+        os.chmod(targetPathname.toOsSpecific(), 0444)
+
         # Now we can safely remove the compressed archive.
         sourcePathname.unlink()
         return True
@@ -503,6 +555,7 @@ class PackageInfo:
                 continue
 
             targetPathname = Filename(self.packageDir, file.filename)
+            targetPathname.unlink()
             if not mf.extractSubfile(i, targetPathname):
                 print "Couldn't extract: %s" % (file.filename)
                 allExtractsOk = False
@@ -513,8 +566,8 @@ class PackageInfo:
                 allExtractsOk = False
                 continue
 
-            # Make sure it's executable.
+            # Make sure it's executable, and not writable.
-            os.chmod(targetPathname.toOsSpecific(), 0755)
+            os.chmod(targetPathname.toOsSpecific(), 0555)
 
             step.bytesDone += file.size
             self.__updateStepProgress(step)

direct/src/p3d/Packager.py

@@ -1993,6 +1993,15 @@ class Packager:
 
         host = appRunner.getHost(hostUrl)
         package = host.getPackage(packageName, version, platform = platform)
+        if not package and version is None:
+            # With no version specified, find the best matching version.
+            packages = host.getPackages(packageName, platform = platform)
+            self.__sortPackageInfos(packages)
+            for p in packages:
+                if p and self.__packageIsValid(p, requires):
+                    package = p
+                    break
+            
         if not package or not package.importDescFile:
             return None
 
@@ -2022,6 +2031,19 @@ class Packager:
 
         return map(lambda t: t[1], tuples)
 
+    def __sortPackageInfos(self, packages):
+        """ Given a list of PackageInfos retrieved from a Host, sorts
+        them in reverse order by version, so that the highest-numbered
+        versions appear first in the list. """
+
+        tuples = []
+        for package in packages:
+            version = self.__makeVersionTuple(package.packageVersion)
+            tuples.append((version, file))
+        tuples.sort(reverse = True)
+
+        return map(lambda t: t[1], tuples)
+
     def __makeVersionTuple(self, version):
         """ Converts a version string into a tuple for sorting, by
         separating out numbers into separate numeric fields, so that

direct/src/p3d/packp3d.py

@@ -148,7 +148,11 @@ def makePackedApp(args):
         packager.setup()
         packager.beginPackage(appBase, p3dApplication = True)
         for requireName in requires:
-            packager.do_require(requireName)
+            tokens = requireName.split(',')
+            while len(tokens) < 3:
+                tokens.append(None)
+            name, version, host = tokens
+            packager.do_require(name, version = version, host = host)
 
         if autoStart:
             packager.do_config(auto_start = True)

direct/src/plugin/mkdir_complete.cxx

@@ -124,6 +124,9 @@ mkdir_complete(const string &dirname, ostream &logfile) {
 ////////////////////////////////////////////////////////////////////
 bool
 mkfile_complete(const string &filename, ostream &logfile) {
+  // Make sure we delete any previously-existing file first.
+  unlink(filename.c_str());
+
 #ifdef _WIN32
   HANDLE file = CreateFile(filename.c_str(), GENERIC_READ | GENERIC_WRITE,
                            FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,

direct/src/plugin/p3dInstanceManager.cxx

@@ -867,6 +867,89 @@ scan_directory(const string &dirname, vector<string> &contents) {
 #endif  // _WIN32
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: P3DInstanceManager::scan_directory_recursively
+//       Access: Public, Static
+//  Description: Fills up the indicated vector with the list of all
+//               files (but not directories) rooted at the indicated
+//               dirname and below.  The filenames generated are
+//               relative to the root of the dirname, with slashes
+//               (not backslashes) as the directory separator
+//               character.
+//
+//               Returns true on success, false if the original
+//               dirname wasn't a directory or something like that.
+////////////////////////////////////////////////////////////////////
+bool P3DInstanceManager::
+scan_directory_recursively(const string &dirname, vector<string> &contents,
+                           const string &prefix) {
+  vector<string> dir_contents;
+  if (!scan_directory(dirname, dir_contents)) {
+    // Apparently dirname wasn't a directory.
+    return false;
+  }
+
+  // Walk through the contents of dirname.
+  vector<string>::const_iterator si;
+  for (si = dir_contents.begin(); si != dir_contents.end(); ++si) {
+    // Here's a particular file within dirname.  Is it another
+    // directory, or is it a regular file?
+    string pathname = dirname + "/" + (*si);
+    string rel_filename = prefix + (*si);
+    if (scan_directory_recursively(pathname, contents, rel_filename + "/")) {
+      // It's a directory, and it's just added its results to the
+      // contents.
+
+    } else {
+      // It's not a directory, so assume it's an ordinary file, and
+      // add it to the contents.
+      contents.push_back(rel_filename);
+    }
+  }
+
+  return true;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: P3DInstanceManager::remove_file_from_list
+//       Access: Public, Static
+//  Description: Removes the first instance of the indicated file
+//               from the given list.  Returns true if removed, false
+//               if it was not found.
+//
+//               On Windows, the directory separator characters are
+//               changed from backslash to forward slash before
+//               searching in the list; so it is assumed that the list
+//               contains filenames with a forward slash used as a
+//               separator.
+////////////////////////////////////////////////////////////////////
+bool P3DInstanceManager::
+remove_file_from_list(vector<string> &contents, const string &filename) {
+#ifdef _WIN32
+  // Convert backslashes to slashes.
+  string clean_filename;
+  for (string::iterator pi = filename.begin(); pi != filename.end(); ++pi) {
+    if ((*pi) == '\\') {
+      clean_filename += '/';
+    } else {
+      clean_filename += (*pi);
+    }
+  }
+#else
+  const string &clean_filename = filename;
+#endif  // _WIN32
+
+  vector<string>::iterator ci;
+  for (ci = contents.begin(); ci != contents.end(); ++ci) {
+    if ((*ci) == clean_filename) {
+      contents.erase(ci);
+      return true;
+    }
+  }
+
+  return false;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: P3DInstanceManager::get_global_ptr
 //       Access: Public, Static

direct/src/plugin/p3dInstanceManager.h

@@ -108,6 +108,9 @@ public:
 
   static inline char encode_hexdigit(int c);
   static bool scan_directory(const string &dirname, vector<string> &contents);
+  static bool scan_directory_recursively(const string &dirname, vector<string> &contents,
+                                         const string &prefix = "");
+  static bool remove_file_from_list(vector<string> &contents, const string &filename);
 private:
   // The notify thread.  This thread runs only for the purpose of
   // generating asynchronous notifications of requests, to callers who

direct/src/plugin/p3dMultifileReader.cxx

@@ -135,7 +135,10 @@ extract_all(const string &to_dir,
     utb.actime = time(NULL);
     utb.modtime = s._timestamp;
     utime(output_pathname.c_str(), &utb);
+
 #ifndef _WIN32
+    // Be sure to execute permissions on the file, in case it's a
+    // program or something.
     chmod(output_pathname.c_str(), 0555);
 #endif
 

direct/src/plugin/p3dPackage.cxx

@@ -246,8 +246,14 @@ download_contents_file() {
     return;
   }
 
-  string url = _host->get_host_url_prefix();
+  // Get the URL for contents.xml.
-  url += "contents.xml";
+  ostringstream strm;
+  strm << _host->get_host_url_prefix() << "contents.xml";
+  // Append a uniquifying query string to the URL to force the
+  // download to go all the way through any caches.  We use the time
+  // in seconds; that's unique enough.
+  strm << "?" << time(NULL);
+  string url = strm.str();
 
   // Download contents.xml to a temporary filename first, in case
   // multiple packages are downloading it simultaneously.
@@ -359,6 +365,11 @@ desc_file_download_finished(bool success) {
     return;
   }
 
+#ifndef _WIN32
+  // Now that we've downloaded the desc file, make it read-only.
+  chmod(_desc_file_pathname.c_str(), 0444);
+#endif
+
   if (_package_solo) {
     // No need to load it: the desc file *is* the package.
     report_done(true);
@@ -426,6 +437,31 @@ got_desc_file(TiXmlDocument *doc, bool freshly_downloaded) {
     extract = extract->NextSiblingElement("extract");
   }
 
+  // Get a list of all of the files in the directory, so we can remove
+  // files that don't belong.
+  P3DInstanceManager *inst_mgr = P3DInstanceManager::get_global_ptr();
+  vector<string> contents;
+  inst_mgr->scan_directory_recursively(_package_dir, contents);
+
+  inst_mgr->remove_file_from_list(contents, _desc_file_basename);
+  inst_mgr->remove_file_from_list(contents, _uncompressed_archive.get_filename());
+  Extracts::iterator ei;
+  for (ei = _extracts.begin(); ei != _extracts.end(); ++ei) {
+    inst_mgr->remove_file_from_list(contents, (*ei).get_filename());
+  }
+
+  // Now, any files that are still in the contents list don't belong.
+  // It's important to remove these files before we start verifying
+  // the files that we expect to find here, in case there is a problem
+  // with ambiguous filenames or something (e.g. case insensitivity).
+  vector<string>::iterator ci;
+  for (ci = contents.begin(); ci != contents.end(); ++ci) {
+    string filename = (*ci);
+    nout << "Removing " << filename << "\n";
+    string pathname = _package_dir + "/" + filename;
+    unlink(pathname.c_str());
+  }
+
   // Verify the uncompressed archive.
   bool all_extracts_ok = true;
   if (!_uncompressed_archive.quick_verify(_package_dir)) {
@@ -434,10 +470,9 @@ got_desc_file(TiXmlDocument *doc, bool freshly_downloaded) {
   }
 
   // Verify all of the extracts.
-  Extracts::iterator ci;
+  for (ei = _extracts.begin(); ei != _extracts.end() && all_extracts_ok; ++ei) {
-  for (ci = _extracts.begin(); ci != _extracts.end() && all_extracts_ok; ++ci) {
+    if (!(*ei).quick_verify(_package_dir)) {
-    if (!(*ci).quick_verify(_package_dir)) {
+      nout << "File is incorrect: " << (*ei).get_filename() << "\n";
-      nout << "File is incorrect: " << (*ci).get_filename() << "\n";
       all_extracts_ok = false;
     }
   }
@@ -446,6 +481,7 @@ got_desc_file(TiXmlDocument *doc, bool freshly_downloaded) {
     // Great, we're ready to begin.
     nout << "All " << _extracts.size() << " extracts of " << _package_name
          << " seem good.\n";
+
     report_done(true);
 
   } else {
@@ -687,6 +723,12 @@ uncompress_archive() {
     return;
   }
 
+#ifndef _WIN32
+  // Now that we've verified the archive, make it read-only.
+  chmod(target_pathname.c_str(), 0444);
+#endif
+
+  // Now we can safely remove the compressed archive.
   unlink(source_pathname.c_str());
 
   // All done uncompressing.
@@ -828,9 +870,9 @@ start_download(P3DPackage::DownloadType dtype, const string &url,
 ////////////////////////////////////////////////////////////////////
 bool P3DPackage::
 is_extractable(const string &filename) const {
-  Extracts::const_iterator ci;
+  Extracts::const_iterator ei;
-  for (ci = _extracts.begin(); ci != _extracts.end(); ++ci) {
+  for (ei = _extracts.begin(); ei != _extracts.end(); ++ei) {
-    if ((*ci).get_filename() == filename) {
+    if ((*ei).get_filename() == filename) {
       return true;
     }
   }

direct/src/plugin_npapi/ppInstance.cxx

@@ -112,7 +112,15 @@ begin() {
     if (!url.empty() && url[url.length() - 1] != '/') {
       url += '/';
     }
-    url += "contents.xml";
+    ostringstream strm;
+    strm << url << "contents.xml";
+
+    // Append a uniquifying query string to the URL to force the
+    // download to go all the way through any caches.  We use the time
+    // in seconds; that's unique enough.
+    strm << "?" << time(NULL);
+    url = strm.str();
+
     PPDownloadRequest *req = new PPDownloadRequest(PPDownloadRequest::RT_contents_file);
     start_download(url, req);
   }

direct/src/plugin_standalone/panda3d.cxx

@@ -382,11 +382,22 @@ get_plugin(const string &download_url, const string &this_platform,
   }
 
   // Couldn't read it, so go get it.
-  string url = download_url;
+  ostringstream strm;
-  url += "contents.xml";
+  strm << download_url << "contents.xml";
+  // Append a uniquifying query string to the URL to force the
+  // download to go all the way through any caches.  We use the time
+  // in seconds; that's unique enough.
+  strm << "?" << time(NULL);
+  string url = strm.str();
+
+  // We might as well explicitly request the cache to be disabled too,
+  // since we have an interface for that via HTTPChannel.
+  DocumentSpec request(url);
+  request.set_cache_control(DocumentSpec::CC_no_cache);
   
   HTTPClient *http = HTTPClient::get_global_ptr();
-  PT(HTTPChannel) channel = http->get_document(url);
+  PT(HTTPChannel) channel = http->make_channel(false);
+  channel->get_document(request);
 
   // First, download it to a temporary file.
   Filename tempfile = Filename::temporary("", "p3d_");