The update-webkit-dependency script could use several improvements. First, every time it is ran it tries to download the entire dependency just in order to get the Last-Modified header so that it can check to see if the dependency is up to date. This could be improved by instead doing a request for just the first byte of the dependency if the server supports range requests and also setting a size limit for the request so the entire dependency is not downloaded in order to get the headers. This can save a decent amount of time and bandwidth for people. Second, it tries to use the Last-Modified header to determine whether the dependency is up to date and if the server doesn't emit Last-Modified headers (like Dropbox's server which is currently used to store the WinCairoRequirements.zip file and also GitHub as well) it then tries to get the Last-Modified header from a separate *.headers file that is also placed on the server. This could be improved by also using ETag headers (which Dropbox and GitHub both support) to see if the dependency is up to date. When using Dropbox this would generally eliminate the need to maintain those separate *.headers file which seems to cause problems from time to time (see https://lists.webkit.org/pipermail/webkit-help/2015-November/003987.html and bug 153876). Third, it would also be nice if before installing a dependency, it first checked the zip file to make sure it contains the expected directories. This would help in preventing problems like those in bug 153876. The patch that I'll be attaching in a moment makes these improvements to the script plus makes a few other minor improvements (more error checking and better error messages mainly).
Created attachment 274258 [details] Patch to improve update-webkit-dependency.
Attachment 274258 [details] did not pass style-queue: ERROR: Tools/ChangeLog:17: Line contains tab character. [whitespace/tab] [5] ERROR: Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 2 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 274259 [details] Patch to improve update-webkit-dependency.
Comment on attachment 274259 [details] Patch to improve update-webkit-dependency. This seems ok to me. Brent, could you double check this? I'm not sure if this will break the ews bots.
Comment on attachment 274259 [details] Patch to improve update-webkit-dependency. View in context: https://bugs.webkit.org/attachment.cgi?id=274259&action=review This looks fine to me. We can roll it out if we encounter problems. r=me. > Tools/Scripts/update-webkit-dependency:197 > +my $prefixDirectoryPathInZipFile = "$file/".(($prefixInZip eq ".") ? "" : "$prefixInZip/"); The use of the UNIX-style slash here *might* be a problem on Windows, but I'm not sure.
Comment on attachment 274259 [details] Patch to improve update-webkit-dependency. Clearing flags on attachment: 274259 Committed r198638: <http://trac.webkit.org/changeset/198638>
All reviewed patches have been landed. Closing bug.
(In reply to comment #5) > > Tools/Scripts/update-webkit-dependency:197 > > +my $prefixDirectoryPathInZipFile = "$file/".(($prefixInZip eq ".") ? "" : "$prefixInZip/"); > > The use of the UNIX-style slash here *might* be a problem on Windows, but > I'm not sure. I was concerned about that too but it turns out that the documentation for the Archive::Zip module says "Regardless of what your local file system uses for file naming, names in a Zip file are in Unix format (forward slashes (/) separating directory names, etc.)." So the use of the forward slashes should work fine and might even be required.