Commit graph

77 commits

Author SHA1 Message Date
Karthik Nayak
47cfc9bd7d attr: add flag --source to work with tree-ish
The contents of the .gitattributes files may evolve over time, but "git
check-attr" always checks attributes against them in the working tree
and/or in the index. It may be beneficial to optionally allow the users
to check attributes taken from a commit other than HEAD against paths.

Add a new flag `--source` which will allow users to check the
attributes against a commit (actually any tree-ish would do). When the
user uses this flag, we go through the stack of .gitattributes files but
instead of checking the current working tree and/or in the index, we
check the blobs from the provided tree-ish object. This allows the
command to also be used in bare repositories.

Since we use a tree-ish object, the user can pass "--source
HEAD:subdirectory" and all the attributes will be looked up as if
subdirectory was the root directory of the repository.

We cannot simply use the `<rev>:<path>` syntax without the `--source`
flag, similar to how it is used in `git show` because any non-flag
parameter before `--` is treated as an attribute and any parameter after
`--` is treated as a pathname.

The change involves creating a new function `read_attr_from_blob`, which
given the path reads the blob for the path against the provided source and
parses the attributes line by line. This function is plugged into
`read_attr()` function wherein we go through the stack of attributes
files.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: Toon Claes <toon@iotcl.com>
Co-authored-by: toon@iotcl.com
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2023-01-14 08:49:55 -08:00
Taylor Blau
be4ac3b197 Merge branch 'rs/no-more-run-command-v'
Simplify the run-command API.

* rs/no-more-run-command-v:
  replace and remove run_command_v_opt()
  replace and remove run_command_v_opt_cd_env_tr2()
  replace and remove run_command_v_opt_tr2()
  replace and remove run_command_v_opt_cd_env()
  use child_process members "args" and "env" directly
  use child_process member "args" instead of string array variable
  sequencer: simplify building argument list in do_exec()
  bisect--helper: factor out do_bisect_run()
  bisect: simplify building "checkout" argument list
  am: simplify building "show" argument list
  run-command: fix return value comment
  merge: remove always-the-same "verbose" arguments
2022-11-08 17:15:12 -05:00
René Scharfe
4120294cbf use child_process member "args" instead of string array variable
Use run_command() with a struct child_process variable and populate its
"args" member directly instead of building a string array and passing it
to run_command_v_opt().  This avoids the use of magic index numbers and
makes simplifies the possible addition of more arguments in the future.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-10-30 14:04:39 -04:00
Jeff King
4b992f0a24 ll-merge: mark unused parameters in callbacks
We have a generic ll_merge_fn, but not every implementation needs every
parameter. In particular, neither binary nor ext merges care about names
(since they do not generate conflict markers), and most do not need to
look at the ll_merge_driver itself.

Ironically, neither ll_xdl_merge() nor ll_union_merge() needs to have
their driver parameter annotated (even though both are named
drv_unused!).  This is because they may fall back to calling
ll_binary_merge() directly. And even though that function won't look at
it, we still pass it along, and hence it is "used" in the caller.

We could get away with passing NULL, but that's likely more confusing
and brittle than just passing along our own driver. And we have to keep
the driver parameter in all callbacks, since ll_ext_merge() uses it.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-17 21:24:04 -07:00
Ævar Arnfjörð Bjarmason
5cf88fd8b0 git-compat-util.h: use "UNUSED", not "UNUSED(var)"
As reported in [1] the "UNUSED(var)" macro introduced in
2174b8c75d (Merge branch 'jk/unused-annotation' into next,
2022-08-24) breaks coccinelle's parsing of our sources in files where
it occurs.

Let's instead partially go with the approach suggested in [2] of
making this not take an argument. As noted in [1] "coccinelle" will
ignore such tokens in argument lists that it doesn't know about, and
it's less of a surprise to syntax highlighters.

This undoes the "help us notice when a parameter marked as unused is
actually use" part of 9b24034754 (git-compat-util: add UNUSED macro,
2022-08-19), a subsequent commit will further tweak the macro to
implement a replacement for that functionality.

1. https://lore.kernel.org/git/220825.86ilmg4mil.gmgdl@evledraar.gmail.com/
2. https://lore.kernel.org/git/220819.868rnk54ju.gmgdl@evledraar.gmail.com/

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-09-01 10:49:48 -07:00
Jeff King
783a86c142 config: mark unused callback parameters
The callback passed to git_config() must conform to a particular
interface. But most callbacks don't actually look at the extra "void
*data" parameter. Let's mark the unused parameters to make
-Wunused-parameter happy.

Note there's one unusual case here in get_remote_default() where we
actually ignore the "value" parameter. That's because it's only checking
whether the option is found at all, and not parsing its value.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-19 12:18:55 -07:00
Junio C Hamano
538dc459a0 Merge branch 'ep/maint-equals-null-cocci'
Introduce and apply coccinelle rule to discourage an explicit
comparison between a pointer and NULL, and applies the clean-up to
the maintenance track.

* ep/maint-equals-null-cocci:
  tree-wide: apply equals-null.cocci
  tree-wide: apply equals-null.cocci
  contrib/coccinnelle: add equals-null.cocci
2022-05-20 15:26:59 -07:00
Junio C Hamano
afe8a9070b tree-wide: apply equals-null.cocci
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-05-02 09:50:37 -07:00
Elijah Newren
35f6967161 ll-merge: make callers responsible for showing warnings
Since some callers may want to send warning messages to somewhere other
than stdout/stderr, stop printing "warning: Cannot merge binary files"
from ll-merge and instead modify the return status of ll_merge() to
indicate when a merge of binary files has occurred.  Message printing
probably does not belong in a "low-level merge" anyway.

This commit continues printing the message as-is, just from the callers
instead of within ll_merge().  Future changes will start handling the
message differently in the merge-ort codepath.

There was one special case here: the callers in rerere.c do NOT check
for and print such a message; since those code paths explicitly skip
over binary files, there is no reason to check for a return status of
LL_MERGE_BINARY_CONFLICT or print the related message.

Note that my methodology included first modifying ll_merge() to return
a struct, so that the compiler would catch all the callers for me and
ensure I had modified all of them.  After modifying all of them, I then
changed the struct to an enum.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-02-02 10:02:27 -08:00
Jeff King
382b601acd ll_union_merge(): rename path_unused parameter
The "path" parameter to ll_union_merge() is named "path_unused", since
we don't ourselves use it. But we do pass it to ll_xdl_merge(), which
may look at it (it gets passed to ll_binary_merge(), which may pass it
to warning()). Let's rename it to correct this inaccuracy (both of the
other functions correctly do not call this "unused").

Note that we also pass drv_unused, but it truly is unused by the rest of
the stack (it only exists at all to provide a generic interface that
matches what ll_ext_merge() needs).

Reported-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-11 12:37:33 +09:00
Jeff King
7f53f78b04 ll_union_merge(): pass name labels to ll_xdl_merge()
Since cd1d61c44f (make union merge an xdl merge favor, 2010-03-01), we
pass NULL to ll_xdl_merge() for the "name" labels of the ancestor, ours
and theirs buffers. We usually use these for annotating conflict markers
left in a file. For a union merge, these shouldn't matter; the point of
it is that we'd never leave conflict markers in the first place.

But there is one code path where we may dereference them: if the file
contents appear to be binary, ll_binary_merge() will give up and pass
them to warning() to generate a message for the user (that was true even
when cd1d61c44f was written, though the warning was in ll_xdl_merge()
back then).

That can result in a segfault, though on many systems (including glibc),
the printf routines will helpfully just say "(null)" instead. We can
extend our binary-union test in t6406 to check stderr, which catches the
problem on all systems.

This also fixes a warning from "gcc -O3". Unlike lower optimization
levels, it inlines enough to see that the NULL can make it to warning()
and complains:

  In function ‘ll_binary_merge’,
      inlined from ‘ll_xdl_merge’ at ll-merge.c:115:10,
      inlined from ‘ll_union_merge’ at ll-merge.c:151:9:
  ll-merge.c:74:4: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
     74 |    warning("Cannot merge binary files: %s (%s vs. %s)",
        |    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
     75 |     path, name1, name2);
        |     ~~~~~~~~~~~~~~~~~~~

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-11 12:37:07 +09:00
Jeff King
7d879ad7e0 ll_binary_merge(): handle XDL_MERGE_FAVOR_UNION
Prior to commit a944af1d86 (merge: teach -Xours/-Xtheirs to binary
ll-merge driver, 2012-09-08), we always reported a conflict from
ll_binary_merge() by returning "1" (in the xdl_merge and ll_merge code,
this value is the number of conflict hunks). After that commit, we
report zero conflicts if the "variant" flag is set, under the assumption
that it is one of XDL_MERGE_FAVOR_OURS or XDL_MERGE_FAVOR_THEIRS.

But this gets confused by XDL_MERGE_FAVOR_UNION. We do not know how to
do a binary union merge, but erroneously report no conflicts anyway (and
just blindly use the "ours" content as the result).

Let's tighten our check to just the cases that a944af1d86 meant to
cover. This fixes the union case (which existed already back when that
commit was made), as well as future-proofing us against any other
variants that get added later.

Note that you can't trigger this from "git merge-file --union", as that
bails on binary files before even calling into the ll-merge machinery.
The test here uses the "union" merge attribute, which does erroneously
report a successful merge.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-06-11 12:37:04 +09:00
René Scharfe
ca56dadb4b use CALLOC_ARRAY
Add and apply a semantic patch for converting code that open-codes
CALLOC_ARRAY to use it instead.  It shortens the code and infers the
element size automatically.

Signed-off-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-13 16:00:09 -08:00
Jeff King
f5914f4b6b parse_config_key(): return subsection len as size_t
We return the length to a subset of a string using an "int *"
out-parameter. This is fine most of the time, as we'd expect config keys
to be relatively short, but it could behave oddly if we had a gigantic
config key. A more appropriate type is size_t.

Let's switch over, which lets our callers use size_t as appropriate
(they are bound by our type because they must pass the out-parameter as
a pointer). This is mostly just a cleanup to make it clear this code
handles long strings correctly. In practice, our config parser already
chokes on long key names (because of a similar int/size_t mixup!).

When doing an int/size_t conversion, we have to be careful that nobody
was trying to assign a negative value to the variable. I manually
confirmed that for each case here. They tend to just feed the result to
xmemdupz() or similar; in a few cases I adjusted the parameter types for
helper functions to make sure the size_t is preserved.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-10 14:44:29 -07:00
brian m. carlson
2c65d90f75 am: reload .gitattributes after patching it
When applying multiple patches with git am, or when rebasing using the
am backend, it's possible that one of our patches has updated a
gitattributes file. Currently, we cache this information, so if a
file in a subsequent patch has attributes applied, the file will be
written out with the attributes in place as of the time we started the
rebase or am operation, not with the attributes applied by the previous
patch. This problem does not occur when using the -m or -i flags to
rebase.

To ensure we write the correct data into the working tree, expire the
cache after each patch that touches a path ending in ".gitattributes".
Since we load these attributes in multiple separate files, we must
expire them accordingly.

Verify that both the am and rebase code paths work correctly, including
the conflict marker size with am -3.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-03 15:16:18 -07:00
Junio C Hamano
ac193e0e0a Merge branch 'en/merge-path-collision'
Updates for corner cases in merge-recursive.

* en/merge-path-collision:
  t6036: avoid non-portable "cp -a"
  merge-recursive: combine error handling
  t6036, t6043: increase code coverage for file collision handling
  merge-recursive: improve rename/rename(1to2)/add[/add] handling
  merge-recursive: use handle_file_collision for add/add conflicts
  merge-recursive: improve handling for rename/rename(2to1) conflicts
  merge-recursive: fix rename/add conflict handling
  merge-recursive: new function for better colliding conflict resolutions
  merge-recursive: increase marker length with depth of recursion
  t6036, t6042: testcases for rename collision of already conflicting files
  t6042: add tests for consistency in file collision conflict handling
2019-01-04 13:33:32 -08:00
Elijah Newren
b2a7942b8b merge-recursive: increase marker length with depth of recursion
Later patches in this series will modify file collision conflict
handling (e.g. from rename/add and rename/rename(2to1) conflicts) so
that multiply nested conflict markers can arise even before considering
conflicts in the virtual merge base.  Including the virtual merge base
will provide a way to get triply (or higher) nested conflict markers.
This new way to get nested conflict markers will force the need for a
more general mechanism to extend the length of conflict markers in order
to differentiate between different nestings.

Along with this change to conflict marker length handling, we want to
make sure that we don't regress handling for other types of conflicts
with nested conflict markers.  Add a more involved testcase using
merge.conflictstyle=diff3, where not only does the virtual merge base
contain conflicts, but its virtual merge base does as well (i.e. a case
with triply nested conflict markers).  While there are multiple
reasonable ways to handle nested conflict markers in the virtual merge
base for this type of situation, the easiest approach that dovetails
well with the new needs for the file collision conflict handling is to
require that the length of the conflict markers increase with each
subsequent nesting.

Subsequent patches which change the rename/add and rename/rename(2to1)
conflict handling will modify the extra_marker_size flag appropriately
for their new needs.

Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-08 14:23:53 +09:00
Junio C Hamano
11877b9ebe Merge branch 'nd/the-index'
Various codepaths in the core-ish part learn to work on an
arbitrary in-core index structure, not necessarily the default
instance "the_index".

* nd/the-index: (23 commits)
  revision.c: reduce implicit dependency the_repository
  revision.c: remove implicit dependency on the_index
  ws.c: remove implicit dependency on the_index
  tree-diff.c: remove implicit dependency on the_index
  submodule.c: remove implicit dependency on the_index
  line-range.c: remove implicit dependency on the_index
  userdiff.c: remove implicit dependency on the_index
  rerere.c: remove implicit dependency on the_index
  sha1-file.c: remove implicit dependency on the_index
  patch-ids.c: remove implicit dependency on the_index
  merge.c: remove implicit dependency on the_index
  merge-blobs.c: remove implicit dependency on the_index
  ll-merge.c: remove implicit dependency on the_index
  diff-lib.c: remove implicit dependency on the_index
  read-cache.c: remove implicit dependency on the_index
  diff.c: remove implicit dependency on the_index
  grep.c: remove implicit dependency on the_index
  diff.c: remove the_index dependency in textconv() functions
  blame.c: rename "repo" argument to "r"
  combine-diff.c: remove implicit dependency on the_index
  ...
2018-10-19 13:34:02 +09:00
Nguyễn Thái Ngọc Duy
32eaa46883 ll-merge.c: remove implicit dependency on the_index
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-21 09:48:10 -07:00
Torsten Bögershausen
d64324cb60 Make git_check_attr() a void function
git_check_attr() returns always 0.
Remove all the error handling code of the callers, which is never executed.
Change git_check_attr() to be a void function.

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-12 15:15:34 -07:00
Nguyễn Thái Ngọc Duy
7a400a2c02 attr: remove an implicit dependency on the_index
Make the attr API take an index_state instead of assuming the_index in
attr code. All call sites are converted blindly to keep the patch
simple and retain current behavior. Individual call sites may receive
further updates to use the right index instead of the_index.

There is one ugly temporary workaround added in attr.c that needs some
more explanation.

Commit c24f3abace (apply: file commited with CRLF should roundtrip
diff and apply - 2017-08-19) forces one convert_to_git() call to NOT
read the index at all. But what do you know, we read it anyway by
falling back to the_index. When "istate" from convert_to_git is now
propagated down to read_attr_from_array() we will hit segfault
somewhere inside read_blob_data_from_index.

The right way of dealing with this is to kill "use_index" variable and
only follow "istate" but at this stage we are not ready for that:
while most git_attr_set_direction() calls just passes the_index to be
assigned to use_index, unpack-trees passes a different one which is
used by entry.c code, which has no way to know what index to use if we
delete use_index. So this has to be done later.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-13 14:14:42 -07:00
Jeff King
06f46f237a avoid "write_in_full(fd, buf, len) != len" pattern
The return value of write_in_full() is either "-1", or the
requested number of bytes[1]. If we make a partial write
before seeing an error, we still return -1, not a partial
value. This goes back to f6aa66cb95 (write_in_full: really
write in full or return error on disk full., 2007-01-11).

So checking anything except "was the return value negative"
is pointless. And there are a couple of reasons not to do
so:

  1. It can do a funny signed/unsigned comparison. If your
     "len" is signed (e.g., a size_t) then the compiler will
     promote the "-1" to its unsigned variant.

     This works out for "!= len" (unless you really were
     trying to write the maximum size_t bytes), but is a
     bug if you check "< len" (an example of which was fixed
     recently in config.c).

     We should avoid promoting the mental model that you
     need to check the length at all, so that new sites are
     not tempted to copy us.

  2. Checking for a negative value is shorter to type,
     especially when the length is an expression.

  3. Linus says so. In d34cf19b89 (Clean up write_in_full()
     users, 2007-01-11), right after the write_in_full()
     semantics were changed, he wrote:

       I really wish every "write_in_full()" user would just
       check against "<0" now, but this fixes the nasty and
       stupid ones.

     Appeals to authority aside, this makes it clear that
     writing it this way does not have an intentional
     benefit. It's a historical curiosity that we never
     bothered to clean up (and which was undoubtedly
     cargo-culted into new sites).

So let's convert these obviously-correct cases (this
includes write_str_in_full(), which is just a wrapper for
write_in_full()).

[1] A careful reader may notice there is one way that
    write_in_full() can return a different value. If we ask
    write() to write N bytes and get a return value that is
    _larger_ than N, we could return a larger total. But
    besides the fact that this would imply a totally broken
    version of write(), it would already invoke undefined
    behavior. Our internal remaining counter is an unsigned
    size_t, which means that subtracting too many byte will
    wrap it around to a very large number. So we'll instantly
    begin reading off the end of the buffer, trying to write
    gigabytes (or petabytes) of data.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-14 15:17:59 +09:00
Junio C Hamano
50f03c6676 Merge branch 'ab/free-and-null'
A common pattern to free a piece of memory and assign NULL to the
pointer that used to point at it has been replaced with a new
FREE_AND_NULL() macro.

* ab/free-and-null:
  *.[ch] refactoring: make use of the FREE_AND_NULL() macro
  coccinelle: make use of the "expression" FREE_AND_NULL() rule
  coccinelle: add a rule to make "expression" code use FREE_AND_NULL()
  coccinelle: make use of the "type" FREE_AND_NULL() rule
  coccinelle: add a rule to make "type" code use FREE_AND_NULL()
  git-compat-util: add a FREE_AND_NULL() wrapper around free(ptr); ptr = NULL
2017-06-24 14:28:41 -07:00
Junio C Hamano
f31d23a399 Merge branch 'bw/config-h'
Fix configuration codepath to pay proper attention to commondir
that is used in multi-worktree situation, and isolate config API
into its own header file.

* bw/config-h:
  config: don't implicitly use gitdir or commondir
  config: respect commondir
  setup: teach discover_git_directory to respect the commondir
  config: don't include config.h by default
  config: remove git_config_iter
  config: create config.h
2017-06-24 14:28:41 -07:00
Ævar Arnfjörð Bjarmason
e140f7afdd coccinelle: make use of the "expression" FREE_AND_NULL() rule
A follow-up to the existing "expression" rule added in an earlier
change. This manually excludes a few occurrences, mostly things that
resulted in many FREE_AND_NULL() on one line, that'll be manually
fixed in a subsequent change.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-16 12:44:07 -07:00
Brandon Williams
b2141fc1d2 config: don't include config.h by default
Stop including config.h by default in cache.h.  Instead only include
config.h in those files which require use of the config system.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-15 12:56:22 -07:00
Brandon Williams
a33e0b2a77 convert: convert renormalize_buffer to take an index
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-13 11:40:51 -07:00
Junio C Hamano
2aef63d31c attr: convert git_check_attrs() callers to use the new API
The remaining callers are all simple "I have N attributes I am
interested in.  I'll ask about them with various paths one by one".

After this step, no caller to git_check_attrs() remains.  After
removing it, we can extend "struct attr_check" struct with data
that can be used in optimizing the query for the specific N
attributes it contains.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-01 13:46:52 -08:00
Junio C Hamano
7bd18054d2 attr: rename function and struct related to checking attributes
The traditional API to check attributes is to prepare an N-element
array of "struct git_attr_check" and pass N and the array to the
function "git_check_attr()" as arguments.

In preparation to revamp the API to pass a single structure, in
which these N elements are held, rename the type used for these
individual array elements to "struct attr_check_item" and rename
the function to "git_check_attrs()".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-01 13:46:52 -08:00
Junio C Hamano
243a7f0557 Merge branch 'jc/ll-merge-internal'
"git rerere" can get confused by conflict markers deliberately left
by the inner merge step, because they are indistinguishable from
the real conflict markers left by the outermost merge which are
what the end user and "rerere" need to look at.  This was fixed by
making the conflict markers left by the inner merges a bit longer.

* jc/ll-merge-internal:
  t6036: remove pointless test that expects failure
  ll-merge: use a longer conflict marker for internal merge
  ll-merge: fix typo in comment
2016-05-17 14:38:32 -07:00
Junio C Hamano
d694a17986 ll-merge: use a longer conflict marker for internal merge
The primary use of conflict markers is to help the user who resolves
the final (outer) merge by hand to show which part came from which
branch by separating the blocks of lines apart.  When the conflicted
parts from a "virtual ancestor" merge created by merge-recursive
remains in the common ancestor part in the final result, however,
the conflict markers that are the same size as the final merge
become harder to see.

Increase the conflict marker size slightly for these inner merges so
that the markers from the final merge and cruft from internal merge
can be distinguished more easily.

This would help reduce the common issue that prevents "rerere" from
being used on a really complex conflict.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-09 15:42:16 -07:00
Junio C Hamano
ed34567c7b ll-merge: fix typo in comment
When a944af1d (merge: teach -Xours/-Xtheirs to binary ll-merge
driver, 2012-09-08) introduced FAVOR_OURS/FAVOR_THEIRS to the binary
ll-merge driver, it changed what happens to the merge result for the
outer merge, and updated the comment from:

    The tentative merge result is "ours" for the final round, or
    common ancestor for an internal merge.  Still return "conflicted
    merge" status.

to

    The tentative merge result is the or common ancestor for an
    internal merge.

What happened is obvious.  I noticed the lack of definitive article
in front of "common" but failed to remove "or".  Also I forgot to
describe what I did for the final merge, probably because I was
satisified by the description in the log message.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-04-14 15:17:56 -07:00
Jeff King
3733e69464 use xmallocz to avoid size arithmetic
We frequently allocate strings as xmalloc(len + 1), where
the extra 1 is for the NUL terminator. This can be done more
simply with xmallocz, which also checks for integer
overflow.

There's no case where switching xmalloc(n+1) to xmallocz(n)
is wrong; the result is the same length, and malloc made no
guarantees about what was in the buffer anyway. But in some
cases, we can stop manually placing NUL at the end of the
allocated buffer. But that's only safe if it's clear that
the contents will always fill the buffer.

In each case where this patch does so, I manually examined
the control flow, and I tried to err on the side of caution.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-02-22 14:51:09 -08:00
Junio C Hamano
78891795df Merge branch 'jk/war-on-sprintf'
Many allocations that is manually counted (correctly) that are
followed by strcpy/sprintf have been replaced with a less error
prone constructs such as xstrfmt.

Macintosh-specific breakage was noticed and corrected in this
reroll.

* jk/war-on-sprintf: (70 commits)
  name-rev: use strip_suffix to avoid magic numbers
  use strbuf_complete to conditionally append slash
  fsck: use for_each_loose_file_in_objdir
  Makefile: drop D_INO_IN_DIRENT build knob
  fsck: drop inode-sorting code
  convert strncpy to memcpy
  notes: document length of fanout path with a constant
  color: add color_set helper for copying raw colors
  prefer memcpy to strcpy
  help: clean up kfmclient munging
  receive-pack: simplify keep_arg computation
  avoid sprintf and strcpy with flex arrays
  use alloc_ref rather than hand-allocating "struct ref"
  color: add overflow checks for parsing colors
  drop strcpy in favor of raw sha1_to_hex
  use sha1_to_hex_r() instead of strcpy
  daemon: use cld->env_array when re-spawning
  stat_tracking_info: convert to argv_array
  http-push: use an argv_array for setup_revisions
  fetch-pack: use argv_array for index-pack / unpack-objects
  ...
2015-10-20 15:24:01 -07:00
Junio C Hamano
11a458befc Sync with 2.4.10 2015-09-28 15:33:56 -07:00
Jeff King
dcd1742e56 xdiff: reject files larger than ~1GB
The xdiff code is not prepared to handle extremely large
files. It uses "int" in many places, which can overflow if
we have a very large number of lines or even bytes in our
input files. This can cause us to produce incorrect diffs,
with no indication that the output is wrong. Or worse, we
may even underallocate a buffer whose size is the result of
an overflowing addition.

We're much better off to tell the user that we cannot diff
or merge such a large file. This patch covers both cases,
but in slightly different ways:

  1. For merging, we notice the large file and cleanly fall
     back to a binary merge (which is effectively "we cannot
     merge this").

  2. For diffing, we make the binary/text distinction much
     earlier, and in many different places. For this case,
     we'll use the xdi_diff as our choke point, and reject
     any diff there before it hits the xdiff code.

     This means in most cases we'll die() immediately after.
     That's not ideal, but in practice we shouldn't
     generally hit this code path unless the user is trying
     to do something tricky. We already consider files
     larger than core.bigfilethreshold to be binary, so this
     code would only kick in when that is circumvented
     (either by bumping that value, or by using a
     .gitattribute to mark a file as diffable).

     In other words, we can avoid being "nice" here, because
     there is already nice code that tries to do the right
     thing. We are adding the suspenders to the nice code's
     belt, so notice when it has been worked around (both to
     protect the user from malicious inputs, and because it
     is better to die() than generate bogus output).

The maximum size was chosen after experimenting with feeding
large files to the xdiff code. It's just under a gigabyte,
which leaves room for two obvious cases:

  - a diff3 merge conflict result on files of maximum size X
    could be 3*X plus the size of the markers, which would
    still be only about 3G, which fits in a 32-bit int.

  - some of the diff code allocates arrays of one int per
    record. Even if each file consists only of blank lines,
    then a file smaller than 1G will have fewer than 1G
    records, and therefore the int array will fit in 4G.

Since the limit is arbitrary anyway, I chose to go under a
gigabyte, to leave a safety margin (e.g., we would not want
to overflow by allocating "(records + 1) * sizeof(int)" or
similar.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-28 14:57:23 -07:00
Jeff King
5096d4909f convert trivial sprintf / strcpy calls to xsnprintf
We sometimes sprintf into fixed-size buffers when we know
that the buffer is large enough to fit the input (either
because it's a constant, or because it's numeric input that
is bounded in size). Likewise with strcpy of constant
strings.

However, these sites make it hard to audit sprintf and
strcpy calls for buffer overflows, as a reader has to
cross-reference the size of the array with the input. Let's
use xsnprintf instead, which communicates to a reader that
we don't expect this to overflow (and catches the mistake in
case we do).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-09-25 10:18:18 -07:00
Junio C Hamano
ef45bb1f81 ll-merge: pass the original path to external drivers
The interface to custom low-level merge driver was modeled to be
capable of driving programs like "merge" (from the RCS suite) that
can produce result solely by looking at three files that hold
contents of common ancestor, ours and theirs.  The information we
feed to the external drivers via the command line placeholders %O,
%A, and %B were designed to be purely about contents by giving
names of the temporary files that hold these variants without
exposing the original pathname.  No matter where the result goes,
merging the same three variants should produce the same result,
contents is the king, that is the Git way.

The external driver interface, however, is meant to help people to
step outside the Git worldview, and sometimes people want to know
the final path that the resulting merged contents would be stored
in.  Expose this to the external drivers via a new placeholder %P.

Requested-by: Andreas Gondek
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-06-04 15:36:32 -07:00
Tanay Abhra
6ea358f784 ll-merge.c: refactor read_merge_config() to use git_config_string()
There is one slight behavior change, previously "merge.default"
silently ignored a NULL value and didn't raise any error. But,
in the same function, all other values raise an error on a NULL
value. So to conform with other call sites in Git, a NULL value
for "merge.default" raises an error.

The the new config-set API is not very useful here, because much of
the function is dedicated to processing "merge.<name>.variable",
which the new API does not handle well.  If it were for variables
like, "merge.summary", "merge.tool", and "merge.verbosity", we could
use the new API.

Signed-off-by: Tanay Abhra <tanayabh@gmail.com>
Reviewed-by: Matthieu Moy <Matthieu.Moy@imag.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2014-08-13 12:36:21 -07:00
Jeff King
d731f0ade1 convert some config callbacks to parse_config_key
These callers can drop some inline pointer arithmetic and
magic offset constants, making them more readable and less
error-prone (those constants had to match the lengths of
strings, but there is no automatic verification of that
fact).

The "ep" pointer (presumably for "end pointer"), which
points to the final key segment of the config variable, is
given the more standard name "key" to describe its function
rather than its derivation.

Signed-off-by: Jeff King <peff@peff.net>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2013-01-23 08:41:50 -08:00
Junio C Hamano
e6d29a4b47 Merge branch 'jc/ll-merge-binary-ours'
"git merge -Xtheirs" did not help content-level merge of binary
files; it should just take their version.  Also "*.jpg binary" in
the attributes did not imply they should use the binary ll-merge
driver.

* jc/ll-merge-binary-ours:
  ll-merge: warn about inability to merge binary files only when we can't
  attr: "binary" attribute should choose built-in "binary" merge driver
  merge: teach -Xours/-Xtheirs to binary ll-merge driver
2012-09-14 21:39:56 -07:00
Junio C Hamano
e0e2065f74 ll-merge: warn about inability to merge binary files only when we can't
When a path being merged is auto detected to be a binary file, we
warned "Cannot merge binary files" before switching to activate the
binary ll-merge driver.  When we are merging with the -Xours/theirs
option, however, we know what the "clean" merge result is, and the
warning is inappropriate.

In addition, when the path is explicitly marked as a binary file,
this warning was not issued, even though without -Xours/theirs, we
cannot cleanly automerge such a path, which was inconsistent.

Move the warning code from ll_xdl_merge() to ll_binary_merge(), and
issue the message only when we cannot cleanly automerge.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-09-12 02:01:52 -07:00
Junio C Hamano
a944af1d86 merge: teach -Xours/-Xtheirs to binary ll-merge driver
The (discouraged) -Xours/-Xtheirs modes of merge are supposed to
give a quick and dirty way to come up with a random mixture of
cleanly merged parts and punted conflict resolution to take contents
from one side in conflicting parts.  These options however were only
passed down to the low level merge driver for text.

Teach the built-in binary merge driver to notice them as well.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-09-08 21:27:19 -07:00
Pete Wyckoff
82247e9bd5 remove superfluous newlines in error messages
The error handling routines add a newline.  Remove
the duplicate ones in error messages.

Signed-off-by: Pete Wyckoff <pw@padd.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2012-04-30 15:45:51 -07:00
Michael Haggerty
d932f4eb9f Rename git_checkattr() to git_check_attr()
Suggested by: Junio Hamano <gitster@pobox.com>

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-08-04 15:53:21 -07:00
Jonathan Nieder
4fb40c2b82 ll-merge: simplify opts == NULL case
As long as sizeof(struct ll_merge_options) is small, there is not
much reason not to keep a copy of the default merge options in the BSS
section.  In return, we get clearer code and one less stack frame in
the opts == NULL case.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2011-01-15 20:34:14 -08:00
Justin Frankel
58a1ece478 merge-recursive --patience
Teach the merge-recursive strategy a --patience option to use the
"patience diff" algorithm, which tends to improve results when
cherry-picking a patch that reorders functions at the same time as
refactoring them.

To support this, struct merge_options and ll_merge_options gain an
xdl_opts member, so programs can use arbitrary xdiff flags (think
"XDF_IGNORE_WHITESPACE") in a git-aware merge.

git merge and git rebase can be passed the -Xpatience option to
use this.

[jn: split from --ignore-space patch; with documentation]

Signed-off-by: Justin Frankel <justin@cockos.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-26 09:20:03 -07:00
Jonathan Nieder
712516bcac ll-merge: replace flag argument with options struct
Keeping track of the flag bits is proving more trouble than it's
worth.  Instead, use a pointer to an options struct like most similar
APIs do.

Callers with no special requests can pass NULL to request the default
options.

Cc: Bert Wesarg <bert.wesarg@googlemail.com>
Cc: Avery Pennarun <apenwarr@gmail.com>
Helped-by: Justin Frankel <justin@cockos.com>
Helped-by: Bert Wesarg <bert.wesarg@googlemail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-26 09:18:51 -07:00
Jonathan Nieder
18b037a5b6 ll-merge: let caller decide whether to renormalize
Add a “renormalize” bit to the ll-merge options word so callers can
decide on a case-by-case basis whether the merge is likely to have
overlapped with a change in smudge/clean rules.

This reveals a few commands that have not been taking that situation
into account, though it does not fix them.

No functional change intended.

Cc: Eyvind Bernhardsen <eyvind.bernhardsen@gmail.com>
Improved-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-06 09:20:01 -07:00
Jonathan Nieder
73cf7f713d ll-merge: make flag easier to populate
ll_merge() takes its options in a flag word, which has a few
advantages:

 - options flags can be cheaply passed around in registers, while
   an option struct passed by pointer cannot;

 - callers can easily pass 0 without trouble for no options,
   while an option struct passed by value would not allow that.

The downside is that code to populate and access the flag word can be
somewhat opaque.  Mitigate that with a few macros.

Cc: Avery Pennarun <apenwarr@gmail.com>
Cc: Bert Wesarg <bert.wesarg@googlemail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2010-08-06 09:20:01 -07:00