From ee041504efd65d4ff2458f6c617d77aaa44c73ca Mon Sep 17 00:00:00 2001 From: faza Date: Mon, 1 Sep 2025 20:24:05 +0530 Subject: [PATCH 1/4] Add the lfs related fixes from feature/storage branch --- .gitignore | 3 ++- cmd/git-remote-gitopia/gitopia.go | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c795b05..d76b74e 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ -build \ No newline at end of file +build +.DS_Store diff --git a/cmd/git-remote-gitopia/gitopia.go b/cmd/git-remote-gitopia/gitopia.go index 7d1619b..8d0ff9b 100644 --- a/cmd/git-remote-gitopia/gitopia.go +++ b/cmd/git-remote-gitopia/gitopia.go @@ -98,6 +98,14 @@ func (h *GitopiaHandler) Initialize(remote *core.Remote) error { h.remoteRepository = *res.Repository + // Configure LFS URL for clone operations to avoid SSH to non-existent "gitopia" hostname + lfsURL := fmt.Sprintf("%v/%v.git", config.GitServerHost, h.remoteRepository.Id) + cmd := core.GitCommand("git", "config", "--local", "lfs.url", lfsURL) + if err := cmd.Run(); err != nil { + // Log but don't fail if LFS config fails (repo might not have LFS) + remote.Logger.Printf("Warning: could not configure LFS URL: %v", err) + } + return nil } @@ -139,9 +147,11 @@ func (h *GitopiaHandler) List(remote *core.Remote, forPush bool) ([]string, erro func (h *GitopiaHandler) Fetch(remote *core.Remote, refsToFetch []core.RefToFetch) error { remoteURL := fmt.Sprintf("%v/%v.git", config.GitServerHost, h.remoteRepository.Id) + lfsURL := remoteURL // Use same URL for LFS if !remote.Force { args := []string{ + "-c", fmt.Sprintf("lfs.url=%s", lfsURL), "fetch", "--no-write-fetch-head", remoteURL, @@ -165,6 +175,7 @@ func (h *GitopiaHandler) Fetch(remote *core.Remote, refsToFetch []core.RefToFetc } args := []string{ + "-c", fmt.Sprintf("lfs.url=%s", lfsURL), "fetch", "--no-write-fetch-head", remoteURL, @@ -215,6 +226,7 @@ func (h *GitopiaHandler) Push(remote *core.Remote, refsToPush []core.RefToPush) } remoteURL := fmt.Sprintf("%v/%v.git", config.GitServerHost, h.remoteRepository.Id) + lfsURL := remoteURL // Use same URL for LFS var newRemoteRefSha string var setBranches []gitopiatypes.MsgMultiSetBranch_Branch @@ -267,6 +279,8 @@ func (h *GitopiaHandler) Push(remote *core.Remote, refsToPush []core.RefToPush) "credential.helper=", "-c", "credential.helper=gitopia", + "-c", + fmt.Sprintf("lfs.url=%s", lfsURL), "push", remoteURL, fmt.Sprintf("%s:%s", ref.Local, ref.Remote), From dd38977cf3c817157660f3eed8b0380d4f8ac5c6 Mon Sep 17 00:00:00 2001 From: faza Date: Tue, 2 Sep 2025 10:58:10 +0530 Subject: [PATCH 2/4] Optimize fetch to use single git command for all refs - Batch all refs into one git fetch command instead of separate commands per ref - Apply --force globally when any ref needs it or remote.Force is set - Reduces git subprocess calls from N to 1, improving performance - Maintains same functionality while eliminating inefficient loop --- cmd/git-remote-gitopia/gitopia.go | 62 ++++++++++++++----------------- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/cmd/git-remote-gitopia/gitopia.go b/cmd/git-remote-gitopia/gitopia.go index 8d0ff9b..3f6b7e5 100644 --- a/cmd/git-remote-gitopia/gitopia.go +++ b/cmd/git-remote-gitopia/gitopia.go @@ -149,45 +149,37 @@ func (h *GitopiaHandler) Fetch(remote *core.Remote, refsToFetch []core.RefToFetc remoteURL := fmt.Sprintf("%v/%v.git", config.GitServerHost, h.remoteRepository.Id) lfsURL := remoteURL // Use same URL for LFS - if !remote.Force { - args := []string{ - "-c", fmt.Sprintf("lfs.url=%s", lfsURL), - "fetch", - "--no-write-fetch-head", - remoteURL, - } - for _, ref := range refsToFetch { - args = append(args, ref.Ref) - } - cmd := core.GitCommand("git", args...) - if err := cmd.Run(); err != nil { - return errors.Wrap(err, "error fetching from remote repository") + // Check if any refs need force + needsForce := false + var processedRefs []string + + for _, ref := range refsToFetch { + refSpec := ref.Ref + if strings.HasPrefix(refSpec, "+") { + refSpec = strings.TrimPrefix(refSpec, "+") + needsForce = true } - - return nil + processedRefs = append(processedRefs, refSpec) } - for _, ref := range refsToFetch { - force := false - if strings.HasPrefix(ref.Ref, "+") { - ref.Ref = strings.TrimPrefix(ref.Ref, "+") - force = true - } + // Build single git fetch command with all refs + args := []string{ + "-c", fmt.Sprintf("lfs.url=%s", lfsURL), + "fetch", + "--no-write-fetch-head", + } + + // Add force flag if any ref needs it + if needsForce || remote.Force { + args = append(args, "--force") + } + + args = append(args, remoteURL) + args = append(args, processedRefs...) - args := []string{ - "-c", fmt.Sprintf("lfs.url=%s", lfsURL), - "fetch", - "--no-write-fetch-head", - remoteURL, - ref.Ref, - } - if force { - args = append(args, "--force") - } - cmd := core.GitCommand("git", args...) - if err := cmd.Run(); err != nil { - return errors.Wrap(err, "error fetching from remote repository") - } + cmd := core.GitCommand("git", args...) + if err := cmd.Run(); err != nil { + return errors.Wrap(err, "error fetching from remote repository") } return nil From faed7fa20c0b083c88052238133562a92420f95a Mon Sep 17 00:00:00 2001 From: faza Date: Fri, 5 Sep 2025 16:10:18 +0530 Subject: [PATCH 3/4] Give more error information in invalid remote url --- core/util.go | 61 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 22 deletions(-) diff --git a/core/util.go b/core/util.go index e3febaf..ec502ef 100644 --- a/core/util.go +++ b/core/util.go @@ -2,6 +2,7 @@ package core import ( "errors" + "fmt" "os" "os/exec" "path" @@ -52,31 +53,47 @@ func CleanUpProcessGroup(cmd *exec.Cmd) { } func ValidateGitopiaRemoteURL(remoteURL string) (remoteUserId string, remoteRepositoryName string, err error) { - if strings.HasPrefix(remoteURL, GITOPIA_PREFIX) { - s := strings.TrimPrefix(remoteURL, GITOPIA_PREFIX) - sp := strings.Split(s, "/") + if !strings.HasPrefix(remoteURL, GITOPIA_PREFIX) { + return "", "", fmt.Errorf("invalid gitopia remote url: must start with '%s', got '%s'", GITOPIA_PREFIX, remoteURL) + } + + s := strings.TrimPrefix(remoteURL, GITOPIA_PREFIX) + sp := strings.Split(s, "/") + + if len(sp) != 2 { + return "", "", fmt.Errorf("invalid gitopia remote url: expected format 'gitopia://user/repository', got '%s' (found %d parts after prefix)", remoteURL, len(sp)) + } + + remoteUserId = sp[0] + remoteRepositoryName = sp[1] - if len(sp) != 2 { - return "", "", ErrInvalidGitopiaRemoteURL + if remoteUserId == "" { + return "", "", fmt.Errorf("invalid gitopia remote url: user ID cannot be empty in '%s'", remoteURL) + } + + if remoteRepositoryName == "" { + return "", "", fmt.Errorf("invalid gitopia remote url: repository name cannot be empty in '%s'", remoteURL) + } + + // Try to parse as bech32 address first + _, err = sdk.AccAddressFromBech32(remoteUserId) + if err != nil { + // If not a valid bech32 address, validate as username + if len(remoteUserId) < 3 { + return "", "", fmt.Errorf("invalid gitopia remote url: user ID '%s' is too short (minimum 3 characters)", remoteUserId) + } + if len(remoteUserId) > 39 { + return "", "", fmt.Errorf("invalid gitopia remote url: user ID '%s' is too long (maximum 39 characters)", remoteUserId) + } + + valid, regexErr := regexp.MatchString("^[a-zA-Z0-9]+(?:[-]?[a-zA-Z0-9])*$", remoteUserId) + if regexErr != nil { + return "", "", fmt.Errorf("invalid gitopia remote url: error validating user ID '%s': %v", remoteUserId, regexErr) } - remoteUserId = sp[0] - remoteRepositoryName = sp[1] - - _, err := sdk.AccAddressFromBech32(remoteUserId) - if err != nil { - if len(remoteUserId) < 3 || len(remoteUserId) > 39 { - return "", "", ErrInvalidGitopiaRemoteURL - } - valid, err := regexp.MatchString("^[a-zA-Z0-9]+(?:[-]?[a-zA-Z0-9])*$", remoteUserId) - if err != nil { - return "", "", ErrInvalidGitopiaRemoteURL - } - if !valid { - return "", "", ErrInvalidGitopiaRemoteURL - } + if !valid { + return "", "", fmt.Errorf("invalid gitopia remote url: user ID '%s' contains invalid characters (only alphanumeric and hyphens allowed)", remoteUserId) } - return remoteUserId, remoteRepositoryName, nil } - return "", "", ErrInvalidGitopiaRemoteURL + return remoteUserId, remoteRepositoryName, nil } From 887ea1d52109de3c2b3d0516a21c11898770d5ab Mon Sep 17 00:00:00 2001 From: faza Date: Sun, 7 Sep 2025 14:14:22 +0530 Subject: [PATCH 4/4] fix: validate default branch exists before setting HEAD in List method The List method was blindly setting HEAD to point to the configured default branch without verifying the branch actually exists. This caused "couldn't find remote ref" errors when cloning repositories with misconfigured default branches (e.g., pointing to non-existent "master" branch). Changes: - Add validation to check if configured default branch exists - Implement fallback logic: prefer "main" branch, then first available branch - Add warning logs when falling back to different branch - Handle edge case of repositories with no branches - Prevent fatal clone errors due to invalid HEAD references Fixes scenarios where repository default branch configuration points to non-existent branches, improving clone reliability and user experience. --- cmd/git-remote-gitopia/gitopia.go | 36 +++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/cmd/git-remote-gitopia/gitopia.go b/cmd/git-remote-gitopia/gitopia.go index 3f6b7e5..a868090 100644 --- a/cmd/git-remote-gitopia/gitopia.go +++ b/cmd/git-remote-gitopia/gitopia.go @@ -122,8 +122,25 @@ func (h *GitopiaHandler) List(remote *core.Remote, forPush bool) ([]string, erro if err != nil { return out, err } + + // Track available branches and find a suitable HEAD candidate + var headCandidate string + defaultBranchExists := false + for _, branch := range branchAllRes.Branch { out = append(out, fmt.Sprintf("%s %s%s", branch.Sha, branchPrefix, branch.Name)) + + // Check if the configured default branch exists + if branch.Name == h.remoteRepository.DefaultBranch { + defaultBranchExists = true + headCandidate = branch.Name + } else if headCandidate == "" { + // Fallback: use the first branch we find + headCandidate = branch.Name + } else if branch.Name == "main" { + // Prefer "main" over other branches as fallback + headCandidate = branch.Name + } } tagAllRes, err := h.queryClient.RepositoryTagAll(context.Background(), &gitopiatypes.QueryAllRepositoryTagRequest{ @@ -140,7 +157,18 @@ func (h *GitopiaHandler) List(remote *core.Remote, forPush bool) ([]string, erro out = append(out, fmt.Sprintf("%s %s%s", tag.Sha, tagPrefix, tag.Name)) } - out = append(out, fmt.Sprintf("@refs/heads/%s HEAD", h.remoteRepository.DefaultBranch)) + // Only set HEAD if we have a valid branch to point to + if headCandidate != "" { + if !defaultBranchExists && headCandidate != h.remoteRepository.DefaultBranch { + // Log a warning if we're falling back to a different branch + remote.Logger.Printf("Warning: configured default branch '%s' doesn't exist, using '%s' as HEAD\n", + h.remoteRepository.DefaultBranch, headCandidate) + } + out = append(out, fmt.Sprintf("@refs/heads/%s HEAD", headCandidate)) + } else { + // No branches exist at all - this is a serious issue + remote.Logger.Printf("Warning: repository has no branches, HEAD will not be set\n") + } return out, nil } @@ -152,7 +180,7 @@ func (h *GitopiaHandler) Fetch(remote *core.Remote, refsToFetch []core.RefToFetc // Check if any refs need force needsForce := false var processedRefs []string - + for _, ref := range refsToFetch { refSpec := ref.Ref if strings.HasPrefix(refSpec, "+") { @@ -168,12 +196,12 @@ func (h *GitopiaHandler) Fetch(remote *core.Remote, refsToFetch []core.RefToFetc "fetch", "--no-write-fetch-head", } - + // Add force flag if any ref needs it if needsForce || remote.Force { args = append(args, "--force") } - + args = append(args, remoteURL) args = append(args, processedRefs...)