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

libprotobuf-static dependencies #164

Closed
1 task done
JanuszL opened this issue Jun 13, 2023 · 9 comments
Closed
1 task done

libprotobuf-static dependencies #164

JanuszL opened this issue Jun 13, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@JanuszL
Copy link

JanuszL commented Jun 13, 2023

Solution to issue cannot be found in the documentation.

  • I checked the documentation.

Issue

It seems that libprotobuf-static depends on libprotobuf and the cmake configuration files it provides. So when one does:

find_package(protobuf REQUIRED CONFIG)
target_link_libraries(foo protobuf::libprotobuf)

(lowercase protobuf makes cmake using the cmake configuration files installed by protobuf, not build-in cmake FindProtobuf)
it will be linked with the dynamic one even if the static is installed. Manually providing libprotobuf.a only partially solves the problem as it doesn't carry the dependency on libutf8_validity.a, which libprotobuf.a requires to link successfully.
I think that this test links dynamic libprotobuf, not a static one so it doesn't test libprotobuf-static.
I'm looking to hear your opinion.

Installed packages

NA

Environment info

NA
@JanuszL JanuszL added the bug Something isn't working label Jun 13, 2023
@h-vetinari
Copy link
Member

This setup has had a long discussion (in particular w.r.t. CMake setup), see here (I'm linking the summary because the thread is long).

In short, to solve this, it's probably necessary to add something like:

   - name: libprotobuf-static
     script: build-lib.sh   # [unix]
     script: build-lib.bat  # [win]
     build:
+      always_include_files:
+        # Must overwrite the CMake metadata from shared libprotobuf
+        - lib/cmake/protobuf/           # [unix]
+        - Library/lib/cmake/protobuf/   # [win]
       ignore_run_exports_from:
         - jsoncpp
     requirements:

PRs welcome.

@JanuszL
Copy link
Author

JanuszL commented Jun 14, 2023

Hi @h-vetinari,

Thank you for the pointers and advice. Based on the linked discussion it sounds like an ecosystem-wide philosophical problem than a quick fix. I will try to tackle it on my own in the project itself.
If any good idea comes to my mind I will definitely fill a PR.

@JanuszL JanuszL closed this as completed Jun 14, 2023
@h-vetinari
Copy link
Member

FWIW, the fix I proposed would be "legitimate" (i.e. in line with the outcome of that discussion), and not a quick fix per se. There are other work-arounds how to enforce using the static libraries (IIRC, micromamba was doing this a lot, see e.g. here).

@JanuszL
Copy link
Author

JanuszL commented Jun 14, 2023

Yes, it would work. My reservation is that still someone may want to use both static and dynamic versions in the save env, like your project X depends on A-static and B, and B depends on A-dynamic. So you end up with a conflict/inconsistency in the system.
On top of that libprotobuf is a channeling beast now as it depends on absl - so when you link you need to discover all the dependencies it carries with it. So the workaround is not that simple/clean anymore.

@h-vetinari
Copy link
Member

My reservation is that still someone may want to use both static and dynamic versions in the save env, like your project X depends on A-static and B, and B depends on A-dynamic. So you end up with a conflict/inconsistency in the system.

Actually that would work. The rule is simple: if you depend on libfoo, you always get the static one. If you depend on libfoo-static, you get the static one (with the patch I propose).

If another dependency (B in your example) transitively depends on A-shared, that's compatible (the environment resolves, and since the dependency doesn't need to be rebuilt the CMake metadata for the shared libs isn't necessary). If another dependency transitively depends on the A-static, then that doesn't percolate further.

The linked thread might make it sound contentious because it's a tricky subject and I didn't understand all the intricacies, but I'm on board with it now - it's the best we can do under a bunch of conflicting constraints.

@JanuszL
Copy link
Author

JanuszL commented Jun 14, 2023

If another dependency (B in your example) transitively depends on A-shared, that's compatible (the environment resolves, and since the dependency doesn't need to be rebuilt the CMake metadata for the shared libs isn't necessary). If another dependency transitively depends on the A-static, then that doesn't percolate further.

What I have in mind is that if you end up installing both A-static and A-shared in your build env, you can have only one protobuf-targets-release.cmake either for the static or dynamic variant, depending on which one comes last.

@h-vetinari
Copy link
Member

As soon as you have both, static wins (which is the point of the always_copy_files), even though conda will complain about a file being clobbered.

@h-vetinari
Copy link
Member

Because static libraries don't self-propagate in terms of requirements, the only way to get into that "confusing situation" in the first place is if a given recipe explicitly demands the static variant, so that matches the intent quite well (the only blemish being the clobberwarning)

@JanuszL
Copy link
Author

JanuszL commented Jun 14, 2023

Yes, I missed that. Thank you for clarification.

JanuszL added a commit to JanuszL/libprotobuf-feedstock that referenced this issue Jun 14, 2023
- addresses conda-forge#164
- makes sure that libprotobuf-static bundles proper cmake
  files (one for static flavor, not dynamic one)

Signed-off-by: Janusz Lisiecki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants