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

Meson: Improve handling of dependencies #38913

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

tobiasdiez
Copy link
Contributor

Make the dependency declaration in the meson build files more precise and add a few missing dependencies for some files.

Extracted from #38872.

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented Nov 3, 2024

Documentation preview for this PR (built with commit 0416a96; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@dimpase
Copy link
Member

dimpase commented Nov 10, 2024

tests won't start:

Traceback (most recent call last):
  File "/mnt/opt/Sage/sage-clang/src/bin/sage-runtests", line 9, in <module>
    sys.exit(main())
             ^^^^^^
  File "/mnt/opt/Sage/sage-clang/src/sage/doctest/__main__.py", line 192, in main
    err = DC.run()
          ^^^^^^^^
  File "/mnt/opt/Sage/sage-clang/src/sage/doctest/control.py", line 1610, in run
    self.run_doctests()
  File "/mnt/opt/Sage/sage-clang/src/sage/doctest/control.py", line 1194, in run_doctests
    self.dispatcher = DocTestDispatcher(self)
                      ^^^^^^^^^^^^^^^^^^^^^^^
  File "/mnt/opt/Sage/sage-clang/src/sage/doctest/forker.py", line 1726, in __init__
    init_sage(controller)
  File "/mnt/opt/Sage/sage-clang/src/sage/doctest/forker.py", line 207, in init_sage
    controller.load_environment()
  File "/mnt/opt/Sage/sage-clang/src/sage/doctest/control.py", line 695, in load_environment
    return import_module(self.options.environment)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.12/importlib/__init__.py", line 90, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1387, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1360, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1331, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 935, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 995, in exec_module
  File "<frozen importlib._bootstrap>", line 488, in _call_with_frames_removed
  File "/mnt/opt/Sage/sage-clang/src/sage/repl/ipython_kernel/all_jupyter.py", line 6, in <module>
    from sage.all_cmdline import *
  File "/mnt/opt/Sage/sage-clang/src/sage/all_cmdline.py", line 17, in <module>
    from sage.all import *
  File "/mnt/opt/Sage/sage-clang/src/sage/all.py", line 86, in <module>
    from sage.rings.all import *
  File "/mnt/opt/Sage/sage-clang/src/sage/rings/all.py", line 57, in <module>
    from sage.rings.number_field.all import *
  File "/mnt/opt/Sage/sage-clang/src/sage/rings/number_field/all.py", line 2, in <module>
    from sage.rings.number_field.number_field import (NumberField, NumberFieldTower, CyclotomicField, QuadraticField,
  File "/mnt/opt/Sage/sage-clang/src/sage/rings/number_field/number_field.py", line 87, in <module>
    import sage.rings.real_mpfi
  File "real_mpfi.pyx", line 1, in init sage.rings.real_mpfi
  File "mpfi.pyx", line 1, in init sage.rings.convert.mpfi
ImportError: /mnt/opt/Sage/sage-clang/build/cp312/src/sage/rings/complex_interval.cpython-312-x86_64-linux-gnu.so: undefined symbol: fmpz_tdiv_q_2exp

The latter symbol is from libflint, but complex_interval.cpython-312-x86_64-linux-gnu.so is not linked to libflint

@dimpase
Copy link
Member

dimpase commented Nov 10, 2024

in order for $ ./sage -tp --all to start, I had to do

diff --git a/src/sage/rings/finite_rings/meson.build b/src/sage/rings/finite_rings/meson.build
index e81724aa5c8..f9dae256d86 100644
--- a/src/sage/rings/finite_rings/meson.build
+++ b/src/sage/rings/finite_rings/meson.build
@@ -41,7 +41,7 @@ extension_data = {
 foreach name, pyx : extension_data
   deps = [py_dep, cysignals, gmp]
   if name == 'element_pari_ffelt'
-    deps += [cypari2]
+    deps += [cypari2, pari]
   elif name == 'residue_field_pari_ffelt'
     deps += [cypari2]
   endif
diff --git a/src/sage/rings/meson.build b/src/sage/rings/meson.build
index a06dd098c06..5ecdbeb00e0 100644
--- a/src/sage/rings/meson.build
+++ b/src/sage/rings/meson.build
@@ -124,13 +124,13 @@ extension_data = {
 foreach name, pyx : extension_data
   deps = [py_dep, cysignals, gmp]
   if name == 'complex_arb'
-    deps += [flint, mpfi]
+    deps += [flint, mpfi, gsl]
   elif name == 'complex_conversion'
     deps += [gsl, mpfr]
   elif name == 'complex_double'
     deps += [gmpy2, gsl]
   elif name == 'complex_interval'
-    deps += [mpfi]
+    deps += [mpfi, flint]
   elif name == 'complex_mpc'
     deps += [gmpy2, mpfr, mpc]
   elif name == 'complex_mpfr'

should I push it? I'm not 100% sure it's a correct fix.

@tobiasdiez
Copy link
Contributor Author

Thanks! This worked well. Tests are passing, except for one on python 3.11 - that looks unrelated.

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tobiasdiez
Copy link
Contributor Author

Thanks for the review!

vbraun pushed a commit to vbraun/sage that referenced this pull request Nov 13, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Make the dependency declaration in the meson build files more precise
and add a few missing dependencies for some files.

Extracted from sagemath#38872.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38913
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
vbraun pushed a commit to vbraun/sage that referenced this pull request Nov 14, 2024
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Make the dependency declaration in the meson build files more precise
and add a few missing dependencies for some files.

Extracted from sagemath#38872.

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [ ] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [ ] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38913
Reported by: Tobias Diez
Reviewer(s): Dima Pasechnik
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants