Russell Sim [Sat, 29 Jul 2017 17:22:48 +0000 (19:22 +0200)]
Merge tag 'upstream/0.26.0+dfsg.1'
Upstream version 0.26.0+dfsg.1
* tag 'upstream/0.26.0+dfsg.1': (367 commits)
DFSG changes
CHANGELOG: document git_filter_init and GIT_FILTER_INIT
Adding git_filter_init for initializing `git_filter` struct + unit test
adding GIT_FILTER_VERSION to GIT_FILTER_INIT as part of convention
tests: odb: add more low-level backend tests
tests: odb: implement `exists_prefix` for the fake backend
tests: odb: use correct OID length
tests: odb: have the fake backend detect ambiguous prefixes
tests: core: test initialization of `git_proxy_options`
tests: network: add missing include for `git_repository_new`
cmake: disable optimization on debug builds
cmake: set "-D_DEBUG" on non-Windows platforms
cmake: remove stale comment on precompiled headers
travis: replace use of deprecated homebrew/dupes tap
travis: install openssl explicitly
remote: ensure we can create an anon remote on inmemory repo
repository: don't fail to create config option in inmemory repo
repository_item_path: return ENOTFOUND when appropriate
tests: odb: add tests with multiple backends
tests: odb: allow passing fake objects to the fake backend
...
Introduce a new test suite "odb::backend::simple", which utilizes the
fake backend to exercise the ODB abstraction layer. While such tests
already exist for the case where multiple backends are put together, no
direct testing for functionality with a single backend exist yet.
tests: odb: implement `exists_prefix` for the fake backend
The fake backend currently implements all reading functions except for
the `exists_prefix` one. Implement it to enable further testing of the
ODB layer.
The `search_object` function takes the OID length as one of its
parameters, where its maximum length is `GIT_OID_HEXSZ`. The `exists`
function of the fake backend used `GIT_OID_RAWSZ` though, leading to
only the first half of the OID being used when finding the correct
object.
tests: odb: have the fake backend detect ambiguous prefixes
In order to be able to test the ODB prefix functions, we need to be able
to detect ambiguous prefixes in case multiple objects with the same
prefix exist in the fake ODB. Extend `search_object` to detect ambiguous
queries and have callers return its error code instead of always
returning `GIT_ENOTFOUND`.
tests: core: test initialization of `git_proxy_options`
Initialization of the `git_proxy_options` structure is never tested
anywhere. Include it in our usual initialization test in
"core::structinit::compare".
tests: network: add missing include for `git_repository_new`
A newly added test uses the `git_repository_new` function without the
corresponding header file being included. While this works due to the
compiler deducing the correct function signature, we should obviously
just include the function's declaration file.
While our debug builds on MSVC platforms already tune the code optimizer
to aid debugging code, all the other platforms still use the default
optimization level. This makes it hard for developers on these platforms
to actually debug code while maintaining his sanity due to optimizations
like inlined code, elided variables etc.
To help this common use case, we can simply follow the MSVC example and
turn off code optimization with "-O0" for debug builds. While it would
be preferable to instead use "-Og" supported by more modern compilers,
we cannot guarantee that this level is available on all supported
platforms.
In our code base, we have some occasions where we use the "_DEBUG"
preprocessor macro to enable additional code which should not be part of
release builds. While we define this flag on MSVC platforms, it is
guarded by the conditional `WIN32 AND NOT CYGWIN` on other platforms
since 19be3f9e6 (Improve MSVC compiler, linker flags, 2013-02-13). While
this condition can be fulfilled by the MSVC platform, it is never
encountered due to being part of the `ELSE` part of `IF (MSVC)`.
The intention of the conditional was most likely to avoid the
preprocessor macro on Cygwin platforms, but to include it on everthing
else. As such, the correct condition here would be `IF (NOT CYGWIN)`
instead. But digging a bit further, the condition is only ever used in
two places:
1. To skip the test in "core::structinit", which should also work on
Cygwin.
2. In "src/win32/git2.rc", where it is used to set additional file
flags. As this file is included in MSVC builds only, it cannot cause
any harm to set "_DEBUG" on Cygwin here.
As such, we can simply drop the conditional and always set "-D_DEBUG" on
all platforms.
cmake: remove stale comment on precompiled headers
In commit 9f75a9ce7 (Turning on runtime checks when building debug under
MSVC., 2012-03-30), we introduced a comment "Precompiled headers", which
actually refers to no related commands. Seeing that the comment never
had anything to refer to, we can simply remove it here.
Edward Thomson [Mon, 12 Jun 2017 11:01:10 +0000 (12:01 +0100)]
repository_item_path: return ENOTFOUND when appropriate
Disambiguate error values: return `GIT_ENOTFOUND` when the item cannot
exist in the repository (perhaps because the repository is inmemory or
otherwise not backed by a filesystem), return `-1` when there is a hard
failure.
Previous to pulling out and extending the fake backend, it was quite
cumbersome to write tests for very specific scenarios regarding
backends. But as we have made it more generic, it has become much easier
to do so. As such, this commit adds multiple tests for scenarios with
multiple backends for the ODB.
The changes also include a test for a very targeted scenario. When one
backend found a matching object via `read_prefix`, but the last backend
returns `GIT_ENOTFOUND` and when object hash verification is turned off,
we fail to reset the error code to `GIT_OK`. This causes us to segfault
later on, when doing a double-free on the returned object.
tests: odb: allow passing fake objects to the fake backend
Right now, the fake backend is quite restrained in the way how it
works: we pass it an OID which it is to return later as well as an error
code we want it to return. While this is sufficient for existing tests,
we can make the fake backend a little bit more generic in order to allow
us testing for additional scenarios.
To do so, we change the backend to not accept an error code and OID
which it is to return for queries, but instead a simple array of OIDs
with their respective blob contents. On each query, the fake backend
simply iterates through this array and returns the first matching
object.
In order to make the fake backend more useful, we want to enable it
holding multiple object references. To do so, we need to decouple it
from the single fake OID it currently holds, which we simply move up
into the calling tests.
The fake backend used by the test suite `odb::backend::nonrefreshing` is
useful to have some low-level tests for the ODB layer. As such, we move
the implementation into its own `backend_helpers` module.
Edward Thomson [Mon, 12 Jun 2017 11:56:40 +0000 (12:56 +0100)]
odb_read_prefix: reset error in backends loop
When looking for an object by prefix, we query all the backends so that
we can ensure that there is no ambiguity. We need to reset the `error`
value between backends; otherwise the first backend may find an object
by prefix, but subsequent backends may not. If we do not reset the
`error` value then it will remain at `GIT_ENOTFOUND` and `read_prefix_1`
will fail, despite having actually found an object.
Edward Thomson [Thu, 8 Jun 2017 21:23:53 +0000 (22:23 +0100)]
checkout: cope with untracked files in directory deletion
When deleting a directory during checkout, do not simply delete the
directory, since there may be untracked files. Instead, go into
the iterator and examine each file.
In the original code (the code with the faulty assumption), we look to
see if there's an index entry beneath the directory that we want to
remove. Eg, it looks to see if we have a workdir entry foo and an
index entry foo/bar.txt. If this is not the case, then the working
directory must have precious files in that directory. This part is okay.
The part that's not okay is if there is an index entry foo/bar.txt. It
just blows away the whole damned directory.
That's not cool.
Instead, by simply pushing the directory itself onto the stack and
iterating each entry, we will deal with the files one by one - whether
they're in the index (and can be force removed) or not (and are
precious).
The original code was a bad optimization, assuming that we didn't need
to git_iterator_advance_into if there was any index entry in the folder.
That's wrong - we could have optimized this iff all folder entries are
in the index.
Instead, we need to simply dig into the directory and analyze its
entries.
checkout: do not delete directories with untracked entries
If the `GIT_CHECKOUT_FORCE` flag is given to any of the `git_checkout`
invocations, we remove files which were previously staged. But while
doing so, we unfortunately also remove unstaged files in a directory
which contains at least one staged file, resulting in potential data
loss.
Roger Gee [Sat, 10 Jun 2017 16:46:09 +0000 (11:46 -0500)]
smart_protocol: fix parsing of server ACK responses
Fix ACK parsing in wait_while_ack() internal function. This patch
handles the case where multi_ack_detailed mode sends 'ready' ACKs. The
existing functionality would bail out too early, thus causing the
processing of the ensuing packfile to fail if/when 'ready' ACKs were
sent.
Initially, the setting has been solely used to enable the use of
`fsync()` when creating objects. Since then, the use has been extended
to also cover references and index files. As the option is not yet part
of any release, we can still correct this by renaming the option to
something more sensible, indicating not only correlation to objects.
This commit renames the option to `GIT_OPT_ENABLE_FSYNC_GITDIR`. We also
move the variable from the object to repository source code.
The `git_buf_init` function has an optional length parameter, which will
cause the buffer to be initialized and allocated in one step. This can
be used instead of static initialization with `GIT_BUF_INIT` followed by
a `git_buf_grow`. This patch does so for two functions where it is
applicable.
buffer: return errors for `git_buf_init` and `git_buf_attach`
Both the `git_buf_init` and `git_buf_attach` functions may call
`git_buf_grow` in case they were given an allocation length as
parameter. As such, it is possible for these functions to fail when we
run out of memory. While it won't probably be used anytime soon, it does
indeed make sense to also record this fact by returning an error code
from both functions. As they belong to the internal API only, this
change does not break our interface.
buffer: consistently use `ENSURE_SIZE` to grow buffers on-demand
The `ENSURE_SIZE` macro can be used to grow a buffer if its currently
allocated size does not suffice a required target size. While most of
the code already uses this macro, the `git_buf_join` and `git_buf_join3`
functions do not yet use it. Due to the macro first checking whether we
have to grow the buffer at all, this has the benefit of saving a
function call when it is not needed. While this is nice to have, it will
probably not matter at all performance-wise -- instead, this only serves
for consistency across the code.
While the `ENSURE_SIZE` macro gets a reference to both the buffer that
is to be resized and a new size, we were not consistently referencing
the passed buffer, but instead a variable `buf`, which is not passed in.
Funnily enough, we never noticed because our buffers seem to always be
named `buf` whenever the macro was being used.
Fix the macro by always using the passed-in buffer. While at it, add
braces around all mentions of passed-in variables as should be done with
macros to avoid subtle errors.
buffer: rely on `GITERR_OOM` set by `git_buf_try_grow`
The function `git_buf_try_grow` consistently calls `giterr_set_oom`
whenever growing the buffer fails due to insufficient memory being
available. So in fact, we do not have to do this ourselves when a call
to any buffer-growing function has failed due to an OOM situation. But
we still do so in two functions, which this patch cleans up.
The updated SHA1DC library allows us to use custom includes instead of
using standard includes. Due to requirements with cross-platform, we
provide some custom system includes files like for example the
"stdint.h" file on Win32. Because of this, we want to make sure to avoid
breaking cross-platform compatibility when SHA1DC is enabled.
To use the new mechanism, we can simply define
`SHA1DC_NO_STANDARD_INCLUDES`. Furthermore, we can specify custom
include files via two defines, which we now use to include our
"common.h" header.