summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* delete_ref(): use the usual convention for old_sha1mh/init-delete-refs-apiMichael Haggerty2015-06-224-15/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The ref_transaction_update() family of functions use the following convention for their old_sha1 parameters: * old_sha1 == NULL: Don't check the old value at all. * is_null_sha1(old_sha1): Ensure that the reference didn't exist before the transaction. * otherwise: Ensure that the reference had the specified value before the transaction. delete_ref() had a different convention, namely treating is_null_sha1(old_sha1) as "don't care". Change it to adhere to the standard convention to reduce the scope for confusion. Please note that it is now a bug to pass old_sha1=NULL_SHA1 to delete_ref() (because it doesn't make sense to delete a reference that you already know doesn't exist). This is consistent with the behavior of ref_transaction_delete(). Most of the callers of delete_ref() never pass old_sha1=NULL_SHA1 to delete_ref(), and are therefore unaffected by this change. The two exceptions are: * The call in cmd_update_ref(), which passed NULL_SHA1 if the old value passed in on the command line was 0{40} or the empty string. Change that caller to pass NULL in those cases. Arguably, it should be an error to call "update-ref -d" with the old value set to "does not exist", just as it is for the `--stdin` command "delete". But since this usage was accepted until now, continue to accept it. * The call in delete_branches(), which could pass NULL_SHA1 if deleting a broken or symbolic ref. Change it to pass NULL in these cases. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* cmd_update_ref(): make logic more straightforwardMichael Haggerty2015-06-221-3/+10
| | | | | | | | | Restructure the code to avoid clearing oldsha1 when oldval is unset. It's value is not needed in that case, so this change makes it more obvious that its initialization is consistent with its later use. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* update_ref(): don't read old reference value before deleteMichael Haggerty2015-06-221-3/+3
| | | | | | | | | | | | If we are deleting the reference, then we don't need to read the reference's old value. It doesn't provide any race safety, because the value read just before the delete is no "better" than the value that would be read under lock during the delete. And even if the reference previously didn't exist, we can call delete_ref() on it if we don't provide an old_sha1 value. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* check_branch_commit(): make first parameter constMichael Haggerty2015-06-221-1/+1
| | | | | | | | Make it clear that this function does not overwrite its first argument. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs.h: add some parameter names to function declarationsMichael Haggerty2015-06-221-11/+11
| | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: move the remaining ref module declarations to refs.hMichael Haggerty2015-06-2210-91/+109
| | | | | | | | Some functions from the refs module were still declared in cache.h. Move them to refs.h. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* initial_ref_transaction_commit(): check for ref D/F conflictsMichael Haggerty2015-06-221-0/+34
| | | | | | | | | | | | | | | | In initial_ref_transaction_commit(), check for D/F conflicts (i.e., the type of conflict that exists between "refs/foo" and "refs/foo/bar") among the references being created and between the references being created and any hypothetical existing references. Ideally, there shouldn't *be* any existing references when this function is called. But, at least in the case of the "testgit" remote helper, "clone" can be called after the remote-tracking "HEAD" and "master" branches have already been created. So let's just do the full-blown check. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* initial_ref_transaction_commit(): check for duplicate refsMichael Haggerty2015-06-221-0/+11
| | | | | | | | Error out if the ref_transaction includes more than one update for any refname. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* refs: remove some functions from the module's public interfaceMichael Haggerty2015-06-222-37/+24
| | | | | | | | | | | | | | | | | | | The following functions are no longer used from outside the refs module: * lock_packed_refs() * add_packed_ref() * commit_packed_refs() * rollback_packed_refs() So make these functions private. This is an important step, because it means that nobody outside of the refs module needs to know the difference between loose and packed references. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* initial_ref_transaction_commit(): function for initial ref creationMichael Haggerty2015-06-223-4/+75
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git clone" uses shortcuts when creating the initial set of references: * It writes them directly to packed-refs. * It doesn't lock the individual references (though it does lock the packed-refs file). * It doesn't check for refname conflicts between two new references or between one new reference and any hypothetical old ones. * It doesn't create reflog entries for the reference creations. This functionality was implemented in builtin/clone.c. But really that file shouldn't have such intimate knowledge of how references are stored. So provide a new function in the refs API, initial_ref_transaction_commit(), which can be used for initial reference creation. The new function is based on the ref_transaction interface. This means that we can make some other functions private to the refs module. That will be done in a followup commit. It would seem to make sense to add a test here that there are no existing references, because that is how the function *should* be used. But in fact, the "testgit" remote helper appears to call it *after* having set up refs/remotes/<name>/HEAD and refs/remotes/<name>/master, so we can't be so strict. For now, the function trusts its caller to only call it when it makes sense. Future commits will add some more limited sanity checks. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* repack_without_refs(): make function privateMichael Haggerty2015-06-222-12/+8
| | | | | | | | It is no longer called from outside of the refs module. Also move its docstring and change it to imperative voice. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* prune_refs(): use delete_refs()Michael Haggerty2015-06-221-8/+17
| | | | | | | | | | | | | | | | The old version just looped over the references to delete, calling delete_ref() on each one. But that has quadratic behavior, because each call to delete_ref() might have to rewrite the packed-refs file. This can be very expensive in a repository with a large number of references. In some (admittedly extreme) repositories, we've seen cases where the ref-pruning part of fetch takes multiple tens of minutes. Instead call delete_refs(), which (aside from being less code) has the optimization that it only rewrites the packed-refs file a single time. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* prune_remote(): use delete_refs()Michael Haggerty2015-06-221-9/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This slightly changes how errors are reported. The old and new code both report errors that come from repack_without_refs() the same way. But if an error occurs within delete_ref(), the old version only emitted an error within delete_ref() without further comment. The new version (in delete_refs()) still emits that error, but then follows it up with error(_("could not remove reference %s"), refname) The "could not remove reference" error originally came from a similar message in remove_branches() (from builtin/remote.c). This is an improvement, because the error from delete_ref() (which usually comes from ref_transaction_commit()) can be pretty low-level, like Cannot lock ref '%s': unable to resolve reference %s: %s where the last "%s" is the original strerror. In any case, I don't think we need to sweat the details too much, because these errors should only ever be seen in the case of a broken repository or a race between two processes; i.e., only in pretty rare and anomalous situations. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* delete_refs(): bail early if the packed-refs file cannot be rewrittenMichael Haggerty2015-06-221-3/+22
| | | | | | | | | | | | | If we fail to delete the doomed references from the packed-refs file, then it is unsafe to delete their loose references, because doing so might expose a value from the packed-refs file that is obsolete and perhaps even points at an object that has been garbage collected. So if repack_without_refs() fails, emit a more explicit error message and bail. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* delete_refs(): make error message more genericMichael Haggerty2015-06-221-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | Change the error message from Could not remove branch %s to could not remove reference %s First of all, the old error message referred to "branch refs/remotes/origin/foo", which was awkward even for the existing caller. Normally we would refer to a reference like that as either "remote-tracking branch origin/foo" or "reference refs/remotes/origin/foo". Here I take the lazier alternative. Moreover, now that this function is part of the refs API, it might be called for refs that are neither branches nor remote-tracking branches. While we're at it, convert the error message to lower case, as per our usual convention. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* delete_refs(): new function for the refs APIMichael Haggerty2015-06-223-20/+27
| | | | | | | | Move the function remove_branches() from builtin/remote.c to refs.c, rename it to delete_refs(), and make it public. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* delete_ref(): handle special case more explicitlyMichael Haggerty2015-06-221-2/+9
| | | | | | | | | | | delete_ref() uses a different convention for its old_sha1 parameter than, say, ref_transaction_delete(): NULL_SHA1 means not to check the old value. Make this fact a little bit clearer in the code by handling it in explicit, commented code rather than burying it in a conditional expression. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* remove_branches(): remove temporaryMichael Haggerty2015-06-221-2/+1
| | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* delete_ref(): move declaration to refs.hMichael Haggerty2015-06-223-4/+13
| | | | | | | | | | | | Also * Add a docstring * Rename the second parameter to "old_sha1", to be consistent with the convention used elsewhere in the refs module Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Second half of seventh batchJunio C Hamano2015-06-111-0/+28
| | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'tb/complete-sequencing'Junio C Hamano2015-06-111-0/+5
|\ | | | | | | | | | | | | | | The bash completion script (in contrib/) learned a few options that "git revert" takes. * tb/complete-sequencing: completion: suggest sequencer commands for revert
| * completion: suggest sequencer commands for reverttb/complete-sequencingThomas Braun2015-06-011-0/+5
| | | | | | | | | | | | Signed-off-by: Thomas Braun <thomas.braun@virtuell-zuhause.de> Acked-by: Ramkumar Ramachandra <artagnon@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/squelch-missing-link-warning-for-unreachable'Junio C Hamano2015-06-117-9/+34
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Recent "git prune" traverses young unreachable objects to safekeep old objects in the reachability chain from them, which sometimes caused error messages that are unnecessarily alarming. * jk/squelch-missing-link-warning-for-unreachable: suppress errors on missing UNINTERESTING links silence broken link warnings with revs->ignore_missing_links add quieter versions of parse_{tree,commit}
| * | suppress errors on missing UNINTERESTING linksjk/squelch-missing-link-warning-for-unreachableJeff King2015-06-011-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we are traversing commit parents along the UNINTERESTING side of a revision walk, we do not care if the parent turns out to be missing. That lets us limit traversals using unreachable and possibly incomplete sections of history. However, we do still print error messages about the missing commits; this patch suppresses the error, as well. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | silence broken link warnings with revs->ignore_missing_linksJeff King2015-06-013-2/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We set revs->ignore_missing_links to instruct the revision-walking machinery that we know the history graph may be incomplete. For example, we use it when walking unreachable but recent objects; we want to add what we can, but it's OK if the history is incomplete. However, we still print error messages for the missing objects, which can be confusing. This is not an error, but just a normal situation when transitioning from a repository last pruned by an older git (which can leave broken segments of history) to a more recent one (where we try to preserve whole reachable segments). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | add quieter versions of parse_{tree,commit}Jeff King2015-06-014-6/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we call parse_commit, it will complain to stderr if the object does not exist or cannot be read. This means that we may produce useless error messages if this situation is expected (e.g., because the object is marked UNINTERESTING, or because revs->ignore_missing_links is set). We can fix this by adding a new "parse_X_gently" form that takes a flag to suppress the messages. The existing "parse_X" form is already gentle in the sense that it returns an error rather than dying, and we could in theory just add a "quiet" flag to it (with existing callers passing "0"). But doing it this way means we do not have to disturb existing callers. Note also that the new flag is "quiet_on_missing", and not just "quiet". We could add a flag to suppress _all_ errors, but besides being a more invasive change (we would have to pass the flag down to sub-functions, too), there is a good reason not to: we would never want to use it. Missing a linked object is expected in some circumstances, but it is never expected to have a malformed commit, or to get a tree when we wanted a commit. We should always complain about these corruptions. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'pt/pull-tests'Junio C Hamano2015-06-112-36/+175
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add more test coverage to "git pull". * pt/pull-tests: t5520: check reflog action in fast-forward merge t5521: test --dry-run does not make any changes t5520: test --rebase failure on unborn branch with index t5520: test --rebase with multiple branches t5520: test work tree fast-forward when fetch updates head t5520: test for failure if index has unresolved entries t5520: test no merge candidates cases t5520: prevent field splitting in content comparisons
| * | | t5520: check reflog action in fast-forward mergept/pull-testsPaul Tan2015-05-291-3/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When testing a fast-forward merge with git-pull, check to see if the reflog action is "pull" with the arguments passed to git-pull. While we are in the vicinity, remove the empty line as well. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t5521: test --dry-run does not make any changesPaul Tan2015-05-291-0/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Test that when --dry-run is provided to git-pull, it does not make any changes, namely: * --dry-run gets passed to git-fetch, so no FETCH_HEAD will be created and no refs will be fetched. * The index and work tree will not be modified. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t5520: test --rebase failure on unborn branch with indexPaul Tan2015-05-291-0/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 19a7fcb (allow pull --rebase on branch yet to be born, 2009-08-11) special cases git-pull on an unborn branch in a different code path such that git-pull --rebase is still valid even though there is no HEAD yet. This code path still ensures that there is no index in order not to lose any staged changes. Implement a test to ensure that this check is triggered. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t5520: test --rebase with multiple branchesPaul Tan2015-05-291-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since rebasing on top of multiple upstream branches does not make sense, since 51b2ead (disallow providing multiple upstream branches to rebase, pull --rebase, 2009-02-18), git-pull explicitly disallowed specifying multiple branches in the rebase case. Implement tests to ensure that git-pull fails and prints out the user-friendly error message in such a case. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t5520: test work tree fast-forward when fetch updates headPaul Tan2015-05-291-0/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since b10ac50 (Fix pulling into the same branch., 2005-08-25), git-pull, upon detecting that git-fetch updated the current head, will fast-forward the working tree to the updated head commit. Implement tests to ensure that the fast-forward occurs in such a case, as well as to ensure that the user-friendly advice is printed upon failure. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t5520: test for failure if index has unresolved entriesPaul Tan2015-05-291-0/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit d38a30d (Be more user-friendly when refusing to do something because of conflict., 2010-01-12) introduced code paths to git-pull which will error out with user-friendly advices if the user is in the middle of a merge or has unmerged files. Implement tests to ensure that git-pull will not run, and will print these advices, if the user is in the middle of a merge or has unmerged files in the index. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t5520: test no merge candidates casesPaul Tan2015-05-181-0/+55
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | a8c9bef (pull: improve advice for unconfigured error case, 2009-10-05) fully established the current advices given by git-pull for the different cases where git-fetch will not have anything marked for merge: 1. We fetched from a specific remote, and a refspec was given, but it ended up not fetching anything. This is usually because the user provided a wildcard refspec which had no matches on the remote end. 2. We fetched from a non-default remote, but didn't specify a branch to merge. We can't use the configured one because it applies to the default remote, and thus the user must specify the branches to merge. 3. We fetched from the branch's or repo's default remote, but: a. We are not on a branch, so there will never be a configured branch to merge with. b. We are on a branch, but there is no configured branch to merge with. 4. We fetched from the branch's or repo's default remote, but the configured branch to merge didn't get fetched (either it doesn't exist, or wasn't part of the configured fetch refspec) Implement tests for the above 5 cases to ensure that the correct code paths are triggered for each of these cases. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | t5520: prevent field splitting in content comparisonsPaul Tan2015-05-181-35/+35
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Many tests in t5520 used the following to test the contents of files: test `cat file` = expected or test $(cat file) = expected These 2 forms, however, will be affected by field splitting and, depending on the value of $IFS, may be split into multiple arguments, making the test fail in mysterious ways. Replace the above 2 forms with: test "$(cat file)" = expected as quoting the command substitution will prevent field splitting. Signed-off-by: Paul Tan <pyokagan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'sb/glossary-submodule'Junio C Hamano2015-06-111-0/+16
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | * sb/glossary-submodule: glossary: add "remote", "submodule", "superproject"
| * | | | glossary: add "remote", "submodule", "superproject"sb/glossary-submoduleStefan Beller2015-05-291-0/+16
| | |_|/ | |/| | | | | | | | | | | | | | | | | | | | | | Noticed-by: Philip Oakley <philipoakley@iee.org> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'sb/submodule-doc-intro'Junio C Hamano2015-06-111-28/+22
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | * sb/submodule-doc-intro: submodule doc: reorder introductory paragraphs
| * | | | submodule doc: reorder introductory paragraphssb/submodule-doc-introStefan Beller2015-05-281-28/+22
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It's better to start the man page with a description of what submodules actually are, instead of saying what they are not. Reorder the paragraphs such that - the first short paragraph introduces the submodule concept, - the second paragraph highlights the usage of the submodule command, - the third paragraph giving background information, and finally - the fourth paragraph discusing alternatives such as subtrees and remotes, which we don't want to be confused with. This ordering deepens the knowledge on submodules with each paragraph. First the basic questions like "How/what" will be answered, while the underlying concepts will be taught at a later time. Making sure it is not confused with subtrees and remotes is not really enhancing knowledge of submodules itself, but rather painting the big picture of git concepts, so you could also argue to have it as the second paragraph. Personally I think this may confuse readers, specially newcomers though. Additionally to reordering the paragraphs, they have been slightly reworded. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'jk/diagnose-config-mmap-failure'Junio C Hamano2015-06-114-11/+29
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The configuration reader/writer uses mmap(2) interface to access the files; when we find a directory, it barfed with "Out of memory?". * jk/diagnose-config-mmap-failure: xmmap(): drop "Out of memory?" config.c: rewrite ENODEV into EISDIR when mmap fails config.c: avoid xmmap error messages config.c: fix mmap leak when writing config read-cache.c: drop PROT_WRITE from mmap of index
| * | | | xmmap(): drop "Out of memory?"jk/diagnose-config-mmap-failureJunio C Hamano2015-05-281-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We show that message with die_errno(), but the OS is ought to know why mmap(2) failed much better than we do. There is no reason for us to say "Out of memory?" here. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | config.c: rewrite ENODEV into EISDIR when mmap failsJeff King2015-05-281-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If we try to mmap a directory, we'll get ENODEV. This translates to "no such device" for the user, which is not very helpful. Since we've just fstat()'d the file, we can easily check whether the problem was a directory to give a better message. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | config.c: avoid xmmap error messagesJeff King2015-05-283-6/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The config-writing code uses xmmap to map the existing config file, which will die if the map fails. This has two downsides: 1. The error message is not very helpful, as it lacks any context about the file we are mapping: $ mkdir foo $ git config --file=foo some.key value fatal: Out of memory? mmap failed: No such device 2. We normally do not die in this code path; instead, we'd rather report the error and return an appropriate exit status (which is part of the public interface documented in git-config.1). This patch introduces a "gentle" form of xmmap which lets us produce our own error message. We do not want to use mmap directly, because we would like to use the other compatibility elements of xmmap (e.g., handling 0-length maps portably). The end result is: $ git.compile config --file=foo some.key value error: unable to mmap 'foo': No such device $ echo $? 3 Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | config.c: fix mmap leak when writing configJeff King2015-05-281-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | We mmap the existing config file, but fail to unmap it if we hit an error. The function already has a shared exit path, so we can fix this by moving the mmap pointer to the function scope and clearing it in the shared exit. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | read-cache.c: drop PROT_WRITE from mmap of indexJeff King2015-05-281-1/+1
| | |_|/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Once upon a time, git's in-memory representation of a cache entry actually pointed to the mmap'd on-disk data. So in 520fc24 (Allow writing to the private index file mapping., 2005-04-26), we specified PROT_WRITE so that we could tweak the entries while we run (in our own MAP_PRIVATE copy-on-write version, of course). Later, 7a51ed6 (Make on-disk index representation separate from in-core one, 2008-01-14) stopped doing this; we copy the data into our in-core representation, and then drop the mmap immediately. We can therefore drop the PROT_WRITE flag. It's probably not hurting anything as it is, but it's potentially confusing. Note that we could also mark the mapping as "const" to verify that we never write to it. However, we don't typically do that for our other maps, as it then requires casting to munmap() it. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'mt/p4-depotFile-at-version'Junio C Hamano2015-06-111-1/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | * mt/p4-depotFile-at-version: p4: retrieve the right revision of the file in UTF-16 codepath
| * | | | p4: retrieve the right revision of the file in UTF-16 codepathmt/p4-depotFile-at-versionMiguel Torroja2015-05-271-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fixing bug with UTF-16 files when they are retrieved by git-p4. It was always getting the tip version of the file and the history of the file was lost. Signed-off-by: Miguel Torroja <miguel.torroja@gmail.com> Acked-by: Luke Diamand <luke@diamand.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'mh/verify-lock-error-report'Junio C Hamano2015-06-112-21/+33
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Bring consistency to error reporting mechanism used in "refs" API. * mh/verify-lock-error-report: ref_transaction_commit(): do not capitalize error messages verify_lock(): do not capitalize error messages verify_lock(): report errors via a strbuf verify_lock(): on errors, let the caller unlock the lock verify_lock(): return 0/-1 rather than struct ref_lock *
| * | | | | ref_transaction_commit(): do not capitalize error messagesmh/verify-lock-error-reportMichael Haggerty2015-05-272-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Our convention is for error messages to start with a lower-case letter. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | verify_lock(): do not capitalize error messagesMichael Haggerty2015-05-271-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Our convention is for error messages to start with a lower-case letter. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>