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

Place build_runner.zig in cache directory #635

Merged
merged 1 commit into from
Sep 7, 2022

Conversation

Techatrix
Copy link
Member

fixes #634

Copy link
Member

@SuperAuguste SuperAuguste left a comment

Choose a reason for hiding this comment

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

LGTM!

@SuperAuguste SuperAuguste merged commit 0428b97 into zigtools:master Sep 7, 2022
@Techatrix Techatrix deleted the cache-build-runner branch September 7, 2022 17:35
Copy link

@LordMZTE LordMZTE left a comment

Choose a reason for hiding this comment

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

Sniped my PR :P

@@ -202,10 +202,13 @@ pub fn configChanged(config: *Config, allocator: std.mem.Allocator, builtin_crea
config.builtin_path = try std.fs.path.join(allocator, &.{ builtin_creation_dir.?, "builtin.zig" });
}

const cache_dir_path = (try known_folders.getPath(allocator, .cache)) orelse {
Copy link

@LordMZTE LordMZTE Sep 7, 2022

Choose a reason for hiding this comment

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

You should append zls to this. It's bad practice to drop stuff straight in ~/.cache.

Also, this is a memory leak since cache_dir_path is never freed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, this is a memory leak since cache_dir_path is never freed.

Yeah, it crops up if you restart a running server

error(gpa): memory address 0x7ff06e67e0d0 leaked: 
zig/lib/std/mem.zig:2182:36: 0x486bdb in std.mem.concatMaybeSentinel (zls)
    const buf = try allocator.alloc(T, total_len);
                                   ^
zig/lib/std/mem.zig:2156:31: 0x37443a in std.mem.concat (zls)
    return concatMaybeSentinel(allocator, T, slices, null);
                              ^
src/known-folders/known-folders.zig:200:38: 0x318943 in .known-folders.getPathXdg (zls)
            return try std.mem.concat(allocator, u8, &[_][]const u8{ home, default[1..] });
                                     ^
src/known-folders/known-folders.zig:129:30: 0x3137f2 in .known-folders.getPath (zls)
            return getPathXdg(allocator, &arena, folder);
                             ^
src/Config.zig:205:54: 0x31e230 in Config.configChanged (zls)
    const cache_dir_path = (try known_folders.getPath(allocator, .cache)) orelse {
                                                     ^
src/Server.zig:2622:36: 0x334f56 in Server.configChanged (zls)
    try server.config.configChanged(server.allocator, builtin_creation_dir);
                                   ^
src/Server.zig:2514:33: 0x32e82e in Server.processJsonRpc (zls)
        try server.configChanged(null);
                                ^
src/main.zig:36:34: 0x30eda1 in loop (zls)
        try server.processJsonRpc(writer, buffer);
                                 ^
src/main.zig:265:13: 0x30a876 in main (zls)
    try loop(&server);
            ^
zig/lib/std/start.zig:578:37: 0x2e08e3 in std.start.posixCallMainAndExit (zls)
            const result = root.main() catch |err| {

@@ -214,10 +217,6 @@ pub fn configChanged(config: *Config, allocator: std.mem.Allocator, builtin_crea
}

if (null == config.global_cache_path) {
Copy link

@LordMZTE LordMZTE Sep 7, 2022

Choose a reason for hiding this comment

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

Might be best to just move this block above the one creating the build runner file instead of using cache_dir_path.
That would fix the user not being able to change where build_runner.zig is placed.

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.

Don't drop build_runner.zig next to executable
4 participants