Code Monkey home page Code Monkey logo

Comments (16)

lidavidm avatar lidavidm commented on September 25, 2024 1

Merged (and I didn't squash this time!)

from cpp-opentelemetry-sdk-feedstock.

h-vetinari avatar h-vetinari commented on September 25, 2024

@lidavidm, any thoughts on this?

from cpp-opentelemetry-sdk-feedstock.

lidavidm avatar lidavidm commented on September 25, 2024

Ah, sorry.

All of these should be fixed, but I haven't had a chance to do much other than merge updates for this.

from cpp-opentelemetry-sdk-feedstock.

h-vetinari avatar h-vetinari commented on September 25, 2024

Ok, this was mostly fixed as of #44.

I'd still like to rename the library to something like libopentelemetry according to conda-forge/conda-forge.github.io#1073, rather than have yet another library naming pattern (we can keep the old name as a compat wrapper for a while).

But the opentelemetry stuff is distributed over quite a few feedstocks1 and I'm not sure if that's conflicting with something else.

Thoughts @lidavidm @wjones127 @xhochy?

Footnotes

  1. also notices that the CMake installation files are somewhere in an API package as well and get clobbered (so possibly broken metadata here)

from cpp-opentelemetry-sdk-feedstock.

lidavidm avatar lidavidm commented on September 25, 2024

renaming is a-ok by me.

the clobbering is an unfortunate aspect of: some things want to just use opentelemetry as a producer/API-only, and don't want to pull in the actual implementation bits, only the headers. But the package itself isn't really designed with that in mind. So we're basically building it twice, and so things get clobbered. I'm not sure how best to deal with this.

from cpp-opentelemetry-sdk-feedstock.

lidavidm avatar lidavidm commented on September 25, 2024

Maybe upstream would be open to splitting up the package further.

from cpp-opentelemetry-sdk-feedstock.

h-vetinari avatar h-vetinari commented on September 25, 2024

renaming is a-ok by me.

Cool! Do you think we should rename the API output in parallel too? Looking at the open-telemetry org and its repos, we might want to keep the -cpp since there are many different languages.

So perhaps:

  • libopentelemetry-cpp & libopentelemetry-cpp-api?

(I'll also note that looking at the content of the package, it's effectively just a header-only package; a similar naming discussion is ongoing for boost currently, where we might use libboost-header-only)

I'm not sure how best to deal with this.

How about not including the CMake metadata in the api package? I opened a PR: conda-forge/cpp-opentelemetry-api-feedstock#17

from cpp-opentelemetry-sdk-feedstock.

lidavidm avatar lidavidm commented on September 25, 2024

libopentelemetry-cpp/-cpp-api sounds good to me.

The -api scheme is because that's what the package itself calls the header-only variant, not sure which is preferable but I don't mind libopentelemetry-cpp-header-only either.

I think we still want the CMake metadata so that downstream packages can still find_package(OpenTelemetry) and get the headers.

from cpp-opentelemetry-sdk-feedstock.

h-vetinari avatar h-vetinari commented on September 25, 2024

I think we still want the CMake metadata so that downstream packages can still find_package(OpenTelemetry) and get the headers.

I think it would be fine to sacrifice that. Adding another include directory is trivial, whereas CMake metadata contains a lot more metadata than just headers.

from cpp-opentelemetry-sdk-feedstock.

lidavidm avatar lidavidm commented on September 25, 2024

Right, but then that means projects that depend on opentelemetry will have to adjust builds to work both against the conda-forge package and a 'regular' installation? I'd like to avoid that if possible, though I suppose that's just the nature of dependencies in C++.

from cpp-opentelemetry-sdk-feedstock.

h-vetinari avatar h-vetinari commented on September 25, 2024

Right, but then that means projects that depend on opentelemetry will have to adjust builds to work both against the conda-forge package and a 'regular' installation?

Hm, I tend to "live" in a world where upstream doesn't care about conda-forge (or barely), so to me it's completely fine if upstream continues working only with find_package(OpenTelemetry), and conda-forge deals with the consequences of its packaging decisions.

I just think that clobbering files is a bad packaging smell, not least because we'll constantly spam every user with warnings about that (but also in general).

from cpp-opentelemetry-sdk-feedstock.

xhochy avatar xhochy commented on September 25, 2024

I just think that clobbering files is a bad packaging smell, not least because we'll constantly spam every user with warnings about that (but also in general).

I would strongly support removing any clobbering as this can easily cause issues if you start to patch some of the files in only one feedstock and you cannot rely that the patch is correctly installed in the clobbering situation.

from cpp-opentelemetry-sdk-feedstock.

xhochy avatar xhochy commented on September 25, 2024

Wouldn't we get around the clobbering problems if this package here depended on libopentelemetry-cpp-api in host and run?

from cpp-opentelemetry-sdk-feedstock.

h-vetinari avatar h-vetinari commented on September 25, 2024

Wouldn't we get around the clobbering problems if this package here depended on libopentelemetry-cpp-api in host and run?

The problem (AFAICT) is that the content of the CMake target files will be different if it's installed as API-only or with the libs. So having it in host would not help.

from cpp-opentelemetry-sdk-feedstock.

lidavidm avatar lidavidm commented on September 25, 2024

Right, but then that means projects that depend on opentelemetry will have to adjust builds to work both against the conda-forge package and a 'regular' installation?

Hm, I tend to "live" in a world where upstream doesn't care about conda-forge (or barely), so to me it's completely fine if upstream continues working only with find_package(OpenTelemetry), and conda-forge deals with the consequences of its packaging decisions.

I just think that clobbering files is a bad packaging smell, not least because we'll constantly spam every user with warnings about that (but also in general).

Ok! I'll defer to you and Uwe. I'll give the PR a review in a little bit.

from cpp-opentelemetry-sdk-feedstock.

h-vetinari avatar h-vetinari commented on September 25, 2024

Opened a PR for the rename here: #46

After this I think we should be good to add it to the global pinning.

from cpp-opentelemetry-sdk-feedstock.

Related Issues (3)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.