From d1411e2c69c36efccda793342731977579147c8e Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Thu, 16 Jan 2025 10:51:36 +0000
Subject: [PATCH] Don't report filepath messages for jobs.

(They'll just point to temp files anyway.)

Also moves schematic netlist fetching to
MAIL_SCH_GET_NETLIST.
Eeschema's generateSchematicNetlist() hasn't
worked in some time.

Also suppresses parity error counts when parity
not run due to failure to find or load schematic.

Also writes out ERC/DRC reports even when they're
set to fail the job.
---
 eeschema/eeschema.cpp              |  66 -----------------
 eeschema/eeschema_jobs_handler.cpp |  20 +-----
 include/kiface_ids.h               |   1 -
 kicad/jobs_runner.cpp              |  43 ++++++++---
 pcbnew/pcbnew_jobs_handler.cpp     | 111 ++++++++++-------------------
 5 files changed, 71 insertions(+), 170 deletions(-)

diff --git a/eeschema/eeschema.cpp b/eeschema/eeschema.cpp
index 2f8e933b6c..8602a948e4 100644
--- a/eeschema/eeschema.cpp
+++ b/eeschema/eeschema.cpp
@@ -76,66 +76,6 @@ SCH_SHEET*  g_RootSheet = nullptr;
 namespace SCH {
 
 
-static std::unique_ptr<SCHEMATIC> readSchematicFromFile( const std::string& aFilename )
-{
-    auto pi = SCH_IO_MGR::FindPlugin( SCH_IO_MGR::SCH_KICAD );
-    std::unique_ptr<SCHEMATIC> schematic = std::make_unique<SCHEMATIC>( nullptr );
-
-    auto &manager = Pgm().GetSettingsManager();
-
-    manager.LoadProject( "" );
-    schematic->Reset();
-    schematic->SetProject( &manager.Prj() );
-    schematic->SetRoot( pi->LoadSchematicFile( aFilename, schematic.get() ) );
-    schematic->CurrentSheet().push_back( &schematic->Root() );
-
-    SCH_SCREENS screens( schematic->Root() );
-
-    for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )
-        screen->UpdateLocalLibSymbolLinks();
-
-    SCH_SHEET_LIST sheets = schematic->Hierarchy();
-
-    // Restore all of the loaded symbol instances from the root sheet screen.
-    sheets.UpdateSymbolInstanceData( schematic->RootScreen()->GetSymbolInstances() );
-
-    if( schematic->RootScreen()->GetFileFormatVersionAtLoad() < 20230221 )
-    {
-        for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )
-            screen->FixLegacyPowerSymbolMismatches();
-    }
-
-    for( SCH_SCREEN* screen = screens.GetFirst(); screen; screen = screens.GetNext() )
-        screen->MigrateSimModels();
-
-    sheets.AnnotatePowerSymbols();
-
-    // NOTE: This is required for multi-unit symbols to be correct
-    for( SCH_SHEET_PATH& sheet : sheets )
-        sheet.UpdateAllScreenReferences();
-
-    // NOTE: SchematicCleanUp is not called; QA schematics must already be clean or else
-    // SchematicCleanUp must be freed from its UI dependencies.
-
-    schematic->ConnectionGraph()->Recalculate( sheets, true );
-
-    return schematic;
-}
-
-
-bool generateSchematicNetlist( const wxString& aFilename, std::string& aNetlist )
-{
-    std::unique_ptr<SCHEMATIC> schematic = readSchematicFromFile( aFilename.ToStdString() );
-    NETLIST_EXPORTER_KICAD exporter( schematic.get() );
-    STRING_FORMATTER formatter;
-
-    exporter.Format( &formatter, GNL_ALL | GNL_OPT_KICAD );
-    aNetlist = formatter.GetString();
-
-    return true;
-}
-
-
 static struct IFACE : public KIFACE_BASE, public UNITS_PROVIDER
 {
     // Of course all are virtual overloads, implementations of the KIFACE.
@@ -325,12 +265,6 @@ static struct IFACE : public KIFACE_BASE, public UNITS_PROVIDER
      */
     void* IfaceOrAddress( int aDataId ) override
     {
-        switch( aDataId )
-        {
-            case KIFACE_NETLIST_SCHEMATIC:
-                return (void*) generateSchematicNetlist;
-        }
-
         return nullptr;
     }
 
diff --git a/eeschema/eeschema_jobs_handler.cpp b/eeschema/eeschema_jobs_handler.cpp
index 2f917e45c6..6c305f925e 100644
--- a/eeschema/eeschema_jobs_handler.cpp
+++ b/eeschema/eeschema_jobs_handler.cpp
@@ -166,10 +166,7 @@ SCHEMATIC* EESCHEMA_JOBS_HANDLER::getSchematic( const wxString& aPath )
         }
 
         if( !m_cliSchematic )
-        {
-            m_reporter->Report( _( "Loading schematic\n" ), RPT_SEVERITY_INFO );
             m_cliSchematic = EESCHEMA_HELPERS::LoadSchematic( schPath, true, false, &project );
-        }
 
         sch = m_cliSchematic;
     }
@@ -179,20 +176,15 @@ SCHEMATIC* EESCHEMA_JOBS_HANDLER::getSchematic( const wxString& aPath )
                 dynamic_cast<SCH_EDIT_FRAME*>( m_kiway->Player( FRAME_SCH, false ) );
 
         if( editFrame )
-        {
             sch = &editFrame->Schematic();
-        }
     }
     else if( !aPath.IsEmpty() )
     {
-        m_reporter->Report( _( "Loading schematic\n" ), RPT_SEVERITY_INFO );
         sch = EESCHEMA_HELPERS::LoadSchematic( aPath, true, false );
     }
 
     if( !sch )
-    {
         m_reporter->Report( _( "Failed to load schematic\n" ), RPT_SEVERITY_ERROR );
-    }
 
     return sch;
 }
@@ -1078,7 +1070,7 @@ int EESCHEMA_JOBS_HANDLER::JobSymUpgrade( JOB* aJob )
         }
         else
         {
-            m_reporter->Report( _( "Symbol library was not updated\n" ), RPT_SEVERITY_INFO );
+            m_reporter->Report( _( "Symbol library was not updated\n" ), RPT_SEVERITY_ERROR );
         }
     }
     else
@@ -1146,8 +1138,6 @@ int EESCHEMA_JOBS_HANDLER::JobSchErc( JOB* aJob )
 
     ERC_TESTER ercTester( sch );
 
-    m_reporter->Report( _( "Running ERC...\n" ), RPT_SEVERITY_INFO );
-
     std::unique_ptr<DS_PROXY_VIEW_ITEM> drawingSheet( getDrawingSheetProxyView( sch ) );
     ercTester.RunTests( drawingSheet.get(), nullptr, m_kiway->KiFACE( KIWAY::FACE_CVPCB ),
                         &sch->Prj(), m_progressReporter );
@@ -1156,7 +1146,7 @@ int EESCHEMA_JOBS_HANDLER::JobSchErc( JOB* aJob )
 
     m_reporter->Report( wxString::Format( _( "Found %d violations\n" ),
                                           markersProvider->GetCount() ),
-                        RPT_SEVERITY_INFO );
+                        RPT_SEVERITY_ERROR );
 
     ERC_REPORT reportWriter( sch, units );
 
@@ -1170,14 +1160,10 @@ int EESCHEMA_JOBS_HANDLER::JobSchErc( JOB* aJob )
     if( !wroteReport )
     {
         m_reporter->Report( wxString::Format( _( "Unable to save ERC report to %s\n" ), outPath ),
-                            RPT_SEVERITY_INFO );
+                            RPT_SEVERITY_ERROR );
         return CLI::EXIT_CODES::ERR_INVALID_OUTPUT_CONFLICT;
     }
 
-    wxFileName outFile( outPath );
-    m_reporter->Report( wxString::Format( _( "Saved ERC Report to %s\n" ), outFile.GetFullName() ),
-                        RPT_SEVERITY_INFO );
-
     if( ercJob->m_exitCodeViolations )
     {
         if( markersProvider->GetCount() > 0 )
diff --git a/include/kiface_ids.h b/include/kiface_ids.h
index 754ffd4e45..3361433e15 100644
--- a/include/kiface_ids.h
+++ b/include/kiface_ids.h
@@ -53,7 +53,6 @@ enum KIFACE_ADDR_ID : int
     KIFACE_GLOBAL_FOOTPRINT_TABLE,
 
     KIFACE_LOAD_SCHEMATIC,
-    KIFACE_NETLIST_SCHEMATIC,
     KIFACE_SCRIPTING_LEGACY,
     KIFACE_SCRIPTING,
 
diff --git a/kicad/jobs_runner.cpp b/kicad/jobs_runner.cpp
index b677a8cf80..36cca44d46 100644
--- a/kicad/jobs_runner.cpp
+++ b/kicad/jobs_runner.cpp
@@ -142,9 +142,22 @@ int JOBS_RUNNER::runSpecialCopyFiles( const JOBSET_JOB* aJob, PROJECT* aProject
 }
 
 
+class JOBSET_OUTPUT_REPORTER : public WX_STRING_REPORTER
+{
+    REPORTER& Report( const wxString& aText, SEVERITY aSeverity ) override
+    {
+        if( aSeverity == RPT_SEVERITY_ERROR || aSeverity == RPT_SEVERITY_WARNING )
+            WX_STRING_REPORTER::Report( aText, aSeverity );
+
+        return *this;
+    }
+};
+
+
 bool JOBS_RUNNER::RunJobsForOutput( JOBSET_OUTPUT* aOutput, bool aBail )
 {
-    bool                       success = true;
+    bool                    genOutputs = true;
+    bool                    success = true;
     std::vector<JOBSET_JOB> jobsForOutput = m_jobsFile->GetJobsForOutput( aOutput );
     wxString msg;
 
@@ -238,11 +251,11 @@ bool JOBS_RUNNER::RunJobsForOutput( JOBSET_OUTPUT* aOutput, bool aBail )
 
         if( !reporterToUse || reporterToUse == &NULL_REPORTER::GetInstance() )
         {
-            reporterToUse = new WX_STRING_REPORTER;
+            reporterToUse = new JOBSET_OUTPUT_REPORTER;
             aOutput->m_lastRunReporters[job.m_id] = reporterToUse;
         }
 
-        int result = 0;
+        int result = CLI::EXIT_CODES::SUCCESS;
 
         if( iface < KIWAY::KIWAY_FACE_COUNT )
         {
@@ -261,11 +274,11 @@ bool JOBS_RUNNER::RunJobsForOutput( JOBSET_OUTPUT* aOutput, bool aBail )
             }
         }
 
-        aOutput->m_lastRunSuccessMap[job.m_id] = result == 0;
+        aOutput->m_lastRunSuccessMap[job.m_id] = ( result == CLI::EXIT_CODES::SUCCESS );
 
         if( m_reporter )
         {
-            if( result == 0 )
+            if( result == CLI::EXIT_CODES::SUCCESS )
             {
                 wxString msg_fmt = wxT( "\033[32;1m%s\033[0m\n" );
                 msg = wxString::Format( msg_fmt, _( "Job successful" ) );
@@ -284,17 +297,25 @@ bool JOBS_RUNNER::RunJobsForOutput( JOBSET_OUTPUT* aOutput, bool aBail )
             m_reporter->Report( msg, RPT_SEVERITY_INFO );
         }
 
-        if( result != 0 )
+        if( result == CLI::EXIT_CODES::ERR_RC_VIOLATIONS )
         {
+            success = false;
+
             if( aBail )
-                return result;
-            else
-                success = false;
+                break;
+        }
+        else if( result != CLI::EXIT_CODES::SUCCESS )
+        {
+            genOutputs = false;
+            success = false;
+
+            if( aBail )
+                break;
         }
     }
 
-    if( success )
-        success = aOutput->m_outputHandler->HandleOutputs( tempDirPath, m_project, outputs );
+    if( genOutputs )
+        success &= aOutput->m_outputHandler->HandleOutputs( tempDirPath, m_project, outputs );
 
     aOutput->m_lastRunSuccess = success;
 
diff --git a/pcbnew/pcbnew_jobs_handler.cpp b/pcbnew/pcbnew_jobs_handler.cpp
index 0186fcdb32..d281664518 100644
--- a/pcbnew/pcbnew_jobs_handler.cpp
+++ b/pcbnew/pcbnew_jobs_handler.cpp
@@ -291,10 +291,7 @@ BOARD* PCBNEW_JOBS_HANDLER::getBoard( const wxString& aPath )
         }
 
         if( !m_cliBoard )
-        {
-            m_reporter->Report( _( "Loading board\n" ), RPT_SEVERITY_INFO );
             m_cliBoard = LoadBoard( pcbPath, true );
-        }
 
         brd = m_cliBoard;
     }
@@ -307,7 +304,6 @@ BOARD* PCBNEW_JOBS_HANDLER::getBoard( const wxString& aPath )
     }
     else
     {
-        m_reporter->Report( _( "Loading board\n" ), RPT_SEVERITY_INFO );
         brd = LoadBoard( aPath, true );
     }
 
@@ -402,14 +398,7 @@ int PCBNEW_JOBS_HANDLER::JobExportStep( JOB* aJob )
                 !aStepJob->m_vrmlModelDir.IsEmpty(), aStepJob->m_vrmlRelativePaths,
                 aStepJob->m_vrmlModelDir, originX, originY );
 
-        if ( success )
-        {
-            wxFileName outFile( outPath );
-            m_reporter->Report( wxString::Format( _( "Successfully exported VRML to %s" ),
-                                                  outFile.GetFullName() ),
-                                RPT_SEVERITY_INFO );
-        }
-        else
+        if( !success )
         {
             m_reporter->Report( _( "Error exporting VRML" ), RPT_SEVERITY_ERROR );
             return CLI::EXIT_CODES::ERR_UNKNOWN;
@@ -691,16 +680,7 @@ int PCBNEW_JOBS_HANDLER::JobExportRender( JOB* aJob )
                                                                             : wxBITMAP_TYPE_JPEG );
     }
 
-    m_reporter->Report( wxString::Format( _( "Actual image size: %dx%d" ), realSize.x, realSize.y )
-                                + wxS( "\n" ),
-                        RPT_SEVERITY_INFO );
-
-    if( success )
-    {
-        m_reporter->Report( _( "Successfully created 3D render image" ) + wxS( "\n" ),
-                            RPT_SEVERITY_INFO );
-    }
-    else
+    if( !success )
     {
         m_reporter->Report( _( "Error creating 3D render image" ) + wxS( "\n" ),
                             RPT_SEVERITY_ERROR );
@@ -1118,8 +1098,6 @@ int PCBNEW_JOBS_HANDLER::JobExportGencad( JOB* aJob )
         return CLI::EXIT_CODES::ERR_UNKNOWN;
     }
 
-    m_reporter->Report( _( "Successfully created genCAD file\n" ), RPT_SEVERITY_INFO );
-
     return CLI::EXIT_CODES::OK;
 }
 
@@ -1471,12 +1449,10 @@ int PCBNEW_JOBS_HANDLER::JobExportFpUpgrade( JOB* aJob )
         if( !wxDir::Exists( upgradeJob->m_libraryPath ) )
         {
             m_reporter->Report( _( "Footprint library path does not exist or is not accessible\n" ),
-                                RPT_SEVERITY_INFO );
+                                RPT_SEVERITY_ERROR );
             return CLI::EXIT_CODES::ERR_INVALID_INPUT_FILE;
         }
 
-        m_reporter->Report( _( "Loading footprint library\n" ), RPT_SEVERITY_INFO );
-
         PCB_IO_KICAD_SEXPR pcb_io( CTL_FOR_LIBRARY );
         FP_CACHE           fpLib( &pcb_io, upgradeJob->m_libraryPath );
 
@@ -1503,8 +1479,6 @@ int PCBNEW_JOBS_HANDLER::JobExportFpUpgrade( JOB* aJob )
 
         if( shouldSave )
         {
-            m_reporter->Report( _( "Saving footprint library\n" ), RPT_SEVERITY_INFO );
-
             try
             {
                 if( !upgradeJob->m_outputLibraryPath.IsEmpty() )
@@ -1520,7 +1494,7 @@ int PCBNEW_JOBS_HANDLER::JobExportFpUpgrade( JOB* aJob )
         }
         else
         {
-            m_reporter->Report( _( "Footprint library was not updated\n" ), RPT_SEVERITY_INFO );
+            m_reporter->Report( _( "Footprint library was not updated\n" ), RPT_SEVERITY_ERROR );
         }
     }
     else
@@ -1544,8 +1518,6 @@ int PCBNEW_JOBS_HANDLER::JobExportFpSvg( JOB* aJob )
     if( svgJob == nullptr )
         return CLI::EXIT_CODES::ERR_UNKNOWN;
 
-    m_reporter->Report( _( "Loading footprint library\n" ), RPT_SEVERITY_INFO );
-
     PCB_IO_KICAD_SEXPR pcb_io( CTL_FOR_LIBRARY );
     FP_CACHE   fpLib( &pcb_io, svgJob->m_libraryPath );
 
@@ -1719,48 +1691,41 @@ int PCBNEW_JOBS_HANDLER::JobExportDrc( JOB* aJob )
     toolManager->SetEnvironment( brd, nullptr, nullptr, Kiface().KifaceSettings(), nullptr );
 
     BOARD_COMMIT commit( toolManager );
+    bool         checkParity = drcJob->m_parity;
+    std::string  netlist_str;
 
-    m_reporter->Report( _( "Running DRC...\n" ), RPT_SEVERITY_INFO );
-
-    if( drcJob->m_parity )
+    if( checkParity )
     {
-        typedef bool (*NETLIST_FN_PTR)( const wxString&, std::string& );
+        wxString annotateMsg = _( "Schematic parity tests require a fully annotated schematic." );
+        netlist_str = annotateMsg;
 
-        KIFACE*        eeschema = m_kiway->KiFACE( KIWAY::FACE_SCH );
-        wxFileName     schematicPath( drcJob->m_filename );
-        NETLIST_FN_PTR netlister = (NETLIST_FN_PTR) eeschema->IfaceOrAddress( KIFACE_NETLIST_SCHEMATIC );
-        std::string    netlist_str;
+        m_kiway->ExpressMail( FRAME_SCH, MAIL_SCH_GET_NETLIST, netlist_str );
 
-        schematicPath.SetExt( FILEEXT::KiCadSchematicFileExtension );
-
-        if( !schematicPath.Exists() )
-            schematicPath.SetExt( FILEEXT::LegacySchematicFileExtension );
-
-        if( !schematicPath.Exists() )
+        if( netlist_str == annotateMsg )
         {
-            m_reporter->Report( _( "Failed to find schematic for parity tests.\n" ),
-                                RPT_SEVERITY_INFO );
+            m_reporter->Report( netlist_str + wxT( "\n" ), RPT_SEVERITY_ERROR );
+            checkParity = false;
         }
-        else
+    }
+
+    if( checkParity )
+    {
+        try
         {
-            (*netlister)( schematicPath.GetFullPath(), netlist_str );
+            STRING_LINE_READER* lineReader = new STRING_LINE_READER( netlist_str,
+                                                                     _( "Eeschema netlist" ) );
+            KICAD_NETLIST_READER netlistReader( lineReader, netlist.get() );
 
-            try
-            {
-                STRING_LINE_READER* lineReader = new STRING_LINE_READER( netlist_str,
-                                                                         _( "Eeschema netlist" ) );
-                KICAD_NETLIST_READER netlistReader( lineReader, netlist.get() );
-
-                netlistReader.LoadNetlist();
-            }
-            catch( const IO_ERROR& )
-            {
-                m_reporter->Report( _( "Failed to fetch schematic netlist for parity tests.\n" ),
-                                    RPT_SEVERITY_INFO );
-            }
-
-            drcEngine->SetSchematicNetlist( netlist.get() );
+            netlistReader.LoadNetlist();
         }
+        catch( const IO_ERROR& )
+        {
+            m_reporter->Report( _( "Failed to fetch schematic netlist for parity tests.\n" ),
+                                RPT_SEVERITY_ERROR );
+            checkParity = false;
+        }
+
+        drcEngine->SetSchematicNetlist( netlist.get() );
     }
 
     drcEngine->SetProgressReporter( nullptr );
@@ -1774,7 +1739,7 @@ int PCBNEW_JOBS_HANDLER::JobExportDrc( JOB* aJob )
 
     brd->RecordDRCExclusions();
     brd->DeleteMARKERs( true, true );
-    drcEngine->RunTests( units, drcJob->m_reportAllTrackErrors, drcJob->m_parity );
+    drcEngine->RunTests( units, drcJob->m_reportAllTrackErrors, checkParity );
     drcEngine->ClearViolationHandler();
 
     commit.Push( _( "DRC" ), SKIP_UNDO | SKIP_SET_DIRTY );
@@ -1797,16 +1762,16 @@ int PCBNEW_JOBS_HANDLER::JobExportDrc( JOB* aJob )
 
     m_reporter->Report( wxString::Format( _( "Found %d violations\n" ),
                                           markersProvider->GetCount() ),
-                        RPT_SEVERITY_INFO );
+                        RPT_SEVERITY_ERROR );
     m_reporter->Report( wxString::Format( _( "Found %d unconnected items\n" ),
                                           ratsnestProvider->GetCount() ),
-                        RPT_SEVERITY_INFO );
+                        RPT_SEVERITY_ERROR );
 
-    if( drcJob->m_parity )
+    if( checkParity )
     {
         m_reporter->Report( wxString::Format( _( "Found %d schematic parity issues\n" ),
                                               fpWarningsProvider->GetCount() ),
-                            RPT_SEVERITY_INFO );
+                            RPT_SEVERITY_ERROR );
     }
 
     DRC_REPORT reportWriter( brd, units, markersProvider, ratsnestProvider, fpWarningsProvider );
@@ -1821,14 +1786,10 @@ int PCBNEW_JOBS_HANDLER::JobExportDrc( JOB* aJob )
     if( !wroteReport )
     {
         m_reporter->Report( wxString::Format( _( "Unable to save DRC report to %s\n" ), outPath ),
-                            RPT_SEVERITY_INFO );
+                            RPT_SEVERITY_ERROR );
         return CLI::EXIT_CODES::ERR_INVALID_OUTPUT_CONFLICT;
     }
 
-    wxFileName outFile( outPath );
-    m_reporter->Report( wxString::Format( _( "Saved DRC Report to %s\n" ), outFile.GetFullName() ),
-                        RPT_SEVERITY_INFO );
-
     if( drcJob->m_exitCodeViolations )
     {
         if( markersProvider->GetCount() > 0 || ratsnestProvider->GetCount() > 0