git/builtin/bugreport.c, branch v2.40.3

built-ins: use free() not UNLEAK() if trivial, rm dead code

2022-11-21T03:32:48Z

For a lot of uses of UNLEAK() it would be quite tricky to release the memory involved, or we're missing the relevant *_(release|clear)() functions. But in these cases we have them already, and can just invoke them on the variable(s) involved, instead of UNLEAK(). For "builtin/worktree.c" the UNLEAK() was also added in [1], but the struct member it's unleaking was removed in [2]. The only non-"int" member of that structure is "const char *keep_locked", which comes to us via "argv" or a string literal[3]. We have good visibility via the compiler and tooling (e.g. SANITIZE=address) on bad free()-ing, but none on UNLEAK() we don't need anymore. So let's prefer releasing the memory when it's easy. For "bugreport", "worktree" and "config" we need to start using a "ret = ..." return pattern. For "builtin/bugreport.c" these UNLEAK() were added in [4], and for "builtin/config.c" in [1]. For "config" the code seen here was the only user of the "value" variable. For "ACTION_{RENAME,REMOVE}_SECTION" we need to be sure to return the right exit code in the cases where we were relying on falling through to the top-level. I think there's still a use-case for UNLEAK(), but hat it's changed since then. Using it so that "we can see the real leaks" is counter-productive in these cases. It's more useful to have UNLEAK() be a marker of the remaining odd cases where it's hard to free() the memory for whatever reason. With this change less than 20 of them remain in-tree. 1. 0e5bba53af7 (add UNLEAK annotation for reducing leak false positives, 2017-09-08) 2. d861d34a6ed (worktree: remove extra members from struct add_opts, 2018-04-24) 3. 0db4961c49b (worktree: teach `add` to accept --reason with --lock, 2021-07-15) 4. 0e5bba53af7 and 00d8c311050 (commit: fix "author_ident" leak, 2022-05-12). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Taylor Blau

doc txt & -h consistency: fix mismatching labels

2022-10-13T16:32:56Z

Fix various inconsistencies between command SYNOPSIS and the corresponding -h output where our translatable labels didn't match up. In some cases we need to adjust the prose that follows the SYNOPSIS accordingly, as it refers back to the changed label. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

doc txt & -h consistency: correct padding around "[]()"

2022-10-13T16:32:55Z

The whitespace padding of alternatives should be of the form "[-f | --force]" not "[-f|--force]". Likewise we should not have padding before the first option, so "(--all | ...)" is correct, not "( --all | ... )". Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

doc txt & -h consistency: balance unbalanced "[" and "]"

2022-10-13T16:32:55Z

Fix a "-h" output syntax issue introduced when "--diagnose" was added in aac0e8ffeee (builtin/bugreport.c: create '--diagnose' option, 2022-08-12): We need to close the "[" we opened. The corresponding *.txt change did not have the same issue. The "help -h" output then had one "]" too many, which is an issue introduced in b40845293b5 (help: correct the usage string in -h and documentation, 2021-09-10). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

doc txt & -h consistency: fix incorrect alternates syntax

2022-10-13T16:32:55Z

Fix the incorrect "[-o | --option ]" syntax, which should be "[(-o | --option) ]", we were previously claiming that only the long option accepted the "", which isn't what we meant. This syntax issue for "bugreport" originated in 238b439d698 (bugreport: add tool to generate debugging info, 2020-04-16), and for "diagnose" in 6783fd3cef0 (builtin/diagnose.c: create 'git diagnose' builtin, 2022-08-12), which copied and adjusted "bugreport" documentation and code. In the case of "Documentation/git-stash.txt" and "builtin/stash.c" this is not a "doc txt & -h consistency" change, as we're changing both versions, doing so here makes a subsequent change smaller. In that case fix the incorrect "[-o | --option ]" syntax, which should be "[(-o | --option) ]", we were previously claiming that only the long option accepted the "", which isn't what we meant. The "stash" issue has been with us in both the "-h" and *.txt versions since bd514cada4b (stash: introduce 'git stash store', 2013-06-15). We could claim that this isn't a syntax issue if a "vertical bar binds tighter than option and its argument", but such a rule would change e.g. this "cat-file" SYNOPSIS example to mean something we don't: ... [: | --path= ] We have various other examples where the post-image here is already used, e.g. for "format-patch" ("-o"), "grep" ("-m"), "submodule" ("set-branch -b") etc. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

doc txt & -h consistency: word-wrap

2022-10-13T16:32:55Z

Change the documentation and -h output for those built-in commands where both the -h output and *.txt were lacking in word-wrapping. There are many more built-ins that could use this treatment, this change is narrowed to those where this whitespace change is needed to make the -h and *.txt consistent in the end. In the case of "Documentation/git-hash-object.txt" and "builtin/hash-object.c" this is not a "doc txt & -h consistency" change, as we're changing both versions, doing so here makes a subsequent change smaller. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

builtin/bugreport.c: create '--diagnose' option

2022-08-12T20:20:02Z

Create a '--diagnose' option for 'git bugreport' to collect additional information about the repository and write it to a zipped archive. The '--diagnose' option behaves effectively as an alias for simultaneously running 'git bugreport' and 'git diagnose'. In the documentation, users are explicitly recommended to attach the diagnostics alongside a bug report to provide additional context to readers, ideally reducing some back-and-forth between reporters and those debugging the issue. Note that '--diagnose' may take an optional string arg (either 'stats' or 'all'). If specified without the arg, the behavior corresponds to running 'git diagnose' without '--mode'. As with 'git diagnose', this default is intended to help reduce unintentional leaking of sensitive information). Users can also explicitly specify '--diagnose=(stats|all)' to generate the respective archive created by 'git diagnose --mode=(stats|all)'. Suggested-by: Ævar Arnfjörð Bjarmason Helped-by: Derrick Stolee Signed-off-by: Victoria Dye Signed-off-by: Junio C Hamano

hook-list.h: add a generated list of hooks, like config-list.h

2021-09-27T16:44:54Z

Make githooks(5) the source of truth for what hooks git supports, and punt out early on hooks we don't know about in find_hook(). This ensures that the documentation and the C code's idea about existing hooks doesn't diverge. We still have Perl and Python code running its own hooks, but that'll be addressed by Emily Shaffer's upcoming "git hook run" command. This resolves a long-standing TODO item in bugreport.c of there being no centralized listing of hooks, and fixes a bug with the bugreport listing only knowing about 1/4 of the p4 hooks. It didn't know about the recent "reference-transaction" hook either. We could make the find_hook() function die() or BUG() out if the new known_hook() returned 0, but let's make it return NULL just as it does when it can't find a hook of a known type. Making it die() is overly anal, and unlikely to be what we need in catching stupid typos in the name of some new hook hardcoded in git.git's sources. By making this be tolerant of unknown hook names, changes in a later series to make "git hook run" run arbitrary user-configured hook names will be easier to implement. I have not been able to directly test the CMake change being made here. Since 4c2c38e800 (ci: modification of main.yml to use cmake for vs-build job, 2020-06-26) some of the Windows CI has a hard dependency on CMake, this change works there, and is to my eyes an obviously correct use of a pattern established in previous CMake changes, namely: - 061c2240b1 (Introduce CMake support for configuring Git, 2020-06-12) - 709df95b78 (help: move list_config_help to builtin/help, 2020-04-16) - 976aaedca0 (msvc: add a Makefile target to pre-generate the Visual Studio solution, 2019-07-29) The LC_ALL=C is needed because at least in my locale the dash ("-") is ignored for the purposes of sorting, which results in a different order. I'm not aware of anything in git that has a hard dependency on the order, but e.g. the bugreport output would end up using whatever locale was in effect when git was compiled. Signed-off-by: Ævar Arnfjörð Bjarmason Helped-by: René Scharfe Signed-off-by: Junio C Hamano

hook.c: add a hook_exists() wrapper and use it in bugreport.c

2021-09-27T16:44:54Z

Add a boolean version of the find_hook() function for those callers who are only interested in checking whether the hook exists, not what the path to it is. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano

hook.[ch]: move find_hook() from run-command.c to hook.c

2021-09-27T16:44:54Z

Move the find_hook() function from run-command.c to a new hook.c library. This change establishes a stub library that's pretty pointless right now, but will see much wider use with Emily Shaffer's upcoming "configuration-based hooks" series. Eventually all the hook related code will live in hook.[ch]. Let's start that process by moving the simple find_hook() function over as-is. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano