aormsby / Fork-Sync-With-Upstream-action

An action to automatically update your fork with new commits from the upstream repo
MIT License
258 stars 70 forks source link

`target_branch_push_args` is actually `target_push_args` #46

Closed endocrimes closed 2 years ago

endocrimes commented 2 years ago

Currently it's documented in the README and Action metadata that target_branch_push_args is the correct field for providing args to git push. However, in the code INPUT_TARGET_PUSH_ARGS, which correlates to target_push_args is used.

Happy to open a PR to either update the documentation or the bash script to match but it's not clear to me which you'd prefer to do. Although my preference would weigh towards updating the documentation - rather than breaking existing usage.

aormsby commented 2 years ago

Ah hmm, I had meant to update the variable to include 'branch' for clarity. I'll have to think about it.

kousu commented 2 years ago

Ah I just noticed this too. Glad to see there's already an issue about it. :)

My Use Case

I'm trying to keep 'main' synced with upstream, but because Actions cronjobs only run on 'main' I need to add this Action to it. Now the default of --ff-only fails, because the branches have diverged. The docs mention this as a common problem:

For example - you forked a repo, and then you added this action to the default branch of the repo to keep it synced with the matching upstream branch. Now your target and upstream branches have divergent histories! The sync fails with a git error.

fatal: refusing to merge unrelated histories

This will probably be a very common scenario. 🤷‍♂️

My Attempted Solution

I tried to solve this with pull --rebase

        upstream_pull_args: '--rebase --tags'

but of course using rebase implies using push --force as well:

        target_branch_push_args: '--force'

https://github.com/neuropoly/gitea/blob/3435ec102d15079da05e67f97cc8f41a8bad0579/.github/workflows/sync-upstream.yml#L22-L29

However this fails:

hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.
ERROR:  exit 1

A working solution

Thanks to this thread, I tried ignoring the docs and just using https://github.com/neuropoly/gitea/blob/ded2781777f9ec7b32034acfbdf7b5645111bc32/.github/workflows/sync-upstream.yml#L29

        target_push_args: '--force'

This gives a warning

Warning: Unexpected input(s) 'target_push_args', valid inputs are ['target_sync_branch', 'target_repo_token', 'upstream_repo_access_token', 'upstream_sync_repo', 'upstream_sync_branch', 'target_branch_checkout_args', 'git_log_format_args', 'upstream_pull_args', 'target_branch_push_args', 'git_config_user', 'git_config_email', 'git_config_pull_rebase', 'test_mode', 'host_domain', 'shallow_since']

but it succeeds:

 Pushing synced data to target branch.
To https://github.com/neuropoly/gitea
 + ded278177...b24a462b0 main -> main (forced update)
SUCCESS

So my vote, if you'll take it under consideration, is to just update the docs.

Another solution

Since "syncing with upstream master" seems like such a common situation, it would be nice to brainstorm a standard way to handle this? Maybe that's worth raising in a separate issue.

aormsby commented 2 years ago

My apologies for letting this sit. This project has been on the backburner for a little while. I'll just update the docs to make life easier for y'all.

aormsby commented 2 years ago

Sorry, lied a little bit. I'll update the code. No matter which I update, the fix needs a minor release. Even if I go the docs route, I have to change the yaml input var, and I'd want to make sure the updated entry point var gets attention. Then the code route... well, that needs a release anyway. So I'm going to go wit the updated names and fix the code to match. Maybe that's crappy reasoning, but either way gets the same result, so I'm not worried.

aormsby commented 2 years ago

Please test and confirm fix for me on aormsby/Fork-Sync-With-Upstream-action@fix-var-name

aormsby commented 2 years ago

Pre-released to v3.3. Closing for now. If there are any issues, we can re-open.