Rework how Python processes are launched

Fixes https://gitlab.com/kicad/code/kicad/-/issues/19465

common/api/api_plugin_manager.cpp

@@ -408,7 +408,7 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
                 [this]( int aRetVal, const wxString& aOutput, const wxString& aError )
                 {
                     wxLogTrace( traceApi,
-                                wxString::Format( "Manager: venv (%d): %s", aRetVal, aOutput ) );
+                                wxString::Format( "Manager: created venv (python returned %d)", aRetVal ) );
 
                     if( !aError.IsEmpty() )
                         wxLogTrace( traceApi, wxString::Format( "Manager: venv err: %s", aError ) );
@@ -455,8 +455,8 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
             manager.Execute( cmd,
                 [this]( int aRetVal, const wxString& aOutput, const wxString& aError )
                 {
-                    wxLogTrace( traceApi, wxString::Format( "Manager: upgrade pip (%d): %s",
+                    wxLogTrace( traceApi, wxString::Format( "Manager: upgrade pip returned %d",
-                                                            aRetVal, aOutput ) );
+                                                            aRetVal ) );
 
                     if( !aError.IsEmpty() )
                     {
@@ -501,15 +501,15 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
             PYTHON_MANAGER manager( *python );
             wxExecuteEnv env;
 
-            if( pythonHome )
-                env.env[wxS( "VIRTUAL_ENV" )] = *pythonHome;
-
 #ifdef _WIN32
             wxString systemRoot;
             wxGetEnv( wxS( "SYSTEMROOT" ), &systemRoot );
             env.env[wxS( "SYSTEMROOT" )] = systemRoot;
 #endif
 
+            if( pythonHome )
+                env.env[wxS( "VIRTUAL_ENV" )] = *pythonHome;
+
             wxString cmd = wxString::Format(
                     wxS( "-m pip install --no-input --isolated --prefer-binary --require-virtualenv "
                          "--exists-action i -r \"%s\"" ),
@@ -520,9 +520,6 @@ void API_PLUGIN_MANAGER::processNextJob( wxCommandEvent& aEvent )
             manager.Execute( cmd,
                 [this, job]( int aRetVal, const wxString& aOutput, const wxString& aError )
                 {
-                    if( !aOutput.IsEmpty() )
-                        wxLogTrace( traceApi, wxString::Format( "Manager: pip: %s", aOutput ) );
-
                     if( !aError.IsEmpty() )
                         wxLogTrace( traceApi, wxString::Format( "Manager: pip stderr: %s", aError ) );
 

common/dialogs/panel_plugin_settings.cpp

@@ -153,5 +153,7 @@ void PANEL_PLUGIN_SETTINGS::validatePythonInterpreter()
 
                          m_stPythonStatus->SetLabel( msg );
                          Layout();
-                     } );
+                    },
+                    /* aEnv = */ nullptr,
+                    /* aSaveOutput = */ true );
 }

scripting/python_manager.cpp

@@ -22,8 +22,10 @@
 #include <gestfich.h>
 #include <wx/process.h>
 
+#include <future>
 #include <utility>
 
+#include <api/api_utils.h>
 #include <paths.h>
 #include <pgm_base.h>
 #include <python_manager.h>
@@ -40,6 +42,9 @@ public:
 
     void OnTerminate( int aPid, int aStatus ) override
     {
+        // Print stdout trace info from the monitor thread
+        wxLog::GetActiveTarget()->Flush();
+
         if( m_callback )
         {
             wxString output, error;
@@ -87,16 +92,50 @@ PYTHON_MANAGER::PYTHON_MANAGER( const wxString& aInterpreterPath )
 void PYTHON_MANAGER::Execute( const wxString& aArgs,
                               const std::function<void( int, const wxString&,
                                   const wxString& )>& aCallback,
-                              const wxExecuteEnv* aEnv )
+                              const wxExecuteEnv* aEnv, bool aSaveOutput )
 {
     PYTHON_PROCESS* process = new PYTHON_PROCESS( aCallback );
     process->Redirect();
 
+    auto monitor = 
+        []( PYTHON_PROCESS* aProcess )
+        {
+            wxInputStream* processOut = aProcess->GetInputStream();
+
+            while( aProcess->IsInputOpened() )
+            {
+                if( processOut->CanRead() )
+                {
+                    char buffer[4096];
+                    buffer[processOut->Read( buffer, sizeof( buffer ) - 1 ).LastRead()] = '\0';
+                    wxString stdOut( buffer, processOut->LastRead() );
+                    stdOut = stdOut.BeforeLast( '\n' );
+                    wxLogTrace( traceApi, wxString::Format( "Python: %s", stdOut ) );
+                }
+            }
+        };
+
     wxString cmd = wxString::Format( wxS( "%s %s" ), m_interpreterPath, aArgs );
     long     pid = wxExecute( cmd, wxEXEC_ASYNC, process, aEnv );
 
     if( pid == 0 )
+    {
+        delete process;
         aCallback( -1, wxEmptyString, _( "Process could not be created" ) );
+    }
+    else
+    {
+        // On Windows, if there is a lot of stdout written by the process, this can
+        // hang up the wxProcess such that it will never call OnTerminate.  To work
+        // around this, we use this monitor thread to just dump the stdout to the
+        // trace log, which prevents the hangup.  This flag is provided to keep the
+        // old behavior for commands where we need to read the output directly,
+        // which is currently only used for detecting the interpreter version.
+        // If we need to use the async monitor thread approach and preserve the stdout
+        // contents in the future, a more complicated hack might be necessary.
+        if( !aSaveOutput )
+            auto future = std::async( std::launch::async, monitor, process );
+    }
 }
 
 

scripting/python_manager.h

@@ -36,7 +36,8 @@ public:
 
     void Execute( const wxString& aArgs,
                   const std::function<void(int, const wxString&, const wxString&)>& aCallback,
-                  const wxExecuteEnv* aEnv = nullptr );
+                  const wxExecuteEnv* aEnv = nullptr,
+                  bool aSaveOutput = false );
 
     wxString GetInterpreterPath() const { return m_interpreterPath; }
     void SetInterpreterPath( const wxString& aPath ) { m_interpreterPath = aPath; }