Skip to content

Commit 7f691b3

Browse files
authored
Merge pull request #14664 from mlugg/feat/new-module-cli
New module CLI
2 parents 05da5b3 + f94cbab commit 7f691b3

34 files changed

+784
-248
lines changed

CMakeLists.txt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -748,7 +748,8 @@ set(BUILD_ZIG2_ARGS
748748
build-exe src/main.zig -ofmt=c -lc
749749
-OReleaseSmall
750750
--name zig2 -femit-bin="${ZIG2_C_SOURCE}"
751-
--pkg-begin build_options "${ZIG_CONFIG_ZIG_OUT}" --pkg-end
751+
--mod "build_options::${ZIG_CONFIG_ZIG_OUT}"
752+
--deps build_options
752753
-target "${HOST_TARGET_TRIPLE}"
753754
)
754755

@@ -765,7 +766,8 @@ set(BUILD_COMPILER_RT_ARGS
765766
build-obj lib/compiler_rt.zig -ofmt=c
766767
-OReleaseSmall
767768
--name compiler_rt -femit-bin="${ZIG_COMPILER_RT_C_SOURCE}"
768-
--pkg-begin build_options "${ZIG_CONFIG_ZIG_OUT}" --pkg-end
769+
--mod "build_options::${ZIG_CONFIG_ZIG_OUT}"
770+
--deps build_options
769771
-target "${HOST_TARGET_TRIPLE}"
770772
)
771773

ci/x86_64-windows-debug.ps1

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ CheckLastExitCode
8787
-OReleaseSmall `
8888
--name compiler_rt `
8989
-femit-bin="compiler_rt-x86_64-windows-msvc.c" `
90-
--pkg-begin build_options config.zig --pkg-end `
90+
--mod build_options::config.zig `
91+
--deps build_options `
9192
-target x86_64-windows-msvc
9293
CheckLastExitCode
9394

ci/x86_64-windows-release.ps1

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,8 @@ CheckLastExitCode
8787
-OReleaseSmall `
8888
--name compiler_rt `
8989
-femit-bin="compiler_rt-x86_64-windows-msvc.c" `
90-
--pkg-begin build_options config.zig --pkg-end `
90+
--mod build_options::config.zig `
91+
--deps build_options `
9192
-target x86_64-windows-msvc
9293
CheckLastExitCode
9394

lib/std/Build/CompileStep.zig

Lines changed: 107 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -955,7 +955,10 @@ pub fn addFrameworkPath(self: *CompileStep, dir_path: []const u8) void {
955955
/// package's module table using `name`.
956956
pub fn addModule(cs: *CompileStep, name: []const u8, module: *Module) void {
957957
cs.modules.put(cs.builder.dupe(name), module) catch @panic("OOM");
958-
cs.addRecursiveBuildDeps(module);
958+
959+
var done = std.AutoHashMap(*Module, void).init(cs.builder.allocator);
960+
defer done.deinit();
961+
cs.addRecursiveBuildDeps(module, &done) catch @panic("OOM");
959962
}
960963

961964
/// Adds a module to be used with `@import` without exposing it in the current
@@ -969,10 +972,12 @@ pub fn addOptions(cs: *CompileStep, module_name: []const u8, options: *OptionsSt
969972
addModule(cs, module_name, options.createModule());
970973
}
971974

972-
fn addRecursiveBuildDeps(cs: *CompileStep, module: *Module) void {
975+
fn addRecursiveBuildDeps(cs: *CompileStep, module: *Module, done: *std.AutoHashMap(*Module, void)) !void {
976+
if (done.contains(module)) return;
977+
try done.put(module, {});
973978
module.source_file.addStepDependencies(&cs.step);
974979
for (module.dependencies.values()) |dep| {
975-
cs.addRecursiveBuildDeps(dep);
980+
try cs.addRecursiveBuildDeps(dep, done);
976981
}
977982
}
978983

@@ -1031,22 +1036,110 @@ fn linkLibraryOrObject(self: *CompileStep, other: *CompileStep) void {
10311036
fn appendModuleArgs(
10321037
cs: *CompileStep,
10331038
zig_args: *ArrayList([]const u8),
1034-
name: []const u8,
1035-
module: *Module,
10361039
) error{OutOfMemory}!void {
1037-
try zig_args.append("--pkg-begin");
1038-
try zig_args.append(name);
1039-
try zig_args.append(module.builder.pathFromRoot(module.source_file.getPath(module.builder)));
1040+
// First, traverse the whole dependency graph and give every module a unique name, ideally one
1041+
// named after what it's called somewhere in the graph. It will help here to have both a mapping
1042+
// from module to name and a set of all the currently-used names.
1043+
var mod_names = std.AutoHashMap(*Module, []const u8).init(cs.builder.allocator);
1044+
var names = std.StringHashMap(void).init(cs.builder.allocator);
1045+
1046+
var to_name = std.ArrayList(struct {
1047+
name: []const u8,
1048+
mod: *Module,
1049+
}).init(cs.builder.allocator);
1050+
{
1051+
var it = cs.modules.iterator();
1052+
while (it.next()) |kv| {
1053+
// While we're traversing the root dependencies, let's make sure that no module names
1054+
// have colons in them, since the CLI forbids it. We handle this for transitive
1055+
// dependencies further down.
1056+
if (std.mem.indexOfScalar(u8, kv.key_ptr.*, ':') != null) {
1057+
@panic("Module names cannot contain colons");
1058+
}
1059+
try to_name.append(.{
1060+
.name = kv.key_ptr.*,
1061+
.mod = kv.value_ptr.*,
1062+
});
1063+
}
1064+
}
1065+
1066+
while (to_name.popOrNull()) |dep| {
1067+
if (mod_names.contains(dep.mod)) continue;
1068+
1069+
// We'll use this buffer to store the name we decide on
1070+
var buf = try cs.builder.allocator.alloc(u8, dep.name.len + 32);
1071+
// First, try just the exposed dependency name
1072+
std.mem.copy(u8, buf, dep.name);
1073+
var name = buf[0..dep.name.len];
1074+
var n: usize = 0;
1075+
while (names.contains(name)) {
1076+
// If that failed, append an incrementing number to the end
1077+
name = std.fmt.bufPrint(buf, "{s}{}", .{ dep.name, n }) catch unreachable;
1078+
n += 1;
1079+
}
1080+
1081+
try mod_names.put(dep.mod, name);
1082+
try names.put(name, {});
1083+
1084+
var it = dep.mod.dependencies.iterator();
1085+
while (it.next()) |kv| {
1086+
// Same colon-in-name check as above, but for transitive dependencies.
1087+
if (std.mem.indexOfScalar(u8, kv.key_ptr.*, ':') != null) {
1088+
@panic("Module names cannot contain colons");
1089+
}
1090+
try to_name.append(.{
1091+
.name = kv.key_ptr.*,
1092+
.mod = kv.value_ptr.*,
1093+
});
1094+
}
1095+
}
10401096

1097+
// Since the module names given to the CLI are based off of the exposed names, we already know
1098+
// that none of the CLI names have colons in them, so there's no need to check that explicitly.
1099+
1100+
// Every module in the graph is now named; output their definitions
10411101
{
1042-
const keys = module.dependencies.keys();
1043-
for (module.dependencies.values(), 0..) |sub_module, i| {
1044-
const sub_name = keys[i];
1045-
try cs.appendModuleArgs(zig_args, sub_name, sub_module);
1102+
var it = mod_names.iterator();
1103+
while (it.next()) |kv| {
1104+
const mod = kv.key_ptr.*;
1105+
const name = kv.value_ptr.*;
1106+
1107+
const deps_str = try constructDepString(cs.builder.allocator, mod_names, mod.dependencies);
1108+
const src = mod.builder.pathFromRoot(mod.source_file.getPath(mod.builder));
1109+
try zig_args.append("--mod");
1110+
try zig_args.append(try std.fmt.allocPrint(cs.builder.allocator, "{s}:{s}:{s}", .{ name, deps_str, src }));
10461111
}
10471112
}
10481113

1049-
try zig_args.append("--pkg-end");
1114+
// Lastly, output the root dependencies
1115+
const deps_str = try constructDepString(cs.builder.allocator, mod_names, cs.modules);
1116+
if (deps_str.len > 0) {
1117+
try zig_args.append("--deps");
1118+
try zig_args.append(deps_str);
1119+
}
1120+
}
1121+
1122+
fn constructDepString(
1123+
allocator: std.mem.Allocator,
1124+
mod_names: std.AutoHashMap(*Module, []const u8),
1125+
deps: std.StringArrayHashMap(*Module),
1126+
) ![]const u8 {
1127+
var deps_str = std.ArrayList(u8).init(allocator);
1128+
var it = deps.iterator();
1129+
while (it.next()) |kv| {
1130+
const expose = kv.key_ptr.*;
1131+
const name = mod_names.get(kv.value_ptr.*).?;
1132+
if (std.mem.eql(u8, expose, name)) {
1133+
try deps_str.writer().print("{s},", .{name});
1134+
} else {
1135+
try deps_str.writer().print("{s}={s},", .{ expose, name });
1136+
}
1137+
}
1138+
if (deps_str.items.len > 0) {
1139+
return deps_str.items[0 .. deps_str.items.len - 1]; // omit trailing comma
1140+
} else {
1141+
return "";
1142+
}
10501143
}
10511144

10521145
fn make(step: *Step) !void {
@@ -1573,13 +1666,7 @@ fn make(step: *Step) !void {
15731666
try zig_args.append("--test-no-exec");
15741667
}
15751668

1576-
{
1577-
const keys = self.modules.keys();
1578-
for (self.modules.values(), 0..) |module, i| {
1579-
const name = keys[i];
1580-
try self.appendModuleArgs(&zig_args, name, module);
1581-
}
1582-
}
1669+
try self.appendModuleArgs(&zig_args);
15831670

15841671
for (self.include_dirs.items) |include_dir| {
15851672
switch (include_dir) {

src/Autodoc.zig

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -860,17 +860,9 @@ fn walkInstruction(
860860
const str_tok = data[inst_index].str_tok;
861861
var path = str_tok.get(file.zir);
862862

863-
const maybe_other_package: ?*Package = blk: {
864-
if (self.module.main_pkg_is_std and std.mem.eql(u8, path, "std")) {
865-
path = "std";
866-
break :blk self.module.main_pkg;
867-
} else {
868-
break :blk file.pkg.table.get(path);
869-
}
870-
};
871863
// importFile cannot error out since all files
872864
// are already loaded at this point
873-
if (maybe_other_package) |other_package| {
865+
if (file.pkg.table.get(path)) |other_package| {
874866
const result = try self.packages.getOrPut(self.arena, other_package);
875867

876868
// Immediately add this package to the import table of our

0 commit comments

Comments
 (0)