Re: [PATCH 0/5] fix up 'jk/pack-bitmap' branch

2013-11-09 Thread Jeff King
On Sat, Nov 09, 2013 at 10:03:56PM +0100, Thomas Rast wrote:

> > Of course. You write little endian and also read back little endian;
> > that should work just fine, no?
> >
> > OTOH, if you write with Intel and read with PPC, then you should observe
> > misbehavior with the above patch.
> 
> Maybe you could check in a simple test that the bitmap for a very
> predictable pack (e.g. only one commit) has a certain content, by
> checking its hash.  That would guard against accidental format changes.

Another option would be to cross-check reading and writing against JGit,
which is valuable for other reasons besides checking endianness. I do
not think most people will have JGit installed, but it's better than
nothing (and if even one person runs it, we have a chance of finding a
bug).

-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 0/5] fix up 'jk/pack-bitmap' branch

2013-11-09 Thread Thomas Rast
Johannes Sixt  writes:

> Am 09.11.2013 12:35, schrieb Torsten Bögershausen:
>>
>> If I do like this in compat/bswap.h
>> 
>> # define ntohll(n) (n)
>> # define htonll(n) (n)
>> (on an Intel processor, little endian)
>> 
>> then t5310 passes, even if the uint64_t words are written
>> in little endian to disc instead of big endian.
>
> Of course. You write little endian and also read back little endian;
> that should work just fine, no?
>
> OTOH, if you write with Intel and read with PPC, then you should observe
> misbehavior with the above patch.

Maybe you could check in a simple test that the bitmap for a very
predictable pack (e.g. only one commit) has a certain content, by
checking its hash.  That would guard against accidental format changes.

-- 
Thomas Rast
t...@thomasrast.ch
--
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] checkout: most of the time we have good leading directories

2013-11-09 Thread Junio C Hamano
Thomas Rast  writes:

> Junio C Hamano  writes:
>
>> When "git checkout" wants to create a path, e.g. a/b/c/d/e, after
>> seeing if the entire thing already exists (in which case we check if
>> that is up-to-date and do not bother to check it out, or we unlink
>> and recreate it), we validate that the leading directory path is
>> without funny symlinks by seeing a/, a/b/, a/b/c/ and then a/b/c/d/
>> are all without funny symlinks, by calling has_dirs_only_path() in
>> this order.
>>
>> When we are checking out many files (imagine: initial checkout),
>> however, it is likely that an earlier checkout would have already
>> made sure that the leading directory a/b/c/d/ is in good order; by
>> first checking the whole path a/b/c/d/ first, we can often bypass
>> calls to has_dirs_only_path() for leading part.
>
> Naively one would think that this is just as much work -- to correctly
> verify that the path consist only of actual directories (not symlinks)
> we have to lstat() every component regardless.  It seems the reason this
> is an optimization is that has_dirs_only_path() caches its results, so
> that we can get 'a/b/c/d/ is okay in every component' from the cache.
>
> Is this analysis correct?  If so, can you spell that out in the commit
> message?

It was done without analysis ;-) but I think you are correct.

If you are checking out a/b/c/d/{m,a,n,y}, after you checked out
a/b/c/d/m, the has_dirs_only_path cache knows a/b/c/d/ is in good
order so when you check out a/b/c/d/{a,n,y}, we can just ask for
a/b/c/d/ and get an OK immediately.  There is no point asking from
a/, a/b/, a/b/c/ and then a/b/c/d/, in the original pessimistic
order.  A change done _right_ to properly optimize this might even
want to change the main loop that the patch bypassed.

I do not think the patch (or the "change done right" for that
matter) will make much difference on a platform with good filesystem
metadata caching. It may be very interesting to see if that simple
patch makes any difference on Windows, though. If it does, then we
may want to look into cleaning up the code further.

Thanks for a comment.



--
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 0/5] fix up 'jk/pack-bitmap' branch

2013-11-09 Thread Johannes Sixt
Am 09.11.2013 12:35, schrieb Torsten Bögershausen:
> On 2013-11-08 23.29, Jeff King wrote:
>> On Fri, Nov 08, 2013 at 06:10:30PM +0100, Torsten Bögershausen wrote:
>>
>>> Side question:
>>> Do we have enough test coverage for htonll()/ntohll(),
>>> or do we want do the "module test" which I send a couple of days before ?
>>
>> The series adds tests for building and using the ewah bitmaps, which in
>> turn rely on the htonll code. So they are being tested in the existing
>> series.
>>
>> -Peff
> You are thinking about t5310-pack-bitmaps.sh ?
> If I do like this in compat/bswap.h
> 
> # define ntohll(n) (n)
> # define htonll(n) (n)
> (on an Intel processor, little endian)
> 
> then t5310 passes, even if the uint64_t words are written
> in little endian to disc instead of big endian.

Of course. You write little endian and also read back little endian;
that should work just fine, no?

OTOH, if you write with Intel and read with PPC, then you should observe
misbehavior with the above patch.

-- 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: [PATCH] l10n: de.po: improve error message when pushing to unknown upstream

2013-11-09 Thread Thomas Rast
Ralf Thielow  writes:

> Signed-off-by: Ralf Thielow 
> ---
>  po/de.po | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/po/de.po b/po/de.po
> index a005dcc..8b824cc 100644
> --- a/po/de.po
> +++ b/po/de.po
> @@ -7923,7 +7923,7 @@ msgid ""
>  "to update which remote branch."
>  msgstr ""
>  "Sie versenden nach '%s', welches kein Upstream-Repository Ihres aktuellen\n"
> -"Branches '%s' ist, ohne mir mitzuteilen, was ich versenden soll, um 
> welchen\n"
> +"Branches '%s' ist, ohne anzugeben, was versendet werden soll, um welchen\n"
>  "Remote-Branch zu aktualisieren."
>  
>  #: builtin/push.c:167

Acked-by: Thomas Rast 

-- 
Thomas Rast
t...@thomasrast.ch
--
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 00/86] replace prefixcmp() with has_prefix()

2013-11-09 Thread Thomas Rast
Christian Couder  writes:

> Christian Couder (86):
>   strbuf: add has_prefix() to be used instead of prefixcmp()
>   diff: replace prefixcmd() with has_prefix()
>   fast-import: replace prefixcmd() with has_prefix()
[...]
>   builtin/update-ref: replace prefixcmd() with has_prefix()
>   builtin/upload-archive: replace prefixcmd() with has_prefix()
>   strbuf: remove prefixcmp() as it has been replaced with has_prefix()

All of your subjects except for the first and last say "prefixcm*d*". :-)

-- 
Thomas Rast
t...@thomasrast.ch
--
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] checkout: most of the time we have good leading directories

2013-11-09 Thread Thomas Rast
Junio C Hamano  writes:

> When "git checkout" wants to create a path, e.g. a/b/c/d/e, after
> seeing if the entire thing already exists (in which case we check if
> that is up-to-date and do not bother to check it out, or we unlink
> and recreate it), we validate that the leading directory path is
> without funny symlinks by seeing a/, a/b/, a/b/c/ and then a/b/c/d/
> are all without funny symlinks, by calling has_dirs_only_path() in
> this order.
>
> When we are checking out many files (imagine: initial checkout),
> however, it is likely that an earlier checkout would have already
> made sure that the leading directory a/b/c/d/ is in good order; by
> first checking the whole path a/b/c/d/ first, we can often bypass
> calls to has_dirs_only_path() for leading part.

Naively one would think that this is just as much work -- to correctly
verify that the path consist only of actual directories (not symlinks)
we have to lstat() every component regardless.  It seems the reason this
is an optimization is that has_dirs_only_path() caches its results, so
that we can get 'a/b/c/d/ is okay in every component' from the cache.

Is this analysis correct?  If so, can you spell that out in the commit
message?

-- 
Thomas Rast
t...@thomasrast.ch
--
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: [PATCHv2] l10n: de.po: translate 68 new messages

2013-11-09 Thread Thomas Rast
Ralf Thielow  writes:

>  #: remote.c:1837
> -#, fuzzy
>  msgid "  (use \"git branch --unset-upstream\" to fixup)\n"
> -msgstr "git branch --set-upstream-to %s\n"
> +msgstr "(benutzen Sie \"git branch --unset-upstream\" zum Beheben)\n"

Minor nit: the indent (it's simply a continuation indent for the "your
upstream is gone" message) is not consistent with other continuation
indents in the same vein, e.g.

  #: remote.c:1794
  msgid "  (use \"git push\" to publish your local commits)\n"
  msgstr "  (benutzen Sie \"git push\" um lokale Commits zu publizieren)\n"

>  #. Maybe the user already did that, don't error out here
>  #: submodule.c:76
> -#, fuzzy, c-format
> +#, c-format
>  msgid "Could not update .gitmodules entry %s"
> -msgstr "Konnte git-Verweis %s nicht erstellen"
> +msgstr "Konnte Eintrag '%s' in .gitmodules nicht aktualisieren"
>  
>  #. Maybe the user already did that, don't error out here
>  #: submodule.c:109
> -#, fuzzy, c-format
> +#, c-format
>  msgid "Could not remove .gitmodules entry for %s"
> -msgstr "Konnte Sektion '%s' nicht aus Konfiguration entfernen"
> +msgstr "Konnte Sektion '%s' nicht aus .gitmodules entfernen"

Here you translate 'entry' once as Eintrag, and once as Sektion?  Is the
latter just a mistake from the fuzzy translation?

>  #: builtin/gc.c:265
>  msgid "force running gc even if there may be another gc running"
> -msgstr ""
> +msgstr "erzwingt Ausführung des Garbage Collectors selbst wenn ein anderer\n"
> +"Gargabe Collector bereits ausgeführt wird"
>  
>  #: builtin/gc.c:305
>  #, c-format
> @@ -5502,6 +5493,8 @@ msgstr ""
>  msgid ""
>  "gc is already running on machine '%s' pid % (use --force if not)"
>  msgstr ""
> +"Gargabe Collector wird bereits auf Maschine '%s' pid % 
> ausgeführt\n"
> +"(benutzen Sie --force falls nicht)"

According to the glossary:

  When a message is referring to the command, e.g. in error messages, we
  do not translate the term.

Isn't this an instance of this rule?  You could translate gc to git-gc
to avoid some confusion, but I think it specifically refers to running
the command.

>  #: builtin/repack.c:143
> -#, fuzzy
>  msgid "pack everything in a single pack"
> -msgstr "packt alles"
> +msgstr "packt alles in ein einzelnes Paket"

"ein einziges Paket" -- or is that my Swiss German again?

Also, the glossary says pack -> Pack-Datei.  So it'd end up at "eine
einzige Pack-Datei".

>  #: builtin/repack.c:145
> -#, fuzzy
>  msgid "same as -a, and turn unreachable objects loose"
> -msgstr "zeigt unerreichbare Objekte"
> +msgstr "genau wie -a, unerreichbare Objekte werden aber nicht gelöscht"

Clever :-)

>  #: builtin/repack.c:150
>  msgid "pass --no-reuse-delta to git-pack-objects"
> -msgstr ""
> +msgstr "übergibt --no-reuse-delta nach git-pack-objects"
>  
>  #: builtin/repack.c:152
>  msgid "pass --no-reuse-object to git-pack-objects"
> -msgstr ""
> +msgstr "übergibt --no-reuse-object nach git-pack-objects"
> 
>  #: builtin/repack.c:157
>  msgid "pass --local to git-pack-objects"
> -msgstr ""
> +msgstr "übergibt --local nach git-pack-objects"

s/nach/an/?

>  #: builtin/repack.c:158
>  msgid "approxidate"
> -msgstr ""
> +msgstr "Datumsangabe"

On an entirely different note, we don't parse German approxidates, do
we?

-- 
Thomas Rast
t...@thomasrast.ch
--
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 0/5] fix up 'jk/pack-bitmap' branch

2013-11-09 Thread Torsten Bögershausen
On 2013-11-08 23.29, Jeff King wrote:
> On Fri, Nov 08, 2013 at 06:10:30PM +0100, Torsten Bögershausen wrote:
> 
>> Side question:
>> Do we have enough test coverage for htonll()/ntohll(),
>> or do we want do the "module test" which I send a couple of days before ?
> 
> The series adds tests for building and using the ewah bitmaps, which in
> turn rely on the htonll code. So they are being tested in the existing
> series.
> 
> -Peff
You are thinking about t5310-pack-bitmaps.sh ?
If I do like this in compat/bswap.h

# define ntohll(n) (n)
# define htonll(n) (n)
(on an Intel processor, little endian)

then t5310 passes, even if the uint64_t words are written
in little endian to disc instead of big endian.

What do I miss?
/torsten






--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git-remote-mediawiki: do not remove installed files in "clean" target

2013-11-09 Thread Matthieu Moy
Jonathan Nieder  writes:

> Running "make clean" after a successful "make install" should not
> result in a broken mediawiki remote helper.

Acked-by: Matthieu Moy 

Ideally, there could be a "make uninstall" removing the installed file.

-- 
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] po/TEAMS: update Thomas Rast's email address

2013-11-09 Thread Thomas Rast
Ralf Thielow  writes:

> Signed-off-by: Ralf Thielow 
> ---
>  po/TEAMS | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/po/TEAMS b/po/TEAMS
> index 107aa59..f9a99ed 100644
> --- a/po/TEAMS
> +++ b/po/TEAMS
> @@ -8,7 +8,7 @@ Leader:   Byrial Jensen 
>  Language:de (German)
>  Repository:  https://github.com/ralfth/git-po-de
>  Leader:  Ralf Thielow 
> -Members: Thomas Rast 
> +Members: Thomas Rast 
>   Jan Krüger 
>   Christian Stimming 

Acked-by: Thomas Rast 

-- 
Thomas Rast
t...@thomasrast.ch
--
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] git-send-email: Added the ability to query the number of smtp password questions

2013-11-09 Thread silvio
From: Silvio F 

Signed-off-by: Silvio F 
---
 Documentation/git-send-email.txt |  4 
 git-send-email.perl  | 32 +---
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index f0e57a5..ac993d6 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -364,6 +364,10 @@ sendemail.confirm::
one of 'always', 'never', 'cc', 'compose', or 'auto'. See '--confirm'
in the previous section for the meaning of these values.
 
+sendmail.askpasswordcount::
+   Number of times the smtp password can be entered before sending mail is
+   aborted. Default is 1.
+
 EXAMPLE
 ---
 Use gmail as the smtp server
diff --git a/git-send-email.perl b/git-send-email.perl
index 3782c3b..9b2eda3 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -203,6 +203,7 @@ my ($validate, $confirm);
 my (@suppress_cc);
 my ($auto_8bit_encoding);
 my ($compose_encoding);
+my ($askpasswordcount) = 1;
 
 my ($debug_net_smtp) = 0;  # Net::SMTP, see send_message()
 
@@ -237,6 +238,7 @@ my %config_settings = (
 "from" => \$sender,
 "assume8bitencoding" => \$auto_8bit_encoding,
 "composeencoding" => \$compose_encoding,
+"askpasswordcount" => \$askpasswordcount,
 );
 
 my %config_path_settings = (
@@ -360,6 +362,10 @@ sub read_config {
}
}
 
+   if ($askpasswordcount < 1) {
+   $askpasswordcount = 1
+   }
+
if (!defined $smtp_encryption) {
my $enc = Git::config(@repo, "$prefix.smtpencryption");
if (defined $enc) {
@@ -1069,17 +1075,21 @@ sub smtp_auth_maybe {
# TODO: Authentication may fail not because credentials were
# invalid but due to other reasons, in which we should not
# reject credentials.
-   $auth = Git::credential({
-   'protocol' => 'smtp',
-   'host' => smtp_host_string(),
-   'username' => $smtp_authuser,
-   # if there's no password, "git credential fill" will
-   # give us one, otherwise it'll just pass this one.
-   'password' => $smtp_authpass
-   }, sub {
-   my $cred = shift;
-   return !!$smtp->auth($cred->{'username'}, $cred->{'password'});
-   });
+   for my $i (1 .. $askpasswordcount) {
+   $auth = Git::credential({
+   'protocol' => 'smtp',
+   'host' => smtp_host_string(),
+   'username' => $smtp_authuser,
+   # if there's no password, "git credential fill" will
+   # give us one, otherwise it'll just pass this one.
+   'password' => $smtp_authpass
+   }, sub {
+   my $cred = shift;
+   return !!$smtp->auth($cred->{'username'}, 
$cred->{'password'});
+   });
+
+   last if ($auth);
+   }
 
return $auth;
 }
-- 
1.8.4.2

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] Emphasize options and force ASCIIDOC escaping of "--"

2013-11-09 Thread Johannes Sixt
Am 09.11.2013 01:48, schrieb Jason St. John:
> -`--since=` limits to commits newer than ``, and using it
> -with `--grep=` further limits to commits whose log message
> +'\--since=' limits to commits newer than ``, and using it
> +with '\--grep=' further limits to commits whose log message

I don't think this kind of change goes in the right direction.
Currently, the general style seems to be that options are typeset in
monospaced font. I looked at git-merge and git-rebase (and git-log, of
course). But after your change, they are typeset in italics, in the HTML
version of the manual pages.

I did not find an advice on the preferred formatting, though, except for
the last sentence in Documentation/CodingGuidelines.

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