Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make lite working on FreeBSD #127

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

DennisCGc
Copy link

This PR makes lite working on FreeBSD, by fixing up build.sh and add proper path detection support on FreeBSD.

Note that I use realpath on FreeBSD. realpath is also available under Linux, but only when _DEFAULT_SOURCE is defined. I'm not going to change path detection on Linux, as it seems to work and you probably had a good reason to use /proc directly.

I didn't change build_release.sh, as I'm not using it at all. I leave it up to you to change it if you so desire.

@DennisCGc
Copy link
Author

@rxi Hi, I've just updated this PR as there was a conflict in src/main.c

I was wondering if you had any remarks on this patch. How can I make sure this gets merged? Is it something you want to merge?

Thanks.

@rxi
Copy link
Owner

rxi commented Jun 8, 2020

Sorry about the delay! I haven't had a chance to grab this and look through it properly, from an initial skim I was a bit confused by use of the restrict keyword; It feels in minor conflict with the existing code base as it isn't used throughout -- if it isn't essential it might be better if it were removed.

Overall I have a little bit of scepticism as this feels like it closely mimics the original path detection lite used (you'll have to look back at the initial commit) which unfortunately didn't work properly, (I believe, if you launched lite from a shell script -- I may be remembering this wrong) though I acknowledge things may be different on BSD. Otherwise BSD support isn't a feature I'm opposed to, I just have to find the time to test the changes myself.

@DennisCGc
Copy link
Author

I've removed the usage of restrict and made it more in line with your code style. It wasn't essential. Thanks.

As for the usage of realpath in the past, I've looked at ba86b58 and from what I can tell ARGS[1] was used as is; no realpath lookup was performed. That may have caused some issues you had before. I've tested lite with some shell script wrappers using my patch and the executable directory is detected properly each time.

@DennisCGc DennisCGc force-pushed the compileOnFreeBsd branch 2 times, most recently from 1e74b7d to 220c5bc Compare June 13, 2020 19:47
@DennisCGc
Copy link
Author

@rxi: Did you have any chance yet to review this pull request properly? You said you wanted to test this before you merge it.

How can I help so this pull request gets merged? :-)

Thanks

@BarrOff
Copy link

BarrOff commented Sep 3, 2020

+1
Just tried the patch on FreeBSD, and now lite works well. Did not encounter any problems so far.

Taking a quick look at the proposed changes, they seem reasonable to me.

clang and gcc both expose cc, so that should be used.

bash doesn't have to be installed at /bin/bash, even on certain Linux distributions
SDL2 is only available at a non-default path on some systems. pkgconf, when installed, may help in those cases
It turns out using argv[0] is a bad idea. This uses sysctl() to retrieve
its own path
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants