Kintyre / ksconf

Kintyre's Splunk Configuration tool
Apache License 2.0
50 stars 13 forks source link

successful ksconf-sort pre-commit hook stops the commit and reported as "failed" #71

Closed vaniovanio closed 3 years ago

vaniovanio commented 4 years ago

The problem

I am using GIT + TortoiseGit client in Windows and was very happy to find tools like KSCONF and pre-commit. I am using GIT+TortoiseGit in my team to develop some apps and we quickly realised that Splunk always puts the updated stanzas at the end of the file, which creates merge issues later...

Anyway, I don't know if this is intended or not, but after I've successfully deployed the KSCONF pre-commit hooks to my repo, I noticed that when ksconf-sort is run during a commit, the operation is reported as "Failed" and it also stops the "commit".

When I check the content of the files in the commit they are sorted OK, and to me the operation was successfull. What am I doing wrong?

Environment

Details

How it looks in TortoiseGit: image

How it looks in Git bash: image

lowell80 commented 4 years ago

I'm glad you found ksconf and thanks for reaching out and sharing your use case. It's always helpful to hear how others are using the tool. I'd also be interested in hearing how you found ksconf.

Regarding this specific issue, I think there's 2 things going on here. It's normal behavior for pre-commit to mark something as "failed" in two scenarios:

  1. If any file was updated by a pre-commit hook (which is frequently the case with the ksconf-sort, if the file needs sorting) then current commit action has to be aborted. (To be clear, this is the way git hooks work, not something ksconf or pre-commit can control.) The reason for this is so that the user has the opportunity to review and presumably approve the changes explicitly by doing a git add or git commit -a. So this part is normal. You can avoid this by running pre-commit run manually on the repo before you do a git commit, but that's not ideal if you're primarily using a GUI frontend to git, like TortoiseGit. So for UI tools like this, practically, you may just have to commit twice.
  2. If the file pre-commit hook finds unresolvable issues, which appears to the case here, then re-running pre-commit will continue to give errors. The Stanaza [...] found more than once seems to be the core issue here.
lowell80 commented 4 years ago

I just saw your update (git bash output). Can you confirm that you don't have duplicate spa_010301101_infoblox_search stanzas?

So git/pre-commit/ksconf should all behave the same way from both TortiseGit and the git bash interface, though admittedly I haven't tried using these with TortoiseGit.

vaniovanio commented 4 years ago

Sorry for the confusing screenshot. My issue wasn't with the duplicate - that was understood. I corrected that and I expected it to fail. What Just ignore the duplicate error, like it never existed in the first place... I am comparing how the hook behaves in TortoiseGit and how it behaves with regular Git bash. The difference is that in the latter, even if the ksconf-sort reports "failed" the commit completes just fine. But performing the commit from the TortoiseGit client requires me to do a second commit, because the first one fails in the GUI (like seen in the screenshot). Probably something to do with TortoiseGit.

vaniovanio commented 4 years ago

...and actually while it is "failing" in the GUI, I see that after all the repository is updated... So definitely this has something to do with how TortoiseGit handles hooks exit codes or something like that...

lowell80 commented 4 years ago

Interesting. If you can track down where the problem is and find that ksconf is handling something incorrectly and know what it should be doing instead then I can attempt to address it.

I haven't talked to other users using ksconf pre-commit hooks with TortoiseGit, so I can't tell if this is something specific to your setup, or if it always happens with TortoiseGit. I'm happy to accept pull requests if you can track down the issue.

A few things that you may want to check:

vaniovanio commented 4 years ago

I think the issue is with TortoiseGit, because using only the Git commands from CLI is working fine, and while TortoiseGit says "git didn't exit cleanly" during commit, I can see that the commit actually did go successfully in the file system.

I could take alook into this "end-of-file-fixer" but I am not sure where to find it/begin with.

vaniovanio commented 4 years ago

Actually, if the ksconf-sort script only sorts the files, why does it report as "failed" during commit in GIT?

lowell80 commented 4 years ago

Were you able to get to the bottom of the compatibility issue?

lmnogues commented 3 years ago

Actually, if the ksconf-sort script only sorts the files, why does it report as "failed" during commit in GIT?

This is not a ksconf "failed" stuff, it's pre-commit, if pre-commit detect a changes in the files that are commited, it's automatically fails for the user to review the changes made automatically