opentelemetry-cpp icon indicating copy to clipboard operation
opentelemetry-cpp copied to clipboard

[vcpkg] Create a feature for OTel ABI 2.0 in vcpkg

Open alevenberg opened this issue 1 year ago • 7 comments

We build OTel in our CI and want to add a build with cmake and otel abi 2.0. We use vcpkg as a package manager.

My ideal vcpkg.json would look like

    {
      "name": "opentelemetry-cpp",
      "features": [
        "abi-2"
      ]
    }

Is your feature request related to a problem? No

Describe the solution you'd like The current vcpkg does not have an otel abi feature (https://github.com/microsoft/vcpkg/blob/42301df395852935d105799de93abf5481f34f1a/ports/opentelemetry-cpp/vcpkg.json)

Describe alternatives you've considered Building from source.

Additional context N/A

alevenberg avatar Dec 13 '23 18:12 alevenberg

Accepting the issue in opentelemetry-cpp as a place holder.

Fix to be implemented in the vcpkg repository.

marcalff avatar Dec 18 '23 21:12 marcalff

Unfortunately, this behaviour is disallowed by the manifests vcpkg docs/spec:

Each feature should be additive with other features: if a project provides features A and B, then it should build successfully with both A and B enabled together. Features should not be used to define alternatives. For example, features cannot be used to choose between multiple, exclusive graphics APIs since it is not possible to choose both.

It is a shame, really - I'd like to be able to enable WITH_STL in the vcpkg build.

The solution is to create a custom port, which is the port in the official vcpkg repository slightly modified (add otel abi 2.0, with_stl etc.). This is what I do.

--

I'd love it if an "official" vcpkg registry existed in open-telemetry/opentelemetry-cpp-vcpkg-registry. This could contain different variants of optentelemetry-cpp as ports (opentelemetry-cpp-stl, opentelemetry-abi2, ... ).

meastp avatar Jan 05 '24 12:01 meastp

Agh, that's unfortunate. Thanks for letting me know @meastp.

I think I'm gonna end up building from source for CI, but thanks for mentioning the solution (maybe it will help someone else).

Maintainers, feel free to close or change the issue as you see fit.

alevenberg avatar Jan 05 '24 18:01 alevenberg

@alevenberg You could try to update the vcpkg-repo's port to use abi v2. imho, newest abi should be the default. I can look into making changes + PR there if @ThomsonTan @latlib @marcalff ... agree?

Also, there are alternatives to making a custom registry. If you make a ports folder in your project repo (or anywhere, really) and use environment variable --https://learn.microsoft.com/en-us/vcpkg/users/config-environment#vcpkg_overlay_ports or command line argument --overlay-ports , it would work well. Just copy the ports/opentelemetry-cpp folder and Just modify the abi-definition sent to cmake in portfile.cmake and go :)

meastp avatar Jan 06 '24 09:01 meastp

@alevenberg You could try to update the vcpkg-repo's port to use abi v2. imho, newest abi should be the default. I can look into making changes + PR there if @ThomsonTan @latlib @marcalff ... agree?

@meastp

Thanks for the offer, but no, please don't.

ABI v2 is unstable, and will change, as there are many things to cleanup, refactor, and improve there.

Exposing an unstable interface can only create more trouble for users, and maintainers as well when dealing with a new flow of bug reports related to breaking changes.

One user testing ABI v2 as an early adopter is fine, even welcomed, but forcing everyone to use ABI v2 by default is another matter.

By the time ABI v2 is published as stable, this can of course be revised.

marcalff avatar Jan 08 '24 17:01 marcalff

@marcalff no worries :)

meastp avatar Jan 08 '24 17:01 meastp

This issue was marked as stale due to lack of activity.

github-actions[bot] avatar Mar 10 '24 01:03 github-actions[bot]