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

Fix publishing documentation #2384

Merged
merged 7 commits into from
Nov 12, 2024
Merged

Conversation

Avasam
Copy link
Collaborator

@Avasam Avasam commented Oct 6, 2024

Since the help files aren't shipped with the wheel in 307, the docs workflow fails.

Given the catch-22 nature of generating docs at the moment, and not wanting to create extra builds when not needed. I kept the PyPI download. But regerenate a .chm from source. (the help file generation honestly isn't that long anyway).

I also added doc generation as a CI test to sanity test that a PR won't break the docs publishing after a tag is created.

This PR makes the following changes:

There are no technical reasons preventing us from still shipping the help files (PyWin32.chm), other than maybe their size (see #1380 (comment)). If it's a long-term choice, then let's also remove the dead setup code to register them. (we could ship a web link file instead) Otherwise I can add it back to the wheel as part of the automated process. Either way that'll be a different PR.

This incidentally works towards #394

@Avasam Avasam requested a review from mhammond October 6, 2024 17:38
ADDOC = /RD "/O$(GENDIR)\$(TARGET).DOC" /D "doc_header=$(DOCHDR)"
ADTAB = 8
HC = hcw /a /c /e
HHC = hhc
Copy link
Collaborator Author

@Avasam Avasam Oct 6, 2024

Choose a reason for hiding this comment

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

If you still want this to be runnable w/o the python script, I think this should work to add a default executable location:

Suggested change
HHC = hhc
HHC = hhc
# PYTHON is set in make.py, defaulting to the one found on PATH
PYTHON = python

@Avasam Avasam marked this pull request as draft October 6, 2024 18:11
@Avasam
Copy link
Collaborator Author

Avasam commented Oct 6, 2024

Ah, another catch-22. py2d.py uses live introspection by importing modules. I feel like this should be feasible to resolve with static inspection APIs/AST, hacking around Python's import system, or just moving the docs job to depend on the self-install in CI.

@Avasam Avasam force-pushed the Fix-publishing-documentation branch from 6ee1a17 to 63f730c Compare October 6, 2024 19:37
@Avasam Avasam removed the request for review from mhammond October 6, 2024 20:54
@Avasam Avasam marked this pull request as ready for review October 14, 2024 21:24
Comment on lines +46 to +52
# This needs to happen *after* installing pywin32 since
# AutoDuck/py2d.py currently relies on runtime imports for introspection
# This isn't included in the wheel (TODO: could we?)
# and only servces as a PR test for the docs.yaml workflow
- name: Generate PyWin32.chm help file
run: python AutoDuck/make.py

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If we merged both workflows, we could push these as artifacts, then no need to download from PyPI

Comment on lines +12 to +14
pull_request: # Temporary just for test
branches:
- main
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this for the sake of testing in my PR, but if we keep this we wouldn't need to add a test in .github/workflows/main.yml

@Avasam Avasam requested a review from mhammond October 14, 2024 21:50
Copy link
Owner

@mhammond mhammond left a comment

Choose a reason for hiding this comment

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

Sorry for the delay, this is great, thanks!

@@ -43,6 +43,13 @@ jobs:
run: |
python setup.py install --user

# This needs to happen *after* installing pywin32 since
# AutoDuck/py2d.py currently relies on runtime imports for introspection
# This isn't included in the wheel (TODO: could we?)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO: could we?

Open question on setuptools discussion: pypa/setuptools#4685

@Avasam Avasam merged commit b64bccb into mhammond:main Nov 12, 2024
33 checks passed
@Avasam Avasam deleted the Fix-publishing-documentation branch November 12, 2024 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants