Test that the am command updates the work tree as expected (for submodule
changes which don't result in conflicts). To make that work add two
helper functions that use format-patch to create the input for am.
Add the KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES
switch to expect the known failure that --no-ff merges attempt to merge
the new files in the former submodule directory with those of the removed
submodule.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test that the cherry-pick command updates the work tree as expected (for
submodule changes which don't result in conflicts).
Set KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES
and KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR to
document that cherry-pick has the same --no-ff known failures merge has.
Implement the KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT switch to expect
the known failure that while cherry picking just a SHA-1 update for an
ignored submodule the commit incorrectly fails with "The previous
cherry-pick is now empty, possibly due to conflict resolution.".
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test that the pull command updates the work tree as expected (for
submodule changes which don't result in conflicts) when used without
arguments or with the '--ff', '--ff-only' and '--no-ff' flag each. Add
helper functions to reset the branch to be updated to to the current
HEAD so that pull is doing the transition from HEAD to the given branch.
Set KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES
and KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR to
document that pull has the same --no-ff known failures merge has.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test that the rebase command updates the work tree as expected for
changes which don't result in conflicts. To make that work add two
helper functions that add a commit only touching files and then
revert it. This allows to rebase the target commit over these two
and to compare the result.
Set KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR to
document that "replace directory with submodule" fails for an
interactive rebase because a directory "sub1" already exists.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test that the merge command updates the work tree as expected (for
submodule changes which don't result in conflicts) when used without
arguments or with the '--ff', '--ff-only' and '--no-ff' flag.
Implement the KNOWN_FAILURE_NOFF_MERGE_DOESNT_CREATE_EMPTY_SUBMODULE_DIR
switch to expect the known failure that --no-ff merges do not create the
empty submodule directory.
The KNOWN_FAILURE_NOFF_MERGE_ATTEMPTS_TO_MERGE_REMOVED_SUBMODULE_FILES
switch is also implemented to expect the known failure that --no-ff
merges attempt to merge the new files in the former submodule directory
with those of the removed submodule.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test that the bisect command updates the work tree as expected. To make
that work with the new submodule test framework a git_bisect helper
function is added. This adds a commit after the one given to be switched
to and makes that one the bad commit. The starting point is then given to
bisect as the good commit which makes bisect change the work tree to the
commit in between, which is the commit given.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test that the reset command updates the work tree as expected for changes
with '--keep', '--merge' (for changes which don't result in conflicts) and
'--hard'.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test that the apply command updates the work tree as expected for the
'--index' and the '--3way' options (for submodule changes which don't
result in conflicts).
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
submodules: add the lib-submodule-update.sh test library
Add this test library to simplify covering all combinations of submodule
update scenarios without having to add those to a test of each work tree
manipulating command over and over again.
The functions test_submodule_switch() and test_submodule_forced_switch()
are intended to be called from a test script with a single argument. This
argument is either a work tree manipulating command (including any command
line options) or a function (when more than a single git command is needed
to switch work trees from the current HEAD to another commit). This
command (or function) is passed a target branch as argument. The two new
functions check that each submodule transition is handled as expected,
which currently means that submodule work trees are not affected until
"git submodule update" is called. The "forced" variant is for commands
using their '-f' or '--hard' option and expects them to overwrite local
modifications as a result. Each of these two functions contains 14
tests_expect_* calls.
Calling one of these test functions the first time creates a repository
named "submodule_update_repo". At first it contains two files, then a
single submodule is added in another commit followed by commits covering
all relevant submodule modifications. This repository is newly cloned into
the "submodule_update" for each test_expect_* to avoid interference
between different parts of the test functions (some to-be-tested commands
also manipulate refs along with the work tree, e.g. "git reset").
Follow-up commits will then call these two test functions for all work
tree manipulating commands (with a combination of all their options
relevant to what they do with the work tree) making sure they work as
expected. Later this test library will be extended to cover merges
resulting in conflicts too. Also it is intended to be easily extendable
for the recursive update functionality, where even more combinations of
submodule modifications have to be tested for.
This version documents two bugs in current Git with expected failures:
*) When a submodule is replaced with a tracked file of the same name the
submodule work tree including any local modifications (and even the
whole history if it uses a .git directory instead of a gitfile!) is
silently removed.
*) Forced work tree updates happily manipulate files in the directory of a
submodule that has just been removed in the superproject (but is of
course still present in the work tree due to the way submodules are
currently handled). This becomes dangerous when files in the submodule
directory are overwritten by files from the new superproject commit, as
any modifications to the submodule files will be lost) and is expected
to also destroy history in the - admittedly unlikely case - the new
commit adds a file named ".git" to the submodule directory.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
refs.c: change ref_transaction_update() to do error checking and return status
Update ref_transaction_update() do some basic error checking and return
non-zero on error. Update all callers to check ref_transaction_update() for
error. There are currently no conditions in _update that will return error but
there will be in the future. Add an err argument that will be updated on
failure. In future patches we will start doing both locking and checking
for name conflicts in _update instead of _commit at which time this function
will start returning errors for these conditions.
Also check for BUGs during update and die(BUG:...) if we are calling
_update with have_old but the old_sha1 pointer is NULL.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: remove the onerr argument to ref_transaction_commit
Since all callers now use QUIET_ON_ERR we no longer need to provide an onerr
argument any more. Remove the onerr argument from the ref_transaction_commit
signature.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
update-ref: use err argument to get error from ref_transaction_commit
Call ref_transaction_commit with QUIET_ON_ERR and use the strbuf that is
returned to print a log message if/after the transaction fails.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: make update_ref_write update a strbuf on failure
Change update_ref_write to also update an error strbuf on failure.
This makes the error available to ref_transaction_commit callers if the
transaction failed due to update_ref_sha1/write_ref_sha1 failures.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: make ref_update_reject_duplicates take a strbuf argument for errors
Make ref_update_reject_duplicates return any error that occurs through a
new strbuf argument. This means that when a transaction commit fails in
this function we will now be able to pass a helpful error message back to the
caller.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: log_ref_write should try to return meaningful errno
Making errno from write_ref_sha1() meaningful, which should fix
* a bug in "git checkout -b" where it prints strerror(errno)
despite errno possibly being zero or clobbered
* a bug in "git fetch"'s s_update_ref, which trusts the result of an
errno == ENOTDIR check to detect D/F conflicts
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: make resolve_ref_unsafe set errno to something meaningful on error
Making errno when returning from resolve_ref_unsafe() meaningful,
which should fix
* a bug in lock_ref_sha1_basic, where it assumes EISDIR
means it failed due to a directory being in the way
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: commit_packed_refs to return a meaningful errno on failure
Making errno when returning from commit_packed_refs() meaningful,
which should fix
* a bug in "git clone" where it prints strerror(errno) based on
errno, despite errno possibly being zero and potentially having
been clobbered by that point
* the same kind of bug in "git pack-refs"
and prepares for repack_without_refs() to get a meaningful
error message when commit_packed_refs() fails without falling into
the same bug.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: make remove_empty_directories always set errno to something sane
Making errno when returning from remove_empty_directories() more
obviously meaningful, which should provide some peace of mind for
people auditing lock_ref_sha1_basic.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: verify_lock should set errno to something meaningful
Making errno when returning from verify_lock() meaningful, which
should almost but not completely fix
* a bug in "git fetch"'s s_update_ref, which trusts the result of an
errno == ENOTDIR check to detect D/F conflicts
ENOTDIR makes sense as a sign that a file was in the way of a
directory we wanted to create. Should "git fetch" also look for
ENOTEMPTY or EEXIST to catch cases where a directory was in the way
of a file to be created?
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: make sure log_ref_setup returns a meaningful errno
Making errno when returning from log_ref_setup() meaningful,
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: add an err argument to repack_without_refs
Update repack_without_refs to take an err argument and update it if there
is a failure. Pass the err variable from ref_transaction_commit to this
function so that callers can print a meaningful error message if _commit
fails due to this function.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
lockfile.c: make lock_file return a meaningful errno on failurei
Making errno when returning from lock_file() meaningful, which should
fix
* an existing almost-bug in lock_ref_sha1_basic where it assumes
errno==ENOENT is meaningful and could waste some work on retries
* an existing bug in repack_without_refs where it prints
strerror(errno) and picks advice based on errno, despite errno
potentially being zero and potentially having been clobbered by
that point
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
lockfile.c: add a new public function unable_to_lock_message
Introducing a new unable_to_lock_message helper, which has nicer
semantics than unable_to_lock_error and cleans up lockfile.c a little.
Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: add a strbuf argument to ref_transaction_commit for error logging
Add a strbuf argument to _commit so that we can pass an error string back to
the caller. So that we can do error logging from the caller instead of from
_commit.
Longer term plan is to first convert all callers to use onerr==QUIET_ON_ERR
and craft any log messages from the callers themselves and finally remove the
onerr argument completely.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: allow passing NULL to ref_transaction_free
Allow ref_transaction_free(NULL) as a no-op. This makes ref_transaction_free
easier to use and more similar to plain 'free'.
In particular, it lets us rollback unconditionally as part of cleanup code
after setting 'transaction = NULL' if a transaction has been committed or
rolled back already.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: constify the sha arguments for ref_transaction_create|delete|update
ref_transaction_create|delete|update has no need to modify the sha1
arguments passed to it so it should use const unsigned char* instead
of unsigned char*.
Some functions, such as fast_forward_to(), already have its old/new
sha1 arguments as consts. This function will at some point need to
use ref_transaction_update() in which case this change is required.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
refs.c: ref_transaction_commit should not free the transaction
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
We do not yet need both a rollback and a free function for transactions.
Remove ref_transaction_rollback and use ref_transaction_free instead.
At a later stage we may reintroduce a rollback function if we want to start
adding reusable transactions and similar.
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Acked-by: Michael Haggerty <mhagger@alum.mit.edu>
wt-status: simplify performance measurement by using getnanotime()
Calculating duration from a single uint64_t is simpler than from a struct
timeval. Change performance measurement for 'advice.statusuoption' from
gettimeofday() to getnanotime().
Also initialize t_begin to prevent uninitialized variable warning.
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git: add performance tracing for git's main() function to debug scripts
Use trace_performance to measure and print execution time and command line
arguments of the entire main() function. In constrast to the shell's 'time'
utility, which measures total time of the parent process, this logs all
involved git commands recursively. This is particularly useful to debug
performance issues of scripted commands (i.e. which git commands were
called with which parameters, and how long did they execute).
Due to git's deliberate use of exit(), the implementation uses an atexit
routine rather than just adding trace_performance_since() at the end of
main().
Usage example: > GIT_TRACE_PERFORMANCE=~/git-trace.log git stash list
trace: add trace_performance facility to debug performance issues
Add trace_performance and trace_performance_since macros that print a
duration and an optional printf-formatted text to the file specified in
environment variable GIT_TRACE_PERFORMANCE.
These macros, in conjunction with getnanotime(), are intended to simplify
performance measurements from within the application (i.e. profiling via
manual instrumentation, rather than using an external profiling tool).
Unless enabled via GIT_TRACE_PERFORMANCE, these macros have no noticeable
impact on performance, so that test code for well known time killers may
be shipped in release builds. Alternatively, a developer could provide an
additional performance patch (not meant for master) that allows reviewers
to reproduce performance tests more easily, e.g. on other platforms or
using their own repositories.
trace: add high resolution timer function to debug performance issues
Add a getnanotime() function that returns nanoseconds since 01/01/1970 as
unsigned 64-bit integer (i.e. overflows in july 2554). This is easier to
work with than e.g. struct timeval or struct timespec. Basing the timer on
the epoch allows using the results with other time-related APIs.
To simplify adaption to different platforms, split the implementation into
a common getnanotime() and a platform-specific highres_nanos() function.
The common getnanotime() function handles errors, falling back to
gettimeofday() if highres_nanos() isn't implemented or doesn't work.
getnanotime() is also responsible for normalizing to the epoch. The offset
to the system clock is calculated only once on initialization, i.e.
manually setting the system clock has no impact on the timer (except if
the fallback gettimeofday() is in use). Git processes are typically short
lived, so we don't need to handle clock drift.
The highres_nanos() function returns monotonically increasing nanoseconds
relative to some arbitrary point in time (e.g. system boot), or 0 on
failure. Providing platform-specific implementations should be relatively
easy, e.g. adapting to clock_gettime() as defined by the POSIX realtime
extensions is seven lines of code.
This version includes highres_nanos() implementations for:
* Linux: using clock_gettime(CLOCK_MONOTONIC)
* Windows: using QueryPerformanceCounter()
Todo:
* enable clock_gettime() on more platforms
* add Mac OSX version, e.g. using mach_absolute_time + mach_timebase_info
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is useful to see where trace output came from.
Add 'const char *file, int line' parameters to the printing functions and
rename them to *_fl.
Add trace_printf* and trace_strbuf macros resolving to the *_fl functions
and let the preprocessor fill in __FILE__ and __LINE__.
As the trace_printf* functions take a variable number of arguments, this
requires variadic macros (i.e. '#define foo(...) foo_impl(__VA_ARGS__)'.
Though part of C99, it is unclear whether older compilers support this.
Thus keep the old functions and only enable variadic macros for GNUC and
MSVC 2005+ (_MSC_VER 1400). This has the nice side effect that the old
C-style declarations serve as documentation how the macros are to be used.
Print 'file:line ' as prefix to each trace line. Align the remaining trace
output at column 40 to accommodate 18 char file names + 4 digit line
number (currently there are 30 *.c files of length 18 and just 11 of 19).
Trace output from longer source files (e.g. builtin/receive-pack.c) will
not be aligned.
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This is useful to tell apart trace output of separate test runs.
It can also be used for basic, coarse-grained performance analysis. Note
that the accuracy is tainted by writing to the trace file, and you have to
calculate the deltas yourself (which is next to impossible if multiple
threads or processes are involved).
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
trace: add infrastructure to augment trace output with additional info
To be able to add a common prefix or suffix to all trace output (e.g.
a timestamp or file:line of the caller), factor out common setup and
cleanup tasks of the trace* functions.
When adding a common prefix, it makes sense that the output of each trace
call starts on a new line. Add '\n' in case the caller forgot.
Note that this explicitly limits trace output to line-by-line, it is no
longer possible to trace-print just part of a line. Until now, this was
just an implicit assumption (trace-printing part of a line worked, but
messed up the trace file if multiple threads or processes were involved).
Thread-safety / inter-process-safety is also the reason why we need to do
the prefixing and suffixing in memory rather than issuing multiple write()
calls. Write_or_whine_pipe() / xwrite() is atomic unless the size exceeds
MAX_IO_SIZE (8MB, see wrapper.c). In case of trace_strbuf, this costs an
additional string copy (which should be irrelevant for performance in light
of actual file IO).
While we're at it, rename trace_strbuf's 'buf' argument, which suggests
that the function is modifying the buffer. Trace_strbuf() currently is the
only trace API that can print arbitrary binary data (without barfing on
'%' or stopping at '\0'), so 'data' seems more appropriate.
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
sha1_file: change GIT_TRACE_PACK_ACCESS logging to use trace API
This changes GIT_TRACE_PACK_ACCESS functionality as follows:
* supports the same options as GIT_TRACE (e.g. printing to stderr)
* no longer supports relative paths
* appends to the trace file rather than overwriting
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Documentation/git.txt: improve documentation of 'GIT_TRACE*' variables
Separate GIT_TRACE description into what it prints and how to configure
where trace output is printed to. Change other GIT_TRACE_* descriptions to
refer to GIT_TRACE.
Add descriptions for GIT_TRACE_SETUP and GIT_TRACE_SHALLOW.
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The trace API currently rechecks the environment variable and reopens the
trace file on every API call. This has the ugly side effect that errors
(e.g. file cannot be opened, or the user specified a relative path) are
also reported on every call. Performance can be improved by about factor
three by remembering the environment state and keeping the file open.
Replace the 'const char *key' parameter in the API with a pointer to a
'struct trace_key' that bundles the environment variable name with
additional, trace-internal state. Change the call sites of these APIs to
use a static 'struct trace_key' instead of a string constant.
In trace.c::get_trace_fd(), save and reuse the file descriptor in 'struct
trace_key'.
Add a 'trace_disable()' API, so that packet_trace() can cleanly disable
tracing when it encounters packed data (instead of using unsetenv()).
Signed-off-by: Karsten Blees <blees@dcon.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We generally want to avoid lookup_unknown_object, because it
results in allocating more memory for the object than may be
strictly necessary.
In this case, it is used to check whether we have an
already-parsed object before calling parse_object, to save
us from reading the object from disk. Using lookup_object
would be fine for that purpose, but we can take it a step
further. Since this code was written, parse_object already
learned the "check lookup_object" optimization, so we can
simply call parse_object directly.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The point of the "index" field of struct commit is that
every allocated commit would have one. It is supposed to be
an invariant that whenever object->type is set to
OBJ_COMMIT, we have a unique index.
Commit 969eba6 (commit: push commit_index update into
alloc_commit_node, 2014-06-10) covered this case for
newly-allocated commits. However, we may also allocate an
"unknown" object via lookup_unknown_object, and only later
convert it to a commit. We must make sure that we set the
commit index when we switch the type field.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We keep a static counter to set the commit index on newly
allocated objects. However, since we also need to set the
index on any_objects which are converted to commits, let's
make the counter available as a public function.
While we're moving it, let's make sure the counter is
allocated as an unsigned integer to match the index field in
"struct commit".
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When we call lookup_commit, lookup_tree, etc, the logic goes
something like:
1. Look for an existing object struct. If we don't have
one, allocate and return a new one.
2. Double check that any object we have is the expected
type (and complain and return NULL otherwise).
3. Convert an object with type OBJ_NONE (from a prior
call to lookup_unknown_object) to the expected type.
We can encapsulate steps 2 and 3 in a helper function which
checks whether we have the expected object type, converts
OBJ_NONE as appropriate, and returns the object.
Not only does this shorten the code, but it also provides
one central location for converting OBJ_NONE objects into
objects of other types. Future patches will use that to
enforce type-specific invariants.
Since this is a refactoring, we would want it to behave
exactly as the current code. It takes a little reasoning to
see that this is the case:
- for lookup_{commit,tree,etc} functions, we are just
pulling steps 2 and 3 into a function that does the same
thing.
- for the call in peel_object, we currently only do step 3
(but we want to consolidate it with the others, as
mentioned above). However, step 2 is a noop here, as the
surrounding conditional makes sure we have OBJ_NONE
(which we want to keep to avoid an extraneous call to
sha1_object_info).
- for the call in lookup_commit_reference_gently, we are
currently doing step 2 but not step 3. However, step 3
is a noop here. The object we got will have just come
from deref_tag, which must have figured out the type for
each object in order to know when to stop peeling.
Therefore the type will never be OBJ_NONE.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The only way that "obj" can be non-NULL is if it came from
one of the lookup_* functions. These functions always ensure
that the object has the expected type (and return NULL
otherwise), so there is no need for us to set the type.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "struct object" type implements basic object
polymorphism. Individual instances are allocated as
concrete types (or as a union type that can store any
object), and a "struct object *" can be cast into its real
type after examining its "type" enum. This means it is
dangerous to have a type field that does not match the
allocation (e.g., setting the type field of a "struct blob"
to "OBJ_COMMIT" would mean that a reader might read past the
allocated memory).
In most of the current code this is not a problem; the first
thing we do after allocating an object is usually to set its
type field by passing it to create_object. However, the
virtual commits we create in merge-recursive.c do not ever
get their type set. This does not seem to have caused
problems in practice, though (presumably because we always
pass around a "struct commit" pointer and never even look at
the type).
We can fix this oversight and also make it harder for future
code to get it wrong by setting the type directly in the
object allocation functions.
This will also make it easier to fix problems with commit
index allocation, as we know that any object allocated by
alloc_commit_node will meet the invariant that an object
with an OBJ_COMMIT type field will have a unique index
number.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Because the allocator functions for tree, blobs, etc are all
very similar, we originally used a macro to avoid repeating
ourselves. Since the prior commit, though, the heavy lifting
is done by an inline helper function. The macro does still
save us a few lines, but at some readability cost. It
obfuscates the function definitions (and makes them hard to
find via grep).
Much worse, though, is the fact that it isn't used
consistently for all allocators. Somebody coming later may
be tempted to modify DEFINE_ALLOCATOR, but they would miss
alloc_commit_node, which is treated specially.
Let's just drop the macro and write everything out
explicitly.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
alloc.c: remove the alloc_raw_commit_node() function
In order to encapsulate the setting of the unique commit index, commit 969eba63 ("commit: push commit_index update into alloc_commit_node",
10-06-2014) introduced a (logically private) intermediary allocator
function. However, this function (alloc_raw_commit_node()) was declared
as a public function, which undermines its entire purpose.
Introduce an inline function, alloc_node(), which implements the main
logic of the allocator used by DEFINE_ALLOCATOR, and redefine the macro
in terms of the new function. In addition, use the new function in the
implementation of the alloc_commit_node() allocator, rather than the
intermediary allocator, which can now be removed.
Noticed by sparse ("symbol 'alloc_raw_commit_node' was not declared.
Should it be static?").
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
http-push.c: make CURLOPT_IOCTLDATA a usable pointer
Fixes a small bug affecting push to remotes which use some sort of
multi-pass authentication. In particular the bug affected SabreDAV as
configured by Box.com [1].
It must be a weird server configuration for the bug to have survived
this long. Someone should write a test for it.
The --sort tests should use the better format for >expect to maintain
indenting and ensure that no substitution is occurring. This makes
parsing and understanding the tests a bit easier.
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
fsck: simplify fsck_commit_buffer() by using commit_list_count()
fsck_commit_buffer() checks that the number of items in the parents
list of a commit matches the number of parent lines in its buffer or --
if a graft is used -- the number of parents in that graft. Simplify
the code by using commit_list_count() instead of counting by hand.
Also use different variables for the number of lines and the number of
list items, making it easier to compare them.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
merge: simplify merge_trivial() by using commit_list_append()
Build the commit_list of parents by calling commit_list_append() twice
instead of allocating and linking the items by hand. This makes the
code shorter and simpler. Rename the commit_list from parent to parents
(plural) while at it because there are two of them.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/Makefile: always test all lint targets when running tests
Only the two targets "test-lint-duplicates" and "test-lint-executable" are
currently executed when running the test target. This was done on purpose
when the TEST_LINT variable was added in 81127d74 to avoid twisted shell
scripting by developers only to avoid false positives that might result
from the rather simple minded tests, e.g. test-lint-shell-syntax. But it
looks like it might be better to include all lint tests to help developers
to detect non portable shell constructs before the patch is sent to the
list and reviewed there.
Change the TEST_LINT variable to run all lint test unless the TEST_LINT
variable is overridden. If we hit false positives more often than helping
developers to avoid non-portable code (or add less accurate or slow tests
later) we could still fall back to exclude them like 81127d74 proposed.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t/Makefile: check helper scripts for non-portable shell commands too
Currently only the "t[0-9][0-9][0-9][0-9]-*.sh" scripts are tested for
shell incompatibilities using the check-non-portable-shell.pl script. This
makes it easy to miss non-POSIX constructs added to one of the t/*lib*.sh
helper scripts, as they aren't automatically detected.
Fix that by adding a THELPERS variable containing all shell scripts that
aren't tests and add these to the "test-lint-shell-syntax" target too.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add 'verify-commit' to be used in a way similar to 'verify-tag' is
used. Further work on verifying the mergetags might be needed.
* mg/verify-commit:
t7510: test verify-commit
t7510: exit for loop with test result
verify-commit: scriptable commit signature verification
gpg-interface: provide access to the payload
gpg-interface: provide clear helper for struct signature_check
"git clone -b brefs/tags/bar" would have mistakenly thought we were
following a single tag, even though it was a name of the branch,
because it incorrectly used strstr().
* jc/fix-clone-single-starting-at-a-tag:
builtin/clone.c: detect a clone starting at a tag correctly
Merge branch 'jk/repack-pack-keep-objects' into maint
* jk/repack-pack-keep-objects:
repack: s/write_bitmap/&s/ in code
repack: respect pack.writebitmaps
repack: do not accidentally pack kept objects by default
remote-curl: mark helper-protocol errors more clearly
When we encounter an error in remote-curl, we generally just
report it to stderr. There is no need for the user to care
that the "could not connect to server" error was generated
by git-remote-https rather than a function in the parent
git-fetch process.
However, when the error is in the protocol between git and
the helper, it makes sense to clearly identify which side is
complaining. These cases shouldn't ever happen, but when
they do, we can make them less confusing by being more
verbose.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We usually prefix our error messages with "error: ", but
many error messages from remote-curl are simply printed with
fprintf. This can make the output a little harder to read
(especially because such message may be intermingled with
errors from the parent git process).
There is no reason to avoid error(), as we are already
calling it many places (in addition to libgit.a functions
which use it).
While we're adjusting the messages, we can also drop the
capitalization which makes them unlike other git error
messages.
Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
remote-curl: do not complain on EOF from parent git
The parent git process is supposed to send us an empty line
to indicate that the conversation is over. However, the
parent process may die() if there is a problem with the
operation (e.g., we try to fetch a ref that does not exist).
In this case, it produces a useful message, but then
remote-curl _also_ produces an unhelpful message:
$ git pull origin matser
fatal: couldn't find remote ref matser
Unexpected end of command stream
The "right" way to fix this is to teach the parent git to
always cleanly close the connection to the helper, letting
it know that we are done. Implementing that is rather
clunky, though, as it would involve either replacing die()
operations with returning errors up the stack (until we
disconnect the transport), or adding an atexit handler to
clean up any transport helpers left open.
It's much simpler to just suppress the EOF message in
remote-curl. It was not added to address any real-world
situation in the first place, but rather a "we should
probably report unexpected things" suggestion[1].
It is the parent git which drives the operation, and whose
exit value actually matters. If the parent dies, then the
helper has no need to complain (except as a debugging aid).
In the off chance that the pipe is closed without the parent
dying, it can still notice the non-zero exit code.
* jk/pretty-G-format-fixes:
move "%G" format test from t7510 to t6006
pretty: avoid reading past end-of-string with "%G"
t7510: check %G* pretty-format output
t7510: test a commit signed by an unknown key
t7510: use consistent &&-chains in loop
t7510: stop referring to master in later tests
* jk/xstrfmt:
setup_git_env(): introduce git_path_from_env() helper
unique_path: fix unlikely heap overflow
walker_fetch: fix minor memory leak
merge: use argv_array when spawning merge strategy
sequencer: use argv_array_pushf
setup_git_env: use git_pathdup instead of xmalloc + sprintf
use xstrfmt to replace xmalloc + strcpy/strcat
use xstrfmt to replace xmalloc + sprintf
use xstrdup instead of xmalloc + strcpy
use xstrfmt in favor of manual size calculations
strbuf: add xstrfmt helper
* jk/skip-prefix:
http-push: refactor parsing of remote object names
imap-send: use skip_prefix instead of using magic numbers
use skip_prefix to avoid repeated calculations
git: avoid magic number with skip_prefix
fetch-pack: refactor parsing in get_ack
fast-import: refactor parsing of spaces
stat_opt: check extra strlen call
daemon: use skip_prefix to avoid magic numbers
fast-import: use skip_prefix for parsing input
use skip_prefix to avoid repeating strings
use skip_prefix to avoid magic numbers
transport-helper: avoid reading past end-of-string
fast-import: fix read of uninitialized argv memory
apply: use skip_prefix instead of raw addition
refactor skip_prefix to return a boolean
avoid using skip_prefix as a boolean
daemon: mark some strings as const
parse_diff_color_slot: drop ofs parameter
The git log --graph --show-signature command incorrectly indents the gpg
information about signed commits and merged signed tags. It does not
follow the level of indentation of the current commit.
Example of garbled output:
$ git log --show-signature --graph
* commit 258e0a237cb69aaa587b0a4fb528bb0316b1b776
|\ gpg: Signature made Mon, Jun 30, 2014 13:22:33 EDT using RSA key ID DA08
gpg: Good signature from "Jason Pyeron <jpye...@pdinc.us>"
Merge: 727c3551ca13ed
| | Author: Jason Pyeron <jpye...@pdinc.us>
| | Date: Mon Jun 30 13:22:29 2014 -0400
| |
| | Merge of 1ca13ed2271d60ba9 branch - rebranding
| |
| * commit 1ca13ed2271d60ba93d40bcc8db17ced8545f172
| | gpg: Signature made Mon, Jun 23, 2014 9:45:47 EDT using RSA key ID DD37
gpg: Good signature from "Stephen Robert Guglielmo <s...@guglielmo.us>"
gpg: aka "Stephen Robert Guglielmo <srguglie...@gmail.com>"
Author: Stephen R Guglielmo <s...@guglielmo.us>
| | Date: Mon Jun 23 09:45:27 2014 -0400
| |
| | Minor URL updates
In log-tree.c modify show_sig_lines() function to call graph_show_oneline()
after each line of gpg information it has printed in order to preserve
the level of indentation for the next output line.
Reported-by: Jason Pyeron <jpyeron@pdinc.us> Signed-off-by: Zoltan Klinger <zoltan.klinger@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add all of the ways in which check_refname_format violates valgrind's
expectations to the valgrind suppression file; remove an assumption about
the call chain of check_refname_format from same.
Signed-off-by: David Turner <dturner@twitter.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
t0027: combinations of core.autocrlf, core.eol and text
Historically there are 3 different parameters controlling how line endings
are handled by Git:
- core.autocrlf
- core.eol
- the "text" attribute in .gitattributes
There are different types of content:
- (1) Files with only LF
- (2) Files with only CRLF
- (3) Files with mixed LF and CRLF
- (4) Files with LF and/or CRLF with CR not followed by LF
- (5) Files which are binary (e.g. have NUL bytes)
Recently the question came up, how files with mixed EOLs are handled by Git
(and libgit2) when they are checked out and core.autocrlf=true.
See
http://git.661346.n2.nabble.com/The-different-EOL-behavior-between-libgit2-based-software-and-official-Git-td7613670.html#a7613801
Add the EXPENSIVE t0027-auto-crlf.sh to test all combination of files
and parameters for both "git add/commit" and "git checkout".
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The current test files are named one, two and three.
Make it clearer what the tests do and rename them into
LFonly, CRLFonly and LFwithNUL.
After the renaming we can see easier that we may want more test cases
for 2 types of files:
- files which have mixed LF and CRLF line endings,
- files which have mixed LF and CR line endings.
See commit fd6cce9e, "Add per-repository eol normalization" and
"the new safer autocrlf handling" in convert.c
Signed-off-by: Torsten Bögershausen <tboegi@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix profile feedback with -jN and add profile-fast
Profile feedback always failed for me with -jN. The problem
was that there was no implicit ordering between the profile generate
stage and the profile use stage. So some objects in the later stage
would be linked with profile generate objects, and fail due
to the missing -lgcov.
This adds a new profile target that implicitely enforces the
correct ordering by using submakes. Plus a profile-install target
to also install. This is also nicer to type that PROFILE=...
Plus I always run the performance test suite now for the full
profile run.
In addition I also added a profile-fast / profile-fast-install
target the only runs the performance test suite instead of the
whole test suite. This significantly speeds up the profile build,
which was totally dominated by test suite run time. However
it may have less coverage of course.
Signed-off-by: Andi Kleen <ak@linux.intel.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In the same way as there is for_each_ref() to iterate on refs,
for_each_mergetag() allows the caller to iterate on the mergetags of
a given commit. Use it to rewrite show_mergetag() used in "git log".
Signed-off-by: Christian Couder <chriscool@tuxfamily.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Add the whole directory of test files at once using git add instead of
calling git update-index on each of them and use git commit instead of
the plumbing commands write-tree, update-ref and commit-tree to build
the commit. This simplifies the code considerably.
Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>