From bf0bd05c7542dee677265f8a233f840bca27d515 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Tue, 18 Mar 2025 17:40:03 -0700 Subject: [PATCH] Fix some regressions in git handling Set default to rebase for pull Update error string on auth failure Improve trace logging (cherry picked from commit b8225ba2d62e01dd94837d6ddd9570486e7d93b5) --- common/dialogs/git/dialog_git_repository.cpp | 8 +- common/dialogs/git/dialog_git_repository.h | 3 + common/git/git_clone_handler.cpp | 5 +- common/git/git_clone_handler.h | 6 +- common/git/git_pull_handler.cpp | 235 +++++++++++++-- common/git/git_pull_handler.h | 1 + common/git/kicad_git_common.cpp | 79 +++-- common/git/kicad_git_common.h | 6 + common/git/kicad_git_memory.h | 8 + kicad/project_tree_pane.cpp | 299 ++++++++++--------- kicad/project_tree_pane.h | 2 + kicad/tools/kicad_manager_control.cpp | 2 +- 12 files changed, 461 insertions(+), 193 deletions(-) diff --git a/common/dialogs/git/dialog_git_repository.cpp b/common/dialogs/git/dialog_git_repository.cpp index b55ef2193c..b5bcae9439 100644 --- a/common/dialogs/git/dialog_git_repository.cpp +++ b/common/dialogs/git/dialog_git_repository.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -240,6 +241,7 @@ void DIALOG_GIT_REPOSITORY::updateURLData() if( valid ) { + m_fullURL = url; m_ConnType->SetSelection( static_cast( KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_HTTPS ) ); SetUsername( username ); SetPassword( password ); @@ -255,6 +257,7 @@ void DIALOG_GIT_REPOSITORY::updateURLData() if( valid ) { + m_fullURL = url; m_ConnType->SetSelection( static_cast( KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_SSH ) ); m_txtUsername->SetValue( username ); m_txtURL->SetValue( repoAddress ); @@ -284,12 +287,13 @@ void DIALOG_GIT_REPOSITORY::OnTestClick( wxCommandEvent& event ) // type, so we need to keep track of how many times we have tried. KIGIT_COMMON common( m_repository ); + common.SetRemote( m_txtURL->GetValue() ); callbacks.credentials = credentials_cb; - callbacks.payload = &common; common.SetPassword( m_txtPassword->GetValue() ); common.SetUsername( m_txtUsername->GetValue() ); common.SetSSHKey( m_fpSSHKey->GetFileName().GetFullPath() ); - + KIGIT_REPO_MIXIN repoMixin( &common ); + callbacks.payload = &repoMixin; wxString txtURL = m_txtURL->GetValue(); git_remote_create_with_fetchspec( &remote, m_repository, "origin", txtURL.mbc_str(), diff --git a/common/dialogs/git/dialog_git_repository.h b/common/dialogs/git/dialog_git_repository.h index 14a9281efe..d2aab6b7ce 100644 --- a/common/dialogs/git/dialog_git_repository.h +++ b/common/dialogs/git/dialog_git_repository.h @@ -72,6 +72,8 @@ public: return url; } + const wxString& GetFullURL() const { return m_fullURL; } + void SetUsername( const wxString& aUsername ) { m_txtUsername->SetValue( aUsername ); } wxString GetUsername() const { return m_txtUsername->GetValue(); } @@ -105,6 +107,7 @@ private: private: git_repository* m_repository; + wxString m_fullURL; wxString m_prevFile; diff --git a/common/git/git_clone_handler.cpp b/common/git/git_clone_handler.cpp index 32819e4456..a58c35bd0e 100644 --- a/common/git/git_clone_handler.cpp +++ b/common/git/git_clone_handler.cpp @@ -73,11 +73,12 @@ bool GIT_CLONE_HANDLER::PerformClone() TestedTypes() = 0; ResetNextKey(); git_repository* newRepo = nullptr; + wxString remote = GetCommon()->m_remote; - if( git_clone( &newRepo, m_URL.ToStdString().c_str(), m_clonePath.ToStdString().c_str(), + if( git_clone( &newRepo, remote.mbc_str(), m_clonePath.mbc_str(), &cloneOptions ) != 0 ) { - AddErrorString( wxString::Format( _( "Could not clone repository '%s'" ), m_URL ) ); + AddErrorString( wxString::Format( _( "Could not clone repository '%s'" ), remote ) ); return false; } diff --git a/common/git/git_clone_handler.h b/common/git/git_clone_handler.h index 9bf85fca3e..61bb18e232 100644 --- a/common/git/git_clone_handler.h +++ b/common/git/git_clone_handler.h @@ -36,19 +36,17 @@ public: bool PerformClone(); - void SetURL( const wxString& aURL ) { m_URL = aURL; } - wxString GetURL() const { return m_URL; } - void SetBranch( const wxString& aBranch ) { m_branch = aBranch; } wxString GetBranch() const { return m_branch; } void SetClonePath( const wxString& aPath ) { m_clonePath = aPath; } wxString GetClonePath() const { return m_clonePath; } + void SetRemote( const wxString& aRemote ) { GetCommon()->m_remote = aRemote; } + void UpdateProgress( int aCurrent, int aTotal, const wxString& aMessage ) override; private: - wxString m_URL; wxString m_branch; wxString m_clonePath; }; diff --git a/common/git/git_pull_handler.cpp b/common/git/git_pull_handler.cpp index adf95fc2a5..95507ec991 100644 --- a/common/git/git_pull_handler.cpp +++ b/common/git/git_pull_handler.cpp @@ -45,7 +45,10 @@ GIT_PULL_HANDLER::~GIT_PULL_HANDLER() bool GIT_PULL_HANDLER::PerformFetch( bool aSkipLock ) { if( !GetRepo() ) + { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformFetch() - No repository found" ); return false; + } std::unique_lock lock( GetCommon()->m_gitActionMutex, std::try_to_lock ); @@ -60,6 +63,7 @@ bool GIT_PULL_HANDLER::PerformFetch( bool aSkipLock ) if( git_remote_lookup( &remote, GetRepo(), "origin" ) != 0 ) { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformFetch() - Failed to lookup remote 'origin'" ); AddErrorString( wxString::Format( _( "Could not lookup remote '%s'" ), "origin" ) ); return false; } @@ -78,8 +82,9 @@ bool GIT_PULL_HANDLER::PerformFetch( bool aSkipLock ) if( git_remote_connect( remote, GIT_DIRECTION_FETCH, &remoteCallbacks, nullptr, nullptr ) ) { - AddErrorString( wxString::Format( _( "Could not connect to remote '%s': %s" ), "origin", - KIGIT_COMMON::GetLastGitError() ) ); + wxString errorMsg = KIGIT_COMMON::GetLastGitError(); + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformFetch() - Failed to connect to remote: %s", errorMsg ); + AddErrorString( wxString::Format( _( "Could not connect to remote '%s': %s" ), "origin", errorMsg ) ); return false; } @@ -89,11 +94,13 @@ bool GIT_PULL_HANDLER::PerformFetch( bool aSkipLock ) if( git_remote_fetch( remote, nullptr, &fetchOptions, nullptr ) ) { - AddErrorString( wxString::Format( _( "Could not fetch data from remote '%s': %s" ), - "origin", KIGIT_COMMON::GetLastGitError() ) ); + wxString errorMsg = KIGIT_COMMON::GetLastGitError(); + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformFetch() - Failed to fetch from remote: %s", errorMsg ); + AddErrorString( wxString::Format( _( "Could not fetch data from remote '%s': %s" ), "origin", errorMsg ) ); return false; } + wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformFetch() - Fetch completed successfully" ); return true; } @@ -130,12 +137,11 @@ PullResult GIT_PULL_HANDLER::PerformPull() } 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; + const git_annotated_commit* merge_commits[] = { fetchhead_commit }; + git_merge_analysis_t merge_analysis; + git_merge_preference_t merge_preference = GIT_MERGE_PREFERENCE_NONE; - if( git_merge_analysis( &merge_analysis, &merge_preference, GetRepo(), merge_commits, - 1 ) ) + if( git_merge_analysis( &merge_analysis, &merge_preference, GetRepo(), merge_commits, 1 ) ) { AddErrorString( _( "Could not analyze merge" ) ); return PullResult::Error; @@ -165,7 +171,8 @@ PullResult GIT_PULL_HANDLER::PerformPull() if( merge_analysis & GIT_MERGE_ANALYSIS_NORMAL ) { wxLogTrace( traceGit, "GIT_PULL_HANDLER::PerformPull() - Normal merge" ); - PullResult ret = handleMerge( merge_commits, 1 ); + PullResult ret = handleRebase( merge_commits, 1 ); + // PullResult ret = handleMerge( merge_commits, 1 ); return ret; } @@ -184,6 +191,9 @@ GIT_PULL_HANDLER::GetFetchResults() const std::string GIT_PULL_HANDLER::getFirstLineFromCommitMessage( const std::string& aMessage ) { + if( aMessage.empty() ) + return aMessage; + size_t firstLineEnd = aMessage.find_first_of( '\n' ); if( firstLineEnd != std::string::npos ) @@ -197,7 +207,9 @@ std::string GIT_PULL_HANDLER::getFormattedCommitDate( const git_time& aTime ) { char dateBuffer[64]; time_t time = static_cast( aTime.time ); - strftime( dateBuffer, sizeof( dateBuffer ), "%Y-%b-%d %H:%M:%S", gmtime( &time ) ); + struct tm timeInfo; + gmtime_r( &time, &timeInfo ); + strftime( dateBuffer, sizeof( dateBuffer ), "%Y-%b-%d %H:%M:%S", &timeInfo ); return dateBuffer; } @@ -206,6 +218,7 @@ PullResult GIT_PULL_HANDLER::handleFastForward() { git_reference* rawRef = nullptr; + // Get the current HEAD reference if( git_repository_head( &rawRef, GetRepo() ) ) { AddErrorString( _( "Could not get repository head" ) ); @@ -214,43 +227,131 @@ PullResult GIT_PULL_HANDLER::handleFastForward() KIGIT::GitReferencePtr headRef( rawRef ); - const char* updatedRefName = git_reference_name( rawRef ); + git_oid updatedRefOid; + const char* currentBranchName = git_reference_name( rawRef ); + wxString remoteBranchName = wxString::Format( "refs/remotes/origin/%s", + currentBranchName + strlen( "refs/heads/" ) ); - git_oid updatedRefOid; - - if( git_reference_name_to_id( &updatedRefOid, GetRepo(), updatedRefName ) ) + // Get the OID of the updated reference (remote-tracking branch) + if( git_reference_name_to_id( &updatedRefOid, GetRepo(), remoteBranchName.c_str() ) != GIT_OK ) { AddErrorString( wxString::Format( _( "Could not get reference OID for reference '%s'" ), - updatedRefName ) ); + remoteBranchName ) ); return PullResult::Error; } + // Get the target commit object + git_commit* targetCommit = nullptr; + + if( git_commit_lookup( &targetCommit, GetRepo(), &updatedRefOid ) != GIT_OK ) + { + AddErrorString( _( "Could not look up target commit" ) ); + return PullResult::Error; + } + + KIGIT::GitCommitPtr targetCommitPtr( targetCommit ); + + // Get the tree from the target commit + git_tree* targetTree = nullptr; + + if( git_commit_tree( &targetTree, targetCommit ) != GIT_OK ) + { + git_commit_free( targetCommit ); + AddErrorString( _( "Could not get tree from target commit" ) ); + return PullResult::Error; + } + + KIGIT::GitTreePtr targetTreePtr( targetTree ); + + // Perform a checkout to update the working directory git_checkout_options checkoutOptions; git_checkout_init_options( &checkoutOptions, GIT_CHECKOUT_OPTIONS_VERSION ); - checkoutOptions.checkout_strategy = GIT_CHECKOUT_SAFE; + auto notify_cb = []( git_checkout_notify_t why, const char* path, const git_diff_file* baseline, + const git_diff_file* target, const git_diff_file* workdir, void* payload ) -> int + { + switch( why ) + { + case GIT_CHECKOUT_NOTIFY_CONFLICT: + wxLogTrace( traceGit, "Checkout conflict: %s", path ? path : "unknown" ); + break; + case GIT_CHECKOUT_NOTIFY_DIRTY: + wxLogTrace( traceGit, "Checkout dirty: %s", path ? path : "unknown" ); + break; + case GIT_CHECKOUT_NOTIFY_UPDATED: + wxLogTrace( traceGit, "Checkout updated: %s", path ? path : "unknown" ); + break; + case GIT_CHECKOUT_NOTIFY_UNTRACKED: + wxLogTrace( traceGit, "Checkout untracked: %s", path ? path : "unknown" ); + break; + case GIT_CHECKOUT_NOTIFY_IGNORED: + wxLogTrace( traceGit, "Checkout ignored: %s", path ? path : "unknown" ); + break; + default: + break; + } - if( git_checkout_head( GetRepo(), &checkoutOptions ) ) + return 0; + }; + + checkoutOptions.checkout_strategy = GIT_CHECKOUT_SAFE | GIT_CHECKOUT_ALLOW_CONFLICTS; + checkoutOptions.notify_flags = GIT_CHECKOUT_NOTIFY_ALL; + checkoutOptions.notify_cb = notify_cb; + + if( git_checkout_tree( GetRepo(), reinterpret_cast( targetTree ), &checkoutOptions ) != GIT_OK ) { AddErrorString( _( "Failed to perform checkout operation." ) ); return PullResult::Error; } - // Collect commit details for updated references + git_reference* updatedRef = nullptr; + + // Update the current branch to point to the new commit + if (git_reference_set_target(&updatedRef, rawRef, &updatedRefOid, nullptr) != GIT_OK) + { + AddErrorString( wxString::Format( _( "Failed to update reference '%s' to point to '%s'" ), currentBranchName, + git_oid_tostr_s( &updatedRefOid ) ) ); + return PullResult::Error; + } + + KIGIT::GitReferencePtr updatedRefPtr( updatedRef ); + + // Clean up the repository state + if( git_repository_state_cleanup( GetRepo() ) != GIT_OK ) + { + AddErrorString( _( "Failed to clean up repository state after fast-forward." ) ); + return PullResult::Error; + } + git_revwalk* revWalker = nullptr; - git_revwalk_new( &revWalker, GetRepo() ); + + // Collect commit details for updated references + if( git_revwalk_new( &revWalker, GetRepo() ) != GIT_OK ) + { + AddErrorString( _( "Failed to initialize revision walker." ) ); + return PullResult::Error; + } + KIGIT::GitRevWalkPtr revWalkerPtr( revWalker ); git_revwalk_sorting( revWalker, GIT_SORT_TIME ); - git_revwalk_push_glob( revWalker, updatedRefName ); + + if( git_revwalk_push_glob( revWalker, currentBranchName ) != GIT_OK ) + { + AddErrorString( _( "Failed to push reference to revision walker." ) ); + return PullResult::Error; + } + + std::pair>& branchCommits = m_fetchResults.emplace_back(); + branchCommits.first = currentBranchName; git_oid commitOid; - while( git_revwalk_next( &commitOid, revWalker ) == 0 ) + while( git_revwalk_next( &commitOid, revWalker ) == GIT_OK ) { git_commit* commit = nullptr; if( git_commit_lookup( &commit, GetRepo(), &commitOid ) ) { - AddErrorString( wxString::Format( _( "Could not lookup commit '{}'" ), + AddErrorString( wxString::Format( _( "Could not lookup commit '%s'" ), git_oid_tostr_s( &commitOid ) ) ); return PullResult::Error; } @@ -263,13 +364,9 @@ PullResult GIT_PULL_HANDLER::handleFastForward() details.m_author = git_commit_author( commit )->name; details.m_date = getFormattedCommitDate( git_commit_author( commit )->when ); - std::pair>& branchCommits = - m_fetchResults.emplace_back(); - branchCommits.first = updatedRefName; branchCommits.second.push_back( details ); } - git_repository_state_cleanup( GetRepo() ); return PullResult::FastForward; } @@ -378,6 +475,90 @@ PullResult GIT_PULL_HANDLER::handleMerge( const git_annotated_commit** aMergeHea } +PullResult GIT_PULL_HANDLER::handleRebase( const git_annotated_commit** aMergeHeads, size_t aMergeHeadsCount ) +{ + // Get the current branch reference + git_reference* head_ref = nullptr; + + if( git_repository_head( &head_ref, GetRepo() ) ) + { + wxString errorMsg = KIGIT_COMMON::GetLastGitError(); + wxLogTrace( traceGit, "GIT_PULL_HANDLER::handleRebase() - Failed to get HEAD: %s", errorMsg ); + return PullResult::Error; + } + + KIGIT::GitReferencePtr headRefPtr(head_ref); + + // Initialize rebase operation + git_rebase* rebase = nullptr; + git_rebase_options rebase_opts = GIT_REBASE_OPTIONS_INIT; + rebase_opts.checkout_options.checkout_strategy = GIT_CHECKOUT_SAFE; + + if( git_rebase_init( &rebase, GetRepo(), nullptr, nullptr, aMergeHeads[0], &rebase_opts ) ) + { + wxString errorMsg = KIGIT_COMMON::GetLastGitError(); + wxLogTrace( traceGit, "GIT_PULL_HANDLER::handleRebase() - Failed to initialize rebase: %s", errorMsg ); + return PullResult::Error; + } + + KIGIT::GitRebasePtr rebasePtr( rebase ); + git_rebase_operation* operation = nullptr; + + while( git_rebase_next( &operation, rebase ) != GIT_ITEROVER ) + { + // Check for conflicts + git_index* index = nullptr; + if( git_repository_index( &index, GetRepo() ) ) + { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::handleRebase() - Failed to get index: %s", + KIGIT_COMMON::GetLastGitError() ); + return PullResult::Error; + } + KIGIT::GitIndexPtr indexPtr( index ); + + if( git_index_has_conflicts( index ) ) + { + // Abort the rebase if there are conflicts because we need to merge manually + git_rebase_abort( rebase ); + AddErrorString( _( "Conflicts detected during rebase" ) ); + return PullResult::MergeFailed; + } + + git_oid commit_id; + git_signature* committer = nullptr; + + if( git_signature_default( &committer, GetRepo() ) ) + { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::handleRebase() - Failed to create signature: %s", + KIGIT_COMMON::GetLastGitError() ); + return PullResult::Error; + } + + KIGIT::GitSignaturePtr committerPtr( committer ); + + if( git_rebase_commit( &commit_id, rebase, nullptr, committer, nullptr, nullptr ) != GIT_OK ) + { + wxString errorMsg = KIGIT_COMMON::GetLastGitError(); + wxLogTrace( traceGit, "GIT_PULL_HANDLER::handleRebase() - Failed to commit operation: %s", errorMsg ); + git_rebase_abort( rebase ); + return PullResult::Error; + } + } + + // Finish the rebase + if( git_rebase_finish( rebase, nullptr ) ) + { + wxLogTrace( traceGit, "GIT_PULL_HANDLER::handleRebase() - Failed to finish rebase: %s", + KIGIT_COMMON::GetLastGitError() ); + return PullResult::Error; + } + + wxLogTrace( traceGit, "GIT_PULL_HANDLER::handleRebase() - Rebase completed successfully" ); + git_repository_state_cleanup( GetRepo() ); + return PullResult::Success; +} + + void GIT_PULL_HANDLER::UpdateProgress( int aCurrent, int aTotal, const wxString& aMessage ) { diff --git a/common/git/git_pull_handler.h b/common/git/git_pull_handler.h index 91861f78aa..c553567c75 100644 --- a/common/git/git_pull_handler.h +++ b/common/git/git_pull_handler.h @@ -85,6 +85,7 @@ private: std::string getFormattedCommitDate( const git_time& aTime ); PullResult handleFastForward(); PullResult handleMerge( const git_annotated_commit** aMergeHeads, size_t aMergeHeadsCount ); + PullResult handleRebase( const git_annotated_commit** aMergeHeads, size_t aMergeHeadsCount ); }; #endif // _GIT_PULL_HANDLER_H_ diff --git a/common/git/kicad_git_common.cpp b/common/git/kicad_git_common.cpp index 4fb4138997..2e6c0adbfb 100644 --- a/common/git/kicad_git_common.cpp +++ b/common/git/kicad_git_common.cpp @@ -321,7 +321,7 @@ std::pair,std::set> KIGIT_COMMON::GetDifferentFiles git_diff_options diff_opts; git_diff_init_options( &diff_opts, GIT_DIFF_OPTIONS_VERSION ); - if( !diff_opts.flags || !m_repo || !parent_tree || !tree ) + if( !m_repo || !parent_tree || !tree ) continue; if( git_diff_tree_to_tree( &diff, m_repo, parent_tree, tree, &diff_opts ) == GIT_OK ) @@ -664,9 +664,14 @@ KIGIT_COMMON::GIT_CONN_TYPE KIGIT_COMMON::GetConnType() const remote = GetRemotename(); if( remote.StartsWith( "https://" ) || remote.StartsWith( "http://" ) ) + { return GIT_CONN_TYPE::GIT_CONN_HTTPS; - else if( remote.StartsWith( "ssh://" ) || remote.StartsWith( "git@" ) || remote.StartsWith( "git+ssh://" ) ) + } + else if( remote.StartsWith( "ssh://" ) || remote.StartsWith( "git@" ) || remote.StartsWith( "git+ssh://" ) + || remote.EndsWith( ".git" ) ) + { return GIT_CONN_TYPE::GIT_CONN_SSH; + } return GIT_CONN_TYPE::GIT_CONN_LOCAL; } @@ -750,6 +755,7 @@ int KIGIT_COMMON::HandleSSHKeyAuthentication( git_cred** aOut, const wxString& a if( sshKey.IsEmpty() ) { + wxLogTrace( traceGit, "Finished testing all possible ssh keys" ); m_testedTypes |= GIT_CREDENTIAL_SSH_KEY; return GIT_PASSTHROUGH; } @@ -757,12 +763,14 @@ int KIGIT_COMMON::HandleSSHKeyAuthentication( git_cred** aOut, const wxString& a wxString sshPubKey = sshKey + ".pub"; wxString password = GetPassword(); + wxLogTrace( traceGit, "Testing %s\n", sshKey ); + if( git_credential_ssh_key_new( aOut, aUsername.mbc_str(), sshPubKey.mbc_str(), sshKey.mbc_str(), password.mbc_str() ) != GIT_OK ) { wxLogTrace( traceGit, "Failed to create SSH key credential for %s: %s", aUsername, KIGIT_COMMON::GetLastGitError() ); - return GIT_ERROR; + return GIT_PASSTHROUGH; } return GIT_OK; @@ -785,7 +793,7 @@ int KIGIT_COMMON::HandleSSHAgentAuthentication( git_cred** aOut, const wxString& { wxLogTrace( traceGit, "Failed to create SSH agent credential for %s: %s", aUsername, KIGIT_COMMON::GetLastGitError() ); - return GIT_ERROR; + return GIT_PASSTHROUGH; } m_testedTypes |= KIGIT_CREDENTIAL_SSH_AGENT; @@ -803,18 +811,18 @@ 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 ) +extern "C" void clone_progress_cb( const char* aStr, size_t aLen, size_t aTotal, void* aPayload ) { - GIT_PROGRESS* parent = (GIT_PROGRESS*) data; + KIGIT_REPO_MIXIN* parent = reinterpret_cast( aPayload ); wxString progressMessage( aStr ); parent->UpdateProgress( aLen, aTotal, progressMessage ); } -extern "C" int progress_cb( const char* str, int len, void* data ) +extern "C" int progress_cb( const char* str, int len, void* aPayload ) { - GIT_PROGRESS* parent = (GIT_PROGRESS*) data; + KIGIT_REPO_MIXIN* parent = reinterpret_cast( aPayload ); wxString progressMessage( str, len ); parent->UpdateProgress( 0, 0, progressMessage ); @@ -825,10 +833,11 @@ 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 ) { - GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; - wxString progressMessage = wxString::Format( _( "Received %u of %u objects" ), - aStats->received_objects, - aStats->total_objects ); + KIGIT_REPO_MIXIN* parent = reinterpret_cast( aPayload ); + + wxString progressMessage = wxString::Format( _( "Received %u of %u objects" ), + aStats->received_objects, + aStats->total_objects ); parent->UpdateProgress( aStats->received_objects, aStats->total_objects, progressMessage ); @@ -843,8 +852,8 @@ 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]; - GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; - wxString status; + KIGIT_REPO_MIXIN* parent = reinterpret_cast( aPayload ); + wxString status; git_oid_tostr( b_str, cstring_len, aSecond ); @@ -871,8 +880,8 @@ extern "C" int update_cb( const char* aRefname, const git_oid* aFirst, const git extern "C" int push_transfer_progress_cb( unsigned int aCurrent, unsigned int aTotal, size_t aBytes, void* aPayload ) { - long long progress = 100; - GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; + long long progress = 100; + KIGIT_REPO_MIXIN* parent = reinterpret_cast( aPayload ); if( aTotal != 0 ) { @@ -889,8 +898,8 @@ 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 ) { - GIT_PROGRESS* parent = (GIT_PROGRESS*) aPayload; - wxString status( aStatus ); + KIGIT_REPO_MIXIN* parent = reinterpret_cast( aPayload ); + wxString status( aStatus ); if( !status.IsEmpty() ) { @@ -913,14 +922,30 @@ extern "C" int credentials_cb( git_cred** aOut, const char* aUrl, const char* aU KIGIT_REPO_MIXIN* parent = reinterpret_cast( aPayload ); KIGIT_COMMON* common = parent->GetCommon(); + wxLogTrace( traceGit, "Credentials callback for %s, testing %d", aUrl, aAllowedTypes ); + if( parent->GetConnType() == KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_LOCAL ) + { + wxLogTrace( traceGit, "Local repository, no credentials needed" ); return GIT_PASSTHROUGH; + } if( aAllowedTypes & GIT_CREDENTIAL_USERNAME && !( parent->TestedTypes() & GIT_CREDENTIAL_USERNAME ) ) { wxString username = parent->GetUsername().Trim().Trim( false ); - git_credential_username_new( aOut, username.ToStdString().c_str() ); + wxLogTrace( traceGit, "Username credential for %s at %s with allowed type %d", + username, aUrl, aAllowedTypes ); + if( git_credential_username_new( aOut, username.ToStdString().c_str() ) != GIT_OK ) + { + wxLogTrace( traceGit, "Failed to create username credential for %s: %s", + username, KIGIT_COMMON::GetLastGitError() ); + } + else + { + wxLogTrace( traceGit, "Created username credential for %s", username ); + } + parent->TestedTypes() |= GIT_CREDENTIAL_USERNAME; } else if( parent->GetConnType() == KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_HTTPS @@ -928,18 +953,28 @@ extern "C" int credentials_cb( git_cred** aOut, const char* aUrl, const char* aU && !( parent->TestedTypes() & GIT_CREDENTIAL_USERPASS_PLAINTEXT ) ) { // Plaintext authentication - return common->HandlePlaintextAuthentication( aOut, aUsername ); + wxLogTrace( traceGit, "Plaintext authentication for %s at %s with allowed type %d", + parent->GetUsername(), aUrl, aAllowedTypes ); + return common->HandlePlaintextAuthentication( aOut, parent->GetUsername() ); } else if( parent->GetConnType() == KIGIT_COMMON::GIT_CONN_TYPE::GIT_CONN_SSH && ( aAllowedTypes & GIT_CREDENTIAL_SSH_KEY ) && !( parent->TestedTypes() & GIT_CREDENTIAL_SSH_KEY ) ) { // SSH key authentication - return common->HandleSSHKeyAuthentication( aOut, aUsername ); + return common->HandleSSHKeyAuthentication( aOut, parent->GetUsername() ); } else { - return GIT_PASSTHROUGH; + // If we didn't find anything to try, then we don't have a callback set that the + // server likes + if( !parent->TestedTypes() ) + return GIT_PASSTHROUGH; + + git_error_clear(); + git_error_set_str( GIT_ERROR_NET, _( "Unable to authenticate" ).mbc_str() ); + // Otherwise, we did try something but we failed, so return an authentication error + return GIT_EAUTH; } return GIT_OK; diff --git a/common/git/kicad_git_common.h b/common/git/kicad_git_common.h index 3dc64b6793..62d49c025f 100644 --- a/common/git/kicad_git_common.h +++ b/common/git/kicad_git_common.h @@ -124,6 +124,12 @@ public: return m_publicKeys[m_nextPublicKey++]; } + void SetRemote( const wxString& aRemote ) + { + m_remote = aRemote; + updateConnectionType(); + } + int HandleSSHKeyAuthentication( git_cred** aOut, const wxString& aUsername ); int HandlePlaintextAuthentication( git_cred** aOut, const wxString& aUsername ); diff --git a/common/git/kicad_git_memory.h b/common/git/kicad_git_memory.h index 8b13e33977..c6529efcb0 100644 --- a/common/git/kicad_git_memory.h +++ b/common/git/kicad_git_memory.h @@ -66,6 +66,14 @@ using GitIndexPtr = std::unique_ptr; +/** + * @brief A unique pointer for git_rebase objects with automatic cleanup. + */ +using GitRebasePtr = std::unique_ptr; + /** * @brief A unique pointer for git_revwalk objects with automatic cleanup. */ diff --git a/kicad/project_tree_pane.cpp b/kicad/project_tree_pane.cpp index 41b286f4fd..b73af2fa16 100644 --- a/kicad/project_tree_pane.cpp +++ b/kicad/project_tree_pane.cpp @@ -2152,6 +2152,7 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() // that is the main status we want to show. if( entry->status & GIT_STATUS_IGNORED ) { + wxLogTrace( traceGit, wxS( "File '%s' is ignored" ), absPath ); auto [it, inserted] = m_gitStatusIcons.try_emplace( iter->second, KIGIT_COMMON::GIT_STATUS::GIT_STATUS_IGNORED ); @@ -2162,6 +2163,8 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } else if( entry->status & ( GIT_STATUS_INDEX_MODIFIED | GIT_STATUS_WT_MODIFIED ) ) { + wxLogTrace( traceGit, wxS( "File '%s' is modified in %s" ), + absPath, ( entry->status & GIT_STATUS_INDEX_MODIFIED )? "index" : "working tree" ); auto [it, inserted] = m_gitStatusIcons.try_emplace( iter->second, KIGIT_COMMON::GIT_STATUS::GIT_STATUS_MODIFIED ); @@ -2172,6 +2175,8 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } else if( entry->status & ( GIT_STATUS_INDEX_NEW | GIT_STATUS_WT_NEW ) ) { + wxLogTrace( traceGit, wxS( "File '%s' is new in %s" ), + absPath, ( entry->status & GIT_STATUS_INDEX_NEW )? "index" : "working tree" ); auto [it, inserted] = m_gitStatusIcons.try_emplace( iter->second, KIGIT_COMMON::GIT_STATUS::GIT_STATUS_ADDED ); @@ -2182,6 +2187,8 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } else if( entry->status & ( GIT_STATUS_INDEX_DELETED | GIT_STATUS_WT_DELETED ) ) { + wxLogTrace( traceGit, wxS( "File '%s' is deleted in %s" ), + absPath, ( entry->status & GIT_STATUS_INDEX_DELETED )? "index" : "working tree" ); auto [it, inserted] = m_gitStatusIcons.try_emplace( iter->second, KIGIT_COMMON::GIT_STATUS::GIT_STATUS_DELETED ); @@ -2192,6 +2199,7 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } else if( localChanges.count( path ) ) { + wxLogTrace( traceGit, wxS( "File '%s' is ahead of remote" ), absPath ); auto [it, inserted] = m_gitStatusIcons.try_emplace( iter->second, KIGIT_COMMON::GIT_STATUS::GIT_STATUS_AHEAD ); @@ -2202,6 +2210,7 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } else if( remoteChanges.count( path ) ) { + wxLogTrace( traceGit, wxS( "File '%s' is behind remote" ), absPath ); auto [it, inserted] = m_gitStatusIcons.try_emplace( iter->second, KIGIT_COMMON::GIT_STATUS::GIT_STATUS_BEHIND ); @@ -2212,6 +2221,9 @@ void PROJECT_TREE_PANE::updateGitStatusIconMap() } else { + wxLogTrace( traceGit, wxS( "File '%s' is status %d" ), absPath, entry->status ); + + // If we are here, the file is unmodified and not ignored auto [it, inserted] = m_gitStatusIcons.try_emplace( iter->second, KIGIT_COMMON::GIT_STATUS::GIT_STATUS_CURRENT ); @@ -2397,141 +2409,146 @@ void PROJECT_TREE_PANE::onGitCommit( wxCommandEvent& aEvent ) modifiedFiles ); auto ret = dlg.ShowModal(); - if( ret == wxID_OK ) + if( ret != wxID_OK ) + return; + + // Commit the changes + git_oid tree_id; + git_tree* tree = nullptr; + git_commit* parent = nullptr; + git_index* index = nullptr; + + std::vector files = dlg.GetSelectedFiles(); + + if( dlg.GetCommitMessage().IsEmpty() ) { - // Commit the changes - git_oid tree_id; - git_tree* tree = nullptr; - git_commit* parent = nullptr; - git_index* index = nullptr; + wxMessageBox( _( "Discarding commit due to empty commit message." ) ); + return; + } - std::vector files = dlg.GetSelectedFiles(); + if( files.empty() ) + { + wxMessageBox( _( "Discarding commit due to empty file selection." ) ); + return; + } - if( dlg.GetCommitMessage().IsEmpty() ) + if( git_repository_index( &index, repo ) != 0 ) + { + wxLogTrace( traceGit, wxString::Format( _( "Failed to get repository index: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + + KIGIT::GitIndexPtr indexPtr( index ); + + for( wxString& file : files ) + { + if( git_index_add_bypath( index, file.mb_str() ) != 0 ) { - wxMessageBox( _( "Discarding commit due to empty commit message." ) ); - return; - } - - if( files.empty() ) - { - wxMessageBox( _( "Discarding commit due to empty file selection." ) ); - return; - } - - if( git_repository_index( &index, repo ) != 0 ) - { - wxLogTrace( traceGit, wxString::Format( _( "Failed to get repository index: %s" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - - 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" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - } - - if( git_index_write( index ) != 0 ) - { - wxLogTrace( traceGit, wxString::Format( _( "Failed to write index: %s" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - - if( git_index_write_tree( &tree_id, index ) != 0) - { - wxLogTrace( traceGit, wxString::Format( _( "Failed to write tree: %s" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - - if( git_tree_lookup( &tree, repo, &tree_id ) != 0 ) - { - wxLogTrace( traceGit, wxString::Format( _( "Failed to lookup tree: %s" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - - KIGIT::GitTreePtr treePtr( tree ); - git_reference* headRef = nullptr; - - if( git_repository_head_unborn( repo ) == 0 ) - { - if( git_repository_head( &headRef, repo ) != 0 ) - { - wxLogTrace( traceGit, wxString::Format( _( "Failed to get HEAD reference: %s" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - - KIGIT::GitReferencePtr headRefPtr( headRef ); - - if( git_reference_peel( (git_object**) &parent, headRef, GIT_OBJECT_COMMIT ) != 0 ) - { - wxLogTrace( traceGit, wxString::Format( _( "Failed to get commit: %s" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - } - - 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 ) - { - wxLogTrace( traceGit, wxString::Format( _( "Failed to create author signature: %s" ), - KIGIT_COMMON::GetLastGitError() ) ); - return; - } - - KIGIT::GitSignaturePtr authorPtr( author ); - git_oid oid; - -#if( LIBGIT2_VER_MAJOR == 1 && LIBGIT2_VER_MINOR == 8 \ - && ( LIBGIT2_VER_REVISION < 2 || LIBGIT2_VER_REVISION == 3 ) ) - /* - * For libgit2 versions 1.8.0, 1.8.1. (cf19ddc52) - * This change was reverted for 1.8.2 (49d3fadfc, main branch) - * The revert for 1.8.2 was not included for 1.8.3 (which is on the maint/v1.8 branch, not main) - * This change was also reverted for 1.8.4 (94ba816f6, also maint/v1.8 branch) - * - * As of 1.8.4, the history is like this: - * - * * 3f4182d15 (tag: v1.8.4, maint/v1.8) - * * 94ba816f6 Revert "commit: fix const declaration" [puts const back] - * * 3353f78e8 (tag: v1.8.3) - * | * 4ce872a0f (tag: v1.8.2-rc1, tag: v1.8.2) - * | * 49d3fadfc Revert "commit: fix const declaration" [puts const back] - * |/ - * * 36f7e21ad (tag: v1.8.1) - * * d74d49148 (tag: v1.8.0) - * * cf19ddc52 commit: fix const declaration [removes const] - */ - git_commit* const parents[1] = { parent }; -#else - // For libgit2 versions older than 1.8.0, or equal to 1.8.2, or 1.8.4+ - const git_commit* parents[1] = { parent }; -#endif - - if( git_commit_create( &oid, repo, "HEAD", author, author, nullptr, commit_msg.mb_str(), tree, - 1, parents ) != 0 ) - { - wxMessageBox( wxString::Format( _( "Failed to create commit: %s" ), + wxMessageBox( wxString::Format( _( "Failed to add file to index: %s" ), KIGIT_COMMON::GetLastGitError() ) ); return; } } + + if( git_index_write( index ) != 0 ) + { + wxLogTrace( traceGit, wxString::Format( _( "Failed to write index: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + + if( git_index_write_tree( &tree_id, index ) != 0) + { + wxLogTrace( traceGit, wxString::Format( _( "Failed to write tree: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + + if( git_tree_lookup( &tree, repo, &tree_id ) != 0 ) + { + wxLogTrace( traceGit, wxString::Format( _( "Failed to lookup tree: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + + KIGIT::GitTreePtr treePtr( tree ); + git_reference* headRef = nullptr; + + if( git_repository_head_unborn( repo ) == 0 ) + { + if( git_repository_head( &headRef, repo ) != 0 ) + { + wxLogTrace( traceGit, wxString::Format( _( "Failed to get HEAD reference: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + + KIGIT::GitReferencePtr headRefPtr( headRef ); + + if( git_reference_peel( (git_object**) &parent, headRef, GIT_OBJECT_COMMIT ) != 0 ) + { + wxLogTrace( traceGit, wxString::Format( _( "Failed to get commit: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + } + + 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 ) + { + wxLogTrace( traceGit, wxString::Format( _( "Failed to create author signature: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + + KIGIT::GitSignaturePtr authorPtr( author ); + git_oid oid; + +#if( LIBGIT2_VER_MAJOR == 1 && LIBGIT2_VER_MINOR == 8 \ + && ( LIBGIT2_VER_REVISION < 2 || LIBGIT2_VER_REVISION == 3 ) ) + /* + * For libgit2 versions 1.8.0, 1.8.1. (cf19ddc52) + * This change was reverted for 1.8.2 (49d3fadfc, main branch) + * The revert for 1.8.2 was not included for 1.8.3 (which is on the maint/v1.8 branch, not main) + * This change was also reverted for 1.8.4 (94ba816f6, also maint/v1.8 branch) + * + * As of 1.8.4, the history is like this: + * + * * 3f4182d15 (tag: v1.8.4, maint/v1.8) + * * 94ba816f6 Revert "commit: fix const declaration" [puts const back] + * * 3353f78e8 (tag: v1.8.3) + * | * 4ce872a0f (tag: v1.8.2-rc1, tag: v1.8.2) + * | * 49d3fadfc Revert "commit: fix const declaration" [puts const back] + * |/ + * * 36f7e21ad (tag: v1.8.1) + * * d74d49148 (tag: v1.8.0) + * * cf19ddc52 commit: fix const declaration [removes const] + */ + git_commit* const parents[1] = { parent }; +#else + // For libgit2 versions older than 1.8.0, or equal to 1.8.2, or 1.8.4+ + const git_commit* parents[1] = { parent }; +#endif + + if( git_commit_create( &oid, repo, "HEAD", author, author, nullptr, commit_msg.mb_str(), tree, + 1, parents ) != 0 ) + { + wxMessageBox( wxString::Format( _( "Failed to create commit: %s" ), + KIGIT_COMMON::GetLastGitError() ) ); + return; + } + + wxLogTrace( traceGit, wxString::Format( _( "Created commit with id: %s" ), + git_oid_tostr_s( &oid ) ) ); + + m_gitStatusTimer.Start( 500, wxTIMER_ONE_SHOT ); } @@ -2662,27 +2679,39 @@ void PROJECT_TREE_PANE::onGitSyncTimer( wxTimerEvent& aEvent ) GIT_PULL_HANDLER handler( gitCommon ); handler.PerformFetch(); + + CallAfter( [this]() + { + gitStatusTimerHandler(); + } ); } ); if( ADVANCED_CFG::GetCfg().m_GitProjectStatusRefreshInterval > 0 ) { - wxLogTrace( traceGit, "onGitSyncTimer: Starting git status timer" ); + wxLogTrace( traceGit, "onGitSyncTimer: Restarting git sync timer" ); m_gitSyncTimer.Start( ADVANCED_CFG::GetCfg().m_GitProjectStatusRefreshInterval, wxTIMER_ONE_SHOT ); } } + +void PROJECT_TREE_PANE::gitStatusTimerHandler() +{ + updateTreeCache(); + thread_pool& tp = GetKiCadThreadPool(); + + tp.push_task( + [this]() + { + updateGitStatusIconMap(); + } ); +} + 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]() - { - updateGitStatusIconMap(); - } ); + gitStatusTimerHandler(); } diff --git a/kicad/project_tree_pane.h b/kicad/project_tree_pane.h index 33362e7402..16ebbf927c 100644 --- a/kicad/project_tree_pane.h +++ b/kicad/project_tree_pane.h @@ -303,6 +303,8 @@ private: void onGitStatusTimer( wxTimerEvent& event ); + void gitStatusTimerHandler(); + public: KICAD_MANAGER_FRAME* m_Parent; PROJECT_TREE* m_TreeProject; diff --git a/kicad/tools/kicad_manager_control.cpp b/kicad/tools/kicad_manager_control.cpp index 044102706f..b96cf204fe 100644 --- a/kicad/tools/kicad_manager_control.cpp +++ b/kicad/tools/kicad_manager_control.cpp @@ -170,7 +170,7 @@ int KICAD_MANAGER_CONTROL::NewFromRepository( const TOOL_EVENT& aEvent ) GIT_CLONE_HANDLER cloneHandler( pane->m_TreeProject->GitCommon() ); - cloneHandler.SetURL( dlg.GetRepoURL() ); + cloneHandler.SetRemote( dlg.GetFullURL() ); cloneHandler.SetClonePath( pro.GetPath() ); cloneHandler.SetUsername( dlg.GetUsername() ); cloneHandler.SetPassword( dlg.GetPassword() );