summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2017-04-16refs: reject ref updates while GIT_QUARANTINE_PATH is setLibravatar Jeff King3-1/+19
As documented in git-receive-pack(1), updating a ref from within the pre-receive hook is dangerous and can corrupt your repo. This patch forbids ref updates entirely during the hook to make it harder for adventurous hook writers to shoot themselves in the foot. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-16receive-pack: document user-visible quarantine effectsLibravatar Jeff King2-0/+31
Commit 722ff7f87 (receive-pack: quarantine objects until pre-receive accepts, 2016-10-03) changed the underlying details of how we take in objects. This is mostly transparent to the user, but there are a few things they might notice. Let's document them. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-16receive-pack: drop tmp_objdir_env from run_update_hookLibravatar Jeff King1-1/+0
Since 722ff7f87 (receive-pack: quarantine objects until pre-receive accepts, 2016-10-03), we have to feed the pre-receive hook the tmp_objdir environment, so that git programs run from the hook know where to find the objects. That commit modified run_update_hook() to do the same, but there it is a noop. By the time we get to the update hooks, we have already migrated the objects from quarantine, and so tmp_objdir_env() will always return NULL. We can drop this useless call. Note that the ordering here and the lack of support for the update hook is intentional. The update hook calls are interspersed with actual ref updates, and we must migrate the objects before any refs are updated (since otherwise those refs would appear broken to outside processes). So the only other options are: - remain in quarantine for the _first_ ref, but not the others. This is sufficiently confusing that it can be rejected outright. - run all the individual update hooks first, then migrate, then update all the refs. But this changes the repository state that the update hooks see (i.e., whether or not refs from the same push are updated yet or not). So the functionality is fine and remains unchanged with this patch; we're just cleaning up a useless and confusing line of code. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10tmp-objdir: do not migrate files starting with '.'Libravatar Jeff King1-1/+1
This avoids "." and "..", as we already do, but also leaves room for index-pack to store extra data in the quarantine area (e.g., for passing back any analysis to be read by the pre-receive hook). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10tmp-objdir: put quarantine information in the environmentLibravatar Jeff King2-0/+3
The presence of the GIT_QUARANTINE_PATH variable lets any called programs know that they're operating in a temporary object directory (and where that directory is). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10receive-pack: quarantine objects until pre-receive acceptsLibravatar Jeff King2-1/+76
When a client pushes objects to us, index-pack checks the objects themselves and then installs them into place. If we then reject the push due to a pre-receive hook, we cannot just delete the packfile; other processes may be depending on it. We have to do a normal reachability check at this point via `git gc`. But such objects may hang around for weeks due to the gc.pruneExpire grace period. And worse, during that time they may be exploded from the pack into inefficient loose objects. Instead, this patch teaches receive-pack to put the new objects into a "quarantine" temporary directory. We make these objects available to the connectivity check and to the pre-receive hook, and then install them into place only if it is successful (and otherwise remove them as tempfiles). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10tmp-objdir: introduce API for temporary object directoriesLibravatar Jeff King3-0/+328
Once objects are added to the object database by a process, they cannot easily be deleted, as we don't know what other processes may have started referencing them. We have to clean them up with git-gc, which will apply the usual reachability and grace-period checks. This patch provides an alternative: it helps callers create a temporary directory inside the object directory, and a temporary environment which can be passed to sub-programs to ask them to write there (the original object directory remains accessible as an alternate of the temporary one). See tmp-objdir.h for details on the API. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10check_connected: accept an env argumentLibravatar Jeff King2-0/+6
This lets callers influence the environment seen by rev-list, which will be useful when we start providing quarantined objects. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10alternates: use fspathcmp to detect duplicatesLibravatar Jeff King3-1/+20
On a case-insensitive filesystem, we should realize that "a/objects" and "A/objects" are the same path. We already use fspathcmp() to check against the main object directory, but until recently we couldn't use it for comparing against other alternates (because their paths were not NUL-terminated strings). But now we can, so let's do so. Note that we also need to adjust count-objects to load the config, so that it can see the setting of core.ignorecase (this is required by the test, but is also a general bugfix for users of count-objects). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10sha1_file: always allow relative paths to alternatesLibravatar Jeff King2-8/+23
We recursively expand alternates repositories, so that if A borrows from B which borrows from C, A can see all objects. For the root object database, we allow relative paths, so A can point to B as "../B/objects". However, we currently do not allow relative paths when recursing, so B must use an absolute path to reach C. That is an ancient protection from c2f493a (Transitively read alternatives, 2006-05-07) that tries to avoid adding the same alternate through two different paths. Since 5bdf0a8 (sha1_file: normalize alt_odb path before comparing and storing, 2011-09-07), we use a normalized absolute path for each alt_odb entry. This means that in most cases the protection is no longer necessary; we will detect the duplicate no matter how we got there (but see below). And it's a good idea to get rid of it, as it creates an unnecessary complication when setting up recursive alternates (B has to know that A is going to borrow from it and make sure to use an absolute path). Note that our normalization doesn't actually look at the filesystem, so it can still be fooled by crossing symbolic links. But that's also true of absolute paths, so it's not a good reason to disallow only relative paths (it's potentially a reason to switch to real_path(), but that's a separate and non-trivial change). We adjust the test script here to demonstrate that this now works, and add new tests to show that the normalization does indeed suppress duplicates. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10count-objects: report alternates via verbose modeLibravatar Jeff King3-0/+25
There's no way to get the list of alternates that git computes internally; our tests only infer it based on which objects are available. In addition to testing, knowing this list may be helpful for somebody debugging their alternates setup. Let's add it to the "count-objects -v" output. We could give it a separate flag, but there's not really any need. "count-objects -v" is already a debugging catch-all for the object database, its output is easily extensible to new data items, and printing the alternates is not expensive (we already had to find them to count the objects). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10fill_sha1_file: write into a strbufLibravatar Jeff King1-24/+10
It's currently the responsibility of the caller to give fill_sha1_file() enough bytes to write into, leading them to manually compute the required lengths. Instead, let's just write into a strbuf so that it's impossible to get this wrong. The alt_odb caller already has a strbuf, so this makes things strictly simpler. The other caller, sha1_file_name(), uses a static PATH_MAX buffer and dies when it would overflow. We can convert this to a static strbuf, which means our allocation cost is amortized (and as a bonus, we no longer have to worry about PATH_MAX being too short for normal use). This does introduce some small overhead in fill_sha1_file(), as each strbuf_addchar() will check whether it needs to grow. However, between the optimization in fec501d (strbuf_addch: avoid calling strbuf_grow, 2015-04-16) and the fact that this is not generally called in a tight loop (after all, the next step is typically to access the file!) this probably doesn't matter. And even if it did, the right place to micro-optimize is inside fill_sha1_file(), by calling a single strbuf_grow() there. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10alternates: store scratch buffer as strbufLibravatar Jeff King3-18/+32
We pre-size the scratch buffer to hold a loose object filename of the form "xx/yyyy...", which leads to allocation code that is hard to verify. We have to use some magic numbers during the initial allocation, and then writers must blindly assume that the buffer is big enough. Using a strbuf makes it more clear that we cannot overflow. Unfortunately, we do still need some magic numbers to grow our strbuf before calling fill_sha1_path(), but the strbuf growth is much closer to the point of use. This makes it easier to see that it's correct, and opens the possibility of pushing it even further down if fill_sha1_path() learns to work on strbufs. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10fill_sha1_file: write "boring" charactersLibravatar Jeff King1-7/+5
This function forms a sha1 as "xx/yyyy...", but skips over the slot for the slash rather than writing it, leaving it to the caller to do so. It also does not bother to put in a trailing NUL, even though every caller would want it (we're forming a path which by definition is not a directory, so the only thing to do with it is feed it to a system call). Let's make the lives of our callers easier by just writing out the internal "/" and the NUL. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10alternates: use a separate scratch spaceLibravatar Jeff King6-37/+24
The alternate_object_database struct uses a single buffer both for storing the path to the alternate, and as a scratch buffer for forming object names. This is efficient (since otherwise we'd end up storing the path twice), but it makes life hard for callers who just want to know the path to the alternate. They have to remember to stop reading after "alt->name - alt->base" bytes, and to subtract one for the trailing '/'. It would be much simpler if they could simply access a NUL-terminated path string. We could encapsulate this in a function which puts a NUL in the scratch buffer and returns the string, but that opens up questions about the lifetime of the result. The first time another caller uses the alternate, the scratch buffer may get other data tacked onto it. Let's instead just store the root path separately from the scratch buffer. There aren't enough alternates being stored for the duplicated data to matter for performance, and this keeps things simple and safe for the callers. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10alternates: encapsulate alt->base mungingLibravatar Jeff King1-6/+13
The alternate_object_database struct holds a path to the alternate objects, but we also use that buffer as scratch space for forming loose object filenames. Let's pull that logic into a helper function so that we can more easily modify it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10alternates: provide helper for allocating alternateLibravatar Jeff King3-15/+26
Allocating a struct alternate_object_database is tricky, as we must over-allocate the buffer to provide scratch space, and then put in particular '/' and NUL markers. Let's encapsulate this in a function so that the complexity doesn't leak into callers (and so that we can modify it later). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10alternates: provide helper for adding to alternates listLibravatar Jeff King3-23/+25
The submodule code wants to temporarily add an alternate object store to our in-memory alt_odb list, but does it manually. Let's provide a helper so it can reuse the code in link_alt_odb_entry(). While we're adding our new add_to_alternates_memory(), let's document add_to_alternates_file(), as the two are related. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10link_alt_odb_entry: refactor string handlingLibravatar Jeff King1-38/+45
The string handling in link_alt_odb_entry() is mostly an artifact of the original version, which took the path as a ptr/len combo, and did not have a NUL-terminated string until we created one in the alternate_object_database struct. But since 5bdf0a8 (sha1_file: normalize alt_odb path before comparing and storing, 2011-09-07), the first thing we do is put the path into a strbuf, which gives us some easy opportunities for cleanup. In particular: - we call strlen(pathbuf.buf), which is silly; we can look at pathbuf.len. - even though we have a strbuf, we don't maintain its "len" field when chomping extra slashes from the end, and instead keep a separate "pfxlen" variable. We can fix this and then drop "pfxlen" entirely. - we don't check whether the path is usable until after we allocate the new struct, making extra cleanup work for ourselves. Since we have a NUL-terminated string, we can bump the "is it usable" checks higher in the function. While we're at it, we can move that logic to its own helper, which makes the flow of link_alt_odb_entry() easier to follow. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10link_alt_odb_entry: handle normalize_path errorsLibravatar Jeff King3-2/+37
When we add a new alternate to the list, we try to normalize out any redundant "..", etc. However, we do not look at the return value of normalize_path_copy(), and will happily continue with a path that could not be normalized. Worse, the normalizing process is done in-place, so we are left with whatever half-finished working state the normalizing function was in. Fortunately, this cannot cause us to read past the end of our buffer, as that working state will always leave the NUL from the original path in place. And we do tend to notice problems when we check is_directory() on the path. But you can see the nonsense that we feed to is_directory with an entry like: this/../../is/../../way/../../too/../../deep/../../to/../../resolve in your objects/info/alternates, which yields: error: object directory /to/e/deep/too/way//ects/this/../../is/../../way/../../too/../../deep/../../to/../../resolve does not exist; check .git/objects/info/alternates. We can easily fix this just by checking the return value. But that makes it hard to generate a good error message, since we're normalizing in-place and our input value has been overwritten by cruft. Instead, let's provide a strbuf helper that does an in-place normalize, but restores the original contents on error. This uses a second buffer under the hood, which is slightly less efficient, but this is not a performance-critical code path. The strbuf helper can also properly set the "len" parameter of the strbuf before returning. Just doing: normalize_path_copy(buf.buf, buf.buf); will shorten the string, but leave buf.len at the original length. That may be confusing to later code which uses the strbuf. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-10t5613: clarify "too deep" recursion testsLibravatar Jeff King1-8/+19
These tests are just trying to show that we allow recursion up to a certain depth, but not past it. But the counting is a bit non-intuitive, and rather than test at the edge of the breakage, we test "OK" cases in the middle of the chain. Let's explain what's going on, and explicitly test the switch between "OK" and "too deep". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03t5613: do not chdir in main processLibravatar Jeff King1-59/+33
Our usual style when working with subdirectories is to chdir inside a subshell or to use "git -C", which means we do not have to constantly return to the main test directory. Let's convert this old test, which does not follow that style. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03t5613: whitespace/style cleanupsLibravatar Jeff King1-52/+62
Our normal test style these days puts the opening quote of the body on the description line, and indents the body with a single tab. This ancient test did not follow this. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03t5613: use test_must_failLibravatar Jeff King1-6/+6
Besides being our normal style, this correctly checks for an error exit() versus signal death. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03t5613: drop test_valid_repo functionLibravatar Jeff King1-12/+7
This function makes sure that "git fsck" does not report any errors. But "--full" has been the default since f29cd39 (fsck: default to "git fsck --full", 2009-10-20), and we can use the exit code (instead of counting the lines) since e2b4f63 (fsck: exit with non-zero status upon errors, 2007-03-05). So we can just use "git fsck", which is shorter and more flexible (e.g., we can use "git -C"). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03t5613: drop reachable_via functionLibravatar Jeff King1-10/+0
This function was never used since its inception in dd05ea1 (test case for transitive info/alternates, 2006-05-07). Which is just as well, since it mutates the repo state in a way that would invalidate further tests, without cleaning up after itself. Let's get rid of it so that nobody is tempted to use it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03Sync with 2.10.1Libravatar Junio C Hamano2-1/+13
* maint: Git 2.10.1
2016-10-03Seventh batch for 2.11Libravatar Junio C Hamano1-0/+40
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03Merge branch 'pb/rev-list-reverse-with-count'Libravatar Junio C Hamano1-2/+3
Doc update to clarify what "log -3 --reverse" does. * pb/rev-list-reverse-with-count: rev-list-options: clarify the usage of --reverse
2016-10-03Merge branch 'dt/tree-fsck'Libravatar Junio C Hamano5-20/+130
The codepath in "git fsck" to detect malformed tree objects has been updated not to die but keep going after detecting them. * dt/tree-fsck: fsck: handle bad trees like other errors tree-walk: be more specific about corrupt tree errors
2016-10-03Merge branch 'kd/mailinfo-quoted-string'Libravatar Junio C Hamano6-33/+159
An author name, that spelled a backslash-quoted double quote in the human readable part "My \"double quoted\" name", was not unquoted correctly while applying a patch from a piece of e-mail. * kd/mailinfo-quoted-string: mailinfo: unescape quoted-pair in header fields t5100-mailinfo: replace common path prefix with variable
2016-10-03Merge branch 'mh/diff-indent-heuristic'Libravatar Junio C Hamano1-7/+7
Clean-up for a recently graduated topic. * mh/diff-indent-heuristic: xdiff: rename "struct group" to "struct xdlgroup"
2016-10-03Merge branch 'dt/mailinfo'Libravatar Junio C Hamano1-0/+1
* dt/mailinfo: add David Turner's Two Sigma address
2016-10-03Merge branch 'va/git-gui-i18n'Libravatar Junio C Hamano3-3/+3012
"git gui" l10n to Portuguese. * va/git-gui-i18n: git-gui: l10n: add Portuguese translation git-gui i18n: mark strings for translation
2016-10-03Merge branch 'rs/git-gui-use-modern-git-merge-syntax'Libravatar Junio C Hamano1-6/+1
The original command line syntax for "git merge", which was "git merge <msg> HEAD <parent>...", has been deprecated for quite some time, and "git gui" was the last in-tree user of the syntax. This is finally fixed, so that we can move forward with the deprecation. * rs/git-gui-use-modern-git-merge-syntax: git-gui: stop using deprecated merge syntax
2016-10-03Merge branch 'jc/verify-loose-object-header'Libravatar Junio C Hamano2-8/+30
Codepaths that read from an on-disk loose object were too loose in validating what they are reading is a proper object file and sometimes read past the data they read from the disk, which has been corrected. H/t to Gustavo Grieco for reporting. * jc/verify-loose-object-header: unpack_sha1_header(): detect malformed object header streaming: make sure to notice corrupt object
2016-10-03Merge branch 'nd/init-core-worktree-in-multi-worktree-world'Libravatar Junio C Hamano4-50/+57
"git init" tried to record core.worktree in the repository's 'config' file when GIT_WORK_TREE environment variable was set and it was different from where GIT_DIR appears as ".git" at its top, but the logic was faulty when .git is a "gitdir:" file that points at the real place, causing trouble in working trees that are managed by "git worktree". This has been corrected. * nd/init-core-worktree-in-multi-worktree-world: init: kill git_link variable init: do not set unnecessary core.worktree init: kill set_git_dir_init() init: call set_git_dir_init() from within init_db() init: correct re-initialization from a linked worktree
2016-10-03Merge branch 'ik/gitweb-force-highlight'Libravatar Junio C Hamano3-14/+29
"gitweb" can spawn "highlight" to show blob contents with (programming) language-specific syntax highlighting, but only when the language is known. "highlight" can however be told to make the guess itself by giving it "--force" option, which has been enabled. * ik/gitweb-force-highlight: gitweb: use highlight's shebang detection gitweb: remove unused guess_file_syntax() parameter
2016-10-03Merge branch 'rs/copy-array'Libravatar Junio C Hamano7-9/+40
Code cleanup. * rs/copy-array: use COPY_ARRAY add COPY_ARRAY
2016-10-03Git 2.10.1Libravatar Junio C Hamano3-2/+14
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-03Merge branch 'jk/ident-ai-canonname-could-be-null' into maintLibravatar Junio C Hamano1-1/+1
In the codepath that comes up with the hostname to be used in an e-mail when the user didn't tell us, we looked at ai_canonname field in struct addrinfo without making sure it is not NULL first. * jk/ident-ai-canonname-could-be-null: ident: handle NULL ai_canonname
2016-10-03Merge branch 'jk/doc-cvs-update' into maintLibravatar Junio C Hamano2-2/+6
Documentation around tools to import from CVS was fairly outdated. * jk/doc-cvs-update: docs/cvs-migration: mention cvsimport caveats docs/cvs-migration: update link to cvsps homepage docs/cvsimport: prefer cvs-fast-export to parsecvs
2016-10-03Merge branch 'jk/pack-tag-of-tag' into maintLibravatar Junio C Hamano2-30/+95
"git pack-objects --include-tag" was taught that when we know that we are sending an object C, we want a tag B that directly points at C but also a tag A that points at the tag B. We used to miss the intermediate tag B in some cases. * jk/pack-tag-of-tag: pack-objects: walk tag chains for --include-tag t5305: simplify packname handling t5305: use "git -C" t5305: drop "dry-run" of unpack-objects t5305: move cleanup into test block
2016-09-29Sync with maintLibravatar Junio C Hamano1-0/+87
* maint: Prepare for 2.10.1
2016-09-29Sixth batch for 2.11Libravatar Junio C Hamano1-0/+28
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-29Merge branch 'jc/worktree-config'Libravatar Junio C Hamano1-0/+2
"git worktree", even though it used the default_abbrev setting that ought to be affected by core.abbrev configuration variable, ignored the variable setting. The command has been taught to read the default set of configuration variables to correct this. * jc/worktree-config: worktree: honor configuration variables
2016-09-29Merge branch 'jk/ident-ai-canonname-could-be-null'Libravatar Junio C Hamano1-1/+1
In the codepath that comes up with the hostname to be used in an e-mail when the user didn't tell us, we looked at ai_canonname field in struct addrinfo without making sure it is not NULL first. * jk/ident-ai-canonname-could-be-null: ident: handle NULL ai_canonname
2016-09-29Merge branch 'jt/fetch-pack-in-vain-count-with-stateless'Libravatar Junio C Hamano1-2/+9
When "git fetch" tries to find where the history of the repository it runs in has diverged from what the other side has, it has a mechanism to avoid digging too deep into irrelevant side branches. This however did not work well over the "smart-http" transport due to a design bug, which has been fixed. * jt/fetch-pack-in-vain-count-with-stateless: fetch-pack: do not reset in_vain on non-novel acks
2016-09-29Merge branch 'jk/verify-packfile-gently'Libravatar Junio C Hamano1-5/+2
A low-level function verify_packfile() was meant to show errors that were detected without dying itself, but under some conditions it didn't and died instead, which has been fixed. * jk/verify-packfile-gently: verify_packfile: check pack validity before accessing data
2016-09-29Merge branch 'jt/mailinfo-fold-in-body-headers'Libravatar Junio C Hamano12-36/+159
When "git format-patch --stdout" output is placed as an in-body header and it uses the RFC2822 header folding, "git am" failed to put the header line back into a single logical line. The underlying "git mailinfo" was taught to handle this properly. * jt/mailinfo-fold-in-body-headers: mailinfo: handle in-body header continuations mailinfo: make is_scissors_line take plain char * mailinfo: separate in-body header processing