-
Notifications
You must be signed in to change notification settings - Fork 180
build(cli): add cmake support #1118
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: master
Are you sure you want to change the base?
Conversation
cmake --build build | ||
|
||
|
||
- run: ./build/mediainfo.exe --help |
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.
Test by at least doing mediainfo.exe mediainfo.exe as just doing --help does not test if it can load mediainfo.dll properly since you are using MEDIAINFO_DLL_RUNTIME.
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 prefer a static build by default (with option for dll runtime?) if not too complicated.
The test in the CI is currently with testing that the result of e.g. ".\MediaInfo.exe" "--Output=General;%Format_Profile%" "MediaInfo.exe" is the expected one.
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.
actually I think it won't work, I didn't add dll path to env.
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 made a local build with this PR and copied the dll but still fail Unable to load MediaInfo.dll
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.
Okay, I see what is going on now, zlib.dll
is needed too.
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 prefer a static build by default (with option for dll runtime?) if not too complicated.
After hours trying, I think it's currently impossible to get a fullly static linked executable, there are some changes that must be done in the upstream packages.
I'll only test the dynamic linked version.
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.
Ya I keep getting zlib linking issues after trying many times too with my version that can build everything in one cmake command.
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 hope this won't be a blocker on this PR
close #1117