-
-
Notifications
You must be signed in to change notification settings - Fork 959
Preliminary support for index format v3 #2081
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Byron
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for tackling this!
I think it would be useful to add a small test-index directly, one that is known to be V3. The Gitoxide repository definitely has them: https://github.com/GitoxideLabs/gitoxide/blob/ecd354119c3a8b150a06df7205ddf022a825d6cd/gix-index/tests/fixtures/loose_index/extended-flags.git-index
Thanks again.
test/test_index.py
Outdated
| def test_index_version_v3(self, tmp_dir): | ||
| tmp_dir = Path(tmp_dir) | ||
| with cwd(tmp_dir): | ||
| subprocess.run(["git", "init", "-q"], check=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to use the Git type instead? That way one would benefit from all the 'extras', especially around finding the right Git binary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK done
test/test_index.py
Outdated
|
|
||
| repo = Repo(tmp_dir) | ||
|
|
||
| assert len(repo.index.entries) == 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test this, I think the index.version must be exposed and validated here.
I don't see at all why it would use index version 3 here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test this, I think the index.version must be exposed and validated here.
Done.
I don't see at all why it would use index version 3 here.
git add -N (--intent-to-add) would produce version 3 index. I added a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
index.version
There's this minor thing that I'm not sure if it's an issue:
Currently, the version field of IndexFile is always set to 2 after __init__. Since it's lazy loaded, it's only after its first read of entries when its version field is set correctly. Do you think we can keep it this way?
OK. I've copied the file here and added a test case |
cca4431 to
acfee98
Compare
acfee98 to
8a884fe
Compare
#1960