From 21bd13626d66c36c327bb317bd09cad979d92327 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sat, 19 Nov 2022 13:48:32 -0700 Subject: Cache: introduce prefixes to manifests Before, cache manifest files would have absolute file paths. This is problematic for two reasons: * Absolute file paths are not portable. Some operating systems such as WASI have trouble with them. The files themselves are less portable; they cannot be migrated from one user's home directory to another's. And finally they can break due to file paths exceeding maximum path component size. * They would prevent some advanced use cases of Zig, where the lib dir has a different path in a different invocation but is ultimately the same Zig version and lib directory as before. This commit adds a new column that specifies the prefix directory for each file. 0 is an escape hatch and has the previous behavior. The other two prefixes introduced are zig lib directory, and the cache directory. This means files in zig-cache manifests can reference files local to these directories. In practice, this means it is possible to use a different file path for the zig lib directory in a subsequent run of zig and have it still take advantage of the global cache, provided that the files inside remain unchanged. closes #13050 --- src/Compilation.zig | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'src/Compilation.zig') diff --git a/src/Compilation.zig b/src/Compilation.zig index 60064fefd1..2c94785618 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -1456,23 +1456,27 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation { else => @as(u8, 3), }; - // We put everything into the cache hash that *cannot be modified during an incremental update*. - // For example, one cannot change the target between updates, but one can change source files, - // so the target goes into the cache hash, but source files do not. This is so that we can - // find the same binary and incrementally update it even if there are modified source files. - // We do this even if outputting to the current directory because we need somewhere to store - // incremental compilation metadata. + // We put everything into the cache hash that *cannot be modified + // during an incremental update*. For example, one cannot change the + // target between updates, but one can change source files, so the + // target goes into the cache hash, but source files do not. This is so + // that we can find the same binary and incrementally update it even if + // there are modified source files. We do this even if outputting to + // the current directory because we need somewhere to store incremental + // compilation metadata. const cache = try arena.create(Cache); cache.* = .{ .gpa = gpa, .manifest_dir = try options.local_cache_directory.handle.makeOpenPath("h", .{}), }; + cache.addPrefix(.{ .path = null, .handle = fs.cwd() }); + cache.addPrefix(options.zig_lib_directory); + cache.addPrefix(options.local_cache_directory); errdefer cache.manifest_dir.close(); // This is shared hasher state common to zig source and all C source files. cache.hash.addBytes(build_options.version); cache.hash.add(builtin.zig_backend); - cache.hash.addBytes(options.zig_lib_directory.path orelse "."); cache.hash.add(options.optimize_mode); cache.hash.add(options.target.cpu.arch); cache.hash.addBytes(options.target.cpu.model.name); @@ -2265,8 +2269,9 @@ pub fn update(comp: *Compilation) !void { const is_hit = man.hit() catch |err| { // TODO properly bubble these up instead of emitting a warning const i = man.failed_file_index orelse return err; - const file_path = man.files.items[i].path orelse return err; - std.log.warn("{s}: {s}", .{ @errorName(err), file_path }); + const pp = man.files.items[i].prefixed_path orelse return err; + const prefix = man.cache.prefixes()[pp.prefix].path orelse ""; + std.log.warn("{s}: {s}{s}", .{ @errorName(err), prefix, pp.sub_path }); return err; }; if (is_hit) { -- cgit v1.2.3 From 58d3ee2a08f5a1f1c66d5e3b4f215361073c6372 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 20 Nov 2022 15:09:56 -0700 Subject: Compilation: avoid Cache hash dependency on zig lib path * Update for the breaking changes to std.fs.path.resolve. This had a happy side effect of deleting some error handling code which is no longer needed. * Introduce cache_exempt_flags field to CSourceFile. This is used only for include directories when building libc++ and libc++abi which depend only on the zig lib path. * libc_include_dir_list is only added to the cache hash when it contains directories which have been obtained from system probing. It is exempt when the directories depend only on the zig lib path. --- src/Compilation.zig | 22 +++++++++++++--------- src/libcxx.zig | 48 ++++++++++++++++++++++++++++++------------------ 2 files changed, 43 insertions(+), 27 deletions(-) (limited to 'src/Compilation.zig') diff --git a/src/Compilation.zig b/src/Compilation.zig index 2c94785618..795eb493e2 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -201,7 +201,9 @@ pub const CRTFile = struct { /// For passing to a C compiler. pub const CSourceFile = struct { src_path: []const u8, - extra_flags: []const []const u8 = &[0][]const u8{}, + extra_flags: []const []const u8 = &.{}, + /// Same as extra_flags except they are not added to the Cache hash. + cache_exempt_flags: []const []const u8 = &.{}, }; const Job = union(enum) { @@ -3251,13 +3253,6 @@ fn processOneJob(comp: *Compilation, job: Job) !void { const module = comp.bin_file.options.module.?; module.semaPkg(pkg) catch |err| switch (err) { - error.CurrentWorkingDirectoryUnlinked, - error.Unexpected, - => comp.lockAndSetMiscFailure( - .analyze_pkg, - "unexpected problem analyzing package '{s}'", - .{pkg.root_src_path}, - ), error.OutOfMemory => return error.OutOfMemory, error.AnalysisFail => return, }; @@ -3562,7 +3557,14 @@ pub fn obtainCObjectCacheManifest(comp: *const Compilation) Cache.Manifest { man.hash.add(comp.sanitize_c); man.hash.addListOfBytes(comp.clang_argv); man.hash.add(comp.bin_file.options.link_libcpp); - man.hash.addListOfBytes(comp.libc_include_dir_list); + + // When libc_installation is null it means that Zig generated this dir list + // based on the zig library directory alone. The zig lib directory file + // path is purposefully either in the cache or not in the cache. The + // decision should not be overridden here. + if (comp.bin_file.options.libc_installation != null) { + man.hash.addListOfBytes(comp.libc_include_dir_list); + } return man; } @@ -3949,6 +3951,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P { try comp.addCCArgs(arena, &argv, ext, null); try argv.appendSlice(c_object.src.extra_flags); + try argv.appendSlice(c_object.src.cache_exempt_flags); const out_obj_path = if (comp.bin_file.options.emit) |emit| try emit.directory.join(arena, &.{emit.sub_path}) @@ -3990,6 +3993,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P try std.fmt.allocPrint(arena, "{s}.d", .{out_obj_path}); try comp.addCCArgs(arena, &argv, ext, out_dep_path); try argv.appendSlice(c_object.src.extra_flags); + try argv.appendSlice(c_object.src.cache_exempt_flags); try argv.ensureUnusedCapacity(5); switch (comp.clang_preprocessor_mode) { diff --git a/src/libcxx.zig b/src/libcxx.zig index 850da698c5..7ca405cf15 100644 --- a/src/libcxx.zig +++ b/src/libcxx.zig @@ -187,15 +187,6 @@ pub fn buildLibCXX(comp: *Compilation) !void { try cflags.append("-faligned-allocation"); } - try cflags.append("-I"); - try cflags.append(cxx_include_path); - - try cflags.append("-I"); - try cflags.append(cxxabi_include_path); - - try cflags.append("-I"); - try cflags.append(cxx_src_include_path); - if (target_util.supports_fpic(target)) { try cflags.append("-fPIC"); } @@ -203,9 +194,24 @@ pub fn buildLibCXX(comp: *Compilation) !void { try cflags.append("-std=c++20"); try cflags.append("-Wno-user-defined-literals"); + // These depend on only the zig lib directory file path, which is + // purposefully either in the cache or not in the cache. The decision + // should not be overridden here. + var cache_exempt_flags = std.ArrayList([]const u8).init(arena); + + try cache_exempt_flags.append("-I"); + try cache_exempt_flags.append(cxx_include_path); + + try cache_exempt_flags.append("-I"); + try cache_exempt_flags.append(cxxabi_include_path); + + try cache_exempt_flags.append("-I"); + try cache_exempt_flags.append(cxx_src_include_path); + c_source_files.appendAssumeCapacity(.{ .src_path = try comp.zig_lib_directory.join(arena, &[_][]const u8{ "libcxx", cxx_src }), .extra_flags = cflags.items, + .cache_exempt_flags = cache_exempt_flags.items, }); } @@ -340,15 +346,6 @@ pub fn buildLibCXXABI(comp: *Compilation) !void { try cflags.append("-D_LIBCPP_HAS_MUSL_LIBC"); } - try cflags.append("-I"); - try cflags.append(cxxabi_include_path); - - try cflags.append("-I"); - try cflags.append(cxx_include_path); - - try cflags.append("-I"); - try cflags.append(cxx_src_include_path); - if (target_util.supports_fpic(target)) { try cflags.append("-fPIC"); } @@ -357,9 +354,24 @@ pub fn buildLibCXXABI(comp: *Compilation) !void { try cflags.append("-funwind-tables"); try cflags.append("-std=c++20"); + // These depend on only the zig lib directory file path, which is + // purposefully either in the cache or not in the cache. The decision + // should not be overridden here. + var cache_exempt_flags = std.ArrayList([]const u8).init(arena); + + try cache_exempt_flags.append("-I"); + try cache_exempt_flags.append(cxxabi_include_path); + + try cache_exempt_flags.append("-I"); + try cache_exempt_flags.append(cxx_include_path); + + try cache_exempt_flags.append("-I"); + try cache_exempt_flags.append(cxx_src_include_path); + c_source_files.appendAssumeCapacity(.{ .src_path = try comp.zig_lib_directory.join(arena, &[_][]const u8{ "libcxxabi", cxxabi_src }), .extra_flags = cflags.items, + .cache_exempt_flags = cache_exempt_flags.items, }); } -- cgit v1.2.3