-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
std.zig.system.NativeTargetInfo: look for a shebang line in /usr/bin/env, if any #12151
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
Conversation
// reasonably reliable paths to check for. | ||
return result: { | ||
break :result abiAndDynamicLinkerFromPath("/usr/bin/env", cpu, os, ld_info_list, cross_target) catch { | ||
// If we can't parse /usr/bin/env ELF file (for example, if coreutils was builded with --enable-single-binary and /usr/bin/env is actually a script), try /bin/bash |
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.
For example, Gentoo's sys-apps/coreutils
with USE flag multicall
, or Red Hat Universal Base Image 9's coreutils-single
#12156 (comment)
On which system do you observe |
It can be a script instead: #!/usr/bin/coreutils --coreutils-prog-shebang=env or similar. |
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.
Thanks for the info!
Here is my serious suggestion: modify the code to look for a shebang line in /usr/bin/env
. If it finds one, then instead of using /usr/bin/env
as the ELF file to examine, it uses the file it references instead. In the above example, it would therefore examine /usr/bin/coreutils
(doing the same logic recursively in case it finds another script).
I think that this will be more robust than checking for hard-coded /usr/bin/bash
.
I think you're hitting a stage1 compiler bug. This diff seems to get things working again: index c5b9454c9..d9467fe2a 100644
--- a/lib/std/zig/system/NativeTargetInfo.zig
+++ b/lib/std/zig/system/NativeTargetInfo.zig
@@ -390,19 +390,16 @@ fn detectAbiAndDynamicLinker(
else => |e| return e,
};
- var should_close = true;
- defer if (should_close) file.close();
const line = file.reader().readUntilDelimiter(&buffer, '\n') catch {
- should_close = false;
break :blk file;
};
if (mem.startsWith(u8, line, "#!")) {
var it = std.mem.tokenize(u8, line[2..], " ");
file_name = it.next() orelse return defaultAbiAndDynamicLinker(cpu, os, cross_target);
+ file.close();
continue;
} else {
- should_close = false;
break :blk file;
}
} |
Thank you! |
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.
@marler8997 's suggestion is a good one, assuming it doesn't affect upstream error handling
Changes look good though 👍
Thank you for your helpful reviews! :) |
UPD: See #12151 (review)