diff --git a/common/dialogs/git/dialog_git_repository.cpp b/common/dialogs/git/dialog_git_repository.cpp index 1eefd69c4f..3e39dd7b3d 100644 --- a/common/dialogs/git/dialog_git_repository.cpp +++ b/common/dialogs/git/dialog_git_repository.cpp @@ -25,6 +25,7 @@ #include <confirm.h> #include <git2.h> +#include <git/kicad_git_memory.h> #include <gestfich.h> #include <cerrno> @@ -326,13 +327,14 @@ void DIALOG_GIT_REPOSITORY::OnTestClick( wxCommandEvent& event ) git_remote_create_with_fetchspec( &remote, m_repository, "origin", txtURL.ToStdString().c_str(), "+refs/heads/*:refs/remotes/origin/*" ); + KIGIT::GitRemotePtr remotePtr( remote ); + if( git_remote_connect( remote, GIT_DIRECTION_FETCH, &callbacks, nullptr, nullptr ) != GIT_OK ) SetTestResult( true, git_error_last()->message ); else SetTestResult( false, wxEmptyString ); git_remote_disconnect( remote ); - git_remote_free( remote ); auto dlg = wxMessageDialog( this, wxEmptyString, _( "Test Connection" ), wxOK | wxICON_INFORMATION ); diff --git a/common/dialogs/git/dialog_git_switch.cpp b/common/dialogs/git/dialog_git_switch.cpp index aa5adbdca4..2450325683 100644 --- a/common/dialogs/git/dialog_git_switch.cpp +++ b/common/dialogs/git/dialog_git_switch.cpp @@ -24,9 +24,13 @@ #include "dialog_git_switch.h" +#include <git/kicad_git_memory.h> +#include <trace_helpers.h> + #include <wx/button.h> #include <wx/checkbox.h> #include <wx/listctrl.h> +#include <wx/log.h> #include <wx/event.h> #include <wx/sizer.h> #include <wx/timer.h> @@ -240,39 +244,42 @@ void DIALOG_GIT_SWITCH::GetBranches() git_reference* currentBranchReference = nullptr; git_repository_head( ¤tBranchReference, m_repository ); - // Get the current branch name - if( currentBranchReference ) + if( !currentBranchReference ) { - m_currentBranch = git_reference_shorthand( currentBranchReference ); - git_reference_free( currentBranchReference ); + wxLogTrace( traceGit, "Failed to get current branch" ); + return; } + KIGIT::GitReferencePtr currentBranch( currentBranchReference ); + m_currentBranch = git_reference_shorthand( currentBranchReference ); + // Initialize branch iterator git_branch_iterator_new( &branchIterator, m_repository, GIT_BRANCH_ALL ); + KIGIT::GitBranchIteratorPtr branchIteratorPtr( branchIterator ); // Iterate over local branches git_reference* branchReference = nullptr; while( git_branch_next( &branchReference, &branchType, branchIterator ) == 0 ) { + KIGIT::GitReferencePtr branchReferencePtr( branchReference ); + // Get the branch OID const git_oid* branchOid = git_reference_target( branchReference ); // Skip this branch if it doesn't have an OID if( !branchOid ) - { - git_reference_free( branchReference ); continue; - } git_commit* commit = nullptr; if( git_commit_lookup( &commit, m_repository, branchOid ) ) { // Skip this branch if it doesn't have a commit - git_reference_free( branchReference ); continue; } + KIGIT::GitCommitPtr commitPtr( commit ); + // Retrieve commit details BranchData branchData; branchData.commitString = git_commit_message( commit ); @@ -280,10 +287,5 @@ void DIALOG_GIT_SWITCH::GetBranches() branchData.isRemote = branchType == GIT_BRANCH_REMOTE; m_branches[git_reference_shorthand( branchReference )] = branchData; - - git_commit_free( commit ); - git_reference_free( branchReference ); } - - git_branch_iterator_free( branchIterator ); } \ No newline at end of file diff --git a/common/dialogs/git/panel_git_repos.cpp b/common/dialogs/git/panel_git_repos.cpp index afe50d74b4..774e8852af 100644 --- a/common/dialogs/git/panel_git_repos.cpp +++ b/common/dialogs/git/panel_git_repos.cpp @@ -24,10 +24,12 @@ #include <kiplatform/secrets.h> #include <pgm_base.h> #include <settings/common_settings.h> +#include <trace_helpers.h> #include <widgets/std_bitmap_button.h> #include <widgets/wx_grid.h> #include <git2.h> +#include <git/kicad_git_memory.h> #include <wx/bmpbuttn.h> #include <wx/button.h> #include <wx/checkbox.h> @@ -67,32 +69,38 @@ static std::pair<wxString, wxString> getDefaultAuthorAndEmail() if( git_config_open_default( &config ) != 0 ) { - printf( "Failed to open default Git config: %s\n", giterr_last()->message ); + wxLogTrace( traceGit, "Failed to open default Git config: %s", giterr_last()->message ); return std::make_pair( name, email ); } + KIGIT::GitConfigPtr configPtr( config ); + if( git_config_get_entry( &name_c, config, "user.name" ) != 0 ) { - printf( "Failed to get user.name from Git config: %s\n", giterr_last()->message ); + wxLogTrace( traceGit, "Failed to get user.name from Git config: %s", giterr_last()->message ); + return std::make_pair( name, email ); } + + KIGIT::GitConfigEntryPtr namePtr( name_c ); + if( git_config_get_entry( &email_c, config, "user.email" ) != 0 ) { - printf( "Failed to get user.email from Git config: %s\n", giterr_last()->message ); + wxLogTrace( traceGit, "Failed to get user.email from Git config: %s", giterr_last()->message ); + return std::make_pair( name, email ); } + KIGIT::GitConfigEntryPtr emailPtr( email_c ); + if( name_c ) name = name_c->value; if( email_c ) email = email_c->value; - git_config_entry_free( name_c ); - git_config_entry_free( email_c ); - git_config_free( config ); - return std::make_pair( name, email ); } + bool PANEL_GIT_REPOS::TransferDataFromWindow() { COMMON_SETTINGS* settings = Pgm().GetCommonSettings(); @@ -184,36 +192,32 @@ static bool testRepositoryConnection( COMMON_SETTINGS::GIT_REPOSITORY& repositor // Initialize the Git repository git_repository* repo = nullptr; - int result = git_repository_init( &repo, tempDirPath.mb_str( wxConvUTF8 ), 0 ); + const char* path = tempDirPath.mb_str( wxConvUTF8 ); - if (result != 0) + if( git_repository_init( &repo, path, 0 ) != 0 ) { - git_repository_free(repo); git_libgit2_shutdown(); wxRmdir(tempDirPath); return false; } - git_remote* remote = nullptr; - result = git_remote_create_anonymous( &remote, repo, tempDirPath.mb_str( wxConvUTF8 ) ); + KIGIT::GitRepositoryPtr repoPtr( repo ); + git_remote* remote = nullptr; - if (result != 0) + if( git_remote_create_anonymous( &remote, repo, path ) != 0 ) { - git_remote_free(remote); - git_repository_free(repo); git_libgit2_shutdown(); wxRmdir(tempDirPath); return false; } + KIGIT::GitRemotePtr remotePtr( remote ); + // We don't really care about the result of this call, the authentication callback // will set the return values we need - git_remote_connect(remote, GIT_DIRECTION_FETCH, &callbacks, nullptr, nullptr); - - git_remote_disconnect(remote); - git_remote_free(remote); - git_repository_free(repo); + git_remote_connect( remote, GIT_DIRECTION_FETCH, &callbacks, nullptr, nullptr ); + git_remote_disconnect( remote ); git_libgit2_shutdown(); // Clean up the temporary directory diff --git a/common/git/git_add_to_index_handler.cpp b/common/git/git_add_to_index_handler.cpp index e765c9eb60..f97bb263be 100644 --- a/common/git/git_add_to_index_handler.cpp +++ b/common/git/git_add_to_index_handler.cpp @@ -22,6 +22,7 @@ */ #include "git_add_to_index_handler.h" +#include <git/kicad_git_memory.h> #include <iterator> @@ -53,15 +54,14 @@ bool GIT_ADD_TO_INDEX_HANDLER::AddToIndex( const wxString& aFilePath ) return false; } + KIGIT::GitIndexPtr indexPtr( index ); + if( git_index_find( &at_pos, index, aFilePath.ToUTF8().data() ) == GIT_OK ) { - git_index_free( index ); wxLogError( "%s already in index", aFilePath ); return false; } - git_index_free( index ); - // Add file to index if not already there m_filesToAdd.push_back( aFilePath ); @@ -82,6 +82,8 @@ bool GIT_ADD_TO_INDEX_HANDLER::PerformAddToIndex() return false; } + KIGIT::GitIndexPtr indexPtr( index ); + for( auto& file : m_filesToAdd ) { if( git_index_add_bypath( index, file.ToUTF8().data() ) != 0 ) @@ -99,11 +101,8 @@ bool GIT_ADD_TO_INDEX_HANDLER::PerformAddToIndex() m_filesFailedToAdd.clear(); std::copy( m_filesToAdd.begin(), m_filesToAdd.end(), std::back_inserter( m_filesFailedToAdd ) ); - git_index_free( index ); return false; } - git_index_free( index ); - return true; } diff --git a/common/git/git_clone_handler.cpp b/common/git/git_clone_handler.cpp index 03d44b22a5..5a46a549dc 100644 --- a/common/git/git_clone_handler.cpp +++ b/common/git/git_clone_handler.cpp @@ -24,9 +24,12 @@ #include "git_clone_handler.h" #include <git/kicad_git_common.h> +#include <git/kicad_git_memory.h> +#include <trace_helpers.h> #include <git2.h> #include <wx/filename.h> +#include <wx/log.h> GIT_CLONE_HANDLER::GIT_CLONE_HANDLER() : KIGIT_COMMON( nullptr ) {} @@ -41,6 +44,14 @@ GIT_CLONE_HANDLER::~GIT_CLONE_HANDLER() bool GIT_CLONE_HANDLER::PerformClone() { + std::unique_lock<std::mutex> lock( m_gitActionMutex, std::try_to_lock ); + + if( !lock.owns_lock() ) + { + wxLogTrace( traceGit, "GIT_CLONE_HANDLER::PerformClone() could not lock" ); + return false; + } + wxFileName clonePath( m_clonePath ); if( !clonePath.DirExists() ) diff --git a/common/git/git_progress.h b/common/git/git_progress.h index bb5787a3f2..47b9adbf9a 100644 --- a/common/git/git_progress.h +++ b/common/git/git_progress.h @@ -62,6 +62,8 @@ public: } } + virtual void UpdateProgress( int aCurrent, int aTotal, const wxString& aMessage ) {}; + protected: int m_previousProgress; diff --git a/common/git/git_pull_handler.cpp b/common/git/git_pull_handler.cpp index a696008815..9522b96243 100644 --- a/common/git/git_pull_handler.cpp +++ b/common/git/git_pull_handler.cpp @@ -23,34 +23,49 @@ #include "git_pull_handler.h" #include <git/kicad_git_common.h> +#include <git/kicad_git_memory.h> +#include <trace_helpers.h> #include <wx/log.h> #include <iostream> #include <time.h> +#include <memory> -GIT_PULL_HANDLER::GIT_PULL_HANDLER( KIGIT_COMMON* aRepo ) : KIGIT_COMMON( *aRepo ) -{} +GIT_PULL_HANDLER::GIT_PULL_HANDLER( KIGIT_COMMON* aCommon ) : KIGIT_REPO_MIXIN( aCommon ) +{ +} GIT_PULL_HANDLER::~GIT_PULL_HANDLER() -{} - - -bool GIT_PULL_HANDLER::PerformFetch() { - if( !m_repo ) +} + + +bool GIT_PULL_HANDLER::PerformFetch( bool aSkipLock ) +{ + if( !GetRepo() ) return false; + std::unique_lock<std::mutex> lock( GetCommon()->m_gitActionMutex, std::try_to_lock ); + + if( !aSkipLock && !lock.owns_lock() ) + { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformFetch() - Could not lock mutex" ); + return false; + } + // Fetch updates from remote repository git_remote* remote = nullptr; - if( git_remote_lookup( &remote, m_repo, "origin" ) != 0 ) + if( git_remote_lookup( &remote, GetRepo(), "origin" ) != 0 ) { AddErrorString( wxString::Format( _( "Could not lookup remote '%s'" ), "origin" ) ); return false; } + KIGIT::GitRemotePtr remotePtr( remote ); + git_remote_callbacks remoteCallbacks; git_remote_init_callbacks( &remoteCallbacks, GIT_REMOTE_CALLBACKS_VERSION ); remoteCallbacks.sideband_progress = progress_cb; @@ -58,12 +73,11 @@ bool GIT_PULL_HANDLER::PerformFetch() remoteCallbacks.credentials = credentials_cb; remoteCallbacks.payload = this; - m_testedTypes = 0; + TestedTypes() = 0; ResetNextKey(); if( git_remote_connect( remote, GIT_DIRECTION_FETCH, &remoteCallbacks, nullptr, nullptr ) ) { - git_remote_free( remote ); AddErrorString( wxString::Format( _( "Could not connect to remote '%s': %s" ), "origin", git_error_last()->message ) ); return false; @@ -75,28 +89,33 @@ bool GIT_PULL_HANDLER::PerformFetch() if( git_remote_fetch( remote, nullptr, &fetchOptions, nullptr ) ) { - git_remote_free( remote ); AddErrorString( wxString::Format( _( "Could not fetch data from remote '%s': %s" ), "origin", git_error_last()->message ) ); return false; } - git_remote_free( remote ); - return true; } PullResult GIT_PULL_HANDLER::PerformPull() { - PullResult result = PullResult::Success; + PullResult result = PullResult::Success; + std::unique_lock<std::mutex> lock( GetCommon()->m_gitActionMutex, std::try_to_lock ); - if( !PerformFetch() ) + if( !lock.owns_lock() ) + { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformPull() - Could not lock mutex" ); + return PullResult::Error; + } + + if( !PerformFetch( true ) ) return PullResult::Error; git_oid pull_merge_oid = {}; - if( git_repository_fetchhead_foreach( m_repo, fetchhead_foreach_cb, &pull_merge_oid ) ) + if( git_repository_fetchhead_foreach( GetRepo(), fetchhead_foreach_cb, + &pull_merge_oid ) ) { AddErrorString( _( "Could not read 'FETCH_HEAD'" ) ); return PullResult::Error; @@ -104,26 +123,24 @@ PullResult GIT_PULL_HANDLER::PerformPull() git_annotated_commit* fetchhead_commit; - if( git_annotated_commit_lookup( &fetchhead_commit, m_repo, &pull_merge_oid ) ) + if( git_annotated_commit_lookup( &fetchhead_commit, GetRepo(), &pull_merge_oid ) ) { AddErrorString( _( "Could not lookup commit" ) ); return PullResult::Error; } + KIGIT::GitAnnotatedCommitPtr fetchheadCommitPtr( fetchhead_commit ); const git_annotated_commit* merge_commits[] = { fetchhead_commit }; - git_merge_analysis_t merge_analysis; - git_merge_preference_t merge_preference = GIT_MERGE_PREFERENCE_NONE; + git_merge_analysis_t merge_analysis; + git_merge_preference_t merge_preference = GIT_MERGE_PREFERENCE_NONE; - if( git_merge_analysis( &merge_analysis, &merge_preference, m_repo, merge_commits, 1 ) ) + if( git_merge_analysis( &merge_analysis, &merge_preference, GetRepo(), merge_commits, + 1 ) ) { AddErrorString( _( "Could not analyze merge" ) ); - git_annotated_commit_free( fetchhead_commit ); return PullResult::Error; } - if( !( merge_analysis & GIT_MERGE_ANALYSIS_NORMAL ) ) - git_annotated_commit_free( fetchhead_commit ); - if( merge_analysis & GIT_MERGE_ANALYSIS_UNBORN ) { AddErrorString( _( "Invalid HEAD. Cannot merge." ) ); @@ -133,23 +150,26 @@ PullResult GIT_PULL_HANDLER::PerformPull() // Nothing to do if the repository is up to date if( merge_analysis & GIT_MERGE_ANALYSIS_UP_TO_DATE ) { - git_repository_state_cleanup( m_repo ); + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformPull() - Repository is up to date" ); + git_repository_state_cleanup( GetRepo() ); return PullResult::UpToDate; } // Fast-forward is easy, just update the local reference if( merge_analysis & GIT_MERGE_ANALYSIS_FASTFORWARD ) { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformPull() - Fast-forward merge" ); return handleFastForward(); } if( merge_analysis & GIT_MERGE_ANALYSIS_NORMAL ) { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformPull() - Normal merge" ); PullResult ret = handleMerge( merge_commits, 1 ); - git_annotated_commit_free( fetchhead_commit ); return ret; } + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformPull() - Merge needs resolution" ); //TODO: handle merges when they need to be resolved return result; @@ -175,7 +195,7 @@ std::string GIT_PULL_HANDLER::getFirstLineFromCommitMessage( const std::string& std::string GIT_PULL_HANDLER::getFormattedCommitDate( const git_time& aTime ) { - char dateBuffer[64]; + char dateBuffer[64]; time_t time = static_cast<time_t>( aTime.time ); strftime( dateBuffer, sizeof( dateBuffer ), "%Y-%b-%d %H:%M:%S", gmtime( &time ) ); return dateBuffer; @@ -184,92 +204,80 @@ std::string GIT_PULL_HANDLER::getFormattedCommitDate( const git_time& aTime ) PullResult GIT_PULL_HANDLER::handleFastForward() { - // Update local references with fetched data - auto git_reference_deleter = - []( void* p ) - { - git_reference_free( static_cast<git_reference*>( p ) ); - }; + git_reference* rawRef = nullptr; - git_reference* rawRef = nullptr; + if( git_repository_head( &rawRef, GetRepo() ) ) + { + AddErrorString( _( "Could not get repository head" ) ); + return PullResult::Error; + } - if( git_repository_head( &rawRef, m_repo ) ) + KIGIT::GitReferencePtr headRef( rawRef ); + + const char* updatedRefName = git_reference_name( rawRef ); + + git_oid updatedRefOid; + + if( git_reference_name_to_id( &updatedRefOid, GetRepo(), updatedRefName ) ) + { + AddErrorString( wxString::Format( _( "Could not get reference OID for reference '%s'" ), + updatedRefName ) ); + return PullResult::Error; + } + + git_checkout_options checkoutOptions; + git_checkout_init_options( &checkoutOptions, GIT_CHECKOUT_OPTIONS_VERSION ); + checkoutOptions.checkout_strategy = GIT_CHECKOUT_SAFE; + + if( git_checkout_head( GetRepo(), &checkoutOptions ) ) + { + AddErrorString( _( "Failed to perform checkout operation." ) ); + return PullResult::Error; + } + + // Collect commit details for updated references + git_revwalk* revWalker = nullptr; + git_revwalk_new( &revWalker, GetRepo() ); + KIGIT::GitRevWalkPtr revWalkerPtr( revWalker ); + git_revwalk_sorting( revWalker, GIT_SORT_TIME ); + git_revwalk_push_glob( revWalker, updatedRefName ); + + git_oid commitOid; + + while( git_revwalk_next( &commitOid, revWalker ) == 0 ) + { + git_commit* commit = nullptr; + + if( git_commit_lookup( &commit, GetRepo(), &commitOid ) ) { - AddErrorString( _( "Could not get repository head" ) ); + AddErrorString( wxString::Format( _( "Could not lookup commit '{}'" ), + git_oid_tostr_s( &commitOid ) ) ); return PullResult::Error; } - // Defer destruction of the git_reference until this function exits somewhere, since - // updatedRefName etc. just point to memory inside the reference - std::unique_ptr<git_reference, decltype( git_reference_deleter )> updatedRef( rawRef ); + KIGIT::GitCommitPtr commitPtr( commit ); - const char* updatedRefName = git_reference_name( updatedRef.get() ); + CommitDetails details; + details.m_sha = git_oid_tostr_s( &commitOid ); + details.m_firstLine = getFirstLineFromCommitMessage( git_commit_message( commit ) ); + details.m_author = git_commit_author( commit )->name; + details.m_date = getFormattedCommitDate( git_commit_author( commit )->when ); - git_oid updatedRefOid; + std::pair<std::string, std::vector<CommitDetails>>& branchCommits = + m_fetchResults.emplace_back(); + branchCommits.first = updatedRefName; + branchCommits.second.push_back( details ); + } - if( git_reference_name_to_id( &updatedRefOid, m_repo, updatedRefName ) ) - { - AddErrorString( wxString::Format( _( "Could not get reference OID for reference '%s'" ), - updatedRefName ) ); - return PullResult::Error; - } - - git_checkout_options checkoutOptions; - git_checkout_init_options( &checkoutOptions, GIT_CHECKOUT_OPTIONS_VERSION ); - checkoutOptions.checkout_strategy = GIT_CHECKOUT_SAFE; - - if( git_checkout_head( m_repo, &checkoutOptions ) ) - { - AddErrorString( _( "Failed to perform checkout operation." ) ); - return PullResult::Error; - } - - // Collect commit details for updated references - git_revwalk* revWalker = nullptr; - git_revwalk_new( &revWalker, m_repo ); - git_revwalk_sorting( revWalker, GIT_SORT_TIME ); - git_revwalk_push_glob( revWalker, updatedRefName ); - - git_oid commitOid; - - while( git_revwalk_next( &commitOid, revWalker ) == 0 ) - { - git_commit* commit = nullptr; - - if( git_commit_lookup( &commit, m_repo, &commitOid ) ) - { - AddErrorString( wxString::Format( _( "Could not lookup commit '{}'" ), - git_oid_tostr_s( &commitOid ) ) ); - git_revwalk_free( revWalker ); - return PullResult::Error; - } - - CommitDetails details; - details.m_sha = git_oid_tostr_s( &commitOid ); - details.m_firstLine = getFirstLineFromCommitMessage( git_commit_message( commit ) ); - details.m_author = git_commit_author( commit )->name; - details.m_date = getFormattedCommitDate( git_commit_author( commit )->when ); - - std::pair<std::string, std::vector<CommitDetails>>& branchCommits = - m_fetchResults.emplace_back(); - branchCommits.first = updatedRefName; - branchCommits.second.push_back( details ); - - //TODO: log these to the parent - git_commit_free( commit ); - } - - git_revwalk_free( revWalker ); - - git_repository_state_cleanup( m_repo ); - return PullResult::FastForward; + git_repository_state_cleanup( GetRepo() ); + return PullResult::FastForward; } PullResult GIT_PULL_HANDLER::handleMerge( const git_annotated_commit** aMergeHeads, size_t aMergeHeadsCount ) { - git_merge_options merge_opts; + git_merge_options merge_opts; git_merge_options_init( &merge_opts, GIT_MERGE_OPTIONS_VERSION ); git_checkout_options checkout_opts; @@ -277,7 +285,7 @@ PullResult GIT_PULL_HANDLER::handleMerge( const git_annotated_commit** aMergeHea checkout_opts.checkout_strategy = GIT_CHECKOUT_SAFE; - if( git_merge( m_repo, aMergeHeads, aMergeHeadsCount, &merge_opts, &checkout_opts ) ) + if( git_merge( GetRepo(), aMergeHeads, aMergeHeadsCount, &merge_opts, &checkout_opts ) ) { AddErrorString( _( "Could not merge commits" ) ); return PullResult::Error; @@ -286,12 +294,14 @@ PullResult GIT_PULL_HANDLER::handleMerge( const git_annotated_commit** aMergeHea // Get the repository index git_index* index = nullptr; - if( git_repository_index( &index, m_repo ) ) + if( git_repository_index( &index, GetRepo() ) ) { AddErrorString( _( "Could not get repository index" ) ); return PullResult::Error; } + KIGIT::GitIndexPtr indexPtr( index ); + // Check for conflicts git_index_conflict_iterator* conflicts = nullptr; @@ -301,9 +311,11 @@ PullResult GIT_PULL_HANDLER::handleMerge( const git_annotated_commit** aMergeHea return PullResult::Error; } - const git_index_entry* ancestor = nullptr; - const git_index_entry* our = nullptr; - const git_index_entry* their = nullptr; + KIGIT::GitIndexConflictIteratorPtr conflictsPtr( conflicts ); + + const git_index_entry* ancestor = nullptr; + const git_index_entry* our = nullptr; + const git_index_entry* their = nullptr; std::vector<ConflictData> conflict_data; while( git_index_conflict_next( &ancestor, &our, &their, conflicts ) == 0 ) @@ -362,14 +374,11 @@ PullResult GIT_PULL_HANDLER::handleMerge( const git_annotated_commit** aMergeHea git_index_write( index ); } - git_index_conflict_iterator_free( conflicts ); - git_index_free( index ); - return conflict_data.empty() ? PullResult::Success : PullResult::MergeFailed; } void GIT_PULL_HANDLER::UpdateProgress( int aCurrent, int aTotal, const wxString& aMessage ) { - ReportProgress( aCurrent, aTotal, aMessage ); + } diff --git a/common/git/git_pull_handler.h b/common/git/git_pull_handler.h index 125e460bc1..ab7f0f29b5 100644 --- a/common/git/git_pull_handler.h +++ b/common/git/git_pull_handler.h @@ -21,17 +21,17 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef GITPULLHANDLER_HPP -#define GITPULLHANDLER_HPP +#ifndef _GIT_PULL_HANDLER_H_ +#define _GIT_PULL_HANDLER_H_ + +#include <git/git_repo_mixin.h> +#include <git/git_progress.h> +#include <git/kicad_git_errors.h> -#include <git2.h> -#include <functional> #include <vector> #include <string> - -#include "kicad_git_common.h" - -#include <git/git_progress.h> +#include <wx/string.h> +#include <git2.h> // Structure to store commit details struct CommitDetails @@ -65,36 +65,28 @@ struct ConflictData }; -class GIT_PULL_HANDLER : public KIGIT_COMMON, public GIT_PROGRESS +class GIT_PULL_HANDLER : public KIGIT_ERRORS, public GIT_PROGRESS, public KIGIT_REPO_MIXIN { public: GIT_PULL_HANDLER( KIGIT_COMMON* aCommon ); ~GIT_PULL_HANDLER(); + bool PerformFetch( bool aSkipLock = false ); PullResult PerformPull(); - bool PerformFetch(); - const std::vector<std::pair<std::string, std::vector<CommitDetails>>>& GetFetchResults() const; - // Set the callback function for conflict resolution - void SetConflictCallback( - std::function<int( std::vector<ConflictData>& aConflicts )> aCallback ) - { - m_conflictCallback = aCallback; - } - + // Implementation for progress updates void UpdateProgress( int aCurrent, int aTotal, const wxString& aMessage ) override; + private: - std::string getFirstLineFromCommitMessage( const std::string& aMessage ); - std::string getFormattedCommitDate( const git_time& aTime );private: - - PullResult handleFastForward(); - PullResult handleMerge( const git_annotated_commit** aMergeHeads, size_t aMergeHeadsCount); - std::vector<std::pair<std::string, std::vector<CommitDetails>>> m_fetchResults; - std::function<int( std::vector<ConflictData>& aConflicts )> m_conflictCallback; + + std::string getFirstLineFromCommitMessage( const std::string& aMessage ); + std::string getFormattedCommitDate( const git_time& aTime ); + PullResult handleFastForward(); + PullResult handleMerge( const git_annotated_commit** aMergeHeads, size_t aMergeHeadsCount ); }; -#endif // GITPULLHANDLER_HPP +#endif // _GIT_PULL_HANDLER_H_ diff --git a/common/git/git_push_handler.cpp b/common/git/git_push_handler.cpp index d849c74fa3..8687830370 100644 --- a/common/git/git_push_handler.cpp +++ b/common/git/git_push_handler.cpp @@ -23,32 +23,44 @@ #include "git_push_handler.h" #include <git/kicad_git_common.h> +#include <git/kicad_git_memory.h> +#include <trace_helpers.h> #include <iostream> -GIT_PUSH_HANDLER::GIT_PUSH_HANDLER( KIGIT_COMMON* aRepo ) : KIGIT_COMMON( *aRepo ) -{} +#include <wx/log.h> +GIT_PUSH_HANDLER::GIT_PUSH_HANDLER( KIGIT_COMMON* aRepo ) : KIGIT_REPO_MIXIN( aRepo ) +{} GIT_PUSH_HANDLER::~GIT_PUSH_HANDLER() {} - PushResult GIT_PUSH_HANDLER::PerformPush() { + std::unique_lock<std::mutex> lock( GetCommon()->m_gitActionMutex, std::try_to_lock ); + + if(!lock.owns_lock()) + { + wxLogTrace(traceGit, "GIT_PUSH_HANDLER::PerformPush: Could not lock mutex"); + return PushResult::Error; + } + PushResult result = PushResult::Success; // Fetch updates from remote repository git_remote* remote = nullptr; - if( git_remote_lookup( &remote, m_repo, "origin" ) != 0 ) + if(git_remote_lookup(&remote, GetRepo(), "origin") != 0) { - AddErrorString( _( "Could not lookup remote" ) ); + AddErrorString(_("Could not lookup remote")); return PushResult::Error; } + KIGIT::GitRemotePtr remotePtr(remote); + git_remote_callbacks remoteCallbacks; - git_remote_init_callbacks( &remoteCallbacks, GIT_REMOTE_CALLBACKS_VERSION ); + git_remote_init_callbacks(&remoteCallbacks, GIT_REMOTE_CALLBACKS_VERSION); remoteCallbacks.sideband_progress = progress_cb; remoteCallbacks.transfer_progress = transfer_progress_cb; remoteCallbacks.update_tips = update_cb; @@ -56,14 +68,13 @@ PushResult GIT_PUSH_HANDLER::PerformPush() remoteCallbacks.credentials = credentials_cb; remoteCallbacks.payload = this; - m_testedTypes = 0; + TestedTypes() = 0; ResetNextKey(); if( git_remote_connect( remote, GIT_DIRECTION_PUSH, &remoteCallbacks, nullptr, nullptr ) ) { AddErrorString( wxString::Format( _( "Could not connect to remote: %s" ), git_error_last()->message ) ); - git_remote_free( remote ); return PushResult::Error; } @@ -74,30 +85,29 @@ PushResult GIT_PUSH_HANDLER::PerformPush() // Get the current HEAD reference git_reference* head = nullptr; - if( git_repository_head( &head, m_repo ) != 0 ) + if( git_repository_head( &head, GetRepo() ) != 0 ) { + git_remote_disconnect( remote ); AddErrorString( _( "Could not get repository head" ) ); - git_remote_free( remote ); return PushResult::Error; } + KIGIT::GitReferencePtr headPtr( head ); + // Create refspec for current branch const char* refs[1]; refs[0] = git_reference_name( head ); const git_strarray refspecs = { (char**) refs, 1 }; - git_reference_free(head); if( git_remote_push( remote, &refspecs, &pushOptions ) ) { AddErrorString( wxString::Format( _( "Could not push to remote: %s" ), git_error_last()->message ) ); git_remote_disconnect( remote ); - git_remote_free( remote ); return PushResult::Error; } git_remote_disconnect( remote ); - git_remote_free( remote ); return result; } diff --git a/common/git/git_push_handler.h b/common/git/git_push_handler.h index a68f65bcff..d0d8951b80 100644 --- a/common/git/git_push_handler.h +++ b/common/git/git_push_handler.h @@ -21,37 +21,37 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef GITPUSHHANDLER_HPP -#define GITPUSHHANDLER_HPP +#ifndef _GIT_PUSH_HANDLER_H_ +#define _GIT_PUSH_HANDLER_H_ -#include <git2.h> -#include <functional> -#include <vector> -#include <string> - -#include "kicad_git_common.h" #include <git/git_progress.h> +#include <git/git_repo_mixin.h> +#include <git/kicad_git_errors.h> +#include <wx/string.h> + +class KIGIT_COMMON; -// Enum for result codes enum class PushResult { Success, - Error, - UpToDate + Error }; -class GIT_PUSH_HANDLER : public KIGIT_COMMON, public GIT_PROGRESS +class GIT_PUSH_HANDLER : public KIGIT_ERRORS, public GIT_PROGRESS, public KIGIT_REPO_MIXIN { public: - GIT_PUSH_HANDLER( KIGIT_COMMON* aRepo ); + GIT_PUSH_HANDLER( KIGIT_COMMON* aCommon ); ~GIT_PUSH_HANDLER(); PushResult PerformPush(); - void UpdateProgress( int aCurrent, int aTotal, const wxString& aMessage ) override; + // Virtual method for progress reporting + virtual void ReportProgress(int aCurrent, int aTotal, const wxString& aMessage) {} private: + // Implementation of GIT_PROGRESS's virtual method + void UpdateProgress(int aCurrent, int aTotal, const wxString& aMessage) override; }; -#endif // GITPUSHHANDLER_HPP +#endif // _GIT_PUSH_HANDLER_H_ diff --git a/common/git/git_remove_from_index_handler.cpp b/common/git/git_remove_from_index_handler.cpp index b3fb590440..f94c0a9116 100644 --- a/common/git/git_remove_from_index_handler.cpp +++ b/common/git/git_remove_from_index_handler.cpp @@ -24,6 +24,7 @@ #include <wx/string.h> #include <wx/log.h> +#include <git/kicad_git_memory.h> #include "git_remove_from_index_handler.h" GIT_REMOVE_FROM_INDEX_HANDLER::GIT_REMOVE_FROM_INDEX_HANDLER( git_repository* aRepository ) @@ -51,15 +52,14 @@ bool GIT_REMOVE_FROM_INDEX_HANDLER::RemoveFromIndex( const wxString& aFilePath ) return false; } + KIGIT::GitIndexPtr indexPtr( index ); + if( git_index_find( &at_pos, index, aFilePath.ToUTF8().data() ) != 0 ) { - git_index_free( index ); wxLogError( "Failed to find index entry for %s", aFilePath ); return false; } - git_index_free( index ); - m_filesToRemove.push_back( aFilePath ); return true; } @@ -78,6 +78,8 @@ void GIT_REMOVE_FROM_INDEX_HANDLER::PerformRemoveFromIndex() return; } + KIGIT::GitIndexPtr indexPtr( index ); + if( git_index_remove_bypath( index, file.ToUTF8().data() ) != 0 ) { wxLogError( "Failed to remove index entry for %s", file ); @@ -95,7 +97,5 @@ void GIT_REMOVE_FROM_INDEX_HANDLER::PerformRemoveFromIndex() wxLogError( "Failed to write index tree" ); return; } - - git_index_free( index ); } } diff --git a/common/git/git_repo_mixin.h b/common/git/git_repo_mixin.h new file mode 100644 index 0000000000..7a08dea3e3 --- /dev/null +++ b/common/git/git_repo_mixin.h @@ -0,0 +1,193 @@ +// This program is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, see <http://www.gnu.org/licenses/>. + +#ifndef GIT_REPO_MIXIN_H +#define GIT_REPO_MIXIN_H + +#include "kicad_git_common.h" + +class KIGIT_REPO_MIXIN +{ +public: + KIGIT_REPO_MIXIN( KIGIT_COMMON* aCommon ) : m_common( aCommon ) + { + } + + virtual ~KIGIT_REPO_MIXIN() + { + // Non-owning, don't delete + } + + /** + * @brief Get the current branch name + * @return The current branch name + */ + wxString GetCurrentBranchName() const + { + return m_common->GetCurrentBranchName(); + } + + /** + * @brief Get a list of branch names + * @return A vector of branch names + */ + std::vector<wxString> GetBranchNames() const + { + return m_common->GetBranchNames(); + } + + /** + * @brief Get a list of project directories + * @return A vector of project directories + */ + std::vector<wxString> GetProjectDirs() + { + return m_common->GetProjectDirs(); + } + + /** + * @brief Get a pointer to the git repository + * @return A pointer to the git repository + */ + git_repository* GetRepo() const + { + return m_common->GetRepo(); + } + + /** + * @brief Get a pair of sets of files that differ locally from the remote repository + * @return A pair of sets of files that differ locally from the remote repository + */ + std::pair<std::set<wxString>, std::set<wxString>> GetDifferentFiles() const + { + return m_common->GetDifferentFiles(); + } + + /** + * @brief Get the common object + * @return The common object + */ + + KIGIT_COMMON* GetCommon() const + { + return m_common; + } + + /** + * @brief Reset the next public key to test + */ + void ResetNextKey() { m_common->ResetNextKey(); } + + /** + * @brief Get the next public key + * @return The next public key + */ + wxString GetNextPublicKey() + { + return m_common->GetNextPublicKey(); + } + + /** + * @brief Get the connection type + * @return The connection type + */ + KIGIT_COMMON::GIT_CONN_TYPE GetConnType() const + { + return m_common->GetConnType(); + } + + /** + * @brief Get the username + * @return The username + */ + wxString GetUsername() const + { + return m_common->GetUsername(); + } + + /** + * @brief Get the password + * @return The password + */ + wxString GetPassword() const + { + return m_common->GetPassword(); + } + + /** + * @brief Set the connection type + * @param aConnType The connection type + */ + void SetConnType( KIGIT_COMMON::GIT_CONN_TYPE aConnType ) + { + m_common->SetConnType( aConnType ); + } + + /** + * @brief Set the username + * @param aUsername The username + */ + void SetUsername( const wxString& aUsername ) + { + m_common->SetUsername( aUsername ); + } + + /** + * @brief Set the password + * @param aPassword The password + */ + void SetPassword( const wxString& aPassword ) + { + m_common->SetPassword( aPassword ); + } + + /** + * @brief Set the SSH key + * @param aSSHKey The SSH key + */ + void SetSSHKey( const wxString& aSSHKey ) + { + m_common->SetSSHKey( aSSHKey ); + } + + /** + * @brief Get the remote name + * @return The remote name + */ + wxString GetRemotename() const + { + return m_common->GetRemotename(); + } + + /** + * @brief Get the git root directory + * @return The git root directory + */ + wxString GetGitRootDirectory() const + { + return m_common->GetGitRootDirectory(); + } + + /** + * @brief Return the connection types that have been tested for authentication + * @return The connection types that have been tested for authentication + */ + unsigned& TestedTypes() { return m_common->TestedTypes(); } + + +private: + KIGIT_COMMON* m_common; +}; + + +#endif // GIT_REPO_MIXIN_H \ No newline at end of file diff --git a/common/git/kicad_git_common.cpp b/common/git/kicad_git_common.cpp index 8d55e77679..b4f7899030 100644 --- a/common/git/kicad_git_common.cpp +++ b/common/git/kicad_git_common.cpp @@ -22,8 +22,11 @@ */ #include "kicad_git_common.h" +#include "kicad_git_memory.h" +#include <git/git_progress.h> #include <kiplatform/secrets.h> +#include <trace_helpers.h> #include <wx/filename.h> #include <wx/log.h> @@ -36,6 +39,22 @@ KIGIT_COMMON::KIGIT_COMMON( git_repository* aRepo ) : m_repo( aRepo ), m_connType( GIT_CONN_TYPE::GIT_CONN_LOCAL ), m_testedTypes( 0 ) {} +KIGIT_COMMON::KIGIT_COMMON( const KIGIT_COMMON& aOther ) : + // Initialize base class and member variables + KIGIT_ERRORS(), + m_repo( aOther.m_repo ), + m_connType( aOther.m_connType ), + m_remote( aOther.m_remote ), + m_hostname( aOther.m_hostname ), + m_username( aOther.m_username ), + m_password( aOther.m_password ), + m_testedTypes( aOther.m_testedTypes ), + // The mutex is default-initialized, not copied + m_gitActionMutex(), + m_publicKeys( aOther.m_publicKeys ), + m_nextPublicKey( aOther.m_nextPublicKey ) +{ +} KIGIT_COMMON::~KIGIT_COMMON() {} @@ -56,25 +75,24 @@ wxString KIGIT_COMMON::GetCurrentBranchName() const if( retval && retval != GIT_EUNBORNBRANCH && retval != GIT_ENOTFOUND ) return wxEmptyString; - git_reference *branch; + KIGIT::GitReferencePtr headPtr( head ); + git_reference* branch; if( git_reference_resolve( &branch, head ) ) { - git_reference_free( head ); + wxLogTrace( traceGit, "Failed to resolve branch" ); return wxEmptyString; } - git_reference_free( head ); - const char* branchName = ""; + KIGIT::GitReferencePtr branchPtr( branch ); + const char* branchName = ""; if( git_branch_name( &branchName, branch ) ) { - git_reference_free( branch ); + wxLogTrace( traceGit, "Failed to get branch name" ); return wxEmptyString; } - git_reference_free( branch ); - return wxString( branchName ); } @@ -91,38 +109,45 @@ std::vector<wxString> KIGIT_COMMON::GetBranchNames() const git_branch_iterator* branchIterator = nullptr; if( git_branch_iterator_new( &branchIterator, m_repo, GIT_BRANCH_LOCAL ) ) + { + wxLogTrace( traceGit, "Failed to get branch iterator" ); return branchNames; + } - git_reference* branchReference = nullptr; - git_branch_t branchType; + KIGIT::GitBranchIteratorPtr branchIteratorPtr( branchIterator ); + git_reference* branchReference = nullptr; + git_branch_t branchType; while( git_branch_next( &branchReference, &branchType, branchIterator ) != GIT_ITEROVER ) { const char* branchName = ""; + KIGIT::GitReferencePtr branchReferencePtr( branchReference ); if( git_branch_name( &branchName, branchReference ) ) + { + wxLogTrace( traceGit, "Failed to get branch name in iter loop" ); continue; + } const git_oid* commitId = git_reference_target( branchReference ); git_commit* commit = nullptr; if( git_commit_lookup( &commit, m_repo, commitId ) ) + { + wxLogTrace( traceGit, "Failed to get commit in iter loop" ); continue; + } - git_time_t commitTime = git_commit_time( commit ); + KIGIT::GitCommitPtr commitPtr( commit ); + git_time_t commitTime = git_commit_time( commit ); if( git_branch_is_head( branchReference ) ) firstName = branchName; else branchNamesMap.emplace( commitTime, branchName ); - - git_commit_free( commit ); - git_reference_free( branchReference ); } - git_branch_iterator_free( branchIterator ); - // Add the current branch to the top of the list if( !firstName.IsEmpty() ) branchNames.push_back( firstName ); @@ -146,22 +171,26 @@ std::vector<wxString> KIGIT_COMMON::GetProjectDirs() if( git_reference_name_to_id( &oid, m_repo, "HEAD" ) != GIT_OK ) { - wxLogError( "An error occurred: %s", git_error_last()->message ); + wxLogTrace( traceGit, "An error occurred: %s", git_error_last()->message ); return projDirs; } if( git_commit_lookup( &commit, m_repo, &oid ) != GIT_OK ) { - wxLogError( "An error occurred: %s", git_error_last()->message ); + wxLogTrace( traceGit, "An error occurred: %s", git_error_last()->message ); return projDirs; } + KIGIT::GitCommitPtr commitPtr( commit ); + if( git_commit_tree( &tree, commit ) != GIT_OK ) { - wxLogError( "An error occurred: %s", git_error_last()->message ); + wxLogTrace( traceGit, "An error occurred: %s", git_error_last()->message ); return projDirs; } + KIGIT::GitTreePtr treePtr( tree ); + // Define callback git_tree_walk( tree, GIT_TREEWALK_PRE, @@ -182,9 +211,6 @@ std::vector<wxString> KIGIT_COMMON::GetProjectDirs() }, &projDirs ); - git_tree_free( tree ); - git_commit_free( commit ); - std::sort( projDirs.begin(), projDirs.end(), []( const wxString& a, const wxString& b ) { @@ -210,12 +236,17 @@ std::pair<std::set<wxString>,std::set<wxString>> KIGIT_COMMON::GetDifferentFiles git_revwalk* walker = nullptr; if( git_revwalk_new( &walker, m_repo ) != GIT_OK ) + { + wxLogTrace( traceGit, "Failed to create revwalker" ); return modified_set; + } + + KIGIT::GitRevWalkPtr walkerPtr( walker ); if( ( git_revwalk_push( walker, from_oid ) != GIT_OK ) || ( git_revwalk_hide( walker, to_oid ) != GIT_OK ) ) { - git_revwalk_free( walker ); + wxLogTrace( traceGit, "Failed to push/hide commits" ); return modified_set; } @@ -226,41 +257,48 @@ std::pair<std::set<wxString>,std::set<wxString>> KIGIT_COMMON::GetDifferentFiles while( git_revwalk_next( &oid, walker ) == GIT_OK ) { if( git_commit_lookup( &commit, m_repo, &oid ) != GIT_OK ) - continue; - - git_tree *tree, *parent_tree = nullptr; - if( git_commit_tree( &tree, commit ) != GIT_OK ) { - git_commit_free( commit ); + wxLogTrace( traceGit, "Failed to lookup commit" ); continue; } + KIGIT::GitCommitPtr commitPtr( commit ); + git_tree *tree, *parent_tree = nullptr; + + if( git_commit_tree( &tree, commit ) != GIT_OK ) + { + wxLogTrace( traceGit, "Failed to get commit tree" ); + continue; + } + + KIGIT::GitTreePtr treePtr( tree ); + // get parent commit tree to diff against if( !git_commit_parentcount( commit ) ) { - git_tree_free( tree ); - git_commit_free( commit ); + wxLogTrace( traceGit, "No parent commit" ); continue; } - git_commit* parent; + if( git_commit_parent( &parent, commit, 0 ) != GIT_OK ) { - git_tree_free( tree ); - git_commit_free( commit ); + wxLogTrace( traceGit, "Failed to get parent commit" ); continue; } + KIGIT::GitCommitPtr parentPtr( parent ); + if( git_commit_tree( &parent_tree, parent ) != GIT_OK ) { - git_tree_free( tree ); - git_commit_free( commit ); - git_commit_free( parent ); + wxLogTrace( traceGit, "Failed to get parent commit tree" ); continue; } + KIGIT::GitTreePtr parentTreePtr( parent_tree ); + git_diff* diff; git_diff_options diff_opts; @@ -278,15 +316,8 @@ std::pair<std::set<wxString>,std::set<wxString>> KIGIT_COMMON::GetDifferentFiles git_diff_free( diff ); } - - git_tree_free( parent_tree ); - git_commit_free( parent ); - git_tree_free( tree ); - git_commit_free( commit ); } - git_revwalk_free( walker ); - return modified_set; }; @@ -299,19 +330,22 @@ std::pair<std::set<wxString>,std::set<wxString>> KIGIT_COMMON::GetDifferentFiles git_reference* remote_head = nullptr; if( git_repository_head( &head, m_repo ) != GIT_OK ) - return modified_files; - - if( git_branch_upstream( &remote_head, head ) != GIT_OK ) { - git_reference_free( head ); + wxLogTrace( traceGit, "Failed to get modified HEAD" ); return modified_files; } - git_oid head_oid = *git_reference_target( head ); - git_oid remote_oid = *git_reference_target( remote_head ); + KIGIT::GitReferencePtr headPtr( head ); - git_reference_free( head ); - git_reference_free( remote_head ); + if( git_branch_upstream( &remote_head, head ) != GIT_OK ) + { + wxLogTrace( traceGit, "Failed to get modified remote HEAD" ); + return modified_files; + } + + KIGIT::GitReferencePtr remoteHeadPtr( remote_head ); + git_oid head_oid = *git_reference_target( head ); + git_oid remote_oid = *git_reference_target( remote_head ); modified_files.first = get_modified_files( &head_oid, &remote_oid ); modified_files.second = get_modified_files( &remote_oid, &head_oid ); @@ -329,29 +363,36 @@ bool KIGIT_COMMON::HasLocalCommits() const git_reference* remote_head = nullptr; if( git_repository_head( &head, m_repo ) != GIT_OK ) - return false; - - if( git_branch_upstream( &remote_head, head ) != GIT_OK ) { - git_reference_free( head ); + wxLogTrace( traceGit, "Failed to get HEAD" ); return false; } - git_oid head_oid = *git_reference_target( head ); - git_oid remote_oid = *git_reference_target( remote_head ); + KIGIT::GitReferencePtr headPtr( head ); - git_reference_free( head ); - git_reference_free( remote_head ); + if( git_branch_upstream( &remote_head, head ) != GIT_OK ) + { + wxLogTrace( traceGit, "Failed to get remote HEAD" ); + return false; + } - git_revwalk* walker = nullptr; + KIGIT::GitReferencePtr remoteHeadPtr( remote_head ); + git_oid head_oid = *git_reference_target( head ); + git_oid remote_oid = *git_reference_target( remote_head ); + git_revwalk* walker = nullptr; if( git_revwalk_new( &walker, m_repo ) != GIT_OK ) + { + wxLogTrace( traceGit, "Failed to create revwalker" ); return false; + } + + KIGIT::GitRevWalkPtr walkerPtr( walker ); if( ( git_revwalk_push( walker, &head_oid ) != GIT_OK ) || ( git_revwalk_hide( walker, &remote_oid ) != GIT_OK ) ) { - git_revwalk_free( walker ); + wxLogTrace( traceGit, "Failed to push/hide commits" ); return false; } @@ -360,11 +401,10 @@ bool KIGIT_COMMON::HasLocalCommits() const // If we can't walk to the next commit, then we are at or behind the remote if( git_revwalk_next( &oid, walker ) != GIT_OK ) { - git_revwalk_free( walker ); + wxLogTrace( traceGit, "Failed to walk to next commit" ); return false; } - git_revwalk_free( walker ); return true; } @@ -376,9 +416,12 @@ bool KIGIT_COMMON::HasPushAndPullRemote() const if( git_remote_lookup( &remote, m_repo, "origin" ) != GIT_OK ) { + wxLogTrace( traceGit, "Failed to get remote for haspushpull" ); return false; } + KIGIT::GitRemotePtr remotePtr( remote ); + // Get the URLs associated with the remote const char* fetch_url = git_remote_url( remote ); const char* push_url = git_remote_pushurl( remote ); @@ -386,13 +429,10 @@ bool KIGIT_COMMON::HasPushAndPullRemote() const // If no push URL is set, libgit2 defaults to using the fetch URL for pushing if( !push_url ) { + wxLogTrace( traceGit, "No push URL set, using fetch URL" ); push_url = fetch_url; } - // Clean up the remote object - git_remote_free( remote ); - - // Check if both URLs are valid (i.e., not NULL) return fetch_url && push_url; } @@ -406,7 +446,12 @@ wxString KIGIT_COMMON::GetRemotename() const git_reference* upstream = nullptr; if( git_repository_head( &head, m_repo ) != GIT_OK ) + { + wxLogTrace( traceGit, "Failed to get remote name" ); return retval; + } + + KIGIT::GitReferencePtr headPtr( head ); if( git_branch_upstream( &upstream, head ) == GIT_OK ) { @@ -421,8 +466,6 @@ wxString KIGIT_COMMON::GetRemotename() const git_reference_free( upstream ); } - git_reference_free( head ); - return retval; } @@ -625,7 +668,7 @@ extern "C" int fetchhead_foreach_cb( const char*, const char*, extern "C" void clone_progress_cb( const char* aStr, size_t aLen, size_t aTotal, void* data ) { - KIGIT_COMMON* parent = (KIGIT_COMMON*) data; + GIT_PROGRESS* parent = (GIT_PROGRESS*) data; wxString progressMessage( aStr ); parent->UpdateProgress( aLen, aTotal, progressMessage ); @@ -634,7 +677,7 @@ extern "C" void clone_progress_cb( const char* aStr, size_t aLen, size_t aTotal, extern "C" int progress_cb( const char* str, int len, void* data ) { - KIGIT_COMMON* parent = (KIGIT_COMMON*) data; + GIT_PROGRESS* parent = (GIT_PROGRESS*) data; wxString progressMessage( str, len ); parent->UpdateProgress( 0, 0, progressMessage ); @@ -645,7 +688,7 @@ extern "C" int progress_cb( const char* str, int len, void* data ) extern "C" int transfer_progress_cb( const git_transfer_progress* aStats, void* aPayload ) { - KIGIT_COMMON* parent = (KIGIT_COMMON*) aPayload; + GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; wxString progressMessage = wxString::Format( _( "Received %u of %u objects" ), aStats->received_objects, aStats->total_objects ); @@ -663,7 +706,7 @@ extern "C" int update_cb( const char* aRefname, const git_oid* aFirst, const git char a_str[cstring_len + 1]; char b_str[cstring_len + 1]; - KIGIT_COMMON* parent = (KIGIT_COMMON*) aPayload; + GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; wxString status; git_oid_tostr( b_str, cstring_len, aSecond ); @@ -692,7 +735,7 @@ extern "C" int push_transfer_progress_cb( unsigned int aCurrent, unsigned int aT void* aPayload ) { long long progress = 100; - KIGIT_COMMON* parent = (KIGIT_COMMON*) aPayload; + GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; if( aTotal != 0 ) { @@ -709,7 +752,7 @@ extern "C" int push_transfer_progress_cb( unsigned int aCurrent, unsigned int aT extern "C" int push_update_reference_cb( const char* aRefname, const char* aStatus, void* aPayload ) { - KIGIT_COMMON* parent = (KIGIT_COMMON*) aPayload; + GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; wxString status( aStatus ); if( !status.IsEmpty() ) @@ -735,34 +778,34 @@ extern "C" int credentials_cb( git_cred** aOut, const char* aUrl, const char* aU if( parent->GetConnType() == KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_LOCAL ) return GIT_PASSTHROUGH; - if( aAllowedTypes & GIT_CREDTYPE_USERNAME - && !( parent->TestedTypes() & GIT_CREDTYPE_USERNAME ) ) + if( aAllowedTypes & GIT_CREDENTIAL_USERNAME + && !( parent->TestedTypes() & GIT_CREDENTIAL_USERNAME ) ) { wxString username = parent->GetUsername().Trim().Trim( false ); - git_cred_username_new( aOut, username.ToStdString().c_str() ); - parent->TestedTypes() |= GIT_CREDTYPE_USERNAME; + git_credential_username_new( aOut, username.ToStdString().c_str() ); + parent->TestedTypes() |= GIT_CREDENTIAL_USERNAME; } else if( parent->GetConnType() == KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_HTTPS - && ( aAllowedTypes & GIT_CREDTYPE_USERPASS_PLAINTEXT ) - && !( parent->TestedTypes() & GIT_CREDTYPE_USERPASS_PLAINTEXT ) ) + && ( aAllowedTypes & GIT_CREDENTIAL_USERPASS_PLAINTEXT ) + && !( parent->TestedTypes() & GIT_CREDENTIAL_USERPASS_PLAINTEXT ) ) { wxString username = parent->GetUsername().Trim().Trim( false ); wxString password = parent->GetPassword().Trim().Trim( false ); - git_cred_userpass_plaintext_new( aOut, username.ToStdString().c_str(), + git_credential_userpass_plaintext_new( aOut, username.ToStdString().c_str(), password.ToStdString().c_str() ); - parent->TestedTypes() |= GIT_CREDTYPE_USERPASS_PLAINTEXT; + parent->TestedTypes() |= GIT_CREDENTIAL_USERPASS_PLAINTEXT; } else if( parent->GetConnType() == KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_SSH - && ( aAllowedTypes & GIT_CREDTYPE_SSH_KEY ) - && !( parent->TestedTypes() & GIT_CREDTYPE_SSH_KEY ) ) + && ( aAllowedTypes & GIT_CREDENTIAL_SSH_KEY ) + && !( parent->TestedTypes() & GIT_CREDENTIAL_SSH_KEY ) ) { // SSH key authentication wxString sshKey = parent->GetNextPublicKey(); if( sshKey.IsEmpty() ) { - parent->TestedTypes() |= GIT_CREDTYPE_SSH_KEY; + parent->TestedTypes() |= GIT_CREDENTIAL_SSH_KEY; return GIT_PASSTHROUGH; } @@ -770,7 +813,7 @@ extern "C" int credentials_cb( git_cred** aOut, const char* aUrl, const char* aU wxString username = parent->GetUsername().Trim().Trim( false ); wxString password = parent->GetPassword().Trim().Trim( false ); - git_cred_ssh_key_new( aOut, username.ToStdString().c_str(), + git_credential_ssh_key_new( aOut, username.ToStdString().c_str(), sshPubKey.ToStdString().c_str(), sshKey.ToStdString().c_str(), password.ToStdString().c_str() ); diff --git a/common/git/kicad_git_common.h b/common/git/kicad_git_common.h index a023931bb4..560aca0448 100644 --- a/common/git/kicad_git_common.h +++ b/common/git/kicad_git_common.h @@ -27,6 +27,7 @@ #include <git/kicad_git_errors.h> #include <git2.h> +#include <mutex> #include <set> #include <wx/string.h> @@ -36,6 +37,7 @@ class KIGIT_COMMON : public KIGIT_ERRORS public: KIGIT_COMMON( git_repository* aRepo ); + KIGIT_COMMON( const KIGIT_COMMON& aOther ); ~KIGIT_COMMON(); git_repository* GetRepo() const; @@ -49,8 +51,6 @@ public: std::vector<wxString> GetBranchNames() const; - virtual void UpdateProgress( int aCurrent, int aTotal, const wxString& aMessage ) {}; - /** * Return a vector of project files in the repository. Sorted by the depth of * the project file in the directory tree @@ -142,13 +142,18 @@ protected: unsigned m_testedTypes; + std::mutex m_gitActionMutex; + + // Make git handlers friends so they can access the mutex + friend class GIT_PUSH_HANDLER; + friend class GIT_PULL_HANDLER; + private: void updatePublicKeys(); void updateConnectionType(); std::vector<wxString> m_publicKeys; int m_nextPublicKey; - }; diff --git a/common/git/kicad_git_memory.h b/common/git/kicad_git_memory.h new file mode 100644 index 0000000000..8b13e33977 --- /dev/null +++ b/common/git/kicad_git_memory.h @@ -0,0 +1,255 @@ +// This program is free software; you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation; either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program; if not, see <http://www.gnu.org/licenses/>. + +#ifndef KICAD_GIT_MEMORY_H +#define KICAD_GIT_MEMORY_H + +#include <memory> +#include <git2.h> + +namespace KIGIT +{ +/** +* @brief A unique pointer for git_repository objects with automatic cleanup. +*/ +using GitRepositoryPtr = std::unique_ptr<git_repository, + decltype([](git_repository* aRepo) { + git_repository_free(aRepo); + })>; + +/** +* @brief A unique pointer for git_reference objects with automatic cleanup. +*/ +using GitReferencePtr = std::unique_ptr<git_reference, + decltype([](git_reference* aRef) { + git_reference_free(aRef); + })>; + +/** +* @brief A unique pointer for git_object objects with automatic cleanup. +*/ +using GitObjectPtr = std::unique_ptr<git_object, + decltype([](git_object* aObject) { + git_object_free(aObject); + })>; +/** +* @brief A unique pointer for git_commit objects with automatic cleanup. +*/ +using GitCommitPtr = std::unique_ptr<git_commit, + decltype([](git_commit* aCommit) { + git_commit_free(aCommit); + })>; + +/** +* @brief A unique pointer for git_tree objects with automatic cleanup. +*/ +using GitTreePtr = std::unique_ptr<git_tree, + decltype([](git_tree* aTree) { + git_tree_free(aTree); + })>; + +/** +* @brief A unique pointer for git_index objects with automatic cleanup. +*/ +using GitIndexPtr = std::unique_ptr<git_index, + decltype([](git_index* aIndex) { + git_index_free(aIndex); + })>; + +/** +* @brief A unique pointer for git_revwalk objects with automatic cleanup. +*/ +using GitRevWalkPtr = std::unique_ptr<git_revwalk, + decltype([](git_revwalk* aWalker) { + git_revwalk_free(aWalker); + })>; + +/** +* @brief A unique pointer for git_diff objects with automatic cleanup. +*/ +using GitDiffPtr = std::unique_ptr<git_diff, + decltype([](git_diff* aDiff) { + git_diff_free(aDiff); + })>; + +/** +* @brief A unique pointer for git_signature objects with automatic cleanup. +*/ +using GitSignaturePtr = std::unique_ptr<git_signature, + decltype([](git_signature* aSignature) { + git_signature_free(aSignature); + })>; + +/** +* @brief A unique pointer for git_config objects with automatic cleanup. +*/ +using GitConfigPtr = std::unique_ptr<git_config, + decltype([](git_config* aConfig) { + git_config_free(aConfig); + })>; + +/** +* @brief A unique pointer for git_remote objects with automatic cleanup. +*/ +using GitRemotePtr = std::unique_ptr<git_remote, + decltype([](git_remote* aRemote) { + git_remote_free(aRemote); + })>; + +/** +* @brief A unique pointer for git_annotated_commit objects with automatic cleanup. +*/ +using GitAnnotatedCommitPtr = std::unique_ptr<git_annotated_commit, + decltype([](git_annotated_commit* aCommit) { + git_annotated_commit_free(aCommit); + })>; + +/** +* @brief A unique pointer for git_oid objects with automatic cleanup. +*/ +using GitOidPtr = std::unique_ptr<git_oid, + decltype([](git_oid* aOid) { + delete aOid; + })>; + +/** +* @brief A unique pointer for git_buf objects with automatic cleanup. +*/ +using GitBufPtr = std::unique_ptr<git_buf, + decltype([](git_buf* aBuf) { + git_buf_free(aBuf); + })>; + +/** +* @brief A unique pointer for git_blame objects with automatic cleanup. +*/ +using GitBlamePtr = std::unique_ptr<git_blame, + decltype([](git_blame* aBlame) { + git_blame_free(aBlame); + })>; + +/** +* @brief A unique pointer for git_blob objects with automatic cleanup. +*/ +using GitBlobPtr = std::unique_ptr<git_blob, + decltype([](git_blob* aBlob) { + git_blob_free(aBlob); + })>; + +/** +* @brief A unique pointer for git_branch_iterator objects with automatic cleanup. +*/ +using GitBranchIteratorPtr = std::unique_ptr<git_branch_iterator, + decltype([](git_branch_iterator* aIter) { + git_branch_iterator_free(aIter); + })>; + +/** +* @brief A unique pointer for git_config_entry objects with automatic cleanup. +*/ +using GitConfigEntryPtr = std::unique_ptr<git_config_entry, + decltype([](git_config_entry* aEntry) { + git_config_entry_free(aEntry); + })>; + +/** +* @brief A unique pointer for git_config_iterator objects with automatic cleanup. +*/ +using GitConfigIteratorPtr = std::unique_ptr<git_config_iterator, + decltype([](git_config_iterator* aIter) { + git_config_iterator_free(aIter); + })>; + +/** +* @brief A unique pointer for git_credential objects with automatic cleanup. +*/ +using GitCredentialPtr = std::unique_ptr<git_credential, + decltype([](git_credential* aCred) { + git_credential_free(aCred); + })>; + +/** +* @brief A unique pointer for git_oidarray objects with automatic cleanup. +*/ +using GitOidArrayPtr = std::unique_ptr<git_oidarray, + decltype([](git_oidarray* aArray) { + git_oidarray_free(aArray); + })>; + +/** +* @brief A unique pointer for git_strarray objects with automatic cleanup. +*/ +using GitStrArrayPtr = std::unique_ptr<git_strarray, + decltype([](git_strarray* aArray) { + git_strarray_free(aArray); + })>; + +/** +* @brief A unique pointer for git_describe_result objects with automatic cleanup. +*/ +using GitDescribeResultPtr = std::unique_ptr<git_describe_result, + decltype([](git_describe_result* aResult) { + git_describe_result_free(aResult); + })>; + +/** +* @brief A unique pointer for git_diff_stats objects with automatic cleanup. +*/ +using GitDiffStatsPtr = std::unique_ptr<git_diff_stats, + decltype([](git_diff_stats* aStats) { + git_diff_stats_free(aStats); + })>; + +/** +* @brief A unique pointer for git_filter_list objects with automatic cleanup. +*/ +using GitFilterListPtr = std::unique_ptr<git_filter_list, + decltype([](git_filter_list* aFilters) { + git_filter_list_free(aFilters); + })>; + +/** +* @brief A unique pointer for git_indexer objects with automatic cleanup. +*/ +using GitIndexerPtr = std::unique_ptr<git_indexer, + decltype([](git_indexer* aIdx) { + git_indexer_free(aIdx); + })>; + +/** +* @brief A unique pointer for git_index_iterator objects with automatic cleanup. +*/ +using GitIndexIteratorPtr = std::unique_ptr<git_index_iterator, + decltype([](git_index_iterator* aIterator) { + git_index_iterator_free(aIterator); + })>; + +/** +* @brief A unique pointer for git_index_conflict_iterator objects with automatic cleanup. +*/ +using GitIndexConflictIteratorPtr = std::unique_ptr<git_index_conflict_iterator, + decltype([](git_index_conflict_iterator* aIterator) { + git_index_conflict_iterator_free(aIterator); + })>; + +/** +* @brief A unique pointer for git_status_list objects with automatic cleanup. +*/ +using GitStatusListPtr = std::unique_ptr<git_status_list, + decltype([](git_status_list* aList) { + git_status_list_free(aList); + })>; + +} // namespace KIGIT + +#endif // KICAD_GIT_MEMORY_H diff --git a/kicad/project_tree_pane.cpp b/kicad/project_tree_pane.cpp index e8041cbdd8..bf929af302 100644 --- a/kicad/project_tree_pane.cpp +++ b/kicad/project_tree_pane.cpp @@ -68,6 +68,7 @@ #include <git/git_sync_handler.h> #include <git/git_clone_handler.h> #include <git/kicad_git_compat.h> +#include <git/kicad_git_memory.h> #include <dialogs/git/dialog_git_repository.h> @@ -193,8 +194,9 @@ PROJECT_TREE_PANE::PROJECT_TREE_PANE( KICAD_MANAGER_FRAME* parent ) : m_selectedItem = nullptr; m_watcherNeedReset = false; m_gitLastError = GIT_ERROR_NONE; - m_watcher = nullptr; + m_gitIconsInitialized = false; + Bind( wxEVT_FSWATCHER, wxFileSystemWatcherEventHandler( PROJECT_TREE_PANE::onFileSystemEvent ), this ); @@ -388,23 +390,20 @@ static git_repository* get_git_repository_for_file( const char* filename ) git_buf repo_path = GIT_BUF_INIT; // Find the repository path for the given file - if( git_repository_discover( &repo_path, filename, 0, NULL ) ) + if( git_repository_discover( &repo_path, filename, 0, NULL ) != GIT_OK ) { -#if 0 - printf( "get_git_repository_for_file: %s\n", git_error_last()->message ); fflush( 0 ); -#endif + wxLogTrace( traceGit, "Can't repo discover %s: %s", filename, git_error_last()->message ); return nullptr; } - if( git_repository_open( &repo, repo_path.ptr ) ) + KIGIT::GitBufPtr repo_path_ptr( &repo_path ); + + if( git_repository_open( &repo, repo_path.ptr ) != GIT_OK ) { - git_buf_free( &repo_path ); + wxLogTrace( traceGit, "Can't open repo for %s: %s", repo_path.ptr, git_error_last()->message ); return nullptr; } - // Free the git_buf memory - git_buf_free( &repo_path ); - return repo; } @@ -643,6 +642,7 @@ void PROJECT_TREE_PANE::ReCreateTreePrj() { git_repository_free( m_TreeProject->GetGitRepo() ); m_TreeProject->SetGitRepo( nullptr ); + m_gitIconsInitialized = false; } wxFileName fn = pro_dir; @@ -733,9 +733,9 @@ void PROJECT_TREE_PANE::ReCreateTreePrj() CallAfter( [this] () { - updateTreeCache(); + wxLogTrace( traceGit, "PROJECT_TREE_PANE::ReCreateTreePrj: starting timers" ); m_gitSyncTimer.Start( 100, wxTIMER_ONE_SHOT ); - m_gitStatusTimer.Start( 150, wxTIMER_ONE_SHOT ); + m_gitStatusTimer.Start( 500, wxTIMER_ONE_SHOT ); } ); } @@ -755,14 +755,15 @@ bool PROJECT_TREE_PANE::hasChangedFiles() | GIT_STATUS_OPT_SORT_CASE_SENSITIVELY; git_status_list* status_list = nullptr; - int error = git_status_list_new( &status_list, repo, &opts ); - if( error != GIT_OK ) + if( git_status_list_new( &status_list, repo, &opts ) != GIT_OK ) + { + wxLogError( _( "Failed to get status list: %s" ), git_error_last()->message ); return false; + } - bool has_changed_files = git_status_list_entrycount( status_list ) > 0; - git_status_list_free( status_list ); - return has_changed_files; + KIGIT::GitStatusListPtr status_list_ptr( status_list ); + return ( git_status_list_entrycount( status_list ) > 0 ); } @@ -1315,6 +1316,16 @@ void PROJECT_TREE_PANE::onFileSystemEvent( wxFileSystemWatcherEvent& event ) if( !m_watcher ) return; + // Ignore events that are not file creation, deletion, renaming or modification because + // they are not relevant to the project tree. + if( !( event.GetChangeType() & ( wxFSW_EVENT_CREATE | + wxFSW_EVENT_DELETE | + wxFSW_EVENT_RENAME | + wxFSW_EVENT_MODIFY ) ) ) + { + return; + } + const wxFileName& pathModified = event.GetPath(); wxString subdir = pathModified.GetPath(); wxString fn = pathModified.GetFullPath(); @@ -1333,8 +1344,8 @@ void PROJECT_TREE_PANE::onFileSystemEvent( wxFileSystemWatcherEvent& event ) CallAfter( [this] () { - updateTreeCache(); - m_gitStatusTimer.Start( 150, wxTIMER_ONE_SHOT ); + wxLogTrace( traceGit, wxS( "File system event detected, updating tree cache" ) ); + m_gitStatusTimer.Start( 1500, wxTIMER_ONE_SHOT ); } ); wxTreeItemIdValue cookie; // dummy variable needed by GetFirstChild() @@ -1412,6 +1423,9 @@ void PROJECT_TREE_PANE::onFileSystemEvent( wxFileSystemWatcherEvent& event ) m_isRenaming = false; } break; + + default: + return; } // Sort filenames by alphabetic order @@ -1619,7 +1633,7 @@ void PROJECT_TREE_PANE::onGitInitializeProject( wxCommandEvent& aEvent ) // Check if the directory is already a git repository git_repository* repo = nullptr; - int error = git_repository_open(&repo, dir.mb_str()); + int error = git_repository_open( &repo, dir.mb_str() ); if( error == 0 ) { @@ -1632,6 +1646,7 @@ void PROJECT_TREE_PANE::onGitInitializeProject( wxCommandEvent& aEvent ) return; } + KIGIT::GitRepositoryPtr repoPtr( repo ); DIALOG_GIT_REPOSITORY dlg( wxGetTopLevelParent( this ), nullptr ); dlg.SetTitle( _( "Set default remote" ) ); @@ -1640,12 +1655,8 @@ void PROJECT_TREE_PANE::onGitInitializeProject( wxCommandEvent& aEvent ) return; // Directory is not a git repository - error = git_repository_init( &repo, dir.mb_str(), 0 ); - - if( error != 0 ) + if( git_repository_init( &repo, dir.mb_str(), 0 ) != GIT_OK ) { - git_repository_free( repo ); - if( m_gitLastError != git_error_last()->klass ) { m_gitLastError = git_error_last()->klass; @@ -1657,7 +1668,7 @@ void PROJECT_TREE_PANE::onGitInitializeProject( wxCommandEvent& aEvent ) } else { - m_TreeProject->SetGitRepo( repo ); + m_TreeProject->SetGitRepo( repoPtr.release() ); m_gitLastError = GIT_ERROR_NONE; } @@ -1793,30 +1804,30 @@ static int git_create_branch( git_repository* aRepo, wxString& aBranchName ) { git_oid head_oid; - if( int error = git_reference_name_to_id( &head_oid, aRepo, "HEAD" ) != 0 ) + if( int error = git_reference_name_to_id( &head_oid, aRepo, "HEAD" ) != GIT_OK ) { - wxLogError( "Failed to lookup HEAD reference" ); + wxLogTrace( traceGit, "Failed to lookup HEAD reference: %s", giterr_last()->message ); return error; } // Lookup the current commit object git_commit* commit = nullptr; + if( int error = git_commit_lookup( &commit, aRepo, &head_oid ) != GIT_OK ) { - wxLogError( "Failed to lookup commit" ); + wxLogTrace( traceGit, "Failed to lookup commit: %s", giterr_last()->message ); return error; } + KIGIT::GitCommitPtr commitPtr( commit ); git_reference* branchRef = nullptr; - if( git_branch_create( &branchRef, aRepo, aBranchName.mb_str(), commit, 0 ) != 0 ) + if( int error = git_branch_create( &branchRef, aRepo, aBranchName.mb_str(), commit, 0 ) != GIT_OK ) { - wxLogError( "Failed to create branch" ); - git_commit_free( commit ); - return -1; + wxLogTrace( traceGit, "Failed to create branch: %s", giterr_last()->message ); + return error; } - git_commit_free( commit ); git_reference_free( branchRef ); return 0; @@ -1867,19 +1878,19 @@ void PROJECT_TREE_PANE::onGitSwitchBranch( wxCommandEvent& aEvent ) return; } - const char* branchRefName = git_reference_name( branchRef ); - - git_object* branchObj = nullptr; + KIGIT::GitReferencePtr branchRefPtr( branchRef ); + const char* branchRefName = git_reference_name( branchRef ); + git_object* branchObj = nullptr; if( git_revparse_single( &branchObj, repo, branchName.mb_str() ) != 0 ) { wxString errorMessage = wxString::Format( _( "Failed to find branch head for '%s'" ), branchName ); DisplayError( m_parent, errorMessage ); - git_reference_free( branchRef ); return; } + KIGIT::GitObjectPtr branchObjPtr( branchObj ); // Switch to the branch if( git_checkout_tree( repo, branchObj, nullptr ) != 0 ) @@ -1887,8 +1898,6 @@ void PROJECT_TREE_PANE::onGitSwitchBranch( wxCommandEvent& aEvent ) wxString errorMessage = wxString::Format( _( "Failed to switch to branch '%s'" ), branchName ); DisplayError( m_parent, errorMessage ); - git_reference_free( branchRef ); - git_object_free( branchObj ); return; } @@ -1898,14 +1907,8 @@ void PROJECT_TREE_PANE::onGitSwitchBranch( wxCommandEvent& aEvent ) wxString errorMessage = wxString::Format( _( "Failed to update HEAD reference for branch '%s'" ), branchName ); DisplayError( m_parent, errorMessage ); - git_reference_free( branchRef ); - git_object_free( branchObj ); return; } - - // Free resources - git_reference_free( branchRef ); - git_object_free( branchObj ); } @@ -1964,13 +1967,18 @@ void PROJECT_TREE_PANE::updateGitStatusIcons() std::unique_lock<std::mutex> lock( m_gitStatusMutex, std::try_to_lock ); if( !lock.owns_lock() ) + { + wxLogTrace( traceGit, wxS( "updateGitStatusIcons: Failed to acquire lock for git status icon update" ) ); + m_gitStatusTimer.Start( 500, wxTIMER_ONE_SHOT ); return; + } if( ADVANCED_CFG::GetCfg().m_EnableGit == false || !m_TreeProject ) - return; + { + wxLogTrace( traceGit, wxS( "updateGitStatusIcons: Git is disabled or tree control is null" ) ); + return; + } - // Note that this function is called from the idle event, so we need to be careful about - // accessing the tree control from a different thread. for( auto&[ item, status ] : m_gitStatusIcons ) m_TreeProject->SetItemState( item, static_cast<int>( status ) ); @@ -1980,16 +1988,28 @@ void PROJECT_TREE_PANE::updateGitStatusIcons() PROJECT_TREE_ITEM* rootItem = GetItemIdData( kid ); wxString filename = wxFileNameFromPath( rootItem->GetFileName() ); m_TreeProject->SetItemText( kid, filename + " [" + m_gitCurrentBranchName + "]" ); + m_gitIconsInitialized = true; } } void PROJECT_TREE_PANE::updateTreeCache() { + wxLogTrace( traceGit, wxS( "updateTreeCache: Updating tree cache" ) ); + std::unique_lock<std::mutex> lock( m_gitTreeCacheMutex, std::try_to_lock ); - if( !lock.owns_lock() || !m_TreeProject ) + if( !lock.owns_lock() ) + { + wxLogTrace( traceGit, wxS( "updateTreeCache: Failed to acquire lock for tree cache update" ) ); return; + } + + if( !m_TreeProject ) + { + wxLogTrace( traceGit, wxS( "updateTreeCache: Tree control is null" ) ); + return; + } wxTreeItemId kid = m_TreeProject->GetRootItem(); @@ -1997,6 +2017,7 @@ void PROJECT_TREE_PANE::updateTreeCache() return; // Collect a map to easily set the state of each item + m_gitTreeCache.clear(); std::stack<wxTreeItemId> items; items.push( kid ); @@ -2027,6 +2048,7 @@ void PROJECT_TREE_PANE::updateTreeCache() void PROJECT_TREE_PANE::updateGitStatusIconMap() { + wxLogTrace( traceGit, wxS( "updateGitStatusIconMap: Updating git status icons" ) ); #if defined( _WIN32 ) int refresh = ADVANCED_CFG::GetCfg().m_GitIconRefreshInterval; @@ -2047,13 +2069,22 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() if( ADVANCED_CFG::GetCfg().m_EnableGit == false || !m_TreeProject ) return; - std::unique_lock<std::mutex> lock1( m_gitStatusMutex ); - std::unique_lock<std::mutex> lock2( m_gitTreeCacheMutex ); + std::unique_lock<std::mutex> lock1( m_gitStatusMutex, std::try_to_lock ); + std::unique_lock<std::mutex> lock2( m_gitTreeCacheMutex, std::try_to_lock ); + + if( !lock1.owns_lock() || !lock2.owns_lock() ) + { + wxLogTrace( traceGit, wxS( "updateGitStatusIconMap: Failed to acquire locks for git status icon update" ) ); + return; + } git_repository* repo = m_TreeProject->GetGitRepo(); if( !repo ) + { + wxLogTrace( traceGit, wxS( "updateGitStatusIconMap: No git repository found" ) ); return; + } // Get Current Branch PROJECT_TREE_ITEM* rootItem = GetItemIdData( m_TreeProject->GetRootItem() ); @@ -2085,16 +2116,17 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() return; } - git_status_list* status_list = nullptr; + KIGIT::GitIndexPtr indexPtr( index ); + git_status_list* status_list = nullptr; if( git_status_list_new( &status_list, repo, &status_options ) != GIT_OK ) { wxLogTrace( traceGit, wxS( "Failed to get git status list: %s" ), giterr_last()->message ); - git_index_free( index ); return; } - auto [ localChanges, remoteChanges ] = m_TreeProject->GitCommon()->GetDifferentFiles(); + KIGIT::GitStatusListPtr statusListPtr( status_list ); + auto [localChanges, remoteChanges] = m_TreeProject->GitCommon()->GetDifferentFiles(); size_t count = git_status_list_entrycount( status_list ); bool updated = false; @@ -2188,17 +2220,14 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } } - git_status_list_free( status_list ); - git_index_free( index ); - git_reference* currentBranchReference = nullptr; int rc = git_repository_head( ¤tBranchReference, repo ); + KIGIT::GitReferencePtr currentBranchReferencePtr( currentBranchReference ); // Get the current branch name if( currentBranchReference ) { m_gitCurrentBranchName = git_reference_shorthand( currentBranchReference ); - git_reference_free( currentBranchReference ); } else if( rc == GIT_EUNBORNBRANCH ) { @@ -2214,7 +2243,7 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } // If the icons are not changed, queue an event to update in the main thread - if( updated ) + if( updated || !m_gitIconsInitialized ) { CallAfter( [this]() @@ -2239,6 +2268,7 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) git_config* config = nullptr; git_repository_config( &config, repo ); + KIGIT::GitConfigPtr configPtr( config ); // Read relevant data from the git config wxString authorName; @@ -2248,6 +2278,7 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) git_config_entry* name_c = nullptr; git_config_entry* email_c = nullptr; int authorNameError = git_config_get_entry( &name_c, config, "user.name" ); + KIGIT::GitConfigEntryPtr namePtr( name_c ); if( authorNameError != 0 || name_c == nullptr ) { @@ -2256,7 +2287,6 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) else { authorName = name_c->value; - git_config_entry_free( name_c ); } // Read author email @@ -2269,12 +2299,8 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) else { authorEmail = email_c->value; - git_config_entry_free( email_c ); } - // Free the config object - git_config_free( config ); - // Collect modified files in the repository git_status_options status_options; git_status_init_options( &status_options, GIT_STATUS_OPTIONS_VERSION ); @@ -2283,6 +2309,7 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) git_status_list* status_list = nullptr; git_status_list_new( &status_list, repo, &status_options ); + KIGIT::GitStatusListPtr statusListPtr( status_list ); std::map<wxString, int> modifiedFiles; @@ -2363,8 +2390,6 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) } } - git_status_list_free( status_list ); - // Create a commit dialog DIALOG_GIT_COMMIT dlg( wxGetTopLevelParent( this ), repo, authorName, authorEmail, modifiedFiles ); @@ -2394,85 +2419,82 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) if( git_repository_index( &index, repo ) != 0 ) { - wxMessageBox( wxString::Format( _( "Failed to get repository index: %s" ), + wxLogTrace( traceGit, wxString::Format( _( "Failed to get repository index: %s" ), giterr_last()->message ) ); return; } - for( wxString& file :files ) + KIGIT::GitIndexPtr indexPtr( index ); + + for( wxString& file : files ) { if( git_index_add_bypath( index, file.mb_str() ) != 0 ) { wxMessageBox( wxString::Format( _( "Failed to add file to index: %s" ), giterr_last()->message ) ); - git_index_free( index ); return; } } if( git_index_write( index ) != 0 ) { - wxMessageBox( wxString::Format( _( "Failed to write index: %s" ), + wxLogTrace( traceGit, wxString::Format( _( "Failed to write index: %s" ), giterr_last()->message ) ); - git_index_free( index ); return; } if( git_index_write_tree( &tree_id, index ) != 0) { - wxMessageBox( wxString::Format( _( "Failed to write tree: %s" ), + wxLogTrace( traceGit, wxString::Format( _( "Failed to write tree: %s" ), giterr_last()->message ) ); - git_index_free( index ); return; } - git_index_free( index ); - if( git_tree_lookup( &tree, repo, &tree_id ) != 0 ) { - wxMessageBox( wxString::Format( _( "Failed to lookup tree: %s" ), + wxLogTrace( traceGit, wxString::Format( _( "Failed to lookup tree: %s" ), giterr_last()->message ) ); return; } + KIGIT::GitTreePtr treePtr( tree ); git_reference* headRef = nullptr; - if( 0 == git_repository_head_unborn( repo ) ) + if( git_repository_head_unborn( repo ) == 0 ) { if( git_repository_head( &headRef, repo ) != 0 ) { - wxMessageBox( wxString::Format( _( "Failed to get HEAD reference: %s" ), + wxLogTrace( traceGit, wxString::Format( _( "Failed to get HEAD reference: %s" ), giterr_last()->message ) ); - git_index_free( index ); return; } + KIGIT::GitReferencePtr headRefPtr( headRef ); + if( git_reference_peel( (git_object**) &parent, headRef, GIT_OBJECT_COMMIT ) != 0 ) { - wxMessageBox( wxString::Format( _( "Failed to get commit: %s" ), + wxLogTrace( traceGit, wxString::Format( _( "Failed to get commit: %s" ), giterr_last()->message ) ); - git_reference_free( headRef ); - git_index_free( index ); return; } - - git_reference_free( headRef ); } - const wxString& commit_msg = dlg.GetCommitMessage(); - const wxString& author_name = dlg.GetAuthorName(); - const wxString& author_email = dlg.GetAuthorEmail(); + KIGIT::GitCommitPtr parentPtr( parent ); + const wxString& commit_msg = dlg.GetCommitMessage(); + const wxString& author_name = dlg.GetAuthorName(); + const wxString& author_email = dlg.GetAuthorEmail(); git_signature* author = nullptr; if( git_signature_now( &author, author_name.mb_str(), author_email.mb_str() ) != 0 ) { - wxMessageBox( wxString::Format( _( "Failed to create author signature: %s" ), + wxLogTrace( traceGit, wxString::Format( _( "Failed to create author signature: %s" ), giterr_last()->message ) ); return; } - git_oid oid; + KIGIT::GitSignaturePtr authorPtr( author ); + git_oid oid; #if( LIBGIT2_VER_MAJOR == 1 && LIBGIT2_VER_MINOR == 8 \ && ( LIBGIT2_VER_REVISION < 2 || LIBGIT2_VER_REVISION == 3 ) ) @@ -2507,10 +2529,6 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) giterr_last()->message ) ); return; } - - git_signature_free( author ); - git_commit_free( parent ); - git_tree_free( tree ); } } @@ -2532,26 +2550,34 @@ bool PROJECT_TREE_PANE::canFileBeAddedToVCS( const wxString& aFile ) return false; if( git_repository_index( &index, repo ) != 0 ) + { + wxLogTrace( traceGit, "Failed to get git index: %s", giterr_last()->message ); return false; + } + + KIGIT::GitIndexPtr indexPtr( index ); // If we successfully find the file in the index, we may not add it to the VCS if( git_index_find( &entry_pos, index, aFile.mb_str() ) == 0 ) { - git_index_free( index ); + wxLogTrace( traceGit, "File already in index: %s", aFile ); return false; } - git_index_free( index ); return true; } void PROJECT_TREE_PANE::onGitSyncProject( wxCommandEvent& aEvent ) { + wxLogTrace( traceGit, "Syncing project" ); git_repository* repo = m_TreeProject->GetGitRepo(); if( !repo ) + { + wxLogTrace( traceGit, "sync: No git repository found" ); return; + } GIT_SYNC_HANDLER handler( repo ); handler.PerformSync(); @@ -2614,6 +2640,7 @@ void PROJECT_TREE_PANE::onRunSelectedJobsFile(wxCommandEvent& event) void PROJECT_TREE_PANE::onGitSyncTimer( wxTimerEvent& aEvent ) { + wxLogTrace( traceGit, "onGitSyncTimer" ); if( ADVANCED_CFG::GetCfg().m_EnableGit == false || !m_TreeProject ) return; @@ -2624,7 +2651,10 @@ void PROJECT_TREE_PANE::onGitSyncTimer( wxTimerEvent& aEvent ) KIGIT_COMMON* gitCommon = m_TreeProject->GitCommon(); if( !gitCommon ) + { + wxLogTrace( traceGit, "onGitSyncTimer: No git repository found" ); return; + } GIT_PULL_HANDLER handler( gitCommon ); handler.PerformFetch(); @@ -2632,6 +2662,7 @@ void PROJECT_TREE_PANE::onGitSyncTimer( wxTimerEvent& aEvent ) if( ADVANCED_CFG::GetCfg().m_GitProjectStatusRefreshInterval > 0 ) { + wxLogTrace( traceGit, "onGitSyncTimer: Starting git status timer" ); m_gitSyncTimer.Start( ADVANCED_CFG::GetCfg().m_GitProjectStatusRefreshInterval, wxTIMER_ONE_SHOT ); } @@ -2639,9 +2670,11 @@ void PROJECT_TREE_PANE::onGitSyncTimer( wxTimerEvent& aEvent ) void PROJECT_TREE_PANE::onGitStatusTimer( wxTimerEvent& aEvent ) { + wxLogTrace( traceGit, "onGitStatusTimer" ); if( ADVANCED_CFG::GetCfg().m_EnableGit == false || !m_TreeProject ) return; + updateTreeCache(); thread_pool& tp = GetKiCadThreadPool(); tp.push_task( [this]() diff --git a/kicad/project_tree_pane.h b/kicad/project_tree_pane.h index 84f0634f3c..33362e7402 100644 --- a/kicad/project_tree_pane.h +++ b/kicad/project_tree_pane.h @@ -325,6 +325,7 @@ private: std::unordered_map<wxString, wxTreeItemId> m_gitTreeCache; std::mutex m_gitStatusMutex; std::map<wxTreeItemId, KIGIT_COMMON::GIT_STATUS> m_gitStatusIcons; + bool m_gitIconsInitialized; DECLARE_EVENT_TABLE() }; diff --git a/pcbnew/drc/drc_test_provider_creepage.cpp b/pcbnew/drc/drc_test_provider_creepage.cpp index 5630eb948b..5ab8ab19f1 100644 --- a/pcbnew/drc/drc_test_provider_creepage.cpp +++ b/pcbnew/drc/drc_test_provider_creepage.cpp @@ -254,7 +254,7 @@ double DRC_TEST_PROVIDER_CREEPAGE::GetMaxConstraint( const std::vector<int>& aNe bci1.SetNetCode( aNet1 ); bci2.SetNetCode( aNet2 ); - for( PCB_LAYER_ID layer : LSET::AllCuMask().CuStack() ) + for( PCB_LAYER_ID layer : LSET::AllCuMask( m_board->GetCopperLayerCount() ) ) { bci1.SetLayer( layer ); bci2.SetLayer( layer ); diff --git a/pcbnew/git/kigit_pcb_merge.cpp b/pcbnew/git/kigit_pcb_merge.cpp index 71c7685ffa..766be3b6ec 100644 --- a/pcbnew/git/kigit_pcb_merge.cpp +++ b/pcbnew/git/kigit_pcb_merge.cpp @@ -22,14 +22,16 @@ */ #include "kigit_pcb_merge.h" +#include <git/kicad_git_blob_reader.h> +#include <git/kicad_git_memory.h> +#include <board.h> #include <pcb_io/kicad_sexpr/pcb_io_kicad_sexpr.h> #include <pcb_io/kicad_sexpr/pcb_io_kicad_sexpr_parser.h> #include <richio.h> +#include <trace_helpers.h> -#include <board.h> - -#include <git/kicad_git_blob_reader.h> +#include <wx/log.h> void KIGIT_PCB_MERGE::findSetDifferences( const BOARD_ITEM_SET& aAncestorSet, const BOARD_ITEM_SET& aOtherSet, std::vector<BOARD_ITEM*>& aAdded, std::vector<BOARD_ITEM*>& aRemoved, @@ -93,22 +95,28 @@ int KIGIT_PCB_MERGE::Merge() if( git_blob_lookup( &ancestor_blob, repo, &ancestor->id ) != 0 ) { + wxLogTrace( traceGit, "Could not find ancestor blob: %s", git_error_last()->message ); return GIT_ENOTFOUND; } + KIGIT::GitBlobPtr ancestor_blob_ptr( ancestor_blob ); + if( git_blob_lookup( &ours_blob, repo, &ours->id ) != 0 ) { - git_blob_free( ancestor_blob ); + wxLogTrace( traceGit, "Could not find ours blob: %s", git_error_last()->message ); return GIT_ENOTFOUND; } + KIGIT::GitBlobPtr ours_blob_ptr( ours_blob ); + if( git_blob_lookup( &theirs_blob, repo, &theirs->id ) != 0 ) { - git_blob_free( ancestor_blob ); - git_blob_free( ours_blob ); + wxLogTrace( traceGit, "Could not find theirs blob: %s", git_error_last()->message ); return GIT_ENOTFOUND; } + KIGIT::GitBlobPtr theirs_blob_ptr( theirs_blob ); + // Get the raw data from the blobs BLOB_READER ancestor_reader( ancestor_blob ); PCB_IO_KICAD_SEXPR_PARSER ancestor_parser( &ancestor_reader, nullptr, nullptr ); @@ -127,17 +135,16 @@ int KIGIT_PCB_MERGE::Merge() ours_board.reset( static_cast<BOARD*>( ours_parser.Parse() ) ); theirs_board.reset( static_cast<BOARD*>( theirs_parser.Parse() ) ); } - catch(const IO_ERROR&) + catch(const IO_ERROR& e) { - git_blob_free( ancestor_blob ); - git_blob_free( ours_blob ); - git_blob_free( theirs_blob ); + wxLogTrace( traceGit, "Could not parse board: %s", e.What() ); + return GIT_EUSER; + } + catch( ... ) + { + wxLogTrace( traceGit, "Could not parse board: unknown error" ); return GIT_EUSER; } - - git_blob_free( ancestor_blob ); - git_blob_free( ours_blob ); - git_blob_free( theirs_blob ); // Parse the differences between the ancestor and ours KIGIT_PCB_MERGE_DIFFERENCES ancestor_ours_differences = compareBoards( ancestor_board.get(), ours_board.get() );