RFC: Accelerate xdiff and begin its rustification
Subsystems (4)
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to Patrick SteinhardtQuoted 3 lines · Patrick Steinhardt
Quoted 1 line · brian m. carlson
On 2025-09-03 at 05:40:54, Patrick Steinhardt wrote:
[snip]
Quoted 9 lines
Quoted 8 lines
Also, the approach of making it an optional component directly contradicts the proposed policy I wrote up. That's a recipe for additional burdensome work maintaining two implementations, when we actually want to make it easier for people to contribute functionality. It also doesn't provide any of the memory safety benefits or address any of the concerns from governments, security professionals, and other parties about the real and substantial risks of continuing to develop in C.
Quoted 8 lines
The only reason why we want to have it as an optional component is to make the transitioning period easier for downstream distributors. And the intent is not to convert major components -- it should be trivial components that we can use as test balloons, similar to how we did it for all of our C99 test balloons.
We cannot just pull the rug away under their feet without advance notice that this is going to happen.
I find this statement a bit problematic for four reasons:
(1) "without advance notice" was already pointed out to be inaccurate in this thread, including in the exact email you are responding to; you could argue that there hasn't been _sufficient_ advance notice, but then there should be more details about what is and isn't sufficient. Merely repeating this claim which brian just barely pointed out to you as false almost feels dishonest.
(2) "pull the rug away" seems hyperbolic. I would have liked some explanation as to how a transition period is expected to help, and how the existing transition period has been insufficient. You do hint a little at the former, which I'll discuss more in point 4, but you
neglect the latter to the point of pretending it didn't exist. In
short, why is a further transition period needed, and how will it differ from the existing one we've already had? It's not clear to me why distributors must immediately update to the latest git version. Taylor discussed this aspect in detail in this thread; you even responded briefly (and tangentially?), but still as far as I can tell presume the latest and greatest is mandatory for them to adopt without stating why. Maybe they do need to adopt the latest and greatest, but I haven't seen folks state why that's the case. Did I miss it?
It also feels like Rust support is being lumped in with "breaking changes", which to me feels misleading. Historically, we have talked about breaking changes and deprecation periods and such so that users could adjust scripts or their command lines such that they would work across multiple versions of Git. The Rust case is somewhat different in that we're not discussing behavioral changes of git, merely implementation differences. If someone has both a C-only version of git and a newer version of git that was built with both Rust and C, any commands they run should behave the same as far as the C-vs-Rust goes (unless we have our normal discussions about specific behavior and any deprecations we want to do related to it, of course).
I do agree that reduced platform support is a negative change (though Rust brings other advantages that may offset this downside depending on your viewpoint), but I don't see why it's a breaking change and especially not a "pull the rug away under their feet" change.
(3) the use of "cannot" presupposes the policy stance which we are having a discussion about, which, whether intended or not, feels like an unfair way to attempt to shut down the conversation.
(4) you suggest that adding Rust as an optional component should avoid the problem, yet we've already had Rust as an optional component for the last three releases, going back to 2.49.0. (libgit-rs and libgit-sys). In this case, you helpfully provided some details distinguishing the type of optional component you want -- the reference to a test balloon suggests you want an optional component that is turned on by default (but which users can easily turn off). Am I correct that this is your intention? If that's the case, then that's a useful distinction, but I think that distinction needs to be made a bit more clearly (and as a side effect, acknowledge that Rust has already been optionally shipped in some form, and was even specifically highlighted by GitHub's and GitLab's blog posts about the v2.49.0 release, among other places)
Quoted 7 lines
Quoted 6 lines
For example, there is zero chance I will implement any of the SHA-1/SHA-256 compatibility code twice. I'm already doing that in my free time without any compensation at all and it's unreasonable to expect me to do it twice or even to #ifdef out all the places it would need to go. I am happy to let someone else take responsibility for the project instead, however, if they would like to do those things.
Quoted 5 lines
And that's totally fair. From my point of view, this compatibility code is a _new_ feature that we are adding to Git. And as I mentioned, I think it is reasonable to say that new features may be implemented in Rust now already, as platforms that aren't yet ready wouldn't lose any existing functionality.
Am I correct to understand that you're suggesting a policy where brian cannot modify any existing code to be written in Rust, and can only add new Rust code? Perhaps the SHA-1/SHA-256 compatibility code is just new code, or can be done with minimal changes to existing C code while adding new code. If so, maybe this is a workable solution for him.
But if it can't be done with minimal changes to existing C code and this policy would impair brian's ability to deliver the compatibility code, then I think this policy would be unworkable. I really don't want to hamstring brian's ability to implement the compatibility code. It has sat dormant for years with no one else stepping up to the plate, it's a really important project, and brian has time and energy now. I don't want any chicken-and-egg problems introduced for him with the 3.0 release. Even though I've been working with Ezekiel on xdiff, and I'm obviously a bit biased in that area, I find the sha1-sha256 compatibility work to be more critical and something we should do everything possible to facilitate.
Quoted 3 lines
I think most or even all of the contributors are on board. But we never really talked about timelines, or how we want to introduce Rust, so that's a discussion we need to have now.
Agreed.
Re: [PATCH v3 02/15] xdiff: introduce rust
Quoted 22 lines · Elijah Newren
Quoted 12 lines · Patrick Steinhardt
Quoted 9 lines · brian m. carlson
On 2025-09-03 at 05:40:54, Patrick Steinhardt wrote:
Also, the approach of making it an optional component directly contradicts the proposed policy I wrote up. That's a recipe for additional burdensome work maintaining two implementations, when we actually want to make it easier for people to contribute functionality. It also doesn't provide any of the memory safety benefits or address any of the concerns from governments, security professionals, and other parties about the real and substantial risks of continuing to develop in C.Quoted 8 lines
The only reason why we want to have it as an optional component is to make the transitioning period easier for downstream distributors. And the intent is not to convert major components -- it should be trivial components that we can use as test balloons, similar to how we did it for all of our C99 test balloons.
We cannot just pull the rug away under their feet without advance notice that this is going to happen.
Quoted 8 lines
I find this statement a bit problematic for four reasons:
(1) "without advance notice" was already pointed out to be inaccurate in this thread, including in the exact email you are responding to; you could argue that there hasn't been _sufficient_ advance notice, but then there should be more details about what is and isn't sufficient. Merely repeating this claim which brian just barely pointed out to you as false almost feels dishonest.
I think there is a difference between communication that happens on the mailing list/contributors summit and communication that is intended for the broader ecosystem:
- The former is basically us developers discussing potential futures and reviewing patches. It would be _nice_ if distro maintainers of Git were to read these, but given the large volume of traffic in general I think it unlikely that majority of maintainers is keeping up with that traffic.
- The latter is in the form of e.g. our release notes as well as our BreakingChanges document. These _are_ intended to be reviewed by maintainers, and the blame is on them if they don't do so.
We have never communicated either via release notes or via any kind of committed document that Rust is going to become mandatory. There have been lots of large threads discussing it, true. But navigating these threads and estimating consensus isn't easy even for us developers, so it's going to be even harder for outsiders to the community.
Quoted 5 lines
(2) "pull the rug away" seems hyperbolic. I would have liked some explanation as to how a transition period is expected to help, and how the existing transition period has been insufficient. You do hint a little at the former, which I'll discuss more in point 4, but you
neglect the latter to the point of pretending it didn't exist. In
Quoted 8 lines
short, why is a further transition period needed, and how will it differ from the existing one we've already had? It's not clear to me why distributors must immediately update to the latest git version. Taylor discussed this aspect in detail in this thread; you even responded briefly (and tangentially?), but still as far as I can tell presume the latest and greatest is mandatory for them to adopt without stating why. Maybe they do need to adopt the latest and greatest, but I haven't seen folks state why that's the case. Did I miss it?
The problem here is that we don't have a story to tell yet. I agree that not everyone always needs the latest and greatest, which is also why I mentioned that I think it's fine for _new_ features to be developed in Rust right away.
But the story is altogether different for bug and security fixes.
- We of course backport security fixes, but would that also be the case if we had ported the subsystem to Rust already and now had to implement the security fix twice?
- What happens if only the old C version has a security bug? Do we still fix it?
- Likewise, what happens with important bug fixes? We tend to backport those that are easy-ish to backport, but if people are potentially stuck with an older Git version for years it will become harder for us to do so.
I think without us having a proper answer to these questions we _are_ pulling the rug away. Distros may be stuck with an old version of Git for a significant time, and from my point of view we have to do a couple of compromises there.
Quoted 12 lines
It also feels like Rust support is being lumped in with "breaking changes", which to me feels misleading. Historically, we have talked about breaking changes and deprecation periods and such so that users could adjust scripts or their command lines such that they would work across multiple versions of Git. The Rust case is somewhat different in that we're not discussing behavioral changes of git, merely implementation differences. If someone has both a C-only version of git and a newer version of git that was built with both Rust and C, any commands they run should behave the same as far as the C-vs-Rust goes (unless we have our normal discussions about specific behavior and any deprecations we want to do related to it, of course).
Quoted 4 lines
I do agree that reduced platform support is a negative change (though Rust brings other advantages that may offset this downside depending on your viewpoint), but I don't see why it's a breaking change and especially not a "pull the rug away under their feet" change.
I honestly don't quite understand this perspective. How isn't it breaking that you cannot use that Git version at all anymore?
Quoted 3 lines
(3) the use of "cannot" presupposes the policy stance which we are having a discussion about, which, whether intended or not, feels like an unfair way to attempt to shut down the conversation.
Sorry, that's not my intent.
Quoted 4 lines
(4) you suggest that adding Rust as an optional component should avoid the problem, yet we've already had Rust as an optional component for the last three releases, going back to 2.49.0. (libgit-rs and libgit-sys).
I don't really think that either libgit-rs or libgit-sys help in any way. These are part of "contrib/", not built by default, and neither are they consumed by anyone out there. So there is no reason for anyone to build that library to the best of my knowledge.
Quoted 9 lines
In this case, you helpfully provided some details distinguishing the type of optional component you want -- the reference to a test balloon suggests you want an optional component that is turned on by default (but which users can easily turn off). Am I correct that this is your intention? If that's the case, then that's a useful distinction, but I think that distinction needs to be made a bit more clearly (and as a side effect, acknowledge that Rust has already been optionally shipped in some form, and was even specifically highlighted by GitHub's and GitLab's blog posts about the v2.49.0 release, among other places)
Yes. I think we need to have a test balloon that allows us to iterate on the build infrastructure and allows distributors to test with them. I think that test balloon needs to be integrated into core Git so that it is part of the normal build process, because otherwise it wouldn't have any exposure at all and thus not serve its purpose.
Quoted 13 lines
Quoted 7 lines
Quoted 6 lines
For example, there is zero chance I will implement any of the SHA-1/SHA-256 compatibility code twice. I'm already doing that in my free time without any compensation at all and it's unreasonable to expect me to do it twice or even to #ifdef out all the places it would need to go. I am happy to let someone else take responsibility for the project instead, however, if they would like to do those things.
Quoted 5 lines
And that's totally fair. From my point of view, this compatibility code is a _new_ feature that we are adding to Git. And as I mentioned, I think it is reasonable to say that new features may be implemented in Rust now already, as platforms that aren't yet ready wouldn't lose any existing functionality.
Quoted 6 lines
Am I correct to understand that you're suggesting a policy where brian cannot modify any existing code to be written in Rust, and can only add new Rust code? Perhaps the SHA-1/SHA-256 compatibility code is just new code, or can be done with minimal changes to existing C code while adding new code. If so, maybe this is a workable solution for him.
Yeah, that's my hope, as well. There's probably nouances to this though, and we'll have to figure it out once the series hits the mailing list. So...
Quoted 11 lines
But if it can't be done with minimal changes to existing C code and this policy would impair brian's ability to deliver the compatibility code, then I think this policy would be unworkable. I really don't want to hamstring brian's ability to implement the compatibility code. It has sat dormant for years with no one else stepping up to the plate, it's a really important project, and brian has time and energy now. I don't want any chicken-and-egg problems introduced for him with the 3.0 release. Even though I've been working with Ezekiel on xdiff, and I'm obviously a bit biased in that area, I find the sha1-sha256 compatibility work to be more critical and something we should do everything possible to facilitate.
... I guess we'll have to see how this looks like in the end. If the series rewrites a bunch of subsystems in Rust I think we should figure out whether we can do without that. Or, in the worst case, whether it is feasible to conditionally compile some of the code with either C or Rust, even though nobody likes that.
Patrick
Re: [PATCH v3 02/15] xdiff: introduce rust
Sorry for the delay; life outside of work is challenging at the moment...
Quoted 47 lines · Patrick Steinhardt
Quoted 22 lines · Elijah Newren
Quoted 12 lines · Patrick Steinhardt
Quoted 9 lines · brian m. carlson
On 2025-09-03 at 05:40:54, Patrick Steinhardt wrote:
Also, the approach of making it an optional component directly contradicts the proposed policy I wrote up. That's a recipe for additional burdensome work maintaining two implementations, when we actually want to make it easier for people to contribute functionality. It also doesn't provide any of the memory safety benefits or address any of the concerns from governments, security professionals, and other parties about the real and substantial risks of continuing to develop in C.Quoted 8 lines
The only reason why we want to have it as an optional component is to make the transitioning period easier for downstream distributors. And the intent is not to convert major components -- it should be trivial components that we can use as test balloons, similar to how we did it for all of our C99 test balloons.
We cannot just pull the rug away under their feet without advance notice that this is going to happen.
Quoted 8 lines
I find this statement a bit problematic for four reasons:
(1) "without advance notice" was already pointed out to be inaccurate in this thread, including in the exact email you are responding to; you could argue that there hasn't been _sufficient_ advance notice, but then there should be more details about what is and isn't sufficient. Merely repeating this claim which brian just barely pointed out to you as false almost feels dishonest.
I think there is a difference between communication that happens on the mailing list/contributors summit and communication that is intended for the broader ecosystem:
- The former is basically us developers discussing potential futures and reviewing patches. It would be _nice_ if distro maintainers of Git were to read these, but given the large volume of traffic in general I think it unlikely that majority of maintainers is keeping up with that traffic.
- The latter is in the form of e.g. our release notes as well as our BreakingChanges document. These _are_ intended to be reviewed by maintainers, and the blame is on them if they don't do so.
Quoted 5 lines
We have never communicated either via release notes or via any kind of committed document that Rust is going to become mandatory. There have been lots of large threads discussing it, true. But navigating these threads and estimating consensus isn't easy even for us developers, so it's going to be even harder for outsiders to the community.
I like this framing; this is useful.
I agree that we haven't communicated that it'll be mandatory, though we have communicated beyond the list that Rust was likely coming:
* The contributor summit notes on Rust (posted at
https://lore.kernel.org/git/Zu2D%[email protected]/) were widely picked up at other sites (e.g.
https://lwn.net/Articles/998115/,
https://www.reddit.com/r/linux/comments/1hcsvk5/nonstop_discussion_around_adding_rust_to_git/)
* The release notes mention initial Rust inclusion
(https://lore.kernel.org/git/[email protected]/, "Foreign language interface for Rust into our code base has been added.")
* The GitHub blog on highlights from 2.49.0 (widely linked at news
sites even in preference to the release notes) adds more detail: "This release marks a major milestone in the Git project with the first pieces of Rust code being checked in"
(https://github.blog/open-source/git/highlights-from-git-2-49/)
Now, I can fully get behind that this may be _inadequate_ notice, and I really like the idea of a test balloon. I'm just noting that I very much disagree with the characterization that there has been no notice beyond the mailing list about Rust likely coming at some point, and want us to make sure that if we delay, we use the time to meaningfully provide more notice than we have already. Another optional Rust component that doesn't build by default, for example, fails that test.
Quoted 33 lines
Quoted 5 lines
(2) "pull the rug away" seems hyperbolic. I would have liked some explanation as to how a transition period is expected to help, and how the existing transition period has been insufficient. You do hint a little at the former, which I'll discuss more in point 4, but you
neglect the latter to the point of pretending it didn't exist. InQuoted 8 lines
short, why is a further transition period needed, and how will it differ from the existing one we've already had? It's not clear to me why distributors must immediately update to the latest git version. Taylor discussed this aspect in detail in this thread; you even responded briefly (and tangentially?), but still as far as I can tell presume the latest and greatest is mandatory for them to adopt without stating why. Maybe they do need to adopt the latest and greatest, but I haven't seen folks state why that's the case. Did I miss it?
The problem here is that we don't have a story to tell yet. I agree that not everyone always needs the latest and greatest, which is also why I mentioned that I think it's fine for _new_ features to be developed in Rust right away.
But the story is altogether different for bug and security fixes.
- We of course backport security fixes, but would that also be the case if we had ported the subsystem to Rust already and now had to implement the security fix twice?
- What happens if only the old C version has a security bug? Do we still fix it?
- Likewise, what happens with important bug fixes? We tend to backport those that are easy-ish to backport, but if people are potentially stuck with an older Git version for years it will become harder for us to do so.
Quoted 4 lines
I think without us having a proper answer to these questions we _are_ pulling the rug away. Distros may be stuck with an old version of Git for a significant time, and from my point of view we have to do a couple of compromises there.
These are good questions...but they are ones to which I suspect delaying will not provide the answer. In fact, I don't think we'll _ever_ have the answer to these questions, no matter how much we delay or discuss. Traditionally, if an issue was more severe, it has been backported to more versions, even if the backport wasn't trivial. There's a cost/benefit tradeoff to be had for each vulnerability, and changes to the area making backports either be easy or hard always need to be weighed against the severity of the vulnerability. I don't see that changing, and overpromising hurts in the long run probably more than having no guidance. I just don't see us coming up with "proper answers" (which I'm guessing means fully spelled out answers?) to these questions ahead of time. The answer to all of them is probably "we'll weigh the severity of the issue and the cost to backport and give the last C-only version significant extra weight in our considerations". I doubt we'll ever be able to promise any more detail than that until we get concrete cases; I'm not even sure that this statement is acceptable to everyone on the list from the overpromising angle despite being as incomplete as it is.
Quoted 12 lines
Quoted 12 lines
It also feels like Rust support is being lumped in with "breaking changes", which to me feels misleading. Historically, we have talked about breaking changes and deprecation periods and such so that users could adjust scripts or their command lines such that they would work across multiple versions of Git. The Rust case is somewhat different in that we're not discussing behavioral changes of git, merely implementation differences. If someone has both a C-only version of git and a newer version of git that was built with both Rust and C, any commands they run should behave the same as far as the C-vs-Rust goes (unless we have our normal discussions about specific behavior and any deprecations we want to do related to it, of course).
Quoted 7 lines
Quoted 4 lines
I do agree that reduced platform support is a negative change (though Rust brings other advantages that may offset this downside depending on your viewpoint), but I don't see why it's a breaking change and especially not a "pull the rug away under their feet" change.
I honestly don't quite understand this perspective. How isn't it breaking that you cannot use that Git version at all anymore?
Users might often face cases where they have to use different versions of git -- at home, at work, on different work machines, etc. As such, when something forces workflow changes, we have to be cognizant of that and provide deprecation periods, release announcement notices, etc. That's the point of our care around breaking changes.
If _distributors_ can't build a new version of git, users can still use older versions. They don't have to change their workflows. When distributors eventually figure out how to build a newer version (because they work around pthreads not existing on their platform, or they add stdbool to their compiler, or they port Rust to their system or whatever), then when the new version becomes available, users can use it without changes to their workflow. The _users_ weren't broken.
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a slightly older version. Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for handling than e.g. lumping it in with 3.0.
Quoted 5 lines
Quoted 3 lines
(3) the use of "cannot" presupposes the policy stance which we are having a discussion about, which, whether intended or not, feels like an unfair way to attempt to shut down the conversation.
Sorry, that's not my intent.
Thanks, and I appreciate you patiently explaining your point of view in more detail.
Quoted 9 lines
Quoted 4 lines
(4) you suggest that adding Rust as an optional component should avoid the problem, yet we've already had Rust as an optional component for the last three releases, going back to 2.49.0. (libgit-rs and libgit-sys).
I don't really think that either libgit-rs or libgit-sys help in any way. These are part of "contrib/", not built by default, and neither are they consumed by anyone out there. So there is no reason for anyone to build that library to the best of my knowledge.
I'm fully willing to accept they are inadequate notice (and perhaps even barely helpful), but disagree with the characterization that they don't help at all:
* they were consumed in the past by Google
* they recently received patches from someone outside Google
(https://lore.kernel.org/git/[email protected]/)
* they were mentioned in the release notes highlighting at a minimum
that Rust is being added to Git.
* they were highlighted in blog posts from both GitLab and GitHub as
being noteworthy new things in the v2.49.0 release
I agree with you there is certainly more we can do, and I like your idea of a test ballon. Let's just avoid repeating the problem of adding an optional component that no one will try to build except for those for whom we know can build it; doing that would provide no more notice and thus provide no incremental benefit over libgit-rs and libgit-sys.
RE: [PATCH v3 02/15] xdiff: introduce rust
Quoted 2 lines · Elijah Newren
Sorry for the delay; life outside of work is challenging at the moment...
I am going to address the critical point mentioned below and snip the rest for brevity.
Quoted 7 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a slightly older version. Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for handling than e.g. lumping it in with 3.0.
I do not see that distributors _must_ ship the latest version. Suppose we are on 2.51.0 and a CVE comes out that prohibits its use in an organization that does not allow any medium-high to high CVEs. This represents hundreds of thousands of impacted users in my community alone. How does the CVE get applied if the latest cannot be built and the git team does not apply the CVE fixes to old versions. Personally, I do not care if git versions are different between work and home, or even between CI/CD and other platforms. I don't even care if I have to use JGit instead of git in some situations (which I see is a likely outcome of this discussion). Is there an official statement of what an LTS means? In other projects LTS is typically, and formally by policy 5 years. From what others have said here, positions of 6 months, 3 years, and "apply it yourself if you want to continue to use git" have been made.
The core problem of adding a breaking dependency is when a CVE comes out that prohibits git from being used at all. If the git team is not going to provide a clear statement, one way or another, if how CVEs (at whatever severity level) will not have a commitment of any kind, then distributors are essentially cast adrift and on our own. It would be helpful of those of us who donate our time, for no compensation, are able to plan for this in a meaningful way. Please remember that we have to justify our participation to our management teams to be allowed to continue to participate. Nothing is free from this end and if fixing (not just applying fixes) CVEs are now 100% our responsibility, if would be critical to know that when we build our business cases to our bosses, who I am fairly certain will say an emphatic no.
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources. This particular point enabled adoption on some platforms, particularly NonStop. Adoption was at 1-2 customers when we had a divergent code based because some platform fixes being different from the standard code-base and could not be certified as valid. Once the code-base because common, adoption was rapid and enthusiastic. If this goes away, I suspect that adoption rates will go negative. I am aware that that particular discussion is actually happening in some organizations in my community right now, with companies looking for alternatives to git based on this discussion thread.
With over a decade of respect and participation,
Randall
Re: [PATCH v3 02/15] xdiff: introduce rust
Hi Randall
Quoted 14 lines · On 07/09/2025 17:09, [email protected] wrote:
Quoted 2 lines · Elijah Newren
Sorry for the delay; life outside of work is challenging at the moment...
I am going to address the critical point mentioned below and snip the rest for brevity.
Quoted 7 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a slightly older version. Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for handling than e.g. lumping it in with 3.0.
Quoted 10 lines
I do not see that distributors _must_ ship the latest version. Suppose we are on 2.51.0 and a CVE comes out that prohibits its use in an organization that does not allow any medium-high to high CVEs. This represents hundreds of thousands of impacted users in my community alone. How does the CVE get applied if the latest cannot be built and the git team does not apply the CVE fixes to old versions. Personally, I do not care if git versions are different between work and home, or even between CI/CD and other platforms. I don't even care if I have to use JGit instead of git in some situations (which I see is a likely outcome of this discussion). Is there an official statement of what an LTS means?
We're currently discussing what promises we can make about supporting a non-rust version of git.
Quoted 1 line
In other projects LTS is typically, and formally by policy 5 years.
I know commercial linux distributions offer that kind of support but are there really open source projects that guarantee 5 years of security updates without any kind of support contract?
Quoted 2 lines
From what others have said here, positions of 6 months, 3 years, and
"apply it yourself if you want to continue to use git" have been made.
Yes it is still being discussed, and no one is volunteering to offer five years of support.
Quoted 7 lines
The core problem of adding a breaking dependency is when a CVE comes out that prohibits git from being used at all. If the git team is not going to provide a clear statement, one way or another, if how CVEs (at whatever severity level) will not have a commitment of any kind, then distributors are essentially cast adrift and on our own. It would be helpful of those of us who donate our time, for no compensation, are able to plan for this in a meaningful way.
Doesn't your company make a front end to git? Are you saying that the management does not allocate any staff time to work on git itself and expects the community to provide it with free security updates?
Quoted 3 lines
Please remember that
we have to justify our participation to our management teams to be allowed to continue to participate.
I'm confused by this, as the sentence before say's you're donating your time for no compensation.
Quoted 5 lines
Nothing is free from this end
and if fixing (not just applying fixes) CVEs are now 100% our responsibility, if would be critical to know that when we build our business cases to our bosses, who I am fairly certain will say an emphatic no.
In the long term, unless your platform gains a rust compiler I'm afraid I think that is most likely outcome.
Quoted 3 lines
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources.
I think I saw a suggestion/question about the possibility of hosting any long term support branch that is maintained by interested parties within the main repository. Would that help?
I appreciate that any move to rust would be very disappointing and disruptive to you but the community has to weigh up the benefits rust has to offer against that.
Phillip
Quoted 14 lines
This particular
point enabled adoption on some platforms, particularly NonStop. Adoption was at 1-2 customers when we had a divergent code based because some platform fixes being different from the standard code-base and could not be certified as valid. Once the code-base because common, adoption was rapid and enthusiastic. If this goes away, I suspect that adoption rates will go negative. I am aware that that particular discussion is actually happening in some organizations in my community right now, with companies looking for alternatives to git based on this discussion thread.With over a decade of respect and participation,
Randall
RE: [PATCH v3 02/15] xdiff: introduce rust
Quoted 30 lines · Phillip Wood
Quoted 5 lines · On 07/09/2025 17:09, [email protected] wrote:
Quoted 2 lines · Elijah Newren
Sorry for the delay; life outside of work is challenging at the moment...
I am going to address the critical point mentioned below and snip the rest for
brevity.
Quoted 3 lines
Quoted 3 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a
slightly older version.
Quoted 5 lines
Quoted 5 lines
Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for
handling than e.g. lumping it in with 3.0.
Quoted 12 lines
I do not see that distributors _must_ ship the latest version. Suppose we are on
2.51.0 and a CVE comes out that prohibits its use in an organization that does not allow any medium-high to high CVEs. This represents hundreds of thousands of impacted users in my community alone. How does the CVE get applied if the latest cannot be built and the git team does not apply the CVE fixes to old versions. Personally, I do not care if git versions are different between work and home, or even between CI/CD and other platforms. I don't even care if I have to use JGit instead of git in some situations (which I see is a likely outcome of this discussion). Is there an official statement of what an LTS means?
Quoted 8 lines
We're currently discussing what promises we can make about supporting a non-rust version of git.
Quoted 1 line
In other projects LTS is typically, and formally by policy 5 years.
I know commercial linux distributions offer that kind of support but are there really open source projects that guarantee 5 years of security updates without any kind of support contract?
OpenSSL provides 5 years of security fix support (at no cost) for LTS designated releases. Currently 3.0 ending Sept 2026 and 3.5 ending around October 2030. After those dates, there is a fee-based support arrangement available.
Quoted 14 lines
Quoted 2 lines
From what others have said here, positions of 6 months, 3 years, and
"apply it yourself if you want to continue to use git" have been made.Yes it is still being discussed, and no one is volunteering to offer five years of support.
Quoted 7 lines
The core problem of adding a breaking dependency is when a CVE comes out that prohibits git from being used at all. If the git team is not going to provide a clear statement, one way or another, if how CVEs (at whatever severity level) will not have a commitment of any kind, then distributors are essentially cast adrift and on our own. It would be helpful of those of us who donate our time, for no compensation, are able to plan for this in a meaningful way.
Quoted 3 lines
Doesn't your company make a front end to git? Are you saying that the management does not allocate any staff time to work on git itself and expects the community to provide it with free security updates?
The REAL PROBLEM that is not being addressed in this thread is that large companies (the ones who process your credit cards, build your cars, manufacture your drugs, and tool your factories (a.k.a. NonStop customers), are generally unwilling to accept CVE fixes from third parties. The fixes have to be part of the official code base or the fixes will not accepted. That means that either:
1. the git team has to officially sanction the fixes; or
2. do the fixes themselves.
A compromise may be possible to keep a support branch around in the official git repo, for those of us who do not have rust available to contribute to, specifically for post C-deprecation CVE fixes, but I am not sure this is practical. It would also require occasional assistance from the git team, to make sense of some of the fixes, If they apply, as none of us are rust experts.
I am already allocated to spending between 10 and 20 hours a month to git, which usually involves running and verifying build/test cycles. Since git tests are flakey, in some cases, I have to manually examine each failure situation and decide whether the failures are sufficient to pass the releases. These have been reported previously without resolution and do not bear discussion here.
It is important to understand that many git customers in high audit situations build git on their own because they do not trust third party builds, so this needs to remain an option.
Quoted 5 lines
Quoted 3 lines
Please remember that
we have to justify our participation to our management teams to be allowed to continue to participate.I'm confused by this, as the sentence before say's you're donating your time for no compensation.
My company pretends to donates my time with very little direct benefit to them. My participation is because I feel it is important for my community. No, I do not get a salary for my git time. It is evenings and weekends. Any time I spend during working hours has to be made up during off hours.
Quoted 10 lines
Quoted 6 lines
Nothing is free from this end
and if fixing (not just applying fixes) CVEs are now 100% our responsibility, if would be critical to know that when we build our business cases to our bosses, who I am fairly certain will say an emphatic no.In the long term, unless your platform gains a rust compiler I'm afraid I think that is most likely outcome.
Quoted 7 lines
Quoted 3 lines
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources.
I think I saw a suggestion/question about the possibility of hosting any long term support branch that is maintained by interested parties within the main repository. Would that help?
DEFINITELY. With assistance as above. With some help, we may be able to make this work. It might require a deeper participation on my part and those on my team to approve changes, which we would consider.
Quoted 2 lines
I appreciate that any move to rust would be very disappointing and disruptive to you but the community has to weigh up the benefits rust has to offer against that.
The community has plans for Rust but they have not taken shape fully as of yet. I have personally been badgering product management to make this happen, and I might know more in a month or so. However, this takes time, and 6 months is not enough.
--Randall
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to [email protected]Quoted 15 lines · On Sun, Sep 7, 2025 at 10:10 AM <[email protected]> wrote:
Quoted 2 lines · Elijah Newren
Sorry for the delay; life outside of work is challenging at the moment...
I am going to address the critical point mentioned below and snip the rest for brevity.
Quoted 7 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a slightly older version. Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for handling than e.g. lumping it in with 3.0.
Quoted 8 lines
I do not see that distributors _must_ ship the latest version. Suppose we are on 2.51.0 and a CVE comes out that prohibits its use in an organization that does not allow any medium-high to high CVEs. This represents hundreds of thousands of impacted users in my community alone. How does the CVE get applied if the latest cannot be built and the git team does not apply the CVE fixes to old versions. Personally, I do not care if git versions are different between work and home, or even between CI/CD and other platforms. I don't even care
...
Ok, that answers the question for NonStop, but that doesn't answer the question for the plethora of other distributions. Most distributions don't ship the latest version of Git in their package manager, and if an organization deems it critical to have the latest they can build it themselves and ignore the Git version in the package manager. So why does Windows, Mac, Linux, etc... _need_ the latest version of Git in the package manager?
If security updates are backported to NonStop, until that platform supports Rust, then I don't see why using an older version of Git in Windows, Mac, Linux, etc... is a catastrophe. Most existing distributions _can_ package the latest version of Git, but they _don't_.
I reiterate Elijah's question "Why _must_ distributors ship the latest version of Git?".
RE: [PATCH v3 02/15] xdiff: introduce rust
Quoted 36 lines · Ezekiel Newren
Quoted 6 lines · On Sun, Sep 7, 2025 at 10:10 AM <[email protected]> wrote:
Quoted 2 lines · Elijah Newren
Sorry for the delay; life outside of work is challenging at the moment...
I am going to address the critical point mentioned below and snip the rest for
brevity.
Quoted 3 lines
Quoted 3 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a
slightly older version.
Quoted 5 lines
Quoted 5 lines
Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for
handling than e.g. lumping it in with 3.0.
Quoted 10 lines
I do not see that distributors _must_ ship the latest version. Suppose we are on
2.51.0 and a CVE comes out that prohibits its use in an organization that does not allow any medium-high to high CVEs. This represents hundreds of thousands of impacted users in my community alone. How does the CVE get applied if the latest cannot be built and the git team does not apply the CVE fixes to old versions. Personally, I do not care if git versions are different between work and home, or even between CI/CD and other platforms. I don't even care ...Ok, that answers the question for NonStop, but that doesn't answer the question for the plethora of other distributions. Most distributions don't ship the latest version of Git in their package manager, and if an organization deems it critical to have the latest they can build it themselves and ignore the Git version in the package manager. So why does Windows, Mac, Linux, etc... _need_ the latest version of Git in the package manager?
Quoted 7 lines
If security updates are backported to NonStop, until that platform supports Rust, then I don't see why using an older version of Git in Windows, Mac, Linux, etc... is a catastrophe. Most existing distributions _can_ package the latest version of Git, but they _don't_.
I reiterate Elijah's question "Why _must_ distributors ship the latest version of Git?".
My emphatic answer is that they do not. There is no requirement from me or anyone I know to ship the latest version. What is crucial is that there be fixes for medium-high and above CVEs that are delivered in 30 days from initial fix availability (that would be in Rust, for this conversation and applied to C). If that were supported, I could live with as would my customers and their auditors for LTS releases. Please see my expectation of LTS defined elsewhere in this thread - essentially 5 years. Perhaps 3 at a bare minimum.
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to [email protected]Quoted 55 lines · On Sun, Sep 7, 2025 at 9:10 AM <[email protected]> wrote:
Quoted 2 lines · Elijah Newren
Sorry for the delay; life outside of work is challenging at the moment...
I am going to address the critical point mentioned below and snip the rest for brevity.
Quoted 7 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a slightly older version. Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for handling than e.g. lumping it in with 3.0.
I do not see that distributors _must_ ship the latest version. Suppose we are on 2.51.0 and a CVE comes out that prohibits its use in an organization that does not allow any medium-high to high CVEs. This represents hundreds of thousands of impacted users in my community alone. How does the CVE get applied if the latest cannot be built and the git team does not apply the CVE fixes to old versions. Personally, I do not care if git versions are different between work and home, or even between CI/CD and other platforms. I don't even care if I have to use JGit instead of git in some situations (which I see is a likely outcome of this discussion). Is there an official statement of what an LTS means? In other projects LTS is typically, and formally by policy 5 years. From what others have said here, positions of 6 months, 3 years, and "apply it yourself if you want to continue to use git" have been made.
The core problem of adding a breaking dependency is when a CVE comes out that prohibits git from being used at all. If the git team is not going to provide a clear statement, one way or another, if how CVEs (at whatever severity level) will not have a commitment of any kind, then distributors are essentially cast adrift and on our own. It would be helpful of those of us who donate our time, for no compensation, are able to plan for this in a meaningful way. Please remember that we have to justify our participation to our management teams to be allowed to continue to participate. Nothing is free from this end and if fixing (not just applying fixes) CVEs are now 100% our responsibility, if would be critical to know that when we build our business cases to our bosses, who I am fairly certain will say an emphatic no.
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources. This particular point enabled adoption on some platforms, particularly NonStop. Adoption was at 1-2 customers when we had a divergent code based because some platform fixes being different from the standard code-base and could not be certified as valid. Once the code-base because common, adoption was rapid and enthusiastic. If this goes away, I suspect that adoption rates will go negative. I am aware that that particular discussion is actually happening in some organizations in my community right now, with companies looking for alternatives to git based on this discussion thread.
Quoted 2 lines
With over a decade of respect and participation,
Randall
Thanks, Randall, this is useful information. In regards to one point not fully covered by Phillip:
Quoted 3 lines
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources.
Why does it need to be "third-party" sources? Linus years ago blessed having someone else be in charge of providing updates for stable releases of Linux. Junio could do the same with Git and similarly mark an individual or group of people as the maintainers for the last Rust-optional version of Git, and those individuals could make official releases of Git with extended security fix support. Then it's not every platform repeating the backporting work that needs to be done, but rather individuals from the affected platform(s) collaborating on that work and then making official first-party releases.
RE: [PATCH v3 02/15] xdiff: introduce rust
Quoted 70 lines · Elijah Newren
Quoted 6 lines · On Sun, Sep 7, 2025 at 9:10 AM <[email protected]> wrote:
Quoted 2 lines · Elijah Newren
Sorry for the delay; life outside of work is challenging at the moment...
I am going to address the critical point mentioned below and snip the rest for
brevity.
Quoted 3 lines
Quoted 3 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a
slightly older version.
Quoted 5 lines
Quoted 5 lines
Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for
handling than e.g. lumping it in with 3.0.
Quoted 12 lines
I do not see that distributors _must_ ship the latest version. Suppose we are on
2.51.0 and a CVE comes out that prohibits its use in an organization that does not allow any medium-high to high CVEs. This represents hundreds of thousands of impacted users in my community alone. How does the CVE get applied if the latest cannot be built and the git team does not apply the CVE fixes to old versions. Personally, I do not care if git versions are different between work and home, or even between CI/CD and other platforms. I don't even care if I have to use JGit instead of git in some situations (which I see is a likely outcome of this discussion). Is there an official statement of what an LTS means?In other projects LTS is typically, and formally by policy 5 years.
Quoted 30 lines
From what others have said here, positions of 6 months, 3 years, and "apply it yourself if you want to continue to use git" have been made.
The core problem of adding a breaking dependency is when a CVE comes out that prohibits git from being used at all. If the git team is not going to provide a clear statement, one way or another, if how CVEs (at whatever severity level) will not have a commitment of any kind, then distributors are essentially cast adrift and on our own. It would be helpful of those of us who donate our time, for no compensation, are able to plan for this in a meaningful way. Please remember that we have to justify our participation to our management teams to be allowed to continue to participate. Nothing is free from this end and if fixing (not just applying fixes) CVEs are now 100% our responsibility, if would be critical to know that when we build our business cases to our bosses, who I am fairly certain will say an emphatic no.
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources. This particular point enabled adoption on some platforms, particularly NonStop.
Adoption was at 1-2 customers when we had a divergent code based because some platform fixes being different from the standard code-base and could not be certified as valid. Once the code-base because common, adoption was rapid and enthusiastic.
If this goes away, I suspect that adoption rates will go negative. I am aware that that particular discussion is actually happening in some organizations in my community right now, with companies looking for alternatives to git based on this discussion thread.Quoted 1 line
With over a decade of respect and participation, Randall
Thanks, Randall, this is useful information. In regards to one point not fully covered by Phillip:
Quoted 3 lines
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources.
Quoted 8 lines
Why does it need to be "third-party" sources? Linus years ago blessed having someone else be in charge of providing updates for stable releases of Linux. Junio could do the same with Git and similarly mark an individual or group of people as the maintainers for the last Rust-optional version of Git, and those individuals could make official releases of Git with extended security fix support. Then it's not every platform repeating the backporting work that needs to be done, but rather individuals from the affected platform(s) collaborating on that work and then making official first-party releases.
Linux has one set of rules, and other platforms have others. I do not define the audit requirements for PCI, SWIFT, or HIPPA compliance (and other rules outside of North America), which apply one way or another to most of my community. The audit teams, which are both internal to the companies and at governmental regulatory levels, do this. It is 100% out of my control but is a reality. Fixes to any code involved in managing financial and health instruments must be done by authorized and recognized sources. I am not one of them.
Re: [PATCH v3 02/15] xdiff: introduce rust
Quoted 22 lines · On Mon, Sep 8, 2025 at 8:37 AM <[email protected]> wrote:
Quoted 11 lines · Elijah Newren
Quoted 2 lines · On Sun, Sep 7, 2025 at 9:10 AM <[email protected]> wrote:
On September 7, 2025 12:10 AM, Elijah Newren wrote:
Thanks, Randall, this is useful information. In regards to one point not fully covered by Phillip:
Quoted 3 lines
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources.
Quoted 8 lines
Why does it need to be "third-party" sources? Linus years ago blessed having someone else be in charge of providing updates for stable releases of Linux. Junio could do the same with Git and similarly mark an individual or group of people as the maintainers for the last Rust-optional version of Git, and those individuals could make official releases of Git with extended security fix support. Then it's not every platform repeating the backporting work that needs to be done, but rather individuals from the affected platform(s) collaborating on that work and then making official first-party releases.
Quoted 7 lines
Linux has one set of rules, and other platforms have others. I do not define the audit requirements for PCI, SWIFT, or HIPPA compliance (and other rules outside of North America), which apply one way or another to most of my community. The audit teams, which are both internal to the companies and at governmental regulatory levels, do this. It is 100% out of my control but is a reality. Fixes to any code involved in managing financial and health instruments must be done by authorized and recognized sources. I am not one of them.
Perhaps I wasn't clear? Let me try to summarize what I've understood of the conversation:
Randall: We need to have official git releases for the last Rust-optional release.
Elijah: Great! Let's enable interested folks to make official git releases for the last Rust-optional release.
Randall: We need to have official git releases for the last Rust-optional release.
Which makes me just want to repeat what I said last time -- let's enable some folks to do that.
RE: [PATCH v3 02/15] xdiff: introduce rust
Quoted 45 lines · Elijah Newren
Quoted 7 lines · On Mon, Sep 8, 2025 at 8:37 AM <[email protected]> wrote:
Quoted 5 lines · Elijah Newren
Quoted 2 lines · On Sun, Sep 7, 2025 at 9:10 AM <[email protected]> wrote:
On September 7, 2025 12:10 AM, Elijah Newren wrote:
Thanks, Randall, this is useful information. In regards to one point not fully
covered
Quoted 8 lines
Quoted 8 lines
by Phillip:
Quoted 3 lines
Also remember that without support from the git team, the code base is no longer the same, meaning the auditors will not necessarily accept fixes from third-party sources.
Why does it need to be "third-party" sources? Linus years ago blessed having someone else be in charge of providing updates for stable releases of Linux.
Junio
Quoted 2 lines
Quoted 2 lines
could do the same with Git and similarly mark an individual or group of people as the maintainers for the last Rust-optional version of Git, and those individuals
could
Quoted 1 line
Quoted 1 line
make official releases of Git with extended security fix support. Then it's not
every
Quoted 11 lines
Quoted 3 lines
platform repeating the backporting work that needs to be done, but rather individuals from the affected platform(s) collaborating on that work and then making official first-party releases.
Linux has one set of rules, and other platforms have others. I do not define the audit requirements for PCI, SWIFT, or HIPPA compliance (and other rules outside of North America), which apply one way or another to most of my community. The audit teams, which are both internal to the companies and at governmental regulatory levels, do this. It is 100% out of my control but is a reality. Fixes to any code involved in managing financial and health instruments must be done by authorized and recognized sources. I am not one of them.
Perhaps I wasn't clear? Let me try to summarize what I've understood of the conversation:
Randall: We need to have official git releases for the last Rust-optional release.
Elijah: Great! Let's enable interested folks to make official git releases for the last Rust-optional release.
Randall: We need to have official git releases for the last Rust-optional release.
Quoted 2 lines
Which makes me just want to repeat what I said last time -- let's enable some folks to do that.
Ok, what does that look like. When and how? Will I get added to the list of committers for this? How many people? Starting when, ending when? It would be very nice to have some kind of project plan for this with dependencies and milestones - I have been asking. If someone sends me a list, I can start officially tracking it.
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to Elijah NewrenQuoted 25 lines · Elijah Newren
Quoted 19 lines · Patrick Steinhardt
The problem here is that we don't have a story to tell yet. I agree that not everyone always needs the latest and greatest, which is also why I mentioned that I think it's fine for _new_ features to be developed in Rust right away.
But the story is altogether different for bug and security fixes.
- We of course backport security fixes, but would that also be the case if we had ported the subsystem to Rust already and now had to implement the security fix twice?
- What happens if only the old C version has a security bug? Do we still fix it?
- Likewise, what happens with important bug fixes? We tend to backport those that are easy-ish to backport, but if people are potentially stuck with an older Git version for years it will become harder for us to do so.
Quoted 4 lines
I think without us having a proper answer to these questions we _are_ pulling the rug away. Distros may be stuck with an old version of Git for a significant time, and from my point of view we have to do a couple of compromises there.
Quoted 12 lines
These are good questions...but they are ones to which I suspect delaying will not provide the answer. In fact, I don't think we'll _ever_ have the answer to these questions, no matter how much we delay or discuss. Traditionally, if an issue was more severe, it has been backported to more versions, even if the backport wasn't trivial. There's a cost/benefit tradeoff to be had for each vulnerability, and changes to the area making backports either be easy or hard always need to be weighed against the severity of the vulnerability. I don't see that changing, and overpromising hurts in the long run probably more than having no guidance. I just don't see us coming up with "proper answers" (which I'm guessing means fully spelled out answers?) to these questions ahead of time.
That's fair, I guess. I don't think we need to fully spell out the answer to each of these questions. But I think we should have some general alignment on how we'll handle the last non-Rust release, and what some guarantees are that we can and want to provide.
Quoted 7 lines
The answer to all of them is
probably "we'll weigh the severity of the issue and the cost to backport and give the last C-only version significant extra weight in our considerations". I doubt we'll ever be able to promise any more detail than that until we get concrete cases; I'm not even sure that this statement is acceptable to everyone on the list from the overpromising angle despite being as incomplete as it is.
True, we don't want to overburden us, either. This is mostly why I proposed the compromise of saying "We provide you with updates for the LTS version for X amount of time. If you still depend on it after that time, we will be happy to pass over maintainership of that branch to the community."
[snip]
Quoted 9 lines
I still don't see why distributors _must_ ship the latest version of Git and why folks on some platforms are considered broken if they are using a slightly older version. Let me ask again: has anyone answered why this is considered mandatory? If they have, I've missed it, but I've asked multiple times. Even if you want to lump "distributors cannot build a newer version" under the umbrella of "breaking changes", I argue it's a much different kind of break and one which merits different timelines for handling than e.g. lumping it in with 3.0.
To me it's not necessarily about the _latest_ version, rather about _any_ version. Some distributions will not be able to build Git at all anymore, so they are stuck at the last non-Rust version for the time being. And seeing that the timeline is years for them to get Rust support they may not be on a slightly older version, but on an ancient version eventually.
So the question to me is less whether users of that distro will miss out on new features, which I think is acceptable. Hence my statement that it is fine from my point of view for new features to be written in Rust immediately.
NB: There's some nuance here. If newer features mean that users cannot interact with modern upstreams anymore the picture would change quite significantly. But the only work that really comes to my mind is SHA256, which already exists. I don't know whether the interop code may fall into this category, I hope it doesn't.
But the bigger question is whether that old version still gets security updates and important bug fixes. If the only available non-Rust version is riddled with security holes then these distros won't be able to provide it at all anymore.
Quoted 10 lines
Quoted 9 lines
Quoted 4 lines
(4) you suggest that adding Rust as an optional component should avoid the problem, yet we've already had Rust as an optional component for the last three releases, going back to 2.49.0. (libgit-rs and libgit-sys).
I don't really think that either libgit-rs or libgit-sys help in any way. These are part of "contrib/", not built by default, and neither are they consumed by anyone out there. So there is no reason for anyone to build that library to the best of my knowledge.
Quoted 10 lines
I'm fully willing to accept they are inadequate notice (and perhaps even barely helpful), but disagree with the characterization that they don't help at all:
* they were consumed in the past by Google
* they recently received patches from someone outside Google
(https://lore.kernel.org/git/[email protected]/)
* they were mentioned in the release notes highlighting at a minimum
that Rust is being added to Git.
* they were highlighted in blog posts from both GitLab and GitHub as
being noteworthy new things in the v2.49.0 release
Okay, fair.
Quoted 6 lines
I agree with you there is certainly more we can do, and I like your idea of a test ballon. Let's just avoid repeating the problem of adding an optional component that no one will try to build except for those for whom we know can build it; doing that would provide no more notice and thus provide no incremental benefit over libgit-rs and libgit-sys.
Fully agreed. My current proposal includes several steps of how we tighten the screws here, where we gradually start to require Rust by default on more platforms. Before Git 3.0 it's still possible to opt out, but eventually distributors need to opt out explicitly. So that should hopefully alert them that something is cooking.
Patrick
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to Elijah NewrenHi Elijah
Quoted 7 lines · Elijah Newren
(1) "without advance notice" was already pointed out to be inaccurate in this thread, including in the exact email you are responding to; you could argue that there hasn't been _sufficient_ advance notice, but then there should be more details about what is and isn't sufficient. Merely repeating this claim which brian just barely pointed out to you as false almost feels dishonest.
I think there is a difference of understanding of what constitutes "advanced notice". While it is true that there have been discussions on the list for a couple of years where people were clearly enthusiastic about adopting rust those discussions have always petered out after concerns about portability were raised without us actually adopting rust. In those discussions there has been no clear conclusion about whether rust would be mandatory or optional. I think from the point of view of an outsider who was following the mailing list it has not been clear exactly where the rust discussion was going. For someone not following the mailing list but just reading the release notes there has been no indication that we're thinking of rust mandatory for building git as opposed to offering rust bindings for our C code.
Quoted 3 lines
(2) "pull the rug away" seems hyperbolic. I would have liked some explanation as to how a transition period is expected to help, and how the existing transition period has been insufficient.
I'm very unclear what "the existing transition period" has been
Quoted 4 lines
[...] > (4) you suggest that adding Rust as an optional component should avoid the problem, yet we've already had Rust as an optional component for the last three releases, going back to 2.49.0. (libgit-rs and libgit-sys).
Right but from the point of view of someone trying to build git on a platform without rust support there is a world of difference between having some optional bindings for rust external projects to use, and making rust mandatory to build git.
I would like us to adopt rust but I am concerned about the implications for platforms without rust and think we should give some notice in the form a clear announcement in the release notes once we have a concrete plan. That plan should include a decision on what commitment we can realistically offer with regard to security updates for platforms without a rust compiler so maintainers on those platforms have a clear idea of how long they will be supported.
Thanks
Phillip
Re: [PATCH v3 02/15] xdiff: introduce rust
Quoted 33 lines · Phillip Wood
Hi Elijah
Quoted 7 lines · Elijah Newren
(1) "without advance notice" was already pointed out to be inaccurate
in this thread, including in the exact email you are responding to; you could argue that there hasn't been _sufficient_ advance notice, but then there should be more details about what is and isn't sufficient. Merely repeating this claim which brian just barely pointed out to you as false almost feels dishonest.I think there is a difference of understanding of what constitutes "advanced notice". While it is true that there have been discussions on the list for a couple of years where people were clearly enthusiastic about adopting rust those discussions have always petered out after concerns about portability were raised without us actually adopting rust. In those discussions there has been no clear conclusion about whether rust would be mandatory or optional. I think from the point of view of an outsider who was following the mailing list it has not been clear exactly where the rust discussion was going. For someone not following the mailing list but just reading the release notes there has been no indication that we're thinking of rust mandatory for building git as opposed to offering rust bindings for our C code.
Quoted 3 lines
(2) "pull the rug away" seems hyperbolic. I would have liked some explanation as to how a transition period is expected to help, and how the existing transition period has been insufficient.
I'm very unclear what "the existing transition period" has been
Quoted 1 line
[...] > (4) you suggest that adding Rust as an optional component
should avoid
Quoted 8 lines
Quoted 3 lines
the problem, yet we've already had Rust as an optional component for the last three releases, going back to 2.49.0. (libgit-rs and libgit-sys).
Right but from the point of view of someone trying to build git on a platform without rust support there is a world of difference between having some optional bindings for rust external projects to use, and making rust mandatory to build git.
Entirely agreed with the whole email.
I'll make some further observations wrt bindings:
1) Distributions often don't enable bindings for languages unless a user requests them, or at the very least they're considered low priority (and bindings existing for a language in a project do *not* imply the project is going to be rewritten in that language);
2) There would be no value in distributions building Rust bindings because Rust doesn't really support "system-wide" libraries. It doesn't make sense as far as I can tell to install the bindings right now. I don't even know where Rust bindings should be installed to, I've never seen a package want them installed before.
3) It's not integrated with the Meson build system we're using so I wouldn't have paid any attention to it, at least unless a user requested it;
4) It's in contrib/.
Quoted 9 lines
I would like us to adopt rust but I am concerned about the implications for platforms without rust and think we should give some notice in the form a clear announcement in the release notes once we have a concrete plan. That plan should include a decision on what commitment we can realistically offer with regard to security updates for platforms without a rust compiler so maintainers on those platforms have a clear idea of how long they will be supported.
Quoted 3 lines
Thanks
Phillip
sam
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to Phillip WoodQuoted 2 lines · Phillip Wood
I would like us to adopt rust but I am concerned about the implications for platforms without rust and think we should give some notice in the form a clear announcement in the release notes once we have a concrete plan. That plan should include a decision on what commitment we can realistically offer with regard to security updates for platforms without a rust compiler so maintainers on those platforms have a clear idea of how long they will be supported.
Here's what such an announcement might look like
This release introduces an optional dependency on rust that is enabled by default. Platforms without a rust compiler can continue to build git by passing NO_RUST=1. In six months time we plan to make rust mandatory for building git. From that point git 2.x.y (the last version that can be built without rust) will continue to receive security updates for three years.
To me the important elements are:
1) There is a short period where rust is optional. This allows
(i) Distributors on platforms without a rust compiler time to notify their users that in the future they will only be able to offer security updates.
(ii) Distributors on platforms with a rust compiler time to adjust
their build procedures to include rust.
(iii) The git project time to gain experience of using rust and writing
the necessary bindings while building with it is optional.
2) Rust is enabled by default so platforms without a rust compiler are made aware of the problem but have an easy way to continue to build git while rust is optional.
3) There is a period of a small number of years where we continue to provide security updates for a version of git that can be built without rust. This is intended to allow a realistic time for distributors on platforms without a rust compiler to port one or make other arrangements for providing future security updates without placing an undue burden on the project to provide security updates for niche platforms indefinitely.
Thanks
Phillip
Re: [PATCH v3 02/15] xdiff: introduce rust
Quoted 34 lines · Phillip Wood
Quoted 8 lines · Phillip Wood
I would like us to adopt rust but I am concerned about the implications for platforms without rust and think we should give some notice in the form a clear announcement in the release notes once we have a concrete plan. That plan should include a decision on what commitment we can realistically offer with regard to security updates for platforms without a rust compiler so maintainers on those platforms have a clear idea of how long they will be supported.
Here's what such an announcement might look like
This release introduces an optional dependency on rust that is enabled by default. Platforms without a rust compiler can continue to build git by passing NO_RUST=1. In six months time we plan to make rust mandatory for building git. From that point git 2.x.y (the last version that can be built without rust) will continue to receive security updates for three years.
To me the important elements are:
1) There is a short period where rust is optional. This allows
(i) Distributors on platforms without a rust compiler time to notify their users that in the future they will only be able to offer security updates.
(ii) Distributors on platforms with a rust compiler time to adjust
their build procedures to include rust.
(iii) The git project time to gain experience of using rust and writing
the necessary bindings while building with it is optional.2) Rust is enabled by default so platforms without a rust compiler are made aware of the problem but have an easy way to continue to build git while rust is optional.
Quoted 7 lines
3) There is a period of a small number of years where we continue to provide security updates for a version of git that can be built without rust. This is intended to allow a realistic time for distributors on platforms without a rust compiler to port one or make other arrangements for providing future security updates without placing an undue burden on the project to provide security updates for niche platforms indefinitely.
Something like this is part of the BreakingChanges document I'm proposing in [1]. I think we should also highlight this upcoming change in the next release notes, with a pointer to that document.
Patrick
[1]: <[email protected]>
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to Phillip WoodQuoted 9 lines · Phillip Wood
This release introduces an optional dependency on rust that is enabled by default. Platforms without a rust compiler can continue to build git by passing NO_RUST=1. In six months time we plan to make rust mandatory for building git. From that point git 2.x.y (the last version that can be built without rust) will continue to receive security updates for three years.
To me the important elements are:
Quoted 8 lines
1) There is a short period where rust is optional. This allows
(i) Distributors on platforms without a rust compiler time to notify their users that in the future they will only be able to offer security updates.
(ii) Distributors on platforms with a rust compiler time to adjust
their build procedures to include rust.
(iii) The git project time to gain experience of using rust and writing
the necessary bindings while building with it is optional.
Good. I am not sure "short" should be an important element, but having a known and agreed-upon deadline helps.
Quoted 3 lines
2) Rust is enabled by default so platforms without a rust compiler are made aware of the problem but have an easy way to continue to build git while rust is optional.
Obviously there is nothing to disagree with here, as it is the definition of the word "optional" ;-).
Quoted 7 lines
3) There is a period of a small number of years where we continue to provide security updates for a version of git that can be built without rust. This is intended to allow a realistic time for distributors on platforms without a rust compiler to port one or make other arrangements for providing future security updates without placing an undue burden on the project to provide security updates for niche platforms indefinitely.
I am not willing to see such a support for multiple years, though. If the first item is 6 months, this backporting stale releases should be on the same order of timeperiod.
If it were "3 years of optional period, 18 months of backporting security updates", I would find it more realistic. It would give those platform maintainers enough time to robby, fundraise, or otherwise campaign to bring Rust on their system. I personally find that 6 months is way too short (if we are _only_ looking for an excuse to say "we have given them ample time to react, and now it is their problem", 6 months may be good enough, though).
Re: [PATCH v3 02/15] xdiff: introduce rust
Quoted 13 lines · Junio C Hamano
Quoted 7 lines · Phillip Wood
3) There is a period of a small number of years where we continue to provide security updates for a version of git that can be built without rust. This is intended to allow a realistic time for distributors on platforms without a rust compiler to port one or make other arrangements for providing future security updates without placing an undue burden on the project to provide security updates for niche platforms indefinitely.
I am not willing to see such a support for multiple years, though. If the first item is 6 months, this backporting stale releases should be on the same order of timeperiod.
Quoted 7 lines
If it were "3 years of optional period, 18 months of backporting security updates", I would find it more realistic. It would give those platform maintainers enough time to robby, fundraise, or otherwise campaign to bring Rust on their system. I personally find that 6 months is way too short (if we are _only_ looking for an excuse to say "we have given them ample time to react, and now it is their problem", 6 months may be good enough, though).
Yeah, I also think that six months is a bit short, but three years on the other hand feels like it will cause quite some pain on our side. My plan is shooting for roughly one year of optional support, which is still way shorter than the three years you mention.
How would you feel about:
- Pinning a date for Git 3.0 at the end of next year and tying mandatory Rust to it.
- Guaranteeing at least one year of security backports for 2.99 (or whatever the last release before Git 3.0 is).
- Explicitly stating that if anybody requires to maintain that version afterwards, we are happy to let the community maintain that branch going forward.
That means that we stop supporting 2.99 in a bit more than ~two years from now, but we don't fully close the door on it if people still rely on it.
Patrick
Re: [PATCH v3 02/15] xdiff: introduce rust
In reply to [email protected]Quoted 2 lines · On Sat, Aug 23, 2025 at 7:44 AM <[email protected]> wrote:
Does this introduce Rust as a mandatory dependency for git? If so, it cuts out numerous platforms.
Yes it does.
PATCH v3 3/15 github workflows: install rust
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Prefer using actions-rs/toolchain@v1 where possible to install rustup, but for docker targets use a script to install rustup. Consolidate the Rust toolchain definitions in main.yaml. Use install-rust-toolchain.sh to ensure the correct toolchain is used. Five overrides are used in main.yaml:
* On Windows: Rust didn't resolve the bcrypt library on Windows correctly until version 1.78.0. Also since rustup mis-identifies the Rust toolchain, the Rust target triple must be set to x86_64-pc-windows-gnu for make (win build), and x86_64-pc-windows-msvc for meson (win+Meson build).
* On musl: libc differences, such as ftruncate64 vs ftruncate, were not accounted for until Rust version 1.72.0. No older version of Rust will work on musl for our needs.
* In a 32-bit docker container running on a 64-bit host, we need to override the Rust target triple. This is because rustup asks the kernel for the bitness of the system and it says 64, even though the container is 32-bit. This also allows us to remove the BITNESS environment variable in ci/lib.sh.
The logic for selecting library names was initially provided in a patch from Johannes, but was reworked and squashed into this commit.
--- .github/workflows/main.yml | 61 +++++++++++++++++++++++++++++++++++- ci/install-dependencies.sh | 14 ++++----- ci/install-rust-toolchain.sh | 30 ++++++++++++++++++ ci/install-rustup.sh | 25 +++++++++++++++ ci/lib.sh | 1 + ci/make-test-artifacts.sh | 9 ++++++ ci/run-build-and-tests.sh | 13 ++++++++ 7 files changed, 145 insertions(+), 8 deletions(-) create mode 100755 ci/install-rust-toolchain.sh create mode 100755 ci/install-rustup.sh
.github/workflows/main.yml +60 -1
@@ -26,6 +26,13 @@ jobs: outputs: enabled: ${{ steps.check-ref.outputs.enabled }}${{ steps.skip-if-redundant.outputs.enabled }} skip_concurrent: ${{ steps.check-ref.outputs.skip_concurrent }} + rust_version_minimum: 1.61.0 + rust_version_windows: 1.78.0 + rust_version_musl: 1.72.0 + ## the rust target is inferred by rustup unless specified + rust_target_windows_make: x86_64-pc-windows-gnu + rust_target_windows_meson: x86_64-pc-windows-msvc + rust_target_32bit_linux: i686-unknown-linux-gnu steps: - name: try to clone ci-config branch run: |
@@ -108,12 +115,26 @@ jobs: needs: ci-config if: needs.ci-config.outputs.enabled == 'yes' runs-on: windows-latest + env: + CARGO_HOME: "/c/Users/runneradmin/.cargo" concurrency: group: windows-build-${{ github.ref }} cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }} steps: - uses: actions/checkout@v4 - uses: git-for-windows/setup-git-for-windows-sdk@v1 + - name: Install rustup via github actions + uses: actions-rs/toolchain@v1 + with: + toolchain: stable + profile: minimal + override: false + - name: Install Rust toolchain + shell: bash + env: + RUST_VERSION: ${{ needs.ci-config.outputs.rust_version_windows }} + RUST_TARGET: ${{ needs.ci-config.outputs.rust_target_windows_make }} + run: ci/install-rust-toolchain.sh - name: build shell: bash env:
@@ -254,12 +275,26 @@ jobs: needs: ci-config if: needs.ci-config.outputs.enabled == 'yes' runs-on: windows-latest + env: + CARGO_HOME: "/c/Users/runneradmin/.cargo" concurrency: group: windows-meson-build-${{ github.ref }} cancel-in-progress: ${{ needs.ci-config.outputs.skip_concurrent == 'yes' }} steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 + - name: Install rustup via github actions + uses: actions-rs/toolchain@v1 + with: + toolchain: stable + profile: minimal + override: false + - name: Install Rust toolchain + shell: bash + env: + RUST_VERSION: ${{ needs.ci-config.outputs.rust_version_windows }} + RUST_TARGET: ${{ needs.ci-config.outputs.rust_target_windows_meson }} + run: ci/install-rust-toolchain.sh - name: Set up dependencies shell: pwsh run: pip install meson ninja
@@ -329,11 +364,24 @@ jobs: jobname: ${{matrix.vector.jobname}} CI_JOB_IMAGE: ${{matrix.vector.pool}} TEST_OUTPUT_DIRECTORY: ${{github.workspace}}/t + CARGO_HOME: "/Users/runner/.cargo" runs-on: ${{matrix.vector.pool}} steps: - uses: actions/checkout@v4 - run: ci/install-dependencies.sh - - run: ci/run-build-and-tests.sh + - name: Install rustup via github actions + uses: actions-rs/toolchain@v1 + with: + toolchain: stable + profile: minimal + override: false + - name: Install Rust toolchain + shell: bash + env: + RUST_VERSION: ${{ needs.ci-config.outputs.rust_version_minimum }} + run: ci/install-rust-toolchain.sh + - name: Run build and tests + run: ci/run-build-and-tests.sh - name: print test failures if: failure() && env.FAILED_TEST_ARTIFACTS != '' run: ci/print-test-failures.sh
@@ -393,9 +441,11 @@ jobs: cc: gcc - jobname: linux-musl-meson image: alpine:latest + rust_version_override: ${{ needs.ci-config.outputs.rust_version_musl }} # Supported until 2025-04-02. - jobname: linux32 image: i386/ubuntu:focal + rust_target_override: ${{ needs.ci-config.outputs.rust_target_32bit_linux }} - jobname: pedantic image: fedora:latest # A RHEL 8 compatible distro. Supported until 2029-05-31.
@@ -408,7 +458,9 @@ jobs: jobname: ${{matrix.vector.jobname}} CC: ${{matrix.vector.cc}} CI_JOB_IMAGE: ${{matrix.vector.image}} + CI_IS_DOCKER: "true" CUSTOM_PATH: /custom + CARGO_HOME: /home/builder/.cargo runs-on: ubuntu-latest container: ${{matrix.vector.image}} steps:
@@ -433,6 +485,13 @@ jobs: - run: ci/install-dependencies.sh - run: useradd builder --create-home - run: chown -R builder . + - name: Install rustup via script + run: sudo --preserve-env --set-home --user=builder ci/install-rustup.sh + - name: Install Rust toolchain + env: + RUST_VERSION: ${{ matrix.vector.rust_version_override || needs.ci-config.outputs.rust_version_minimum }} + RUST_TARGET: ${{ matrix.vector.rust_target_override || '' }} + run: sudo --preserve-env --set-home --user=builder ci/install-rust-toolchain.sh - run: sudo --preserve-env --set-home --user=builder ci/run-build-and-tests.sh - name: print test failures if: failure() && env.FAILED_TEST_ARTIFACTS != ''
ci/install-dependencies.sh +7 -7
@@ -24,14 +24,14 @@ fi case "$distro" in alpine-*) - apk add --update shadow sudo meson ninja-build gcc libc-dev curl-dev openssl-dev expat-dev gettext \ + apk add --update shadow sudo meson ninja-build gcc libc-dev curl curl-dev openssl-dev expat-dev gettext \ zlib-ng-dev pcre2-dev python3 musl-libintl perl-utils ncurses \ apache2 apache2-http2 apache2-proxy apache2-ssl apache2-webdav apr-util-dbd_sqlite3 \ bash cvs gnupg perl-cgi perl-dbd-sqlite perl-io-tty >/dev/null ;; fedora-*|almalinux-*) dnf -yq update >/dev/null && - dnf -yq install shadow-utils sudo make gcc findutils diffutils perl python3 gawk gettext zlib-devel expat-devel openssl-devel curl-devel pcre2-devel >/dev/null + dnf -yq install shadow-utils sudo make gcc findutils diffutils perl python3 gawk gettext zlib-devel expat-devel openssl-devel curl curl-devel pcre2-devel >/dev/null ;; ubuntu-*|i386/ubuntu-*|debian-*) # Required so that apt doesn't wait for user input on certain packages.
@@ -55,8 +55,8 @@ ubuntu-*|i386/ubuntu-*|debian-*) sudo apt-get -q update sudo apt-get -q -y install \ $LANGUAGES apache2 cvs cvsps git gnupg $SVN \ - make libssl-dev libcurl4-openssl-dev libexpat-dev wget sudo default-jre \ - tcl tk gettext zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \ + make libssl-dev curl libcurl4-openssl-dev libexpat-dev wget sudo default-jre \ + tcl tk gettext zlib1g zlib1g-dev perl-modules liberror-perl libauthen-sasl-perl \ libemail-valid-perl libio-pty-perl libio-socket-ssl-perl libnet-smtp-ssl-perl libdbd-sqlite3-perl libcgi-pm-perl \ libsecret-1-dev libpcre2-dev meson ninja-build pkg-config \ ${CC_PACKAGE:-${CC:-gcc}} $PYTHON_PACKAGE
@@ -121,13 +121,13 @@ ClangFormat) ;; StaticAnalysis) sudo apt-get -q update - sudo apt-get -q -y install coccinelle libcurl4-openssl-dev libssl-dev \ + sudo apt-get -q -y install coccinelle curl libcurl4-openssl-dev libssl-dev \ libexpat-dev gettext make ;; sparse) sudo apt-get -q update -q - sudo apt-get -q -y install libssl-dev libcurl4-openssl-dev \ - libexpat-dev gettext zlib1g-dev sparse + sudo apt-get -q -y install libssl-dev curl libcurl4-openssl-dev \ + libexpat-dev gettext zlib1g zlib1g-dev sparse ;; Documentation) sudo apt-get -q update
ci/install-rust-toolchain.sh new file +30 -0
@@ -0,0 +1,30 @@ +#!/bin/sh + +if [ "$CARGO_HOME" = "" ]; then + echo >&2 "::error:: CARGO_HOME is not set" + exit 2 +fi +export PATH="$CARGO_HOME/bin:$PATH" +rustup -vV || exit $? + +## Enforce the correct Rust toolchain +rustup override unset || true + +## install a specific version of rust +if [ "$RUST_TARGET" != "" ]; then + rustup default --force-non-host "$RUST_VERSION-$RUST_TARGET" || exit $? +else + rustup default "$RUST_VERSION" || exit $? +fi + +rustc -vV || exit $? + +RE_RUST_TARGET="$RUST_TARGET" +if [ "$RUST_TARGET" = "" ]; then + RE_RUST_TARGET="[^ ]+" +fi + +if ! rustup show active-toolchain | grep -E "^$RUST_VERSION-$RE_RUST_TARGET \(default\)$"; then + echo >&2 "::error:: wrong Rust toolchain, active-toolchain: $(rustup show active-toolchain)" + exit 3 +fi
ci/install-rustup.sh new file +25 -0
@@ -0,0 +1,25 @@ +#!/bin/sh + +## github workflows actions-rs/toolchain@v1 doesn't work for docker +## targets. This script should only be used if the ci pipeline +## doesn't support installing rust on a particular target. + +if [ "$(id -u)" -eq 0 ]; then + echo >&2 "::warning:: installing rust as root" +fi + +if [ "$CARGO_HOME" = "" ]; then + echo >&2 "::error:: CARGO_HOME is not set" + exit 2 +fi + +export RUSTUP_HOME=$CARGO_HOME + +## install rustup +curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- --default-toolchain none -y +if [ ! -f $CARGO_HOME/env ]; then + echo "PATH=$CARGO_HOME/bin:\$PATH" > $CARGO_HOME/env +fi +. $CARGO_HOME/env + +rustup -vV
ci/lib.sh +1 -0
@@ -1,5 +1,6 @@ # Library of functions shared by all CI scripts + if test true = "$GITHUB_ACTIONS" then begin_group () {
ci/make-test-artifacts.sh +9 -0
@@ -7,6 +7,15 @@ mkdir -p "$1" # in case ci/lib.sh decides to quit early . ${0%/*}/lib.sh +## ensure rustup is in the PATH variable +if [ "$CARGO_HOME" = "" ]; then + echo >&2 "::error:: CARGO_HOME is not set" + exit 2 +fi +export PATH="$CARGO_HOME/bin:$PATH" + +rustc -vV + group Build make artifacts-tar ARTIFACTS_DIRECTORY="$1" check_unignored_build_artifacts
ci/run-build-and-tests.sh +13 -0
@@ -5,6 +5,15 @@ . ${0%/*}/lib.sh +## ensure rustup is in the PATH variable +if [ "$CARGO_HOME" = "" ]; then + echo >&2 "::error:: CARGO_HOME is not set" + exit 2 +fi +. $CARGO_HOME/env + +rustc -vV || exit $? + run_tests=t case "$jobname" in
@@ -72,5 +81,9 @@ case "$jobname" in ;; esac +if [ -d "$CARGO_HOME" ]; then + rm -rf $CARGO_HOME +fi + check_unignored_build_artifacts save_good_tree
Signature · 1 line
gitgitgadget
PATCH v3 4/15 win+Meson: do allow linking with the Rust-built xdiff
In reply to Ezekiel Newren via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
When linking against the Rust-built xdiff, there is now a new required
dependency: Without _also_ linking to the system library userenv, the compile would fail with this error message:
xdiff.lib(std-c85e9beb7923f636.std.df32d1bc89881d89-cgu.0.rcgu.o) :
error LNK2019: unresolved external symbol __imp_GetUserProfileDirectoryW referenced in function _ZN3std3env8home_dir17hfd1c3b6676cd78f6E
Therefore, just like we do in case of Makefile-based builds on Windows, we now also link to that library when building with Meson.
Note that if we only have Rust depend upon libuserenv then at link time GCC would complain about:
undefined reference to `GetUserProfileDirectoryW'
Apparently there is _some_ closure that gets compiled in that requires this function, and that in turn forces Git to link to libuserenv.
This is a new requirement, and therefore has not been made part of the "minimal Git for Windows SDK".
In the near future, I intend to include it, but for now let's just ensure that the file is added manually if it is missing.
[en: Squashed a few of Johannes's patches, and moved lib userenv
handling from an earlier patch]
--- .github/workflows/main.yml | 8 ++++++++ config.mak.uname | 4 ++++ meson.build | 1 + 3 files changed, 13 insertions(+)
.github/workflows/main.yml +8 -0
@@ -123,6 +123,14 @@ jobs: steps: - uses: actions/checkout@v4 - uses: git-for-windows/setup-git-for-windows-sdk@v1 + - name: ensure that libuserenv.a is present + shell: bash + run: | + cd /mingw64/lib && { + test -f libuserenv.a || + /c/Program\ Files/Git/mingw64/bin/curl -Lo libuserenv.a \ + https://github.com/git-for-windows/git-sdk-64/raw/HEAD/mingw64/lib/libuserenv.a + } - name: Install rustup via github actions uses: actions-rs/toolchain@v1 with:
config.mak.uname +4 -0
@@ -740,6 +740,10 @@ ifeq ($(uname_S),MINGW) COMPAT_CFLAGS += -D_USE_32BIT_TIME_T BASIC_LDFLAGS += -Wl,--large-address-aware endif + + # Unfortunately now needed because of Rust + EXTLIBS += -luserenv + CC = gcc COMPAT_CFLAGS += -D__USE_MINGW_ANSI_STDIO=0 -DDETECT_MSYS_TTY \ -fstack-protector-strong
meson.build +1 -0
@@ -1267,6 +1267,7 @@ elif host_machine.system() == 'windows' ] libgit_dependencies += compiler.find_library('ntdll') + libgit_dependencies += compiler.find_library('userenv') libgit_include_directories += 'compat/win32' if compiler.get_id() == 'msvc' libgit_include_directories += 'compat/vcbuild/include'
Signature · 1 line
gitgitgadget
PATCH v3 5/15 github workflows: upload Cargo.lock
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Make each ci workflow upload its Cargo.lock file as a build artifact so that we can audit build dependencies.
--- .github/workflows/main.yml | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
.github/workflows/main.yml +20 -0
@@ -156,6 +156,11 @@ jobs: with: name: windows-artifacts path: artifacts + - name: upload Cargo.lock + uses: actions/upload-artifact@v4 + with: + name: cargo-lock-windows + path: rust/Cargo.lock windows-test: name: win test runs-on: windows-latest
@@ -317,6 +322,11 @@ jobs: with: name: windows-meson-artifacts path: build + - name: Upload Cargo.lock + uses: actions/upload-artifact@v4 + with: + name: cargo-lock-windows-meson + path: rust/Cargo.lock windows-meson-test: name: win+Meson test runs-on: windows-latest
@@ -399,6 +409,11 @@ jobs: with: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} + - name: Upload Cargo.lock + uses: actions/upload-artifact@v4 + with: + name: cargo-lock-${{matrix.vector.jobname}} + path: rust/Cargo.lock fuzz-smoke-test: name: fuzz smoke test needs: ci-config
@@ -510,6 +525,11 @@ jobs: with: name: failed-tests-${{matrix.vector.jobname}} path: ${{env.FAILED_TEST_ARTIFACTS}} + - name: Upload Cargo.lock + uses: actions/upload-artifact@v4 + with: + name: cargo-lock-${{matrix.vector.jobname}} + path: rust/Cargo.lock static-analysis: needs: ci-config if: needs.ci-config.outputs.enabled == 'yes'
Signature · 1 line
gitgitgadget
PATCH v3 6/15 ivec: create a vector type that is interoperable between C and Rust
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Trying to use Rust's Vec in C, or git's ALLOC_GROW() macros (via wrapper functions) in Rust is painful because:
* C doing vector things the Rust way would require wrapper functions, and Rust doing vector things the C way would require wrapper functions, so ivec was created to ensure a consistent contract between the 2 languages for how to manipulate a vector.
* Currently, Rust defines its own 'Vec' type that is generic, but its memory allocator and struct layout weren't designed for interoperability with C (or any language for that matter), meaning that the C side cannot push to or expand a 'Vec' without defining wrapper functions in Rust that C can call. Without special care, the two languages might use different allocators (malloc/free on the C side, and possibly something else in Rust), which would make it difficult for a function in one language to free elements allocated by a call from a function in the other language.
* Similarly, git defines ALLOC_GROW() and related macros in git-compat-util.h. While we could add functions allowing Rust to invoke something similar to those macros, passing three variables (pointer, length, allocated_size) instead of a single variable (vector) across the language boundary requires more cognitive overhead for readers to keep track of and makes it easier to make mistakes. Further, for low-level components that we want to eventually convert to pure Rust, such triplets would feel very out of place.
To address these issue, introduce a new type, ivec -- short for interoperable vector. (We refer to it as 'ivec' generally, though on the Rust side the struct is called IVec to match Rust style.) This new type is specifically designed for FFI purposes, so that both languages handle the vector in the same way, though it could be used on either side independently. This type is designed such that it can easily be replaced by a standard Rust 'Vec' once interoperability is no longer a concern.
One particular item to note is that Git's macros to handle vec operations infer the amount that a vec needs to grow from the size of a pointer, but that makes it somewhat specific to the macros used in C. To avoid defining every ivec function as a macro I opted to also include an element_size field that allows concrete functions like push() to know how much to grow the memory. This element_size also helps in verifying that the ivec is correct when passing from C to Rust.
--- Makefile | 16 +- interop/ivec.c | 151 +++++++++++++ interop/ivec.h | 52 +++++ meson.build | 1 + rust/interop/src/ivec.rs | 462 +++++++++++++++++++++++++++++++++++++++ rust/interop/src/lib.rs | 10 + rust/xdiff/src/lib.rs | 1 + 7 files changed, 690 insertions(+), 3 deletions(-) create mode 100644 interop/ivec.c create mode 100644 interop/ivec.h create mode 100644 rust/interop/src/ivec.rs
Makefile +13 -3
@@ -672,6 +672,7 @@ BUILTIN_OBJS = BUILT_INS = COMPAT_CFLAGS = COMPAT_OBJS = +INTEROP_OBJS = XDIFF_OBJS = GENERATED_H = EXTRA_CPPFLAGS =
@@ -918,6 +919,7 @@ export PYTHON_PATH TEST_SHELL_PATH = $(SHELL_PATH) LIB_FILE = libgit.a +INTEROP_LIB = interop/lib.a XDIFF_LIB = xdiff/lib.a EXTLIBS =
@@ -1412,7 +1414,7 @@ UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/test-lib.o UNIT_TEST_OBJS += $(UNIT_TEST_DIR)/lib-reftable.o # xdiff and reftable libs may in turn depend on what is in libgit.a -GITLIBS = common-main.o $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE) +GITLIBS = common-main.o $(LIB_FILE) $(INTEROP_LIB) $(XDIFF_LIB) $(REFTABLE_LIB) $(LIB_FILE) GIT_USER_AGENT = git/$(GIT_VERSION)
@@ -2747,6 +2749,10 @@ reconfigure config.mak.autogen: config.status .PHONY: reconfigure # This is a convenience target. endif +INTEROP_OBJS += interop/ivec.o +.PHONY: interop-objs +interop-objs: $(INTEROP_OBJS) + XDIFF_OBJS += xdiff/xdiffi.o XDIFF_OBJS += xdiff/xemit.o XDIFF_OBJS += xdiff/xhistogram.o
@@ -2791,6 +2797,7 @@ OBJECTS += $(GIT_OBJS) OBJECTS += $(SCALAR_OBJS) OBJECTS += $(PROGRAM_OBJS) OBJECTS += $(TEST_OBJS) +OBJECTS += $(INTEROP_OBJS) OBJECTS += $(XDIFF_OBJS) OBJECTS += $(FUZZ_OBJS) OBJECTS += $(REFTABLE_OBJS) $(REFTABLE_TEST_OBJS)
@@ -2945,7 +2952,10 @@ scalar$X: scalar.o GIT-LDFLAGS $(GITLIBS) compile_rust $(LIB_FILE): $(LIB_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ -$(XDIFF_LIB): $(XDIFF_OBJS) +$(INTEROP_LIB): $(INTEROP_OBJS) + $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^ + +$(XDIFF_LIB): $(INTEROP_OBJS) $(XDIFF_OBJS) $(QUIET_AR)$(RM) $@ && $(AR) $(ARFLAGS) $@ $^
@@ -3790,7 +3800,7 @@ clean: profile-clean coverage-clean cocciclean rustclean $(RM) git.rc git.res $(RM) $(OBJECTS) $(RM) headless-git.o - $(RM) $(LIB_FILE) $(XDIFF_LIB) $(REFTABLE_LIB) + $(RM) $(LIB_FILE) $(INTEROP_LIB) $(XDIFF_LIB) $(REFTABLE_LIB) $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) $(OTHER_PROGRAMS) $(RM) $(TEST_PROGRAMS) $(RM) $(FUZZ_PROGRAMS)
interop/ivec.c new file +151 -0
151 lines not on this page · raw message
interop/ivec.h new file +52 -0
52 lines not on this page · raw message
meson.build +1 -0
@@ -395,6 +395,7 @@ libgit_sources = [ 'hex-ll.c', 'hook.c', 'ident.c', + 'interop/ivec.c', 'json-writer.c', 'kwset.c', 'levenshtein.c',
rust/interop/src/ivec.rs new file +462 -0
462 lines not on this page · raw message
rust/interop/src/lib.rs +10 -0
@@ -0,0 +1,10 @@ +pub mod ivec; + +use std::ffi::c_void; + +extern "C" { +pub fn malloc (size:usize ) -> *mut c_void ; +pub fn calloc (nmemb:usize , size:usize ) -> *mut c_void ; +pub fn realloc (ptr: *mut c_void , size:usize ) -> *mut c_void ; +pub fn free (ptr: *mut c_void ); +}
rust/xdiff/src/lib.rs +1 -0
@@ -0,0 +1 @@ +
Signature · 1 line
gitgitgadget
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 4 lines · Ezekiel Newren via GitGitGadget
From: Ezekiel Newren <[email protected]>
Trying to use Rust's Vec in C, or git's
ALLOC_GROW()macros (via wrapper functions) in Rust is painful because:
nit: s/git's/Git's/
rust/interop/src/ivec.rs
Quoted 4 lines
[snip]
rust/interop/src/ivec.rs +1
-0+// assert_eq!(vec.capacity, vec.slice.len());
Why are there three commented-out assertions? (all capacity/length)
Quoted 6 lines
rust/interop/src/ivec.rs +5
-0+assert_eq! (expected, vec.length); +assert! (vec.capacity >= expected); +for iin 0 ..vec.length { +assert_eq! (default_value, vec[i]); + }
Signature · 1 line
Kristoffer Haugsbakk
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 23 lines · Kristoffer Haugsbakk
Quoted 4 lines · Ezekiel Newren via GitGitGadget
From: Ezekiel Newren <[email protected]>
Trying to use Rust's Vec in C, or git's
ALLOC_GROW()macros (via wrapper functions) in Rust is painful because:nit: s/git's/Git's/
rust/interop/src/ivec.rs
Quoted 4 lines
[snip]
rust/interop/src/ivec.rs +1
-0+// assert_eq!(vec.capacity, vec.slice.len()); Why are there three commented-out assertions? (all capacity/length)
rust/interop/src/ivec.rs line 425
Quoted 6 lines
rust/interop/src/ivec.rs +5
-0+assert_eq! (expected, vec.length); +assert! (vec.capacity >= expected); +for iin 0 ..vec.length { +assert_eq! (default_value, vec[i]); + }
Quoted 2 lines
--
Kristoffer Haugsbakk
Good catch, I should have removed those commented out lines. Looking back through the code I also missed calling std::ptr::drop_in_place() if the IVec shrinks. I'll apply those changes in the next version.
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
In reply to Ezekiel Newren via GitGitGadgetrust/xdiff/src/lib.rs line 1
Quoted 6 lines · Ezekiel Newren via GitGitGadget
rust/xdiff/src/lib.rs +1
-0@@ -0,0 +1 @@ +
This triggers an "new blank line at EOF" whitespace error while applying. Intended?
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 12 lines · Junio C Hamano
rust/xdiff/src/lib.rs line 1
Quoted 6 lines · Ezekiel Newren via GitGitGadget
rust/xdiff/src/lib.rs +1
-0@@ -0,0 +1 @@ +This triggers an "new blank line at EOF" whitespace error while applying. Intended?
"new blank line at EOF" is intentional, but it is showing up in the wrong place in this patch series. Cargo format automatically creates a blank line for empty files. These warnings should have shown up on the "xdiff: introduce rust" commit. I will fix this.
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
In reply to Ezekiel Newren via GitGitGadgetQuoted 5 lines · Ezekiel Newren via GitGitGadget
From: Ezekiel Newren <[email protected]>
Trying to use Rust's Vec in C, or git's
ALLOC_GROW()macros (via wrapper functions) in Rust is painful because:
Quoted 10 lines
* C doing vector things the Rust way would require wrapper functions, and Rust doing vector things the C way would require wrapper
...
* Currently, Rust defines its own 'Vec' type that is generic, but its memory allocator and struct layout weren't designed for interoperability with C (or any language for that matter), meaning
...
* Similarly, git definesALLOC_GROW()and related macros ingit-compat-util.h. While we could add functions allowing Rust to
...
All the good reasons any C (or any non-Rust language for that matter) projects would want to have an interoperability Shim for their dynamically allocated and grown array-like things.
Quoted 3 lines
To address these issue, introduce a new type, ivec -- short for interoperable vector. (We refer to it as 'ivec' generally, though on the Rust side the struct is called IVec to match Rust style.)
I however was hoping by now Rust getting used more widely, somebody has already created a generic "this is how you make C-array and Rust vectors interoperate" wrapper that latecomer projects like us can use without inventing our own.
Makefile reconfigure config.mak.autogen: config.status · line 2754
Quoted 3 lines
Makefile +3
-0+INTEROP_OBJS += interop/ivec.o +.PHONY: interop-objs +interop-objs: $(INTEROP_OBJS)
What is this phony target used for? No other targets seem to depend on this one (I am wondering if we need the latter two lines).
interop/ivec.c
Quoted 5 lines
interop/ivec.c new file +0
-0
I am wondering if this needs a new hierarchy "interop"; shouldn't the existing "compat" be a good fit enough? I dunno.
Even though this is a shim to somebody else's code, it still is a part of our codebase, so our CodingGuidelines for C programs should apply.
interop/ivec.c line 4
Quoted 5 lines
interop/ivec.c new file +4
-0@@ -0,0 +1,151 @@ +#include "ivec.h" + +static void ivec_set_capacity (void * self,usize new_capacity) { +struct rawivec *this = self;
- Asterisk sticks to the variable, not type.
- The opening and closing {braces} for the function body are written at the leftmost column on its own line.
- There should be a blank line between the declarations and the first statement.
Quoted 4 lines
interop/ivec.c new file +4
-0+if (new_capacity ==0 ) +FREE_AND_NULL (this->ptr); +else + this->ptr =xrealloc (this->ptr, new_capacity * this->element_size);
interop/ivec.c line 18
Quoted 10 lines
interop/ivec.c new file +10
-0+ this->capacity = new_capacity; +} + +void ivec_init (void * self,usize element_size) { +struct rawivec *this = self; + this->ptr =NULL ; + this->length =0 ; + this->capacity =0 ; + this->element_size = element_size; +}
I notice that this reintroduces a variable named "this", which was eradicated in 585c0e2e (diff: rename 'this' variables, 2018-02-14).
I do not think those who want to use C++ compilers on our C code would not mind "self", so how about doing something like...
void ivec_init(void *self_, usize element_size)
{
struct rawivec *self = self;
self->ptr = NULL;
self->len = 0;
self->capacity = 0;
self->element_size = element_size;
}... perhaps?
interop/ivec.h line 4
Quoted 10 lines
interop/ivec.h new file +4
-0@@ -0,0 +1,52 @@ +#ifndef IVEC_H +#define IVEC_H + +#include "../git-compat-util.h"
As we use -I. on the command line, there is no need to add "../" here; just writing
#include <git-compat-util.h>
should be enough. Also, if this file does not depend on the services compat-util header provides (and I do not think it does from a brief look at its contents), it is better not to include it.
Instead, the sources (like ivec.c next door we just saw) should begin themselves with #include of git-compat-util.h header before including ivec.h.
rust/xdiff/src/lib.rs line 1
Quoted 6 lines
rust/xdiff/src/lib.rs +1
-0@@ -0,0 +1 @@ +
If this empty line in an otherwise empty file is absolutely necessary to make Rust work, then please arrange .gitattributes to tell git that this file is excempt from the usual blank-at-eof whitespace rule we use. If not, remove that unnecessary empty line.
Or perhaps remove the file altogether if nobody looks at it???
In any case, given that our top-level .gitattributes file starts with
* whitespace=!indent,trail,space *.[ch] whitespace=indent,trail,space diff=cpp *.sh whitespace=indent,trail,space text eol=lf
...
*.bat text eol=crlf
CODE_OF_CONDUCT.md -whitespace
...
I think a new rule to cover "*.rs" and perhaps *.toml files right before rules for each specific file begin would be in order.
Thanks.
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 8 lines · Junio C Hamano
Quoted 3 lines
To address these issue, introduce a new type, ivec -- short for interoperable vector. (We refer to it as 'ivec' generally, though on the Rust side the struct is called IVec to match Rust style.)
I however was hoping by now Rust getting used more widely, somebody has already created a generic "this is how you make C-array and Rust vectors interoperate" wrapper that latecomer projects like us can use without inventing our own.
I've looked for code that will do what Git needs, but as far as I know nothing can do everything that my ivec can.
Quoted 6 lines
Makefile reconfigure config.mak.autogen: config.status · line 2754
Quoted 3 lines
Makefile +3
-0+INTEROP_OBJS += interop/ivec.o +.PHONY: interop-objs +interop-objs: $(INTEROP_OBJS)What is this phony target used for? No other targets seem to depend on this one (I am wondering if we need the latter two lines).
You are correct. I will remove those lines.
Quoted 8 lines
interop/ivec.c
Quoted 5 lines
interop/ivec.c new file +0
-0I am wondering if this needs a new hierarchy "interop"; shouldn't the existing "compat" be a good fit enough? I dunno.
I had considered compat/, but I thought it didn’t fit. I thought it meant that the same API would exist everywhere, as opposed to “We speak different languages, but we’ve agreed on a translator or common protocol”. In particular, an example from ivec, a function on both the C and Rust sides:
void ivec_extend_from_slice(void *_self, void const *ptr, usize size); pub fn extend_from_slice(&mut self, slice: &[T]) where T: Clone,
The Rust side uses a slice or “fat” pointer, where the C side uses two arguments (a pointer and a size) in its place. The API is different, even if semantically they are the same and they are interoperable.
Was I reading too much into the meaning of compat/? Do folks object to using interop/?
Quoted 3 lines
Even though this is a shim to somebody else's code, it still is a part of our codebase, so our CodingGuidelines for C programs should apply.
Sorry I missed those. I will fix them up.
Quoted 8 lines
interop/ivec.c line 4
Quoted 5 lines
interop/ivec.c new file +4
-0@@ -0,0 +1,151 @@ +#include "ivec.h" + +static void ivec_set_capacity (void * self,usize new_capacity) { +struct rawivec *this = self;- Asterisk sticks to the variable, not type.
Quoted 5 lines
- The opening and closing {braces} for the function body are written at the leftmost column on its own line.
- There should be a blank line between the declarations and the first statement.
I will make those changes.
Quoted 32 lines
Quoted 4 lines
interop/ivec.c new file +4
-0+if (new_capacity ==0 ) +FREE_AND_NULL (this->ptr); +else + this->ptr =xrealloc (this->ptr, new_capacity * this->element_size);interop/ivec.c line 18
Quoted 10 lines
interop/ivec.c new file +10
-0+ this->capacity = new_capacity; +} + +void ivec_init (void * self,usize element_size) { +struct rawivec *this = self; + this->ptr =NULL ; + this->length =0 ; + this->capacity =0 ; + this->element_size = element_size; +}I notice that this reintroduces a variable named "this", which was eradicated in
585c0e2e(diff: rename 'this' variables, 2018-02-14).I do not think those who want to use C++ compilers on our C code would not mind "self", so how about doing something like...
void ivec_init(void *self_, usize element_size) { struct rawivec *self = self; self->ptr = NULL; self->len = 0; self->capacity = 0; self->element_size = element_size; }
Quoted 1 line
... perhaps?
That sounds good. I will make those changes.
Quoted 14 lines
interop/ivec.h line 4
Quoted 10 lines
interop/ivec.h new file +4
-0@@ -0,0 +1,52 @@ +#ifndef IVEC_H +#define IVEC_H + +#include "../git-compat-util.h" As we use -I. on the command line, there is no need to add "../" here; just writing
Quoted 5 lines
#include <git-compat-util.h>should be enough. Also, if this file does not depend on the services compat-util header provides (and I do not think it does from a brief look at its contents), it is better not to include it.
This file actually does depend on git-compat-util.h, particularly the Rust primitive definitions (e.g. usize, u64, etc...).
I'll use the include style you mentioned.
Quoted 25 lines
rust/xdiff/src/lib.rs line 1
Quoted 6 lines
rust/xdiff/src/lib.rs +1
-0@@ -0,0 +1 @@ +If this empty line in an otherwise empty file is absolutely necessary to make Rust work, then please arrange .gitattributes to tell git that this file is excempt from the usual blank-at-eof whitespace rule we use. If not, remove that unnecessary empty line.
Or perhaps remove the file altogether if nobody looks at it???
In any case, given that our top-level .gitattributes file starts with
* whitespace=!indent,trail,space *.[ch] whitespace=indent,trail,space diff=cpp *.sh whitespace=indent,trail,space text eol=lf
...
*.bat text eol=crlf
CODE_OF_CONDUCT.md -whitespace
...
Quoted 2 lines
I think a new rule to cover "*.rs" and perhaps *.toml files right before rules for each specific file begin would be in order.
This is only a problem for empty files, and we only have empty .rs files to setup the basic Rust build in this commit. It's not a problem later in this series and shouldn't be a problem in the future. I will remove those blank lines from those commits.
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
In reply to Junio C HamanoQuoted 9 lines · Junio C Hamano
interop/ivec.c
Quoted 5 lines
interop/ivec.c new file +0
-0Even though this is a shim to somebody else's code, it still is a part of our codebase, so our CodingGuidelines for C programs should apply.
Sorry, I should have caught these in my preliminary review before he sent this off to the list. One question, though...
Quoted 14 lines
interop/ivec.h line 4
Quoted 10 lines
interop/ivec.h new file +4
-0@@ -0,0 +1,52 @@ +#ifndef IVEC_H +#define IVEC_H + +#include "../git-compat-util.h" As we use -I. on the command line, there is no need to add "../" here; just writing
Quoted 5 lines
#include <git-compat-util.h>should be enough. Also, if this file does not depend on the services compat-util header provides (and I do not think it does from a brief look at its contents), it is better not to include it.
Should this rather be
#include "git-compat-util.h"
with quotes rather than angle brackets? In particular:
$ git grep include.*git-compat-util -- '*.[ch]' | wc -l 362 $ git grep include.*git-compat-util -- '*/*.[ch]' | wc -l 125
So, we have 362 includes of git-compat-util.h in our codebase, 125 from subdirectories. Of those:
$ git grep include.*git-compat-util -- '*.[ch]' | grep '"' | wc -l 361 $ git grep include.*git-compat-util -- '*.[ch]' | grep '<' | wc -l 1
Only one of these include statements uses angle brackets -- the compiler-tricks/not-constant.c file (which appears to be a temporary hack that we'll eventually delete). I had always assumed <> were for system includes and "" for project includes, but a quick Google search shows the actual situation is quite a bit murkier than I'd realized. Still, our current project practice appears to be double quotes; is that fine here or are you suggesting you'd like the current project practice to be changed?
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 7 lines · Elijah Newren
Quoted 7 lines
Quoted 1 line
+#include "
../git-compat-util.h"As we use -I. on the command line, there is no need to add "../" here; just writing
#include <git-compat-util.h>
Quoted 7 lines
Quoted 3 lines
should be enough. Also, if this file does not depend on the services compat-util header provides (and I do not think it does from a brief look at its contents), it is better not to include it.
Should this rather be
#include "git-compat-util.h"
I meant <>; when "" included header is not found, it falls back as if it were <> included, IIRC, so writing <> when you specify exactly where your headers are with -I. avoids such unnecessary fallback in theory, but as both <> and "" search for implementation-defined places, the distinction does not make much practical difference.
Quoted 3 lines
Still, our current project practice appears to be double quotes; is that fine here or are you suggesting you'd like the current project practice to be changed?
It would be nice if we could do so, but I do not think it is worth the patch churn.
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
In reply to Ezekiel Newren via GitGitGadgetQuoted 22 lines · Ezekiel Newren via GitGitGadget
From: Ezekiel Newren <[email protected]>
Trying to use Rust's Vec in C, or git's
ALLOC_GROW()macros (via wrapper functions) in Rust is painful because:* C doing vector things the Rust way would require wrapper functions, and Rust doing vector things the C way would require wrapper functions, so ivec was created to ensure a consistent contract between the 2 languages for how to manipulate a vector.
* Currently, Rust defines its own 'Vec' type that is generic, but its memory allocator and struct layout weren't designed for interoperability with C (or any language for that matter), meaning that the C side cannot push to or expand a 'Vec' without defining wrapper functions in Rust that C can call. Without special care, the two languages might use different allocators (malloc/free on the C side, and possibly something else in Rust), which would make it difficult for a function in one language to free elements allocated by a call from a function in the other language.
* Similarly, git definesALLOC_GROW()and related macros in
Quoted 8 lines
git-compat-util.h. While we could add functions allowing Rust to invoke something similar to those macros, passing three variables (pointer, length,allocated_size) instead of a single variable (vector) across the language boundary requires more cognitive overhead for readers to keep track of and makes it easier to make mistakes. Further, for low-level components that we want to eventually convert to pure Rust, such triplets would feel very out of place.
I’m mildly surprised Vec isn’t a good fit: isn’t it a pointer, length, capacity triple? But it sounds like the main issue is allocator interop… which I would also have thought was supported? At least the current version is documented as being generic against an Allocator, too.
Quoted 9 lines
To address these issue, introduce a new type, ivec -- short for interoperable vector. (We refer to it as 'ivec' generally, though on the Rust side the struct is called IVec to match Rust style.) This new type is specifically designed for FFI purposes, so that both languages handle the vector in the same way, though it could be used on either side independently. This type is designed such that it can easily be replaced by a standard Rust 'Vec' once interoperability is no longer a concern.
Am I reading the patch correctly that the ivec implementation is primarily C? I’m not familiar with too many FFI projects in Rust, but I might have hoped we could write parts in Rust to gain any benefits from that, too. Is that a fool’s errand I’m thinking of?
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 1 line · Ben Knoble
I’m mildly surprised Vec isn’t a good fit: isn’t it a pointer, length, capacity triple? But it sounds like the main issue is allocator interop… which I would also have thought was supported? At least the current version is documented as being generic against an Allocator, too.
Conceptually yes, semantically and syntactically no. On top of Vec<T> not being defined with #[repr(C)] (which ensures field order, C ABI layout, padding, etc...) the struct definition for Vec isn't constant between Rust versions. I'd be open to suggestions for an alternative to my ivec type.
=== Rust version 1.61.0 ===
from: https://doc.rust-lang.org/1.61.0/src/alloc/vec/mod.rs.html#400
#[stable(feature = "rust1", since = "1.0.0")] #[cfg_attr(not(test), rustc_diagnostic_item = "Vec")] #[rustc_insignificant_dtor]
pub struct Vec<T, #[unstable(feature = "allocator_api", issue =
"32838")] A: Allocator = Global> {
buf: RawVec<T, A>,
len: usize,
}from: https://doc.rust-lang.org/1.61.0/src/alloc/raw_vec.rs.html#52
#[allow(missing_debug_implementations)]
pub(crate) struct RawVec<T, A: Allocator = Global> {
ptr: Unique<T>,
cap: usize,
alloc: A,
}=== Rust version 1.89.0 ===
from: https://doc.rust-lang.org/1.89.0/src/alloc/vec/mod.rs.html#414
#[stable(feature = "rust1", since = "1.0.0")] #[rustc_diagnostic_item = "Vec"] #[rustc_insignificant_dtor]
pub struct Vec<T, #[unstable(feature = "allocator_api", issue =
"32838")] A: Allocator = Global> {
buf: RawVec<T, A>,
len: usize,
}from: https://doc.rust-lang.org/1.89.0/src/alloc/raw_vec/mod.rs.html#74
#[allow(missing_debug_implementations)]
pub(crate) struct RawVec<T, A: Allocator = Global> {
inner: RawVecInner<A>,
_marker: PhantomData<T>,
}
from: https://doc.rust-lang.org/1.89.0/src/alloc/raw_vec/mod.rs.html#86
#[allow(missing_debug_implementations)]
struct RawVecInner<A: Allocator = Global> {
ptr: Unique<u8>,
/// Never used for ZSTs; it's `capacity()`'s responsibility to
return usize::MAX in that case.
///
/// # Safety
///
/// `cap` must be in the `0..=isize::MAX` range.
cap: Cap,
alloc: A,
}Quoted 1 line
Am I reading the patch correctly that the ivec implementation is primarily C? I’m not familiar with too many FFI projects in Rust, but I might have hoped we could write parts in Rust to gain any benefits from that, too. Is that a fool’s errand I’m thinking of?
The ivec type is defined and implemented in C (interop/ivec.[ch]) and Rust (rust/interop/src/ivec.rs). When I started writing the ivec type I didn't know if the Git community would accept a hard dependency on Rust, so I made ivec usable in C without needing Rust.
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 9 lines · Ezekiel Newren
Quoted 1 line · Ben Knoble
I’m mildly surprised Vec isn’t a good fit: isn’t it a pointer, length, capacity triple? But it sounds like the main issue is allocator interop… which I would also have thought was supported? At least the current version is documented as being generic against an Allocator, too.
Conceptually yes, semantically and syntactically no. On top of Vec<T> not being defined with #[repr(C)] (which ensures field order, C ABI layout, padding, etc...) the struct definition for Vec isn't constant between Rust versions. I'd be open to suggestions for an alternative to my ivec type.
Ah, thanks—I had forgotten about the #[repr(C)] needs and changes. Makes sense.
Quoted 6 lines
Quoted 1 line
Am I reading the patch correctly that the ivec implementation is primarily C? I’m not familiar with too many FFI projects in Rust, but I might have hoped we could write parts in Rust to gain any benefits from that, too. Is that a fool’s errand I’m thinking of?
The ivec type is defined and implemented in C (interop/ivec.[ch]) and Rust (
rust/interop/src/ivec.rs). When I started writing the ivec type I didn't know if the Git community would accept a hard dependency on Rust, so I made ivec usable in C without needing Rust.
Right—I saw both implementations, but it looked like C did most of the work, which was my main question. Re-reading, it looks like Rust does more work than I thought (with implementations of insert/push/etc.)
That said, I think it's sensible to leave the type useable from just C unless/until Rust becomes required (and then we can move things over).
Thanks!
Signature · 1 line
D. Ben Knoble
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 7 lines · D. Ben Knoble
Quoted 6 lines
Quoted 1 line
Am I reading the patch correctly that the ivec implementation is primarily C? I’m not familiar with too many FFI projects in Rust, but I might have hoped we could write parts in Rust to gain any benefits from that, too. Is that a fool’s errand I’m thinking of?
The ivec type is defined and implemented in C (interop/ivec.[ch]) and Rust (
rust/interop/src/ivec.rs). When I started writing the ivec type I didn't know if the Git community would accept a hard dependency on Rust, so I made ivec usable in C without needing Rust.
Quoted 6 lines
Right—I saw both implementations, but it looked like C did most of the work, which was my main question. Re-reading, it looks like Rust does more work than I thought (with implementations of insert/push/etc.)
That said, I think it's sensible to leave the type useable from just C unless/until Rust becomes required (and then we can move things over).
I like your idea of implementation consolidation. I just don't know what that would look like yet.
It's not straightforward because C doesn't have generics. I'll use IVec as an example, but this applies to any generic type in Rust. For a function like push() in IVec<T> it will have N definitions if there are N IVec types. e.g. If your code uses IVec<u64>, IVec<u8>, IVec<i32> that would mean that pub fn push(&mut self) {} would compile to 3 functions. If you don't use #[no_mangle] you'd have to figure out the Rust compiler's exact behavior for function names when calling it from C, which isn't stable or easily predictable. If you do use #[no_mangle] then the Rust compiler can't generate a generic function for each type.
Another problem is that the functions in ivec mostly deal with resizing the memory rather than controlling access to memory for the C side. Even if the C side used Rust defined functions, that wouldn't solve memory access issues to the pointer on the C side. We could enforce access to each element by requiring C to call a Rust defined function for each element, but that sounds very painful and slow. ivec is meant to be used as a scaffolding type to help transition C to Rust.
Other projects that do use Rust's builtin Vec (or some other collection type) often Box it and write wrapper functions. This means that the C side sees an opaque void* instead of a transparent struct like ivec with ptr, length, capacity, and element_size.
I'm curious if the community has more design feedback, or suggestions for an alternative to my ivec type.
Re: [PATCH v3 06/15] ivec: create a vector type that is interoperable between C and Rust
Quoted 2 lines · Ezekiel Newren
I'm curious if the community has more design feedback, or suggestions for an alternative to my ivec type.
I think this is a fine approach. We used a similar Vec<u8>-like structure when porting a service from C to Rust at $DAYJOB, but we've also used the boxing approach to good success.
Ultimately, I think boxing is the right choice when we don't need access from C. For instance, if I were to convert the loose object mapping code to Rust, I would store the data as a pair of Box<HashMap<_, _>> and then attach them to struct repository as void *.
But it's less definitive when you need access from C. Because of the way we intimately mess with the internals of our data structures in this codebase, the ivec is probably the right choice here, so I'd keep that.
Signature · 2 lines
brian m. carlson (they/them) Toronto, Ontario, CA
PATCH v3 7/15 xdiff/xprepare: remove superfluous forward declarations
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Move xdl_prepare_env() later in the file to avoid the need for forward declarations.
--- xdiff/xprepare.c | 116 ++++++++++++++++++++--------------------------- 1 file changed, 50 insertions(+), 66 deletions(-)
xdiff/xprepare.c +50 -66
@@ -53,21 +53,6 @@ typedef struct s_xdlclassifier { -static int xdl_init_classifier (xdlclassifier_t *cf,long size,long flags); -static void xdl_free_classifier (xdlclassifier_t *cf); -static int xdl_classify_record (unsigned int pass,xdlclassifier_t *cf,xrecord_t **rhash, -unsigned int hbits,xrecord_t *rec); -static int xdl_prepare_ctx (unsigned int pass,mmfile_t *mf,long narec,xpparam_t const *xpp, -xdlclassifier_t *cf,xdfile_t *xdf); -static void xdl_free_ctx (xdfile_t *xdf); -static int xdl_clean_mmatch (char const *dis,long i,long s,long e); -static int xdl_cleanup_records (xdlclassifier_t *cf,xdfile_t *xdf1,xdfile_t *xdf2); -static int xdl_trim_ends (xdfile_t *xdf1,xdfile_t *xdf2); -static int xdl_optimize_ctxs (xdlclassifier_t *cf,xdfile_t *xdf1,xdfile_t *xdf2); - - - -static int xdl_init_classifier (xdlclassifier_t *cf,long size,long flags) { cf->flags = flags;
@@ -242,57 +227,6 @@ static void xdl_free_ctx(xdfile_t *xdf) { } -int xdl_prepare_env (mmfile_t *mf1,mmfile_t *mf2,xpparam_t const *xpp, -xdfenv_t *xe) { -long enl1, enl2, sample; -xdlclassifier_t cf; - -memset (&cf,0 ,sizeof (cf)); - -/* -* For histogram diff, we can afford a smaller sample size and -* thus a poorer estimate of the number of lines, as the hash -* table (rhash) won't be filled up/grown. The number of lines -* (nrecs) will be updated correctly anyway by -* xdl_prepare_ctx(). -*/ - sample = (XDF_DIFF_ALG (xpp->flags) ==XDF_HISTOGRAM_DIFF - ?XDL_GUESS_NLINES2 :XDL_GUESS_NLINES1 ); - - enl1 =xdl_guess_lines (mf1, sample) +1 ; - enl2 =xdl_guess_lines (mf2, sample) +1 ; - -if (xdl_init_classifier (&cf, enl1 + enl2 +1 , xpp->flags) <0 ) -return -1 ; - -if (xdl_prepare_ctx (1 , mf1, enl1, xpp, &cf, &xe->xdf1) <0 ) { - -xdl_free_classifier (&cf); -return -1 ; - } -if (xdl_prepare_ctx (2 , mf2, enl2, xpp, &cf, &xe->xdf2) <0 ) { - -xdl_free_ctx (&xe->xdf1); -xdl_free_classifier (&cf); -return -1 ; - } - -if ((XDF_DIFF_ALG (xpp->flags) !=XDF_PATIENCE_DIFF ) && - (XDF_DIFF_ALG (xpp->flags) !=XDF_HISTOGRAM_DIFF ) && -xdl_optimize_ctxs (&cf, &xe->xdf1, &xe->xdf2) <0 ) { - -xdl_free_ctx (&xe->xdf2); -xdl_free_ctx (&xe->xdf1); -xdl_free_classifier (&cf); -return -1 ; - } - -xdl_free_classifier (&cf); - -return 0 ; -} - -void xdl_free_env (xdfenv_t *xe) {xdl_free_ctx (&xe->xdf2);
@@ -460,3 +394,53 @@ static int xdl_optimize_ctxs(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdf2return 0 ; } + +int xdl_prepare_env (mmfile_t *mf1,mmfile_t *mf2,xpparam_t const *xpp, +xdfenv_t *xe) { +long enl1, enl2, sample; +xdlclassifier_t cf; + +memset (&cf,0 ,sizeof (cf)); + +/* +* For histogram diff, we can afford a smaller sample size and +* thus a poorer estimate of the number of lines, as the hash +* table (rhash) won't be filled up/grown. The number of lines +* (nrecs) will be updated correctly anyway by +* xdl_prepare_ctx(). +*/ + sample = (XDF_DIFF_ALG (xpp->flags) ==XDF_HISTOGRAM_DIFF + ?XDL_GUESS_NLINES2 :XDL_GUESS_NLINES1 ); + + enl1 =xdl_guess_lines (mf1, sample) +1 ; + enl2 =xdl_guess_lines (mf2, sample) +1 ; + +if (xdl_init_classifier (&cf, enl1 + enl2 +1 , xpp->flags) <0 ) +return -1 ; + +if (xdl_prepare_ctx (1 , mf1, enl1, xpp, &cf, &xe->xdf1) <0 ) { + +xdl_free_classifier (&cf); +return -1 ; + } +if (xdl_prepare_ctx (2 , mf2, enl2, xpp, &cf, &xe->xdf2) <0 ) { + +xdl_free_ctx (&xe->xdf1); +xdl_free_classifier (&cf); +return -1 ; + } + +if ((XDF_DIFF_ALG (xpp->flags) !=XDF_PATIENCE_DIFF ) && + (XDF_DIFF_ALG (xpp->flags) !=XDF_HISTOGRAM_DIFF ) && +xdl_optimize_ctxs (&cf, &xe->xdf1, &xe->xdf2) <0 ) { + +xdl_free_ctx (&xe->xdf2); +xdl_free_ctx (&xe->xdf1); +xdl_free_classifier (&cf); +return -1 ; + } + +xdl_free_classifier (&cf); + +return 0 ; +}
Signature · 1 line
gitgitgadget
PATCH v3 8/15 xdiff: delete unnecessary fields from xrecord_t and xdfile_t
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
xrecord_t.next, xdfile_t.hbits, xdfile_t.rhash are initialized, but never used for anything by the code. Remove them.
--- xdiff/xprepare.c | 24 +++--------------------- xdiff/xtypes.h | 3 --- 2 files changed, 3 insertions(+), 24 deletions(-)
xdiff/xprepare.c +3 -21
@@ -91,8 +91,7 @@ static void xdl_free_classifier(xdlclassifier_t *cf) { } -static int xdl_classify_record (unsigned int pass,xdlclassifier_t *cf,xrecord_t **rhash, -unsigned int hbits,xrecord_t *rec) { +static int xdl_classify_record (unsigned int pass,xdlclassifier_t *cf,xrecord_t *rec) {long hi;char const *line;xdlclass_t *rcrec;
@@ -126,23 +125,17 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t rec->ha = (unsigned long ) rcrec->idx; - hi = (long )XDL_HASHLONG (rec->ha, hbits); - rec->next = rhash[hi]; - rhash[hi] = rec; -return 0 ; }static int xdl_prepare_ctx (unsigned int pass,mmfile_t *mf,long narec,xpparam_t const *xpp,xdlclassifier_t *cf,xdfile_t *xdf) { -unsigned int hbits; -long nrec, hsize, bsize; +long nrec, bsize;unsigned long hav;char const *blk, *cur, *top, *prev;xrecord_t *crec;xrecord_t **recs; -xrecord_t **rhash;unsigned long *ha;char *rchg;long *rindex;
@@ -150,7 +143,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ ha =NULL ; rindex =NULL ; rchg =NULL ; - rhash =NULL ; recs =NULL ;if (xdl_cha_init (&xdf->rcha,sizeof (xrecord_t), narec /4 +1 ) <0 )
@@ -158,11 +150,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_if (!XDL_ALLOC_ARRAY (recs, narec)) goto abort; - hbits =xdl_hashbits ((unsigned int ) narec); - hsize =1 << hbits; -if (!XDL_CALLOC_ARRAY (rhash, hsize)) - goto abort; - nrec =0 ;if ((cur = blk =xdl_mmfile_first (mf, &bsize))) {for (top = blk + bsize; cur < top; ) {
@@ -176,7 +163,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ crec->size = (long ) (cur - prev); crec->ha = hav; recs[nrec++] = crec; -if (xdl_classify_record (pass, cf, rhash, hbits, crec) <0 ) +if (xdl_classify_record (pass, cf, crec) <0 ) goto abort; } }
@@ -194,8 +181,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ xdf->nrec = nrec; xdf->recs = recs; - xdf->hbits = hbits; - xdf->rhash = rhash; xdf->rchg = rchg +1 ; xdf->rindex = rindex; xdf->nreff =0 ;
@@ -209,7 +194,6 @@ abort:xdl_free (ha);xdl_free (rindex);xdl_free (rchg); -xdl_free (rhash);xdl_free (recs);xdl_cha_free (&xdf->rcha);return -1 ;
@@ -217,8 +201,6 @@ abort:static void xdl_free_ctx (xdfile_t *xdf) { - -xdl_free (xdf->rhash);xdl_free (xdf->rindex);xdl_free (xdf->rchg -1 );xdl_free (xdf->ha);
xdiff/xtypes.h +0 -3
@@ -39,7 +39,6 @@ typedef struct s_chastore { } chastore_t; typedef struct s_xrecord { - struct s_xrecord *next; char const *ptr; long size; unsigned long ha;
@@ -48,8 +47,6 @@ typedef struct s_xrecord { typedef struct s_xdfile { chastore_t rcha; long nrec; - unsigned int hbits; - xrecord_t **rhash; long dstart, dend; xrecord_t **recs; char *rchg;
Signature · 1 line
gitgitgadget
PATCH v3 9/15 xdiff: make fields of xrecord_t Rust friendly
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
A few commits ago, we added definitions for Rust primitive types, to facilitate interoperability between C and Rust. Switch a few variables to use these types. Which, for now, will require adding some casts.
Also change xdlclass_t::ha to be u64 to match xrecord_t::ha, as pointed out by Johannes.
--- xdiff/xdiffi.c | 8 ++++---- xdiff/xemit.c | 2 +- xdiff/xmerge.c | 14 +++++++------- xdiff/xpatience.c | 2 +- xdiff/xprepare.c | 8 ++++---- xdiff/xtypes.h | 6 +++--- xdiff/xutils.c | 4 ++-- 7 files changed, 22 insertions(+), 22 deletions(-)
xdiff/xdiffi.c +4 -4
@@ -418,7 +418,7 @@ static int get_indent(xrecord_t *rec)long i;int ret =0 ; -for (i =0 ; i < rec->size; i++) { +for (i =0 ; i < (long ) rec->size; i++) {char c = rec->ptr[i];if (!XDL_ISSPACE (c))
@@ -1005,11 +1005,11 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags) rec = &xe->xdf1.recs[xch->i1];for (i =0 ; i < xch->chg1 && ignore; i++) - ignore =xdl_blankline (rec[i]->ptr, rec[i]->size, flags); + ignore =xdl_blankline ((const char *) rec[i]->ptr, rec[i]->size, flags); rec = &xe->xdf2.recs[xch->i2];for (i =0 ; i < xch->chg2 && ignore; i++) - ignore =xdl_blankline (rec[i]->ptr, rec[i]->size, flags); + ignore =xdl_blankline ((const char *)rec[i]->ptr, rec[i]->size, flags); xch->ignore = ignore; }
@@ -1020,7 +1020,7 @@ static int record_matches_regex(xrecord_t *rec, xpparam_t const *xpp) {size_t i;for (i =0 ; i < xpp->ignore_regex_nr; i++) -if (!regexec_buf (xpp->ignore_regex[i], rec->ptr, rec->size,1 , +if (!regexec_buf (xpp->ignore_regex[i], (const char *) rec->ptr, rec->size,1 , ®match,0 ))return 1 ;
xdiff/xemit.c +1 -1
@@ -24,7 +24,7 @@static long xdl_get_rec (xdfile_t *xdf,long ri,char const **rec) { - *rec = xdf->recs[ri]->ptr; + *rec = (char const *) xdf->recs[ri]->ptr;return xdf->recs[ri]->size; }
xdiff/xmerge.c +7 -7
@@ -101,8 +101,8 @@ static int xdl_merge_cmp_lines(xdfenv_t *xe1, int i1, xdfenv_t *xe2, int i2,xrecord_t **rec2 = xe2->xdf2.recs + i2;for (i =0 ; i < line_count; i++) { -int result =xdl_recmatch (rec1[i]->ptr, rec1[i]->size, - rec2[i]->ptr, rec2[i]->size, flags); +int result =xdl_recmatch ((const char *) rec1[i]->ptr, rec1[i]->size, + (const char *) rec2[i]->ptr, rec2[i]->size, flags);if (!result)return -1 ; }
@@ -324,8 +324,8 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,static int recmatch (xrecord_t *rec1,xrecord_t *rec2,unsigned long flags) { -return xdl_recmatch (rec1->ptr, rec1->size, - rec2->ptr, rec2->size, flags); +return xdl_recmatch ((char const *) rec1->ptr, rec1->size, + (char const *) rec2->ptr, rec2->size, flags); }/*
@@ -383,10 +383,10 @@ static int xdl_refine_conflicts(xdfenv_t *xe1, xdfenv_t *xe2, xdmerge_t *m,*/ t1.ptr = (char *)xe1->xdf2.recs[m->i1]->ptr; t1.size = xe1->xdf2.recs[m->i1 + m->chg1 -1 ]->ptr - + xe1->xdf2.recs[m->i1 + m->chg1 -1 ]->size - t1.ptr; + + xe1->xdf2.recs[m->i1 + m->chg1 -1 ]->size - (u8 const *) t1.ptr; t2.ptr = (char *)xe2->xdf2.recs[m->i2]->ptr; t2.size = xe2->xdf2.recs[m->i2 + m->chg2 -1 ]->ptr - + xe2->xdf2.recs[m->i2 + m->chg2 -1 ]->size - t2.ptr; + + xe2->xdf2.recs[m->i2 + m->chg2 -1 ]->size - (u8 const *) t2.ptr;if (xdl_do_diff (&t1, &t2, xpp, &xe) <0 )return -1 ; if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
@@ -440,7 +440,7 @@ static int line_contains_alnum(const char *ptr, long size)static int lines_contain_alnum (xdfenv_t *xe,int i,int chg) {for (; chg; chg--, i++) -if (line_contains_alnum (xe->xdf2.recs[i]->ptr, +if (line_contains_alnum ((char const *) xe->xdf2.recs[i]->ptr, xe->xdf2.recs[i]->size))return 1 ;return 0 ;
xdiff/xpatience.c +1 -1
@@ -121,7 +121,7 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,return ; map->entries[index].line1 = line; map->entries[index].hash = record->ha; - map->entries[index].anchor =is_anchor (xpp, map->env->xdf1.recs[line -1 ]->ptr); + map->entries[index].anchor =is_anchor (xpp, (const char *) map->env->xdf1.recs[line -1 ]->ptr);if (!map->first) map->first = map->entries + index;if (map->last) {
xdiff/xprepare.c +4 -4
@@ -32,7 +32,7 @@ typedef struct s_xdlclass { struct s_xdlclass *next; - unsigned long ha; + u64 ha; char const *line; long size; long idx;
@@ -96,12 +96,12 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_tchar const *line;xdlclass_t *rcrec; - line = rec->ptr; + line = (char const *) rec->ptr; hi = (long )XDL_HASHLONG (rec->ha, cf->hbits);for (rcrec = cf->rchash[hi]; rcrec; rcrec = rcrec->next)if (rcrec->ha == rec->ha &&xdl_recmatch (rcrec->line, rcrec->size, - rec->ptr, rec->size, cf->flags)) + (const char *) rec->ptr, rec->size, cf->flags))break ;if (!rcrec) {
@@ -159,7 +159,7 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ goto abort;if (!(crec =xdl_cha_alloc (&xdf->rcha))) goto abort; - crec->ptr = prev; + crec->ptr = (u8 const *) prev; crec->size = (long ) (cur - prev); crec->ha = hav; recs[nrec++] = crec;
xdiff/xtypes.h +3 -3
@@ -39,9 +39,9 @@ typedef struct s_chastore { } chastore_t;typedef struct s_xrecord { -char const *ptr; -long size; -unsigned long ha; +u8 const * ptr; +usize size; +u64 ha; }xrecord_t ; typedef struct s_xdfile {
xdiff/xutils.c +2 -2
@@ -418,10 +418,10 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp, subfile1.ptr = (char *)diff_env->xdf1.recs[line1 -1 ]->ptr; subfile1.size = diff_env->xdf1.recs[line1 + count1 -2 ]->ptr + - diff_env->xdf1.recs[line1 + count1 -2 ]->size - subfile1.ptr; + diff_env->xdf1.recs[line1 + count1 -2 ]->size - (u8 const *) subfile1.ptr; subfile2.ptr = (char *)diff_env->xdf2.recs[line2 -1 ]->ptr; subfile2.size = diff_env->xdf2.recs[line2 + count2 -2 ]->ptr + - diff_env->xdf2.recs[line2 + count2 -2 ]->size - subfile2.ptr; + diff_env->xdf2.recs[line2 + count2 -2 ]->size - (u8 const *) subfile2.ptr;if (xdl_do_diff (&subfile1, &subfile2, xpp, &env) <0 )return -1 ;
Signature · 1 line
gitgitgadget
PATCH v3 10/15 xdiff: use one definition for freeing xdfile_t
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Simplify xdl_prepare_ctx() by using xdl_free_ctx() instead of using local variables with hand rolled memory management.
--- xdiff/xprepare.c | 60 +++++++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 36 deletions(-)
xdiff/xprepare.c +24 -36
@@ -129,86 +129,74 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_t } +static void xdl_free_ctx (xdfile_t *xdf) { +xdl_free (xdf->rindex); +xdl_free (xdf->rchg -1 ); +xdl_free (xdf->ha); +xdl_free (xdf->recs); +xdl_cha_free (&xdf->rcha); +} + +static int xdl_prepare_ctx (unsigned int pass,mmfile_t *mf,long narec,xpparam_t const *xpp,xdlclassifier_t *cf,xdfile_t *xdf) { -long nrec, bsize; +long bsize;unsigned long hav;char const *blk, *cur, *top, *prev;xrecord_t *crec; -xrecord_t **recs; -unsigned long *ha; -char *rchg; -long *rindex; - ha =NULL ; - rindex =NULL ; - rchg =NULL ; - recs =NULL ; + xdf->ha =NULL ; + xdf->rindex =NULL ; + xdf->rchg =NULL ; + xdf->recs =NULL ; + xdf->nrec =0 ;if (xdl_cha_init (&xdf->rcha,sizeof (xrecord_t), narec /4 +1 ) <0 ) goto abort; -if (!XDL_ALLOC_ARRAY (recs, narec)) +if (!XDL_ALLOC_ARRAY (xdf->recs, narec)) goto abort; - nrec =0 ;if ((cur = blk =xdl_mmfile_first (mf, &bsize))) {for (top = blk + bsize; cur < top; ) { prev = cur; hav =xdl_hash_record (&cur, top, xpp->flags); -if (XDL_ALLOC_GROW (recs, nrec +1 , narec)) +if (XDL_ALLOC_GROW (xdf->recs, xdf->nrec +1 , narec)) goto abort;if (!(crec =xdl_cha_alloc (&xdf->rcha))) goto abort; crec->ptr = (u8 const *) prev; crec->size = (long ) (cur - prev); crec->ha = hav; - recs[nrec++] = crec; + xdf->recs[xdf->nrec++] = crec;if (xdl_classify_record (pass, cf, crec) <0 ) goto abort; } } -if (!XDL_CALLOC_ARRAY (rchg, nrec +2 )) +if (!XDL_CALLOC_ARRAY (xdf->rchg, xdf->nrec +2 )) goto abort;if ((XDF_DIFF_ALG (xpp->flags) !=XDF_PATIENCE_DIFF ) && (XDF_DIFF_ALG (xpp->flags) !=XDF_HISTOGRAM_DIFF )) { -if (!XDL_ALLOC_ARRAY (rindex, nrec +1 )) +if (!XDL_ALLOC_ARRAY (xdf->rindex, xdf->nrec +1 )) goto abort; -if (!XDL_ALLOC_ARRAY (ha, nrec +1 )) +if (!XDL_ALLOC_ARRAY (xdf->ha, xdf->nrec +1 )) goto abort; } - xdf->nrec = nrec; - xdf->recs = recs; - xdf->rchg = rchg +1 ; - xdf->rindex = rindex; + xdf->rchg +=1 ; xdf->nreff =0 ; - xdf->ha = ha; xdf->dstart =0 ; - xdf->dend = nrec -1 ; + xdf->dend = xdf->nrec -1 ;return 0 ; abort: -xdl_free (ha); -xdl_free (rindex); -xdl_free (rchg); -xdl_free (recs); -xdl_cha_free (&xdf->rcha); +xdl_free_ctx (xdf);return -1 ; } -static void xdl_free_ctx (xdfile_t *xdf) { -xdl_free (xdf->rindex); -xdl_free (xdf->rchg -1 ); -xdl_free (xdf->ha); -xdl_free (xdf->recs); -xdl_cha_free (&xdf->rcha); -} - -void xdl_free_env (xdfenv_t *xe) {xdl_free_ctx (&xe->xdf2);
Signature · 1 line
gitgitgadget
PATCH v3 11/15 xdiff: replace chastore with an ivec in xdfile_t
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
xdfile_t currently uses a chastore which functions as a memory pool and a vector which maps to the alocations created by the chastore. It seems like xrecord_t used to be a linked list until the recs and nrec fields were added. I think that xrecord_t.next was meant to be removed, but was overlooked. This dual data structure setup make the code somewhat confusing.
Additionally the C type chastore_t isn't FFI friendly. While it could be implemented in Rust, since the data structure is confusing anyway, replace it with an ivec whose purpose is to be interoperable. This makes the fields nrec and recs in xdfile_t redundant, which will be removed in the next 2 commits.
--- xdiff/xprepare.c | 34 +++++++++++++++++----------------- xdiff/xtypes.h | 6 ++++-- 2 files changed, 21 insertions(+), 19 deletions(-)
xdiff/xprepare.c +17 -17
@@ -130,11 +130,11 @@ static int xdl_classify_record(unsigned int pass, xdlclassifier_t *cf, xrecord_tstatic void xdl_free_ctx (xdfile_t *xdf) { +ivec_free (&xdf->record);xdl_free (xdf->rindex);xdl_free (xdf->rchg -1 );xdl_free (xdf->ha);xdl_free (xdf->recs); -xdl_cha_free (&xdf->rcha); }
@@ -143,35 +143,35 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_long bsize;unsigned long hav;char const *blk, *cur, *top, *prev; -xrecord_t *crec; xdf->ha =NULL ; xdf->rindex =NULL ; xdf->rchg =NULL ; xdf->recs =NULL ; xdf->nrec =0 ; - -if (xdl_cha_init (&xdf->rcha,sizeof (xrecord_t), narec /4 +1 ) <0 ) - goto abort; -if (!XDL_ALLOC_ARRAY (xdf->recs, narec)) - goto abort; +IVEC_INIT (xdf->record);if ((cur = blk =xdl_mmfile_first (mf, &bsize))) {for (top = blk + bsize; cur < top; ) { +xrecord_t crec; prev = cur; hav =xdl_hash_record (&cur, top, xpp->flags); -if (XDL_ALLOC_GROW (xdf->recs, xdf->nrec +1 , narec)) - goto abort; -if (!(crec =xdl_cha_alloc (&xdf->rcha))) - goto abort; - crec->ptr = (u8 const *) prev; - crec->size = (long ) (cur - prev); - crec->ha = hav; - xdf->recs[xdf->nrec++] = crec; -if (xdl_classify_record (pass, cf, crec) <0 ) - goto abort; + crec.ptr = (u8 const *) prev; + crec.size = cur - prev; + crec.ha = hav; +ivec_push (&xdf->record, &crec); } } +ivec_shrink_to_fit (&xdf->record); + + xdf->nrec = (long ) xdf->record.length; +if (!XDL_ALLOC_ARRAY (xdf->recs, xdf->record.length)) + goto abort; +for (usize i =0 ; i < xdf->record.length; i++) { +if (xdl_classify_record (pass, cf, &xdf->record.ptr[i]) <0 ) + goto abort; + xdf->recs[i] = &xdf->record.ptr[i]; + }if (!XDL_CALLOC_ARRAY (xdf->rchg, xdf->nrec +2 )) goto abort;
xdiff/xtypes.h +4 -2
@@ -23,7 +23,7 @@ #if !defined(XTYPES_H) #define XTYPES_H - +#include "../interop/ivec.h" typedef struct s_chanode { struct s_chanode *next;
@@ -44,8 +44,10 @@ typedef struct s_xrecord {u64 ha; } xrecord_t; +DEFINE_IVEC_TYPE (xrecord_t, xrecord); + typedef struct s_xdfile { - chastore_t rcha; + struct ivec_xrecord record; long nrec; long dstart, dend; xrecord_t **recs;
Signature · 1 line
gitgitgadget
PATCH v3 12/15 xdiff: delete nrec field from xdfile_t
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Because of the data structure cleanup in the previous commit, the nrec field is no longer necessary. Use record.length in place of nrec.
This commit is best viewed with --color-words.
--- xdiff/xdiffi.c | 10 +++++----- xdiff/xemit.c | 20 ++++++++++---------- xdiff/xmerge.c | 10 +++++----- xdiff/xpatience.c | 2 +- xdiff/xprepare.c | 34 ++++++++++++++++------------------ xdiff/xtypes.h | 1 - 6 files changed, 37 insertions(+), 40 deletions(-)
xdiff/xdiffi.c +5 -5
@@ -496,7 +496,7 @@ static void measure_split(const xdfile_t *xdf, long split, {long i; -if (split >= xdf->nrec) { +if (split >= (long ) xdf->record.length) { m->end_of_file =1 ; m->indent =-1 ; }else {
@@ -519,7 +519,7 @@ static void measure_split(const xdfile_t *xdf, long split, m->post_blank =0 ; m->post_indent =-1 ; -for (i = split +1 ; i < xdf->nrec; i++) { +for (i = split +1 ; i < (long ) xdf->record.length; i++) { m->post_indent =get_indent (xdf->recs[i]);if (m->post_indent !=-1 )break ;
@@ -730,7 +730,7 @@ static void group_init(xdfile_t *xdf, struct xdlgroup *g)*/ static inline int group_next (xdfile_t *xdf,struct xdlgroup *g) { -if (g->end == xdf->nrec) +if (g->end == (long ) xdf->record.length)return -1 ; g->start = g->end +1 ;
@@ -763,7 +763,7 @@ static inline int group_previous(xdfile_t *xdf, struct xdlgroup *g)*/ static int group_slide_down (xdfile_t *xdf,struct xdlgroup *g) { -if (g->end < xdf->nrec && +if (g->end < (long ) xdf->record.length &&recs_match (xdf->recs[g->start], xdf->recs[g->end])) { xdf->rchg[g->start++] =0 ; xdf->rchg[g->end++] =1 ;
@@ -950,7 +950,7 @@ int xdl_build_script(xdfenv_t *xe, xdchange_t **xscr) {/* * Trivial. Collects "groups" of changes and creates an edit script. */ -for (i1 = xe->xdf1.nrec, i2 = xe->xdf2.nrec; i1 >=0 || i2 >=0 ; i1--, i2--) +for (i1 = xe->xdf1.record.length, i2 = xe->xdf2.record.length; i1 >=0 || i2 >=0 ; i1--, i2--)if (rchg1[i1 -1 ] || rchg2[i2 -1 ]) {for (l1 = i1; rchg1[i1 -1 ]; i1--);for (l2 = i2; rchg2[i2 -1 ]; i2--);
xdiff/xemit.c +10 -10
@@ -147,7 +147,7 @@ static long get_func_line(xdfenv_t *xe, xdemitconf_t const *xecfg, buf = func_line ? func_line->buf : dummy; size = func_line ?sizeof (func_line->buf) :sizeof (dummy); -for (l = start; l != limit &&0 <= l && l < xe->xdf1.nrec; l += step) { +for (l = start; l != limit &&0 <= l && l < (long ) xe->xdf1.record.length; l += step) {long len =match_func_rec (&xe->xdf1, xecfg, l, buf, size);if (len >=0 ) {if (func_line)
@@ -191,14 +191,14 @@ pre_context_calculation:long fs1, i1 = xch->i1;/* Appended chunk? */ -if (i1 >= xe->xdf1.nrec) { +if (i1 >= (long ) xe->xdf1.record.length) {long i2 = xch->i2;/* * We don't need additional context if * a whole function was added. */ -while (i2 < xe->xdf2.nrec) { +while (i2 < (long ) xe->xdf2.record.length) {if (is_func_rec (&xe->xdf2, xecfg, i2)) goto post_context_calculation; i2++;
@@ -208,7 +208,7 @@ pre_context_calculation:* Otherwise get more context from the * pre-image. */ - i1 = xe->xdf1.nrec -1 ; + i1 = xe->xdf1.record.length -1 ; } fs1 =get_func_line (xe, xecfg,NULL , i1,-1 );
@@ -240,8 +240,8 @@ pre_context_calculation: post_context_calculation: lctx = xecfg->ctxlen; - lctx =XDL_MIN (lctx, xe->xdf1.nrec - (xche->i1 + xche->chg1)); - lctx =XDL_MIN (lctx, xe->xdf2.nrec - (xche->i2 + xche->chg2)); + lctx =XDL_MIN (lctx, (long ) xe->xdf1.record.length - (xche->i1 + xche->chg1)); + lctx =XDL_MIN (lctx, (long ) xe->xdf2.record.length - (xche->i2 + xche->chg2)); e1 = xche->i1 + xche->chg1 + lctx; e2 = xche->i2 + xche->chg2 + lctx;
@@ -249,13 +249,13 @@ pre_context_calculation:if (xecfg->flags &XDL_EMIT_FUNCCONTEXT ) {long fe1 =get_func_line (xe, xecfg,NULL , xche->i1 + xche->chg1, - xe->xdf1.nrec); + xe->xdf1.record.length);while (fe1 >0 &&is_empty_rec (&xe->xdf1, fe1 -1 )) fe1--;if (fe1 <0 ) - fe1 = xe->xdf1.nrec; + fe1 = xe->xdf1.record.length;if (fe1 > e1) { - e2 =XDL_MIN (e2 + (fe1 - e1), xe->xdf2.nrec); + e2 =XDL_MIN (e2 + (fe1 - e1), (long ) xe->xdf2.record.length); e1 = fe1; }
@@ -266,7 +266,7 @@ pre_context_calculation:*/ if (xche->next) {long l =XDL_MIN (xche->next->i1, - xe->xdf1.nrec -1 ); + (long ) xe->xdf1.record.length -1 );if (l - xecfg->ctxlen <= e1 ||get_func_line (xe, xecfg,NULL , l, e1) <0 ) { xche = xche->next;
xdiff/xmerge.c +5 -5
@@ -158,11 +158,11 @@ static int is_eol_crlf(xdfile_t *file, int i) {long size; -if (i < file->nrec -1 ) +if (i < (isize ) file->record.length -1 )/* All lines before the last *must* end in LF */ return (size = file->recs[i]->size) >1 && file->recs[i]->ptr[size -2 ] =='\r' ; -if (!file->nrec) +if (!file->record.length)/* Cannot determine eol style from empty file */ return -1 ; if ((size = file->recs[i]->size) &&
@@ -317,7 +317,7 @@ static int xdl_fill_merge_buffer(xdfenv_t *xe1, const char *name1,continue ; i = m->i1 + m->chg1; } - size +=xdl_recs_copy (xe1, i, xe1->xdf2.nrec - i,0 ,0 , + size +=xdl_recs_copy (xe1, i, (int ) xe1->xdf2.record.length - i,0 ,0 , dest ? dest + size :NULL );return size; }
@@ -622,7 +622,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1, changes = c; i0 = xscr1->i1; i1 = xscr1->i2; - i2 = xscr1->i1 + xe2->xdf2.nrec - xe2->xdf1.nrec; + i2 = xscr1->i1 + xe2->xdf2.record.length - xe2->xdf1.record.length; chg0 = xscr1->chg1; chg1 = xscr1->chg2; chg2 = xscr1->chg1;
@@ -637,7 +637,7 @@ static int xdl_do_merge(xdfenv_t *xe1, xdchange_t *xscr1,if (!changes) changes = c; i0 = xscr2->i1; - i1 = xscr2->i1 + xe1->xdf2.nrec - xe1->xdf1.nrec; + i1 = xscr2->i1 + xe1->xdf2.record.length - xe1->xdf1.record.length; i2 = xscr2->i2; chg0 = xscr2->chg1; chg1 = xscr2->chg1;
xdiff/xpatience.c +1 -1
@@ -370,5 +370,5 @@ static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,int xdl_do_patience_diff (xpparam_t const *xpp,xdfenv_t *env) { -return patience_diff (xpp, env,1 , env->xdf1.nrec,1 , env->xdf2.nrec); +return patience_diff (xpp, env,1 , env->xdf1.record.length,1 , env->xdf2.record.length); }
xdiff/xprepare.c +16 -18
@@ -138,7 +138,7 @@ static void xdl_free_ctx(xdfile_t *xdf) { } -static int xdl_prepare_ctx (unsigned int pass,mmfile_t *mf,long narec,xpparam_t const *xpp, +static int xdl_prepare_ctx (unsigned int pass,mmfile_t *mf,xpparam_t const *xpp,xdlclassifier_t *cf,xdfile_t *xdf) {long bsize;unsigned long hav;
@@ -148,7 +148,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ xdf->rindex =NULL ; xdf->rchg =NULL ; xdf->recs =NULL ; - xdf->nrec =0 ;IVEC_INIT (xdf->record);if ((cur = blk =xdl_mmfile_first (mf, &bsize))) {
@@ -164,7 +163,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ }ivec_shrink_to_fit (&xdf->record); - xdf->nrec = (long ) xdf->record.length;if (!XDL_ALLOC_ARRAY (xdf->recs, xdf->record.length)) goto abort;for (usize i =0 ; i < xdf->record.length; i++) {
@@ -173,21 +171,21 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, long narec, xpparam_ xdf->recs[i] = &xdf->record.ptr[i]; } -if (!XDL_CALLOC_ARRAY (xdf->rchg, xdf->nrec +2 )) +if (!XDL_CALLOC_ARRAY (xdf->rchg, xdf->record.length +2 )) goto abort;if ((XDF_DIFF_ALG (xpp->flags) !=XDF_PATIENCE_DIFF ) && (XDF_DIFF_ALG (xpp->flags) !=XDF_HISTOGRAM_DIFF )) { -if (!XDL_ALLOC_ARRAY (xdf->rindex, xdf->nrec +1 )) +if (!XDL_ALLOC_ARRAY (xdf->rindex, xdf->record.length +1 )) goto abort; -if (!XDL_ALLOC_ARRAY (xdf->ha, xdf->nrec +1 )) +if (!XDL_ALLOC_ARRAY (xdf->ha, xdf->record.length +1 )) goto abort; } xdf->rchg +=1 ; xdf->nreff =0 ; xdf->dstart =0 ; - xdf->dend = xdf->nrec -1 ; + xdf->dend = xdf->record.length -1 ;return 0 ;
@@ -274,12 +272,12 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdchar *dis, *dis1, *dis2;int need_min = !!(cf->flags &XDF_NEED_MINIMAL ); -if (!XDL_CALLOC_ARRAY (dis, xdf1->nrec + xdf2->nrec +2 )) +if (!XDL_CALLOC_ARRAY (dis, xdf1->record.length + xdf2->record.length +2 ))return -1 ; dis1 = dis; - dis2 = dis1 + xdf1->nrec +1 ; + dis2 = dis1 + xdf1->record.length +1 ; -if ((mlim =xdl_bogosqrt (xdf1->nrec)) >XDL_MAX_EQLIMIT ) +if ((mlim =xdl_bogosqrt (xdf1->record.length)) >XDL_MAX_EQLIMIT ) mlim =XDL_MAX_EQLIMIT ;for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) { rcrec = cf->rcrecs[(*recs)->ha];
@@ -287,7 +285,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd dis1[i] = (nm ==0 ) ?0 : (nm >= mlim && !need_min) ?2 :1 ; } -if ((mlim =xdl_bogosqrt (xdf2->nrec)) >XDL_MAX_EQLIMIT ) +if ((mlim =xdl_bogosqrt (xdf2->record.length)) >XDL_MAX_EQLIMIT ) mlim =XDL_MAX_EQLIMIT ;for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) { rcrec = cf->rcrecs[(*recs)->ha];
@@ -334,21 +332,21 @@ static int xdl_trim_ends(xdfile_t *xdf1, xdfile_t *xdf2) { recs1 = xdf1->recs; recs2 = xdf2->recs; -for (i =0 , lim =XDL_MIN (xdf1->nrec, xdf2->nrec); i < lim; +for (i =0 , lim =XDL_MIN (xdf1->record.length, xdf2->record.length); i < lim; i++, recs1++, recs2++)if ((*recs1)->ha != (*recs2)->ha)break ; xdf1->dstart = xdf2->dstart = i; - recs1 = xdf1->recs + xdf1->nrec -1 ; - recs2 = xdf2->recs + xdf2->nrec -1 ; + recs1 = xdf1->recs + xdf1->record.length -1 ; + recs2 = xdf2->recs + xdf2->record.length -1 ;for (lim -= i, i =0 ; i < lim; i++, recs1--, recs2--)if ((*recs1)->ha != (*recs2)->ha)break ; - xdf1->dend = xdf1->nrec - i -1 ; - xdf2->dend = xdf2->nrec - i -1 ; + xdf1->dend = xdf1->record.length - i -1 ; + xdf2->dend = xdf2->record.length - i -1 ;return 0 ; }
@@ -388,12 +386,12 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,if (xdl_init_classifier (&cf, enl1 + enl2 +1 , xpp->flags) <0 )return -1 ; -if (xdl_prepare_ctx (1 , mf1, enl1, xpp, &cf, &xe->xdf1) <0 ) { +if (xdl_prepare_ctx (1 , mf1, xpp, &cf, &xe->xdf1) <0 ) {xdl_free_classifier (&cf);return -1 ; } -if (xdl_prepare_ctx (2 , mf2, enl2, xpp, &cf, &xe->xdf2) <0 ) { +if (xdl_prepare_ctx (2 , mf2, xpp, &cf, &xe->xdf2) <0 ) {xdl_free_ctx (&xe->xdf1);xdl_free_classifier (&cf);
xdiff/xtypes.h +0 -1
@@ -48,7 +48,6 @@ DEFINE_IVEC_TYPE(xrecord_t, xrecord); typedef struct s_xdfile { struct ivec_xrecord record; - long nrec; long dstart, dend; xrecord_t **recs; char *rchg;
Signature · 1 line
gitgitgadget
PATCH v3 13/15 xdiff: delete recs field from xdfile_t
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Because of the change from chastore to ivec a few commits ago, recs now points to record's elements in a 1:1 mapping. Since both recs and record are vectors, this additional mapping is superfluous. Remove recs.
This commit is best viewed with --color-words.
--- xdiff/xdiffi.c | 30 ++++++++++++------------ xdiff/xemit.c | 4 ++-- xdiff/xhistogram.c | 2 +- xdiff/xmerge.c | 58 +++++++++++++++++++++++----------------------- xdiff/xpatience.c | 14 +++++------ xdiff/xprepare.c | 37 +++++++++++++---------------- xdiff/xtypes.h | 1 - xdiff/xutils.c | 12 +++++----- 8 files changed, 76 insertions(+), 82 deletions(-)
xdiff/xdiffi.c +15 -15
@@ -501,13 +501,13 @@ static void measure_split(const xdfile_t *xdf, long split, m->indent =-1 ; }else { m->end_of_file =0 ; - m->indent =get_indent (xdf->recs[split]); + m->indent =get_indent (&xdf->record.ptr[split]); } m->pre_blank =0 ; m->pre_indent =-1 ;for (i = split -1 ; i >=0 ; i--) { - m->pre_indent =get_indent (xdf->recs[i]); + m->pre_indent =get_indent (&xdf->record.ptr[i]);if (m->pre_indent !=-1 )break ; m->pre_blank +=1 ;
@@ -520,7 +520,7 @@ static void measure_split(const xdfile_t *xdf, long split, m->post_blank =0 ; m->post_indent =-1 ;for (i = split +1 ; i < (long ) xdf->record.length; i++) { - m->post_indent =get_indent (xdf->recs[i]); + m->post_indent =get_indent (&xdf->record.ptr[i]);if (m->post_indent !=-1 )break ; m->post_blank +=1 ;
@@ -764,7 +764,7 @@ static inline int group_previous(xdfile_t *xdf, struct xdlgroup *g)static int group_slide_down (xdfile_t *xdf,struct xdlgroup *g) {if (g->end < (long ) xdf->record.length && -recs_match (xdf->recs[g->start], xdf->recs[g->end])) { +recs_match (&xdf->record.ptr[g->start], &xdf->record.ptr[g->end])) { xdf->rchg[g->start++] =0 ; xdf->rchg[g->end++] =1 ;
@@ -785,7 +785,7 @@ static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g)static int group_slide_up (xdfile_t *xdf,struct xdlgroup *g) {if (g->start >0 && -recs_match (xdf->recs[g->start -1 ], xdf->recs[g->end -1 ])) { +recs_match (&xdf->record.ptr[g->start -1 ], &xdf->record.ptr[g->end -1 ])) { xdf->rchg[--g->start] =1 ; xdf->rchg[--g->end] =0 ;
@@ -1000,16 +1000,16 @@ static void xdl_mark_ignorable_lines(xdchange_t *xscr, xdfenv_t *xe, long flags)for (xch = xscr; xch; xch = xch->next) {int ignore =1 ; -xrecord_t **rec; +xrecord_t *rec;long i; - rec = &xe->xdf1.recs[xch->i1]; + rec = &xe->xdf1.record.ptr[xch->i1];for (i =0 ; i < xch->chg1 && ignore; i++) - ignore =xdl_blankline ((const char *) rec[i]->ptr, rec[i]->size, flags); + ignore =xdl_blankline ((const char *) rec[i].ptr, rec[i].size, flags); - rec = &xe->xdf2.recs[xch->i2]; + rec = &xe->xdf2.record.ptr[xch->i2];for (i =0 ; i < xch->chg2 && ignore; i++) - ignore =xdl_blankline ((const char *)rec[i]->ptr, rec[i]->size, flags); + ignore =xdl_blankline ((const char *)rec[i].ptr, rec[i].size, flags); xch->ignore = ignore; }
@@ -1033,7 +1033,7 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe,xdchange_t *xch;for (xch = xscr; xch; xch = xch->next) { -xrecord_t **rec; +xrecord_t *rec;int ignore =1 ;long i;
@@ -1043,13 +1043,13 @@ static void xdl_mark_ignorable_regex(xdchange_t *xscr, const xdfenv_t *xe,if (xch->ignore)continue ; - rec = &xe->xdf1.recs[xch->i1]; + rec = &xe->xdf1.record.ptr[xch->i1];for (i =0 ; i < xch->chg1 && ignore; i++) - ignore =record_matches_regex (rec[i], xpp); + ignore =record_matches_regex (&rec[i], xpp); - rec = &xe->xdf2.recs[xch->i2]; + rec = &xe->xdf2.record.ptr[xch->i2];for (i =0 ; i < xch->chg2 && ignore; i++) - ignore =record_matches_regex (rec[i], xpp); + ignore =record_matches_regex (&rec[i], xpp); xch->ignore = ignore; }
xdiff/xemit.c +2 -2
@@ -24,9 +24,9 @@static long xdl_get_rec (xdfile_t *xdf,long ri,char const **rec) { - *rec = (char const *) xdf->recs[ri]->ptr; + *rec = (char const *) xdf->record.ptr[ri].ptr; -return xdf->recs[ri]->size; +return xdf->record.ptr[ri].size; }
xdiff/xhistogram.c +1 -1
@@ -86,7 +86,7 @@ struct region { ((LINE_MAP(index, ptr))->cnt)#define REC (env, s, l) \ - (env->xdf##s.recs[l - 1]) + (&env->xdf##s.record.ptr[l - 1])static int cmp_recs (xrecord_t *r1,xrecord_t *r2) {
xdiff/xmerge.c +29 -29
125 lines not on this page · raw message
xdiff/xpatience.c +7 -7
@@ -88,9 +88,9 @@ static int is_anchor(xpparam_t const *xpp, const char *line)static void insert_record (xpparam_t const *xpp,int line,struct hashmap *map,int pass) { -xrecord_t **records = pass ==1 ? - map->env->xdf1.recs : map->env->xdf2.recs; -xrecord_t *record = records[line -1 ]; +xrecord_t *records = pass ==1 ? + map->env->xdf1.record.ptr : map->env->xdf2.record.ptr; +xrecord_t *record = &records[line -1 ];/* * After xdl_prepare_env() (or more precisely, due to * xdl_classify_record()), the "ha" member of the records (AKA lines)
@@ -121,7 +121,7 @@ static void insert_record(xpparam_t const *xpp, int line, struct hashmap *map,return ; map->entries[index].line1 = line; map->entries[index].hash = record->ha; - map->entries[index].anchor =is_anchor (xpp, (const char *) map->env->xdf1.recs[line -1 ]->ptr); + map->entries[index].anchor =is_anchor (xpp, (const char *) map->env->xdf1.record.ptr[line -1 ].ptr);if (!map->first) map->first = map->entries + index;if (map->last) {
@@ -246,9 +246,9 @@ static int find_longest_common_sequence(struct hashmap *map, struct entry **res)static int match (struct hashmap *map,int line1,int line2) { -xrecord_t *record1 = map->env->xdf1.recs[line1 -1 ]; -xrecord_t *record2 = map->env->xdf2.recs[line2 -1 ]; -return record1->ha == record2->ha; +u64 mph1 = map->env->xdf1.record.ptr[line1 -1 ].ha; +u64 mph2 = map->env->xdf2.record.ptr[line2 -1 ].ha; +return mph1 == mph2; } static int patience_diff(xpparam_t const *xpp, xdfenv_t *env,
xdiff/xprepare.c +16 -21
@@ -134,7 +134,6 @@ static void xdl_free_ctx(xdfile_t *xdf) {xdl_free (xdf->rindex);xdl_free (xdf->rchg -1 );xdl_free (xdf->ha); -xdl_free (xdf->recs); }
@@ -147,7 +146,6 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, xpparam_t const *xpp xdf->ha =NULL ; xdf->rindex =NULL ; xdf->rchg =NULL ; - xdf->recs =NULL ;IVEC_INIT (xdf->record);if ((cur = blk =xdl_mmfile_first (mf, &bsize))) {
@@ -163,12 +161,9 @@ static int xdl_prepare_ctx(unsigned int pass, mmfile_t *mf, xpparam_t const *xpp }ivec_shrink_to_fit (&xdf->record); -if (!XDL_ALLOC_ARRAY (xdf->recs, xdf->record.length)) - goto abort;for (usize i =0 ; i < xdf->record.length; i++) {if (xdl_classify_record (pass, cf, &xdf->record.ptr[i]) <0 ) goto abort; - xdf->recs[i] = &xdf->record.ptr[i]; }if (!XDL_CALLOC_ARRAY (xdf->rchg, xdf->record.length +2 ))
@@ -267,7 +262,7 @@ static int xdl_clean_mmatch(char const *dis, long i, long s, long e) {*/ static int xdl_cleanup_records (xdlclassifier_t *cf,xdfile_t *xdf1,xdfile_t *xdf2) {long i, nm, nreff, mlim; -xrecord_t **recs; +xrecord_t *recs;xdlclass_t *rcrec;char *dis, *dis1, *dis2;int need_min = !!(cf->flags &XDF_NEED_MINIMAL );
@@ -279,38 +274,38 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xdif ((mlim =xdl_bogosqrt (xdf1->record.length)) >XDL_MAX_EQLIMIT ) mlim =XDL_MAX_EQLIMIT ; -for (i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; i <= xdf1->dend; i++, recs++) { - rcrec = cf->rcrecs[(*recs)->ha]; +for (i = xdf1->dstart, recs = &xdf1->record.ptr[xdf1->dstart]; i <= xdf1->dend; i++, recs++) { + rcrec = cf->rcrecs[recs->ha]; nm = rcrec ? rcrec->len2 :0 ; dis1[i] = (nm ==0 ) ?0 : (nm >= mlim && !need_min) ?2 :1 ; }if ((mlim =xdl_bogosqrt (xdf2->record.length)) >XDL_MAX_EQLIMIT ) mlim =XDL_MAX_EQLIMIT ; -for (i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; i <= xdf2->dend; i++, recs++) { - rcrec = cf->rcrecs[(*recs)->ha]; +for (i = xdf2->dstart, recs = &xdf2->record.ptr[xdf2->dstart]; i <= xdf2->dend; i++, recs++) { + rcrec = cf->rcrecs[recs->ha]; nm = rcrec ? rcrec->len1 :0 ; dis2[i] = (nm ==0 ) ?0 : (nm >= mlim && !need_min) ?2 :1 ; } -for (nreff =0 , i = xdf1->dstart, recs = &xdf1->recs[xdf1->dstart]; +for (nreff =0 , i = xdf1->dstart, recs = &xdf1->record.ptr[xdf1->dstart]; i <= xdf1->dend; i++, recs++) {if (dis1[i] ==1 || (dis1[i] ==2 && !xdl_clean_mmatch (dis1, i, xdf1->dstart, xdf1->dend))) { xdf1->rindex[nreff] = i; - xdf1->ha[nreff] = (*recs)->ha; + xdf1->ha[nreff] = recs->ha; nreff++; }else xdf1->rchg[i] =1 ; } xdf1->nreff = nreff; -for (nreff =0 , i = xdf2->dstart, recs = &xdf2->recs[xdf2->dstart]; +for (nreff =0 , i = xdf2->dstart, recs = &xdf2->record.ptr[xdf2->dstart]; i <= xdf2->dend; i++, recs++) {if (dis2[i] ==1 || (dis2[i] ==2 && !xdl_clean_mmatch (dis2, i, xdf2->dstart, xdf2->dend))) { xdf2->rindex[nreff] = i; - xdf2->ha[nreff] = (*recs)->ha; + xdf2->ha[nreff] = recs->ha; nreff++; }else xdf2->rchg[i] =1 ;
@@ -328,21 +323,21 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd*/ static int xdl_trim_ends (xdfile_t *xdf1,xdfile_t *xdf2) {long i, lim; -xrecord_t **recs1, **recs2; +xrecord_t *recs1, *recs2; - recs1 = xdf1->recs; - recs2 = xdf2->recs; + recs1 = xdf1->record.ptr; + recs2 = xdf2->record.ptr;for (i =0 , lim =XDL_MIN (xdf1->record.length, xdf2->record.length); i < lim; i++, recs1++, recs2++) -if ((*recs1)->ha != (*recs2)->ha) +if (recs1->ha != recs2->ha)break ; xdf1->dstart = xdf2->dstart = i; - recs1 = xdf1->recs + xdf1->record.length -1 ; - recs2 = xdf2->recs + xdf2->record.length -1 ; + recs1 = xdf1->record.ptr + xdf1->record.length -1 ; + recs2 = xdf2->record.ptr + xdf2->record.length -1 ;for (lim -= i, i =0 ; i < lim; i++, recs1--, recs2--) -if ((*recs1)->ha != (*recs2)->ha) +if (recs1->ha != recs2->ha)break ; xdf1->dend = xdf1->record.length - i -1 ;
xdiff/xtypes.h +0 -1
@@ -49,7 +49,6 @@ DEFINE_IVEC_TYPE(xrecord_t, xrecord); typedef struct s_xdfile { struct ivec_xrecord record; long dstart, dend; - xrecord_t **recs; char *rchg; long *rindex; long nreff;
xdiff/xutils.c +6 -6
@@ -416,12 +416,12 @@ int xdl_fall_back_diff(xdfenv_t *diff_env, xpparam_t const *xpp,mmfile_t subfile1, subfile2;xdfenv_t env; - subfile1.ptr = (char *)diff_env->xdf1.recs[line1 -1 ]->ptr; - subfile1.size = diff_env->xdf1.recs[line1 + count1 -2 ]->ptr + - diff_env->xdf1.recs[line1 + count1 -2 ]->size - (u8 const *) subfile1.ptr; - subfile2.ptr = (char *)diff_env->xdf2.recs[line2 -1 ]->ptr; - subfile2.size = diff_env->xdf2.recs[line2 + count2 -2 ]->ptr + - diff_env->xdf2.recs[line2 + count2 -2 ]->size - (u8 const *) subfile2.ptr; + subfile1.ptr = (char *)diff_env->xdf1.record.ptr[line1 -1 ].ptr; + subfile1.size = diff_env->xdf1.record.ptr[line1 + count1 -2 ].ptr + + diff_env->xdf1.record.ptr[line1 + count1 -2 ].size - (u8 const *) subfile1.ptr; + subfile2.ptr = (char *)diff_env->xdf2.record.ptr[line2 -1 ].ptr; + subfile2.size = diff_env->xdf2.record.ptr[line2 + count2 -2 ].ptr + + diff_env->xdf2.record.ptr[line2 + count2 -2 ].size - (u8 const *) subfile2.ptr;if (xdl_do_diff (&subfile1, &subfile2, xpp, &env) <0 )return -1 ;
Signature · 1 line
gitgitgadget
PATCH v3 14/15 xdiff: make xdfile_t more rust friendly
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Convert the remaining ambiguous fields in xdfile_t from C types to Rust types for interoperability.
--- xdiff/xdiffi.c | 16 ++++++++-------- xdiff/xdiffi.h | 8 ++++---- xdiff/xtypes.h | 10 +++++----- 3 files changed, 17 insertions(+), 17 deletions(-)
xdiff/xdiffi.c +8 -8
@@ -42,8 +42,8 @@ typedef struct s_xdpsplit {* using this algorithm, so a little bit of heuristic is needed to cut the * search and to return a suboptimal point. */ -static long xdl_split (unsigned long const *ha1,long off1,long lim1, -unsigned long const *ha2,long off2,long lim2, +static long xdl_split (u64 const *ha1,long off1,long lim1, +u64 const *ha2,long off2,long lim2,long *kvdf,long *kvdb,int need_min,xdpsplit_t *spl,xdalgoenv_t *xenv) {long dmin = off1 - lim2, dmax = lim1 - off2;
@@ -260,7 +260,7 @@ static long xdl_split(unsigned long const *ha1, long off1, long lim1,int xdl_recs_cmp (diffdata_t *dd1,long off1,long lim1,diffdata_t *dd2,long off2,long lim2,long *kvdf,long *kvdb,int need_min,xdalgoenv_t *xenv) { -unsigned long const *ha1 = dd1->ha, *ha2 = dd2->ha; +u64 const *ha1 = dd1->ha, *ha2 = dd2->ha;/* * Shrink the box by walking through each diagonal snake (SW and NE).
@@ -273,14 +273,14 @@ int xdl_recs_cmp(diffdata_t *dd1, long off1, long lim1,* be obviously changed. */ if (off1 == lim1) { -char *rchg2 = dd2->rchg; -long *rindex2 = dd2->rindex; +u8 *rchg2 = dd2->rchg; +usize *rindex2 = dd2->rindex;for (; off2 < lim2; off2++) rchg2[rindex2[off2]] =1 ; }else if (off2 == lim2) { -char *rchg1 = dd1->rchg; -long *rindex1 = dd1->rindex; +u8 *rchg1 = dd1->rchg; +usize *rindex1 = dd1->rindex;for (; off1 < lim1; off1++) rchg1[rindex1[off1]] =1 ;
@@ -944,7 +944,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {int xdl_build_script (xdfenv_t *xe,xdchange_t **xscr) {xdchange_t *cscr =NULL , *xch; -char *rchg1 = xe->xdf1.rchg, *rchg2 = xe->xdf2.rchg; +u8 *rchg1 = xe->xdf1.rchg, *rchg2 = xe->xdf2.rchg;long i1, i2, l1, l2;/*
xdiff/xdiffi.h +4 -4
@@ -25,10 +25,10 @@typedef struct s_diffdata { -long nrec; -unsigned long const *ha; -long *rindex; -char *rchg; +usize nrec; +u64 const *ha; +usize *rindex; +u8 *rchg; }diffdata_t ; typedef struct s_xdalgoenv {
xdiff/xtypes.h +5 -5
@@ -48,11 +48,11 @@ DEFINE_IVEC_TYPE(xrecord_t, xrecord);typedef struct s_xdfile {struct ivec_xrecord record; -long dstart, dend; -char *rchg; -long *rindex; -long nreff; -unsigned long *ha; +isize dstart, dend; +u8 *rchg; +usize *rindex; +usize nreff; +u64 *ha; }xdfile_t ; typedef struct s_xdfenv {
Signature · 1 line
gitgitgadget
PATCH v3 15/15 xdiff: implement xdl_trim_ends() in Rust
In reply to Ezekiel Newren via GitGitGadgetFrom: Ezekiel Newren <[email protected]>
Replace the C implementation of xdl_trim_ends() with a Rust implementation.
--- rust/xdiff/src/lib.rs | 14 ++++++++++++++ rust/xdiff/src/xprepare.rs | 27 +++++++++++++++++++++++++++ rust/xdiff/src/xtypes.rs | 19 +++++++++++++++++++ xdiff/xprepare.c | 28 +--------------------------- 4 files changed, 61 insertions(+), 27 deletions(-) create mode 100644 rust/xdiff/src/xprepare.rs create mode 100644 rust/xdiff/src/xtypes.rs
rust/xdiff/src/lib.rs +14 -0
@@ -1 +1,15 @@ +pub mod xprepare; +pub mod xtypes; +use crate ::xprepare::trim_ends; +use crate ::xtypes::xdfile; + +#[no_mangle] +unsafe extern "C" fn xdl_trim_ends (xdf1: *mut xdfile , xdf2: *mut xdfile ) ->i32 { +let xdf1 = xdf1.as_mut ().expect ("null pointer" ); +let xdf2 = xdf2.as_mut ().expect ("null pointer" ); + +trim_ends (xdf1, xdf2); + +0 +}
rust/xdiff/src/xprepare.rs new file +27 -0
@@ -0,0 +1,27 @@ +use crate ::xtypes::xdfile; + +/// +/// Early trim initial and terminal matching records. +/// +pub (crate )fn trim_ends (xdf1: &mut xdfile , xdf2: &mut xdfile ) { +let mut lim = std::cmp::min (xdf1.record.len (), xdf2.record.len ()); + +for iin 0 ..lim { +if xdf1.record[i].ha != xdf2.record[i].ha { + xdf1.dstart = ias isize ; + xdf2.dstart = ias isize ; + lim -= i; +break ; + } + } + +for iin 0 ..lim { +let f1i = xdf1.record.len () -1 - i; +let f2i = xdf2.record.len () -1 - i; +if xdf1.record[f1i].ha != xdf2.record[f2i].ha { + xdf1.dend = f1ias isize ; + xdf2.dend = f2ias isize ; +break ; + } + } +}
rust/xdiff/src/xtypes.rs new file +19 -0
@@ -0,0 +1,19 @@ +use interop::ivec::IVec ; + +#[repr( C )] +pub (crate )struct xrecord { +pub (crate ) ptr: *const u8 , +pub (crate ) size:usize , +pub (crate ) ha:u64 , +} + +#[repr( C )] +pub (crate )struct xdfile { +pub (crate ) record:IVec <xrecord >, +pub (crate ) dstart:isize , +pub (crate ) dend:isize , +pub (crate ) rchg: *mut u8 , +pub (crate ) rindex: *mut usize , +pub (crate ) nreff:usize , +pub (crate ) ha: *mut u64 , +}
xdiff/xprepare.c +1 -27
@@ -318,33 +318,7 @@ static int xdl_cleanup_records(xdlclassifier_t *cf, xdfile_t *xdf1, xdfile_t *xd } -/* -* Early trim initial and terminal matching records. -*/ -static int xdl_trim_ends (xdfile_t *xdf1,xdfile_t *xdf2) { -long i, lim; -xrecord_t *recs1, *recs2; - - recs1 = xdf1->record.ptr; - recs2 = xdf2->record.ptr; -for (i =0 , lim =XDL_MIN (xdf1->record.length, xdf2->record.length); i < lim; - i++, recs1++, recs2++) -if (recs1->ha != recs2->ha) -break ; - - xdf1->dstart = xdf2->dstart = i; - - recs1 = xdf1->record.ptr + xdf1->record.length -1 ; - recs2 = xdf2->record.ptr + xdf2->record.length -1 ; -for (lim -= i, i =0 ; i < lim; i++, recs1--, recs2--) -if (recs1->ha != recs2->ha) -break ; - - xdf1->dend = xdf1->record.length - i -1 ; - xdf2->dend = xdf2->record.length - i -1 ; - -return 0 ; -} +extern i32 xdl_trim_ends (xdfile_t *xdf1,xdfile_t *xdf2);static int xdl_optimize_ctxs (xdlclassifier_t *cf,xdfile_t *xdf1,xdfile_t *xdf2) {
Signature · 1 line
gitgitgadget