Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: force Git fetch during updates #1101

Closed
wants to merge 1 commit into from
Closed

fix: force Git fetch during updates #1101

wants to merge 1 commit into from

Conversation

antoineco
Copy link
Contributor

Fixes #1100

I believe --force is a sensible default in the context of packer updates. In most cases, users don't care about things such as force-pushed tags, and simply want packer to pull whatever Git ref was indicated in their config.

@overhacked
Copy link

Due to the condition:

if plugin.commit or plugin.tag then
update_cmd = fetch_cmd
end

The fetch command also needs to be --force, like:

      fetch = 'fetch --depth 999999 --progress --force',

@antoineco
Copy link
Contributor Author

antoineco commented Dec 10, 2022

@overhacked in what context have you been running into issues with the fetch command?

Edit: I just saw your comment on the original issue. Good point.

@wbthomason
Copy link
Owner

@antoineco @overhacked Thanks, and sorry for the delay here. If we're to merge this, we should add @overhacked's suggested change.

Also, are there cases where --force might not do what we want? I don't know of any (as you say, users typically just want "get the latest" or "get the thing I specified", and afaik --force will do the right thing in these cases), but I don't entirely trust my knowledge of git to be exhaustive enough here.

Ensures packer updates don't fail when upstream force-pushes tags.
@antoineco
Copy link
Contributor Author

@wbthomason absolutely 👍 Done in e1979615..3a4fff64

@antoineco antoineco closed this Dec 16, 2022
@antoineco antoineco deleted the force-fetch branch December 16, 2022 07:45
@overhacked
Copy link

overhacked commented Jul 30, 2023

@wbthomason @antoineco Not sure why this got closed? #1100 is still open; I posted my enthusiastic agreement (#1100 (comment)) with this PR's approach there. I'm closing #1171; it would over-complicate the matter.

@antoineco
Copy link
Contributor Author

antoineco commented Jul 30, 2023

@overhacked it was closed because you disagreed with the approach in the original issue and opened #1171 instead.

I have since deleted my fork and no longer use packer, but by any mean feel free to re-submit.

@overhacked
Copy link

Understood. I was wrong about the approach being harmful. I'll resubmit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sync fails when upstream force pushes Git tags
3 participants