Skip to content

Commit db9c760

Browse files
committed
fix tests on windows
Closes #4 Turns out the harness code which deals with filepaths is pretty brittle. If the fixture path has '\' seperators (default on windows!) it'll break. Fun.
1 parent 1e2afcc commit db9c760

File tree

2 files changed

+44
-21
lines changed

2 files changed

+44
-21
lines changed

ClarTestStep.zig

+1-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ fn make(step: *Step, options: Step.MakeOptions) !void {
6363
var child: std.process.Child = .init(argv_list.items, arena);
6464
child.stdin_behavior = .Ignore;
6565
child.stdout_behavior = .Pipe;
66-
child.stderr_behavior = .Ignore;
66+
child.stderr_behavior = .Inherit;
6767

6868
try child.spawn();
6969

build.zig

+43-20
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
const std = @import("std");
2+
const builtin = @import("builtin");
23

34
pub fn build(b: *std.Build) !void {
45
const target = b.standardTargetOptions(.{});
@@ -22,26 +23,36 @@ pub fn build(b: *std.Build) !void {
2223
.{
2324
.GIT_THREADS = 1,
2425
.GIT_USE_NSEC = 1,
25-
.GIT_RAND_GETLOADAVG = 1,
2626
},
2727
);
2828

29-
// TODO: are there more subtleties for other platforms?
30-
// TODO: do we need iconv on other platforms? original cmake enabled it by
29+
// @Cleanup: all these platform and TLS variants are starting to become unweildly. Compress these codepaths to avoid edgecases.
30+
31+
// @Todo: are there more subtleties for other platforms?
32+
// @Todo: do we need iconv on other platforms? original cmake enabled it by
3133
// default on APPLE targets
32-
if (target.result.os.tag == .macos) {
33-
lib.linkSystemLibrary("iconv");
34-
features.addValues(.{
35-
.GIT_USE_ICONV = 1,
36-
.GIT_USE_STAT_MTIMESPEC = 1,
37-
.GIT_REGEX_REGCOMP_L = 1,
38-
.GIT_QSORT_BSD = 1,
39-
});
40-
} else {
41-
features.addValues(.{
42-
.GIT_USE_STAT_MTIM = 1,
43-
.GIT_RAND_GETENTROPY = 1,
44-
});
34+
switch (target.result.os.tag) {
35+
.macos => {
36+
lib.linkSystemLibrary("iconv");
37+
features.addValues(.{
38+
.GIT_USE_ICONV = 1,
39+
.GIT_USE_STAT_MTIMESPEC = 1,
40+
.GIT_REGEX_REGCOMP_L = 1,
41+
.GIT_QSORT_BSD = 1,
42+
});
43+
},
44+
.windows => {
45+
features.addValues(.{
46+
.GIT_QSORT_MSC = 1,
47+
});
48+
},
49+
else => {
50+
features.addValues(.{
51+
.GIT_USE_STAT_MTIM = 1,
52+
.GIT_RAND_GETENTROPY = 1,
53+
.GIT_RAND_GETLOADAVG = 1,
54+
});
55+
},
4556
}
4657

4758
const flags = [_][]const u8{
@@ -419,8 +430,7 @@ pub fn build(b: *std.Build) !void {
419430

420431
const test_step = b.step("test", "Run core unit tests (requires python)");
421432
{
422-
if (@import("builtin").os.tag != .windows) {
423-
433+
if (builtin.os.tag != .windows) {
424434
// Fix the test fixture file permissions. This is necessary because Zig does
425435
// not respect the execute permission on arbitrary files it extracts from dependencies.
426436
// Since we need those files to have the execute permission set for tests to
@@ -452,7 +462,7 @@ pub fn build(b: *std.Build) !void {
452462
.name = "libgit2_tests",
453463
.root_module = b.createModule(.{
454464
.target = target,
455-
.optimize = .ReleaseSafe,
465+
.optimize = .Debug,
456466
.link_libc = true,
457467
}),
458468
});
@@ -477,7 +487,8 @@ pub fn build(b: *std.Build) !void {
477487
.flags = &.{
478488
b.fmt(
479489
"-DCLAR_FIXTURE_PATH=\"{s}\"",
480-
.{libgit_src.path("tests/resources").getPath2(b, &runner.step)}, // @Cleanup
490+
// clar expects the fixture path to only have posix seperators or else some tests will break on windows
491+
.{try getNormalizedPath(libgit_src.path("tests/resources"), b, &runner.step)},
481492
),
482493
"-DCLAR_TMPDIR=\"libgit2_tests\"",
483494
"-DCLAR_WIN32_LONGPATHS",
@@ -542,6 +553,18 @@ pub fn build(b: *std.Build) !void {
542553
}
543554
}
544555

556+
/// Returns the absolute lazy path with posix seperators
557+
fn getNormalizedPath(lp: std.Build.LazyPath, b: *std.Build, asking_step: *std.Build.Step) ![]const u8 {
558+
const p = lp.getPath3(b, asking_step);
559+
const result = b.pathResolve(&.{ p.root_dir.path orelse ".", p.sub_path });
560+
if (builtin.os.tag == .windows) {
561+
for (result) |*c| {
562+
if (c.* == '\\') c.* = '/';
563+
}
564+
}
565+
return result;
566+
}
567+
545568
pub const TlsBackend = enum { openssl, mbedtls, securetransport };
546569

547570
fn maybeAddTlsIncludes(

0 commit comments

Comments
 (0)