Re: [PATCH v5 03/14] packfile-uri: support for excluding commit objects

2 messages, 2 authors, 2021-09-02 · open the first message on its own page

Re: [PATCH v5 03/14] packfile-uri: support for excluding commit objects

From: Junio C Hamano <hidden>
Date: 2021-08-26 20:56:33

Teng Long [off-list ref] writes:
This commit is used to support the recursive exclusion of a commit
object, which means that if the exclusion of a commit is configured as
packfile-uri, the commit itself and all the objects it contains will
also be recursively excluded.
Whenever you say "which means", e.g. "We do X, which means we do Y",
think twice to see if you do not even have to say X.  In this
particular sentence, I think you can simplify the description
greatly and you do not even have to use the word "recursive".  

Also, because this "exclusion by a commit" does not work like the
usual "reachability" relationship Git users are familiar with, it
would help to highlight what is special that is done here to the
readers.

Taking the above together, along the lines of ...

    When a commit is specified to be excluded as packfile-uri,
    exclude all trees and blobs contained in its top-level tree, as
    well as the commit itself, but not the ancestors of the commit
    and objects that are reachable by them.

or something like that, perhaps.
quoted hunk
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 5f9ec3566f..63f3aed70a 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1311,11 +1311,14 @@ static int want_object_in_pack_one(struct packed_git *p,
 static int want_object_in_pack(const struct object_id *oid,
 			       int exclude,
 			       struct packed_git **found_pack,
-			       off_t *found_offset)
+			       off_t *found_offset,
+			       struct object *referred_commit)
As the caller is limited to pass commit and no other types of
object, the new parameter should be of type "struct commit", no?

Re: [PATCH v5 03/14] packfile-uri: support for excluding commit objects

From: Teng Long <hidden>
Date: 2021-09-02 12:51:13

Junio C Hamano wrote:
Whenever you say "which means", e.g. "We do X, which means we do Y",
think twice to see if you do not even have to say X.  In this
particular sentence, I think you can simplify the description
greatly and you do not even have to use the word "recursive".  
Forgive my English for another accident.

Will be fixed in the next patch.
Also, because this "exclusion by a commit" does not work like the
usual "reachability" relationship Git users are familiar with, it
would help to highlight what is special that is done here to the
readers.

Taking the above together, along the lines of ...

    When a commit is specified to be excluded as packfile-uri,
    exclude all trees and blobs contained in its top-level tree, as
    well as the commit itself, but not the ancestors of the commit
    and objects that are reachable by them.

or something like that, perhaps.
Agree.

I will use your suggestion in the next patch (may be slightly modified).

In the next patch, I originally planned to introduce the exclusion of the
commit and with all it's ancestor objects. The problems in the current patch
will also be fixed in the next patch.

Thanks.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help