Improve submodule loading

[HeavyWombat]

Changes

Add --depth flag to be able to configure the shallow depth setting.

Add depth setting to Git submodule command to improve loading times.

Fixes #785

Submitter Checklist

• Includes tests if functionality changed/was added
• Includes docs if changes are user-facing
• Set a kind label on this PR
• Release notes block has been filled in, or marked NONE

See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.

Release Notes

Fixed issue in source retrieval code that led to slow loading times compared to the previous implementation.

[gabemontero]

@HeavyWombat should we not expose a way to manipulate the setting of the new arg from https://github.com/shipwright-io/build/blob/main/pkg/reconciler/buildrun/resources/sources/git.go#L36-L43

otherwise why make it and arg at all and just hard code it

or do you plan to do that in a separate PR

[HeavyWombat]

@HeavyWombat should we not expose a way to manipulate the setting of the new arg from https://github.com/shipwright-io/build/blob/main/pkg/reconciler/buildrun/resources/sources/git.go#L36-L43

otherwise why make it and arg at all and just hard code it

or do you plan to do that in a separate PR

@gabemontero Good question. I added the depth flag mostly for two reasons, one being that it was like that in the Tekton Git resource and the other one that we would be able to configure it in case we run into issues with the default value.

@SaschaSchwarze0 What is your take on this? Do you want the --depth 1 in the step configuration right away or should we tackle that with another PR?

[SaschaSchwarze0]

@SaschaSchwarze0 What is your take on this? Do you want the --depth 1 in the step configuration right away or should we tackle that with another PR?

To the best I can tell is that setting this to a different value makes sense when you want to access the history of the repository for whatever reason, for example to run git log to list commits. I can understand that use case in the context of Tekton where one can run generic pipelines that might have that use case (like one that builds the commit history of a release). In the context of Shipwright and the focus on just the image building aspect, I have not come across a use case.

So, the flag on the command does not hurt. Before exposing it as a flag on a Git source item under spec.sources or the currently existing Git-specific spec.source, I would like to see a use case first.

[gabemontero]

@SaschaSchwarze0 What is your take on this? Do you want the --depth 1 in the step configuration right away or should we tackle that with another PR?

To the best I can tell is that setting this to a different value makes sense when you want to access the history of the repository for whatever reason, for example to run git log to list commits. I can understand that use case in the context of Tekton where one can run generic pipelines that might have that use case (like one that builds the commit history of a release). In the context of Shipwright and the focus on just the image building aspect, I have not come across a use case.

So, the flag on the command does not hurt. Before exposing it as a flag on a Git source item under spec.sources or the currently existing Git-specific spec.source, I would like to see a use case first.

I'm good with waiting, as long as we put a comment somewhere near the flag summarizing what @SaschaSchwarze0 spelled out.

/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gabemontero

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 [gabemontero]

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

[HeavyWombat]

@SaschaSchwarze0 What is your take on this? Do you want the --depth 1 in the step configuration right away or should we tackle that with another PR?

To the best I can tell is that setting this to a different value makes sense when you want to access the history of the repository for whatever reason, for example to run git log to list commits. I can understand that use case in the context of Tekton where one can run generic pipelines that might have that use case (like one that builds the commit history of a release). In the context of Shipwright and the focus on just the image building aspect, I have not come across a use case.
So, the flag on the command does not hurt. Before exposing it as a flag on a Git source item under spec.sources or the currently existing Git-specific spec.source, I would like to see a use case first.

I'm good with waiting, as long as we put a comment somewhere near the flag summarizing what @SaschaSchwarze0 spelled out.

/approve

@gabemontero @SaschaSchwarze0 Thanks for the feedback. I put in two comment sections explaining the different types of flags and that the --depth is optional and probably fine for all our use cases.

[SaschaSchwarze0]

/lgtm