Add a Python script for hitting the GitHub API and querying for the latest release of a project. Then the location should be passed in for downloading. This will be important for WinCairoRequirements and for downloading something like vswhere for detection of Visual Studio instances.
Created attachment 317863 [details] Patch
The only thing I'm not sure of is how we prevent multiple downloads. Maybe just a file with the same extension and a .version so like WinCairoRequirements.zip.version. Think we would then need a force flag as well where it'll do the logic no matter what.
Comment on attachment 317863 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=317863&action=review > Tools/Scripts/get-latest-github-release.py:59 > + data = json.loads(urllib2.urlopen(request).read()) Ugh, this must've been a Cmd-Z fail. Should be response.read(), of course.
Created attachment 318091 [details] Patch
Comment on attachment 318091 [details] Patch - Add functionality to save / check a .version file of JSON format: {"tag_name": "2.1.3", "updated_at": "2017-07-29T03:53:05Z"} - Add a --force/-f flag to skip this check.
(In reply to Ross Kirsling from comment #4) > Created attachment 318091 [details] > Patch I am by no means a Python expert, but I think this looks good. Brent, is this something you would like to weigh in on?
Alexey or Alex, do you have a take on this? I think the patch looks good, but I thought I'd run it by you before approving. The script will be used to download 'vswhere' (detecting installation location of Visual Studio 2017), and to download WinCairo requirements (dlls and libs for building WinCairo).
Comment on attachment 318091 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=318091&action=review I'm not an expert either. My only comment is about usability - with a somewhat generic name, the tool should probably better explain what it does. It's too easy to assume that it gets the source code for the latest Gtk release or something like that. Maybe some example invocations would help. Also, if it's actually downloading something, it should be called "download", not "get" I think. > Tools/Scripts/get-latest-github-release.py:39 > + parser.add_argument('--output', '-o', default='.', help='output directory') It is a little annoying that curl's "-o" option has a different meaning, but I don't have a proposal for how to improve this. > Tools/Scripts/get-latest-github-release.py:40 > + parser.add_argument('--endpoint', '-e', default=PUBLIC_GITHUB_API_ENDPOINT, help='GitHub API endpoint (defaults to public)') Do you envision a need to use a different endpoint? Will the users of the tool know how to make this choice? > Tools/Scripts/get-latest-github-release.py:41 > + parser.add_argument('--token', '-t', default=None, help='GitHub API OAuth token (where applicable)') Again, will the users know if it's applicable? > Tools/Scripts/get-latest-github-release.py:42 > + parser.add_argument('--force', '-f', action='store_true', help='download latest release even if it already exists') Exists where? Will this overwrite an existing file with the same name? Why is this option needed?
(In reply to Alexey Proskuryakov from comment #8) > I'm not an expert either. My only comment is about usability - with a > somewhat generic name, the tool should probably better explain what it does. > It's too easy to assume that it gets the source code for the latest Gtk > release or something like that. Maybe some example invocations would help. This script can be used to download any binary asset included in the latest release of a GitHub repo to an arbitrary output directory, so it actually *is* extremely generic. I can certainly add a docblock with example usage though. > Also, if it's actually downloading something, it should be called > "download", not "get" I think. Can do. > > Tools/Scripts/get-latest-github-release.py:39 > > + parser.add_argument('--output', '-o', default='.', help='output directory') > > It is a little annoying that curl's "-o" option has a different meaning, but > I don't have a proposal for how to improve this. This is the same sort of "-o" as GCC though. > > Tools/Scripts/get-latest-github-release.py:40 > > + parser.add_argument('--endpoint', '-e', default=PUBLIC_GITHUB_API_ENDPOINT, help='GitHub API endpoint (defaults to public)') > > Do you envision a need to use a different endpoint? Will the users of the > tool know how to make this choice? > > > Tools/Scripts/get-latest-github-release.py:41 > > + parser.add_argument('--token', '-t', default=None, help='GitHub API OAuth token (where applicable)') > > Again, will the users know if it's applicable? These ensure that the script is able to target private repositories or private GitHub (Enterprise) instances. Whether or not these options are used upstream, they are necessary for completeness. > > Tools/Scripts/get-latest-github-release.py:42 > > + parser.add_argument('--force', '-f', action='store_true', help='download latest release even if it already exists') > > Exists where? Will this overwrite an existing file with the same name? Why > is this option needed? The need is explained in the thread above (i.e. "downloads are time-consuming so avoid doing so unnecessarily by default"), but we can be explicit about it existing "in the specified output directory".
> > > Tools/Scripts/get-latest-github-release.py:39 > > > + parser.add_argument('--output', '-o', default='.', help='output directory') > > > > It is a little annoying that curl's "-o" option has a different meaning, but > > I don't have a proposal for how to improve this. > > This is the same sort of "-o" as GCC though. Well, I guess I take this back; I suppose your point was that you expected it to specify the output file and not just the output directory. Still, we already know the asset's filename, and I thought it would be most convenient to have an option that defaults to the working directory instead of a third positional argument.
Created attachment 318446 [details] Patch
Comment on attachment 318446 [details] Patch - Change "get" to "download" in script name. - Make help page more descriptive about the purpose and usage of this script. - Remove -f/--force option after all. (Don and I discussed this further and decided that we're not specifically expecting a case for this to be used anyway.)
Comment on attachment 318446 [details] Patch I assume this new script will be used in 'update-webkit' or something? I think this patch looks fine, and seems like a good idea. We have a number of things this could be useful for retrieving.
Comment on attachment 318446 [details] Patch Clearing flags on attachment: 318446 Committed r220918: <http://trac.webkit.org/changeset/220918>
All reviewed patches have been landed. Closing bug.
<rdar://problem/33963620>