[action] git_commit - skip commit if git status is clean for paths #17913
Conversation
Revert the commit ef1ccf1. There was an issue when the git_commit lane was executed alone (meaning without git_add before). In this case, it was not possible to commit because there are no staged files and `git --no-pager diff --name-only --staged` always returns an empty string.
| allow(Fastlane::Actions).to receive(:sh) | ||
| .with("git status ./fastlane/README.md --porcelain") | ||
| .and_return("M ./fastlane/README.md") | ||
| allow(Fastlane::Actions).to receive(:sh) |
There was a problem hiding this comment.
| allow(Fastlane::Actions).to receive(:sh) | |
| expect(Fastlane::Actions).to receive(:sh) |
It feels like this must be called in the given scenario so expect makes sure future change will also behave the same. Though it is checked by below assertion, this would produce a slightly better error message when failing.
There was a problem hiding this comment.
You are right, I made the change
| nothing_staged = Actions.sh("git --no-pager diff --name-only --staged").empty? | ||
| UI.success("Nothing staged to commit ✅.") if nothing_staged | ||
| return if nothing_staged | ||
| repo_clean = Actions.sh("git status #{paths} --porcelain").empty? |
There was a problem hiding this comment.
Can you please add a comment about this behaviour here, and at the description for "allow_nothing_to_commit" param, which "unstaged" changes are taken account and this way allows you to keep the repo dirty without failing?
We missed the behaviour in the last PR so it's definitely worth documenting. Ideally, we should cover it in unit tests but the way we test doesn't allow us to describe the exact situation for it due to sh being mocked or the difficulty to prepare a complete git repo just for a minor action. I can only come up leaving a comment as a way to avoid regressions for now.
There was a problem hiding this comment.
Well done🎉
@joshdholtz will double-check and merge this PR when he can!
There was a problem hiding this comment.
Congratulations! 🎉 This was released as part of fastlane 2.172.0 🚀
Checklist
bundle exec rspecfrom the root directory to see all new and existing tests passbundle exec rubocop -ato ensure the code style is validMotivation and Context
Reverts the commit ef1ccf1 and fix a regression introduced in #17804.
There is an issue when the
git_commitlane is executed alone (meaning withoutgit_addbefore).In this case, it is not possible to run the sh command
git commitbecause there is no staged files andgit --no-pager diff --name-only --stagedalways returns an empty string.Description
The proposed solution is to run
git status #{paths} --porcelainto check if the git status is clean, only for the paths we want to commit. This way, even if there are other files modified, thegit commitcan be skipped.In practice, there are two cases:
fileA is modified (meaning
git status fileA --porcelainwill return a non empty string)1.1 if we call
git_commit(path: "fileA", allow_nothing_to_commit: true)-> the commit is created1.2 if we call
git_commit(path: "fileA", allow_nothing_to_commit: false)-> the commit is createdfileA is not modified (meaning
git status fileA --porcelainwill return an empty string)2.1 we call
git_commit(path: "fileA", allow_nothing_to_commit: true)-> the commit is skipped (even if there are other files modified, i.e.git status --porcelaindoes not return an empty string)2.2 we call
git_commit(path: "fileA", allow_nothing_to_commit: false)-> an error occurs because fileA can't be stagedTesting Steps