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

Add parallel build files to build usable minimal shared sentencepiece dll with MSVC #1

Merged
merged 5 commits into from
Nov 26, 2018

Conversation

anthonyaue
Copy link
Collaborator

The existing Cmake files do not build usable shared libraries with MSVC. These files allow you to build a minimal shared library using MSVC. I tried to just add the relevant changes to the existing CMake files, but was unable to get something that builds everything correctly, so I backed off to checking in these alternatives.

@emjotde
Copy link
Member

emjotde commented Nov 26, 2018

I am wondering whether we should try to get this into the original repo. The owner is very open to suggestions, see for instance this issue google#252

Although we would probably need to figure this out in a correct way first. Since this does not affect the main build, I am OK with pulling this in, but we should definitely go for a nice build in the original CMakeFile.

We should probably sit down together and take a look where the problems are.

Copy link
Member

@emjotde emjotde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving this so we can move forward, but ultimately, we should have one CMake build that can be shared back with the original repo.

@anthonyaue
Copy link
Collaborator Author

I'd love to get it into the main branch, but we'd need to merge Cmake files and build *.bat files. I'm sure this is doable, but I ran out of patience fighting with CMake+MSVC. The dependency on protobuf makes the whole mess much more complicated because CMake wants to export all of the symbols, including the ones in the C++ runtime library, so you end up with multiple definitions of runtime library symbols while linking. I'll take this as a work item for a later date when I have time to do it right.

@anthonyaue anthonyaue merged commit b92ee5f into master Nov 26, 2018
@anthonyaue anthonyaue deleted the anthonyaue/min_export_dll branch November 26, 2018 16:57
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.

2 participants