* [PATCH v2] ls-files: adding support for submodules
From: Brandon Williams @ 2016-09-13 0:33 UTC (permalink / raw)
To: git; +Cc: Brandon Williams
In-Reply-To: <1473458004-41460-1-git-send-email-bmwill@google.com>
Allow ls-files to recognize submodules in order to retrieve a list of
files from a repository's submodules. This is done by forking off a
process to recursively call ls-files on all submodules. Also added an
output-path-prefix command in order to prepend paths to child processes.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
Documentation/git-ls-files.txt | 11 +++-
builtin/ls-files.c | 60 +++++++++++++++++++++
t/t3007-ls-files-recurse-submodules.sh | 99 ++++++++++++++++++++++++++++++++++
3 files changed, 169 insertions(+), 1 deletion(-)
create mode 100755 t/t3007-ls-files-recurse-submodules.sh
diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
index 0d933ac..a623ebf 100644
--- a/Documentation/git-ls-files.txt
+++ b/Documentation/git-ls-files.txt
@@ -18,7 +18,9 @@ SYNOPSIS
[--exclude-per-directory=<file>]
[--exclude-standard]
[--error-unmatch] [--with-tree=<tree-ish>]
- [--full-name] [--abbrev] [--] [<file>...]
+ [--full-name] [--recurse-submodules]
+ [--output-path-prefix=<path>]
+ [--abbrev] [--] [<file>...]
DESCRIPTION
-----------
@@ -137,6 +139,13 @@ a space) at the start of each line:
option forces paths to be output relative to the project
top directory.
+--recurse-submodules::
+ Recursively calls ls-files on each submodule in the repository.
+ Currently there is only support for the --cached mode.
+
+--output-path-prefix=<path>::
+ Prepend the provided path to the output of each file
+
--abbrev[=<n>]::
Instead of showing the full 40-byte hexadecimal object
lines, show only a partial prefix.
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 00ea91a..c0bce00 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -14,6 +14,7 @@
#include "resolve-undo.h"
#include "string-list.h"
#include "pathspec.h"
+#include "run-command.h"
static int abbrev;
static int show_deleted;
@@ -28,6 +29,8 @@ static int show_valid_bit;
static int line_terminator = '\n';
static int debug_mode;
static int show_eol;
+static const char *output_path_prefix = "";
+static int recurse_submodules;
static const char *prefix;
static int max_prefix_len;
@@ -68,6 +71,21 @@ static void write_eolinfo(const struct cache_entry *ce, const char *path)
static void write_name(const char *name)
{
/*
+ * NEEDSWORK: To make this thread-safe, full_name would have to be owned
+ * by the caller.
+ *
+ * full_name get reused across output lines to minimize the allocation
+ * churn.
+ */
+ static struct strbuf full_name = STRBUF_INIT;
+ if (output_path_prefix != '\0') {
+ strbuf_reset(&full_name);
+ strbuf_addstr(&full_name, output_path_prefix);
+ strbuf_addstr(&full_name, name);
+ name = full_name.buf;
+ }
+
+ /*
* With "--full-name", prefix_len=0; this caller needs to pass
* an empty string in that case (a NULL is good for "").
*/
@@ -152,6 +170,25 @@ static void show_killed_files(struct dir_struct *dir)
}
}
+/**
+ * Recursively call ls-files on a submodule
+ */
+static void show_gitlink(const struct cache_entry *ce)
+{
+ struct child_process cp = CHILD_PROCESS_INIT;
+ int status;
+
+ argv_array_push(&cp.args, "ls-files");
+ argv_array_push(&cp.args, "--recurse-submodules");
+ argv_array_pushf(&cp.args, "--output-path-prefix=%s%s/",
+ output_path_prefix, ce->name);
+ cp.git_cmd = 1;
+ cp.dir = ce->name;
+ status = run_command(&cp);
+ if (status)
+ exit(status);
+}
+
static void show_ce_entry(const char *tag, const struct cache_entry *ce)
{
int len = max_prefix_len;
@@ -163,6 +200,10 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
len, ps_matched,
S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
return;
+ if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
+ show_gitlink(ce);
+ return;
+ }
if (tag && *tag && show_valid_bit &&
(ce->ce_flags & CE_VALID)) {
@@ -468,6 +509,10 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
{ OPTION_SET_INT, 0, "full-name", &prefix_len, NULL,
N_("make the output relative to the project top directory"),
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL },
+ OPT_STRING(0, "output-path-prefix", &output_path_prefix,
+ N_("path"), N_("prepend <path> to each file")),
+ OPT_BOOL(0, "recurse-submodules", &recurse_submodules,
+ N_("recurse through submodules")),
OPT_BOOL(0, "error-unmatch", &error_unmatch,
N_("if any <file> is not in the index, treat this as an error")),
OPT_STRING(0, "with-tree", &with_tree, N_("tree-ish"),
@@ -519,6 +564,21 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
if (require_work_tree && !is_inside_work_tree())
setup_work_tree();
+ if (recurse_submodules &&
+ (show_stage || show_deleted || show_others || show_unmerged ||
+ show_killed || show_modified || show_resolve_undo ||
+ show_valid_bit || show_tag || show_eol))
+ die("ls-files --recurse-submodules can only be used in "
+ "--cached mode");
+
+ if (recurse_submodules && error_unmatch)
+ die("ls-files --recurse-submodules does not support "
+ "--error-unmatch");
+
+ if (recurse_submodules && argc)
+ die("ls-files --recurse-submodules does not support path "
+ "arguments");
+
parse_pathspec(&pathspec, 0,
PATHSPEC_PREFER_CWD |
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
diff --git a/t/t3007-ls-files-recurse-submodules.sh b/t/t3007-ls-files-recurse-submodules.sh
new file mode 100755
index 0000000..caf3815
--- /dev/null
+++ b/t/t3007-ls-files-recurse-submodules.sh
@@ -0,0 +1,99 @@
+#!/bin/sh
+
+test_description='Test ls-files recurse-submodules feature
+
+This test verifies the recurse-submodules feature correctly lists files from
+submodules.
+'
+
+. ./test-lib.sh
+
+test_expect_success 'setup directory structure and submodules' '
+ echo a >a &&
+ mkdir b &&
+ echo b >b/b &&
+ git add a b &&
+ git commit -m "add a and b" &&
+ git init submodule &&
+ echo c >submodule/c &&
+ git -C submodule add c &&
+ git -C submodule commit -m "add c" &&
+ git submodule add ./submodule &&
+ git commit -m "added submodule"
+'
+
+test_expect_success 'ls-files correctly outputs files in submodule' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/c
+ EOF
+
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ls-files does not output files not added to a repo' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/c
+ EOF
+
+ echo a >not_added &&
+ echo b >b/not_added &&
+ echo c >submodule/not_added &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'ls-files recurses more than 1 level' '
+ cat >expect <<-\EOF &&
+ .gitmodules
+ a
+ b/b
+ submodule/.gitmodules
+ submodule/c
+ submodule/subsub/d
+ EOF
+
+ git init submodule/subsub &&
+ echo d >submodule/subsub/d &&
+ git -C submodule/subsub add d &&
+ git -C submodule/subsub commit -m "add d" &&
+ git -C submodule submodule add ./subsub &&
+ git -C submodule commit -m "added subsub" &&
+ git ls-files --recurse-submodules >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success '--recurse-submodules does not support using path arguments' '
+ test_must_fail git ls-files --recurse-submodules b 2>actual &&
+ test_i18ngrep "does not support path arguments" actual
+'
+
+test_expect_success '--recurse-submodules does not support --error-unmatch' '
+ test_must_fail git ls-files --recurse-submodules --error-unmatch 2>actual &&
+ test_i18ngrep "does not support --error-unmatch" actual
+'
+
+test_incompatible_with_recurse_submodules () {
+ test_expect_success "--recurse-submodules and $1 are incompatible" "
+ test_must_fail git ls-files --recurse-submodules $1 2>actual &&
+ test_i18ngrep 'can only be used in --cached mode' actual
+ "
+}
+
+test_incompatible_with_recurse_submodules -v
+test_incompatible_with_recurse_submodules -t
+test_incompatible_with_recurse_submodules --deleted
+test_incompatible_with_recurse_submodules --modified
+test_incompatible_with_recurse_submodules --others
+test_incompatible_with_recurse_submodules --stage
+test_incompatible_with_recurse_submodules --killed
+test_incompatible_with_recurse_submodules --unmerged
+test_incompatible_with_recurse_submodules --eol
+
+test_done
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* Re: Potential Same Name Bug
From: Andrew Ardill @ 2016-09-13 0:35 UTC (permalink / raw)
To: Kevin Smith; +Cc: git@vger.kernel.org
In-Reply-To: <CAOY7EmCYjajQPPAVhAhzt7g0yA=QH2R9HZEAhztUo2JbfSiCwQ@mail.gmail.com>
Hi Kevin,
On 13 September 2016 at 09:29, Kevin Smith <noiz77@gmail.com> wrote:
> So when I move from master to develop that status would come up. If I
> ran "git reset --hard" I would no longer have that message. I also
> saw that when I do a git clone and specify to clone the develop branch
> that I would not see the git status above. Is this an issue where if
> one branch has two files of the same name where one gets removed that
> it will remove both instances of that file in another branch when you
> switch to it? I fixed this issue in our repo by removing the
> "file_NAME.png" file in the master branch, but it seems like this
> should be handled better in the case I described.
Just to clarify, is the machine you are cloning to on a
case-insensitive file system, or have you set core.ignorecase=true?
If so, I would imagine the behaviour would have been _interesting_ in
that repository before you removed one of the versions - that may be
why you removed it in the first place.
For future reference, the typical way I have seen this situation dealt
with is to git mv one file to a different filename, or using git rm.
There may be better ways to do it, and it's possible git should deal
with the specific situation you mentioned better, but I'm not able to
comment on that.
This situation commonly arises when you set core.ignorecase=false on a
case insensitive file system and then rename a file. When you commit
that change, git doesn't notice that the old filename has been
'deleted' (git sees a rename as a delete+create with the same file
contents) and so now thinks that you have two files with very similar
names in your working directory. To avoid this, make sure
core.ignorecase is set correctly, and use git mv -f to rename files on
case insensitive file systems.
Details at https://stackoverflow.com/questions/17683458/how-do-i-commit-case-sensitive-only-filename-changes-in-git
Regards,
Andrew Ardill
^ permalink raw reply
* Re: [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*
From: Stefan Beller @ 2016-09-13 0:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <xmqqk2egwsq3.fsf@gitster.mtv.corp.google.com>
On Mon, Sep 12, 2016 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <stefanbeller@gmail.com> writes:
>
>> diff --git a/diff.c b/diff.c
>> index 2aefd0f..7dcef73 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -493,6 +493,19 @@ static void emit_line(struct diff_options *o, const char *set, const char *reset
>> emit_line_0(o, set, reset, line[0], line+1, len-1);
>> }
>>
>> +static void emit_line_fmt(struct diff_options *o,
>> + const char *set, const char *reset,
>> + const char *fmt, ...)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> + va_list ap;
>> + va_start(ap, fmt);
>> + strbuf_vaddf(&sb, fmt, ap);
>> + va_end(ap);
>> +
>> + emit_line(o, set, reset, sb.buf, sb.len);
>> +}
>> +
>> static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
>> {
>> if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
>> @@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>> const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
>> const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
>> struct diff_options *o = ecbdata->opt;
>> - const char *line_prefix = diff_line_prefix(o);
>>
>> o->found_changes = 1;
>>
>> @@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>> name_a_tab = strchr(ecbdata->label_path[0], ' ') ? "\t" : "";
>> name_b_tab = strchr(ecbdata->label_path[1], ' ') ? "\t" : "";
>>
>> - fprintf(o->file, "%s%s--- %s%s%s\n",
>> - line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);
>> - fprintf(o->file, "%s%s+++ %s%s%s\n",
>> - line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);
>> + emit_line_fmt(o, meta, reset, "--- %s%s\n",
>> + ecbdata->label_path[0], name_a_tab);
>> +
>> + emit_line_fmt(o, meta, reset, "+++ %s%s\n",
>> + ecbdata->label_path[1], name_b_tab);
>
> Hmph, the original showed the following for the name-a line:
>
> diff_line_prefix(o) META "--- " label_path RESET name_a_tab LF
>
> The updated one calls emit_line_fmt() with o, meta, reset, fmt and
> args, and then
>
> * strbuf_vaddf(&buf, "--- %s%s\n", label_path, name_a_tab) creates
> a string "--- " + label_path + LF
>
> * emit_line() is called on the whole thing with META and RESET
Oh right, that is a real issue, i.e. name_b_tab is not colored. I'll
have to think about that.
>
> * which is emit_line_0() that encloses the whole thing between META
> and RESET but knows the trailing LF should come after RESET.
>
> So the coloring seems to be correct, but I am not sure where the
> line-prefix went.
emit_line_0 puts the line_prefix by default in front, i.e.
it emits
line_prefix (SET) (first char) line (RESET) (CR/LF)
with things in parenthesis optional.
As said in 6/10 I think the emit_line_0 function is a great starting point
to build up a buffer when we do a two passes output. For that I want to channel
all output through emit_line_*.
---
I am done converting all diff output to emit_line for a rought first series,
and now all tests pass when asking for a buffered output.
So I started building the --color-moved on top of that, which seems
to work as well. The diff stat is ~700 insertions and ~300 deletions
compared to 2.10, however we call out to the xdl stuff only once.
Comparing that to the original quick-hack-patch
https://public-inbox.org/git/20160906070151.15163-1-stefanbeller@gmail.com/
which has just 280 additions, 10 deletions;
we seem to need about ~350 refactor lines and
slightly slower emissions in the non-color-moved case
(all the calls to emit_line instead of directly using fprintf/fputs/fputc)
^ permalink raw reply
* [PATCH 0/16] fix config-reading in non-repos
From: Jeff King @ 2016-09-13 3:22 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
The motivation for this series is to fix the regression in v2.9 where
core.logallrefupdates is sometimes not set properly in a newly
initialized repository, as described in this thread:
http://public-inbox.org/git/c46d36ef-3c2e-374f-0f2e-ffe31104e023@gmx.de/T/#u
The root of the problem is that we are overly eager to read and use
config from ".git/config", even when we have not established that it is
part of a repository. This is especially bad for git-init, which would
not want to read anything until we've created the new repo.
So the two interesting parts of the fix are:
1. We stop blindly reading ".git/config" when we don't know there's an
actual git directory. This is in patch 14, and is actually enough
to fix the v2.9 regression.
2. We are more thorough about dropping any cached config values when
we move into the new repository in git-init (patch 16).
I didn't dig into when this was broken, but it was probably when we
switched git_config() to use cached values in the v2.2.0
time-frame.
Doing (1) required fixing up some builtins that depended on the blind
.git/config thing, as the tests demonstrated. But I think this is a sign
that we are moving in the right direction, because each one of those
programs could easily be demonstrated to be broken in scenarios only
slightly more exotic than the test scripts (e.g., see patch 3 for one of
the simplest cases).
So I think notwithstanding their use as prep for patch 14, patches 1-13
fix useful bugs.
I won't be surprised if there are other fallouts that were not caught by
the test suite (i.e., programs that expect to read config, don't do
RUN_SETUP, but aren't covered well by tests). I poked around the list of
builtins in git.c that do not use RUN_SETUP, and they seem to correctly
end up in setup_git_directory_gently() before reading config. But it's
possible I missed a case.
So this is definitely a bit larger than I'd hope for a regression-fix to
maint. But anything that doesn't address this issue at the config layer
is going to end up as a bit of a hack, and I'd rather not pile up hacks
if we can avoid it.
I have a few patches on top that go even further and disallow the
auto-fallback of looking in ".git" at all for non-repositories. I think
that's the right thing to do, and after years of chipping away at the
setup code, I think we're finally at a point to make that change (with a
few fixes of course). But that's an even riskier change and not fixing
an immediate regression. So I'll hold that back for now, and hopefully
it would become "master" material once this is sorted out.
I've cc'd Dennis, who helped investigate solutions in the thread
mentioned above, and Duy, because historically he has been the one most
willing and able to battle the dragon of our setup code. :)
[01/16]: t1007: factor out repeated setup
[02/16]: hash-object: always try to set up the git repository
[03/16]: patch-id: use RUN_SETUP_GENTLY
[04/16]: diff: skip implicit no-index check when given --no-index
[05/16]: diff: handle --no-index prefixes consistently
[06/16]: diff: always try to set up the repository
[07/16]: pager: remove obsolete comment
[08/16]: pager: stop loading git_default_config()
[09/16]: pager: make pager_program a file-local static
[10/16]: pager: use callbacks instead of configset
[11/16]: pager: handle early config
[12/16]: t1302: use "git -C"
[13/16]: test-config: setup git directory
[14/16]: config: only read .git/config from configured repos
[15/16]: init: expand comments explaining config trickery
[16/16]: init: reset cached config when entering new repo
-Peff
^ permalink raw reply
* [PATCH 01/16] t1007: factor out repeated setup
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
We have a series of 3 CRLF tests that do exactly the same
(long) setup sequence. Let's pull it out into a common setup
test, which is shorter, more efficient, and will make it
easier to add new tests.
Note that we don't have to worry about cleaning up any of
the setup which was previously per-test; we call pop_repo
after the CRLF tests, which cleans up everything.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t1007-hash-object.sh | 32 ++++++++------------------------
1 file changed, 8 insertions(+), 24 deletions(-)
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index 7d2baa1..c89faa6 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -101,7 +101,7 @@ test_expect_success 'git hash-object --stdin file1 <file0 first operates on file
test "$obname1" = "$obname1new"
'
-test_expect_success 'check that appropriate filter is invoke when --path is used' '
+test_expect_success 'set up crlf tests' '
echo fooQ | tr Q "\\015" >file0 &&
cp file0 file1 &&
echo "file0 -crlf" >.gitattributes &&
@@ -109,7 +109,10 @@ test_expect_success 'check that appropriate filter is invoke when --path is used
git config core.autocrlf true &&
file0_sha=$(git hash-object file0) &&
file1_sha=$(git hash-object file1) &&
- test "$file0_sha" != "$file1_sha" &&
+ test "$file0_sha" != "$file1_sha"
+'
+
+test_expect_success 'check that appropriate filter is invoke when --path is used' '
path1_sha=$(git hash-object --path=file1 file0) &&
path0_sha=$(git hash-object --path=file0 file1) &&
test "$file0_sha" = "$path0_sha" &&
@@ -117,38 +120,19 @@ test_expect_success 'check that appropriate filter is invoke when --path is used
path1_sha=$(cat file0 | git hash-object --path=file1 --stdin) &&
path0_sha=$(cat file1 | git hash-object --path=file0 --stdin) &&
test "$file0_sha" = "$path0_sha" &&
- test "$file1_sha" = "$path1_sha" &&
- git config --unset core.autocrlf
+ test "$file1_sha" = "$path1_sha"
'
test_expect_success 'check that --no-filters option works' '
- echo fooQ | tr Q "\\015" >file0 &&
- cp file0 file1 &&
- echo "file0 -crlf" >.gitattributes &&
- echo "file1 crlf" >>.gitattributes &&
- git config core.autocrlf true &&
- file0_sha=$(git hash-object file0) &&
- file1_sha=$(git hash-object file1) &&
- test "$file0_sha" != "$file1_sha" &&
nofilters_file1=$(git hash-object --no-filters file1) &&
test "$file0_sha" = "$nofilters_file1" &&
nofilters_file1=$(cat file1 | git hash-object --stdin) &&
- test "$file0_sha" = "$nofilters_file1" &&
- git config --unset core.autocrlf
+ test "$file0_sha" = "$nofilters_file1"
'
test_expect_success 'check that --no-filters option works with --stdin-paths' '
- echo fooQ | tr Q "\\015" >file0 &&
- cp file0 file1 &&
- echo "file0 -crlf" >.gitattributes &&
- echo "file1 crlf" >>.gitattributes &&
- git config core.autocrlf true &&
- file0_sha=$(git hash-object file0) &&
- file1_sha=$(git hash-object file1) &&
- test "$file0_sha" != "$file1_sha" &&
nofilters_file1=$(echo "file1" | git hash-object --stdin-paths --no-filters) &&
- test "$file0_sha" = "$nofilters_file1" &&
- git config --unset core.autocrlf
+ test "$file0_sha" = "$nofilters_file1"
'
pop_repo
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 02/16] hash-object: always try to set up the git repository
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
When "hash-object" is run without "-w", we don't need to be
in a git repository at all; we can just hash the object and
write its sha1 to stdout. However, if we _are_ in a git
repository, we would want to know that so we can follow the
normal rules for respecting config, .gitattributes, etc.
This happens to work at the top-level of a git repository
because we blindly read ".git/config", but as the included
test shows, it does not work when you are in a subdirectory.
The solution is to just do a "gentle" setup in this case. We
already take care to use prefix_filename() on any filename
arguments we get (to handle the "-w" case), so we don't need
to do anything extra to handle the side effects of repo
setup.
An alternative would be to specify RUN_SETUP_GENTLY for this
command in git.c, and then die if "-w" is set but we are not
in a repository. However, the error messages generated at
the time of setup_git_directory() are more detailed, so it's
better to find out which mode we are in, and then call the
appropriate function.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/hash-object.c | 13 ++++++++-----
t/t1007-hash-object.sh | 11 +++++++++++
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/builtin/hash-object.c b/builtin/hash-object.c
index f7d3567..9028e1f 100644
--- a/builtin/hash-object.c
+++ b/builtin/hash-object.c
@@ -87,6 +87,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
int stdin_paths = 0;
int no_filters = 0;
int literally = 0;
+ int nongit = 0;
unsigned flags = HASH_FORMAT_CHECK;
const char *vpath = NULL;
const struct option hash_object_options[] = {
@@ -107,12 +108,14 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, NULL, hash_object_options,
hash_object_usage, 0);
- if (flags & HASH_WRITE_OBJECT) {
+ if (flags & HASH_WRITE_OBJECT)
prefix = setup_git_directory();
- prefix_length = prefix ? strlen(prefix) : 0;
- if (vpath && prefix)
- vpath = prefix_filename(prefix, prefix_length, vpath);
- }
+ else
+ prefix = setup_git_directory_gently(&nongit);
+
+ prefix_length = prefix ? strlen(prefix) : 0;
+ if (vpath && prefix)
+ vpath = prefix_filename(prefix, prefix_length, vpath);
git_config(git_default_config, NULL);
diff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh
index c89faa6..acca9ac 100755
--- a/t/t1007-hash-object.sh
+++ b/t/t1007-hash-object.sh
@@ -123,6 +123,17 @@ test_expect_success 'check that appropriate filter is invoke when --path is used
test "$file1_sha" = "$path1_sha"
'
+test_expect_success 'gitattributes also work in a subdirectory' '
+ mkdir subdir &&
+ (
+ cd subdir &&
+ subdir_sha0=$(git hash-object ../file0) &&
+ subdir_sha1=$(git hash-object ../file1) &&
+ test "$file0_sha" = "$subdir_sha0" &&
+ test "$file1_sha" = "$subdir_sha1"
+ )
+'
+
test_expect_success 'check that --no-filters option works' '
nofilters_file1=$(git hash-object --no-filters file1) &&
test "$file0_sha" = "$nofilters_file1" &&
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 06/16] diff: always try to set up the repository
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
If we see an explicit "--no-index", we do not bother calling
setup_git_directory_gently() at all. This means that we may
miss out on reading repo-specific config.
It's arguable whether this is correct or not. If we were
designing from scratch, making "git diff --no-index"
completely ignore the repository makes some sense. But we
are nowhere near scratch, so let's look at the existing
behavior:
1. If you're in the top-level of a repository and run an
explicit "diff --no-index", the config subsystem falls
back to reading ".git/config", and we will respect repo
config.
2. If you're in a subdirectory of a repository, then we
still try to read ".git/config", but it generally
doesn't exist. So "diff --no-index" there does not
respect repo config.
3. If you have $GIT_DIR set in the environment, we read
and respect $GIT_DIR/config,
4. If you run "git diff /tmp/foo /tmp/bar" to get an
implicit no-index, we _do_ run the repository setup,
and set $GIT_DIR (or respect an existing $GIT_DIR
variable). We find the repo config no matter where we
started, and respect it.
So we already respect the repository config in a number of
common cases, and case (2) is the only one that does not.
And at least one of our tests, t4034, depends on case (1)
behaving as it does now (though it is just incidental, not
an explicit test for this behavior).
So let's bring case (2) in line with the others by always
running the repository setup, even with an explicit
"--no-index". We shouldn't need to change anything else, as the
implicit case already handles the prefix.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/diff.c | 4 ++--
t/t4053-diff-no-index.sh | 20 ++++++++++++++++++++
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index a31643c..7f91f6d 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -301,9 +301,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
break;
}
- if (!no_index) {
- prefix = setup_git_directory_gently(&nongit);
+ prefix = setup_git_directory_gently(&nongit);
+ if (!no_index) {
/*
* Treat git diff with at least one path outside of the
* repo the same as if the command would have been executed
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index e60c951..453e6c3 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -107,4 +107,24 @@ test_expect_success 'diff from repo subdir shows real paths (implicit)' '
test_cmp expect actual.head
'
+test_expect_success 'diff --no-index from repo subdir respects config (explicit)' '
+ echo "diff --git ../../non/git/a ../../non/git/b" >expect &&
+ test_config -C repo diff.noprefix true &&
+ test_expect_code 1 \
+ git -C repo/sub \
+ diff --no-index ../../non/git/a ../../non/git/b >actual &&
+ head -n 1 <actual >actual.head &&
+ test_cmp expect actual.head
+'
+
+test_expect_success 'diff --no-index from repo subdir respects config (implicit)' '
+ echo "diff --git ../../non/git/a ../../non/git/b" >expect &&
+ test_config -C repo diff.noprefix true &&
+ test_expect_code 1 \
+ git -C repo/sub \
+ diff ../../non/git/a ../../non/git/b >actual &&
+ head -n 1 <actual >actual.head &&
+ test_cmp expect actual.head
+'
+
test_done
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 04/16] diff: skip implicit no-index check when given --no-index
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
We can invoke no-index mode in two ways: by an explicit
request from the user, or implicitly by noticing that we
have two paths, and at least one is outside the repository.
If the user already told us --no-index, there is no need for
us to do the implicit test at all. However, we currently
do, and downgrade our "explicit" to DIFF_NO_INDEX_IMPLICIT.
This doesn't have any user-visible behavior, though it's not
immediately obvious why. We only trigger the implicit check
when we have exactly two non-option arguments. And the only
code that cares about implicit versus explicit is an error
message that we show when we _don't_ have two non-option
arguments.
However, it's worth fixing anyway. Besides being slightly
more efficient, it makes the code easier to follow, which
will help when we modify it in future patches.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/diff.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index b7a9405..a31643c 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -301,20 +301,21 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
break;
}
- if (!no_index)
+ if (!no_index) {
prefix = setup_git_directory_gently(&nongit);
- /*
- * Treat git diff with at least one path outside of the
- * repo the same as if the command would have been executed
- * outside of a git repository. In this case it behaves
- * the same way as "git diff --no-index <a> <b>", which acts
- * as a colourful "diff" replacement.
- */
- if (nongit || ((argc == i + 2) &&
- (!path_inside_repo(prefix, argv[i]) ||
- !path_inside_repo(prefix, argv[i + 1]))))
- no_index = DIFF_NO_INDEX_IMPLICIT;
+ /*
+ * Treat git diff with at least one path outside of the
+ * repo the same as if the command would have been executed
+ * outside of a git repository. In this case it behaves
+ * the same way as "git diff --no-index <a> <b>", which acts
+ * as a colourful "diff" replacement.
+ */
+ if (nongit || ((argc == i + 2) &&
+ (!path_inside_repo(prefix, argv[i]) ||
+ !path_inside_repo(prefix, argv[i + 1]))))
+ no_index = DIFF_NO_INDEX_IMPLICIT;
+ }
if (!no_index)
gitmodules_config();
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 07/16] pager: remove obsolete comment
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
The comment at the top of pager.c claims that we've split
the code out so that Windows can do something different.
This dates back to f67b45f (Introduce trivial new pager.c
helper infrastructure, 2006-02-28), because the original
implementation used fork(). Later, we ended up sticking the
Windows #ifdefs into this file anyway. And then even later,
in ea27a18 (spawn pager via run_command interface,
2008-07-22) we unified the implementations.
So these days this comment is really saying nothing at all.
Let's drop it.
Signed-off-by: Jeff King <peff@peff.net>
---
| 5 -----
1 file changed, 5 deletions(-)
--git a/pager.c b/pager.c
index 6470b81..ae46da9 100644
--- a/pager.c
+++ b/pager.c
@@ -6,11 +6,6 @@
#define DEFAULT_PAGER "less"
#endif
-/*
- * This is split up from the rest of git so that we can do
- * something different on Windows.
- */
-
static struct child_process pager_process = CHILD_PROCESS_INIT;
static void wait_for_pager(int in_signal)
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 03/16] patch-id: use RUN_SETUP_GENTLY
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
Patch-id does not require a repository because it is just
processing the incoming diff on stdin, but it may look at
git config for keys like patchid.stable.
Even though we do not setup_git_directory(), this works from
the top-level of a repository because we blindly look at
".git/config" in this case. But as the included test
demonstrates, it does not work from a subdirectory.
We can fix it by using RUN_SETUP_GENTLY. We do not take any
filenames from the user on the command line, so there's no
need to adjust them via prefix_filename().
Signed-off-by: Jeff King <peff@peff.net>
---
git.c | 2 +-
t/t4204-patch-id.sh | 14 ++++++++++++++
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/git.c b/git.c
index 1c61151..ab5c99c 100644
--- a/git.c
+++ b/git.c
@@ -444,7 +444,7 @@ static struct cmd_struct commands[] = {
{ "pack-objects", cmd_pack_objects, RUN_SETUP },
{ "pack-redundant", cmd_pack_redundant, RUN_SETUP },
{ "pack-refs", cmd_pack_refs, RUN_SETUP },
- { "patch-id", cmd_patch_id },
+ { "patch-id", cmd_patch_id, RUN_SETUP_GENTLY },
{ "pickaxe", cmd_blame, RUN_SETUP },
{ "prune", cmd_prune, RUN_SETUP },
{ "prune-packed", cmd_prune_packed, RUN_SETUP },
diff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh
index 84a8096..0288c17 100755
--- a/t/t4204-patch-id.sh
+++ b/t/t4204-patch-id.sh
@@ -143,6 +143,20 @@ test_expect_success 'patch-id supports git-format-patch MIME output' '
test_cmp patch-id_master patch-id_same
'
+test_expect_success 'patch-id respects config from subdir' '
+ test_config patchid.stable true &&
+ mkdir subdir &&
+
+ # copy these because test_patch_id() looks for them in
+ # the current directory
+ cp bar-then-foo foo-then-bar subdir &&
+
+ (
+ cd subdir &&
+ test_patch_id irrelevant patchid.stable=true
+ )
+'
+
cat >nonl <<\EOF
diff --git i/a w/a
index e69de29..2e65efe 100644
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 05/16] diff: handle --no-index prefixes consistently
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
If we see an explicit "git diff --no-index ../foo ../bar",
then we do not set up the git repository at all (we already
know we are in --no-index mode, so do not have to check "are
we in a repository?"), and hence have no "prefix" within the
repository. A patch generated by this command will have the
filenames "a/../foo" and "b/../bar", no matter which
directory we are in with respect to any repository.
However, in the implicit case, where we notice that the
files are outside the repository, we will have chdir()'d to
the top-level of the repository. We then feed the prefix
back to the diff machinery. As a result, running the same
diff from a subdirectory will result in paths that look like
"a/subdir/../../foo".
Besides being unnecessarily long, this may also be confusing
to the user: they don't care about the subdir or the
repository at all; it's just where they happened to be when
running the command. We should treat this the same as the
explicit --no-index case.
One way to address this would be to chdir() back to the
original path before running our diff. However, that's a bit
hacky, as we would also need to adjust $GIT_DIR, which could
be a relative path from our top-level.
Instead, we can reuse the diff machinery's RELATIVE_NAME
option, which automatically strips off the prefix. Note that
this _also_ restricts the diff to this relative prefix, but
that's OK for our purposes: we queue our own diff pairs
manually, and do not rely on that part of the diff code.
Signed-off-by: Jeff King <peff@peff.net>
---
diff-no-index.c | 3 +++
t/t4053-diff-no-index.sh | 18 ++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/diff-no-index.c b/diff-no-index.c
index 1f8999b..f420786 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -281,6 +281,9 @@ void diff_no_index(struct rev_info *revs,
DIFF_OPT_SET(&revs->diffopt, NO_INDEX);
+ DIFF_OPT_SET(&revs->diffopt, RELATIVE_NAME);
+ revs->diffopt.prefix = prefix;
+
revs->max_count = -2;
diff_setup_done(&revs->diffopt);
diff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh
index 6eb8321..e60c951 100755
--- a/t/t4053-diff-no-index.sh
+++ b/t/t4053-diff-no-index.sh
@@ -89,4 +89,22 @@ test_expect_success 'turning a file into a directory' '
)
'
+test_expect_success 'diff from repo subdir shows real paths (explicit)' '
+ echo "diff --git a/../../non/git/a b/../../non/git/b" >expect &&
+ test_expect_code 1 \
+ git -C repo/sub \
+ diff --no-index ../../non/git/a ../../non/git/b >actual &&
+ head -n 1 <actual >actual.head &&
+ test_cmp expect actual.head
+'
+
+test_expect_success 'diff from repo subdir shows real paths (implicit)' '
+ echo "diff --git a/../../non/git/a b/../../non/git/b" >expect &&
+ test_expect_code 1 \
+ git -C repo/sub \
+ diff ../../non/git/a ../../non/git/b >actual &&
+ head -n 1 <actual >actual.head &&
+ test_cmp expect actual.head
+'
+
test_done
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 08/16] pager: stop loading git_default_config()
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
In git_pager(), we really only care about getting the value
of core.pager. But to do so, we use the git_default_config()
callback, which loads many other values. Ordinarily it
isn't a big deal to load this config an extra time, as it
simply overwrites the values from the previous run. But it's
a bad idea here, for two reasons:
1. The pager setup may be called very early in the
program, before we have found the git repository. As a
result, we may fail to read the correct repo-level
config file. This is a problem for core.pager, too,
but we should at least try to minimize the pollution to
other configured values.
2. Because we call setup_pager() from git.c, basically
every builtin command _may_ end up reading this config
and getting an implicit git_default_config() setup.
Which doesn't sound like a terrible thing, except that
we don't do it consistently; it triggers only when
stdout is a tty. So if a command forgets to load the
default config itself (but depends on it anyway), it
may appear to work, and then mysteriously fail when the
pager is not in use.
We can improve this by loading _just_ the core.pager config
from git_pager().
Signed-off-by: Jeff King <peff@peff.net>
---
config.c | 3 ---
| 9 ++++++++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/config.c b/config.c
index 0dfed68..8b28447 100644
--- a/config.c
+++ b/config.c
@@ -927,9 +927,6 @@ static int git_default_core_config(const char *var, const char *value)
return 0;
}
- if (!strcmp(var, "core.pager"))
- return git_config_string(&pager_program, var, value);
-
if (!strcmp(var, "core.editor"))
return git_config_string(&editor_program, var, value);
--git a/pager.c b/pager.c
index ae46da9..d747f50 100644
--- a/pager.c
+++ b/pager.c
@@ -35,6 +35,13 @@ static void wait_for_pager_signal(int signo)
raise(signo);
}
+static int core_pager_config(const char *var, const char *value, void *data)
+{
+ if (!strcmp(var, "core.pager"))
+ return git_config_string(&pager_program, var, value);
+ return 0;
+}
+
const char *git_pager(int stdout_is_tty)
{
const char *pager;
@@ -45,7 +52,7 @@ const char *git_pager(int stdout_is_tty)
pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
- git_config(git_default_config, NULL);
+ git_config(core_pager_config, NULL);
pager = pager_program;
}
if (!pager)
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 09/16] pager: make pager_program a file-local static
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
This variable is only ever used by the routines in pager.c,
and other parts of the code should always use those routines
(like git_pager()) to make decisions about which pager to
use. Let's reduce its scope to prevent accidents.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 1 -
environment.c | 1 -
| 1 +
3 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/cache.h b/cache.h
index 6738050..4f23952 100644
--- a/cache.h
+++ b/cache.h
@@ -1786,7 +1786,6 @@ extern void write_file(const char *path, const char *fmt, ...);
/* pager.c */
extern void setup_pager(void);
-extern const char *pager_program;
extern int pager_in_use(void);
extern int pager_use_color;
extern int term_columns(void);
diff --git a/environment.c b/environment.c
index ca72464..c266428 100644
--- a/environment.c
+++ b/environment.c
@@ -40,7 +40,6 @@ size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;
size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;
size_t delta_base_cache_limit = 96 * 1024 * 1024;
unsigned long big_file_threshold = 512 * 1024 * 1024;
-const char *pager_program;
int pager_use_color = 1;
const char *editor_program;
const char *askpass_program;
--git a/pager.c b/pager.c
index d747f50..8aa0a82 100644
--- a/pager.c
+++ b/pager.c
@@ -7,6 +7,7 @@
#endif
static struct child_process pager_process = CHILD_PROCESS_INIT;
+static const char *pager_program;
static void wait_for_pager(int in_signal)
{
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 10/16] pager: use callbacks instead of configset
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
While the cached configset interface is more pleasant to
use, it is not appropriate for "early" config like pager
setup, which must sometimes do tricky things like reading
from ".git/config" even when we have not set up the
repository.
As a preparatory step to handling these cases better, let's
switch back to using the callback interface, which gives us
more control.
Note that this is essentially a revert of 586f414 (pager.c:
replace `git_config()` with `git_config_get_value()`,
2014-08-07), but with some minor style fixups and
modernizations.
Signed-off-by: Jeff King <peff@peff.net>
---
| 47 +++++++++++++++++++++++++++++++++--------------
1 file changed, 33 insertions(+), 14 deletions(-)
--git a/pager.c b/pager.c
index 8aa0a82..46cc411 100644
--- a/pager.c
+++ b/pager.c
@@ -183,23 +183,42 @@ int decimal_width(uintmax_t number)
return width;
}
-/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
-int check_pager_config(const char *cmd)
+struct pager_command_config_data {
+ const char *cmd;
+ int want;
+ char *value;
+};
+
+static int pager_command_config(const char *var, const char *value, void *vdata)
{
- int want = -1;
- struct strbuf key = STRBUF_INIT;
- const char *value = NULL;
- strbuf_addf(&key, "pager.%s", cmd);
- if (git_config_key_is_valid(key.buf) &&
- !git_config_get_value(key.buf, &value)) {
- int b = git_config_maybe_bool(key.buf, value);
+ struct pager_command_config_data *data = vdata;
+ const char *cmd;
+
+ if (skip_prefix(var, "pager.", &cmd) && !strcmp(cmd, data->cmd)) {
+ int b = git_config_maybe_bool(var, value);
if (b >= 0)
- want = b;
+ data->want = b;
else {
- want = 1;
- pager_program = xstrdup(value);
+ data->want = 1;
+ data->value = xstrdup(value);
}
}
- strbuf_release(&key);
- return want;
+
+ return 0;
+}
+
+/* returns 0 for "no pager", 1 for "use pager", and -1 for "not specified" */
+int check_pager_config(const char *cmd)
+{
+ struct pager_command_config_data data;
+
+ data.cmd = cmd;
+ data.want = -1;
+ data.value = NULL;
+
+ git_config(pager_command_config, &data);
+
+ if (data.value)
+ pager_program = data.value;
+ return data.want;
}
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 11/16] pager: handle early config
From: Jeff King @ 2016-09-13 3:23 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
The pager code is often run early in the git.c startup,
before we have actually found the repository. When we ask
git_config() to look for values like core.pager, it doesn't
know where to find the repo-level config, and will blindly
examine ".git/config" if it exists. That's why t7006 shows
that many pager-related features happen to work from the
top-level of a repository, but not from a subdirectory.
This patch pulls that ".git/config" hack explicitly into the
pager code. There are two reasons for this:
1. We'd like to clean up the git_config() behavior, as
looking at ".git/config" when we do not have a
configured repository is often the wrong thing to do.
But we'd prefer not to break the pager config any worse
than it already is.
2. It's one very tiny step on the road to ultimately
making the pager config work consistently. If we
eventually get an equivalent of setup_git_directory()
that _just_ finds the directory and doesn't chdir() or
set up any global state, we could plug it in here
(instead of blindly looking at ".git/config").
Signed-off-by: Jeff King <peff@peff.net>
---
| 35 +++++++++++++++++++++++++++++++++--
1 file changed, 33 insertions(+), 2 deletions(-)
--git a/pager.c b/pager.c
index 46cc411..ae79643 100644
--- a/pager.c
+++ b/pager.c
@@ -43,6 +43,37 @@ static int core_pager_config(const char *var, const char *value, void *data)
return 0;
}
+static void read_early_config(config_fn_t cb, void *data)
+{
+ git_config_with_options(cb, data, NULL, 1);
+
+ /*
+ * Note that this is a really dirty hack that does the wrong thing in
+ * many cases. The crux of the problem is that we cannot run
+ * setup_git_directory() early on in git's setup, so we have no idea if
+ * we are in a repository or not, and therefore are not sure whether
+ * and how to read repository-local config.
+ *
+ * So if we _aren't_ in a repository (or we are but we would reject its
+ * core.repositoryformatversion), we'll read whatever is in .git/config
+ * blindly. Similarly, if we _are_ in a repository, but not at the
+ * root, we'll fail to find .git/config (because it's really
+ * ../.git/config, etc). See t7006 for a complete set of failures.
+ *
+ * However, we have historically provided this hack because it does
+ * work some of the time (namely when you are at the top-level of a
+ * valid repository), and would rarely make things worse (i.e., you do
+ * not generally have a .git/config file sitting around).
+ */
+ if (!startup_info->have_repository) {
+ struct git_config_source repo_config;
+
+ memset(&repo_config, 0, sizeof(repo_config));
+ repo_config.file = ".git/config";
+ git_config_with_options(cb, data, &repo_config, 1);
+ }
+}
+
const char *git_pager(int stdout_is_tty)
{
const char *pager;
@@ -53,7 +84,7 @@ const char *git_pager(int stdout_is_tty)
pager = getenv("GIT_PAGER");
if (!pager) {
if (!pager_program)
- git_config(core_pager_config, NULL);
+ read_early_config(core_pager_config, NULL);
pager = pager_program;
}
if (!pager)
@@ -216,7 +247,7 @@ int check_pager_config(const char *cmd)
data.want = -1;
data.value = NULL;
- git_config(pager_command_config, &data);
+ read_early_config(pager_command_config, &data);
if (data.value)
pager_program = data.value;
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 12/16] t1302: use "git -C"
From: Jeff King @ 2016-09-13 3:24 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
This is shorter, and saves a subshell.
Signed-off-by: Jeff King <peff@peff.net>
---
t/t1302-repo-version.sh | 30 ++++++------------------------
1 file changed, 6 insertions(+), 24 deletions(-)
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index 9bcd349..f859809 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -25,10 +25,7 @@ test_expect_success 'setup' '
test_expect_success 'gitdir selection on normal repos' '
echo 0 >expect &&
git config core.repositoryformatversion >actual &&
- (
- cd test &&
- git config core.repositoryformatversion >../actual2
- ) &&
+ git -C test config core.repositoryformatversion >actual2 &&
test_cmp expect actual &&
test_cmp expect actual2
'
@@ -36,35 +33,20 @@ test_expect_success 'gitdir selection on normal repos' '
test_expect_success 'gitdir selection on unsupported repo' '
# Make sure it would stop at test2, not trash
echo 99 >expect &&
- (
- cd test2 &&
- git config core.repositoryformatversion >../actual
- ) &&
+ git -C test2 config core.repositoryformatversion >actual &&
test_cmp expect actual
'
test_expect_success 'gitdir not required mode' '
git apply --stat test.patch &&
- (
- cd test &&
- git apply --stat ../test.patch
- ) &&
- (
- cd test2 &&
- git apply --stat ../test.patch
- )
+ git -C test apply --stat ../test.patch &&
+ git -C test2 apply --stat ../test.patch
'
test_expect_success 'gitdir required mode' '
git apply --check --index test.patch &&
- (
- cd test &&
- git apply --check --index ../test.patch
- ) &&
- (
- cd test2 &&
- test_must_fail git apply --check --index ../test.patch
- )
+ git -C test apply --check --index ../test.patch &&
+ test_must_fail git -C test2 apply --check --index ../test.patch
'
check_allow () {
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 13/16] test-config: setup git directory
From: Jeff King @ 2016-09-13 3:24 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
The t1308 test script uses our test-config helper to read
repository-level config, but never actually sets up the
repository. This works because git_config() blindly reads
".git/config" even if we have not configured a repository.
This means that test-config won't work from a subdirectory,
though since it's just a helper for the test scripts, that's
not a big deal.
More important is that the behavior of git_config() is going
to change, and we want to make sure that t1308 continues to
work. We can just use setup_git_directory(), and not the
gentle form; there's no point in being flexible, as it's
just a helper for the tests.
Signed-off-by: Jeff King <peff@peff.net>
---
t/helper/test-config.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 3c6d08c..83a4f2a 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -72,6 +72,9 @@ int cmd_main(int argc, const char **argv)
const char *v;
const struct string_list *strptr;
struct config_set cs;
+
+ setup_git_directory();
+
git_configset_init(&cs);
if (argc < 2) {
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 14/16] config: only read .git/config from configured repos
From: Jeff King @ 2016-09-13 3:24 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
When git_config() runs, it looks in the system, user-wide,
and repo-level config files. It gets the latter by calling
git_pathdup(), which in turn calls get_git_dir(). If we
haven't set up the git repository yet, this may simply
return ".git", and we will look at ".git/config". This
seems like it would be helpful (presumably we haven't set up
the repository yet, so it tries to find it), but it turns
out to be a bad idea for a few reasons:
- it's not sufficient, and therefore hides bugs in a
confusing way. Config will be respected if commands are
run from the top-level of the working tree, but not from
a subdirectory.
- it's not always true that we haven't set up the
repository _yet_; we may not want to do it at all. For
instance, if you run "git init /some/path" from inside
another repository, it should not load config from the
existing repository.
- there might be a path ".git/config", but it is not the
actual repository we would find via setup_git_directory().
This may happen, e.g., if you are storing a git
repository inside another git repository, but have
munged one of the files in such a way that the
inner repository is not valid (e.g., by removing HEAD).
We have at least two bugs of the second type in git-init,
introduced by ae5f677 (lazily load core.sharedrepository,
2016-03-11). It causes init to use git_configset(), which
loads all of the config, including values from the current
repo (if any). This shows up in two ways:
1. If we happen to be in an existing repository directory,
we'll read and respect core.sharedrepository from it,
even though it should have no bearing on the new
repository. A new test in t1301 covers this.
2. Similarly, if we're in an existing repo that sets
core.logallrefupdates, that will cause init to fail to
set it in a newly created repository (because it thinks
that the user's templates already did so). A new test
in t0001 covers this.
We also need to adjust an existing test in t1302, which
gives another example of why this patch is an improvement.
That test creates an embedded repository with a bogus
core.repositoryformatversion of "99". It wants to make sure
that we actually stop at the bogus repo rather than
continuing upward to find the outer repo. So it checks that
"git config core.repositoryformatversion" returns 99. But
that only works because we blindly read ".git/config", even
though we _know_ we're in a repository whose vintage we do
not understand.
After this patch, we avoid reading config from the unknown
vintage repository at all, which is a safer choice. But we
need to tweak the test, since core.repositoryformatversion
will not return 99; it will claim that it could not find the
variable at all.
Signed-off-by: Jeff King <peff@peff.net>
---
cache.h | 6 ++++++
config.c | 2 +-
environment.c | 7 +++++++
t/t0001-init.sh | 9 +++++++++
t/t1301-shared-repo.sh | 9 +++++++++
t/t1302-repo-version.sh | 4 +---
6 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/cache.h b/cache.h
index 4f23952..e9592d3 100644
--- a/cache.h
+++ b/cache.h
@@ -453,6 +453,12 @@ static inline enum object_type object_type(unsigned int mode)
*/
extern const char * const local_repo_env[];
+/*
+ * Returns true iff we have a configured git repository (either via
+ * setup_git_directory, or in the environment via $GIT_DIR).
+ */
+int have_git_dir(void);
+
extern int is_bare_repository_cfg;
extern int is_bare_repository(void);
extern int is_inside_git_dir(void);
diff --git a/config.c b/config.c
index 8b28447..1e4b617 100644
--- a/config.c
+++ b/config.c
@@ -1286,7 +1286,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)
int ret = 0;
char *xdg_config = xdg_config_home("config");
char *user_config = expand_user_path("~/.gitconfig");
- char *repo_config = git_pathdup("config");
+ char *repo_config = have_git_dir() ? git_pathdup("config") : NULL;
current_parsing_scope = CONFIG_SCOPE_SYSTEM;
if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))
diff --git a/environment.c b/environment.c
index c266428..c0cce6b 100644
--- a/environment.c
+++ b/environment.c
@@ -195,6 +195,13 @@ int is_bare_repository(void)
return is_bare_repository_cfg && !get_git_work_tree();
}
+int have_git_dir(void)
+{
+ return startup_info->have_repository
+ || git_dir
+ || getenv(GIT_DIR_ENVIRONMENT);
+}
+
const char *get_git_dir(void)
{
if (!git_dir)
diff --git a/t/t0001-init.sh b/t/t0001-init.sh
index a6fdd5e..8ffbbea 100755
--- a/t/t0001-init.sh
+++ b/t/t0001-init.sh
@@ -384,4 +384,13 @@ test_expect_success MINGW 'bare git dir not hidden' '
! is_hidden newdir
'
+test_expect_success 'remote init from does not use config from cwd' '
+ rm -rf newdir &&
+ test_config core.logallrefupdates true &&
+ git init newdir &&
+ echo true >expect &&
+ git -C newdir config --bool core.logallrefupdates >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index ac10875..7c28642 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -172,4 +172,13 @@ test_expect_success POSIXPERM 'forced modes' '
}" actual)"
'
+test_expect_success POSIXPERM 'remote init does not use config from cwd' '
+ git config core.sharedrepository 0666 &&
+ umask 0022 &&
+ git init --bare child.git &&
+ echo "-rw-r--r--" >expect &&
+ modebits child.git/config >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh
index f859809..ce4cff1 100755
--- a/t/t1302-repo-version.sh
+++ b/t/t1302-repo-version.sh
@@ -32,9 +32,7 @@ test_expect_success 'gitdir selection on normal repos' '
test_expect_success 'gitdir selection on unsupported repo' '
# Make sure it would stop at test2, not trash
- echo 99 >expect &&
- git -C test2 config core.repositoryformatversion >actual &&
- test_cmp expect actual
+ test_expect_code 1 git -C test2 config core.repositoryformatversion >actual
'
test_expect_success 'gitdir not required mode' '
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 15/16] init: expand comments explaining config trickery
From: Jeff King @ 2016-09-13 3:24 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
git-init may copy "config" from the templates directory and
then re-read it. There are some comments explaining what's
going on here, but they are not grouped very well with the
matching code. Let's rearrange and expand them.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/init-db.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 3a45f0b..e935895 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -191,16 +191,19 @@ static int create_default_files(const char *template_path)
/* Just look for `init.templatedir` */
git_config(git_init_db_config, NULL);
- /* First copy the templates -- we might have the default
+ /*
+ * First copy the templates -- we might have the default
* config file there, in which case we would want to read
* from it after installing.
*/
copy_templates(template_path);
-
git_config(git_default_config, NULL);
- is_bare_repository_cfg = init_is_bare_repository;
- /* reading existing config may have overwrote it */
+ /*
+ * We must make sure command-line options continue to override any
+ * values we might have just re-read from the config.
+ */
+ is_bare_repository_cfg = init_is_bare_repository;
if (init_shared_repository != -1)
set_shared_repository(init_shared_repository);
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* [PATCH 16/16] init: reset cached config when entering new repo
From: Jeff King @ 2016-09-13 3:24 UTC (permalink / raw)
To: git; +Cc: Dennis Kaarsemaker, Nguyễn Thái Ngọc Duy
In-Reply-To: <20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net>
After we copy the templates into place, we re-read the
config in case we copied in a default config file. But since
git_config() is backed by a cache these days, it's possible
that the call will not actually touch the filesystem at all;
we need to tell it that something has changed behind the
scenes.
Note that we also need to reset the shared_repository
config. At first glance, it seems like this should probably
just be folded into git_config_clear(). But unfortunately
that is not quite right. The shared repository value may
come from config, _or_ it may have been set manually. So
only the caller who knows whether or not they set it is the
one who can clear it (and indeed, if you _do_ put it into
git_config_clear(), then many tests fail, as we have to
clear the config cache any time we set a new config
variable).
There are three tests here. The first two actually pass
already, though it's largely luck: they just don't happen to
actually read any config before we enter the new repo.
But the third one does fail without this patch; we look at
core.sharedrepository while creating the directory, but need
to make sure the value from the template config overrides
it.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/init-db.c | 6 ++++++
cache.h | 7 +++++++
environment.c | 5 +++++
t/t1301-shared-repo.sh | 32 ++++++++++++++++++++++++++++++++
4 files changed, 50 insertions(+)
diff --git a/builtin/init-db.c b/builtin/init-db.c
index e935895..cea4550 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -195,8 +195,14 @@ static int create_default_files(const char *template_path)
* First copy the templates -- we might have the default
* config file there, in which case we would want to read
* from it after installing.
+ *
+ * Before reading that config, we also need to clear out any cached
+ * values (since we've just potentially changed what's available on
+ * disk).
*/
copy_templates(template_path);
+ git_config_clear();
+ reset_shared_repository();
git_config(git_default_config, NULL);
/*
diff --git a/cache.h b/cache.h
index e9592d3..4c8d85b 100644
--- a/cache.h
+++ b/cache.h
@@ -671,8 +671,15 @@ extern size_t delta_base_cache_limit;
extern unsigned long big_file_threshold;
extern unsigned long pack_size_limit_cfg;
+/*
+ * Accessors for the core.sharedrepository config which lazy-load the value
+ * from the config (if not already set). The "reset" function can be
+ * used to unset "set" or cached value, meaning that the value will be loaded
+ * fresh from the config file on the next call to get_shared_repository().
+ */
void set_shared_repository(int value);
int get_shared_repository(void);
+void reset_shared_repository(void);
/*
* Do replace refs need to be checked this run? This variable is
diff --git a/environment.c b/environment.c
index c0cce6b..cd5aa57 100644
--- a/environment.c
+++ b/environment.c
@@ -351,3 +351,8 @@ int get_shared_repository(void)
}
return the_shared_repository;
}
+
+void reset_shared_repository(void)
+{
+ need_shared_repository_from_config = 1;
+}
diff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh
index 7c28642..1312004 100755
--- a/t/t1301-shared-repo.sh
+++ b/t/t1301-shared-repo.sh
@@ -181,4 +181,36 @@ test_expect_success POSIXPERM 'remote init does not use config from cwd' '
test_cmp expect actual
'
+test_expect_success POSIXPERM 're-init respects core.sharedrepository (local)' '
+ git config core.sharedrepository 0666 &&
+ umask 0022 &&
+ echo whatever >templates/foo &&
+ git init --template=templates &&
+ echo "-rw-rw-rw-" >expect &&
+ modebits .git/foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)' '
+ rm -rf child.git &&
+ umask 0022 &&
+ git init --bare --shared=0666 child.git &&
+ test_path_is_missing child.git/foo &&
+ git init --bare --template=../templates child.git &&
+ echo "-rw-rw-rw-" >expect &&
+ modebits child.git/foo >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success POSIXPERM 'template can set core.sharedrepository' '
+ rm -rf child.git &&
+ umask 0022 &&
+ git config core.sharedrepository 0666 &&
+ cp .git/config templates/config &&
+ git init --bare --template=../templates child.git &&
+ echo "-rw-rw-rw-" >expect &&
+ modebits child.git/HEAD >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.10.0.230.g6f8d04b
^ permalink raw reply related
* Re: [PATCH v2] ls-files: adding support for submodules
From: Brandon Williams @ 2016-09-13 3:35 UTC (permalink / raw)
To: git; +Cc: Brandon Williams
In-Reply-To: <1473726835-143289-1-git-send-email-bmwill@google.com>
> static void write_name(const char *name)
> {
> /*
> + * NEEDSWORK: To make this thread-safe, full_name would have to be owned
> + * by the caller.
> + *
> + * full_name get reused across output lines to minimize the allocation
> + * churn.
> + */
> + static struct strbuf full_name = STRBUF_INIT;
> + if (output_path_prefix != '\0') {
It was pointed out to me that this should be:
if (*output_path_prefix != '\0') {
> + strbuf_reset(&full_name);
> + strbuf_addstr(&full_name, output_path_prefix);
> + strbuf_addstr(&full_name, name);
> + name = full_name.buf;
> + }
^ permalink raw reply
* [RFC/PATCH 00/17]
From: Stefan Beller @ 2016-09-13 4:45 UTC (permalink / raw)
To: gitster, peff, chriscool; +Cc: git, Stefan Beller
About a week ago, I started experimenting to add a new functionality to the
diff machinery: We want to color moved lines differently! This will aid developers
to review patches such as the patch "Move libified code from builtin/apply.c to apply.{c,h}"
Or a smaller example:
git show 11979b9
In the original patch series I just used the xdl diff machinery multiple times
to obtain the output for a multi pass algorithm I need for this coloring.
This was one of Junios main concerns (We have to rely on the xdl stuff to
yield the same output in the two passes).
This brings in another approach:
* run the xdl stuff only once
* In case we do not ask for coloring moves, put it out immediately
(small detour: we do not use fprintf/fputs directly but we channel
all output through the emit_line_* interface)
* When asking for coloring moves: buffer all its output for now.
Feed the line by line into the move detection algorithm.
* then output it all at once using the information from the move detection
to recolor moves while outputting.
While viewing the output of 11979b9 again, I notice a small bug in the proposal,
but I'd rather be asking for review of the whole design approach.
Thanks,
Stefan
Stefan Beller (17):
diff: move line ending check into emit_hunk_header
diff: emit_{add, del, context}_line to increase {pre,post}image line
count
diff.c: drop tautologous condition in emit_line_0
diff.c: factor out diff_flush_patch_all_file_pairs
diff.c: emit_line_0 can handle no color setting
diff.c: convert fn_out_consume to use emit_line_*
diff.c: convert builtin_diff to use emit_line_*
diff.c: convert emit_rewrite_diff to use emit_line_*
diff.c: convert emit_rewrite_lines to use emit_line_*
submodule.c: convert show_submodule_summary to use emit_line_fmt
diff.c: convert emit_binary_diff_body to use emit_line_*
diff.c: convert show_stats to use emit_line_*
diff.c: convert word diffing to use emit_line_*
diff.c: convert diff_flush to use emit_line_*
diff.c: convert diff_summary to use emit_line_*
diff: buffer output in emit_line_0
diff.c: color moved lines differently
Documentation/config.txt | 12 +-
Documentation/diff-options.txt | 7 +
contrib/completion/git-completion.bash | 2 +
diff.c | 666 +++++++++++++++++++++++----------
diff.h | 35 +-
submodule.c | 42 +--
submodule.h | 3 +-
t/t4015-diff-whitespace.sh | 229 ++++++++++++
8 files changed, 760 insertions(+), 236 deletions(-)
--
2.10.0.21.g1da280f.dirty
^ permalink raw reply
* [RFC/PATCH 01/17] diff: move line ending check into emit_hunk_header
From: Stefan Beller @ 2016-09-13 4:45 UTC (permalink / raw)
To: gitster, peff, chriscool; +Cc: git, Stefan Beller
In-Reply-To: <20160913044613.1037-1-sbeller@google.com>
In a later patch, I want to propose an option to detect&color
moved lines in a diff, which cannot be done in a one-pass over
the diff. Instead we need to go over the whole diff twice,
because we cannot detect the first line of the two corresponding
lines (+ and -) that got moved.
So to prepare the diff machinery for two pass algorithms
(i.e. buffer it all up and then operate on the result),
move all emissions to places, such that the only emitting
function is emit_line_0.
This patch moves code that is conceptually part of
emit_hunk_header, but was using output in fn_out_consume,
back to emit_hunk_header.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index cc8e812..aa50b2d 100644
--- a/diff.c
+++ b/diff.c
@@ -610,6 +610,9 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
}
strbuf_add(&msgbuf, line + len, org_len - len);
+ if (line[org_len - 1] != '\n')
+ strbuf_addch(&msgbuf, '\n');
+
emit_line(ecbdata->opt, "", "", msgbuf.buf, msgbuf.len);
strbuf_release(&msgbuf);
}
@@ -1247,8 +1250,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
len = sane_truncate_line(ecbdata, line, len);
find_lno(line, ecbdata);
emit_hunk_header(ecbdata, line, len);
- if (line[len-1] != '\n')
- putc('\n', o->file);
return;
}
--
2.10.0.21.g1da280f.dirty
^ permalink raw reply related
* [RFC/PATCH 02/17] diff: emit_{add, del, context}_line to increase {pre,post}image line count
From: Stefan Beller @ 2016-09-13 4:45 UTC (permalink / raw)
To: gitster, peff, chriscool; +Cc: git, Stefan Beller
In-Reply-To: <20160913044613.1037-1-sbeller@google.com>
At all call sites of emit_{add, del, context}_line we increment the line
count, so move it into the respective functions to make the code at the
calling site a bit clearer.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/diff.c b/diff.c
index aa50b2d..156c2aa 100644
--- a/diff.c
+++ b/diff.c
@@ -536,6 +536,7 @@ static void emit_add_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ ecbdata->lno_in_postimage++;
emit_line_checked(reset, ecbdata, line, len,
DIFF_FILE_NEW, WSEH_NEW, '+');
}
@@ -544,6 +545,7 @@ static void emit_del_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ ecbdata->lno_in_preimage++;
emit_line_checked(reset, ecbdata, line, len,
DIFF_FILE_OLD, WSEH_OLD, '-');
}
@@ -552,6 +554,8 @@ static void emit_context_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ ecbdata->lno_in_postimage++;
+ ecbdata->lno_in_preimage++;
emit_line_checked(reset, ecbdata, line, len,
DIFF_CONTEXT, WSEH_CONTEXT, ' ');
}
@@ -662,13 +666,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
endp = memchr(data, '\n', size);
len = endp ? (endp - data + 1) : size;
- if (prefix != '+') {
- ecb->lno_in_preimage++;
+ if (prefix != '+')
emit_del_line(reset, ecb, data, len);
- } else {
- ecb->lno_in_postimage++;
+ else
emit_add_line(reset, ecb, data, len);
- }
size -= len;
data += len;
}
@@ -1293,16 +1294,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
switch (line[0]) {
case '+':
- ecbdata->lno_in_postimage++;
emit_add_line(reset, ecbdata, line + 1, len - 1);
break;
case '-':
- ecbdata->lno_in_preimage++;
emit_del_line(reset, ecbdata, line + 1, len - 1);
break;
case ' ':
- ecbdata->lno_in_postimage++;
- ecbdata->lno_in_preimage++;
emit_context_line(reset, ecbdata, line + 1, len - 1);
break;
default:
--
2.10.0.21.g1da280f.dirty
^ permalink raw reply related
* [RFC/PATCH 03/17] diff.c: drop tautologous condition in emit_line_0
From: Stefan Beller @ 2016-09-13 4:45 UTC (permalink / raw)
To: gitster, peff, chriscool; +Cc: git, Stefan Beller
In-Reply-To: <20160913044613.1037-1-sbeller@google.com>
When `first` equals '\r', then it cannot equal '\n', which makes
`!has_trailing_newline` always true if `first` is '\r'.
Drop that check.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index 156c2aa..9d2e704 100644
--- a/diff.c
+++ b/diff.c
@@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
if (len == 0) {
has_trailing_newline = (first == '\n');
- has_trailing_carriage_return = (!has_trailing_newline &&
- (first == '\r'));
+ has_trailing_carriage_return = (first == '\r');
nofirst = has_trailing_newline || has_trailing_carriage_return;
} else {
has_trailing_newline = (len > 0 && line[len-1] == '\n');
--
2.10.0.21.g1da280f.dirty
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox