From bd71beaf765ea6f82a06bde820f7bd9ce44f8b50 Mon Sep 17 00:00:00 2001 From: David Rose Date: Sun, 20 Sep 2009 18:47:04 +0000 Subject: [PATCH] more cache-busting, keep install directory clean --- direct/src/p3d/HostInfo.py | 38 +++++++++-- direct/src/p3d/PackageInfo.py | 59 ++++++++++++++++- direct/src/p3d/Packager.py | 22 +++++++ direct/src/p3d/packp3d.py | 6 +- direct/src/plugin/mkdir_complete.cxx | 3 + direct/src/plugin/p3dInstanceManager.cxx | 83 ++++++++++++++++++++++++ direct/src/plugin/p3dInstanceManager.h | 3 + direct/src/plugin/p3dMultifileReader.cxx | 3 + direct/src/plugin/p3dPackage.cxx | 60 ++++++++++++++--- direct/src/plugin_npapi/ppInstance.cxx | 10 ++- direct/src/plugin_standalone/panda3d.cxx | 17 ++++- 11 files changed, 283 insertions(+), 21 deletions(-) diff --git a/direct/src/p3d/HostInfo.py b/direct/src/p3d/HostInfo.py index d6520b74ea..1e2dc9d696 100644 --- a/direct/src/p3d/HostInfo.py +++ b/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') - print "Downloading %s" % (url) + url = self.hostUrlPrefix + '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. + 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. diff --git a/direct/src/p3d/PackageInfo.py b/direct/src/p3d/PackageInfo.py index a1e2ab6ed7..b61e2e4ab9 100644 --- a/direct/src/p3d/PackageInfo.py +++ b/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: @@ -274,12 +278,52 @@ class PackageInfo: # There are no patches to download, oh well. Stick with # 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. - os.chmod(targetPathname.toOsSpecific(), 0755) + # Make sure it's executable, and not writable. + os.chmod(targetPathname.toOsSpecific(), 0555) step.bytesDone += file.size self.__updateStepProgress(step) diff --git a/direct/src/p3d/Packager.py b/direct/src/p3d/Packager.py index c078313755..c740cd46cb 100644 --- a/direct/src/p3d/Packager.py +++ b/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 diff --git a/direct/src/p3d/packp3d.py b/direct/src/p3d/packp3d.py index 0ccb109220..56078107f7 100755 --- a/direct/src/p3d/packp3d.py +++ b/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) diff --git a/direct/src/plugin/mkdir_complete.cxx b/direct/src/plugin/mkdir_complete.cxx index bdcded13a2..65eaa3ecd5 100755 --- a/direct/src/plugin/mkdir_complete.cxx +++ b/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, diff --git a/direct/src/plugin/p3dInstanceManager.cxx b/direct/src/plugin/p3dInstanceManager.cxx index d21f510d7a..558a28fe17 100644 --- a/direct/src/plugin/p3dInstanceManager.cxx +++ b/direct/src/plugin/p3dInstanceManager.cxx @@ -867,6 +867,89 @@ scan_directory(const string &dirname, vector &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 &contents, + const string &prefix) { + vector dir_contents; + if (!scan_directory(dirname, dir_contents)) { + // Apparently dirname wasn't a directory. + return false; + } + + // Walk through the contents of dirname. + vector::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 &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::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 diff --git a/direct/src/plugin/p3dInstanceManager.h b/direct/src/plugin/p3dInstanceManager.h index 2a3d7afdba..9e56b91b52 100644 --- a/direct/src/plugin/p3dInstanceManager.h +++ b/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 &contents); + static bool scan_directory_recursively(const string &dirname, vector &contents, + const string &prefix = ""); + static bool remove_file_from_list(vector &contents, const string &filename); private: // The notify thread. This thread runs only for the purpose of // generating asynchronous notifications of requests, to callers who diff --git a/direct/src/plugin/p3dMultifileReader.cxx b/direct/src/plugin/p3dMultifileReader.cxx index f6d6833fe2..51ac788762 100644 --- a/direct/src/plugin/p3dMultifileReader.cxx +++ b/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 diff --git a/direct/src/plugin/p3dPackage.cxx b/direct/src/plugin/p3dPackage.cxx index 98b26ea7da..c7736ecdfe 100755 --- a/direct/src/plugin/p3dPackage.cxx +++ b/direct/src/plugin/p3dPackage.cxx @@ -246,8 +246,14 @@ download_contents_file() { return; } - string url = _host->get_host_url_prefix(); - url += "contents.xml"; + // Get the URL for 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 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::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 (ci = _extracts.begin(); ci != _extracts.end() && all_extracts_ok; ++ci) { - if (!(*ci).quick_verify(_package_dir)) { - nout << "File is incorrect: " << (*ci).get_filename() << "\n"; + for (ei = _extracts.begin(); ei != _extracts.end() && all_extracts_ok; ++ei) { + if (!(*ei).quick_verify(_package_dir)) { + nout << "File is incorrect: " << (*ei).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; - for (ci = _extracts.begin(); ci != _extracts.end(); ++ci) { - if ((*ci).get_filename() == filename) { + Extracts::const_iterator ei; + for (ei = _extracts.begin(); ei != _extracts.end(); ++ei) { + if ((*ei).get_filename() == filename) { return true; } } diff --git a/direct/src/plugin_npapi/ppInstance.cxx b/direct/src/plugin_npapi/ppInstance.cxx index 7d7a96b1df..1719a77d6d 100644 --- a/direct/src/plugin_npapi/ppInstance.cxx +++ b/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); } diff --git a/direct/src/plugin_standalone/panda3d.cxx b/direct/src/plugin_standalone/panda3d.cxx index 2e24429a05..0dc7b5abc9 100644 --- a/direct/src/plugin_standalone/panda3d.cxx +++ b/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; - url += "contents.xml"; + ostringstream strm; + 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_");