-
-
Notifications
You must be signed in to change notification settings - Fork 336
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this 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 { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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.
fixes #634