Skip to content

zig build package hashes are missing path names and executable bit for each file #14308

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

Closed
2 tasks
Tracked by #14265
andrewrk opened this issue Jan 14, 2023 · 0 comments
Closed
2 tasks
Tracked by #14265
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. zig build system std.Build, the build runner, `zig build` subcommand, package management
Milestone

Comments

@andrewrk
Copy link
Member

zig/src/Package.zig

Lines 453 to 510 in d813cef

fn computePackageHash(
thread_pool: *ThreadPool,
pkg_dir: fs.IterableDir,
) ![Hash.digest_length]u8 {
const gpa = thread_pool.allocator;
// We'll use an arena allocator for the path name strings since they all
// need to be in memory for sorting.
var arena_instance = std.heap.ArenaAllocator.init(gpa);
defer arena_instance.deinit();
const arena = arena_instance.allocator();
// Collect all files, recursively, then sort.
var all_files = std.ArrayList(*HashedFile).init(gpa);
defer all_files.deinit();
var walker = try pkg_dir.walk(gpa);
defer walker.deinit();
{
// The final hash will be a hash of each file hashed independently. This
// allows hashing in parallel.
var wait_group: WaitGroup = .{};
defer wait_group.wait();
while (try walker.next()) |entry| {
switch (entry.kind) {
.Directory => continue,
.File => {},
else => return error.IllegalFileTypeInPackage,
}
const hashed_file = try arena.create(HashedFile);
hashed_file.* = .{
.path = try arena.dupe(u8, entry.path),
.hash = undefined, // to be populated by the worker
.failure = undefined, // to be populated by the worker
};
wait_group.start();
try thread_pool.spawn(workerHashFile, .{ pkg_dir.dir, hashed_file, &wait_group });
try all_files.append(hashed_file);
}
}
std.sort.sort(*HashedFile, all_files.items, {}, HashedFile.lessThan);
var hasher = Hash.init(.{});
var any_failures = false;
for (all_files.items) |hashed_file| {
hashed_file.failure catch |err| {
any_failures = true;
std.log.err("unable to hash '{s}': {s}", .{ hashed_file.path, @errorName(err) });
};
hasher.update(&hashed_file.hash);
}
if (any_failures) return error.PackageHashUnavailable;
return hasher.finalResult();
}

2 problems to solve:

  • file names are not included in the hash
  • executable bit (true/false) is not included in the hash

This will cause breakage because the hashes will change. It's fine, nobody is using the package manager yet. Go for it. Another breaking change to the hash is #14284.

@andrewrk andrewrk added bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. breaking Implementing this issue could cause existing code to no longer compile or have different behavior. zig build system std.Build, the build runner, `zig build` subcommand, package management labels Jan 14, 2023
@andrewrk andrewrk added this to the 0.11.0 milestone Jan 14, 2023
@mlugg mlugg moved this to Urgent Enhancements in Package Manager Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. bug Observed behavior contradicts documented or intended behavior contributor friendly This issue is limited in scope and/or knowledge of Zig internals. zig build system std.Build, the build runner, `zig build` subcommand, package management
Projects
Archived in project
Development

No branches or pull requests

1 participant