Skip to content

Conversation

@bnuuydev
Copy link
Contributor

If there's willingness to maintain a separate branch tracking master, then this should be good progress towards that! If not, I'll continue to maintain this branch, and it'll make the next release of Zig easier to catch up to.

The main breakage is caused by Writergate. There's some weirdness going on to do with file paths and the build system too though, so that needs to be dealt with. Also the new writer and reader interfaces don't provide any error specificity, so we can no longer know why a read or a write failed. I'm not massively happy with this, but it does at least work.

the main breakage here is caused by writergate. there's some weirdness
going on to do with file paths and the build system too though, so that
should be dealt with before merging. also the new writer and reader
don't provide any error specificity, so we don't know why a read or a
write failed now. this should also be considered: perhaps creating a
custom interface is warranted?
@bnuuydev
Copy link
Contributor Author

Haven't tested any of this against the included tests yet, just getting things to at least work for my personal project first.

global_deps_file = try std.fs.cwd().createFile(deps_file_path, .{});

try global_deps_file.?.writer().print(
var writer = global_deps_file.?.writerStreaming(&.{});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This writer has to be explicitly streaming, otherwise use of File.write will overwrite what we've written here. Personally I think that the writer should be stored & shared instead of the file itself. This would allow the buffer to be shared, and removes this risk (and allows us to use the positional writer).

src/Parser.zig Outdated
// else
// "";

const top_path = ""; // TODO what the fuck, the actual issue here needs to be triaged. this workaround fixes things for me for now though.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When using the Build.zig API, dimmer ends up looking for files relative to the generated .dis script, instead of relative to the source root. So for example, if given a file at ./.zig-cache/o/1eb2d1abdd34c6d52258035aa208072d/kernel, and the .dis file is at ./.zig-cache/o/ffa5f2243aee3c8ad3ca07398429ae02/image.dis, then it tries to access ./.zig-cache/o/ffa5f2243aee3c8ad3ca07398429ae02/.zig-cache/o/1eb2d1abdd34c6d52258035aa208072d/kernel instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking further, with zig build --verbose, the generated files are given as relative files:
./.zig-cache/o/36993686b43d105ede9f00b59dbe8bd1/dimmer --deps-file=./.zig-cache/tmp/4e707a3bd9780a4e/image.d --size=34603008 --script=./.zig-cache/o/ffa5f2243aee3c8ad3ca07398429ae02/image.dis --output=./.zig-cache/tmp/4e707a3bd9780a4e/disk.img PATH1=./.zig-cache/o/a06faaf8c35540b30582535b9d4fa305/bootloader.efi PATH2=./.zig-cache/o/1eb2d1abdd34c6d52258035aa208072d/kernel

This would seem like the correct behaviour for the script to have, and the build interface should in-fact be passing absolute paths. Let me know if I'm wrong on this, otherwise I'll make that fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding compile_script.setCwd(script_file.dirname()); to BuildInterface.createDisk fixes the argument passing. However, I get error: FileNotFound being output by zig build, after dimmer has successfully run, but before the run step has completed. It's really weird and there's no error or stack trace to debug with.

we set the cwd of dimmer to the script directory such that the run step passes
the correct relative paths. this caused weirdness in zig's dependency
file handling - zig expects relative paths to be relative to the project
root instead of the cwd of the run step. changing paths to be absolute
is a suitable workaround.
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.

1 participant