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

intentional @compileError limits zig to a very narrow band of posix systems #13565

Closed
jrmarino opened this issue Nov 16, 2022 · 16 comments
Closed
Assignees
Labels
os-dragonfly os-freebsd os-macos os-netbsd os-openbsd standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@jrmarino
Copy link

zig/lib/std/os.zig

Lines 5284 to 5300 in aea617c

.freebsd => {
comptime if (builtin.os.version_range.semver.max.order(.{ .major = 13, .minor = 0 }) == .lt)
@compileError("querying for canonical path of a handle is unsupported on FreeBSD 12 and below");
var kfile: system.kinfo_file = undefined;
kfile.structsize = system.KINFO_FILE_SIZE;
switch (errno(system.fcntl(fd, system.F.KINFO, @ptrToInt(&kfile)))) {
.SUCCESS => {},
.BADF => return error.FileNotFound,
else => |err| return unexpectedErrno(err),
}
const len = mem.indexOfScalar(u8, &kfile.path, 0) orelse MAX_PATH_BYTES;
mem.copy(u8, out_buffer, kfile.path[0..len]);
return out_buffer[0..len];
},
else => @compileError("querying for canonical path of a handle is unsupported on this host"),

so zig 10.0.0 will intentionally does not compile on < FreeBSD 13 and not at all on DragonFly or NetBSD?

There wouldn't be a reason to leave it in Ravenports if that's the case.

@nektro
Copy link
Contributor

nektro commented Nov 16, 2022

.netbsd and .dragonfly get their own OS values respectively

edit: see std.Target.Os.Tag

@leecannon
Copy link
Contributor

Also this compile error is only hit if someone calls getFdPath on < FreeBSD 13, if they don't try to get the path of an fd then no compile error.

@jrmarino
Copy link
Author

I understand the code. The point is that they don't have it now, which means it doesn't build now.
And @leecannon is incorrect. It doesn't compile. I assume that's the point of @compilerror

More to the point, there should be code to fallback on, not a compile error.

What other projects have done in cases like this is return nothing, perhaps with some kind of emitted notice.

Unless zig is intentionally supporting ONLY FreeBSD (and linux, macos, windows) the current state shouldn't be acceptable.

@ghost
Copy link

ghost commented Nov 17, 2022

@compileError only triggers a compile error if it is semantically analyzed. If getFdPath is never called, its body is never semantically analyzed, so the program will compile.

What other projects have done in cases like this is return nothing, perhaps with some kind of emitted notice.

I think this is very much a bad idea and contrary to Zig's values. If you're calling an API that doesn't exist on your platform, you should find out about that as soon as possible. If you really want it to fall back on returning null, you could branch on builtin.os.tag yourself and use null instead of calling the function on FreeBSD < 13. It should be explicit. And Zig does not have warnings, so it's either a compile error or quietly doing something incorrect at runtime.

@jrmarino
Copy link
Author

jrmarino commented Nov 17, 2022

I'm not sure what your point is.
I found it because zig 9 compiles on DragonFly and trying to update to zig 10 failed to compile on DragonFly.
Ravenports is still using FreeBSD 12.2 as the basis of FreeBSD packaged software. I have not attempted to compile it there, but I fully expect it to also fail to compile, because it will be semantically analyzed.

The bottom is at least 2 platforms stopped building between zig 9 and zig 10. I cannot remember if our netbsd packages included zig yet or not, so that could be 3 platforms.

so you are leaving me with 3 choices:

  1. stay at zig 9
  2. move to zig 10 and remove support for dragonfly, freebsd, and netbsd (at least until RP moves to base packages on FreeBSD 13 or later)
  3. just remove the port completely

Obviously I'm not going to try to implement the missing functionality

@nektro
Copy link
Contributor

nektro commented Nov 17, 2022

if you're on dragonfly and zig is detecting it as freebsd, that's a different issue, and a valid one for sure

@mikdusan
Copy link
Member

mikdusan commented Nov 17, 2022

Unfortunately, it seems os.getFdPath() is referenced in the normal course of building zig. I think it's indirect via os.realpath() implementation.

At a quick glance, perhaps on platforms where libc is used, the impl be changed to use libc's realpath() instead of getFdPath() .

EDIT: err wait. It already does that! gonna have to dig a bit further and find out what is causing getFdPath() to be analyzed.

@jrmarino
Copy link
Author

@nektro no it's not a case of dragonfly being identified as freebsd. I was just saying that I was building on Dragonfly, but I expect to hit the same issue on FreeBSD 12.2.

@semarie
Copy link
Contributor

semarie commented Nov 17, 2022

My digging is it comes from realpathAlloc() call from src/Compilation.zig, which call Dir.realpath(), which call Dir.realpathZ(), which call getFdPath()

@squeek502
Copy link
Collaborator

squeek502 commented Nov 17, 2022

Unless zig is intentionally supporting ONLY FreeBSD (and linux, macos, windows) the current state shouldn't be acceptable.

This is an unintentional regression. Looks like it was introduced in 9db2934 via #12540

@mikdusan mikdusan self-assigned this Nov 18, 2022
@mikdusan mikdusan added standard library This issue involves writing Zig code for the standard library. os-macos os-freebsd os-netbsd os-openbsd os-dragonfly labels Nov 18, 2022
@mikdusan
Copy link
Member

mikdusan commented Nov 22, 2022

I have setup quite a few VMs to work this issue and confirmed the following status re: obtaining a path via filehandle and std.getFdPath():

✅ Linux already has an adequate /proc/self implementation.

✅ macOs already has a good getFdPath F_GETPATH implementation; caveat: it currently misses handling too-big pathnames. It is undocumented, but xnu will use E_NOSPC for this condition (tested against Ventura 13.0.1).

✅ Solaris already appears to have a similar /proc/self getFdPath implementation (not tested here).

✅ FreeBSD already has a getFdPath F_KINFO implementation which only works for the most recent 13.1 version. I do have a fallback systctl based implementation which works for < 13.1 (tested against 13.1, 13.0 and 12.3). The fallback solution isn't very efficient (has to loop through each fd for current proc and match).

✅ DragonFly BSD has F_GETPATH support and I have a working implementation tested on latest 6.2.2 and < 6.0.0 is not supported (tested against 6.2.2 and 6.0.1).

✅ OpenBSD has kvm_openfiles() and kvm_getfiles() but they require linking -lkvm for any executable (including zig.exe) that uses std.getFdPath() (tested against 7.2). The kvm interface does seem to work. It's efficiency is about par with FreeBSD < 13.1 solution. We can overcome most of the linking pain with -lkvm thanks to zig's export syntax:

export "kvm" kvm_getfiles(...) ...;
export "kvm" kvm_openfiles(...) ...;

❌ NetBSD has kvm_openfiles() and kvm_getfiles() and like OpenBSD, it requires -lkvm but unfortunately it also requires to run as root (tested against 9.3). This is a show-stopper for that platform. I haven't found another viable solution. Note, that walking a filesystem and matching for inode is not an acceptable solution.

Usage of std.getFdPath() has permeated stage1 and stage2 compilers. In other words, we cannot build a compiler without std.getFdPath() implemented on that platform.

MY CONCLUSION

  • remove zig.exe dependency on std.getFdPath(). zig wants to be portable and already there is a lot of friction and 1 show-stopper to make that happen for BSDs.
  • keep std.getFdPath() and improve docs to indicate platform support

cc: @andrewrk, @daurnimator

@semarie
Copy link
Contributor

semarie commented Nov 23, 2022

Regarding OpenBSD and kvm, you should be able to get some informations about the descriptor (like inode va_fileid and device va_fsid), but the filepath isn't part of that.

@The-King-of-Toasters
Copy link
Contributor

The-King-of-Toasters commented Nov 30, 2022

Since I created this problem (re: FreeBSD impl.), I may be of help for the NetBSD fix. I remember a WIP patch on the FreeBSD phabricator that referenced implemented a NetBSD function that IIRC did what getFdPath does.

Here it is: fcntl(F_GETPATH). Unfortunately it seems it'll arrive with NetBSD 9.4, which is still in development.

@mikdusan
Copy link
Member

Now that I have patch to remove zig compiler dependency on getFdPath(), we can add platforms with bonafide F_GETPATH/similar support and not worry about older versions that don't have it.

Great news about netbsd 9.4 support! Probably about a year away.

@nikkicoon
Copy link
Contributor

NetBSD 10.0 should be out soon, as far as I can guess hopefully not a year from now.

@andrewrk
Copy link
Member

andrewrk commented Jan 2, 2023

The compiler is not supposed to call realpath under any circumstances. I apologize for not noticing this earlier. Problematic commit reverted in 9bcd48e.

@andrewrk andrewrk closed this as completed Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
os-dragonfly os-freebsd os-macos os-netbsd os-openbsd standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

10 participants