[v6,0/7] scalar: implement the subcommand "diagnose"

Message

Philippe Blain via GitGitGadget May 21, 2022, 3:08 p.m. UTC

Over the course of the years, we developed a sub-command that gathers
diagnostic data into a .zip file that can then be attached to bug reports.
This sub-command turned out to be very useful in helping Scalar developers
identify and fix issues.

Changes since v5:

 * Reworded the missed mentions of the old name of the --add-virtual-file
   option (thanks René!).
 * Renamed misleading variable name from $QUOTED to $PATHNAME (thanks
   Junio!).

Changes since v4:

 * Squashed in Junio's suggested fixups
 * Renamed the option from --add-file-with-content=<name>:<content> to
   --add-virtual-file=<name>:<content>
 * Fixed one instance where I had used error() instead of error_errno().

Changes since v3:

 * We're now using unquote_c_style() instead of rolling our own unquoter.
 * Fixed the added regression test.
 * As pointed out by Scalar's Functional Tests, the
   add_directory_to_archiver() function should not fail when scalar diagnose
   encounters FSMonitor's Unix socket, but only warn instead.
 * Related: add_directory_to_archiver() needs to propagate errors from
   processing subdirectories so that the top-level call returns an error,
   too.

Changes since v2:

 * Clarified in the commit message what the biggest benefit of
   --add-file-with-content is.
 * The <path> part of the -add-file-with-content argument can now contain
   colons. To do this, the path needs to start and end in double-quote
   characters (which are stripped), and the backslash serves as escape
   character in that case (to allow the path to contain both colons and
   double-quotes).
 * Fixed incorrect grammar.
 * Instead of strcmp(<what-we-don't-want>), we now say
   !strcmp(<what-we-want>).
 * The help text for --add-file-with-content was improved a tiny bit.
 * Adjusted the commit message that still talked about spawning plenty of
   processes and about a throw-away repository for the sake of generating a
   .zip file.
 * Simplified the code that shows the diagnostics and adds them to the .zip
   file.
 * The final message that reports that the archive is complete is now
   printed to stderr instead of stdout.

Changes since v1:

 * Instead of creating a throw-away repository, staging the contents of the
   .zip file and then using git write-tree and git archive to write the .zip
   file, the patch series now introduces a new option to git archive and
   uses write_archive() directly (avoiding any separate process).
 * Since the command avoids separate processes, it is now blazing fast on
   Windows, and I dropped the spinner() function because it's no longer
   needed.
 * While reworking the test case, I noticed that scalar [...] <enlistment>
   failed to verify that the specified directory exists, and would happily
   "traverse to its parent directory" on its quest to find a Scalar
   enlistment. That is of course incorrect, and has been fixed as a "while
   at it" sort of preparatory commit.
 * I had forgotten to sign off on all the commits, which has been fixed.
 * Instead of some "home-grown" readdir()-based function, the code now uses
   for_each_file_in_pack_dir() to look through the pack directories.
 * If any alternates are configured, their pack directories are now included
   in the output.
 * The commit message that might be interpreted to promise information about
   large loose files has been corrected to no longer promise that.
 * The test cases have been adjusted to test a little bit more (e.g.
   verifying that specific paths are mentioned in the output, instead of
   merely verifying that the output is non-empty).

Johannes Schindelin (5):
  archive: optionally add "virtual" files
  archive --add-virtual-file: allow paths containing colons
  scalar: validate the optional enlistment argument
  Implement `scalar diagnose`
  scalar diagnose: include disk space information

Matthew John Cheetham (2):
  scalar: teach `diagnose` to gather packfile info
  scalar: teach `diagnose` to gather loose objects information

 Documentation/git-archive.txt    |  17 ++
 archive.c                        |  63 ++++++-
 contrib/scalar/scalar.c          | 292 ++++++++++++++++++++++++++++++-
 contrib/scalar/scalar.txt        |  12 ++
 contrib/scalar/t/t9099-scalar.sh |  27 +++
 t/t5003-archive-zip.sh           |  20 +++
 6 files changed, 421 insertions(+), 10 deletions(-)


base-commit: ddc35d833dd6f9e8946b09cecd3311b8aa18d295
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1128%2Fdscho%2Fscalar-diagnose-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1128/dscho/scalar-diagnose-v6
Pull-Request: https://github.com/gitgitgadget/git/pull/1128

Range-diff vs v5:

 1:  42e73fb0aac ! 1:  0005cfae31d archive: optionally add "virtual" files
     @@ Metadata
       ## Commit message ##
          archive: optionally add "virtual" files
      
     -    With the `--add-file-with-content=<path>:<content>` option, `git
     -    archive` now supports use cases where relatively trivial files need to
     -    be added that do not exist on disk.
     +    With the `--add-virtual-file=<path>:<content>` option, `git archive` now
     +    supports use cases where relatively trivial files need to be added that
     +    do not exist on disk.
      
          This will allow us to generate `.zip` files with generated content,
          without having to add said content to the object database and without
 2:  b5ebd61066a ! 2:  7eebcf27b45 archive --add-file-with-contents: allow paths containing colons
     @@ Metadata
      Author: Johannes Schindelin <[e-mail]>
      
       ## Commit message ##
     -    archive --add-file-with-contents: allow paths containing colons
     +    archive --add-virtual-file: allow paths containing colons
      
          By allowing the path to be enclosed in double-quotes, we can avoid
          the limitation that paths cannot contain colons.
     @@ t/t5003-archive-zip.sh: check_zip with_untracked
       test_expect_success UNZIP 'git archive --format=zip --add-virtual-file' '
      +	if test_have_prereq FUNNYNAMES
      +	then
     -+		QUOTED=quoted:colon
     ++		PATHNAME=quoted:colon
      +	else
     -+		QUOTED=quoted
     ++		PATHNAME=quoted
      +	fi &&
       	git archive --format=zip >with_file_with_content.zip \
     -+		--add-virtual-file=\"$QUOTED\": \
     ++		--add-virtual-file=\"$PATHNAME\": \
       		--add-virtual-file=hello:world $EMPTY_TREE &&
       	test_when_finished "rm -rf tmp-unpack" &&
       	mkdir tmp-unpack && (
       		cd tmp-unpack &&
       		"$GIT_UNZIP" ../with_file_with_content.zip &&
       		test_path_is_file hello &&
     -+		test_path_is_file $QUOTED &&
     ++		test_path_is_file $PATHNAME &&
       		test world = $(cat hello)
       	)
       '
 3:  f1ba69c02d7 = 3:  ca83ddd5eed scalar: validate the optional enlistment argument
 4:  3fb90194744 = 4:  89c13a45e00 Implement `scalar diagnose`
 5:  2e645b08a9e = 5:  8ffbaad3086 scalar diagnose: include disk space information
 6:  0fa20d73750 = 6:  15cd7f17896 scalar: teach `diagnose` to gather packfile info
 7:  62e173b47cf = 7:  a4a74d5ef58 scalar: teach `diagnose` to gather loose objects information