On Tue, Sep 4, 2018 at 1:31 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Matthew DeVore <matvore@xxxxxxxxxx> writes: > > > diff --git a/revision.h b/revision.h > > index 5118aaaa9..2d381e636 100644 > > --- a/revision.h > > +++ b/revision.h > > @@ -8,7 +8,11 @@ > > #include "diff.h" > > #include "commit-slab-decl.h" > > > > -/* Remember to update object flag allocation in object.h */ > > +/* Remember to update object flag allocation in object.h > > + * NEEDSWORK: NOT_USER_GIVEN doesn't apply to commits because we only support > > + * filtering trees and blobs, but it may be useful to support filtering commits > > + * in the future. > > + */ > > Just a minor style nit, but our multi-line comment begins with the > opening "/*" (and closing "*/", too, but you got that right) on its > own line, i.e. > Fixed. > /* > * Remember to update ... > > > -#define USER_GIVEN (1u<<25) /* given directly by the user */ > > +#define NOT_USER_GIVEN (1u<<25) /* tree or blob not given directly by user */ > > Is "given directly by user" equivalent to "given on the command > line"? Do objects given via "--stdin" count the same way? How abot > those given via "--branches" or "A^@"? Does "not given directly by > user" mean roughly the same thing as "discovered by traversal"? Note that --branches and A^@ expands to commits, and commits can't be filtered, so perhaps these questions will be unlikely for the time being. I did clarify the comment a bit. I also noticed that this commit fixes a bug - before this patch, "git rev-list" would potentially filter objects given on the command line/--stdin, while "git pack-objects" would not. That was because each command used a different function to populate the rev_info struct, and rev-list's code path did not contain the hack which turned on USER_GIVEN. So I added a test to check the desired behavior. Here is an interdiff for this commit only: diff --git a/revision.h b/revision.h index fe4ff1fec..83e164039 100644 --- a/revision.h +++ b/revision.h @@ -9,11 +9,7 @@ #include "diff.h" #include "commit-slab-decl.h" -/* Remember to update object flag allocation in object.h - * NEEDSWORK: NOT_USER_GIVEN doesn't apply to commits because we only support - * filtering trees and blobs, but it may be useful to support filtering commits - * in the future. - */ +/* Remember to update object flag allocation in object.h */ #define SEEN (1u<<0) #define UNINTERESTING (1u<<1) #define TREESAME (1u<<2) @@ -25,7 +21,14 @@ #define SYMMETRIC_LEFT (1u<<8) #define PATCHSAME (1u<<9) #define BOTTOM (1u<<10) -#define NOT_USER_GIVEN (1u<<25) /* tree or blob not given directly by user */ +/* + * Indicates object was reached by traversal. i.e. not given by user on + * command-line or stdin. + * NEEDSWORK: NOT_USER_GIVEN doesn't apply to commits because we only support + * filtering trees and blobs, but it may be useful to support filtering commits + * in the future. + */ +#define NOT_USER_GIVEN (1u<<25) #define TRACK_LINEAR (1u<<26) #define ALL_REV_FLAGS (((1u<<11)-1) | NOT_USER_GIVEN | TRACK_LINEAR) diff --git a/t/t6112-rev-list-filters-objects.sh b/t/t6112-rev-list-filters-objects.sh index 2e6a6a32e..a989a7082 100755 --- a/t/t6112-rev-list-filters-objects.sh +++ b/t/t6112-rev-list-filters-objects.sh @@ -30,6 +30,16 @@ test_expect_success 'verify blob:none omits all 5 blobs' ' test_cmp observed expected ' +test_expect_success 'specify blob explicitly prevents filtering' ' + file_3=$(git -C r1 ls-files -s file.3 \ + | awk -f print_2.awk) && + file_4=$(git -C r1 ls-files -s file.4 \ + | awk -f print_2.awk) && + git -C r1 rev-list HEAD --objects --filter=blob:none HEAD $file_3 >observed && + grep -q "$file_3" observed && + test_must_fail grep -q "$file_4" observed +' + test_expect_success 'verify emitted+omitted == all' ' git -C r1 rev-list HEAD --objects \ | awk -f print_1.awk \ > > Not a suggestion to change anything in this patch, but if you can > come up with a better phrase that helps new readers' understanding > so that they do not have to ask a question like this, that would be > great. > > Thanks. >
