Skip to content
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

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

Open
alevenberg opened this issue Dec 13, 2023 · 7 comments
Open

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

alevenberg opened this issue Dec 13, 2023 · 7 comments
Assignees
Labels
Stale triage/accepted Indicates an issue or PR is ready to be actively worked on.

Comments

@alevenberg
Copy link

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

@github-actions github-actions bot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Dec 13, 2023
@marcalff marcalff assigned marcalff and ThomsonTan and unassigned marcalff Dec 18, 2023
@marcalff marcalff added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Dec 18, 2023
@marcalff
Copy link
Member

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

Fix to be implemented in the vcpkg repository.

@meastp
Copy link
Contributor

meastp commented Jan 5, 2024

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, ... ).

@alevenberg
Copy link
Author

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.

@meastp
Copy link
Contributor

meastp commented Jan 6, 2024

@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 :)

@marcalff
Copy link
Member

marcalff commented Jan 8, 2024

@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.

@meastp
Copy link
Contributor

meastp commented Jan 8, 2024

@marcalff no worries :)

Copy link

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

@github-actions github-actions bot added the Stale label Mar 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Stale triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

No branches or pull requests

4 participants