wip: Revision parsing

[kalkin]

direct port of my already available code. This is not a final version.

[Byron]

Thanks for giving it a shot!

I think it's only fair for me to comment as early as possible to avoid you from putting too much time into this with a focus that I think needs to be changed fundamentally. This is certainly attributed to me under-specifying what's needed and to the code to pre-exist in another code-base.

First of all, this PR should focus on git-revision exclusively. git-repository shouldn't implement anything until it's clear what kind of Delegate (here Database) is actually needed. The development of rev-spec parsing has to be test-driven and it's required to leverage the existing git test harness at some point. It's OK to start with simple tests but ultimately we need to try to replicate the hard cases that are typically found in the git source code to not miss any corner-cases that git found over its 20 years or so.

The latter is the source of truth and must be studied to see what the baseline is. Usually this means zero-copy parsing which is not happening here in this PR.

If all this sounds like a chore, then it probably is, but it's required to be as correct as git and this isn't easy. Also I am here to help along the way.

I would fully understand if that this level of diligence isn't needed for what glv wants right now, and getting to what gitoxide needs will take considerable time, effort and close collaboration. The latter includes me reviewing mostly by refactoring or making adjustments while leaving TODOs. This is most efficient for me but might not be for everyone either.

What's your thoughts? (I spend 38 minutes to review the work and write this message)

[kalkin]

I think it's only fair for me to comment as early as possible to avoid you from putting too much time into this with a focus that I think needs to be changed fundamentally.

No worries. I'm pretty sure this will go through multiple reworks and I expect this.

First of all, this PR should focus on git-revision exclusively. git-repository shouldn't implement anything until it's clear what kind of Delegate (here Database) is actually needed.

The delegate and it's name are work in progress. Like I said it's just a direct port of my code. It's just for getting the train rolling.

The development of rev-spec parsing has to be test-driven … we need to try to replicate the hard cases that are typically found in the git source code to not miss any corner-cases that git found over its 20 years or so.

The latter is the source of truth and must be studied to see what the baseline is.

FULL ACK. I will start on this on Monday

Usually this means zero-copy parsing which is not happening here in this PR.

I think i know which parts you mean. Will fix that.

If all this sounds like a chore, then it probably is, but it's required to be as correct as git and this isn't easy. Also I am here to help along the way.

I think it's normal being so strict, given the project size and it's importance. Thank you for your guidance.

The latter includes me reviewing mostly by refactoring or making adjustments while leaving TODOs. This is most efficient for me but might not be for everyone either.

I would prefer if you comment on source code what you think is better and I rework the PR. This way I actively learn the gitoxide-way to do things and can propose better solutions if I see one. This might be even less work on you?

Thank you for your time.

[Byron]

Good to hear!

I would prefer if you comment on source code what you think is better and I rework the PR. This way I actively learn the gitoxide-way to do things and can propose better solutions if I see one. This might be even less work on you?

It's more effective and faster for me to edit code as part of the review.

I will start on this on Monday.

You would do me a favor by absorbing the structure of existing crates and follow the conventions present there. git-sec is a recent and small example, and so is git-validate or git-url.

[kalkin]

I studied the original git tests and come to the conclusion we should port the following tests:

• t/t1505-rev-parse-last.sh
• t/t1506-rev-parse-diagnosis.sh
• t/t1507-rev-parse-upstream.sh
• t/t1511-rev-parse-caret.sh
• t/t1512-rev-parse-disambiguation.sh
• t/t1513-rev-parse-prefix.sh
• t/t1514-rev-parse-push.sh
• t/t6101-rev-parse-parents.sh

The tests need some prepared repositories. I would put commands for generating the repository into git-repository/tests/fixtures/*.sh. The tests themselves would go to git-repository/tests/rev_parse/$NAME.rs, right?

I would start with porting tests and then rework my parsing code to pass them.

[Byron]

Warning: I force-pushed here to edit commit messages

I hope you don't mind that I reset the work to where I think it has to be. The motivation is usually in the body of the commit messages.

The high-level summary is this:

• don't test on git-repository, test on git-revision
• every piece of parsing code should be motivated by at least one test - it's fine to have multiple assertions in one test of course but it's nice to have special cases with their own headline.
• I have adjusted the structure to fit test-driven development and added two example tests
• don't use conventional commit messages for every commit (these go into changelogs)
• don't use emojis in commit message subject lines

It's probably worth working through the revspec docs and make sure these cases parse correctly.

I studied the original git tests and come to the conclusion we should port the following tests:

That's great, I think cases from there can be used to amend the test-suite. If I were to do it I'd look at these only after the docs are fully implemented.

For scoping this PR, it would be OK to only implement one portion of the capabilities of rev-spec parsing and consider making that available in git-repository early. Then it becomes usable right away and more features/capabilities can be implemented step by step.

Right now this PR is under-specified and I recommend implementing ref-name handling and object names via hashes first, along with navigation. This could be a baseline for typical revspecs which can be brought into git-repository in another PR.

Video: https://youtu.be/V1Xe0qEPyFg

(This review took ~2h on and off, and I don't feel great having had to remove that much code as it doesn't seem to value your time. I highly recommend to take baby steps and stick to my guidance.)

[Byron]

I am closing this PR in favor of #428, which starts at the parsing stage with more PRs to follow up with integrating it into git-repository. Since this PR started out very much centered around git-repository, and if the integration is something you are interested in, please let me know.