aboutsummaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAge
* Merge branch 'ak/lazy-prereq-mktemp' into maintJunio C Hamano2017-01-17
|\ | | | | | | | | | | | | Test code clean-up. * ak/lazy-prereq-mktemp: t7610: clean up foo.XXXXXX tmpdir
| * t7610: clean up foo.XXXXXX tmpdirJeff King2016-11-29
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The lazy prereq for MKTEMP uses "mktemp -t" to see if mergetool's internal mktemp call will be able to run. But unlike the call inside mergetool, we do not ever bother to clean up the result, and the /tmp of git developers will slowly fill up with "foo.XXXXXX" directories as they run the test suite over and over. Let's clean up the directory after we've verified its creation. Note that we don't use test_when_finished here, and instead just make rmdir part of the &&-chain. We should only remove something that we're confident we just created. A failure in the middle of the chain either means there's nothing to clean up, or we are very confused and should err on the side of caution. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'nd/qsort-in-merge-recursive' into maintJunio C Hamano2017-01-17
|\ \ | | | | | | | | | | | | | | | | | | Code simplification. * nd/qsort-in-merge-recursive: merge-recursive.c: use string_list_sort instead of qsort
| * | merge-recursive.c: use string_list_sort instead of qsortNguyễn Thái Ngọc Duy2016-11-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Merge-recursive sorts a string list using a raw qsort(), where it feeds the "items" from one struct but the "nr" and size fields from another struct. This isn't a bug because one list is a copy of the other, but it's unnecessarily confusing (and also caused our recent QSORT() cleanups via coccinelle to miss this call site). Let's use string_list_sort() instead, which is more concise and harder to get wrong. Note that we need to adjust our comparison function, which gets fed only the strings now, not the string_list_items. That's OK because we don't use the "util" field as part of our sort. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'dt/smart-http-detect-server-going-away' into maintJunio C Hamano2017-01-17
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the http server gives an incomplete response to a smart-http rpc call, it could lead to client waiting for a full response that will never come. Teach the client side to notice this condition and abort the transfer. An improvement counterproposal has failed. cf. <20161114194049.mktpsvgdhex2f4zv@sigill.intra.peff.net> * dt/smart-http-detect-server-going-away: upload-pack: optionally allow fetching any sha1 remote-curl: don't hang when a server dies before any output
| * | | upload-pack: optionally allow fetching any sha1David Turner2016-11-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It seems a little silly to do a reachabilty check in the case where we trust the user to access absolutely everything in the repository. Also, it's racy in a distributed system -- perhaps one server advertises a ref, but another has since had a force-push to that ref, and perhaps the two HTTP requests end up directed to these different servers. Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | remote-curl: don't hang when a server dies before any outputDavid Turner2016-11-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the event that a HTTP server closes the connection after giving a 200 but before giving any packets, we don't want to hang forever waiting for a response that will never come. Instead, we should die immediately. One case where this happens is when attempting to fetch a dangling object by its object name. In this case, the server dies before sending any data. Prior to this patch, fetch-pack would wait for data from the server, and remote-curl would wait for fetch-pack, causing a deadlock. Despite this patch, there is other possible malformed input that could cause the same deadlock (e.g. a half-finished pktline, or a pktline but no trailing flush). There are a few possible solutions to this: 1. Allowing remote-curl to tell fetch-pack about the EOF (so that fetch-pack could know that no more data is coming until it says something else). This is tricky because an out-of-band signal would be required, or the http response would have to be re-framed inside another layer of pkt-line or something. 2. Make remote-curl understand some of the protocol. It turns out that in addition to understanding pkt-line, it would need to watch for ack/nak. This is somewhat fragile, as information about the protocol would end up in two places. Also, pkt-lines which are already at the length limit would need special handling. Both of these solutions would require a fair amount of work, whereas this hack is easy and solves at least some of the problem. Still to do: it would be good to give a better error message than "fatal: The remote end hung up unexpectedly". Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'mk/mingw-winansi-ttyname-termination-fix' into maintJunio C Hamano2017-01-17
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A potential but unlikely buffer overflow in Windows port has been fixed. * mk/mingw-winansi-ttyname-termination-fix: mingw: consider that UNICODE_STRING::Length counts bytes
| * | | | mingw: consider that UNICODE_STRING::Length counts bytesMax Kirillov2016-12-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | UNICODE_STRING::Length field means size of buffer in bytes[1], despite of buffer itself being array of wchar_t. Because of that terminating zero is placed twice as far. Fix it. [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa380518.aspx Signed-off-by: Max Kirillov <max@max630.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'gv/p4-multi-path-commit-fix' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git p4" that tracks multile p4 paths imported a single changelist that touches files in these multiple paths as one commit, followed by many empty commits. This has been fixed. * gv/p4-multi-path-commit-fix: git-p4: fix multi-path changelist empty commits
| * | | | | git-p4: fix multi-path changelist empty commitsGeorge Vanburgh2016-12-19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When importing from multiple perforce paths - we may attempt to import a changelist that contains files from two (or more) of these depot paths. Currently, this results in multiple git commits - one containing the changes, and the other(s) as empty commit(s). This behavior was introduced in commit 1f90a64891 ("git-p4: reduce number of server queries for fetches", 2015-12-19). Reproduction Steps: 1. Have a git repo cloned from a perforce repo using multiple depot paths (e.g. //depot/foo and //depot/bar). 2. Submit a single change to the perforce repo that makes changes in both //depot/foo and //depot/bar. 3. Run "git p4 sync" to sync the change from #2. Change is synced as multiple commits, one for each depot path that was affected. Using a set, instead of a list inside p4ChangesForPaths() ensures that each changelist is unique to the returned list, and therefore only a single commit is generated for each changelist. Reported-by: James Farwell <jfarwell@vmware.com> Signed-off-by: George Vanburgh <gvanburgh@bloomberg.net> Reviewed-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | Merge branch 'jk/difftool-in-subdir' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Even though an fix was attempted in Git 2.9.3 days, but running "git difftool --dir-diff" from a subdirectory never worked. This has been fixed. * jk/difftool-in-subdir: difftool: rename variables for consistency difftool: chdir as early as possible difftool: sanitize $workdir as early as possible difftool: fix dir-diff index creation when in a subdirectory
| * | | | | | difftool: rename variables for consistencyDavid Aguilar2016-12-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Always call the list of files @files. Always call the worktree $worktree. Signed-off-by: David Aguilar <davvid@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | difftool: chdir as early as possibleDavid Aguilar2016-12-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make difftool chdir to the top-level of the repository as soon as it can so that we can simplify how paths are handled. Replace construction of absolute paths via string concatenation with relative paths wherever possible. The bulk of the code no longer needs to use absolute paths. Signed-off-by: David Aguilar <davvid@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | difftool: sanitize $workdir as early as possibleDavid Aguilar2016-12-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The double-slash fixup on the $workdir variable was being performed just-in-time to avoid double-slashes in symlink targets, but the rest of the code was silently using paths with embedded "//" in them. A recent user-reported error message contained double-slashes. Eliminate the issue by sanitizing inputs as soon as they arrive. Signed-off-by: David Aguilar <davvid@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | difftool: fix dir-diff index creation when in a subdirectoryDavid Aguilar2016-12-11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 9ec26e7977 (difftool: fix argument handling in subdirs, 2016-07-18) corrected how path arguments are handled in a subdirectory, but it introduced a regression in how entries outside of the subdirectory are handled by dir-diff. When preparing the right-side of the diff we only include the changed paths in the temporary area. The left side of the diff is constructed from a temporary index that is built from the same set of changed files, but it was being constructed from within the subdirectory. This is a problem because the indexed paths are toplevel-relative, and thus they were not getting added to the index. Teach difftool to chdir to the toplevel of the repository before preparing its temporary indexes. This ensures that all of the toplevel-relative paths are valid. Add test cases to more thoroughly exercise this scenario. Reported-by: Frank Becker <fb@mooflu.com> Signed-off-by: David Aguilar <davvid@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | Merge branch 'ld/p4-compare-dir-vs-symlink' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git p4" misbehaved when swapping a directory and a symbolic link. * ld/p4-compare-dir-vs-symlink: git-p4: avoid crash adding symlinked directory
| * | | | | | | git-p4: avoid crash adding symlinked directoryLuke Diamand2016-12-18
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When submitting to P4, if git-p4 came across a symlinked directory, then during the generation of the submit diff, it would try to open it as a normal file and fail. Spot symlinks (of any type) and output a description of the symlink instead. Add a test case. Signed-off-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | Merge branch 'jc/push-default-explicit' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A lazy "git push" without refspec did not internally use a fully specified refspec to perform 'current', 'simple', or 'upstream' push, causing unnecessary "ambiguous ref" errors. * jc/push-default-explicit: push: test pushing ambiguously named branches push: do not use potentially ambiguous default refspec
| * | | | | | | | push: test pushing ambiguously named branchesDennis Kaarsemaker2016-10-31
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | push: do not use potentially ambiguous default refspecJunio C Hamano2016-10-28
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the user does the lazy "git push" with no parameters with push.default set to either "upstream", "simple" or "current", we internally generated a refspec that has the current branch name on the source side and used it to push. However, the branch name (say "test") may be an ambiguous refname in the context of the source repository---there may be a tag with the same name, for example. This would trigger an unnecessary error without any fault on the end-user's side. Be explicit and give a full refname as the source side to avoid the ambiguity. The destination side when pushing with the "current" sent only the name of the branch and forcing the receiving end to guess, which is the same issue. Be explicit there as well. Reported-by: Kannan Goundan <kannan@cakoose.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | Merge branch 'jt/mailinfo-fold-in-body-headers' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix for NDEBUG builds. * jt/mailinfo-fold-in-body-headers: mailinfo.c: move side-effects outside of assert
| * | | | | | | | | mailinfo.c: move side-effects outside of assertKyle J. McKay2016-12-20
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since 6b4b013f18 (mailinfo: handle in-body header continuations, 2016-09-20, v2.11.0) mailinfo.c has contained new code with an assert of the form: assert(call_a_function(...)) The function in question, check_header, has side effects. This means that when NDEBUG is defined during a release build the function call is omitted entirely, the side effects do not take place and tests (fortunately) start failing. Since the only time that mi->inbody_header_accum is appended to is in check_inbody_header, and appending onto a blank mi->inbody_header_accum always happens when is_inbody_header is true, this guarantees a prefix that causes check_header to always return true. Therefore replace the assert with an if !check_header + DIE combination to reflect this. Helped-by: Jonathan Tan <jonathantanmy@google.com> Helped-by: Jeff King <peff@peff.net> Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Kyle J. McKay <mackyle@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | | Merge branch 'jk/index-pack-wo-repo-from-stdin' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git index-pack --stdin" needs an access to an existing repository, but "git index-pack file.pack" to generate an .idx file that corresponds to a packfile does not. * jk/index-pack-wo-repo-from-stdin: index-pack: skip collision check when not in repository t: use nongit() function where applicable index-pack: complain when --stdin is used outside of a repo t5000: extract nongit function to test-lib-functions.sh
| * | | | | | | | | | index-pack: skip collision check when not in repositoryJeff King2016-12-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | You can run "git index-pack path/to/foo.pack" outside of a repository to generate an index file, or just to verify the contents. There's no point in doing a collision check, since we obviously do not have any objects to collide with. The current code will blindly look in .git/objects based on the result of setup_git_env(). That effectively gives us the right answer (since we won't find any objects), but it's a waste of time, and it conflicts with our desire to eventually get rid of the "fallback to .git" behavior of setup_git_env(). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | t: use nongit() function where applicableJeff King2016-12-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many tests want to run a command outside of any git repo; with the nongit() function this is now a one-liner. It saves a few lines, but more importantly, it's immediately obvious what the code is trying to accomplish. This doesn't convert every such case in the test suite; it just covers those that want to do a one-off command. Other cases, such as the ones in t4035, are part of a larger scheme of outside-repo files, and it's less confusing for them to stay consistent with the surrounding tests. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | index-pack: complain when --stdin is used outside of a repoJeff King2016-12-16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The index-pack builtin is marked as RUN_SETUP_GENTLY, because it's perfectly fine to index a pack in the filesystem outside of any repository. However, --stdin mode will write the result to the object database, which does not make sense outside of a repository. Doing so creates a bogus ".git" directory with nothing in it except the newly-created pack and its index. Instead, let's flag this as an error and abort. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | t5000: extract nongit function to test-lib-functions.shJeff King2016-12-16
| | |_|/ / / / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This function abstracts the idea of running a command outside of any repository (which is slightly awkward to do because even if you make a non-repo directory, git may keep walking up outside of the trash directory). There are several scripts that use the same technique, so let's make the function available for everyone. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | | Merge branch 'jk/parseopt-usage-msg-opt' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function usage_msg_opt() has been updated to say "fatal:" before the custom message programs give, when they want to die with a message about wrong command line options followed by the standard usage string. * jk/parseopt-usage-msg-opt: parse-options: print "fatal:" before usage_msg_opt()
| * | | | | | | | | | parse-options: print "fatal:" before usage_msg_opt()Jeff King2016-12-14
| | |_|_|_|/ / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Programs may use usage_msg_opt() to print a brief message followed by the program usage, and then exit. The message isn't prefixed at all, though, so it doesn't match our usual error output and is easy to overlook: $ git clone 1 2 3 Too many arguments. usage: git clone [<options>] [--] <repo> [<dir>] -v, --verbose be more verbose -q, --quiet be more quiet --progress force progress reporting -n, --no-checkout don't create a checkout --bare create a bare repository [...and so on for another 31 lines...] It looks especially bad when the message starts with an option, like: $ git replace -e -e needs exactly one argument usage: git replace [-f] <object> <replacement> or: git replace [-f] --edit <object> [...etc...] Let's put our usual "fatal:" prefix in front of it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | | Merge branch 'jk/quote-env-path-list-component' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A recent update to receive-pack to make it easier to drop garbage objects made it clear that GIT_ALTERNATE_OBJECT_DIRECTORIES cannot have a pathname with a colon in it (no surprise!), and this in turn made it impossible to push into a repository at such a path. This has been fixed by introducing a quoting mechanism used when appending such a path to the colon-separated list. * jk/quote-env-path-list-component: t5615-alternate-env: double-quotes in file names do not work on Windows t5547-push-quarantine: run the path separator test on Windows, too tmp-objdir: quote paths we add to alternates alternates: accept double-quoted paths
| * | | | | | | | | | t5615-alternate-env: double-quotes in file names do not work on WindowsJohannes Sixt2016-12-21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Protect a recently added test case with !MINGW. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | t5547-push-quarantine: run the path separator test on Windows, tooJohannes Sixt2016-12-13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To perform the test case on Windows in a way that corresponds to the POSIX version, inject the semicolon in a directory name. Typically, an absolute POSIX style path, such as the one in $PWD, is translated into a Windows style path by bash when it invokes git.exe. However, the presence of the semicolon suppresses this translation; but the untranslated POSIX style path is useless for git.exe. Therefore, instead of $PWD pass the Windows style path that $(pwd) produces. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | tmp-objdir: quote paths we add to alternatesJeff King2016-12-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 722ff7f87 (receive-pack: quarantine objects until pre-receive accepts, 2016-10-03) regressed pushes to repositories with colon (or semi-colon in Windows in them) because it adds the repository's main object directory to GIT_ALTERNATE_OBJECT_DIRECTORIES. The receiver interprets the colon as a delimiter, not as part of the path, and index-pack is unable to find objects which it needs to resolve deltas. The previous commit introduced a quoting mechanism for the alternates list; let's use it here to cover this case. We'll avoid quoting when we can, though. This alternate setup is also used when calling hooks, so it's possible that the user may call older git implementations which don't understand the quoting mechanism. By quoting only when necessary, this setup will continue to work unless the user _also_ has a repository whose path contains the delimiter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | alternates: accept double-quoted pathsJeff King2016-12-12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We read lists of alternates from objects/info/alternates files (delimited by newline), as well as from the GIT_ALTERNATE_OBJECT_DIRECTORIES environment variable (delimited by colon or semi-colon, depending on the platform). There's no mechanism for quoting the delimiters, so it's impossible to specify an alternate path that contains a colon in the environment, or one that contains a newline in a file. We've lived with that restriction for ages because both alternates and filenames with colons are relatively rare, and it's only a problem when the two meet. But since 722ff7f87 (receive-pack: quarantine objects until pre-receive accepts, 2016-10-03), which builds on the alternates system, every push causes the receiver to set GIT_ALTERNATE_OBJECT_DIRECTORIES internally. It would be convenient to have some way to quote the delimiter so that we can represent arbitrary paths. The simplest thing would be an escape character before a quoted delimiter (e.g., "\:" as a literal colon). But that creates a backwards compatibility problem: any path which uses that escape character is now broken, and we've just shifted the problem. We could choose an unlikely escape character (e.g., something from the non-printable ASCII range), but that's awkward to use. Instead, let's treat names as unquoted unless they begin with a double-quote, in which case they are interpreted via our usual C-stylke quoting rules. This also breaks backwards-compatibility, but in a smaller way: it only matters if your file has a double-quote as the very _first_ character in the path (whereas an escape character is a problem anywhere in the path). It's also consistent with many other parts of git, which accept either a bare pathname or a double-quoted one, and the sender can choose to quote or not as required. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | Merge branch 'jk/alt-odb-cleanup' into jk/quote-env-path-list-componentJunio C Hamano2016-12-12
| |\ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * jk/alt-odb-cleanup: alternates: re-allow relative paths from environment
* | \ \ \ \ \ \ \ \ \ \ Merge branch 'nd/shallow-fixup' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Code cleanup in shallow boundary computation. * nd/shallow-fixup: shallow.c: remove useless code shallow.c: bit manipulation tweaks shallow.c: avoid theoretical pointer wrap-around shallow.c: make paint_alloc slightly more robust shallow.c: stop abusing COMMIT_SLAB_SIZE for paint_info's memory pools shallow.c: rename fields in paint_info to better express their purposes
| * | | | | | | | | | | | shallow.c: remove useless codeNguyễn Thái Ngọc Duy2016-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some context before we talk about the removed code. This paint_down() is part of step 6 of 58babff (shallow.c: the 8 steps to select new commits for .git/shallow - 2013-12-05). When we fetch from a shallow repository, we need to know if one of the new/updated refs needs new "shallow commits" in .git/shallow (because we don't have enough history of those refs) and which one. The question at step 6 is, what (new) shallow commits are required in other to maintain reachability throughout the repository _without_ cutting our history short? To answer, we mark all commits reachable from existing refs with UNINTERESTING ("rev-list --not --all"), mark shallow commits with BOTTOM, then for each new/updated refs, walk through the commit graph until we either hit UNINTERESTING or BOTTOM, marking the ref on the commit as we walk. After all the walking is done, we check the new shallow commits. If we have not seen any new ref marked on a new shallow commit, we know all new/updated refs are reachable using just our history and .git/shallow. The shallow commit in question is not needed and can be thrown away. So, the code. The loop here (to walk through commits) is basically 1. get one commit from the queue 2. ignore if it's SEEN or UNINTERESTING 3. mark it 4. go through all the parents and.. 5a. mark it if it's never marked before 5b. put it back in the queue What we do in this patch is drop step 5a because it is not necessary. The commit being marked at 5a is put back on the queue, and will be marked at step 3 at the next iteration. The only case it will not be marked is when the commit is already marked UNINTERESTING (5a does not check this), which will be ignored at step 2. But we don't care about refs marking on UNINTERESTING. We care about the marking on _shallow commits_ that are not reachable from our current history (and having UNINTERESTING on it means it's reachable). So it's ok for an UNINTERESTING not to be ref-marked. Reported-by: Rasmus Villemoes <rv@rasmusvillemoes.dk> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | shallow.c: bit manipulation tweaksRasmus Villemoes2016-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | First of all, 1 << 31 is technically undefined behaviour, so let's just use an unsigned literal. If i is 'signed int' and gcc doesn't know that i is positive, gcc generates code to compute the C99-mandated values of "i / 32" and "i % 32", which is a lot more complicated than simple a simple shifts/mask. The only caller of paint_down actually passes an "unsigned int" value, but the prototype of paint_down causes (completely well-defined) conversion to signed int, and gcc has no way of knowing that the converted value is non-negative. Just make the id parameter unsigned. In update_refstatus, the change in generated code is much smaller, presumably because gcc is smart enough to see that i starts as 0 and is only incremented, so it is allowed (per the UD of signed overflow) to assume that i is always non-negative. But let's just help less smart compilers generate good code anyway. Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | shallow.c: avoid theoretical pointer wrap-aroundRasmus Villemoes2016-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The expression info->free+size is technically undefined behaviour in exactly the case we want to test for. Moreover, the compiler is likely to translate the expression to (unsigned long)info->free + size > (unsigned long)info->end where there's at least a theoretical chance that the LHS could wrap around 0, giving a false negative. This might as well be written using pointer subtraction avoiding these issues. Signed-off-by: Rasmus Villemoes <rv@rasmusvillemoes.dk> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | shallow.c: make paint_alloc slightly more robustNguyễn Thái Ngọc Duy2016-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | paint_alloc() allocates a big block of memory and splits it into smaller, fixed size, chunks of memory whenever it's called. Each chunk contains enough bits to present all "new refs" [1] in a fetch from a shallow repository. We do not check if the new "big block" is smaller than the requested memory chunk though. If it happens, we'll happily pass back a memory region smaller than expected. Which will lead to problems eventually. A normal fetch may add/update a dozen new refs. Let's stay on the "reasonably extreme" side and say we need 16k refs (or bits from paint_alloc's perspective). Each chunk of memory would be 2k, much smaller than the memory pool (512k). So, normally, the under-allocation situation should never happen. A bad guy, however, could make a fetch that adds more than 4m new/updated refs to this code which results in a memory chunk larger than pool size. Check this case and abort. Noticed-by: Rasmus Villemoes <rv@rasmusvillemoes.dk> Reviewed-by: Jeff King <peff@peff.net> [1] Details are in commit message of 58babff (shallow.c: the 8 steps to select new commits for .git/shallow - 2013-12-05), step 6. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | shallow.c: stop abusing COMMIT_SLAB_SIZE for paint_info's memory poolsNguyễn Thái Ngọc Duy2016-12-07
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We need to allocate a "big" block of memory in paint_alloc(). The exact size does not really matter. But the pool size has no relation with commit-slab. Stop using that macro here. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | shallow.c: rename fields in paint_info to better express their purposesNguyễn Thái Ngọc Duy2016-12-07
| | |_|/ / / / / / / / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | paint_alloc() is basically malloc(), tuned for allocating a fixed number of bits on every call without worrying about freeing any individual allocation since all will be freed at the end. It does it by allocating a big block of memory every time it runs out of "free memory". "slab" is a poor choice of name, at least poorer than "pool". Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | | | | | | | | Merge branch 'sb/sequencer-abort-safety' into maintJunio C Hamano2017-01-17
|\ \ \ \ \ \ \ \ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unlike "git am --abort", "git cherry-pick --abort" moved HEAD back to where cherry-pick started while picking multiple changes, when the cherry-pick stopped to ask for help from the user, and the user did "git reset --hard" to a different commit in order to re-attempt the operation. * sb/sequencer-abort-safety: Revert "sequencer: remove useless get_dir() function" sequencer: remove useless get_dir() function sequencer: make sequencer abort safer t3510: test that cherry-pick --abort does not unsafely change HEAD am: change safe_to_abort()'s not rewinding error into a warning am: fix filename in safe_to_abort() error message
| * | | | | | | | | | | | Revert "sequencer: remove useless get_dir() function"Junio C Hamano2016-12-14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This reverts commit 39784cd3620cc47415c9010ec58a9616f040125c. The function had only one caller when the "remove useless" was written, but another topic will soon make heavy use of it and more importantly the function will return different paths depending on the value in opts.
| * | | | | | | | | | | | sequencer: remove useless get_dir() functionStephan Beyer2016-12-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This function is used only once, for the removal of the directory. It is not used for the creation of the directory nor anywhere else. Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | sequencer: make sequencer abort saferStephan Beyer2016-12-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In contrast to "git am --abort", a sequencer abort did not check whether the current HEAD is the one that is expected. This can lead to loss of work (when not spotted and resolved using reflog before the garbage collector chimes in). This behavior is now changed by mimicking "git am --abort". The abortion is done but HEAD is not changed when the current HEAD is not the expected HEAD. A new file "sequencer/abort-safety" is added to save the expected HEAD. The new behavior is only active when --abort is invoked on multiple picks. The problem does not occur for the single-pick case because it is handled differently. Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | t3510: test that cherry-pick --abort does not unsafely change HEADStephan Beyer2016-12-09
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | am: change safe_to_abort()'s not rewinding error into a warningStephan Beyer2016-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The error message tells the user that something went terribly wrong and the --abort could not be performed. But the --abort is performed, only without rewinding. By simply changing the error into a warning, we indicate the user that she must not try something like "git am --abort --force", instead she just has to check the HEAD. Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | | | | | | | | am: fix filename in safe_to_abort() error messageStephan Beyer2016-12-08
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Signed-off-by: Stephan Beyer <s-beyer@gmx.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>