Git 1.8.2 l10n round 2

2013-02-13 Thread Jiang Xin
Hi,

Leaders of Git language teams please note that a new "git.pot" is
generated from v1.8.1.3-568-g5bf72 in the master branch. See
commit:

l10n: Update git.pot (35 new, 14 removed messages)

L10n for git 1.8.2 round 2: Generate po/git.pot from v1.8.1.3-568-g5bf72.

Signed-off-by: Jiang Xin 

This update is for the l10n of the upcoming git 1.8.2. You can get it
from the usual place:

https://github.com/git-l10n/git-po/

--
Jiang Xin
--
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] Documentation: filter-branch env-filter example

2013-02-13 Thread Johannes Sixt
Am 2/13/2013 20:47, schrieb Tade:
> filter-branch --env-filter example that shows how to change the email address
> in all commits by a certain developer.
> ---

You should sign off your patch. Use a full real name, please.

>  Documentation/git-filter-branch.txt | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/Documentation/git-filter-branch.txt
> b/Documentation/git-filter-branch.txt
> index dfd12c9..2664cec 100644
> --- a/Documentation/git-filter-branch.txt
> +++ b/Documentation/git-filter-branch.txt
> @@ -329,6 +329,19 @@ git filter-branch --msg-filter '
>  ' HEAD~10..HEAD
>  
>  
> +You can modify committer/author personal information using `--env-filter`.
> +For example, to update some developer's email address use this command:
> +
> +
> +git filter-branch --env-filter '
> +if [ $GIT_AUTHOR_EMAIL =j...@old.example.com  ]

This should read

if [ "$GIT_AUTHOR_EMAIL" = j...@old.example.com  ]

(double quotes, spaces around '='). The paragraph before the example talks
about both author and committer, but the example handles only the author;
it should handle the committer as well.

> +then
> +GIT_AUTHOR_EMAIL=j...@new.example.com
> +fi
> +export GIT_AUTHOR_EMAIL
> +' -- --all
> +
> +

The place where you inserted the example is reasonable, IMO.

>  To restrict rewriting to only part of the history, specify a revision
>  range in addition to the new branch name.  The new branch name will
>  point to the top-most revision that a 'git rev-list' of this range

-- Hannes
--
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: Very Urgent.

2013-02-13 Thread ROY PADAYACHY
EASY LOAN.  esp.ce.gov.br> writes:

> 
> 
> Hello, do you know that you can now get the LOAN you deserve without the fear
of been denied by any one? We are
> currently offering Personal and Business Loan at 2% interest rate per annual.
For urgent attention
> E-mail: kdf.ls001  xyan.us
> 
> 




--
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: What's cooking in git.git (Feb 2013, #05; Tue, 12)

2013-02-13 Thread Andrew Ardill
On 14 February 2013 15:36, Junio C Hamano  wrote:
>> That is, currently git add defaults to not staging file deletions, and
>> we provide command line flags to include them. The consensus in the
>> thread is that it is better to stage them by default; it seems
>> reasonable to me that if we stage deletions by default we should
>> provide flags to _not_ stage them. If that was the entirety of the
>> change, would your position from that thread, "if we need this
>> optional, then it is not worth doing this", still hold?
>
> If that is the change we are going to make, and if you can guarantee
> that nobody who is used to the historical behaviour will complain,
> then I am fine with it, but I think the latter part of the condition
> will not hold.

Does the impossibility of asserting that no-one will complain put this
in the 'too hard' bucket?

>> Some people would be adversely affected by this change, but any
>> objections I can come up with are not game stoppers.
>> - It is possible newcomers might stumble at deleted files being staged
>> for commit by a command called 'add',...
>
> New people are fair game; we haven't trained them with the
> "inconsistent" behaviour, and the default being different from
> historical behaviour will not affect them adversely.
>
>> - For people who rely heavily on file deletions remaining out of the
>> index, providing a flag allows them to keep their workflow.
>
> Allowing to do the things they used to be able to do is a bare
> minimum.  You are still forcing them to do things differently.

The implication here is that a relatively small number of people will
be inconvenienced by needing to specify extra flags/set up an alias.
Compare this to the many for whom the expected behaviour is now
default, and we have a net win.

>> Finally, making this change makes sense from a consistency point of
>> view.
>
> That is a given. Otherwise we wouldn't be even discussing this.

Obviously I agree. I was actually bringing up a point about patch mode
and it got incorporated into the bigger picture; patch mode includes
deletions by default and I don't even know if you can turn that
behaviour off. So, when we talk about git add -u and git add -A, we
should also mention git add -p.

Regards,

Andrew Ardill
--
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: What's cooking in git.git (Feb 2013, #05; Tue, 12)

2013-02-13 Thread Junio C Hamano
Andrew Ardill  writes:

>> We've discussed that before.
>>
>> http://thread.gmane.org/gmane.comp.version-control.git/171811/focus=171818
>
> Something that I couldn't find discussed was the option of, rather
> than providing a config to 'turn it off', inverting the current
> default/flags combo.
>
> That is, currently git add defaults to not staging file deletions, and
> we provide command line flags to include them. The consensus in the
> thread is that it is better to stage them by default; it seems
> reasonable to me that if we stage deletions by default we should
> provide flags to _not_ stage them. If that was the entirety of the
> change, would your position from that thread, "if we need this
> optional, then it is not worth doing this", still hold?

If that is the change we are going to make, and if you can guarantee
that nobody who is used to the historical behaviour will complain,
then I am fine with it, but I think the latter part of the condition
will not hold.

> Some people would be adversely affected by this change, but any
> objections I can come up with are not game stoppers.
> - It is possible newcomers might stumble at deleted files being staged
> for commit by a command called 'add',...

New people are fair game; we haven't trained them with the
"inconsistent" behaviour, and the default being different from
historical behaviour will not affect them adversely.

> - For people who rely heavily on file deletions remaining out of the
> index, providing a flag allows them to keep their workflow.

Allowing to do the things they used to be able to do is a bare
minimum.  You are still forcing them to do things differently.

> - For scripts that rely on this behaviour, a flag allows it to be
> updated, though it may break in the meantime.

Likewise.

> Finally, making this change makes sense from a consistency point of
> view.

That is a given. Otherwise we wouldn't be even discussing this.
--
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: What's cooking in git.git (Feb 2013, #05; Tue, 12)

2013-02-13 Thread Andrew Ardill
On 14 February 2013 02:27, Junio C Hamano  wrote:
>> If we need to support this behaviour than I would suppose a config
>> option is required. A default config transition path similar to git
>> push defaults would probably work well, in the case where breaking
>> these expectations is unacceptable.
>
> We've discussed that before.
>
> http://thread.gmane.org/gmane.comp.version-control.git/171811/focus=171818

Something that I couldn't find discussed was the option of, rather
than providing a config to 'turn it off', inverting the current
default/flags combo.

That is, currently git add defaults to not staging file deletions, and
we provide command line flags to include them. The consensus in the
thread is that it is better to stage them by default; it seems
reasonable to me that if we stage deletions by default we should
provide flags to _not_ stage them. If that was the entirety of the
change, would your position from that thread, "if we need this
optional, then it is not worth doing this", still hold?

Some people would be adversely affected by this change, but any
objections I can come up with are not game stoppers.
- It is possible newcomers might stumble at deleted files being staged
for commit by a command called 'add', but if they can already grok the
concept of staging then including deletions in that is trivial. If
they don't understand staging then we have a different issue.
- For people who rely heavily on file deletions remaining out of the
index, providing a flag allows them to keep their workflow. No data
would be lost, and most accidents would be easily recoverable.
- For scripts that rely on this behaviour, a flag allows it to be
updated, though it may break in the meantime. (I would presume that
not many of these scripts exist in the first place, but I don't really
know)

Finally, making this change makes sense from a consistency point of
view. For example, we don't track file renames because (and I
paraphrase) we can work that out from the content that is moved.
However if I rename a file and then 'git add .' I see that a new file
is added, not that it has been renamed! Manually adding the deletion
to the index causes git to correctly detect the rename, however this
is unintuitive and not consistent with how git works and is
communicated in general.

Git add is also inconsistent with git add -p (although that might be
due to unclear documentation for -p). When in patch mode, git add will
propose deletions get added to the index as well, not just additions
and modifications.

Regards,

Andrew Ardill
--
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: inotify to minimize stat() calls

2013-02-13 Thread Karsten Blees
Am 13.02.2013 23:55, schrieb Jeff King:
> On Wed, Feb 13, 2013 at 09:25:59PM +0100, Karsten Blees wrote:
> 
>> Alternatively, we could simply create normal cache_entries for the
>> directories that are linked via ce->next, but have a trailing '/' in
>> their name?
>>
>> Reference counting sounds good to me, at least better than allocating
>> directory entries per cache entry * parent dirs.
> 
> I think that is more or less what my patch does, but it splits the
> ref-counted fake cache_entries out into a separate hash of "struct
> dir_hash_entry" rather than storing it in the regular hash. Which IMHO
> is a bit cleaner for two reasons:
> 
>   1. You do not have to pay the memory price of storing fake
>  cache_entries (the name+refcount struct for each directory is much
>  smaller than a real cache_entry).
> 

Yes, but considering the small number of directories compared to files, I think 
this is a relatively small price to pay.

>   2. It makes the code a bit simpler, as you do not have to do any
>  "check for trailing /" magic on the result of index_name_exists to
>  determine if it is a "real" name or just a fake dir entry.
> 

True for dir.c. On the other hand, you need a lot of new find / find_or_create 
logic in name-hash.c.

Just to illustrate what I mean, here's a quick sketch (there's still a segfault 
somewhere, but I don't have time to debug right now...).

Note that hash_index_entry_directories works from right to left - if the 
immediate parent directory is there, there's no need to check the parent's 
parent.

cache_entry.dir points to the parent directory so that we don't need to lookup 
all path components for reference counting when adding / removing entries.

As directory entries are 'real' cache_entries, we can reuse the existing 
index_name_exists and hash_index_entry code.

I feel slightly guilty for abusing ce_size as reference counter...well :-)

---
 cache.h |  4 +++-
 name-hash.c | 80 -
 2 files changed, 39 insertions(+), 45 deletions(-)

diff --git a/cache.h b/cache.h
index 665b512..2bc1372 100644
--- a/cache.h
+++ b/cache.h
@@ -131,7 +131,7 @@ struct cache_entry {
unsigned int ce_namelen;
unsigned char sha1[20];
struct cache_entry *next;
-   struct cache_entry *dir_next;
+   struct cache_entry *dir;
char name[FLEX_ARRAY]; /* more */
 };
 
@@ -285,6 +285,8 @@ extern void add_name_hash(struct index_state *istate, 
struct cache_entry *ce);
 static inline void remove_name_hash(struct cache_entry *ce)
 {
ce->ce_flags |= CE_UNHASHED;
+   if (ce->dir && !(--ce->dir->ce_size))
+   remove_name_hash(ce->dir);
 }
 
 
diff --git a/name-hash.c b/name-hash.c
index d8d25c2..01e8320 100644
--- a/name-hash.c
+++ b/name-hash.c
@@ -32,6 +32,9 @@ static unsigned int hash_name(const char *name, int namelen)
return hash;
 }
 
+static struct cache_entry *lookup_index_entry(struct index_state *istate, 
const char *name, int namelen, int icase);
+static void hash_index_entry(struct index_state *istate, struct cache_entry 
*ce);
+
 static void hash_index_entry_directories(struct index_state *istate, struct 
cache_entry *ce)
 {
/*
@@ -40,30 +43,25 @@ static void hash_index_entry_directories(struct index_state 
*istate, struct cach
 * closing slash.  Despite submodules being a directory, they never
 * reach this point, because they are stored without a closing slash
 * in the cache.
-*
-* Note that the cache_entry stored with the directory does not
-* represent the directory itself.  It is a pointer to an existing
-* filename, and its only purpose is to represent existence of the
-* directory in the cache.  It is very possible multiple directory
-* hash entries may point to the same cache_entry.
 */
-   unsigned int hash;
-   void **pos;
+   int len = ce_namelen(ce);
+   if (len && ce->name[len - 1] == '/')
+   len--;
+   while (len && ce->name[len - 1] != '/')
+   len--;
+   if (!len)
+   return;
 
-   const char *ptr = ce->name;
-   while (*ptr) {
-   while (*ptr && *ptr != '/')
-   ++ptr;
-   if (*ptr == '/') {
-   ++ptr;
-   hash = hash_name(ce->name, ptr - ce->name);
-   pos = insert_hash(hash, ce, &istate->name_hash);
-   if (pos) {
-   ce->dir_next = *pos;
-   *pos = ce;
-   }
-   }
+   ce->dir = lookup_index_entry(istate, ce->name, len, ignore_case);
+   if (!ce->dir) {
+   ce->dir = xcalloc(1, cache_entry_size(len));
+   memcpy(ce->dir->name, ce->name, len);
+   ce->dir->ce_namelen = len;
+   ce->dir->name[len] = 0;
+   ha

[BUG] Veryfing signatures in git log fails when language is not english

2013-02-13 Thread XANi
Hi,

any functionality that depends on exact exit msg of program
 can potentially fail because of that
ᛯ export |grep LANG
declare -x LANG="pl_PL.UTF-8"

ᛯ ~/src/os/git/git log --format="%G? %h" |head -2 
 0d19377
 5b9d7f8

ᛯ unset LANG
ᛯ ~/src/os/git/git log --format="%G? %h" |head -2
G 0d19377
G 5b9d7f8

tested against maint (d32805d) and master (5bf72ed)

maybe git should set up some output-changing variables before calling
external programs? I think setting LC_ALL=C should be enougth.

-- 
Mariusz Gronczewski (XANi) 
GnuPG: 0xEA8ACE64




signature.asc
Description: PGP signature


Re: Cumulative "git read-tree -m" rejected with overwriting warning

2013-02-13 Thread Junio C Hamano
Marcin Owsiany  writes:

>  "the index file saves and restores with all this information, so you
>  can merge things incrementally,"
>
> which I took to mean that I can read from multiple trees one by one
> before writing the tree.

That "incrementally" refers to "after a three-way merge stops with
conflicts in multiple files, you can start from file A, concentrate
only on that file, resolve it, and then go on to resolve conflicts
in the next file B, continue, until you resolve the conflicts in the
last file Z".  Until you resolve a single tree-way merge fully and
write the results out as a tree, you cannot merge in the next tree.

That is why N-way octopus is internally implemented as a series of
three-way merges.
--
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


Cumulative "git read-tree -m" rejected with overwriting warning

2013-02-13 Thread Marcin Owsiany
Hello,

Consider the following use case:

  git init
  seq 0 9 > f
  git add f
  git commit -m start
  
  git checkout -b b1
  perl -pi -e 's,0,b1,' f
  git commit -a -m b1
  
  git checkout -b b2
  perl -pi -e 's,9,b2,' f
  git commit -a -m b2
  
  git checkout master
  git merge b1 b1

As the changes to "f" don't conflict, the octopus merge deals with them
just fine, and I get a merge in a single commit object.


Now, let's say my b1 and b2 branches are a bit more special, and apart
from the main contents (i.e. the "f" file), each branch contains a
couple of files with branch-specific metadata. The names of those files
are the same in each branch.  (People familiar with topgit can probably
guess I'm talking about topgit branches here.)

I'd like to merge all such branches into "master" in one octopus-like
commit, but the metadata files introduce a conflict during simple
octopus merge.  However I don't care about that metadata when the
branches are merged into "master", so I'm trying to write a script which
would do such merge while discarding the metadata files.

The problem is, I cannot get it to work, when two branches modify the
same file (like "f" above), even if the changes don't conflict. I get
either:

error: Entry 'f' would be overwritten by merge. Cannot merge.
 - when trying to use the two-arg form of "git read-tree -m -i"

or:
f: needs merge
 - when trying to use the three-arg form

Attached is the minimal test case that reproduces the problem.

It might just be that git-read-tree cannot do what I need, and I'm just
misinterpreting the meaning of:

 "the index file saves and restores with all this information, so you
 can merge things incrementally,"

which I took to mean that I can read from multiple trees one by one
before writing the tree.

Could anyone give me some hints?

-- 
Marcin Owsiany   http://marcin.owsiany.pl/
GnuPG: 2048R/02F946FC  35E9 1344 9F77 5F43 13DD  6423 DBF4 80C6 02F9 46FC

"Every program in development at MIT expands until it can read mail."
  -- Unknown


testcase.sh
Description: Bourne shell script


Re: inotify to minimize stat() calls

2013-02-13 Thread Jeff King
On Wed, Feb 13, 2013 at 09:25:59PM +0100, Karsten Blees wrote:

> Am 13.02.2013 19:18, schrieb Jeff King:
> > Moreover, looking at it again, I
> > don't think my patch produces the right behavior: we have a single
> > dir_next pointer, even though the same ce_entry may appear under many
> > directory hashes. So the cache_entries that has to "dir/foo/" and those
> > that hash to "dir/bar/" may get confused, because they will also both be
> > found under "dir/", and both try to create a linked list from the
> > dir_next pointer.
> > 
> 
> Indeed. In the worst case, this causes an endless loop if ce->dir_next == ce
> ---8<---
> mkdir V
> mkdir V/XQANY
> mkdir WURZAUP
> touch V/XQANY/test
> git init
> git config core.ignorecase true
> git add .
> git status
> ---8<---

Great, thanks for the test case. I can trivially replicate the endless
loop. The patch I sent earlier fixes that. So it's at least a step in
the (possible) right direction. I'm slightly concerned that there is
some other case that is expecting the directories in the main hash, but
I think I got them all.

> Note: "V/", "V/XQANY/" and "WURZAUP/" all have the same hash_name.
> Although I found those strange values by brute force, hash collisions
> in 32 bit values are not that uncommon in real life :-)

Cute. :)

> Alternatively, we could simply create normal cache_entries for the
> directories that are linked via ce->next, but have a trailing '/' in
> their name?
>
> Reference counting sounds good to me, at least better than allocating
> directory entries per cache entry * parent dirs.

I think that is more or less what my patch does, but it splits the
ref-counted fake cache_entries out into a separate hash of "struct
dir_hash_entry" rather than storing it in the regular hash. Which IMHO
is a bit cleaner for two reasons:

  1. You do not have to pay the memory price of storing fake
 cache_entries (the name+refcount struct for each directory is much
 smaller than a real cache_entry).

  2. It makes the code a bit simpler, as you do not have to do any
 "check for trailing /" magic on the result of index_name_exists to
 determine if it is a "real" name or just a fake dir entry.

-Peff
--
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 v2] git-multimail: a replacement for post-receive-email

2013-02-13 Thread Michael Haggerty
On 02/13/2013 03:56 PM, Matthieu Moy wrote:
> Michael Haggerty  writes:
> 
>> A while ago, I submitted an RFC for adding a new email notification
>> script to "contrib" [1].  The reaction seemed favorable and it was
>> suggested that the new script should replace post-receive-email rather
>> than be added separately, ideally with some kind of migration support.
> 
> I think replacing the old post-receive-email is a sane goal in the long
> run, but a good first step would be to have git-multimail merged in
> contrib, and start considering the old script as deprecated (keeping the
> old script doesn't harm IMHO, it's a one-file, 3 commits/year script,
> not really a maintainance burden).
> 
> I started playing with git-multimail. In short, I do like it but had to
> fight a bit with python to get it to work, and couldn't get it to do
> exactly what I expect. Pull request attached :-).

Thanks very much for your feedback and patches.

> Installation troubles:
> 
> I had an old python installation (Red Hat package, and I'm not root),
> that did not include the email.utils package, so I couldn't use my
> system's python. I found no indication about python version in README,
> so I installed the latest python by hand, just to find out that
> git-multimail wasn't compatible with Python 3.x. 2to3 can fix
> automatically a number of 3.x compatibility issues, but not all of them
> so I gave up and installed Python 2.7.

What version of Python was it that caused problems?  I just discovered
that the script wouldn't have worked with Python 2.4, where
"email.utils" used to be called "email.Utils".  But I pushed a fix to
GitHub:

ddb1796660 Accommodate older versions of Python's email module.

With this change, I think that git-multimail will work with any version
of Python 2.4 <= x < 3.0.  So if your original problem was with Python
2.4, maybe you could try the new version and see if it works with that
interpreter.

> I think adding a short "dependencies" section in the README (or in an
> INSTALL file) saying which Python version works could save new users the
> trouble (I see the sheebang inside the scripts says python2 but since I
> couldn't use my system's python and called
> "path/to/python git_multimail.py", this didn't help).

Yes, I'm working on a "Requirements" section with that information and
more.  I'd like to list a minimum git version too, but it would be quite
a bit of work to figure out when each command and each option was added.
 It would be helpful if anybody who has used the script with an old
version of git lets me know whether they were successful or not.

> Making the script
> portable with python 2 and 3 would be awesome ;-).

Agreed, but I doubt I will be able to get to it very soon.

> Missing feature:
> 
> git-multimail can send a summary for each push, with the "git log --oneline"
> of the new revisions, and then 1 mail per patch with the complete log
> and the patch.
> 
> I'd like to have the intermediate: allow the summary email to include
> the complete log (not just --oneline). My colleagues already think they
> receive too many emails so I don't think they'd like the "one email per
> commit" way, but the 1 line summary is really short OTOH.
> 
> I wrote a quick and hopefully not-too-dirty implementation of it,
> there's a pull request here:
> 
> https://github.com/mhagger/git-multimail/pull/6
> 
> essentially, it boils down to:
> 
> @@ -835,6 +837,17 @@ class ReferenceChange(Change):
>  for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
>  yield line
>  
> +if adds and self.showlog:
> +yield '\n'
> +yield 'Detailed log of added commits:\n\n'
> +for line in read_lines(
> +['git', 'log']
> ++ self.logopts
> ++ ['%s..%s' % (self.old.commit, self.new.commit,)],
> +keepends=True,
> +):
> +yield line
> +
>  # The diffstat is shown from the old revision to the new
>  # revision.  This is to show the truth of what happened in
>  # this change.  There's no point showing the stat from the
> 

Thanks for the patch.  I like the idea, but I think the implementation
is incorrect.  Your code will not only list new commits but will also
list commits that were already in the repository on another branch
(e.g., if an existing feature branch is merged into master, all of the
commits on the feature branch will be listed).  (Or was that your
intention?)  But even worse, it will fail to list commits that were
added at the same time that a branch was created (e.g., if I create a
feature branch with a number of commits on it and then push it for the
first time).

Probably the Push object has to negotiate with its constituent
ReferenceChange objects to figure out which one is responsible for
summarizing each of the commits newly added 

[PATCH v3 3/3] t9903: add extra tests for bash.showDirtyState

2013-02-13 Thread Martin Erik Werner
Add 3 extra tests for the bash.showDirtyState config option, the test
now cover all combinations of the shell var being set/unset and the
config option being missing/enabled/disabled, given a dirty file.

* Renamed test 'disabled by config' to 'shell variable set with config
  disabled' for consistency

Signed-off-by: Martin Erik Werner 
---
 t/t9903-bash-prompt.sh |   38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index dd9ac6a..2101d91 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - 
before root commit' '
test_cmp expected "$actual"
 '
 
-test_expect_success 'prompt - dirty status indicator - disabled by config' '
+test_expect_success 'prompt - dirty status indicator - shell variable unset 
with config disabled' '
printf " (master)" > expected &&
echo "dirty" > file &&
test_when_finished "git reset --hard" &&
test_config bash.showDirtyState false &&
(
+   sane_unset GIT_PS1_SHOWDIRTYSTATE &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - shell variable unset 
with config enabled' '
+   printf " (master)" > expected &&
+   echo "dirty" > file &&
+   test_when_finished "git reset --hard" &&
+   test_config bash.showDirtyState true &&
+   (
+   sane_unset GIT_PS1_SHOWDIRTYSTATE &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - shell variable set with 
config disabled' '
+   printf " (master)" > expected &&
+   echo "dirty" > file &&
+   test_when_finished "git reset --hard" &&
+   test_config bash.showDirtyState false &&
+   (
+   GIT_PS1_SHOWDIRTYSTATE=y &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - shell variable set with 
config enabled' '
+   printf " (master *)" > expected &&
+   echo "dirty" > file &&
+   test_when_finished "git reset --hard" &&
+   test_config bash.showDirtyState true &&
+   (
GIT_PS1_SHOWDIRTYSTATE=y &&
__git_ps1 > "$actual"
) &&
-- 
1.7.10.4

--
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 v3 2/3] t9903: add tests for bash.showUntrackedFiles

2013-02-13 Thread Martin Erik Werner
Add 4 test for the bash.showUntrackedFiles config option, the tests now
cover all combinations of the shell var being set/unset and the config
option being missing/enabled/disabled.

Signed-off-by: Martin Erik Werner 
---
 t/t9903-bash-prompt.sh |   40 
 1 file changed, 40 insertions(+)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index f17c1f8..dd9ac6a 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status 
indicator - untracked files
test_cmp expected "$actual"
 '
 
+test_expect_success 'prompt - untracked files status indicator - shell 
variable unset with config disabled' '
+   printf " (master)" > expected &&
+   test_config bash.showUntrackedFiles false &&
+   (
+   sane_unset GIT_PS1_SHOWUNTRACKEDFILES &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - shell 
variable unset with config enabled' '
+   printf " (master)" > expected &&
+   test_config bash.showUntrackedFiles true &&
+   (
+   sane_unset GIT_PS1_SHOWUNTRACKEDFILES &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - shell 
variable set with config disabled' '
+   printf " (master)" > expected &&
+   test_config bash.showUntrackedFiles false &&
+   (
+   GIT_PS1_SHOWUNTRACKEDFILES=y &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - shell 
variable set with config enabled' '
+   printf " (master %%)" > expected &&
+   test_config bash.showUntrackedFiles true &&
+   (
+   GIT_PS1_SHOWUNTRACKEDFILES=y &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
 test_expect_success 'prompt - untracked files status indicator - not shown 
inside .git directory' '
printf " (GIT_DIR!)" > expected &&
(
-- 
1.7.10.4

--
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 3/3] t9903: add extra tests for bash.showDirtyState

2013-02-13 Thread Junio C Hamano
Martin Erik Werner  writes:

>> OK, I'll locally amend the patch.  Thanks.
>
> Ok, so I shouldn't reroll them with s/unset -v/sane_unset/ and reworded
> commits + sign-off then, I can if you prefer that?

You can if you wanted to.  That would be less work for me ;-).
--
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 3/3] t9903: add extra tests for bash.showDirtyState

2013-02-13 Thread Martin Erik Werner
On Wed, 2013-02-13 at 11:53 -0800, Junio C Hamano wrote:
> Martin Erik Werner  writes:
> 
> >> Strictly speaking, you have 6 not 4 combinations (shell variable
> >> set/unset * config missing/set to false/set to true).  I think these
> >> additional tests cover should all 6 because "config missing" case
> >> should already have had tests before bash.showDirtyState was added.
> >> 
> >
> > Indeed, I only mentioned 4 since the other ones existed already, and I
> > didn't change them, but maybe it should be mentioned as "combined with
> > previous tests (...) cover all 6 combinations (...)" then?
> 
> It should be sufficient to change the third line of your original to
> say "the config option being missing/enabled/disabled, given a dirty
> file." and nothing else, I think.
> 
> >> Sign-off?
> >
> > Ah, just forgot the -s flag on that commit, yes it should be Signed-off
> > by me.
> 
> OK, I'll locally amend the patch.  Thanks.

Ok, so I shouldn't reroll them with s/unset -v/sane_unset/ and reworded
commits + sign-off then, I can if you prefer that?

-- 
Martin Erik Werner 

--
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: inotify to minimize stat() calls

2013-02-13 Thread Karsten Blees
Am 13.02.2013 19:18, schrieb Jeff King:
> Moreover, looking at it again, I
> don't think my patch produces the right behavior: we have a single
> dir_next pointer, even though the same ce_entry may appear under many
> directory hashes. So the cache_entries that has to "dir/foo/" and those
> that hash to "dir/bar/" may get confused, because they will also both be
> found under "dir/", and both try to create a linked list from the
> dir_next pointer.
> 

Indeed. In the worst case, this causes an endless loop if ce->dir_next == ce
---8<---
mkdir V
mkdir V/XQANY
mkdir WURZAUP
touch V/XQANY/test
git init
git config core.ignorecase true
git add .
git status
---8<---
Note: "V/", "V/XQANY/" and "WURZAUP/" all have the same hash_name. Although I 
found those strange values by brute force, hash collisions in 32 bit values are 
not that uncommon in real life :-)

> Looking at Karsten's patch, it seems like it will not add a cache entry
> if there is one of the same name. But I'm not sure if that is right, as
> the old one might be CE_UNHASHED (or it might get removed later). You
> actually want to be able to find each cache_entry that has a file under
> the directory at the hash of that directory, so you can make sure it is
> still valid.
> 

Yes, the patch was just to show potential performance savings, I didn't 
consider CE_UNHASHED at all.

> I think the best way forward is to actually create a separate hash table
> for the directory lookups. I note that we only care about these entries
> in directory_exists_in_index_icase, which is really about whether
> something is there, versus what exactly is there. So could we maybe get
> by with a separate hash table that stores a count of entries at each
> directory, and increment/decrement the count when we add/remove entries?
> 

Alternatively, we could simply create normal cache_entries for the directories 
that are linked via ce->next, but have a trailing '/' in their name?

Reference counting sounds good to me, at least better than allocating directory 
entries per cache entry * parent dirs.

--
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] Documentation: filter-branch env-filter example

2013-02-13 Thread Tade

filter-branch --env-filter example that shows how to change the email address
in all commits by a certain developer.
---
 Documentation/git-filter-branch.txt | 13 +
 1 file changed, 13 insertions(+)

diff --git a/Documentation/git-filter-branch.txt 
b/Documentation/git-filter-branch.txt
index dfd12c9..2664cec 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -329,6 +329,19 @@ git filter-branch --msg-filter '
 ' HEAD~10..HEAD
 
 
+You can modify committer/author personal information using `--env-filter`.

+For example, to update some developer's email address use this command:
+
+
+git filter-branch --env-filter '
+   if [ $GIT_AUTHOR_EMAIL =j...@old.example.com  ]
+   then
+   GIT_AUTHOR_EMAIL=j...@new.example.com
+   fi
+   export GIT_AUTHOR_EMAIL
+' -- --all
+
+
 To restrict rewriting to only part of the history, specify a revision
 range in addition to the new branch name.  The new branch name will
 point to the top-most revision that a 'git rev-list' of this range
-- 1.7.11.7

--
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] Makefile: don't run rm without any files

2013-02-13 Thread Jonathan Nieder
Junio C Hamano wrote:

> I amended the log message like so:
>
> commit bd9df384b16077337fffe9836c9255976b0e7b91
> Author: Matt Kraai 
> Date:   Wed Feb 13 07:57:48 2013 -0800
>
> Makefile: don't run rm without any files
>
> When COMPUTE_HEADER_DEPENDENCIES is set to "auto" and the compiler
> does not support it, $(dep_dirs) becomes empty.  "make clean" runs
> "rm -rf $(dep_dirs)", which fails in such a case.

To pedantic, that only fails on some platforms.  The autoconf manual
explains:

It is not portable to invoke rm without options or operands. On the
other hand, Posix now requires rm -f to silently succeed when there are
no operands (useful for constructs like rm -rf $filelist without first
checking if ‘$filelist’ was empty). But this was not always portable; at
least NetBSD rm built before 2008 would fail with a diagnostic.

Anyway, looks like a good fix.  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] Makefile: don't run rm without any files

2013-02-13 Thread Junio C Hamano
Matt Kraai  writes:

> I don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to "auto".
> The automatic detection determines that the compiler doesn't support
> it, so it's then set to "no".  CHECK_HEADER_DEPENDENCIES isn't set
> either, so about 20 lines below the dep_dirs assignment you quoted,
> dep_dirs is cleared:
>
>  ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
>  ifndef CHECK_HEADER_DEPENDENCIES
>  dep_dirs =
>  ...
>
> Should I submit an updated patch with a different commit message?

I amended the log message like so:

commit bd9df384b16077337fffe9836c9255976b0e7b91
Author: Matt Kraai 
Date:   Wed Feb 13 07:57:48 2013 -0800

Makefile: don't run rm without any files

When COMPUTE_HEADER_DEPENDENCIES is set to "auto" and the compiler
does not support it, $(dep_dirs) becomes empty.  "make clean" runs
"rm -rf $(dep_dirs)", which fails in such a case.

Signed-off-by: Matt Kraai 
Signed-off-by: Junio C Hamano 

--
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 v4] submodule: add 'deinit' command

2013-02-13 Thread Junio C Hamano
Jens Lehmann  writes:

> Junio, this looks like a we have v5 as soon as we decide what to do
> with the "not initialized" messages when '.' is used, right?

OK.  I myself do not deeply care if we end up special casing "." or
not; I'll leave it up to you and other submodule folks.

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 v2 3/3] t9903: add extra tests for bash.showDirtyState

2013-02-13 Thread Junio C Hamano
Martin Erik Werner  writes:

>> Strictly speaking, you have 6 not 4 combinations (shell variable
>> set/unset * config missing/set to false/set to true).  I think these
>> additional tests cover should all 6 because "config missing" case
>> should already have had tests before bash.showDirtyState was added.
>> 
>
> Indeed, I only mentioned 4 since the other ones existed already, and I
> didn't change them, but maybe it should be mentioned as "combined with
> previous tests (...) cover all 6 combinations (...)" then?

It should be sufficient to change the third line of your original to
say "the config option being missing/enabled/disabled, given a dirty
file." and nothing else, I think.

>> Sign-off?
>
> Ah, just forgot the -s flag on that commit, yes it should be Signed-off
> by me.

OK, I'll locally amend the patch.  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 v2 2/3] t9903: add tests for bash.showUntrackedFiles

2013-02-13 Thread Junio C Hamano
Martin Erik Werner  writes:

> So would it make sense to do:
>   GIT_PS1_SHOWUNTRACKEDFILES="dummy" &&
>   unset GIT_PS1_SHOWUNTRACKEDFILES &&
>   (...)
> instead then?

I think we have sane_unset exactly for this reason.
--
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: inotify to minimize stat() calls

2013-02-13 Thread Jeff King
On Wed, Feb 13, 2013 at 01:18:51PM -0500, Jeff King wrote:

> I think the best way forward is to actually create a separate hash table
> for the directory lookups. I note that we only care about these entries
> in directory_exists_in_index_icase, which is really about whether
> something is there, versus what exactly is there. So could we maybe get
> by with a separate hash table that stores a count of entries at each
> directory, and increment/decrement the count when we add/remove entries?
> 
> The biggest problem I see with that is that we do indeed care a little
> bit what is at the directory: we check the mode to see if it is a gitdir
> or not. But I think we can maybe sneak around that: gitdirs have actual
> entries in the index, whereas the directories do not. So we would find
> them via index_name_exists; anything that is not there, but _is_ in the
> special directory hash would therefore be a directory.
> 
> I realize it got pretty esoteric there in the middle. I'll see if I can
> work up a patch that expresses what I'm thinking.

So here's a patch. It's mostly meant to illustrate what I'm thinking,
and I have no clue if it introduces regressions. It does pass the test
suite, but we have virtually no ignorecase tests.  It seems to behave
sanely when I set core.ignorecase on my Linux box, but I have no idea
what it will do on a real case-insensitive system (nor even, to be
honest, what kinds of scenarios should be tested for the dir-hashing
stuff).

---
diff --git a/cache.h b/cache.h
index e493563..6630a35 100644
--- a/cache.h
+++ b/cache.h
@@ -131,7 +131,6 @@ struct cache_entry {
unsigned int ce_namelen;
unsigned char sha1[20];
struct cache_entry *next;
-   struct cache_entry *dir_next;
char name[FLEX_ARRAY]; /* more */
 };
 
@@ -267,26 +266,14 @@ extern void add_name_hash(struct index_state *istate, 
struct cache_entry *ce);
unsigned name_hash_initialized : 1,
 initialized : 1;
struct hash_table name_hash;
+   struct hash_table dir_hash;
 };
 
 extern struct index_state the_index;
 
 /* Name hashing */
 extern void add_name_hash(struct index_state *istate, struct cache_entry *ce);
-/*
- * We don't actually *remove* it, we can just mark it invalid so that
- * we won't find it in lookups.
- *
- * Not only would we have to search the lists (simple enough), but
- * we'd also have to rehash other hash buckets in case this makes the
- * hash bucket empty (common). So it's much better to just mark
- * it.
- */
-static inline void remove_name_hash(struct cache_entry *ce)
-{
-   ce->ce_flags |= CE_UNHASHED;
-}
-
+extern void remove_name_hash(struct index_state *istate, struct cache_entry 
*ce);
 
 #ifndef NO_THE_INDEX_COMPATIBILITY_MACROS
 #define active_cache (the_index.cache)
@@ -443,6 +430,7 @@ extern struct cache_entry *index_name_exists(struct 
index_state *istate, const c
 extern int unmerged_index(const struct index_state *);
 extern int verify_path(const char *path);
 extern struct cache_entry *index_name_exists(struct index_state *istate, const 
char *name, int namelen, int igncase);
+extern int index_icase_dir_exists(struct index_state *istate, const char 
*name, int namelen);
 extern int index_name_pos(const struct index_state *, const char *name, int 
namelen);
 #define ADD_CACHE_OK_TO_ADD 1  /* Ok to add */
 #define ADD_CACHE_OK_TO_REPLACE 2  /* Ok to replace file/directory */
diff --git a/dir.c b/dir.c
index 57394e4..f73ac34 100644
--- a/dir.c
+++ b/dir.c
@@ -927,29 +927,27 @@ static enum exist_status 
directory_exists_in_index_icase(const char *dirname, in
  */
 static enum exist_status directory_exists_in_index_icase(const char *dirname, 
int len)
 {
-   struct cache_entry *ce = index_name_exists(&the_index, dirname, len + 
1, ignore_case);
-   unsigned char endchar;
-
-   if (!ce)
-   return index_nonexistent;
-   endchar = ce->name[len];
+   struct cache_entry *ce = index_name_exists(&the_index, dirname, len, 
ignore_case);
 
/*
-* The cache_entry structure returned will contain this dirname
-* and possibly additional path components.
+* We found something in the index, which means it is either an actual
+* file, or a gitdir.
 */
-   if (endchar == '/')
-   return index_directory;
+   if (ce) {
+   if (S_ISGITLINK(ce->ce_mode))
+   return index_gitdir;
+   /* We call a file "index_nonexistent" here, because the caller is
+* asking about a directory.  */
+   return index_nonexistent;
+   }
 
/*
-* If there are no additional path components, then this cache_entry
-* represents a submodule.  Submodules, despite being directories,
-* are stored in the cache without a closing slash.
+* Otherwise, it might be a leading path of something that is in the
+* index. We can look it up in the special dir hash.
   

Re: [PATCH v4] submodule: add 'deinit' command

2013-02-13 Thread Jens Lehmann
Am 12.02.2013 18:11, schrieb Phil Hord:
> On Wed, Feb 6, 2013 at 4:11 PM, Jens Lehmann  wrote:
> +   die_if_unmatched "$mode"
>> +   name=$(module_name "$sm_path") || exit
>> +   url=$(git config submodule."$name".url)
>> +   if test -z "$url"
>> +   then
>> +   say "$(eval_gettext "No url found for submodule path 
>> '\$sm_path' in .git/config")"
> 
> Is it safe to shelter the user a little bit more from the git
> internals here and say instead:
> 
>Submodule '\$sm_path' is not initialized.

Yeah, that makes much more sense. But I'd rather use the name too:

   Submodule '\$name' is not initialized for path '\$sm_path'

> Also, I think this code will show this message for each submodule on
> 'git submodule deinit .'  But I think I would prefer to suppress it in
> that case.  If I have not explicitly stated which submodules to
> deinit, then I do not think git should complain that some of them are
> not initialized.

Yes, that message gets printed for each uninitialized submodule. We
could easily suppress that for '.', but it would be really hard to
get that right for other wildcards like 'foo*'. (And e.g. running a
"submodule update" also lists all submodules it skips because of an
"update=none" setting, so I'm not sure if it's that important here)

But if people really want that, I'd suppress that for the '.' case.
Further opinions?

>> +   continue
>> +   fi
>> +
>> +   # Remove the submodule work tree (unless the user already 
>> did it)
>> +   if test -d "$sm_path"
>> +   then
>> +   # Protect submodules containing a .git directory
>> +   if test -d "$sm_path/.git"
>> +   then
>> +   echo >&2 "$(eval_gettext "Submodule work 
>> tree $sm_path contains a .git directory")"
>> +   die "$(eval_gettext "(use 'rm -rf' if you 
>> really want to remove it including all of its history)")"
> 
> I expect this is the right thing to do for now.  But I wonder if we
> can also move $sm_path/.git to $GIT_DIR/modules/$sm_path in this case
> (though I think I am not spelling this path correctly).  Would that be
> ok?  What extra work is needed to relocate the .git dir like this?

Hmm, I'm a bit torn about automagically moving the repo somewhere
else. While I think it is a sane solution for most users I suspect
some users might hate us for doing that without asking (and with no
option to turn that off). What about adding a separate "git submodule
to-gitfile" command which does that and hinting that here? However I
do have the feeling that this should be done in another commit.

>> +   die "$(eval_gettext "Submodule work tree 
>> $sm_path contains local modifications, use '-f' to discard them")"
> 
> Nit, grammar: use a semicolon instead of a comma.

Ok. And while looking at it I noticed that "$sm_path" should be
"'\$sm_path'" here, same a few lines above ... sigh

>> +test_expect_success 'set up a second submodule' '
>> +   git submodule add ./init2 example2 &&
>> +   git commit -m "submodle example2 added"
> 
> Nit: submodule is misspelled

Thanks.

Junio, this looks like a we have v5 as soon as we decide what to do
with the "not initialized" messages when '.' is used, right?

My current diff to v4 looks like this:

-8<-
diff --git a/git-submodule.sh b/git-submodule.sh
index f1b552f..27f8e12 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -591,7 +591,7 @@ cmd_deinit()
url=$(git config submodule."$name".url)
if test -z "$url"
then
-   say "$(eval_gettext "No url found for submodule path 
'\$sm_path' in .git/config")"
+   say "$(eval_gettext "Submodule '\$name' is not 
initialized for path '\$sm_path'")"
continue
fi

@@ -601,14 +601,14 @@ cmd_deinit()
# Protect submodules containing a .git directory
if test -d "$sm_path/.git"
then
-   echo >&2 "$(eval_gettext "Submodule work tree 
$sm_path contains a .git directory")"
+   echo >&2 "$(eval_gettext "Submodule work tree 
'\$sm_path' contains a .git directory")"
die "$(eval_gettext "(use 'rm -rf' if you 
really want to remove it including all of its history)")"
fi

if test -z "$force"
then
git rm -n "$sm_path" ||
-   die "$(eval_gettext "Submodule work tree 
$sm_path contains local modifications, use '-f' to discard them")"
+   die "$(eval_gettext "Submodule work tree 
'\$sm_path' contains local modificatio

Re: inotify to minimize stat() calls

2013-02-13 Thread Jeff King
On Wed, Feb 13, 2013 at 07:15:47PM +0700, Nguyen Thai Ngoc Duy wrote:

> On Wed, Feb 13, 2013 at 3:48 AM, Karsten Blees  
> wrote:
> > 2.) 0.135 s is spent in name-hash.c/hash_index_entry_directories, 
> > reindexing the same directories over and over again. In the end, the 
> > hashtable contains 939k directory entries, even though the WebKit test repo 
> > only has 7k directories. Checking if a directory entry already exists could 
> > reduce that, i.e.:
> 
> This function is only used when core.ignorecase = true. I probably
> won't be able to test this, so I'll leave this to other people who
> care about ignorecase.
> 
> This function used to have lookup_hash, but it was removed by Jeff in
> 2548183 (fix phantom untracked files when core.ignorecase is set -
> 2011-10-06). There's a looong commit message which I'm too lazy to
> read. Anybody who works on this should though.

Yeah, the problem that commit tried to solve is that linking to a single
cache entry through the hash is not enough, because we may remove cache
items. Imagine you have "dir/one" and "dir/two", and you add them to the
in-memory index in that order. The original code hashed "dir/" and
inserted a link to the "dir/one" cache entry. When it came time to put
in the "dir/two" entry, we noticed that there was already a "dir/" entry
and did nothing. Then later, if we remove "dir/one", we do so by marking
it with CE_UNHASHED. So a later query for "dir/" will see "nope, nothing
here that wasn't CE_UNHASHED", which is wrong. We never recorded that
"dir/two" existed under the hash for "dir/", so we can't know about it.

My patch just stores the cache_entry for both under the "dir/" hash.
As Karsten noticed, that can lead to a large number of hash entries,
because adding "some/deep/hierarchy/with/files" will add 4 directory
entries for just that single file. Moreover, looking at it again, I
don't think my patch produces the right behavior: we have a single
dir_next pointer, even though the same ce_entry may appear under many
directory hashes. So the cache_entries that has to "dir/foo/" and those
that hash to "dir/bar/" may get confused, because they will also both be
found under "dir/", and both try to create a linked list from the
dir_next pointer.

Looking at Karsten's patch, it seems like it will not add a cache entry
if there is one of the same name. But I'm not sure if that is right, as
the old one might be CE_UNHASHED (or it might get removed later). You
actually want to be able to find each cache_entry that has a file under
the directory at the hash of that directory, so you can make sure it is
still valid.

And of course that still leaves the existing correctness problem I
mentioned above.

I think the best way forward is to actually create a separate hash table
for the directory lookups. I note that we only care about these entries
in directory_exists_in_index_icase, which is really about whether
something is there, versus what exactly is there. So could we maybe get
by with a separate hash table that stores a count of entries at each
directory, and increment/decrement the count when we add/remove entries?

The biggest problem I see with that is that we do indeed care a little
bit what is at the directory: we check the mode to see if it is a gitdir
or not. But I think we can maybe sneak around that: gitdirs have actual
entries in the index, whereas the directories do not. So we would find
them via index_name_exists; anything that is not there, but _is_ in the
special directory hash would therefore be a directory.

I realize it got pretty esoteric there in the middle. I'll see if I can
work up a patch that expresses what I'm thinking.

-Peff
--
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 3/3] t9903: add extra tests for bash.showDirtyState

2013-02-13 Thread Martin Erik Werner
On Wed, 2013-02-13 at 08:28 -0800, Junio C Hamano wrote:
> Martin Erik Werner  writes:
> 
> > Added 3 extra tests for the bash.showDirtyState config option, tests
> > should now cover all combinations of the shell var being set/unset and
> > the config option being enabled/disabled, given a dirty file.
> 
> Strictly speaking, you have 6 not 4 combinations (shell variable
> set/unset * config missing/set to false/set to true).  I think these
> additional tests cover should all 6 because "config missing" case
> should already have had tests before bash.showDirtyState was added.
> 

Indeed, I only mentioned 4 since the other ones existed already, and I
didn't change them, but maybe it should be mentioned as "combined with
previous tests (...) cover all 6 combinations (...)" then?

> > * Renamed test 'disabled by config' to 'shell variable set with config
> >   disabled' for consistency
> > ---
> 
> Sign-off?

Ah, just forgot the -s flag on that commit, yes it should be Signed-off
by me.

> 
> >  t/t9903-bash-prompt.sh |   38 +-
> >  1 file changed, 37 insertions(+), 1 deletion(-)
> >
> > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> > index cb008e2..fa81b09 100755
> > --- a/t/t9903-bash-prompt.sh
> > +++ b/t/t9903-bash-prompt.sh
> > @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator 
> > - before root commit' '
> > test_cmp expected "$actual"
> >  '
> >  
> > -test_expect_success 'prompt - dirty status indicator - disabled by config' 
> > '
> > +test_expect_success 'prompt - dirty status indicator - shell variable 
> > unset with config disabled' '
> > printf " (master)" > expected &&
> > echo "dirty" > file &&
> > test_when_finished "git reset --hard" &&
> > test_config bash.showDirtyState false &&
> > (
> > +   unset -v GIT_PS1_SHOWDIRTYSTATE &&
> > +   __git_ps1 > "$actual"
> > +   ) &&
> > +   test_cmp expected "$actual"
> > +'
> > +
> > +test_expect_success 'prompt - dirty status indicator - shell variable 
> > unset with config enabled' '
> > +   printf " (master)" > expected &&
> > +   echo "dirty" > file &&
> > +   test_when_finished "git reset --hard" &&
> > +   test_config bash.showDirtyState true &&
> > +   (
> > +   unset -v GIT_PS1_SHOWDIRTYSTATE &&
> > +   __git_ps1 > "$actual"
> > +   ) &&
> > +   test_cmp expected "$actual"
> > +'
> > +
> > +test_expect_success 'prompt - dirty status indicator - shell variable set 
> > with config disabled' '
> > +   printf " (master)" > expected &&
> > +   echo "dirty" > file &&
> > +   test_when_finished "git reset --hard" &&
> > +   test_config bash.showDirtyState false &&
> > +   (
> > +   GIT_PS1_SHOWDIRTYSTATE=y &&
> > +   __git_ps1 > "$actual"
> > +   ) &&
> > +   test_cmp expected "$actual"
> > +'
> > +
> > +test_expect_success 'prompt - dirty status indicator - shell variable set 
> > with config enabled' '
> > +   printf " (master *)" > expected &&
> > +   echo "dirty" > file &&
> > +   test_when_finished "git reset --hard" &&
> > +   test_config bash.showDirtyState true &&
> > +   (
> > GIT_PS1_SHOWDIRTYSTATE=y &&
> > __git_ps1 > "$actual"
> > ) &&

--
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 2/3] t9903: add tests for bash.showUntrackedFiles

2013-02-13 Thread Martin Erik Werner
On Wed, 2013-02-13 at 08:23 -0800, Junio C Hamano wrote:
> Martin Erik Werner  writes:
> 
> > Add 4 test for the bash.showUntrackedFiles config option, covering all
> > combinations of the shell var being set/unset and the config option
> > being enabled/disabled.
> >
> > Signed-off-by: Martin Erik Werner 
> > ---
> >  t/t9903-bash-prompt.sh |   40 
> >  1 file changed, 40 insertions(+)
> >
> > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> > index f17c1f8..cb008e2 100755
> > --- a/t/t9903-bash-prompt.sh
> > +++ b/t/t9903-bash-prompt.sh
> > @@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status 
> > indicator - untracked files
> > test_cmp expected "$actual"
> >  '
> >  
> > +test_expect_success 'prompt - untracked files status indicator - shell 
> > variable unset with config disabled' '
> > +   printf " (master)" > expected &&
> > +   test_config bash.showUntrackedFiles false &&
> > +   (
> > +   unset -v GIT_PS1_SHOWUNTRACKEDFILES &&
> 
> We do not use "unset -v" anywhere else in our system.  Shells
> mimicking SysV may choke on it.  A Portable POSIX script can omit
> "-v" when unsetting a variable.
> 
> Also "unset" can return false when the variable is not set to begin
> with with some shells.
> 
> Neither of these matters for this particular case because we know we
> are running this under bash in non-posix mode.  I however wonder if
> we can do something to prevent careless coders to copy and paste
> this piece when updating other tests that are not limited to bash.
> Commenting each and every use of "unset -v" does not sound like a
> good solution and perhaps I am being unnecessarily worried too much.
> 

Yeah, my (ba)sh foo is a bit limited, I was just basing on
http://wiki.bash-hackers.org/commands/builtin/unset#portability_considerations 
which seemed to recommend using -v.

So would it make sense to do:
GIT_PS1_SHOWUNTRACKEDFILES="dummy" &&
unset GIT_PS1_SHOWUNTRACKEDFILES &&
(...)
instead then?

-- 
Martin Erik Werner 

--
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] Makefile: don't run rm without any files

2013-02-13 Thread Matt Kraai
On Wed, Feb 13, 2013 at 08:51:45AM -0800, Junio C Hamano wrote:
> Matt Kraai  writes:
> 
> > From: Matt Kraai 
> >
> > "rm -f -r" fails on QNX when not passed any files to remove.
> 
> I do not think it is limited to QNX.
> 
> > the clean target, since dep_dirs is empty.
> 
> And dep_dirs being empty under some circumstance shouldn't be
> limited to QNX, either.
> 
> I think your change does no harm, may be a good change if dep_dirs
> goes empty, but the justification is lacking.  What caused your
> dep_dirs to become empty in the first place?
> 
> I am scratching my head because I see
> 
> OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
>   $(XDIFF_OBJS) \
>   $(VCSSVN_OBJS) \
>   git.o
> dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS

I don't set COMPUTE_HEADER_DEPENDENCIES, so it defaults to "auto".
The automatic detection determines that the compiler doesn't support
it, so it's then set to "no".  CHECK_HEADER_DEPENDENCIES isn't set
either, so about 20 lines below the dep_dirs assignment you quoted,
dep_dirs is cleared:

 ifneq ($(COMPUTE_HEADER_DEPENDENCIES),yes)
 ifndef CHECK_HEADER_DEPENDENCIES
 dep_dirs =
 ...

Should I submit an updated patch with a different commit message?
--
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 v4 3/4] count-objects: report garbage files in pack directory too

2013-02-13 Thread Junio C Hamano
Junio C Hamano  writes:

>> +if (has_extension(de->d_name, ".idx") ||
>> +has_extension(de->d_name, ".pack") ||
>> +has_extension(de->d_name, ".keep"))
>> +string_list_append(&garbage, path);
>
> It might be OK to put .pack and .keep in the same "if (A || B)" as
> it may happen to be that they do not need any special treatment
> right now, but I do not think this is a good idea in general.

Actually I take this part back.  I can see that the condition part
grow over time but I do not think the body should.  That is the
whole point of collecting paths that cannot be judged as garbage by
themselves into a list; we shouldn't be doing anything else by
definition in the body.

Everything else I said in the review still stands, though..

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] Makefile: don't run rm without any files

2013-02-13 Thread Junio C Hamano
Matt Kraai  writes:

> From: Matt Kraai 
>
> "rm -f -r" fails on QNX when not passed any files to remove.

I do not think it is limited to QNX.

> the clean target, since dep_dirs is empty.

And dep_dirs being empty under some circumstance shouldn't be
limited to QNX, either.

I think your change does no harm, may be a good change if dep_dirs
goes empty, but the justification is lacking.  What caused your
dep_dirs to become empty in the first place?

I am scratching my head because I see

OBJECTS := $(LIB_OBJS) $(BUILTIN_OBJS) $(PROGRAM_OBJS) $(TEST_OBJS) \
$(XDIFF_OBJS) \
$(VCSSVN_OBJS) \
git.o
dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS



> Avoid this by merging two rm
> command lines.
>
> Signed-off-by: Matt Kraai 
> ---
>  Makefile | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 5a2e02d..c2e3666 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2414,8 +2414,7 @@ clean: profile-clean
>   builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
>   $(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
>   $(RM) $(TEST_PROGRAMS)
> - $(RM) -r bin-wrappers
> - $(RM) -r $(dep_dirs)
> + $(RM) -r bin-wrappers $(dep_dirs)
>   $(RM) -r po/build/
>   $(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) 
> tags cscope*
>   $(RM) -r $(GIT_TARNAME) .doc-tmp-dir
--
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 3/3] t9903: add extra tests for bash.showDirtyState

2013-02-13 Thread Junio C Hamano
Martin Erik Werner  writes:

> Added 3 extra tests for the bash.showDirtyState config option, tests
> should now cover all combinations of the shell var being set/unset and
> the config option being enabled/disabled, given a dirty file.

Strictly speaking, you have 6 not 4 combinations (shell variable
set/unset * config missing/set to false/set to true).  I think these
additional tests cover should all 6 because "config missing" case
should already have had tests before bash.showDirtyState was added.

> * Renamed test 'disabled by config' to 'shell variable set with config
>   disabled' for consistency
> ---

Sign-off?

>  t/t9903-bash-prompt.sh |   38 +-
>  1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> index cb008e2..fa81b09 100755
> --- a/t/t9903-bash-prompt.sh
> +++ b/t/t9903-bash-prompt.sh
> @@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - 
> before root commit' '
>   test_cmp expected "$actual"
>  '
>  
> -test_expect_success 'prompt - dirty status indicator - disabled by config' '
> +test_expect_success 'prompt - dirty status indicator - shell variable unset 
> with config disabled' '
>   printf " (master)" > expected &&
>   echo "dirty" > file &&
>   test_when_finished "git reset --hard" &&
>   test_config bash.showDirtyState false &&
>   (
> + unset -v GIT_PS1_SHOWDIRTYSTATE &&
> + __git_ps1 > "$actual"
> + ) &&
> + test_cmp expected "$actual"
> +'
> +
> +test_expect_success 'prompt - dirty status indicator - shell variable unset 
> with config enabled' '
> + printf " (master)" > expected &&
> + echo "dirty" > file &&
> + test_when_finished "git reset --hard" &&
> + test_config bash.showDirtyState true &&
> + (
> + unset -v GIT_PS1_SHOWDIRTYSTATE &&
> + __git_ps1 > "$actual"
> + ) &&
> + test_cmp expected "$actual"
> +'
> +
> +test_expect_success 'prompt - dirty status indicator - shell variable set 
> with config disabled' '
> + printf " (master)" > expected &&
> + echo "dirty" > file &&
> + test_when_finished "git reset --hard" &&
> + test_config bash.showDirtyState false &&
> + (
> + GIT_PS1_SHOWDIRTYSTATE=y &&
> + __git_ps1 > "$actual"
> + ) &&
> + test_cmp expected "$actual"
> +'
> +
> +test_expect_success 'prompt - dirty status indicator - shell variable set 
> with config enabled' '
> + printf " (master *)" > expected &&
> + echo "dirty" > file &&
> + test_when_finished "git reset --hard" &&
> + test_config bash.showDirtyState true &&
> + (
>   GIT_PS1_SHOWDIRTYSTATE=y &&
>   __git_ps1 > "$actual"
>   ) &&
--
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 2/3] t9903: add tests for bash.showUntrackedFiles

2013-02-13 Thread Junio C Hamano
Martin Erik Werner  writes:

> Add 4 test for the bash.showUntrackedFiles config option, covering all
> combinations of the shell var being set/unset and the config option
> being enabled/disabled.
>
> Signed-off-by: Martin Erik Werner 
> ---
>  t/t9903-bash-prompt.sh |   40 
>  1 file changed, 40 insertions(+)
>
> diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> index f17c1f8..cb008e2 100755
> --- a/t/t9903-bash-prompt.sh
> +++ b/t/t9903-bash-prompt.sh
> @@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status 
> indicator - untracked files
>   test_cmp expected "$actual"
>  '
>  
> +test_expect_success 'prompt - untracked files status indicator - shell 
> variable unset with config disabled' '
> + printf " (master)" > expected &&
> + test_config bash.showUntrackedFiles false &&
> + (
> + unset -v GIT_PS1_SHOWUNTRACKEDFILES &&

We do not use "unset -v" anywhere else in our system.  Shells
mimicking SysV may choke on it.  A Portable POSIX script can omit
"-v" when unsetting a variable.

Also "unset" can return false when the variable is not set to begin
with with some shells.

Neither of these matters for this particular case because we know we
are running this under bash in non-posix mode.  I however wonder if
we can do something to prevent careless coders to copy and paste
this piece when updating other tests that are not limited to bash.
Commenting each and every use of "unset -v" does not sound like a
good solution and perhaps I am being unnecessarily worried too much.

--
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 v2] git-multimail: a replacement for post-receive-email

2013-02-13 Thread Matthieu Moy
Andy Parkins  writes:

> On Wednesday 13 February 2013 14:56:25 Matthieu Moy wrote:
>> Michael Haggerty  writes:
>
>> I think adding a short "dependencies" section in the README (or in an
>> INSTALL file) saying which Python version works could save new users the
>> trouble (I see the sheebang inside the scripts says python2 but since I
>> couldn't use my system's python and called
>> "path/to/python git_multimail.py", this didn't help). Making the script
>> portable with python 2 and 3 would be awesome ;-).
>
> For my 2p worth, I don't like seeing hooks called like this.  Particular 
> those 
> that come as part of the standard installation.

What do you mean by "like this" ?

> I call mine by installing little scripts like this (on Debian):
>
>   #!/bin/sh
>   # stored as $GIT_WORK_DIR/.git/hooks/post-receive-email
>   exec /bin/sh /usr/share/git-core/contrib/hooks/post-receive-email

Yes, this is what I was doing (with path/to/python instead of /bin/sh,
and git_multimail.py, or more precisely path/to/git_multimail.py,
instead of post-receive-email).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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 1/3] shell prompt: add bash.showUntrackedFiles option

2013-02-13 Thread Junio C Hamano
Martin Erik Werner  writes:

> Add a config option 'bash.showUntrackedFiles' which allows enabling
> the prompt showing untracked files on a per-repository basis. This is
> useful for some repositories where the 'git ls-files ...' command may
> take a long time.
>
> Signed-off-by: Martin Erik Werner 
> ---
>  contrib/completion/git-prompt.sh |   11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/completion/git-prompt.sh 
> b/contrib/completion/git-prompt.sh
> index 9bef053..9b2eec2 100644
> --- a/contrib/completion/git-prompt.sh
> +++ b/contrib/completion/git-prompt.sh
> @@ -43,7 +43,10 @@
>  #
>  # If you would like to see if there're untracked files, then you can set
>  # GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If there're untracked
> -# files, then a '%' will be shown next to the branch name.
> +# files, then a '%' will be shown next to the branch name.  You can
> +# configure this per-repository with the bash.showUntrackedFiles
> +# variable, which defaults to true once GIT_PS1_SHOWUNTRACKEDFILES is
> +# enabled.
>  #
>  # If you would like to see the difference between HEAD and its upstream,
>  # set GIT_PS1_SHOWUPSTREAM="auto".  A "<" indicates you are behind, ">"
> @@ -332,8 +335,10 @@ __git_ps1 ()
>   fi
>  
>   if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
> - if [ -n "$(git ls-files --others 
> --exclude-standard)" ]; then
> - u="%"
> + if [ "$(git config --bool 
> bash.showUntrackedFiles)" != "false" ]; then
> + if [ -n "$(git ls-files --others 
> --exclude-standard)" ]; then
> + u="%"
> + fi
>   fi
>   fi

Somebody should simplify this deeply nested "if/then/if/then/fi/fi"
sequence to a single if/then/fi statement, i.e. something like:

if test -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" &&
   test "$(git config --bool bash.showUntrackedFiles)" != false
then
u='%'
fi

And do the same for the other one this patch copies the above from.

No need to re-roll this patch, though.  It is a separate clean-up to
be done on top, once this series is settles.

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


[PATCH] Makefile: don't run rm without any files

2013-02-13 Thread Matt Kraai
From: Matt Kraai 

"rm -f -r" fails on QNX when not passed any files to remove.  This breaks
the clean target, since dep_dirs is empty.  Avoid this by merging two rm
command lines.

Signed-off-by: Matt Kraai 
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 5a2e02d..c2e3666 100644
--- a/Makefile
+++ b/Makefile
@@ -2414,8 +2414,7 @@ clean: profile-clean
builtin/*.o $(LIB_FILE) $(XDIFF_LIB) $(VCSSVN_LIB)
$(RM) $(ALL_PROGRAMS) $(SCRIPT_LIB) $(BUILT_INS) git$X
$(RM) $(TEST_PROGRAMS)
-   $(RM) -r bin-wrappers
-   $(RM) -r $(dep_dirs)
+   $(RM) -r bin-wrappers $(dep_dirs)
$(RM) -r po/build/
$(RM) *.spec *.pyc *.pyo */*.pyc */*.pyo common-cmds.h $(ETAGS_TARGET) 
tags cscope*
$(RM) -r $(GIT_TARNAME) .doc-tmp-dir
-- 
1.8.1.3.570.g3074c9d

--
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 v4 3/4] count-objects: report garbage files in pack directory too

2013-02-13 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy   writes:

> prepare_packed_git_one() is modified to allow count-objects to hook a
> report function to so we don't need to duplicate the pack searching
> logic in count-objects.c. When report_pack_garbage is NULL, the
> overhead is insignificant.
>
> The garbage is reported with warning() instead of error() in packed
> garbage case because it's not an error to have garbage. Loose garbage
> is still reported as errors and will be converted to warnings later.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---

Thanks.

Tests look good and the series is getting much closer.

> diff --git a/sha1_file.c b/sha1_file.c
> index 239bee7..5bedf78 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -21,6 +21,7 @@
>  #include "sha1-lookup.h"
>  #include "bulk-checkin.h"
>  #include "streaming.h"
> +#include "dir.h"
>  
>  #ifndef O_NOATIME
>  #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
> @@ -1000,6 +1001,57 @@ void install_packed_git(struct packed_git *pack)
>   packed_git = pack;
>  }
>  
> +void (*report_garbage)(const char *desc, const char *path);
> +
> +static void report_helper(const struct string_list *list,
> +   int seen_bits, int first, int last)
> +{
> + const char *msg;
> + switch (seen_bits) {
> + case 0: msg = "no corresponding .idx nor .pack"; break;
> + case 1: msg = "no corresponding .idx"; break;
> + case 2: msg = "no corresponding .pack"; break;

That's dense.

> + default:
> + return;
> + }
> + for (; first <= last; first++)

This looks odd.  If you use the usual last+1 convention between the
caller and callee, you do not have to do this, or call this function
with "i - 1" and "list->nr -1" as the last parameter.

> +static void report_pack_garbage(struct string_list *list)
> +{
> + int i, baselen = -1, first = 0, seen_bits = 0;
> +
> + if (!report_garbage)
> + return;
> +
> + sort_string_list(list);
> +
> + for (i = 0; i < list->nr; i++) {
> + const char *path = list->items[i].string;
> + if (baselen != -1 &&
> + strncmp(path, list->items[first].string, baselen)) {
> + report_helper(list, seen_bits, first, i - 1);
> + baselen = -1;
> + seen_bits = 0;
> + }
> + if (baselen == -1) {
> + const char *dot = strrchr(path, '.');
> + if (!dot) {
> + report_garbage("garbage found", path);
> + continue;
> + }
> + baselen = dot - path + 1;
> + first = i;
> + }
> + if (!strcmp(path + baselen, "pack"))
> + seen_bits |= 1;
> + else if (!strcmp(path + baselen, "idx"))
> + seen_bits |= 2;
> + }
> + report_helper(list, seen_bits, first, list->nr - 1);
> +}

> @@ -1009,6 +1061,7 @@ static void prepare_packed_git_one(char *objdir, int 
> local)
>   int len;
>   DIR *dir;
>   struct dirent *de;
> + struct string_list garbage = STRING_LIST_INIT_DUP;
>  
>   sprintf(path, "%s/pack", objdir);
>   len = strlen(path);
> ...
> @@ -1043,8 +1106,20 @@ static void prepare_packed_git_one(char *objdir, int 
> local)
>   (p = add_packed_git(path, len + namelen, local)) != 
> NULL)
>   install_packed_git(p);
>   }
> +
> + if (!report_garbage)
> + continue;
> +
> + if (has_extension(de->d_name, ".idx") ||
> + has_extension(de->d_name, ".pack") ||
> + has_extension(de->d_name, ".keep"))
> + string_list_append(&garbage, path);

It might be OK to put .pack and .keep in the same "if (A || B)" as
it may happen to be that they do not need any special treatment
right now, but I do not think this is a good idea in general.

You would want to do things differently for ".idx", e.g.

diff --git a/sha1_file.c b/sha1_file.c
index 5bedf78..450521f 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1076,6 +1076,7 @@ static void prepare_packed_git_one(char *objdir, int 
local)
while ((de = readdir(dir)) != NULL) {
int namelen = strlen(de->d_name);
struct packed_git *p;
+   int is_a_bad_idx = 0;
 
if (len + namelen + 1 > sizeof(path)) {
if (report_garbage) {
@@ -1105,12 +1106,14 @@ static void prepare_packed_git_one(char *objdir, int 
local)
 */
(p = add_packed_git(path, len + namelen, local)) != 
NULL)
install_packed_git(p);
+   else
+   is_a_bad_idx = 1;
}
 
if (!report_garbage)
continue;
 
-

Re: Git-aware HTTP transport docs

2013-02-13 Thread Junio C Hamano
Scott Chacon  writes:

> I don't believe it was ever merged into the Git docs.  I have a copy of it 
> here:
>
> https://www.dropbox.com/s/pwawp8kmwgyc3w2/http-protocol.txt

Thanks for a pointer.  It seems that it wasn't in a shape ready to
be "merged" yet.

Does somebody want to pick it up and polish it further?
--
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: What's cooking in git.git (Feb 2013, #05; Tue, 12)

2013-02-13 Thread Junio C Hamano
Andrew Ardill  writes:

> On 13 February 2013 11:34, Junio C Hamano  wrote:
>> The change could negatively affect people who expect that removing
>> files that are not used for their purpose (e.g. a large file that is
>> unnecessary for their build) will _not_ affect what they get from
>> "git add .";
>
> How big a problem is this?

As you said below, it could be fairly big, if you expect a lot of
people do not use "git add -u".

> If we need to support this behaviour than I would suppose a config
> option is required. A default config transition path similar to git
> push defaults would probably work well, in the case where breaking
> these expectations is unacceptable.

We've discussed that before.

http://thread.gmane.org/gmane.comp.version-control.git/171811/focus=171818

>> obviously they must have trained themselves not to do
>> "git add -u" or "git commit -a".
>
> Many people use git add -p by default, so I would not be surprised
> about people not using -u or -a.
--
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 v2] git-multimail: a replacement for post-receive-email

2013-02-13 Thread Andy Parkins
On Wednesday 13 February 2013 14:56:25 Matthieu Moy wrote:
> Michael Haggerty  writes:

> I think adding a short "dependencies" section in the README (or in an
> INSTALL file) saying which Python version works could save new users the
> trouble (I see the sheebang inside the scripts says python2 but since I
> couldn't use my system's python and called
> "path/to/python git_multimail.py", this didn't help). Making the script
> portable with python 2 and 3 would be awesome ;-).

For my 2p worth, I don't like seeing hooks called like this.  Particular those 
that come as part of the standard installation.

I call mine by installing little scripts like this (on Debian):

  #!/bin/sh
  # stored as $GIT_WORK_DIR/.git/hooks/post-receive-email
  exec /bin/sh /usr/share/git-core/contrib/hooks/post-receive-email

This means I don't have to make the sample script executable, it gets upgraded 
automatically as git gets upgraded, and the interpreter is easily changed by 
changing a file in my work directory, rather than altering a packaged file.

I'd prefer to see the /usr/share/git-core/templates/hooks/ using a similar 
technique, as to my mind, installing a full copy of the sample script in every 
new repository is wasteful and leaves you with potentially out-of-date scripts 
when you update git.


Andy

-- 
Dr Andy Parkins
andypark...@gmail.com
--
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 v2] git-multimail: a replacement for post-receive-email

2013-02-13 Thread Matthieu Moy
Michael Haggerty  writes:

> A while ago, I submitted an RFC for adding a new email notification
> script to "contrib" [1].  The reaction seemed favorable and it was
> suggested that the new script should replace post-receive-email rather
> than be added separately, ideally with some kind of migration support.

I think replacing the old post-receive-email is a sane goal in the long
run, but a good first step would be to have git-multimail merged in
contrib, and start considering the old script as deprecated (keeping the
old script doesn't harm IMHO, it's a one-file, 3 commits/year script,
not really a maintainance burden).

I started playing with git-multimail. In short, I do like it but had to
fight a bit with python to get it to work, and couldn't get it to do
exactly what I expect. Pull request attached :-).


Installation troubles:

I had an old python installation (Red Hat package, and I'm not root),
that did not include the email.utils package, so I couldn't use my
system's python. I found no indication about python version in README,
so I installed the latest python by hand, just to find out that
git-multimail wasn't compatible with Python 3.x. 2to3 can fix
automatically a number of 3.x compatibility issues, but not all of them
so I gave up and installed Python 2.7.

I think adding a short "dependencies" section in the README (or in an
INSTALL file) saying which Python version works could save new users the
trouble (I see the sheebang inside the scripts says python2 but since I
couldn't use my system's python and called
"path/to/python git_multimail.py", this didn't help). Making the script
portable with python 2 and 3 would be awesome ;-).


Missing feature:

git-multimail can send a summary for each push, with the "git log --oneline"
of the new revisions, and then 1 mail per patch with the complete log
and the patch.

I'd like to have the intermediate: allow the summary email to include
the complete log (not just --oneline). My colleagues already think they
receive too many emails so I don't think they'd like the "one email per
commit" way, but the 1 line summary is really short OTOH.

I wrote a quick and hopefully not-too-dirty implementation of it,
there's a pull request here:

https://github.com/mhagger/git-multimail/pull/6

essentially, it boils down to:

@@ -835,6 +837,17 @@ class ReferenceChange(Change):
 for line in self.expand_lines(NO_NEW_REVISIONS_TEMPLATE):
 yield line
 
+if adds and self.showlog:
+yield '\n'
+yield 'Detailed log of added commits:\n\n'
+for line in read_lines(
+['git', 'log']
++ self.logopts
++ ['%s..%s' % (self.old.commit, self.new.commit,)],
+keepends=True,
+):
+yield line
+
 # The diffstat is shown from the old revision to the new
 # revision.  This is to show the truth of what happened in
 # this change.  There's no point showing the stat from the

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
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


[ANN] First beta: Git export with hardlinks

2013-02-13 Thread Thomas Koch
Hi,

my git_export_hardlink command should now be in a usable state. I'd appreciate 
any feedback: https://github.com/thkoch2001/git_export_hardlinks

I still have to choose a license: BSD/GPL/?

Jeff King:

> It looks like you create the sha1->path mapping by asking the user to
> provide , pairs, and then assuming that the exported
> tree at  exactly matches . Which it would in the
> workflow you've proposed, but it is also easy for that not to be the
> case (e.g., somebody munges a file in  after it has been
> exported).
> 
> So it's a bit dangerous as a general purpose tool, IMHO. It's also a
> slight pain in that you have to keep track of the tree sha1 for each
> exported path somehow.
You're right. I'd run a git reset --hard after each export to guarantee a 
pristine export.

The tree sha1 of the exported tree might be part of the folder name of the 
export or in some meta file related to the export, like

/deployments
  /2012-03-05_14-23-02_0b96bf5f72d2c282b31726b3fbff279a89220b15
/export <- exported tree goes here
/meta  <- git config file holding all relevant metadata: (who, when, tree,
  commit, ref)
/index <- git index file corresponding to the exported tree (maybe?)
  
Regards,

Thomas Koch, http://www.koch.ro
--
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: git svn init throws Not a git repository (or any of the parent directories): .git

2013-02-13 Thread Konstantin Khomoutov
On Wed, 13 Feb 2013 14:01:36 +0100
"amccl...@gmail.com"  wrote:

> I have problem with git svn init:
> When I execute
> git svn init svn+ssh://usern...@example.com/path/repo
> I see:
> fatal: Not a git repository (or any of the parent directories): .git
> Already at toplevel, but .git not found
>  at /usr/lib/git-core/git-svn line 342

Are you doing `git init` first before running `git svn init ...`?
--
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: A good Git technique for referring back to original files

2013-02-13 Thread MikeW
MikeW  yahoo.co.uk> writes:

> 
> Paul Campbell  kemitix.net> writes:
> 
> > 
> > Hi Mike,
> > 
> > I think git-cvsimport and git-subtree could help you here.
> > 
> 
> That looks very interesting, had not considered git subtree and it looks like
> the right kind of method.
> 
> Thanks.
> Mike

The only alternative I can think of is to scan through Working_SDK and replace
the files there with symlinks back to the matching files within the
original subprojects - such scripts exist !

Then any changes in Working_SDK will update the (baselined) originals in-place.

But then no explicit use of git except for tracking work prior to pushing
back to CVS.

Oh well, thanks for ideas, will see which work best.

Mike

--
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: [git-multimail] License unknown (#1)

2013-02-13 Thread Michael Haggerty
On 02/12/2013 04:28 PM, Andy Parkins wrote:
> On Sunday 27 January 2013 18:52:58 Michael Haggerty wrote:
>> I have a question about the license of contrib/hooks/post-commit-email.
>> [...]
> 
> Keeping up with the git mailing list got a bit much, [...]

Very understandable :-)

> My apologies to everyone; I've been lax supporting the script -- I never 
> really expected it to be as widely used as it has been, I was always 
> expecting 
> someone would come along and replace it so I'm pleased that Michael has done 
> so (although it's a little disheartening to read of it being called "hacky", 
> when I tried very hard to make it as clear and modular as I could).

FWIW I think the script was quite well-structured (within the
limitations of shell scripting anyway) and I had no problem
understanding it and translating it to Python.

>> If somebody can explain what license the code is under and how they come
>> to that conclusion, I would be very grateful.
>>
>> And if Andy Parkins (the original author) is listening, please indicate
>> whether you had any intent *other* than GPLv2.
> 
> I intended it to be under the same license as Git.  I had read in one of the 
> patch submission files (which I can't seem to find now) that all submissions 
> were considered part of git.

Thanks.  Given that you submitted the original version under GPLv2, and
later contributors certainly had no reason to assume any *other*
license, I think any reasonable person will be satisfied that the whole
script in its current form is under GPLv2.

> If an explicit declaration is needed, I am happy to give it.  Let me know 
> what 
> form this should take and I'll supply it.

Personally, I am satisfied by your email statement.  I will leave the
derived work, git-multimail, also under GPLv2.  Assuming that
git-multimail supersedes yours, its explicit choice of license will make
the situation clear for future users.

Thanks!
Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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: A good Git technique for referring back to original files

2013-02-13 Thread MikeW
Paul Campbell  kemitix.net> writes:

> 
> Hi Mike,
> 
> I think git-cvsimport and git-subtree could help you here.
> 

That looks very interesting, had not considered git subtree and it looks like
the right kind of method.

Thanks.
Mike

> Hope that helps.
> 
> --
> Paul
> 
... Super-Snip ...
> 
> --
> Paul [W] Campbell
> 

--
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 v2 3/3] t9903: add extra tests for bash.showDirtyState

2013-02-13 Thread Martin Erik Werner
Added 3 extra tests for the bash.showDirtyState config option, tests
should now cover all combinations of the shell var being set/unset and
the config option being enabled/disabled, given a dirty file.

* Renamed test 'disabled by config' to 'shell variable set with config
  disabled' for consistency
---
 t/t9903-bash-prompt.sh |   38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index cb008e2..fa81b09 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -360,12 +360,48 @@ test_expect_success 'prompt - dirty status indicator - 
before root commit' '
test_cmp expected "$actual"
 '
 
-test_expect_success 'prompt - dirty status indicator - disabled by config' '
+test_expect_success 'prompt - dirty status indicator - shell variable unset 
with config disabled' '
printf " (master)" > expected &&
echo "dirty" > file &&
test_when_finished "git reset --hard" &&
test_config bash.showDirtyState false &&
(
+   unset -v GIT_PS1_SHOWDIRTYSTATE &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - shell variable unset 
with config enabled' '
+   printf " (master)" > expected &&
+   echo "dirty" > file &&
+   test_when_finished "git reset --hard" &&
+   test_config bash.showDirtyState true &&
+   (
+   unset -v GIT_PS1_SHOWDIRTYSTATE &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - shell variable set with 
config disabled' '
+   printf " (master)" > expected &&
+   echo "dirty" > file &&
+   test_when_finished "git reset --hard" &&
+   test_config bash.showDirtyState false &&
+   (
+   GIT_PS1_SHOWDIRTYSTATE=y &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - dirty status indicator - shell variable set with 
config enabled' '
+   printf " (master *)" > expected &&
+   echo "dirty" > file &&
+   test_when_finished "git reset --hard" &&
+   test_config bash.showDirtyState true &&
+   (
GIT_PS1_SHOWDIRTYSTATE=y &&
__git_ps1 > "$actual"
) &&
-- 
1.7.10.4

--
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 v2 2/3] t9903: add tests for bash.showUntrackedFiles

2013-02-13 Thread Martin Erik Werner
Add 4 test for the bash.showUntrackedFiles config option, covering all
combinations of the shell var being set/unset and the config option
being enabled/disabled.

Signed-off-by: Martin Erik Werner 
---
 t/t9903-bash-prompt.sh |   40 
 1 file changed, 40 insertions(+)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index f17c1f8..cb008e2 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -437,6 +437,46 @@ test_expect_success 'prompt - untracked files status 
indicator - untracked files
test_cmp expected "$actual"
 '
 
+test_expect_success 'prompt - untracked files status indicator - shell 
variable unset with config disabled' '
+   printf " (master)" > expected &&
+   test_config bash.showUntrackedFiles false &&
+   (
+   unset -v GIT_PS1_SHOWUNTRACKEDFILES &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - shell 
variable unset with config enabled' '
+   printf " (master)" > expected &&
+   test_config bash.showUntrackedFiles true &&
+   (
+   unset -v GIT_PS1_SHOWUNTRACKEDFILES &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - shell 
variable set with config disabled' '
+   printf " (master)" > expected &&
+   test_config bash.showUntrackedFiles false &&
+   (
+   GIT_PS1_SHOWUNTRACKEDFILES=y &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
+test_expect_success 'prompt - untracked files status indicator - shell 
variable set with config enabled' '
+   printf " (master %%)" > expected &&
+   test_config bash.showUntrackedFiles true &&
+   (
+   GIT_PS1_SHOWUNTRACKEDFILES=y &&
+   __git_ps1 > "$actual"
+   ) &&
+   test_cmp expected "$actual"
+'
+
 test_expect_success 'prompt - untracked files status indicator - not shown 
inside .git directory' '
printf " (GIT_DIR!)" > expected &&
(
-- 
1.7.10.4

--
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 v2 1/3] shell prompt: add bash.showUntrackedFiles option

2013-02-13 Thread Martin Erik Werner
Add a config option 'bash.showUntrackedFiles' which allows enabling
the prompt showing untracked files on a per-repository basis. This is
useful for some repositories where the 'git ls-files ...' command may
take a long time.

Signed-off-by: Martin Erik Werner 
---
 contrib/completion/git-prompt.sh |   11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh
index 9bef053..9b2eec2 100644
--- a/contrib/completion/git-prompt.sh
+++ b/contrib/completion/git-prompt.sh
@@ -43,7 +43,10 @@
 #
 # If you would like to see if there're untracked files, then you can set
 # GIT_PS1_SHOWUNTRACKEDFILES to a nonempty value. If there're untracked
-# files, then a '%' will be shown next to the branch name.
+# files, then a '%' will be shown next to the branch name.  You can
+# configure this per-repository with the bash.showUntrackedFiles
+# variable, which defaults to true once GIT_PS1_SHOWUNTRACKEDFILES is
+# enabled.
 #
 # If you would like to see the difference between HEAD and its upstream,
 # set GIT_PS1_SHOWUPSTREAM="auto".  A "<" indicates you are behind, ">"
@@ -332,8 +335,10 @@ __git_ps1 ()
fi
 
if [ -n "${GIT_PS1_SHOWUNTRACKEDFILES-}" ]; then
-   if [ -n "$(git ls-files --others 
--exclude-standard)" ]; then
-   u="%"
+   if [ "$(git config --bool 
bash.showUntrackedFiles)" != "false" ]; then
+   if [ -n "$(git ls-files --others 
--exclude-standard)" ]; then
+   u="%"
+   fi
fi
fi
 
-- 
1.7.10.4

--
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 v2 0/3] Add bash.showUntrackedFiles config option

2013-02-13 Thread Martin Erik Werner
On Tue, 2013-02-12 at 14:29 -0800, Junio C Hamano wrote:
> Martin Erik Werner  writes:
> 
> > Add a test case for the bash.showUntrackedFiles config option, which
> > checks that the config option can disable the global effect of the
> > GIT_PS1_SHOWUNTRACKEDFILES environmant variable.
> >
> > Signed-off-by: Martin Erik Werner 
> > ---
> >  t/t9903-bash-prompt.sh |   11 +++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
> > index f17c1f8..c9417b9 100755
> > --- a/t/t9903-bash-prompt.sh
> > +++ b/t/t9903-bash-prompt.sh
> > @@ -447,6 +447,17 @@ test_expect_success 'prompt - untracked files status 
> > indicator - not shown insid
> > test_cmp expected "$actual"
> >  '
> >  
> > +test_expect_success 'prompt - untracked files status indicator - disabled 
> > by config' '
> > +   printf " (master)" > expected &&
> > +   echo "untracked" > file_untracked &&
> > +   test_config bash.showUntrackedFiles false &&
> > +   (
> > +   GIT_PS1_SHOWUNTRACKEDFILES=y &&
> > +   __git_ps1 > "$actual"
> > +   ) &&
> > +   test_cmp expected "$actual"
> > +'
> 
> All six combinations need checking:
> 
>  * not having the configuration at all and not having the shell
>variable should not show the untracked indicator (already tested).
> 
>  * not having the configuration at all and having the shell variable
>should show the untracked indicator (already tested).
> 
>  * setting configuration to true without having the shell variable
>should not show the untracked indicator.
> 
>  * setting configuration to true and having the shell variable
>should show the unttracked indicator.
> 
>  * setting configuration to false and having the shell variable
>should not show the untracked indicator (the above test checks
>this).
> 
>  * setting configuration to false without having the shell variable
>should not show the untracked indicator.
> 
> to prevent others from breaking the code you wrote for [PATCH 1/2],
> so you need three more tests, I guess?

Ah, yes, I was mimicing what the test did for bash.showDirtyState, I've
now added the three extra tests for bash.showUntrackedFiles, which
should cover all of the above cases, hopefully?

I've also added in the three extra tests for bash.showDirtyState,
equivalently. These only cover the case of dirty files and not
combinations with content in index, which I felt was a bit overkill, is
that reasonable?

Thanks for the review :)

-- 
Martin Erik Werner 

--
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: inotify to minimize stat() calls

2013-02-13 Thread Duy Nguyen
On Tue, Feb 12, 2013 at 09:48:18PM +0100, Karsten Blees wrote:

> However, the difference between git status -uall and -uno was always
> about 1.3 s in all fscache versions, even though
> opendir/readdir/closedir was served entirely from the cache. I added
> a bit of performance tracing to find the cause, and I think most of
> the time spent in wt_status_collect_untracked can be eliminated:
> 
> 1.) 0.939 s is spent in dir.c/excluded (i.e. checking
> .gitignore). This check is done for *every* file in the working
> copy, including files in the index. Checking the index first could
> eliminate most of that, i.e.:
> 
> (Note: patches are for discussion only, I'm aware that they may have
> unintended side effects...)
>
> @@ -1097,6 +1097,8 @@ static enum path_treatment treat_path(struct dir_struct 
> *dir,
> return path_ignored;
> strbuf_setlen(path, baselen);
> strbuf_addstr(path, de->d_name);
> +   if (cache_name_exists(path->buf, path->len, ignore_case))
> +   return path_ignored;
> if (simplify_away(path->buf, path->len, simplify))
> return path_ignored;

The below patch passes the test suite for me and still does the same
thing. On my Linux box, running "git status" on gentoo-x86.git with
this patch saves 0.05s (0.548s without the patch, 0.505s with the
patch, best number of 20 runs).

And I just realized gentoo-x86.git does not have any .gitignore. On
webkit.git, it cuts "git status" time from 1.121s down to
0.762s. Unless I'm mistaken, "git add" should have the same benefit on
normal case too. Good finding!

-- 8< --
diff --git a/dir.c b/dir.c
index 57394e4..4b4cf60 100644
--- a/dir.c
+++ b/dir.c
@@ -1244,7 +1244,19 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
  const struct path_simplify *simplify,
  int dtype, struct dirent *de)
 {
-   int exclude = is_excluded(dir, path->buf, &dtype);
+   int exclude;
+
+   if (dtype == DT_UNKNOWN)
+   dtype = get_dtype(de, path->buf, path->len);
+
+   if (!(dir->flags & DIR_SHOW_IGNORED) &&
+   !(dir->flags & DIR_COLLECT_IGNORED) &&
+   dtype == DT_REG &&
+   cache_name_exists(path->buf, path->len, ignore_case))
+   return path_ignored;
+
+   exclude = is_excluded(dir, path->buf, &dtype);
+
if (exclude && (dir->flags & DIR_COLLECT_IGNORED)
&& exclude_matches_pathspec(path->buf, path->len, simplify))
dir_add_ignored(dir, path->buf, path->len);
@@ -1256,9 +1268,6 @@ static enum path_treatment treat_one_path(struct 
dir_struct *dir,
if (exclude && !(dir->flags & DIR_SHOW_IGNORED))
return path_ignored;
 
-   if (dtype == DT_UNKNOWN)
-   dtype = get_dtype(de, path->buf, path->len);
-
switch (dtype) {
default:
return path_ignored;
-- 8< --
--
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 v4 4/4] count-objects: report how much disk space taken by garbage files

2013-02-13 Thread Nguyễn Thái Ngọc Duy
Also issue warnings on loose garbages instead of errors as a result of
using report_garbage() function in count_objects()

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-count-objects.txt |  2 ++
 builtin/count-objects.c | 18 --
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-count-objects.txt 
b/Documentation/git-count-objects.txt
index 1611d7c..da6e72e 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -35,6 +35,8 @@ the packs. These objects could be pruned using `git 
prune-packed`.
 +
 garbage: the number of files in object database that are not valid
 loose objects nor valid packs
++
+size-garbage: disk space consumed by garbage files, in KiB
 
 GIT
 ---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 1706c8b..3a01a8d 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -10,9 +10,13 @@
 #include "parse-options.h"
 
 static unsigned long garbage;
+static off_t size_garbage;
 
 static void real_report_garbage(const char *desc, const char *path)
 {
+   struct stat st;
+   if (!stat(path, &st))
+   size_garbage += st.st_size;
warning("%s: %s", desc, path);
garbage++;
 }
@@ -20,8 +24,7 @@ static void real_report_garbage(const char *desc, const char 
*path)
 static void count_objects(DIR *d, char *path, int len, int verbose,
  unsigned long *loose,
  off_t *loose_size,
- unsigned long *packed_loose,
- unsigned long *garbage)
+ unsigned long *packed_loose)
 {
struct dirent *ent;
while ((ent = readdir(d)) != NULL) {
@@ -54,9 +57,11 @@ static void count_objects(DIR *d, char *path, int len, int 
verbose,
}
if (bad) {
if (verbose) {
-   error("garbage found: %.*s/%s",
- len + 2, path, ent->d_name);
-   (*garbage)++;
+   struct strbuf sb = STRBUF_INIT;
+   strbuf_addf(&sb, "%.*s/%s",
+   len + 2, path, ent->d_name);
+   report_garbage("garbage found", sb.buf);
+   strbuf_release(&sb);
}
continue;
}
@@ -107,7 +112,7 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
if (!d)
continue;
count_objects(d, path, len, verbose,
- &loose, &loose_size, &packed_loose, &garbage);
+ &loose, &loose_size, &packed_loose);
closedir(d);
}
if (verbose) {
@@ -132,6 +137,7 @@ int cmd_count_objects(int argc, const char **argv, const 
char *prefix)
printf("size-pack: %lu\n", (unsigned long) (size_pack / 1024));
printf("prune-packable: %lu\n", packed_loose);
printf("garbage: %lu\n", garbage);
+   printf("size-garbage: %lu\n", (unsigned long) (size_garbage / 
1024));
}
else
printf("%lu objects, %lu kilobytes\n",
-- 
1.8.1.2.536.gf441e6d

--
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 v4 2/4] sha1_file: reorder code in prepare_packed_git_one()

2013-02-13 Thread Nguyễn Thái Ngọc Duy
The current loop does

while (...) {
if (!not .idx file)
continue;
process .idx file;
}

and is reordered to

while (...) {
if (!.idx file) {
process .idx file;
}
}

This makes it easier to add new extension file processing.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 sha1_file.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 40b2329..239bee7 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1024,27 +1024,25 @@ static void prepare_packed_git_one(char *objdir, int 
local)
int namelen = strlen(de->d_name);
struct packed_git *p;
 
-   if (!has_extension(de->d_name, ".idx"))
-   continue;
-
if (len + namelen + 1 > sizeof(path))
continue;
 
-   /* Don't reopen a pack we already have. */
strcpy(path + len, de->d_name);
-   for (p = packed_git; p; p = p->next) {
-   if (!memcmp(path, p->pack_name, len + namelen - 4))
-   break;
+
+   if (has_extension(de->d_name, ".idx")) {
+   /* Don't reopen a pack we already have. */
+   for (p = packed_git; p; p = p->next) {
+   if (!memcmp(path, p->pack_name, len + namelen - 
4))
+   break;
+   }
+   if (p == NULL &&
+   /*
+* See if it really is a valid .idx file with
+* corresponding .pack file that we can map.
+*/
+   (p = add_packed_git(path, len + namelen, local)) != 
NULL)
+   install_packed_git(p);
}
-   if (p)
-   continue;
-   /* See if it really is a valid .idx file with corresponding
-* .pack file that we can map.
-*/
-   p = add_packed_git(path, len + namelen, local);
-   if (!p)
-   continue;
-   install_packed_git(p);
}
closedir(dir);
 }
-- 
1.8.1.2.536.gf441e6d

--
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 v4 3/4] count-objects: report garbage files in pack directory too

2013-02-13 Thread Nguyễn Thái Ngọc Duy
prepare_packed_git_one() is modified to allow count-objects to hook a
report function to so we don't need to duplicate the pack searching
logic in count-objects.c. When report_pack_garbage is NULL, the
overhead is insignificant.

The garbage is reported with warning() instead of error() in packed
garbage case because it's not an error to have garbage. Loose garbage
is still reported as errors and will be converted to warnings later.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-count-objects.txt |  4 +-
 builtin/count-objects.c | 12 +-
 cache.h |  3 ++
 sha1_file.c | 77 -
 t/t5304-prune.sh| 26 +
 5 files changed, 118 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-count-objects.txt 
b/Documentation/git-count-objects.txt
index e816823..1611d7c 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -33,8 +33,8 @@ size-pack: disk space consumed by the packs, in KiB
 prune-packable: the number of loose objects that are also present in
 the packs. These objects could be pruned using `git prune-packed`.
 +
-garbage: the number of files in loose object database that are not
-valid loose objects
+garbage: the number of files in object database that are not valid
+loose objects nor valid packs
 
 GIT
 ---
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index 9afaa88..1706c8b 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -9,6 +9,14 @@
 #include "builtin.h"
 #include "parse-options.h"
 
+static unsigned long garbage;
+
+static void real_report_garbage(const char *desc, const char *path)
+{
+   warning("%s: %s", desc, path);
+   garbage++;
+}
+
 static void count_objects(DIR *d, char *path, int len, int verbose,
  unsigned long *loose,
  off_t *loose_size,
@@ -76,7 +84,7 @@ int cmd_count_objects(int argc, const char **argv, const char 
*prefix)
const char *objdir = get_object_directory();
int len = strlen(objdir);
char *path = xmalloc(len + 50);
-   unsigned long loose = 0, packed = 0, packed_loose = 0, garbage = 0;
+   unsigned long loose = 0, packed = 0, packed_loose = 0;
off_t loose_size = 0;
struct option opts[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
@@ -87,6 +95,8 @@ int cmd_count_objects(int argc, const char **argv, const char 
*prefix)
/* we do not take arguments other than flags for now */
if (argc)
usage_with_options(count_objects_usage, opts);
+   if (verbose)
+   report_garbage = real_report_garbage;
memcpy(path, objdir, len);
if (len && objdir[len-1] != '/')
path[len++] = '/';
diff --git a/cache.h b/cache.h
index 7339f21..73de68c 100644
--- a/cache.h
+++ b/cache.h
@@ -1051,6 +1051,9 @@ extern const char *parse_feature_value(const char 
*feature_list, const char *fea
 
 extern struct packed_git *parse_pack_index(unsigned char *sha1, const char 
*idx_path);
 
+/* A hook for count-objects to report invalid files in pack directory */
+extern void (*report_garbage)(const char *desc, const char *path);
+
 extern void prepare_packed_git(void);
 extern void reprepare_packed_git(void);
 extern void install_packed_git(struct packed_git *pack);
diff --git a/sha1_file.c b/sha1_file.c
index 239bee7..5bedf78 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -21,6 +21,7 @@
 #include "sha1-lookup.h"
 #include "bulk-checkin.h"
 #include "streaming.h"
+#include "dir.h"
 
 #ifndef O_NOATIME
 #if defined(__linux__) && (defined(__i386__) || defined(__PPC__))
@@ -1000,6 +1001,57 @@ void install_packed_git(struct packed_git *pack)
packed_git = pack;
 }
 
+void (*report_garbage)(const char *desc, const char *path);
+
+static void report_helper(const struct string_list *list,
+ int seen_bits, int first, int last)
+{
+   const char *msg;
+   switch (seen_bits) {
+   case 0: msg = "no corresponding .idx nor .pack"; break;
+   case 1: msg = "no corresponding .idx"; break;
+   case 2: msg = "no corresponding .pack"; break;
+   default:
+   return;
+   }
+   for (; first <= last; first++)
+   report_garbage(msg, list->items[first].string);
+}
+
+static void report_pack_garbage(struct string_list *list)
+{
+   int i, baselen = -1, first = 0, seen_bits = 0;
+
+   if (!report_garbage)
+   return;
+
+   sort_string_list(list);
+
+   for (i = 0; i < list->nr; i++) {
+   const char *path = list->items[i].string;
+   if (baselen != -1 &&
+   strncmp(path, list->items[first].string, baselen)) {
+   report_helper(list, seen_bits, first, i - 1);
+   baselen = -1;
+   seen_bits = 0;
+ 

[PATCH v4 1/4] git-count-objects.txt: describe each line in -v output

2013-02-13 Thread Nguyễn Thái Ngọc Duy
The current description requires a bit of guessing (what clause
corresponds to what printed line?) and lacks information, such as
the unit of size and size-pack.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-count-objects.txt | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-count-objects.txt 
b/Documentation/git-count-objects.txt
index 23c80ce..e816823 100644
--- a/Documentation/git-count-objects.txt
+++ b/Documentation/git-count-objects.txt
@@ -20,11 +20,21 @@ OPTIONS
 ---
 -v::
 --verbose::
-   In addition to the number of loose objects and disk
-   space consumed, it reports the number of in-pack
-   objects, number of packs, disk space consumed by those packs,
-   and number of objects that can be removed by running
-   `git prune-packed`.
+   Report in more detail:
++
+count: the number of loose objects
++
+size: disk space consumed by loose objects, in KiB
++
+in-pack: the number of in-pack objects
++
+size-pack: disk space consumed by the packs, in KiB
++
+prune-packable: the number of loose objects that are also present in
+the packs. These objects could be pruned using `git prune-packed`.
++
+garbage: the number of files in loose object database that are not
+valid loose objects
 
 GIT
 ---
-- 
1.8.1.2.536.gf441e6d

--
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 v4 0/4] count-objects improvements

2013-02-13 Thread Nguyễn Thái Ngọc Duy
On Wed, Feb 13, 2013 at 12:23 AM, Junio C Hamano  wrote:
> Nguyễn Thái Ngọc Duy  writes:
>
>> +/* A hook for count-objects to report invalid files in pack directory */
>> +extern void (*report_garbage)(const char *desc, const char *path, int len, 
>> const char *name);
>
> We may want to document the strange way the last three parameters
> are used somewhere.  e.g.
>
> shows "path" (if "name" is NULL), or prepends "path" in
> front of name (otherwise); only for the latter, "path" can
> be a string that is not NUL-terminated but its length
> specified with "len" and in that case a slash is inserted
> between the path and the "name".
>
> When described clearly, it sounds somewhat ugly and incoherent API,
> even though it covers the immediate need X-<.

One of the reasons why I did not export it explicitly. Changed it to

void (*report_garbage)(const char *desc, const char *path);

and pushed the ugly part back to callers.

> How about doing it something along this line, perhaps?
>
> int i;
> int beginning_of_this_name = -1;
> int seen_bits = 0; /* 01 for .idx, 02 for .pack */
> for (i = 0; i < list->nr; i++) {
> if (beginning_of_this_name < 0)
> beginning_of_this_name = i;
> else if (list->items[i] and 
> list->items[beginning_of_this_name]
>  share the same basename)
> ; /* keep scanning */
> else {
> /* one name ended at (i-1) */
> if (seen_bits == 3)
> ; /* both .idx and .pack exist; good */
> else
> report_garbage_for_one_name(list, 
> beginning_of_this_name, i,
> seen_bits);
> seen_bits = 0;
> beginning_of_this_name = i;
> }
> if (list->items[i] is ".idx")
> seen_bits |= 1;
> if (list->items[i] is ".pack")
> seen_bits |= 2;
>
> }
> if (0 <= beginning_of_this_name && seen_bits != 3)
> report_garbages_for_one_name(list, beginning_of_this_name, 
> list->nr, seen_bits);
>
> with a helper function report_garbage_for_one_name() that would look like 
> this:
>
> report_garbage_for_one_name(...) {
> int j;
> const char *msg;
> switch (seen_bits) {
> case 0: msg = "no corresponding .idx nor .pack"; break;
> case 1: msg = "no corresponding .pack"; break;
> case 2: msg = "no corresponding .idx; break;
> }
> for (j = beginning_of_this_name; j < i; j++)
> report_garbage(msg, list->items[j]);
> }
>
> For the above to work, prepare_packed_git_one() needs to retain only the
> paths with known extensions in garbage list. "pack-deadbeef.unk" can and
> should be reported as a garbage immediately when it is seen without being
> placed in the list.

Yup. Looks good.

>> + } else if (has_extension(de->d_name, ".idx")) {
>> + struct string_list_item *item;
>> + int n = strlen(path) - 4;
>> + item = string_list_append_nodup(&garbage,
>> + xstrndup(path, n));
>> + item->util = ".idx";
>> + continue;
>> + } else
>> + report_garbage("garbage found", path, 0, NULL);
>
> Hmm, where is a ".keep" file handled in this flow?

Apparently I smoked/drank while coding or something. .idx is supposed
to be .keep. This calls for a test to guard my code (part of this v4).

> The structure of the if/else cascade is much nicer than the earlier
> iterations, but wouldn't it be even more clear to do this?
>
> if (is .idx file) {
> ... do that .idx thing ...
> }
>
> if (!report_garbage)
> continue; /* it does not matter what the file is */
>
> if (is .pack) {
> ... remember that we saw this .pack ...
> } else if (is .idx) {
> ... remember that we saw this .idx ...
> } else if (is .keep) {
> ... remember that we saw this .keep ...
> } else {
> ... all else --- report as garbage immediately ...
> }

Done. 2/4 is updated to make sure the "if (is .idx file)" block does
not shortcut the loop with "continue;" so that we always get .idx
file in the end of the loop.

Nguyễn Thái Ngọc Duy (4):
  git-count-objects.txt: describe each line in -v output
  sha1_file: reorder code in prepare_packed_git_one()
  count-objects: report garbage files in pack directory too
  count-objects: report how much disk spa

Re: Pushing a git repository to a new server

2013-02-13 Thread Michael J Gruber
Jeff King venit, vidit, dixit 12.02.2013 21:42:
> On Tue, Feb 12, 2013 at 12:28:53PM +0100, Michael J Gruber wrote:
> 
>> I'm not sure providers like GitHub would fancy an interface which allows
>> the programmatic creation of repos (giving a new meaning to "fork
>> bomb"). But I bet you know better ;-)
> 
> You can already do that:
> 
>   http://developer.github.com/v3/repos/#create

Nice.

I knew you knew ;)

> We rate-limit API requests, and I imagine we might do something similar
> with create-over-git. But that is exactly the kind of implementation
> detail that can go into a custom create-repo script.
> 
>> An alternative would be to teach git (the client) about repo types and
>> how to create them. After all, a repo URL "ssh://host/path" gives a
>> clear indication that "ssh host git init path" will create a repo.
> 
> But that's the point of a microformat. It _doesn't_ always work, because
> the server may not allow arbitrary commands, or may have special
> requirements on top of the "init". You can make the microformat be "git
> init path", and servers can intercept calls to "git init" and translate
> them into custom magic. But I think the world is a little simpler if we
> define a new service type (alongside git-upload-pack, git-receive-pack,
> etc), and let clients request it. Then it's clear what the client is
> trying to do, it's easy for servers to hook into it, we can request it
> over http, etc. And it can be extended over time to take more fields
> (like repo description, etc).
> 
> I'm really not suggesting anything drastic. The wrapper case for ssh
> would be as simple as a 3-line shell script which calls "git init" under
> the hood, but it provides one level of indirection that makes
> replacing/hooking it much simpler for servers. So the parts that are in
> stock git would not be much work (most of the work would be on _calling_
> it, but that is the same for adding a call to "git init").
> 
> I think the main reason the idea hasn't gone anywhere is that nobody
> really cares _that_ much. People just don't create repositories that
> often. I feel like this is one of those topics that comes up once a
> year, and then nothing happens on it, because people just make their
> repo manually and then stop caring about it.
> 
> Just my two cents, of course. :)

Most repos are probably created by a local "git init" or "git clone", or
by clicking a button on a provider's web interface. The need for
git-create-repo seems to be restricted to:

- "command line folks" who use a provider for it's hosting service and
don't fancy a web interface for repo creation

- noobs who need to get their head wrapped around local, remote,
push/pull 'n' stuff...

For the server side git-create-repo to take off we would probably need
two things (besides the client support):

- Implement and ship a git-create-repo which makes this work for git
over ssh seamlessly. (Will take some to trickle down to servers in the
wild.)

- Get a large provider to offer this.

Gitosis/Gitolite are probably to follow easily. I'm beginning to like
idea ;)

Michael
--
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