bpo-45020: Drop the frozen .h files from the repo. #28375
Conversation
ericsnowcurrently
requested review from
rhettinger and
a team
as code owners
.gitignore
Outdated
| # Python/frozen_modules/MANIFEST is enough to tell if the generated | ||
| # files have changed. See Tools/scripts/freeze_modules.py. |
There was a problem hiding this comment.
I still feel we are having a disconnect over the check whether generated files have changed. Surely if they are not in the repo there's no need to check anything? Because everyone who builds Python will always first generate them. The best a check for MANIFEST can do is checking whether the user who submitted the PR actually ran freeze_modules.py. But why would we care? The changes to MANIFEST seem to just cause noise in PRs. I think the situation is very different from before, when not generating the .h files for importlib would silently break stuff (there's nothing that tells you those .h files are out of date in the old workflow other than the GitHub Action that tries to regenerate them and checks there are no differences).
There was a problem hiding this comment.
Good point. I don't mind dropping MANIFEST from the repo too.
There was a problem hiding this comment.
Would you still generate it? Why exactly? Just to satisfy that GitHub action? I'd kill the action too (or remove this part from it, if it's checking for other things as well).
There was a problem hiding this comment.
The GitHub action doesn't care about files that are not in the repo (and ignored). I want to keep generating the manifest because I find it helpful to have a flat, clean list of all the frozen modules. On several occasions that has been useful. However, it isn't a huge benefit. If you think it still isn't worth it then I'm okay with removing the generation code too.
ericsnowcurrently
force-pushed
the
frozen-modules-drop-h-files
branch
from
f60d112 to
5533204
Compare
Tools/scripts/freeze_modules.py
Outdated
| # If MODULES_DIR is changed then the .gitattributes file needs to be updated. | ||
| MODULES_DIR = os.path.join(ROOT_DIR, 'Python/frozen_modules') | ||
| # If MODULES_DIR is changed then the .gitattributes and .gitignore files | ||
| # needs to be updated. |
gh-28375 broke one of the buildbots. Until I figure out why, I'm rolling the change back. https://bugs.python.org/issue45020
The main advantage is that the files will no longer show up in diffs and PRs. That means, for a PR, the number of files / lines changed will more clearly reflect the actual change. (This is essentially an un-revert of gh-28375.) https://bugs.python.org/issue45020
The main advantage is that the files will no longer show up in diffs and PRs. That means, for a PR, the number of files / lines changed will more clearly reflect the actual change.
We can do this now that GH-28322 has landed. The "check_generated_files" CI job will still catch out-of-date frozen modules due to Python/frozen_modules/MANIFEST.
https://bugs.python.org/issue45020