Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Bugfix checkouting existing branch. Fixes issue #109 #149

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

Open
navrkald wants to merge 3 commits into gruntwork-io:master
base: master
Choose a base branch
Loading
from navrkald:bug/fix-checkouting-existing-branch-109

Conversation

@navrkald
Copy link

@navrkald navrkald commented Feb 14, 2024 •
edited
Loading

Description

Fixes #109.

Original code did equivalent of following:

git clone <some repo>
git checkout -b <branch name>
git merge --ff origin/<branch name>

new code is properly checkouting remote branch if exists

git clone <some repo>
git fetch
# Try to checkout remote branch
git checkout <branch name>
# If above command fails then new branch is created
git checkout -b <branch name>

Problem with original code is not only that's not convenient, but it's as well bugy, it's silently supposing that <branch name> was created on top of default remote branch name plus it's supposing remote branch name haven't changed in meanwhile until remote <branch name> was created. Ofc this conditions are often not met and thats the case when the bug #109 appears.

TODOs

Read the Gruntwork contribution guidelines.

  • Update the docs.
  • Run the relevant tests successfully, including pre-commit checks.
  • Ensure any 3rd party code adheres with our license policy or delete this line if its not applicable.
  • Include release notes. If this PR is backward incompatible, include a migration guide.

Release Notes

Fixed #109 checkout remote branch locally if exists.

Copy link
Author

navrkald commented Apr 18, 2024 •
edited
Loading

Hi @brikis98 , @hongil0316 It's been 2 months since creation of this PR. Can you please review?

Copy link
Author

navrkald commented Dec 2, 2025

@ceschae, @james00012, @ZachGoldberg Can you please have a look on this PR?

// Create new branch
checkoutNewBranchErr := worktree.Checkout(
&git.CheckoutOptions{
Hash: ref.Hash(),
Copy link
Member

@denis256 denis256 Dec 2, 2025

Choose a reason for hiding this comment

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

Create a new branch from the default HEAD, so any existing remote branch history is skipped

Copy link
Author

@navrkald navrkald Dec 2, 2025

Choose a reason for hiding this comment

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

Fist of all thanks @denis256 for review, then please let me describe how new code works.

Code first fetches remote branches, then it tries to checkout existing branch if exist and return from this function on line 204. If checkout of existing branch is not successful then it creates new branch from current HEAD here.

Copy link
Member

denis256 commented Dec 2, 2025

I think PR needs to be updated with changes from master, now it fails on CICD

golang.org/x/tools/cmd/goimports imports
 golang.org/x/tools/internal/imports imports
 golang.org/x/tools/internal/modindex imports
 iter: package iter is not in GOROOT (/usr/local/go/src/iter)
go: downloading golang.org/x/text v0.31.0
go: downloading golang.org/x/net v0.47.0
go: downloading golang.org/x/term v0.37.0
go: downloading github.com/google/go-cmp v0.6.0
go: downloading golang.org/x/crypto v0.44.0
Exited with code exit status 1

I also noticed that there are no tests - I think we need to add such tests, without tests we can't track that checkout of existing branches will continue to work over time
navrkald reacted with thumbs up emoji

Copy link
Member

denis256 commented Dec 2, 2025 •
edited
Loading

I think there is a risk of losing changes from branches:

  • user-1 pushes a branch named new-feature with their recent work.
  • user-2 git-xargs on that same new-feature branch.
  • Instead of using user-1's branch, the tool ignores it and creates a new, empty branch that also happens to be named new-feature.
  • When git-xargs finishes and tries to push, Git will throw an error because user-2 new, empty branch conflicts with your user-1 existing one.

repository/repo-operations.go 207-231

Copy link
Author

navrkald commented Dec 2, 2025 •
edited
Loading

Again thanks for the review, I will try this week to update tests and back-merge latest master to my branch. How code works I have described in conversation: #149 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@denis256 denis256 denis256 left review comments

@brikis98 brikis98 Awaiting requested review from brikis98

@james00012 james00012 Awaiting requested review from james00012

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

git-xargs doesn't update existing PRs

AltStyle によって変換されたページ (->オリジナル) /