-
Notifications
You must be signed in to change notification settings - Fork 618
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
Add support for SHA-256 repositories #1302
base: master
Are you sure you want to change the base?
Conversation
src/tree.c
Outdated
@@ -58,7 +58,7 @@ push_tree_stack_entry(struct view *view, const char *name, struct position *posi | |||
*/ | |||
|
|||
#define SIZEOF_TREE_ATTR \ | |||
STRING_SIZE("100644 blob f931e1d229c3e185caad4449bf5b66ed72462657\t") | |||
STRING_SIZE("100644 blob ") + repo.hash_len + 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.
maybe
STRING_SIZE("100644 blob ") + repo.hash_len + 1 | |
STRING_SIZE("100644 blob ") + repo.hash_len + STRING_SIZE("\t") |
@@ -203,6 +203,8 @@ following variables. | |||
|%(repo:is-inside-work-tree) | |||
|Whether Tig is running inside a work tree, | |||
either `true` or `false`. | |||
|%(repo:hash_len) |The hash algorithm used for the repository, e.g. `sha1` | |||
or `sha256`. |
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.
Maybe this should be called %(repo:extensions.objectformat)
to use the standard Git term.
Actually, is there even a need to expose this at all?
I don't think we expose any other config options. Users can already consult something like
git --git-dir=%(repo:git-dir) config extensions.objectformat
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 for the review @krobelus. %(repo:hash_len) is actually a by-product of storing hash_len in the repo struct, but it's the best compromise I have found. I'll make it more elegant.
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.
oh it is exposed by the REPO_INFO
macro. Maybe define it outside that
src/options.c
Outdated
@@ -1531,6 +1531,9 @@ read_repo_config_option(char *name, size_t namelen, char *value, size_t valuelen | |||
else if (!strcmp(name, "core.abbrev")) | |||
parse_int(&opt_id_width, value, 0, SIZEOF_REV - 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.
Not really important: we have two places where we call
parse_int()
with a max of SIZEOF_REV - 1
.
The effective max should maybe be lower for sha1 repos.
But I'm not sure if that would cause unnecessary friction, so better keep it.
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.
My initial code used hash_len but we haven't read extension.objectFormat yet here, so we can't use it. I'll improve this part so that the same configuration can be used in global files for both sha1 and sha256 repositories.
@@ -19,7 +19,7 @@ | |||
#include "tig/util.h" | |||
|
|||
struct commit { | |||
char id[SIZEOF_REV]; /* SHA1 ID. */ | |||
char id[SIZEOF_REV]; /* Hash ID. */ |
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.
this looks good apart from one comment about %(repo:hash_len)
.
I haven't used sha256, I'll try it if I get around to.
It's a shame we can't reuse existing unit tests without modification.
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.
Actually git fast-export
and git fast-import
allow to port the tests without too much effort and also to get a nice sandbox from an existing repository.
f4c4757
to
6475d09
Compare
98b589d
to
357af1a
Compare
Use variable extensions.objectformat, if available, to determine the length of the hash id in the current repository.
With Git 2.42, SHA-256 repositories are no longer experimental. This PR adds support to Tig for SHA-256 repositories.