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

doc: force UTF-8 on windows #35234

Conversation

mbolivar-nordic
Copy link
Contributor

Use '-X utf8' to force open() in text mode to use that encoding.

Unfortunately this fix is not available on Python 3.6, but this should
work on 3.7 or later.

Fixes: #34076

Signed-off-by: Martí Bolívar marti.bolivar@nordicsemi.no

Use '-X utf8' to force open() in text mode to use that encoding.

Unfortunately this fix is not available on Python 3.6, but this should
work on 3.7 or later.

Fixes: zephyrproject-rtos#34076

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

Sphinx offers source_encoding option which defaults to utf-8-sig, I suppose it is used internally when opening files. I'd prefer to not touch PYTHONOPTS and offer a solution in that direction.

@@ -264,7 +280,7 @@ add_custom_target(
PYTHONPATH=${ZEPHYR_BASE}/scripts/dts/python-devicetree/src${SEP}$ENV{PYTHONPATH}
ZEPHYR_BASE=${ZEPHYR_BASE}
GEN_DEVICETREE_REST_ZEPHYR_DOCSET=${GEN_DEVICETREE_REST_ZEPHYR_DOCSET}
${PYTHON_EXECUTABLE} ${GEN_DEVICETREE_REST_SCRIPT}
${PYTHON_EXECUTABLE} ${PYTHONOPTS} ${GEN_DEVICETREE_REST_SCRIPT}
Copy link
Member

Choose a reason for hiding this comment

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

#34076 may be fixed by just adjusting the encoding used by the DTS generator script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#34076 may be fixed by just adjusting the encoding used by the DTS generator script.

If we start that game of whackamole then we'll never stop; I'm trying to find a solution that doesn't require us to take special care everywhere.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's the script the responsible to handle the encoding and not CMake via Python option. As mentioned, Sphinx takes care of doing that when dealing with project sources, we should do the same. I think current CMake is convoluted enough to not add more flags that, by the way, are not available on all Python versions we support.

Copy link
Member

Choose a reason for hiding this comment

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

Another point: if any of the current scripts ends up being proper Sphinx extensions, we can't rely on -X utf8.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's fair for Zephyr scripts to expect to be run in a UTF-8 compatible way. A future question about converting to sphinx extensions seems like a hypothetical.

Copy link
Member

@gmarull gmarull May 13, 2021

Choose a reason for hiding this comment

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

Since the script knows that the files going to be handled are UTF-8 here, it's the responsible to read/write with the appropriate encoding. It's annoying that Windows doesn't default to utf-8...

There's another solution (not tested) that may not depend on having Python >= 3.7: set PYTHONUTF8=1 on CMake

edit: PYTHONUTF8 is also >= 3.7, https://vstinner.github.io/python37-new-utf8-mode.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To me this boils down to 2 things:

  • the reporter confirms this is a fix
  • we already have a precedent set in the Zephyr build system's python.cmake file that we are willing to add in Windows-specific workarounds

So this is the fix I would suggest. Feel free to file your own patch of course!

@mbolivar-nordic
Copy link
Contributor Author

Sphinx offers source_encoding option which defaults to utf-8-sig, I suppose it is used internally when opening files. I'd prefer to not touch PYTHONOPTS and offer a solution in that direction.

What do you mean by "touch PYTHONOPTS"? That's just a random variable name; it's not special to FindPython3 or anything.

@galak
Copy link
Collaborator

galak commented May 19, 2021

@gmarull ping

@mbolivar-nordic
Copy link
Contributor Author

obsoleted by #35470

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Documentation bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unrecognized characters generated during document construction
4 participants