| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
A downside of how I currently have it is that it need the entire docs job to finish (i.e. it includes waiting for the doc tests), we could have it run earlier by moving it under the docs job (though I don't think that is particularly nice), however since it runs in 7 seconds I think it is a negligible issue. |
Sorry, something went wrong.
|
@webknjaz could you please take a peek at this? |
Sorry, something went wrong.
There was a problem hiding this comment.
Should this new job also be required by "All required checks pass"?
https://github.com/python/cpython/actions/runs/20930506392?pr=143742
A downside of how I currently have it is that it need the entire docs job to finish (i.e. it includes waiting for the doc tests), we could have it run earlier by moving it under the docs job (though I don't think that is particularly nice), however since it runs in 7 seconds I think it is a negligible issue.
The time isn't a problem, but there's quite a bit of extra complexity and config to achieve this, when it could be ~10 extra lines to add a couple of steps to the existing docs job.
Sorry, something went wrong.
There was a problem hiding this comment.
Could this stay in Lib/idlelib/help.py?
Sorry, something went wrong.
There was a problem hiding this comment.
We would have to build CPython if we want to be able to modify it, that's not particularly fast?
Sorry, something went wrong.
There was a problem hiding this comment.
I don't understand the purported need to move copy_strip (and fairly strongly prefer not to). What is 'it'? I cannot imagine a need to modify the code just to check the file modification state.
I am not sure about adding a new make target. Currently, I update help.html (either in a .bat or manually) with
python -c "from idlelib.help import copy_strip; copy_strip()" in the repository, so that 'python' is the repository build and the update happens in the repository. I don't know if there is the equivalent for non-Windows, but copy pasting this command into command prompt is easier than changing directories and invoking make. I would personally prefer a reminder to run the above than to find make.
Sorry, something went wrong.
There was a problem hiding this comment.
If we don’t want to move, currently it is useless where it is with a system python, as the paths are relative to the location of the installed idlelib, and the Doc folder is not in /usr/lib/ or somewhere like that.
We could update the function, but then we would have to wait for the next 3.14 release (and then for actions to pick it up) before we could it.
The other option is building CPython, but that would just make the check over 50x times slower.
Sorry, something went wrong.
Terry asked me to not make it required on the issue. |
Sorry, something went wrong.
Co-authored-by: Hugo van Kemenade <1324225+hugovk@users.noreply.github.com>
There was a problem hiding this comment.
This is getting messy, so lets back up. IDLE's objective is to make an offlline frozen copy of its doc available. At one time, the Windows installer installed propriety .chm files derived from the .html files. IDLE's F1 brought up the python .chm index. I believe that this was true at the time the IDLE doc display, using the help.html copy, was added.
Currently, the Windows installer installs the Doc/build/html tree beside python.exe and F1 displays index.html. Ditto for macOS, though the location is more obscure. In either case, Modules/idlelib displays idle.html. So on Windows and Mac, the local copy is not needed. I don't know what the situation is on Linux and *nix in general, but I have the impression that at least some distributions also include the docs with Python. So maybe we can make this issue moot.
EDIT: I could look for the F1 code to see what it does on *nix, but I need sleep.
Sorry, something went wrong.
| .PHONY: idlehelp | ||
| idlehelp: build/html/library/idle.html | ||
| $(PYTHON) ../Tools/build/generate_idle_help.py | ||
|
|
There was a problem hiding this comment.
Is it the case that make can run Python with a file argument but not a -c "code" argument?
Sorry, something went wrong.
There was a problem hiding this comment.
No, make can handle it.
Sorry, something went wrong.
| with: | ||
| python-version: '3' | ||
| - name: 'Regenerate Lib/idlelib/help.html' | ||
| run: make idlehelp |
There was a problem hiding this comment.
It appears that rather than compare last change dates you are making a potentially revised file and looking for a non-empty diff (but not committing). The latter seems much harder. Is yaml unable to do the former?
Sorry, something went wrong.
There was a problem hiding this comment.
Are you suggesting we simply fail if idle.rST is modified?
Sorry, something went wrong.
There was a problem hiding this comment.
FTR, if the only check we want here is comparing last edit dates of two files, I'd place it in makefile and would drop that separate step. Though, the semantics of such a check is unclear — the timestamps are likely to differ by seconds unless there's an automation that sets these to the same value. It seems like the only reliable check would be whether their last change happened in the same Git commit, if I understand the implied relation between the two files.
Sorry, something went wrong.
There was a problem hiding this comment.
I don't understand the purported need to move copy_strip (and fairly strongly prefer not to). What is 'it'? I cannot imagine a need to modify the code just to check the file modification state.
I am not sure about adding a new make target. Currently, I update help.html (either in a .bat or manually) with
python -c "from idlelib.help import copy_strip; copy_strip()" in the repository, so that 'python' is the repository build and the update happens in the repository. I don't know if there is the equivalent for non-Windows, but copy pasting this command into command prompt is easier than changing directories and invoking make. I would personally prefer a reminder to run the above than to find make.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request. |
Sorry, something went wrong.
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: idle-html | ||
| path: Doc/build/html/library/idle.html |
There was a problem hiding this comment.
I'd prefer passing Doc/build/html/library/ between the jobs too: an artifact is an abstract'ish GHA concept for "some/arbitrary files stored for later use" and the path is where to put it when downloading+uploading. But together, the the pair of artifact ID+path to unpack it, is something that shouldn't be maintained in two disconnected contexts w/ an implicit expectation that somebody will keep them in sync.
Though, this would require another step to store it into an output and reuse here + workflow outputs. We could probably pack both into the same output as a JSON string so they aren't separated in refactorings...
Sorry, something went wrong.
Some Linux distributions do (often not installed by default, as is done with IDLE), but, Terry, I don't quite understand what you want to achieve with this check? For the check, I think the best option is moving copy_strip() out from the standard library, otherwise we cannot use the latest version in the step (further on that point, currently it is unusable if installed, since paths are relative to it). |
Sorry, something went wrong.
|
I asked about docs and installations because if Python were always installed with a frozen local (and up to date) copy of all the docs, as I believe the case on Windows and mac, installed IDLE would not need a separate copy (possibly not up to date) of its file. But that seems not the case, so help.html is needed even after installation. |
Sorry, something went wrong.
|
@terryjreedy sounds like a chance to collaborate with distro packages and figure it out together? cc @befeleme @hroncok @mgorny could you shed the light on the downstream handling of the IDLE packaging? It should be possible to make python3.x-idle (Fedora) and dev-lang/python:3.x w/ the USE-flag tk (Gentoo) include the HTML file from docs, right? |
Sorry, something went wrong.
|
Sorry, I don't understand what I'm being asked here. We install whatever make altinstall installs, which includes idlelib/help.html. |
Sorry, something went wrong.
|
Same here. If we can regenerate that HTML during the build, even better, but not mandatory. (Assuming the source HTML file from Doc is actually built by Sphinx, we will probably avoid building it and ship what is there.) |
Sorry, something went wrong.
|
@mgorny @hroncok my understanding is that currently there's Doc/library/idle.rst in the repo which is the ultimate source. It gets built with Sphinx into an HTML file (Doc/build/html/library/idle.html, one of many). This HTML file eventually ends up on docs.python.org. However, additionally, a human makes a slightly reduced copy of that file, puts it into Lib/idlelib/help.html and commits it into the CPython repo, effectively hardcoding a certain state of this HTML file (I think this bit is prone to human error). Then it gets installed through downstream-specific mechanisms (e.g. dnf install python3) and those mechanisms don't go through the pipeline of building the Sphinx docs. Now, Doc/build/html/library/idle.html and Lib/idlelib/help.html are close enough so theoretically, it should be possible to get rid of the Lib/idlelib/help.html copy and make IDLE use the one from the docs. What people here don't know is whether it'd be difficult/a burden for downstreams to handle such a migration. Some non-Linux installers bundle the docs and so "install python" for such users results in the docs being available so IDLE could rely on that w/o issues. But since downstreams typically separate the docs into standalone packages, it's not clear if it's possible to package things in a way that, for example, dnf install python3.x-idle would rely on Sphinx-built docs. Does this make the context more clear? Would it make your lives more difficult downstream if Lib/idlelib/help.html was removed from the repo and a Sphinx docs build was required to get it included? |
Sorry, something went wrong.
That would be difficult/a burden for for Fedora. Namely because:
My understanding of this PR was that Lib/idlelib/help.html would be kept in the repo (and the source tarball) but a CI job would assert it is up to date. If it were removed, it would certainly be a problem. |
Sorry, something went wrong.
|
Yes, it would definitely be a problem for Gentoo as well. We support installing docs for multiple Python versions but:
|
Sorry, something went wrong.
|
This PR is stale because it has been open for 30 days with no activity. |
Sorry, something went wrong.
|
Closing, we need to decide what it is we want. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
I expect it to fail till #143718 is merged.
I had to refactor copy_strip to a separate file to avoid having to build Python.
📚 Documentation preview 📚: https://cpython-previews--143742.org.readthedocs.build/
🏃 Run preview 🏃: https://github.com/python/cpython/actions/runs/20930506392/job/60140508484?pr=143742