[PRETEST]: Added support for bare repo in clone function of git module

[Manoramsharma]

1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):

• N
• Y

Fix #384

2. What is the scope of this PR (e.g. component or file name):

3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Other

I have tried to add the support to download a bare repository without introducing a new function but modifying the current clone function making user to access the bare flag based on the cloneOptions.

4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):

• N
• Y

5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:

• Unit test
• Integration test
• Benchmark (add benchmark stats below)
• Manual test (add detailed scripts or steps below)
• Other

[Peefy]

PR conflicted

[Manoramsharma]

PR conflicted

Resolved the conflicts @Peefy.

[Peefy]

Why comment here?

[Manoramsharma]

I have left this function commented because I need review from @zong-zhe over the checkout implementation from bare, which I don't feel would be achieved something like that I did.

[Peefy]

CI failed.

[coveralls]

Pull Request Test Coverage Report for Build 10317935447

Details

• 10 of 14 (71.43%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.1%) to 40.336%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/git/git.go	10	14	71.43%

Totals
Change from base Build 10315058045:	-0.1%
Covered Lines:	3120
Relevant Lines:	7735

---

💛 - Coveralls

[Manoramsharma]

CI failed.

Hi @Peefy I have fixed the issues but I don't know why I am facing this license issue.

I have modified the clone() function itself to handle the cloning of bare repository rather introducing the new function as you told @zong-zhe . The user can handle the bare clone from the cli itself using clone with options.

[zong-zhe]

Hi @Manoramsharma 😃

Is this PR ready for review ?

[Manoramsharma]

Hi @Manoramsharma 😃

Is this PR ready for review ?

Hi @zong-zhe I am about to finish the changes , but before that I want to ask about some doubts I am facing in the current implementation of the git module:

1. Currently the go-getter client function clones the remote repository as a normal repo, and checkout to particular commit/branch/tag is handled by modifying the repoURL using ForceGitUrl() in getter.go. Right?
2. While handling the bare repo clone, do We need to have a separate function to ignore the commit/branch/tag?
3. How can a repo be identified as bare/normal before performing the actual clone operation, to know either to clone it as bare repo or clone it as normal repo?

[zong-zhe]

Hi @Manoramsharma 😃

1. Currently the go-getter client function clones the remote repository as a normal repo, and checkout to particular commit/branch/tag is handled by modifying the repoURL using ForceGitUrl() in getter.go. Right?

Yes, the API provided bygo-getter passes parameters through a URL.

2. While handling the bare repo clone, do We need to have a separate function to ignore the commit/branch/tag?

Personally, I don't think it's necessary to separate the two public methods, users just need to call one Clone() and control it with option. Users who know the bare repo should know that commit, branch, and tag do not affect the bare repo. And branch, commit, and tag are optional, not required.

3. How can a repo be identified as bare/normal before performing the actual clone operation, to know either to clone it as bare repo or clone it as normal repo ?

There is no need to distinguish the bare/normal repository before downloading; failure to download will return an error. The PR's job is to extend the functionality of the git so that kpm can download the bare repo.

[Manoramsharma]

Hi @zong-zhe I have updated the git_test.go to handle the different test case scenarios to clone bare and normal repo to the local path.

But here I am stuck with a test case when handling
4. Clone() a bare repo as a normal repo and checkout to branch, commit and tag.

The test suit is failing with the error below because I think I am not able to handle the local bare repositories path for the non bare cloning in the clone() function in git.go, Can you help me with the logic to be adopted for supporting this test scenario ?

--- FAIL: TestCloneWithOptions (14.40s)
    --- FAIL: TestCloneWithOptions/LocalBareCloneAsNonBareWithCommit (3.26s)
        /Users/manoramsharma/Desktop/kpm/pkg/git/git_test.go:178: assertion failed: invalid source string: git::/var/folders/4z/k675yy811dv47rnw3tqsmd7w0000gn/T/local_bare_repo972358456?ref=4e59d5852cd76542f9f0ec65e5773ca9f4e02462 (err *errors.errorString) != <nil> (nil <nil>)
FAIL
FAIL	kcl-lang.io/kpm/pkg/git	15.344s
FAIL

I am thinking to add a logic to clone() to check the whether the repoURL to be fetched is a localPath to handle the non-bare clone for the bare repository, something like this:

// Handle local bare repositories differently for non-bare cloning
	if cloneOpts.isLocalPath(cloneOpts.RepoURL) {
		cmdArgs := []string{"clone", cloneOpts.RepoURL, cloneOpts.LocalPath}
		cmd := exec.Command("git", cmdArgs...)

		output, err := cmd.CombinedOutput()
		if err != nil {
			return nil, fmt.Errorf("failed to clone repository: %s, error: %w", string(output), err)
		}

		repo, err := git.PlainOpen(cloneOpts.LocalPath)
		if err != nil {
			return nil, err
		}

		if cloneOpts.Commit != "" {
			w, err := repo.Worktree()
			if err != nil {
				return nil, err
			}

			err = w.Checkout(&git.CheckoutOptions{
				Hash: plumbing.NewHash(cloneOpts.Commit),
			})
			if err != nil {
				return nil, err
			}
		}

		return repo, nil
	}

[zong-zhe]

But here I am stuck with a test case when handling 4. Clone() a bare repo as a normal repo and checkout to branch, commit and tag.

The test suit is failing with the error below because I think I am not able to handle the local bare repositories path for the non bare cloning in the clone() function in git.go, Can you help me with the logic to be adopted for supporting this test scenario ?

Hi @Manoramsharma 😃

I tried the problem you mentioned, and your test case could be written like this：

repo, err := CloneWithOpts(
		WithRepoURL("file:///Users/xxx/xxx/xxx.git"),
		WithCommit("54c083093d"),
		WithWriter(&buf),
		WithLocalPath("/Users/xxx/"),
	)

How to add prefilx file:// for the path is not the work for this PR.

[Manoramsharma]

Hi @zong-zhe Thanks for the support. I have got all the checks passed 🎉

[zong-zhe]

LGTM

[zong-zhe]

Hi @Manoramsharma 😄

Congratulations! you have done this part of the work, next, you can try to complete the content of the second phase, in GitDownloader and OciDownloader to add some content to complete the previously mentioned file path structure support.

Downloader is the integration of the underlying functions, complete the download function, and organize the storage structure of the local third-party library, including OCI and Git.

Downloader is here: https://github.com/kcl-lang/kpm/blob/main/pkg/downloader/downloader.go

[Manoramsharma]

Hi @Manoramsharma 😄

Congratulations! you have done this part of the work, next, you can try to complete the content of the second phase, in GitDownloader and OciDownloader to add some content to complete the previously mentioned file path structure support.

Downloader is the integration of the underlying functions, complete the download function, and organize the storage structure of the local third-party library, including OCI and Git.

Downloader is here: https://github.com/kcl-lang/kpm/blob/main/pkg/downloader/downloader.go

Hi @zong-zhe This was made possible because of your support. Thanks for all those reviews upon my changes.

Actually I was busy in designing the cover letter for submitting as a part LFX mentee application for KCL. I already have an approach in my mind for refactoring of downloader module to support the file path as designed to achieve the local full storage path for a third-party dependencies. Will soon come up with a PR !! 🚀