Cleanup !1322 for Project Backup

Do conservative check for time offset before making the expensive
backup.  Don't clear and re-check the file list for a single file change

common/settings/settings_manager.cpp

@@ -1210,36 +1210,36 @@ wxString SETTINGS_MANAGER::GetProjectBackupsPath() const
 wxString SETTINGS_MANAGER::backupDateTimeFormat = wxT( "%Y-%m-%d_%H%M%S" );
 
 
-bool SETTINGS_MANAGER::BackupProject( REPORTER& aReporter ) const
+bool SETTINGS_MANAGER::BackupProject( REPORTER& aReporter, wxFileName& aTarget ) const
 {
     wxDateTime timestamp = wxDateTime::Now();
 
     wxString fileName = wxString::Format( wxT( "%s-%s" ), Prj().GetProjectName(),
                                           timestamp.Format( backupDateTimeFormat ) );
 
-    wxFileName target;
-    target.SetPath( GetProjectBackupsPath() );
-    target.SetName( fileName );
-    target.SetExt( FILEEXT::ArchiveFileExtension );
+    if( !aTarget.IsOk() )
+    {
+        aTarget.SetPath( GetProjectBackupsPath() );
+        aTarget.SetName( fileName );
+        aTarget.SetExt( FILEEXT::ArchiveFileExtension );
+    }
 
-    if( !target.DirExists() && !wxMkdir( target.GetPath() ) )
+    if( !aTarget.DirExists() && !wxMkdir( aTarget.GetPath() ) )
     {
         wxLogTrace( traceSettings, wxT( "Could not create project backup path %s" ),
-                    target.GetPath() );
+                    aTarget.GetPath() );
         return false;
     }
 
-    if( !target.IsDirWritable() )
+    if( !aTarget.IsDirWritable() )
     {
-        wxLogTrace( traceSettings, wxT( "Backup directory %s is not writable" ), target.GetPath() );
+        wxLogTrace( traceSettings, wxT( "Backup directory %s is not writable" ), aTarget.GetPath() );
         return false;
     }
 
-    wxLogTrace( traceSettings, wxT( "Backing up project to %s" ), target.GetPath() );
+    wxLogTrace( traceSettings, wxT( "Backing up project to %s" ), aTarget.GetPath() );
 
-    PROJECT_ARCHIVER archiver;
-
-    return archiver.Archive( Prj().GetProjectPath(), target.GetFullPath(), aReporter );
+    return PROJECT_ARCHIVER::Archive( Prj().GetProjectPath(), aTarget.GetFullPath(), aReporter );
 }
 
 
@@ -1339,37 +1339,35 @@ bool SETTINGS_MANAGER::TriggerBackupIfNeeded( REPORTER& aReporter ) const
                    return first.GetTicks() > second.GetTicks();
                } );
 
+    // Do we even need to back up?
+    if( !files.empty() )
+    {
+        wxDateTime lastTime = modTime( files[0] );
+
+        if( lastTime.IsValid() )
+        {
+            wxTimeSpan delta = wxDateTime::Now() - modTime( files[0] );
+
+            if( delta.IsShorterThan( wxTimeSpan::Seconds( settings.min_interval ) ) )
+                return true;
+        }
+    }
+
     // Backup
-    bool backupSuccessful = BackupProject( aReporter );
+    wxFileName target;
+    bool backupSuccessful = BackupProject( aReporter, target );
+
     if( !backupSuccessful )
         return false;
 
     // Update the file list
-    files.clear();
-    dir.Traverse( traverser, wxT( "*.zip" ) );
-
-    // Sort newest-first
-    std::sort( files.begin(), files.end(),
-               [&]( const wxString& aFirst, const wxString& aSecond ) -> bool
-               {
-                   wxDateTime first = modTime( aFirst );
-                   wxDateTime second = modTime( aSecond );
-
-                   return first.GetTicks() > second.GetTicks();
-               } );
+    files.insert( files.begin(), target.GetFullPath() );
 
     // Are there any changes since the last backup?
-    if( files.size() > 1 )
+    if( PROJECT_ARCHIVER::AreZipArchivesIdentical( files[0], files[1], aReporter ) )
     {
-        PROJECT_ARCHIVER archiver;
-        bool             identicalToPrevious =
-                archiver.AreZipArchivesIdentical( files[0], files[1], aReporter );
-
-        if( identicalToPrevious )
-        {
-            wxRemoveFile( files[0] );
-            return true;
-        }
+        wxRemoveFile( files[0] );
+        return true;
     }
 
     // Now that we know a backup is needed, apply the retention policy

include/project/project_archiver.h

@@ -43,8 +43,8 @@ public:
      * @param aReporter is used to report status
      * @return true if the archives are identical
      */
-    bool AreZipArchivesIdentical( const wxString& aZipFileA, const wxString& aZipFileB,
-                                  REPORTER& aReporter );
+    static bool AreZipArchivesIdentical( const wxString& aZipFileA, const wxString& aZipFileB,
+                                        REPORTER& aReporter );
 
     /**
      * Creates an archive of the project
@@ -55,8 +55,8 @@ public:
      * @param aIncludeExtraFiles if true will archive legacy and output files
      * @return true if the archive was created successfully
      */
-    bool Archive( const wxString& aSrcDir, const wxString& aDestFile, REPORTER& aReporter,
-                  bool aVerbose = true, bool aIncludeExtraFiles = false );
+    static bool Archive( const wxString& aSrcDir, const wxString& aDestFile, REPORTER& aReporter,
+                         bool aVerbose = true, bool aIncludeExtraFiles = false );
 
     /**
      * Extracts an archive of the current project over existing files
@@ -67,7 +67,7 @@ public:
      * @param aReporter is used to report status
      * @return true if the archive was created successfully
      */
-    bool Unarchive( const wxString& aSrcFile, const wxString& aDestDir, REPORTER& aReporter );
+    static bool Unarchive( const wxString& aSrcFile, const wxString& aDestDir, REPORTER& aReporter );
 };
 
 #endif // KICAD_PROJECT_ARCHIVER_H

include/settings/settings_manager.h

@@ -34,6 +34,7 @@ class PROJECT;
 class PROJECT_FILE;
 class REPORTER;
 class wxSingleInstanceChecker;
+class wxFileName;
 class LOCKFILE;
 
 
@@ -301,9 +302,11 @@ public:
     /**
      * Creates a backup archive of the current project
      * @param aReporter is used for progress reporting
+     * @param aTarget is the full path to the backup file.  If empty, will generate and return the
+     * full path to the backup file.
      * @return true if everything succeeded
      */
-    bool BackupProject( REPORTER& aReporter ) const;
+    bool BackupProject( REPORTER& aReporter, wxFileName& aTarget ) const;
 
     /**
      * Calls BackupProject if a new backup is needed according to the current backup policy.

kicad/files-io.cpp

@@ -90,9 +90,7 @@ void KICAD_MANAGER_FRAME::OnUnarchiveFiles( wxCommandEvent& event )
 
     STATUSBAR_REPORTER reporter( GetStatusBar(), 1 );
 
-    PROJECT_ARCHIVER archiver;
-
-    archiver.Unarchive( zipfiledlg.GetPath(), unzipDir, reporter );
+    PROJECT_ARCHIVER::Unarchive( zipfiledlg.GetPath(), unzipDir, reporter );
 
     if( unzipDir == Prj().GetProjectPath() )
     {