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

git-clone task: Support fetching tags of submodules #1817

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

andreasgerstmayr
Copy link

Add a new fetchSubmoduleTags param to the git-clone task to fetch tags from all submodules.

The change should be straightforward, but I didn't test it so far. Is there a simple way to build a container for this task, push it to a public registry and flip this line: https://github.com/os-observability/konflux-tempo/blob/8090ec116d33d2a2bb1bcdc3967159f18a258b3f/.tekton/multi-arch-build-pipeline.yaml#L42 to test this change?

Add a new `fetchSubmoduleTags` param to the git-clone task to fetch
tags from all submodules.

Signed-off-by: Andreas Gerstmayr <[email protected]>
@andreasgerstmayr andreasgerstmayr requested a review from a team as a code owner January 14, 2025 14:05
@chmeliik
Copy link
Contributor

chmeliik commented Jan 14, 2025

Is there a simple way to build a container for this task, push it to a public registry and flip this line: https://github.com/os-observability/konflux-tempo/blob/8090ec116d33d2a2bb1bcdc3967159f18a258b3f/.tekton/multi-arch-build-pipeline.yaml#L42 to test this change?

tkn bundle push quay.io/<your-namespace>/tekton-catalog/task-git-clone:some-tag -f task/git-clone/0.1/git-clone.yaml

Then use quay.io/<your-namespace>/tekton-catalog/task-git-clone:some-tag in your pipeline

tkn: https://tekton.dev/docs/cli/

@chmeliik
Copy link
Contributor

chmeliik commented Jan 14, 2025

tkn bundle push quay.io/<your-namespace>/tekton-catalog/task-git-clone:some-tag -f task/git-clone/0.1/git-clone.yaml

Then use quay.io/<your-namespace>/tekton-catalog/task-git-clone:some-tag in your pipeline

tkn: https://tekton.dev/docs/cli/

Oh, and you'll have to go to https://quay.io/\<your-namespace>/tekton-catalog/task-git-clone and make the repository public

@andreasgerstmayr
Copy link
Author

Thank you for the instructions! I tested it successfully.

Now we can set fetchSubmoduleTags=true and depth=10, and git describe --tags --abbrev=0 in a Dockerfile returns the correct version of a component whose sources are in a submodule.

Fun fact, I tried to run git describe without my changes and suddenly it also worked when using depth=10. That was quite unexpected, and after a lot of debugging I figured out that cachi2 already fetches the tags (afaics here: https://github.com/containerbuildsystem/cachi2/blob/7557f2c6ab01523de5a544e2ab34e4134b976069/cachi2/core/package_managers/gomod.py#L1255). But that's a side effect of using the prefetch-dependencies task, imho it's much cleaner to explicitly configure the git-clone task to fetch the tags.

@chmeliik
Copy link
Contributor

chmeliik commented Jan 15, 2025

Fun fact, I tried to run git describe without my changes and suddenly it also worked when using depth=10. That was quite unexpected, and after a lot of debugging I figured out that cachi2 already fetches the tags (afaics here: https://github.com/containerbuildsystem/cachi2/blob/7557f2c6ab01523de5a544e2ab34e4134b976069/cachi2/core/package_managers/gomod.py#L1255). But that's a side effect of using the prefetch-dependencies task, imho it's much cleaner to explicitly configure the git-clone task to fetch the tags.

Hmm, interesting. I think cachi2 does more or less an equivalent of git fetch --tags, with no explicit submodule handling. Maybe git fetch is already submodule-aware to some extent?

@andreasgerstmayr
Copy link
Author

Fun fact, I tried to run git describe without my changes and suddenly it also worked when using depth=10. That was quite unexpected, and after a lot of debugging I figured out that cachi2 already fetches the tags (afaics here: https://github.com/containerbuildsystem/cachi2/blob/7557f2c6ab01523de5a544e2ab34e4134b976069/cachi2/core/package_managers/gomod.py#L1255). But that's a side effect of using the prefetch-dependencies task, imho it's much cleaner to explicitly configure the git-clone task to fetch the tags.

Hmm, interesting. I think cachi2 does more or less an equivalent of git fetch --tags, with no explicit submodule handling. Maybe git fetch is already submodule-aware to some extent?

I think cachi2 simply runs this command in the subfolder. This is the config for the prefetch-dependencies task: [{"type": "gomod", "path": "./tempo"}, {"type": "rpm"}].

When run outside the folder, git fetch --tags doesn't recursively fetch tags of subfolders (unless you have submodule.recurse enabled in the git config).

@chmeliik
Copy link
Contributor

I think cachi2 simply runs this command in the subfolder. This is the config for the prefetch-dependencies task: [{"type": "gomod", "path": "./tempo"}, {"type": "rpm"}].

When run outside the folder, git fetch --tags doesn't recursively fetch tags of subfolders (unless you have submodule.recurse enabled in the git config).

I believe cachi2 runs git fetch in the repo root. But I'm fine with that staying a mystery, this change seems reasonable 😄

@chmeliik
Copy link
Contributor

/ok-to-test

@chmeliik
Copy link
Contributor

/ok-to-test

@chmeliik chmeliik enabled auto-merge January 15, 2025 12:08
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.

3 participants