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

Circular run-exports lead to overdepending #72

Open
h-vetinari opened this issue Oct 13, 2023 · 15 comments · May be fixed by #73
Open

Circular run-exports lead to overdepending #72

h-vetinari opened this issue Oct 13, 2023 · 15 comments · May be fixed by #73

Comments

@h-vetinari
Copy link
Member

h-vetinari commented Oct 13, 2023

With the solution picked for #67 / #68, we need a somewhat circuitous mechanism to avoid that different libre2-{X,Y} versions can be co-installed. In particular,

- name: re2
script: install.sh # [unix]
script: install.bat # [win]
build:
run_exports:
- {{ pin_subpackage("libre2-" ~ soversion) }}
# by adding this run-export, we avoid that different libre2-{X,Y} builds
# can be co-installed, because they then depend on different re2-versions
# (see below), and each re2 in turn requires a specific soversion.
- re2

To recap, re2 depends exactly on libre2-X (which it also run-exports) and libre2-X has a run-constraint on the exact re2 version that was used to build it. It's somewhat unintuitive that this works out as intended but if re2 v2023.a.b and re2 v2024.c.d have the same SOVERSION, a downstream package built against v2023.a.b (at some point in the past) will be able to pull in the newest libre2-X built from v2024.c.d at runtime.

However, the run-constraint is not enough to avoid that builds against different libre2-{X,Y} cannot be installed side-by-side (which we want to avoid due to this), because the constraint only fires if re2 is actually present in the environment.

So additionally, we have a run-export of re2 to re2, that ensures that the run-constraint of libre2-X fires and so all should be well.

However, because re2 doesn't contain the actual libraries, the link-check now starts to complain. This will lead to errors in feedstocks that opt into error_overlinking: true and might confuse maintainers that manually try to keep their link-check clean:

WARNING (libgrpc): run-exports library package conda-forge::re2-2023.06.02-h2873b5e_0 in requirements/run but it is not used (i.e. it is overdepending or perhaps statically linked? If that is what you want then add it to `build/ignore_run_exports`)

In particular, removing re2 as a host-dependence will not solve the issue for such feedstocks, and worse: if they do the seemingly right thing of

ignore_run_exports:
  - re2

that just brings us back to square one, because that breaks the above mechanism to avoid co-installation.

I think the fix is to elevate the run-constraint to an actual run-dependence of libre2-X on re2, and then remove the re2 run-export from re2, but perhaps I'm missing something.

CC @isuruf @xhochy

@h-vetinari
Copy link
Member Author

I think the fix is to elevate the run-constraint to an actual run-dependence of libre2-X on re2, and then remove the re2 run-export from re2, but perhaps I'm missing something.

Yup, missed the fact that that's not possible because it introduces a build cycle. 😑

@isuruf
Copy link
Member

isuruf commented Oct 13, 2023

This will lead to errors in feedstocks that opt into error_overlinking: true and might confuse maintainers that manually try to keep their link-check clean:

It does? Do you have an example log?

@h-vetinari
Copy link
Member Author

Do you have an example log?

Yes

@isuruf
Copy link
Member

isuruf commented Oct 13, 2023

The log you linked to passes fine. You said it will error if error_overlinking: true. Do you have an example of that?

@h-vetinari
Copy link
Member Author

h-vetinari commented Oct 13, 2023

Yes, because that feedstock does not use error_overlinking: true or error_overdepending: true (which are kind of like -Werror for the link check).

Still, the warning is clear from the log, including the nudge to add re2 to ignore_run_exports, which would break the mechanism we have in place.

WARNING (libgrpc): run-exports library package conda-forge::re2-2023.06.02-h2873b5e_0 in requirements/run but it is not used (i.e. it is overdepending or perhaps statically linked? If that is what you want then add it to `build/ignore_run_exports`)

That's because the link check picks up the output which actually contains the DSO:

   INFO (libgrpc,lib/libgrpc.so.35.0.0): Needed DSO lib/libre2.so.11 found in conda-forge::libre2-11-2023.06.02-h7a70373_0

leaving re2 unused (from its POV).

@isuruf
Copy link
Member

isuruf commented Oct 13, 2023

All I'm saying is that your statement about error_overlinking: true is false.

@h-vetinari
Copy link
Member Author

OK. Strike that off the record then. The warning still instructs maintainers to do the very thing that will break this mechanism.

@isuruf
Copy link
Member

isuruf commented Oct 13, 2023

Same can be said of numpy. From scipy-feedstock:

WARNING (scipy): run-exports library package conda-forge::numpy-1.22.4-py310h4ef5377_0 in requirements/run but it is not used (i.e. it is overdepending or perhaps statically linked? If that is what you want then add it to `build/ignore_run_exports`)

@h-vetinari
Copy link
Member Author

All I'm saying is that your statement about error_overlinking: true is false.

Here you go with a failing CI run now:

ERROR (libgrpc): run-exports library package conda-forge::re2-2023.06.02-h2873b5e_0 in requirements/run but it is not used (i.e. it is overdepending or perhaps statically linked? If that is what you want then add it to `build/ignore_run_exports`)

Same can be said of numpy.

Numpy is different. Quoting @xhochy:

Due to the nature of NumPy's linking mechanism (which is really great!), you should never see an actual linkage against it. If the warning isn't always showing up, I would be very curious why it happens. All references are filled at runtime through the C function import_numpy().

@h-vetinari h-vetinari mentioned this issue Nov 14, 2023
3 tasks
@h-vetinari h-vetinari changed the title Circular run-exports lead to overlinking Circular run-exports lead to overdepending Dec 10, 2023
@h-vetinari h-vetinari mentioned this issue Feb 7, 2024
3 tasks
@xhochy
Copy link
Member

xhochy commented Mar 18, 2024

Should we have instead named it simply libre2 instead of adding the version to the name of the library?

@xhochy
Copy link
Member

xhochy commented Mar 18, 2024

My proposal would be to have a libre2-mutex package that is

a) empty
b) noarch: generic
c) has the SOVERSION as the version
d) re2 and libre2-SOVERSION depend on it as run dependency.

This should solve the overlinking/overdepending issues as it is not a run_exports but also always present to the solver.

@isuruf
Copy link
Member

isuruf commented Mar 18, 2024

I think we should just ignore overdepending checks. It's a broken check. Overlinking check should work just fine.

@xhochy
Copy link
Member

xhochy commented Mar 18, 2024

I find them useful to detect unnecessary dependencies and thus would like to see them working. Looking through the implementation in `conda-build, ' I don't see an easy way to ignore them; therefore, the proposal is with a shared mutex package. With that, we should get around the warning/error of the check and support it also with other build tools (like rattler-build) as it isn't a conda-build-specific workaround.

@isuruf
Copy link
Member

isuruf commented Mar 18, 2024

I'd like to avoid adding another package to work around a conda-build bug. I think the current recipe here is a good one that we want to generalize to other packages, but having a mutex would make this not elegant.

@h-vetinari
Copy link
Member Author

I think the patterns should be adapted to what they should achieve:

  • enable co-installability between versions: use versioned outputs (see libllvm<major>, libclang<sover>)
  • avoid co-installability between versions: use a single output (which automatically ensures that)

Here we want to avoid co-installability, but use versioned outputs, which to me is upside down. It also requires complicated and almost circular constraints and run-exports, which in itself is not something I want to see spread more widely, because it's hard to grasp and substantially raises the bar for contributions.

All that would still make it somewhat viable, but given the additional false positives for the overdepending-check (which many maintainers do react to; which would in turn undo the constraints that prevent co-installability), I really think this is not something we should propagate.

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

Successfully merging a pull request may close this issue.

3 participants