Projeto-Pindorama / heirloom-ng

A collection of standard Unix utilities that is intended to provide maximum compatibility with traditional Unix while incorporating additional features necessary today.
http://heirloom-ng.pindorama.dob.jp
Other
24 stars 7 forks source link

Unable to compile on OpenBSD 7.2 #16

Closed ghost closed 1 year ago

ghost commented 1 year ago

libcommon compiles and includes dirent.h on OpenBSD, however the function getdirentries(2) has been removed since OpenBSD 5.4 according to the manual pages. It has been replaced by readdir(3), see manual pages for getdirentries and readdir. This causes an "undefined symbol" error at compile time when attempting to use said function on another binary, in this case du.

gmake[1]: Entering directory '/home/hex7c/code/heirloom-ng/du'
cc -static -L/usr/lib -L/usr/ccs/lib -L/usr/local/lib  du.o -L../libcommon -lcommon   -o du
ld: error: undefined symbol: getdirentries
>>> referenced by getdir.c
>>>               getdir.o:(getdir) in archive ../libcommon/libcommon.a
cc: error: linker command failed with exit code 1 (use -v to see invocation)
gmake[1]: *** [Makefile:319: du] Error 1
gmake[1]: Leaving directory '/home/hex7c/code/heirloom-ng/du'
gmake: *** [makefile:23: all] Error 2

I will attempt to look into accommodating for this with ifdefs for post-5.4 OpenBSD machines; however I will not remove the function for pre-5.4 for compatibility with older versions. I will later test other BSDs, and if the same issue arises, I will comment here as well with my findings and look into fixing it.

takusuman commented 1 year ago

G'day, @hex7c! Sorry for the delay (again), I was a little busy yesterday. 😅

I will attempt to look into accommodating for this with ifdefs for post-5.4 OpenBSD machines; however I will not remove the function for pre-5.4 for compatibility with older versions.

I think the solution may be that: use an #ifdef directive that defines getdirentries as a "boilerplate"/"interface" to readdir on OpenBSD versions that are greater than 5.4 (I think that some header in the system must define the OpenBSD major and minor release), but doing this to libcommon, not just to du --- I think you've stated this before by the way, sorry for being redundant, I'm just a little sleepy right now.

takusuman commented 1 year ago

I think I can try to help with some hacking maybe, but I don't know about readdir(3) so well yet.

ghost commented 1 year ago

use an #ifdef directive that defines getdirentries as a "boilerplate"/"interface" to readdir on OpenBSD versions that are greater than 5.4

Good idea! I wasn't sure what to actually do with the ifdefs after defining them, so I'm glad you came up to this. I'll do some reading later today on the manual pages on how exactly readdir works, and try to see if I can come up with something. (Currently I've got my hands full playing around with distcc on 4 Gentoo servers :smile: )

takusuman commented 1 year ago

Good idea!

Oh, thanks. I just was thinking I said something stupid/redundant.

I'll do some reading later today on the manual pages on how exactly readdir works, and try to see if I can come up with something.

Great! I didn't found a lot of information (maybe I'm searching in the wrong way) about how to transition between getdirentries and openddir/readdir, perhaps these Linux manual page can ironically help in this BSD question, since its pretty detailed about the function declaration itself and how its used: https://man7.org/linux/man-pages/man3/getdirentries.3.html https://man7.org/linux/man-pages/man3/readdir.3.html https://man7.org/linux/man-pages/man3/opendir.3.html

(Currently I've got my hands full playing around with distcc on 4 Gentoo servers :smile: )

Well, that's interesting. If you don't mind me asking, are you in an University or something? At least I've never saw this type of infrastructure outside of one.

ghost commented 1 year ago

perhaps these Linux manual page can ironically help in this BSD question

In the initial comment on the issue, I put links to the OpenBSD versions of the manual pages; I'll re-post them just for reference again: The last man page for getdirentries (from 5.4): https://man.openbsd.org/OpenBSD-5.4/getdirentries.2 The current man page for readdir: https://man.openbsd.org/readdir.3

If you don't mind me asking, are you in an University or something?

Nope! I go to both a high school and a career technical center (for sysadmin/netadmin stuff) in the same day, though the Gentoo servers are unrelated to it. The servers are my own I host in my basement, they're pretty fun to play with.

takusuman commented 1 year ago

In the initial comment on the issue, I put links to the OpenBSD versions of the manual pages;

I saw them, but Linux ones are more detailed somehow, that's why I posted it.

I go to both a high school and a career technical center (for sysadmin/netadmin stuff) in the same day, though the Gentoo servers are unrelated to it. The servers are my own I host in my basement, they're pretty fun to play with.

That sounds cool! I'm in the end of my last year in high school by the way. I used to have a "server" in my room, on which I hosted Copacabana and other projects (Heirloom NG didn't existed at the time), but it went down after my ISP blocking the ports, since then I've been using a VPS for hosting the websites.

ghost commented 1 year ago

Oh -- I also forgot to mention: malloc.h can't be found on OpenBSD; it's Linux-specific last I recall, and commenting it out in libcommon/ib_alloc.c compiles libcommon just fine since malloc is defined in stdlib.h on the BSDs (and other systems). This is unrelated to the linking error that the issue is about, though.

ghost commented 1 year ago

I'm thinking to test what OpenBSD version the system is, we can use OpenBSD definition from `<sys/param.h>. It shows the year+month of the release; we can test if the definition has a higher value than the one shown on line 44 of this file from 5.4.

https://github.com/openbsd/src/blob/d3031145007cbcaacf358dd2018498355a088b8f/sys/sys/param.h#L44

takusuman commented 1 year ago

Oh -- I also forgot to mention: malloc.h can't be found on OpenBSD; it's Linux-specific last I recall, and commenting it out in libcommon/ib_alloc.c compiles libcommon just fine since malloc is defined in stdlib.h on the BSDs (and other systems).

So I think we could disable #include malloc.h anyway or it was present in some ancient Linux (or GNU C Library) version? In this case, we could do a preprocessor condition that only includes malloc.h if it's Linux with an C library (be GNU, diet libc or other that included it) older than the one which dropped this header. Just a suggestion, of course.

This is unrelated to the linking error that the issue is about, though.

No problem! It's always interesting to know when we can strip redundant code (even for older systems) and make things better, even if it is a separate issue.

I'm thinking to test what OpenBSD version the system is, we can use OpenBSD definition from `<sys/param.h>. It shows the year+month of the release; we can test if the definition has a higher value than the one shown on line 44 of this file from 5.4.

I think it's the best way. May I can do a sketch here:

#if  defined(__OpenBSD__)
#include <sys/param.h>
#if OpenBSD >= 201311
int 
getdirentries(int fd, char buf[], int nbytes, long basep[]) {
/* TODO: Write boilerplate for opendir() and readdir(). */
}
#endif
#endif
ghost commented 1 year ago

or it was present in some ancient Linux (or GNU C Library) version?

It appears to still be present in glibc, which is likely why it doesn't throw an error whilst compiling using the header. I think we could leave it in for now for Linux systems, and once we get at least OpenBSD working, then we can possibly look into stripping redundant code on modern systems.

May I can do a sketch here:

That's similar to what I was thinking. I've been tired & busy all day (and probably will be tomorrow as well) so I didnt get the chance to do a lot, but this definitely looks good. Apologies for the lack of results, I got a lot of life stuff to juggle.

takusuman commented 1 year ago

Apologies for the lack of results, I got a lot of life stuff to juggle.

It's O.k., implement it when you can/want to. 😃 I hope my sketch can help you a little.

takusuman commented 1 year ago

Good afternoon! I've added the "bug" label to this, just a pure formality.

ghost commented 1 year ago

Good afternoon! I've added the "bug" label to this, just a pure formality.

Alright, that looks good to me. Apologies for not working on this, I've been spending time off on the holidays to work on personal stuff and hang out with friends and family. Hope you understand.

takusuman commented 1 year ago

Good afternoon! I've added the "bug" label to this, just a pure formality.

Alright, that looks good to me. Apologies for not working on this, I've been spending time off on the holidays to work on personal stuff and hang out with friends and family. Hope you understand.

Sure, me too! Merry Christmas and Happy New Year!

takusuman commented 1 year ago

Sorry for bothering again, but I've just added the sketch to libcommon/getdir.c, since the getdirentries() function was actually used there (https://github.com/Projeto-Pindorama/heirloom-ng/blob/master/libcommon/getdir.c#L46-L55).

ghost commented 1 year ago

Hey there! Just wanted to say I'm gonna start working on this again. Thanks for being patient. :slightly_smiling_face:

takusuman commented 1 year ago

Hey there! Just wanted to say I'm gonna start working on this again.

Oh, good. Thanks so much!

Thanks for being patient.

Oh, you're welcome.

mamccollum commented 1 year ago

Hey there! So I wanted to say that I was the original user who created this issue (hex7c) but it ends up something happened to my old GitHub account while I was away & busy with life stuff. I wanted to give an update so hopefully @takusuman and anyone else reading this issue understands what happened. I've been incredibly busy with schoolwork, amongst other things. I'm going to take a look into this issue when time allows; I hope you understand.

mamccollum commented 1 year ago

I believe this bug can be closed as #21 was merged. Note that there are some other OpenBSD platform-specific issues mentioned in the pull request, but those can be for another issue. @takusuman

takusuman commented 1 year ago

I believe this bug can be closed as #21 was merged. Note that there are some other OpenBSD platform-specific issues mentioned in the pull request, but those can be for another issue. @takusuman

Sure.