[FreeBSD] User's login class ignored

[rhaberkorn]

Hello! I am on FreeBSD 14. Ly version 0.6.0. I was configuring a special login class for my user. Login classes allow customizing resource limits on FreeBSD. An ordinary VT login respects my settings and sets the correct login class for the login shell. Ly does not unfortunately. It appears that login(1) is responsible on VTs for setting the user's login class.

I am happy to build and test and even create a pull request. Some pointers on where to start looking would be appreciated, though.

Best regards!

[rhaberkorn]

Looking through XDM, which does not suffer from this problem, I thought that a call to setlogin(2) might be necessary after setgid(). So I patched it into Ly. Didn't help, though.

Even if I find the solution, the question is whether this should be merged upstream. The current v0.6.0 FreeBSD port contains a bunch of OS-specific patches ontop of Ly. I don't know whether you intend to upstream all of this. Or whether everything BSD-specific should remain in the FreeBSD ports tree instead.

[rhaberkorn]

Yeah, it seems I am also supposed to call setusercontext() instead of setuid().

From XDM:

#ifndef HAVE_SETUSERCONTEXT
        if (setuid(verify->uid) < 0) {
            LogError ("setuid %d (user \"%s\") failed: %s\n",
                      verify->uid, name, _SysErrorMsg (errno));
            return (0);
        }
#else /* HAVE_SETUSERCONTEXT */
        /*
         * Set the user's credentials: uid, gid, groups,
         * environment variables, resource limits, and umask.
         */
        /* destroy user environment before calling setusercontext */
        environ = verify->userEnviron;
        pwd = getpwnam(name);
        if (pwd) {
            if (setusercontext(NULL, pwd, pwd->pw_uid, LOGIN_SETALL) < 0) {
                LogError ("setusercontext for \"%s\" failed: %s\n",
                          name, _SysErrorMsg (errno));
                return (0);
            }
            verify->userEnviron = environ;
            endpwent();
        } else {
            LogError ("getpwnam for \"%s\" failed: %s\n",
                      name, _SysErrorMsg (errno));
            return (0);
        }
#endif /* HAVE_SETUSERCONTEXT */

Will try this next.

[rhaberkorn]

Yes, this worked. So I do have a working patch! I still need to know whether you would like to merge it or whether I should try to get it into the FreeBSD ports tree. As I said earlier, Ly is currently practically not buildable from the Git repository without applying all the FreeBSD patches first.

Is there anybody feeling responsible for this repository at all?

[rhaberkorn]

I officially submitted my patch to the FreeBSD ports tree.

[rhaberkorn]

I noticed that the default PATH from FreeBSD's /etc/login.conf is also currently not applied - in contrast to ttys. If it is supposed to be applied by the display manager at all.

[AnErrupTion]

@rhaberkorn Ly applies its own PATH which can be found in its configuration file.

[rhaberkorn]

@rhaberkorn Ly applies its own PATH which can be found in its configuration file.

Does not appear to work. I am still trying to figure out where my PATH in X11 is actually coming from, so I can extend it.

Is ly supposed to expand ~ in paths? That would be a useful feature, so you can add ~/.local/bin. Also it would be nice if it would be possible to inherit the PATH from login.conf, so we can keep configuration in a single place on FreeBSD.

[AnErrupTion]

Is ly supposed to expand ~ in paths?

It's not supposed to do that, since Ly treats PATH as a big blob of characters and doesn't parse it in any way. It is simply set as an environment variable. I'm pretty sure that's the responsibility of the shell to expand the home directory, but I'm not 100% sure.

Also it would be nice if it would be possible to inherit the PATH from login.conf, so we can keep configuration in a single place on FreeBSD.

The problem with this is it would somewhat break portability, since we'd have to add a feature specific to FreeBSD. But then, if Ly does it, then what would the user expect if Ly's being run on FreeBSD? To source PATH from /etc/login.conf or to use Ly's own PATH? We could add an option to toggle that behavior, but then we'd add an OS-specific option to the configuration file, which isn't something I particularly want to do. But maybe there are better options that I'm not aware of.

[rhaberkorn]

It's not supposed to do that, since Ly treats PATH as a big blob of characters and doesn't parse it in any way. It is simply set as an environment variable. I'm pretty sure that's the responsibility of the shell to expand the home directory, but I'm not 100% sure.

Yes, ~ expansion is not done by the syscalls or their libc wrappers. Apps will have to do this if they want to. It's usually, but not exclusively the shell. FreeBSD for instance expands ~ in /etc/login.conf as well.

But anyway, the path inheriting from ly does not currently work anyway. The only way I found for extending PATH in X11 is via ~/.xsession which I am also using to set other global environment variables (it execs to my window manager, so it inherits the environment).

But maybe there are better options that I'm not aware of.

I think ideally, the default value of the path setting should simply be the PATH inherited from login.conf. So if you comment out that statement in the config, will get the centrally configured PATH, but you can still overwrite it if you wish.

btw. I am still using the heavily-patched v0.6.0 version of ly from FreeBSD ports. We cannot switch to the Zig remake unless you integrate all of the FreeBSD-specific stuff.

[AnErrupTion]

@rhaberkorn So, after quite a bit of thinking, I'm ready to merge as much FreeBSD-specific code as possible. Could you point me to where those patches are, please? That is, if you don't want to make a PR, of course. If you want to instead of me doing it, go ahead!

The only way I found for extending PATH in X11 is via ~/.xsession which I am also using to set other global environment variables (it execs to my window manager, so it inherits the environment).

That's pretty odd. Perhaps PATH somehow gets reset before logging in? This probably nees further investigation.

I think ideally, the default value of the path setting should simply be the PATH inherited from login.conf. So if you comment out that statement in the config, will get the centrally configured PATH,

Sure, but in this case adding a comment above the path option in the config file is a good idea, so the user is aware of this behavior.

but you can still overwrite it if you wish.

Eh? I thought you weren't able to do so?

[rhaberkorn]

Could you point me to where those patches are, please?

https://github.com/freebsd/freebsd-ports/tree/main/x11/ly/files

You will notice that these patches are not organized by feature, but rather by file. Every patch file contains all the changes to one individual source file. Totally idiotic, but that's what most FreeBSD ports do! Since you will have to translate everything to Zig, it shouldn't hurt as much as it would in other cases.

Eh? I thought you weren't able to do so?

That's right. I was just describing how it would ideally work.

[AnErrupTion]

@rhaberkorn I've gone ahead and applied pretty much all of the FreeBSD authentication patches (like switching to utmpx which was done even a bit earlier in the commits) except the VT patch, since I have a few questions. Is there any fundamental difference between VTs and TTYs on FreeBSD? Especially when the default VT value is 9 in the patched config, it makes me wonder how it relates to the TTY (if it does at all).

And, speaking of VTs, PR #623 tries to set the current active VT as the TTY at runtime:

// This will set the current active VT as our TTY.
var vtstat: interop.vt.vt_stat = undefined;
_ = std.c.ioctl(std.c.STDIN_FILENO, interop.VT_GETSTATE, &vtstat);
config.tty = @intCast(vtstat.v_active);

To do this, it includes the linux/vt.h header file. Do you know how well this code would work on FreeBSD, if it does at all? (Of course, given the proper header file is included, if any exists)

The other patches seem to simply be replacing (a lot of) paths. I wonder if I should make the Zig build script be able to patch some specific strings inside resources like the config file, the X/Wayland setup scripts, etc... to accodomate for this use case. This way we could also modify the default TTY ID that's in the config file. One thing struck me though, and that's in the Makefile patch: why is it using a brand new variable PREFIX when DESTDIR exists? The latter was specifically created for this use case, so creating a whole new variable seems a bit counter-intuitive.

Sorry for the huge wall of text, haha! And it's no problem if you can't answer all the questions of course 😄

[rhaberkorn]

Is there any fundamental difference between VTs and TTYs on FreeBSD? Especially when the default VT value is 9 in the patched config, it makes me wonder how it relates to the TTY (if it does at all).

I have no idea why they introduced an additional vt setting. This effectively goes into xorg(). Perhaps just leave it away for the time being and wait until somebody complains and demand we add it again.

To do this, it includes the linux/vt.h header file. Do you know how well this code would work on FreeBSD, if it does at all?

There is no such header or structure in FreeBSD. But I think you could use ttyname(0) as well and parse the result. This should be portable across all UNIX-like operating systems including Linux.

The other patches seem to simply be replacing (a lot of) paths. I wonder if I should make the Zig build script be able to patch some specific strings inside resources

Build systems typically allow that - should allow that. Take for instance Autoconf's --prefix. Makefiles typically use a PREFIX variable. You cannot even assume that all Linux users will want to install stuff directly under /usr.

One thing struck me though, and that's in the Makefile patch: why is it using a brand new variable PREFIX when DESTDIR exists?

That's exactly the PREFIX variable I talked about earlier. Most typically it will be set to /usr/local by default. The patch-makefile doesn't use it consistently, though. It's just a hack, so it works with the ports system. Also, using PREFIX as in the patch, you can no longer install stuff directly under /etc if PREFIX=/usr/local. Consequently, when writing a portable build system, the etc-path should be configurable separately (cf. Autoconf's --sysconfdir).

DESTDIR is something different. Unlike PREFIX, DESTDIR is supposed to influence installation only. So for instance, if you set PREFIX=/usr/local, you expect these paths to be used everywhere, possibly compiled into the binary regardless of the value of DESTDIR. DESTDIR is then prepended only during installation. This way, you can for instance install the app into a temporary staging directory during packaging as in make install DESTDIR=$PWD/temp. temp/ would then contain everything that would normally be installed into the root.

[AnErrupTion]

@rhaberkorn I believe I have implemented pretty much all of the patches, except for the VT one, and one last which I forgot to mention because I'm not quite sure: in the config.ini patch, there's this:

 #path = /sbin:/bin:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin
+path = /sbin:/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/usr/bin/env

It swaps the /usr/bin and /usr/sbin paths around and appends a /usr/bin/env path. For the first one it's understandable since /sbin has a higher "priority" than /bin, but for the second one, I'm not really sure what it is. I know that env is a program (on Linux at least), but maybe it's a directory on FreeBSD? And if it's a program (I saw online that it may be), why is it directly added to $PATH?

[rhaberkorn]

It swaps the /usr/bin and /usr/sbin paths around and appends a /usr/bin/env path. For the first one it's understandable since /sbin has a higher "priority" than /bin, but for the second one, I'm not really sure what it is. I know that env is a program (on Linux at least), but maybe it's a directory on FreeBSD? And if it's a program (I saw online that it may be), why is it directly added to $PATH?

I have no idea. But this is easy to patch in by the FreeBSD maintainer if it indeed serves any purpose.

I will give the current version a try soon.

[AnErrupTion]

It swaps the /usr/bin and /usr/sbin paths around and appends a /usr/bin/env path. For the first one it's understandable since /sbin has a higher "priority" than /bin, but for the second one, I'm not really sure what it is. I know that env is a program (on Linux at least), but maybe it's a directory on FreeBSD? And if it's a program (I saw online that it may be), why is it directly added to $PATH?

I have no idea. But this is easy to patch in by the FreeBSD maintainer if it indeed serves any purpose.

I will give the current version a try soon.

I believe right now it won't build out of the box due to one Linux-specific (or at least I think it is) function, interop.switchTty(). Otherwise everything should be all good.

[rhaberkorn]

Actually there are other problems. Zig v0.12.1 from the FreeBSD ports comes with a C function definition that does not match the system headers (???):

# zig build
install
└─ install ly
   └─ zig build-exe ly Debug native 1 errors
/usr/local/lib/zig/std/c.zig:1924:72: error: root struct of file 'c' has no member named 'timezone'
    extern "c" fn gettimeofday(noalias tv: ?*c.timeval, noalias tz: ?*c.timezone) c_int;
                                                                      ~^~~~~~~~~
/usr/local/lib/zig/std/c.zig:1:1: note: struct declared here
const std = @import("std");
^~~~~
referenced by:
    gettimeofday: /usr/local/lib/zig/std/c.zig:1549:20
    main: src/main.zig:71:14
    remaining reference traces hidden; use '-freference-trace' to see all reference traces
error: the following command failed with 1 compilation errors:
/usr/local/bin/zig build-exe -lpam -I/usr/local/include -L/usr/local/lib -lxcb -ODebug -I /home/rhaberkorn/working-copies/ly/include --dep zigini --dep build_options --dep clap --dep termbox2 -Mroot=/home/rhaberkorn/working-copies/ly/src/main.zig --dep ini -Mzigini=/home/rhaberkorn/.cache/zig/p/12209b971367b4066d40ecad4728e6fdffc4cc4f19356d424c2de57f5b69ac7a619a/src/main.zig -Mbuild_options=/home/rhaberkorn/working-copies/ly/zig-cache/c/1af3e51f873899a1a1119ab51c573410/options.zig -Mclap=/home/rhaberkorn/.cache/zig/p/122062d301a203d003547b414237229b09a7980095061697349f8bef41be9c30266b/clap.zig -Mtermbox2=/home/rhaberkorn/working-copies/ly/zig-cache/o/692f7c65db26c288a255b2727fc499c8/termbox2.zig -Mini=/home/rhaberkorn/.cache/zig/p/1220b0979ea9891fa4aeb85748fc42bc4b24039d9c99a4d65d893fb1c83e921efad8/src/ini.zig -lc --cache-dir /home/rhaberkorn/working-copies/ly/zig-cache --global-cache-dir /home/rhaberkorn/.cache/zig --name ly --listen=-
Build Summary: 2/5 steps succeeded; 1 failed (disable with --summary none)
install transitive failure
└─ install ly transitive failure
   └─ zig build-exe ly Debug native 1 errors
error: the following build command failed with exit code 1:
/home/rhaberkorn/working-copies/ly/zig-cache/o/4078dbb7bcff589477f98070a41b553b/build /usr/local/bin/zig /home/rhaberkorn/working-copies/ly /home/rhaberkorn/working-copies/ly/zig-cache /home/rhaberkorn/.cache/zig --seed 0xf790034d -Zd5e9ac4ed08f6946

Although, I do not see where the problem is with gettimeofday().

And when I try Zig-devel-0.12.0.d.1879+e19219.f0, I get the following error:

# zig build
/home/rhaberkorn/working-copies/ly/build.zig:54:30: error: no field or member function named 'path' in 'Build'
        .root_source_file = b.path("src/main.zig"),
                            ~^~~~~
/usr/local/lib/zig/std/Build.zig:1:1: note: struct declared here
const std = @import("std.zig");
^~~~~
referenced by:
    runBuild__anon_5955: /usr/local/lib/zig/std/Build.zig:1893:37
    steps__anon_5755: /usr/local/lib/zig/build_runner.zig:969:29
    remaining reference traces hidden; use '-freference-trace' to see all reference traces

[AnErrupTion]

Actually there are other problems. Zig v0.12.1 from the FreeBSD ports comes with a C function definition that does not match the system headers (???):

Ah yes. So what's happening here is that, for FreeBSD at least, Zig doesn't define the timezone struct, but it does on Linux. The std.c namespace manually defines some functions and structs only for certain operating systems and not for others. The fix here would be to import the sys/time.h header and to use the functions and structs from there instead.

And when I try Zig-devel-0.12.0.d.1879+e19219.f0, I get the following error:

That version is definitely too old. Ly needs the final version of v0.12.0 or newer.

[AnErrupTion]

I've pushed the fix to master. If you still encounter such errors after that, you can try to manually modify the source code yourself like how I did it in the latest commit until it builds successfully.

[rhaberkorn]

The std.c namespace manually defines some functions and structs only for certain operating systems and not for others.

Why does it do that if it can parse the system headers in the first place? I thought that was a big selling point for Zig?

Now it does that:

# zig build
install
└─ install ly
   └─ zig build-exe ly Debug native 2 errors
src/interop.zig:59:16: error: C import failed
pub const vt = @cImport({
               ^~~~~~~~
referenced by:
    switchTty: src/interop.zig:84:25
    main: src/main.zig:346:12
    remaining reference traces hidden; use '-freference-trace' to see all reference traces
/home/rhaberkorn/working-copies/ly/zig-cache/o/34aa814271f2db20e5b322496d2ced65/cimport.h:1:10: error: 'sys/vt.h' file not found
#include <sys/vt.h>
         ^
error: the following command failed with 2 compilation errors:
/usr/local/bin/zig build-exe -lpam -I/usr/local/include -L/usr/local/lib -lxcb -ODebug -I /home/rhaberkorn/working-copies/ly/include --dep zigini --dep build_options --dep clap --dep termbox2 -Mroot=/home/rhaberkorn/working-copies/ly/src/main.zig --dep ini -Mzigini=/home/rhaberkorn/.cache/zig/p/12209b971367b4066d40ecad4728e6fdffc4cc4f19356d424c2de57f5b69ac7a619a/src/main.zig -Mbuild_options=/home/rhaberkorn/working-copies/ly/zig-cache/c/7297af1b7d884b123be853dcca958df4/options.zig -Mclap=/home/rhaberkorn/.cache/zig/p/122062d301a203d003547b414237229b09a7980095061697349f8bef41be9c30266b/clap.zig -Mtermbox2=/home/rhaberkorn/working-copies/ly/zig-cache/o/692f7c65db26c288a255b2727fc499c8/termbox2.zig -Mini=/home/rhaberkorn/.cache/zig/p/1220b0979ea9891fa4aeb85748fc42bc4b24039d9c99a4d65d893fb1c83e921efad8/src/ini.zig -lc --cache-dir /home/rhaberkorn/working-copies/ly/zig-cache --global-cache-dir /home/rhaberkorn/.cache/zig --name ly --listen=-
Build Summary: 2/5 steps succeeded; 1 failed (disable with --summary none)
install transitive failure
└─ install ly transitive failure
   └─ zig build-exe ly Debug native 2 errors
error: the following build command failed with exit code 1:
/home/rhaberkorn/working-copies/ly/zig-cache/o/4078dbb7bcff589477f98070a41b553b/build /usr/local/bin/zig /home/rhaberkorn/working-copies/ly /home/rhaberkorn/working-copies/ly/zig-cache /home/rhaberkorn/.cache/zig --seed 0x94064adc -Zbd2236f0e7f1e0d4

Shouldn't that @cInclude be conditional somehow?

NOTE: I am still new to Zig, although I wanted to learn it anyway.

[AnErrupTion]

Why does it do that if it can parse the system headers in the first place? I thought that was a big selling point for Zig?

It can do that, but the std.c is slightly different in that it uses extern "C" functions. I assume the reason why they only define some based on the target OS is to detect if some used functions don't exist on that target OS before the linker kicks in.

Shouldn't that @cInclude be conditional somehow?

That's the function I was talking about earlier. Zig does lazy compilation, which means that if a statement isn't used, it won't get compiled in. However, currently the interop.switchTty() that's erroring only works on Linux (as I expected), so it only tries to use the vt.h header. I don't know how switching between TTYs works programatically on FreeBSD, though. I tried looking online but didn't find the information I needed.

[rhaberkorn]

That's the function I was talking about earlier. Zig does lazy compilation, which means that if a statement isn't used, it won't get compiled in.

You say that if the vt namespace is not used, it reliably won't even try to parse the header? Interesting. Yeah, I will try to fix it myself and keep you up to date.

[AnErrupTion]

That's the function I was talking about earlier. Zig does lazy compilation, which means that if a statement isn't used, it won't get compiled in.

You say that if the vt namespace is not used, it reliably won't even try to parse the header? Interesting.

Yup, that's exactly what'll happen. As long as the vt constant, which imports vt.h, isn't actually used by the code with the target OS (i.e. a check to builtin.os.tag is done) then the header won't even be parsed by Zig.

[rhaberkorn]

Sorry, you have to serve as my Zig-mentor, but why the hell it cannot automatically cast *cimport.struct_passwd to ?*const cimport.struct_passwd?

# zig build
install
└─ install ly
   └─ zig build-exe ly Debug native 1 errors
src/auth.zig:137:63: error: expected type '?*const cimport.struct_passwd', found '*cimport.struct_passwd'
        const result = interop.login_cap.setusercontext(null, pwd, pwd.pw_uid, interop.logincap.LOGIN_SETALL);
                                                              ^~~
src/auth.zig:137:63: note: pointer type child 'cimport.struct_passwd' cannot cast into pointer type child 'cimport.struct_passwd'
/home/rhaberkorn/working-copies/ly/zig-cache/o/be285ee4fe9c548b9dd2074b231ef775/cimport.zig:163:34: note: struct declared here
pub const struct_passwd = extern struct {
                          ~~~~~~~^~~~~~
/home/rhaberkorn/working-copies/ly/zig-cache/o/449be036e3ef767812d1bae25b78ba13/cimport.zig:318:27: note: opaque declared here
pub const struct_passwd = opaque {};
                          ^~~~~~~~~
/home/rhaberkorn/working-copies/ly/zig-cache/o/449be036e3ef767812d1bae25b78ba13/cimport.zig:336:47: note: parameter type declared here
pub extern fn setusercontext([*c]login_cap_t, ?*const struct_passwd, uid_t, c_uint) c_int;
                                              ^~~~~~~~~~~~~~~~~~~~~
referenced by:
    authenticate: src/auth.zig:89:9
    main: src/main.zig:689:29
    remaining reference traces hidden; use '-freference-trace' to see all reference traces
error: the following command failed with 1 compilation errors:
/usr/local/bin/zig build-exe -lpam -I/usr/local/include -L/usr/local/lib -lxcb -ODebug -I /home/rhaberkorn/working-copies/ly/include --dep zigini --dep build_options --dep clap --dep termbox2 -Mroot=/home/rhaberkorn/working-copies/ly/src/main.zig --dep ini -Mzigini=/home/rhaberkorn/.cache/zig/p/12209b971367b4066d40ecad4728e6fdffc4cc4f19356d424c2de57f5b69ac7a619a/src/main.zig -Mbuild_options=/home/rhaberkorn/working-copies/ly/zig-cache/c/7297af1b7d884b123be853dcca958df4/options.zig -Mclap=/home/rhaberkorn/.cache/zig/p/122062d301a203d003547b414237229b09a7980095061697349f8bef41be9c30266b/clap.zig -Mtermbox2=/home/rhaberkorn/working-copies/ly/zig-cache/o/692f7c65db26c288a255b2727fc499c8/termbox2.zig -Mini=/home/rhaberkorn/.cache/zig/p/1220b0979ea9891fa4aeb85748fc42bc4b24039d9c99a4d65d893fb1c83e921efad8/src/ini.zig -lc --cache-dir /home/rhaberkorn/working-copies/ly/zig-cache --global-cache-dir /home/rhaberkorn/.cache/zig --name ly --listen=-
Build Summary: 2/5 steps succeeded; 1 failed (disable with --summary none)
install transitive failure
└─ install ly transitive failure
   └─ zig build-exe ly Debug native 1 errors
error: the following build command failed with exit code 1:
/home/rhaberkorn/working-copies/ly/zig-cache/o/4078dbb7bcff589477f98070a41b553b/build /usr/local/bin/zig /home/rhaberkorn/working-copies/ly /home/rhaberkorn/working-copies/ly/zig-cache /home/rhaberkorn/.cache/zig --seed 0x10f49d -Z08b67636b944d817

Even more strangely, I declare a variable like const pwd_opt: ?*const interop.pwd.passwd = pwd; which does cast implicitly, but then get a completely absurd error:

# zig build
install
└─ install ly
   └─ zig build-exe ly Debug native 1 errors
src/auth.zig:138:63: error: expected type '?*const cimport.struct_passwd', found '?*const cimport.struct_passwd'
        const result = interop.login_cap.setusercontext(null, pwd_opt, pwd.pw_uid, interop.logincap.LOGIN_SETALL);
                                                              ^~~~~~~
src/auth.zig:138:63: note: pointer type child 'cimport.struct_passwd' cannot cast into pointer type child 'cimport.struct_passwd'
/home/rhaberkorn/working-copies/ly/zig-cache/o/be285ee4fe9c548b9dd2074b231ef775/cimport.zig:163:34: note: struct declared here
pub const struct_passwd = extern struct {
                          ~~~~~~~^~~~~~
/home/rhaberkorn/working-copies/ly/zig-cache/o/449be036e3ef767812d1bae25b78ba13/cimport.zig:318:27: note: opaque declared here
pub const struct_passwd = opaque {};
                          ^~~~~~~~~
/home/rhaberkorn/working-copies/ly/zig-cache/o/449be036e3ef767812d1bae25b78ba13/cimport.zig:336:47: note: parameter type declared here
pub extern fn setusercontext([*c]login_cap_t, ?*const struct_passwd, uid_t, c_uint) c_int;
                                              ^~~~~~~~~~~~~~~~~~~~~
referenced by:
    authenticate: src/auth.zig:89:9
    main: src/main.zig:689:29
    remaining reference traces hidden; use '-freference-trace' to see all reference traces
error: the following command failed with 1 compilation errors:
/usr/local/bin/zig build-exe -lpam -I/usr/local/include -L/usr/local/lib -lxcb -ODebug -I /home/rhaberkorn/working-copies/ly/include --dep zigini --dep build_options --dep clap --dep termbox2 -Mroot=/home/rhaberkorn/working-copies/ly/src/main.zig --dep ini -Mzigini=/home/rhaberkorn/.cache/zig/p/12209b971367b4066d40ecad4728e6fdffc4cc4f19356d424c2de57f5b69ac7a619a/src/main.zig -Mbuild_options=/home/rhaberkorn/working-copies/ly/zig-cache/c/7297af1b7d884b123be853dcca958df4/options.zig -Mclap=/home/rhaberkorn/.cache/zig/p/122062d301a203d003547b414237229b09a7980095061697349f8bef41be9c30266b/clap.zig -Mtermbox2=/home/rhaberkorn/working-copies/ly/zig-cache/o/692f7c65db26c288a255b2727fc499c8/termbox2.zig -Mini=/home/rhaberkorn/.cache/zig/p/1220b0979ea9891fa4aeb85748fc42bc4b24039d9c99a4d65d893fb1c83e921efad8/src/ini.zig -lc --cache-dir /home/rhaberkorn/working-copies/ly/zig-cache --global-cache-dir /home/rhaberkorn/.cache/zig --name ly --listen=-
Build Summary: 2/5 steps succeeded; 1 failed (disable with --summary none)
install transitive failure
└─ install ly transitive failure
   └─ zig build-exe ly Debug native 1 errors
error: the following build command failed with exit code 1:
/home/rhaberkorn/working-copies/ly/zig-cache/o/4078dbb7bcff589477f98070a41b553b/build /usr/local/bin/zig /home/rhaberkorn/working-copies/ly /home/rhaberkorn/working-copies/ly/zig-cache /home/rhaberkorn/.cache/zig --seed 0x846c11ed -Z81cb1d20f1759580

Is that a Zig bug?

[AnErrupTion]

@rhaberkorn What's happening here is that setusercontext() directly references the passwd struct in its arguments. However, pwd.h is imported (but not included) separately from login_cap.h (as in, it's in a separate import statement). That means the two structs will be defined twice and separately. So, even if both structs are the same, you won't be able to cast them easily (and you shouldn't anyway since pwd.h isn't included alongside login_cap.h, so you get an empty struct). The fix for this is to include both headers in the same import statement, and to make sure to use the passwd struct and associated functions from within that same import statement, like this:

pub const passwd = if (builtin.os.tag == .freebsd) logincap.passwd else pwd.passwd;
pub const endpwent = if (builtin.os.tag == .freebsd) logincap.endpwent else pwd.endpwent;
pub const getpwnam = if (builtin.os.tag == .freebsd) logincap.getpwnam else pwd.getpwnam;

I had also forgotten to use the setusercontext() function from login_cap.h instead. I've pushed all fixes to master so now it should work.

[rhaberkorn]

That means the two structs will be defined twice and separately. So, even if both structs are the same, you won't be able to cast them easily

I see, but that's very unintuitive. IMHO the compiler should at least make clear that he's talking about 2 different types.

I think these problems could also be avoided if you used one big @cImport with conditional @cInclude statements. The Zig documentation also recommends to avoid multiple @cImport statements.

The fix for this is to include both headers in the same import statement, and to make sure to use the passwd struct and associated functions from within that same import statement, like this:

This workaround could also be avoided if you had only one big @cImport.

I've pushed all fixes to master so now it should work.

Not quite, but you will get the remaining changes from me once everything works. :-) It's compiling now, but I will still have to test it.

[AnErrupTion]

I think these problems could also be avoided if you used one big @cImport with conditional @cInclude statements. The Zig documentation also recommends to avoid multiple @cImport statements.

I probably could do that, but I like having multiple import statements since it makes it clearer which headers provide which functions, unlike in C.

[rhaberkorn]

I probably could do that, but I like having multiple import statements since it makes it clearer which headers provide which functions, unlike in C.

Sure, but namespaces won't collide anyway as C programmers keep an eye on that. Perhaps simply move the FreeBSD-specific logincap stuff with a conditional into the pwd-cImport? And that should be documented, as it is not clear for any third person - and probably not even for us in a month - why this is happening.

[AnErrupTion]

That does indeed sound like a better solution, which I've now pushed to master.

[rhaberkorn]

ly still crashes in src/main.c:60, ie. in a defer block. Unfortunately, GDB does not show where this comes from (what caused returning from main). Will have to debug this further.

[AnErrupTion]

ly still crashes in src/main.c:60, ie. in a defer block. Unfortunately, GDB does not show where this comes from (what caused returning from main). Will have to debug this further.

It probably returned from a Zig error, in which case the terminal should give you more information. It'd be interesting to run this in a regular terminal (with no root permissions) to see if it still crashes, and if so, what error it returns.

[rhaberkorn]

Okay I managed to login via the new version. Although I still see ly in the process tree. I believe it should have execed to the setup script or something. Does it no longer do that?

I also still have a weird behavior with my virtual terminals, which is probably related to the vt setting that was patched into the FreeBSD ports' version of Ly.

A few remarks:

• Zig already has --prefix. Shouldn't that be used instead of a custom -Dprefix_directory?
• The prefix directory should be /usr/local by default. That is what you want on most Unix-like systems as you are optimizing your defaults for people building from source. Packaging scripts will overwrite this to /usr.
• It would be great if you placed all the preprocessed files in zig-out as well even before installing into rootfs. This way, it's easier to test without installing into rootfs.

[rhaberkorn]

And logging in with the new Ly worked only manually from one of the VTs. If I try to start it at as the login process, it fails. Unfortunately, the system does not tell me why it's failing, just that getty repeating too quickly on port /dev/ttyv8, sleeping 30 secs. But Ly no longer drops core.

For Ly 0.6, things were as follows: You would have getty with the "Ly" session on ttyv8. getty would exec to Ly according to /etc/gettytab. Ly would then have tty = 9 in its config.ini, but Xorg gets passed vt10 according to the patched-in vt = 10 option in config.ini. When switching to ttyv8 (Ctrl+Alt+F9), I see some log messages apparently from Xorg. My Xorg session is on ttyv9 (Ctrl+Alt+F10).

The missing vt config.ini option shouldn't play any role until you try to log in. So, since we do not even get to the Ly UI, something else must still be wrong.

[AnErrupTion]

Although I still see ly in the process tree. I believe it should have execed to the setup script or something. Does it no longer do that?

I don't think Ly was ever supposed to do that. Right now, what it does is fork() when starting authentication, then fork() again when actually starting the shell, X11 or Wayland session. With X11 however, it forks a bit more (for xauth, the X server & the actual WM/DE respectively). It needs to do that so it can clean up at the end & handle logouts as well.

I also still have a weird behavior with my virtual terminals, which is probably related to the vt setting that was patched into the FreeBSD ports' version of Ly.

I'm assuming that happens when logging inside an X11 session. I noticed that the X server allows you to specify a ttyXX value instead of a vtXX value, perhaps that'd work?

Zig already has --prefix. Shouldn't that be used instead of a custom -Dprefix_directory?

Hm, I wasn't aware of this option being available. It looks like you can also add custom install files, so I may end up using that instead of the custom option before releasing Ly v1.1.0. For now though, it works, so I'll leave it as-is.

The prefix directory should be /usr/local by default. That is what you want on most Unix-like systems as you are optimizing your defaults for people building from source. Packaging scripts will overwrite this to /usr.

The problem with this is that most Linux distributions, as far as I'm aware, don't symlink packaged binaries to /usr/local/bin, so unless you override the prefix directory yourself as well when building from source, it most likely won't work.

It would be great if you placed all the preprocessed files in zig-out as well even before installing into rootfs. This way, it's easier to test without installing into rootfs.

You mean when using zig build installexe alone, or do you also mean when installing everything? Either way, you can still test it if you have the configuration & PAM files already installed beforehand. That's how I test Ly, at least.

For Ly 0.6, things were as follows: You would have getty with the "Ly" session on ttyv8. getty would exec to Ly according to /etc/gettytab. Ly would then have tty = 9 in its config.ini, but Xorg gets passed vt10 according to the patched-in vt = 10 option in config.ini. When switching to ttyv8 (Ctrl+Alt+F9), I see some log messages apparently from Xorg. My Xorg session is on ttyv9 (Ctrl+Alt+F10).

I'd expect Ly to be executed instead of getty, not for getty to execute Ly. Perhaps you can try to disable getty on ttyv8? If it's even possible, that is.

[rhaberkorn]

The problem with this is that most Linux distributions, as far as I'm aware, don't symlink packaged binaries to /usr/local/bin, so unless you override the prefix directory yourself as well when building from source, it most likely won't work.

I don't get that. Both /usr/bin and /usr/local/bin are in PATH by default.

I'd expect Ly to be executed instead of getty, not for getty to execute Ly. Perhaps you can try to disable getty on ttyv8? If it's even possible, that is.

I honestly don't know why they are doing that but I already tried with something like that in /etc/ttys:

ttyv8 "/usr/local/bin/ly" xterm on secure

Didn't make a difference.

You mean when using zig build installexe alone, or do you also mean when installing everything?

I am still confused. It seems that zig build by default calls the "install" target which installs into zig-out since that's the current default value of --prefix - although you have zig-out hardcoded in build.zig anyway. You are currently installing things into rootfs only on zig build installnoconf, installexe etc. Not overwriting config files is usually solved by installing the example config files into the data-dir (/usr/local/share/ly). Users are then supposed to copy these config files manually. Or at the packaging-level, they are installed into /etc but asking users what to do with existing config files (keep, theirs, merge...).

Does the Zig build system not foresee a separate build-stage (cf. make all) as opposed to the installation stage (cf. make install)?

It's a recurring theme. People - especially language designers - underestimate the complexity of build systems and think they are unnecessarily bloated. They then reimplement them, supposedly simplifying things, but end up with idiosyncratic and half-working systems, that won't integrate into existing infrastructure and expectations, need to be patched to be usable etc. If you need to reimplement proper installation behavior from scratch, you would be better off using Autotools, CMake, Meson(?) or just properly written Makefiles.

[rhaberkorn]

Reading through the documentation, I fear that "reinventing the wheel poorly" is exactly what was done here. The "install" target is supposed to create some local directory that includes the complete directory hierarchy. --prefix is like the DESTDIR in this case, so zig build --prefix=FOO corresponds to make install DESTDIR=FOO in other build systems. How to create the proper directory structure with configurable prefixes is left to the user. And how to install this into the rootfs is apparently completely left to the end user!? If you call third-party tools, their location isn't configurable by default etc.

zig build is at best a framework for building a more or less working build system.

[AnErrupTion]

I don't get that. Both /usr/bin and /usr/local/bin are in PATH by default.

What I mean is the paths in the config file. Some, like X, default to /usr/bin/X since the prefix defaults to /usr. Making it default to /usr/local wouldn't work because /usr/local/bin isn't a symlink to /usr/bin AFAIK.

Didn't make a difference.

Does the same thing happen if you try to spawn Ly on ttyv9 instead?

I am still confused. It seems that zig build by default calls the "install" target which installs into zig-out since that's the current default value of --prefix - although you have zig-out hardcoded in build.zig anyway. You are currently installing things into rootfs only on zig build installnoconf, installexe etc.

It technically does the correct thing- it builds the binary and puts it in an out directory, like what you would expect. I mean, you have to put it somewhere, right? But I agree the terminology can be confusing.

Not overwriting config files is usually solved by installing the example config files into the data-dir (/usr/local/share/ly). Users are then supposed to copy these config files manually. Or at the packaging-level, they are installed into /etc but asking users what to do with existing config files (keep, theirs, merge...).

That's actually a pretty fair change I should probably make.

If you need to reimplement proper installation behavior from scratch, you would be better off using Autotools, CMake, Meson(?) or just properly written Makefiles.

I'm personally fine with making a Makefile wrapper over the current build system if it makes packaging easier. And, with the proper custom stages in the Zig build system, we shouldn't have to entirely rely on the Makefile for building.

Reading through the documentation, I fear that "reinventing the wheel poorly" is exactly what was done here. The "install" target is supposed to create some local directory that includes the complete directory hierarchy. --prefix is like the DESTDIR in this case, so zig build --prefix=FOO corresponds to make install DESTDIR=FOO in other build systems. How to create the proper directory structure with configurable prefixes is left to the user. And how to install this into the rootfs is apparently completely left to the end user!? If you call third-party tools, their location isn't configurable by default etc.

I think that's fair, since with Makefiles you also need to handle the install step separately. Technically, since they're much older, there are some standards (like the fact the step is named install, that we have DESTDIR, etc...), but ultimately it's not that different from the Zig build system. At least that's what I think. I think what you're supposed to do is unify the build & install processes, such that when you call zig build (potentially with root permissions as well), it builds the binary & installs it to the chosen destination directory using --prefix. With that said, I do agree with the general concerns this could imply with packaging, and I feel like your concerns should best be raised as an issue in the Zig repository itself. The build system should probably provide a --dest-directory option, only available to users building from source or for package maintainers, and the --prefix option should be exposed to the build system so it can be included in the code if needed.

zig build is at best a framework for building a more or less working build system.

I more or less agree with idea, but let's not forget that Zig is still pre-v1.0.0, so voicing concerns right now is best before it reaches that milestone, IMO.

[rhaberkorn]

What I mean is the paths in the config file. Some, like X, default to /usr/bin/X since the prefix defaults to /usr. Making it default to /usr/local wouldn't work because /usr/local/bin isn't a symlink to /usr/bin AFAIK.

Yes, you are right. And in fact you cannot that easily predict where X lives. On Linux, it will usually be in /usr/bin, unless you built Xorg yourself, while on BSD, everything which is not base system lives under /usr/local. I see several solutions to this: Either make these paths configurable at build time (X is an external tool); or support finding tools in PATH if they don't contain paths, just file names, so x_cmd=X will be a working default; or just don't try to generate a working config.ini - leave this to the packagers to patch.

On Autotools and CMake you could also find these tools wherever they are installed, make them configurable, and use the absolute paths further on.

I think what you're supposed to do is unify the build & install processes, such that when you call zig build (potentially with root permissions as well), it builds the binary & installs it to the chosen destination directory using --prefix.

After thinking about it a bit, I guess they could get away with it. The --prefix should replace your -Ddest_directory, but you will require a number of settings to define the hierarchy below the chosen "prefix". Perhaps instead of -Dprefix_directory, you could have a -Dbin_directory, -Ddata_directory etc, thus avoiding the confusion around the term "prefix".

I more or less agree with idea, but let's not forget that Zig is still pre-v1.0.0, so voicing concerns right now is best before it reaches that milestone, IMO.

Probably true, but it's nothing I want to take care of now. Enough side projects for now. Should I ever start something in Zig from scratch, I will probably get more involved.

[AnErrupTion]

Either make these paths configurable at build time (X is an external tool)

If you mean each one individually, the number of options can get rather bloated with that.

or support finding tools in PATH if they don't contain paths, just file names, so x_cmd=X will be a working default

That's a possible feature, using the path config option to look for the executable. I'll look into it.

or just don't try to generate a working config.ini - leave this to the packagers to patch.

I think the less patches needed downstream, the better.