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

tekton pipeline git resource should init and update submodules recursively. #1523

Closed
NicolasRouquette opened this issue Nov 4, 2019 · 5 comments · Fixed by #1531
Closed
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.

Comments

@NicolasRouquette
Copy link

Expected Behavior

Suppose we have a git pipeline resource as described in the doc:
https://github.com/tektoncd/pipeline/blob/master/docs/resources.md#git-resource

apiVersion: tekton.dev/v1alpha1
kind: PipelineResource
metadata:
  name: wizzbang-git
  namespace: default
spec:
  type: git
  params:
    - name: url
      value: https://github.com/wizzbangcorp/wizzbang.git
    - name: revision
      value: master

Suppose that https://github.com/wizzbangcorp/wizzbang.git has .gitmodules with one or more git submodules.

When the git resource is cloned, the behavior should be equivalent to this:

git init <dir>
cd <dir>
git remote add origin <remote url>
git fetch --depth=1 --recurse-submodules=yes origin <revision>
git submodule init
git submodule update --recursive

where <remote url> would be https://github.com/wizzbangcorp/wizzbang.git

Actual Behavior

Based on the latest commit of pkg/git/git.go here:

func Fetch(logger *zap.SugaredLogger, revision, path, url string) error {

This is equivalent to the following:

git init <dir>
cd <dir>
git remote add origin <remote url>
git fetch --depth=1 --recurse-submodules=yes origin <revision>

In other words, the submodules have not been initialized nor updated recursively.

Steps to Reproduce the Problem

  1. Execute a pipeline or task run that depends on submodule resources.

Additional Info

NicolasRouquette added a commit to NicolasRouquette/pipeline that referenced this issue Nov 4, 2019
@NicolasRouquette
Copy link
Author

I was debating whether to submit a PR for a simple fix to this.

There are two ways to address this issue:

  1. Change the behavior of the git resource such that it always performs git submodule update --init --recursive

  2. Add to the documentation that, for submodules, one needs to add a task step somewhere to do git submodule update --init --recursive

There are pros/cons for either option:

pro: it is simpler for the few cases where someone needs to clone submodules
cons: it adds a bit of overhead that is wasteful when a repository does not use submodules
cons: perhaps someone wants to be in charge of handling their submodules

pro: the documentation provides useful guidance where it is applicable
pro: the documentation does not impose a particular strategy for handling submodules
cons: it is something extra to do where submodules are needed

Personally, I prefer (2).

@vdemeester vdemeester added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. labels Nov 5, 2019
@chengjingtao
Copy link

I think a better logic is to first determine if submodule exists.
We can make a judgement according output of git submodule status.
@vdemeester do you have any opinions ?

@vdemeester
Copy link
Member

@chengjingtao I thought of that initially, that said, those commands are fully no-op if there is no submodules so, running them almost doesn't cost anything 👼

I needed for a task (that uses the git-init) so I opened a PR #1531 😅

@chengjingtao
Copy link

@chengjingtao I thought of that initially, that said, those commands are fully no-op if there is no submodules so, running them almost doesn't cost anything 👼

I needed for a task (that uses the git-init) so I opened a PR #1531 😅

Wow, I understand what you mean. It really makes sense
It seems I lost a chance to make a contribution to this project. 😆😅 Your PR is great.
I am trying to find some good-first-issue to resolve it.

@vdemeester
Copy link
Member

@chengjingtao sorry about that 😅 There is plenty (and if not enough, we need to triage them to add more good first issue 👼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants