GeezerVison

I like the idea in principle, but existing code using onerror will stop working, because onerror arg is accepted but silently ignored. I think a retro-compatible solution to this could be to call onexc AND onerror for a certain period of time.

I'm not sure what's the current policy re. deprecations, but I suppose there can be a transitional time (2 releases? 3?) during which using onerror arg results in DeprecationWarning and then it's finally removed. Something like (not tested):

try:
    ...
except OSError as err:
    if onerror is not None:
        warnings.warn("'onerror' argument is deprecated and will be removed in XXX. Use 'onexc' argument instead.", 
                      DeprecationWarning)
        onerror(..., ..., sys.exc_info())
    if onexc is not None:
        onerror(..., ..., err)

existing code using onerror will stop working

It will still work, because onerror gets wrapped by onexc in Lib/shutil.py:708-714.

Note that I didn't remove any tests, so all previous functionality is still there.

Oh you're right sorry. I misread your patch.

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Thanks for making the requested changes!

@giampaolo: please review the changes made to this pull request.

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot wasm32-wasi 3.x has failed when building commit d51a6dc.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1046/builds/1597) and take a look at the build logs.
4. Check if the failure is related to this commit (d51a6dc) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1046/builds/1597

Failed tests:

• test_shutil

Failed subtests:

• test_both_onerror_and_onexc - test.test_shutil.TestRmTree.test_both_onerror_and_onexc

Summary of the results of the build (if available):

== Tests result: FAILURE ==

325 tests OK.

10 slowest tests:

• test_compile: 5 min 40 sec
• test_math: 1 min 13 sec
• test_tokenize: 48.5 sec
• test_lib2to3: 40.6 sec
• test_unparse: 33.8 sec
• test_capi: 24.1 sec
• test_hashlib: 24.1 sec
• test_unicodedata: 18.5 sec
• test_statistics: 13.9 sec
• test_pickle: 13.7 sec

1 test failed:
test_shutil

107 tests skipped:
test__xxinterpchannels test__xxsubinterpreters test_asyncgen
test_asyncio test_bz2 test_clinic test_cmd_line
test_concurrent_futures test_contextlib_async test_ctypes
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_doctest
test_docxmlrpc test_dtrace test_embed test_epoll test_faulthandler
test_fcntl test_file_eintr test_fork1 test_ftplib test_gdb
test_grp test_gzip test_httplib test_httpservers test_idle
test_imaplib test_interpreters test_ioctl test_kqueue
test_launcher test_lzma test_mailbox test_mmap test_msilib
test_multiprocessing_fork test_multiprocessing_forkserver
test_multiprocessing_main_handling test_multiprocessing_spawn
test_nis test_openpty test_ossaudiodev test_pdb test_peg_generator
test_perf_profiler test_pipes test_poll test_poplib test_pty
test_pwd test_queue test_readline test_regrtest test_repl
test_resource test_select test_selectors test_smtplib test_smtpnet
test_socket test_socketserver test_spwd test_sqlite3 test_ssl
test_stable_abi_ctypes test_startfile test_subprocess
test_sys_settrace test_syslog test_tcl test_telnetlib test_thread
test_threadedtempfile test_threading test_threading_local test_tix
test_tkinter test_tools test_ttk test_ttk_textonly test_turtle
test_urllib test_urllib2 test_urllib2_localnet test_urllib2net
test_urllib_response test_urllibnet test_venv test_wait3
test_wait4 test_webbrowser test_winconsoleio test_winreg
test_winsound test_wmi test_wsgiref test_xmlrpc test_xmlrpc_net
test_xxlimited test_zipfile64 test_zipimport_support test_zlib
test_zoneinfo
0:08:09 load avg: 1.99
0:08:09 load avg: 1.99 Re-running failed tests is not supported with --python host runner option.

Total duration: 8 min 9 sec

Traceback (most recent call last):
  File "/Lib/test/test_shutil.py", line 532, in test_both_onerror_and_onexc
    self.assertTrue(onexc_called)
AssertionError: False is not true


Traceback (most recent call last):
  File "/Lib/shutil.py", line 762, in rmtree
    return _rmtree_unsafe(path, onexc)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Lib/shutil.py", line 583, in _rmtree_unsafe
    onexc(os.scandir, path, err)
  File "/Lib/shutil.py", line 580, in _rmtree_unsafe
    with os.scandir(path) as scandir_it:
         ^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 44] No such file or directory: '@test_42_tmpæ'

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Debian root 3.x has failed when building commit d51a6dc.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/345/builds/4352) and take a look at the build logs.
4. Check if the failure is related to this commit (d51a6dc) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/345/builds/4352

Failed tests:

• test_shutil

Failed subtests:

• test_both_onerror_and_onexc - test.test_shutil.TestRmTree.test_both_onerror_and_onexc

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

411 tests OK.

10 slowest tests:

• test_tools: 10 min 20 sec
• test_math: 5 min 20 sec
• test_multiprocessing_spawn: 3 min 57 sec
• test_compile: 3 min 33 sec
• test_concurrent_futures: 2 min 56 sec
• test_asyncio: 2 min 40 sec
• test_tokenize: 2 min 6 sec
• test_unparse: 2 min 6 sec
• test_lib2to3: 1 min 52 sec
• test_capi: 1 min 52 sec

1 test failed:
test_shutil

21 tests skipped:
test_dbm_ndbm test_devpoll test_gdb test_idle test_ioctl
test_kqueue test_launcher test_msilib test_peg_generator
test_startfile test_tcl test_tix test_tkinter test_ttk
test_ttk_textonly test_turtle test_winconsoleio test_winreg
test_winsound test_wmi test_zipfile64

1 re-run test:
test_shutil

Total duration: 39 min 34 sec

Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
    onexc(os.lstat, path, err)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
    orig_st = os.lstat(path, dir_fd=dir_fd)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3079655_tmpæ'


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
    onexc(os.lstat, path, err)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
    orig_st = os.lstat(path, dir_fd=dir_fd)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3084007_tmpæ'


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_shutil.py", line 532, in test_both_onerror_and_onexc
    self.assertTrue(onexc_called)
AssertionError: False is not true

@iritkatriel looks like this broke some build bots.

Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
    onexc(os.lstat, path, err)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
    orig_st = os.lstat(path, dir_fd=dir_fd)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3079655_tmpæ'


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 725, in rmtree
    onexc(os.lstat, path, err)
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/shutil.py", line 723, in rmtree
    orig_st = os.lstat(path, dir_fd=dir_fd)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '@test_3084007_tmpæ'


Traceback (most recent call last):
  File "/root/buildarea/3.x.angelico-debian-amd64/build/Lib/test/test_shutil.py", line 532, in test_both_onerror_and_onexc
    self.assertTrue(onexc_called)
AssertionError: False is not true

I see what I did - I copied part of this test from another test, but I didn't copy the skip instructions that make it not run where it doesn't work. Will fix.

Ignore my experiment with the 1st buildbot report.

Hey, why does onexc accept three arguments (function, path, exc) rather than just the exception?

If it accepted only one argument, then:

1. It would match the onerror argument to os.walk() and os.fwalk()
2. Users could still get the file path that failed via exc.filename
3. Users could still print the exception/traceback to display exactly what failed

The only thing lost would be the function value, which seems like an unreasonably detailed insight into the internals of rmtree(). In fact, rmtree() already bends the truth about the function that actually failed - for example os.path.islink is given if we fail a junction check on Windows. And no internal code looks at the value of function.

Hey, why does onexc accept three arguments (function, path, exc) rather than just the exception?

Because that's what onerror did. This PR was just about replacing exc_info by exc. Feel free to suggest additional changes in a separate issue.

Righto, thanks. I've logged #103218.