Mailman 3 March 2020 - Python-checkins

[3.8] bpo-39815: add cached_property to all (GH-18726) (GH-18728)
by Pablo Galindo March 2, 2020

March 2, 2020

https://github.com/python/cpython/commit/12b714391e485d0150b343b114999bae4a… commit: 12b714391e485d0150b343b114999bae4a0d34dd branch: 3.8 author: Pablo Galindo <Pablogsal(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2020-03-02T00:08:29Z summary: [3.8] bpo-39815: add cached_property to all (GH-18726) (GH-18728) Automerge-Triggered-By: @pablogsal. (cherry picked from commit 217dce9ee6e3cf27a0cedbe1e4a6455776373ec2) Co-authored-by: Hakan Çelik <hakancelik96(a)… [View More]

1 0

closes bpo-39803: Remove unused str from _PyLong_FormatAdvancedWriter. (GH-18709)
by Andy Lester March 1, 2020

March 1, 2020

https://github.com/python/cpython/commit/3fe9117779f0c75f7a0c3d7748c5bf281f… commit: 3fe9117779f0c75f7a0c3d7748c5bf281fbc1e4c branch: master author: Andy Lester <andy(a)petdance.com> committer: GitHub <noreply(a)github.com> date: 2020-03-01T13:26:43-08:00 summary: closes bpo-39803: Remove unused str from _PyLong_FormatAdvancedWriter. (GH-18709) files: M Python/formatter_unicode.c diff --git a/Python/formatter_unicode.c b/Python/formatter_unicode.c index 41a2478e0a817..… [View More]

1 0

(no subject)
by Hakan Çelik March 1, 2020

March 1, 2020

To: python-checkins(a)python.org Subject: bpo-39815: add cached_property to all (GH-18726) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 https://github.com/python/cpython/commit/217dce9ee6e3cf27a0cedbe1e4a6455776… 3ec2 commit: 217dce9ee6e3cf27a0cedbe1e4a6455776373ec2 branch: master author: Hakan =C3=87elik <hakancelik96(a)outlook.com> committer: GitHub <noreply(a)github.com> date: 2020-03-01T13:01:34-08:00 summary: bpo-… [View More]

1 0

bpo-39678: refactor queue manager thread (GH-18551)
by Thomas Moreau March 1, 2020

March 1, 2020

https://github.com/python/cpython/commit/0e89076247580ba0e570c4816f0e5628a7… commit: 0e89076247580ba0e570c4816f0e5628a7e36e83 branch: master author: Thomas Moreau <thomas.moreau.2010(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2020-03-01T21:49:14+01:00 summary: bpo-39678: refactor queue manager thread (GH-18551) files: A Misc/NEWS.d/next/Library/2020-02-28-12-59-30.bpo-39678.3idfxM.rst M Lib/concurrent/futures/process.py M Lib/test/test_concurrent_futures.py diff -… [View More]-git a/Lib/concurrent/futures/process.py b/Lib/concurrent/futures/process.py index d77322831a6c6..39fadcce027c2 100644 --- a/Lib/concurrent/futures/process.py +++ b/Lib/concurrent/futures/process.py @@ -49,7 +49,6 @@ import os from concurrent.futures import _base import queue -from queue import Full import multiprocessing as mp import multiprocessing.connection from multiprocessing.queues import Queue @@ -176,8 +175,9 @@ def _on_queue_feeder_error(self, e, obj): e.__cause__ = _RemoteTraceback('\n"""\n{}"""'.format(''.join(tb))) work_item = self.pending_work_items.pop(obj.work_id, None) self.thread_wakeup.wakeup() - # work_item can be None if another process terminated. In this case, - # the queue_manager_thread fails all work_items with BrokenProcessPool + # work_item can be None if another process terminated. In this + # case, the executor_manager_thread fails all work_items + # with BrokenProcessPool if work_item is not None: work_item.future.set_exception(e) else: @@ -193,6 +193,7 @@ def _get_chunks(*iterables, chunksize): return yield chunk + def _process_chunk(fn, chunk): """ Processes a chunk of an iterable passed to map. @@ -256,122 +257,123 @@ def _process_worker(call_queue, result_queue, initializer, initargs): del call_item -def _add_call_item_to_queue(pending_work_items, - work_ids, - call_queue): - """Fills call_queue with _WorkItems from pending_work_items. +class _ExecutorManagerThread(threading.Thread): + """Manages the communication between this process and the worker processes. - This function never blocks. + The manager is run in a local thread. Args: - pending_work_items: A dict mapping work ids to _WorkItems e.g. - {5: <_WorkItem...>, 6: <_WorkItem...>, ...} - work_ids: A queue.Queue of work ids e.g. Queue([5, 6, ...]). Work ids - are consumed and the corresponding _WorkItems from - pending_work_items are transformed into _CallItems and put in - call_queue. - call_queue: A multiprocessing.Queue that will be filled with _CallItems - derived from _WorkItems. + executor: A reference to the ProcessPoolExecutor that owns + this thread. A weakref will be own by the manager as well as + references to internal objects used to introspect the state of + the executor. """ - while True: - if call_queue.full(): - return - try: - work_id = work_ids.get(block=False) - except queue.Empty: - return - else: - work_item = pending_work_items[work_id] - - if work_item.future.set_running_or_notify_cancel(): - call_queue.put(_CallItem(work_id, - work_item.fn, - work_item.args, - work_item.kwargs), - block=True) - else: - del pending_work_items[work_id] - continue + def __init__(self, executor): + # Store references to necessary internals of the executor. -def _queue_management_worker(executor_reference, - processes, - pending_work_items, - work_ids_queue, - call_queue, - result_queue, - thread_wakeup): - """Manages the communication between this process and the worker processes. + # A _ThreadWakeup to allow waking up the queue_manager_thread from the + # main Thread and avoid deadlocks caused by permanently locked queues. + self.thread_wakeup = executor._executor_manager_thread_wakeup - This function is run in a local thread. + # A weakref.ref to the ProcessPoolExecutor that owns this thread. Used + # to determine if the ProcessPoolExecutor has been garbage collected + # and that the manager can exit. + # When the executor gets garbage collected, the weakref callback + # will wake up the queue management thread so that it can terminate + # if there is no pending work item. + def weakref_cb(_, thread_wakeup=self.thread_wakeup): + mp.util.debug('Executor collected: triggering callback for' + ' QueueManager wakeup') + thread_wakeup.wakeup() - Args: - executor_reference: A weakref.ref to the ProcessPoolExecutor that owns - this thread. Used to determine if the ProcessPoolExecutor has been - garbage collected and that this function can exit. - process: A list of the ctx.Process instances used as - workers. - pending_work_items: A dict mapping work ids to _WorkItems e.g. - {5: <_WorkItem...>, 6: <_WorkItem...>, ...} - work_ids_queue: A queue.Queue of work ids e.g. Queue([5, 6, ...]). - call_queue: A ctx.Queue that will be filled with _CallItems - derived from _WorkItems for processing by the process workers. - result_queue: A ctx.SimpleQueue of _ResultItems generated by the - process workers. - thread_wakeup: A _ThreadWakeup to allow waking up the - queue_manager_thread from the main Thread and avoid deadlocks - caused by permanently locked queues. - """ - executor = None + self.executor_reference = weakref.ref(executor, weakref_cb) - def shutting_down(): - return (_global_shutdown or executor is None - or executor._shutdown_thread) + # A list of the ctx.Process instances used as workers. + self.processes = executor._processes - def shutdown_worker(): - # This is an upper bound on the number of children alive. - n_children_alive = sum(p.is_alive() for p in processes.values()) - n_children_to_stop = n_children_alive - n_sentinels_sent = 0 - # Send the right number of sentinels, to make sure all children are - # properly terminated. - while n_sentinels_sent < n_children_to_stop and n_children_alive > 0: - for i in range(n_children_to_stop - n_sentinels_sent): - try: - call_queue.put_nowait(None) - n_sentinels_sent += 1 - except Full: - break - n_children_alive = sum(p.is_alive() for p in processes.values()) + # A ctx.Queue that will be filled with _CallItems derived from + # _WorkItems for processing by the process workers. + self.call_queue = executor._call_queue - # Release the queue's resources as soon as possible. - call_queue.close() - call_queue.join_thread() - thread_wakeup.close() - # If .join() is not called on the created processes then - # some ctx.Queue methods may deadlock on Mac OS X. - for p in processes.values(): - p.join() + # A ctx.SimpleQueue of _ResultItems generated by the process workers. + self.result_queue = executor._result_queue - result_reader = result_queue._reader - wakeup_reader = thread_wakeup._reader - readers = [result_reader, wakeup_reader] + # A queue.Queue of work ids e.g. Queue([5, 6, ...]). + self.work_ids_queue = executor._work_ids - while True: - _add_call_item_to_queue(pending_work_items, - work_ids_queue, - call_queue) + # A dict mapping work ids to _WorkItems e.g. + # {5: <_WorkItem...>, 6: <_WorkItem...>, ...} + self.pending_work_items = executor._pending_work_items + + # Set this thread to be daemonized + super().__init__() + self.daemon = True + def run(self): + # Main loop for the executor manager thread. + + while True: + self.add_call_item_to_queue() + + result_item, is_broken, cause = self.wait_result_broken_or_wakeup() + + if is_broken: + self.terminate_broken(cause) + return + if result_item is not None: + self.process_result_item(result_item) + # Delete reference to result_item to avoid keeping references + # while waiting on new results. + del result_item + + if self.is_shutting_down(): + self.flag_executor_shutting_down() + + # Since no new work items can be added, it is safe to shutdown + # this thread if there are no pending work items. + if not self.pending_work_items: + self.join_executor_internals() + return + + def add_call_item_to_queue(self): + # Fills call_queue with _WorkItems from pending_work_items. + # This function never blocks. + while True: + if self.call_queue.full(): + return + try: + work_id = self.work_ids_queue.get(block=False) + except queue.Empty: + return + else: + work_item = self.pending_work_items[work_id] + + if work_item.future.set_running_or_notify_cancel(): + self.call_queue.put(_CallItem(work_id, + work_item.fn, + work_item.args, + work_item.kwargs), + block=True) + else: + del self.pending_work_items[work_id] + continue + + def wait_result_broken_or_wakeup(self): # Wait for a result to be ready in the result_queue while checking # that all worker processes are still running, or for a wake up # signal send. The wake up signals come either from new tasks being # submitted, from the executor being shutdown/gc-ed, or from the # shutdown of the python interpreter. - worker_sentinels = [p.sentinel for p in processes.values()] + result_reader = self.result_queue._reader + wakeup_reader = self.thread_wakeup._reader + readers = [result_reader, wakeup_reader] + worker_sentinels = [p.sentinel for p in self.processes.values()] ready = mp.connection.wait(readers + worker_sentinels) cause = None is_broken = True + result_item = None if result_reader in ready: try: result_item = result_reader.recv() @@ -381,97 +383,135 @@ def shutdown_worker(): elif wakeup_reader in ready: is_broken = False - result_item = None - thread_wakeup.clear() - if is_broken: - # Mark the process pool broken so that submits fail right now. - executor = executor_reference() - if executor is not None: - executor._broken = ('A child process terminated ' - 'abruptly, the process pool is not ' - 'usable anymore') - executor._shutdown_thread = True - executor = None - bpe = BrokenProcessPool("A process in the process pool was " - "terminated abruptly while the future was " - "running or pending.") - if cause is not None: - bpe.__cause__ = _RemoteTraceback( - f"\n'''\n{''.join(cause)}'''") - # All futures in flight must be marked failed - for work_id, work_item in pending_work_items.items(): - work_item.future.set_exception(bpe) - # Delete references to object. See issue16284 - del work_item - pending_work_items.clear() - # Terminate remaining workers forcibly: the queues or their - # locks may be in a dirty state and block forever. - for p in processes.values(): - p.terminate() - shutdown_worker() - return + self.thread_wakeup.clear() + + return result_item, is_broken, cause + + def process_result_item(self, result_item): + # Process the received a result_item. This can be either the PID of a + # worker that exited gracefully or a _ResultItem + if isinstance(result_item, int): # Clean shutdown of a worker using its PID # (avoids marking the executor broken) - assert shutting_down() - p = processes.pop(result_item) + assert self.is_shutting_down() + p = self.processes.pop(result_item) p.join() - if not processes: - shutdown_worker() + if not self.processes: + self.join_executor_internals() return - elif result_item is not None: - work_item = pending_work_items.pop(result_item.work_id, None) + else: + # Received a _ResultItem so mark the future as completed. + work_item = self.pending_work_items.pop(result_item.work_id, None) # work_item can be None if another process terminated (see above) if work_item is not None: if result_item.exception: work_item.future.set_exception(result_item.exception) else: work_item.future.set_result(result_item.result) - # Delete references to object. See issue16284 - del work_item - # Delete reference to result_item - del result_item - # Check whether we should start shutting down. - executor = executor_reference() + def is_shutting_down(self): + # Check whether we should start shutting down the executor. + executor = self.executor_reference() # No more work items can be added if: # - The interpreter is shutting down OR # - The executor that owns this worker has been collected OR # - The executor that owns this worker has been shutdown. - if shutting_down(): - try: - # Flag the executor as shutting down as early as possible if it - # is not gc-ed yet. - if executor is not None: - executor._shutdown_thread = True - # Unless there are pending work items, we have nothing to cancel. - if pending_work_items and executor._cancel_pending_futures: - # Cancel all pending futures and update pending_work_items - # to only have futures that are currently running. - new_pending_work_items = {} - for work_id, work_item in pending_work_items.items(): - if not work_item.future.cancel(): - new_pending_work_items[work_id] = work_item - - pending_work_items = new_pending_work_items - # Drain work_ids_queue since we no longer need to - # add items to the call queue. - while True: - try: - work_ids_queue.get_nowait() - except queue.Empty: - break + return (_global_shutdown or executor is None + or executor._shutdown_thread) - # Since no new work items can be added, it is safe to shutdown - # this thread if there are no pending work items. - if not pending_work_items: - shutdown_worker() - return - except Full: - # This is not a problem: we will eventually be woken up (in - # result_queue.get()) and be able to send a sentinel again. - pass - executor = None + def terminate_broken(self, cause): + # Terminate the executor because it is in a broken state. The cause + # argument can be used to display more information on the error that + # lead the executor into becoming broken. + + # Mark the process pool broken so that submits fail right now. + executor = self.executor_reference() + if executor is not None: + executor._broken = ('A child process terminated ' + 'abruptly, the process pool is not ' + 'usable anymore') + executor._shutdown_thread = True + executor = None + + # All pending tasks are to be marked failed with the following + # BrokenProcessPool error + bpe = BrokenProcessPool("A process in the process pool was " + "terminated abruptly while the future was " + "running or pending.") + if cause is not None: + bpe.__cause__ = _RemoteTraceback( + f"\n'''\n{''.join(cause)}'''") + + # Mark pending tasks as failed. + for work_id, work_item in self.pending_work_items.items(): + work_item.future.set_exception(bpe) + # Delete references to object. See issue16284 + del work_item + self.pending_work_items.clear() + + # Terminate remaining workers forcibly: the queues or their + # locks may be in a dirty state and block forever. + for p in self.processes.values(): + p.terminate() + + # clean up resources + self.join_executor_internals() + + def flag_executor_shutting_down(self): + # Flag the executor as shutting down and cancel remaining tasks if + # requested as early as possible if it is not gc-ed yet. + executor = self.executor_reference() + if executor is not None: + executor._shutdown_thread = True + # Cancel pending work items if requested. + if executor._cancel_pending_futures: + # Cancel all pending futures and update pending_work_items + # to only have futures that are currently running. + new_pending_work_items = {} + for work_id, work_item in self.pending_work_items.items(): + if not work_item.future.cancel(): + new_pending_work_items[work_id] = work_item + self.pending_work_items = new_pending_work_items + # Drain work_ids_queue since we no longer need to + # add items to the call queue. + while True: + try: + self.work_ids_queue.get_nowait() + except queue.Empty: + break + # Make sure we do this only once to not waste time looping + # on running processes over and over. + executor._cancel_pending_futures = False + + def shutdown_workers(self): + n_children_to_stop = self.get_n_children_alive() + n_sentinels_sent = 0 + # Send the right number of sentinels, to make sure all children are + # properly terminated. + while (n_sentinels_sent < n_children_to_stop + and self.get_n_children_alive() > 0): + for i in range(n_children_to_stop - n_sentinels_sent): + try: + self.call_queue.put_nowait(None) + n_sentinels_sent += 1 + except queue.Full: + break + + def join_executor_internals(self): + self.shutdown_workers() + # Release the queue's resources as soon as possible. + self.call_queue.close() + self.call_queue.join_thread() + self.thread_wakeup.close() + # If .join() is not called on the created processes then + # some ctx.Queue methods may deadlock on Mac OS X. + for p in self.processes.values(): + p.join() + + def get_n_children_alive(self): + # This is an upper bound on the number of children alive. + return sum(p.is_alive() for p in self.processes.values()) _system_limits_checked = False @@ -562,7 +602,7 @@ def __init__(self, max_workers=None, mp_context=None, self._initargs = initargs # Management thread - self._queue_management_thread = None + self._executor_manager_thread = None # Map of pids to processes self._processes = {} @@ -576,12 +616,12 @@ def __init__(self, max_workers=None, mp_context=None, self._cancel_pending_futures = False # _ThreadWakeup is a communication channel used to interrupt the wait - # of the main loop of queue_manager_thread from another thread (e.g. + # of the main loop of executor_manager_thread from another thread (e.g. # when calling executor.submit or executor.shutdown). We do not use the - # _result_queue to send the wakeup signal to the queue_manager_thread + # _result_queue to send wakeup signals to the executor_manager_thread # as it could result in a deadlock if a worker process dies with the # _result_queue write lock still acquired. - self._queue_management_thread_wakeup = _ThreadWakeup() + self._executor_manager_thread_wakeup = _ThreadWakeup() # Create communication channels for the executor # Make the call queue slightly larger than the number of processes to @@ -591,7 +631,7 @@ def __init__(self, max_workers=None, mp_context=None, self._call_queue = _SafeQueue( max_size=queue_size, ctx=self._mp_context, pending_work_items=self._pending_work_items, - thread_wakeup=self._queue_management_thread_wakeup) + thread_wakeup=self._executor_manager_thread_wakeup) # Killed worker processes can produce spurious "broken pipe" # tracebacks in the queue's own worker thread. But we detect killed # processes anyway, so silence the tracebacks. @@ -599,32 +639,14 @@ def __init__(self, max_workers=None, mp_context=None, self._result_queue = mp_context.SimpleQueue() self._work_ids = queue.Queue() - def _start_queue_management_thread(self): - if self._queue_management_thread is None: - # When the executor gets garbarge collected, the weakref callback - # will wake up the queue management thread so that it can terminate - # if there is no pending work item. - def weakref_cb(_, - thread_wakeup=self._queue_management_thread_wakeup): - mp.util.debug('Executor collected: triggering callback for' - ' QueueManager wakeup') - thread_wakeup.wakeup() + def _start_executor_manager_thread(self): + if self._executor_manager_thread is None: # Start the processes so that their sentinels are known. self._adjust_process_count() - self._queue_management_thread = threading.Thread( - target=_queue_management_worker, - args=(weakref.ref(self, weakref_cb), - self._processes, - self._pending_work_items, - self._work_ids, - self._call_queue, - self._result_queue, - self._queue_management_thread_wakeup), - name="QueueManagerThread") - self._queue_management_thread.daemon = True - self._queue_management_thread.start() - _threads_wakeups[self._queue_management_thread] = \ - self._queue_management_thread_wakeup + self._executor_manager_thread = _ExecutorManagerThread(self) + self._executor_manager_thread.start() + _threads_wakeups[self._executor_manager_thread] = \ + self._executor_manager_thread_wakeup def _adjust_process_count(self): for _ in range(len(self._processes), self._max_workers): @@ -654,9 +676,9 @@ def submit(self, fn, /, *args, **kwargs): self._work_ids.put(self._queue_count) self._queue_count += 1 # Wake up queue management thread - self._queue_management_thread_wakeup.wakeup() + self._executor_manager_thread_wakeup.wakeup() - self._start_queue_management_thread() + self._start_executor_manager_thread() return f submit.__doc__ = _base.Executor.submit.__doc__ @@ -694,20 +716,20 @@ def shutdown(self, wait=True, *, cancel_futures=False): self._cancel_pending_futures = cancel_futures self._shutdown_thread = True - if self._queue_management_thread: + if self._executor_manager_thread: # Wake up queue management thread - self._queue_management_thread_wakeup.wakeup() + self._executor_manager_thread_wakeup.wakeup() if wait: - self._queue_management_thread.join() + self._executor_manager_thread.join() # To reduce the risk of opening too many files, remove references to # objects that use file descriptors. - self._queue_management_thread = None + self._executor_manager_thread = None self._call_queue = None self._result_queue = None self._processes = None - if self._queue_management_thread_wakeup: - self._queue_management_thread_wakeup = None + if self._executor_manager_thread_wakeup: + self._executor_manager_thread_wakeup = None shutdown.__doc__ = _base.Executor.shutdown.__doc__ diff --git a/Lib/test/test_concurrent_futures.py b/Lib/test/test_concurrent_futures.py index a7381f9d13eb1..868415ab29916 100644 --- a/Lib/test/test_concurrent_futures.py +++ b/Lib/test/test_concurrent_futures.py @@ -508,15 +508,15 @@ def test_context_manager_shutdown(self): def test_del_shutdown(self): executor = futures.ProcessPoolExecutor(max_workers=5) res = executor.map(abs, range(-5, 5)) - queue_management_thread = executor._queue_management_thread + executor_manager_thread = executor._executor_manager_thread processes = executor._processes call_queue = executor._call_queue - queue_management_thread = executor._queue_management_thread + executor_manager_thread = executor._executor_manager_thread del executor # Make sure that all the executor resources were properly cleaned by # the shutdown process - queue_management_thread.join() + executor_manager_thread.join() for p in processes.values(): p.join() call_queue.join_thread() @@ -532,12 +532,12 @@ def test_shutdown_no_wait(self): res = executor.map(abs, range(-5, 5)) processes = executor._processes call_queue = executor._call_queue - queue_management_thread = executor._queue_management_thread + executor_manager_thread = executor._executor_manager_thread executor.shutdown(wait=False) # Make sure that all the executor resources were properly cleaned by # the shutdown process - queue_management_thread.join() + executor_manager_thread.join() for p in processes.values(): p.join() call_queue.join_thread() @@ -1139,11 +1139,11 @@ def test_shutdown_deadlock_pickle(self): mp_context=get_context(self.ctx)) as executor: self.executor = executor # Allow clean up in fail_on_deadlock - # Start the executor and get the queue_management_thread to collect + # Start the executor and get the executor_manager_thread to collect # the threads and avoid dangling thread that should be cleaned up # asynchronously. executor.submit(id, 42).result() - queue_manager = executor._queue_management_thread + executor_manager = executor._executor_manager_thread # Submit a task that fails at pickle and shutdown the executor # without waiting @@ -1154,7 +1154,7 @@ def test_shutdown_deadlock_pickle(self): # Make sure the executor is eventually shutdown and do not leave # dangling threads - queue_manager.join() + executor_manager.join() create_executor_tests(ExecutorDeadlockTest, diff --git a/Misc/NEWS.d/next/Library/2020-02-28-12-59-30.bpo-39678.3idfxM.rst b/Misc/NEWS.d/next/Library/2020-02-28-12-59-30.bpo-39678.3idfxM.rst new file mode 100644 index 0000000000000..8b18e2259c5c7 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-02-28-12-59-30.bpo-39678.3idfxM.rst @@ -0,0 +1,2 @@ +Refactor queue_manager in :class:`concurrent.futures.ProcessPoolExecutor` to +make it easier to maintain. \ No newline at end of file [View Less]

1 0

(no subject)
by Batuhan Taşkaya March 1, 2020

March 1, 2020

To: python-checkins(a)python.org Subject: bpo-38870: Implement a precedence algorithm in ast.unparse (GH-17377) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 https://github.com/python/cpython/commit/397b96f6d7a89f778ebc0591e32216a818… e667 commit: 397b96f6d7a89f778ebc0591e32216a8183fe667 branch: master author: Batuhan Ta=C5=9Fkaya <47358913+isidentical(a)users.noreply.github.com> committer: GitHub <noreply(a)github.com> date:… [View More] 2020-03-01T20:12:17Z summary: bpo-38870: Implement a precedence algorithm in ast.unparse (GH-17377) Implement a simple precedence algorithm for ast.unparse in order to avoid red= undant parenthesis for nested structures in the final output. files: M Lib/ast.py M Lib/test/test_ast.py M Lib/test/test_unparse.py diff --git a/Lib/ast.py b/Lib/ast.py index 511f0956a00b0..4839201e2e234 100644 --- a/Lib/ast.py +++ b/Lib/ast.py @@ -27,6 +27,7 @@ import sys from _ast import * from contextlib import contextmanager, nullcontext +from enum import IntEnum, auto =20 =20 def parse(source, filename=3D'<unknown>', mode=3D'exec', *, @@ -560,6 +561,35 @@ def __new__(cls, *args, **kwargs): # We unparse those infinities to INFSTR. _INFSTR =3D "1e" + repr(sys.float_info.max_10_exp + 1) =20 +class _Precedence(IntEnum): + """Precedence table that originated from python grammar.""" + + TUPLE =3D auto() + YIELD =3D auto() # 'yield', 'yield from' + TEST =3D auto() # 'if'-'else', 'lambda' + OR =3D auto() # 'or' + AND =3D auto() # 'and' + NOT =3D auto() # 'not' + CMP =3D auto() # '<', '>', '=3D=3D', '>=3D', '<=3D', '!=3D', + # 'in', 'not in', 'is', 'is not' + EXPR =3D auto() + BOR =3D EXPR # '|' + BXOR =3D auto() # '^' + BAND =3D auto() # '&' + SHIFT =3D auto() # '<<', '>>' + ARITH =3D auto() # '+', '-' + TERM =3D auto() # '*', '@', '/', '%', '//' + FACTOR =3D auto() # unary '+', '-', '~' + POWER =3D auto() # '**' + AWAIT =3D auto() # 'await' + ATOM =3D auto() + + def next(self): + try: + return self.__class__(self + 1) + except ValueError: + return self + class _Unparser(NodeVisitor): """Methods in this class recursively traverse an AST and output source code for the abstract syntax; original formatting @@ -568,6 +598,7 @@ class _Unparser(NodeVisitor): def __init__(self): self._source =3D [] self._buffer =3D [] + self._precedences =3D {} self._indent =3D 0 =20 def interleave(self, inter, f, seq): @@ -625,6 +656,17 @@ def delimit_if(self, start, end, condition): else: return nullcontext() =20 + def require_parens(self, precedence, node): + """Shortcut to adding precedence related parens""" + return self.delimit_if("(", ")", self.get_precedence(node) > precede= nce) + + def get_precedence(self, node): + return self._precedences.get(node, _Precedence.TEST) + + def set_precedence(self, precedence, *nodes): + for node in nodes: + self._precedences[node] =3D precedence + def traverse(self, node): if isinstance(node, list): for item in node: @@ -645,10 +687,12 @@ def visit_Module(self, node): =20 def visit_Expr(self, node): self.fill() + self.set_precedence(_Precedence.YIELD, node.value) self.traverse(node.value) =20 def visit_NamedExpr(self, node): - with self.delimit("(", ")"): + with self.require_parens(_Precedence.TUPLE, node): + self.set_precedence(_Precedence.ATOM, node.target, node.value) self.traverse(node.target) self.write(" :=3D ") self.traverse(node.value) @@ -723,24 +767,27 @@ def visit_Nonlocal(self, node): self.interleave(lambda: self.write(", "), self.write, node.names) =20 def visit_Await(self, node): - with self.delimit("(", ")"): + with self.require_parens(_Precedence.AWAIT, node): self.write("await") if node.value: self.write(" ") + self.set_precedence(_Precedence.ATOM, node.value) self.traverse(node.value) =20 def visit_Yield(self, node): - with self.delimit("(", ")"): + with self.require_parens(_Precedence.YIELD, node): self.write("yield") if node.value: self.write(" ") + self.set_precedence(_Precedence.ATOM, node.value) self.traverse(node.value) =20 def visit_YieldFrom(self, node): - with self.delimit("(", ")"): + with self.require_parens(_Precedence.YIELD, node): self.write("yield from ") if not node.value: raise ValueError("Node can't be used without a value attribu= te.") + self.set_precedence(_Precedence.ATOM, node.value) self.traverse(node.value) =20 def visit_Raise(self, node): @@ -907,7 +954,9 @@ def _fstring_Constant(self, node, write): =20 def _fstring_FormattedValue(self, node, write): write("{") - expr =3D type(self)().visit(node.value).rstrip("\n") + unparser =3D type(self)() + unparser.set_precedence(_Precedence.TEST.next(), node.value) + expr =3D unparser.visit(node.value).rstrip("\n") if expr.startswith("{"): write(" ") # Separate pair of opening brackets as "{ {" write(expr) @@ -983,19 +1032,23 @@ def visit_comprehension(self, node): self.write(" async for ") else: self.write(" for ") + self.set_precedence(_Precedence.TUPLE, node.target) self.traverse(node.target) self.write(" in ") + self.set_precedence(_Precedence.TEST.next(), node.iter, *node.ifs) self.traverse(node.iter) for if_clause in node.ifs: self.write(" if ") self.traverse(if_clause) =20 def visit_IfExp(self, node): - with self.delimit("(", ")"): + with self.require_parens(_Precedence.TEST, node): + self.set_precedence(_Precedence.TEST.next(), node.body, node.tes= t) self.traverse(node.body) self.write(" if ") self.traverse(node.test) self.write(" else ") + self.set_precedence(_Precedence.TEST, node.orelse) self.traverse(node.orelse) =20 def visit_Set(self, node): @@ -1016,6 +1069,7 @@ def write_item(item): # for dictionary unpacking operator in dicts {**{'y': 2}} # see PEP 448 for details self.write("**") + self.set_precedence(_Precedence.EXPR, v) self.traverse(v) else: write_key_value_pair(k, v) @@ -1035,11 +1089,20 @@ def visit_Tuple(self, node): self.interleave(lambda: self.write(", "), self.traverse, nod= e.elts) =20 unop =3D {"Invert": "~", "Not": "not", "UAdd": "+", "USub": "-"} + unop_precedence =3D { + "~": _Precedence.FACTOR, + "not": _Precedence.NOT, + "+": _Precedence.FACTOR, + "-": _Precedence.FACTOR + } =20 def visit_UnaryOp(self, node): - with self.delimit("(", ")"): - self.write(self.unop[node.op.__class__.__name__]) + operator =3D self.unop[node.op.__class__.__name__] + operator_precedence =3D self.unop_precedence[operator] + with self.require_parens(operator_precedence, node): + self.write(operator) self.write(" ") + self.set_precedence(operator_precedence, node.operand) self.traverse(node.operand) =20 binop =3D { @@ -1058,10 +1121,38 @@ def visit_UnaryOp(self, node): "Pow": "**", } =20 + binop_precedence =3D { + "+": _Precedence.ARITH, + "-": _Precedence.ARITH, + "*": _Precedence.TERM, + "@": _Precedence.TERM, + "/": _Precedence.TERM, + "%": _Precedence.TERM, + "<<": _Precedence.SHIFT, + ">>": _Precedence.SHIFT, + "|": _Precedence.BOR, + "^": _Precedence.BXOR, + "&": _Precedence.BAND, + "//": _Precedence.TERM, + "**": _Precedence.POWER, + } + + binop_rassoc =3D frozenset(("**",)) def visit_BinOp(self, node): - with self.delimit("(", ")"): + operator =3D self.binop[node.op.__class__.__name__] + operator_precedence =3D self.binop_precedence[operator] + with self.require_parens(operator_precedence, node): + if operator in self.binop_rassoc: + left_precedence =3D operator_precedence.next() + right_precedence =3D operator_precedence + else: + left_precedence =3D operator_precedence + right_precedence =3D operator_precedence.next() + + self.set_precedence(left_precedence, node.left) self.traverse(node.left) - self.write(" " + self.binop[node.op.__class__.__name__] + " ") + self.write(f" {operator} ") + self.set_precedence(right_precedence, node.right) self.traverse(node.right) =20 cmpops =3D { @@ -1078,20 +1169,32 @@ def visit_BinOp(self, node): } =20 def visit_Compare(self, node): - with self.delimit("(", ")"): + with self.require_parens(_Precedence.CMP, node): + self.set_precedence(_Precedence.CMP.next(), node.left, *node.com= parators) self.traverse(node.left) for o, e in zip(node.ops, node.comparators): self.write(" " + self.cmpops[o.__class__.__name__] + " ") self.traverse(e) =20 boolops =3D {"And": "and", "Or": "or"} + boolop_precedence =3D {"and": _Precedence.AND, "or": _Precedence.OR} =20 def visit_BoolOp(self, node): - with self.delimit("(", ")"): - s =3D " %s " % self.boolops[node.op.__class__.__name__] - self.interleave(lambda: self.write(s), self.traverse, node.value= s) + operator =3D self.boolops[node.op.__class__.__name__] + operator_precedence =3D self.boolop_precedence[operator] + + def increasing_level_traverse(node): + nonlocal operator_precedence + operator_precedence =3D operator_precedence.next() + self.set_precedence(operator_precedence, node) + self.traverse(node) + + with self.require_parens(operator_precedence, node): + s =3D f" {operator} " + self.interleave(lambda: self.write(s), increasing_level_traverse= , node.values) =20 def visit_Attribute(self, node): + self.set_precedence(_Precedence.ATOM, node.value) self.traverse(node.value) # Special case: 3.__abs__() is a syntax error, so if node.value # is an integer literal then we need to either parenthesize @@ -1102,6 +1205,7 @@ def visit_Attribute(self, node): self.write(node.attr) =20 def visit_Call(self, node): + self.set_precedence(_Precedence.ATOM, node.func) self.traverse(node.func) with self.delimit("(", ")"): comma =3D False @@ -1119,18 +1223,21 @@ def visit_Call(self, node): self.traverse(e) =20 def visit_Subscript(self, node): + self.set_precedence(_Precedence.ATOM, node.value) self.traverse(node.value) with self.delimit("[", "]"): self.traverse(node.slice) =20 def visit_Starred(self, node): self.write("*") + self.set_precedence(_Precedence.EXPR, node.value) self.traverse(node.value) =20 def visit_Ellipsis(self, node): self.write("...") =20 def visit_Index(self, node): + self.set_precedence(_Precedence.TUPLE, node.value) self.traverse(node.value) =20 def visit_Slice(self, node): @@ -1212,10 +1319,11 @@ def visit_keyword(self, node): self.traverse(node.value) =20 def visit_Lambda(self, node): - with self.delimit("(", ")"): + with self.require_parens(_Precedence.TEST, node): self.write("lambda ") self.traverse(node.args) self.write(": ") + self.set_precedence(_Precedence.TEST, node.body) self.traverse(node.body) =20 def visit_alias(self, node): diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py index 2ed4657822e54..e78848537d47a 100644 --- a/Lib/test/test_ast.py +++ b/Lib/test/test_ast.py @@ -247,6 +247,13 @@ def to_tuple(t): =20 class AST_Tests(unittest.TestCase): =20 + def _is_ast_node(self, name, node): + if not isinstance(node, type): + return False + if "ast" not in node.__module__: + return False + return name !=3D 'AST' and name[0].isupper() + def _assertTrueorder(self, ast_node, parent_pos): if not isinstance(ast_node, ast.AST) or ast_node._fields is None: return @@ -335,7 +342,7 @@ def test_base_classes(self): =20 def test_field_attr_existence(self): for name, item in ast.__dict__.items(): - if isinstance(item, type) and name !=3D 'AST' and name[0].isuppe= r(): + if self._is_ast_node(name, item): x =3D item() if isinstance(x, ast.AST): self.assertEqual(type(x._fields), tuple) diff --git a/Lib/test/test_unparse.py b/Lib/test/test_unparse.py index e8b0d4b06f9e9..f7fcb2bffe891 100644 --- a/Lib/test/test_unparse.py +++ b/Lib/test/test_unparse.py @@ -125,6 +125,13 @@ def check_roundtrip(self, code1): def check_invalid(self, node, raises=3DValueError): self.assertRaises(raises, ast.unparse, node) =20 + def check_src_roundtrip(self, code1, code2=3DNone, strip=3DTrue): + code2 =3D code2 or code1 + code1 =3D ast.unparse(ast.parse(code1)) + if strip: + code1 =3D code1.strip() + self.assertEqual(code2, code1) + =20 class UnparseTestCase(ASTTestCase): # Tests for specific bugs found in earlier versions of unparse @@ -281,6 +288,40 @@ def test_invalid_set(self): def test_invalid_yield_from(self): self.check_invalid(ast.YieldFrom(value=3DNone)) =20 +class CosmeticTestCase(ASTTestCase): + """Test if there are cosmetic issues caused by unnecesary additions""" + + def test_simple_expressions_parens(self): + self.check_src_roundtrip("(a :=3D b)") + self.check_src_roundtrip("await x") + self.check_src_roundtrip("x if x else y") + self.check_src_roundtrip("lambda x: x") + self.check_src_roundtrip("1 + 1") + self.check_src_roundtrip("1 + 2 / 3") + self.check_src_roundtrip("(1 + 2) / 3") + self.check_src_roundtrip("(1 + 2) * 3 + 4 * (5 + 2)") + self.check_src_roundtrip("(1 + 2) * 3 + 4 * (5 + 2) ** 2") + self.check_src_roundtrip("~ x") + self.check_src_roundtrip("x and y") + self.check_src_roundtrip("x and y and z") + self.check_src_roundtrip("x and (y and x)") + self.check_src_roundtrip("(x and y) and z") + self.check_src_roundtrip("(x ** y) ** z ** q") + self.check_src_roundtrip("x >> y") + self.check_src_roundtrip("x << y") + self.check_src_roundtrip("x >> y and x >> z") + self.check_src_roundtrip("x + y - z * q ^ t ** k") + self.check_src_roundtrip("P * V if P and V else n * R * T") + self.check_src_roundtrip("lambda P, V, n: P * V =3D=3D n * R * T") + self.check_src_roundtrip("flag & (other | foo)") + self.check_src_roundtrip("not x =3D=3D y") + self.check_src_roundtrip("x =3D=3D (not y)") + self.check_src_roundtrip("yield x") + self.check_src_roundtrip("yield from x") + self.check_src_roundtrip("call((yield x))") + self.check_src_roundtrip("return x + (yield x)") + + class DirectoryTestCase(ASTTestCase): """Test roundtrip behaviour on all files in Lib and Lib/test.""" =20 [View Less]

1 0

(no subject)
by Batuhan Taşkaya March 1, 2020

March 1, 2020

To: python-checkins(a)python.org Subject: bpo-39520: Fix un-parsing of ext slices with no dimensions (GH-18304) Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 https://github.com/python/cpython/commit/185903de12de8837bf0dc0008a16e5e56c… a019 commit: 185903de12de8837bf0dc0008a16e5e56c66a019 branch: master author: Batuhan Ta=C5=9Fkaya <47358913+isidentical(a)users.noreply.github.com> committer: GitHub <noreply(a)github.com> date:… [View More]

1 0

bpo-38641: Add lib2to3 support for starred expressions in return/yield statements (GH-16994)
by Vlad Emelianov March 1, 2020

March 1, 2020

https://github.com/python/cpython/commit/768d739c1cd8c1d41902229581811a9b86… commit: 768d739c1cd8c1d41902229581811a9b86bcc76e branch: master author: Vlad Emelianov <volshebnyi(a)gmail.com> committer: GitHub <noreply(a)github.com> date: 2020-03-01T19:59:26Z summary: bpo-38641: Add lib2to3 support for starred expressions in return/yield statements (GH-16994) files: A Misc/NEWS.d/next/Library/2019-10-30-15-31-09.bpo-38641.HrTL9k.rst M Lib/lib2to3/Grammar.txt M Lib/lib2to3/tests/data/… [View More]py3_test_grammar.py M Misc/ACKS diff --git a/Lib/lib2to3/Grammar.txt b/Lib/lib2to3/Grammar.txt index 68b73868b5828..51f58209f036f 100644 --- a/Lib/lib2to3/Grammar.txt +++ b/Lib/lib2to3/Grammar.txt @@ -49,7 +49,7 @@ pass_stmt: 'pass' flow_stmt: break_stmt | continue_stmt | return_stmt | raise_stmt | yield_stmt break_stmt: 'break' continue_stmt: 'continue' -return_stmt: 'return' [testlist] +return_stmt: 'return' [testlist_star_expr] yield_stmt: yield_expr raise_stmt: 'raise' [test ['from' test | ',' test [',' test]]] import_stmt: import_name | import_from @@ -151,4 +151,4 @@ testlist1: test (',' test)* encoding_decl: NAME yield_expr: 'yield' [yield_arg] -yield_arg: 'from' test | testlist +yield_arg: 'from' test | testlist_star_expr diff --git a/Lib/lib2to3/tests/data/py3_test_grammar.py b/Lib/lib2to3/tests/data/py3_test_grammar.py index e0b682837e1d8..d06223207e1ec 100644 --- a/Lib/lib2to3/tests/data/py3_test_grammar.py +++ b/Lib/lib2to3/tests/data/py3_test_grammar.py @@ -473,15 +473,27 @@ def test_inner(extra_burning_oil = 1, count=0): test_inner() def testReturn(self): - # 'return' [testlist] + # 'return' [testlist_star_expr] def g1(): return def g2(): return 1 + return_list = [2, 3] + def g3(): return 1, *return_list g1() x = g2() + x3 = g3() check_syntax_error(self, "class foo:return 1") def testYield(self): + # 'yield' [yield_arg] + def g1(): yield 1 + yield_list = [2, 3] + def g2(): yield 1, *yield_list + def g3(): yield from iter(yield_list) + x1 = g1() + x2 = g2() + x3 = g3() check_syntax_error(self, "class foo:yield 1") + check_syntax_error(self, "def g4(): yield from *a") def testRaise(self): # 'raise' test [',' test] diff --git a/Misc/ACKS b/Misc/ACKS index 1b5febb1d19d8..b36e2de4c2ba5 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -1914,5 +1914,6 @@ Jelle Zijlstra Gennadiy Zlobin Doug Zongker Peter Åstrand +Vlad Emelianov (Entries should be added in rough alphabetical order by last names) diff --git a/Misc/NEWS.d/next/Library/2019-10-30-15-31-09.bpo-38641.HrTL9k.rst b/Misc/NEWS.d/next/Library/2019-10-30-15-31-09.bpo-38641.HrTL9k.rst new file mode 100644 index 0000000000000..c547712d81367 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-10-30-15-31-09.bpo-38641.HrTL9k.rst @@ -0,0 +1,2 @@ +Added starred expressions support to ``return`` and ``yield`` statements for +``lib2to3``. Patch by Vlad Emelianov. [View Less]

1 0

[3.7] bpo-39769: Fix compileall ddir for subpkgs. (GH-18676) (GH-18718) (GH-18725)
by Gregory P. Smith March 1, 2020

March 1, 2020

https://github.com/python/cpython/commit/7c64726ced3d6b5d04537386d6a9ca6d17… commit: 7c64726ced3d6b5d04537386d6a9ca6d179c3be4 branch: 3.7 author: Gregory P. Smith <greg(a)krypto.org> committer: GitHub <noreply(a)github.com> date: 2020-03-01T11:06:54-08:00 summary: [3.7] bpo-39769: Fix compileall ddir for subpkgs. (GH-18676) (GH-18718) (GH-18725) Fix compileall.compile_dir() ddir= behavior on sub-packages. Fixes compileall.compile_dir's ddir parameter and compileall command line … [View More]flag `-d` to no longer write the wrong pathname to the generated pyc file for submodules beneath the root of the directory tree being compiled. This fixes a regression introduced with Python 3.5. Tests backported from GH 02673352b5db6ca4d3dc804965facbedfe66425d, the implementation is different due to intervening code changes. But still quiet simple. Why was the bug ever introduced? The refactoring to add parallel execution kept the ddir -> dfile computations but discarded the results instead of sending them to compile_file(). This fixes that. Lack of tests meant this went unnoticed.. (cherry picked from commit ce720d3e0674d6ac6f1b950c20a89be4cfde7853) Co-authored-by: Gregory P. Smith <greg(a)krypto.org> [Google] files: A Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst M Lib/compileall.py M Lib/test/test_compileall.py M Lib/test/test_importlib/util.py diff --git a/Lib/compileall.py b/Lib/compileall.py index aa65c6b904e79..ff5af96d5bc92 100644 --- a/Lib/compileall.py +++ b/Lib/compileall.py @@ -41,7 +41,7 @@ def _walk_dir(dir, ddir=None, maxlevels=10, quiet=0): else: dfile = None if not os.path.isdir(fullname): - yield fullname + yield fullname, ddir elif (maxlevels > 0 and name != os.curdir and name != os.pardir and os.path.isdir(fullname) and not os.path.islink(fullname)): yield from _walk_dir(fullname, ddir=dfile, @@ -77,27 +77,32 @@ def compile_dir(dir, maxlevels=10, ddir=None, force=False, rx=None, from concurrent.futures import ProcessPoolExecutor except ImportError: workers = 1 - files = _walk_dir(dir, quiet=quiet, maxlevels=maxlevels, - ddir=ddir) + files_and_ddirs = _walk_dir(dir, quiet=quiet, maxlevels=maxlevels, + ddir=ddir) success = True if workers is not None and workers != 1 and ProcessPoolExecutor is not None: workers = workers or None with ProcessPoolExecutor(max_workers=workers) as executor: - results = executor.map(partial(compile_file, - ddir=ddir, force=force, - rx=rx, quiet=quiet, - legacy=legacy, - optimize=optimize, - invalidation_mode=invalidation_mode), - files) + results = executor.map( + partial(_compile_file_tuple, + force=force, rx=rx, quiet=quiet, + legacy=legacy, optimize=optimize, + invalidation_mode=invalidation_mode, + ), + files_and_ddirs) success = min(results, default=True) else: - for file in files: - if not compile_file(file, ddir, force, rx, quiet, + for file, dfile in files_and_ddirs: + if not compile_file(file, dfile, force, rx, quiet, legacy, optimize, invalidation_mode): success = False return success +def _compile_file_tuple(file_and_dfile, **kwargs): + """Needs to be toplevel for ProcessPoolExecutor.""" + file, dfile = file_and_dfile + return compile_file(file, dfile, **kwargs) + def compile_file(fullname, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, invalidation_mode=None): diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py index 2e2552303f8d5..07c31cedb17ba 100644 --- a/Lib/test/test_compileall.py +++ b/Lib/test/test_compileall.py @@ -578,6 +578,47 @@ def test_workers_available_cores(self, compile_dir): self.assertTrue(compile_dir.called) self.assertEqual(compile_dir.call_args[-1]['workers'], None) + def _test_ddir_only(self, *, ddir, parallel=True): + """Recursive compile_dir ddir must contain package paths; bpo39769.""" + fullpath = ["test", "foo"] + path = self.directory + mods = [] + for subdir in fullpath: + path = os.path.join(path, subdir) + os.mkdir(path) + script_helper.make_script(path, "__init__", "") + mods.append(script_helper.make_script(path, "mod", + "def fn(): 1/0\nfn()\n")) + compileall.compile_dir( + self.directory, quiet=True, ddir=ddir, + workers=2 if parallel else 1) + self.assertTrue(mods) + for mod in mods: + self.assertTrue(mod.startswith(self.directory), mod) + modcode = importlib.util.cache_from_source(mod) + modpath = mod[len(self.directory+os.sep):] + _, _, err = script_helper.assert_python_failure(modcode) + expected_in = os.path.join(ddir, modpath) + mod_code_obj = test.test_importlib.util._get_code_from_pyc(modcode) + self.assertEqual(mod_code_obj.co_filename, expected_in) + self.assertIn(f'"{expected_in}"', os.fsdecode(err)) + + def test_ddir_only_one_worker(self): + """Recursive compile_dir ddir= contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="<a prefix>", parallel=False) + + def test_ddir_multiple_workers(self): + """Recursive compile_dir ddir= contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="<a prefix>", parallel=True) + + def test_ddir_empty_only_one_worker(self): + """Recursive compile_dir ddir='' contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=False) + + def test_ddir_empty_multiple_workers(self): + """Recursive compile_dir ddir='' contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=True) + class CommmandLineTestsWithSourceEpoch(CommandLineTestsBase, unittest.TestCase, diff --git a/Lib/test/test_importlib/util.py b/Lib/test/test_importlib/util.py index b0badebc2b8c0..101b7d22bf922 100644 --- a/Lib/test/test_importlib/util.py +++ b/Lib/test/test_importlib/util.py @@ -7,6 +7,7 @@ from importlib import machinery, util, invalidate_caches from importlib.abc import ResourceReader import io +import marshal import os import os.path from pathlib import Path, PurePath @@ -118,6 +119,16 @@ def submodule(parent, name, pkg_dir, content=''): return '{}.{}'.format(parent, name), path +def _get_code_from_pyc(pyc_path): + """Reads a pyc file and returns the unmarshalled code object within. + + No header validation is performed. + """ + with open(pyc_path, 'rb') as pyc_f: + pyc_f.seek(16) + return marshal.load(pyc_f) + + @contextlib.contextmanager def uncache(*names): """Uncache a module from sys.modules. diff --git a/Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst b/Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst new file mode 100644 index 0000000000000..9b564bd10d3b3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst @@ -0,0 +1,4 @@ +The :func:`compileall.compile_dir` function's *ddir* parameter and the +compileall command line flag `-d` no longer write the wrong pathname to the +generated pyc file for submodules beneath the root of the directory tree +being compiled. This fixes a regression introduced with Python 3.5. [View Less]

1 0

bpo-39769: Fix compileall ddir for subpkgs. (GH-18676) (GH-18718)
by Gregory P. Smith March 1, 2020

March 1, 2020

https://github.com/python/cpython/commit/ce720d3e0674d6ac6f1b950c20a89be4cf… commit: ce720d3e0674d6ac6f1b950c20a89be4cfde7853 branch: 3.8 author: Gregory P. Smith <greg(a)krypto.org> committer: GitHub <noreply(a)github.com> date: 2020-03-01T10:42:56-08:00 summary: bpo-39769: Fix compileall ddir for subpkgs. (GH-18676) (GH-18718) Fix compileall.compile_dir() ddir= behavior on sub-packages. Fixes compileall.compile_dir's ddir parameter and compileall command line flag `-d` to no … [View More]longer write the wrong pathname to the generated pyc file for submodules beneath the root of the directory tree being compiled. This fixes a regression introduced with Python 3.5. Tests backported from GH 02673352b5db6ca4d3dc804965facbedfe66425d, the implementation is different due to intervening code changes. But still quiet simple. Why was the bug ever introduced? The refactoring to add parallel execution kept the ddir -> dfile computations but discarded the results instead of sending them to compile_file(). This fixes that. Lack of tests meant this went unnoticed. files: A Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst M Lib/compileall.py M Lib/test/test_compileall.py M Lib/test/test_importlib/util.py diff --git a/Lib/compileall.py b/Lib/compileall.py index 49306d9dabbc5..bfac8efc804d0 100644 --- a/Lib/compileall.py +++ b/Lib/compileall.py @@ -41,7 +41,7 @@ def _walk_dir(dir, ddir=None, maxlevels=10, quiet=0): else: dfile = None if not os.path.isdir(fullname): - yield fullname + yield fullname, ddir elif (maxlevels > 0 and name != os.curdir and name != os.pardir and os.path.isdir(fullname) and not os.path.islink(fullname)): yield from _walk_dir(fullname, ddir=dfile, @@ -76,28 +76,33 @@ def compile_dir(dir, maxlevels=10, ddir=None, force=False, rx=None, from concurrent.futures import ProcessPoolExecutor except ImportError: workers = 1 - files = _walk_dir(dir, quiet=quiet, maxlevels=maxlevels, - ddir=ddir) + files_and_ddirs = _walk_dir(dir, quiet=quiet, maxlevels=maxlevels, + ddir=ddir) success = True if workers != 1 and ProcessPoolExecutor is not None: # If workers == 0, let ProcessPoolExecutor choose workers = workers or None with ProcessPoolExecutor(max_workers=workers) as executor: - results = executor.map(partial(compile_file, - ddir=ddir, force=force, - rx=rx, quiet=quiet, - legacy=legacy, - optimize=optimize, - invalidation_mode=invalidation_mode), - files) + results = executor.map( + partial(_compile_file_tuple, + force=force, rx=rx, quiet=quiet, + legacy=legacy, optimize=optimize, + invalidation_mode=invalidation_mode, + ), + files_and_ddirs) success = min(results, default=True) else: - for file in files: - if not compile_file(file, ddir, force, rx, quiet, + for file, dfile in files_and_ddirs: + if not compile_file(file, dfile, force, rx, quiet, legacy, optimize, invalidation_mode): success = False return success +def _compile_file_tuple(file_and_dfile, **kwargs): + """Needs to be toplevel for ProcessPoolExecutor.""" + file, dfile = file_and_dfile + return compile_file(file, dfile, **kwargs) + def compile_file(fullname, ddir=None, force=False, rx=None, quiet=0, legacy=False, optimize=-1, invalidation_mode=None): diff --git a/Lib/test/test_compileall.py b/Lib/test/test_compileall.py index 04f6e1e049dde..64f092b53b500 100644 --- a/Lib/test/test_compileall.py +++ b/Lib/test/test_compileall.py @@ -577,6 +577,47 @@ def test_workers_available_cores(self, compile_dir): self.assertTrue(compile_dir.called) self.assertEqual(compile_dir.call_args[-1]['workers'], 0) + def _test_ddir_only(self, *, ddir, parallel=True): + """Recursive compile_dir ddir must contain package paths; bpo39769.""" + fullpath = ["test", "foo"] + path = self.directory + mods = [] + for subdir in fullpath: + path = os.path.join(path, subdir) + os.mkdir(path) + script_helper.make_script(path, "__init__", "") + mods.append(script_helper.make_script(path, "mod", + "def fn(): 1/0\nfn()\n")) + compileall.compile_dir( + self.directory, quiet=True, ddir=ddir, + workers=2 if parallel else 1) + self.assertTrue(mods) + for mod in mods: + self.assertTrue(mod.startswith(self.directory), mod) + modcode = importlib.util.cache_from_source(mod) + modpath = mod[len(self.directory+os.sep):] + _, _, err = script_helper.assert_python_failure(modcode) + expected_in = os.path.join(ddir, modpath) + mod_code_obj = test.test_importlib.util._get_code_from_pyc(modcode) + self.assertEqual(mod_code_obj.co_filename, expected_in) + self.assertIn(f'"{expected_in}"', os.fsdecode(err)) + + def test_ddir_only_one_worker(self): + """Recursive compile_dir ddir= contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="<a prefix>", parallel=False) + + def test_ddir_multiple_workers(self): + """Recursive compile_dir ddir= contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="<a prefix>", parallel=True) + + def test_ddir_empty_only_one_worker(self): + """Recursive compile_dir ddir='' contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=False) + + def test_ddir_empty_multiple_workers(self): + """Recursive compile_dir ddir='' contains package paths; bpo39769.""" + return self._test_ddir_only(ddir="", parallel=True) + class CommmandLineTestsWithSourceEpoch(CommandLineTestsBase, unittest.TestCase, diff --git a/Lib/test/test_importlib/util.py b/Lib/test/test_importlib/util.py index e016ea49119ae..e6a1476875b5e 100644 --- a/Lib/test/test_importlib/util.py +++ b/Lib/test/test_importlib/util.py @@ -7,6 +7,7 @@ from importlib import machinery, util, invalidate_caches from importlib.abc import ResourceReader import io +import marshal import os import os.path from pathlib import Path, PurePath @@ -118,6 +119,16 @@ def submodule(parent, name, pkg_dir, content=''): return '{}.{}'.format(parent, name), path +def _get_code_from_pyc(pyc_path): + """Reads a pyc file and returns the unmarshalled code object within. + + No header validation is performed. + """ + with open(pyc_path, 'rb') as pyc_f: + pyc_f.seek(16) + return marshal.load(pyc_f) + + @contextlib.contextmanager def uncache(*names): """Uncache a module from sys.modules. diff --git a/Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst b/Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst new file mode 100644 index 0000000000000..9b564bd10d3b3 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-02-29-13-20-33.bpo-39769.hJmxu4.rst @@ -0,0 +1,4 @@ +The :func:`compileall.compile_dir` function's *ddir* parameter and the +compileall command line flag `-d` no longer write the wrong pathname to the +generated pyc file for submodules beneath the root of the directory tree +being compiled. This fixes a regression introduced with Python 3.5. [View Less]

1 0