1
0
Fork 0
mirror of https://github.com/git/git.git synced 2024-10-31 14:27:54 +01:00
Commit graph

92 commits

Author SHA1 Message Date
Junio C Hamano
0e6450039a Merge branch 'ma/t1400-undebug-test'
Test fix.

* ma/t1400-undebug-test:
  t1400: drop debug `echo` to actually execute `test`
2018-10-16 16:16:06 +09:00
Junio C Hamano
d555663f16 Merge branch 'bc/hash-independent-tests'
Various tests have been updated to make it easier to swap the
hash function used for object identification.

* bc/hash-independent-tests:
  t5318: use test_oid for HASH_LEN
  t1407: make hash size independent
  t1406: make hash-size independent
  t1405: make hash size independent
  t1400: switch hard-coded object ID to variable
  t1006: make hash size independent
  t0064: make hash size independent
  t0002: abstract away SHA-1 specific constants
  t0000: update tests for SHA-256
  t0000: use hash translation table
  t: add test functions to translate hash-related values
2018-10-10 12:37:16 +09:00
Martin Ågren
fc0503b04e t1400: drop debug echo to actually execute test
Instead of running `test "foo" = "$(bar)"`, we prefix the whole thing
with `echo`. Comparing to nearby tests makes it clear that this is just
debug leftover. This line has actually been modified four times since it
was introduced in e52290428b (General ref log reading improvements.,
2006-05-19) and the `echo` has always survived. Let's finally drop it.

This script could need some more cleanups. This is just an immediate fix
so that we actually test what we intend to.

All other hits for `git grep "\<echo test " -- t/` seem fine. They want
to create some input or expected output data.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-28 11:45:31 -07:00
brian m. carlson
44171e5bda t1400: switch hard-coded object ID to variable
Switch a hard-coded all-zeros object ID to use a variable instead.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-17 08:10:32 -07:00
Elijah Newren
d345e9fbe7 update-ref: allow --no-deref with --stdin
If passed both --no-deref and --stdin, update-ref would error out with a
general usage message that did not at all suggest these options were
incompatible.  The manpage for update-ref did suggest through its
synopsis line that --no-deref and --stdin were incompatible, but it sadly
also incorrectly suggested that -d and --no-deref were incompatible.  So
the help around the --no-deref option is buggy in a few ways.

The --stdin option did provide a different mechanism for avoiding
dereferencing symbolic-refs: adding a line reading
  option no-deref
before every other directive in the input.  (Technically, if the user
wants to do the extra work of first determining which refs they want to
update or delete are symbolic, then they only need to put the extra
"option no-deref" lines before the updates of those refs.  But in some
cases, that's more work than just adding the "option no-deref" before
every other directive.)

It's easier to allow the user to just pass --no-deref along with --stdin
in order to tell update-ref that the user doesn't want any symbolic ref
to be dereferenced.  It also makes the update-ref documentation simpler.
Implement that, and update the documentation to match.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-12 15:17:17 -07:00
Nguyễn Thái Ngọc Duy
661558f0a5 refs.c: mark more strings for translation
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-23 11:19:10 -07:00
Nguyễn Thái Ngọc Duy
1a07e59c3e Update messages in preparation for i18n
Many messages will be marked for translation in the following
commits. This commit updates some of them to be more consistent and
reduce diff noise in those commits. Changes are

- keep the first letter of die(), error() and warning() in lowercase
- no full stop in die(), error() or warning() if it's single sentence
  messages
- indentation
- some messages are turned to BUG(), or prefixed with "BUG:" and will
  not be marked for i18n
- some messages are improved to give more information
- some messages are broken down by sentence to be i18n friendly
  (on the same token, combine multiple warning() into one big string)
- the trailing \n is converted to printf_ln if possible, or deleted
  if not redundant
- errno_errno() is used instead of explicit strerror()

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-23 11:19:09 -07:00
Junio C Hamano
9472b13201 Merge branch 'bc/hash-independent-tests'
Many tests hardcode the raw object names, which would change once
we migrate away from SHA-1.  While some of them must test against
exact object names, most of them do not have to use hardcoded
constants in the test.  The latter kind of tests have been updated
to test the moral equivalent of the original without hardcoding the
actual object names.

* bc/hash-independent-tests: (28 commits)
  t5300: abstract away SHA-1-specific constants
  t4208: abstract away SHA-1-specific constants
  t4045: abstract away SHA-1-specific constants
  t4042: abstract away SHA-1-specific constants
  t4205: sort log output in a hash-independent way
  t/lib-diff-alternative: abstract away SHA-1-specific constants
  t4030: abstract away SHA-1-specific constants
  t4029: abstract away SHA-1-specific constants
  t4029: fix test indentation
  t4022: abstract away SHA-1-specific constants
  t4020: abstract away SHA-1-specific constants
  t4014: abstract away SHA-1-specific constants
  t4008: abstract away SHA-1-specific constants
  t4007: abstract away SHA-1-specific constants
  t3905: abstract away SHA-1-specific constants
  t3702: abstract away SHA-1-specific constants
  t3103: abstract away SHA-1-specific constants
  t2203: abstract away SHA-1-specific constants
  t: skip pack tests if not using SHA-1
  t4044: skip test if not using SHA-1
  ...
2018-05-30 21:51:28 +09:00
brian m. carlson
8125a58b91 t: switch $_z40 to $ZERO_OID
Switch all uses of $_z40 to $ZERO_OID so that they work correctly with
larger hashes.  This commit was created by using the following sed
command to modify all files in the t directory except t/test-lib.sh:

  sed -i 's/\$_z40/$ZERO_OID/g'

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-14 11:02:00 +09:00
Martin Ågren
db0210d445 refs: handle zero oid for pseudorefs
According to the documentation, it is possible to "specify 40 '0' or an
empty string as <oldvalue> to make sure that the ref you are creating
does not exist." But in the code for pseudorefs, we do not implement
this, as demonstrated by the failing tests added in the previous commit.
If we fail to read the old ref, we immediately die. But a failure to
read would actually be a good thing if we have been given the zero oid.

With the zero oid, allow -- and even require -- the ref-reading to fail.
This implements the "make sure that the ref ... does not exist" part of
the documentation and fixes both failing tests from the previous commit.

Since we have a `strbuf err` for collecting errors, let's use it and
signal an error to the caller instead of dying hard.

Reported-by: Rafael Ascensão <rafa.almas@gmail.com>
Helped-by: Rafael Ascensão <rafa.almas@gmail.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-13 10:36:11 +09:00
Martin Ågren
65eb8fc344 t1400: add tests around adding/deleting pseudorefs
I have not been able to find any tests around adding pseudorefs using
`git update-ref`. Add some as outlined in this table (original design by
Michael Haggerty; modified and extended by me):

Pre-update value   | ref-update old OID   | Expected result
-------------------|----------------------|----------------
missing            | value                | reject
missing            | none given           | accept
set                | none given           | accept
set                | correct value        | accept
set                | wrong value          | reject
missing            | zero                 | accept *
set                | zero                 | reject *

The tests marked with a * currently fail, despite git-update-ref(1)
claiming that it is possible to "specify 40 '0' or an empty string as
<oldvalue> to make sure that the ref you are creating does not exist."
These failing tests will be fixed in the next commit.

It is only natural to test deletion as well. Test deletion without an
old OID, with a correct one and with an incorrect one.

Suggested-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-13 10:36:11 +09:00
Ramsay Jones
21dac1deee test-lib: don't use ulimit in test prerequisites on cygwin
On cygwin (and MinGW), the 'ulimit' built-in bash command does not have
the desired effect of limiting the resources of new processes, at least
for the stack and file descriptors. However, it always returns success
and leads to several test prerequisites being erroneously set to true.

Add a check for cygwin and MinGW to the prerequisite expressions, using
a 'test_have_prereq !MINGW,!CYGWIN' clause, to guard against using ulimit.
This affects the prerequisite expressions for the ULIMIT_STACK_SIZE,
CMDLINE_LIMIT and ULIMIT_FILE_DESCRIPTORS prerequisites.

Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-15 11:52:00 +09:00
Kyle Meyer
1cec9c2db2 t1400: use consistent style for test_expect_success calls
Structure calls as

    test_expect_success 'description' '
    	body
    '

Use double quotes for the description if it requires parameter
expansion or contains a single quote.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-16 23:42:59 -07:00
Kyle Meyer
2405ed07ca t1400: use test_when_finished for cleanup
Move cleanup lines that occur after test blocks into
test_when_finished calls within the test bodies.  Don't move cleanup
lines that seem to be related to mutiple tests rather than a single
test.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 10:50:17 -07:00
Kyle Meyer
e063e5d7da t1400: remove a set of unused output files
This test case redirects stdout and stderr to output files, but,
unlike the other cases of redirection in the t1400 tests, these files
are not examined downstream.  Remove the redirection so that the
output is visible when running the tests verbosely.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 10:50:16 -07:00
Kyle Meyer
cd81af4bd7 t1400: use test_path_is_* helpers
Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 10:50:16 -07:00
Kyle Meyer
3e5584ca61 t1400: set core.logAllRefUpdates in "logged by touch" tests
A group of update-ref tests verifies that logs are created when either
the log file for the ref already exists or core.logAllRefUpdates is
"true".  However, when the default for core.logAllRefUpdates was
changed in 0bee59186 (Enable reflogs by default in any repository with
a working directory., 2006-12-14), the setup for the tests was not
updated.  As a result, the "logged by touch" tests would pass even if
the log file did not exist (i.e., if "--create-reflog" was removed
from the first "git update-ref" call).

Update the "logged by touch" tests to disable core.logAllRefUpdates
explicitly so that the behavior does not depend on the default value.
While we're here, update the "logged by config" tests to use
test_config() rather than setting core.logAllRefUpdates to "true"
outside of the tests.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 10:50:16 -07:00
Kyle Meyer
f950e89fc4 t1400: rename test descriptions to be unique
A few tests share their description with another test.  Extend the
descriptions to indicate how the tests differ.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-21 10:50:16 -07:00
Junio C Hamano
c13c783c9d Merge branch 'km/delete-ref-reflog-message'
"git update-ref -d" and other operations to delete references did
not leave any entry in HEAD's reflog when the reference being
deleted was the current branch.  This is not a problem in practice
because you do not want to delete the branch you are currently on,
but caused renaming of the current branch to something else not to
be logged in a useful way.

* km/delete-ref-reflog-message:
  branch: record creation of renamed branch in HEAD's log
  rename_ref: replace empty message in HEAD's log
  update-ref: pass reflog message to delete_ref()
  delete_ref: accept a reflog message argument
2017-02-27 13:57:18 -08:00
Junio C Hamano
3ad8b5bf26 Merge branch 'mh/ref-remove-empty-directory'
Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
once there no longer is any other branch whose name begins with
"foo/", but we didn't do so so far.  Now we do.

* mh/ref-remove-empty-directory: (23 commits)
  files_transaction_commit(): clean up empty directories
  try_remove_empty_parents(): teach to remove parents of reflogs, too
  try_remove_empty_parents(): don't trash argument contents
  try_remove_empty_parents(): rename parameter "name" -> "refname"
  delete_ref_loose(): inline function
  delete_ref_loose(): derive loose reference path from lock
  log_ref_write_1(): inline function
  log_ref_setup(): manage the name of the reflog file internally
  log_ref_write_1(): don't depend on logfile argument
  log_ref_setup(): pass the open file descriptor back to the caller
  log_ref_setup(): improve robustness against races
  log_ref_setup(): separate code for create vs non-create
  log_ref_write(): inline function
  rename_tmp_log(): improve error reporting
  rename_tmp_log(): use raceproof_create_file()
  lock_ref_sha1_basic(): use raceproof_create_file()
  lock_ref_sha1_basic(): inline constant
  raceproof_create_file(): new function
  safe_create_leading_directories(): set errno on SCLD_EXISTS
  safe_create_leading_directories_const(): preserve errno
  ...
2017-02-27 13:57:12 -08:00
Kyle Meyer
de922669ab update-ref: pass reflog message to delete_ref()
Now that delete_ref() accepts a reflog message, pass the user-provided
message to delete_ref() rather than silently dropping it.

Signed-off-by: Kyle Meyer <kyle@kyleam.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-20 22:04:47 -08:00
Cornelius Weig
b1421a43d5 update-ref: add test cases for bare repository
The default behavior of update-ref to create reflogs differs in
repositories with worktree and bare ones. The existing tests cover only
the behavior of repositories with worktree.

This commit adds tests that assert the correct behavior in bare
repositories for update-ref. Two cases are covered:

 - If core.logAllRefUpdates is not set, no reflogs should be created
 - If core.logAllRefUpdates is true, reflogs should be created

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-31 10:01:24 -08:00
Cornelius Weig
341fb28621 refs: add option core.logAllRefUpdates = always
When core.logallrefupdates is true, we only create a new reflog for refs
that are under certain well-known hierarchies. The reason is that we
know that some hierarchies (like refs/tags) are not meant to change, and
that unknown hierarchies might not want reflogs at all (e.g., a
hypothetical refs/foo might be meant to change often and drop old
history immediately).

However, sometimes it is useful to override this decision and simply log
for all refs, because the safety and audit trail is more important than
the performance implications of keeping the log around.

This patch introduces a new "always" mode for the core.logallrefupdates
option which will log updates to everything under refs/, regardless
where in the hierarchy it is (we still will not log things like
ORIG_HEAD and FETCH_HEAD, which are known to be transient).

Based-on-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-31 10:01:24 -08:00
Michael Haggerty
446397774a files_transaction_commit(): clean up empty directories
When deleting/pruning references, remove any directories that are made
empty by the deletion of loose references or of reflogs. Otherwise such
empty directories can survive forever and accumulate over time. (Even
'pack-refs', which is smart enough to remove the parent directories of
loose references that it prunes, leaves directories that were already
empty.)

And now that files_transaction_commit() takes care of deleting the
parent directories of loose references that it prunes, we don't have to
do that in prune_ref() anymore.

This change would be unwise if the *creation* of these directories could
race with our deletion of them. But the earlier changes in this patch
series made the creation paths robust against races, so now it is safe
to tidy them up more aggressively.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 19:30:09 -08:00
Junio C Hamano
6b34ce90a7 Merge branch 'mh/split-under-lock'
Further preparatory work on the refs API before the pluggable
backend series can land.

* mh/split-under-lock: (33 commits)
  lock_ref_sha1_basic(): only handle REF_NODEREF mode
  commit_ref_update(): remove the flags parameter
  lock_ref_for_update(): don't resolve symrefs
  lock_ref_for_update(): don't re-read non-symbolic references
  refs: resolve symbolic refs first
  ref_transaction_update(): check refname_is_safe() at a minimum
  unlock_ref(): move definition higher in the file
  lock_ref_for_update(): new function
  add_update(): initialize the whole ref_update
  verify_refname_available(): adjust constness in declaration
  refs: don't dereference on rename
  refs: allow log-only updates
  delete_branches(): use resolve_refdup()
  ref_transaction_commit(): correctly report close_ref() failure
  ref_transaction_create(): disallow recursive pruning
  refs: make error messages more consistent
  lock_ref_sha1_basic(): remove unneeded local variable
  read_raw_ref(): move docstring to header file
  read_raw_ref(): improve docstring
  read_raw_ref(): rename symref argument to referent
  ...
2016-07-25 14:13:32 -07:00
Vasco Almeida
1edbaac3bb tests: use test_i18n* functions to suppress false positives
The test functions test_i18ncmp and test_i18ngrep pretend success if run
under GETTEXT_POISON. By using those functions to test output which is
correctly marked as translatable, enables one to detect if the strings
newly marked for translation are from plumbing output. If they are
indeed from plumbing, the test would fail, and the string should be
unmarked, since it is not seen by users.

Thus, it is productive to not have false positives when running the test
under GETTEXT_POISON. This commit replaces normal test functions by
their i18n aware variants in use-cases know to be correctly marked for
translation, suppressing false positives.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 15:45:48 -07:00
Michael Haggerty
92b1551b1d refs: resolve symbolic refs first
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
2016-06-13 11:23:50 +02:00
Michael Haggerty
8a679de6f1 ref_transaction_update(): check refname_is_safe() at a minimum
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>
2016-06-13 11:23:50 +02:00
Michael Haggerty
0568c8e9dc refs: make error messages more consistent
* 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>
2016-06-13 11:23:49 +02:00
David Turner
ce414b33ec refs: make refs/bisect/* per-worktree
We need the place we stick refs for bisects in progress to not be
shared between worktrees.  So we make the refs/bisect/ hierarchy
per-worktree.

The is_per_worktree_ref function and associated docs learn that
refs/bisect/ is per-worktree, as does the git_path code in path.c

The ref-packing functions learn that per-worktree refs should not be
packed (since packed-refs is common rather than per-worktree).

Since refs/bisect is per-worktree, logs/refs/bisect should be too.

Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-01 10:37:39 -07:00
Junio C Hamano
0188f32304 Merge branch 'dt/reflog-tests'
Tests that assume how reflogs are represented on the filesystem too
much have been corrected.

* dt/reflog-tests:
  tests: remove some direct access to .git/logs
  t/t7509: remove unnecessary manipulation of reflog
2015-08-12 14:09:56 -07:00
David Turner
d0ab058498 tests: remove some direct access to .git/logs
Alternate refs backends might store reflogs somewhere other than
.git/logs.  Change most test code that directly accesses .git/logs to
instead use git reflog commands.

There are still a few tests which need direct access to reflogs: to
check reflog permissions, to manually create reflogs from scratch, to
save/restore reflogs, to check the format of raw reflog data, and to
remove not just reflog contents, but the reflogs themselves. All cases
which don't need direct access have been modified.

Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-07-28 11:46:46 -07:00
David Turner
144c76fa39 update-ref and tag: add --create-reflog arg
Allow the creation of a ref (e.g. stash) with a reflog already in
place. For most refs (e.g. those under refs/heads), this happens
automatically, but for others, we need this option.

Currently, git does this by pre-creating the reflog, but alternate ref
backends might store reflogs somewhere other than .git/logs.  Code
that now directly manipulates .git/logs should instead use git
plumbing commands.

I also added --create-reflog to git tag, just for completeness.

In a moment, we will use this argument to make git stash work with
alternate ref backends.

Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-07-21 14:08:35 -07:00
Junio C Hamano
7c997bcbf6 Merge branch 'mh/write-refs-sooner-2.4' into maint
Multi-ref transaction support we merged a few releases ago
unnecessarily kept many file descriptors open, risking to fail with
resource exhaustion.  This is for 2.4.x track.

* mh/write-refs-sooner-2.4:
  ref_transaction_commit(): fix atomicity and avoid fd exhaustion
  ref_transaction_commit(): remove the local flags variable
  ref_transaction_commit(): inline call to write_ref_sha1()
  rename_ref(): inline calls to write_ref_sha1() from this function
  commit_ref_update(): new function, extracted from write_ref_sha1()
  write_ref_to_lockfile(): new function, extracted from write_ref_sha1()
  t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE
  update-ref: test handling large transactions properly
  ref_transaction_commit(): fix atomicity and avoid fd exhaustion
  ref_transaction_commit(): remove the local flags variable
  ref_transaction_commit(): inline call to write_ref_sha1()
  rename_ref(): inline calls to write_ref_sha1() from this function
  commit_ref_update(): new function, extracted from write_ref_sha1()
  write_ref_to_lockfile(): new function, extracted from write_ref_sha1()
  t7004: rename ULIMIT test prerequisite to ULIMIT_STACK_SIZE
  update-ref: test handling large transactions properly
2015-06-05 12:00:17 -07:00
Michael Haggerty
c2e0a718c6 ref_transaction_commit(): do not capitalize error messages
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>
2015-05-27 15:58:42 -07:00
Junio C Hamano
faa4b2ecbb Merge branch 'mh/ref-directory-file'
The ref API did not handle cases where 'refs/heads/xyzzy/frotz' is
removed at the same time as 'refs/heads/xyzzy' is added (or vice
versa) very well.

* mh/ref-directory-file:
  reflog_expire(): integrate lock_ref_sha1_basic() errors into ours
  ref_transaction_commit(): delete extra "the" from error message
  ref_transaction_commit(): provide better error messages
  rename_ref(): integrate lock_ref_sha1_basic() errors into ours
  lock_ref_sha1_basic(): improve diagnostics for ref D/F conflicts
  lock_ref_sha1_basic(): report errors via a "struct strbuf *err"
  verify_refname_available(): report errors via a "struct strbuf *err"
  verify_refname_available(): rename function
  refs: check for D/F conflicts among refs created in a transaction
  ref_transaction_commit(): use a string_list for detecting duplicates
  is_refname_available(): use dirname in first loop
  struct nonmatching_ref_data: store a refname instead of a ref_entry
  report_refname_conflict(): inline function
  entry_matches(): inline function
  is_refname_available(): convert local variable "dirname" to strbuf
  is_refname_available(): avoid shadowing "dir" variable
  is_refname_available(): revamp the comments
  t1404: new tests of ref D/F conflicts within transactions
2015-05-22 12:41:53 -07:00
Michael Haggerty
cf018ee0cd ref_transaction_commit(): fix atomicity and avoid fd exhaustion
The old code was roughly

    for update in updates:
        acquire locks and check old_sha
    for update in updates:
        if changing value:
            write_ref_to_lockfile()
            commit_ref_update()
    for update in updates:
        if deleting value:
            unlink()
    rewrite packed-refs file
    for update in updates:
        if reference still locked:
            unlock_ref()

This has two problems.

Non-atomic updates
==================

The atomicity of the reference transaction depends on all pre-checks
being done in the first loop, before any changes have started being
committed in the second loop. The problem is that
write_ref_to_lockfile() (previously part of write_ref_sha1()), which
is called from the second loop, contains two more checks:

* It verifies that new_sha1 is a valid object

* If the reference being updated is a branch, it verifies that
  new_sha1 points at a commit object (as opposed to a tag, tree, or
  blob).

If either of these checks fails, the "transaction" is aborted during
the second loop. But this might happen after some reference updates
have already been permanently committed. In other words, the
all-or-nothing promise of "git update-ref --stdin" could be violated.

So these checks have to be moved to the first loop.

File descriptor exhaustion
==========================

The old code locked all of the references in the first loop, leaving
all of the lockfiles open until later loops. Since we might be
updating a lot of references, this could result in file descriptor
exhaustion.

The solution
============

After this patch, the code looks like

    for update in updates:
        acquire locks and check old_sha
        if changing value:
            write_ref_to_lockfile()
        else:
            close_ref()
    for update in updates:
        if changing value:
            commit_ref_update()
    for update in updates:
        if deleting value:
            unlink()
    rewrite packed-refs file
    for update in updates:
        if reference still locked:
            unlock_ref()

This fixes both problems:

1. The pre-checks in write_ref_to_lockfile() are now done in the first
   loop, before any changes have been committed. If any of the checks
   fails, the whole transaction can now be rolled back correctly.

2. All lockfiles are closed in the first loop immediately after they
   are created (either by write_ref_to_lockfile() or by close_ref()).
   This means that there is never more than one open lockfile at a
   time, preventing file descriptor exhaustion.

To simplify the bookkeeping across loops, add a new REF_NEEDS_COMMIT
bit to update->flags, which keeps track of whether the corresponding
lockfile needs to be committed, as opposed to just unlocked. (Since
"struct ref_update" is internal to the refs module, this change is not
visible to external callers.)

This change fixes two tests in t1400.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-12 21:28:03 -07:00
Stefan Beller
d415ad022d update-ref: test handling large transactions properly
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-05-12 21:28:02 -07:00
Michael Haggerty
3553944aa8 ref_transaction_commit(): delete extra "the" from error message
While we are in the area, let's remove a superfluous definite article
from the error message that is emitted when the reference cannot be
locked. This improves how it reads and makes it a bit shorter.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
2015-05-11 11:50:20 -07:00
Michael Haggerty
0e729c7ed5 update-ref: fix "verify" command with missing <oldvalue>
If "git update-ref --stdin" was given a "verify" command with no
"<newvalue>" at all (not even zeros), the code was mistakenly setting
have_old=0 (and leaving old_sha1 uninitialized). But this is
incorrect: this command is supposed to verify that the reference
doesn't exist. So in this case we really need old_sha1 to be set to
null_sha1 and have_old to be set to 1.

Moreover, since have_old was being set to zero, *no* check of the old
value was being done, so the new value of the reference was being set
unconditionally to the value in new_sha1. new_sha1, in turn, was set
to null_sha1 in the expectation that that was the old value and it
shouldn't be changed. But because the precondition was not being
checked, the result was that the reference was being deleted
unconditionally.

So, if <oldvalue> is missing, set have_old unconditionally and set
old_sha1 to null_sha1.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Acked-by: Brad King <brad.king@kitware.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-12-11 11:56:53 -08:00
Michael Haggerty
a46e41fca3 t1400: add some more tests of "update-ref --stdin"'s verify command
Two of the tests fail because

    verify refs/heads/foo

with no argument (not even zeros) actually *deletes* refs/heads/foo.
This problem will be fixed in the next commit.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-12-11 11:56:26 -08:00
Ronnie Sahlberg
8159f4af7d test: put tests for handling of bad ref names in one place
There's no straightforward way to grep for all tests dealing with
invalid refs.  Put them in a single test script so it is easy to see
what functionality has not been exercised with bad ref names yet.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-15 10:47:25 -07:00
Jonathan Nieder
62a2d52514 branch -d: avoid repeated symref resolution
If a repository gets in a broken state with too much symref nesting,
it cannot be repaired with "git branch -d":

 $ git symbolic-ref refs/heads/nonsense refs/heads/nonsense
 $ git branch -d nonsense
 error: branch 'nonsense' not found.

Worse, "git update-ref --no-deref -d" doesn't work for such repairs
either:

 $ git update-ref -d refs/heads/nonsense
 error: unable to resolve reference refs/heads/nonsense: Too many levels of symbolic links

Fix both by teaching resolve_ref_unsafe a new RESOLVE_REF_NO_RECURSE
flag and passing it when appropriate.

Callers can still read the value of a symref (for example to print a
message about it) with that flag set --- resolve_ref_unsafe will
resolve one level of symrefs and stop there.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Reviewed-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-10-15 10:47:25 -07:00
Junio C Hamano
bb0ced7581 Merge branch 'rs/read-ref-at'
* rs/read-ref-at:
  refs.c: change read_ref_at to use the reflog iterators
2014-06-16 12:18:48 -07:00
Junio C Hamano
2cc70cefdd Merge branch 'mh/ref-transaction'
Update "update-ref --stdin [-z]" and then introduce a transactional
support for (multi-)reference updates.

* mh/ref-transaction: (27 commits)
  ref_transaction_commit(): work with transaction->updates in place
  struct ref_update: add a type field
  struct ref_update: add a lock field
  ref_transaction_commit(): simplify code using temporary variables
  struct ref_update: store refname as a FLEX_ARRAY
  struct ref_update: rename field "ref_name" to "refname"
  refs: remove API function update_refs()
  update-ref --stdin: reimplement using reference transactions
  refs: add a concept of a reference transaction
  update-ref --stdin: harmonize error messages
  update-ref --stdin: improve the error message for unexpected EOF
  t1400: test one mistake at a time
  update-ref --stdin -z: deprecate interpreting the empty string as zeros
  update-ref.c: extract a new function, parse_next_sha1()
  t1400: test that stdin -z update treats empty <newvalue> as zeros
  update-ref --stdin: simplify error messages for missing oldvalues
  update-ref --stdin: make error messages more consistent
  update-ref --stdin: improve error messages for invalid values
  update-ref.c: extract a new function, parse_refname()
  parse_cmd_verify(): copy old_sha1 instead of evaluating <oldvalue> twice
  ...
2014-06-03 12:06:41 -07:00
Ronnie Sahlberg
4207ed285f refs.c: change read_ref_at to use the reflog iterators
read_ref_at has its own parsing of the reflog file for no really good reason
so lets change this to use the existing reflog iterators. This removes one
instance where we manually unmarshall the reflog file format.

Remove the now redundant ref_msg function.

Log messages for errors are changed slightly. We no longer print the file
name for the reflog, instead we refer to it as 'Log for ref <refname>'.
This might be a minor useability regression, but I don't really think so, since
experienced users would know where the log is anyway and inexperienced users
would not know what to do about/how to repair 'Log ... has gap ...' anyway.

Adapt the t1400 test to handle the change in log messages.

Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-06-03 11:09:32 -07:00
Michael Haggerty
f11b09fb60 update-ref --stdin: harmonize error messages
Make (most of) the error messages for invalid input have the same
format [1]:

    $COMMAND [SP $REFNAME]: $MESSAGE

Update the tests accordingly.

[1] A few error messages are left with their old form, because
    $COMMAND and $REFNAME aren't passed all the way down the call
    stack.  Maybe those sites should be changed some day, too.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-07 12:09:14 -07:00
Michael Haggerty
726f69166f update-ref --stdin: improve the error message for unexpected EOF
Distinguish this error from the error that an argument is missing for
another reason.  Update the tests accordingly.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-07 12:09:14 -07:00
Michael Haggerty
ff6ee39525 t1400: test one mistake at a time
This case wants to test passing a bad refname to the "update" command.
But it also passes too few arguments to "update", which muddles the
situation: which error should be diagnosed?  So split this test into
two:

* One that passes too few arguments to update

* One that passes all three arguments to "update", but with a bad
  refname.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-07 12:09:13 -07:00
Michael Haggerty
1fbd504942 update-ref --stdin -z: deprecate interpreting the empty string as zeros
In the original version of this command, for the single case of the
"update" command's <newvalue>, the empty string was interpreted as
being equivalent to 40 "0"s.  This shorthand is unnecessary (binary
input will usually be generated programmatically anyway), and it
complicates the parser and the documentation.

So gently deprecate this usage: remove its description from the
documentation and emit a warning if it is found.  But for reasons of
backwards compatibility, continue to accept it.

Helped-by: Brad King <brad.king@kitware.com>
Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-04-07 12:09:13 -07:00