Skip to content

bpo-45783: Preserve file moves and deletions in the tests for the freeze tool. #29527

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Nov 23, 2021
Merged

bpo-45783: Preserve file moves and deletions in the tests for the freeze tool. #29527

merged 12 commits into from
Nov 23, 2021

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Nov 11, 2021 •
edited by miss-islington
Loading

The prep code for the test wasn't handling the case where uncommitted files were deleted or moved. This change takes care of that. We also ensure more consistent results by using git status --porcelain instead of git status -s, as well as git clean -x and git reset --hard.

https://bugs.python.org/issue45783

@ericsnowcurrently ericsnowcurrently changed the title bpo-45783: Preserve file moves and deletions in the tests for the freeze tool.' bpo-45783: Preserve file moves and deletions in the tests for the freeze tool. Nov 11, 2021
@encukou
Copy link
Member

encukou commented Nov 12, 2021

Thanks for the quick PR!
Unfortunately, this uncovered another issue :(

The .gitignore file is not included in CPython releases. We take the release from python.org and add it into a Git repo (because that helps us manage the source). Now, because .gitignore is missing, outdir is not marked as ignored, and copytree tries to copy the source into itself (Python-3.11.0a2/Tools/freeze/test/outdir → Python-3.11.0a2/Tools/freeze/test/outdir/cpython/Tools/freeze/test/outdir), eating up disk space until it bursts in a RecursionError.

Granted, git init; git add . on the unpacked source release is weird; maybe we shouldn't do that. But without that, when building from a source release git clone doesn't have a repo to clone from and the test fails anyway.

@ericsnowcurrently
Copy link
Member Author

ericsnowcurrently commented Nov 12, 2021 •
edited
Loading

Ah, that's not great. I took the approach I did to save some time on each run when running locally. However, it isn't that big a difference, so I'll dial it back. That should take care of the problem. Sorry about that.

Oh, I misunderstood. The problem is just where "outdir" is in relation to the test. Making it a temp dir should fix it.

@encukou
Copy link
Member

encukou commented Nov 12, 2021

The problem is just where "outdir" is in relation to the test.

That, and the behavior when tests are run from a source release (no Git repo at all).

@ericsnowcurrently
Copy link
Member Author

@encukou, any objections?

@encukou
Copy link
Member

encukou commented Nov 16, 2021 •
edited
Loading

Almost. Sorry for giving you the info piecewise – I don't know the buildsystem well enough to see ahead :(

It turns out that CPython can be built "out of tree" by running e.g.

mkdir mybuilddir
cd mybuilddir
../configure
make

We use this to produce regular and debug builds from the same source.

In this case, Makefile is created in mybuilddir, and not in the root of the source directory, so the make clean in copy_source_tree fails.

@ericsnowcurrently
Copy link
Member Author

Thanks for the feedback. I'll change that.

@ericsnowcurrently
Copy link
Member Author

@encukou, that should be fixed now.

@encukou
Copy link
Member

encukou commented Nov 18, 2021

You should also run ./configure to generate the Makefile for the compilation.

Or maybe do this as an extra CI/buildbot check? The deeper this goes, the more it seems like compiling the project from within the test suite cuts against the grain.

@ericsnowcurrently
Copy link
Member Author

You should also run ./configure to generate the Makefile for the compilation.

That already happens. Did something happen that suggests otherwise?

Or maybe do this as an extra CI/buildbot check? The deeper this goes, the more it seems like compiling the project from within the test suite cuts against the grain.

I can look into that but wouldn't be able to any time soon.

encukou
encukou approved these changes Nov 23, 2021
View reviewed changes
Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

My bad, I must have been using a wrong commit/patch at the time.

Thank you for the fix!

@encukou encukou merged commit 8ed1495 into python:main Nov 23, 2021
@ericsnowcurrently ericsnowcurrently deleted the freeze-tool-preserve-deletions branch November 23, 2021 17:02
remykarem pushed a commit to remykarem/cpython that referenced this pull request Dec 7, 2021
@ericsnowcurrently @remykarem
bpo-45783: Preserve file moves and deletions in the tests for the fre…
58d4947 
…eze tool. (pythonGH-29527)

Use shutil.copytree rather than Git, which might be missing (or configured
differently) when testing Python built from a source release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@encukou encukou encukou approved these changes

@Uselessman79 Uselessman79 Uselessman79 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ericsnowcurrently @encukou @Uselessman79 @the-knights-who-say-ni @bedevere-bot