Skip to content

Conversation

dimpase
Copy link
Member

@dimpase dimpase commented Sep 10, 2025

after switch to meson doc-build, SAGE_DOC points to a wrong location,
as reported on sage-devel. We fix it here

📝 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

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 10, 2025

The solution should be making meson install the doc into the "old" default location, not changing the default location to where meson installs the doc. I think that changing the "old" default location is an "API change".

@dimpase
Copy link
Member Author

dimpase commented Sep 10, 2025

it's not an API change: the user doesn't need to worry about the value of$SAGE_DOC.

@antonio-rojas
Copy link
Contributor

antonio-rojas commented Sep 10, 2025

The solution should be making meson install the doc into the "old" default location, not changing the default location to where meson installs the doc.

Completely agree. This change is sage-the-distro-centric and breaks the default setting for distro packaging. Nothing in sagelib should reference or depend on any sage-the-distro specific paths.

@tobiasdiez
Copy link
Contributor

@antonio-rojas how do you make the reference() method (src/sage/misc/sagedoc.py) work in Arch, assuming the user has installed the sagemath-doc package?

@antonio-rojas
Copy link
Contributor

@antonio-rojas how do you make the reference() method (src/sage/misc/sagedoc.py) work in Arch, assuming the user has installed the sagemath-doc package?

I don't have to do anything special. SAGE_DOC is currently join(SAGE_SHARE, "doc", "sage") by default, which translates to /usr/share/doc/sage on Arch, so everything works out of the box if I install the docs there. This change would require me to patch the downstream packages (SAGE_ROOT is not even defined outside sage-the-distro).

Ideally meson would provide an install-docs target that installs the built docs in their expected location.

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 10, 2025

Ideally meson would provide an install-docs target that installs the built docs in their expected location.

I agree. The expected location is provided by SAGE_DOC environment variable while sage-the-distro sets SAGE_DOC (via SAGE_SHARE) to its default locations.

@dimpase
Copy link
Member Author

dimpase commented Sep 10, 2025

The solution should be making meson install the doc into the "old" default location, not changing the default location to where meson installs the doc.

Completely agree. This change is sage-the-distro-centric and breaks the default setting for distro packaging. Nothing in sagelib should reference or depend on any sage-the-distro specific paths.

is SAGE_DOC considered part of sagelib? (I don't consider it this way).

I would say that the content of $SAGE_LOCAL and anything there is sage-distro, and the old convention of having docs there is a sage-distro convention.

From my point of view this PR is a hotfix for reference()

@antonio-rojas
Copy link
Contributor

is SAGE_DOC considered part of sagelib? (I don't consider it this way).

As long as sagelib uses it (as it currently does in src/sage/misc/sagedoc.py), then it should be considered so.

From my point of view this PR is a hotfix for reference()

A hotfix for one use case should not break other use cases (and even CI)

@dimpase
Copy link
Member Author

dimpase commented Sep 10, 2025

is SAGE_DOC considered part of sagelib? (I don't consider it this way).

As long as sagelib uses it (as it currently does in src/sage/misc/sagedoc.py), then it should be considered so.

I hear conflicting views on whether sage.env (used by src/sage/misc/sagedoc.py to get SAGE_DOC)
is even a part of sagelib.

Before we converge on a solution, we need to set up the rules on what the acceptable solution is.

As we have it now, src/bin/sage-env sets SAGE_DOC completely disjointly from sage.env.
(and src/bin/sage-env is not a part of sagelib, is it?)
This is wrong, from sage-distro point of view, assuming that sage.env is a part of sagelib rather than sage the distro. A reason for the latter view, that sage.env is not a part of sagelib, might be that it is used for
installing at least one spkg: database_odlyzko_zeta.

From my point of view this PR is a hotfix for reference()

A hotfix for one use case should not break other use cases (and even CI)

sorry, this PR has broken an unwritten, non-official, rule that SAGE_DOC is a particular function of SAGE_SHARE, that's all it broke, isn't it?

As long as we don't have a coherent policy describing where the border between sagelib and sage the distro is, we are bound to have these kinds of SNAFUs.

@antonio-rojas
Copy link
Contributor

antonio-rojas commented Sep 10, 2025

I hear conflicting views on whether sage.env (used by src/sage/misc/sagedoc.py to get SAGE_DOC) is even a part of sagelib.

There is a package in sage-the-distro called literally sagelib. I don't think this leaves much room for interpretation.

As we have it now, src/bin/sage-env sets SAGE_DOC completely disjointly from sage.env. (and src/bin/sage-env is not a part of sagelib, is it?) This is wrong, from sage-distro point of view, assuming that sage.env is a part of sagelib rather than sage the distro. A reason for the latter view, that sage.env is not a part of sagelib, might be that it is used for installing at least one spkg: database_odlyzko_zeta.

src/bin/sage-env is completely irrelevant after #39030. It used to be sourced from the sage executable, but in meson times this executable is a python entry point and src/bin/sage-env is nowhere used at runtime. But even before #39030, this duplication made sense: SAGE_DOC in sage.env defined the (distro-agnostic) fallbacks to be used if the SAGE_DOC env variable is undefined. SAGE_DOC in src/bin/sage-env defined the proper value for sage-the-distro.

sorry, this PR has broken an unwritten, non-official, rule that SAGE_DOC is a particular function of SAGE_SHARE, that's all it broke, isn't it?

It broke the sane fallback that was set in sage.env for the case where SAGE_DOC was undefined, replacing it by something that only works in sage-the-distro, and forcing all other distributions to explicitly set that variable (either by patching or by writing a wrapper, neither of which is desirable).

Anyway: if you want a quick and dirty fix, then just add the new fallback path to var("SAGE_DOC",...), but please don't remove the previous one (and do it after SAGE_ROOT is defined so sage doesn't just crash at startup).

@@ -231,7 +231,7 @@ if [ -n "$SAGE_LOCAL" ]; then
export SAGE_SPKG_INST="$SAGE_LOCAL/var/lib/sage/installed" # deprecated
fi
if [ -n "$SAGE_SHARE" ]; then
export SAGE_DOC="$SAGE_SHARE/doc/sage"
export SAGE_DOC="$SAGE_ROOT/build/sage-distro/src/doc"
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to check for SAGE_SHARE being defined anymore.

@dimpase
Copy link
Member Author

dimpase commented Sep 10, 2025

AFAIK, anything which an external build tool will set/supply, such as SAGE_DOC, should be declared in "dynamic" subsection of [project] in pyproject.toml

Can we prepare a list to put there? So far, only "version" is there.

@tobiasdiez

@tobiasdiez
Copy link
Contributor

AFAIK, anything which an external build tool will set/supply, such as SAGE_DOC, should be declared in "dynamic" subsection of [project] in pyproject.toml

It's not really set as an env variable but embedded into config.py via

conf_data.set('SAGE_SHARE', datadir)

which usually results into SAGE_SHARE = "/usr/share" (using the built-in datadir option of meson) and thus SAGE_DOC = "/usr/share/doc/sage" (via the default in sage.env).

So that also explains why it's currently working for Arch and other Linux distros that install the docs in the standard way (thanks @antonio-rojas for the explanation)

@dimpase
Copy link
Member Author

dimpase commented Sep 11, 2025

It's not really set as an env variable but embedded into config.py

how is this done for the sagelib's version (currently the only member of "dynamic" in [project])? I can't find the place it's dealt with by meson.

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 11, 2025

which usually results into SAGE_SHARE = "/usr/share" (using the built-in datadir option of meson) and thus SAGE_DOC = "/usr/share/doc/sage" (via the default in sage.env).

Then for sage-the-distro (or for those who are not a root user), SAGE_SHARE can be imported from sage.env to src/sage/meson.build (or src/doc/meson.build)?

@tobiasdiez
Copy link
Contributor

tobiasdiez commented Sep 11, 2025

It's not really set as an env variable but embedded into config.py

how is this done for the sagelib's version (currently the only member of "dynamic" in [project])? I can't find the place it's dealt with by meson.

If you specific the version as dynamic, then meson-python uses the version specified in the meson.build file (which is read from VERSION.txt).

which usually results into SAGE_SHARE = "/usr/share" (using the built-in datadir option of meson) and thus SAGE_DOC = "/usr/share/doc/sage" (via the default in sage.env).

Then for sage-the-distro (or for those who are not a root user), SAGE_SHARE can be imported from sage.env to src/sage/meson.build (or src/doc/meson.build)?

Not sure I quite understand the question. sage.env uses the info in config.py to calculate SAGE_SHARE that it exports. So, no, meson cannot query sage.env to set SAGE_SHARE.

It also works without any problem for non-root installs. For example, in the sage-dev conda env you will get something like

SAGE_SHARE = "<path to conda>/envs/sage-dev/share"

probably a similar path in a venv (although I don't have one lying around to test it). Of course, SAGE_DOC will also not work in these situations because the docs are not installed there...

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 11, 2025

As far as I understand, in sage-the-distro, SAGE_DOC defined in src/sage/env.py is read from the environment set by src/bin/sage-env at runtime, which computes the variables based on SAGE_LOCAL environment variable, which is set by src/bin/sage-env-config generated (perhaps by ./configure) from src/bin/sage-env-config.in, which sets SAGE_LOCAL to @prefix@, which is by default the same with the root build directory and can be customized by ./configure --prefix.

Hence meson may use src/bin/sage-env-config to compute SAGE_DOC (by the same formula in src/bin/sage-env) for sage-the-distro to find the doc install location.

@tobiasdiez
Copy link
Contributor

I'm sorry, but I cannot say anything intelligent about this - I never was able to understand sage-env deep enough and are happy that sagelib is not using it anymore.

(It would probably worth moving sage-the-distro stuff from src/bin to build to reduce the source of potential misunderstanding.)

@kwankyu kwankyu mentioned this pull request Sep 12, 2025
5 tasks
@kwankyu
Copy link
Collaborator

kwankyu commented Sep 12, 2025

Ideally meson would provide an install-docs target that installs the built docs in their expected location.

#40807 solves the problem along that line. Now needs review.

@dimpase
Copy link
Member Author

dimpase commented Sep 15, 2025

closing this in favour of #40807

@kwankyu
Copy link
Collaborator

kwankyu commented Sep 15, 2025

Thanks.

@dimpase dimpase deleted the fix_SAGE_DOC branch September 15, 2025 23:56
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 this pull request may close these issues.

4 participants