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

[bug] impossible to consume all shared #9712

Closed
SSE4 opened this issue Oct 2, 2021 · 6 comments
Closed

[bug] impossible to consume all shared #9712

SSE4 opened this issue Oct 2, 2021 · 6 comments
Assignees
Milestone

Comments

@SSE4
Copy link
Contributor

SSE4 commented Oct 2, 2021

found in conan-io/conan-center-index#7142

there is a Glib requirement - it has to exist only once in the address space.
https://gitlab.freedesktop.org/gstreamer/gst-build/-/issues/133

it means, if I have the following graph:
gst-plugins-base -> gstreamer -> glib

there are only two valid combinations:

  1. gst-plugins-base:shared=False, gstreamer:shared=False, glib:shared=False
  2. gst-plugins-base:shared=True, gstreamer:shared=True, glib:shared=True

anything besides that is undefined behavior and will not work.
now, I install gst-plugins-base with the following command line:

$ conan create . gst-plugins-base/1.19.1@ -k -o gst-plugins-base:shared=True -o gstreamer:shared=True -o glib:shared=True 2>&1 | tee .out

I believe it should install all of them as shared, right? yes, but not exactly... it doesn't work and crashes:

objc[3021]: Class GNotificationCenterDelegate is implemented in both /Users/sse4/.conan/data/glib/2.67.1/_/_/package/965829c9da846fd7962b309ca20947986a345dc7/lib/libgio-2.0.0.dylib (0x100db0280) and /Users/sse4/.conan/data/gstreamer/1.19.1/_/_/package/5a1e3781444d60ee53a8d7e762ece8f559cf15ca/lib/libgstnet-1.0.0.dylib (0x100be0378). One of the two will be used. Which one is undefined.

(process:3022): GLib-GObject-CRITICAL **: 21:45:46.965: g_param_spec_pool_lookup: assertion 'pool != NULL' failed

researching it, I can see in the logs it uses gstreamer package from the conan-center:

gstreamer/1.19.1:5a1e3781444d60ee53a8d7e762ece8f559cf15ca - Download

going to the cache, I am checking the conaninfo.txt and it has:

[full_options]
    shared=True
    ....
    glib:shared=False

surprisingly, it consumed static glib! (glib:shared=False).
from the command line perspective, unfortunately, I don't see any way to force shared gstreamer to link with shared glib.
digging even more into the issue, it seems like it's due to the incorrect package id computation.

e.g. running the following two commands:

$ conan info gstreamer/1.19.1@ -o glib:shared=False -o gstreamer:shared=True --package-filter "gstreamer*" | grep ID
    ID: 5a1e3781444d60ee53a8d7e762ece8f559cf15ca

$ conan info gstreamer/1.19.1@ -o glib:shared=True -o gstreamer:shared=True --package-filter "gstreamer*" | grep ID
    ID: 5a1e3781444d60ee53a8d7e762ece8f559cf15ca

it seems like they both have the same package ID - 5a1e3781444d60ee53a8d7e762ece8f559cf15ca.
I don't think it's correct - they are definitely two different libraries.
the first one has glib code embedded into the gstreamer shared library, and the second one just links to the glib shared library. no doubt, they are different things and have different behavior.

I think it's a bug and should be fixed (and not in conan 2.0, but in conan 1.x).

Environment Details (include every applicable attribute)

  • Operating System+version:
  • Compiler+version:
  • Conan version:
  • Python version:

Steps to reproduce (Include if Applicable)

Logs (Executed commands with output) (Include/Attach if Applicable)

@memsharded
Copy link
Member

The issue with shared libraries is known from some time ago. The shared_library_package_id() helper was introduced to try to alleviate this issue, but it was not matured and it was not adopted in ConanCenter recipes.

I am sorry, but this behavior cannot be changed for Conan 1.X. It would be massively breaking, if we make the package_id depend on the options of the dependencies, then almost all binaries in ConanCenter would become immediately invalid, as they would compute new package_ids. And we will certainly break many thousands of private user builds. Tagging this as Conan 2.0, I don't think it is worth the huge effort to improve and adopt the shared_library_package_id(), but if anything is to be done, it should be opt-in. Another workaround is to force the rebuild from source of gstreamer when this happens. Yet antoher workaround, the package_id() for gstreamer package could be changed in ConanCenter to depend on full_package_mode on glib.

@jgsogo
Copy link
Contributor

jgsogo commented Oct 4, 2021

We should think about this issue in the context of ConanCenter. We should open an issue there at least. We can investigate and adopt one of those approaches, or maybe we can build packages with *:shared=True when shared, instead of building only the final consumer as shared. This, together with some checks in the validate, can probably provide some meaningful output.

I would focus just on the recipes where this can become a problem, of course, we don't want to add something to every recipe out there... but modifying 4-10 recipes to address this issue is something we can afford.

@memsharded memsharded added this to the 2.0 milestone Oct 4, 2021
@SSE4
Copy link
Contributor Author

SSE4 commented Oct 5, 2021

@memsharded for 1.x, is it possible do anything to improve the situation?
if we can't change package id, which is definetely breaking, could be there some additional validation maybe?
(maybe activated with some special flag or config entry, e.g. conan install --check-package-id ...)
like if I pass glib:shared=True, can it check conaninfo.txt for all the dependencies in the graph and raise an error/warning if some dependency (e.g. gstreamer) contains glib:shared=False, suggesting to build from source?

@memsharded
Copy link
Member

if we can't change package id, which is definetely breaking, could be there some additional validation maybe?

No, there is no infrastructure or place to check or put this. This issue requires taking actions in the recipes to alleviate it, until it is improved with a new package_id computation.

@SSE4
Copy link
Contributor Author

SSE4 commented Oct 17, 2021

just in case, we have found a "workaround" for 1.x in private discussion with @jgsogo :

def package_id(self):
    ...
    self.info.requires["glib"].full_package_mode()

@memsharded
Copy link
Member

Solved in 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants