opus icon indicating copy to clipboard operation
opus copied to clipboard

Add version header

Open evpobr opened this issue 5 years ago • 14 comments

The only way to detect installed Opus version without pkg-config (Windows platform) is parsing some version header and reading values from defines, e.g OPUS_VERSION_MAJOR, OPUS_VERSION_MAJOR etc.

So, can opus_version.h public header be added to libopus?

evpobr avatar Jan 10 '19 06:01 evpobr

The real question is why do you need the version at compile time in the first place?

jmvalin avatar Jan 10 '19 11:01 jmvalin

I need it to find opus package with CMake and retrieve its version. Find module uses pkg-config when possible. But when it is missing (Windows), the common way is to parse public header with version defines (if provided). Bundled with CMake FindZLIB module uses this approach, and many other find modules.

If Opus will ever have CMake support, all information could be retrieved from installed CMake package module on any platform. But CMake support is still missing.

evpobr avatar Jan 10 '19 16:01 evpobr

There's someone working on CMake support. That being said, the idea is that getting the version from the header should not be needed. Instead, it's best to query actual features. For example, if you want to know if you can disable phase inversion, you can use #ifdef OPUS_SET_PHASE_INVERSION_DISABLED rather than attempt to see in what version that got added.

Note that in some cases, the header version may differ from the run-time version (especially with dynamic libraries), so the run-time version is available through an API call.

jmvalin avatar Jan 10 '19 16:01 jmvalin

There's someone working on CMake support. That being said, the idea is that getting the version from the header should not be needed.

I'm not sure we will see CMake support soon. As i see development is stopped.

it's best to query actual features. For example, if you want to know if you can disable phase inversion, you can use #ifdef OPUS_SET_PHASE_INVERSION_DISABLED rather than attempt to see in what version that got added.

It is better, indeed. But pkg-config uses version, and CMake just follows the same way. Maybe it is not optimal, but it simple and it works.

Also, our test results differs depending on libopus version. Don't know what feature it is, but we know the version.

Note that in some cases, the header version may differ from the run-time version (especially with dynamic libraries), so the run-time version is available through an API call.

We tried. Vcpkg build of Opus retrieves "unknown" version. I guess it is because Vcpkg downloads sources from Opus repo as tar.gz, not clones it. So .git directory is missing and genversion.bat script fails.

There is fallback in genversion.bat to use package_version file if found. But it fails again. version.h is generated, but PACKAGE_VERSION is set to "unknown".

evpobr avatar Jan 10 '19 17:01 evpobr

In release tarballs, the version is set without any script, so it shouldn't be a problem. When cloning, the version scripts should work. If someone makes a tar.gz of the current master without any of the version info, then there's nothing we can do about it.

jmvalin avatar Jan 11 '19 05:01 jmvalin

If someone makes a tar.gz of the current master without any of the version info, then there's nothing we can do about it.

So, if we dowload tar.gz from GitHub Releases page and we have "unknown" version reported by library built with Visual Studio it is OK?

evpobr avatar Jan 12 '19 05:01 evpobr

But idea about download tarball is good... It has package_version and works! Now i know how to fix Vcpkg opus port 😄

evpobr avatar Jan 12 '19 05:01 evpobr

About version header - if you add it, many CMake developers will thank you. You can make it standalone, do not include in any other public header. No harm to anything IMHO.

Why? Because even if CMake project is megred, it doesn't mean every maintrainer will use CMake build to create Opus packages. So cmake package script will not be installed.

evpobr avatar Jan 12 '19 05:01 evpobr

You don't understand, the contents of the git repo does not include the version number anywhere. The version is always extracted from the git tags (and commits), which is the only way it can be accurate. Otherwise it's guaranteed to be messed up, out of sync, ... (we've been there).

jmvalin avatar Jan 12 '19 07:01 jmvalin

The version is always extracted from the git tags (and commits), which is the only way it can be accurate.

I understand. That's why Vcpkg build failed: no .git and no package_version.

Otherwise it's guaranteed to be messed up, out of sync, ... (we've been there).

Ok, it looks reasonable. And still no problem with version header.

Keep template opus_version.h.in in repo. Create opus_version.h and fill values at configuration stage. Add opus_version.h to .gitignore to be sure it is never commited.

evpobr avatar Jan 12 '19 08:01 evpobr

OK, back to the start. It seems like I don't actually understand your problem. What is it that a header version would solve? Especially considering that Opus 1.0 through 1.3 will never have one.

jmvalin avatar Jan 15 '19 01:01 jmvalin

I need this

find_package(Opus 1.3)

in my CMake project to work on any platfrom.

API version is important when configuring project, right?

On Unix-like systems i can run PkgConfig and retrive all the information i want.

On Windows i can find include directory and library path with CMake commands, but not API version. PkgConfig doesn't works properly here.

So what i can do?

Some projects define version information in headers:

#define ZLIB_VERSION "1.2.11"
#define ZLIB_VERNUM 0x12b0
#define ZLIB_VER_MAJOR 1
#define ZLIB_VER_MINOR 2
#define ZLIB_VER_REVISION 11
#define ZLIB_VER_SUBREVISION 0

Then i can parse it and set my OPUS_VERSION variable at last.

Everything works, find_package(Opus 1.3) fails if package version < 1.3, everybody happy.

evpobr avatar Jan 16 '19 07:01 evpobr

OK, I think I get the problem. Now, the issue I see with your solution is how you handle older versions. You won't ever be able to see query version numbers 1.0 through 1.3 that way. The first you'd be able to query is 1.4 and it's likely more than a year away (could be multiple years). Maybe there's another way -- ideally one that would work for existing versions.

jmvalin avatar Jan 17 '19 19:01 jmvalin

Maybe there's another way -- ideally one that would work for existing versions.

I think this is common way. According to find modules bundled with CMake.

Now, the issue I see with your solution is how you handle older versions.

In my case Vcpkg always have the last version, so it is not problem. Anyway, if opus_export.h is not required by any other public header, package manager may just generate & install this file for Opus <= 1.3.

evpobr avatar Jan 19 '19 06:01 evpobr