Improve autodetection for which version of ZLS to download #64
Conversation
tlaferriere
changed the title
tlaferriere
changed the title
|
I've reverted changes that made anyzig exit with the child process exit code, because they are not strictly necessary for this feature. I do think it would make sense to keep exit code compatibility with the underlying command, but I would integrate in another PR. |
| 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); | ||
| } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| .{ 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}); |
There was a problem hiding this comment.
This doesn't work with Zig master versions, since the + in the version is unescaped, and this ZLS API seems to expect that:
anyzig: error: fetch 'https://releases.zigtools.org/v1/zls/select-version?compatibility=only-runtime&zig_version=0.16.0-dev.1484+d0ba6642b': HTTP response 400 "Bad Request"
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:
| const info_url = try std.fmt.allocPrint(arena, "https://releases.zigtools.org/v1/zls/select-version?compatibility=only-runtime&zig_version={}", .{semantic_version}); | |
| const fmt_version = try std.fmt.allocPrint(arena, "{}", .{semantic_version}); | |
| const fmt_version_escaped = try std.mem.replaceOwned(u8, arena, fmt_version, "+", "%2B"); | |
| const info_url = try std.fmt.allocPrint(arena, "https://releases.zigtools.org/v1/zls/select-version?compatibility=only-runtime&zig_version={s}", .{fmt_version_escaped}); |
4 participants
This includes the previous PR #61, with fixes to the tests. The tests were failing due to newer versions of zig formatting the unknown command error differently than used to. I simply made the test more flexible, while retaining the principle of it (that it should output an error message and exit code 1).