-
Notifications
You must be signed in to change notification settings - Fork 28
Improve autodetection for which version of ZLS to download #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -878,11 +878,23 @@ fn getVersionUrl( | |
| app_data_path: []const u8, | ||
| semantic_version: SemanticVersion, | ||
| ) !DownloadUrl { | ||
| if (build_options.exe == .zls) return DownloadUrl.initOfficial(std.fmt.allocPrint( | ||
| arena, | ||
| "https://builds.zigtools.org/zls-{s}-{}.{s}", | ||
| .{ os_arch, semantic_version, archive_ext }, | ||
| ) catch |e| oom(e)); | ||
| if (build_options.exe == .zls) { | ||
| const info_url = try std.fmt.allocPrint(arena, "https://releases.zigtools.org/v1/zls/select-version?compatibility=only-runtime&zig_version={}", .{semantic_version}); | ||
| const info_uri = try std.Uri.parse(info_url); | ||
| const info_basename = try std.fmt.allocPrint(arena, "download-index-zls-{}.json", .{semantic_version}); | ||
| const index_path = try std.fs.path.join(arena, &.{ app_data_path, info_basename }); | ||
|
|
||
| try fetchFile(arena, info_url, info_uri, index_path); | ||
| const index_content = blk: { | ||
| // since we just downloaded the file, this should always succeed now | ||
| const file = try std.fs.cwd().openFile(index_path, .{}); | ||
| defer file.close(); | ||
| break :blk try file.readToEndAlloc(arena, std.math.maxInt(usize)); | ||
| }; | ||
| defer arena.free(index_content); | ||
|
|
||
| return extractUrlFromZlsDownloadIndex(arena, index_path, index_content); | ||
| } | ||
|
Comment on lines
+881
to
+897
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I'm reading this right, this appears to do the same thing as what we do in the mach case, download the index and get the actual download URL from there. However, you're missing the index cache we do for the mach case. Seems like we should re-use the same code for both cases.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. I'll try to generalize the URL fetching, because I have in mind to implement downloading from community mirrors eventually and this would be an important part. |
||
|
|
||
| if (!isMachVersion(semantic_version)) return makeOfficialUrl(arena, semantic_version); | ||
|
|
||
|
|
@@ -942,6 +954,45 @@ fn extractMasterVersion( | |
| ); | ||
| } | ||
|
|
||
| fn extractUrlFromZlsDownloadIndex( | ||
| allocator: std.mem.Allocator, | ||
| index_filepath: []const u8, | ||
| download_index: []const u8, | ||
| ) DownloadUrl { | ||
| const root = std.json.parseFromSlice(std.json.Value, allocator, download_index, .{ | ||
| .allocate = .alloc_if_needed, | ||
| }) catch |e| std.debug.panic( | ||
| "failed to parse download index '{s}' as JSON with {s}\n{s}", | ||
| .{ index_filepath, @errorName(e), download_index }, | ||
| ); | ||
| defer root.deinit(); | ||
| if (root.value != .object) std.debug.panic( | ||
| "zls index root value is not an object\n{s}", | ||
| .{download_index}, | ||
| ); | ||
| const arch_os_obj = root.value.object.get(arch_os) orelse std.debug.panic( | ||
| "zls index does not contain an entry for arch-os '{s}'\n{s}", | ||
| .{ arch_os, download_index }, | ||
| ); | ||
| const fetch_url = arch_os_obj.object.get("tarball") orelse std.debug.panic( | ||
| "zls index arch-os '{s}' is missing the 'tarball' property\n{s}", | ||
| .{ arch_os, download_index }, | ||
| ); | ||
| if (fetch_url != .string) std.debug.panic( | ||
| "zls index arch-os '{s}' tarball property is not a string\n{s}", | ||
| .{ arch_os, download_index }, | ||
| ); | ||
| const shasum_url = arch_os_obj.object.get("shasum") orelse std.debug.panic( | ||
| "zls index arch-os '{s}' is missing the 'shasum' property\n{s}", | ||
| .{ arch_os, download_index }, | ||
| ); | ||
| _ = shasum_url; // TODO: verify shasum | ||
| return .{ | ||
| .fetch = allocator.dupe(u8, fetch_url.string) catch |e| oom(e), | ||
| .official = allocator.dupe(u8, fetch_url.string) catch |e| oom(e), | ||
| }; | ||
| } | ||
|
|
||
| fn extractUrlFromMachDownloadIndex( | ||
| allocator: std.mem.Allocator, | ||
| semantic_version: SemanticVersion, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work with Zig master versions, since the
+in the version is unescaped, and this ZLS API seems to expect that:The working URL in this case is
https://releases.zigtools.org/v1/zls/select-version?compatibility=only-runtime&zig_version=0.16.0-dev.1484%2Bd0ba6642b.I applied this fix in my copy and it works for me: