| Message ID | 011e5eaea3f48eeacac1614f769e8fd809e0c093.1629805396.git.dyroneteng@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | packfile-uris: commits, trees and tags exclusion | expand |
On Wed, Aug 25 2021, Teng Long wrote: > Signed-off-by: Teng Long <dyroneteng@gmail.com> > --- > commit.h | 5 +++++ > revision.c | 8 ++++++-- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/commit.h b/commit.h > index df42eb434f..1e99e9ae8a 100644 > --- a/commit.h > +++ b/commit.h > @@ -38,6 +38,11 @@ struct commit { > */ > struct tree *maybe_tree; > unsigned int index; > + /* > + * wrapped tags or NULL. If the commit is peeled from tag(s), > + * then save the wraps, otherwise will be NULL. > + */ > + struct object_list *wraps; > }; > > extern int save_commit_buffer; > diff --git a/revision.c b/revision.c > index 65e0926d25..aecf493f46 100644 > --- a/revision.c > +++ b/revision.c > @@ -416,14 +416,17 @@ static struct commit *handle_commit(struct rev_info *revs, > const char *path = entry->path; > unsigned int mode = entry->mode; > unsigned long flags = object->flags; > - > + struct object_list *wraps = NULL; > /* > * Tag object? Look what it points to.. > */ > while (object->type == OBJ_TAG) { > struct tag *tag = (struct tag *) object; > - if (revs->tag_objects && !(flags & UNINTERESTING)) > + if (revs->tag_objects && !(flags & UNINTERESTING)) { > + object_list_insert(object, &wraps); > add_pending_object(revs, object, tag->tag); > + } > + > object = parse_object(revs->repo, get_tagged_oid(tag)); > if (!object) { > if (revs->ignore_missing_links || (flags & UNINTERESTING)) > @@ -449,6 +452,7 @@ static struct commit *handle_commit(struct rev_info *revs, > */ > if (object->type == OBJ_COMMIT) { > struct commit *commit = (struct commit *)object; > + commit->wraps = wraps; > > if (repo_parse_commit(revs->repo, commit) < 0) > die("unable to parse commit %s", name); Can't we store this info on the side between these two static functions somehow, instead of adding this "wraps" to all commit structs?
Ævar Arnfjörð Bjarmason wrote: > Can't we store this info on the side between these two static functions > somehow, instead of adding this "wraps" to all commit structs? > Thanks very much and I have some doubts. > ... instead of adding this "wraps" to all commit structs? I think "adding this "wraps" to all commit struct" is an easy but a little rough indeed. I didn't know if this is okay at the time. So I pushed the patch, hoping to ask some different opinions. > Can't we store this info on the side between these two static functions... Do you mean to use static storage to share the "wraps", or other way? I want to make sure that I understand your opinion accurately. Thank you.
Teng Long <dyroneteng@gmail.com> 于2021年8月25日周三 上午10:24写道： > > Signed-off-by: Teng Long <dyroneteng@gmail.com> > --- > commit.h | 5 +++++ > revision.c | 8 ++++++-- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/commit.h b/commit.h > index df42eb434f..1e99e9ae8a 100644 > --- a/commit.h > +++ b/commit.h > @@ -38,6 +38,11 @@ struct commit { > */ > struct tree *maybe_tree; > unsigned int index; > + /* > + * wrapped tags or NULL. If the commit is peeled from tag(s), > + * then save the wraps, otherwise will be NULL. > + */ > + struct object_list *wraps; > }; > > extern int save_commit_buffer; > diff --git a/revision.c b/revision.c > index 65e0926d25..aecf493f46 100644 > --- a/revision.c > +++ b/revision.c > @@ -416,14 +416,17 @@ static struct commit *handle_commit(struct rev_info *revs, > const char *path = entry->path; > unsigned int mode = entry->mode; > unsigned long flags = object->flags; > - > + struct object_list *wraps = NULL; > /* > * Tag object? Look what it points to.. > */ > while (object->type == OBJ_TAG) { > struct tag *tag = (struct tag *) object; > - if (revs->tag_objects && !(flags & UNINTERESTING)) > + if (revs->tag_objects && !(flags & UNINTERESTING)) { > + object_list_insert(object, &wraps); > add_pending_object(revs, object, tag->tag); > + } > + > object = parse_object(revs->repo, get_tagged_oid(tag)); > if (!object) { > if (revs->ignore_missing_links || (flags & UNINTERESTING)) > @@ -449,6 +452,7 @@ static struct commit *handle_commit(struct rev_info *revs, > */ > if (object->type == OBJ_COMMIT) { > struct commit *commit = (struct commit *)object; > + commit->wraps = wraps; > > if (repo_parse_commit(revs->repo, commit) < 0) > die("unable to parse commit %s", name); > -- > 2.31.1.456.gec51e24953 > /* * The size of this struct matters in full repo walk operations like * 'git clone' or 'git gc'. Consider using commit-slab to attach data * to a commit instead of adding new fields here. */ struct commit { struct object object; timestamp_t date; struct commit_list *parents; /* * If the commit is loaded from the commit-graph file, then this * member may be NULL. Only access it through repo_get_commit_tree() * or get_commit_tree_oid(). */ struct tree *maybe_tree; unsigned int index; }; According to the instructions above, I wonder if you should use "commit_slab" to store part of the data related to the commit object instead of modifying the member of struct commit itself? See: https://github.com/git/git/blob/master/commit-slab.h https://github.com/git/git/blob/master/commit-slab-impl.h https://github.com/git/git/blob/master/commit-slab-decl.h https://lore.kernel.org/git/CAOLTT8Q8BEKCVwPDypW1w66P9_xP7QC0T-CnLqamqAL4haGzwA@mail.gmail.com/ Thanks. -- ZheNing Hu
ZheNing Hu wrote: > /* > * The size of this struct matters in full repo walk operations like > * 'git clone' or 'git gc'. Consider using commit-slab to attach data > * to a commit instead of adding new fields here. > */ > struct commit { > struct object object; > timestamp_t date; > struct commit_list *parents; > > /* > * If the commit is loaded from the commit-graph file, then this > * member may be NULL. Only access it through repo_get_commit_tree() > * or get_commit_tree_oid(). > */ > struct tree *maybe_tree; > unsigned int index; > }; > > According to the instructions above, I wonder if you should use "commit_slab" to > store part of the data related to the commit object instead of > modifying the member > of struct commit itself? > > See: > https://github.com/git/git/blob/master/commit-slab.h > https://github.com/git/git/blob/master/commit-slab-impl.h > https://github.com/git/git/blob/master/commit-slab-decl.h > https://lore.kernel.org/git/CAOLTT8Q8BEKCVwPDypW1w66P9_xP7QC0T-CnLqamqAL4haGzwA@mail.gmail.com/ Awesome! Maybe it's what I really need now, I will make a try. Thanks(比心).
diff --git a/commit.h b/commit.h index df42eb434f..1e99e9ae8a 100644 --- a/commit.h +++ b/commit.h @@ -38,6 +38,11 @@ struct commit { */ struct tree *maybe_tree; unsigned int index; + /* + * wrapped tags or NULL. If the commit is peeled from tag(s), + * then save the wraps, otherwise will be NULL. + */ + struct object_list *wraps; }; extern int save_commit_buffer; diff --git a/revision.c b/revision.c index 65e0926d25..aecf493f46 100644 --- a/revision.c +++ b/revision.c @@ -416,14 +416,17 @@ static struct commit *handle_commit(struct rev_info *revs, const char *path = entry->path; unsigned int mode = entry->mode; unsigned long flags = object->flags; - + struct object_list *wraps = NULL; /* * Tag object? Look what it points to.. */ while (object->type == OBJ_TAG) { struct tag *tag = (struct tag *) object; - if (revs->tag_objects && !(flags & UNINTERESTING)) + if (revs->tag_objects && !(flags & UNINTERESTING)) { + object_list_insert(object, &wraps); add_pending_object(revs, object, tag->tag); + } + object = parse_object(revs->repo, get_tagged_oid(tag)); if (!object) { if (revs->ignore_missing_links || (flags & UNINTERESTING)) @@ -449,6 +452,7 @@ static struct commit *handle_commit(struct rev_info *revs, */ if (object->type == OBJ_COMMIT) { struct commit *commit = (struct commit *)object; + commit->wraps = wraps; if (repo_parse_commit(revs->repo, commit) < 0) die("unable to parse commit %s", name);
Signed-off-by: Teng Long <dyroneteng@gmail.com> --- commit.h | 5 +++++ revision.c | 8 ++++++-- 2 files changed, 11 insertions(+), 2 deletions(-)