bpo-38470: Fix test_compileall.test_compile_dir_maxlevels()

[vstinner]

Fix test_compile_dir_maxlevels() on Windows without long path
support: only create 3 subdirectories instead of between 20 and 100
subdirectories.

Fix also compile_dir() to use the current sys.getrecursionlimit()
value as the default maxlevels value, rather than using
sys.getrecursionlimit() value read at startup.

https://bugs.python.org/issue38470

[vstinner]

Fix also compile_dir() to use the current sys.getrecursionlimit()
value as the default maxlevels value, rather than using
sys.getrecursionlimit() value read at startup.

IMHO using the sys.getrecursionlimit() value read at startup is surprising, since it can be modified at runtime.

Fix test_compile_dir_maxlevels() on Windows without long path support: only create 3
subdirectories instead of between 20 and 100 subdirectories.

Python 3.8 used 10 as the default value of maxlevels. But the default changed to sys.getrecusionlimit() in the master branch.

I don't think that it's worth it to test that the default is sys.getrecusionlimit().

--

By the way, I'm not sure about using sys.getrecusionlimit() as the default. Maybe it should be unlimited by default. I'm not sure neither why the default was 10 previously.

In practice, using sys.getrecusionlimit() as the default means "unlimited": Python will raise RecursionError before the function will reach maxlevels=0.

[vstinner]

cc @frenzymadness

[vstinner]

I also wrote a working create_long_path() which fix the check for "too long path" using importlib._bootstrap_external._write_atomic(longer_cache, b''), but honestly it looks like overkill just to test maxlevels parameter. The function becomes 51 lines long. So I chose to not propose this overcomplicated solution, but propose this way simpler approach.

[aeros]

@vstinner

IMHO using the sys.getrecursionlimit() value read at startup is surprising, since it can be modified at runtime.

By the way, I'm not sure about using sys.getrecusionlimit() as the default. Maybe it should be unlimited by default. I'm not sure neither why the default was 10 previously.

I can agree that using sys.getrecursionlimit() might not make sense and 10 is certainly way too low, but is there a reason why we don't have a constant sys.defaultrecursionlimit? That seems far more suited to something like this rather than setting the recursion limit to unlimited, which could result in running out of memory. IIRC, the current default recursion limit is 1000, which is perfectly reasonable IMO.

Edit: Confirmed the default recursion limit:

cpython/Python/ceval.c

Line 635 in 4d20228

#define Py_DEFAULT_RECURSION_LIMIT 1000

[encukou]

Thanks.
Yes, this really needed taking a step back, rather than adding to the complexity.

I ran this test through buildbot-custom, and the bots look happy!

[encukou]

@aeros, compileall doesn't change the recursion limit (that could lead to crashes, yes). It only uses it (as an integer that effectively means "infinity" in this case).

[vstinner]

I can agree that using sys.getrecursionlimit() might not make sense and 10 is certainly way too low, but is there a reason why we don't have a constant sys.defaultrecursionlimit?

I'm not sure why anyone would need the "default recursion limit". sys.setrecursionlimit() exists because the developer knows better than Python what the limit should be. With my merged change, compileall now uses the current limit.

Thanks for merging it @encukou.

[aeros]

@encukou

@aeros, compileall doesn't change the recursion limit (that could lead to crashes, yes). It only uses it (as an integer that effectively means "infinity" in this case).

@vstinner

sys.setrecursionlimit() exists because the developer knows better than Python what the limit should be. With my merged change, compileall now uses the current limit.

Oh I see. I had misunderstood what Victor was saying in his earlier comments. It sounded like with the proposed changes, the recursion limit would become infinite, rather than using whatever the user specified:

In practice, using sys.getrecusionlimit() as the default means "unlimited": Python will raise RecursionError before the function will reach maxlevels=0.

Between the options of using the default recursion limit and unlimited, I was in favor of using the default instead. Now I realize that he was referring to the previous version, and not the current one with the proposed changes in the PR.

I completely agree though that it's far better to allow the user the specify their own limit.

I'm not sure why anyone would need the "default recursion limit".

Disregard that suggestion, it was only made because of my earlier misunderstanding.