-
Notifications
You must be signed in to change notification settings - Fork 94
Fix static windows build #3528
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
base: main
Are you sure you want to change the base?
Fix static windows build #3528
Conversation
b8da0e6 to
954a610
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM!
Please check the comments
| install_dir: application_install_dir, | ||
| link_args: [tokenizer_path], | ||
| cpp_args: '-DPLUGGABLE' | ||
| cpp_args: cpp_args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it okay to have the same naming?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok, it's key - value relation
meson_options.txt
Outdated
| option('libiomp_root', type:'string', value: './libiomp_win') | ||
|
|
||
| # use layers as pluggable modules | ||
| option('enable-pluggable', type: 'boolean', value: true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about adding default_library=static and enable-pluggable=false to windows-native.ini?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could the following condition be possible?
default_library is static, and enable-pluggable is true; default_library is not static, and enable-pluggable is false.
If this is not feasible, we might need to remove the new option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configuration in which default_library is static, and enable-pluggable is true would not work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, we could remove this option and revise it.
cpp_args = []
if get_option('default_library') != 'static'
cpp_args += [ '-DPLUGGABLE' ]
endif
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
fba2842 to
2ab7b50
Compare
Make possible to build nntrainer in static or shared version on Windows platform **Self-evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: Grzegorz Kisala <[email protected]>
2ab7b50 to
deeafc9
Compare
Make possible to build nntrainer in static or shared version on
Windows platform
Use following commandt for static build:
meson setup --wipe --native-file windows-native.ini builddir -Ddefault_library=static -Denable-pluggable=false -Denable-opencl=true
meson compile -C builddir