Commit graph

317 commits

Author SHA1 Message Date
Junio C Hamano
1725c4c64b Merge branch 'jk/apply-binary-hunk-parsing-fix' into maint
"git apply" miscounted the bytes and failed to read to the end of
binary hunks.

* jk/apply-binary-hunk-parsing-fix:
  apply: keep buffer/size pair in sync when parsing binary hunks
2021-10-12 13:51:37 -07:00
Junio C Hamano
c76fcf3e46 Merge branch 'jc/trivial-threeway-binary-merge'
The "git apply -3" code path learned not to bother the lower level
merge machinery when the three-way merge can be trivially resolved
without the content level merge.

* jc/trivial-threeway-binary-merge:
  apply: resolve trivial merge without hitting ll-merge with "--3way"
2021-09-15 13:15:26 -07:00
Junio C Hamano
57f183b698 apply: resolve trivial merge without hitting ll-merge with "--3way"
The ll_binary_merge() function assumes that the ancestor blob is
different from either side of the new versions, and always fails
the merge in conflict, unless -Xours or -Xtheirs is in effect.

The normal "merge" machineries all resolve the trivial cases
(e.g. if our side changed while their side did not, the result
is ours) without triggering the file-level merge drivers, so the
assumption is warranted.

The code path in "git apply --3way", however, does not check for
the trivial three-way merge situation and always calls the
file-level merge drivers.  This used to be perfectly OK back
when we always first attempted a straight patch application and
used the three-way code path only as a fallback.  Any binary
patch that can be applied as a trivial three-way merge (e.g. the
patch is based exactly on the version we happen to have) would
always cleanly apply, so the ll_binary_merge() that is not
prepared to see the trivial case would not have to handle such a
case.

This no longer is true after we made "--3way" to mean "first try
three-way and then fall back to straight application", and made
"git apply -3" on a binary patch that is based on the current
version no longer apply.

Teach "git apply -3" to first check for the trivial merge cases
and resolve them without hitting the file-level merge drivers.

Signed-off-by: Jerry Zhang <jerry@skydio.com>
[jc: stolen tests from Jerry's patch]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-05 15:39:02 -07:00
Junio C Hamano
7e3b9d1534 Merge branch 'jk/apply-binary-hunk-parsing-fix'
"git apply" miscounted the bytes and failed to read to the end of
binary hunks.

* jk/apply-binary-hunk-parsing-fix:
  apply: keep buffer/size pair in sync when parsing binary hunks
2021-08-30 16:06:04 -07:00
Jeff King
46d723ce57 apply: keep buffer/size pair in sync when parsing binary hunks
We parse through binary hunks by looping through the buffer with code
like:

    llen = linelen(buffer, size);

    ...do something with the line...

    buffer += llen;
    size -= llen;

However, before we enter the loop, there is one call that increments
"buffer" but forgets to decrement "size". As a result, our "size" is off
by the length of that line, and subsequent calls to linelen() may look
past the end of the buffer for a newline.

The fix is easy: we just need to decrement size as we do elsewhere.

This bug goes all the way back to 0660626caf (binary diff: further
updates., 2006-05-05). Presumably nobody noticed because it only
triggers if the patch is corrupted, and even then we are often "saved"
by luck. We use a strbuf to store the incoming patch, so we overallocate
there, plus we add a 16-byte run of NULs as slop for memory comparisons.
So if this happened accidentally, the common case is that we'd just read
a few uninitialized bytes from the end of the strbuf before producing
the expected "this patch is corrupted" error complaint.

However, it is possible to carefully construct a case which reads off
the end of the buffer. The included test does so. It will pass both
before and after this patch when run normally, but using a tool like
ASan shows that we get an out-of-bounds read before this patch, but not
after.

Reported-by: Xingman Chen <xichixingman@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-08-10 11:38:13 -07:00
Ævar Arnfjörð Bjarmason
bc40dfb10a string-list.h users: change to use *_{nodup,dup}()
Change all in-tree users of the string_list_init(LIST, BOOL) API to
use string_list_init_{nodup,dup}(LIST) instead.

As noted in the preceding commit let's leave the now-unused
string_list_init() wrapper in-place for any in-flight users, it can be
removed at some later date.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-07-01 12:32:22 -07:00
Junio C Hamano
6d99f31dda Merge branch 'jz/apply-3way-first-message-fix'
When we swapped the order of --3way fallback, we forgot to adjust
the message we give when the first method fails and the second
method is attempted (which used to be "direct application failed
hence we try 3way", now it is the other way around).

* jz/apply-3way-first-message-fix:
  apply: adjust messages to account for --3way changes
2021-05-07 12:47:38 +09:00
Jerry Zhang
526705fd3d apply: adjust messages to account for --3way changes
"git apply" specifically calls out when it is falling back to 3way
merge application.  Since the order changed to preferring 3way and
falling back to direct application, continue that behavior by
printing whenever 3way fails and git has to fall back.

Signed-off-by: Jerry Zhang <jerry@skydio.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-29 12:27:45 +09:00
Jerry Zhang
c0c2a37ac2 git-apply: allow simultaneous --cached and --3way options
"git apply" does not allow "--cached" and "--3way" to be used
together, since "--3way" writes conflict markers into the working
tree.

Allow "git apply" to accept "--cached" and "--3way" at the same
time.  When a single file auto-resolves cleanly, the result is
placed in the index at stage #0 and the command exits with 0 status.

For a file that has a conflict which cannot be cleanly
auto-resolved, the original contents from common ancestor (stage
conflict at the content level, and the command exists with non-zero
status, because there is no place (like the working tree) to leave a
half-resolved merge for the user to resolve.

The user can use `git diff` to view the contents of the conflict, or
`git checkout -m -- .` to regenerate the conflict markers in the
working directory.

Don't attempt rerere in this case since it depends on conflict
markers written to file for its database storage and lookup. There
would be two main changes required to get rerere working:

1. Allow the rerere api to accept in memory object rather than
   files, which would allow us to pass in the conflict markers
   contained in the result from ll_merge().

2. Rerere can't write to the working directory, so it would have to
   apply the result to cache stage #0 directly. A flag would be
   needed to control this.

Signed-off-by: Jerry Zhang <jerry@skydio.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-07 22:20:33 -07:00
Jerry Zhang
923cd87ac8 git-apply: try threeway first when "--3way" is used
The apply_fragments() method of "git apply"
can silently apply patches incorrectly if
a file has repeating contents. In these
cases a three-way merge is capable of applying
it correctly in more situations, and will
show a conflict rather than applying it
incorrectly. However, because the patches
apply "successfully" using apply_fragments(),
git will never fall back to the merge, even
if the "--3way" flag is used, and the user has
no way to ensure correctness by forcing the
three-way merge method.

Change the behavior so that when "--3way" is used,
git will always try the three-way merge first and
will only fall back to apply_fragments() in cases
where blobs are not available or some other error
(but not in the case of a merge conflict).

Since user-facing results will be different,
this has backwards compatibility implications
for users depending on the old behavior. In
addition, the three-way merge will be slower
than direct patch application.

Signed-off-by: Jerry Zhang <jerry@skydio.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-06 17:11:41 -07:00
Junio C Hamano
c47679d040 Merge branch 'mt/parallel-checkout-part-1'
Preparatory API changes for parallel checkout.

* mt/parallel-checkout-part-1:
  entry: add checkout_entry_ca() taking preloaded conv_attrs
  entry: move conv_attrs lookup up to checkout_entry()
  entry: extract update_ce_after_write() from write_entry()
  entry: make fstat_output() and read_blob_entry() public
  entry: extract a header file for entry.c functions
  convert: add classification for conv_attrs struct
  convert: add get_stream_filter_ca() variant
  convert: add [async_]convert_to_working_tree_ca() variants
  convert: make convert_attrs() and convert structs public
2021-04-02 14:43:14 -07:00
Matheus Tavares
d052cc0382 entry: extract a header file for entry.c functions
The declarations of entry.c's public functions and structures currently
reside in cache.h. Although not many, they contribute to the size of
cache.h and, when changed, cause the unnecessary recompilation of
modules that don't really use these functions. So let's move them to a
new entry.h header. While at it let's also move a comment related to
checkout_entry() from entry.c to entry.h as it's more useful to describe
the function there.

Original-patch-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-23 10:34:05 -07: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
Junio C Hamano
3517022568 Merge branch 'ab/unreachable-break'
Code clean-up.

* ab/unreachable-break:
  style: do not "break" in switch() after "return"
2020-12-18 15:15:18 -08:00
Ævar Arnfjörð Bjarmason
56f56ac50b style: do not "break" in switch() after "return"
Remove this unreachable code. It was found by SunCC, it's found by a
non-fatal warning emitted by SunCC. It's one of the things it's more
vehement about than GCC & Clang.

It complains about a lot of other similarly unreachable code, e.g. a
BUG(...) without a "return", and a "return 0" after a long if/else,
both of whom have "return" statements. Those are also genuine
redundancies to a compiler, but arguably make the code a bit easier to
read & less fragile to maintain.

These return/break cases are just unnecessary however, and as seen
here the surrounding code just did a plain "return" without a "break"
already.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-15 16:32:50 -08:00
Junio C Hamano
9b3b4adb3f Merge branch 'mt/do-not-use-scld-in-working-tree'
"git apply" adjusted the permission bits of working-tree files and
directories according core.sharedRepository setting by mistake and
for a long time, which has been corrected.

* mt/do-not-use-scld-in-working-tree:
  apply: don't use core.sharedRepository to create working tree files
2020-12-08 15:11:20 -08:00
Matheus Tavares
eb3c027e17 apply: don't use core.sharedRepository to create working tree files
core.sharedRepository defines which permissions Git should set when
creating files in $GIT_DIR, so that the repository may be shared with
other users. But (in its current form) the setting shouldn't affect how
files are created in the working tree. This is not respected by apply
and am (which uses apply), when creating leading directories:

$ cat d.patch
 diff --git a/d/f b/d/f
 new file mode 100644
 index 0000000..e69de29

Apply without the setting:
$ umask 0077
$ git apply d.patch
$ ls -ld d
 drwx------

Apply with the setting:
$ umask 0077
$ git -c core.sharedRepository=0770 apply d.patch
$ ls -ld d
 drwxrws---

Only the leading directories are affected. That's because they are
created with safe_create_leading_directories(), which calls
adjust_shared_perm() to set the directories' permissions based on
core.sharedRepository. To fix that, let's introduce a variant of this
function that ignores the setting, and use it in apply. Also add a
regression test and a note in the function documentation about the use
of each variant according to the destination (working tree or git
dir).

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-02 14:35:51 -08:00
Jonathan Tan
b0f266de11 apply: when -R, also reverse list of sections
A patch changing a symlink into a file is written with 2 sections (in
the code, represented as "struct patch"): firstly, the deletion of the
symlink, and secondly, the creation of the file. When applying that
patch with -R, the sections are reversed, so we get:

 (1) creation of a symlink, then
 (2) deletion of a file.

This causes an issue when the "deletion of a file" section is checked,
because Git observes that the so-called file is not a file but a
symlink, resulting in a "wrong type" error message.

What we want is:

 (1) deletion of a file, then
 (2) creation of a symlink.

In the code, this is reflected in the behavior of previous_patch() when
invoked from check_preimage() when the deletion is checked. Creation
then deletion means that when the deletion is checked, previous_patch()
returns the creation section, triggering a mode conflict resulting in
the "wrong type" error message. But deletion then creation means that
when the deletion is checked, previous_patch() returns NULL, so the
deletion mode is checked against lstat, which is what we want.

There are also other ways a patch can contain 2 sections referencing the
same file, for example, in 7a07841c0b ("git-apply: handle a patch that
touches the same path more than once better", 2008-06-27). "git apply
-R" fails in the same way, and this commit makes this case succeed.

Therefore, when building the list of sections, build them in reverse
order (by adding to the front of the list instead of the back) when -R
is passed.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-20 15:21:41 -07:00
Junio C Hamano
0d9a8e33f9 Merge branch 'jk/leakfix'
Code clean-up.

* jk/leakfix:
  submodule--helper: fix leak of core.worktree value
  config: fix leak in git_config_get_expiry_in_days()
  config: drop git_config_get_string_const()
  config: fix leaks from git_config_get_string_const()
  checkout: fix leak of non-existent branch names
  submodule--helper: use strbuf_release() to free strbufs
  clear_pattern_list(): clear embedded hashmaps
2020-08-27 14:04:49 -07:00
Junio C Hamano
ca81676a10 Merge branch 'rp/apply-cached-with-i-t-a'
Recent versions of "git diff-files" shows a diff between the index
and the working tree for "intent-to-add" paths as a "new file"
patch; "git apply --cached" should be able to take "git diff-files"
and should act as an equivalent to "git add" for the path, but the
command failed to do so for such a path.

* rp/apply-cached-with-i-t-a:
  t4140: test apply with i-t-a paths
  apply: make i-t-a entries never match worktree
  apply: allow "new file" patches on i-t-a entries
2020-08-17 17:02:46 -07:00
Jeff King
9a53219f69 config: drop git_config_get_string_const()
As evidenced by the leak fixes in the previous commit, the "const" in
git_config_get_string_const() clearly misleads people into thinking that
it does not allocate a copy of the string. We can fix this by renaming
it, but it's easier still to just drop it. Of the four remaining
callers:

  - The one in git_config_parse_expiry() still needs to allocate, since
    that's what its callers expect. We can just use the non-const
    version and cast our pointer. Slightly ugly, but the damage is
    contained in one spot.

  - The two in apply are writing to global "const char *" variables, and
    need to continue allocating. We often mark these as const because we
    assign default string literals to them. But in this case we don't do
    that, so we can just declare them as real "char *" pointers and use
    the non-const version.

  - The call in checkout doesn't actually need a copy; it can just use
    the non-allocating "tmp" version of the function.

The function is also mentioned in the MyFirstContribution document. We
can swap that call out for the non-allocating "tmp" variant, which fits
well in the example given.

We'll drop the "configset" and "repo" variants, as well (which are
unused).

Note that this frees up the "const" name, so we could rename the "tmp"
variant back to that. But let's give some time for topics in flight to
adapt to the new code before doing so (if we do it too soon, the
function semantics will change but the compiler won't alert us).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-17 15:35:47 -07:00
Raymond E. Pasco
e3cc41b4f9 apply: make i-t-a entries never match worktree
By definition, an intent-to-add index entry can never match the
worktree, because worktrees have no concept of intent-to-add entries.
Therefore, "apply --index" should always fail on intent-to-add paths.

Because check_preimage() calls verify_index_match(), it already fails
for patches other than creation patches, which check_preimage() ignores.
This patch adds a check to check_preimage()'s rough equivalent for
creation patches, check_to_create().

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-09 11:00:46 -07:00
Raymond E. Pasco
7cfde3fa0f apply: allow "new file" patches on i-t-a entries
diff-files recently changed to treat changes to paths marked "intent to
add" in the index as new file diffs rather than diffs from the empty
blob.  However, apply refuses to apply new file diffs on top of existing
index entries, except in the case of renames. This causes "git add -p",
which uses apply, to fail when attempting to stage hunks from a file
when intent to add has been recorded.

This changes the logic in check_to_create() which checks if an entry
already exists in an index in two ways: first, we only search for an
index entry at all if ok_if_exists is false; second, we check for the
CE_INTENT_TO_ADD flag on any index entries we find and allow the apply
to proceed if it is set.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Raymond E. Pasco <ray@ameretat.dev>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-06 13:07:52 -07:00
Jonathan Tan
3318238db9 apply: do not lazy fetch when applying binary
When applying a binary patch, as an optimization, "apply" checks if the
postimage is already present. During this fetch, it is perfectly
expected for the postimage not to be present, so there is no need to
lazy-fetch missing objects. Teach "apply" not to lazy-fetch in this
case.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-06 13:01:02 -07:00
Denton Liu
203c85339f Use OPT_CALLBACK and OPT_CALLBACK_F
In the codebase, there are many options which use OPTION_CALLBACK in a
plain ol' struct definition. However, we have the OPT_CALLBACK and
OPT_CALLBACK_F macros which are meant to abstract these plain struct
definitions away. These macros are useful as they semantically signal to
developers that these are just normal callback option with nothing fancy
happening.

Replace plain struct definitions of OPTION_CALLBACK with OPT_CALLBACK or
OPT_CALLBACK_F where applicable. The heavy lifting was done using the
following (disgusting) shell script:

	#!/bin/sh

	do_replacement () {
		tr '\n' '\r' |
			sed -e 's/{\s*OPTION_CALLBACK,\s*\([^,]*\),\([^,]*\),\([^,]*\),\([^,]*\),\([^,]*\),\s*0,\(\s*[^[:space:]}]*\)\s*}/OPT_CALLBACK(\1,\2,\3,\4,\5,\6)/g' |
			sed -e 's/{\s*OPTION_CALLBACK,\s*\([^,]*\),\([^,]*\),\([^,]*\),\([^,]*\),\([^,]*\),\([^,]*\),\(\s*[^[:space:]}]*\)\s*}/OPT_CALLBACK_F(\1,\2,\3,\4,\5,\6,\7)/g' |
			tr '\r' '\n'
	}

	for f in $(git ls-files \*.c)
	do
		do_replacement <"$f" >"$f.tmp"
		mv "$f.tmp" "$f"
	done

The result was manually inspected and then reformatted to match the
style of the surrounding code. Finally, using
`git grep OPTION_CALLBACK \*.c`, leftover results which were not handled
by the script were manually transformed.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-04-28 10:47:10 -07:00
brian m. carlson
ab90ecae99 convert: permit passing additional metadata to filter processes
There are a variety of situations where a filter process can make use of
some additional metadata.  For example, some people find the ident
filter too limiting and would like to include the commit or the branch
in their smudged files.  This information isn't available during
checkout as HEAD hasn't been updated at that point, and it wouldn't be
available in archives either.

Let's add a way to pass this metadata down to the filter.  We pass the
blob we're operating on, the treeish (preferring the commit over the
tree if one exists), and the ref we're operating on.  Note that we won't
pass this information in all cases, such as when renormalizing or when
we're performing diffs, since it doesn't make sense in those cases.

The data we currently get from the filter process looks like the
following:

  command=smudge
  pathname=git.c
  0000

With this change, we'll get data more like this:

  command=smudge
  pathname=git.c
  refname=refs/tags/v2.25.1
  treeish=c522f061d551c9bb8684a7c3859b2ece4499b56b
  blob=7be7ad34bd053884ec48923706e70c81719a8660
  0000

There are a couple things to note about this approach.  For operations
like checkout, treeish will always be a commit, since we cannot check
out individual trees, but for other operations, like archive, we can end
up operating on only a particular tree, so we'll provide only a tree as
the treeish.  Similar comments apply for refname, since there are a
variety of cases in which we won't have a ref.

This commit wires up the code to print this information, but doesn't
pass any of it at this point.  In a future commit, we'll have various
code paths pass the actual useful data down.

Signed-off-by: brian m. carlson <bk2204@github.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-16 11:37:02 -07:00
Matheus Tavares
2dcde20e1c sha1-file: pass git_hash_algo to hash_object_file()
Allow hash_object_file() to work on arbitrary repos by introducing a
git_hash_algo parameter. Change callers which have a struct repository
pointer in their scope to pass on the git_hash_algo from the said repo.
For all other callers, pass on the_hash_algo, which was already being
used internally at hash_object_file(). This functionality will be used
in the following patch to make check_object_signature() be able to work
on arbitrary repos (which, in turn, will be used to fix an
inconsistency at object.c:parse_object()).

Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-31 10:45:39 -08:00
Junio C Hamano
011fc2e88e Merge branch 'js/add-i-a-bit-more-tests'
Test coverage update in preparation for further work on "git add -i".

* js/add-i-a-bit-more-tests:
  apply --allow-overlap: fix a corner case
  git add -p: use non-zero exit code when the diff generation failed
  t3701: verify that the diff.algorithm config setting is handled
  t3701: verify the shown messages when nothing can be added
  t3701: add a test for the different `add -p` prompts
  t3701: avoid depending on the TTY prerequisite
  t3701: add a test for advanced split-hunk editing
2019-12-16 13:08:47 -08:00
Junio C Hamano
3b3d9ea6a8 Merge branch 'jk/lore-is-the-archive'
Doc update for the mailing list archiving and nntp service.

* jk/lore-is-the-archive:
  doc: replace public-inbox links with lore.kernel.org
  doc: recommend lore.kernel.org over public-inbox.org
2019-12-06 15:09:23 -08:00
Johannes Schindelin
b4bbbbd5a2 apply --allow-overlap: fix a corner case
Yes, yes, this is supposed to be only a band-aid option for `git add -p`
not Doing The Right Thing. But as long as we carry the `--allow-overlap`
option, we might just as well get it right.

This fixes the case where one hunk inserts a line before the first line,
and is followed by a hunk whose context overlaps with the first one's
and which appends a line at the end.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06 08:57:34 -08:00
Junio C Hamano
d3096d2ba6 Merge branch 'en/doc-typofix'
Docfix.

* en/doc-typofix:
  Fix spelling errors in no-longer-updated-from-upstream modules
  multimail: fix a few simple spelling errors
  sha1dc: fix trivial comment spelling error
  Fix spelling errors in test commands
  Fix spelling errors in messages shown to users
  Fix spelling errors in names of tests
  Fix spelling errors in comments of testcases
  Fix spelling errors in code comments
  Fix spelling errors in documentation outside of Documentation/
  Documentation: fix a bunch of typos, both old and new
2019-12-01 09:04:35 -08:00
Jeff King
3eae30e464 doc: replace public-inbox links with lore.kernel.org
Since we're now recommending lore.kernel.org (and because the
public-inbox.org domain might eventually go away), let's update our
internal references to use it, too. That future-proofs our references,
and sets the example we want people to follow.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-30 09:12:26 -08:00
Junio C Hamano
eff313f8a7 Merge branch 'dl/apply-3way-diff3'
"git apply --3way" learned to honor merge.conflictStyle
configuration variable, like merges would.

* dl/apply-3way-diff3:
  apply: respect merge.conflictStyle in --3way
  t4108: demonstrate bug in apply
  t4108: use `test_config` instead of `git config`
  t4108: remove git command upstream of pipe
  t4108: replace create_file with test_write_lines
2019-11-10 18:02:15 +09:00
Elijah Newren
15beaaa3d1 Fix spelling errors in code comments
Reported-by: Jens Schleusener <Jens.Schleusener@fossies.org>
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10 16:00:54 +09:00
Denton Liu
091489d068 apply: respect merge.conflictStyle in --3way
Before, when doing a 3-way merge, the merge.conflictStyle option was not
respected and the "merge" style was always used, even if "diff3" was
specified.

Call git_xmerge_config() at the end of git_apply_config() so that the
merge.conflictStyle config is read.

Signed-off-by: Denton Liu <liu.denton@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-24 11:32:53 +09:00
Junio C Hamano
b6d712fa4e Merge branch 'tg/range-diff-output-update'
"git range-diff" failed to handle mode-only change, which has been
corrected.

* tg/range-diff-output-update:
  range-diff: don't segfault with mode-only changes
2019-10-15 13:48:02 +09:00
Thomas Gummerer
2b6a9b13ca range-diff: don't segfault with mode-only changes
In ef283b3699 ("apply: make parse_git_diff_header public", 2019-07-11)
the 'parse_git_diff_header' function was made public and useable by
callers outside of apply.c.

However it was missed that its (then) only caller, 'find_header' did
some error handling, and completing 'struct patch' appropriately.

range-diff then started using this function, and tried to handle this
appropriately itself, but fell short in some cases.  This in turn
would lead to range-diff segfaulting when there are mode-only changes
in a range.

Move the error handling and completing of the struct into the
'parse_git_diff_header' function, so other callers can take advantage
of it.  This fixes the segfault in 'git range-diff'.

Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-09 10:41:11 +09:00
Junio C Hamano
c8ada15456 Merge branch 'bc/reread-attributes-during-rebase'
The "git am" based backend of "git rebase" ignored the result of
updating ".gitattributes" done in one step when replaying
subsequent steps.

* bc/reread-attributes-during-rebase:
  am: reload .gitattributes after patching it
  path: add a function to check for path suffix
2019-09-09 12:26:40 -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
Thomas Gummerer
ef283b3699 apply: make parse_git_diff_header public
Make 'parse_git_header()' (renamed to 'parse_git_diff_header()') a
"public" function in apply.h, so we can re-use it in range-diff in a
subsequent commit.  We're renaming the function to make it clearer in
other parts of the codebase that we're talking about a diff header and
not just any header.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11 14:29:27 -07:00
Thomas Gummerer
80e1841232 apply: only pass required data to gitdiff_* functions
Currently the 'gitdiff_*()' functions take 'struct apply_state' as
parameter, even though they only needs the root, linenr and p_value
from that struct.

These functions are in the callchain of 'parse_git_header()', which we
want to make more generally useful in a subsequent commit.  To make
that happen we only want to pass in the required data to
'parse_git_header()', and not the whole 'struct apply_state', and thus
we want functions in the callchain of 'parse_git_header()' to only
take arguments they really need.

As these functions are called in a loop using their function pointers,
each function needs to be passed all the parameters even if only one
of the functions actually needs it.  We therefore pass this data along
in a struct to avoid adding too many unused parameters to each
function and making the code very verbose in the process.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-11 14:29:27 -07:00
Thomas Gummerer
877a833b51 apply: only pass required data to find_name_*
Currently the 'find_name_*()' functions take 'struct apply_state' as
parameter, even though they only need the 'root' member from that
struct.

These functions are in the callchain of 'parse_git_header()', which we
want to make more generally useful in a subsequent commit.  To make
that happen we only want to pass in the required data to
'parse_git_header()', and not the whole 'struct apply_state', and thus
we want functions in the callchain of 'parse_git_header()' to only
take arguments they really need.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-09 11:29:03 -07:00
Thomas Gummerer
570fe9911b apply: only pass required data to check_header_line
Currently the 'check_header_line()' function takes 'struct
apply_state' as parameter, even though it only needs the linenr from
that struct.

This function is in the callchain of 'parse_git_header()', which we
want to make more generally useful in a subsequent commit.  To make
that happen we only want to pass in the required data to
'parse_git_header()', and not the whole 'struct apply_state', and thus
we want functions in the callchain of 'parse_git_header()' to only
take arguments they really need.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-09 11:29:02 -07:00
Thomas Gummerer
85c3713df5 apply: only pass required data to git_header_name
Currently the 'git_header_name()' function takes 'struct apply_state'
as parameter, even though it only needs the p_value from that struct.

This function is in the callchain of 'parse_git_header()', which we
want to make more generally useful in a subsequent commit.  To make
that happen we only want to pass in the required data to
'parse_git_header()', and not the whole 'struct apply_state', and thus
we want functions in the callchain of 'parse_git_header()' to only
take arguments they really need.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-09 11:29:02 -07:00
Thomas Gummerer
d6c88c4fdc apply: only pass required data to skip_tree_prefix
Currently the 'skip_tree_prefix()' function takes 'struct apply_state'
as parameter, even though it only needs the p_value from that struct.

This function is in the callchain of 'parse_git_header()', which we
want to make more generally useful in a subsequent commit.  To make
that happen we only want to pass in the required data to
'parse_git_header()', and not the whole 'struct apply_state', and thus
we want functions in the callchain of 'parse_git_header()' to only
take arguments they really need.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-09 11:29:02 -07:00
Thomas Gummerer
5af40877a4 apply: replace marc.info link with public-inbox
public-inbox.org links include the whole message ID by default.  This
means the message can still be found even if the site goes away, which
is not the case with the marc.info link.  Replace the marc.info link
with a more future proof one.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-09 11:29:02 -07:00
Johannes Schindelin
d4c0a3ac78 fill_stat_cache_info(): prepare for an fsmonitor fix
We will need to pass down the `struct index_state` to
`mark_fsmonitor_valid()` for an upcoming bug fix, and this here function
calls that there function, so we need to extend the signature of
`fill_stat_cache_info()` first.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-28 12:43:42 -07:00
Nguyễn Thái Ngọc Duy
5a59a2301f completion: add more parameter value completion
This adds value completion for a couple more paramters. To make it
easier to maintain these hard coded lists, add a comment at the original
list/code to remind people to update git-completion.bash too.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-20 12:31:56 -08:00
Junio C Hamano
cba595ab1a Merge branch 'jk/loose-object-cache-oid'
Code clean-up.

* jk/loose-object-cache-oid:
  prefer "hash mismatch" to "sha1 mismatch"
  sha1-file: avoid "sha1 file" for generic use in messages
  sha1-file: prefer "loose object file" to "sha1 file" in messages
  sha1-file: drop has_sha1_file()
  convert has_sha1_file() callers to has_object_file()
  sha1-file: convert pass-through functions to object_id
  sha1-file: modernize loose header/stream functions
  sha1-file: modernize loose object file functions
  http: use struct object_id instead of bare sha1
  update comment references to sha1_object_info()
  sha1-file: fix outdated sha1 comment references
2019-02-06 22:05:27 -08:00
Junio C Hamano
b2fc9d2fb0 Merge branch 'jk/unused-parameter-cleanup'
Code cleanup.

* jk/unused-parameter-cleanup:
  convert: drop path parameter from actual conversion functions
  convert: drop len parameter from conversion checks
  config: drop unused parameter from maybe_remove_section()
  show_date_relative(): drop unused "tz" parameter
  column: drop unused "opts" parameter in item_length()
  create_bundle(): drop unused "header" parameter
  apply: drop unused "def" parameter from find_name_gnu()
  match-trees: drop unused path parameter from score functions
2019-02-06 22:05:23 -08:00
Junio C Hamano
7589e63648 Merge branch 'nd/the-index-final'
The assumption to work on the single "in-core index" instance has
been reduced from the library-ish part of the codebase.

* nd/the-index-final:
  cache.h: flip NO_THE_INDEX_COMPATIBILITY_MACROS switch
  read-cache.c: remove the_* from index_has_changes()
  merge-recursive.c: remove implicit dependency on the_repository
  merge-recursive.c: remove implicit dependency on the_index
  sha1-name.c: remove implicit dependency on the_index
  read-cache.c: replace update_index_if_able with repo_&
  read-cache.c: kill read_index()
  checkout: avoid the_index when possible
  repository.c: replace hold_locked_index() with repo_hold_locked_index()
  notes-utils.c: remove the_repository references
  grep: use grep_opt->repo instead of explict repo argument
2019-02-06 22:05:23 -08:00
Jeff King
74f8a9c954 apply: drop unused "def" parameter from find_name_gnu()
We use the "def" parameter in find_name_common() for some heuristics,
but they are not necessary with the less-ambiguous GNU format. Let's
drop this unused parameter.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-24 12:35:44 -08:00
Junio C Hamano
4084df42c2 Merge branch 'nd/checkout-noisy'
"git checkout [<tree-ish>] path..." learned to report the number of
paths that have been checked out of the index or the tree-ish,
which gives it the same degree of noisy-ness as the case in which
the command checks out a branch.

* nd/checkout-noisy:
  t0027: squelch checkout path run outside test_expect_* block
  checkout: print something when checking out paths
2019-01-14 15:29:29 -08:00
Nguyễn Thái Ngọc Duy
e1ff0a32e4 read-cache.c: kill read_index()
read_index() shares the same problem as hold_locked_index(): it
assumes $GIT_DIR/index. Move all call sites to repo_read_index()
instead. read_index_preload() and read_index_unmerged() are also
killed as a consequence.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-14 12:13:04 -08:00
Nguyễn Thái Ngọc Duy
3a95f31d1c repository.c: replace hold_locked_index() with repo_hold_locked_index()
hold_locked_index() assumes the index path at $GIT_DIR/index. This is
not good for places that take an arbitrary index_state instead of
the_index, which is basically everywhere except builtin/.

Replace it with repo_hold_locked_index(). hold_locked_index() remains
as a wrapper around repo_hold_locked_index() to reduce changes in builtin/

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-14 12:13:04 -08:00
Jeff King
98374a07c9 convert has_sha1_file() callers to has_object_file()
The only remaining callers of has_sha1_file() actually have an object_id
already. They can use the "object" variant, rather than dereferencing
the hash themselves.

The code changes here were completely generated by the included
coccinelle patch.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-08 09:41:06 -08:00
Junio C Hamano
bda53f4185 Merge branch 'js/apply-recount-allow-noop'
When editing a patch in a "git add -i" session, a hunk could be
made to no-op.  The "git apply" program used to reject a patch with
such a no-op hunk to catch user mistakes, but it is now updated to
explicitly allow a no-op hunk in an edited patch.

* js/apply-recount-allow-noop:
  apply --recount: allow "no-op hunks"
2018-11-18 18:23:56 +09:00
Nguyễn Thái Ngọc Duy
0f086e6dca checkout: print something when checking out paths
One of the problems with "git checkout" is that it does so many
different things and could confuse people specially when we fail to
handle ambiguation correctly.

One way to help with that is tell the user what sort of operation is
actually carried out. When switching branches, we always print
something unless --quiet, either

 - "HEAD is now at ..."
 - "Reset branch ..."
 - "Already on ..."
 - "Switched to and reset ..."
 - "Switched to a new branch ..."
 - "Switched to branch ..."

Checking out paths however is silent. Print something so that if we
got the user intention wrong, they won't waste too much time to find
that out. For the remaining cases of checkout we now print either

 - "Checked out ... paths out of the index"
 - "Checked out ... paths out of <abbrev hash>"

Since the purpose of printing this is to help disambiguate. Only do it
when "--" is missing.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-14 15:10:35 +09:00
Johannes Schindelin
22cb3835b9 apply --recount: allow "no-op hunks"
When editing patches e.g. in `git add -e`, it is quite common that a
hunk ends up having no -/+ lines, i.e. it is now supposed to do nothing.

This use case was broken by ad6e8ed37b (apply: reject a hunk that does
not do anything, 2015-06-01) with the good intention of catching a very
real, different issue in hand-edited patches.

So let's use the `--recount` option as the tell-tale whether the user
would actually be okay with no-op hunks.

Add a test case to make sure that this use case does not regress again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Reviewed-by: Josh Steadmon <steadmon@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-13 13:02:52 +09:00
Jeff King
517fe807d6 assert NOARG/NONEG behavior of parse-options callbacks
When we define a parse-options callback, the flags we put in the option
struct must match what the callback expects. For example, a callback
which does not handle the "unset" parameter should only be used with
PARSE_OPT_NONEG. But since the callback and the option struct are not
defined next to each other, it's easy to get this wrong (as earlier
patches in this series show).

Fortunately, the compiler can help us here: compiling with
-Wunused-parameters can show us which callbacks ignore their "unset"
parameters (and likewise, ones that ignore "arg" expect to be triggered
with PARSE_OPT_NOARG).

But after we've inspected a callback and determined that all of its
callers use the right flags, what do we do next? We'd like to silence
the compiler warning, but do so in a way that will catch any wrong calls
in the future.

We can do that by actually checking those variables and asserting that
they match our expectations. Because this is such a common pattern,
we'll introduce some helper macros. The resulting messages aren't
as descriptive as we could make them, but the file/line information from
BUG() is enough to identify the problem (and anyway, the point is that
these should never be seen).

Each of the annotated callbacks in this patch triggers
-Wunused-parameters, and was manually inspected to make sure all callers
use the correct options (so none of these BUGs should be triggerable).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-06 12:56:29 +09:00
Jeff King
735ca208c5 apply: return -1 from option callback instead of calling exit(1)
The option callback for "apply --whitespace" exits with status "1" on
error. It makes more sense for it to just return an error to
parse-options. That code will exit, too, but it will use status "129"
that is customary for option errors.

The exit() dates back to aaf6c447aa (builtin/apply: make
parse_whitespace_option() return -1 instead of die()ing, 2016-08-08).
That commit gives no reason why we'd prefer the current exit status (it
looks like it was just bumping the "die" up a level in the callstack,
but did not go as far as it could have).

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-06 12:55:36 +09:00
Jeff King
d5d202537f apply: mark include/exclude options as NONEG
The options callback for "git apply --no-include" is not ready to handle
the "unset" parameter, and as a result will segfault when it adds a NULL
argument to the include list (likewise for "--no-exclude").

In theory this might be used to clear the list, but since both
"--include" and "--exclude" add to the same list, it's not immediately
obvious what the semantics should be. Let's punt on that for now and
just disallow the broken options.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-06 12:55:35 +09:00
Junio C Hamano
d829d491ee Merge branch 'bc/hash-transition-part-15'
More codepaths are moving away from hardcoded hash sizes.

* bc/hash-transition-part-15:
  rerere: convert to use the_hash_algo
  submodule: make zero-oid comparison hash function agnostic
  apply: rename new_sha1_prefix and old_sha1_prefix
  apply: replace hard-coded constants
  tag: express constant in terms of the_hash_algo
  transport: use parse_oid_hex instead of a constant
  upload-pack: express constants in terms of the_hash_algo
  refs/packed-backend: express constants using the_hash_algo
  packfile: express constants in terms of the_hash_algo
  pack-revindex: express constants in terms of the_hash_algo
  builtin/fetch-pack: remove constants with parse_oid_hex
  builtin/mktree: remove hard-coded constant
  builtin/repack: replace hard-coded constants
  pack-bitmap-write: use GIT_MAX_RAWSZ for allocation
  object_id.cocci: match only expressions of type 'struct object_id'
2018-10-30 15:43:42 +09:00
brian m. carlson
eccb5a5f3d apply: rename new_sha1_prefix and old_sha1_prefix
Rename these structure members to "new_oid_prefix" and "old_oid_prefix".

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-15 12:53:16 +09:00
brian m. carlson
93eb00f719 apply: replace hard-coded constants
Replace several 40-based constants with references to GIT_MAX_HEXSZ or
the_hash_algo, as appropriate.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-10-15 12:53:16 +09:00
Nguyễn Thái Ngọc Duy
26d024ecf0 ws.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:51:18 -07:00
Nguyễn Thái Ngọc Duy
35843b1123 rerere.c: remove implicit dependency on the_index
The reason rerere(), rerere_forget() and rerere_remaining() take a
struct repository instead of struct index_state is not obvious from
the patch:

Deep in update_paths() and find_conflict(), hold_locked_index() and
read_index() are called. These functions assumes the index path at
$GIT_DIR/index which is not always true when you take an arbitrary
index state. Taking a repository will allow us to point to the right
index path later when we replace them with repo_ versions.

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:11 -07: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
Nguyễn Thái Ngọc Duy
1b5c6c1e53 apply.c: remove implicit dependency on the_index
Use apply_state->repo->index instead of the_index (in most cases,
unless we need to use a temporary index in some functions). Let the
callers (am and apply) tell us what to use, instead of always assuming
to operate 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-08-13 14:14:44 -07:00
Nguyễn Thái Ngọc Duy
82ea77eca7 apply.c: make init_apply_state() take a struct repository
We're moving away from the_index in this code. "struct index_state *"
could be added to struct apply_state. But let's aim long term and put
struct repository here instead so that we could even avoid more global
states in the future. The index will be available via
apply_state->repo->index.

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:44 -07:00
Nguyễn Thái Ngọc Duy
332a82a522 apply.c: pass struct apply_state to more functions
we're going to remove the dependency on the_index by moving 'struct
index_state *' to somewhere inside struct apply_state. Let's make sure
relevant functions have access to this struct now and reduce the diff
noise when the actual conversion happens.

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:44 -07:00
Nguyễn Thái Ngọc Duy
7f944e264e convert.c: remove an implicit dependency on the_index
Make the convert API take an index_state instead of assuming the_index
in convert.c. All external 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.

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
Junio C Hamano
ae533c4a92 Merge branch 'jm/cache-entry-from-mem-pool'
For a large tree, the index needs to hold many cache entries
allocated on heap.  These cache entries are now allocated out of a
dedicated memory pool to amortize malloc(3) overhead.

* jm/cache-entry-from-mem-pool:
  block alloc: add validations around cache_entry lifecyle
  block alloc: allocate cache entries from mem_pool
  mem-pool: fill out functionality
  mem-pool: add life cycle management functions
  mem-pool: only search head block for available space
  block alloc: add lifecycle APIs for cache_entry structs
  read-cache: teach make_cache_entry to take object_id
  read-cache: teach refresh_cache_entry to take istate
2018-08-02 15:30:43 -07:00
Junio C Hamano
4922a8587b Merge branch 'en/apply-comment-fix'
* en/apply-comment-fix:
  apply: fix grammar error in comment
2018-07-24 14:50:43 -07:00
Junio C Hamano
00624d608c Merge branch 'sb/object-store-grafts'
The conversion to pass "the_repository" and then "a_repository"
throughout the object access API continues.

* sb/object-store-grafts:
  commit: allow lookup_commit_graft to handle arbitrary repositories
  commit: allow prepare_commit_graft to handle arbitrary repositories
  shallow: migrate shallow information into the object parser
  path.c: migrate global git_path_* to take a repository argument
  cache: convert get_graft_file to handle arbitrary repositories
  commit: convert read_graft_file to handle arbitrary repositories
  commit: convert register_commit_graft to handle arbitrary repositories
  commit: convert commit_graft_pos() to handle arbitrary repositories
  shallow: add repository argument to is_repository_shallow
  shallow: add repository argument to check_shallow_file_for_update
  shallow: add repository argument to register_shallow
  shallow: add repository argument to set_alternate_shallow_file
  commit: add repository argument to lookup_commit_graft
  commit: add repository argument to prepare_commit_graft
  commit: add repository argument to read_graft_file
  commit: add repository argument to register_commit_graft
  commit: add repository argument to commit_graft_pos
  object: move grafts to object parser
  object-store: move object access functions to object-store.h
2018-07-18 12:20:28 -07:00
Jameson Miller
a849735bfb block alloc: add lifecycle APIs for cache_entry structs
It has been observed that the time spent loading an index with a large
number of entries is partly dominated by malloc() calls. This change
is in preparation for using memory pools to reduce the number of
malloc() calls made to allocate cahce entries when loading an index.

Add an API to allocate and discard cache entries, abstracting the
details of managing the memory backing the cache entries. This commit
does actually change how memory is managed - this will be done in a
later commit in the series.

This change makes the distinction between cache entries that are
associated with an index and cache entries that are not associated with
an index. A main use of cache entries is with an index, and we can
optimize the memory management around this. We still have other cases
where a cache entry is not persisted with an index, and so we need to
handle the "transient" use case as well.

To keep the congnitive overhead of managing the cache entries, there
will only be a single discard function. This means there must be enough
information kept with the cache entry so that we know how to discard
them.

A summary of the main functions in the API is:

make_cache_entry: create cache entry for use in an index. Uses specified
                  parameters to populate cache_entry fields.

make_empty_cache_entry: Create an empty cache entry for use in an index.
                        Returns cache entry with empty fields.

make_transient_cache_entry: create cache entry that is not used in an
                            index. Uses specified parameters to populate
                            cache_entry fields.

make_empty_transient_cache_entry: create cache entry that is not used in
                                  an index. Returns cache entry with
                                  empty fields.

discard_cache_entry: A single function that knows how to discard a cache
                     entry regardless of how it was allocated.

Signed-off-by: Jameson Miller <jamill@microsoft.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03 10:58:27 -07:00
Jameson Miller
825ed4d9a0 read-cache: teach make_cache_entry to take object_id
Teach make_cache_entry function to take object_id instead of a SHA-1.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03 10:58:15 -07:00
Elijah Newren
59caacab2a apply: fix grammar error in comment
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-28 13:27:57 -07:00
Junio C Hamano
ac997db0c1 Merge branch 'nd/diff-apply-ita'
"git diff" compares the index and the working tree.  For paths
added with intent-to-add bit, the command shows the full contents
of them as added, but the paths themselves were not marked as new
files.  They are now shown as new by default.

"git apply" learned the "--intent-to-add" option so that an
otherwise working-tree-only application of a patch will add new
paths to the index marked with the "intent-to-add" bit.

* nd/diff-apply-ita:
  apply: add --intent-to-add
  t2203: add a test about "diff HEAD" case
  diff: turn --ita-invisible-in-index on by default
  diff: ignore --ita-[in]visible-in-index when diffing worktree-to-tree
2018-06-25 13:22:36 -07:00
Junio C Hamano
50f08db594 Merge branch 'js/use-bug-macro'
Developer support update, by using BUG() macro instead of die() to
mark codepaths that should not happen more clearly.

* js/use-bug-macro:
  BUG_exit_code: fix sparse "symbol not declared" warning
  Convert remaining die*(BUG) messages
  Replace all die("BUG: ...") calls by BUG() ones
  run-command: use BUG() to report bugs, not die()
  test-tool: help verifying BUG() code paths
2018-05-30 14:04:07 +09:00
Junio C Hamano
2f76ebc93c Merge branch 'ma/lockfile-cleanup'
Code clean-up to adjust to a more recent lockfile API convention that
allows lockfile instances kept on the stack.

* ma/lockfile-cleanup:
  lock_file: move static locks into functions
  lock_file: make function-local locks non-static
  refs.c: do not die if locking fails in `delete_pseudoref()`
  refs.c: do not die if locking fails in `write_pseudoref()`
  t/helper/test-write-cache: clean up lock-handling
2018-05-30 14:04:05 +09:00
Junio C Hamano
7913f53b56 Sync with Git 2.17.1
* maint: (25 commits)
  Git 2.17.1
  Git 2.16.4
  Git 2.15.2
  Git 2.14.4
  Git 2.13.7
  fsck: complain when .gitmodules is a symlink
  index-pack: check .gitmodules files with --strict
  unpack-objects: call fsck_finish() after fscking objects
  fsck: call fsck_finish() after fscking objects
  fsck: check .gitmodules content
  fsck: handle promisor objects in .gitmodules check
  fsck: detect gitmodules files
  fsck: actually fsck blob data
  fsck: simplify ".git" check
  index-pack: make fsck error message more specific
  verify_path: disallow symlinks in .gitmodules
  update-index: stat updated files earlier
  verify_dotfile: mention case-insensitivity in comment
  verify_path: drop clever fallthrough
  skip_prefix: add case-insensitive variant
  ...
2018-05-29 17:10:05 +09:00
Nguyễn Thái Ngọc Duy
cff5dc09ed apply: add --intent-to-add
Similar to 'git reset -N', this option makes 'git apply' automatically
mark new files as intent-to-add so they are visible in the following
'git diff' command and could also be committed with 'git commit -a'.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-29 12:42:30 +09:00
Junio C Hamano
68f95b26e4 Sync with Git 2.16.4
* maint-2.16:
  Git 2.16.4
  Git 2.15.2
  Git 2.14.4
  Git 2.13.7
  verify_path: disallow symlinks in .gitmodules
  update-index: stat updated files earlier
  verify_dotfile: mention case-insensitivity in comment
  verify_path: drop clever fallthrough
  skip_prefix: add case-insensitive variant
  is_{hfs,ntfs}_dotgitmodules: add tests
  is_ntfs_dotgit: match other .git files
  is_hfs_dotgit: match other .git files
  is_ntfs_dotgit: use a size_t for traversing string
  submodule-config: verify submodule names as paths
2018-05-22 14:25:26 +09:00
Junio C Hamano
023020401d Sync with Git 2.15.2
* maint-2.15:
  Git 2.15.2
  Git 2.14.4
  Git 2.13.7
  verify_path: disallow symlinks in .gitmodules
  update-index: stat updated files earlier
  verify_dotfile: mention case-insensitivity in comment
  verify_path: drop clever fallthrough
  skip_prefix: add case-insensitive variant
  is_{hfs,ntfs}_dotgitmodules: add tests
  is_ntfs_dotgit: match other .git files
  is_hfs_dotgit: match other .git files
  is_ntfs_dotgit: use a size_t for traversing string
  submodule-config: verify submodule names as paths
2018-05-22 14:18:06 +09:00
Junio C Hamano
9e0f06d55d Sync with Git 2.14.4
* maint-2.14:
  Git 2.14.4
  Git 2.13.7
  verify_path: disallow symlinks in .gitmodules
  update-index: stat updated files earlier
  verify_dotfile: mention case-insensitivity in comment
  verify_path: drop clever fallthrough
  skip_prefix: add case-insensitive variant
  is_{hfs,ntfs}_dotgitmodules: add tests
  is_ntfs_dotgit: match other .git files
  is_hfs_dotgit: match other .git files
  is_ntfs_dotgit: use a size_t for traversing string
  submodule-config: verify submodule names as paths
2018-05-22 14:15:14 +09:00
Junio C Hamano
7b01c71b64 Sync with Git 2.13.7
* maint-2.13:
  Git 2.13.7
  verify_path: disallow symlinks in .gitmodules
  update-index: stat updated files earlier
  verify_dotfile: mention case-insensitivity in comment
  verify_path: drop clever fallthrough
  skip_prefix: add case-insensitive variant
  is_{hfs,ntfs}_dotgitmodules: add tests
  is_ntfs_dotgit: match other .git files
  is_hfs_dotgit: match other .git files
  is_ntfs_dotgit: use a size_t for traversing string
  submodule-config: verify submodule names as paths
2018-05-22 14:10:49 +09:00
Jeff King
10ecfa7649 verify_path: disallow symlinks in .gitmodules
There are a few reasons it's not a good idea to make
.gitmodules a symlink, including:

  1. It won't be portable to systems without symlinks.

  2. It may behave inconsistently, since Git may look at
     this file in the index or a tree without bothering to
     resolve any symbolic links. We don't do this _yet_, but
     the config infrastructure is there and it's planned for
     the future.

With some clever code, we could make (2) work. And some
people may not care about (1) if they only work on one
platform. But there are a few security reasons to simply
disallow it:

  a. A symlinked .gitmodules file may circumvent any fsck
     checks of the content.

  b. Git may read and write from the on-disk file without
     sanity checking the symlink target. So for example, if
     you link ".gitmodules" to "../oops" and run "git
     submodule add", we'll write to the file "oops" outside
     the repository.

Again, both of those are problems that _could_ be solved
with sufficient code, but given the complications in (1) and
(2), we're better off just outlawing it explicitly.

Note the slightly tricky call to verify_path() in
update-index's update_one(). There we may not have a mode if
we're not updating from the filesystem (e.g., we might just
be removing the file). Passing "0" as the mode there works
fine; since it's not a symlink, we'll just skip the extra
checks.

Signed-off-by: Jeff King <peff@peff.net>
2018-05-21 23:50:11 -04:00
Stefan Beller
cbd53a2193 object-store: move object access functions to object-store.h
This should make these functions easier to find and cache.h less
overwhelming to read.

In particular, this moves:
- read_object_file
- oid_object_info
- write_object_file

As a result, most of the codebase needs to #include object-store.h.
In this patch the #include is only added to files that would fail to
compile otherwise.  It would be better to #include wherever
identifiers from the header are used.  That can happen later
when we have better tooling for it.

Signed-off-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-16 11:42:03 +09:00
Martin Ågren
b227586831 lock_file: make function-local locks non-static
Placing `struct lock_file`s on the stack used to be a bad idea, because
the temp- and lockfile-machinery would keep a pointer into the struct.
But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap,
2017-09-05), we can safely have lockfiles on the stack. (This applies
even if a user returns early, leaving a locked lock behind.)

These `struct lock_file`s are local to their respective functions and we
can drop their staticness.

For good measure, I have inspected these sites and come to believe that
they always release the lock, with the possible exception of bailing out
using `die()` or `exit()` or by returning from a `cmd_foo()`.

As pointed out by Jeff King, it would be bad if someone held on to a
`struct lock_file *` for some reason. After some grepping, I agree with
his findings: no-one appears to be doing that.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-10 14:54:45 +09:00
Johannes Schindelin
033abf97fc Replace all die("BUG: ...") calls by BUG() ones
In d8193743e0 (usage.c: add BUG() function, 2017-05-12), a new macro
was introduced to use for reporting bugs instead of die(). It was then
subsequently used to convert one single caller in 588a538ae5
(setup_git_env: convert die("BUG") to BUG(), 2017-05-12).

The cover letter of the patch series containing this patch
(cf 20170513032414.mfrwabt4hovujde2@sigill.intra.peff.net) is not
terribly clear why only one call site was converted, or what the plan
is for other, similar calls to die() to report bugs.

Let's just convert all remaining ones in one fell swoop.

This trick was performed by this invocation:

	sed -i 's/die("BUG: /BUG("/g' $(git grep -l 'die("BUG' \*.c)

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-06 19:06:13 +09:00
Junio C Hamano
a5bbc29994 Merge branch 'bc/object-id'
Conversion from uchar[20] to struct object_id continues.

* bc/object-id: (36 commits)
  convert: convert to struct object_id
  sha1_file: introduce a constant for max header length
  Convert lookup_replace_object to struct object_id
  sha1_file: convert read_sha1_file to struct object_id
  sha1_file: convert read_object_with_reference to object_id
  tree-walk: convert tree entry functions to object_id
  streaming: convert istream internals to struct object_id
  tree-walk: convert get_tree_entry_follow_symlinks internals to object_id
  builtin/notes: convert static functions to object_id
  builtin/fmt-merge-msg: convert remaining code to object_id
  sha1_file: convert sha1_object_info* to object_id
  Convert remaining callers of sha1_object_info_extended to object_id
  packfile: convert unpack_entry to struct object_id
  sha1_file: convert retry_bad_packed_offset to struct object_id
  sha1_file: convert assert_sha1_type to object_id
  builtin/mktree: convert to struct object_id
  streaming: convert open_istream to use struct object_id
  sha1_file: convert check_sha1_signature to struct object_id
  sha1_file: convert read_loose_object to use struct object_id
  builtin/index-pack: convert struct ref_delta_entry to object_id
  ...
2018-04-10 08:25:45 +09:00
Junio C Hamano
7fb6aefd2a Merge branch 'nd/parseopt-completion'
Teach parse-options API an option to help the completion script,
and make use of the mechanism in command line completion.

* nd/parseopt-completion: (45 commits)
  completion: more subcommands in _git_notes()
  completion: complete --{reuse,reedit}-message= for all notes subcmds
  completion: simplify _git_notes
  completion: don't set PARSE_OPT_NOCOMPLETE on --rerere-autoupdate
  completion: use __gitcomp_builtin in _git_worktree
  completion: use __gitcomp_builtin in _git_tag
  completion: use __gitcomp_builtin in _git_status
  completion: use __gitcomp_builtin in _git_show_branch
  completion: use __gitcomp_builtin in _git_rm
  completion: use __gitcomp_builtin in _git_revert
  completion: use __gitcomp_builtin in _git_reset
  completion: use __gitcomp_builtin in _git_replace
  remote: force completing --mirror= instead of --mirror
  completion: use __gitcomp_builtin in _git_remote
  completion: use __gitcomp_builtin in _git_push
  completion: use __gitcomp_builtin in _git_pull
  completion: use __gitcomp_builtin in _git_notes
  completion: use __gitcomp_builtin in _git_name_rev
  completion: use __gitcomp_builtin in _git_mv
  completion: use __gitcomp_builtin in _git_merge_base
  ...
2018-03-14 12:01:07 -07:00
brian m. carlson
b4f5aca40e sha1_file: convert read_sha1_file to struct object_id
Convert read_sha1_file to take a pointer to struct object_id and rename
it read_object_file.  Do the same for read_sha1_file_extended.

Convert one use in grep.c to use the new function without any other code
change, since the pointer being passed is a void pointer that is already
initialized with a pointer to struct object_id.  Update the declaration
and definitions of the modified functions, and apply the following
semantic patch to convert the remaining callers:

@@
expression E1, E2, E3;
@@
- read_sha1_file(E1.hash, E2, E3)
+ read_object_file(&E1, E2, E3)

@@
expression E1, E2, E3;
@@
- read_sha1_file(E1->hash, E2, E3)
+ read_object_file(E1, E2, E3)

@@
expression E1, E2, E3, E4;
@@
- read_sha1_file_extended(E1.hash, E2, E3, E4)
+ read_object_file_extended(&E1, E2, E3, E4)

@@
expression E1, E2, E3, E4;
@@
- read_sha1_file_extended(E1->hash, E2, E3, E4)
+ read_object_file_extended(E1, E2, E3, E4)

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14 09:23:50 -07:00
Junio C Hamano
169c9c0169 Merge branch 'bw/c-plus-plus'
Avoid using identifiers that clash with C++ keywords.  Even though
it is not a goal to compile Git with C++ compilers, changes like
this help use of code analysis tools that targets C++ on our
codebase.

* bw/c-plus-plus: (37 commits)
  replace: rename 'new' variables
  trailer: rename 'template' variables
  tempfile: rename 'template' variables
  wrapper: rename 'template' variables
  environment: rename 'namespace' variables
  diff: rename 'template' variables
  environment: rename 'template' variables
  init-db: rename 'template' variables
  unpack-trees: rename 'new' variables
  trailer: rename 'new' variables
  submodule: rename 'new' variables
  split-index: rename 'new' variables
  remote: rename 'new' variables
  ref-filter: rename 'new' variables
  read-cache: rename 'new' variables
  line-log: rename 'new' variables
  imap-send: rename 'new' variables
  http: rename 'new' variables
  entry: rename 'new' variables
  diffcore-delta: rename 'new' variables
  ...
2018-03-06 14:54:07 -08:00
Junio C Hamano
177bd65cf8 Merge branch 'tk/apply-dev-null-verify-name-fix'
Many places in "git apply" knew that "/dev/null" that signals
"there is no such file on this side of the diff" can be followed by
whitespace and garbage when parsing a patch, except for one, which
made an otherwise valid patch (e.g. ones from subversion) rejected.

* tk/apply-dev-null-verify-name-fix:
  apply: handle Subversion diffs with /dev/null gracefully
  apply: demonstrate a problem applying svn diffs
2018-02-28 13:37:55 -08:00
Brandon Williams
f1ae97d333 apply: rename 'new' variables
Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-22 10:08:05 -08:00
Brandon Williams
6cbc7cfdfe apply: rename 'try' variables
Rename C++ keyword in order to bring the codebase closer to being able
to be compiled with a C++ compiler.

Signed-off-by: Brandon Williams <bmwill@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-22 10:08:05 -08:00
Junio C Hamano
8be8342b4c Merge branch 'po/object-id'
Conversion from uchar[20] to struct object_id continues.

* po/object-id:
  sha1_file: rename hash_sha1_file_literally
  sha1_file: convert write_loose_object to object_id
  sha1_file: convert force_object_loose to object_id
  sha1_file: convert write_sha1_file to object_id
  notes: convert write_notes_tree to object_id
  notes: convert combine_notes_* to object_id
  commit: convert commit_tree* to object_id
  match-trees: convert splice_tree to object_id
  cache: clear whole hash buffer with oidclr
  sha1_file: convert hash_sha1_file to object_id
  dir: convert struct sha1_stat to use object_id
  sha1_file: convert pretend_sha1_file to object_id
2018-02-15 14:55:43 -08:00
Tatyana Krasnukha
e454ad4bec apply: handle Subversion diffs with /dev/null gracefully
Subversion generates diffs that can contain lines like this one:

	--- /dev/null  (nonexistent)

Let's teach Git's apply machinery to handle such a line gracefully.

This fixes https://github.com/git-for-windows/git/isues/1489

Signed-off-by: Tatyana Krasnukha <tatyana@synopsys.com>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-15 11:09:02 -08:00