mbox series

[v6,00/15,GSOC,RFC] cat-file: reuse ref-filter logic

Message ID pull.980.v6.git.1624797350.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series cat-file: reuse ref-filter logic | expand

Message

Linus Arver via GitGitGadget June 27, 2021, 12:35 p.m. UTC
This patch series make cat-file reuse ref-filter logic.

Change from last version:

 1. Amend part of the description of git for-each-ref.txt.
 2. Modify the code style.
 3. Do not assign the 0 to the variable ret during it's initialization.

ZheNing Hu (15):
  [GSOC] ref-filter: add obj-type check in grab contents
  [GSOC] ref-filter: add %(raw) atom
  [GSOC] ref-filter: --format=%(raw) re-support --perl
  [GSOC] ref-filter: use non-const ref_format in *_atom_parser()
  [GSOC] ref-filter: add %(rest) atom
  [GSOC] ref-filter: pass get_object() return value to their callers
  [GSOC] ref-filter: introduce free_ref_array_item_value() function
  [GSOC] ref-filter: add cat_file_mode in struct ref_format
  [GSOC] ref-filter: modify the error message and value in get_object
  [GSOC] cat-file: add has_object_file() check
  [GSOC] cat-file: change batch_objects parameter name
  [GSOC] cat-file: reuse ref-filter logic
  [GSOC] cat-file: reuse err buf in batch_object_write()
  [GSOC] cat-file: re-implement --textconv, --filters options
  [GSOC] ref-filter: remove grab_oid() function

 Documentation/git-cat-file.txt     |   6 +
 Documentation/git-for-each-ref.txt |   9 +
 builtin/cat-file.c                 | 277 ++++++----------------
 builtin/tag.c                      |   2 +-
 quote.c                            |  17 ++
 quote.h                            |   1 +
 ref-filter.c                       | 357 ++++++++++++++++++++++-------
 ref-filter.h                       |  14 +-
 t/t1006-cat-file.sh                | 252 ++++++++++++++++++++
 t/t3203-branch-output.sh           |   4 +
 t/t6300-for-each-ref.sh            | 235 +++++++++++++++++++
 t/t6301-for-each-ref-errors.sh     |   2 +-
 t/t7004-tag.sh                     |   4 +
 t/t7030-verify-tag.sh              |   4 +
 14 files changed, 888 insertions(+), 296 deletions(-)


base-commit: 1197f1a46360d3ae96bd9c15908a3a6f8e562207
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-980%2Fadlternative%2Fcat-file-batch-refactor-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-980/adlternative/cat-file-batch-refactor-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/980

Range-diff vs v5:

  1:  f72ad9cc5e8 =  1:  f72ad9cc5e8 [GSOC] ref-filter: add obj-type check in grab contents
  2:  4e473838b9d !  2:  d9bc50c4ae6 [GSOC] ref-filter: add %(raw) atom
     @@ Commit message
      
          Mentored-by: Christian Couder <christian.couder@gmail.com>
          Mentored-by: Hariom Verma <hariom18599@gmail.com>
     +    Helped-by: Bagas Sanjaya <bagasdotme@gmail.com>
          Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
          Helped-by: Felipe Contreras <felipe.contreras@gmail.com>
          Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
     @@ Documentation/git-for-each-ref.txt: and `date` to extract the named component.
      +	The raw data size of the object.
      +
      +Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,
     -+`--perl` because the host language may not support arbitrary binary data in the
     -+variables of its string type.
     ++`--perl` because the such language may not support arbitrary binary data in their
     ++string variable type.
      +
       The message in a commit or a tag object is `contents`, from which
       `contents:<part>` can be used to extract various parts out of:
     @@ t/t6300-for-each-ref.sh: test_atom refs/myblobs/first contents:body ""
      +	printf "  " >blob7 &&
      +	>blob8 &&
      +	obj=$(git hash-object -w blob1) &&
     -+        git update-ref refs/myblobs/blob1 "$obj" &&
     ++	git update-ref refs/myblobs/blob1 "$obj" &&
      +	obj=$(git hash-object -w blob2) &&
     -+        git update-ref refs/myblobs/blob2 "$obj" &&
     ++	git update-ref refs/myblobs/blob2 "$obj" &&
      +	obj=$(git hash-object -w blob3) &&
     -+        git update-ref refs/myblobs/blob3 "$obj" &&
     ++	git update-ref refs/myblobs/blob3 "$obj" &&
      +	obj=$(git hash-object -w blob4) &&
     -+        git update-ref refs/myblobs/blob4 "$obj" &&
     ++	git update-ref refs/myblobs/blob4 "$obj" &&
      +	obj=$(git hash-object -w blob5) &&
     -+        git update-ref refs/myblobs/blob5 "$obj" &&
     ++	git update-ref refs/myblobs/blob5 "$obj" &&
      +	obj=$(git hash-object -w blob6) &&
     -+        git update-ref refs/myblobs/blob6 "$obj" &&
     ++	git update-ref refs/myblobs/blob6 "$obj" &&
      +	obj=$(git hash-object -w blob7) &&
     -+        git update-ref refs/myblobs/blob7 "$obj" &&
     ++	git update-ref refs/myblobs/blob7 "$obj" &&
      +	obj=$(git hash-object -w blob8) &&
     -+        git update-ref refs/myblobs/blob8 "$obj"
     ++	git update-ref refs/myblobs/blob8 "$obj"
      +'
      +
      +test_expect_success 'Verify sorts with raw' '
  3:  765cf08a108 !  3:  47f868f63d9 [GSOC] ref-filter: --format=%(raw) re-support --perl
     @@ Documentation/git-for-each-ref.txt: raw:size::
       	The raw data size of the object.
       
       Note that `--format=%(raw)` can not be used with `--python`, `--shell`, `--tcl`,
     --`--perl` because the host language may not support arbitrary binary data in the
     -+because the host language may not support arbitrary binary data in the
     - variables of its string type.
     +-`--perl` because the such language may not support arbitrary binary data in their
     ++because the such language may not support arbitrary binary data in their
     + string variable type.
       
       The message in a commit or a tag object is `contents`, from which
      
  4:  d2aeafd0ef3 =  4:  debca156470 [GSOC] ref-filter: use non-const ref_format in *_atom_parser()
  5:  1ca3a42f041 =  5:  cb0df2b8207 [GSOC] ref-filter: add %(rest) atom
  6:  67f1a3cca9a !  6:  9873354930a [GSOC] ref-filter: pass get_object() return value to their callers
     @@ ref-filter.c: static int populate_value(struct ref_array_item *ref, struct strbu
       {
       	struct object *obj;
       	int i;
     -+	int ret = 0;
     ++	int ret;
       	struct object_info empty = OBJECT_INFO_INIT;
       
       	CALLOC_ARRAY(ref->value, used_atom_cnt);
     @@ ref-filter.c: int format_ref_array_item(struct ref_array_item *info,
       {
       	const char *cp, *sp, *ep;
       	struct ref_formatting_state state = REF_FORMATTING_STATE_INIT;
     -+	int ret = 0;
     ++	int ret;
       
       	state.quote_style = format->quote_style;
       	push_stack_element(&state.stack);
  7:  2a48a48e81c =  7:  e592c21ea1d [GSOC] ref-filter: introduce free_ref_array_item_value() function
  8:  be55005be75 =  8:  b6e7757de4c [GSOC] ref-filter: add cat_file_mode in struct ref_format
  9:  937f88b7837 =  9:  85686187d49 [GSOC] ref-filter: modify the error message and value in get_object
 10:  45657499c55 = 10:  6037295ee58 [GSOC] cat-file: add has_object_file() check
 11:  bf5c0a017ad = 11:  32e1ca56389 [GSOC] cat-file: change batch_objects parameter name
 12:  370101ba65f ! 12:  9a1f0732940 [GSOC] cat-file: reuse ref-filter logic
     @@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
      -		fflush(stdout);
      -		return;
      -	}
     -+	int ret = 0;
     ++	int ret;
      +	struct strbuf err = STRBUF_INIT;
      +	struct ref_array_item item = { data->oid, data->rest };
       
     @@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
      -		print_object_or_die(opt, data);
      -		batch_write(opt, "\n", 1);
      +	ret = format_ref_array_item(&item, &opt->format, scratch, &err);
     -+	if (ret < 0) {
     ++	if (ret < 0)
      +		die("%s\n", err.buf);
     -+	} if (ret) {
     ++	if (ret) {
      +		/* ret > 0 means when the object corresponding to oid
      +		 * cannot be found in format_ref_array_item(), we only print
      +		 * the error message.
 13:  69eef47065d ! 13:  3fb47584924 [GSOC] cat-file: reuse err buf in batch_object_write()
     @@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
       			       struct batch_options *opt,
       			       struct expand_data *data)
       {
     - 	int ret = 0;
     + 	int ret;
      -	struct strbuf err = STRBUF_INIT;
       	struct ref_array_item item = { data->oid, data->rest };
       
     @@ builtin/cat-file.c: static void batch_write(struct batch_options *opt, const voi
       
      -	ret = format_ref_array_item(&item, &opt->format, scratch, &err);
      +	ret = format_ref_array_item(&item, &opt->format, scratch, err);
     - 	if (ret < 0) {
     + 	if (ret < 0)
      -		die("%s\n", err.buf);
      +		die("%s\n", err->buf);
     - 	} if (ret) {
     + 	if (ret) {
       		/* ret > 0 means when the object corresponding to oid
       		 * cannot be found in format_ref_array_item(), we only print
       		 * the error message.
 14:  a7ac037a946 = 14:  e0b1a05e711 [GSOC] cat-file: re-implement --textconv, --filters options
 15:  843de8864a9 = 15:  891d62fd93f [GSOC] ref-filter: remove grab_oid() function

Comments

Junio C Hamano June 30, 2021, 10:04 p.m. UTC | #1
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This patch series make cat-file reuse ref-filter logic.

Unfortunately this seems to interact with your own
zh/cat-file-batch-fix rather badly.
ZheNing Hu July 1, 2021, 12:39 p.m. UTC | #2
Junio C Hamano <gitster@pobox.com> 于2021年7月1日周四 上午6:04写道:
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > This patch series make cat-file reuse ref-filter logic.
>
> Unfortunately this seems to interact with your own
> zh/cat-file-batch-fix rather badly.
>

Well, it's because I didn't base this patch on it.
That should be easy to achieve.

By the way,  I think patches before "[GSOC] ref-filter: add %(rest) atom"
should belong to "zh/ref-filter-raw-data", and patches after that should belong
to "zh/cat-file-batch-refactor".

Thanks.
--
ZheNing Hu
Junio C Hamano July 1, 2021, 2:17 p.m. UTC | #3
ZheNing Hu <adlternative@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> 于2021年7月1日周四 上午6:04写道:
>>
>> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>>
>> > This patch series make cat-file reuse ref-filter logic.
>>
>> Unfortunately this seems to interact with your own
>> zh/cat-file-batch-fix rather badly.
>>
>
> Well, it's because I didn't base this patch on it.
> That should be easy to achieve.

It is preferrable for contributors try merging their individual
topics with the rest of 'seen' to see if there are potential
conflicts (either textual or semantic) before sending their topics
out.  Not all topics need to build on top of other topics (in fact,
the fewer inter-dependencies they have, the better), but in this
case, I think it makes sense to build one on top of the other.
ZheNing Hu July 9, 2021, 10:04 a.m. UTC | #4
Hi, Junio,

Junio C Hamano <gitster@pobox.com> 于2021年7月1日周四 下午10:17写道:
>
> ZheNing Hu <adlternative@gmail.com> writes:
>
> > Junio C Hamano <gitster@pobox.com> 于2021年7月1日周四 上午6:04写道:
> >>
> >> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >>
> >> > This patch series make cat-file reuse ref-filter logic.
> >>
> >> Unfortunately this seems to interact with your own
> >> zh/cat-file-batch-fix rather badly.
> >>
> >
> > Well, it's because I didn't base this patch on it.
> > That should be easy to achieve.
>
> It is preferrable for contributors try merging their individual
> topics with the rest of 'seen' to see if there are potential
> conflicts (either textual or semantic) before sending their topics
> out.  Not all topics need to build on top of other topics (in fact,
> the fewer inter-dependencies they have, the better), but in this
> case, I think it makes sense to build one on top of the other.
>

I have a "rebase" trouble:

My new feature branch "cat-file-batch-refactor-rebase-version" should base on
zh/cat-file-batch-fix and  zh/ref-filter-atom-type, so last time I choice
(bb9a3a8f77 Merge branch 'zh/cat-file-batch-fix' into jch) as the patch base.

But github only allow me base the patch on a branch, so I choice
"gitgitgadget:seen"
as my github PR base. It causes that some merge commit include in it. [1]

So In order to prevent these "merge" commits from being sent, the GGG mechanism
is modified to reject their merge commits.

Now I can't choice a good branch as my patch base... Have any ideas?

Thanks.

[1]: https://github.com/gitgitgadget/git/pull/989

--
ZheNing Hu