From 3f470700b46f14c458e48cb5adacdb0fed37fe01 Mon Sep 17 00:00:00 2001 From: ShashwatDadhich Date: Mon, 4 Mar 2024 12:23:30 +0530 Subject: [PATCH 1/6] code refactored --- pkg/RepoManages.go | 8 +++++--- pkg/git/RepositoryManager.go | 36 +++++++++++++----------------------- pkg/git/Watcher.go | 2 +- 3 files changed, 19 insertions(+), 27 deletions(-) diff --git a/pkg/RepoManages.go b/pkg/RepoManages.go index ddcd9dcc..f3bc63a0 100644 --- a/pkg/RepoManages.go +++ b/pkg/RepoManages.go @@ -199,7 +199,8 @@ func (impl RepoManagerImpl) updatePipelineMaterialCommit(gitCtx git.GitContext, WithCloningMode(impl.configuration.CloningMode) fetchCount := impl.configuration.GitHistoryCount - commits, err := impl.repositoryManager.ChangesSince(gitCtx, material.CheckoutLocation, pipelineMaterial.Value, "", "", fetchCount) + var repository *git.GitRepository + commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repository, pipelineMaterial.Value, "", "", fetchCount, material.CheckoutLocation, true) //commits, err := impl.FetchChanges(pipelineMaterial.Id, "", "", 0) if err == nil { impl.logger.Infow("commits found", "commit", commits) @@ -669,7 +670,7 @@ func (impl RepoManagerImpl) GetLatestCommitForBranch(gitCtx git.GitContext, pipe return nil, err } - commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, branchName, "", "", 1, gitMaterial.CheckoutLocation) + commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, branchName, "", "", 1, gitMaterial.CheckoutLocation, false) if commits == nil { return nil, err @@ -719,7 +720,8 @@ func (impl RepoManagerImpl) GetCommitMetadataForPipelineMaterial(gitCtx git.GitC repoLock.Mutex.Unlock() impl.locker.ReturnLocker(gitMaterial.Id) }() - commits, err := impl.repositoryManager.ChangesSince(gitCtx, gitMaterial.CheckoutLocation, branchName, "", gitHash, 1) + var repository *git.GitRepository + commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repository, branchName, "", gitHash, 1, gitMaterial.CheckoutLocation, true) if err != nil { impl.logger.Errorw("error while fetching commit info", "pipelineMaterialId", pipelineMaterialId, "gitHash", gitHash, "err", err) return nil, err diff --git a/pkg/git/RepositoryManager.go b/pkg/git/RepositoryManager.go index 58c3ffa0..4126a42f 100644 --- a/pkg/git/RepositoryManager.go +++ b/pkg/git/RepositoryManager.go @@ -48,10 +48,8 @@ type RepositoryManager interface { TrimLastGitCommit(gitCommits []*GitCommitBase, count int) []*GitCommitBase // Clean cleans a directory Clean(cloneDir string) error - // ChangesSince given the checkput path, retrieves the latest commits for the gt repo existing on the path - ChangesSince(gitCtx GitContext, checkoutPath string, branch string, from string, to string, count int) ([]*GitCommitBase, error) // ChangesSinceByRepository returns the latest commits list for the given range and count for an existing repo - ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string) ([]*GitCommitBase, error) + ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string, openNewGitRepo bool) ([]*GitCommitBase, error) // GetCommitMetadata retrieves the commit metadata for given hash GetCommitMetadata(gitCtx GitContext, checkoutPath, commitHash string) (*GitCommitBase, error) // GetCommitForTag retrieves the commit metadata for given tag @@ -239,12 +237,23 @@ func (impl *RepositoryManagerImpl) GetCommitMetadata(gitCtx GitContext, checkout // from -> old commit // to -> new commit -func (impl *RepositoryManagerImpl) ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string) ([]*GitCommitBase, error) { +func (impl *RepositoryManagerImpl) ChangesSinceByRepository(gitCtx GitContext, repository *GitRepository, branch string, from string, to string, count int, checkoutPath string, openNewGitRepo bool) ([]*GitCommitBase, error) { // fix for azure devops (manual trigger webhook bases pipeline) : // branch name comes as 'refs/heads/master', we need to extract actual branch name out of it. // https://stackoverflow.com/questions/59956206/how-to-get-a-branch-name-with-a-slash-in-azure-devops var err error + if count == 0 { + count = impl.configuration.GitHistoryCount + } + + if openNewGitRepo { + repository, err = impl.gitManager.OpenRepoPlain(checkoutPath) + if err != nil { + return nil, err + } + } + start := time.Now() defer func() { util.TriggerGitOperationMetrics("changesSinceByRepository", start, err) @@ -332,25 +341,6 @@ func (impl *RepositoryManagerImpl) TrimLastGitCommit(gitCommits []*GitCommitBase return gitCommits } -func (impl *RepositoryManagerImpl) ChangesSince(gitCtx GitContext, checkoutPath string, branch string, from string, to string, count int) ([]*GitCommitBase, error) { - var err error - start := time.Now() - defer func() { - util.TriggerGitOperationMetrics("changesSince", start, err) - }() - if count == 0 { - count = impl.configuration.GitHistoryCount - } - r, err := impl.gitManager.OpenRepoPlain(checkoutPath) - if err != nil { - return nil, err - } - ///--------------------- - return impl.ChangesSinceByRepository(gitCtx, r, branch, from, to, count, checkoutPath) - ///---------------------- - -} - func (impl *RepositoryManagerImpl) CreateSshFileIfNotExistsAndConfigureSshCommand(gitCtx GitContext, location string, gitProviderId int, sshPrivateKeyContent string) (string, error) { // add private key var err error diff --git a/pkg/git/Watcher.go b/pkg/git/Watcher.go index 6a3c9a1f..0ec38d39 100644 --- a/pkg/git/Watcher.go +++ b/pkg/git/Watcher.go @@ -246,7 +246,7 @@ func (impl GitWatcherImpl) pollGitMaterialAndNotify(material *sql.GitMaterial) e lastSeenHash = material.LastSeenHash } fetchCount := impl.configuration.GitHistoryCount - commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, material.Value, lastSeenHash, "", fetchCount, checkoutLocation) + commits, err := impl.repositoryManager.ChangesSinceByRepository(gitCtx, repo, material.Value, lastSeenHash, "", fetchCount, checkoutLocation, false) if err != nil { material.Errored = true material.ErrorMsg = err.Error() From 893f958bf0632750b61d6898d4341e9f0bb3765c Mon Sep 17 00:00:00 2001 From: ShashwatDadhich Date: Mon, 4 Mar 2024 12:45:11 +0530 Subject: [PATCH 2/6] code refactored --- pkg/RepoManages.go | 4 +++- pkg/git/RepositoryManager.go | 13 +++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/pkg/RepoManages.go b/pkg/RepoManages.go index f3bc63a0..4161c8c6 100644 --- a/pkg/RepoManages.go +++ b/pkg/RepoManages.go @@ -354,11 +354,13 @@ func (impl RepoManagerImpl) checkoutMaterial(gitCtx git.GitContext, material *sq gitCtx = gitCtx.WithCredentials(userName, password). WithCloningMode(impl.configuration.CloningMode) - checkoutPath, checkoutLocationForFetching, err := impl.repositoryManager.GetCheckoutPathAndLocation(gitCtx, material, gitProvider.Url) + checkoutPath, _, _, err := impl.repositoryManager.GetLocationForMaterial(material, gitCtx.CloningMode) if err != nil { return material, err } + checkoutLocationForFetching := impl.repositoryManager.GetCheckoutLocation(checkoutPath) + err = impl.repositoryManager.Add(gitCtx, material.GitProviderId, checkoutPath, material.Url, gitProvider.AuthMode, gitProvider.SshPrivateKey) if err == nil { material.CheckoutLocation = checkoutLocationForFetching diff --git a/pkg/git/RepositoryManager.go b/pkg/git/RepositoryManager.go index 4126a42f..eccecbd3 100644 --- a/pkg/git/RepositoryManager.go +++ b/pkg/git/RepositoryManager.go @@ -44,7 +44,7 @@ type RepositoryManager interface { GetSshPrivateKeyPath(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) (string, error) FetchRepo(gitCtx GitContext, location string) error GetLocationForMaterial(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) - GetCheckoutPathAndLocation(gitCtx GitContext, material *sql.GitMaterial, url string) (string, string, error) + GetCheckoutLocation(checkoutPath string) string TrimLastGitCommit(gitCommits []*GitCommitBase, count int) []*GitCommitBase // Clean cleans a directory Clean(cloneDir string) error @@ -102,15 +102,8 @@ func (impl *RepositoryManagerImpl) GetLocationForMaterial(material *sql.GitMater return "", httpsMatched, sshMatched, fmt.Errorf("unsupported format url %s", material.Url) } -func (impl *RepositoryManagerImpl) GetCheckoutPathAndLocation(gitCtx GitContext, material *sql.GitMaterial, url string) (string, string, error) { - var checkoutPath string - var checkoutLocationForFetching string - checkoutPath, _, _, err := impl.GetLocationForMaterial(material, gitCtx.CloningMode) - if err != nil { - return checkoutPath, checkoutLocationForFetching, err - } - checkoutLocationForFetching = checkoutPath - return checkoutPath, checkoutLocationForFetching, nil +func (impl *RepositoryManagerImpl) GetCheckoutLocation(checkoutPath string) string { + return checkoutPath } func (impl *RepositoryManagerImpl) Add(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) error { From b8655f5cff2c3b5018a6a3499811b84d1befdad1 Mon Sep 17 00:00:00 2001 From: ShashwatDadhich Date: Mon, 4 Mar 2024 12:54:19 +0530 Subject: [PATCH 3/6] code refactored --- pkg/RepoManages.go | 2 +- pkg/git/RepositoryManager.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/RepoManages.go b/pkg/RepoManages.go index 4161c8c6..e7ac85dd 100644 --- a/pkg/RepoManages.go +++ b/pkg/RepoManages.go @@ -359,7 +359,7 @@ func (impl RepoManagerImpl) checkoutMaterial(gitCtx git.GitContext, material *sq return material, err } - checkoutLocationForFetching := impl.repositoryManager.GetCheckoutLocation(checkoutPath) + checkoutLocationForFetching := impl.repositoryManager.GetCheckoutLocation(gitCtx, material, gitProvider.Url, checkoutPath) err = impl.repositoryManager.Add(gitCtx, material.GitProviderId, checkoutPath, material.Url, gitProvider.AuthMode, gitProvider.SshPrivateKey) if err == nil { diff --git a/pkg/git/RepositoryManager.go b/pkg/git/RepositoryManager.go index eccecbd3..f7114626 100644 --- a/pkg/git/RepositoryManager.go +++ b/pkg/git/RepositoryManager.go @@ -44,7 +44,7 @@ type RepositoryManager interface { GetSshPrivateKeyPath(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) (string, error) FetchRepo(gitCtx GitContext, location string) error GetLocationForMaterial(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) - GetCheckoutLocation(checkoutPath string) string + GetCheckoutLocation(gitCtx GitContext, material *sql.GitMaterial, url, checkoutPath string) string TrimLastGitCommit(gitCommits []*GitCommitBase, count int) []*GitCommitBase // Clean cleans a directory Clean(cloneDir string) error @@ -102,7 +102,7 @@ func (impl *RepositoryManagerImpl) GetLocationForMaterial(material *sql.GitMater return "", httpsMatched, sshMatched, fmt.Errorf("unsupported format url %s", material.Url) } -func (impl *RepositoryManagerImpl) GetCheckoutLocation(checkoutPath string) string { +func (impl *RepositoryManagerImpl) GetCheckoutLocation(gitCtx GitContext, material *sql.GitMaterial, url, checkoutPath string) string { return checkoutPath } From 37b91cdcae3585fa6b94ef61da063d97f9f3bcc1 Mon Sep 17 00:00:00 2001 From: ShashwatDadhich Date: Tue, 5 Mar 2024 15:28:15 +0530 Subject: [PATCH 4/6] code refactored --- pkg/git/Util.go | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/git/Util.go b/pkg/git/Util.go index 334bc7e8..644a33a0 100644 --- a/pkg/git/Util.go +++ b/pkg/git/Util.go @@ -46,8 +46,22 @@ const ( func GetProjectName(url string) string { //if url = https://github.com/devtron-labs/git-sensor.git then it will return git-sensor - projName := strings.Split(url, ".")[1] - projectName := projName[strings.LastIndex(projName, "/")+1:] + // Split the URL by dots + parts := strings.Split(url, "/") + + // Extract the last part after the last slash + projectName := parts[len(parts)-1] + + // Split the project name by dots + projectNameParts := strings.Split(projectName, ".") + + // Check if the last part is "git" and exclude it + if projectNameParts[len(projectNameParts)-1] == "git" { + projectName = strings.Join(projectNameParts[:len(projectNameParts)-1], ".") + } else { + projectName = strings.Join(projectNameParts, ".") + } + return projectName } func GetCheckoutPath(url string, cloneLocation string) string { From ca4268a950bae225450bd8993ac4ad5534289203 Mon Sep 17 00:00:00 2001 From: ShashwatDadhich Date: Thu, 7 Mar 2024 12:34:24 +0530 Subject: [PATCH 5/6] code review comments incorporated --- pkg/git/Util.go | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/pkg/git/Util.go b/pkg/git/Util.go index 644a33a0..e096b742 100644 --- a/pkg/git/Util.go +++ b/pkg/git/Util.go @@ -46,23 +46,8 @@ const ( func GetProjectName(url string) string { //if url = https://github.com/devtron-labs/git-sensor.git then it will return git-sensor - // Split the URL by dots - parts := strings.Split(url, "/") - - // Extract the last part after the last slash - projectName := parts[len(parts)-1] - - // Split the project name by dots - projectNameParts := strings.Split(projectName, ".") - - // Check if the last part is "git" and exclude it - if projectNameParts[len(projectNameParts)-1] == "git" { - projectName = strings.Join(projectNameParts[:len(projectNameParts)-1], ".") - } else { - projectName = strings.Join(projectNameParts, ".") - } - - return projectName + url = url[strings.LastIndex(url, "/")+1:] + return strings.TrimSuffix(url, ".git") } func GetCheckoutPath(url string, cloneLocation string) string { //url= https://github.com/devtron-labs/git-sensor.git cloneLocation= git-base/1/github.com/prakash100198 From 07e5e1737ec5855fc126b6625dc09564a8c39fae Mon Sep 17 00:00:00 2001 From: ShashwatDadhich Date: Tue, 12 Mar 2024 16:51:02 +0530 Subject: [PATCH 6/6] code review comments incorporated --- pkg/RepoManages.go | 2 +- pkg/git/RepositoryManager.go | 4 ++-- pkg/git/Watcher.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/RepoManages.go b/pkg/RepoManages.go index e7ac85dd..05017077 100644 --- a/pkg/RepoManages.go +++ b/pkg/RepoManages.go @@ -354,7 +354,7 @@ func (impl RepoManagerImpl) checkoutMaterial(gitCtx git.GitContext, material *sq gitCtx = gitCtx.WithCredentials(userName, password). WithCloningMode(impl.configuration.CloningMode) - checkoutPath, _, _, err := impl.repositoryManager.GetLocationForMaterial(material, gitCtx.CloningMode) + checkoutPath, _, _, err := impl.repositoryManager.GetCheckoutLocationFromGitUrl(material, gitCtx.CloningMode) if err != nil { return material, err } diff --git a/pkg/git/RepositoryManager.go b/pkg/git/RepositoryManager.go index f7114626..97c85d09 100644 --- a/pkg/git/RepositoryManager.go +++ b/pkg/git/RepositoryManager.go @@ -43,7 +43,7 @@ type RepositoryManager interface { Add(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) error GetSshPrivateKeyPath(gitCtx GitContext, gitProviderId int, location, url string, authMode sql.AuthMode, sshPrivateKeyContent string) (string, error) FetchRepo(gitCtx GitContext, location string) error - GetLocationForMaterial(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) + GetCheckoutLocationFromGitUrl(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) GetCheckoutLocation(gitCtx GitContext, material *sql.GitMaterial, url, checkoutPath string) string TrimLastGitCommit(gitCommits []*GitCommitBase, count int) []*GitCommitBase // Clean cleans a directory @@ -82,7 +82,7 @@ func (impl *RepositoryManagerImpl) IsSpaceAvailableOnDisk() bool { return availableSpace > int64(impl.configuration.MinLimit)*1024*1024 } -func (impl *RepositoryManagerImpl) GetLocationForMaterial(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) { +func (impl *RepositoryManagerImpl) GetCheckoutLocationFromGitUrl(material *sql.GitMaterial, cloningMode string) (location string, httpMatched bool, shMatched bool, err error) { //gitRegex := `/(?:git|ssh|https?|git@[-\w.]+):(\/\/)?(.*?)(\.git)(\/?|\#[-\d\w._]+?)$/` httpsRegex := `^https.*` httpsMatched, err := regexp.MatchString(httpsRegex, material.Url) diff --git a/pkg/git/Watcher.go b/pkg/git/Watcher.go index 0ec38d39..fb97bfa6 100644 --- a/pkg/git/Watcher.go +++ b/pkg/git/Watcher.go @@ -199,7 +199,7 @@ func (impl GitWatcherImpl) pollGitMaterialAndNotify(material *sql.GitMaterial) e // there might be the case if ssh private key gets flush from disk, so creating and single retrying in this case if gitProvider.AuthMode == sql.AUTH_MODE_SSH { if strings.Contains(material.CheckoutLocation, "/.git") { - location, _, _, err = impl.repositoryManager.GetLocationForMaterial(material, gitCtx.CloningMode) + location, _, _, err = impl.repositoryManager.GetCheckoutLocationFromGitUrl(material, gitCtx.CloningMode) if err != nil { impl.logger.Errorw("error in getting clone location ", "material", material, "err", err) return err