Skip to content

Meson build: build dir is hardcoded in sage.config #39870

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

Closed
2 tasks done
antonio-rojas opened this issue Apr 4, 2025 · 2 comments · Fixed by #39885
Closed
2 tasks done

Meson build: build dir is hardcoded in sage.config #39870

antonio-rojas opened this issue Apr 4, 2025 · 2 comments · Fixed by #39885
Labels

Comments

@antonio-rojas
Copy link
Contributor

Steps To Reproduce

sage: from sage.env import SAGE_ROOT
sage: SAGE_ROOT
'/build/sagemath-git/src/sage/src/sage/../..'

SAGE_ROOT should be empty in sagelib except if explicitly specified (eg. by sage-the-distro). Otherwise things break badly for distro packages:

**********************************************************************
File "sage/src/sage/doctest/forker.py", line 1764, in sage.doctest.forker.DocTestDispatcher.serial_dispatch
Failed example:
    DC.expand_files_into_sources()
Exception raised:
    Traceback (most recent call last):
      File "/usr/lib/python3.13/site-packages/sage/doctest/forker.py", line 730, in _run
        self.compile_and_execute(example, compiler, test.globs)
        ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.13/site-packages/sage/doctest/forker.py", line 1154, in compile_and_execute
        exec(compiled, globs)
        ~~~~^^^^^^^^^^^^^^^^^
      File "<doctest sage.doctest.forker.DocTestDispatcher.serial_dispatch[8]>", line 1, in <module>
        DC.expand_files_into_sources()
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
      File "/usr/lib/python3.13/site-packages/sage/doctest/control.py", line 1048, in expand_files_into_sources
        self.sources = [FileDocTestSource(path, self.options) for path in expand()]
                                                                          ~~~~~~^^
      File "/usr/lib/python3.13/site-packages/sage/doctest/control.py", line 1045, in expand
        elif not skipfile(path, bool(self.options.optional),
                 ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                        if_installed=self.options.if_installed, log=self.log):  # log when directly specified filenames are skipped
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      File "/usr/lib/python3.13/site-packages/sage/doctest/control.py", line 322, in skipfile
        with open(filename) as F:
             ~~~~^^^^^^^^^^
    FileNotFoundError: [Errno 2] No such file or directory: '/build/sagemath-git/src/sage/src/sage/../../src/sage/rings/homset.py'
**********************************************************************

Expected Behavior

SAGE_ROOT is unset (so expand_files_into_sources() works properly)

Actual Behavior

See above

Additional Information

No response

Environment

  • OS: Arch Linux
  • Sage Version: 10.7.beta0

Checklist

  • I have searched the existing issues for a bug report that matches the one I want to file, without success.
  • I have read the documentation and troubleshoot guide
@Krishnadubey1008
Copy link
Contributor

@antonio-rojas please review the pr

@antonio-rojas
Copy link
Contributor Author

antonio-rojas commented Apr 5, 2025

The PR doesn't work, the env command reads the value of SAGE_ROOT from config.py, which is where the wrong value is set (also, setting it to the empty string wouldn't work anyway, it needs to be unset)

SAGE_ROOT is a sage-the-distro thing, it should not be set by sagelib. In fact, I don't think config.py should be installed by sagelib at all, it contains distribution-specific data that should be set by the distribution. sagelib should simply set sane defaults in env.py, as it has been doing so far.

vbraun pushed a commit to vbraun/sage that referenced this issue Apr 10, 2025
sagemathgh-39885: Provided a default value for SAGE_ROOT that can be overridden by an environment variable or configuration file
    
<!-- ^ 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". -->
This PR fixes sagemath#39870
added following code to `env.py`
```
DEFAULT_SAGE_ROOT = ""
SAGE_ROOT = var("SAGE_ROOT", DEFAULT_SAGE_ROOT)
```
to ensure that `SAGE_ROOT` is not hardcoded and can be dynamically set
or overridden
### 📝 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#39885
Reported by: Krishna Dubey
Reviewer(s): Antonio Rojas, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 13, 2025
sagemathgh-39885: Provided a default value for SAGE_ROOT that can be overridden by an environment variable or configuration file
    
<!-- ^ 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". -->
This PR fixes sagemath#39870
added following code to `env.py`
```
DEFAULT_SAGE_ROOT = ""
SAGE_ROOT = var("SAGE_ROOT", DEFAULT_SAGE_ROOT)
```
to ensure that `SAGE_ROOT` is not hardcoded and can be dynamically set
or overridden
### 📝 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#39885
Reported by: Krishna Dubey
Reviewer(s): Antonio Rojas, Tobias Diez
vbraun pushed a commit to vbraun/sage that referenced this issue Apr 18, 2025
sagemathgh-39885: Provided a default value for SAGE_ROOT that can be overridden by an environment variable or configuration file
    
<!-- ^ 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". -->
This PR fixes sagemath#39870
added following code to `env.py`
```
DEFAULT_SAGE_ROOT = ""
SAGE_ROOT = var("SAGE_ROOT", DEFAULT_SAGE_ROOT)
```
to ensure that `SAGE_ROOT` is not hardcoded and can be dynamically set
or overridden
### 📝 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#39885
Reported by: Krishna Dubey
Reviewer(s): Antonio Rojas, Tobias Diez
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants