dcantrell / bsdutils

Alternative to GNU coreutils using software from FreeBSD
Other
169 stars 9 forks source link

Removed libxo dependency (df and wc) #43

Open Connor-GH opened 1 year ago

Connor-GH commented 1 year ago

This Pull Request removes the libxo dependency from wc and df, allowing them both to build without a dependency most users will not have. This actually maintains the FreeBSD codebase, as historically these were the implementations before patches were introduced alongside the libxo library (https://github.com/Juniper/libxo/tree/a46bb3585fb7cbbd4add46e8af99763531bfda4c/patches).

The main motivation behind this change was mostly to revert the libxo dependency, as explained above. I would have just dealt with the dependency, but it is not in the Gentoo repos and I couldn't get it to build.

dcantrell commented 1 year ago

I don't follow here. How does this maintain the FreeBSD code base? I'm importing from releases of FreeBSD as they come out and libxo functionality is present.

libxo is easy to build on linux, but you need to remove '#include <sys/sysctl.h>' from libxo/xo_syslog.c

I am not opposed to making libxo an optional build time dependency. Variadic macros could be provided via a new header for the existing xo_* usage. I am trying to think minimal changes to the main source files from FreeBSD.

Connor-GH commented 1 year ago

The initial rationale was that you have removed features in the past, but a quick grep "removed" DIFFERENCES shows that it was because of OS incompatabilities.

How does this maintain the FreeBSD code base?

Older versions of wc did not use libxo (and had this exact same logic): https://cgit.freebsd.org/src/tree/usr.bin/wc/wc.c?h=stable/10 Same thing applies with df: https://cgit.freebsd.org/src/tree/bin/df/df.c?h=stable/10

It can be said that dropping this dependency (at least as part of a compile-time switch) maintains the codebase because it restores the exact behavior of the standard switch arguments before the libxo ones were added. I can see why you disagree with this immediate PR (especially when considering that the man pages haven't been touched) because it was honestly more of a proof of concept more than anything and I did not make that clear; I apologize for that.

If you are open to this idea, I can work on a compile-time switch for this. It would mean, however, that either a separate file or patch will have to be maintained for these two programs and I imagine extra work is not welcome.

I will provide the patches for the code. In the meantime, I would like to hear your opinion on what should be done with the man pages; how shall they be altered when the switch is activated?

Edit: Patches would be irreversible for users that may want to compile multiple ways; it would probably be better to have two files for each program: src/wc/wc.c and src/wc/wc-nolibxo.c etc. The alternative is an uncomfortable amount of #ifdef LIBXOs in your file.

dcantrell commented 1 year ago

The initial rationale was that you have removed features in the past, but a quick grep "removed" DIFFERENCES shows that it was because of OS incompatabilities.

Right, that was kind of what I was going for. Remove the things that are just not available or implemented on Linux. While libxo is annoying, it's not something prohibited from Linux.

How does this maintain the FreeBSD code base?

Older versions of wc did not use libxo (and had this exact same logic): https://cgit.freebsd.org/src/tree/usr.bin/wc/wc.c?h=stable/10 Same thing applies with df: https://cgit.freebsd.org/src/tree/bin/df/df.c?h=stable/10

Do you know the history of the introduction of libxo support to these commands? I'm just curious. I do remember being surprised that these commands grew libxo use and that sort of went away from what I was aiming to do and make smaller alternatives to GNU coreutils on Linux.

It can be said that dropping this dependency (at least as part of a compile-time switch) maintains the codebase because it restores the exact behavior of the standard switch arguments before the libxo ones were added. I can see why you disagree with this immediate PR (especially when considering that the man pages haven't been touched) because it was honestly more of a proof of concept more than anything and I did not make that clear; I apologize for that.

If you are open to this idea, I can work on a compile-time switch for this. It would mean, however, that either a separate file or patch will have to be maintained for these two programs and I imagine extra work is not welcome.

I will provide the patches for the code. In the meantime, I would like to hear your opinion on what should be done with the man pages; how shall they be altered when the switch is activated?

Edit: Patches would be irreversible for users that may want to compile multiple ways; it would probably be better to have two files for each program: src/wc/wc.c and src/wc/wc-nolibxo.c etc. The alternative is an uncomfortable amount of #ifdef LIBXOs in your file.

I would like to be able to keep this project going forward with FreeBSD releases such that new releases can be imported here and patches applied a new release made. These commands should not change that often, but that won't be zero change.

I like the idea of a compile-time switch to enable libxo support. We can make libxo support off by default and if users want it, enable it at build time. If that means this project carries two source files for wc and df to avoid ifdefs, I don't think that's a big deal.

Connor-GH commented 1 year ago

I made two separate directories for both df and wc for the libxo change. Libxo is off by default as you requested. Type make and it works. If you want libxo, you enable it in meson_options.txt.