use IsClean to detect if changes need committing

[jack-evans]

Signed-off-by: Jack Evans j.evans.1310@gmail.com

related to fluxcd/flux2#3467. Changed will only be false if options.Files is nil or an empty map, this uses the IsClean function on the status to determine if there are actually any changes to the files.

I build this locally into the flux2 CLI...

first run was to set the flux version to v0.0.0-dev:

../../../github.com/jack-evans/flux2/bin/flux bootstrap github ....
► connecting to xxxxxx
► cloning branch "jacktest" from Git repository "xxxxxxxx"
✔ cloned repository
► generating component manifests
✔ generated component manifests
✔ committed sync manifests to "jacktest" ("07670d54846f96fdb0bf8049af717626152e2c5a")
► pushing component manifests to "xxxxxxxxxxxxxx"

second run correctly didnt push any commits:

../../../github.com/jack-evans/flux2/bin/flux bootstrap github .....
► connecting to xxxxxx
► cloning branch "jacktest" from Git repository "xxxxxx"
✔ cloned repository
► generating component manifests
✔ generated component manifests
hello
✔ component manifests are up to date
✔ reconciled components
► determining if source secret "flux-system/flux-system" exists
✔ source secret up to date
► generating sync manifests
✔ generated sync manifests
hello
✔ sync manifests are up to date
► applying sync manifests
✔ reconciled sync configuration

[jack-evans]

not sure I understand why we do this here as well as after calling g.writeFile in the other for loop but changed as minimal as I could

[pjbgf]

My understanding is that it stages files that were changed at the fs, whether or not they were passed on via options.Files.

Please note that line 309 does the same loop as status.IsClean():

func (s Status) IsClean() bool {
	for _, status := range s {
		if status.Worktree != Unmodified || status.Staging != Unmodified {
			return false
		}
	}

	return true
}

So to avoid going through twice over the statuses, we could just add the if status.Worktree != Unmodified || status.Staging != Unmodified in 310 here. However, as I mentioned on the other comment, this does not seem to be the problem, as by default go-git should not allow "empty commits".

[aryan9600]

@jack-evans thanks for catching this issue and raising this PR, but i'm not sure that this is the fix we need.
upon closer inspection, this is happening because of auto staging files when WithFiles is used introduced in #424. @pjbgf i think we might need to revert that commit. wdyt?

[jack-evans]

No problem! I was pretty sure when I wrote this there would be a better fix as I don't have a lot of knowledge in this area 😊

[pjbgf]

@aryan9600 I am not sure 80ff3d5 is the culprit, as re-staging unmodified files should not yield a new commit specially after go-git/go-git@a513415. It should behave as the CLI:

But given that it wasn't the final fix for fluxcd/image-automation-controller#466, we should be OK to revert it if it fixes the new cli issue.

@jack-evans can you test whether reverting the commit above fixes the issue you are experiencing please?

[aryan9600]

@pjbgf i reverted the commit and after manually testing, i can confirm it works

[jack-evans]

@aryan9600 beat me too it! 🙂 Just tested as well and confirmed the same. I'll close this Pr in favour of the revert!