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

feat: add git sparse checkout #14272

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yordis
Copy link

@yordis yordis commented Jun 29, 2023

No description provided.

@yordis yordis force-pushed the add-git-sparse-checkout branch 2 times, most recently from 9d2a848 to 10ab6f3 Compare August 25, 2023 04:49
@yordis
Copy link
Author

yordis commented Aug 25, 2023

Well go-git/go-git#90 🤷🏻

@yordis
Copy link
Author

yordis commented Aug 25, 2023

I made it work using the git binary instead of the Git Go library ... downstream support is required unless we are OK doing everything using the git binary (which I do not see what the problem will be honestly)

@yordis
Copy link
Author

yordis commented Aug 25, 2023

Note: #11198

Comment on lines +322 to +339

_, err = m.runCmd("init")
if err != nil {
return err
}
_, err = repo.CreateRemote(&config.RemoteConfig{
Name: git.DefaultRemoteName,
URLs: []string{m.repoURL},
})
return err

_, err = m.runCmd("remote", "add", git.DefaultRemoteName, m.repoURL)
if err != nil {
return err
}
return nil
}
Copy link
Author

Choose a reason for hiding this comment

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

I HAD to do this to make things work! Whatever that package is doing, it doesn't work with sparse checkout

@@ -62,6 +62,8 @@ type gitRefCache interface {
type Client interface {
Root() string
Init() error
SparseCheckout(patterns []string) error
Pull(revision string) error
Copy link
Author

Choose a reason for hiding this comment

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

I need this one over Fetch with sparse checkout

@yordis yordis force-pushed the add-git-sparse-checkout branch 5 times, most recently from 61748e1 to 888a97f Compare August 25, 2023 07:02
@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Attention: 47 lines in your changes are missing coverage. Please review.

Comparison is base (2730170) 49.98% compared to head (594056a) 49.95%.
Report is 8 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #14272      +/-   ##
==========================================
- Coverage   49.98%   49.95%   -0.03%     
==========================================
  Files         266      266              
  Lines       46043    46110      +67     
==========================================
+ Hits        23016    23036      +20     
- Misses      20770    20805      +35     
- Partials     2257     2269      +12     
Files Coverage Δ
pkg/apis/application/v1alpha1/types.go 58.44% <0.00%> (-0.14%) ⬇️
reposerver/repository/repository.go 59.51% <52.63%> (-0.34%) ⬇️
util/git/client.go 50.85% <42.22%> (-1.41%) ⬇️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yordis yordis marked this pull request as ready for review August 25, 2023 20:52
@yordis yordis marked this pull request as draft August 25, 2023 20:55
@yordis yordis force-pushed the add-git-sparse-checkout branch 3 times, most recently from 0622736 to f017698 Compare October 2, 2023 07:35
err = gitClient.Fetch(revision)
if err != nil {
return status.Errorf(codes.Internal, "Failed to checkout revision %s: %v", revision, err)
// TODO: I am not sure about this Pull vs Fetch situation
Copy link
Author

Choose a reason for hiding this comment

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

Need help here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Have you tested sparse checkouts using fetch instead of pull? Seems like it would be ideal to limit the number of ways we retrieve repo contents.

Copy link
Author

Choose a reason for hiding this comment

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

I tried locally using a git bash script and didn't work for me. I used https://github.com/derrickstolee/sparse-checkout-example repo for it.

I am gonna try again, but I think it is require, I am not sure.

I leaved a note here to follow on it

@yordis yordis force-pushed the add-git-sparse-checkout branch 3 times, most recently from 271e714 to 6689cd5 Compare October 2, 2023 08:43
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>

add notes

run codegen

asd

asdsadasda

run codegen

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
sb.WriteString(pattern)
}

if _, err := m.runCmd("sparse-checkout", "set", sb.String()); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

If I understand this correctly, the sparse-checkout setting "sticks." I think this means we need to prevent concurrent access to the repo for apps where the sparse checkout setting doesn't match (similar to what we do for Kustomize apps where some on-disk state is set by a config option).

Copy link
Author

Choose a reason for hiding this comment

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

It does stick to the configuration of a git repo. Gonna be working on that next.

Copy link
Author

Choose a reason for hiding this comment

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

Note:

git sparse-checkout disable

Copy link
Author

Choose a reason for hiding this comment

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

similar to what we do for Kustomize apps where some on-disk state is set by a config option

@crenshaw-dev I am gonna be digging into it, but in the meantime, could you share some links to where the Kustomize version is happening?

Trying to figure out how the codebase reasons about such a situation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a semaphore mechanism to allow multiple read-only processes to use a repo checkout out at a particular commit at the same time.

I think this is the field which determines whether concurrency is allowed for a certain repo operation:

allowConcurrent bool

@sahotay
Copy link

sahotay commented Mar 3, 2024

As the repo size is growing I am seeing error
failed to generate manifest for source 1 of 1: rpc error: code = Internal desc = Failed to fetch default: git fetch origin --tags --force --prunefailed exit status 128: fatal: fetch-pack: invalid index-pack output

I hope this issue will resolve this error, is this already released in any specific version?

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.

None yet

3 participants