summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2022-08-04git docs: add a category for file formats, protocols and interfacesÆvar Arnfjörð Bjarmason11-13/+94
Create a new "File formats, protocols and other developer interfaces" section in the main "git help git" manual page and start moving the documentation that now lives in "Documentation/technical/*.git" over to it. This complements the newly added and adjacent "Repository, command and file interfaces" section. This makes the technical documentation more accessible and discoverable. Before this we wouldn't install it by default, and had no ability to build man page versions of them. The links to them from our existing documentation link to the generated HTML version of these docs. So let's start moving those over, starting with just the "bundle-format.txt" documentation added in 7378ec90e1c (doc: describe Git bundle format, 2020-02-07). We'll now have a new gitformat-bundle(5) man page. Subsequent commits will move more git internal format documentation over. Unfortunately the syntax of the current Documentation/technical/*.txt is not the same (when it comes to section headings etc.) as our Documentation/*.txt documentation, so change the relevant bits of syntax as we're moving this over. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-04git docs: add a category for user-facing file, repo and command UXÆvar Arnfjörð Bjarmason9-9/+70
Create a new "Repository, command and file interfaces" section in the main "git help git" manual page. Move things that belong under this new criteria from the generic "Guides" section. The "Guides" section was added in f442f28a81b (git.txt: add list of guides, 2020-08-05). It makes sense to have e.g. "giteveryday(7)" and "gitfaq(7)" listed under "Guides". But placing e.g. "gitignore(5)" in it is stretching the meaning of what a "guide" is, ideally that section should list things similar to "giteveryday(7)" and "gitcore-tutorial(7)". An alternate name that was considered for this new section was "User formats", for consistency with the nomenclature used for man section 5 in general. My man(1) lists it as "File formats and conventions, e.g. /etc/passwd". So calling this "git help --formats" or "git help --user-formats" would make sense for e.g. gitignore(5), but would be stretching it somewhat for githooks(5), and would seem really suspect for the likes of gitcli(7). Let's instead pick a name that's closer to the generic term "User interface", which is really what this documentation discusses: General user-interface documentation that doesn't obviously belong elsewhere. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-04git help doc: use "<doc>" instead of "<guide>"Ævar Arnfjörð Bjarmason2-5/+5
Replace the use of "<guide>" originally introduced (as "GUIDE") in a133737b809 (doc: include --guide option description for "git help", 2013-04-02) with the more generic "<doc>". The "<doc>" placeholder is more generic, and one we'll be able to use as we introduce new documentation categories. Let's also add "<doc>" to the "git help -h" output, when it was made to use parse_option() in in 41eb33bd0cb (help: use parseopt, 2008-02-24). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-04help.c: remove common category behavior from drop_prefix() behaviorÆvar Arnfjörð Bjarmason1-4/+5
Change the behavior of the "git" prefix stripping for CAT_guide so that we don't try to strip the "git-" prefix in that case. We should be stripping either "git" or "git-" depending on the category. This change makes it easier to add extra "category" conditions in subsequent commits. Before this we'd in principle strip a "git-" prefix from a "guide" in command-list.txt, in practice we have no such entry there. As we don't have any entry that looks like "git-foo" in command-list.txt this changes nothing in practice, but it makes the intent of the code clearer. In that hypothetical case we'd now strip it down to "-foo", not "foo". When this code was added in cfb22a02ab5 (help: use command-list.h for common command list, 2018-05-10) the only entries in command-list.txt that didn't begin with "git-" were "gitweb" and "gitk". Then when the "guides" special-case was added in 1b81d8cb19d (help: use command-list.txt for the source of guides, 2018-05-20) we had the various "git" (not "git-") prefixed "guide" entries, which the "CAT_guide" case handles. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-04help.c: refactor drop_prefix() to use a "switch" statement"Ævar Arnfjörð Bjarmason1-2/+8
Refactor the drop_prefix() function in in help.c to make it easier to strip prefixes from categories that aren't "CAT_guide". There are no functional changes here, by doing this we make a subsequent functional change's diff smaller. As before we first try to strip "git-" unconditionally, if that works we'll return the stripped string. Then we'll strip "git" if the command is in "CAT_guide". This means that we'd in principle strip "git-foo" down to "foo" if it's in CAT_guide. That doesn't make much sense, and we don't have such an entry in command-list.txt, but let's preserve that behavior for now. While we're at it remove a stray newline that had been added after the "return name;" statement. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-04submodule merge: update conflict error messageCalvin Wan3-17/+231
When attempting to merge in a superproject with conflicting submodule pointers that cannot be fast-forwarded or trivially resolved, the merge fails and Git prints an error message that accurately describes the failure, but does not provide steps for the user to resolve the error. Git is left in a conflicted state, which requires the user to: 1. merge submodules or update submodules to an already existing commit that reflects the merge 2. add submodules changes to the superproject 3. finish merging superproject These steps are non-obvious for newer submodule users to figure out based on the error message and neither `git submodule status` nor `git status` provide any useful pointers. Update error message to provide steps to resolve submodule merge conflict. Future work could involve adding an advice flag to the message. Although the message is long, it also has the id of the submodule commit that needs to be merged, which could be useful information for the user. Additionally, 5 merge failures that resulted in an early return have been updated to reflect the status of the merge. 1. Null merge base (null o): CONFLICT_SUBMODULE_NULL_MERGE_BASE added as a new conflict type and will print updated error message. 2. Null merge side a (null a): BUG(). See [1] for discussion 3. Null merge side b (null b): BUG(). See [1] for discussion 4. Submodule not checked out: added NEEDSWORK bit 5. Submodule commits not present: added NEEDSWORK bit The errors with a NEEDSWORK bit deserve a more detailed explanation of how to resolve them. See [2] for more context. [1] https://lore.kernel.org/git/CABPp-BE0qGwUy80dmVszkJQ+tcpfLRW0OZyErymzhZ9+HWY1mw@mail.gmail.com/ [2] https://lore.kernel.org/git/xmqqpmhjjwo9.fsf@gitster.g/ Signed-off-by: Calvin Wan <calvinwan@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-04tests: cache glibc version checkPhillip Wood1-3/+8
131b94a10a ("test-lib.sh: Use GLIBC_TUNABLES instead of MALLOC_CHECK_ on glibc >= 2.34", 2022-03-04) introduced a check for the version of glibc that is in use. This check is performed as part of setup_malloc_check() which is called at least once for each test. As the test involves forking `getconf` and `expr` cache the result and use that within setup_malloc_check() to avoid forking these extra processes for each test. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03doc: clarify rerere-autoupdateJunio C Hamano1-2/+7
The "--[no-]rerere-autoupdate" option controls what happens _after_ the rerere mechanism kicks in to reuse recorded resolutions and does not prevent from the rerere mechanism to trigger in the first place. It is unclear in the current text if "--no-rerere-autoupdate" stops the auto-resolution. Rewrite the sentence to clarify. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03doc: consolidate --rerere-autoupdate descriptionJunio C Hamano6-20/+9
The `--rerere-autoupdate` option is shared across 5 commands, and are described the same way because it works exactly the same way in these commands. Create a separate file and include it from the help pages for these commands, so that we can improve the description at one place to improve all of them at once, and keep them in sync. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03The ninth batchJunio C Hamano1-0/+25
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03Merge branch 'jc/string-list-cleanup'Junio C Hamano1-2/+1
Code clean-up. * jc/string-list-cleanup: builtin/remote.c: use the right kind of STRING_LIST_INIT
2022-08-03Merge branch 'en/merge-restore-to-pristine'Junio C Hamano7-17/+154
When "git merge" finds that it cannot perform a merge, it should restore the working tree to the state before the command was initiated, but in some corner cases it didn't. * en/merge-restore-to-pristine: merge: do not exit restore_state() prematurely merge: ensure we can actually restore pre-merge state merge: make restore_state() restore staged state too merge: fix save_state() to work when there are stat-dirty files merge: do not abort early if one strategy fails to handle the merge merge: abort if index does not match HEAD for trivial merges merge-resolve: abort if index does not match HEAD merge-ort-wrappers: make printed message match the one from recursive
2022-08-03Merge branch 'rs/mergesort'Junio C Hamano12-199/+134
Make our mergesort implementation type-safe. * rs/mergesort: mergesort: remove llist_mergesort() packfile: use DEFINE_LIST_SORT fetch-pack: use DEFINE_LIST_SORT commit: use DEFINE_LIST_SORT blame: use DEFINE_LIST_SORT test-mergesort: use DEFINE_LIST_SORT test-mergesort: use DEFINE_LIST_SORT_DEBUG mergesort: add macros for typed sort of linked lists mergesort: tighten merge loop mergesort: unify ranks loops
2022-08-03Merge branch 'sa/cat-file-mailmap'Junio C Hamano6-48/+190
"git cat-file" learned an option to use the mailmap when showing commit and tag objects. * sa/cat-file-mailmap: cat-file: add mailmap support ident: rename commit_rewrite_person() to apply_mailmap_to_header() ident: move commit_rewrite_person() to ident.c revision: improve commit_rewrite_person()
2022-08-03Merge branch 'zh/ls-files-format'Junio C Hamano3-1/+228
"git ls-files" learns the "--format" option to tweak its output. * zh/ls-files-format: ls-files: introduce "--format" option
2022-08-03Merge branch 'tb/commit-graph-genv2-upgrade-fix'Junio C Hamano4-8/+56
There was a bug in the codepath to upgrade generation information in commit-graph from v1 to v2 format, which has been corrected. * tb/commit-graph-genv2-upgrade-fix: commit-graph: fix corrupt upgrade from generation v1 to v2 commit-graph: introduce `repo_find_commit_pos_in_graph()` t5318: demonstrate commit-graph generation v2 corruption
2022-08-03Merge branch 'tk/untracked-cache-with-uall'Junio C Hamano2-0/+7
Fix for a bug that makes write-tree to fail to write out a non-existent index as a tree, introduced in 2.37. * tk/untracked-cache-with-uall: read-cache: make `do_read_index()` always set up `istate->repo`
2022-08-03Merge branch 'pw/xdiff-alloc'Junio C Hamano8-50/+60
Add a level of redirection to array allocation API in xdiff part, to make it easier to share with the libgit2 project. * pw/xdiff-alloc: xdiff: introduce XDL_ALLOC_GROW() xdiff: introduce XDL_CALLOC_ARRAY() xdiff: introduce xdl_calloc xdiff: introduce XDL_ALLOC_ARRAY()
2022-08-03Merge branch 'ds/midx-with-less-memory'Junio C Hamano3-28/+48
The codepath to write multi-pack index has been taught to release a large chunk of memory that holds an array of objects in the packs, as soon as it is done with the array, to reduce memory consumption. * ds/midx-with-less-memory: write_midx_bitmap(): drop unused refs_snapshot parameter midx: reduce memory pressure while writing bitmaps midx: extract bitmap write setup pack-bitmap-write: use const for hashes
2022-08-03revisions API: don't leak memory on argv elements that need free()-ingÆvar Arnfjörð Bjarmason6-5/+17
Add a "free_removed_argv_elements" member to "struct setup_revision_opt", and use it to fix several memory leaks. We have various memory leaks in APIs that take and munge "const char **argv", e.g. parse_options(). Sometimes these APIs are given the "argv" we get to the "main" function, in which case we don't leak memory, but other times we're giving it the "v" member of a "struct strvec" we created. There's several potential ways to fix those sort of leaks, we could add a "nodup" mode to "struct strvec", which would work for the cases where we push constant strings to it. But that wouldn't work as soon as we used strvec_pushf(), or otherwise needed to duplicate or create a string for that "struct strvec". Let's instead make it the responsibility of the revisions API. If it's going to clobber elements of argv it can also free() them, which it will now do if instructed to do so via "free_removed_argv_elements". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03bisect.c: partially fix bisect_rev_setup() memory leakÆvar Arnfjörð Bjarmason1-9/+13
Partially fix the memory leak noted in in 8a534b61241 (bisect: use argv_array API, 2011-09-13), which added the "XXX" comment seen in the context. We can partially fix it by having the bisect_rev_setup() function take a "struct strvec", rather than constructing it. As the comment notes we need to keep the construct "rev_argv" around while the "struct rev_info" is around, which as seen in the newly added "strvec_clear()" calls here we do after "release_revisions()". This "partially" fixes the memory leak because we're leaking the "--" added to the "rev_argv" here still, which will be addressed in a subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03log: refactor "rev.pending" code in cmd_show()Ævar Arnfjörð Bjarmason1-10/+15
Refactor the juggling of "rev.pending" and our replacement for it amended in the preceding commit so that: * We use an "unsigned int" instead of an "int" for "i", this matches the types of "struct rev_info" itself. * We don't need the "count" and "objects" variables introduced in 5d7eeee2ac6 (git-show: grok blobs, trees and tags, too, 2006-12-14). They were originally added since we'd clobber rev.pending in the loop without restoring it. Since the preceding commit we are restoring it when we handle OBJ_COMMIT, so the main for-loop can refer to "rev.pending" didrectly. * We use the "memcpy a &blank" idiom introduced in 5726a6b4012 (*.c *_init(): define in terms of corresponding *_INIT macro, 2021-07-01). This is more obvious than relying on us enumerating all of the relevant members of the "struct object_array" that we need to clear. * We comment on why we don't need an object_array_clear() here, see the analysis in [1]. 1. https://lore.kernel.org/git/YuQtJ2DxNKX%2Fy70N@coredump.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03log: fix a memory leak in "git show <revision>..."Ævar Arnfjörð Bjarmason10-2/+14
Fix a memory leak in code added in 5d7eeee2ac6 (git-show: grok blobs, trees and tags, too, 2006-12-14). As we iterate over a "<revision>..." command-line and encounter ad OBJ_COMMIT we want to use our "struct rev_info", but with a "pending" array of one element: the one commit we're showing in the loop. To do this 5d7eeee2ac6 saved away a pointer to rev.pending.objects and rev.pending.nr for its iteration. We'd then clobber those (and alloc) when we needed to show an OBJ_COMMIT. We'd therefore leak the "rev.pending" we started out with, and only free the new "rev.pending" in the "OBJ_COMMIT" case arm as prepare_revision_walk() would draw it down. Let's fix this memory leak. Now when we encounter an OBJ_COMMIT we save away the "rev.pending" before clearing it. We then add a single commit to it, which our indirect invocation of prepare_revision_walk() will remove. After that we restore the "rev.pending". Our "rev.pending" will then get free'd by the release_revisions() added in f6bfea0ad01 (revisions API users: use release_revisions() in builtin/log.c, 2022-04-13) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03test-fast-rebase helper: use release_revisions() (again)Ævar Arnfjörð Bjarmason1-2/+0
Fix a bug in 0139c58ab95 (revisions API users: add "goto cleanup" for release_revisions(), 2022-04-13), in that commit a release_revisions() call was added to this function, but it never did anything due to this TODO memset() added in fe1a21d5267 (fast-rebase: demonstrate merge-ort's API via new test-tool command, 2020-10-29). Simply removing the memset() will fix the "cmdline" which can be seen when running t5520-pull.sh. This sort of thing could be detected automatically with a rule similar to the unused.cocci merged in 7fa60d2a5b6 (Merge branch 'ab/cocci-unused' into next, 2022-07-11). The following rule on top would catch the case being fixed here: @@ type T; identifier I; identifier REL1 =~ "^[a-z_]*_(release|reset|clear|free)$"; identifier REL2 =~ "^(release|clear|free)_[a-z_]*$"; @@ - memset(\( I \| &I \), 0, ...); ... when != \( I \| &I \) ( \( REL1 \| REL2 \)( \( I \| &I \), ...); | \( REL1 \| REL2 \)( \( &I \| I \) ); ) ... when != \( I \| &I \) That rule should arguably use only &I, not I (as we might be passed a pointer). The distinction would matter if anyone cared about the side-effects of a memset() followed by release() of a pointer to a variable passed into the function. As such a pattern would be at best very confusing, and most likely point to buggy code as in this case, the above rule is probably fine as-is. But as this rule only found one such bug in the entire codebase let's not add it to contrib/coccinelle/unused.cocci for now, we can always dig it up in the future if it's deemed useful. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-03bisect.c: add missing "goto" for release_revisions()Ævar Arnfjörð Bjarmason1-1/+1
Add a missing "goto cleanup", this fixes a bug in f196c1e908d (revisions API users: use release_revisions() needing REV_INFO_INIT, 2022-04-13). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-01Documentation/git-reflog: remove unneeded \ from \{Glen Choo1-1/+1
There are some inconsistencies with how different asciidoc environments handle different combinations of "\{<>}", e.g. these results were observed with asciidoc on two different environments: | Input | Output (env A) | Output (env B) | same/different | |-----------+----------------+------------------+----------------| | \{<foo>\} | {&lt;foo&gt;} | \{&lt;foo&gt;}^M | different | | {<foo>} | {&lt;foo&gt;} | {&lt;foo&gt;} | same | | \{<foo>} | {&lt;foo&gt;} | \{&lt;foo&gt;}^M | different | | \{foo\} | {foo} | {foo} | same | | \{\} | {} | \{}^M | different | | \{} | {} | {} | same | | {\} | {} | {} | same | The only instance of this biting us is "@\{<specifier>\}" in Documentation/git-reflog.txt; all other combinations of "\{<>}" (e.g. in Documentation/revisions.txt) seem to render consistently. Fix this inconsistent rendering by removing the unnecessary "\" in Documentation/git-reflog.txt. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-01symbolic-ref: refuse to set syntactically invalid targetLinus Torvalds3-2/+14
You can feed absolute garbage to symbolic-ref as a target like: git symbolic-ref HEAD refs/heads/foo..bar While this doesn't technically break the repo entirely (our "is it a git directory" detector looks only for "refs/" at the start), we would never resolve such a ref, as the ".." is invalid within a refname. Let's flag these as invalid at creation time to help the caller realize that what they're asking for is bogus. A few notes: - We use REFNAME_ALLOW_ONELEVEL here, which lets: git update-ref refs/heads/foo FETCH_HEAD continue to work. It's unclear whether anybody wants to do something so odd, but it does work now, so this is erring on the conservative side. There's a test to make sure we didn't accidentally break this, but don't take that test as an endorsement that it's a good idea, or something we might not change in the future. - The test in t4202-log.sh checks how we handle such an invalid ref on the reading side, so it has to be updated to touch the HEAD file directly. - We need to keep our HEAD-specific check for "does it start with refs/". The ALLOW_ONELEVEL flag means we won't be enforcing that for other refs, but HEAD is special here because of the checks in validate_headref(). Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-01The eighth batchJunio C Hamano1-0/+16
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-01Merge branch 'mt/pkt-line-comment-tweak'Junio C Hamano1-8/+8
In-code comment clarification. source: <6a14443c101fa132498297af6d7a483520688d75.1658488203.git.matheus.bernardino@usp.br> * mt/pkt-line-comment-tweak: pkt-line.h: move comment closer to the associated code
2022-08-01Merge branch 'mt/checkout-count-fix'Junio C Hamano10-24/+113
"git checkout" miscounted the paths it updated, which has been corrected. source: <cover.1657799213.git.matheus.bernardino@usp.br> * mt/checkout-count-fix: checkout: fix two bugs on the final count of updated entries checkout: show bug about failed entries being included in final report checkout: document bug where delayed checkout counts entries twice
2022-08-01Merge branch 'cl/rerere-train-with-no-sign'Junio C Hamano1-1/+1
"rerere-train" script (in contrib/) used to honor commit.gpgSign while recreating the throw-away merges. source: <PH7PR14MB5594A27B9295E95ACA4D6A69CE8F9@PH7PR14MB5594.namprd14.prod.outlook.com> * cl/rerere-train-with-no-sign: contrib/rerere-train: avoid useless gpg sign in training
2022-08-01Merge branch 'ds/rebase-update-ref'Junio C Hamano10-44/+895
"git rebase -i" learns to update branches whose tip appear in the rebased range with "--update-refs" option. source: <pull.1247.v5.git.1658255624.gitgitgadget@gmail.com> * ds/rebase-update-ref: sequencer: notify user of --update-refs activity sequencer: ignore HEAD ref under --update-refs rebase: add rebase.updateRefs config option sequencer: rewrite update-refs as user edits todo list rebase: update refs from 'update-ref' commands rebase: add --update-refs option sequencer: add update-ref command sequencer: define array with enum values rebase-interactive: update 'merge' description branch: consider refs under 'update-refs' t2407: test branches currently using apply backend t2407: test bisect and rebase as black-boxes
2022-08-01Merge branch 'kk/p4-client-name-encoding-fix'Junio C Hamano1-9/+42
"git p4" did not handle non-ASCII client name well, which has been corrected. source: <pull.1285.v3.git.git.1658394440.gitgitgadget@gmail.com> * kk/p4-client-name-encoding-fix: git-p4: refactoring of p4CmdList() git-p4: fix bug with encoding of p4 client name
2022-07-31merge-ort: do leave trace2 region even if checkout failsJohannes Schindelin1-0/+3
In 557ac0350d9 (merge-ort: begin performance work; instrument with trace2_region_* calls, 2021-01-23), we added Trace2 instrumentation, but in the error path that returns early, we forgot to tell Trace2 that we're leaving the region. Let's fix that. Pointed-out-by: Elijah Newren <newren@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-31merge-ort: clean up after failed mergeJohannes Schindelin1-0/+2
In 9fefce68dc8 (merge-ort: basic outline for merge_switch_to_result(), 2020-12-13), we added functionality to lay down the result of a merge on disk. But we forgot to release the data structures in case `unpack_trees()` failed to run properly. This was pointed out by the `linux-leaks` job in our CI runs. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-31config.mak.dev: squelch -Wno-missing-braces for older gccJeff King1-0/+4
Versions of gcc prior to 4.9 complain about an initialization like: struct inner { int x; }; struct outer { struct inner; }; struct outer foo = { 0 }; and insist on: struct outer foo = { { 0 } }; Newer compilers handle this just fine. And ignoring the window even on older compilers is fine; the resulting code is correct, but we just get caught by -Werror. Let's relax this for older compilers to make developer lives easier (we don't care much about non-developers on old compilers; they may see a warning, but it won't stop compilation). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-29lstat(mingw): correctly detect ENOTDIR scenariosJohannes Schindelin1-2/+2
Files' attributes can indicate more than just whether they are files or directories. It was reported in Git for Windows that on certain network shares, this led to a nasty problem trying to create tags: $ git tag -a -m "automatic tag creation" test_dir/test_tag fatal: cannot lock ref 'refs/tags/test_dir/test_tag': unable to resolve reference 'refs/tags/test_dir/test_tag': Not a directory Note: This does not necessarily happen with all types of network shares. One setup where it _did_ happen is a Windows Server 2019 VM, and as hinted in http://woshub.com/slow-network-shared-folder-refresh-windows-server/ in the indicated instance the following commands worked around the bug: Set-SmbClientConfiguration -DirectoryCacheLifetime 0 Set-SmbClientConfiguration -FileInfoCacheLifetime 0 Set-SmbClientConfiguration -FileNotFoundCacheLifetime 0 This would impact performance negatively, though, as it essentially turns off all caching, therefore we do not want to require users to do that just to be able to use Git on Windows. The underlying bug is in the code added in 4b0abd5c695 (mingw: let lstat() fail with errno == ENOTDIR when appropriate, 2016-01-26) that emulates the POSIX behavior where `lstat()` should return `ENOENT` if the file or directory simply does not exist but could be created, and `ENOTDIR` if there is no file or directory nor could there be because a leading path already exists and is not a directory. In that code, the return value of `GetFileAttributesW()` is interpreted as an enum value, not as a bit field, so that a perfectly fine leading directory can be misdetected as "not a directory". As a consequence, the `read_refs_internal()` function would return `ENOTDIR`, suggesting not only that the tag in the `git tag` invocation above does not exist, but that it cannot even be created. Let's fix the code so that it interprets the return value of the `GetFileAttributesW()` call correctly. This fixes https://github.com/git-for-windows/git/issues/3727 Reported-by: Pierre Garnier <pgarnier@mega.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-29mingw: remove unneeded `NO_CURL` directiveJohannes Schindelin1-1/+0
In df5218b4c30 (config.mak.uname: support MSys2, 2016-01-13), we introduced support for building Git for Windows in the then-brand new Git for Windows v2.x build environment that was based off of MSYS2. To do that, we split the non-msysGit part (that targeted MSys1) in two, and instead of sharing the `NO_CURL = YesPlease` setting with MSys1, we overrode it for MSYS2 with the empty value because we very much want to build Git for Windows with libcurl. But that was unnecessary: we never set that variable beforehand, therefore there is no need to override it. Let's just remove that unnecessary line. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-29mingw: remove unneeded `NO_GETTEXT` directiveJohannes Schindelin1-1/+0
In f9206ce2681 (mingw: let's use gettext with MSYS2, 2016-01-26), we flipped the switch to build Git for Windows with support for gettext. However, the way we flipped the switch was by changing the value of the `NO_GETTEXT` variable from a non-empty string to the empty string, as if there was any `NO_GETTEXT` definition we needed to override. But that was a mistake: while there _is_ a definition, it is in the `THIS_IS_MSYSGIT` section, i.e. it does not affect the Git for Windows part at all. Let's just remove that unnecessary line. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-29windows: include the Python bits when building Git for WindowsJohannes Schindelin1-1/+2
While Git for Windows does not _ship_ Python (in order to save on bandwidth), MSYS2 provides very fine Python interpreters that users can easily take advantage of, by using Git for Windows within its SDK. Previously, we excluded the Python bits, mostly due to historical reasons: In the Git for Windows v1.x days, we built Git using MSys/MinGW, without support for any Python scripts. Therefore, let's move out the `NO_PYTHON` definition from the generic part of the MINGW section (which includes special handling for MSYS2/Git for Windows, for the long-superseded msysGit environment, as well as for the setup of probably just one developer remaining with their MSys1) into the two sections that cover different environments than Git for Windows' SDK. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-29t5351: avoid using `test_cmp` for binary dataJohannes Schindelin1-1/+1
The `test_cmp` function is meant to provide nicer output than `cmp` when expected and actual output of Git commands disagree. The implicit assumption is that the output is line-based and human readable. However, aaf81223f48 (unpack-objects: use stream_loose_object() to unpack large objects, 2022-06-11) introduced a call that compares the contents of pack files, which are distinctly not line-based nor human readable. This causes problems because on Windows, we hand off to the Bash function `mingw_test_cmp` that compares the lines while ignoring line ending differences. And this Bash function spends an insane amount of cycles trying to read in that binary pack file, so that it is almost indistinguishable from an infinite loop. For example, t5351 took 1486 seconds in the CI run at https://github.com/git/git/runs/7398490747?check_suite_focus=true#step:5:171, to complete. And yes, that is almost half an hour. Since Git's tests already use `cmp` consistently when comparing pack files, let's change this instance to use `cmp` instead of `test_cmp`, too, and fix that performance problem. Now t5351 takes all of 22 seconds. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-29t5351: avoid relying on `core.fsyncMethod = batch` to be supportedJohannes Schindelin2-2/+10
On FreeBSD, this mode is not supported. But since 3a251bac0d1a (trace2: only include "fsync" events if we git_fsync(), 2022-07-18) t5351 will fail if this mode is unsupported. Let's address this in the minimal fashion, by detecting that that mode is unsupported and expecting a different count of hardware flushes in that case. This fixes the CI/PR builds on FreeBSD again. Note: A better way would be to test only what is relevant in t5351.6 "unpack big object in stream (core.fsyncmethod=batch)" again instead of blindly comparing the output against some exact text. But that would pretty much revert the idea of above-mentioned commit, and that commit was _just_ accepted into Git's main branch so one must assume that it was intentional. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27CI: use "GIT_TEST_SANITIZE_LEAK_LOG=true" in linux-leaksÆvar Arnfjörð Bjarmason1-0/+1
As noted in a preceding commit the leak checking done by "GIT_TEST_PASSING_SANITIZE_LEAK=true" (added in [1]) is incomplete without combining it with "GIT_TEST_SANITIZE_LEAK_LOG=true". Let's run our CI with that, to ensure that we catch cases where our tests are missing the abort() exit code resulting from a leak for whatever reason. The reasons for that are discussed in detail in a preceding commit. 1. 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI, 2021-09-23) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27upload-pack: fix a memory leak in create_pack_file()Ævar Arnfjörð Bjarmason2-0/+2
Fix a memory leak that's been reported by some versions of "gcc" since "output_state" became malloc'd in 55a9651d26a (upload-pack.c: increase output buffer size, 2021-12-14). In e75d2f7f734 (revisions API: have release_revisions() release "filter", 2022-04-13) it was correctly marked as leak-free, the only path through this function that doesn't reach the free(output_state) is if we "goto fail", and that will invoke "die()". Such leaks are not included with SANITIZE=leak (but e.g. valgrind will still report them), but under some gcc optimization (I have not been able to reproduce it with "clang") we'll report a leak here anyway. E.g. gcc v12 with "-O2" and above will trigger it, but not clang v13 with any "-On". The GitHub CI would also run into this leak if the "linux-leaks" job was made to run with "GIT_TEST_SANITIZE_LEAK_LOG=true". See [1] for a past case where gcc had similar trouble analyzing leaks involving a die() invocation in the function. 1. https://lore.kernel.org/git/patch-v3-5.6-9a44204c4c9-20211022T175227Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27leak tests: mark passing SANITIZE=leak tests as leak-freeÆvar Arnfjörð Bjarmason37-1/+38
Mark those remaining tests that pass when run under SANITIZE=leak with TEST_PASSES_SANITIZE_LEAK=true, these were either omitted in f346fcb62a0 (Merge branch 'ab/mark-leak-free-tests-even-more', 2021-12-15) and 5a4f8381b68 (Merge branch 'ab/mark-leak-free-tests', 2021-10-25), or have had their memory leaks fixed since then. With this change there's now a a one-to-one mapping between those tests that we have opted-in via "TEST_PASSES_SANITIZE_LEAK=true", and those that pass with the new "check" mode: GIT_TEST_PASSING_SANITIZE_LEAK=check \ GIT_TEST_SANITIZE_LEAK_LOG=true \ make test SANITIZE=leak Note that the "GIT_TEST_SANITIZE_LEAK_LOG=true" is needed due to the edge cases noted in a preceding commit, i.e. in some cases we'd pass the test itself, but still have outstanding leaks due to ignored exit codes. The "GIT_TEST_SANITIZE_LEAK_LOG=true" corrects for that, we're only marking those tests as passing that really don't have any leaks, whether that was reflected in their exit code or not. Note that the change here to "t9100-git-svn-basic.sh" is marking that test as passing under SANITIZE=leak, we're removing a "TEST_FAILS_SANITIZE_LEAK=true" line, not "TEST_PASSES_SANITIZE_LEAK=true". See 7a98d9ab00d (revisions API: have release_revisions() release "cmdline", 2022-04-13) for the introduction of that t/lib-git-svn.sh-specific variable. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27leak tests: don't skip some tests under SANITIZE=leakÆvar Arnfjörð Bjarmason7-10/+10
The '!SANITIZE_LEAK' prerequisite added in 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI, 2021-09-23) has been used in various tests to skip individual tests in otherwise leak-free tests. Let's change the cases that have become leak-free since then to run under SANITIZE=leak. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27test-lib: have the "check" mode for SANITIZE=leak consider leak logsÆvar Arnfjörð Bjarmason4-2/+95
As noted in previous on-list discussions[1] we have various tests that will falsely report being leak-free because we're missing the relevant exit code from LSAN as summarized below. We should fix those issues, but in the meantime and as an additional sanity check we can and should consider our own ASAN logs before reporting that a test is leak-free. Before this compiling with SANITIZE=leak and running: ./t6407-merge-binary.sh Will exit successfully, now we'll get an error and an informative message on: GIT_TEST_SANITIZE_LEAK_LOG=true ./t6407-merge-binary.sh Even better, as noted in the updated t/README we'll now error out when combined with the "check" mode: GIT_TEST_PASSING_SANITIZE_LEAK=check \ GIT_TEST_SANITIZE_LEAK_LOG=true \ ./t4058-diff-duplicates.sh Why do we miss these leaks? Because: * We have leaks inside "test_expect_failure" blocks, which by design will not distinguish a "normal" failure from an abort() or segfault. See [1] for a discussion of it shortcomings. * We have "git" invocations outside of "test_expect_success", e.g. setup code in the main body of the test, or in test helper functions that don't use &&-chaining. * Our tests will otherwise catch segfaults and abort(), but if we invoke a command that invokes another command it needs to ferry the exit code up to us. Notably a command that e.g. might invoke "git pack-objects" might itself exit with status 128 if that "pack-objects" segfaults or abort()'s. If the test invoking the parent command(s) is using "test_must_fail" we'll consider it an expected "ok" failure. * run-command.c doesn't (but probably should) ferry up such exit codes, so for e.g. "git push" tests where we expect a failure and an underlying "git" command fails we won't ferry up the segfault or abort exit code. * We have gitweb.perl and some other perl code ignoring return values from close(), i.e. ignoring exit codes from "git rev-parse" et al. * We have in-tree shellscripts like "git-merge-one-file.sh" invoking git commands, they'll usually return their own exit codes on "git" failure, rather then ferrying up segfault or abort() exit code. E.g. these invocations in git-merge-one-file.sh leak, but aren't reflected in the "git merge" exit code: src1=$(git unpack-file $2) src2=$(git unpack-file $3) That case would be easily "fixed" by adding a line like this after each assignment: test $? -ne 0 && exit $? But we'd then in e.g. "t6407-merge-binary.sh" run into write_tree_trivial() in "builtin/merge.c" calling die() instead of ferrying up the relevant exit code. Let's remove "TEST_PASSES_SANITIZE_LEAK=true" from tests we were falsely marking as leak-free. In the case of t6407-merge-binary.sh it was marked as leak-free in 9081a421a6d (checkout: fix "branch info" memory leaks, 2021-11-16). I'd previously removed other bad "TEST_PASSES_SANITIZE_LEAK=true" opt-ins in the series merged in ea05fd5fbf7 (Merge branch 'ab/keep-git-exit-codes-in-tests', 2022-03-16). The case of t1060-object-corruption.sh is more subtle, and will be discussed in a subsequent commit. 1. https://lore.kernel.org/git/cover-0.7-00000000000-20220318T002951Z-avarab@gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27test-lib: add a GIT_TEST_PASSING_SANITIZE_LEAK=check modeÆvar Arnfjörð Bjarmason2-8/+43
Add a new "GIT_TEST_PASSING_SANITIZE_LEAK=check" mode to the test-lib.sh. As noted in the updated "t/README" this compliments the existing "GIT_TEST_PASSING_SANITIZE_LEAK=true" mode added in 956d2e4639b (tests: add a test mode for SANITIZE=leak, run it in CI, 2021-09-23). Rather than document this all in one (even more) dense paragraph split up the discussion of how it combines with --immediate into its own paragraph following the discussion of "GIT_TEST_SANITIZE_LEAK_LOG=true". Before the removal of "test_external" in a preceding commit we would have had to special-case t9700-perl-git.sh and t0202-gettext-perl.sh. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27test-lib: simplify by removing test_externalÆvar Arnfjörð Bjarmason6-154/+28
Remove the "test_external" function added in [1]. This arguably makes the output of t9700-perl-git.sh and friends worse. But as we'll argue below the trade-off is worth it, since "chaining" to another TAP emitter in test-lib.sh is more trouble than it's worth. The new output of t9700-perl-git.sh is now: $ ./t9700-perl-git.sh ok 1 - set up test repository ok 2 - use t9700/test.pl to test Git.pm # passed all 2 test(s) 1..2 Whereas before this change it would be: $ ./t9700-perl-git.sh ok 1 - set up test repository # run 1: Perl API (perl /home/avar/g/git/t/t9700/test.pl) ok 2 - use Git; [... omitting tests 3..46 from t/t9700/test.pl ...] ok 47 - unquote escape sequences 1..47 # test_external test Perl API was ok # test_external_without_stderr test no stderr: Perl API was ok At the time of its addition supporting "test_external" was easy, but when test-lib.sh itself started to emit TAP in [2] we needed to make everything surrounding the emission of the plan consider "test_external". I added that support in [2] so that we could run: prove ./t9700-perl-git.sh :: -v But since then in [3] the door has been closed on combining $HARNESS_ACTIVE and -v, we'll now just die: $ prove ./t9700-perl-git.sh :: -v Bailout called. Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log FAILED--Further testing stopped: verbose mode forbidden under TAP harness; try --verbose-log So the only use of this has been that *if* we had failure in one of these tests we could e.g. in CI see which test failed based on the test number. Now we'll need to look at the full verbose logs to get that same information. I think this trade-off is acceptable given the reduction in complexity, and it brings these tests in line with other similar tests, e.g. the reftable tests added in [4] will be condensed down to just one test, which invokes the C helper: $ ./t0032-reftable-unittest.sh ok 1 - unittests # passed all 1 test(s) 1..1 It would still be nice to have that ":: -v" form work again, it never *really* worked, but even though we've had edge cases test output screwing up the TAP it mostly worked between d998bd4ab67 and [3], so we may have been overzealous in forbidding it outright. I have local patches which I'm planning to submit sooner than later that get us to that goal, and in a way that isn't buggy. In the meantime getting rid of this special case makes hacking on this area of test-lib.sh easier, as we'll do in subsequent commits. The switch from "perl" to "$PERL_PATH" here is because "perl" is defined as a shell function in the test suite, see a5bf824f3b4 (t: prevent '-x' tracing from interfering with test helpers' stderr, 2018-02-25). On e.g. the OSX CI the "command perl"... will be part of the emitted stderr. 1. fb32c410087 (t/test-lib.sh: add test_external and test_external_without_stderr, 2008-06-19) 2. d998bd4ab67 (test-lib: Make the test_external_* functions TAP-aware, 2010-06-24) 3. 614fe015212 (test-lib: bail out when "-v" used under "prove", 2016-10-22) 4. ef8a6c62687 (reftable: utility functions, 2021-10-07) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-27tests: move copy/pasted PERL + Test::More checks to a lib-perl.shÆvar Arnfjörð Bjarmason4-28/+25
Since the original "perl -MTest::More" prerequisite check was added in [1] it's been copy/pasted in [2], [3] and [4]. As we'll be changing these codepaths in a subsequent commit let's consolidate these. While we're at it let's move these to a lazy prereq, and make them conform to our usual coding style (e.g. "\nthen", not "; then"). 1. e46f9c8161a (t9700: skip when Test::More is not available, 2008-06-29) 2. 5e9637c6297 (i18n: add infrastructure for translating Git with gettext, 2011-11-18) 3. 8d314d7afec (send-email: reduce dependencies impact on parse_address_line, 2015-07-07) 4. f07eeed123b (git-credential-netrc: adapt to test framework for git, 2018-05-12) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>