When diff encounters an untracked directory, there was a shortcut
that it took which is not compatible with core git. This makes
the default behavior no longer take that shortcut and instead look
inside the untracked directory to see if there are any untracked
files within it. If there are not, then the directory is treated
as an ignore directory instead of an untracked directory. This
has implications for the git_status APIs.
In preparation for more changes to the internal diff logic, it
seemed wise to split the very large git_diff__from_iterators into
separate functions that handle the four main cases (unmatched old
item, unmatched new item, unmatched new directory, and matched
old and new items). Hopefully this will keep the logic easier to
follow even as more cases have to be added to this code.
This makes diff use the cvar cache for config options where
possible, and also adds support for a number of other config
options to diff including "diff.context", "diff.ignoreSubmodules",
"diff.noprefix", "diff.mnemonicprefix", and "core.abbrev".
To make this natural, this involved a rearrangement of the code
that allocates the diff object vs. the code that initializes it
based on the combination of options passed in by the user and
read from the config.
This commit includes tests for most of these new options as well.
When case insensitive tree iterators were added, we started reading
the case sensitivity of the index to decide if the tree should be
case sensitive. This is good for index-to-tree comparisons, but
for tree-to-tree comparisons, we should really default to doing a
case sensitive comparison unless the user really wants otherwise.
This adds tests for diffs with submodules in them and (perhaps
unsurprisingly) requires further fixes to be made. Specifically,
this fixes:
- when considering if a submodule is dirty in the workdir, it was
being treated as dirty even if only the index was dirty.
- git_diff_patch_to_str (and git_diff_patch_print) were "printing"
the headers for files (and submodules) that were unmodified or
had no meaningful content.
- added comment to previous fix and removed unneeded parens.
This started out trying to look at the problems from issue #1425
and gradually grew to a broader set of fixes. There are two core
things fixed here:
1. When you had an ignore like "/bin" which is rooted at the top
of your tree, instead of immediately adding the "bin/" entry
as an ignored item in the diff, we were returning all of the
direct descendants of the directory as ignored items. This
changes things to immediately ignore the directory. Note that
this effects the behavior in test_status_ignore__subdirectories
so that we no longer exactly match core gits ignore behavior,
but the new behavior probably makes more sense (i.e. we now
will include an ignored directory inside an untracked directory
that we previously would have left off).
2. When a submodule only contained working directory changes, the
diff code was always considering it unmodified which was just
an outright bug. The HEAD SHA of the submodule matches the SHA
in the parent repo index, and since the SHAs matches, the diff
code was overwriting the actual status with UNMODIFIED.
These fixes broke existing tests test_diff_workdir__submodules and
test_status_ignore__subdirectories but looking it over, I actually
think the new results are correct and the old results were wrong.
@nulltoken had actually commented on the subdirectory ignore issue
previously.
I also included in the tests some debugging versions of the
shared iteration callback routines that print status or diff
information. These aren't used actively in the tests, but can be
quickly swapped in to test code to give a better picture of what
is being scanned in some of the complex test scenarios.
This option has been sitting unimplemented for a while, so I
finally went through and implemented it along with some tests.
As part of this, I improved the implementation of
GIT_DIFF_IGNORE_SUBMODULES so it be more diligent about avoiding
extra work and about leaving off delta records for submodules to
the greatest extent possible (though it may include them still
if you are request TYPECHANGE records).
This implements working versions of GIT_DIFF_RECURSE_IGNORED_DIRS
and GIT_STATUS_OPT_RECURSE_IGNORED_DIRS along with some tests for
the newly available behaviors. This is not turned on by default
for status, but can be accessed via the options to the extended
version of the command.
1. Fix sort order problem with submodules where "mod" was sorting
after "mod-plus" because they were being sorted as "mod/" and
"mod-plus/". This involved pushing the "contains a .git entry"
test significantly lower in the stack.
2. Reinstate behavior that a directory which contains a .git entry
will be treated as a submodule during iteration even if it is
not yet added to the .gitmodules.
3. Now that any directory containing .git is reported as submodule,
we have to be more careful checking for GIT_EEXISTS when we
do a submodule lookup, because that is the error code that is
returned by git_submodule_lookup when you try to look up a
directory containing .git that has no record in gitmodules or
the index.
Previously, 0 meant default. This is problematic, as asking for 0
context lines is a valid thing to do.
Change GIT_DIFF_OPTIONS_INIT to default to three and stop treating 0
as a magic value. In case no options are provided, make sure the
options in the diff object default to 3.
This standardizes iterator behavior across all three iterators
(index, tree, and working directory). Previously the working
directory iterator behaved differently from the other two.
Each iterator can now operate in one of three modes:
1. *No tree results, auto expand trees* means that only non-
tree items will be returned and when a tree/directory is
encountered, we will automatically descend into it.
2. *Tree results, auto expand trees* means that results will
be given for every item found, including trees, but you
only need to call normal git_iterator_advance to yield
every item (i.e. trees returned with pre-order iteration).
3. *Tree results, no auto expand* means that calling the
normal git_iterator_advance when looking at a tree will
not descend into the tree, but will skip over it to the
next entry in the parent.
Previously, behavior 1 was the only option for index and tree
iterators, and behavior 3 was the only option for workdir.
The main public API implications of this are that the
`git_iterator_advance_into()` call is now valid for all
iterators, not just working directory iterators, and all the
existing uses of working directory iterators explicitly use
the GIT_ITERATOR_DONT_AUTOEXPAND (for now).
Interestingly, the majority of the implementation was in the
index iterator, since there are no tree entries there and now
have to fake them. The tree and working directory iterators
only required small modifications.
The iterator APIs are not currently consistent with the parameter
ordering of the rest of the codebase. This rearranges the order
of parameters, simplifies the naming of a number of functions, and
makes somewhat better use of macros internally to clean up the
iterator code.
This also expands the test coverage of iterator functionality,
making sure that case sensitive range-limited iteration works
correctly.
Previously the git_diff_delta recorded if the delta was binary.
This replaces that (with no net change in structure size) with
a full set of flags. The flag values that were already in use
for individual git_diff_file objects are reused for the delta
flags, too (along with renaming those flags to make it clear that
they are used more generally).
This (a) makes things somewhat more consistent (because I was
using a -1 value in the "boolean" binary field to indicate unset,
whereas now I can just use the flags that are easier to understand),
and (b) will make it easier for me to add some additional flags to
the delta object in the future, such as marking the results of a
copy/rename detection or other deltas that might want a special
indicator.
While making this change, I officially moved some of the flags that
were internal only into the private diff header.
This also allowed me to remove a gross hack in rename/copy detect
code where I was overwriting the status field with an internal
value.
The callback will be called for each file, just before the `git_delta_t` gets inserted into the diff list.
When the callback:
- returns < 0, the diff process will be aborted
- returns > 0, the delta will not be inserted into the diff list, but the diff process continues
- returns 0, the delta is inserted into the diff list, and the diff process continues
Instead of returning directly the pattern as the return value, I used an
out parameter, because the function also tests if the passed pathspecs
vector is empty. If yes, it considers that the path "matches", but in
that case there is no matched pattern per se.
This changes the iterator API so that flags can be passed in to
the constructor functions to control the ignore_case behavior.
At this point, the flags are not supported on tree iterators (i.e.
there is no functional change over the old API), but the API
changes are all made to accomodate this.
By the way, I went with a flags parameter because in the future
I have a couple of other ideas for iterator flags that will make
it easier to fix some diff/status/checkout bugs.
The spoolandsort iterator changes got sort-of cherry picked out of
this branch and so I dropped the commit when rebasing; however,
there were a few small changes that got dropped as well (since the
version merged upstream wasn't quite the same as what I dropped).
This flips checkout back to be driven off the changes between
the baseline and the target trees. This reinstates the complex
code for tracking the contents of the working directory, but
overall, I think the resulting logic is easier to follow.
An earlier change to `git_diff_from_iterators` introduced a
memory leak where the allocated spoolandsort iterator was not
returned to the caller and thus not freed.
One proposal changes all iterator APIs to use git_iterator** so
we can reallocate the iterator at will, but that seems unexpected.
This commit makes it so that an iterator can be changed in place.
The callbacks are isolated in a separate structure and a pointer
to that structure can be reassigned by the spoolandsort extension.
This means that spoolandsort doesn't create a new iterator; it
just allocates a new block of callbacks (along with space for its
own extra data) and swaps that into the iterator.
Additionally, since spoolandsort is only needed to switch the
case sensitivity of an iterator, this simplifies the API to only
take the ignore_case boolean and to be a no-op if the iterator
already matches the requested case sensitivity.
The diff constructor functions had some confusing names, where the
"old" side of the diff was coming after the "new" side. This
reverses the order in the function name to make it less confusing.
Specifically...
* git_diff_index_to_tree becomes git_diff_tree_to_index
* git_diff_workdir_to_index becomes git_diff_index_to_workdir
* git_diff_workdir_to_tree becomes git_diff_tree_to_workdir
This removes the need to explicitly pass the repo into iterators
where the repo is implied by the other parameters. This moves
the repo to be owned by the parent struct. Also, this has some
iterator related updates to the internal diff API to lay the
groundwork for checkout improvements.
A number of diff APIs and the `git_checkout_index` API take a
`git_repository` object an operate on the index. This updates
them to take a `git_index` pointer explicitly and only fall back
on the `git_repository` index if the index input is NULL. This
makes it easier to operate on a temporary index.
The index iterator could previously only be created from a repo
object, but this allows creating an iterator from a `git_index`
object instead (while keeping, though renaming, the old function).
Using the builtin strcmp and strcasecmp as function pointers is
problematic on win32. This adds internal implementations and
divorces us from the platform linkage.
This fixes a number of warnings and problems with cross-platform
builds. Among other things, it's not safe to name a member of a
structure "strcmp" because that may be #defined.
There are some diff functions that are useful in a rewritten
checkout and this lays some groundwork for that. This contains
three main things:
1. Share the function diff uses to calculate the OID for a file
in the working directory (now named `git_diff__oid_for_file`
2. Add a `git_diff__paired_foreach` function to iterator over
two diff lists concurrently. Convert status to use it.
3. Move all the string/prefix/index entry comparisons into
function pointers inside the `git_diff_list` object so they
can be switched between case sensitive and insensitive
versions. This makes them easier to reuse in various
functions without replicating logic. As part of this, move
a couple of index functions out of diff.c and into index.c.
Diff uses a `git_strarray` of path specs to represent a subset
of all files to be processed. It is useful to be able to reuse
this filtering in other places outside diff, so I've moved it
into a standalone set of utilities.
This improves the naming for the rename related functionality
moving it to be called `git_diff_find_similar()` and renaming
all the associated constants, etc. to make more sense.
I also moved the new code (plus the existing `git_diff_merge`)
into a new file `diff_tform.c` where I can put new functions
related to manipulating git diff lists.
This also updates the implementation significantly from the
last revision fixing some ordering issues (where break-rewrite
needs to be handled prior to copy and rename detection) and
improving config option handling.
This implements the basis for diff rename and copy detection,
although it is based on simple SHA comparison right now instead
of using a matching algortihm. Just as `git_diff_merge` can be
used as a post-pass on diffs to emulate certain command line
behaviors, there is a new API `git_diff_detect` which will
update a diff list in-place, adjusting some deltas to RENAMED
or COPIED state (and also, eventually, splitting MODIFIED deltas
where the change is too large into DELETED/ADDED pairs).
This also adds a new test repo that will hold rename/copy/split
scenarios. Right now, it just has exact-match rename and copy,
but the tests are written to use tree diffs, so we should be able
to add new test scenarios easily without breaking tests.
This started as a complex new test for checkout going through the
"typechanges" test repository, but that revealed numerous issues
with checkout, including:
* complete failure with submodules
* failure to create blobs with exec bits
* problems when replacing a tree with a blob because the tree
"example/" sorts after the blob "example" so the delete was
being processed after the single file blob was created
This fixes most of those problems and includes a number of other
minor changes that made it easier to do that, including improving
the TYPECHANGE support in diff/status, etc.
When I wrote the diff code, I based it on core git's diff output
which tends to split a type change into an add and a delete. But
core git's status has the notion of a T (typechange) flag for a
file. This introduces that into our status APIs and modifies the
diff code so it can be forced to not split type changes.
The adds a test for the submodule diff capabilities and then
fixes a few bugs with how the output is generated. It improves
the accuracy of OIDs in the diff delta object and makes the
submodule output more closely mirror the OIDs that will be used
by core git.
There are a few cases where diff should leave directories in
the diff list if we want to match core git, such as when the
directory contains a .git dir. That feature was lost when I
introduced some of the new submodule handling.
This restores that and then fixes a couple of related to diff
output that are triggered by having diffs with directories in
them.
Also, this adds a new flag that can be passed to diff if you
want diff output to actually include the file content of any
untracked files.
We don't have anything useful that we could do with that oid anyway (We
need to query the submodule for the HEAD commit instead).
Without this, the following code creates the error "Failed to read
descriptor: Is a directory" when run against the submod2 test-case:
const char* oidstr = "873585b94bdeabccea991ea5e3ec1a277895b698";
git_tree* tree = resolve_commit_oid_to_tree(g_repo, oidstr);
git_diff_list* diff = NULL;
cl_assert(tree);
cl_git_pass(git_diff_workdir_to_tree(g_repo, NULL, tree, &diff));
1. teach diff.c:maybe_modified to query git_submodule_status for the
modification state of a submodule. According to the
git_submodule_status docs, it will filter for to-ignore states
already.
2. teach diff_output.c:get_workdir_content to check the submodule status
again and create a line like:
Subproject commit <SHA-1>\n
or
Subproject comimt <SHA-1>-dirty\n
like git.git does.
There are a lot of places where the diff API gives the user access
to internal data structures and many of these were being exposed
through non-const pointers. This replaces them all with const
pointers for any object that the user can access but is still
owned internally to the git_diff_list or git_diff_patch objects.
This will probably break some bindings... Sorry!
This fixes all the bugs in the new diff patch code. The only
really interesting one is that when we merge two diffs, we now
have to actually exclude diff delta records that are not supposed
to be tracked, as opposed to before where they could be included
because they would be skipped silently by `git_diff_foreach()`.
Other than that, there are just minor errors.
Replacing the `git_iterator` object, this creates a simple API
for accessing the "patch" for any file pair in a diff list and
then gives indexed access to the hunks in the patch and the lines
in the hunk. This is the initial implementation of this revised
API - it is still broken, but at least builds cleanly.
This adds support to diff and status for running filters (a la crlf)
on blobs in the workdir before computing SHAs and before generating
text diffs. This ended up being a bit more code change than I had
thought since I had to reorganize some of the diff logic to minimize
peak memory use when filtering blobs in a diff.
This also adds a cap on the maximum size of data that will be loaded
to diff. I set it at 512Mb which should match core git. Right now
it is a #define in src/diff.h but it could be moved into the public
API if desired.
This refactors the diff output code so that an iterator object
can be used to traverse and generate the diffs, instead of just
the `foreach()` style with callbacks. The code has been rearranged
so that the two styles can still share most functions.
This also replaces `GIT_REVWALKOVER` with `GIT_ITEROVER` and uses
that as a common error code for marking the end of iteration when
using a iterator style of object.
In looking at PR #878, I found a few small bugs in the diff code,
mostly related to work that can be avoided when processing tree-
to-tree diffs that was always being carried out. This commit has
some small fixes in it.
A diff that is created with a NULL options parameter could result
in a NULL prefix string, but diff merge was unconditionally
strdup'ing it. I added a test to replicate the issue and then a
new method that does the right thing with NULL values.
This fixes git_index_add and git_index_append to behave more like
core git, preserving old filemode data in the index when adding
and/or appending with core.filemode = false.
This also has placeholder support for core.symlinks and
core.ignorecase, but those flags are not implemented (well,
symlinks has partial support for preserving mode information in
the same way that git does, but it isn't tested).
There are three actual changes in this commit:
1. When the trailing newline of a file is removed in a diff, the
change will now be reported with `GIT_DIFF_LINE_DEL_EOFNL` passed
to the callback. Previously, the `ADD_EOFNL` constant was given
which was just an error in my understanding of when the various
circumstances arose. `GIT_DIFF_LINE_ADD_EOFNL` is deprecated and
should never be generated. A new newline is simply an `ADD`.
2. Rewrote the `diff_delta__merge_like_cgit` function that contains
the core logic of the `git_diff_merge` implementation. The new
version doesn't actually have significantly different behavior,
but the logic should be much more obvious, I think.
3. Fixed a bug in `git_diff_merge` where it freed a string pool
while some of the string data was still in use. This led to
`git_diff_print_patch` accessing memory that had been freed.
The rest of this commit contains improved documentation in `diff.h`
to make the behavior and the equivalencies with core git clearer,
and a bunch of new tests to cover the various cases, oh and a minor
simplification of `examples/diff.c`.
File modes were both not being ignored properly on platforms
where they should be ignored, nor be diffed consistently on
platforms where they are supported.
This change adds a number of diff and status filemode change
tests. This also makes sure that filemode-only changes are
included in the diff output when they occur and that filemode
changes are ignored successfully when core.filemode is false.
There is no code that automatically toggles core.filemode
based on the capabilities of the current platform, so the user
still needs to be careful in their .git/config file.
This fixes two bugs:
* Issue #728 where git_status_file was not working for files
that contain spaces. This was caused by reusing the "fnmatch"
parsing code from ignore and attribute files to interpret the
"pathspec" that constrained the files to apply the status to.
In that code, unescaped whitespace was considered terminal to
the pattern, so a file with internal whitespace was excluded
from the matched files. The fix was to add a mode to that code
that allows spaces and tabs inside patterns. This mode only
comes into play when parsing in-memory strings.
* The other issue was undetected, but it was in the recently
added code to reload gitattributes / gitignores when they were
changed on disk. That code was not clearing out the old values
from the cached file content before reparsing which meant that
newly added patterns would be read in, but deleted patterns
would not be removed. The fix was to clear the vector of
patterns in a cached file before reparsing the file.
There was a bug where tracked files inside directories that were
inside ignored directories where not being found by status. To
make that a little clearer, if you have a .gitignore with:
ignore/
And then have the following files:
ignore/dir/tracked <-- actually a tracked file
ignore/dir/untracked <-- should be ignored
Then we would show the tracked file as being removed (because
when we got the to contained item "dir/" inside the ignored
directory, we decided it was safe to skip -- bzzt, wrong!).
This update is much more careful about checking that we are
not skipping over any prefix of a tracked item, regardless of
whether it is ignored or not.
As documented in diff.c, this commit does create behavior that
still differs from core git with regards to the handling of
untracked files contained inside ignored directories. With
libgit2, those files will just not show up in status or diff.
With core git, those files don't show up in status or diff
either *unless* they are explicitly ignored by a .gitignore
pattern in which case they show up as ignored files.
Needless to say, this is a local behavior difference only, so
it should not be important and (to me) the libgit2 behavior
seems more consistent.
The goal of this work is to rewrite git_status_file to use the
same underlying code as git_status_foreach.
This is done in 3 phases:
1. Extend iterators to allow ranged iteration with start and
end prefixes for the range of file names to be covered.
2. Improve diff so that when there is a pathspec and there is
a common non-wildcard prefix of the pathspec, it will use
ranged iterators to minimize excess iteration.
3. Rewrite git_status_file to call git_status_foreach_ext
with a pathspec that covers just the one file being checked.
Since ranged iterators underlie the status & diff implementation,
this is actually fairly efficient. The workdir iterator does
end up loading the contents of all the directories down to the
single file, which should ideally be avoided, but it is pretty
good.
When a repo is first created, there is no HEAD yet and attempting
to diff files in the index was showing nothing because a tree
iterator could not be constructed. This adds an "empty" iterator
and falls back on that when the head cannot be looked up.
This converts the git attr related code (including ignores) and
the git diff related code (and implicitly the status code) to use
`git_pools` for storing strings. This reduces the number of small
blocks allocated dramatically.
This adds preliminary support for pathspecs to diff and status.
The implementation is not very optimized (it still looks at
every single file and evaluated the the pathspec match against
them), but it works.
Add a new command `git_repository_open_ext` with extended options
that control how searching for a repository will be done. The
existing `git_repository_open` and `git_repository_discover` are
reimplemented on top of it. We may want to change the default
behavior of `git_repository_open` but this commit does not do that.
Improve support for "gitdir" files where the work dir is separate
from the repo and support for the "separate-git-dir" config. Also,
add support for opening repos created with `git-new-workdir` script
(although I have only confirmed that they can be opened, not that
all functions work correctly).
There are also a few minor changes that came up:
- Fix `git_path_prettify` to allow in-place prettifying.
- Fix `git_path_root` to support backslashes on Win32. This fix
should help many repo open/discover scenarios - it is the one
function called when opening before prettifying the path.
- Tweak `git_config_get_string` to set the "out" pointer to NULL
if the config value is not found. Allows some other cleanup.
- Fix a couple places that should have been calling
`git_repository_config__weakptr` and were not.
- Fix `cl_git_sandbox_init` clar helper to support bare repos.
This adds support for a bunch of core.* settings that affect
diff and status, plus fixes up some incorrect implementations
of those settings from before. Also, this cleans up the
handling of config settings in the new submodules code and
in the old attrs/ignore code.
I decided that the COITERATE macro was, in the end causing
more confusion that it would save and decided just to write
out the loops that I needed for parallel diff list iteration.
It is not that much code and this just feels less obfuscated.
This fixes the bug that @nulltoken found (thank you!) where
if there were untracked directories alphabetically after the
last tracked item, the diff implementation would deref a NULL
pointer.
The fix involved the code which decides if it is necessary
to recurse into a directory in the working dir, so it was
easy to add a new option `GIT_STATUS_OPT_RECURSE_UNTRACKED_DIRS`
to control if the contents of untracked directories should be
included in status.
This adds support for roughly-right tracking of submodules
(although it does not recurse into submodules to detect
internal modifications a la core git), and it adds support
for including unmodified files in diff iteration if requested.
This reverts the changes to the GIT_STATUS constants and adds a
new enumeration to describe the type of change in a git_diff_delta.
I don't love this solution, but it should prevent strange errors
from occurring for now. Eventually, I would like to unify the
various status constants, but it needs a larger plan and I just
wanted to eliminate this breakage quickly.
It turns out that commit 31e9cfc4cbcaf1b38cdd3dbe3282a8f57e5366a5
did not fix the GIT_USUSED behavior on all platforms. This commit
walks through and really cleans things up more thoroughly, getting
rid of the unnecessary stuff.
To remove the use of some GIT_UNUSED, I ended up adding a couple
of new iterators for hashtables that allow you to iterator just
over keys or just over values.
In making this change, I found a bug in the clar tests (where we
were doing *count++ but meant to do (*count)++ to increment the
value). I fixed that but then found the test failing because it
was not really using an empty repo. So, I took some of the code
that I wrote for iterator testing and moved it to clar_helpers.c,
then made use of that to make it easier to open fixtures on a
per test basis even within a single test file.
This is a major reorganization of the diff code. This changes
the diff functions to use the iterators for traversing the
content. This allowed a lot of code to be simplified. Also,
this moved the functions relating to outputting a diff into a
new file (diff_output.c).
This includes a number of other changes - adding utility
functions, extending iterators, etc. plus more tests for the
diff code. This also takes the example diff.c program much
further in terms of emulating git-diff command line options.
This is an initial version of git_diff_workdir_to_index. It
also includes renaming some structures and some refactoring
of the existing code so that it could be shared better with
the new function.
This is not complete since it needs a rebase to get some
new odb functions from the upstream branch.
Once I added tests for the whitespace handling options of
diff, I realized that there were some bugs. This fixes
those and adds the new tests into the test suite.
* Implemented git_diff_index_to_tree
* Reworked git_diff_options structure to handle more options
* Made most of the options in git_diff_options actually work
* Reorganized code a bit to remove some redundancy
* Added option parsing to examples/diff.c to test most options
File mode flags are not all defined on WIN32, but since git
is so rigid in how it uses file modes, there is no reason not
to hard code a particular value. Also, this is only used in
the git_diff_print_compact helper function, so it is really
really not important.
