search

martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 259 forks source link

Use /proc/curproc/file in freebsd when adding directories to package.path #1089

Closed Nomarian closed 10 months ago

Nomarian commented 1 year ago

only activates if using ./configure freebsd.

rnpnr commented 1 year ago

This doesn't seem very portable and I don't think its even guaranteed to work on FreeBSD (procfs is entirely optional as far as I know).

A better interface which is available on all the major BSDs is getprogname(3).

Nomarian commented 1 year ago

It's not portable, that's why __BSD_VISIBLE is used because that's only set on ./configure freebsd

And it does work on freebsd. I used it.

getprogname.3 just prints the program name/filename. but not the filepath, which is what readlink /proc/self/exe does. It's why its used, because when you make vis, you will ./vis, which will use ./lua, Honestly, I would remove the entirety of it and just set VIS_PATH=$PWD/lua ./vis instead or get rid of the entirety of checking for /lua in the build directory and instead would use the equivalent of 

realpath $( dirname $( readlink /proc/self/exe ) )/../share/vis/lua

FWIW IDC if this gets merged, maybe this is better off on the wiki.

rnpnr commented 1 year ago

Sorry I should have said "Not portable between BSDs". If you were fixing it on one I thought it should be fixed on all of them.

But yes now looking at the surrounding code I would also rather it just get removed. I don't see the point of having a specific edge case for running out of the build directory. I don't think anyone is putting a /lua folder next to the binary elsewhere because they probably already have a lua binary there.

Nomarian commented 1 year ago

Sorry I should have said "Not portable between BSDs". If you were fixing it on one I thought it should be fixed on all of them.

That's asking a lot.

Anyway, I guess its good for testing, which is why its necessary... It should be removed and replaced with the whole realpath thing, but that would require a build/ directory, which would require changing make or configure to reflect the install directory.

mcepl commented 10 months ago

So, what is the decision about this PR, will we close it or not?

Nomarian commented 10 months ago

I will close it, its better to put this in freebsd's ports itself but this raises some issues

rnpnr commented 10 months ago

There is no FREEBSD_SOURCE defined add a OPENBSD_SOURCE

I think in general we are against adding #ifdef mess and these defines would definitely be incorrect.

BSD_SOURCE should be defined for all BSDs in configure

No it shouldn't. _BSD_SOURCE is for exposing BSD derived functions such as strlcpy(3) and reallocarray(3) on linux. As far as I know it does nothing on *BSD.

/proc/self/exe is linux only

Yes, this should be removed. A patch removing this behavior which pretty much only exists for running the tests from the directory with the vis source files would be accepted. I think the appropriate solution is setting VIS_PATH before running tests.