kernel/git - git - PowerEL Git System

Commit Graph

Author	SHA1	Message	Date
Jeff King	2ba582ba4c	prune: save reachable-from-recent objects with bitmaps We pass our prune expiration to mark_reachable_objects(), which will traverse not only the reachable objects, but consider any recent ones as tips for reachability; see `d3038d22f9` (prune: keep objects reachable from recent objects, 2014-10-15) for details. However, this interacts badly with the bitmap code path added in `fde67d6896` (prune: use bitmaps for reachability traversal, 2019-02-13). If we hit the bitmap-optimized path, we return immediately to avoid the regular traversal, accidentally skipping the "also traverse recent" code. Instead, we should do an if-else for the bitmap versus regular traversal, and then follow up with the "recent" traversal in either case. This reuses the "rev_info" for a bitmap and then a regular traversal, but that should work OK (the bitmap code clears the pending array in the usual way, just like a regular traversal would). Note that I dropped the comment above the regular traversal here. It has little explanatory value, and makes the if-else logic much harder to read. Here are a few variants that I rejected: - it seems like both the reachability and recent traversals could be done in a single traversal. This was rejected by `d3038d22f9` (prune: keep objects reachable from recent objects, 2014-10-15), though the balance may be different when using bitmaps. However, there's a subtle correctness issue, too: we use revs->ignore_missing_links for the recent traversal, but not the reachability one. - we could try using bitmaps for the recent traversal, too, which could possibly improve performance. But it would require some fixes in the bitmap code, which uses ignore_missing_links for its own purposes. Plus it would probably not help all that much in practice. We use the reachable tips to generate bitmaps, so those objects are likely not covered by bitmaps (unless they just became unreachable). And in general, we expect the set of unreachable objects to be much smaller anyway, so there's less to gain. The test in t5304 detects the bug and confirms the fix. I also beefed up the tests in t6501, which covers the mtime-checking code more thoroughly, to handle the bitmap case (in addition to just "loose" and "packed" cases). Interestingly, this test doesn't actually detect the bug, because it is running "git gc", and not "prune" directly. And "gc" will call "repack" first, which does not suffer the same bug. So the old-but-reachable-from-recent objects get scooped up into the new pack along with the actually-recent objects, which gives both a recent mtime. But it seemed prudent to get more coverage of the bitmap case for related code. Reported-by: David Emett <dave@sp4m.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Jeff King	1e951c6473	pack-bitmap: clean up include_check after use When a bitmap walk has to traverse (to fill in non-bitmapped objects), we use rev_info's include_check mechanism to let us stop the traversal early. But after setting the function and its data parameter, we never clean it up. This means that if the rev_info is used for a subsequent traversal without bitmaps, it will unexpectedly call into our include_check function (worse, it will do so pointing to a now-defunct stack variable in include_check_data, likely resulting in a segfault). There's no code which does this now, but it's an accident waiting to happen. Let's clean up after ourselves in the bitmap code. Reported-by: David Emett <dave@sp4m.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	9a3e3ca2ba	subtree: be stricter about validating flags Don't silently ignore a flag that's invalid for a given subcommand. The user expected it to do something; we should tell the user that they are mistaken, instead of surprising the user. It could be argued that this change might break existing users. I'd argue that those existing users are already broken, and they just don't know it. Let them know that they're broken. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	49470cd445	subtree: push: allow specifying a local rev other than HEAD 'git subtree split' lets you specify a rev other than HEAD. 'git push' lets you specify a mapping between a local thing and a remot ref. So smash those together, and have 'git subtree push' let you specify which local thing to run split on and push the result of that split to the remote ref. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	94389e7c81	subtree: allow 'split' flags to be passed to 'push' 'push' does a 'split' internally, but it doesn't pass flags through to the 'split'. This is silly, if you need to pass flags to 'split', then it means that you can't use 'push'! So, have 'push' accept 'split' flags, and pass them through to 'split'. Add tests for this by copying split's tests with minimal modification. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	cb6551447b	subtree: allow --squash to be used with --rejoin Besides being a genuinely useful thing to do, this also just makes sense and harmonizes which flags may be used when. `git subtree split --rejoin` amounts to "automatically go ahead and do a `git subtree merge` after doing the main `git subtree split`", so it's weird and arbitrary that you can't pass `--squash` to `git subtree split --rejoin` like you can `git subtree merge`. It's weird that `git subtree split --rejoin` inherits `git subtree merge`'s `--message` but not `--squash`. Reconcile the situation by just having `split --rejoin` actually just call `merge` internally (or call `add` instead, as appropriate), so it can get access to the full `merge` behavior, including `--squash`. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	6468784dd2	subtree: give the docs a once-over Just went through the docs looking for anything inaccurate or that can be improved. In the '-h' text, in the man page synopsis, and in the man page description: Normalize the ordering of the list of sub-commands: 'add', 'merge', 'split', 'pull', 'push'. This allows us to kinda separate the lower-level add/merge/split from the higher-level pull/push. '-h' text: - correction: Indicate that split's arg is optional. - clarity: Emphasize that 'pull' takes the 'add'/'merge' flags. man page: - correction: State that all subcommands take options (it seemed to indicate that only 'split' takes any options other than '-P'). - correction: 'split' only guarantees that the results are identical if the flags are identical. - correction: The flag is named '--ignore-joins', not '--ignore-join'. - completeness: Clarify that 'push' always operates on HEAD, and that 'split' operates on HEAD if no local commit is given. - clarity: In the description, when listing commands, repeat what their arguments are. This way the reader doesn't need to flip back and forth between the command description and the synopsis and the full description to understand what's being said. - clarity: In the <variables> used to give command arguments, give slightly longer, descriptive names. Like <local-commit> instead of just <commit>. - clarity: Emphasize that 'pull' takes the 'add'/'merge' flags. - style: In the synopsis, list options before the subcommand. This makes things line up and be much more readable when shown non-monospace (such as in `make html`), and also more closely matches other man pages (like `git-submodule.txt`). - style: Use the correct syntax for indicating the options ([<options>] instead of [OPTIONS]). - style: In the synopsis, separate 'pull' and 'push' from the other lower-level commands. I think this helps readability. - style: Code-quote things in prose that seem like they should be code-quoted, like '.gitmodules', flags, or full commands. - style: Minor wording improvements, like more consistent mood (many of the command descriptions start in the imperative mood and switch to the indicative mode by the end). That sort of thing. - style: Capitalize "ID". - style: Remove the "This option is only valid for XXX command" remarks from each option, and instead rely on the section headings. - style: Since that line is getting edited anyway, switch "behaviour" to American "behavior". - style: Trim trailing whitespace. `todo`: - style: Trim trailing whitespace. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	e9525a8a02	subtree: have $indent actually affect indentation Currently, the $indent variable is just used to track how deeply we're nested, and the debug log is indented by things like debug " foo" That is: The indentation-level is hard-coded. It used to be that the code couldn't recurse, so the indentation level could be known statically, so it made sense to just hard-code it in the output. However, since `315a84f9aa` ("subtree: use commits before rejoins for splits", 2018-09-28), it can now recurse, and the debug log is misleading. So fix that. Indent according to $indent. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	534ff90dbd	subtree: don't let debug and progress output clash Currently, debug output (triggered by passing '-d') and progress output stomp on each other. The debug output is just streamed as lines to stderr, and the progress output is sent to stderr as '%s\r'. When writing to a file, it is awkward to read and difficult to distinguish between the debug output and a progress line. When writing to a terminal the debug lines hide progress lines. So, when '-d' has been passed, spit out progress as 'progress: %s\n', instead of as '%s\r', so that it can be detected, and so that the debug lines don't overwrite the progress when written to a terminal. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	5cdae0f6fd	subtree: add comments and sanity checks For each function in subtree, add a usage comment saying what the arguments are, and add an `assert` checking the number of arguments. In figuring out each thing's arguments in order to write those comments and assertions, it turns out that find_existing_splits is written as if it takes multiple 'revs', but it is in fact only ever passed a single 'rev': unrevs="$(find_existing_splits "$dir" "$rev")" \|\| exit $? So go ahead and codify that by documenting and asserting that it takes exactly two arguments, one dir and one rev. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	cbb5de8b83	subtree: remove duplicate check `cmd_add` starts with a check that the directory doesn't yet exist. However, the `main` function performs the exact same check before calling `cmd_add`. So remove the check from `cmd_add`. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	e4f8baa88a	subtree: parse revs in individual cmd_ functions The main argument parser goes ahead and tries to parse revs to make things simpler for the sub-command implementations. But, it includes enough special cases for different sub-commands. And it's difficult having having to think about "is this info coming from an argument, or a global variable?". So the main argument parser's effort to make things "simpler" ends up just making it more confusing and complicated. Begone with the 'revs' global variable; parse 'rev=$(...)' as needed in individual 'cmd_*' functions. Begone with the 'default' global variable. Its would-be value is knowable just from which function we're in. Begone with the 'ensure_single_rev' function. Its functionality can be achieved by passing '--verify' to 'git rev-parse'. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	bbffb02383	subtree: use "^{commit}" instead of "^0" They are synonyms. Both are used in the file. ^{commit} is clearer, so "standardize" on that. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	22d5507493	subtree: don't fuss with PATH Scripts needing to fuss with with adding $(git --exec-prefix) PATH before loading git-sh-setup is a thing of the past. As far as I can tell, it's been a thing of the past since since Git v1.2.0 (2006-02-12), or more specifically, since `77cb17e940` (Exec git programs without using PATH, 2006-01-10). However, it stuck around in contrib scripts and in third-party scripts for long enough that it wasn't unusual to see. Originally `git subtree` didn't fuss with PATH, but when people (including the original subtree author) had problems, because it was a common thing to see, it seemed that having subtree fuss with PATH was a reasonable solution. Here is an abridged history of fussing with PATH in subtree: `2987e6add3` (Add explicit path of git installation by 'git --exec-path', Gianluca Pacchiella, 2009-08-20) As pointed out by documentation, the correct use of 'git-sh-setup' is using $(git --exec-path) to avoid problems with not standard installations. -. git-sh-setup +. $(git --exec-path)/git-sh-setup `33aaa697a2` (Improve patch to use git --exec-path: add to PATH instead, Avery Pennarun, 2009-08-26) If you (like me) are using a modified git straight out of its source directory (ie. without installing), then --exec-path isn't actually correct. Add it to the PATH instead, so if it is correct, it'll work, but if it's not, we fall back to the previous behaviour. -. $(git --exec-path)/git-sh-setup +PATH=$(git --exec-path):$PATH +. git-sh-setup `9c632ea29c` ((Hopefully) fix PATH setting for msysgit, Avery Pennarun, 2010-06-24) Reported by Evan Shaw. The problem is that $(git --exec-path) includes a 'git' binary which is incompatible with the one in /usr/bin; if you run it, it gives you an error about libiconv2.dll. +OPATH=$PATH PATH=$(git --exec-path):$PATH . git-sh-setup +PATH=$OPATH # apparently needed for some versions of msysgit `df2302d774` (Another fix for PATH and msysgit, Avery Pennarun, 2010-06-24) Evan Shaw tells me the previous fix didn't work. Let's use this one instead, which he says does work. This fix is kind of wrong because it will run the "correct" git-sh-setup after the one in /usr/bin, if there is one, which could be weird if you have multiple versions of git installed. But it works on my Linux and his msysgit, so it's obviously better than what we had before. -OPATH=$PATH -PATH=$(git --exec-path):$PATH +PATH=$PATH:$(git --exec-path) . git-sh-setup -PATH=$OPATH # apparently needed for some versions of msysgit First of all, I disagree with Gianluca's reading of the documentation: - I haven't gone back to read what the documentation said in 2009, but in my reading of the 2021 documentation is that it includes "$(git --exec-path)/" in the synopsis for illustrative purposes, not to say it's the proper way. - After being executed by `git`, the git exec path should be the very first entry in PATH, so it shouldn't matter. - None of the scripts that are part of git do it that way. But secondly, the root reason for fussing with PATH seems to be that Avery didn't know that he needs to set GIT_EXEC_PATH if he's going to use git from the source directory without installing. And finally, Evan's issue is clearly just a bug in msysgit. I assume that msysgit has since fixed the issue, and also msysgit has been deprecated for 6 years now, so let's drop the workaround for it. So, remove the line fussing with PATH. However, since subtree is in 'contrib/' and it might get installed in funny ways by users after-the-fact, add a sanity check to the top of the script, checking that it is installed correctly. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	a94f911072	subtree: use "$" instead of "$@" as appropriate "$" is for when you want to concatenate the args together, whitespace-separated; and "$@" is for when you want them to be separate strings. There are several places in subtree that erroneously use $@ when concatenating args together into an error message. For instance, if the args are argv[1]="dead" and argv[2]="beef", then the line die "You must provide exactly one revision. Got: '$@'" surely intends to call 'die' with the argument argv[1]="You must provide exactly one revision. Got: 'dead beef'" however, because the line used $@ instead of $, it will actually call 'die' with the arguments argv[1]="You must provide exactly one revision. Got: 'dead" argv[2]="beef'" This isn't a big deal, because 'die' concatenates its arguments together anyway (using "$"). But that doesn't change the fact that it was a mistake to use $@ instead of $*, even though in the end $@ still ended up doing the right thing. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	e2b11e4211	subtree: use more explicit variable names for cmdline args Make it painfully obvious when reading the code which variables are direct parsings of command line arguments. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	6d43585a68	subtree: use git-sh-setup's `say` subtree currently defines its own `say` implementation, rather than using git-sh-setups's implementation. Change that, don't re-invent the wheel. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	f664304836	subtree: use `git merge-base --is-ancestor` Instead of writing a slow `rev_is_descendant_of_branch $a $b` function in shell, just use the fast `git merge-base --is-ancestor $b $a`. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	8dc3240f5f	subtree: drop support for git < 1.7 Suport for Git versions older than 1.7.0 (older than February 2010) was nice to have when git-subtree lived out-of-tree. But now that it lives in git.git, it's not necessary to keep around. While it's technically in contrib, with the standard 'git' packages for common systems (including Arch Linux and macOS) including git-subtree, it seems vanishingly likely to me that people are separately installing git-subtree from git.git alongside an older 'git' install (although it also seems vanishingly likely that people are still using >11 year old git installs). Not that there's much reason to remove it either, it's not much code, and none of my changes depend on a newer git (to my knowledge, anyway; I'm not actually testing against older git). I just figure it's an easy piece of fat to trim, in the journey to making the whole thing easier to hack on. "Ignore space change" is probably helpful when viewing this diff. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	d2f0f81954	subtree: more consistent error propagation Ensure that every $(subshell) that calls a function (as opposed to an external executable) is followed by `\|\| exit $?`. Similarly, ensure that every `cmd \| while read; do ... done` loop is followed by `\|\| exit $?`. Both of those constructs mean that it can miss `die` calls, and keep running when it shouldn't. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	5a3569774f	subtree: don't have loose code outside of a function Shove all of the loose code inside of a main() function. This comes down to personal preference more than anything else. A preference that I've developed over years of maintaining large Bash scripts, but still a mere personal preference. In this specific case, it's also moving the `set -- -h`, the `git rev-parse --parseopt`, and the `. git-sh-setup` to be closer to all the rest of the argument parsing, which is a readability win on its own, IMO. "Ignore space change" is probably helpful when viewing this diff. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	b04538d99f	subtree: t7900: add porcelain tests for 'pull' and 'push' The 'pull' and 'push' subcommands deserve their own sections in the tests. Add some basic tests for them. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	b269976979	subtree: t7900: add a test for the -h flag It's a dumb test, but it's surprisingly easy to break. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	db6952b2b2	subtree: t7900: rename last_commit_message to last_commit_subject t7900-subtree.sh defines a helper function named last_commit_message. However, it only returns the subject line of the commit message, not the entire commit message. So rename it, to make the name less confusing. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	f1cd2d93c2	subtree: t7900: fix 'verify one file change per commit' As far as I can tell, this test isn't actually testing anything, because someone forgot to tack on `--name-only` to `git log`. This seems to have been the case since the test was first written, back in `fa16ab36ad` ("test.sh: make sure no commit changes more than one file at a time.", 2009-04-26), unless `git log` used to do that by default and didn't need the flag back then? Convincing myself that it's not actually testing anything was tricky, the code is a little hard to reason about. It can be made a lot simpler if instead of trying to parse all of the info from a single `git log`, we're OK calling `git log` from inside of a loop. And it's my opinion that tests are not the place for clever optimized code. So, fix and simplify the test, so that it's actually testing something and is simpler to reason about. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	63ac4f1ade	subtree: t7900: delete some dead code Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	c4566ab429	subtree: t7900: use 'test' for string equality t7900-subtree.sh defines its own `check_equal A B` function, instead of just using `test A = B` like all of the other tests. Don't be special, get rid of `check_equal` in favor of `test`. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	40b1e1ec58	subtree: t7900: comment subtree_test_create_repo It's unclear what the purpose of t7900-subtree.sh's `subtree_test_create_repo` helper function is. It wraps test-lib.sh's, `test_create_repo` but follows that up by setting log.date=relative. Why does it set log.date=relative? My first guess was that at one point the tests required that, but no longer do, and that the function is now vestigial. I even wrote a patch to get rid of it and was moments away from `git send-email`ing it. However, by chance when looking for something else in the history, I discovered the true reason, from `e7aac44ed2` (contrib/subtree: ignore log.date configuration, 2015-07-21). It's testing that setting log.date=relative doesn't break `git subtree`, as at one point in the past that did break `git subtree`. So, add a comment about this, to avoid future such confusion. And while at it, go ahead and (1) touch up the function to avoid a pointless subshell and (2) update the one test that didn't use it. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	f700406957	subtree: t7900: use consistent formatting The formatting in t7900-subtree.sh isn't even consistent throughout the file. Fix that; make it consistent throughout the file. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	f2bb7fef7a	subtree: t7900: use test-lib.sh's test_count Use test-lib.sh's `test_count`, instead instead of having t7900-subtree.sh do its own book-keeping with `subtree_test_count` that has to be explicitly incremented by calling `next_test`. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	914d512551	subtree: t7900: update for having the default branch name be 'main' Most of the tests had been converted to support `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main`, but `contrib/subtree/t/` hadn't. Convert it. Most of the mentions of 'master' can just be replaced with 'HEAD'. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Luke Shumaker	4c996deb4a	.gitignore: ignore 'git-subtree' as a build artifact Running `make -C contrib/subtree/ test` creates a `git-subtree` executable in the root of the repo. Add it to the .gitignore so that anyone hacking on subtree won't have to deal with that noise. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Rafael Silva	a643157d5a	repack: avoid loosening promisor objects in partial clones When `git repack -A -d` is run in a partial clone, `pack-objects` is invoked twice: once to repack all promisor objects, and once to repack all non-promisor objects. The latter `pack-objects` invocation is with --exclude-promisor-objects and --unpack-unreachable, which loosens all objects unused during this invocation. Unfortunately, this includes promisor objects. Because the -d argument to `git repack` subsequently deletes all loose objects also in packs, these just-loosened promisor objects will be immediately deleted. However, this extra disk churn is unnecessary in the first place. For example, in a newly-cloned partial repo that filters all blob objects (e.g. `--filter=blob:none`), `repack` ends up unpacking all trees and commits into the filesystem because every object, in this particular case, is a promisor object. Depending on the repo size, this increases the disk usage considerably: In my copy of the linux.git, the object directory peaked 26GB of more disk usage. In order to avoid this extra disk churn, pass the names of the promisor packfiles as --keep-pack arguments to the second invocation of `pack-objects`. This informs `pack-objects` that the promisor objects are already in a safe packfile and, therefore, do not need to be loosened. For testing, we need to validate whether any object was loosened. However, the "evidence" (loosened objects) is deleted during the process which prevents us from inspecting the object directory. Instead, let's teach `pack-objects` to count loosened objects and emit via trace2 thus allowing inspecting the debug events after the process is finished. This new event is used on the added regression test. Lastly, add a new perf test to evaluate the performance impact made by this changes (tested on git.git): Test HEAD^ HEAD ---------------------------------------------------------- 5600.3: gc 134.38(41.93+90.95) 7.80(6.72+1.35) -94.2% For a bigger repository, such as linux.git, the improvement is even bigger: Test HEAD^ HEAD ------------------------------------------------------------------- 5600.3: gc 6833.00(918.07+3162.74) 268.79(227.02+39.18) -96.1% These improvements are particular big because every object in the newly-cloned partial repository is a promisor object. Reported-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Jeff King <peff@peff.net> Helped-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Rafael Silva <rafaeloliveira.cs@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Peter Oliver	7a14acdbe6	doc: point to diff attribute in patch format docs From the documentation for generating patch text with diff-related commands, refer to the documentation for the diff attribute. This attribute influences the way that patches are generated, but this was previously not mentioned in e.g., the git-diff manpage. Signed-off-by: Peter Oliver <git@mavit.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	37be11994f	builtin/rm: avoid leaking pathspec and seen parse_pathspec() populates pathspec, hence we need to clear it once it's no longer needed. seen is xcalloc'd within the same function and likewise needs to be freed once its no longer needed. cmd_rm() has multiple early returns, therefore we need to clear or free as soon as this data is no longer needed, as opposed to doing a cleanup at the end. LSAN output from t0020: Direct leak of 112 byte(s) in 1 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac0a4 in do_xmalloc wrapper.c:41:8 #2 0x9ac07a in xmalloc wrapper.c:62:9 #3 0x873277 in parse_pathspec pathspec.c:582:2 #4 0x646ffa in cmd_rm builtin/rm.c:266:2 #5 0x4cd91d in run_builtin git.c:467:11 #6 0x4cb5f3 in handle_builtin git.c:719:3 #7 0x4ccf47 in run_argv git.c:808:4 #8 0x4caf49 in cmd_main git.c:939:19 #9 0x69dc0e in main common-main.c:52:11 #10 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 65 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9ac2a6 in xrealloc wrapper.c:126:8 #2 0x93b14d in strbuf_grow strbuf.c:98:2 #3 0x93ccf6 in strbuf_vaddf strbuf.c:392:3 #4 0x93f726 in xstrvfmt strbuf.c:979:2 #5 0x93f8b3 in xstrfmt strbuf.c:989:8 #6 0x92ad8a in prefix_path_gently setup.c:115:15 #7 0x873a8d in init_pathspec_item pathspec.c:439:11 #8 0x87334f in parse_pathspec pathspec.c:589:3 #9 0x646ffa in cmd_rm builtin/rm.c:266:2 #10 0x4cd91d in run_builtin git.c:467:11 #11 0x4cb5f3 in handle_builtin git.c:719:3 #12 0x4ccf47 in run_argv git.c:808:4 #13 0x4caf49 in cmd_main git.c:939:19 #14 0x69dc0e in main common-main.c:52:11 #15 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 15 byte(s) in 1 object(s) allocated from: #0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0x9ac048 in xstrdup wrapper.c:29:14 #2 0x873ba2 in init_pathspec_item pathspec.c:468:20 #3 0x87334f in parse_pathspec pathspec.c:589:3 #4 0x646ffa in cmd_rm builtin/rm.c:266:2 #5 0x4cd91d in run_builtin git.c:467:11 #6 0x4cb5f3 in handle_builtin git.c:719:3 #7 0x4ccf47 in run_argv git.c:808:4 #8 0x4caf49 in cmd_main git.c:939:19 #9 0x69dc0e in main common-main.c:52:11 #10 0x7f948825b349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 1 byte(s) in 1 object(s) allocated from: #0 0x49a9d2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3 #1 0x9ac392 in xcalloc wrapper.c:140:8 #2 0x647108 in cmd_rm builtin/rm.c:294:9 #3 0x4cd91d in run_builtin git.c:467:11 #4 0x4cb5f3 in handle_builtin git.c:719:3 #5 0x4ccf47 in run_argv git.c:808:4 #6 0x4caf49 in cmd_main git.c:939:19 #7 0x69dbfe in main common-main.c:52:11 #8 0x7f4fac1b0349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	805b789a69	builtin/rebase: release git_format_patch_opt too options.git_format_patch_opt can be populated during cmd_rebase's setup, and will therefore leak on return. Although we could just UNLEAK all of options, we choose to strbuf_release() the individual member, which matches the existing pattern (where we're freeing invidual members of options). Leak found when running t0021: Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9ac296 in xrealloc wrapper.c:126:8 #2 0x93b13d in strbuf_grow strbuf.c:98:2 #3 0x93bd3a in strbuf_add strbuf.c:295:2 #4 0x60ae92 in strbuf_addstr strbuf.h:304:2 #5 0x605f17 in cmd_rebase builtin/rebase.c:1759:3 #6 0x4cd91d in run_builtin git.c:467:11 #7 0x4cb5f3 in handle_builtin git.c:719:3 #8 0x4ccf47 in run_argv git.c:808:4 #9 0x4caf49 in cmd_main git.c:939:19 #10 0x69dbfe in main common-main.c:52:11 #11 0x7f66dae91349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	a317a553b8	builtin/for-each-ref: free filter and UNLEAK sorting. sorting might be a list allocated in ref_default_sorting() (in this case it's a fixed single item list, which has nevertheless been xcalloc'd), or it might be a list allocated in parse_opt_ref_sorting(). In either case we could free these lists - but instead we UNLEAK as we're at the end of cmd_for_each_ref. (There's no existing implementation of clear_ref_sorting(), and writing a loop to free the list seems more trouble than it's worth.) filter.with_commit/no_commit are populated via OPT_CONTAINS/OPT_NO_CONTAINS, both of which create new entries via parse_opt_commits(), and also need to be free'd or UNLEAK'd. Because free_commit_list() already exists, we choose to use that over an UNLEAK. LSAN output from t0041: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49a9d2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3 #1 0x9ac252 in xcalloc wrapper.c:140:8 #2 0x8a4a55 in ref_default_sorting ref-filter.c:2486:32 #3 0x56c6b1 in cmd_for_each_ref builtin/for-each-ref.c:72:13 #4 0x4cd91d in run_builtin git.c:467:11 #5 0x4cb5f3 in handle_builtin git.c:719:3 #6 0x4ccf47 in run_argv git.c:808:4 #7 0x4caf49 in cmd_main git.c:939:19 #8 0x69dabe in main common-main.c:52:11 #9 0x7f2bdc570349 in __libc_start_main (/lib64/libc.so.6+0x24349) Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9abf54 in do_xmalloc wrapper.c:41:8 #2 0x9abf2a in xmalloc wrapper.c:62:9 #3 0x717486 in commit_list_insert commit.c:540:33 #4 0x8644cf in parse_opt_commits parse-options-cb.c:98:2 #5 0x869bb5 in get_value parse-options.c:181:11 #6 0x8677dc in parse_long_opt parse-options.c:378:10 #7 0x8659bd in parse_options_step parse-options.c:817:11 #8 0x867fcd in parse_options parse-options.c:870:10 #9 0x56c62b in cmd_for_each_ref builtin/for-each-ref.c:59:2 #10 0x4cd91d in run_builtin git.c:467:11 #11 0x4cb5f3 in handle_builtin git.c:719:3 #12 0x4ccf47 in run_argv git.c:808:4 #13 0x4caf49 in cmd_main git.c:939:19 #14 0x69dabe in main common-main.c:52:11 #15 0x7f2bdc570349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	f3a9680791	mailinfo: also free strbuf lists when clearing mailinfo mailinfo.p_hdr_info/s_hdr_info are null-terminated lists of strbuf's, with entries pointing either to NULL or an allocated strbuf. Therefore we need to free those strbuf's (and not just the data they contain) whenever we're done with a given entry. (See handle_header() where those new strbufs are malloc'd.) Once we no longer need the list (and not just its entries) we can switch over to strbuf_list_free() instead of manually iterating over the list, which takes care of those additional details for us. We can only do this in clear_mailinfo() - in handle_commit_message() we are only clearing the array contents but want to reuse the array itself, hence we can't use strbuf_list_free() there. However, strbuf_list_free() cannot handle a NULL input, and the lists we are freeing might be NULL. Therefore we add a NULL check in strbuf_list_free() to make it safe to use with a NULL input (which is a pattern used by some of the other *_free() functions around git). Leak output from t0023: Direct leak of 72 byte(s) in 3 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac9f4 in do_xmalloc wrapper.c:41:8 #2 0x9ac9ca in xmalloc wrapper.c:62:9 #3 0x7f6cf7 in handle_header mailinfo.c:205:10 #4 0x7f5abf in check_header mailinfo.c:583:4 #5 0x7f5524 in mailinfo mailinfo.c:1197:3 #6 0x4dcc95 in parse_mail builtin/am.c:1167:6 #7 0x4d9070 in am_run builtin/am.c:1732:12 #8 0x4d5b7a in cmd_am builtin/am.c:2398:3 #9 0x4cd91d in run_builtin git.c:467:11 #10 0x4cb5f3 in handle_builtin git.c:719:3 #11 0x4ccf47 in run_argv git.c:808:4 #12 0x4caf49 in cmd_main git.c:939:19 #13 0x69e43e in main common-main.c:52:11 #14 0x7fc1fadfa349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 72 byte(s) leaked in 3 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	52a9436aa7	builtin/checkout: clear pending objects after diffing add_pending_object() populates rev.pending, we need to take care of clearing it once we're done. This code is run close to the end of a checkout, therefore this leak seems like it would have very little impact. See also LSAN output from t0020 below: Direct leak of 2048 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc46 in xrealloc wrapper.c:126:8 #2 0x83e3a3 in add_object_array_with_path object.c:337:3 #3 0x8f672a in add_pending_object_with_path revision.c:329:2 #4 0x8eaeab in add_pending_object_with_mode revision.c:336:2 #5 0x8eae9d in add_pending_object revision.c:342:2 #6 0x5154a0 in show_local_changes builtin/checkout.c:602:2 #7 0x513b00 in merge_working_tree builtin/checkout.c:979:3 #8 0x512cb3 in switch_branches builtin/checkout.c:1242:9 #9 0x50f8de in checkout_branch builtin/checkout.c:1646:9 #10 0x50ba12 in checkout_main builtin/checkout.c:2003:9 #11 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 #12 0x4cd91d in run_builtin git.c:467:11 #13 0x4cb5f3 in handle_builtin git.c:719:3 #14 0x4ccf47 in run_argv git.c:808:4 #15 0x4caf49 in cmd_main git.c:939:19 #16 0x69e43e in main common-main.c:52:11 #17 0x7f5dd1d50349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 2048 byte(s) leaked in 1 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	265644367f	builtin/check-ignore: clear_pathspec before returning parse_pathspec() allocates new memory into pathspec, therefore we need to free it when we're done. An UNLEAK would probably be just as good here - but clear_pathspec() is not much more work so we might as well use it. check_ignore() is either called once directly from cmd_check_ignore() (in which case the leak really doesnt matter), or it can be called multiple times in a loop from check_ignore_stdin_paths(), in which case we're potentially leaking multiple times - but even in this scenario the leak is so small as to have no real consequence. Found while running t0008: Direct leak of 112 byte(s) in 1 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9aca44 in do_xmalloc wrapper.c:41:8 #2 0x9aca1a in xmalloc wrapper.c:62:9 #3 0x873c17 in parse_pathspec pathspec.c:582:2 #4 0x503eb8 in check_ignore builtin/check-ignore.c:90:2 #5 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17 #6 0x4cd91d in run_builtin git.c:467:11 #7 0x4cb5f3 in handle_builtin git.c:719:3 #8 0x4ccf47 in run_argv git.c:808:4 #9 0x4caf49 in cmd_main git.c:939:19 #10 0x69e43e in main common-main.c:52:11 #11 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 65 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc46 in xrealloc wrapper.c:126:8 #2 0x93baed in strbuf_grow strbuf.c:98:2 #3 0x93d696 in strbuf_vaddf strbuf.c:392:3 #4 0x9400c6 in xstrvfmt strbuf.c:979:2 #5 0x940253 in xstrfmt strbuf.c:989:8 #6 0x92b72a in prefix_path_gently setup.c:115:15 #7 0x87442d in init_pathspec_item pathspec.c:439:11 #8 0x873cef in parse_pathspec pathspec.c:589:3 #9 0x503eb8 in check_ignore builtin/check-ignore.c:90:2 #10 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17 #11 0x4cd91d in run_builtin git.c:467:11 #12 0x4cb5f3 in handle_builtin git.c:719:3 #13 0x4ccf47 in run_argv git.c:808:4 #14 0x4caf49 in cmd_main git.c:939:19 #15 0x69e43e in main common-main.c:52:11 #16 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 2 byte(s) in 1 object(s) allocated from: #0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0x9ac9e8 in xstrdup wrapper.c:29:14 #2 0x874542 in init_pathspec_item pathspec.c:468:20 #3 0x873cef in parse_pathspec pathspec.c:589:3 #4 0x503eb8 in check_ignore builtin/check-ignore.c:90:2 #5 0x5038af in cmd_check_ignore builtin/check-ignore.c:190:17 #6 0x4cd91d in run_builtin git.c:467:11 #7 0x4cb5f3 in handle_builtin git.c:719:3 #8 0x4ccf47 in run_argv git.c:808:4 #9 0x4caf49 in cmd_main git.c:939:19 #10 0x69e43e in main common-main.c:52:11 #11 0x7f18bb0dd349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 179 byte(s) leaked in 3 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	4fa268738c	builtin/bugreport: don't leak prefixed filename prefix_filename() returns newly allocated memory, and strbuf_addstr() doesn't take ownership of its inputs. Therefore we have to make sure to store and free prefix_filename()'s result. As this leak is in cmd_bugreport(), we could just as well UNLEAK the prefix - but there's no good reason not to just free it properly. This leak was found while running t0091, see output below: Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9acc66 in xrealloc wrapper.c:126:8 #2 0x93baed in strbuf_grow strbuf.c:98:2 #3 0x93c6ea in strbuf_add strbuf.c:295:2 #4 0x69f162 in strbuf_addstr ./strbuf.h:304:2 #5 0x69f083 in prefix_filename abspath.c:277:2 #6 0x4fb275 in cmd_bugreport builtin/bugreport.c:146:9 #7 0x4cd91d in run_builtin git.c:467:11 #8 0x4cb5f3 in handle_builtin git.c:719:3 #9 0x4ccf47 in run_argv git.c:808:4 #10 0x4caf49 in cmd_main git.c:939:19 #11 0x69df9e in main common-main.c:52:11 #12 0x7f523a987349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	d895804b5a	branch: FREE_AND_NULL instead of NULL'ing real_ref real_ref was previously populated by dwim_ref(), which allocates new memory. We need to make sure to free real_ref when discarding it. (real_ref is already being freed at the end of create_branch() - but if we discard it early then it will leak.) This fixes the following leak found while running t0002-t0099: Direct leak of 5 byte(s) in 1 object(s) allocated from: #0 0x486954 in strdup /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0xdd6484 in xstrdup wrapper.c:29:14 #2 0xc0f658 in expand_ref refs.c:671:12 #3 0xc0ecf1 in repo_dwim_ref refs.c:644:22 #4 0x8b1184 in dwim_ref ./refs.h:162:9 #5 0x8b0b02 in create_branch branch.c:284:10 #6 0x550cbb in update_refs_for_switch builtin/checkout.c:1046:4 #7 0x54e275 in switch_branches builtin/checkout.c:1274:2 #8 0x548828 in checkout_branch builtin/checkout.c:1668:9 #9 0x541306 in checkout_main builtin/checkout.c:2025:9 #10 0x5395fa in cmd_checkout builtin/checkout.c:2077:8 #11 0x4d02a8 in run_builtin git.c:467:11 #12 0x4cbfe9 in handle_builtin git.c:719:3 #13 0x4cf04f in run_argv git.c:808:4 #14 0x4cb85a in cmd_main git.c:939:19 #15 0x820cf6 in main common-main.c:52:11 #16 0x7f30bd9dd349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	b180c681bb	bloom: clear each bloom_key after use fill_bloom_key() allocates memory into bloom_key, we need to clean that up once the key is no longer needed. This leak was found while running t0002-t0099. Although this leak is happening in code being called from a test-helper, the same code is also used in various locations around git, and can therefore happen during normal usage too. Gabor's analysis shows that peak-memory usage during 'git commit-graph write' is reduced on the order of 10% for a selection of larger repos (along with an even larger reduction if we override modified path bloom filter limits): https://lore.kernel.org/git/20210411072651.GF2947267@szeder.dev/ LSAN output: Direct leak of 308 byte(s) in 11 object(s) allocated from: #0 0x49a5e2 in calloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:154:3 #1 0x6f4032 in xcalloc wrapper.c:140:8 #2 0x4f2905 in fill_bloom_key bloom.c:137:28 #3 0x4f34c1 in get_or_compute_bloom_filter bloom.c:284:4 #4 0x4cb484 in get_bloom_filter_for_commit t/helper/test-bloom.c:43:11 #5 0x4cb072 in cmd__bloom t/helper/test-bloom.c:97:3 #6 0x4ca7ef in cmd_main t/helper/test-tool.c:121:11 #7 0x4caace in main common-main.c:52:11 #8 0x7f798af95349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 308 byte(s) leaked in 11 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	4c217a4c34	ls-files: free max_prefix when done common_prefix() returns a new string, which we store in max_prefix - this string needs to be freed to avoid a leak. This leak is happening in cmd_ls_files, hence is of no real consequence - an UNLEAK would be just as good, but we might as well free the string properly. Leak found while running t0002, see output below: Direct leak of 8 byte(s) in 1 object(s) allocated from: #0 0x49a85d in malloc /home/abuild/rpmbuild/BUILD/llvm-11.0.0.src/build/../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ab1b4 in do_xmalloc wrapper.c:41:8 #2 0x9ab248 in do_xmallocz wrapper.c:75:8 #3 0x9ab22a in xmallocz wrapper.c:83:9 #4 0x9ab2d7 in xmemdupz wrapper.c:99:16 #5 0x78d6a4 in common_prefix dir.c:191:15 #6 0x5aca48 in cmd_ls_files builtin/ls-files.c:669:16 #7 0x4cd92d in run_builtin git.c:453:11 #8 0x4cb5fa in handle_builtin git.c:704:3 #9 0x4ccf57 in run_argv git.c:771:4 #10 0x4caf49 in cmd_main git.c:902:19 #11 0x69ce2e in main common-main.c:52:11 #12 0x7f64d4d94349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	5493ce7af9	wt-status: fix multiple small leaks rev.prune_data is populated (in multiple functions) via copy_pathspec, and therefore needs to be cleared after running the diff in those functions. rev(_info).pending is populated indirectly via setup_revisions, and also needs to be cleared once diffing is done. These leaks were found while running t0008 or t0021. The rev.prune_data leaks are small (80B) but noisy, hence I won't bother including their logs - the rev.pending leaks are bigger, and can happen early in the course of other commands, and therefore possibly more valuable to fix - see example log from a rebase below: Direct leak of 2048 byte(s) in 1 object(s) allocated from: #0 0x49ab79 in realloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:164:3 #1 0x9ac2a6 in xrealloc wrapper.c:126:8 #2 0x83da03 in add_object_array_with_path object.c:337:3 #3 0x8f5d8a in add_pending_object_with_path revision.c:329:2 #4 0x8ea50b in add_pending_object_with_mode revision.c:336:2 #5 0x8ea4fd in add_pending_object revision.c:342:2 #6 0x8ea610 in add_head_to_pending revision.c:354:2 #7 0x9b55f5 in has_uncommitted_changes wt-status.c:2474:2 #8 0x9b58c4 in require_clean_work_tree wt-status.c:2553:6 #9 0x606bcc in cmd_rebase builtin/rebase.c:1970:6 #10 0x4cd91d in run_builtin git.c:467:11 #11 0x4cb5f3 in handle_builtin git.c:719:3 #12 0x4ccf47 in run_argv git.c:808:4 #13 0x4caf49 in cmd_main git.c:939:19 #14 0x69dc0e in main common-main.c:52:11 #15 0x7f2d18909349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 5 byte(s) in 1 object(s) allocated from: #0 0x486834 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3 #1 0x9ac048 in xstrdup wrapper.c:29:14 #2 0x83da8d in add_object_array_with_path object.c:349:17 #3 0x8f5d8a in add_pending_object_with_path revision.c:329:2 #4 0x8ea50b in add_pending_object_with_mode revision.c:336:2 #5 0x8ea4fd in add_pending_object revision.c:342:2 #6 0x8ea610 in add_head_to_pending revision.c:354:2 #7 0x9b55f5 in has_uncommitted_changes wt-status.c:2474:2 #8 0x9b58c4 in require_clean_work_tree wt-status.c:2553:6 #9 0x606bcc in cmd_rebase builtin/rebase.c:1970:6 #10 0x4cd91d in run_builtin git.c:467:11 #11 0x4cb5f3 in handle_builtin git.c:719:3 #12 0x4ccf47 in run_argv git.c:808:4 #13 0x4caf49 in cmd_main git.c:939:19 #14 0x69dc0e in main common-main.c:52:11 #15 0x7f2d18909349 in __libc_start_main (/lib64/libc.so.6+0x24349) SUMMARY: AddressSanitizer: 2053 byte(s) leaked in 2 allocation(s). Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
Andrzej Hunt	db69bf608d	revision: free remainder of old commit list in limit_list limit_list() iterates over the original revs->commits list, and consumes many of its entries via pop_commit. However we might stop iterating over the list early (e.g. if we realise that the rest of the list is uninteresting). If we do stop iterating early, list will be pointing to the unconsumed portion of revs->commits - and we need to free this list to avoid a leak. (revs->commits itself will be an invalid pointer: it will have been free'd during the first pop_commit.) However the list pointer is later reused to iterate over our new list, but only for the limiting_can_increase_treesame() branch. We therefore need to introduce a new variable for that branch - and while we're here we can rename the original list to original_list as that makes its purpose more obvious. This leak was found while running t0090. It's not likely to be very impactful, but it can happen quite early during some checkout invocations, and hence seems to be worth fixing: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac084 in do_xmalloc wrapper.c:41:8 #2 0x9ac05a in xmalloc wrapper.c:62:9 #3 0x7175d6 in commit_list_insert commit.c:540:33 #4 0x71800f in commit_list_insert_by_date commit.c:604:9 #5 0x8f8d2e in process_parents revision.c:1128:5 #6 0x8f2f2c in limit_list revision.c:1418:7 #7 0x8f210e in prepare_revision_walk revision.c:3577:7 #8 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6 #9 0x512f05 in switch_branches builtin/checkout.c:1250:3 #10 0x50f8de in checkout_branch builtin/checkout.c:1646:9 #11 0x50ba12 in checkout_main builtin/checkout.c:2003:9 #12 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 #13 0x4cd91d in run_builtin git.c:467:11 #14 0x4cb5f3 in handle_builtin git.c:719:3 #15 0x4ccf47 in run_argv git.c:808:4 #16 0x4caf49 in cmd_main git.c:939:19 #17 0x69dc0e in main common-main.c:52:11 #18 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) Indirect leak of 48 byte(s) in 3 object(s) allocated from: #0 0x49a85d in malloc ../projects/compiler-rt/lib/asan/asan_malloc_linux.cpp:145:3 #1 0x9ac084 in do_xmalloc wrapper.c:41:8 #2 0x9ac05a in xmalloc wrapper.c:62:9 #3 0x717de6 in commit_list_append commit.c:1609:35 #4 0x8f1f9b in prepare_revision_walk revision.c:3554:12 #5 0x514170 in orphaned_commit_warning builtin/checkout.c:1185:6 #6 0x512f05 in switch_branches builtin/checkout.c:1250:3 #7 0x50f8de in checkout_branch builtin/checkout.c:1646:9 #8 0x50ba12 in checkout_main builtin/checkout.c:2003:9 #9 0x5086c0 in cmd_checkout builtin/checkout.c:2055:8 #10 0x4cd91d in run_builtin git.c:467:11 #11 0x4cb5f3 in handle_builtin git.c:719:3 #12 0x4ccf47 in run_argv git.c:808:4 #13 0x4caf49 in cmd_main git.c:939:19 #14 0x69dc0e in main common-main.c:52:11 #15 0x7faaabd0e349 in __libc_start_main (/lib64/libc.so.6+0x24349) Signed-off-by: Andrzej Hunt <ajrhunt@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
brian m. carlson	3dd71461e2	hex: print objects using the hash algorithm member Now that all code paths correctly set the hash algorithm member of struct object_id, write an object's hex representation using the hash algorithm member embedded in it. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
brian m. carlson	b8505ecbf2	hex: default to the_hash_algo on zero algorithm value There are numerous places in the codebase where we assume we can initialize data by zeroing all its bytes. However, when we do that with a struct object_id, it leaves the structure with a zero value for the algorithm, which is invalid. We could forbid this pattern and require that all struct object_id instances be initialized using oidclr, but this seems burdensome and it's unnatural to most C programmers. Instead, if the algorithm is zero, assume we wanted to use the default hash algorithm instead. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
brian m. carlson	71b7672b67	builtin/pack-objects: avoid using struct object_id for pack hash We use struct object_id for the names of objects. It isn't intended to be used for other hash values that don't name objects such as the pack hash. Because struct object_id will soon need to have its algorithm member set, using it in this code path would mean that we didn't set that member, only the hash member, which would result in a crash. For both of these reasons, switch to using an unsigned char array of size GIT_MAX_RAWSZ. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago
brian m. carlson	72871b132c	commit-graph: don't store file hashes as struct object_id The idea behind struct object_id is that it is supposed to represent the identifier of a standard Git object or a special pseudo-object like the all-zeros object ID. In this case, we have file hashes, which, while similar, are distinct from the identifiers of objects. Switch these code paths to use an unsigned char array. This is both more logically consistent and it means that we need not set the algorithm identifier for the struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>	4 years ago

... 3 4 5 6 7 ...

63149 Commits (f4dc9432fd287bde9100488943baf3c6a04d90d1) All Branches Search

63149 Commits (f4dc9432fd287bde9100488943baf3c6a04d90d1)

All Branches