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

force C locale in docbuild and remove some obsolete stuff #30002

Closed
dimpase opened this issue Jun 27, 2020 · 21 comments
Closed

force C locale in docbuild and remove some obsolete stuff #30002

dimpase opened this issue Jun 27, 2020 · 21 comments

Comments

@dimpase
Copy link
Member

dimpase commented Jun 27, 2020

currently WARNINGs are treated as errors, and we filter out harmless ones. Since Sphinx 3.0, docbuilds are localised, and fail if locale is e.g. French, cf.

https://groups.google.com/d/msg/sage-release/7wBxNRbJaaU/hzptnsQpCQAJ

Currently we have the list of these warnings maintained in src/sage_setup/docbuild/sphinxbuild.py.

Setting LANG=C provides a workaround.

However, this is far from optimal. At some point sphinx had ignore_warnings: cfelder/sphinx@2600eef#diff-639ce3a52583644f64e3dce953a93a39

which then got renamed to suppress_warnings (but still only allows to suppress a subset of warnings, it appears)

CC: @jhpalmieri @heluani

Component: documentation

Author: Dima Pasechnik

Branch/Commit: u/dimpase/build/setlocale_to_C_docbuild @ cf52d9a

Reviewer: Matthias Koeppe

Issue created by migration from https://trac.sagemath.org/ticket/30002

@dimpase dimpase added this to the sage-9.2 milestone Jun 27, 2020
@dimpase

This comment has been minimized.

@antonio-rojas
Copy link
Contributor

comment:2

I think this is what you're looking for sphinx-doc/sphinx@ae9d786

@dimpase
Copy link
Member Author

dimpase commented Jun 27, 2020

comment:3

current sphinx has suppress_warnings

@EmmanuelCharpentier
Copy link
Mannequin

EmmanuelCharpentier mannequin commented Jun 27, 2020

comment:4

FWIW

LANG=C.UTF-8 make succeeds.

With qualification : after this (successful) build of the documentation, make in an environment where LANG is fr_FR.utf-8 fails. LANG=C.UTF-8 make succeeds.

In other words, any use of make entailing an attempt to make/check the domumentation will fail if LANG amounts to french...

@dimpase
Copy link
Member Author

dimpase commented Jun 27, 2020

comment:5

we can force locale in the Makefile, but this is a hack. I would like an intelligent suppression of warnings, rather than the current scraping on the output.

@dimpase
Copy link
Member Author

dimpase commented Jun 28, 2020

comment:6

this should do the job, but we should have also have a long hard look at the mess of favicon and other warnings. Sphinx people say that

[reference] WARNING: l'entrée html_static_path '/home/dimpase/sage/src/doc/common/static' n'existe pas
[reference] WARNING: l'entrée html_static_path 'static' n'existe pas
[reference] WARNING: le fichier de favicon 'favicon.ico' n'existe pas

is an indication of an incorrect configuraion.

There are also still some apparent leftovers of sagenb, e.g. stuff in src/sage/ext_data/images/ - all of it added by commit 0f0517c0c90:

commit 0f0517c0c90716340239eb348757449d1e36d194
Author: agc <agc@kubuntu>
Date:   Sun Dec 10 20:39:48 2006 -0800

    Added a dir "images", and put in several static images for the notebook.

and never touched ever since.


New commits:

e959d56workaround to allow non-EN locales for docbuild

@dimpase
Copy link
Member Author

dimpase commented Jun 28, 2020

@dimpase
Copy link
Member Author

dimpase commented Jun 28, 2020

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Jun 28, 2020

Commit: e959d56

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

cf52d9aget rid of old data added by commit 0f0517c0c90

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 28, 2020

Changed commit from e959d56 to cf52d9a

@dimpase dimpase changed the title force C locale in docbuild force C locale in docbuild and remove some obsolete stuff Jun 28, 2020
@dimpase

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Jun 28, 2020

Reviewer: Matthias Koeppe

@heluani
Copy link

heluani commented Jun 29, 2020

comment:12

Nevermind, I had not applied it yet. Sorry for the noise.

In my system this patch results in a lot of warnings:

bash: warning: setlocale: LC_ALL: cannot change locale (C.UTF-8)

I suppose this is because of

$ locale -a
C
en_US.utf8
POSIX

@dimpase
Copy link
Member Author

dimpase commented Jun 29, 2020

comment:13

does this break anything?

it could be that your locale setup is faulty - what kind of OS is this?

@heluani
Copy link

heluani commented Jul 2, 2020

comment:14

Replying to @dimpase:

does this break anything?

it could be that your locale setup is faulty - what kind of OS is this?

I missed this message, it does not break anything that I can see, it's arch linux, I only generate those locales by default:

$ grep -v '^#' /etc/locale.gen
en_US.UTF-8 UTF-8

@antonio-rojas
Copy link
Contributor

comment:16

Replying to @heluani:

In my system this patch results in a lot of warnings:

bash: warning: setlocale: LC_ALL: cannot change locale (C.UTF-8)

Those warnings are unrelated to this ticket, they are caused by #29033

@dimpase
Copy link
Member Author

dimpase commented Jul 3, 2020

comment:17

ok, I see. Should we fix it here, as #29033 is closed?

@antonio-rojas
Copy link
Contributor

comment:18

Replying to @dimpase:

ok, I see. Should we fix it here, as #29033 is closed?

It really has nothing to do with the change here, but the branch already contains another unrelated change so up to you, I guess.

@dimpase
Copy link
Member Author

dimpase commented Jul 3, 2020

comment:19

Replying to @antonio-rojas:

Replying to @dimpase:

ok, I see. Should we fix it here, as #29033 is closed?

It really has nothing to do with the change here, but the branch already contains another unrelated change so up to you, I guess.

let us do this on #30053

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 8, 2020

comment:20

These have been merged into 9.2.beta4

@mkoeppe mkoeppe closed this as completed in 1e9f325 Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants