Skip to content

Conversation

@kdschlosser
Copy link
Contributor

@kdschlosser kdschlosser commented Sep 17, 2024

This problem only occurs when compiling under windows. This is due to the default encoding for loading files being set to cp1252. To change this behavior we need to supply the proper encoding of utf-8

@vwheeler63

Give this a try and see if it corrects the build problem under Windows. It might need some tweaking to make it work correctly bit it should do the trick after any needed tweaks.


📚 Documentation preview 📚: https://writethedocs-www--2221.org.readthedocs.build/

This problem only occurs when compiling under windows. This is due to the default encoding for loading files being set to cp1252. To change this behavior we need to supply the proper encoding of utf-8
@plaindocs
Copy link
Contributor

Oh thanks @kdschlosser !

I don't think any of us can test this, but if @vwheeler63 can get it to work I'm happy to merge it in to improve the experience of folks using Windows!

@vwheeler63
Copy link
Collaborator

vwheeler63 commented Sep 17, 2024

Wow Kevin! ( @kdschlosser )

I'm honored that you would chip in to help me get this local build resolved. 😄

My Python 3.8 is now getting farther with conf.py, using the make html command, but we're not quite there yet. Now I am getting this:

Traceback (most recent call last):
  File "C:\Python\Python312\Lib\site-packages\sphinx\config.py", line 509, in eval_config_file
    exec(code, namespace)  # NoQA: S102
    ^^^^^^^^^^^^^^^^^^^^^
  File "E:\Dev\Clients\WGA\writethedocs\www\docs\conf.py", line 8, in <module>
    import ablog
ModuleNotFoundError: No module named 'ablog'

What is weird about it is that in the activated virtual environment I have set up, I did do the

pip install -r requirements.txt

step, and ablog is in the file, plus I re-verified that no other part of my conf.py has been altered, and 'ablog' string an element in the extensions array therein (the first element)! Just to be sure, with the virtual environment still active, I did

pip install ablog

No change. So I deactivated the virtual environment and did

pip install ablog

in case it needed to be in the root Python installation, and did it again. No change. And just to verify,

C:\Python\Python38\Scripts;C:\Python\Python38;...

is the first part of my PATH (alongside my other versions), and no other python version paths are present.

Any clues? I have my phone nearby if you wanna connect screens. I have the free version of TeamViewer if it helps.

Kind regards,
Vic

@vwheeler63
Copy link
Collaborator

P.S. @kdschlosser

I also looked in the local .\venv\Scripts\ and ablog.exe is there, and in .\venv\Lib\site-packages\, both expected directories are present:

09/17/2024  10:14    <DIR>          ablog
09/17/2024  10:14    <DIR>          ablog-0.10.33.post1.dist-info

@vwheeler63
Copy link
Collaborator

@kdschlosser I forgot to mention that I also tried adding the

    'sphinx.ext.intersphinx',

from the https://ablog.readthedocs.io/en/stable/ documentation, but it didn't change the results, so I took it out again.

@kdschlosser
Copy link
Contributor Author

delete the clone and clone it again and set up the virtual env again. Then give it a go and see what happens.

@kdschlosser
Copy link
Contributor Author

make sure you have both python in your path statement and also PYTHON_PATH set to point to your python installation.

@kdschlosser
Copy link
Contributor Author

ahh OK I see what is going on.... There is zero detection of installed python versions in the batch file. The batch file just calls "sphinx-build" and whatever {python}/Scripts folder is in the %PATH% variable first is the winner. I would check your path statement. Remember on Windows there are 2 of them....

@kdschlosser
Copy link
Contributor Author

There needs to be code like this...

@echo off
setlocal
setlocal enabledelayedexpansion

set "_version_major=0"
set "_version_minor=0"
set _found_python_path=""


for /f "tokens=1" %%a in ('REG QUERY HKEY_LOCAL_MACHINE\SOFTWARE\Python\PythonCore /s /f InstallPath ^| findstr "HKEY_LOCAL_MACHINE"') do (
  set _python_reg=%%a

  for /f "tokens=3" %%b in ('REG QUERY !_python_reg! /v ExecutablePath') do (
    set _python_path=%%b
  )

  for /f "tokens=5 delims=\" %%v in ("%%a") Do (
    set _version=%%v

    for /f "tokens=1,2 delims=." %%p in ("%%v") Do (
      set "_major=%%p"
      set "_minor=%%q"

      IF !_major! gtr !_version_major! (
        set "_version_major=%%p"
        set "_version_minor=%%q"
        set _found_python_path=!_python_path!
      ) ELSE (
        IF !_major!==!_version_major! IF !_minor! gtr !_version_minor! (
          set "_version_major=%%p"
          set "_version_minor=%%q"
          set _found_python_path=!_python_path!
        )
      )
    )
  )
)
echo !_found_python_path!
endlocal

in the batch file. This will detect whatever Python version is wanting to be used regardless of it being in the PATH. This is a better thing to use due to Python having an option on install if you want it to set the environment variables.

@kdschlosser
Copy link
Contributor Author

I would make it so that when detecting the Python version if 3.8 is found then it will use that and if it is not found then it will use the highest installed version.

Add a command line option to disable the detection and rely on whatever PATH is pointing to.

@vwheeler63
Copy link
Collaborator

There needs to be code like this...

So far I have simply NOT included ANY python path by default, and then when I need it, I run one of my batch files to put the appropriate directories at the head of my PATH for the version I want to use. It keeps my environment (e.g. PATH var) from getting kludged up.

You're a wiz with batch files! Did you just write that?

@kdschlosser
Copy link
Contributor Author

@plaindocs

I saw that you had said that most of the people that would compile this are not Windows users. However, you should be able to build on Windows considering Windows has ~75% market share.. That's a whole lot of possible contributors you are missing out on if the project is not able to be compiled on Windows.

@kdschlosser
Copy link
Contributor Author

You're a wiz with batch files! Did you just write that?

It's funny because same exact issue came up when dealing with the ESP32 SDK on Windows and I wrote that because they were using where to detect python installations which relies mostly on what is in the PATH variable.

Funny the same thing is also happening here as well.

@vwheeler63
Copy link
Collaborator

It's funny because same exact issue came up when dealing with...

Fascinating. Well, you did a thorough job!

Me: I used to be (1994-95-ish) a Unix administrator, so I'm pretty careful about where things go, and I periodically de-kludge my PATH, and followed a tip from someone on stackoverflow about not putting ANY python paths in PATH and only include them when needed. Which is how I am set up.

@vwheeler63
Copy link
Collaborator

@plaindocs Hi, Sam. I don't know how to evaluate the failing check (links-internal), but we haven't resolved the build issue yet, so maybe it will resolve.

@vwheeler63
Copy link
Collaborator

vwheeler63 commented Sep 17, 2024

@kdschlosser Kevin: is that new version on ablog in requirements.txt supposed to resolve my build issue -- where conf.py is reporting "import ablog" no such module? If so, do I need to re-make my virtual environment? Or are you still working on it?

@vwheeler63
Copy link
Collaborator

make sure you have both python in your path statement and also PYTHON_PATH set to point to your python installation.

Oops. I missed this one. Let me give that a try. (However, without it, I don't get that "no such module" for any other of my installed modules....)

@vwheeler63
Copy link
Collaborator

@kdschlosser Ohhhh.... I figured out why....

@kdschlosser
Copy link
Contributor Author

why?

I updated the make.bat file to target a python 3.8 install and I also added the proper command to run sphinx build using {python-path}\python.exe -m sphinx_build which will use the sphinx_build from the scripts directory wherever it is located.

@vwheeler63
Copy link
Collaborator

@kdschlosser I owe you an apology: just today I added a SPHINXBUILD environment variable and it was pointing to Python312. I eliminated it again (was only needed in a GitBash environment where it was actually using the Makefile instead of make.bat). Now SPHINX is actually starting and getting well into the build without the modification of the version.

Now I am getting E:\Dev\Clients\WGA\writethedocs\www\docs\about\learning-resources.rst:5: WARNING: toctree contains reference to nonexisting document 'videos/index'. The good thing is: it's no longer choking on the yaml utf-8 characters!

Nice work. Now let's see if I can resolve the missing files, or work around them.

Could you kindly:

  1. undo the change in requirements.txt, and
  2. rebase your branch fix-encoding-problem-on-windows on the latest master? (Evidently this repository requires the branch be an immediate "offspring" of master for it to be accepted. Otherwise, we get:

image

Kind regards,
Vic

@vwheeler63
Copy link
Collaborator

vwheeler63 commented Sep 17, 2024

@kdschlosser I tried testing your MAKE.BAT and there appear to be 2 things wrong with it:

  1. One ERROR: The system was unable to find the specified registry key or value because Python 2.7 does not have the same registry key structure as Python 3.x does.
  2. On my system, sphinx-build does not exist as a .py file -- only an .exe, so sphinx-build does not execute at all.

That said, let's back up a couple of paces and make sure we're addressing the right problem.

Background info: this repository's README cleverly tells the user how to set up a virtual environment which already correctly handles the Python version and installed packages forever more, and due to that, the make.bat file needs no changes. I feel it is the user's responsibility to follow those instructions. (I violated that with my SPHINXBUILD environment variable, so I removed it, and that was the correct handling for it not finding the ablog module.) Once the user gets that right, there is no reason for the batch file to also try to reach out to a different python environment (from the registry, which will try to access the the system-wide python installation instead of the virtual environment, which is what the user is supposed to be using). And I don't think mixing Python environments is the right answer here. Would you agree?

However, the yaml-utf-8 problem WAS the right target, and it does seem that you nailed that in the conf.py file as far as I can tell.... Now I am running into something else....

With make.bat in its original form, Sphinx is currently getting 31% through the first pass of the build. Then I get a WARNING followed by an error:

E:\Dev\Clients\WGA\writethedocs\www\docs\about\learning-resources.rst:5: WARNING: toctree contains reference to nonexisting document 'videos/index'

Extension error (_ext.core):
Handler <function render_rst_with_jinja at 0x000001F2445E61F0> for event 'source-read' threw an exception (exception: Invalid format string)

Unfortunately, that's all the detail it generated. No logfiles in TMPDIR.... I can ignore the WARNING at the moment, but the error is blocking the build.

Q1: Is it building for you on your system?

Q2: Any insights on that error?

Kind regards,
Vic

@vwheeler63
Copy link
Collaborator

@kdschlosser P.S. I do agree about using setlocal and endlocal to keep the user's environment from being altered. Be advised, the rest of that make.bat file uses leading TABS, not spaces, so if you submit an alteration, it should be consistent with the existing scheme (leading tabs).

@kdschlosser
Copy link
Contributor Author

I didn't even check the spacing. I wanted you to test it to see if it works properly before spending any time to beautify it.

@kdschlosser
Copy link
Contributor Author

I am going to kill this PR and create a new one. I want to keep the PR clean and not have it jumbled up with a bunch of commits and then reversing the commits.

@vwheeler63
Copy link
Collaborator

I am going to kill this PR

Alright.

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.

3 participants