thathoff / kirby-git-content

Commit and Push changes made via the Panel
MIT License
136 stars 20 forks source link

Catch Error when working tree is clean #84

Closed graphichavoc closed 2 years ago

graphichavoc commented 3 years ago

Been using kirby-git-content successfully on a bunch of sites without problems (thanks a lot for creating this useful plugin! 🙏 )

But on this one I set up git lfs for larger files and I get git error messages like the following when reordering pages in the panel (Other operations seem to be fine). The reordered content is committed and gets pushed, but the error leaves me with an uneasy feeling and is distracting for my panel users (I could of course turn of error display, but then I wouldn't be able to diagnose if real issues should arise).

Screenshot 2021-10-06 at 21 03 35

Kirby config.php extract

'thathoff' => [
    'git-content' => [
      // use the main git repo at the root of the project
      // (instead of the default, a separate git repo for "content")
      'path' => realpath(__DIR__ . '/../../'),
      // silent git errors on production systems
      // so normal users aren’t confused by cryptic messages
      'displayErrors' => true,
      // Push to GitHub after every change. NOTE: makes things a little slower
      // but is easier to maintain for us than a separate cron job
      'push'=> true,
    ],

.gitattributes

content/**/*.png filter=lfs diff=lfs merge=lfs -text
content/**/*.jpg filter=lfs diff=lfs merge=lfs -text
content/**/*.pdf filter=lfs diff=lfs merge=lfs -text
content/**/*.tif filter=lfs diff=lfs merge=lfs -text
content/**/*.tiff filter=lfs diff=lfs merge=lfs -text
content/**/*.jpeg filter=lfs diff=lfs merge=lfs -text
content/**/*.psd filter=lfs diff=lfs merge=lfs -text
content/**/*.mp4 filter=lfs diff=lfs merge=lfs -text
content/**/*.mp3 filter=lfs diff=lfs merge=lfs -text
content/**/*.mov filter=lfs diff=lfs merge=lfs -text
content/**/*.m4a filter=lfs diff=lfs merge=lfs -text
content/**/*.m4p filter=lfs diff=lfs merge=lfs -text
content/**/*.ogg filter=lfs diff=lfs merge=lfs -text
thathoff commented 2 years ago

The problem is that we sometimes run into a race condition where Kirby triggers an update/sorted hook when things have not changed or have already been committed. Thats one of the reasons why we disable errors by default because most of the time errors are no real problems (like this one here). I think this is not a special git lfs problem but commiting with lfs is sometimes a bit slower so it’s more likely to trigger this race condition.

Maybe we should run a git status before committing and thus catching the error but it would trigger a git status every commit which can get slow when running with large repos. I think the better solution is to simply catch this specific „nothing to commit” error.