Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing oid type for "fake" repositories #6554

Merged
merged 1 commit into from May 6, 2023

Conversation

oreiche
Copy link
Contributor

@oreiche oreiche commented May 3, 2023

... otherwise git_tree__parse_raw() will fail to obtain the correct oid size, which causes the entire parse to fail.

Fixes issue #6553.


repo = repository_alloc();
GIT_ERROR_CHECK_ALLOC(repo);

git_repository_set_odb(repo, odb);

if ((error = obtain_config_and_set_oid_type(&config, repo)) < 0)
Copy link
Member

Choose a reason for hiding this comment

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

Interesting. I was going to argue that this was not necessary because a fake repository would never have a config of its own. But then I poked around to validate this thinking. This made me realize that a) git limits the repository format version config and / or per-repo extensions to the repo config and doesn't look at system / global. and b) we do not do this.

That's something that we should fix independently of this problem. But it means that we can simplify this to not bother looking at the config. So I think that the simpler solution is just to repo->oid_type = GIT_OID_DEFAULT;...

But I think that in the long term, though, this function needs to take an oid type. I think that I might do:

#ifdef GIT_EXPERIMENTAL_SHA256
int git_repository_wrap_odb(git_repository **repo_out, git_odb *odb, git_oid_t oid_type)
#else
int git_repository_wrap_odb(git_repository **repo_out, git_odb *odb)
#endif
{
#ifndef GIT_EXPERIMENTAL_SHA256
	git_oid_t oid_type = GIT_OID_DEFAULT;
#endif

	repo = repository_alloc();
	GIT_ERROR_CHECK_ALLOC(repo);

	repo->oid_type = oid_type;

	git_repository_set_odb(repo, odb);

	*repo_out = repo;
	return 0;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I updated the branch to implement the simple solution :)

@ethomson
Copy link
Member

ethomson commented May 3, 2023

Good catch, thanks for the fix. I made a simplifying suggestion -- and a more complex suggestion as well. 😁

... otherwise git_tree__parse_raw() will fail to obtain
the correct oid size, which causes the entire parse to fail.
@oreiche oreiche force-pushed the oreiche/fix-oid-type-fake-repo branch from 00ce2c0 to 47ebf58 Compare May 3, 2023 12:30
@ethomson ethomson added the v1.6 label May 3, 2023
@ethomson ethomson merged commit 3091757 into libgit2:main May 6, 2023
26 checks passed
@ethomson
Copy link
Member

ethomson commented May 6, 2023

Thanks again for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants