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: create/move refs for main #199

Closed
wants to merge 1 commit into from
Closed

fix: create/move refs for main #199

wants to merge 1 commit into from

Conversation

retr0h
Copy link
Owner

@retr0h retr0h commented May 7, 2024

Corrects the issue where Gilt does not overlay when the target repo has been updated.

Fixes: #190

Corrects the issue where Gilt does not overlay when the target repo has
been updated.

Fixes: #190
@retr0h
Copy link
Owner Author

retr0h commented May 7, 2024

@0xDEC0DE this looks to do the correct thing based on your steps.

RunCmdInDir("git", []string{"fetch", "--tags", "--force"}, suite.cloneDir).
RunCmdInDir("git", []string{"fetch", "--tags", "--force", "origin", "+refs/heads/*:refs/heads/*"}, suite.cloneDir).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

giphy

This will work, right up until somebody has a ~/.gitconfig that reconfigures clone.defaultRemoteName to something else. Which might never, ever, happen, but if it does, hilarity will ensue.

How it should work, to be 100% surprise-free, is the Clone method should call git clone --origin $SOME_GILT_VALUE, and git fetch should use the same origin name. (I suggest gilt)

Of course, this has knock-on effects, since an existing cached clone without the "special" name should be invalidated and re-cloned. Which means more code, and more tests.

It's fairly straightforward, but it WILL require a small refactor of the Git workflow, since "yes, it exists" is no longer necessarily a viable cache hit. I'll endeavor to get you something in the next few days.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of this would become moot if/when #72 is implemented.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either moot, or requiring explicit implementation anyhow, depending on how the library manages default values, pays attention to config files, etc.

@0xDEC0DE
Copy link
Contributor

This is superseded by #201

@retr0h retr0h closed this May 13, 2024
@retr0h retr0h removed the next label May 13, 2024
@github-actions github-actions bot deleted the issue/190 branch May 13, 2024 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not update cache
2 participants