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

gogit: Add WithSingleBranch #433

Merged
merged 1 commit into from
Dec 16, 2022
Merged

gogit: Add WithSingleBranch #433

merged 1 commit into from
Dec 16, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Dec 14, 2022

At present go-git does not support the MULTI_ACK capability, which means that follow-up fetches on a given remote will fail.

To support Image Automation Controller use cases, the SwitchBranch was initially short-circuited to avoid additional fetches. However, this has the side effect of the controller pushing the same change to the target repository multiple times. (fluxcd/flux2#3384)

In order to avoid this, a new WithSingleBranch option was created to enable the download of all references at the initial clone. From now on SwitchBranch has the single responsibility of switching branches, and no longer pulling references.

The package git/gogit's primary goal is to support Flux use cases, currently there is no need to expand the current API to expose ways for users to refresh repository references outside the initial clone.

@pjbgf pjbgf added the area/git Git and SSH related issues and pull requests label Dec 14, 2022
@pjbgf pjbgf added this to the Bootstrap GA milestone Dec 14, 2022
@pjbgf pjbgf requested a review from aryan9600 December 14, 2022 13:42
pjbgf pushed a commit to fluxcd/image-automation-controller that referenced this pull request Dec 14, 2022
The new feature gate enables the download of
all branch head references when push branches are configured.

Fix fluxcd/flux2#3384.
Relates to fluxcd/pkg#433.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
@pjbgf pjbgf force-pushed the single-branch branch 2 times, most recently from d8dda52 to 6928ef2 Compare December 15, 2022 17:25
Copy link
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Left a few minor comments. Overall LGTM!

return nil
refName := plumbing.NewBranchReferenceName(branchName)
_, err = g.repository.Reference(refName, true)
if err == plumbing.ErrReferenceNotFound {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use errors.Is() here and above?

err = createBranch(repo, "ahead")
g.Expect(err).ToNot(HaveOccurred())

cc, err := commitFile(repo, "test", "testing gogit switch ahead branch", time.Now())
Copy link
Contributor

Choose a reason for hiding this comment

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

Not that it really matters but the content of the file may not be accurate for this test case.
Same for new cases below.

At present go-git does not support the MULTI_ACK capability, which
means that follow-up fetches on a given remote will fail.

To support Image Automation Controller use cases, the SwitchBranch
was initially short-circuited to avoid additional fetches. However,
this has the side effect of the controller pushing the same change
to the target repository multiple times. (fluxcd/flux2#3384)

In order to avoid this, a new WithSingleBranch option was created
to enable the download of all references at the initial clone.
From now on SwitchBranch has the single responsibility of switching
branches, and no longer pulling references.

The package git/gogit's primary goal is to support Flux use cases,
currently there is no need to expand the current API to expose ways
for users to refresh repository references outside the initial clone.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
@pjbgf pjbgf merged commit 90080a7 into main Dec 16, 2022
@pjbgf pjbgf deleted the single-branch branch December 16, 2022 09:23
aryan9600 added a commit to aryan9600/pkg that referenced this pull request May 12, 2023
Add `PushConfig.Force` for force pushing and remove
`gogit.Client.forcePush` in favor of the former.

Remove some stale test cases from `TestSwitchBranch` related to force
push since we don't check if force pushing is enabled while switching
branches anymore. Ref: fluxcd#433

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
aryan9600 added a commit that referenced this pull request May 15, 2023
Add `PushConfig.Force` for force pushing and remove
`gogit.Client.forcePush` in favor of the former.

Remove some stale test cases from `TestSwitchBranch` related to force
push since we don't check if force pushing is enabled while switching
branches anymore. Ref: #433

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
aryan9600 added a commit that referenced this pull request May 15, 2023
Add `PushConfig.Force` for force pushing and remove
`gogit.Client.forcePush` in favor of the former.

Remove some stale test cases from `TestSwitchBranch` related to force
push since we don't check if force pushing is enabled while switching
branches anymore. Ref: #433

Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git and SSH related issues and pull requests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants