-
Notifications
You must be signed in to change notification settings - Fork 82
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
cmake issues #33
Comments
Any comments please? |
PING - anyone? |
@sezero Hi, I am not so good with all the version stuff. But can you check if this change would fix the issues you have for opus? |
@madebr: cmake is your language, can you have a look? |
I like @sezero's idea of using "if" instead of "else if". That way if decoding the package_version fails, it tries again from git. Like this:
Side note, I think it would be useful to keep a basic "version" file maintained in-repo so we don't need to fallback to 0.0. (e.g. ogg keeps the version in the configure.ac file in the repo, but opus/opusfile currently do not.) In that case, we'd still need to get the commit id from git if available (and only if the git tag matches the version file). @mark4o thoughts? |
In SDL_mixer, we submodule'd opus and opusfile through our forks
and attempted cmake build relying on the submodules. Issues we had:
The git tree, obviously, is missing a
package_version
file. Weadded to both opus and opusfile. However:
If everything fails opusfile cmake'ry sets
OPUSFILE_PROJECT_VERSION
to 0, as in integer:
https://github.com/xiph/opusfile/blob/master/cmake/OpusFilePackageVersion.cmake#L63
... But the caller expects a version instead, and fails:
https://github.com/xiph/opusfile/blob/master/CMakeLists.txt#L6-L9
Should you change the fallback
OPUSFILE_PROJECT_VERSION
to0.0
instead? (Same is true in Opus project.)
Opusfile cmake'ry doesn't look for
package_version
file if itfinds git, because the
package_version
case is anelseif
casewhich seems wrong:
https://github.com/xiph/opusfile/blob/master/cmake/OpusFilePackageVersion.cmake#L31
Finalizing the
git
case with anendif
changingpackage_version
case to an
if
fixes things for us.Opus: Going from v1.3.1 to git master, the issue described above is
present in opus too:
https://github.com/xiph/opus/blob/master/cmake/OpusPackageVersion.cmake#L31
The 1.3.1 release was good:
https://github.com/xiph/opus/blob/e85ed7726db5d677c9c0677298ea0cb9c65bdd23/opus_functions.cmake#L45
(Note that
get_package_version
used to return only if git case succeeded.)So the same suggestion as in the opusfile case above.
The text was updated successfully, but these errors were encountered: