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

Adds support for compiling shared library for Luac #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

guyzmo
Copy link

@guyzmo guyzmo commented Feb 25, 2017

  • compiles lua with .so shared lib per default
  • added --no-shared to skip building shared lib (and avoid -fPIC addition)

Fixes #31

@ccinv
Copy link

ccinv commented Mar 12, 2017

I'm just a people passing by(in fact,a user of hererocks).I found your PR failed just because of pep8.
This patch is of great help.Could you fix it so that I can do CI with shared lua library soon?
Much Thanks. Ping @guyzmo.

@mpeterv
Copy link
Owner

mpeterv commented Mar 12, 2017

@guyzmo @avaicode sorry for lack of review on this. @guyzmo I would prefer if this feature was opt-in, mostly because I want default behaviour to be as close to standard Lua makefile as possible.

- compiles lua with .so shared lib per default
- added --shared to add building shared lib (and -fPIC addition)

Fixes mpeterv#31

Signed-off-by: Guyzmo <[email protected]>
@guyzmo
Copy link
Author

guyzmo commented Mar 12, 2017

fixed the pep8 and made it --shared.

@guyzmo
Copy link
Author

guyzmo commented Mar 12, 2017

i'm not gonna be around much more today, so feel free to make changes to the PR for inclusion 😉

@mpeterv
Copy link
Owner

mpeterv commented Mar 21, 2017

@guyzmo I can not really get into this to fix the PR right now. If you could, please fix some issues:

  • On Windows so_file is set to None but is used when building with MinGW, causing Windows tests to fail
  • Name of the option was negated but behaviour wasnt
  • On Mac Os X some of the used compiler options are not supported, see travis build

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