* Re: What's cooking in git.git (Sep 2016, #03; Fri, 9)
From: Jeff King @ 2016-09-12 21:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqlgyx2aui.fsf@gitster.mtv.corp.google.com>
On Mon, Sep 12, 2016 at 12:10:13PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > I happened to notice today that this topic needs a minor tweak:
> >
> > -- >8 --
> > Subject: [PATCH] add_delta_base_cache: use list_for_each_safe
> >
> > We may remove elements from the list while we are iterating,
> > which requires using a second temporary pointer. Otherwise
> > stepping to the next element of the list might involve
> > looking at freed memory (which generally works in practice,
> > as we _just_ freed it, but of course is wrong to rely on;
> > valgrind notices it).
>
> I failed to notice it, too. Thanks.
After staring at this, I was wondering how the _original_ ever worked.
Because the problem is in the linked-list code, which I did not really
change (I switched it to LIST_HEAD(), but the code is equivalent).
The answer is that in the original, there was no free() in the original
code when we released an entry; it just went back to the (static) pool.
So the bug is in the conversion to hashmap, where we start allocating
(and freeing) the entries individually.
-Peff
^ permalink raw reply
* Re: [PATCH v3 2/4] update-index: use the same structure for chmod as add
From: Junio C Hamano @ 2016-09-12 21:59 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Johannes Schindelin, Jeff King, Jan Keromnes,
Ingo Brückl, Edward Thomson
In-Reply-To: <20160912210818.26282-3-t.gummerer@gmail.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> + char flip = force_mode == 0777 ? '+' : '-';
>
> pos = cache_name_pos(path, strlen(path));
> if (pos < 0)
> @@ -432,17 +433,11 @@ static void chmod_path(int flip, const char *path)
> mode = ce->ce_mode;
> if (!S_ISREG(mode))
> goto fail;
> - switch (flip) {
> - case '+':
> - ce->ce_mode |= 0111; break;
> - case '-':
> - ce->ce_mode &= ~0111; break;
> - default:
> - goto fail;
> - }
> + ce->ce_mode = create_ce_mode(force_mode);
create_ce_mode() is supposed to take the st_mode taken from a
"struct stat", but here force_mode is 0777 or something else. The
above to feed only 0777 or 0666 may happen to work with the way how
create_ce_mode() is implemented now, but it is a time-bomb waiting
to go off.
Instead of using force_mode that is unsigned, keep the 'flip' as
argument, and do:
if (!S_ISREG(mode))
goto fail;
+ if (flip == '+')
+ mode |= 0111;
+ else
+ mode &= ~0111;
ce->ce_mode = create_ce_mode(mode);
perhaps, as you do not need and are not using the full 9 bits in
force_mode anyway.
That would mean chmod_index_entry() introduced in 3/4 and its user
in 4/4 need to be updated to pass '+' or '-' down the callchain, but
I think that is a good change for the same reason. We do not allow
you to set full 9 bits; let's not pretend as if we do so by passing
just a single bit '+' or '-' around.
I think 3/4 needs to be fixed where it calls create_ce_mode() with
only the 0666/0777 in chmod_index_entry() anyway, regardless of the
above suggested change.
> diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
> index dfe02f4..32ac6e0 100755
> --- a/t/t2107-update-index-basic.sh
> +++ b/t/t2107-update-index-basic.sh
> @@ -80,4 +80,17 @@ test_expect_success '.lock files cleaned up' '
> )
> '
>
> +test_expect_success '--chmod=+x and chmod=-x in the same argument list' '
> + >A &&
> + >B &&
> + git add A B &&
> + git update-index --chmod=+x A --chmod=-x B &&
> + cat >expect <<-\EOF &&
> + 100755 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 A
> + 100644 e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 0 B
> + EOF
> + git ls-files --stage A B >actual &&
> + test_cmp expect actual
> +'
Thanks for adding this test. We may want to cross the b/c bridge in
a later version bump, but until then it is good to make sure we know
what the currently expected behaviour is.
^ permalink raw reply
* [PATCH] [git-p4.py] Add --checkpoint-period option to sync/clone
From: Ori Rawlings @ 2016-09-12 22:02 UTC (permalink / raw)
To: git; +Cc: Vitor Antunes, Lars Schneider, Luke Diamand, Pete Wyckoff,
Ori Rawlings
Importing a long history from Perforce into git using the git-p4 tool
can be especially challenging. The `git p4 clone` operation is based
on an all-or-nothing transactionality guarantee. Under real-world
conditions like network unreliability or a busy Perforce server,
`git p4 clone` and `git p4 sync` operations can easily fail, forcing a
user to restart the import process from the beginning. The longer the
history being imported, the more likely a fault occurs during the
process. Long enough imports thus become statistically unlikely to ever
succeed.
I'm looking for feedback on a potential approach for addressing the
problem. My idea was to leverage the checkpoint feature of git
fast-import. I've included a patch which exposes a new option to the
sync/clone commands in the git-p4 tool. The option enables explict
checkpoints on a periodic basis (approximately every x seconds).
If the sync/clone command fails during processing of Perforce changes,
the user can craft a new git p4 sync command that will identify
changes that have already been imported and proceed with importing
only changes more recent than the last successful checkpoint.
Assuming this approach makes sense, there are a few questions/items I
have:
1. To add tests for this option, I'm thinking I'd need to simulate a
Perforce server or client that exits abnormally after first
processing some operations successfully. I'm looking for
suggestions on sane approaches for implementing that.
2. From a usability perspective, I think it makes sense to print
out a message upon clone/sync failure if the user has enabled the
option. This message would describe how long ago the last
successful checkpoint was completed and document what command/s
to execute to continue importing Perforce changes. Ideally, the
commmand to continue would be exactly the same as the command
which failed, but today, clone will ignore any commits already
imported to git. There are some lingering TODO comments in
git-p4.py suggesting that clone should try to avoid reimporting
changes. I don't mind taking a stab at addressing the TODO, but
am worried I'll quickly encounter edge cases in the clone/sync
features I don't understand.
3. This is my first attempt at a git contribution, so I'm definitely
looking for feedback on commit messages, etc.
Cheers!
Ori Rawlings (1):
[git-p4.py] Add --checkpoint-period option to sync/clone
git-p4.py | 8 ++++++++
1 file changed, 8 insertions(+)
--
2.7.4 (Apple Git-66)
^ permalink raw reply
* [PATCH] [git-p4.py] Add --checkpoint-period option to sync/clone
From: Ori Rawlings @ 2016-09-12 22:02 UTC (permalink / raw)
To: git; +Cc: Vitor Antunes, Lars Schneider, Luke Diamand, Pete Wyckoff,
Ori Rawlings
In-Reply-To: <1473717733-65682-1-git-send-email-orirawlings@gmail.com>
Importing a long history from Perforce into git using the git-p4 tool
can be especially challenging. The `git p4 clone` operation is based
on an all-or-nothing transactionality guarantee. Under real-world
conditions like network unreliability or a busy Perforce server,
`git p4 clone` and `git p4 sync` operations can easily fail, forcing a
user to restart the import process from the beginning. The longer the
history being imported, the more likely a fault occurs during the
process. Long enough imports thus become statistically unlikely to ever
succeed.
The underlying git fast-import protocol supports an explicit checkpoint
command. The idea here is to optionally allow the user to force an
explicit checkpoint every <x> seconds. If the sync/clone operation fails
branches are left updated at the appropriate commit available during the
latest checkpoint. This allows a user to resume importing Perforce
history while only having to repeat at most approximately <x> seconds
worth of import activity.
Signed-off-by: Ori Rawlings <orirawlings@gmail.com>
---
git-p4.py | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/git-p4.py b/git-p4.py
index fd5ca52..40cb64f 100755
--- a/git-p4.py
+++ b/git-p4.py
@@ -2244,6 +2244,7 @@ class P4Sync(Command, P4UserMap):
optparse.make_option("-/", dest="cloneExclude",
action="append", type="string",
help="exclude depot path"),
+ optparse.make_option("--checkpoint-period", dest="checkpointPeriod", type="int", help="Period in seconds between explict git fast-import checkpoints (by default, no explicit checkpoints are performed)"),
]
self.description = """Imports from Perforce into a git repository.\n
example:
@@ -2276,6 +2277,7 @@ class P4Sync(Command, P4UserMap):
self.tempBranches = []
self.tempBranchLocation = "refs/git-p4-tmp"
self.largeFileSystem = None
+ self.checkpointPeriod = -1
if gitConfig('git-p4.largeFileSystem'):
largeFileSystemConstructor = globals()[gitConfig('git-p4.largeFileSystem')]
@@ -3031,6 +3033,8 @@ class P4Sync(Command, P4UserMap):
def importChanges(self, changes):
cnt = 1
+ if self.checkpointPeriod > -1:
+ self.lastCheckpointTime = time.time()
for change in changes:
description = p4_describe(change)
self.updateOptionDict(description)
@@ -3107,6 +3111,10 @@ class P4Sync(Command, P4UserMap):
self.initialParent)
# only needed once, to connect to the previous commit
self.initialParent = ""
+
+ if self.checkpointPeriod > -1 and time.time() - self.lastCheckpointTime > self.checkpointPeriod:
+ self.checkpoint()
+ self.lastCheckpointTime = time.time()
except IOError:
print self.gitError.read()
sys.exit(1)
--
2.7.4 (Apple Git-66)
^ permalink raw reply related
* Re: [PATCH v3 4/4] add: modify already added files when --chmod is given
From: Junio C Hamano @ 2016-09-12 22:23 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Johannes Schindelin, Jeff King, Jan Keromnes,
Ingo Brückl, Edward Thomson
In-Reply-To: <20160912210818.26282-5-t.gummerer@gmail.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> When the chmod option was added to git add, it was hooked up to the diff
> machinery, meaning that it only works when the version in the index
> differs from the version on disk.
>
> As the option was supposed to mirror the chmod option in update-index,
> which always changes the mode in the index, regardless of the status of
> the file, make sure the option behaves the same way in git add.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
> builtin/add.c | 36 +++++++++++++++++++++++++-----------
> builtin/checkout.c | 2 +-
> builtin/commit.c | 2 +-
> cache.h | 10 +++++-----
> read-cache.c | 14 ++++++--------
> t/t3700-add.sh | 21 +++++++++++++++++++++
> 6 files changed, 59 insertions(+), 26 deletions(-)
The change looks quite large but this in essense reverts what Edward
did in the first round by hooking into "we add only modified files
here" and "we add new files here", both of which are made unnecessary,
because this adds the final "now we finished adding things, let's
fix modes of paths that match the pathspec".
Which makes sense.
> +static void chmod_pathspec(struct pathspec *pathspec, int force_mode)
> +{
> + int i;
> +
> + for (i = 0; i < active_nr; i++) {
> + struct cache_entry *ce = active_cache[i];
> +
> + if (pathspec && !ce_path_match(ce, pathspec, NULL))
> + continue;
> +
> + if (chmod_cache_entry(ce, force_mode) < 0)
> + fprintf(stderr, "cannot chmod '%s'", ce->name);
> + }
> +}
If pathspec is NULL, this will chmod all paths in the index, which
is probably not very useful and quite risky operation at the same
time.
However ...
> + if (force_mode)
> + chmod_pathspec(&pathspec, force_mode);
... the caller never passes a NULL as pathspec.
In any case, I somehow expected to see
if (force_mode && pathspec.nr)
chmod_pathspec(&pathspec, force_mode);
because it would make it very easy to see in the caller that
git add --chmod=+x ;# no pathspec
cd subdir && git add --chmod=+x ;# no pathspec
will be a no-op, which is what we want, if I am not mistaken. Of
course, if somebody really wants to drop executable bit from
everything, she can do
git add --chmod=-x .
pretty easily.
Above three may want to be added as new tests. The first two should
be a no-op, while the last one should drop executable bits from
everywhere.
Thanks.
^ permalink raw reply
* Re: [GIT PULL] l10n updates for 2.10.0 maint branch
From: Junio C Hamano @ 2016-09-12 22:24 UTC (permalink / raw)
To: Jiang Xin; +Cc: Ray, Ray Chen, Vasco Almeida, Git List
In-Reply-To: <CANYiYbFa42-q7=dbJHir5hfipK_L=x37NKQFX8WdBeeFU4V1Gg@mail.gmail.com>
Jiang Xin <worldhello.net@gmail.com> writes:
> There are some l10n updates for Git 2.10.0, please merge them to the
> maint branch.
Thanks.
^ permalink raw reply
* Re: build issues on AIX - aka non-GNU environment, no gnu grep, no gcc
From: Michael Felt @ 2016-09-12 22:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqy42wzx1f.fsf@gitster.mtv.corp.google.com>
Try again, reply all this time...
On 12-Sep-16 22:24, Junio C Hamano wrote:
> Michael Felt <aixtools@gmail.com> writes:
>
>> I had a couple of issues when packaging git for AIX
>> a) option -Wall by default - works fine with gcc I am sure, but not so
>> well when gcc is not your compiler
> That is expected. As you said, it is merely "by default" and there
> are mechanisms like config.mak provided for people to override them.
>
>> b) needs a special (GNU) grep argument (-a from memory). This I
>> resolved by downloading and packaging GNU grep. However, I hope this
>> has not introduced a new dependency.
> Read the Makefile and find SANE_TEXT_GREP, perhaps?
Ah - read the manual heh?. Generally, I just download, unpack, and run
configure.
I do not mind needing an extra tool - as long as it is only for the build.
And, since you have something about it in your (still unread) install
Notes - it has not slipped in by accident.
>
> Thanks for helping to spread Git to minority platforms.
The link is: http://www.aixtools.net/index.php/git
I do not use it much (I prefer to package "distributed" src tarballs,
but when testing a potential fix - git can be extremely useful!
All in all, thanks for an easy to port to a "minority" platform!
^ permalink raw reply
* Re: [PATCH] t/perf/run: Don't forget to copy config.mak.autogen & friends to build area
From: Junio C Hamano @ 2016-09-12 23:10 UTC (permalink / raw)
To: Kirill Smelkov
Cc: Jeff King, Vicent Marti, Jérome Perrin, Isabelle Vallet,
Kazuhiko Shiozaki, Julien Muchembled, git
In-Reply-To: <xmqqd1k92aif.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> In other words, something along this line, perhaps.
> ...
Not quite. There is no guanratee that the user is using autoconf at
all. It should be more like this, I think.
t/perf/run | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/t/perf/run b/t/perf/run
index aa383c2..7ec3734 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -30,7 +30,13 @@ unpack_git_rev () {
}
build_git_rev () {
rev=$1
- cp -t build/$rev ../../{config.mak,config.mak.autogen,config.status}
+ for config in config.mak config.mak.autogen config.status
+ do
+ if test -f "../../$config"
+ then
+ cp "../../$config" "build/$rev/"
+ fi
+ done
(cd build/$rev && make $GIT_PERF_MAKE_OPTS) ||
die "failed to build revision '$mydir'"
}
^ permalink raw reply related
* Re: Git Miniconference at Plumbers
From: Lars Schneider @ 2016-09-12 23:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jon Loeliger, David Bainbridge, Jeff King, git@vger.kernel.org
In-Reply-To: <xmqqeg4o27zw.fsf@gitster.mtv.corp.google.com>
> On 12 Sep 2016, at 21:11, Junio C Hamano <gitster@pobox.com> wrote:
>
> [..]
> properly; supporting "huge objects" better in the object layer,
> without having to resort to ugly hacks like GitLFS that will never
> be part of the core Git. [...]
I agree with you that GitLFS is an ugly hack.
Some applications have test data, image assets, and other data sets that
need to be versioned along with the source code.
How would you deal with these kind of "huge objects" _today_?
Thanks,
Lars
^ permalink raw reply
* Potential Same Name Bug
From: Kevin Smith @ 2016-09-12 23:29 UTC (permalink / raw)
To: git
I hit an issue in Git today that seemed to be a bug. Basically what
happened is in our master branch we had two files, one named something
like "file_NAME.png" and another named "file_name.png" in the same
folder. In the develop branch in the same repo we had removed the
"file_NAME.png" file so that only the "file_name.png" file was left.
If I clone the repo so I get master and then do "git checkout develop"
I would see when running "git status" that I would have this message:
On branch develop
Your branch is up-to-date with 'origin/develop'.
Changes not staged for commit:
(use "git add/rm <file>..." to update what will be committed)
(use "git checkout -- <file>..." to discard changes in working directory)
deleted: file_name.png
no changes added to commit (use "git add" and/or "git commit -a")
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.
^ permalink raw reply
* Re: [PATCH v7 04/10] pkt-line: add packet_flush_gently()
From: Junio C Hamano @ 2016-09-12 23:30 UTC (permalink / raw)
To: larsxschneider
Cc: git, peff, sbeller, Johannes.Schindelin, jnareb, mlbright, tboegi,
jacob.keller
In-Reply-To: <20160908182132.50788-5-larsxschneider@gmail.com>
larsxschneider@gmail.com writes:
> From: Lars Schneider <larsxschneider@gmail.com>
>
> packet_flush() would die in case of a write error even though for some
> callers an error would be acceptable. Add packet_flush_gently() which
> writes a pkt-line flush packet and returns `0` for success and `-1` for
> failure.
> ...
> +int packet_flush_gently(int fd)
> +{
> + packet_trace("0000", 4, 1);
> + if (write_in_full(fd, "0000", 4) == 4)
> + return 0;
> + error("flush packet write failed");
> + return -1;
It is more idiomatic to do
return error(...);
but more importantly, does the caller even want an error message
unconditionally printed here?
I suspect that it is a strong sign that the caller wants to be in
control of when and what error message is produced; otherwise it
wouldn't be calling the _gently() variant, no?
Of course, if you have written callers to this function in later
patches in this series, they would be responsible for reporting (or
choosing not to report) this failure, but I think making this
function silent is a better course in the longer term.
^ permalink raw reply
* Re: [PATCH v7 05/10] pkt-line: add packet_write_gently()
From: Junio C Hamano @ 2016-09-12 23:31 UTC (permalink / raw)
To: larsxschneider
Cc: git, peff, sbeller, Johannes.Schindelin, jnareb, mlbright, tboegi,
jacob.keller
In-Reply-To: <20160908182132.50788-6-larsxschneider@gmail.com>
larsxschneider@gmail.com writes:
> +int packet_write_gently(const int fd_out, const char *buf, size_t size)
> +{
> + static char packet_write_buffer[LARGE_PACKET_MAX];
> +
> + if (size > sizeof(packet_write_buffer) - 4) {
> + error("packet write failed");
> + return -1;
> + }
> + packet_trace(buf, size, 1);
> + size += 4;
> + set_packet_header(packet_write_buffer, size);
> + memcpy(packet_write_buffer + 4, buf, size - 4);
> + if (write_in_full(fd_out, packet_write_buffer, size) == size)
> + return 0;
> +
> + error("packet write failed");
> + return -1;
> +}
The same comment as 4/10 applies here.
^ permalink raw reply
* Re: [PATCH 01/10] diff: move line ending check into emit_hunk_header
From: Junio C Hamano @ 2016-09-12 23:35 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-2-git-send-email-stefanbeller@gmail.com>
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
The reason being...?
"Doing this would not change any behaviour and would not break
anything" may be true, but that is not a reason to do a change.
Hopefully it will become clear why this is needed once we look at a
later patch in the series.
> 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;
> }
^ permalink raw reply
* Re: [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count
From: Junio C Hamano @ 2016-09-12 23:47 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-3-git-send-email-stefanbeller@gmail.com>
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <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(-)
I am mostly in favor of this change, as the calls to these three
functions are always preceded by increment of these fields, but it
is "mostly" exactly because the reverse is not true. Namely ...
> @@ -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:
... there still needs an increment in the context lines, not shown
in the patch, just after this "default:". I think the patch is OK
as the comment after this "default:" (also not shown in the patch)
makes it clear what is going on.
Thanks.
^ permalink raw reply
* Re: [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
From: Junio C Hamano @ 2016-09-12 23:53 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-4-git-send-email-stefanbeller@gmail.com>
Stefan Beller <stefanbeller@gmail.com> writes:
> 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');
Interesting.
This may be a mis-conversion at 250f7993 ("diff.c: split emit_line()
from the first char and the rest of the line", 2009-09-14), I
suspect. The original took line[] with length and peeked for '\n',
and when it saw one, it decremented length before checking
line[len-1] for '\r'.
But of course if there is only one byte on the line (i.e. len == 0
after first is stripped off), it cannot be both '\n' or '\r' at the
same time.
Thanks for spotting.
^ permalink raw reply
* Re: [PATCH 04/10] diff.c: rename diff_flush_patch to diff_flush_patch_filepair
From: Junio C Hamano @ 2016-09-12 23:53 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-5-git-send-email-stefanbeller@gmail.com>
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
The reason being...?
^ permalink raw reply
* Re: [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files
From: Junio C Hamano @ 2016-09-13 0:05 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-6-git-send-email-stefanbeller@gmail.com>
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> ---
This shows why 04/10 should have had the overall plan for these two
steps. We want a short-and-sweet name "diff-flush-patch" to mean
"flush all the queued diff elements" so rename the singleton one
from diff-flush-patch to diff-flush-patch-filepair to make room in
04/10 and then introduce the "diff-flush-patch-all" in 05/10.
I just said with the above explanation the changes in 04/10 and
05/10 become undertandable, which is a bit different from being
justifiable. "diff_flush_raw()", "diff_flush_stat()", etc. are
_all_ about a single filepair. I'd rather see diff_flush_patch()
to be also about a single filepair.
It may be helpful to have a helper that calls diff_flush_patch() for
all filepairs in the queue, but can't that function get a new name
instead? By definition, it will be called much less often than a
pair-wise one, so it can afford to have a longer name.
diff_flush_patch_queue() or something, perhaps? I am not sure if it
should be diff_flush_queue_patch(), or even
diff_flush_queue(struct diff_options *o, diff_flush_fn fn);
where diff_flush_fn is
typedef void (*diff_flush_fn)(struct diff_filepair *p,
struct diff_options *o, void *other_data)
that can be used to flush the queue by calling any of these
filepair-wise flush functions like diff_flush_{raw,stat,checkdiff,patch}.
This last approach might be overkill, but if you want to try it,
you'd need a preparatory step to give an unused "void *other" to
diff_flush_{raw,patch,checkdiff} as diff_flush_stat() is an oddball
that needs an extra "accumulator" pointer. I actually wonder if
that "diffstat" pointer should become part of "struct diff_options",
though. Anyway.
> diff.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 85fb887..87b1bb2 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4608,6 +4608,17 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
> warning(rename_limit_advice, varname, needed);
> }
>
> +static void diff_flush_patch(struct diff_options *o)
> +{
> + int i;
> + struct diff_queue_struct *q = &diff_queued_diff;
> + for (i = 0; i < q->nr; i++) {
> + struct diff_filepair *p = q->queue[i];
> + if (check_pair_status(p))
> + diff_flush_patch_filepair(p, o);
> + }
> +}
> +
> void diff_flush(struct diff_options *options)
> {
> struct diff_queue_struct *q = &diff_queued_diff;
> @@ -4702,11 +4713,7 @@ void diff_flush(struct diff_options *options)
> }
> }
>
> - for (i = 0; i < q->nr; i++) {
> - struct diff_filepair *p = q->queue[i];
> - if (check_pair_status(p))
> - diff_flush_patch_filepair(p, options);
> - }
> + diff_flush_patch(options);
> }
>
> if (output_format & DIFF_FORMAT_CALLBACK)
^ permalink raw reply
* Re: [PATCH 06/10] diff.c: emit_line_0 can handle no color
From: Junio C Hamano @ 2016-09-13 0:11 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-7-git-send-email-stefanbeller@gmail.com>
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> ---
"X can do Y" can be taken as a statement of fact (to which "so
what?" is an appropriate response), a desire (to which "then please
say 'make X do Y' instead" is an appropriate response), or a report
of a bug (to which "please explain why X should be forbidden from
doing Y" is an appropriate response).
This is way under-explained. I think this is "make X do Y" kind,
and if so, please say so and possibly why it is a good idea to teach
X how to do Y.
Thanks.
> diff.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 87b1bb2..2aefd0f 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -473,11 +473,13 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
> }
>
> if (len || !nofirst) {
> - fputs(set, file);
> + if (set)
> + fputs(set, file);
> if (!nofirst)
> fputc(first, file);
> fwrite(line, len, 1, file);
> - fputs(reset, file);
> + if (reset)
> + fputs(reset, file);
> }
> if (has_trailing_carriage_return)
> fputc('\r', file);
^ permalink raw reply
* Re: [PATCH 06/10] diff.c: emit_line_0 can handle no color
From: Stefan Beller @ 2016-09-13 0:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git@vger.kernel.org
In-Reply-To: <xmqqoa3swteo.fsf@gitster.mtv.corp.google.com>
On Mon, Sep 12, 2016 at 5:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <stefanbeller@gmail.com> writes:
>
>> From: Stefan Beller <sbeller@google.com>
>>
>> ---
>
> "X can do Y" can be taken as a statement of fact (to which "so
> what?" is an appropriate response), a desire (to which "then please
> say 'make X do Y' instead" is an appropriate response), or a report
> of a bug (to which "please explain why X should be forbidden from
> doing Y" is an appropriate response).
>
> This is way under-explained. I think this is "make X do Y" kind,
> and if so, please say so and possibly why it is a good idea to teach
> X how to do Y.
>
> Thanks.
Ok, I see the general pattern of your answers: Add more explanations.
Answering for
patch 01/10 as well as this one here:
I want to propose an option to detect&color moved lines in a patch,
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 a
line pair that got moved in the first pass. So I aim for
* collecting all output into a buffer as a first pass,
* as the second pass output the buffer.
So in a later patch I will split up the emit_line_* machinery to either
emitting to options->file or buffering if we do the 2 pass thing.
To make sure the 2 passes work correctly, we need to make sure all output
is routed through the emit_line functions, and there will be no direct writes.
Now that we will be using the emit_lines functions for non colored
output as well,
we want to pass in "no color" which I think is best done via NULL and then not
calling the output of the color writes.
^ permalink raw reply
* [RFC 0/3] http: avoid repeatedly adding curl easy to curlm
From: Eric Wong @ 2016-09-13 0:25 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <20160909221942.GS9830@onerussian.com>
(I have some hours online today, so I decided to work on this)
The key patch here is 3/3 which seems like an obvious fix to
adding the problem of adding a curl easy handle to a curl multi
handle repeatedly.
What is unclear to me is how only Yaroslav's repository seems to
trigger this bug after all these years...
However, I am fairly sure this fixes the bug Yaroslav
encountered. This patch series is also needed for 2.9.3 and
perhaps older maintenance tracks for distros.
In PATCH 2/3, I originally had error checking, but removed it
after noticing it was causing failures on wheezy.
I will investigate those failures in a week or two when I regain
regular computer access.
These patches are needed for 2.9.x (and probably earlier), too.
The following changes since commit cda1bbd474805e653dda8a71d4ea3790e2a66cbb:
Sync with maint (2016-09-08 22:00:53 -0700)
are available in the git repository at:
git://bogomips.org/git-svn.git dumb-http-release
for you to fetch changes up to 3f561d0f0f78fd841708b5e81122e9d825919fd3:
http: always remove curl easy from curlm session on release (2016-09-12 23:59:35 +0000)
----------------------------------------------------------------
Eric Wong (3):
http: warn on curl_multi_add_handle failures
http: consolidate #ifdefs for curl_multi_remove_handle
http: always remove curl easy from curlm session on release
http.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
--
EW
^ permalink raw reply
* [RFC 1/3] http: warn on curl_multi_add_handle failures
From: Eric Wong @ 2016-09-13 0:25 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <20160913002557.10671-1-e@80x24.org>
This will be useful for tracking down curl usage errors.
Signed-off-by: Eric Wong <e@80x24.org>
---
http.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/http.c b/http.c
index cd40b01..cac5db9 100644
--- a/http.c
+++ b/http.c
@@ -1022,6 +1022,8 @@ int start_active_slot(struct active_request_slot *slot)
if (curlm_result != CURLM_OK &&
curlm_result != CURLM_CALL_MULTI_PERFORM) {
+ warning("curl_multi_add_handle failed: %s",
+ curl_multi_strerror(curlm_result));
active_requests--;
slot->in_use = 0;
return 0;
--
EW
^ permalink raw reply related
* Re: [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*
From: Junio C Hamano @ 2016-09-13 0:25 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
In-Reply-To: <1473572530-25764-8-git-send-email-stefanbeller@gmail.com>
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
* 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.
^ permalink raw reply
* [RFC 2/3] http: consolidate #ifdefs for curl_multi_remove_handle
From: Eric Wong @ 2016-09-13 0:25 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <20160913002557.10671-1-e@80x24.org>
I find #ifdefs makes code difficult-to-follow.
An early version of this patch had error checking for
curl_multi_remove_handle calls, but caused some tests (e.g.
t5541) to fail under curl 7.26.0 on old Debian wheezy.
Signed-off-by: Eric Wong <e@80x24.org>
---
http.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/http.c b/http.c
index cac5db9..a7eaf7b 100644
--- a/http.c
+++ b/http.c
@@ -201,6 +201,13 @@ static void finish_active_slot(struct active_request_slot *slot)
slot->callback_func(slot->callback_data);
}
+static void xmulti_remove_handle(struct active_request_slot *slot)
+{
+#ifdef USE_CURL_MULTI
+ curl_multi_remove_handle(curlm, slot->curl);
+#endif
+}
+
#ifdef USE_CURL_MULTI
static void process_curl_messages(void)
{
@@ -216,7 +223,7 @@ static void process_curl_messages(void)
slot->curl != curl_message->easy_handle)
slot = slot->next;
if (slot != NULL) {
- curl_multi_remove_handle(curlm, slot->curl);
+ xmulti_remove_handle(slot);
slot->curl_result = curl_result;
finish_active_slot(slot);
} else {
@@ -881,9 +888,7 @@ void http_cleanup(void)
while (slot != NULL) {
struct active_request_slot *next = slot->next;
if (slot->curl != NULL) {
-#ifdef USE_CURL_MULTI
- curl_multi_remove_handle(curlm, slot->curl);
-#endif
+ xmulti_remove_handle(slot);
curl_easy_cleanup(slot->curl);
}
free(slot);
@@ -1164,9 +1169,7 @@ static void release_active_slot(struct active_request_slot *slot)
{
closedown_active_slot(slot);
if (slot->curl && curl_session_count > min_curl_sessions) {
-#ifdef USE_CURL_MULTI
- curl_multi_remove_handle(curlm, slot->curl);
-#endif
+ xmulti_remove_handle(slot);
curl_easy_cleanup(slot->curl);
slot->curl = NULL;
curl_session_count--;
--
EW
^ permalink raw reply related
* [RFC 3/3] http: always remove curl easy from curlm session on release
From: Eric Wong @ 2016-09-13 0:25 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git, Jeff King, Junio C Hamano
In-Reply-To: <20160913002557.10671-1-e@80x24.org>
We must call curl_multi_remove_handle when releasing the slot to
prevent subsequent calls to curl_multi_add_handle from failing
with CURLM_ADDED_ALREADY (in curl 7.32.1+; older versions
returned CURLM_BAD_EASY_HANDLE)
Signed-off-by: Eric Wong <e@80x24.org>
---
http.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/http.c b/http.c
index a7eaf7b..3c4c3f5 100644
--- a/http.c
+++ b/http.c
@@ -1168,11 +1168,13 @@ void run_active_slot(struct active_request_slot *slot)
static void release_active_slot(struct active_request_slot *slot)
{
closedown_active_slot(slot);
- if (slot->curl && curl_session_count > min_curl_sessions) {
+ if (slot->curl) {
xmulti_remove_handle(slot);
- curl_easy_cleanup(slot->curl);
- slot->curl = NULL;
- curl_session_count--;
+ if (curl_session_count > min_curl_sessions) {
+ curl_easy_cleanup(slot->curl);
+ slot->curl = NULL;
+ curl_session_count--;
+ }
}
#ifdef USE_CURL_MULTI
fill_active_slots();
--
EW
^ permalink raw reply related
* [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
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