* Re: [PATCH] Makefile: don't run rm without any files
From: Jonathan Nieder @ 2013-02-13 20:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Matt Kraai, git, Matt Kraai
In-Reply-To: <7vehgk6l11.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> I amended the log message like so:
>
> commit bd9df384b16077337fffe9836c9255976b0e7b91
> Author: Matt Kraai <matt.kraai@amo.abbott.com>
> 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.
^ permalink raw reply
* [PATCH] Documentation: filter-branch env-filter example
From: Tade @ 2013-02-13 19:47 UTC (permalink / raw)
To: git
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 =john@old.example.com ]
+ then
+ GIT_AUTHOR_EMAIL=john@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
^ permalink raw reply related
* Re: inotify to minimize stat() calls
From: Karsten Blees @ 2013-02-13 20:25 UTC (permalink / raw)
To: Jeff King
Cc: Duy Nguyen, kusmabite, Ramkumar Ramachandra, Robert Zeh,
Junio C Hamano, Git List, finnag
In-Reply-To: <20130213181851.GA5603@sigill.intra.peff.net>
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.
^ permalink raw reply
* Re: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
From: Martin Erik Werner @ 2013-02-13 20:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, trsten
In-Reply-To: <7vmwv86le8.fsf@alter.siamese.dyndns.org>
On Wed, 2013-02-13 at 11:53 -0800, Junio C Hamano wrote:
> Martin Erik Werner <martinerikwerner@gmail.com> 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 <martinerikwerner@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 3/3] t9903: add extra tests for bash.showDirtyState
From: Junio C Hamano @ 2013-02-13 20:42 UTC (permalink / raw)
To: Martin Erik Werner; +Cc: git, trsten
In-Reply-To: <1360788036.22870.4.camel@mas>
Martin Erik Werner <martinerikwerner@gmail.com> 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 ;-).
^ permalink raw reply
* [PATCH v3 2/3] t9903: add tests for bash.showUntrackedFiles
From: Martin Erik Werner @ 2013-02-13 20:58 UTC (permalink / raw)
To: gitster; +Cc: git, trsten, Martin Erik Werner
In-Reply-To: <7vmwv854ji.fsf@alter.siamese.dyndns.org>
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 <martinerikwerner@gmail.com>
---
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
^ permalink raw reply related
* [PATCH v3 3/3] t9903: add extra tests for bash.showDirtyState
From: Martin Erik Werner @ 2013-02-13 20:58 UTC (permalink / raw)
To: gitster; +Cc: git, trsten, Martin Erik Werner
In-Reply-To: <1360789100-26285-1-git-send-email-martinerikwerner@gmail.com>
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 <martinerikwerner@gmail.com>
---
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
^ permalink raw reply related
* Re: [RFC v2] git-multimail: a replacement for post-receive-email
From: Michael Haggerty @ 2013-02-13 21:42 UTC (permalink / raw)
To: Matthieu Moy
Cc: git discussion list, Andy Parkins, Sitaram Chamarty,
Junio C Hamano, Marc Branchaud,
Ævar Arnfjörð Bjarmason, Chris Hiestand
In-Reply-To: <vpqtxpgb6ue.fsf@grenoble-inp.fr>
On 02/13/2013 03:56 PM, Matthieu Moy wrote:
> Michael Haggerty <mhagger@alum.mit.edu> 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 by the push (i.e., the ones
returned by push.get_new_commits(None)).
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Jeff King @ 2013-02-13 22:55 UTC (permalink / raw)
To: blees
Cc: Duy Nguyen, kusmabite, Ramkumar Ramachandra, Robert Zeh,
Junio C Hamano, Git List, finnag
In-Reply-To: <511BF6D7.3030404@gmail.com>
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
^ permalink raw reply
* Cumulative "git read-tree -m" rejected with overwriting warning
From: Marcin Owsiany @ 2013-02-13 23:02 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 2117 bytes --]
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 <marcin@owsiany.pl> 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
[-- Attachment #2: testcase.sh --]
[-- Type: application/x-sh, Size: 1488 bytes --]
^ permalink raw reply
* Re: Cumulative "git read-tree -m" rejected with overwriting warning
From: Junio C Hamano @ 2013-02-13 23:40 UTC (permalink / raw)
To: Marcin Owsiany; +Cc: git
In-Reply-To: <20130213230213.GT20333@localhost>
Marcin Owsiany <marcin@owsiany.pl> 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.
^ permalink raw reply
* [BUG] Veryfing signatures in git log fails when language is not english
From: XANi @ 2013-02-14 0:18 UTC (permalink / raw)
To: git
[-- Attachment #1: Type: text/plain, Size: 600 bytes --]
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) <xani666@gmail.com>
GnuPG: 0xEA8ACE64
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: inotify to minimize stat() calls
From: Karsten Blees @ 2013-02-14 0:48 UTC (permalink / raw)
To: Jeff King
Cc: Duy Nguyen, kusmabite, Ramkumar Ramachandra, Robert Zeh,
Junio C Hamano, Git List, finnag
In-Reply-To: <20130213225529.GA25353@sigill.intra.peff.net>
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;
+ hash_index_entry(istate, ce->dir);
}
+ ce->dir->ce_flags &= ~CE_UNHASHED;
+ ce->dir->ce_size++;
}
static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
@@ -74,7 +72,7 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
if (ce->ce_flags & CE_HASHED)
return;
ce->ce_flags |= CE_HASHED;
- ce->next = ce->dir_next = NULL;
+ ce->next = ce->dir = NULL;
hash = hash_name(ce->name, ce_namelen(ce));
pos = insert_hash(hash, ce, &istate->name_hash);
if (pos) {
@@ -137,38 +135,32 @@ static int same_name(const struct cache_entry *ce, const char *name, int namelen
if (!icase)
return 0;
- /*
- * If the entry we're comparing is a filename (no trailing slash), then compare
- * the lengths exactly.
- */
- if (name[namelen - 1] != '/')
- return slow_same_name(name, namelen, ce->name, len);
-
- /*
- * For a directory, we point to an arbitrary cache_entry filename. Just
- * make sure the directory portion matches.
- */
- return slow_same_name(name, namelen, ce->name, namelen < len ? namelen : len);
+ return slow_same_name(name, namelen, ce->name, len);
}
-struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int icase)
+static struct cache_entry *lookup_index_entry(struct index_state *istate, const char *name, int namelen, int icase)
{
unsigned int hash = hash_name(name, namelen);
- struct cache_entry *ce;
-
- lazy_init_name_hash(istate);
- ce = lookup_hash(hash, &istate->name_hash);
+ struct cache_entry *ce = lookup_hash(hash, &istate->name_hash);
while (ce) {
if (!(ce->ce_flags & CE_UNHASHED)) {
if (same_name(ce, name, namelen, icase))
return ce;
}
- if (icase && name[namelen - 1] == '/')
- ce = ce->dir_next;
- else
- ce = ce->next;
+ ce = ce->next;
}
+ return NULL;
+}
+
+struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int icase)
+{
+ struct cache_entry *ce;
+
+ lazy_init_name_hash(istate);
+ ce = lookup_index_entry(istate, name, namelen, icase);
+ if (ce)
+ return ce;
/*
* Might be a submodule. Despite submodules being directories,
@@ -182,7 +174,7 @@ struct cache_entry *index_name_exists(struct index_state *istate, const char *na
* true.
*/
if (icase && name[namelen - 1] == '/') {
- ce = index_name_exists(istate, name, namelen - 1, icase);
+ ce = lookup_index_entry(istate, name, namelen - 1, icase);
if (ce && S_ISGITLINK(ce->ce_mode))
return ce;
}
^ permalink raw reply related
* Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
From: Andrew Ardill @ 2013-02-14 2:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <7vpq04b5e2.fsf@alter.siamese.dyndns.org>
On 14 February 2013 02:27, Junio C Hamano <gitster@pobox.com> 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
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
From: Junio C Hamano @ 2013-02-14 4:36 UTC (permalink / raw)
To: Andrew Ardill; +Cc: git@vger.kernel.org
In-Reply-To: <CAH5451kogwuzOs+BrHksDSdECbHrmW8DwTve0_kKq+-PTx+4bw@mail.gmail.com>
Andrew Ardill <andrew.ardill@gmail.com> 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.
^ permalink raw reply
* Re: What's cooking in git.git (Feb 2013, #05; Tue, 12)
From: Andrew Ardill @ 2013-02-14 4:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git@vger.kernel.org
In-Reply-To: <7vtxpf341w.fsf@alter.siamese.dyndns.org>
On 14 February 2013 15:36, Junio C Hamano <gitster@pobox.com> 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
^ permalink raw reply
* Re: [PATCH] Documentation: filter-branch env-filter example
From: Johannes Sixt @ 2013-02-14 6:49 UTC (permalink / raw)
To: Tade; +Cc: git
In-Reply-To: <511BEDDF.7010800@hell.org.pl>
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 =john@old.example.com ]
This should read
if [ "$GIT_AUTHOR_EMAIL" = john@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=john@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
^ permalink raw reply
* Git 1.8.2 l10n round 2
From: Jiang Xin @ 2013-02-14 7:36 UTC (permalink / raw)
To: Byrial Jensen, Ralf Thielow,
Ævar Arnfjörð Bjarmason, Marco Paolone,
Vincent van Ravesteijn, Marco Sousa, Peter Krefting,
Trần Ngọc Quân, David Hrbáč
Cc: Git List
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 <worldhello.net@gmail.com>
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
^ permalink raw reply
* Re: [PATCH v4 3/4] count-objects: report garbage files in pack directory too
From: Duy Nguyen @ 2013-02-14 9:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vehgkb43v.fsf@alter.siamese.dyndns.org>
On Wed, Feb 13, 2013 at 10:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> + 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.
I know. I just don't know how to name the variable to say "the element
after the last one".
--
Duy
^ permalink raw reply
* Re: [PATCH v3] branch: show rebase/bisect info when possible instead of "(no branch)"
From: Duy Nguyen @ 2013-02-14 9:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Matthieu Moy, Jonathan Niedier
In-Reply-To: <7v1ucq3b7e.fsf@alter.siamese.dyndns.org>
On Sat, Feb 9, 2013 at 1:35 AM, Junio C Hamano <gitster@pobox.com> wrote:
> We may want to refactor wt_status_print_state() and its callee a bit
> so that it and this part can share the logic without duplication and
> risk implementing subtly different decision. wt_status used to have
> clean separation between collection phase and presentation phase,
> but the wall between the phases seems deteriorated over time as more
> "in progress" crufts have been piled on top.
>
> Such a refactoring may look larger than necessary, but on the other
> hand, I do not see this feature very useful if it can over time
> drift away from what we will see in "git status", so...
OK. I'll wait until nd/status-show-in-progress is merged to master,
then rework this patch on top.
--
Duy
^ permalink raw reply
* Re: [BUG] Veryfing signatures in git log fails when language is not english
From: Michael J Gruber @ 2013-02-14 10:55 UTC (permalink / raw)
To: XANi; +Cc: git
In-Reply-To: <20130214011837.04880b3e@hydra.devrandom.pl>
XANi venit, vidit, dixit 14.02.2013 01:18:
> 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.
>
There are really multiple problems here:
1. git calls gpg without setting LANG but expects output in LANG=C
2. git looks at the textual output from gpg to check the validity.
3. In fact, it does so only for %G and the display of signed merge
commits, in all other cases it checks the return code only.
gpg is not supposed to be used like that.
Since the callers of verify_signed_buffer do that craziness there is
some refactoring to be done.
A false hotfix would be to set LANG=C when calling gpg from git, but
that wouldn't solve the real problem. Besides, we do want LANG dependent
output for the user.
I'll have a closer look.
BTW: Thanks for the clear report :)
Michael
^ permalink raw reply
* Re: [BUG] Veryfing signatures in git log fails when language is not english
From: Mariusz Gronczewski @ 2013-02-14 12:42 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <511CC288.30607@drmicha.warpmail.net>
2013/2/14 Michael J Gruber <git@drmicha.warpmail.net>:
> XANi venit, vidit, dixit 14.02.2013 01:18:
>> 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.
>>
>
> There are really multiple problems here:
>
> 1. git calls gpg without setting LANG but expects output in LANG=C
>
> 2. git looks at the textual output from gpg to check the validity.
>
> 3. In fact, it does so only for %G and the display of signed merge
> commits, in all other cases it checks the return code only.
>
> gpg is not supposed to be used like that.
>
> Since the callers of verify_signed_buffer do that craziness there is
> some refactoring to be done.
>
> A false hotfix would be to set LANG=C when calling gpg from git, but
> that wouldn't solve the real problem. Besides, we do want LANG dependent
> output for the user.
>
> I'll have a closer look.
>
> BTW: Thanks for the clear report :)
>
> Michael
What is really missing is an ability to display used key ID without
hammering git log output with regexps, it would be much easier to
validate incoming commits if there was format option to just display
key ID instead of signer name. %GS isn't really good solution for that
because it will show only one of email addresses used in the key and
script checking signatures would have to always pick "right" one.
--
Mariusz Gronczewski (XANi) <xani666@gmail.com>
GnuPG: 0xEA8ACE64
^ permalink raw reply
* Re: [RFC v2] git-multimail: a replacement for post-receive-email
From: Matthieu Moy @ 2013-02-14 12:55 UTC (permalink / raw)
To: Michael Haggerty
Cc: git discussion list, Andy Parkins, Sitaram Chamarty,
Junio C Hamano, Marc Branchaud,
Ævar Arnfjörð Bjarmason, Chris Hiestand
In-Reply-To: <511C08AF.7090502@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> On 02/13/2013 03:56 PM, Matthieu Moy wrote:
>
>> 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?
Python 2.4.3, installed with RHEL 5.9.
> I just discovered that the script wouldn't have worked with Python
> 2.4, where "email.utils" used to be called "email.Utils".
Indeed, "import email.Utils" works with this Python.
> But I pushed a fix to GitHub:
>
> ddb1796660 Accommodate older versions of Python's email module.
Not sufficient, but I added a pull request that works for me with 2.4.
>> @@ -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?)
I did not think very carefully about this case, but the behavior of my
code seems sensible (although not uncontroversial): it's just showing
the detailed log for the same commits as the summary at the top of the
email. I have no personnal preferences.
> 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).
Right.
> 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 by the push (i.e., the ones
> returned by push.get_new_commits(None)).
I updated the pull request with a version that works for new branches,
and takes the list of commits to display from the call to
get_new_commits (which were already there for other purpose). Then, it
essentially calls "git log --no-walk $list_of_sha1s".
This should be better.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* [PATCH/RFC] clone: suggest index version 4 when the index is bigger than 512 KiB
From: Nguyễn Thái Ngọc Duy @ 2013-02-14 13:34 UTC (permalink / raw)
To: git; +Cc: Nguyễn Thái Ngọc Duy
I just realized that many of my big repos are still on index v2 while
v4 should reduce its size significantly (3.8M -> 2.9M for linux-2.6
and 25M -> 14M for webkit, for example). I wanted to propose index v4
as the new default version, because I guess that not many people
outside git@vger are aware of it, but perhaps this approach is less
drastic. It only suggest index v4 when the index size after clone hits
512K limit.
I have 170 git repositories (most of them Gnome projects) and only big
ones exceed the limit: webkit, linux-2.6, libreoffice-core, wine
(530K), gentoo-x86. Gimp and banshee are close (510K and 321K). The
rest barely hits 256K. So this hint won't show up often. On second
thought, maybe we should lower it down to 300K to show more often and
raise awareness on index v4 :)
Something I should have done in this patch too, is state that index v4
is only supported since vXXX, that git clients older than that will
not work. But maybe just put that in update-index man page and refer
there.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
builtin/clone.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/builtin/clone.c b/builtin/clone.c
index e0aaf13..7cd1b60 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -543,6 +543,7 @@ static int checkout(void)
struct tree *tree;
struct tree_desc t;
int err = 0, fd;
+ struct stat st;
if (option_no_checkout)
return 0;
@@ -591,6 +592,15 @@ static int checkout(void)
if (!err && option_recursive)
err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
+ if (!err &&
+ !stat(git_path("index"), &st) &&
+ st.st_size > 512 * 1024)
+ advise(_("Your index is quite large (%d KiB).\n"
+ "You may want to update to index version 4 to reduce its size,\n"
+ "as large index files may affect performance, using the command:\n"
+ " git update-index --index-version 4"),
+ st.st_size / 1024);
+
return err;
}
--
1.8.1.2.536.gf441e6d
^ permalink raw reply related
* Re: inotify to minimize stat() calls
From: Magnus Bäck @ 2013-02-14 14:36 UTC (permalink / raw)
To: demerphq
Cc: Duy Nguyen, Ramkumar Ramachandra, Robert Zeh, Junio C Hamano,
Git List, finnag
In-Reply-To: <CANgJU+WYSD8RHb19EP0M89=Y_XskfjDtFWf51qjg4ur+rDb3ug@mail.gmail.com>
On Sunday, February 10, 2013 at 08:26 EST,
demerphq <demerphq@gmail.com> wrote:
> Is windows stat really so slow?
Well, the problem is that there is no such thing as "Windows stat" :-)
> I encountered this perception in windows Perl in the past, and I know
> that on windows Perl stat *appears* slow compared to *nix, because in
> order to satisfy the full *nix stat interface, specifically the nlink
> field, it must open and close the file*. As of 5.10 this can be
> disabled by setting a magic var ${^WIN32_SLOPPY_STAT} to a true value,
> which makes a significant improvement to the performance of the Perl
> level stat implementation. I would not be surprised if the cygwin
> implementation of stat() has the same issue as Perl did, and that stat
> appears much slower than it actually need be if you don't care about
> the nlink field.
I suggested a few years ago that FindFirstFile() be used to implement
stat() since it's way faster than opening and closing the file, but
FindFirstFile() apparently produces unreliable mtime results when DST
shifts are involved.
http://thread.gmane.org/gmane.comp.version-control.git/114041
(The reference link in Johannes Sixt's first email is broken, but I'm
sure the information can be dug up.)
Based on a quick look it seems GetFileAttributesEx() is still used for
mingw and cygwin Git.
--
Magnus Bäck
baeck@google.com
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox