Skip to content

[RFC] transition to gitoxide#112

Closed
Byron wants to merge 2 commits into
rustyhorde:masterfrom
Byron:master
Closed

[RFC] transition to gitoxide#112
Byron wants to merge 2 commits into
rustyhorde:masterfrom
Byron:master

Conversation

@Byron

@Byron Byron commented Apr 7, 2022

Copy link
Copy Markdown
Contributor

This PR shows how a first step towards transitioning to gitoxide could look like.

It adds a gitoxide feature toggle which if set uses gitoxide to fill in all git related values. It works additively.

Motivation

The best way for me to see how useful gitoxide is for applications and libraries is to seek out mature projects that use git2 and see how gitoxide fits in there. vergen was special as it brought the notion of MSRV support into `gitoxide''s understanding of
stability and required it to run on and work on various targets that aren't typically tested in its own CI.

Users of vergen expect stability and compilation in a variety of environment and gitoxide wants to cater to that.

Benefits for vergen to use gitoxide

  • gitoxide is generally faster than git2 and uses less system resources.
  • gitoxide aims to support all features of git focussing correctness, usability and performance, in that order.
  • gitoxide supports a minimal pure Rust build.
  • In case of issues, one deals with only a single project, whereas for git2 there is libgit2 and git2.
  • gitoxide guarantees to hide breaking changes behind major/minor version bumps to never break downstream. Changing the MSRV is considered a breaking change. All this is described in the Stability
    Guide    .
  • The maintainers of gitoxide are happy to maintain the gitoxide integration until it's first major release.

Downsides

  • after the transition period a major version bump for vergen is required to allow removing the gitoxide feature toggle, unless it should stay a no-op.
  • compile times for gitoxide are higher overall.
  • …there is probably more that I can't think of.

What missing in gitoxide for feature parity with git2

What's missing is to obtain a worktree status, effectively a diff between index and worktree, to support the optional 'dirty' suffix in the semver description key.

Request for feedback

The above is a writeup of all I know about such a transition, and I hope it can serve as foundation for figuring out how to proceed with this. Even though I'd love this to be interesting for you, I would fully understand that it might not fit into the plans you have for vergen - in any case gitoxide got so much better merely by preparing for this RFC so I am happy either way.

Thanks for sharing your thoughts.

@Byron Byron mentioned this pull request Apr 7, 2022
Closed
13 tasks
Byron
Byron commented Apr 7, 2022
View reviewed changes
Comment thread Cargo.toml Outdated
cfg-if = "1"
enum-iterator = "0"
getset = "0"
git-repository = { version = "0.16.0", optional = true, default-features = false, git = "https://github.com/Byron/gitoxide", branch = "main" }

@Byron Byron Apr 7, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note that his is only for the time being - when it's clear this PR should go ahead it will be switched to the published version on crates.io.

@Byron Byron mentioned this pull request Apr 19, 2022
Closed
@Byron

Byron commented Apr 19, 2022

Copy link
Copy Markdown
Contributor Author

In the related ticket in the windows crate I was asked when vergen is going to raise its MSRV. Maybe the question could also be answered by sharing the motivation for the MSRV. Thanks for your help.

@Byron

Byron commented May 2, 2022

Copy link
Copy Markdown
Contributor Author

Note that given the communication with the windows crates maintainers I think that using gitoxide would effectively tie the MSRV to the one of the windows crates.

gitoxide could use the winapi crate instead but before doing so it would be good to get any feedback on this RFC as it's clearly my preference to build on top of the windows crates for best possible support and future proofing.

Byron added 2 commits May 17, 2022 18:01
@Byron @CraZySacX
[RFC] transition to gitoxide
9be7eee 
This PR shows how a first step towards transitioning to `gitoxide` could
look like.

It adds a `gitoxide` feature toggle which if set uses `gitoxide` to fill
in all `git` related values. It works additively.

The best way for me to see how useful `gitoxide` is for applications and
libraries is to seek out mature projects that use `git2` and see how
`gitoxide` fits in there. `vergen` was special as it brought the notion
of MSRV support into [`gitoxide''s understanding of
stability](https://github.com/Byron/gitoxide/blob/498072ea42dca7b9d00bedba42829bdac92195b9/STABILITY.md#L124)
and required it to run on and work on various targets that aren't
typically tested in its own CI.

Users of `vergen` expect stability and compilation in a variety of
environment and `gitoxide` wants to cater to that.

- `gitoxide` is generally faster than `git2` and uses less system
  resources.
- `gitoxide` aims to support all features of `git` focussing on
  correctness, usability and performance, in that order.
- `gitoxide` supports a minimal pure Rust build.
- In case of issues, one deals with only a single project, whereas with
  `git2` there is `libgit2` and `git2`.
- `gitoxide` guarantees to hide breaking changes behind major/minor
  version bumps to never break downstream. Changing the MSRV is
  considered a breaking change. All this is described in the
  [Stability
  Guide](https://github.com/Byron/gitoxide/blob/498072ea42dca7b9d00bedba42829bdac92195b9/STABILITY.md).
- The maintainers of `gitoxide` are happy to maintain the `gitoxide`
  integration until it's first major release.

- after the transition period a major version bump for `vergen` is
  required to allow removing the `gitoxide` feature toggle, unless it
  should stay a no-op.
- compile times for `gitoxide` are higher overall.
- …there is probably more that I can't think of.

What's missing is to obtain a worktree status, effectively a diff
between index and worktree, to support the optional 'dirty' suffix
in the semver description key.

The above is a writeup of all I know about this transition, and I hope
it can serve as foundation for figuring out how (or if) to proceed with
this.

Thanks for sharing your thoughts.
@Byron @CraZySacX
uprgade to latest gitoxide release
dad7a76 
Maybe it's better to refer to a stable version from crates now.
@Byron

Byron commented Oct 12, 2022

Copy link
Copy Markdown
Contributor Author

Is there a chance for feedback here? If not I am happy to close this PR as well.
Thanks for your consideration.

@psibi

psibi commented Dec 23, 2022

Copy link
Copy Markdown

@CraZySacX Is there anything that can be done to make progress here ? I would love to use vergen + gitoxide in my projects.

@CraZySacX

CraZySacX commented Dec 23, 2022

Copy link
Copy Markdown
Member

Sorry for the delay here. Let me take a closer look at gitoxide. If it's indeed slimmer than libgit2 that would be a big bonus. There are tickets up about compilation speed that this may help solve. I took a cursory look at the change set. At first, I think this would be an optional feature explicitly requested by the end user. If this ends up being used much more that libgit2, we could eventually switch this to the default. I'll spend some time over the next couple days getting this RFC reviewed.

@Byron

Byron commented Dec 24, 2022

Copy link
Copy Markdown
Contributor Author

Sorry for the delay here. Let me take a closer look at gitoxide. If it's indeed slimmer than libgit2 that would be a big bonus. There are tickets up about compilation speed that this may help solve.

Last time the folks over at helix compared not too long ago gitoxide without connectivity was within 0.5s of the compilation speed of git2. On the bright side, gitoxide isn't very optimized for compilation speed yet and thus it can improved over time, whereas that will be much harder if not impossible with git2.

If you have any questions or suggestions, I will be happy to help.

@CraZySacX

CraZySacX commented Dec 27, 2022

Copy link
Copy Markdown
Member

There is a bit of a nasty conflict resolution on this PR. I could work through it, but I think I'm going to go down this path instead:

  1. Start a new git feature PR.
  2. Include gitoxide as a feature as it was here. I like the module pattern implemented in this PR for that and will do that same pattern again.
  3. Keep git2 as a feature, include that module in the git feature as well.
  4. Re-implement the old-style command line version as an additional feature (maybe git-cmd) for those that don't want to incur the compile time penalty for the above features. I would consider allowing the user to customize the command to use, i.e. git or gix. This may be limited in scope compared to the others.

As gitoxide matures, I would eventually pull the git2 feature out completely.

@Byron

Byron commented Dec 28, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for taking the time to review the PR, it's much appreciated! The planned path forward looks sound. Implementing git-cmd as a feature seems great for most as not compiling either git2 or gitoxide will be a great asset for some.

[…] I would consider allowing the user to customize the command to use, i.e. git or gix. This may be limited in scope compared to the others.

gix cannot be relied upon in scripts as it is merely a development tool and as such will probably never have a stable interface. It's also not trying to be as feature-rich as git so might be unsuitable for that reason alone.

@CraZySacX

CraZySacX commented Dec 29, 2022

Copy link
Copy Markdown
Member

gix cannot be relied upon in scripts as it is merely a development tool and as such will probably never have a stable interface. It's also not trying to be as feature-rich as git so might be unsuitable for that reason alone.

Ok, will keep the command based implementation on regular git

@CraZySacX

CraZySacX commented Jan 5, 2023

Copy link
Copy Markdown
Member

I have published v8.0.0-beta.0. This release includes gitoxide as an implementation for the git feature. Docs are at https://docs.rs/vergen/8.0.0-beta.0/vergen/index.html. I'm going to close this in lieu of #146. Feel free to comment on that PR if you think I've missed something.

@CraZySacX CraZySacX closed this Jan 5, 2023
@benmkw benmkw mentioned this pull request Feb 17, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Byron @psibi @CraZySacX