akuity / kargo

Application lifecycle orchestration
https://kargo.akuity.io/
Apache License 2.0
1.39k stars 114 forks source link

fix: allow obtaining diff paths for root commit #2141

Closed hiddeco closed 3 weeks ago

hiddeco commented 3 weeks ago

The previous approach was inherited from when there was a need to be able to obtain the diff paths for a range of commits. With the refactoring in v0.7.0, however, this is no longer a requirement.

Because of this, we can switch to git show to obtain the diff paths for a specific commit. Using --name-only to print the paths, while further supplying --pretty="" to omit any other commit (message) data. Effectively yielding the same output as the previous command, except that it now also works for root commits.

Fixes: #2140

netlify[bot] commented 3 weeks ago

Deploy Preview for docs-kargo-akuity-io ready!

Name Link
Latest commit ea3a8f8709f476c5c3162e00f6aeb07a2a7bac79
Latest deploy log https://app.netlify.com/sites/docs-kargo-akuity-io/deploys/666887652493110009ec2607
Deploy Preview https://deploy-preview-2141.kargo.akuity.io
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

krancour commented 3 weeks ago

time="2024-06-11T16:21:54Z" level=error msg="Reconciler error" Warehouse="{test demo}" controller=warehouse controllerGroup=kargo.akuity.io controllerKind=Warehouse error="error discovering artifacts: error discovering commits: error listing commits from git repo \"https://github.com/krancour/kargo-demo-gitops-2.git\": error getting diff paths for commit \"5e48a597589d86abb2e46db8753468209ef33528\" in git repo \"https://github.com/krancour/kargo-demo-gitops-2.git\": error getting diff paths for commit \"5e48a597589d86abb2e46db8753468209ef33528\": error executing cmd [/usr/bin/git show --pretty=\"\" --name-only 5e48a597589d86abb2e46db8753468209ef33528]: fatal: invalid --pretty format: \"\"\n" name=test namespace=demo reconcileID="\"5a57ce2e-cbbd-43fc-acab-721b858a0d46\""

krancour commented 3 weeks ago

The git binary on the image is too old. It doesn't have the --pretty option on git show

krancour commented 3 weeks ago

https://github.com/akuity/kargo-render/pull/289

hiddeco commented 3 weeks ago

Think the error is not related to this, but to a newline which somehow gets injected into the empty format string. (Updating Alpine is nonetheless a great idea.)

Give me a bit to figure out what the culprit is.

krancour commented 3 weeks ago

The git binary on the image is too old. It doesn't have the --pretty option on git show

I stand corrected on this matter. I tried it again and it's fine. idk what I was looking at before. 🤷‍♂️

codecov[bot] commented 3 weeks ago

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 45.93%. Comparing base (e2a8e5f) to head (ea3a8f8).

Files Patch % Lines
internal/controller/git/git.go 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2141 +/- ## ======================================= Coverage 45.93% 45.93% ======================================= Files 238 238 Lines 16542 16542 ======================================= Hits 7599 7599 Misses 8571 8571 Partials 372 372 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

krancour commented 3 weeks ago

Can confirm https://github.com/akuity/kargo/pull/2141/commits/ea3a8f8709f476c5c3162e00f6aeb07a2a7bac79 does the trick.

akuitybot commented 3 weeks ago

Successfully created backport PR for release-0.7: