dcantrell / bsdutils

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

Port sort(1) #18

Closed q66 closed 3 years ago

q66 commented 3 years ago
q66 commented 3 years ago

caveat: we'll need to find a good way to preprocess the manpage (it's currently not installed)

dcantrell commented 3 years ago

sort(1) across the BSDs is fascinating. So much work and they vary greatly. pthreads here? Ugh.

OK, so everything looks fine in this but I do have the following nits:

  1. glibc has queue.h and that would work fine here. I would prefer to use queue.h, but I think musl is missing it. To avoid carrying a copy of queue.h can we ifdef around the queue.h usage and let that be used for glibc builds but take your approach for musl builds?
  2. The #ifdef Linux stuff confuses me, do we need those preprocessor directives? From my point of view this project is a port from BSD to Linux so everything we're doing is in the name of making these things work on Linux systems.
  3. I am fine with your approach around fgets for wide characters. I'm not sure that's going to work, but I also don't have any reasonable input to try that with. I'm ok going with this and dealing with fallout later.
q66 commented 3 years ago

queue.h is a BSD-ism, glibc carries it for compatibility, i don't really see any point in using it (considering the stuff i replaced it with is extremely trivial, and it's literally what those macros do as well) - ifdef'ing around it would just make it worse

as for linux ifdefs, i felt like keeping it portable, but i can drop them if you want, i don't really care either way

pthreads are optional, but they seem to work so might as well use them (NLS is also optional, and i disabled it because it loads extra stuff and the only translation in the tree is for Hungarian)

q66 commented 3 years ago

i'm also porting tail, btw; gonna submit it tomorrow if i get to finishing it... after that we'll only have stty and timeout left, not quite sure what to do about stty

q66 commented 3 years ago

dealt with the linux bits, added manpage

i think this should be fine now

dcantrell commented 3 years ago

queue.h is a BSD-ism, glibc carries it for compatibility, i don't really see any point in using it (considering the stuff i replaced it with is extremely trivial, and it's literally what those macros do as well) - ifdef'ing around it would just make it worse

I'm aware that queue.h is a BSD-ism. As far as the BSD things go, I kind of like queue.h. But after thinking about this more, the worst part of it is that all of the queue.h implementations vary slightly. I do wish that musl just carried the same one that glibc has for the sake of portability across Linux distributions, but I also don't want to go down the path of carrying queue.h in this tree.

So, TL;DR, what you did is fine here.

as for linux ifdefs, i felt like keeping it portable, but i can drop them if you want, i don't really care either way

Normally I am all in favor of this, but at this point in time for this project I would prefer that we keep it scoped to "this stuff is specifically ported to Linux systems". If there is a strong desire from people to have this portable elsewhere, then we can explore that.

pthreads are optional, but they seem to work so might as well use them (NLS is also optional, and i disabled it because it loads extra stuff and the only translation in the tree is for Hungarian)

I'm fine with it, I was just making an observation that I find it amusing that sort(1) is threaded. Also, agreed on disabling the NLS parts.

Thanks!

dcantrell commented 3 years ago

i'm also porting tail, btw; gonna submit it tomorrow if i get to finishing it... after that we'll only have stty and timeout left, not quite sure what to do about stty

I've got stty ready to go and will probably commit that during my day today. It was mostly a repeat of what I did porting the one from OpenBSD to Linux, but more simple. I've also got timeout partway through, so you can leave that one with me too.

dcantrell commented 3 years ago

dealt with the linux bits, added manpage

i think this should be fine now

Looks good, yeah.

q66 commented 3 years ago

alright, cool; my tail port is complete except for the kqueue bits, which i'm gonna replace with inotify+poll probably during today