Re: [PATCH 2/2] completion: learn about --man-path
SZEDER Gábor writes: > Without the '-c' part it's "obviously correct" and together with patch > 1/2 is > > Acked-by: SZEDER Gábor Thanks, both. Will queue. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CYGWIN: Use a TCP socket for pipe()
Torsten Bögershausen writes: > I testet "rj/cygwin-remove-cheating-lstat" with the "socket pipe" on top: > no hanging. > > Then I run "rj/cygwin-remove-cheating-lstat" without "socket pipe", > (or in other words git.git/pu): > No hanging. So an immediate conclusion is that we can forget about this patch? > So at the moment I don't have any problems to report for cygwin, which is > good. > > And it looks as if "rj/cygwin-remove-cheating-lstat" prevents the "hanging", > so there is another +1 to keep it and move it into next. Ramsay started a "mark places we call lstat() when we do not really need fully correct lstat" topic, and I think it may be a sane direction to go, as long as the helper function's semantic is clearly defined. It would be worth seeing where it leads us, before ripping that "cheating and incomplete lstat out, I think. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Ramsay Jones writes:
> Ramsay Jones wrote:
>> Michael Haggerty wrote:
>>> On 06/27/2013 12:35 AM, Jeff King wrote:
>> [ ... ]
I think Michael's assessment above is missing one thing.
>>>
>>> Peff is absolutely right; for some unknown reason I was thinking of the
>>> consistency check as having been already fixed.
>>
>> Well, the "cygwin: Remove the Win32 l/stat() functions" patch *does* fix
>> the problem. :-D It's just a pity we can't use it on performance grounds. :(
>>
[...#ifdef out consistency check on cygwin when lock is held...]
>>>
>>> Yes, this would work.
>>>
>>> But, taking a step back, I think it is a bad idea to have an unreliable
>>> stat() masquerading as a real stat(). If we want to allow the use of an
>>> unreliable stat for certain purposes, let's have two stat() interfaces:
>>>
>>> * the true stat() (in this case I guess cygwin's slow-but-correct
>>> implementation)
>>>
>>> * some fast_but_maybe_unreliable_stat(), which would map to stat() on
>>> most platforms but might map to the Windows stat() on cygwin when so
>>> configured.
>>>
>>> By default the true stat() would always be used. It should have to be a
>>> conscious decision, taken only in specific, vetted scenarios, to use the
>>> unreliable stat.
>> ...
I like the part that gets rid of that "get-mode-bits" but at the
same time, I find this part wanting a reasonable in-code comment.
At least, with the earlier get-mode-bits, it was clear why we are
doing something special in that codepath, both from the name of the
helper function/macro and the comment attached to it describing how
the "regular" one is cheating.
We must say why this "fast" is not used everywhere and what criteria
you should apply when deciding to use it (or not use it). And the
"fast" name is much less descriptive.
I suspect (without thinking it through) that the rule would be
something like:
The "fast" variant is to be used to read from the filesystem the
"stat" bits that are stuffed into the index for quick "touch
detection" (aka "cached stat info") and/or that are compared
with the cached stat info, and should not be used anywhere else.
But then we always use lstat(2) and not stat(2) for that, so...
> +#ifndef GIT_FAST_STAT
> +static inline int fast_stat(const char *path, struct stat *st)
> +{
> + return stat(path, st);
> +}
> +static inline int fast_lstat(const char *path, struct stat *st)
> +{
> + return lstat(path, st);
> +}
> +#endif
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] CYGWIN: Use a TCP socket for pipe()
On 2013-06-28 04.46, Mark Levedahl wrote: > On 06/27/2013 01:38 PM, Junio C Hamano wrote: >> Torsten Bögershausen writes: >> >>> Work around issues that git hangs when doing fetch or pull under >>> various protocols under CYGWIN. >>> >>> Replace pipe() with a socket connection using a TCP/IP. >>> Introduce a new function socket_pipe() in compat/socket_pipe.c >> Sounds like sweeping the real problem, whatever it is, under rug. >> Is it that we are assuming a pipe buffer that is sufficiently large >> and expecting a write that we deem to be small enough not to block, >> causing a deadlock on a platform with very small pipe buffer, or >> something? >> > > There were issues in early v1.7 Cygwin release for overlapping I/O such that > the pipe was sometimes terminated early resulting in data loss. If the pipe > implementation in Cygwin is still a problem a good test case sent to the > Cygwin developers would be the right approach rather than a one-off patch in > git to try to work around a current platform bug. > > Note - I do not see random hangs with the stat/lstat hack removed, rather the > sole test suite hang I see is repeatable in t0008.sh. So, if the patch > remains, we may be able to run this remaining hang to ground. > > Mark Thanks, I testet "rj/cygwin-remove-cheating-lstat" with the "socket pipe" on top: no hanging. Then I run "rj/cygwin-remove-cheating-lstat" without "socket pipe", (or in other words git.git/pu): No hanging. Then I run a "stress test" with many (but not all) "git fetch" tests: t1507, t1514, t2015, t2024, t3200, t3409, t3600, t4041, t6050, t6200 repeat those tests in a forever loop. All these test run 24 hours in a row on a single core machine, no hanging. (I need to re-do the test on a dual-core machine) So at the moment I don't have any problems to report for cygwin, which is good. And it looks as if "rj/cygwin-remove-cheating-lstat" prevents the "hanging", so there is another +1 to keep it and move it into next. /Torsten -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] check-ignore doc: fix broken link to ls-files page
Ramkumar Ramachandra writes: > Signed-off-by: Ramkumar Ramachandra > --- Thanks, good eyes. I wonder if we can come up with an automated and reliable way to add to check-docs target of the main Makefile to catch this kind of thing. > Documentation/git-check-ignore.txt | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Documentation/git-check-ignore.txt > b/Documentation/git-check-ignore.txt > index 8e1f7ab..d2df487 100644 > --- a/Documentation/git-check-ignore.txt > +++ b/Documentation/git-check-ignore.txt > @@ -102,7 +102,7 @@ SEE ALSO > > linkgit:gitignore[5] > linkgit:gitconfig[5] > -linkgit:git-ls-files[5] > +linkgit:git-ls-files[1] > > GIT > --- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: spell 'ls-files --delete' option correctly in test descriptions
SZEDER Gábor writes: > The option is spelled '--deleted'. > > Signed-off-by: SZEDER Gábor > --- Thanks. > t/t7011-skip-worktree-reading.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/t/t7011-skip-worktree-reading.sh > b/t/t7011-skip-worktree-reading.sh > index 8f3b54d8..88d60c1c 100755 > --- a/t/t7011-skip-worktree-reading.sh > +++ b/t/t7011-skip-worktree-reading.sh > @@ -91,12 +91,12 @@ test_expect_success 'update-index --remove' ' > test_cmp expected 1 > ' > > -test_expect_success 'ls-files --delete' ' > +test_expect_success 'ls-files --deleted' ' > setup_absent && > test -z "$(git ls-files -d)" > ' > > -test_expect_success 'ls-files --delete' ' > +test_expect_success 'ls-files --deleted' ' > setup_dirty && > test -z "$(git ls-files -d)" > ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] commit: reject invalid UTF-8 codepoints
"brian m. carlson" writes: > On Sat, Jun 29, 2013 at 07:13:40PM -0700, Junio C Hamano wrote: >> "brian m. carlson" writes: >> Does this correspond to the following comment in the same file, and >> if so, shouldn't this part of your patch? > > Yes, yes, it should. > ... >> As that comment I quoted at the beginning says, we did not check for >> invalid encoded values and the primary reason for it is beccause >> this function did not want to look into the actual values here. But >> now you are looking into "codepoint", you can now also check for >> "overlong" form (e.g. sequence "C0 80" turning into U+)? > > Correct. That's what my second patch does. Ah, I started with that quoted comment when I saw 1/2, thinking that you may not even have noticed that these two things are our "todo" list, hence I thought it would be natural to do them both in the same commit as the logic to do so is in the single place (the function you patched). It is not _wrong_ per-se to do this in two steps, but I do not think it is necessary. You *could* break this to even smaller steps to go to the other extreme, first limiting only upto 4-byte forms, then rejecting a half of the 4-byte form, then rejecting the range for surrogates, and then rejecting overlong forms, and the split may be technically logical and "one step at a time" but that kind of granularity is probably more noisy than it is valuable. I think "1/2 is about rejecting a codepoint outside valid ranges and 2/2 is about rejecting a valid codepoint inside the valid range but expressed in an invalid way" is on the borderline between the two extremes, but it probably is on the saner side, so let's keep these two patches separate. > If you ended up with an encoding of U+D800, then you got it. I think we are seeing ? U+003F in our mailboxes. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] [submodule] Add depth to submodule update
Fredrik Gustafsson writes: >> OK, then "--depth" it is. >> >> The points in your review on the last version with "--depth" (which >> I picked up and parked on 'pu') still need to be addressed, I think? > > I agree, I'm on it Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git stash: Avoid data loss when saving a stash
Petr Baudis writes: > Hi! > > On Fri, Jun 28, 2013 at 11:39:16AM -0700, Junio C Hamano wrote: >> Thanks. I'll queue it with a pair of fix-up commits on top, so that >> they can later be squashed in. >> >> The result of squashing the fix-ups would look like this. > > Thanks! I agree with all of your changes. > >> -- >8 -- >> From: Petr Baudis >> Date: Fri, 28 Jun 2013 17:05:32 +0200 >> Subject: [PATCH] git stash: avoid data loss when "git stash save" kills a >> directory > > Hmm, it's a pity that the note that `git reset --hard` itself should > perhaps also abort in that case got lost. I don't insist on mentioning > it in the commit message, though. I do not agree with your `git reset --hard` at all. With the command, the user demands "no matter what, I want get rid of any funny state in my working tree so that I can start my work from that specified commit (default to HEAD)". Imagine that this is you did to arrive that "funny state": $ git rm foo ;# foo used to be tracked and in HEAD $ cp /somewhere/else/foo foo $ cp /somewhere/else/bar bar ;# bar is not in HEAD $ cp /somewhere/else/bar baz ;# baz is in HEAD ... do various other things ... and then "git reset --hard". At that point, "foo" and "bar" are not tracked and completely unrelated to the project. "baz" is tracked and have unrelated contents from that of "HEAD". In order to satisfy your desire to go back to the state of HEAD with minimal collateral amage, we need to get rid of the updated "foo" and "baz" and replace them with those from HEAD. We do not have to touch "bar" so we leave it as-is. And the "killed" case is just like "foo" and "baz". If the state you want to go back to with "--hard" has a directory (a file) where your working tree's funny state has a file (a directory), the local cruft needs to go away to satisify your request. I do not mind if you are proposing a different and new kind of reset that fails if it has to overwrite any local changes (be it tracked or untracked), but that is not "reset --hard". It is something else. > On Fri, Jun 28, 2013 at 02:30:15PM -0700, Junio C Hamano wrote: >> -- >8 -- >> Subject: treat_directory(): do not declare submodules in index to be >> untracked > > Oh, you are truly awesome! I admit that properly reviewing this patch > is a little out of my depth right now as I'm not familiar with this > infrastructure. I'd just like to note... > >> case index_gitdir: >> if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) >> return path_none; >> -return path_untracked; >> +return path_none; > > ...that the if-test can be removed now as both branches are the same. Thanks for noticing. What was queued on 'pu' should already have fixed that one. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] completion: add completer for rev-parse
SZEDER Gábor writes:
> On Fri, Jun 28, 2013 at 07:48:07PM +0530, Ramkumar Ramachandra wrote:
>> Signed-off-by: Ramkumar Ramachandra
>> ---
>> contrib/completion/git-completion.bash | 14 ++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/contrib/completion/git-completion.bash
>> b/contrib/completion/git-completion.bash
>> index 278018f..f2959a7 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2247,6 +2247,20 @@ _git_reset ()
>> __gitcomp_nl "$(__git_refs)"
>> }
>>
>> +_git_rev_parse ()
>> +{
>> +case "$cur" in
>> +--*)
>> +__gitcomp "
>> +--short --show-toplevel --is-inside-work-tree
>> +--symbolic-full-name --verify
>> +"
>
> In the completion script we support porcelain commands. I'm not sure
> about 'git rev-parse', but I think it's more plumbing than porcelain.
> However, I think the same about 'git ls-tree' and 'git reflog', too,
> yet we have support for them in the completion script.
>
> Either way, why these five options? 'git rev-parse' has a lot more
> options than that.
I think most of the options not listed here are indeed very low
level plumbings that end users have no reason to type on the command
line, except when they are learning to see how they would use it in
their scripts, of course.
Adding a select few that current Git gives no other easy way to
achieve what the users may want to do is a good short-to-medium term
compromise for usability, and I think the above is a good starting
point.
But our goal should be _not_ to grow that set, but to shrink them by
making "rev-parse" less end-user facing.
A longer term direction should be to make sure that we do _not_ need
to run "rev-parse" from the command line by giving usability updates
to other commands and shell helpers, though.
For example, some subcommands of "git submodule" always required you
to run from the top-level, so you needed some way to find out where
the top level was, but the need to find the top-level is _not_ the
ultimate end-user _want_. There was no other easy way to achieve
what the users wanted to do (i.e. run "git submodule foo" command)
without first finding out where the top-level is and to go there
before running it. The user did not necessarily want to go there,
and giving an easy way to find the top may merely be a workaround.
The true solution for that particular issue may be to teach that
subcommand of "git submodule" to run from anywhere, which I think
has happened recently.
Another example on the same option. Nobody should have to type
$ cd $(git rev-parse --show-toplevel)
on the command line, even if there is a legitimate reason why it is
necessary to go to the top. If it is common enough, just like we
ship completion and prompt in contrib/, we should ship a set of
common shell functions and aliases to let you do the above with:
$ cdtop
which may be defined to be something like [*1*].
And these are illustrations of how we can lose needs to use that
option from the command line. We should continue to go in that
direction.
For --short and --symbolic-full-name, I have a feeling that we
should make "describe" a front-end for these.
Just like "describe" already acts as a front-end for "name-rev" and
behaves differently, we treat the command as a way to transform an
object name into another form, and in addition to the current "do so
by expressing the given rev relative to an appropriate anchor point"
mode, teach it more ways to represent the given rev by doing
something different (e.g. these two are about expressing them as
themselves and not as relative to something else, so the "describe"
command in that mode would not walk the history to find nearby anchor
points).
As to completing "--verify", I can see how it may be useful for
people who interactively play with the command examples they find in
scripts, but otherwise I do not see much value for real end-users.
"rev-parse foobar" would say "foobar" if that does not refer to an
object and end users are more intelligent than a script to see the
difference without seeing the value of $? and error message when
running on the command line.
[Footnote]
*1* This is typed in my MUA for illustration purposes only, not
tested:
cdtop () {
local eh
eh=$(git rev-parse --is-inside-work-tree) || return
case "$eh" in
true)
eh=$(git rev-parse --show-toplevel)
test -z "$eh" || cd "$eh"
;;
false)
eh=$(git rev-parse --git-dir) || return
cd "$eh" || return
eh=$(git rev-parse --is-bare-repository) || return
test "z$eh" = ztrue || cd ..
;;
esac
}
--
To unsubscribe from this l
Re: [RFC/PATCH 0/1] cygwin: Remove the Win32 l/stat() functions
Ramsay Jones wrote:
> Michael Haggerty wrote:
>> On 06/27/2013 12:35 AM, Jeff King wrote:
> [ ... ]
>>> I think Michael's assessment above is missing one thing.
>>
>> Peff is absolutely right; for some unknown reason I was thinking of the
>> consistency check as having been already fixed.
>
> Well, the "cygwin: Remove the Win32 l/stat() functions" patch *does* fix
> the problem. :-D It's just a pity we can't use it on performance grounds. :(
>
>>> [...#ifdef out consistency check on cygwin when lock is held...]
>>
>> Yes, this would work.
>>
>> But, taking a step back, I think it is a bad idea to have an unreliable
>> stat() masquerading as a real stat(). If we want to allow the use of an
>> unreliable stat for certain purposes, let's have two stat() interfaces:
>>
>> * the true stat() (in this case I guess cygwin's slow-but-correct
>> implementation)
>>
>> * some fast_but_maybe_unreliable_stat(), which would map to stat() on
>> most platforms but might map to the Windows stat() on cygwin when so
>> configured.
>>
>> By default the true stat() would always be used. It should have to be a
>> conscious decision, taken only in specific, vetted scenarios, to use the
>> unreliable stat.
>
> You have just described my second patch! :D
Unfortunately, I have not had any time to work on the patch this weekend.
However, despite the patch being a bit rough around the edges, I decided
to send it out (see below) to get some early feedback.
Note that it passes the t3210, t3211, t5500, t3200, t3301, t7606 and t1301
tests, but I have not run the full test suite.
Comments welcome.
ATB,
Ramsay Jones
-- >8 --
Subject: [PATCH] cygwin: Add fast_[l]stat() functions
Signed-off-by: Ramsay Jones
---
builtin/apply.c| 6 +++---
builtin/commit.c | 2 +-
builtin/ls-files.c | 2 +-
builtin/rm.c | 2 +-
builtin/update-index.c | 2 +-
check-racy.c | 2 +-
compat/cygwin.c| 12
compat/cygwin.h| 10 +++---
diff-lib.c | 2 +-
diff.c | 2 +-
entry.c| 4 ++--
git-compat-util.h | 13 +++--
help.c | 5 +
path.c | 9 +
preload-index.c| 2 +-
read-cache.c | 6 +++---
unpack-trees.c | 8
17 files changed, 36 insertions(+), 53 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 0e9b631..ca26caa 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3253,7 +3253,7 @@ static int load_current(struct image *image, struct patch
*patch)
if (pos < 0)
return error(_("%s: does not exist in index"), name);
ce = active_cache[pos];
- if (lstat(name, &st)) {
+ if (fast_lstat(name, &st)) {
if (errno != ENOENT)
return error(_("%s: %s"), name, strerror(errno));
if (checkout_target(ce, &st))
@@ -3396,7 +3396,7 @@ static int check_preimage(struct patch *patch, struct
cache_entry **ce, struct s
if (previous) {
st_mode = previous->new_mode;
} else if (!cached) {
- stat_ret = lstat(old_name, st);
+ stat_ret = fast_lstat(old_name, st);
if (stat_ret && errno != ENOENT)
return error(_("%s: %s"), old_name, strerror(errno));
}
@@ -3850,7 +3850,7 @@ static void add_index_file(const char *path, unsigned
mode, void *buf, unsigned
die(_("corrupt patch for subproject %s"), path);
} else {
if (!cached) {
- if (lstat(path, &st) < 0)
+ if (fast_lstat(path, &st) < 0)
die_errno(_("unable to stat newly created file
'%s'"),
path);
fill_stat_cache_info(ce, &st);
diff --git a/builtin/commit.c b/builtin/commit.c
index 6b693c1..1d208c6 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -231,7 +231,7 @@ static void add_remove_files(struct string_list *list)
if (p->util)
continue;
- if (!lstat(p->string, &st)) {
+ if (!fast_lstat(p->string, &st)) {
if (add_to_cache(p->string, &st, 0))
die(_("updating files failed"));
} else
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index 08d9786..db66a0e 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -251,7 +251,7 @@ static void show_files(struct dir_struct *dir)
continue;
if (ce_skip_worktree(ce))
continue;
- err = lstat(ce->name, &st);
+ err = fast_lstat(ce->name, &st);
if (show_deleted && err)
show_ce_entry(tag_removed, ce);
if (show_modified
Re: [RFC/PATCH] submodule: add 'exec' option to submodule update
Am 29.06.2013 11:11, schrieb Chris Packham: > On 28/06/13 22:42, Fredrik Gustafsson wrote: >> technically it looks fine to me (except for the lack of tests) but I'm >> not sure I follow the use case. >> >> In your case, you want to run a script to determinate if that certain >> submodule should use merge or rebase depending on "whatever". And this >> can't be done with git submodule foreach because you want to know the >> sha1 to update to. Have I understood you correctly? > > Correct. We tend to have submodules that are just normal detached heads > which we don't usually touch and others that are actively developed > where we would use submodule.x.update=rebase (I personally do) but some > developers want to use stgit on those repositories. > > Another approach could be to do a 'git pull --no-recurse-submodule' then > use 'git submodule foreach script-that-does-the-rebase'. The benefit of > the patch I sent is that it can be setup using the config variables[1] > and updated the normal way along with the detached HEADs and those using > plain git branches. Wouldn't a "stgit submodule update" (which would do the Right Thing for submodules initialized with stgit by maybe just using the pull & foreach logic you described) be a better UI for solving your problem? > There may be other use-cases for integration with other tools as well > (e.g. something that updates a review tool when commits get rebased). > > -- > [1] I'm not crazy about the name of submodule.*.update.command but I > couldn't think of a better one. Hmm, if we go that route, why not do the same we do for aliases? If the submodule.*.update setting is prefixed with a '!', we just execute the shell command following. This would give everyone the freedom to do arbitrary stuff if the current none, checkout, merge & rebase won't do the trick without having to add another config option. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git stash: Avoid data loss when saving a stash
Hi! On Fri, Jun 28, 2013 at 11:39:16AM -0700, Junio C Hamano wrote: > Thanks. I'll queue it with a pair of fix-up commits on top, so that > they can later be squashed in. > > The result of squashing the fix-ups would look like this. Thanks! I agree with all of your changes. > -- >8 -- > From: Petr Baudis > Date: Fri, 28 Jun 2013 17:05:32 +0200 > Subject: [PATCH] git stash: avoid data loss when "git stash save" kills a > directory Hmm, it's a pity that the note that `git reset --hard` itself should perhaps also abort in that case got lost. I don't insist on mentioning it in the commit message, though. On Fri, Jun 28, 2013 at 02:30:15PM -0700, Junio C Hamano wrote: > -- >8 -- > Subject: treat_directory(): do not declare submodules in index to be untracked Oh, you are truly awesome! I admit that properly reviewing this patch is a little out of my depth right now as I'm not familiar with this infrastructure. I'd just like to note... > case index_gitdir: > if (dir->flags & DIR_SHOW_OTHER_DIRECTORIES) > return path_none; > - return path_untracked; > + return path_none; ...that the if-test can be removed now as both branches are the same. Petr "Pasky" Baudis -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] check-ignore doc: fix broken link to ls-files page
Signed-off-by: Ramkumar Ramachandra --- Documentation/git-check-ignore.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-check-ignore.txt b/Documentation/git-check-ignore.txt index 8e1f7ab..d2df487 100644 --- a/Documentation/git-check-ignore.txt +++ b/Documentation/git-check-ignore.txt @@ -102,7 +102,7 @@ SEE ALSO linkgit:gitignore[5] linkgit:gitconfig[5] -linkgit:git-ls-files[5] +linkgit:git-ls-files[1] GIT --- -- 1.8.3.1.643.gebeea52.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] completion: learn about --man-path
Hi, On Sat, Jun 22, 2013 at 12:25:18PM +0100, John Keeping wrote: > Signed-off-by: John Keeping > --- > contrib/completion/git-completion.bash | 2 ++ > t/t9902-completion.sh | 1 + > 2 files changed, 3 insertions(+) > > diff --git a/contrib/completion/git-completion.bash > b/contrib/completion/git-completion.bash > index 8fbf941..c3290af 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -2513,11 +2513,13 @@ __git_main () > --exec-path > --exec-path= > --html-path > + --man-path > --info-path > --work-tree= > --namespace= > --no-replace-objects > --help > + -c There are a couple of issues with this '-c' here: - We normally offer only --long-options in the completion script. - The log message doesn't mention it. - And finally the most important: it will never be offered for completion. This is the condition of this case branch: case "$cur" in --*) __gitcomp " i.e. this case branch is executed only when the current word on the command line begins with '--', but then '-c' will never match. Without the '-c' part it's "obviously correct" and together with patch 1/2 is Acked-by: SZEDER Gábor > " > ;; > *) __git_compute_porcelain_commands > diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh > index 81a1657..14d605a 100755 > --- a/t/t9902-completion.sh > +++ b/t/t9902-completion.sh > @@ -231,6 +231,7 @@ test_expect_success 'double dash "git" itself' ' > --exec-path Z > --exec-path= > --html-path Z > + --man-path Z > --info-path Z > --work-tree= > --namespace= > -- > 1.8.3.1.676.gaae6535 > > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] completion: add completer for status
SZEDER Gábor wrote: > The code is OK, the rest of the function is pretty straightforward, > but I think this line would warrant a sentence in the log message, Okay. Complete untracked pathspecs (--others), and overlay HEAD tree on index (--with-tree=HEAD) to complete pathspecs that have been removed from the filesystem + staged in the index. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] completion: add completer for status
On Fri, Jun 28, 2013 at 07:48:06PM +0530, Ramkumar Ramachandra wrote: > + __git_complete_index_file "--with-tree=HEAD --cached --others" The code is OK, the rest of the function is pretty straightforward, but I think this line would warrant a sentence in the log message, considering that at first you also wondered what '--with-tree=HEAD' is about. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] test: spell 'ls-files --delete' option correctly in test descriptions
SZEDER Gábor wrote: > -test_expect_success 'ls-files --delete' ' > +test_expect_success 'ls-files --deleted' ' > setup_absent && > test -z "$(git ls-files -d)" While at it, change this to --deleted? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] completion: add completer for rev-parse
SZEDER Gábor wrote: > Either way, why these five options? 'git rev-parse' has a lot more > options than that. We have to start somewhere, so I put in the options that I personally use. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] completion: add completer for rev-parse
On Fri, Jun 28, 2013 at 07:48:07PM +0530, Ramkumar Ramachandra wrote:
> Signed-off-by: Ramkumar Ramachandra
> ---
> contrib/completion/git-completion.bash | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash
> b/contrib/completion/git-completion.bash
> index 278018f..f2959a7 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2247,6 +2247,20 @@ _git_reset ()
> __gitcomp_nl "$(__git_refs)"
> }
>
> +_git_rev_parse ()
> +{
> + case "$cur" in
> + --*)
> + __gitcomp "
> + --short --show-toplevel --is-inside-work-tree
> + --symbolic-full-name --verify
> + "
In the completion script we support porcelain commands. I'm not sure
about 'git rev-parse', but I think it's more plumbing than porcelain.
However, I think the same about 'git ls-tree' and 'git reflog', too,
yet we have support for them in the completion script.
Either way, why these five options? 'git rev-parse' has a lot more
options than that.
> + return
> + ;;
> + esac
> + __gitcomp_nl "$(__git_refs)"
> +}
> +
> _git_revert ()
> {
> case "$cur" in
> --
> 1.8.3.1.585.g9832cb9
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 2/4] contrib: contacts: add support for multiple patches
Accept multiple patch files rather than only one. For example:
% git contacts feature/*.patch
Signed-off-by: Eric Sunshine
---
contrib/contacts/git-contacts | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts
index 9007bae..ab11670 100755
--- a/contrib/contacts/git-contacts
+++ b/contrib/contacts/git-contacts
@@ -3,7 +3,7 @@
# List people who might be interested in a patch. Useful as the argument to
# git-send-email --cc-cmd option, and in other situations.
#
-# Usage: git contacts
+# Usage: git contacts ...
use strict;
use warnings;
@@ -13,6 +13,7 @@ my $since = '5-years-ago';
my $min_percent = 10;
my $labels_rx = qr/(?:Signed-off|Reviewed|Acked)-by/;
my $id_rx = qr/[0-9a-f]{40}/i;
+my %seen;
sub format_contact {
my ($name, $email) = @_;
@@ -68,7 +69,9 @@ sub get_blame {
while (<$f>) {
if (/^$id_rx/o) {
my $id = $&;
- $commits->{$id} = { id => $id, contacts => {} };
+ $commits->{$id} = { id => $id, contacts => {} }
+ unless $seen{$id};
+ $seen{$id} = 1;
}
}
close $f;
@@ -93,6 +96,7 @@ sub commits_from_patch {
while (<$f>) {
if (/^From ($id_rx) /o) {
$id = $1;
+ $seen{$id} = 1;
last;
}
}
@@ -100,10 +104,8 @@ sub commits_from_patch {
close $f;
}
-exit 1 unless @ARGV == 1;
-
my %commits;
-commits_from_patch(\%commits, $ARGV[0]);
+commits_from_patch(\%commits, $_) for (@ARGV);
import_commits(\%commits);
my %count_per_person;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 4/4] contrib: contacts: interpret committish akin to format-patch
As a convenience, accept the same style committish as accepted by
git-format-patch. For example:
% git contacts master
will consider commits in the current branch built atop 'master', just as
"git format-patch master" will format commits built atop 'master'.
Signed-off-by: Eric Sunshine
---
contrib/contacts/git-contacts | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts
index abb90a1..10d77d3 100755
--- a/contrib/contacts/git-contacts
+++ b/contrib/contacts/git-contacts
@@ -104,9 +104,26 @@ sub commits_from_patch {
close $f;
}
+sub parse_rev_args {
+ my @args = @_;
+ open my $f, '-|',
+ qw(git rev-parse --revs-only --default HEAD --symbolic), @args
+ or die;
+ my @revs;
+ while (<$f>) {
+ chomp;
+ push @revs, $_;
+ }
+ close $f;
+ return @revs if scalar(@revs) != 1;
+ return "^$revs[0]", 'HEAD' unless $revs[0] =~ /^-/;
+ return $revs[0], 'HEAD';
+}
+
sub commits_from_rev_args {
my ($commits, $args) = @_;
- open my $f, '-|', qw(git rev-list --reverse), @$args or die;
+ my @revs = parse_rev_args(@$args);
+ open my $f, '-|', qw(git rev-list --reverse), @revs or die;
while (<$f>) {
chomp;
my $id = $_;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 3/4] contrib: contacts: add ability to parse from committish
Committishes can be mentioned along with patch files in the same
invocation. For example:
% git contacts master..feature extra/*.patch
Signed-off-by: Eric Sunshine
---
contrib/contacts/git-contacts | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts
index ab11670..abb90a1 100755
--- a/contrib/contacts/git-contacts
+++ b/contrib/contacts/git-contacts
@@ -3,7 +3,7 @@
# List people who might be interested in a patch. Useful as the argument to
# git-send-email --cc-cmd option, and in other situations.
#
-# Usage: git contacts ...
+# Usage: git contacts ...
use strict;
use warnings;
@@ -104,8 +104,32 @@ sub commits_from_patch {
close $f;
}
+sub commits_from_rev_args {
+ my ($commits, $args) = @_;
+ open my $f, '-|', qw(git rev-list --reverse), @$args or die;
+ while (<$f>) {
+ chomp;
+ my $id = $_;
+ $seen{$id} = 1;
+ open my $g, '-|', qw(git show -C --oneline), $id or die;
+ scan_hunks($commits, $id, $g);
+ close $g;
+ }
+ close $f;
+}
+
+my (@files, @rev_args);
+for (@ARGV) {
+ if (-e) {
+ push @files, $_;
+ } else {
+ push @rev_args, $_;
+ }
+}
+
my %commits;
-commits_from_patch(\%commits, $_) for (@ARGV);
+commits_from_patch(\%commits, $_) for (@files);
+commits_from_rev_args(\%commits, \@rev_args) if @rev_args;
import_commits(\%commits);
my %count_per_person;
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH/RFC 1/4] contrib: add git-contacts helper
This script lists people that might be interested in a patch by going
back through the history for each patch hunk, and finding people that
reviewed, acknowledge, signed, or authored the code the patch is
modifying.
It does this by running git-blame incrementally on each hunk and then
parsing the commit message. After gathering all participants, it
determines each person's relevance by considering how many commits
mentioned that person compared with the total number of commits under
consideration. The final output consists only of participants who pass a
minimum threshold of participation.
For example:
% git contacts 0001-remote-hg-trivial-cleanups.patch
Felipe Contreras
Jeff King
Max Horn
Junio C Hamano
Thus, it can be invoked as git-send-email's --cc-cmd option, among other
possible uses.
This is a Perl rewrite of Felipe Contreras' git-related patch series[1]
written in Ruby.
[1]: http://thread.gmane.org/gmane.comp.version-control.git/226065/
Signed-off-by: Eric Sunshine
---
To better support Windows, a follow-up patch may want to add
functionality similar to run_cmd_pipe() from git-add--interactive.perl.
contrib/contacts/git-contacts | 121 ++
1 file changed, 121 insertions(+)
create mode 100755 contrib/contacts/git-contacts
diff --git a/contrib/contacts/git-contacts b/contrib/contacts/git-contacts
new file mode 100755
index 000..9007bae
--- /dev/null
+++ b/contrib/contacts/git-contacts
@@ -0,0 +1,121 @@
+#!/usr/bin/perl
+
+# List people who might be interested in a patch. Useful as the argument to
+# git-send-email --cc-cmd option, and in other situations.
+#
+# Usage: git contacts
+
+use strict;
+use warnings;
+use IPC::Open2;
+
+my $since = '5-years-ago';
+my $min_percent = 10;
+my $labels_rx = qr/(?:Signed-off|Reviewed|Acked)-by/;
+my $id_rx = qr/[0-9a-f]{40}/i;
+
+sub format_contact {
+ my ($name, $email) = @_;
+ return "$name <$email>";
+}
+
+sub parse_commit {
+ my ($commit, $data) = @_;
+ my $contacts = $commit->{contacts};
+ my $inbody = 0;
+ for (split(/^/m, $data)) {
+ if (not $inbody) {
+ if (/^author ([^<>]+) <(\S+)> .+$/) {
+ $contacts->{format_contact($1, $2)} = 1;
+ } elsif (/^$/) {
+ $inbody = 1;
+ }
+ } elsif (/^$labels_rx:\s+([^<>]+)\s+<(\S+?)>$/o) {
+ $contacts->{format_contact($1, $2)} = 1;
+ }
+ }
+}
+
+sub import_commits {
+ my ($commits) = @_;
+ return unless %$commits;
+ my $pid = open2 my $reader, my $writer, qw(git cat-file --batch);
+ for my $id (keys(%$commits)) {
+ print $writer "$id\n";
+ my $line = <$reader>;
+ if ($line =~ /^($id_rx) commit (\d+)/o) {
+ my ($cid, $len) = ($1, $2);
+ die "expected $id but got $cid" unless $id eq $cid;
+ my $data;
+ # cat-file emits newline after data, so read len+1
+ read $reader, $data, $len + 1;
+ parse_commit($commits->{$id}, $data);
+ }
+ }
+ close $reader;
+ close $writer;
+ waitpid($pid, 0);
+ die "git-cat-file error: $?" if $?;
+}
+
+sub get_blame {
+ my ($commits, $source, $start, $len, $from) = @_;
+ $len = 1 unless defined($len);
+ return if $len == 0;
+ open my $f, '-|',
+ qw(git blame --incremental -C -C), '-L', "$start,+$len",
+ '--since', $since, "$from^", '--', $source or die;
+ while (<$f>) {
+ if (/^$id_rx/o) {
+ my $id = $&;
+ $commits->{$id} = { id => $id, contacts => {} };
+ }
+ }
+ close $f;
+}
+
+sub scan_hunks {
+ my ($commits, $id, $f) = @_;
+ my $source;
+ while (<$f>) {
+ if (/^---\s+(\S+)/) {
+ $source = substr($1, 2) unless $1 eq '/dev/null';
+ } elsif (/^@@ -(\d+)(?:,(\d+))?/ && $source) {
+ get_blame($commits, $source, $1, $2, $id);
+ }
+ }
+}
+
+sub commits_from_patch {
+ my ($commits, $file) = @_;
+ open my $f, '<', $file or die "read failure: $file: $!";
+ my $id;
+ while (<$f>) {
+ if (/^From ($id_rx) /o) {
+ $id = $1;
+ last;
+ }
+ }
+ scan_hunks($commits, $id, $f) if $id;
+ close $f;
+}
+
+exit 1 unless @ARGV == 1;
+
+my %commits;
+commits_from_patch(\%commits, $ARGV[0]);
+import_commits(\%commits);
+
+my %count_per_person;
+for my $commit (values %commits) {
+ for my $contact (keys %{$commit->{contacts}}) {
+ $count_per_person{$contact}++;
+ }
+}
+
+my $ncommits = scalar(keys %commi
[PATCH/RFC 0/4] Perl rewrite of Ruby git-related
This is a Perl rewrite of Felipe Contreras' git-related v9 patch series[1] which was written in Ruby. Although that series was ejected from 'pu'[2], Junio suggested[3,4] that such functionality may be a useful addition to the official tool-chest, hence this Perl rewrite. In this submission, the command name has changed to git-contacts since git-related felt too generic. (git-contacts seemed best of several possibilities I surveyed: git-people, git-interested, git-mentioned, git-blame-us.) This rewrite does not maintain perfect 1-to-1 parity with Felipe's v9 series, however, it is close: minor refactoring was done to eliminate a small amount of duplicate code; patch files and revision arguments are allowed in the same invocation rather than being exclusive; "git cat-file --batch" pipe deadlock is avoided; commit messages are expanded. No attempt is made to answer Junio's v9 review[5], as I lack sufficient insight with '-C' options to be able to respond properly. My Perl may be rusty and idiomatic usage may be absent. [1]: http://thread.gmane.org/gmane.comp.version-control.git/226065/ [2]: http://article.gmane.org/gmane.comp.version-control.git/229164/ [3]: http://article.gmane.org/gmane.comp.version-control.git/226425/ [4]: http://thread.gmane.org/gmane.comp.version-control.git/221728/focus=221796 [5]: http://article.gmane.org/gmane.comp.version-control.git/226265/ Eric Sunshine (4): contrib: add git-contacts helper contrib: contacts: add support for multiple patches contrib: contacts: add ability to parse from committish contrib: contacts: interpret committish akin to format-patch contrib/contacts/git-contacts | 164 ++ 1 file changed, 164 insertions(+) create mode 100755 contrib/contacts/git-contacts -- 1.8.3.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] completion: add completer for status
On Fri, Jun 28, 2013 at 07:33:21PM +0530, Ramkumar Ramachandra wrote: > Ramkumar Ramachandra wrote: > >> + __git_complete_index_file "--with-tree=HEAD --cached --deleted" > > > > Might as well go all the way with "--cached --deleted --unmerged > > --others" no? What is the point of --with-tree=HEAD? > > Ugh, --deleted doesn't work as advertised (terrible documentation). Why not? In my experiments it worked well, as you can see in my previous emails. What behavior did you observe which differs from the advertised? > The minimally correct combination we need seems to be > "--with-tree=HEAD --cached --others". Yeah, once we use '--with-tree=HEAD' we don't need '--deleted' anymore. Gábor -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/4] Update linux-2.6.git location and related examples
On Sat, Jun 29, 2013 at 06:44:34PM -0700, Junio C Hamano wrote: > "W. Trevor King" writes: > > > On Sat, Jun 22, 2013 at 10:46:23AM -0400, W. Trevor King wrote: > >> David and Junio mentioned that I'd missed a few 2.6 references in my > >> initial pass. Here's a second attempt that does some deeper > >> reworking of the effected sections. > > > > No comments after a week, so I'm giving this patch series a bump ;). > > Hmph, didn't I queue them on 'pu' already? Oops, that would be my first time having a patch series queued without comment ;). Sorry for the noise. Thanks, Trevor -- This email may be signed or encrypted with GnuPG (http://www.gnupg.org). For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy signature.asc Description: OpenPGP digital signature
[PATCH] test: spell 'ls-files --delete' option correctly in test descriptions
The option is spelled '--deleted'. Signed-off-by: SZEDER Gábor --- t/t7011-skip-worktree-reading.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t7011-skip-worktree-reading.sh b/t/t7011-skip-worktree-reading.sh index 8f3b54d8..88d60c1c 100755 --- a/t/t7011-skip-worktree-reading.sh +++ b/t/t7011-skip-worktree-reading.sh @@ -91,12 +91,12 @@ test_expect_success 'update-index --remove' ' test_cmp expected 1 ' -test_expect_success 'ls-files --delete' ' +test_expect_success 'ls-files --deleted' ' setup_absent && test -z "$(git ls-files -d)" ' -test_expect_success 'ls-files --delete' ' +test_expect_success 'ls-files --deleted' ' setup_dirty && test -z "$(git ls-files -d)" ' -- 1.8.3.1.684.g8c62402 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
