Skip to content
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

Updates and cleanup in self hosted compiler #3748

Merged
merged 20 commits into from
Nov 24, 2019

Conversation

Vexu
Copy link
Member

@Vexu Vexu commented Nov 23, 2019

Changes

  • use enum literals
  • remove await (async .. catch unreachable) pattern
  • cleanup target.zig into util.zig and use std.Target
  • update use of std.event

TODO after this

  • update std.event.fs
  • review and update use of async functions

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

Due to #3190 this patch is needed to compile.

Can you go ahead and make this workaround, with a TODO comment linking to that issue, to remind us to remove the workaround when the issue is fixed?

@andrewrk
Copy link
Member

Does this get it building again successfully? If so we could re-enable building it as part of the test suite:

zig/build.zig

Lines 75 to 78 in 29d7b5a

if (!skip_self_hosted) {
// TODO re-enable this after https://github.com/ziglang/zig/issues/2377
//test_step.dependOn(&exe.step);
}

@Vexu
Copy link
Member Author

Vexu commented Nov 23, 2019

Does this get it building again successfully? If so we could re-enable building it as part of the test suite:

zig/build.zig

Lines 75 to 78 in 29d7b5a

if (!skip_self_hosted) {
// TODO re-enable this after https://github.com/ziglang/zig/issues/2377
//test_step.dependOn(&exe.step);
}

It should.

@Vexu
Copy link
Member Author

Vexu commented Nov 23, 2019

It should.

Apparently only on Linux. Can someone on Mac or Windows give detailed error logs?

@andrewrk
Copy link
Member

Apparently only on Linux. Can someone on Mac or Windows give detailed error logs?

I can help you with that in a bit. If you click on the "Details" link in the CI above, do you see the error? It should be open access with no login required.

@andrewrk
Copy link
Member

Oh, I can tell you what's going on here though. Evented I/O mode integration isn't hooked up with non-Linux operating systems yet. Specifically, the waitUntilFdIsReadable and related functions don't have non-Linux implementations yet.

It would still be helpful to enable building the self-hosted compiler as part of the test suite when the target is Linux though. That's still more test coverage than before, as we work towards full coverage and full support.

build.zig Outdated Show resolved Hide resolved
@andrewrk
Copy link
Member

Oh, nice you even got the stage2 tests passing again. I'm guessing for the Windows failure, this code will need to chop off the extra .lib in the library names that we get from llvm-config:

zig/build.zig

Lines 229 to 242 in 29d7b5a

{
var it = mem.tokenize(libs_output, " \r\n");
while (it.next()) |lib_arg| {
if (mem.startsWith(u8, lib_arg, "-l")) {
try result.system_libs.append(lib_arg[2..]);
} else {
if (fs.path.isAbsolute(lib_arg)) {
try result.libs.append(lib_arg);
} else {
try result.system_libs.append(lib_arg);
}
}
}
}

I'd be happy to help with this; I have a Windows machine sitting right over here.

if (false) {
test_step.dependOn(test_stage2_step);
}
test_step.dependOn(test_stage2_step);
Copy link
Member

@andrewrk andrewrk Nov 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the stage2 tests are ready to be resurrected yet, if you look at src-self-hosted/test.zig, it creates its own event loop, and tries to do evented I/O, but zig test doesn't support that yet, so it's this weird mixture of evented and blocking (which amazingly apparently did work successfully on Linux and macOS (or maybe not, I see "stage2...SKIP" in the log output)

@Vexu Vexu force-pushed the modernize-stage2 branch 2 times, most recently from 5e0132d to 9613bd7 Compare November 24, 2019 08:11
@andrewrk andrewrk merged commit 2b1faa1 into ziglang:master Nov 24, 2019
@Vexu Vexu deleted the modernize-stage2 branch November 24, 2019 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants