twisted / pydoctor

This is pydoctor, an API documentation generator that works by static analysis.
https://pydoctor.readthedocs.io
Other
181 stars 49 forks source link

Fix of pydoctor crash when creating symlink to index.html #809

Open rocketengineer1982 opened 1 month ago

rocketengineer1982 commented 1 month ago

When using pydoctor to create documentation for a single root module the symlink to index.html is created before index.html is created. This causes a crash when running pydoctor in Windows.

The creation of the symlink has been moved to a separate function that is called after index.html is created. This resolves the issue.

tristanlatr commented 1 month ago

Thanks a lot for the PR, would it be possible to add a unit test so we're sure the issue doesn't happen again? Thanks

rocketengineer1982 commented 1 month ago

After some wrangling with Windows I've got the unit test added and working.

unit_test_console.txt

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.57%. Comparing base (f3b1c12) to head (73d2120).

Files Patch % Lines
pydoctor/driver.py 50.00% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #809 +/- ## ========================================== - Coverage 92.59% 92.57% -0.02% ========================================== Files 47 47 Lines 8402 8408 +6 Branches 1853 1854 +1 ========================================== + Hits 7780 7784 +4 Misses 357 357 - Partials 265 267 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tristanlatr commented 1 month ago

I’de like to see a traceback in the CI to demonstrate the bug before I merge this.

If you have some time, could you open another PR that only adds a new test to demonstrate the crash, the test should be of the highest level possible - like calling driver.main() for instance. Then we merge that new branch into this one to verify the patch fixes the crash.

Tell me if you have time to do this, otherwise I’ll do it when I have a moment…

Thanks

rocketengineer1982 commented 1 month ago

After spending a few days not banging my head against this problem, I believe I misdiagnosed the issue.

It appears that I have insufficient permissions on the university system to create any symlinks. This then causes pydoctor (release version) to crash before it creates any of the individual files. I verified this by starting ipython and attempting to create symlinks to both files that do not exist and files that do exist.

PermissionError: [WinError 5] Access is denied: 'index.html' -> ........

As part of my monkeypatch I added a return to writeSummaryPages in writer.py

try:
    root_module_path.unlink()
    # not using missing_ok=True because that was only added in Python 3.8 and we still support Python 3.6
except FileNotFoundError:
    return
    pass
root_module_path.symlink_to('index.html')

When I duplicated my changes in the fork I forgot about the return, which was what actually allowed it to (mostly) work.

I have a single base module, "test_project". When pydoctor runs, the exception is triggered because "test_project.html" does not exist. The monkeypatched return statement triggers which prevents a crash when trying to create a symlink. The symlink from "test_project.html" to "index.html" is not created. The remaining individual documentation files (for each module and file) are created whether or not the symlink code has been moved to a new function.

I'm pretty sure I was able to a) generate test_project.html on the university system and b) reproduce the WinError 5 on my personal computer (followed by a WinError 1314 if I forgot to run pydoctor as administrator, as noted in https://github.com/twisted/pydoctor/issues/720#issuecomment-1650443994). However, I cannot confirm that test_project.html was ever generated on the university system (my original monkeypatch was overwritten), and after having to reinstall Anaconda on my personal computer I've been unable to reproduce a WinError 5.

Throws hands up in the air.

I would like to apologize for occupying your time with this.

I believe issue https://github.com/twisted/pydoctor/issues/808 can be closed. So can this pull request, unless you would still like the creation of the symlink to occur after "index.html" is created.

rocketengineer1982 commented 1 month ago

I had one further thought. Adding a return or moving where the symlink is created allows pydoctor to generate 99% of the documentation before it crashes due to a Windows permissions error (WinError 5 or WinError 1314).

tristanlatr commented 1 month ago

Thanks a lot for your in depth investigation of this issue. The matter is that it does make sense to work out of the box on windows with no need to be administrator. So I believe this PR is not totally misguided. If the symlink creation fails we might be able to create a hard link (which is basically a copy of the file) without any special privileges. Do you think it will solve the issue for all the cases ?

rocketengineer1982 commented 1 month ago

I believe a hard link would solve the issues.