git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/8] ls-files: add --json to dump the index

From
Derrick Stolee <stolee@gmail.com>
Date
Jun 19, 2019, 13:03 UTC
Message-ID
<e3b8d7bf-486d-d52f-9a33-6a7ff837552e@gmail.com>
In-Reply-To
<20190619095858.30124-2-pclouds@gmail.com>
On 6/19/2019 5:58 AM, Nguyễn Thái Ngọc Duy wrote:
Show 8 quoted lines
> So far we don't have a command to basically dump the index file out,
> with all its glory details. Checking some info, for example, stat
> time, usually involves either writing new code or firing up "xxd" and
> decoding values by yourself.
> 
> This --json is supposed to help that. It dumps the index in a human
> readable format but also easy to be processed with tools. And it will
> print almost enough info to reconstruct the index later.

In an earlier message, I stated that I like the idea of this feature. I know that you wanted to get that feedback before working too hard on the patch series, so now that interest is declared, please add some tests that verify this output looks as you expect.

> In this patch we only dump the main part, not extensions. But at the
> end of the series, the entire index is dumped. The end result could be
> very verbose even on a small repository such as git.git.

I would expect this commit to include a complete "output matches expected" test, and the later patches can update the index to include these extensions then verify that their sections appear in the output using a grep.

Show 5 quoted lines
> +--json::
> +	Dump the entire index content in JSON format. This is for
> +	debugging purposes and the JSON structure may change from time
> +	to time.
> +

"...purposes and the JSON structure may change from time to time" may be better as "...purposes. The JSON structure is subject to change."

> +		OPT_BOOL(0, "json", &show_json,
> +			N_("dump index content in json format")),
We should probably use "JSON" here in the help text.
Show 23 quoted lines
> -	show_files(the_repository, &dir);
> -
> -	if (show_resolve_undo)
> -		show_ru_info(the_repository->index);
> +	if (!show_json) {
> +		show_files(the_repository, &dir);
> +
> +		if (show_resolve_undo)
> +			show_ru_info(the_repository->index);
> +	} else {
> +		struct json_writer jw = JSON_WRITER_INIT;
> +
> +		discard_index(the_repository->index);
> +		the_repository->index->jw = &jw;
> +		if (repo_read_index(the_repository) < 0)
> +			die("index file corrupt");
> +		puts(jw.json.buf);
> +		the_repository->index->jw = NULL;
> +		jw_release(&jw);
> +	}
>  
>  	if (ps_matched) {
>  		int bad;

I see this 'ps_matched' condition at the end, which is related to the '--error-unmatch' option. I added "--error-unmatch foo" to my command and got the appropriate error message:

  error: pathspec 'foo' did not match any file(s) known to git
  Did you forget to 'git add'?

This was sent to stderr while the JSON was in stdout, so this should be appropriate to allow both options. Just pointing it out to make sure this is intended.

Show 15 quoted lines
> +void jw_object_stat_data(struct json_writer *jw, const char *name,
> +			 const struct stat_data *sd)
> +{
> +	jw_object_inline_begin_object(jw, name);
> +	jw_object_intmax(jw, "st_ctime.sec", sd->sd_ctime.sec);
> +	jw_object_intmax(jw, "st_ctime.nsec", sd->sd_ctime.nsec);
> +	jw_object_intmax(jw, "st_mtime.sec", sd->sd_mtime.sec);
> +	jw_object_intmax(jw, "st_mtime.nsec", sd->sd_mtime.nsec);
> +	jw_object_intmax(jw, "st_dev", sd->sd_dev);
> +	jw_object_intmax(jw, "st_ino", sd->sd_ino);
> +	jw_object_intmax(jw, "st_uid", sd->sd_uid);
> +	jw_object_intmax(jw, "st_gid", sd->sd_gid);
> +	jw_object_intmax(jw, "st_size", sd->sd_size);
> +	jw_end(jw);
> +}

If these are all part of the same object, are the "st_" prefixes necessary for every member?

Show 12 quoted lines
> +	/*
> +	 * again redundant info, just so you don't have to decode
> +	 * flags values manually
> +	 */
> +	if (ce->ce_flags & CE_VALID)
> +		jw_object_true(jw, "assume-unchanged");
> +	if (ce->ce_flags & CE_INTENT_TO_ADD)
> +		jw_object_true(jw, "intent-to-add");
> +	if (ce->ce_flags & CE_SKIP_WORKTREE)
> +		jw_object_true(jw, "skip-worktree");
> +	if (ce_stage(ce))
> +		jw_object_intmax(jw, "stage", ce_stage(ce));

I'm really glad these flags are getting expanded! Much easier to understand what's going on this way.

Show 6 quoted lines
> +	if (istate->jw) {
> +		jw_object_begin(istate->jw, jw_pretty);
> +		jw_object_intmax(istate->jw, "version", istate->version);
> +		jw_object_string(istate->jw, "oid", oid_to_hex(&istate->oid));
> +		jw_object_intmax(istate->jw, "st_mtime.sec", istate->timestamp.sec);
> +		jw_object_intmax(istate->jw, "st_mtime.nsec", istate->timestamp.nsec);

Here, the "st_" prefixes are not on every member, but would it be confusing if they were not there? Also, including a "." in a member name may be troublesome for JSON, as that typically means we are accessing a member of an object. Perhaps use _sec and _nsec here and in the earlier stat_data block.

Thanks, -Stolee

Previous: Ævar Arnfjörð BjarmasonNext: Johannes Schindelin
Message 4 of 36 in “Add 'ls-files --json' to dump the index in json”
  1. 0/8 Add 'ls-files --json' to dump the index in jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  2. 1/8 ls-files: add --json to dump the indexNguyễn Thái Ngọc Duy, Jun 19, 2019
  3. Ævar Arnfjörð BjarmasonJun 19, 2019
  4. Derrick StoleeJun 19, 2019
  5. Johannes SchindelinJun 21, 2019
  6. Duy NguyenJun 24, 2019
  7. 2/8 split-index.c: dump "link" extension as jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  8. 3/8 fsmonitor.c: dump "FSMN" extension as jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  9. 4/8 resolve-undo.c: dump "REUC" extension as jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  10. Derrick StoleeJun 19, 2019
  11. 5/8 read-cache.c: dump "EOIE" extension as jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  12. 6/8 read-cache.c: dump "IEOT" extension as jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  13. Derrick StoleeJun 19, 2019
  14. Duy NguyenJun 19, 2019
  15. Derrick StoleeJun 19, 2019
  16. 7/8 cache-tree.c: dump "TREE" extension as jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  17. 8/8 dir.c: dump "UNTR" extension as jsonNguyễn Thái Ngọc Duy, Jun 19, 2019
  18. Derrick StoleeJun 19, 2019
  19. Duy NguyenJun 19, 2019
  20. Derrick StoleeJun 19, 2019
  21. Jeff KingJun 19, 2019
  22. Duy NguyenJun 21, 2019
  23. Jeff KingJun 21, 2019
  24. Johannes SchindelinJun 21, 2019
  25. Duy NguyenJun 21, 2019
  26. Junio C HamanoJun 21, 2019
  27. Jeff KingJun 21, 2019
  28. Johannes SchindelinJun 24, 2019
  29. Johannes SchindelinJun 24, 2019
  30. Duy NguyenJun 24, 2019
  31. Jeff KingJun 21, 2019
  32. Johannes SchindelinJun 24, 2019
  33. Junio C HamanoJun 20, 2019
  34. Jeff HostetlerJun 20, 2019
  35. brian m. carlsonJun 21, 2019
  36. Duy NguyenJun 22, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.