Cannot --amend --no-edit

[OJFord]

I'm rendering markdown in an Action on a cron, and want it to just amend the last commit rather than create a continuous stream of 'render' commits that don't have any value.

Unfortunately though (I've tested this), this prevents --amend --no-edit from working:

git-auto-commit-action/entrypoint.sh

Lines 86 to 88 in 5dd17c3

	commit -m "$INPUT_COMMIT_MESSAGE" \
	--author="$INPUT_COMMIT_AUTHOR" \
	${INPUT_COMMIT_OPTIONS:+"${INPUT_COMMIT_OPTIONS_ARRAY[@]}"};

since the --no-edit is effectively ignored if a --message is specified.

I'm not sure what to suggest as a fix, but two options I can think of are:

1. Check if --no-edit appears in commit_options; do not specify -m .. if it does;
2. Add an explicit with: amend: true config, which adds --amend --no-edit to options and doesn't specify -m ..

[stefanzweifel]

Thanks for reporting.
Seems you have found an edge-case for git-auto-commit. The --amend docs mention that -m -F or -c has to be missing in order to reuse the previous message. 🤔

Am I understanding this right that you only set the commit_options in your workflow? Like this?

- uses: stefanzweifel/git-auto-commit-action@v4
  with:
    commit_options: '--amend --no-edit'

Or do you also use the --force push option?
(Just wondering how this could even work if you're just ammending something to a commit which has already been pushed to a remote repository)

---

Checking for certain values in commit_options feels a bit dirty. I would rather like to keep things simple.
To you think it would work, if you extract the previous commit message and supply it to the Action? (See code below)

- name: Get last commit message
  id: last-commit-message
  run: |
    echo "::set-output name=msg::$(git log -1 --pretty=%B | cat)"

- uses: stefanzweifel/git-auto-commit-action@v4
  with:
    commit_message: ${{ steps.last-commit-message.outputs.msg }}
    commit_options: '--amend --no-edit'

Or does --ammend --no-edit not work at all with --message?

[OJFord]

Am I understanding this right that you only set the commit_options in your workflow? Like this?

Yes, at least that's what I tried.

Or do you also use the --force push option?

... 🤦🏻 I should, thanks, just hadn't had it work yet so hadn't realised my omission.

To you think it would work, if you extract the previous commit message and supply it to the Action? (See code below)

That's exactly what I've done (I don't think the cat's necessary?) to work around it, yep. Just a bit messy/annoying - it also needs (at least, when runs-on: ubuntu-latest it does) an additional step to install git.

[stefanzweifel]

I can understand the frustration.

However, I've kept the Action low level by design.
Adding if-clauses that detect if --ammend is in the argument list and then changing the executed code doesn't sit well with me right now. Hope you understand.

it also needs (at least, when runs-on: ubuntu-latest it does) an additional step to install git.

This seems weird. According to the virtual-environments repo, ubuntu-latest (Ubuntu 20.04) ships with git. (Would also be surprising to me if it didn't. We're talking about GitHub after all ☺️)

---

I tried to test the approach with providing the commit_message as I mentioned above, but the global Actions outage today blocks me from actually running this.

If this works as expected I'more than happy to add it as an example to the README.

[stefanzweifel]

Actions is working again and I did a little test in a test-repo.

The approach of defining the message in combination with --amend --no-edit and --force seems to work.

    - name: Get last commit message
      id: last-commit-message
      run: |
        echo "::set-output name=msg::$(git log -1 --pretty=%B)"

    - uses: stefanzweifel/git-auto-commit-action@v4
      id: commit
      with:
        commit_message: ${{ steps.last-commit-message.outputs.msg }}
        commit_options: '--amend --no-edit'
        push_options: '--force'

But using the Action in that way vaporized my previous git history - as is expected when using --amend in combination with already published commits.
https://github.com/stefanzweifel/git-auto-commit-action-demo-app/commits/master

You should understand the implications of rewriting history if you amend a commit that has already been published. (See the "RECOVERING FROM UPSTREAM REBASE" section in git-rebase[1].)
https://git-scm.com/docs/git-commit#Documentation/git-commit.txt---amend

So I wouldn't recommend doing that. Just let the Action generate a new commit.
Or I would encourage to write your own git-commit and push logic to solve the problem in your project.

[OJFord]

As I mentioned, my use case is (re-)rendering a template on a schedule; there's no value to the rendering changes, the important commits are the template changes. So at first I had it amend when the latest commit was also a CI render, but otherwise create one. Then I realised, there's no point even in that, semantically, the rendering should change at the same time as the template, and if it also renders differently at a later time (without other template change), that's still semantically part of the same commit, so, now I have it always amend.

I'm not sure what you mean by 'vaporized my previous git history', that seems quite an exaggeration? Just the latest commit would be updated with whatever diff, and the committer date & user (but not author date & user) would update.

To me it seems unexpected that commit_options has certain options that are essentially silently ignored/overridden; -m/--message is already special-cased, so it seems like amend: [bool, default false] could be too? Understand if you don't want to, would be good to document at least though.

Cheers.

[stefanzweifel]

I'm not sure what you mean by 'vaporized my previous git history', that seems quite an exaggeration? Just the latest commit would be updated with whatever diff, and the committer date & user (but not author date & user) would update.

I should have been more specific. I've been using that test repository since the beginning to test the Action. Had something like 150 commits. After running my test workflow with --amend --no-edit and --force the history collapsed to 1 single commit.

I reran the Workflow this morning to reproduce this behaviour: https://github.com/stefanzweifel/git-auto-commit-action-demo-app/runs/2627309671?check_suite_focus=true#step:6:78
Now the project is back to only have a 1 single commit.

I'm not sure if this is just a mis-configuration on my side or if this is the intended result of using --amend --no-edit. (I'm by no means an expert in git. This Action is basically just a fancy wrapper around 3-4 git commands which solves a common problem for me)

Just as I was writing this response I've browsed through your profile and landed on https://github.com/OJFord/OJFord/blob/master/.github/workflows/render.yml.
The repo also just has a single commit, so I assume this is the expected behaviour.

---

For now, I will document this behaviour in the README, so that others don't stumble over this edge case.
Might consider adding an option for amend (or possible other arguments that stand in conflict with -m) in a future version.

[OJFord]

No, --amend edits the 'current' commit, i.e. wherever HEAD points.

You're right about my own repo though, that was not the case when I last checked it. (Also something up with the message adding quotes by the look of it.)

What must be happening here is HEAD pointing at the 'root' commit (a sort of dummy commit that doesn't really exist, at the beginning of time, useful in rebase -i --root for example to rebase an entire history). This is not expected behaviour.

If you had 150 commits in sequence, you had the 150th checked out, commit --amend and push --force should result in 149 unchanged commits (and so not pushed, literally the same, same SHA) and a modified 150th.

It also shows that it doesn't work with multiline commit messages - even with my attempt to quote around the output (which just duplicated each time, and didn't quote the multiline successfully) - so I'm trying with xxd (and then xxd -r to reverse it in arg to this action)... Maybe I'm just using set-output wrong, I really struggle with Action's docs... Ah, yep, not possible, silently truncates multiline strings 🙄 https://trstringer.com/github-actions-multiline-strings/

[OJFord]

Oh, I see what's happening with the 'single commit' - it's because of a shallow fetch (--depth=1) by default both here and in GH's checkout action. So also need to use the latter, tell it to do a full (depth=2 might be sufficient? I'd have to check that) clone, and then tell this action to skip fetch, otherwise its own --depth=1 would enshallow it to that depth...

🤞🏻

      - run: sudo apt update && sudo apt -y install git

      - uses: actions/checkout@master
        with:
          # so we can commit --amend and push -f
          fetch-depth: 2

# [ cause the diff ]

      - name: Reuse message
        run: |
          # Actions do not support multiline `::set-output`s
          echo 'COMMIT_MSG<<EOM' >> "$GITHUB_ENV"
          git log -n1 --pretty='%B' >> "$GITHUB_ENV"
          echo 'EOM' >> "$GITHUB_ENV"
         
      - uses: stefanzweifel/git-auto-commit-action@v4
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        with:
          # --amend --no-edit doesn't work because of default --message
          # (effectively --no-edit is ignored)
          commit_message: ${{ env.COMMIT_MSG }}
          commit_options: --amend --no-edit
          push_options: --force
          # so as not to make too shallow for commit --amend and push -f
          skip_fetch: true

Worked on push, just waiting for the scheduled run to check that works too. It's quite a lot of setup just to avoid a default --message though.

Might consider adding an option for amend (or possible other arguments that stand in conflict with -m) in a future version.

Another option, if it's possible, I'm not sure, might be if you could distinguish a null from an argument not provided at all? I've never written an Action, so I'm not sure how it works. As in:

with:
  commit_message: null

if it can be determined apart from with: {}, could mean 'do not set -m'.

[OJFord]

Just a note that the default skip_dirty_check: false could also potentially be unexpected here / gets in the way of testing it - and should perhaps really be redundant.

By default commit will not allow empty, but will with --allow-empty. So really skip_dirty_check: true should just be commit_options: --allow-empty; as it is, both are required if you want an empty commit.

That is:

• commit_options: --allow-empty will not work without skip_dirty_check: true
• commit_options: --amend will not work if there's no diff without skip_dirty_check: true
• skip_dirty_check: true doesn't achieve anything without also using one of the above commit_options

I would suggest removing the dirty check, and just leaving it to git-commit, and its behaviour according to user-supplied options.

[stefanzweifel]

@OJFord Thanks for all the research and comments. Especially the find that the actions/checkout and fetch-depth plays a crucial part here. (Which makes total sense when you think about it; I totally forgot about this).

The skip_dirty_check option was added in #84 in reaction to #82. Someone had a similar issue with using --amend – like you did.

My decision to add a skip_dirty_check option was in hindsight not the best option. (As someone who uses git-commit ususally through a GUI, I probably wasn't aware of the --allow-empty options back then.)

I consider removing this in v5.

🙌