docs: Added fix for building on Windows and resolved other build errors

[munsterlander]

Description

@eukleshnin identified issues with building the documents locally on a Windows workstation. Namely, the following was occurring:

reading sources... [  2%] flame/layout/align_component
Exception occurred:

FileNotFoundError: [WinError 2]

This was determined to be a combination of several things.

1. In dart_domain.py, the subprocess.run calling dartdoc_json did not have the shell parameter set to true. This solves the error about the file not being found. This then generates errors that subsequent references to the temp_file did not exist.
2. This was due to the default setting with Python tempfile where when it determines the temp file has been closed, it deletes it; however, it was still needed, so by setting the delete=False parameter, the file would still remain.
3. Unfortunately, because it still remains, it needs to be deleted once it is no longer needed. Trying to use finally: with the try block failed to produce the results desired, so the temp file name was registered with the class so it can be deleted in the original calling function if it exists. This proved successful.
4. Although not critical, the same temp file uses a suffix of json so it was creating files xxxxjson. By adding the "." in the suffix, it creates valid file names now. This doesn't actually fix anything, it just seemed wrong, so I fixed it to be valid files if ever needed down the road.

Now that the docs built, there were several warnings that could be resolved:

1. Overlays.md was not referenced in the TOC tree.
2. Since overlays were removed in docs: Break out overlays docs #2384, the Platformer tutorial had a link to the old path and it needed to be updated.

Finally and open for discussion, during this process of debugging, I upgraded all packages to the most current to see what impacts there were. The following is the old and new potential requirements.txt:

----Old
linkify-it-py==2.0.0
myst-parser==0.18.1
Pygments==2.12.0
Sphinx==5.0.2
sphinxcontrib-mermaid==0.8.1
sphinx-autobuild==2021.3.14
jinja2==3.1.2

----- New
linkify-it-py==2.0.0
myst-parser==1.0.0
Pygments==2.14.0
Sphinx==6.1.3
sphinxcontrib-mermaid==0.8.1
sphinx-autobuild==2021.3.14
Jinja2==3.1.2

The only byproduct of this upgrade was a deprecated package warning for attrs_image in conf.py which was updated to attrs_inline. I made that change initially for this PR, but backed it out as I didn't know if there was a desire to update the requirements.txt and felt some discussion may be warranted.

Regardless, with everything upgraded or left as is, the other fixes resolve the issues on Windows.

Checklist

• I have followed the Contributor Guide when preparing my PR.
• I have updated/added tests for ALL new/updated/fixed functionality.
• I have updated/added relevant documentation in docs and added dartdoc comments with ///.
• I have updated/added relevant examples in examples or docs.

Breaking Change?

• Yes, this PR is a breaking change.
• No, this PR is not a breaking change.

Related Issues

[st-pasha]

It looks like temp_file is only really needed on lines 176-183, and can be deleted afterwards. Which means you can add a finally block for try: on line 174, and have the file deleted within that block -- then the file name can remain a local variable within _scan_source_file(), and doesn't need to become an instance member.

[munsterlander]

It looks like temp_file is only really needed on lines 176-183, and can be deleted afterwards. Which means you can add a finally block for try: on line 174, and have the file deleted within that block -- then the file name can remain a local variable within _scan_source_file(), and doesn't need to become an instance member.

That is what I tried initially, hence why I called it out in the PR description (3). When you do the finally and delete of the file, you get an error in the JSON decoder. I presume because when you call delete, it then deletes the JSON from memory which is then being set in the parent function. I presume that when finally gets called it overwrites the return and thus you have the issue. It likely works for Unix but not on windows - not certain.

A fairly comprehensive explanation is here: https://stackoverflow.com/questions/19805654/python-try-finally-block-returns.

[st-pasha]

Here's the docs for tempfile.NamedTemporaryFile:

This function operates exactly as TemporaryFile() does, except that the file is guaranteed to have a visible name in the file system (on Unix, the directory entry is not unlinked). That name can be retrieved from the .name attribute of the returned file-like object. Whether the name can be used to open the file a second time, while the named temporary file is still open, varies across platforms (it can be so used on Unix; it cannot on Windows). If delete is true (the default), the file is deleted as soon as it is closed.

So, I think the best approach would be to:

• add delete=False parameter to NamedTemporaryFile constructor;
• close the file immediately (as first statement inside with);
• delete the file within the finally: block.

[munsterlander]

Ok, so these commands are not executing in the way I would think they should:

# In try:
temp_file.close # no error occurs
temp_file.close() # no error occurs

# In finally:
temp_file.delete # does not delete the file
temp_file.delete() # throws TypeError: 'bool' object is not callable

I have tried all combinations and yeah, no go. My Python is not the strongest here.

Full Code:

def _scan_source_file(self):
        with tempfile.NamedTemporaryFile(mode='rt', suffix='.json', delete=False) as temp_file:
            try:
                #self.temp_file_path = temp_file.name
                temp_file.close
                subprocess.run(
                    ['dartdoc_json', self.source_file, '--output', temp_file.name],
                    stdout=subprocess.PIPE,
                    stderr=subprocess.STDOUT,
                    check=True,
                    shell=True,
                )
                json_string = temp_file.read()
                return self._extract_symbol(json_string)
            except subprocess.CalledProcessError as e:
                cmd = ' '.join(e.cmd)
                raise RuntimeError(
                    f'Command `{cmd}` returned with exit status'
                    f' {e.returncode}\n{e.output.decode("utf-8")}'
                )
            finally:
                temp_file.delete

[st-pasha]

Right, so:

• temp_file.close is just a function object, you need () to call this method.
• temp_file.delete is the "delete" property passed in the constructor, so this is just a boolean value False. This cannot be called (there is no such method as temp_file.delete()).
• in order to delete a file you can say os.remove(temp_file.name).

[munsterlander]

Make sense. Updated and ran:

  File "C:\Python39\lib\tempfile.py", line 474, in func_wrapper
    return func(*args, **kwargs)
ValueError: I/O operation on closed file.

Eliminating the close gets you:

  File "C:\Users\munsterlander\AndroidStudioProjects\flame\doc\_sphinx\extensions\dart_domain.py", line 193, in _scan_source_file
    os.remove(temp_file.name)
PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\munsterlander\\AppData\\Local\\Temp\\tmpe87i6lol.json'

These are the errors that I was getting earlier today when I was solving it and which led to just putting the name in the class.

[st-pasha]

Ah, I guess reading the file now needs an explicit open:

with open(temp_file.name, 'r') as t:
    json_string = t.read()

[munsterlander]

So let me just make sure I have this straight, create a temp file, then immediately close it, pipe data to it, reopen it to read it to put the contents into a var to be returned, and on exit delete the temp file.

As crazy as that seems, it worked.

Edit: Just went back and confirmed it was deleting the files from the temp directory.

[st-pasha]

Yes, that's correct:

1. Create a temporary file and automatically open it.
2. Realize that we don't need it open (because Windows), so close the file.
3. Let an external script dartdoc_json write into the file.
4. NOW we can open the file again and read its contents.
5. Parse the file contents and return the result.
6. Oh, and the finally block ensures that the file gets deleted in the end, regardless of whether we return normally or there was an exception.

[munsterlander]

Crazy and I had tried numerous variances, but didn't think to close and read back to memory. Anyway, all the code has been updated and I appreciate the help!

I don't know if you saw my comments on your already merged PR for melos doc-setup, but it's a similar windows issue thing. The command is there and working, just needs some tweaks to pass the formatter.

[munsterlander]

Just added another fix for melos doc-serve. @eukleshnin observed that while doc-build worked, doc-serve still generated 404 errors. Looking into it, doc-serve was building the docs under the build directory and not under build\html as expected. The make.bat and Makefile had different paths, so this resolves that.

Sphinx docs discuss the defaults for autobuild: https://pypi.org/project/sphinx-autobuild/
and for regular build here: https://www.sphinx-doc.org/en/master/man/sphinx-build.html.

Additionally, I have added the flags that currently exist under the Makefile so they both match.