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

xplat: reduce source changes between Static and Shared Libraries. #2131

Closed

Conversation

kphillisjr
Copy link
Contributor

This greatly reduces the complexity of switching between static and shared libraries by converting most of the static libraries to Shared libraries. Now it is possible to test a static build of chakracore by simply linking
aginst libChakraCoreStatic. Also, this greatly reduces compile time when changing between the two since the changes between static and shared libraries has been isolated to within the bin folder.

As a quick note, This patch greatly reduces the runtime to run the Following commands...

./build.sh -n
./build.sh -n --static
./build.sh -n --debug
./build.sh -n --static --debug

Also as a quick note, This has not been tested against OSX/Windows, but I do not believe this will break any builds.

v2 changes: attempt to fix compile/runtime errors with MSVC/OSX, and errors on native tests.

This greatly reduces the complexity of switching between static and shared
libraries by converting most of the static libraries to Shared libraries.
Now it is possible to test a static build of chakracore by simply linking
aginst libChakraCoreStatic. Also, this greatly reduces compile time when
changing between the two since the changes between static and shared
libraries has been isolated to within the bin folder.

v2: Add Compile Fixes for MSVC/OSX, and fix native tests on ubuntu.
@msftclas
Copy link

msftclas commented Dec 1, 2016

Hi @kphillisjr, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

for some xplat platforms the filesystem is case sensitive. This fixes
the issue with opening the file on Ubuntu.
This hopefully fixes the rest of the compile issues.
This fixes build errors on windows using Visual Studio 2015.
This should force all symbols to load on OSX.
This ensures that ChakraCore only gets Initialized once per process on xplat.
This should fix the shared library problems on xplat.
@kphillisjr
Copy link
Contributor Author

I fixed the issue with most of the Compile/Test errors, so all that is left is to fix the OSX build. Could someone with OSX please help me to fix these errors. The problem seems limited to linker errors.

Once OSX has a fix, I will squash all of these commits and submit this version 3.

/CC @obastemur

This is an attempt to fix linker issues on linux/osx.
@kphillisjr
Copy link
Contributor Author

I think I just fixed the OSX Build by simplifying the linker flags. That means all that is required is probably some code cleanups, suggestions, and then finally squashing all of the commits on this pull request into a single pull request and commit.

@kphillisjr
Copy link
Contributor Author

@obastemur - With this Pull request, The required minimum version for cmake must change to version 3.5.0 or newer. The currently failed OSX test uses a version earlier than this, and in my tests cmake 3.4.3 or older all produce this error due to bugs in how macros, and target_sources. are handled.

@mmitche - I think that dci-macpro-10 needs updating. It appears that CMake version 3.2 includes bugs that lead to the failure of the "osx_osx_test" build parameter.

@obastemur
Copy link
Collaborator

@kphillisjr The CI OSX issues are due to linker problems. We are unable to manage cross dependencies as we would like to. They are being optimized away. We could however manage that by some extra fake calls on final interface. However, this workaround would give us headaches later. Good intention but we would probably keep it as is.

@kphillisjr
Copy link
Contributor Author

@obastemur, I believe that the CI OSX Linker issue is caused by OSX bugs within CMake. Is there any chance to retry the test with an updated version of CMake only? I know that CMake 3.6 pre-built binaries target OS X 10.7 or newer.

This should fix CMake build on OSX.
@kphillisjr
Copy link
Contributor Author

Made changes, and submitting as pull request #2138.

@kphillisjr kphillisjr closed this Dec 3, 2016
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