summaryrefslogtreecommitdiff
path: root/t/t3305-notes-fanout.sh
AgeCommit message (Collapse)AuthorFilesLines
2020-02-04notes.c: fix off-by-one error when decreasing notes fanoutLibravatar Johan Herland1-0/+6
As noted in the previous commit, the nature of the fanout heuristic in the notes code causes the exact point at which we increase or decrease the notes fanout to vary with the objects being annotated. Since the object ids generated by the test environment are deterministic (by design), the notes generated and tested by t3305 are always the same, and we therefore happen to see the same fanout behavior from one run to the next. Coincidentally, if we were to change the test environment slightly (say by making a test commit on an unrelated branch before we start the t3305 test proper), we not only see the fanout switch happen at different points, we also manage to trigger a _bug_ in the notes code where the fanout 1 -> 0 switch is not applied uniformly across the notes tree, but instead yields a notes tree like this: ... bdeafb301e44b0e4db0f738a2d2a7beefdb70b70 bff2d39b4f7122bd4c5caee3de353a774d1e632a d3/8ec8f851adf470131178085bfbaab4b12ad2a7 e0b173960431a3e692ae929736df3c9b73a11d5b eb3c3aede523d729990ac25c62a93eb47c21e2e3 ... The bug occurs when we are writing out a notes tree with a newly decreased fanout, and the notes tree contains unexpanded subtrees that should be consolidated into the parent tree as a consequence of the decreased fanout): Subtrees that happen to sit at an _even_ level in the internal notes 16-tree structure (in other words: subtrees whose path - "d3" in the example above - is unique in the first nibble - i.e. there are no other note paths that start with "d") are _not_ unpacked as part of the tree writeout. This error will repeat itself in subsequent note trees until the subtree is forced to be unpacked. In t3305 this only happens when the d38ec8f8 note is itself removed from the tree. The error is not severe (no information is lost, and the notes code is able to read/decode this tree and manipulate it correctly), but this is nonetheless a bug in the current implementation that should be fixed. That said, fixing the off-by-one error is not without complications: We must take into account that the load_subtree() call from for_each_note_helper() (that is now done to correctly unpack the subtree while we're writing out the notes tree) may end up inserting unpacked non-notes into the linked list of non_note entries held by the struct notes_tree. Since we are in the process of writing out the notes tree, this linked list is currently in the process of being traversed by write_each_non_note_until(). The unpacked non-notes are necessarily inserted between the last non-note we wrote out, and the next non-note to be written. Hence, we cannot simply hold the next_non_note to write in struct write_each_note_data (as we would then silently skip these newly inserted notes), but must instead always follow the ->next pointer from the last non-note we wrote. (This part was caught by an existing test in t3304.) Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: Brian M. Carlson <sandals@crustytoothpaste.net> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-04t3305: check notes fanout more carefully and robustlyLibravatar Johan Herland1-25/+76
In short, before this patch, this test script: - creates many notes - verifies that all notes in the notes tree has a fanout of 1 - removes most notes - verifies that the notes in the notes tree now has a fanout of 0 The fanout verification only happened twice: after creating all the notes, and after removing most of them. This patch strengthens the test by checking the fanout after _each_ added/removed note: We assert that the switch from fanout 0 -> 1 happens exactly once while adding notes (and that the switch pervades the entire notes tree). Likewise, we assert that the switch from fanout 1 -> 0 happens exactly once while removing notes. Additionally, we decrease the number of notes left after removal, from 50 to 15 notes, in order to ensure that fanout 1 -> 0 transition keeps happening regardless of external factors[1]. [1]: Currently (with the SHA1 hash function and the deterministic object ids of the test environment) the fanout heuristic in the notes code happens to switch from 0 -> 1 at 109 notes, and from 1 -> 0 at 59 notes. However, changing the hash function or other external factors will vary these numbers, and the latter may - in theory - go as low as 15. For more details, please see the discussion at https://public-inbox.org/git/20200125230035.136348-4-sandals@crustytoothpaste.net/ Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de> Cc: Brian M. Carlson <sandals@crustytoothpaste.net> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-20t3305: make hash size independentLibravatar brian m. carlson1-16/+6
Instead of hard-coding 40-character shell patterns, use grep to determine if all of the paths have either zero or one levels of fanout, as appropriate. Note that the final test is implicitly dependent on the hash algorithm. Depending on the algorithm in use, the fanout may or may not completely compress. In its current state, this is not a problem, but it could be if the hash algorithm changes again. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-03-25t3305: fix ignored exit code inside loopLibravatar Jeff King1-6/+3
When we test deleting notes, we run "git notes remove" in a loop. However, the exit value of the loop will only reflect the final note we process. We should break out of the loop with a failing exit code as soon as we see a problem. Note that we can call "exit 1" here without explicitly creating a subshell, because the while loop on the right-hand side of a pipe executes in its own implicit subshell. Note also that the "break" above does not suffer the same problem; it is meant to exit the loop early at a certain number of iterations. We can bump it into the conditional of the loop to make this more obvious. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-13builtin-notes: Deprecate the -m/-F options for "git notes edit"Libravatar Johan Herland1-1/+1
The semantics for "git notes edit -m/-F" overlap with those for "git notes add -f", and the behaviour (i.e. overwriting existing notes with the given message/file) is more intuitively captured by (and better documented with) "git notes add -f". Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-13t3305: Verify that removing notes triggers automatic fanout consolidationLibravatar Johan Herland1-1/+46
Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-02-13t3305: Verify that adding many notes with git-notes triggers increased fanoutLibravatar Johan Herland1-0/+50
Add a test verifying that the notes code automatically restructures the notes tree into a deeper fanout level, when many notes are added with "git notes". Signed-off-by: Johan Herland <johan@herland.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>