Skip to content

compiler: resolve framework paths in the frontend #16888

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
Aug 21, 2023
Merged

compiler: resolve framework paths in the frontend #16888

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
e687c87
macho: we no longer need to resolve framework dirs against sysroot
kubkon Aug 20, 2023
4793daf
frontend: move framework path resolution from the linker to frontend
kubkon Aug 20, 2023
b73ef34
frontend: directly pass resolved frameworks container to the linker
kubkon Aug 21, 2023
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
10 changes: 5 additions & 5 deletions src/Compilation.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ pub const InitOptions = struct {
c_source_files: []const CSourceFile = &[0]CSourceFile{},
link_objects: []LinkObject = &[0]LinkObject{},
framework_dirs: []const []const u8 = &[0][]const u8{},
frameworks: std.StringArrayHashMapUnmanaged(Framework) = .{},
frameworks: []const Framework = &.{},
system_lib_names: []const []const u8 = &.{},
system_lib_infos: []const SystemLib = &.{},
/// These correspond to the WASI libc emulated subcomponents including:
Expand Down Expand Up @@ -830,7 +830,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
// Our linker can't handle objects or most advanced options yet.
if (options.link_objects.len != 0 or
options.c_source_files.len != 0 or
options.frameworks.count() != 0 or
options.frameworks.len != 0 or
options.system_lib_names.len != 0 or
options.link_libc or options.link_libcpp or
link_eh_frame_hdr or
Expand Down Expand Up @@ -2267,7 +2267,7 @@ fn prepareWholeEmitSubPath(arena: Allocator, opt_emit: ?EmitLoc) error{OutOfMemo
/// to remind the programmer to update multiple related pieces of code that
/// are in different locations. Bump this number when adding or deleting
/// anything from the link cache manifest.
pub const link_hash_implementation_version = 9;
pub const link_hash_implementation_version = 10;

fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifest) !void {
const gpa = comp.gpa;
Expand All @@ -2277,7 +2277,7 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
defer arena_allocator.deinit();
const arena = arena_allocator.allocator();

comptime assert(link_hash_implementation_version == 9);
comptime assert(link_hash_implementation_version == 10);

if (comp.bin_file.options.module) |mod| {
const main_zig_file = try mod.main_pkg.root_src_directory.join(arena, &[_][]const u8{
Expand Down Expand Up @@ -2386,7 +2386,7 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes

// Mach-O specific stuff
man.hash.addListOfBytes(comp.bin_file.options.framework_dirs);
link.hashAddFrameworks(&man.hash, comp.bin_file.options.frameworks);
try link.hashAddFrameworks(man, comp.bin_file.options.frameworks);
try man.addOptionalFile(comp.bin_file.options.entitlements);
man.hash.addOptional(comp.bin_file.options.pagezero_size);
man.hash.addOptional(comp.bin_file.options.headerpad_size);
Expand Down
19 changes: 7 additions & 12 deletions src/link.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ pub const SystemLib = struct {
pub const Framework = struct {
needed: bool = false,
weak: bool = false,
path: []const u8,
};

pub const SortSection = enum { name, alignment };
Expand All @@ -56,15 +57,11 @@ pub fn hashAddSystemLibs(
}
}

pub fn hashAddFrameworks(
hh: *Cache.HashHelper,
hm: std.StringArrayHashMapUnmanaged(Framework),
) void {
const keys = hm.keys();
hh.addListOfBytes(keys);
for (hm.values()) |value| {
hh.add(value.needed);
hh.add(value.weak);
pub fn hashAddFrameworks(man: *Cache.Manifest, hm: []const Framework) !void {
for (hm) |value| {
man.hash.add(value.needed);
man.hash.add(value.weak);
_ = try man.addFile(value.path, null);
}
}

Expand Down Expand Up @@ -208,7 +205,7 @@ pub const Options = struct {

objects: []Compilation.LinkObject,
framework_dirs: []const []const u8,
frameworks: std.StringArrayHashMapUnmanaged(Framework),
frameworks: []const Framework,
/// These are *always* dynamically linked. Static libraries will be
/// provided as positional arguments.
system_libs: std.StringArrayHashMapUnmanaged(SystemLib),
Expand Down Expand Up @@ -276,7 +273,6 @@ pub const Options = struct {

pub fn move(self: *Options) Options {
const copied_state = self.*;
self.frameworks = .{};
self.system_libs = .{};
self.force_undefined_symbols = .{};
return copied_state;
Expand Down Expand Up @@ -642,7 +638,6 @@ pub const File = struct {
base.releaseLock();
if (base.file) |f| f.close();
if (base.intermediary_basename) |sub_path| base.allocator.free(sub_path);
base.options.frameworks.deinit(base.allocator);
base.options.system_libs.deinit(base.allocator);
base.options.force_undefined_symbols.deinit(base.allocator);
switch (base.tag) {
Expand Down
2 changes: 1 addition & 1 deletion src/link/Coff/lld.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub fn linkWithLLD(self: *Coff, comp: *Compilation, prog_node: *std.Progress.Nod
man = comp.cache_parent.obtain();
self.base.releaseLock();

comptime assert(Compilation.link_hash_implementation_version == 9);
comptime assert(Compilation.link_hash_implementation_version == 10);

for (self.base.options.objects) |obj| {
_ = try man.addFile(obj.path, null);
Expand Down
2 changes: 1 addition & 1 deletion src/link/Elf.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1367,7 +1367,7 @@ fn linkWithLLD(self: *Elf, comp: *Compilation, prog_node: *std.Progress.Node) !v
// We are about to obtain this lock, so here we give other processes a chance first.
self.base.releaseLock();

comptime assert(Compilation.link_hash_implementation_version == 9);
comptime assert(Compilation.link_hash_implementation_version == 10);

try man.addOptionalFile(self.base.options.linker_script);
try man.addOptionalFile(self.base.options.version_script);
Expand Down
64 changes: 1 addition & 63 deletions src/link/MachO.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -870,49 +870,7 @@ fn resolveLibSystemInDirs(arena: Allocator, dirs: []const []const u8, out_libs:
return false;
}

pub fn resolveSearchDir(
arena: Allocator,
dir: []const u8,
syslibroot: ?[]const u8,
) !?[]const u8 {
var candidates = std.ArrayList([]const u8).init(arena);

if (fs.path.isAbsolute(dir)) {
if (syslibroot) |root| {
const common_dir = if (builtin.os.tag == .windows) blk: {
// We need to check for disk designator and strip it out from dir path so
// that we can concat dir with syslibroot.
// TODO we should backport this mechanism to 'MachO.Dylib.parseDependentLibs()'
const disk_designator = fs.path.diskDesignatorWindows(dir);

if (mem.indexOf(u8, dir, disk_designator)) |where| {
break :blk dir[where + disk_designator.len ..];
}

break :blk dir;
} else dir;
const full_path = try fs.path.join(arena, &[_][]const u8{ root, common_dir });
try candidates.append(full_path);
}
}

try candidates.append(dir);

for (candidates.items) |candidate| {
// Verify that search path actually exists
var tmp = fs.cwd().openDir(candidate, .{}) catch |err| switch (err) {
error.FileNotFound => continue,
else => |e| return e,
};
defer tmp.close();

return candidate;
}

return null;
}

pub fn resolveLib(
fn resolveLib(
arena: Allocator,
search_dir: []const u8,
name: []const u8,
Expand All @@ -931,26 +889,6 @@ pub fn resolveLib(
return full_path;
}

pub fn resolveFramework(
arena: Allocator,
search_dir: []const u8,
name: []const u8,
ext: []const u8,
) !?[]const u8 {
const search_name = try std.fmt.allocPrint(arena, "{s}{s}", .{ name, ext });
const prefix_path = try std.fmt.allocPrint(arena, "{s}.framework", .{name});
const full_path = try fs.path.join(arena, &[_][]const u8{ search_dir, prefix_path, search_name });

// Check if the file exists.
const tmp = fs.cwd().openFile(full_path, .{}) catch |err| switch (err) {
error.FileNotFound => return null,
else => |e| return e,
};
defer tmp.close();

return full_path;
}

const ParseDylibError = error{
OutOfMemory,
EmptyStubFile,
Expand Down
70 changes: 17 additions & 53 deletions src/link/MachO/zld.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3396,7 +3396,7 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
// We are about to obtain this lock, so here we give other processes a chance first.
macho_file.base.releaseLock();

comptime assert(Compilation.link_hash_implementation_version == 9);
comptime assert(Compilation.link_hash_implementation_version == 10);

for (options.objects) |obj| {
_ = try man.addFile(obj.path, null);
Expand All @@ -3417,7 +3417,7 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
man.hash.add(options.strip);
man.hash.addListOfBytes(options.lib_dirs);
man.hash.addListOfBytes(options.framework_dirs);
link.hashAddFrameworks(&man.hash, options.frameworks);
try link.hashAddFrameworks(&man, options.frameworks);
man.hash.addListOfBytes(options.rpath_list);
if (is_dyn_lib) {
man.hash.addOptionalBytes(options.install_name);
Expand Down Expand Up @@ -3512,9 +3512,6 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
try zld.atoms.append(gpa, Atom.empty); // AtomIndex at 0 is reserved as null atom
try zld.strtab.buffer.append(gpa, 0);

var lib_not_found = false;
var framework_not_found = false;

// Positional arguments to the linker such as object files and static archives.
var positionals = std.ArrayList([]const u8).init(arena);
try positionals.ensureUnusedCapacity(options.objects.len);
Expand Down Expand Up @@ -3557,43 +3554,16 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
for (vals) |v| libs.putAssumeCapacity(v.path.?, v);
}

try MachO.resolveLibSystem(arena, comp, options.sysroot, target, options.lib_dirs, &libs);

// frameworks
var framework_dirs = std.ArrayList([]const u8).init(arena);
for (options.framework_dirs) |dir| {
if (try MachO.resolveSearchDir(arena, dir, options.sysroot)) |search_dir| {
try framework_dirs.append(search_dir);
} else {
log.warn("directory not found for '-F{s}'", .{dir});
}
}

outer: for (options.frameworks.keys()) |f_name| {
for (framework_dirs.items) |dir| {
for (&[_][]const u8{ ".tbd", ".dylib", "" }) |ext| {
if (try MachO.resolveFramework(arena, dir, f_name, ext)) |full_path| {
const info = options.frameworks.get(f_name).?;
try libs.put(full_path, .{
.needed = info.needed,
.weak = info.weak,
.path = full_path,
});
continue :outer;
}
}
} else {
log.warn("framework not found for '-framework {s}'", .{f_name});
framework_not_found = true;
}
{
try libs.ensureUnusedCapacity(options.frameworks.len);
for (options.frameworks) |v| libs.putAssumeCapacity(v.path, .{
.needed = v.needed,
.weak = v.weak,
.path = v.path,
});
}

if (framework_not_found) {
log.warn("Framework search paths:", .{});
for (framework_dirs.items) |dir| {
log.warn(" {s}", .{dir});
}
}
try MachO.resolveLibSystem(arena, comp, options.sysroot, target, options.lib_dirs, &libs);

if (options.verbose_link) {
var argv = std.ArrayList([]const u8).init(arena);
Expand Down Expand Up @@ -3693,14 +3663,14 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
try argv.append(try std.fmt.allocPrint(arena, "-L{s}", .{lib_dir}));
}

for (options.frameworks.keys()) |framework| {
const info = options.frameworks.get(framework).?;
const arg = if (info.needed)
try std.fmt.allocPrint(arena, "-needed_framework {s}", .{framework})
else if (info.weak)
try std.fmt.allocPrint(arena, "-weak_framework {s}", .{framework})
for (options.frameworks) |framework| {
const name = std.fs.path.stem(framework.path);
const arg = if (framework.needed)
try std.fmt.allocPrint(arena, "-needed_framework {s}", .{name})
else if (framework.weak)
try std.fmt.allocPrint(arena, "-weak_framework {s}", .{name})
else
try std.fmt.allocPrint(arena, "-framework {s}", .{framework});
try std.fmt.allocPrint(arena, "-framework {s}", .{name});
try argv.append(arg);
}

Expand Down Expand Up @@ -3740,12 +3710,6 @@ pub fn linkWithZld(macho_file: *MachO, comp: *Compilation, prog_node: *std.Progr
if (resolver.unresolved.count() > 0) {
return error.UndefinedSymbolReference;
}
if (lib_not_found) {
return error.LibraryNotFound;
}
if (framework_not_found) {
return error.FrameworkNotFound;
}

if (options.output_mode == .Exe) {
const entry_name = options.entry orelse load_commands.default_entry_point;
Expand Down
4 changes: 2 additions & 2 deletions src/link/Wasm.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3193,7 +3193,7 @@ fn linkWithZld(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) l
// We are about to obtain this lock, so here we give other processes a chance first.
wasm.base.releaseLock();

comptime assert(Compilation.link_hash_implementation_version == 9);
comptime assert(Compilation.link_hash_implementation_version == 10);

for (options.objects) |obj| {
_ = try man.addFile(obj.path, null);
Expand Down Expand Up @@ -4254,7 +4254,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
// We are about to obtain this lock, so here we give other processes a chance first.
wasm.base.releaseLock();

comptime assert(Compilation.link_hash_implementation_version == 9);
comptime assert(Compilation.link_hash_implementation_version == 10);

for (wasm.base.options.objects) |obj| {
_ = try man.addFile(obj.path, null);
Expand Down
Loading