[WIP]: Extend gitDownloader to support new cache structure #496
Conversation
Signed-off-by: Manoramsharma <[email protected]>
| // If caching is enabled, clone the repo as a bare repository to the cache. | ||
| if opts.EnableCache { | ||
| // Check if the bare repository already exists in the cache. | ||
| if utils.DirExists(opts.CachePath) { |
There was a problem hiding this comment.
Hi @Manoramsharma 😄
are some logical issues here that need to be considered:
- Whether the bare repository exists.
- Whether the corresponding
tag/commit/branchexists in the bare repository.
Only when both conditions are met, that the bare repository exists and that the corresponding tag/commit/branch exists in the repository will the cache be hit, and the corresponding content will be cloned from the cache. Your current if statement only considers the first condition. The second condition needs to be added.
And before going to the bare repository to look for dependencies, it can look directly in the download path and use them if they already exist.
| if err != nil { | ||
| return err | ||
| // Now checkout the specific version (commit/tag/branch) from the bare repository. | ||
| checkoutPath := filepath.Join(opts.LocalPath, "git", "checkouts", fmt.Sprintf("%s-%s", filepath.Base(gitSource.Url), utils.GenerateHash(gitSource.Url))) |
There was a problem hiding this comment.
Hi @Manoramsharma 😄
The hash is not only used here, the path to the bare repo should also be a hash value.
kpm/git
├── checkouts. # checkout the specific version of git repository from cache bare repository
│ ├── kcl-2a81898195a215f1
│ │ └── 33bb450. . # All the version of kcl package from git repository will be replaced with commit id
│ ├── kcl-578669463c900b87
│ │ └── 33bb450
└── db # A bare git repository for cache git repo
├── kcl-2a81898195a215f1. # <NAME>-<HASH> <NAME> is the name of git repo,
├── kcl-578669463c900b87. # <HASH> is calculated by the git full url.Add more details, the hashed paths are default paths, that is, the user has not specified localpath and cachepath in DownloadOptions, we use the hash value to compute a default path, and if the user specifies paths, we use the specified paths directly.
There was a problem hiding this comment.
Hi @zong-zhe can you help me with this one, how to use hash value to generate a default path? Currently I have covered only the part when the localpath and cachepath are defined by the user in DownloadOptions.
Where can I get the reference of using hash-value and genrate default storage path?
There was a problem hiding this comment.
This GenerateHash function I have added in the utils should I move forward with this one? OR you have thought something different with hash-value and default path generation
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
fix #384
2. What is the scope of this PR (e.g. component or file name):
pkg/downloader/downloader.go
pkg/downloader/downloader_test.go
pkg/utils/utils.go
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
We already have completed
CloneWithOptsto support bare cloning , in this PR I have extended thegitDownloaderto clone the dependency as bare repo to the cache path.