summaryrefslogtreecommitdiff
Commit message (Collapse)AuthorAgeFilesLines
* lock_ref_for_update(): avoid a symref resolutionmh/update-ref-errorsMichael Haggerty2016-06-201-1/+1
| | | | | | | | | | | If we're overwriting a symref with a SHA-1, we need to resolve the value of the symref (1) to check against update->old_sha1 and (2) to write to its reflog. However, we've already read the symref itself and know its referent. So there is no need to read the symref's value through the symref; we can read the referent directly. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* lock_ref_for_update(): make error handling more uniformMichael Haggerty2016-06-202-39/+49
| | | | | | | | | | | To aid the effort, extract a new function, check_old_oid(), and use it in the two places where the read value of the reference has to be checked against update->old_sha1. Update tests to reflect the improvements. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t1404: add more tests of update-ref error handlingMichael Haggerty2016-06-201-2/+219
| | | | | | | Some of the error messages will be improved in subsequent commits. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t1404: document function test_update_rejectedMichael Haggerty2016-06-201-0/+10
| | | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t1404: remove "prefix" argument to test_update_rejectedMichael Haggerty2016-06-201-14/+13
| | | | | | | | | | | | | The tests already set a variable called prefix and passed its value as the first argument to this function. The old argument handling was overwriting the global variable with its same value rather than creating a local variable. So change test_update_rejected to refer to the global variable rather than taking the prefix as an argument. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* t1404: rename file to t1404-update-ref-errors.shMichael Haggerty2016-06-201-1/+1
| | | | | | | I want to broaden the scope of this test file, so rename it accordingly. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* lock_ref_sha1_basic(): only handle REF_NODEREF modemh/split-under-lockMichael Haggerty2016-06-131-34/+20
| | | | | | | | | | | | | | | | | Now lock_ref_sha1_basic() is only called with flags==REF_NODEREF. So we don't have to handle other cases anymore. This enables several simplifications, the most interesting of which come from the fact that ref_lock::orig_ref_name is now always the same as ref_lock::ref_name: * Remove ref_lock::orig_ref_name * Remove local variable orig_refname from lock_ref_sha1_basic() * ref_name can be initialize once and its value reused * commit_ref_update() never has to write to the reflog for lock->orig_ref_name Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* commit_ref_update(): remove the flags parameterMichael Haggerty2016-06-131-7/+7
| | | | | | | commit_ref_update() is now only called with flags=0. So remove the flags parameter entirely. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* lock_ref_for_update(): don't resolve symrefsMichael Haggerty2016-06-132-30/+95
| | | | | | | | | | | | | | | | | | | | | | | | | | | If a transaction includes a non-NODEREF update to a symbolic reference, we don't have to look it up in lock_ref_for_update(). The reference will be dereferenced anyway when the split-off update is processed. This change requires that we store a backpointer from the split-off update to its parent update, for two reasons: * We still want to report the original reference name in error messages. So if an error occurs when checking the split-off update's old_sha1, walk the parent_update pointers back to find the original reference name, and report that one. * We still need to write the old_sha1 of the symref to its reflog. So after we read the split-off update's reference value, walk the parent_update pointers back and fill in their old_sha1 fields. Aside from eliminating unnecessary reads, this change fixes a subtle (though not very serious) race condition: in the old code, the old_sha1 of the symref was resolved before the reference that it pointed at was locked. So it was possible that the old_sha1 value logged to the symref's reflog could be wrong if another process changed the downstream reference before it was locked. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* lock_ref_for_update(): don't re-read non-symbolic referencesMichael Haggerty2016-06-131-18/+30
| | | | | | | | | | | Before the previous patch, our first read of the reference happened before the reference was locked, so we couldn't trust its value and had to read it again. But now that our first read of the reference happens after acquiring the lock, there is no need to read it a second time. So move the read_ref_full() call into the (update->type & REF_ISSYMREF) block. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* refs: resolve symbolic refs firstMichael Haggerty2016-06-133-40/+514
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Before committing ref updates, split symbolic ref updates into two parts: an update to the underlying ref, and a log-only update to the symbolic ref. This ensures that both references are locked correctly during the transaction, including while their reflogs are updated. Similarly, if the reference pointed to by HEAD is modified directly, add a separate log-only update to HEAD, rather than leaving the job of updating HEAD's reflog to commit_ref_update(). This change ensures that HEAD is locked correctly while its reflog is being modified, as well as being cheaper (HEAD only needs to be resolved once). This makes use of a new function, lock_raw_ref(), which is analogous to read_raw_ref(), but acquires a lock on the reference before reading it. This change still has two problems: * There are redundant read_ref_full() reference lookups. * It is still possible to get incorrect reflogs for symbolic references if there is a concurrent update by another process, since the old_oid of a symref is determined before the lock on the pointed-to ref is held. Both problems will soon be fixed. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> WIP
* ref_transaction_update(): check refname_is_safe() at a minimumMichael Haggerty2016-06-133-4/+5
| | | | | | | | If the user has asked that a new value be set for a reference, we use check_refname_format() to verify that the reference name satisfies all of the rules. But in other cases, at least check that refname_is_safe(). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* unlock_ref(): move definition higher in the fileMichael Haggerty2016-06-131-10/+10
| | | | | | This avoids the need for a forward declaration in the next patch. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* lock_ref_for_update(): new functionMichael Haggerty2016-06-131-67/+85
| | | | | | | Extract a new function, lock_ref_for_update(), from ref_transaction_commit(). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* add_update(): initialize the whole ref_updateMichael Haggerty2016-06-132-22/+40
| | | | | | | | | | Change add_update() to initialize all of the fields in the new ref_update object. Rename the function to ref_transaction_add_update(), and increase its visibility to all of the refs-related code. All of this makes the function more useful for other future callers. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* verify_refname_available(): adjust constness in declarationMichael Haggerty2016-06-132-4/+4
| | | | | | The two string_list arguments can be const. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* refs: don't dereference on renameDavid Turner2016-06-132-5/+25
| | | | | | | | | | | | | | | | | | When renaming refs, don't dereference either the origin or the destination before renaming. The origin does not need to be dereferenced because it is presently forbidden to rename symbolic refs. Not dereferencing the destination fixes a bug where renaming on top of a broken symref would use the pointed-to ref name for the moved reflog. Add a test for the reflog bug. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* refs: allow log-only updatesDavid Turner2016-06-132-6/+17
| | | | | | | | | | The refs infrastructure learns about log-only ref updates, which only update the reflog. Later, we will use this to separate symbolic reference resolution from ref updating. Signed-off-by: David Turner <dturner@twopensource.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* delete_branches(): use resolve_refdup()Michael Haggerty2016-06-131-8/+11
| | | | | | | The return value of resolve_ref_unsafe() is not guaranteed to stay around as long as we need it, so use resolve_refdup() instead. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* ref_transaction_commit(): correctly report close_ref() failureMichael Haggerty2016-06-131-0/+1
| | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* ref_transaction_create(): disallow recursive pruningMichael Haggerty2016-06-133-2/+5
| | | | | | | | It is nonsensical (and a little bit dangerous) to use REF_ISPRUNING without REF_NODEREF. Forbid it explicitly. Change the one REF_ISPRUNING caller to pass REF_NODEREF too. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* refs: make error messages more consistentMichael Haggerty2016-06-133-22/+22
| | | | | | | | * Always start error messages with a lower-case letter. * Always enclose reference names in single quotes. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* lock_ref_sha1_basic(): remove unneeded local variableMichael Haggerty2016-06-131-6/+3
| | | | | | | | resolve_ref_unsafe() can cope with being called with NULL passed to its flags argument. So lock_ref_sha1_basic() can just hand its own type parameter through. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* read_raw_ref(): move docstring to header fileMichael Haggerty2016-06-132-38/+38
| | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* read_raw_ref(): improve docstringMichael Haggerty2016-06-131-17/+24
| | | | | | | Among other things, document the (important!) requirement that input refname be checked for safety before calling this function. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* read_raw_ref(): rename symref argument to referentMichael Haggerty2016-06-132-11/+12
| | | | | | | After all, it doesn't hold the symbolic reference, but rather the reference referred to. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* read_raw_ref(): clear *type at start of functionMichael Haggerty2016-06-131-0/+1
| | | | | | This is more convenient and less error-prone for callers. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* read_raw_ref(): rename flags argument to typeMichael Haggerty2016-06-132-10/+10
| | | | | | | | This will hopefully reduce confusion with the "flags" arguments that are used in many functions in this module as an input parameter to choose how the function should operate. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* ref_transaction_commit(): remove local variables n and updatesMichael Haggerty2016-06-131-22/+20
| | | | | | | | | | | These microoptimizations don't make a significant difference in speed. And they cause problems if somebody ever wants to modify the function to add updates to a transaction as part of processing it, as will happen shortly. Make the same changes in initial_ref_transaction_commit(). Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* rename_ref(): remove unneeded local variableMichael Haggerty2016-05-051-6/+3
| | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* commit_ref_update(): write error message to *err, not stderrMichael Haggerty2016-05-051-1/+1
| | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* refname_is_safe(): insist that the refname already be normalizedMichael Haggerty2016-05-051-2/+7
| | | | | | | The reference name is going to be compared to other reference names, so it should be in its normalized form. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* refname_is_safe(): don't allow the empty stringMichael Haggerty2016-05-051-2/+3
| | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* refname_is_safe(): use skip_prefix()Michael Haggerty2016-05-051-3/+5
| | | | Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* remove_dir_recursively(): add docstringMichael Haggerty2016-05-051-0/+23
| | | | | | | Add a docstring for the remove_dir_recursively() function and the REMOVE_DIR_* flags that can be passed to it. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* safe_create_leading_directories(): improve docstringMichael Haggerty2016-05-051-0/+5
| | | | | | | | Document the difference between this function and safe_create_leading_directories_const(), and that the former restores path before returning. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* read_raw_ref(): don't get confused by an empty directoryMichael Haggerty2016-05-052-2/+11
| | | | | | | | Even if there is an empty directory where we look for the loose version of a reference, check for a packed reference before giving up. This fixes the failing test that was introduced two commits ago. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* commit_ref(): if there is an empty dir in the way, delete itMichael Haggerty2016-05-051-0/+24
| | | | | | | | | | | | | | | | | | | | Part of the bug revealed in the last commit is that resolve_ref_unsafe() incorrectly returns EISDIR if it finds a directory in the place where it is looking for a loose reference, even if the corresponding packed reference exists. lock_ref_sha1_basic() notices the bogus EISDIR, and use it as an indication that it should call remove_empty_directories() and call resolve_ref_unsafe() again. But resolve_ref_unsafe() shouldn't report EISDIR in this case. If we would simply make that change, then remove_empty_directories() wouldn't get called anymore, and the empty directory would get in the way when commit_ref() calls commit_lock_file() to rename the lockfile into place. So instead of relying on lock_ref_sha1_basic() to delete empty directories, teach commit_ref(), just before calling commit_lock_file(), to check whether a directory is in the way, and if so, try to delete it. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* t1404: demonstrate a bug resolving referencesMichael Haggerty2016-05-051-1/+75
| | | | | | | | | | | | | | | Add some tests checking that it is possible to work with a reference even if there is an empty directory where the loose ref would be stored. One of the new tests demonstrates a bug that has been with us since at least 2.5.0--single reference lookup gives up when it sees the directory, even if the reference exists as a packed ref. This probably hasn't been reported before because Git usually cleans up empty directories when packing references. This bug will be fixed shortly. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
* Sync with maintJunio C Hamano2016-04-251-5/+5
|\ | | | | | | | | | | | | | | * maint: l10n: fr: don't translate "merge" as a parameter l10n: fr: change "id de clé" to match "id-clé" l10n: fr: fix wrongly translated option name l10n: fr: fix transcation of "dir"
| * Merge tag 'l10n-2.8.0-rnd3-fr' of git://github.com/git-l10n/git-po into maintJunio C Hamano2016-04-251-5/+5
| |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | l10n-2.8.0-rnd3-fr * tag 'l10n-2.8.0-rnd3-fr' of git://github.com/git-l10n/git-po: l10n: fr: don't translate "merge" as a parameter l10n: fr: change "id de clé" to match "id-clé" l10n: fr: fix wrongly translated option name l10n: fr: fix transcation of "dir"
| | * Merge branch 'fr_v2.8.0_r3' of git://github.com/jnavila/git into maintJiang Xin2016-04-241-5/+5
| | |\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | * 'fr_v2.8.0_r3' of git://github.com/jnavila/git: l10n: fr: don't translate "merge" as a parameter l10n: fr: change "id de clé" to match "id-clé" l10n: fr: fix wrongly translated option name l10n: fr: fix transcation of "dir"
| | | * Merge pull request #9 from vascool/frJean-Noël Avila2016-03-251-5/+5
| | | |\ | | | | | | | | | | Fix inconsistencies
| | | | * l10n: fr: don't translate "merge" as a parameterVasco Almeida2016-03-251-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | At builtin/checkout.c:1154, merge is a parameter to --conflict=<style> (git checkout --conflict=merge). Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
| | | | * l10n: fr: change "id de clé" to match "id-clé"Vasco Almeida2016-03-251-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | At builtin/tag.c:23 French message translation, "<key-id>" was translated to "<id-clé>", but at builtin/tag.c:355 "key-id" was translated to "id de clé", hence an inconsistency in git tag -h output. Translate "key-id" to "id-clé". Alternatively, both places could use "id de clé" instead of "id-clé". Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
| | | | * l10n: fr: fix wrongly translated option nameVasco Almeida2016-03-251-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In the original source, tags and heads refer to that options (--head and --tags) for git show-ref. Don't translate that terms, since they refer to actual option names. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
| | | | * l10n: fr: fix transcation of "dir"Vasco Almeida2016-03-251-1/+1
| | | |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "dir" was translated to the same string at builtin/log.c:1236, but, also at that code line, "<dir>" was translate to "<répertoire>". Before this commit, git format-patch -h would output "-o, --output-directory <dir>" and <répertoire> in its description. Use <répertoire> in both the parameter and description. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
* | | | Seventh batch for post 2.8 cycleJunio C Hamano2016-04-251-0/+22
| | | | | | | | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'sb/submodule-path-misc-bugs'Junio C Hamano2016-04-253-11/+133
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git submodule" reports the paths of submodules the command recurses into, but this was incorrect when the command was not run from the root level of the superproject. * sb/submodule-path-misc-bugs: t7407: make expectation as clear as possible submodule update: test recursive path reporting from subdirectory submodule update: align reporting path for custom command execution submodule status: correct path handling in recursive submodules submodule update --init: correct path handling in recursive submodules submodule foreach: correct path display in recursive submodules
| * | | | t7407: make expectation as clear as possiblesb/submodule-path-misc-bugsStefan Beller2016-03-301-2/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Not everyone (including me) grasps the sed expression in a split second as they would grasp the 4 lines printed as is. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>