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 new ForceGoGitImplementation FeatureGate #945

Merged
merged 3 commits into from
Nov 11, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Oct 27, 2022

ForceGoGitImplementation ignores the value set for gitImplementation and ensures that go-git is used for all GitRepository objects. When enabled, libgit2 won't be initialized, nor will any git2go cgo code be called.

This is a soft-decommissioning of libgit2, to confirm that Flux instances won't break when that implementation is deprecated.

@pjbgf pjbgf added the area/git Git related issues and pull requests label Oct 27, 2022
@pjbgf pjbgf added this to the GA milestone Oct 27, 2022
internal/features/features.go Outdated Show resolved Hide resolved
@pjbgf pjbgf modified the milestones: GA, Bootstrap GA Nov 1, 2022
@pjbgf pjbgf mentioned this pull request Nov 1, 2022
7 tasks
@pjbgf pjbgf force-pushed the feature-nolibgit2 branch 4 times, most recently from 9a9fa2c to d5872c2 Compare November 2, 2022 07:26
Comment on lines 431 to 428
if enabled, _ := r.features[features.ForceGoGitImplementation]; enabled {
gitImplementation = sourcev1.GoGitImplementation
}
Copy link
Member

Choose a reason for hiding this comment

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

As this is done multiple times, I am wondering if it might be better to factor this out into a helper function which takes the object and string map and returns the correct implementation?

Copy link
Member Author

Choose a reason for hiding this comment

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

@darkowlzz gave a similar feedback on this and suggested the output of the feature gate to be passed as parameter to gitCheckout, which aligns with the current implementation for optimized clones. PTAL

@pjbgf pjbgf self-assigned this Nov 3, 2022
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
@pjbgf pjbgf force-pushed the feature-nolibgit2 branch 2 times, most recently from af226fa to 4f323e9 Compare November 11, 2022 10:11
Paulo Gomes added 2 commits November 11, 2022 10:25
ForceGoGitImplementation ignores the value set for gitImplementation
and ensures that go-git is used for all GitRepository objects.
This can be used to confirm that Flux instances won't break if/when
the libgit2 implementation was to be deprecated.

When enabled, libgit2 won't be initialized, nor will any git2go cgo
code be called.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>
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.

LGTM!

@pjbgf pjbgf merged commit 396e3cb into fluxcd:main Nov 11, 2022
@pjbgf pjbgf deleted the feature-nolibgit2 branch November 11, 2022 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/git Git related issues and pull requests
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants