add sparse checkout to git #3646

a-rothwell · 2020-12-23T21:17:44Z

Changes

Hi,

I have added functionality to support sparse-checkout for git

This works by enabling the feature in the git config and then updating the sparse-checkout file before fetch.

The feature request is here

I have also added a test to check that .git/info/sparse-checkout is updated as expected

Thanks!

/kind feature

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

Includes tests (if functionality changed/added)
Includes docs (if user facing)
Commit messages follow commit message best practices
Release notes block has been filled in or deleted (only if no user facing changes)

See the contribution guide for more details.

Double check this list of stuff that's easy to miss:

If you are adding a new binary/image to the cmd dir, please update
the release Task to build and release this image.

Reviewer Notes

If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.

Release Notes

tekton-robot · 2020-12-23T21:17:59Z

Hi @a-rothwell. Thanks for your PR.

I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

dlorenc · 2020-12-24T14:29:58Z

/ok-to-test

tekton-robot · 2020-12-24T14:32:25Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/git/git.go	55.6%	55.4%	-0.2

tekton-robot · 2020-12-24T21:41:28Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/git/git.go	55.6%	55.4%	-0.2

tekton-robot · 2020-12-24T21:59:58Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/git/git.go	55.6%	55.4%	-0.2

a-rothwell · 2021-01-15T20:38:28Z

/test pull-tekton-pipeline-integration-tests

chmouel · 2021-01-21T20:56:04Z

/cc @sbwsg

ghost · 2021-01-22T00:20:59Z

pkg/git/git.go

+		}
+		file, err := os.OpenFile(filepath.Join(cwd, ".git/info/sparse-checkout"), os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+		if err != nil {
+			logger.Errorf("failed to open sparse-checkout file; err: %v", err)


nit: no need for err in the logged message here. Suggest "failed to open sparse-checkout file: %v". Similar comment for the other messages passed to logger.Errorf in this func.

pkg/git/git.go

chmouel · 2021-01-22T12:27:11Z

cmd/git-init/main.go

@@ -39,6 +39,8 @@ func init() {
 	flag.BoolVar(&fetchSpec.Submodules, "submodules", true, "Initialize and fetch Git submodules")
 	flag.UintVar(&fetchSpec.Depth, "depth", 1, "Perform a shallow clone to this depth")
 	flag.StringVar(&terminationMessagePath, "terminationMessagePath", "/tekton/termination", "Location of file containing termination message")
+	flag.StringVar(&fetchSpec.SparseCheckoutDirectories, "sparseCheckoutDirectories", "", "String of directory patterns separated by a delimiter")
+	flag.StringVar(&fetchSpec.SparseCheckoutDelimiter, "sparseCheckoutDelimiter", ";", "Delimiter used to separate sparse checkout directories")


I don't think we need that level of granularity as a flag

Moreover the delimiter would cause some issues if the user doesn't quote the ";", it will be interepreted as another command.

What about removing the flag and setting the delimiter as a standard comma "," ?

That is true, I understand how semicolons could cause issues for users.
Removing the delimiter flag and using a comma will make it simpler.

chmouel · 2021-01-22T12:32:43Z

pkg/git/git_test.go

+					sparsePatterns = append(sparsePatterns, scanner.Text())
+				}
+
+				if !reflect.DeepEqual(dirPatterns, sparsePatterns) {


what about using cmp.Diff, which is a library used thought-out all the tests in pipeline ? correct me if I am wrong but it does the same thing ?

cmp.Diff will work here. I will make that change!

tekton-robot · 2021-01-25T19:54:05Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/git/git.go	55.6%	55.0%	-0.5

tekton-robot · 2021-01-25T20:11:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sbwsg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sbwsg]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ghost · 2021-01-25T20:13:45Z

@a-rothwell thanks for making those changes. You'll need to squash your commits into 1 prior to merge.

tekton-robot · 2021-01-25T20:24:39Z

The following is the coverage report on the affected files.
Say /test pull-tekton-pipeline-go-coverage to re-run this coverage report

File	Old Coverage	New Coverage	Delta
pkg/git/git.go	55.6%	55.0%	-0.5

chmouel · 2021-01-26T12:06:54Z

/lgtm

Thanks 🙇🏽

chmouel · 2021-01-26T12:09:05Z

it woudl be great if we can have the git-clone task updated to support this feature too 🙏🏽 @a-rothwell

a-rothwell · 2021-01-26T16:28:31Z

Thank you both for helping me with this!
I will update the task in the catalog

tekton-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. labels Dec 23, 2020

tekton-robot requested review from bobcatfish and dibyom December 23, 2020 21:17

tekton-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 23, 2020

tekton-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Dec 23, 2020

tekton-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 24, 2020

tekton-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 24, 2020

tekton-robot requested a review from a user January 21, 2021 20:56

ghost reviewed Jan 22, 2021

View reviewed changes

chmouel suggested changes Jan 22, 2021

View reviewed changes

chmouel reviewed Jan 22, 2021

View reviewed changes

ghost approved these changes Jan 25, 2021

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2021

add sparse checkout

82e6a68

a-rothwell force-pushed the add_sparse_checkout branch from aa42142 to 82e6a68 Compare January 25, 2021 20:22

tekton-robot assigned chmouel Jan 26, 2021

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 26, 2021

tekton-robot merged commit 26d303d into tektoncd:master Jan 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add sparse checkout to git #3646

add sparse checkout to git #3646

a-rothwell commented Dec 23, 2020 •

edited

Loading

tekton-robot commented Dec 23, 2020

dlorenc commented Dec 24, 2020

tekton-robot commented Dec 24, 2020

tekton-robot commented Dec 24, 2020

tekton-robot commented Dec 24, 2020

a-rothwell commented Jan 15, 2021

chmouel commented Jan 21, 2021

ghost Jan 22, 2021

chmouel Jan 22, 2021 •

edited

Loading

a-rothwell Jan 22, 2021

chmouel Jan 22, 2021

a-rothwell Jan 22, 2021

tekton-robot commented Jan 25, 2021

tekton-robot commented Jan 25, 2021

ghost commented Jan 25, 2021

tekton-robot commented Jan 25, 2021

chmouel commented Jan 26, 2021

chmouel commented Jan 26, 2021 •

edited

Loading

a-rothwell commented Jan 26, 2021

add sparse checkout to git #3646

add sparse checkout to git #3646

Conversation

a-rothwell commented Dec 23, 2020 • edited Loading

Changes

Submitter Checklist

Reviewer Notes

Release Notes

tekton-robot commented Dec 23, 2020

dlorenc commented Dec 24, 2020

tekton-robot commented Dec 24, 2020

tekton-robot commented Dec 24, 2020

tekton-robot commented Dec 24, 2020

a-rothwell commented Jan 15, 2021

chmouel commented Jan 21, 2021

ghost Jan 22, 2021

Choose a reason for hiding this comment

chmouel Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

a-rothwell Jan 22, 2021

Choose a reason for hiding this comment

chmouel Jan 22, 2021

Choose a reason for hiding this comment

a-rothwell Jan 22, 2021

Choose a reason for hiding this comment

tekton-robot commented Jan 25, 2021

tekton-robot commented Jan 25, 2021

ghost commented Jan 25, 2021

tekton-robot commented Jan 25, 2021

chmouel commented Jan 26, 2021

chmouel commented Jan 26, 2021 • edited Loading

a-rothwell commented Jan 26, 2021

a-rothwell commented Dec 23, 2020 •

edited

Loading

chmouel Jan 22, 2021 •

edited

Loading

chmouel commented Jan 26, 2021 •

edited

Loading