Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sparse checkout feature - Download subdirectory #387

Closed

Conversation

officialasishkumar
Copy link
Contributor

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

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

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

Signed-off-by: Asish Kumar <[email protected]>
@officialasishkumar officialasishkumar marked this pull request as draft July 17, 2024 11:28
@officialasishkumar
Copy link
Contributor Author

officialasishkumar commented Jul 17, 2024

I am not able to access the sub-package flag that I am passing in the cli. Can someone help me out why it's not working?
/home/charon/coding/open-source/kcl-lang/kpm/kpm add --git https://github.com/KusionStack/catalog --sub-package modules --commit 3891e96

cc: @Peefy @zong-zhe

// ForceGitUrl will add the branch, tag or commit to the git URL and force it to the git protocol
// `<URL>` will return `Git::<URL>?ref=<branch|tag|commit>`
// ForceGitUrl will add the subpackage and branch, tag or commit to the git URL and force it to the git protocol
// `<URL>` will return `Git::<URL>//<SubPackage>?ref=<branch|tag|commit>`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might need to make sure that go-getter can do subdirectory download directly in this form

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Peefy
Copy link
Contributor

Peefy commented Aug 1, 2024

PR conflicted cc @officialasishkumar

@zong-zhe
Copy link
Contributor

zong-zhe commented Aug 8, 2024

Hi @officialasishkumar 😃

Is this part of the work ready to be reviewed? If you finish or run into any difficulties, feel free to ping me.

@officialasishkumar
Copy link
Contributor Author

Sure thing, I will finish it up during the weekends and open for review.

@officialasishkumar
Copy link
Contributor Author

2024-08-14.00-31-29.mp4

@officialasishkumar officialasishkumar marked this pull request as ready for review August 13, 2024 19:14
@officialasishkumar
Copy link
Contributor Author

PTAL

cc: @zong-zhe @Peefy

@officialasishkumar officialasishkumar changed the title [WIP] Sparse Checkout Feature Sparse Checkout Feature Aug 13, 2024
@@ -482,10 +482,11 @@ func (deps *Dependencies) loadLockFile(filepath string) error {
func ParseOpt(opt *opt.RegistryOptions) (*Dependency, error) {
if opt.Git != nil {
gitSource := downloader.Git{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add more test cases for GitDownloader to test the field you added.

@@ -67,6 +67,7 @@ func (registry *Registry) MarshalTOML() string {
const GIT_URL_PATTERN = "git = \"%s\""
const TAG_PATTERN = "tag = \"%s\""
const GIT_COMMIT_PATTERN = "commit = \"%s\""
const GIT_SUB_PACKAGE_PATTERN = "sub-package = \"%s\""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think use package is better.

a = { git = "https://github.com/test/test.git" package = "test" } # means select the package called "test" from git repo.

newRepoUrl := cloneOpts.RepoURL
if cloneOpts.SubPackage != "" {
newRepoUrl += "//" + cloneOpts.SubPackage
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The implementation idea here is still wrong, the idea we mentioned before is clone the entire repo. Recursively finds the dependency with the specified package name and loads it

Copy link
Contributor Author

@officialasishkumar officialasishkumar Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay will open a PR with that changes.

I am still not sure why we are discarding downloading a subdirectory (this method )? In case of large repo (let's say 100 MB +) with only one directory of kcl dependecies, it will defeat the purpose of sparse checkout since it will download the entire repo when the user only needs that particular subdirectory any day in the future.

cc: @zong-zhe @Peefy

@officialasishkumar officialasishkumar changed the title Sparse Checkout Feature Sparse checkout feature - Download subdirectory Aug 17, 2024
@Peefy
Copy link
Contributor

Peefy commented Aug 20, 2024

#453 is merged. Do we need this PR? @officialasishkumar

@officialasishkumar
Copy link
Contributor Author

#453 is merged. Do we need this PR? @officialasishkumar

I don't think so

@Peefy
Copy link
Contributor

Peefy commented Aug 20, 2024

#335 Can you modify the description in the plan to identify which tasks have been completed and which ones are not yet completed?

@Peefy
Copy link
Contributor

Peefy commented Aug 20, 2024

However, the PR conflicted.

#453 is merged. Do we need this PR? @officialasishkumar

I don't think so

However, the PR conflicted.

@officialasishkumar
Copy link
Contributor Author

@Peefy Fixed the conflict.

@officialasishkumar
Copy link
Contributor Author

#335 Can you modify the description in the plan to identify which tasks have been completed and which ones are not yet completed?

Done

@Peefy
Copy link
Contributor

Peefy commented Aug 20, 2024

#335 Can you modify the description in the plan to identify which tasks have been completed and which ones are not yet completed?

Done

Where?

@officialasishkumar
Copy link
Contributor Author

#335 Can you modify the description in the plan to identify which tasks have been completed and which ones are not yet completed?

Done

Where?

In the design doc,

Implementation and conclusion

@Peefy Peefy closed this Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: kpm add command displaying optimization
3 participants