Skip to content

Follow-up: zig fetch: resolve branch/tag names to commit SHA #19941

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
May 11, 2024
Merged

Follow-up: zig fetch: resolve branch/tag names to commit SHA #19941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69d53c9
Revert "Revert "Merge pull request #19349 from nolanderc/save-commit""
nolanderc May 11, 2024
d1f0a34
update to new URI changes, rework `--save` type
nolanderc May 11, 2024
19dfc5c
initialize `latest_commit` to null everywhere
nolanderc May 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/Package/Fetch.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ omit_missing_hash_error: bool,
/// which specifies inclusion rules. This is intended to be true for the first
/// fetch task and false for the recursive dependencies.
allow_missing_paths_field: bool,
/// If true and URL points to a Git repository, will use the latest commit.
use_latest_commit: bool,

// Above this are fields provided as inputs to `run`.
// Below this are fields populated by `run`.
Expand All @@ -59,6 +61,10 @@ actual_hash: Manifest.Digest,
has_build_zig: bool,
/// Indicates whether the task aborted due to an out-of-memory condition.
oom_flag: bool,
/// If `use_latest_commit` was true, this will be set to the commit that was used.
/// If the resource pointed to by the location is not a Git-repository, this
/// will be left unchanged.
latest_commit: ?git.Oid,
Copy link
Contributor

@rohlem rohlem May 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has an optional type, but is it ever null?
From a quick glance at the diff it seems to only be initialized to undefined, and only at one point assigned want_oid, which afaict is non-optional. (It's possible I missed something though.)
If there's no logical state using the null value we could make the field non-optional.

Copy link
Contributor Author

@nolanderc nolanderc May 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!
This was a few months ago, so my memory is a bit hazy, but I think it was made optional because not everything you can fetch is a git-repository, so the field would not necessary get initialized. But you are right, it should not be set to undefined everywhere, but null!


// This field is used by the CLI only, untouched by this file.

Expand Down Expand Up @@ -699,6 +705,7 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
.job_queue = f.job_queue,
.omit_missing_hash_error = false,
.allow_missing_paths_field = true,
.use_latest_commit = false,

.package_root = undefined,
.error_bundle = undefined,
Expand All @@ -707,6 +714,7 @@ fn queueJobsForDeps(f: *Fetch) RunError!void {
.actual_hash = undefined,
.has_build_zig = false,
.oom_flag = false,
.latest_commit = null,

.module = null,
};
Expand Down Expand Up @@ -994,7 +1002,9 @@ fn initResource(f: *Fetch, uri: std.Uri, server_header_buffer: []u8) RunError!Re
}
return f.fail(f.location_tok, try eb.printString("ref not found: {s}", .{want_ref}));
};
if (uri.fragment == null) {
if (f.use_latest_commit) {
f.latest_commit = want_oid;
} else if (uri.fragment == null) {
const notes_len = 1;
try eb.addRootErrorMessage(.{
.msg = try eb.addString("url field is missing an explicit ref"),
Expand Down
60 changes: 52 additions & 8 deletions src/main.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5097,6 +5097,7 @@ fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !void {
.job_queue = &job_queue,
.omit_missing_hash_error = true,
.allow_missing_paths_field = false,
.use_latest_commit = false,

.package_root = undefined,
.error_bundle = undefined,
Expand All @@ -5105,6 +5106,7 @@ fn cmdBuild(gpa: Allocator, arena: Allocator, args: []const []const u8) !void {
.actual_hash = undefined,
.has_build_zig = true,
.oom_flag = false,
.latest_commit = null,

.module = build_mod,
};
Expand Down Expand Up @@ -6894,6 +6896,8 @@ const usage_fetch =
\\ --debug-hash Print verbose hash information to stdout
\\ --save Add the fetched package to build.zig.zon
\\ --save=[name] Add the fetched package to build.zig.zon as name
\\ --save-exact Add the fetched package to build.zig.zon, storing the URL verbatim
\\ --save-exact=[name] Add the fetched package to build.zig.zon as name, storing the URL verbatim
\\
;

Expand All @@ -6908,7 +6912,11 @@ fn cmdFetch(
var opt_path_or_url: ?[]const u8 = null;
var override_global_cache_dir: ?[]const u8 = try EnvVar.ZIG_GLOBAL_CACHE_DIR.get(arena);
var debug_hash: bool = false;
var save: union(enum) { no, yes, name: []const u8 } = .no;
var save: union(enum) {
no,
yes: ?[]const u8,
exact: ?[]const u8,
} = .no;

{
var i: usize = 0;
Expand All @@ -6926,9 +6934,13 @@ fn cmdFetch(
} else if (mem.eql(u8, arg, "--debug-hash")) {
debug_hash = true;
} else if (mem.eql(u8, arg, "--save")) {
save = .yes;
save = .{ .yes = null };
} else if (mem.startsWith(u8, arg, "--save=")) {
save = .{ .name = arg["--save=".len..] };
save = .{ .yes = arg["--save=".len..] };
} else if (mem.eql(u8, arg, "--save-exact")) {
save = .{ .exact = null };
} else if (mem.startsWith(u8, arg, "--save-exact=")) {
save = .{ .exact = arg["--save=".len..] };
} else {
fatal("unrecognized parameter: '{s}'", .{arg});
}
Expand Down Expand Up @@ -6988,6 +7000,7 @@ fn cmdFetch(
.job_queue = &job_queue,
.omit_missing_hash_error = true,
.allow_missing_paths_field = false,
.use_latest_commit = true,

.package_root = undefined,
.error_bundle = undefined,
Expand All @@ -6996,6 +7009,7 @@ fn cmdFetch(
.actual_hash = undefined,
.has_build_zig = false,
.oom_flag = false,
.latest_commit = null,

.module = null,
};
Expand All @@ -7022,12 +7036,12 @@ fn cmdFetch(
try io.getStdOut().writeAll(hex_digest ++ "\n");
return cleanExit();
},
.yes => n: {
.yes, .exact => |name| name: {
if (name) |n| break :name n;
const fetched_manifest = fetch.manifest orelse
fatal("unable to determine name; fetched package has no build.zig.zon file", .{});
break :n fetched_manifest.name;
break :name fetched_manifest.name;
},
.name => |n| n,
};

const cwd_path = try process.getCwdAlloc(arena);
Expand All @@ -7052,13 +7066,43 @@ fn cmdFetch(
var fixups: Ast.Fixups = .{};
defer fixups.deinit(gpa);

var saved_path_or_url = path_or_url;

if (fetch.latest_commit) |*latest_commit| resolved: {
const latest_commit_hex = try std.fmt.allocPrint(arena, "{}", .{std.fmt.fmtSliceHexLower(latest_commit)});

var uri = try std.Uri.parse(path_or_url);

if (uri.fragment) |fragment| {
const target_ref = try fragment.toRawMaybeAlloc(arena);

// the refspec may already be fully resolved
if (std.mem.eql(u8, target_ref, latest_commit_hex)) break :resolved;

std.log.info("resolved ref '{s}' to commit {s}", .{ target_ref, latest_commit_hex });

// include the original refspec in a query parameter, could be used to check for updates
uri.query = .{ .percent_encoded = try std.fmt.allocPrint(arena, "ref={%}", .{fragment}) };
} else {
std.log.info("resolved to commit {s}", .{latest_commit_hex});
}

// replace the refspec with the resolved commit SHA
uri.fragment = .{ .raw = latest_commit_hex };

switch (save) {
.yes => saved_path_or_url = try std.fmt.allocPrint(arena, "{}", .{uri}),
.no, .exact => {}, // keep the original URL
}
}

const new_node_init = try std.fmt.allocPrint(arena,
\\.{{
\\ .url = "{}",
\\ .hash = "{}",
\\ }}
, .{
std.zig.fmtEscapes(path_or_url),
std.zig.fmtEscapes(saved_path_or_url),
std.zig.fmtEscapes(&hex_digest),
});

Expand All @@ -7078,7 +7122,7 @@ fn cmdFetch(
if (dep.hash) |h| {
switch (dep.location) {
.url => |u| {
if (mem.eql(u8, h, &hex_digest) and mem.eql(u8, u, path_or_url)) {
if (mem.eql(u8, h, &hex_digest) and mem.eql(u8, u, saved_path_or_url)) {
std.log.info("existing dependency named '{s}' is up-to-date", .{name});
process.exit(0);
}
Expand Down