Incomplete data: Delta source ended unexpectedly, now what ?

2014-01-23 Thread Mihai Marinescu
Hi,
I have been migrating our svn repo to Git and we are not quite ready to move 
yet so I am keeping the Git repo update everyday by doing git svn fetch but 
today I ran into an issue which I don't know how to solve :

I get following error when I run git svn fetch :

Incomplete data: Delta source ended unexpectedly at 
/usr/lib/perl5/site_perl/Git/SVN/Ra.pm line 290

Anybody who can help ?

Btw. I am using Git 1.8.5.2.

/Mihai


This e-mail is confidential and may be read, copied and used only by the 
intended recipient. If you have received it in error, please contact the sender 
immediately by return e-mail. Please then delete the e-mail and do not disclose 
its contents to any other person.

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


Incomplete data: Delta source ended unexpectedly

2014-01-23 Thread Mihai Marinescu
Hi,
I have been migrating our svn repo to Git and we are not quite ready to move 
yet so I am keeping the Git repo update everyday by doing git svn fetch but 
today I ran into an issue which I don't know how to solve :

I get following error when I run git svn fetch :

Incomplete data: Delta source ended unexpectedly at 
/usr/lib/perl5/site_perl/Git/SVN/Ra.pm line 290

Anybody who can help ?

Btw. I am using Git 1.8.5.2.

/Mihai

This e-mail is confidential and may be read, copied and used only by the 
intended recipient. If you have received it in error, please contact the sender 
immediately by return e-mail. Please then delete the e-mail and do not disclose 
its contents to any other person.

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


submodules

2014-01-23 Thread shawn wilson
I've got a git repo that just contains a bunch of small projects
(scripts, configs, etc). The idea was (is) to make it so that each
project is tracked seperately (different branches, not needing to
filter out logs for unrelated commits). And it works great for that.
The problem is that I guess I don't know how to manage it and keep
stumbling and tripping over myself here.

My issue is in trying to update the submodules, I'm getting:
 % git submodule update --init
gits/kt (master ⚡)
swlap1
fatal: reference is not a tree: 98f1e9f99fca32ab3de901219eb2f600d38ab3a5
Unable to checkout '98f1e9f99fca32ab3de901219eb2f600d38ab3a5' in
submodule path 'repo_a'

Ok, so a bit of googling and I found this:
http://stackoverflow.com/questions/13425298/git-submodule-update-fatal-error-reference-is-not-a-tree
Ok, so update the repo that contains the submodules every time you
push from a sub repo? How / where do I create a hook to do this?

And then:
http://stackoverflow.com/questions/2155887/git-submodule-head-reference-is-not-a-tree-error
So, the first example didn't really work:
 % cd repo_a
gits/kt (master ⚡) swlap1
 % git checkout 98f1e9
  kt/repo_a (master)
swlap1
error: pathspec '98f1e9' did not match any file(s) known to git.

So, at this point I realized I have pushed from two repos (maybe
others have as well) and not updated the repo of submodules. So, as
long as the issue is just with this one repo, I'm ok with loosing a
little data (not much has changed in a while and maybe I'll go and
find the blobs and merge them back in later). So I try the other
method:
 % git log --oneline -p -- repo_a
gits/kt (master ⚡) swlap1
7911a1e Bump.
diff --git a/repo_a b/repo_a
index 206635e..98f1e9f 16
--- a/repo_a
+++ b/repo_a
@@ -1 +1 @@
-Subproject commit 206635eed8b94e4133a7689b34a570e2fb0b6069
...
 % git checkout d92f592- -- repo_a
gits/kt (master ⚡) swlap1
fatal: invalid reference: d92f592-



So, (after this is solved) is there a post hook (not even sure where
such a thing would go) after pushing from a submodule repo (maybe a
concatination of commit messages or something) so that this stays in
sync and I never have to think about this again (though I think I'll
remember after winning this fight).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/4] subtree: support split --rejoin --squash

2014-01-23 Thread Pierre Penninckx
Hi again,

After using the patched git-subtree (with patches 1 to 3) for a while,
I suspect the added functionality does not do exactly what I wanted.
So yes, now when doing a rejoin, the squash of the split commits is
used. But how can I push this squash instead of the individual
commits? The problem is I don't know how to reference that squashed
commit.

I tried adding the --branch option but it adds the branch to the top
of the individual commits so no luck there.
This is maybe obvious but I'm not at ease with commit references in git.

Pierre

2014/1/23 Matthew Ogilvie mmogilvi_...@miniinfo.net:
 On Wed, Jan 22, 2014 at 03:58:28PM +0100, Pierre Penninckx wrote:
 2013/12/7 Matthew Ogilvie mmogilvi_...@miniinfo.net
  Subject: [PATCH 1/4] subtree: support split --rejoin --squash
 
  Allow using --squash with git subtree split --rejoin.  It
  will still split off (and save to --branch) the complete
  subtree history, but the merge done for the --rejoin will
  be merging a squashed representation of the new subtree
  commits, instead of the commits themselves (similar to
  how git subtree merge --squash works).
 
  Signed-off-by: Matthew Ogilvie mmogilvi_...@miniinfo.net
  ---
 
  I can think of a couple of possible objections to this patch.
  Are these (or any others) worth fixing?
 
  1. Perhaps someone want the saved subtree (--branch) to have
 a squashed representation as well, as an option?  Maybe we
 need two different --squash options?  Something
 like --rejoin-squash?
  2. It could definitely use some automated tests.  In fact,
 pre-existing --squash functionality is hardly tested at
 all, either.
See patch 4 comments for a script I use to help with
 mostly-manual testing.

 Sorry to bother you with this again, but I was wondering if those patches
 would be integrated into git anytime soon.
 And if not, if there is something I can do to help.

 I found them by the way, thanks a lot!

 Pierre

 I'm not sure when or if the patches will make it in.  Junio's
 weekly What's cooking... email has asked for Comments? about
 them for the past several weeks, but I have yet to see
 anyone actually comment about them.

 Searching throught the last couple of years of mailing list
 archives for subtree reveals a general lack of a active
 maintainer(s) to help review and improve patches for git
 subtree.  Given the general lack of help and feedback, it is
 understandable that Junio has largely limited inclusion of
 subtree patches to trivially obvious bug fixes.

 - Matthew Ogilvie
--
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] gitk: Replace next and prev buttons with down and up arrows.

2014-01-23 Thread Paul Mackerras
On Wed, Jan 22, 2014 at 12:18:27PM -0800, Junio C Hamano wrote:
 Is this a good time for me to pull from you?  I see these on your
 'master' branch.
 
 8f86339 gitk: Comply with XDG base directory specification
 786f15c gitk: Replace next and prev buttons with down and up arrows
 c61f3a9 gitk: chmod +x po2msg.sh
 6c626a0 gitk: Update copyright dates
 45f884c gitk: Add Bulgarian translation (304t)
 1f3c872 gitk: Fix mistype
 
 Thanks.

Yes, please pull.  I have just pushed one more:

76d64ca gitk: Indent word-wrapped lines in commit display header

Thanks,
Paul.
--
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: [msysGit] [PATCH v2] Makefile: Fix compilation of Windows resource file

2014-01-23 Thread Pat Thoyts
On 23 January 2014 07:28, Johannes Sixt j.s...@viscovery.net wrote:
 From: Johannes Sixt j...@kdbg.org

 If the git version number consists of less than three period
 separated numbers, then the Windows resource file compilation
 issues a syntax error:

   $ touch git.rc
   $ make V=1 git.res
   GIT_VERSION = 1.9.rc0
   windres -O coff \
 -DMAJOR=1 -DMINOR=9 -DPATCH=rc0 \
 -DGIT_VERSION=\\\1.9.rc0\\\ git.rc -o git.res
   C:\msysgit\msysgit\mingw\bin\windres.exe: git.rc:2: syntax error
   make: *** [git.res] Error 1
   $

 Note that -DPATCH=rc0.

 The values passed via -DMAJOR=, -DMINOR=, and -DPATCH= are used in
 FILEVERSION and PRODUCTVERSION statements, which expect up to four numeric
 values. These version numbers are intended for machine consumption. They
 are typically inspected by installers to decide whether a file to be
 installed is newer than one that exists on the system, but are not used
 for much else.

 We can be pretty certain that there are no tools that look at these
 version numbers, not even the installer of Git for Windows does.
 Therefore, to fix the syntax error, fill in only the first two numbers,
 which we are guaranteed to find in Git version numbers.

 Signed-off-by: Johannes Sixt j...@kdbg.org
 ---
  That not even the installer of Git for Windows uses the FILEVERSION
  numbers is a bold statement of mine (I did not check). If I am wrong,
  this approach for a fix is not viable, and a better fix would be
  needed. Otherwise, an Acked-By would be appreciated so that we can
  have this fix in upstream ASAP.

  Makefile | 2 +-
  git.rc   | 4 ++--
  2 files changed, 3 insertions(+), 3 deletions(-)

 diff --git a/Makefile b/Makefile
 index b4af1e2..99b2b89 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES

  git.res: git.rc GIT-VERSION-FILE
 $(QUIET_RC)$(RC) \
 - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, 
 ,$(subst ., ,$(GIT_VERSION) \
 + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., 
 ,$(GIT_VERSION) \
   -DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@

  ifndef NO_PERL
 diff --git a/git.rc b/git.rc
 index bce6db9..33aafb7 100644
 --- a/git.rc
 +++ b/git.rc
 @@ -1,6 +1,6 @@
  1 VERSIONINFO
 -FILEVERSION MAJOR,MINOR,PATCH,0
 -PRODUCTVERSION  MAJOR,MINOR,PATCH,0
 +FILEVERSION MAJOR,MINOR,0,0
 +PRODUCTVERSION  MAJOR,MINOR,0,0
  BEGIN
BLOCK StringFileInfo
BEGIN
 --
 1.9.rc0.1179.g5088b55

This was put in as a response to
https://github.com/msysgit/git/issues/5 where a request was made to be
able to check the version without actually executing the file. Given
that the majority of versions has the same first two digits this
becomes fairly useless without the patchlevel digit. So it would be
preferable to try to maintain all three digits. The following should
do this:

GIT_VERSION=1.9.rc0
all:
echo $(join -DMAJOR= -DMINOR= -DPATCH=, \
$(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst .,
,$(GIT_VERSION 0 0))

This removes any rc* parts and appends a couple of zeros so that all
missing elements should appear as 0 in the final list.

Pat Thoyts
--
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 2/2] t4010: match_pathspec_depth() and trailing slash after submodule

2014-01-23 Thread Nguyễn Thái Ngọc Duy
When you do git diff HEAD submodule/, submodule from the index is
picked out and match_pathspec_depth() in charge of matching it with
the pathspec submodule/.

Unlike tree_entry_interesting(), match_pathspec_depth() has no
knowledge about entry mode to realize submodule is a directory and
treat the trailing slash specially. And it does not have too, mostly,
because the index only contains files, not directories (not until
submodules come)

I have no solutions for it (no, stripping '/' at pathspec
preprocessing phase seems like a workaround than a solution). So let's
mark it. Maybe I or somebody else could revisit it later.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 t/t4010-diff-pathspec.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index 15a4912..b54251a 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on 
submodule path' '
test_cmp expect actual
 '
 
+test_expect_failure 'diff-cache ignores trailing slash on submodule path' '
+   git diff --name-only HEAD^ submod expect 
+   git diff --name-only HEAD^ submod/ actual 
+   test_cmp expect actual
+'
+
 test_done
-- 
1.8.5.2.240.g8478abd

--
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 1/2] tree-walk.c: ignore trailing slash on submodule in tree_entry_interesting()

2014-01-23 Thread Nguyễn Thái Ngọc Duy
We do ignore trailing slash on a directory, so pathspec abc/ matches
directory abc. A submodule is also a directory. Apply the same logic
to it. This makes git log submodule-path and git log submodule-path/
produce the same output.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 t/t4010-diff-pathspec.sh | 17 +
 tree-walk.c  |  2 +-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
index af5134b..15a4912 100755
--- a/t/t4010-diff-pathspec.sh
+++ b/t/t4010-diff-pathspec.sh
@@ -110,4 +110,21 @@ test_expect_success 'diff-tree -r with wildcard' '
test_cmp expected result
 '
 
+test_expect_success 'setup submodules' '
+   test_tick 
+   git init submod 
+   ( cd submod  test_commit first; ) 
+   git add submod 
+   git commit -m first 
+   ( cd submod  test_commit second; ) 
+   git add submod 
+   git commit -m second
+'
+
+test_expect_success 'diff-tree ignores trailing slash on submodule path' '
+   git diff --name-only HEAD^ HEAD submod expect 
+   git diff --name-only HEAD^ HEAD submod/ actual 
+   test_cmp expect actual
+'
+
 test_done
diff --git a/tree-walk.c b/tree-walk.c
index 680afda..c29b6a3 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -543,7 +543,7 @@ static int match_entry(const struct pathspec_item *item,
if (matchlen  pathlen) {
if (match[pathlen] != '/')
return 0;
-   if (!S_ISDIR(entry-mode))
+   if (!S_ISDIR(entry-mode)  !S_ISGITLINK(entry-mode))
return 0;
}
 
-- 
1.8.5.2.240.g8478abd

--
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: libz and RHEL 5.9 compile of Git

2014-01-23 Thread Torsten Bögershausen
On 2014-01-22 22.27, salmansheikh wrote:
 Got it working but then I had some issues with the perl portions of the
 install and I subsequently thought I could eliminate those portions and
 tried setting export NO_PERL=1 and that installed everything else...and got
 pass this error but when I tried to checkout a git repository as follows, I
 get some remote helper error. Is that related to the perl parts of the git? 

No

 git clone https://github.com/m-labs/migen.git
 Cloning into 'migen'...
 fatal: Unable to find remote helper for 'https'

Please have a look at libcurl.
It seems that libcurl on your system does not support https

The Makefile of Git is your friend:
# Define CURLDIR=/foo/bar if your curl header and library files are in
# /foo/bar/include and /foo/bar/lib directories.

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


New Contact.

2014-01-23 Thread Peter Luk
Dear friend,

It is understandable that you might be a little bit apprehensive because you do 
not know me but I have a lucrative business proposal of mutual interest to 
share with you. I got your reference in my search for someone who suits my 
proposed business relationship.

I am the foreign operations director of the Bank of Korea. My name is Peter 
Luk. I have an obscured business suggestion for you. I will need you to assist 
me in executing a business project from Korea to your country. It involves the 
transfer of large sums of money worth 11.5M U.S.D (Eleven Million five hundred 
thousand United States Dollars). Everything concerning this transaction shall 
be legally done without hitch. Please endeavour to observe utmost discretion in 
all matters concerning this issue.

Once the funds have been successfully transferred into your account, we shall 
share in the ratio to be agreed by both of us.

I will prefer you reach me on my private email address:( peterl...@w.cn ) and 
finally after that I shall furnish you with more information’s about this 
operation.

Please if you are not interested delete this email and do not hunt me because I 
am putting my career and the life of my family at stake with this venture.

Kind Regards,
Mr. Peter Luk
--
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: [msysGit] [PATCH v2] Makefile: Fix compilation of Windows resource file

2014-01-23 Thread Johannes Sixt
Am 1/23/2014 13:02, schrieb Pat Thoyts:
 On 23 January 2014 07:28, Johannes Sixt j.s...@viscovery.net wrote:
 @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES

  git.res: git.rc GIT-VERSION-FILE
 $(QUIET_RC)$(RC) \
 - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, 
 ,$(subst ., ,$(GIT_VERSION) \
 + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., 
 ,$(GIT_VERSION) \
   -DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@

  ifndef NO_PERL

 This was put in as a response to
 https://github.com/msysgit/git/issues/5 where a request was made to be
 able to check the version without actually executing the file.

If I understand the request correctly, it is about manual inspection. The
correct version string for this purpose is recorded via -DGIT_VERSION.

 Given
 that the majority of versions has the same first two digits this
 becomes fairly useless without the patchlevel digit. So it would be
 preferable to try to maintain all three digits. The following should
 do this:
 
 GIT_VERSION=1.9.rc0
 all:
 echo $(join -DMAJOR= -DMINOR= -DPATCH=, \
 $(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst .,
 ,$(GIT_VERSION 0 0))
 
 This removes any rc* parts and appends a couple of zeros so that all
 missing elements should appear as 0 in the final list.

As Junio already pointed out, this records the wrong number in the 1.9
track before 1.9.1 is out because the third position is the commit count,
not the patch level.

-- 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 1/4] subtree: support split --rejoin --squash

2014-01-23 Thread Matthew Ogilvie
On Thu, Jan 23, 2014 at 09:51:49AM +0100, Pierre Penninckx wrote:
 Hi again,
 
 After using the patched git-subtree (with patches 1 to 3) for a while,
 I suspect the added functionality does not do exactly what I wanted.
 So yes, now when doing a rejoin, the squash of the split commits is
 used. But how can I push this squash instead of the individual
 commits? The problem is I don't know how to reference that squashed
 commit.
 
 I tried adding the --branch option but it adds the branch to the top
 of the individual commits so no luck there.
 This is maybe obvious but I'm not at ease with commit references in git.

Note that there are essentially two trees output by subtree --join.

The first output is the main branch (with --join).  With my
patches and --squash, the main branch merges in a squashed
representation of the subtree changes, so that the main
project history doesn't have two copies of potentially
tons of different commits in it's history (the
original and the subtree, shown merged together).

The second output is the new branch tip of the subtree itself.
My patch always outputs the full history of the subtree, not
a squashed representation.  This is what's different from your
patch, and is what I wanted.  If you want this subtree output
to ALSO be squashed, then it would need another option to
support this.

Note that there is at least one technical reason to prefer my
strategy.  git subtree tries to make it so you can
re-run it (potentially from scratch) on the main project at
any point in time, and re-generate exactly the same final
subtree history, regardless of previous runs of git subtree.
But if some of that history was originally squashed, it currently
has no way of knowing which commits should be squashed together
to properly regenerate exactly the same subtree history.
This is especially true if you use --ignore-joins, which
is currently the only practical workaround to the bug described
in my patch 4 (about merging in history that originally branched
off before the previous subtree split point).  Perhaps this
issue could be addressed by enhancing subtree to recognize
specially-formatted squash messages, and intentionally
regenerate the squashed based on them?

[Side note: I think the convention on this list is to respond
inline or after the previous message, not at the top, so new
people can more easily pick up the discussion.]

   - Matthew

 2014/1/23 Matthew Ogilvie mmogilvi_...@miniinfo.net:
  On Wed, Jan 22, 2014 at 03:58:28PM +0100, Pierre Penninckx wrote:
  2013/12/7 Matthew Ogilvie mmogilvi_...@miniinfo.net
   Subject: [PATCH 1/4] subtree: support split --rejoin --squash
  
   Allow using --squash with git subtree split --rejoin.  It
   will still split off (and save to --branch) the complete
   subtree history, but the merge done for the --rejoin will
   be merging a squashed representation of the new subtree
   commits, instead of the commits themselves (similar to
   how git subtree merge --squash works).
  
   Signed-off-by: Matthew Ogilvie mmogilvi_...@miniinfo.net
   ---
  
   I can think of a couple of possible objections to this patch.
   Are these (or any others) worth fixing?
  
   1. Perhaps someone want the saved subtree (--branch) to have
  a squashed representation as well, as an option?  Maybe we
  need two different --squash options?  Something
  like --rejoin-squash?
   2. It could definitely use some automated tests.  In fact,
  pre-existing --squash functionality is hardly tested at
  all, either.
 See patch 4 comments for a script I use to help with
  mostly-manual testing.
 
  Sorry to bother you with this again, but I was wondering if those patches
  would be integrated into git anytime soon.
  And if not, if there is something I can do to help.
 
  I found them by the way, thanks a lot!
 
  Pierre
 
  I'm not sure when or if the patches will make it in.  Junio's
  weekly What's cooking... email has asked for Comments? about
  them for the past several weeks, but I have yet to see
  anyone actually comment about them.
 
  Searching throught the last couple of years of mailing list
  archives for subtree reveals a general lack of a active
  maintainer(s) to help review and improve patches for git
  subtree.  Given the general lack of help and feedback, it is
  understandable that Junio has largely limited inclusion of
  subtree patches to trivially obvious bug fixes.
 
  - Matthew Ogilvie
--
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: [msysGit] [PATCH v2] Makefile: Fix compilation of Windows resource file

2014-01-23 Thread Pat Thoyts
On 23 January 2014 14:16, Johannes Sixt j.s...@viscovery.net wrote:
 Am 1/23/2014 13:02, schrieb Pat Thoyts:
 On 23 January 2014 07:28, Johannes Sixt j.s...@viscovery.net wrote:
 @@ -1773,7 +1773,7 @@ $(SCRIPT_LIB) : % : %.sh GIT-SCRIPT-DEFINES

  git.res: git.rc GIT-VERSION-FILE
 $(QUIET_RC)$(RC) \
 - $(join -DMAJOR= -DMINOR= -DPATCH=, $(wordlist 1,3,$(subst -, 
 ,$(subst ., ,$(GIT_VERSION) \
 + $(join -DMAJOR= -DMINOR=, $(wordlist 1,2,$(subst -, ,$(subst ., 
 ,$(GIT_VERSION) \
   -DGIT_VERSION=\\\$(GIT_VERSION)\\\ $ -o $@

  ifndef NO_PERL

 This was put in as a response to
 https://github.com/msysgit/git/issues/5 where a request was made to be
 able to check the version without actually executing the file.

 If I understand the request correctly, it is about manual inspection. The
 correct version string for this purpose is recorded via -DGIT_VERSION.

 Given
 that the majority of versions has the same first two digits this
 becomes fairly useless without the patchlevel digit. So it would be
 preferable to try to maintain all three digits. The following should
 do this:

 GIT_VERSION=1.9.rc0
 all:
 echo $(join -DMAJOR= -DMINOR= -DPATCH=, \
 $(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst .,
 ,$(GIT_VERSION 0 0))

 This removes any rc* parts and appends a couple of zeros so that all
 missing elements should appear as 0 in the final list.

 As Junio already pointed out, this records the wrong number in the 1.9
 track before 1.9.1 is out because the third position is the commit count,
 not the patch level.

 -- Hannes

OK - I cehcked and you are right in that the GIT_VERSION value is the
one showing up the properties dialog at least under Windows 7. As this
is the most likely to be examined I agree that just taking the first
two digits is the simplest fix here. So, fine by me then.

Acked-by: Pat Thoyts pattho...@users.sourceforge.net

Cheers,
Pat.
--
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 v3] git-svn: memoize _rev_list and rebuild

2014-01-23 Thread Junio C Hamano
Eric Wong normalper...@yhbt.net writes:

 Pushed for Junio.

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: [PATCHv2] gitk: Replace next and prev buttons with down and up arrows.

2014-01-23 Thread Junio C Hamano
Paul Mackerras pau...@samba.org writes:

 Yes, please pull.  I have just pushed one more:

   76d64ca gitk: Indent word-wrapped lines in commit display header

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 2/3] setup_pager: set MORE=R

2014-01-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 ..., but it feels awfully wrong to be so intimate with
 a subprogram that we do not control.

Yeah, I think we are in agreement on that point.
--
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: submodules

2014-01-23 Thread W. Trevor King
On Thu, Jan 23, 2014 at 03:38:49AM -0500, shawn wilson wrote:
 My issue is in trying to update the submodules, I'm getting:
  % git submodule update --init
 gits/kt (master ⚡)
 swlap1
 fatal: reference is not a tree: 98f1e9f99fca32ab3de901219eb2f600d38ab3a5
 Unable to checkout '98f1e9f99fca32ab3de901219eb2f600d38ab3a5' in
 submodule path 'repo_a'
 
 Ok, so a bit of googling and I found this:
 http://stackoverflow.com/questions/13425298/git-submodule-update-fatal-error-reference-is-not-a-tree
 Ok, so update the repo that contains the submodules every time you
 push from a sub repo? How / where do I create a hook to do this?

You've got it switched.  You *did* push the superproject, but forgot
to push the new submodule commits that it references.  An easy way to
avoid this problem is to always push the superproject using:

  $ git push --recurse-submodules=on-demand …

If you no longer have access to the submodule repositories that
weren't pushed, you'll have to roll back the superproject so it
references submodule commits that were pushed.  The easiest way to do
this is probably to just cd into the submodule directory and checkout
an appropriate commit (e.g. origin/master):

  $ cd repo_a
  $ git fetch origin
  $ git checkout origin/master

That will put you on a detached HEAD, so you might want to replace the
checkout with:

  $ git checkout -B master origin/master

or some such to get a local branch.

Then cd back into the superproject and commit the submodule
(referencing the current submodule commit):

  $ cd ..
  $ git commit -m 'repo_a: Roll back to last public commit' repo_a

When you push that commit, don't forget to push everything ;)

  $ git push --recurse-submodules=on-demand

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy


signature.asc
Description: OpenPGP digital signature


Re: Want to do some cleanups in this round of l10n

2014-01-23 Thread Junio C Hamano
Jiang Xin worldhello@gmail.com writes:

 Maybe three weeks left. You can estimate it by checking the date
 for history tags, such as v1.8.5-rc0 and v1.8.5-rc3.

 v1.8.5-rc0: Wed Oct 30 12:17:56 2013 -0700
 v1.8.5-rc3: Wed Nov 20 11:27:39 2013 -0800
 v1.8.5: Wed Nov 27 12:14:52 2013 -0800

Or please consult https://tinyurl.com/gitcal

 Yes, fuzz messages are not included. I double checked the history
 of da.po and nl.po, and I'm sure there are no further updates since
 their maintainers sent me Email like Hi, add me as the maintainer
 for that language, and I will send translations latter...

 Remove them can make the Git package smaller and give
 opportunities to other contributors.

Yeah, I think removing them is sane and fine.

--
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] Makefile: Fix compilation of Windows resource file

2014-01-23 Thread Junio C Hamano
Pat Thoyts pattho...@gmail.com writes:

 GIT_VERSION=1.9.rc0
 all:
 echo $(join -DMAJOR= -DMINOR= -DPATCH=, \
 $(wordlist 1,3,$(filter-out rc%,$(subst -, ,$(subst .,
 ,$(GIT_VERSION 0 0))

 This removes any rc* parts and appends a couple of zeros so that all
 missing elements should appear as 0 in the final list.

 As Junio already pointed out, this records the wrong number in the 1.9
 track before 1.9.1 is out because the third position is the commit count,
 not the patch level.

 -- Hannes

 OK - I cehcked and you are right in that the GIT_VERSION value is the
 one showing up the properties dialog at least under Windows 7. As this
 is the most likely to be examined I agree that just taking the first
 two digits is the simplest fix here. So, fine by me then.

 Acked-by: Pat Thoyts pattho...@users.sourceforge.net

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: [ANNOUNCE] Git v1.9-rc0

2014-01-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Junio, since you prepare such tarballs[1] anyway for kernel.org, it
 might be worth uploading them to the Releases page of git/git.  I
 imagine there is a programmatic way to do so via GitHub's API, but I
 don't know offhand. I can look into it if you are interested.

I already have a script that takes the three tarballs and uploads
them to two places, so adding GitHub as the third destination should
be a natural and welcome way to automate it.
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Wed, Jan 22, 2014 at 06:05:36PM -0800, Jonathan Nieder wrote:

 Jeff King wrote:
 
  EWAH is a word-aligned compressed variant of a bitset (i.e. a data
  structure that acts as a 0-indexed boolean array for many entries).
 
 I suspect that for some callers it's not word-aligned.

Yes, the mmap'd buffers aren't necessarily word-aligned. I don't think
we can fix that easily without changing the on-disk format (which comes
from JGit anyway). However, since we are memcpying the bulk of the data
into a newly allocated buffer (which must be aligned), we can do that
first, and then fix the endian-ness in place.

The only SPARC machine I have access to is running Solaris, but after
some slight wrestling with the BYTE_ORDER macros, I managed to get it to
compile and reproduced the bus error.

Here's a patch series (on top of jk/pack-bitmap, naturally) that lets
t5310 pass there. I assume the ARM problem is the same, though seeing
the failure in realloc() is unexpected. Can you try it on both your
platforms with these patches?

  [1/2]: compat: move unaligned helpers to bswap.h
  [2/2]: ewah: support platforms that require aligned reads

 Hopefully it's possible to get the alignment right in the caller
 and tweak the signature to require that instead of using unaligned
 reads like this.  There's still something wrong after this patch ---
 the new result is a NULL pointer dereference in t5310.7 enumerate
 --objects (full bitmap).

After my patches, t5310 runs fine for me. I didn't try your patch, but
mine are similar. Let me know if you still see the problem (there may
simply be a bug in yours, but I didn't see it).

-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


[PATCH 1/2] compat: move unaligned helpers to bswap.h

2014-01-23 Thread Jeff King
From: Vicent Marti tan...@gmail.com

Commit d60c49c (read-cache.c: allow unaligned mapping of the
index file, 2012-04-03) introduced helpers to access
unaligned data. Let's factor them out to make them more
widely available.

While we're at it, we'll give the helpers more readable
names, add a helper for the ntohll form, and add the
appropriate Makefile knob.

Signed-off-by: Vicent Marti tan...@gmail.com
Signed-off-by: Jeff King p...@peff.net
---
 Makefile   |  7 +++
 compat/bswap.h | 28 
 read-cache.c   | 44 
 3 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/Makefile b/Makefile
index 4136c4f..5711c0e 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,9 @@ all::
 # Define DEFAULT_HELP_FORMAT to man, info or html
 # (defaults to man) if you want to have a different default when
 # git help is called without a parameter specifying the format.
+#
+# Define NEEDS_ALIGNED_ACCESS if your platform cannot handle unaligned
+# access to integers in mmap'd files.
 
 GIT-VERSION-FILE: FORCE
@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -1505,6 +1508,10 @@ ifneq (,$(XDL_FAST_HASH))
BASIC_CFLAGS += -DXDL_FAST_HASH
 endif
 
+ifdef NEEDS_ALIGNED_ACCESS
+   BASIC_CFLAGS += -DNEEDS_ALIGNED_ACCESS
+endif
+
 ifeq ($(TCLTK_PATH),)
 NO_TCLTK = NoThanks
 endif
diff --git a/compat/bswap.h b/compat/bswap.h
index c18a78e..80abc54 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -122,3 +122,31 @@ static inline uint64_t git_bswap64(uint64_t x)
 #endif
 
 #endif
+
+#ifndef NEEDS_ALIGNED_ACCESS
+#define align_ntohs(var) ntohs(var)
+#define align_ntohl(var) ntohl(var)
+#define align_ntohll(var) ntohll(var)
+#else
+static inline uint16_t ntohs_force_align(void *p)
+{
+   uint16_t x;
+   memcpy(x, p, sizeof(x));
+   return ntohs(x);
+}
+static inline uint32_t ntohl_force_align(void *p)
+{
+   uint32_t x;
+   memcpy(x, p, sizeof(x));
+   return ntohl(x);
+}
+static inline uint64_t ntohll_force_align(void *p)
+{
+   uint64_t x;
+   memcpy(x, p, sizeof(x));
+   return ntohll(x);
+}
+#define align_ntohs(var) ntohs_force_align((var))
+#define align_ntohl(var) ntohl_force_align((var))
+#define align_ntohll(var) ntohll_force_align((var))
+#endif
diff --git a/read-cache.c b/read-cache.c
index 33dd676..fa53504 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1313,26 +1313,6 @@ int read_index(struct index_state *istate)
return read_index_from(istate, get_index_file());
 }
 
-#ifndef NEEDS_ALIGNED_ACCESS
-#define ntoh_s(var) ntohs(var)
-#define ntoh_l(var) ntohl(var)
-#else
-static inline uint16_t ntoh_s_force_align(void *p)
-{
-   uint16_t x;
-   memcpy(x, p, sizeof(x));
-   return ntohs(x);
-}
-static inline uint32_t ntoh_l_force_align(void *p)
-{
-   uint32_t x;
-   memcpy(x, p, sizeof(x));
-   return ntohl(x);
-}
-#define ntoh_s(var) ntoh_s_force_align((var))
-#define ntoh_l(var) ntoh_l_force_align((var))
-#endif
-
 static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry 
*ondisk,
   unsigned int flags,
   const char *name,
@@ -1340,16 +1320,16 @@ static struct cache_entry 
*cache_entry_from_ondisk(struct ondisk_cache_entry *on
 {
struct cache_entry *ce = xmalloc(cache_entry_size(len));
 
-   ce-ce_stat_data.sd_ctime.sec = ntoh_l(ondisk-ctime.sec);
-   ce-ce_stat_data.sd_mtime.sec = ntoh_l(ondisk-mtime.sec);
-   ce-ce_stat_data.sd_ctime.nsec = ntoh_l(ondisk-ctime.nsec);
-   ce-ce_stat_data.sd_mtime.nsec = ntoh_l(ondisk-mtime.nsec);
-   ce-ce_stat_data.sd_dev   = ntoh_l(ondisk-dev);
-   ce-ce_stat_data.sd_ino   = ntoh_l(ondisk-ino);
-   ce-ce_mode  = ntoh_l(ondisk-mode);
-   ce-ce_stat_data.sd_uid   = ntoh_l(ondisk-uid);
-   ce-ce_stat_data.sd_gid   = ntoh_l(ondisk-gid);
-   ce-ce_stat_data.sd_size  = ntoh_l(ondisk-size);
+   ce-ce_stat_data.sd_ctime.sec = align_ntohl(ondisk-ctime.sec);
+   ce-ce_stat_data.sd_mtime.sec = align_ntohl(ondisk-mtime.sec);
+   ce-ce_stat_data.sd_ctime.nsec = align_ntohl(ondisk-ctime.nsec);
+   ce-ce_stat_data.sd_mtime.nsec = align_ntohl(ondisk-mtime.nsec);
+   ce-ce_stat_data.sd_dev   = align_ntohl(ondisk-dev);
+   ce-ce_stat_data.sd_ino   = align_ntohl(ondisk-ino);
+   ce-ce_mode  = align_ntohl(ondisk-mode);
+   ce-ce_stat_data.sd_uid   = align_ntohl(ondisk-uid);
+   ce-ce_stat_data.sd_gid   = align_ntohl(ondisk-gid);
+   ce-ce_stat_data.sd_size  = align_ntohl(ondisk-size);
ce-ce_flags = flags  ~CE_NAMEMASK;
ce-ce_namelen = len;
hashcpy(ce-sha1, ondisk-sha1);
@@ -1389,14 +1369,14 @@ static struct cache_entry *create_from_disk(struct 
ondisk_cache_entry *ondisk,
unsigned int flags;
 
/* On-disk flags are just 16 bits */
-   flags = ntoh_s(ondisk-flags);
+   flags = align_ntohs(ondisk-flags);
   

[PATCH 2/2] ewah: support platforms that require aligned reads

2014-01-23 Thread Jeff King
From: Vicent Marti tan...@gmail.com

The caller may hand us an unaligned buffer (e.g., because it
is an mmap of a file with many ewah bitmaps). On some
platforms (like SPARC) this can cause a bus error. We can
fix it with a combination of force-align macros and moving
the data into an aligned buffer (which we would do anyway,
but we can move it before fixing the endianness).

Signed-off-by: Vicent Marti tan...@gmail.com
Signed-off-by: Jeff King p...@peff.net
---
 ewah/ewah_io.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c
index aed0da6..1948ba5 100644
--- a/ewah/ewah_io.c
+++ b/ewah/ewah_io.c
@@ -112,23 +112,38 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)
 
 int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)
 {
-   uint32_t *read32 = map;
-   eword_t *read64;
-   size_t i;
+   uint8_t *ptr = map;
+
+   self-bit_size = align_ntohl(*(uint32_t *)ptr);
+   ptr += sizeof(uint32_t);
+
+   self-buffer_size = self-alloc_size = align_ntohl(*(uint32_t *)ptr);
+   ptr += sizeof(uint32_t);
 
-   self-bit_size = ntohl(*read32++);
-   self-buffer_size = self-alloc_size = ntohl(*read32++);
self-buffer = ewah_realloc(self-buffer,
self-alloc_size * sizeof(eword_t));
 
if (!self-buffer)
return -1;
 
-   for (i = 0, read64 = (void *)read32; i  self-buffer_size; ++i)
-   self-buffer[i] = ntohll(*read64++);
+   /*
+* Copy the raw data for the bitmap as a whole chunk;
+* if we're in a little-endian platform, we'll perform
+* the endianness conversion in a separate pass to ensure
+* we're loading 8-byte aligned words.
+*/
+   memcpy(self-buffer, ptr, self-buffer_size * sizeof(uint64_t));
+   ptr += self-buffer_size * sizeof(uint64_t);
+
+#if __BYTE_ORDER != __BIG_ENDIAN
+   {
+   size_t i;
+   for (i = 0; i  self-buffer_size; ++i)
+   self-buffer[i] = ntohll(self-buffer[i]);
+   }
+#endif
 
-   read32 = (void *)read64;
-   self-rlw = self-buffer + ntohl(*read32++);
+   self-rlw = self-buffer + align_ntohl(*(uint32_t *)ptr);
 
return (3 * 4) + (self-buffer_size * 8);
 }
-- 
1.8.5.2.500.g8060133
--
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/2] repack: accept larger window-memory and max-pack-size

2014-01-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Wed, Jan 22, 2014 at 08:06:42PM -0500, Jeff King wrote:

 But I think there is a subtle problem. Here (and elsewhere) we use the
 parsed value of 0 as a sentinel. I think that is OK for
 --max-pack-size, where 0 is not a reasonable value. But git-repack(1)
 says:
 
   --window-memory=0 makes memory usage unlimited, which is the default.
 
 What does:
 
   git config pack.windowMemory 256m
   git repack --window-memory=0
 
 do? It should override the config, but I think it does not with your
 patch (nor with the current code). Using a string would fix that (though
 you could also fix it by using a different sentinel, like ULONG_MAX).

 Here is a series that does that (and fixes the other issue I found). It
 would probably be nice to test these things, but checking that they
 actually had an impact is tricky (how do you know that --window-memory
 did the right thing?).

   [1/3]: repack: fix typo in max-pack-size option
   [2/3]: repack: make parsed string options const-correct
   [3/3]: repack: propagate pack-objects options as strings

 -Peff

Sounds sensible; will chuck the hum-ulong series and replace with
these patches.

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 1/2] compat: move unaligned helpers to bswap.h

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

 Commit d60c49c (read-cache.c: allow unaligned mapping of the
 index file, 2012-04-03) introduced helpers to access
 unaligned data. Let's factor them out to make them more
 widely available.

 While we're at it, we'll give the helpers more readable
 names, add a helper for the ntohll form, and add the
 appropriate Makefile knob.

Weird.  Why wasn't git broken on the relevant platforms before (given
that no one has been setting NEEDS_ALIGNED_ACCESS for them)?

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


Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 So this relaxes the remote matching, and allows using the local:remote 
 syntax to say that the local branch is differently named from the remote 
 one.

 It is probably worth folding it into the previous patch if you think this 
 whole approach is workable.

Haven't thought enough to decide on that part, yet.

  # $3 must be a symbolic ref, a unique ref, or
 -# a SHA object expression
 +# a SHA object expression. It can also be of
 +# the format 'local-name:remote-name'.
  #
 -head=$(git symbolic-ref -q ${3-HEAD})
 -head=${head:-$(git show-ref ${3-HEAD} | cut -d' ' -f2)}
 -head=${head:-$(git rev-parse --quiet --verify $3)}
 +local=${3%:*}
 +local=${local:-HEAD}
 +remote=${3#*:}
 +head=$(git symbolic-ref -q $local)
 +head=${head:-$(git show-ref --heads --tags $local | cut -d' ' -f2)}
 +head=${head:-$(git rev-parse --quiet --verify $local)}
  
  # None of the above? Bad.
 -test -z $head  die fatal: Not a valid revision: $3
 +test -z $head  die fatal: Not a valid revision: $local
  
  # This also verifies that the resulting head is unique:

I am not sure if it is a good idea to hand-craft resulting head is
unique constraint here.  We already have disambiguation rules (and
warning mechanism) we use in other places---this part should use the
same rule, I think.

A short experiment tells me that we are almost there:

  $ git init  git commit --allow-empty -m initial
  $ git branch other  git tag master
  $ git -c core.warnambiguousrefs=true \
  rev-parse --symbolic-full-name other
  refs/heads/other
  $ git -c core.warnambiguousrefs=true \
  rev-parse --symbolic-full-name master; echo $?
  warning: refname 'master' is ambiguous.
  error: refname 'master' is ambiguous
  0

We say error but we do not actually error out, but that shouldn't
be too hard to fix.

  # git show-ref could have shown multiple matching refs..
  headrev=$(git rev-parse --verify --quiet $head^0)
 -test -z $headrev  die fatal: Ambiguous revision: $3
 +test -z $headrev  die fatal: Ambiguous revision: $local
  
  # Was it a branch with a description?
  branch_name=${head#refs/heads/}
 @@ -69,9 +73,6 @@ then
   branch_name=
  fi
  
 -prettyhead=${head#refs/}
 -prettyhead=${prettyhead#heads/}
 -
  merge_base=$(git merge-base $baserev $headrev) ||
  die fatal: No commits in common between $base and $head
  
 @@ -81,30 +82,37 @@ die fatal: No commits in common between $base and $head
  #
  # Otherwise find a random ref that matches $headrev.
  find_matching_ref='
 + my ($head,$headrev) = (@ARGV);
 + my ($found);
 +
   while (STDIN) {
 + chomp;
   my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
 + my ($pattern);
 + next unless ($sha1 eq $headrev);
 +
 + $pattern=/$head\$;

I think $head is constant inside the loop, so lift it outside?

 + if ($ref eq $head) {
 + $found = $ref;
 + }
 + if ($ref =~ /$pattern/) {
 + $found = $ref;
   }
 + if ($sha1 eq $head) {

I think this is $headrev ($head may be $remote or HEAD), but then
anything that does not point at $headrev has already been rejected
at the beginning of this loop, so...?

   $found = $sha1;
   }
   }
 + if ($found) {
   print $found\n;
   }
  '

I somehow feel that this is inadequate to catch the delayed
propagation error in the opposite direction.  The publish
repository may have an unrelated ref pointing at the $headrev and we
may guess that is the ref to be fetched by the integrator based on
that, but by the time the integrator fetches from the repository,
the ref may have been updated to its new value that does not match
$headrev.  But I do not think of a way to solve that one.

In any case, shouldn't we be catching duplicate matches here, if the
real objective is to make it less likely for the users to make
mistakes?  Otherwise, if there are two 'master' over there
(e.g. refs/heads/master and refs/remotes/origin/master), it seems to
me that $ref =~ /$pattern/ will trigger twice in your loop and ends
up reporting the last match.

In other words,

my (@found) = ();
my (@guessed) = ();
while (STDIN) {
next unless ($sha1 eq $headrev);
...
if ($ref =~ /$pattern/) {
push @found, $ref;
} else {
push @guessed, $ref;
}
}
if (@found == 1) {
print $found[0]\n;
} else if (@guessed == 1) {
print $guessed[0]\n;
}

or something like that?

 -ref=$(git ls-remote $url | @@PERL@@ -e $find_matching_ref $head 
 $headrev)
 +ref=$(git ls-remote $url | @@PERL@@ -e $find_matching_ref 
 ${remote:-HEAD} $headrev)

There are three cases as far as ${remote:-HEAD} aka $head in the
script is concerned:

 1. The 

Re: [PATCH 1/2] compat: move unaligned helpers to bswap.h

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 11:41:18AM -0800, Jonathan Nieder wrote:

 Jeff King wrote:
 
  Commit d60c49c (read-cache.c: allow unaligned mapping of the
  index file, 2012-04-03) introduced helpers to access
  unaligned data. Let's factor them out to make them more
  widely available.
 
  While we're at it, we'll give the helpers more readable
  names, add a helper for the ntohll form, and add the
  appropriate Makefile knob.
 
 Weird.  Why wasn't git broken on the relevant platforms before (given
 that no one has been setting NEEDS_ALIGNED_ACCESS for them)?

Because most of our data structures support aligned access. Thomas
mentioned this as a potential issue earlier, and I said in a re-roll
cover letter:

  I did not include the NEEDS_ALIGNED_ACCESS patch. I note that we do
  not even have a Makefile knob for this, and the code in read-cache.c
  has probably never actually been used. Are there real systems that
  have a problem? The read-cache code was in support of the index v4
  experiment, which did away with the 8-byte padding. So it could be
  that we simply don't see it, because everything is currently aligned.

I think it was a bug waiting to surface if index v4 ever got wide use.

-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 v4 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

 Here's a patch series (on top of jk/pack-bitmap, naturally) that lets
 t5310 pass there. I assume the ARM problem is the same, though seeing
 the failure in realloc() is unexpected. Can you try it on both your
 platforms with these patches?

Thanks.  Trying it out now.

[...]
 Hopefully it's possible to get the alignment right in the caller
 and tweak the signature to require that instead of using unaligned
 reads like this.  There's still something wrong after this patch ---
 the new result is a NULL pointer dereference in t5310.7 enumerate
 --objects (full bitmap).

 After my patches, t5310 runs fine for me. I didn't try your patch, but
 mine are similar. Let me know if you still see the problem (there may
 simply be a bug in yours, but I didn't see it).

I had left out a cast to unsigned, producing an overflow.

My main worry about the patches is that they will probably run into
an analagous problem to the one that v1.7.12-rc0~1^2~2 (block-sha1:
avoid pointer conversion that violates alignment constraints,
2012-07-22) solved.  By casting the pointer to (uint32_t *) we are
telling the compiler it is 32-bit aligned (C99 section 6.3.2.3).

Thanks,
Jonathan
--
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 0/2] solaris test fixups

2014-01-23 Thread Jeff King
Due to the alignment bug in another thread, I had the pleasure of
visiting my old friend Solaris 9 today. The tests _almost_ all run out
of the box.

This series features two minor fixes:

  [1/2]: t7501: fix empty commit test with NO_PERL
  [2/2]: t7700: do not use touch -r

I had a few other failures related to encodings; I suspect the problem
is simply that the machine in question doesn't have eucJP support at
all.

The big one that I did not fix is in t7001-mv. We do this:

  test_must_fail git mv some-file no-such-dir/

and assume that it will fail. It doesn't. Solaris happily renames
some-file to a regular file named no-such-dir. So we fail later during
the index-update, complaining about adding the entry no-such-dir/, but
still exit(0) at the end. I'm mostly willing to just call Solaris crazy
for allowing the rename (Linux returns ENOTDIR), but I do wonder if
the index codepath could be improved (and especially to return an
error).

-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


[PATCH 2/2] t7700: do not use touch -r

2014-01-23 Thread Jeff King
Some versions of touch (such as /usr/ucb/touch on Solaris)
do not know about the -r option. This would make sense as
a feature of test-chmtime, but fortunately this fix is even
easier. The test does not care about the timestamp of the
.keep file it creates at all, only that it exists. So we can
simply drop the use of -r.

Signed-off-by: Jeff King p...@peff.net
---
 t/t7700-repack.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index d954b84..f9f9014 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -17,7 +17,7 @@ test_expect_success 'objects in packs marked .keep are not 
repacked' '
# The second pack will contain the excluded object
packsha1=$(git rev-list --objects --all | grep file2 |
git pack-objects pack) 
-   touch -r pack-$packsha1.pack pack-$packsha1.keep 
+   touch pack-$packsha1.keep 
objsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |
sed -e s/^\([0-9a-f]\{40\}\).*/\1/) 
mv pack-* .git/objects/pack/ 
-- 
1.8.5.2.500.g8060133
--
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 1/2] t7501: fix empty commit test with NO_PERL

2014-01-23 Thread Jeff King
t7501.9 tries to check that git commit will fail when the
index is unchanged. It relies on previous tests not to have
modified the index. When it was originally written, this was
always the case. However, commit c65dc35 (t7501: test the
right kind of breakage, 2012-03-30) changed earlier tests (4
and 5) to leave a modification in the index.

We never noticed, however, because t7501.7, between the two,
clears the index state as a side effect. However, that test
depends on the PERL prerequisite, and so it does not always
run. Therefore if NO_PERL is set, we do not run the
intervening test, the index is left unclean, and t7501.9
fails.

We could fix this by moving t7501.9 up in the script.
However, this patch instead leaves it in place and adds a
git reset before the commit. This makes the test more
explicit about its preconditions, and will future-proof it
against any other changes in the test state.

Signed-off-by: Jeff King p...@peff.net
---
 t/t7501-commit.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index f04798f..94eec83 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -57,6 +57,7 @@ test_expect_success 'using invalid commit with -C' '
 '
 
 test_expect_success 'nothing to commit' '
+   git reset --hard 
test_must_fail git commit -m initial
 '
 
-- 
1.8.5.2.500.g8060133

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


Re: [PATCH 1/2] compat: move unaligned helpers to bswap.h

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

 I think it was a bug waiting to surface if index v4 ever got wide use.

Ah, ok.

In that case I think git-compat-util.h should include something like
what block-sha1/sha1.c has:

#if !defined(__i386__)  !defined(__x86_64__)  \
!defined(_M_IX86)  !defined(_M_X64)  \
!defined(__ppc__)  !defined(__ppc64__)  \
!defined(__powerpc__)  !defined(__powerpc64__)  \
!defined(__s390__)  !defined(__s390x__)
#define NEEDS_ALIGNED_ACCESS
#endif

Otherwise we are relying on the person building to know their own
architecture intimately, which shouldn't be necessary.

Meanwhile, as mentioned in the other message, I suspect the
NEEDS_ALIGNED_ACCESS code path is broken for aggressive compilers
anyway.  Looking more.

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


Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Linus Torvalds
On Thu, Jan 23, 2014 at 11:43 AM, Junio C Hamano gits...@pobox.com wrote:

 I am not sure if it is a good idea to hand-craft resulting head is
 unique constraint here.  We already have disambiguation rules (and
 warning mechanism) we use in other places---this part should use the
 same rule, I think.

If you can fix that, then yes, that would be lovely. As it is, I
couldn't find any easily scriptable way to do that.

  #
  # Otherwise find a random ref that matches $headrev.
  find_matching_ref='
 + my ($head,$headrev) = (@ARGV);
 + my ($found);
 +
   while (STDIN) {
 + chomp;
   my ($sha1, $ref, $deref) = /^(\S+)\s+([^^]+)(\S*)$/;
 + my ($pattern);
 + next unless ($sha1 eq $headrev);
 +
 + $pattern=/$head\$;

 I think $head is constant inside the loop, so lift it outside?

Yes. I'm not really a perl person, and this came from me trying to
make the code more readable (and it used to do that magic quoting
thing inside the loop, I just used a helper pattern variable).

 + if ($sha1 eq $head) {

 I think this is $headrev ($head may be $remote or HEAD), but then
 anything that does not point at $headrev has already been rejected
 at the beginning of this loop, so...?

No, this is for when head ends up not being a ref, but a SHA1 expression.

IOW, for when you do something odd like

git request-pull HEAD^^ origin HEAD^

when hacking things together. It doesn't actually generate the right
request-pull message (because there's no valid branch name), but it
*works* in the sense that you can get the diffstat etc and edit things
manually.

It's not a big deal - it has never really worked, and I actually
broke that when I then used $remote that doesn't actually have the
SHA1 any more.

 + if ($found) {
   print $found\n;
   }
  '

 I somehow feel that this is inadequate to catch the delayed
 propagation error in the opposite direction.  The publish
 repository may have an unrelated ref pointing at the $headrev and we
 may guess that is the ref to be fetched by the integrator based on
 that, but by the time the integrator fetches from the repository,
 the ref may have been updated to its new value that does not match
 $headrev.  But I do not think of a way to solve that one.

Yes, so you'll get a warning (or, if you get a partial match, maybe
not even that), but the important part about all these changes is that
it DOESN'T MATTER.

Why? Because it no longer re-writes the target branch name based on
that match or non-match. So the pull request will be fine.

In other words, the really fundamental change here i that the oops, I
couldn't find things on the remote no longer affects the output. It
only affects the warning. And I think that's important.

It used to be that the remote matching actually changed the output of
the request-pull, and *THAT* was the fundamental problem.

 In any case, shouldn't we be catching duplicate matches here, if the
 real objective is to make it less likely for the users to make
 mistakes?

It would be good, yes. But my perl-fu is weak, and I really didn't
want to worry about it. Also, as above: my primary issue was to not
screw up the output, so the remote matching actually has become much
less important, and now the warning about it is purely about being
helpful, it no longer fundamentally alters any semantics.

So I agree that there is room for improvement, but that's kind of
separate from the immediate problem I was trying to solve.

  Linus
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 11:52:06AM -0800, Jonathan Nieder wrote:

  After my patches, t5310 runs fine for me. I didn't try your patch, but
  mine are similar. Let me know if you still see the problem (there may
  simply be a bug in yours, but I didn't see it).
 
 I had left out a cast to unsigned, producing an overflow.
 
 My main worry about the patches is that they will probably run into
 an analagous problem to the one that v1.7.12-rc0~1^2~2 (block-sha1:
 avoid pointer conversion that violates alignment constraints,
 2012-07-22) solved.  By casting the pointer to (uint32_t *) we are
 telling the compiler it is 32-bit aligned (C99 section 6.3.2.3).

Yeah, maybe. We go via memcpy, which takes a void *, so that part is
good. However, the new code looks like:

  foo = align_ntohl(*(uint32_t *)ptr);

I think this probably works in practice because align_ntohl is inlined,
and any sane compiler will never actually load the variable. If we
change the signature of align_ntohl, we can do this:

  uint32_t align_ntohl(void *ptr)
  {
  uint32_t x;
  memcpy(x, ptr, sizeof(x));
  return ntohl(x);
  }

  ...

  foo = align_ntohl(ptr);

The memcpy solution is taken from read-cache.c, but as we noted, it
probably hasn't been used a lot. The blk_sha1 get_be may be faster, as
it converts as it reads. However, the bulk of the data is copied via
a single memcpy and then modified in place. I don't know if that would
be faster or not (for a big-endian system it probably is, since we can
omit the modification loop entirely).

-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 1/2] compat: move unaligned helpers to bswap.h

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 12:08:04PM -0800, Jonathan Nieder wrote:

 Jeff King wrote:
  On Thu, Jan 23, 2014 at 11:56:43AM -0800, Jonathan Nieder wrote:
 
  In that case I think git-compat-util.h should include something like
  what block-sha1/sha1.c has:
  
 #if !defined(__i386__)  !defined(__x86_64__)  \
 !defined(_M_IX86)  !defined(_M_X64)  \
 !defined(__ppc__)  !defined(__ppc64__)  \
 !defined(__powerpc__)  !defined(__powerpc64__)  \
 !defined(__s390__)  !defined(__s390x__)
 #define NEEDS_ALIGNED_ACCESS
 #endif
 
  Otherwise we are relying on the person building to know their own
  architecture intimately, which shouldn't be necessary.
 
  Yeah, I agree it would be nice to autodetect.
 
 The nice thing is that false positives are harmless, modulo slowing
 down git a little if the compiler doesn't figure out how to optimize
 the NEEDS_ALIGNED_ACCESS codepath when on an unlisted platform that
 doesn't, in fact, need aligned access.

OK, I'll refactor the knob.

-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 1/2] compat: move unaligned helpers to bswap.h

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:
 On Thu, Jan 23, 2014 at 11:56:43AM -0800, Jonathan Nieder wrote:

 In that case I think git-compat-util.h should include something like
 what block-sha1/sha1.c has:
 
  #if !defined(__i386__)  !defined(__x86_64__)  \
  !defined(_M_IX86)  !defined(_M_X64)  \
  !defined(__ppc__)  !defined(__ppc64__)  \
  !defined(__powerpc__)  !defined(__powerpc64__)  \
  !defined(__s390__)  !defined(__s390x__)
  #define NEEDS_ALIGNED_ACCESS
  #endif

 Otherwise we are relying on the person building to know their own
 architecture intimately, which shouldn't be necessary.

 Yeah, I agree it would be nice to autodetect.

The nice thing is that false positives are harmless, modulo slowing
down git a little if the compiler doesn't figure out how to optimize
the NEEDS_ALIGNED_ACCESS codepath when on an unlisted platform that
doesn't, in fact, need aligned access.

In other words, it would work out of the box for everybody.
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:
 On Thu, Jan 23, 2014 at 11:52:06AM -0800, Jonathan Nieder wrote:

 My main worry about the patches is that they will probably run into
 an analagous problem to the one that v1.7.12-rc0~1^2~2
[...]
 I think this probably works in practice because align_ntohl is inlined,
 and any sane compiler will never actually load the variable.

I don't think that's safe to rely on.  The example named above didn't
pose any problems except on one platform.  All the relevant functions
were static and easy to inline.  GCC just followed the standard
literally and chose to break by reading one word at a time, just like
in this case it could break e.g. by copying one word at a time in
__builtin_memcpy (which seems perfectly reasonable to me ---
optimization involves a lot of constraint solving, and if you can't
trust your constraints then there's not much you can do).
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Shawn Pearce
On Thu, Jan 23, 2014 at 10:33 AM, Jeff King p...@peff.net wrote:
 On Wed, Jan 22, 2014 at 06:05:36PM -0800, Jonathan Nieder wrote:

 Jeff King wrote:

  EWAH is a word-aligned compressed variant of a bitset (i.e. a data
  structure that acts as a 0-indexed boolean array for many entries).

 I suspect that for some callers it's not word-aligned.

 Yes, the mmap'd buffers aren't necessarily word-aligned. I don't think
 we can fix that easily without changing the on-disk format (which comes
 from JGit anyway).

Ouch, sorry about that. JGit doesn't mmap the file so we didn't think
about the impact of words not being aligned. I should have caught
that, but I didn't.
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 12:12:23PM -0800, Jonathan Nieder wrote:

 Jeff King wrote:
  On Thu, Jan 23, 2014 at 11:52:06AM -0800, Jonathan Nieder wrote:
 
  My main worry about the patches is that they will probably run into
  an analagous problem to the one that v1.7.12-rc0~1^2~2
 [...]
  I think this probably works in practice because align_ntohl is inlined,
  and any sane compiler will never actually load the variable.
 
 I don't think that's safe to rely on.  The example named above didn't
 pose any problems except on one platform.  All the relevant functions
 were static and easy to inline.  GCC just followed the standard
 literally and chose to break by reading one word at a time, just like
 in this case it could break e.g. by copying one word at a time in
 __builtin_memcpy (which seems perfectly reasonable to me ---
 optimization involves a lot of constraint solving, and if you can't
 trust your constraints then there's not much you can do).

I wasn't disagreeing with you. I was guessing at why it did not fail out
of the box when I tested it.  What do you think of the alternative I
posted?

-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 v4 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

   [1/2]: compat: move unaligned helpers to bswap.h
   [2/2]: ewah: support platforms that require aligned reads

After setting NEEDS_ALIGNED_ACCESS,
Tested-by: Jonathan Nieder jrnie...@gmail.com # ARMv5
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

  If we
 change the signature of align_ntohl, we can do this:

   uint32_t align_ntohl(void *ptr)
   {
   uint32_t x;
   memcpy(x, ptr, sizeof(x));
   return ntohl(x);
   }

   ...

   foo = align_ntohl(ptr);

 The memcpy solution is taken from read-cache.c, but as we noted, it
 probably hasn't been used a lot. The blk_sha1 get_be may be faster, as
 it converts as it reads.

I doubt there's much difference either way, especially after an
optimizer gets its hands on it.  According to [1] ARM has no fast
byte swap instruction so with -O0 the byte-at-a-time implementation is
probably faster there.  I can try a performance test if you like.

Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/125737
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 12:14:03PM -0800, Shawn Pearce wrote:

  Yes, the mmap'd buffers aren't necessarily word-aligned. I don't think
  we can fix that easily without changing the on-disk format (which comes
  from JGit anyway).
 
 Ouch, sorry about that. JGit doesn't mmap the file so we didn't think
 about the impact of words not being aligned. I should have caught
 that, but I didn't.

Looking over the format, I think the only thing preventing 4-byte
alignment is the 1-byte XOR-offset and 1-byte flags field for each
bitmap. If we ever have a v2, we could pad the sum of those out to 4
bytes. Is 4-byte alignment enough? We do treat the actual data as 64-bit
integers. I wonder if that would have problems on Sparc64, for example.

-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 v4 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 12:23:42PM -0800, Jonathan Nieder wrote:

  The memcpy solution is taken from read-cache.c, but as we noted, it
  probably hasn't been used a lot. The blk_sha1 get_be may be faster, as
  it converts as it reads.
 
 I doubt there's much difference either way, especially after an
 optimizer gets its hands on it.  According to [1] ARM has no fast
 byte swap instruction so with -O0 the byte-at-a-time implementation is
 probably faster there.  I can try a performance test if you like.

If you're curious and have time, go ahead and benchmark what I posted
against what you posted (with your fix). But you'll probably need a big
repo like the kernel to notice anything.

But I don't mind that much if we just use the memcpy trick for now. It's
nice and obvious, and we can always change it later if somebody has
numbers (I doubt it will be all that noticeable anyway; this isn't
nearly as tight a loop as the BLK_SHA1 code).

-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 v4 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 03:03:11PM -0500, Jeff King wrote:

  My main worry about the patches is that they will probably run into
  an analagous problem to the one that v1.7.12-rc0~1^2~2 (block-sha1:
  avoid pointer conversion that violates alignment constraints,
  2012-07-22) solved.  By casting the pointer to (uint32_t *) we are
  telling the compiler it is 32-bit aligned (C99 section 6.3.2.3).
 
 Yeah, maybe. We go via memcpy, which takes a void *, so that part is
 good. However, the new code looks like:
 
   foo = align_ntohl(*(uint32_t *)ptr);
 
 I think this probably works in practice because align_ntohl is inlined,
 and any sane compiler will never actually load the variable. If we
 change the signature of align_ntohl, we can do this:

Actually, it is a little trickier than that. We actually take the
address in the macro. So even without inlining, we end up casting to
void. I still think this:

   uint32_t align_ntohl(void *ptr)
   {
   uint32_t x;
   memcpy(x, ptr, sizeof(x));
   return ntohl(x);
   }

is a little more obvious, though. It does mean that everybody has to
pass a pointer, though, and on platforms where non-aligned reads are OK,
we do the cast ourselves. That means that:

  foo = align_ntohl(bar);

will not be able to do any type-checking for bar (say, when we are
pulling bar straight out of a packed struct). I don't know how much
we care.

-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/2] solaris test fixups

2014-01-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 and assume that it will fail. It doesn't. Solaris happily renames
 some-file to a regular file named no-such-dir. So we fail later during
 the index-update, complaining about adding the entry no-such-dir/, but
 still exit(0) at the end. I'm mostly willing to just call Solaris crazy
 for allowing the rename (Linux returns ENOTDIR), but I do wonder if
 the index codepath could be improved (and especially to return an
 error).

I think j6t has a patch for that, a8933469 (mv: let 'git mv file
no-such-dir/' error out on Windows, too, 2014-01-08).


--
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/2] solaris test fixups

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 12:52:30PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  and assume that it will fail. It doesn't. Solaris happily renames
  some-file to a regular file named no-such-dir. So we fail later during
  the index-update, complaining about adding the entry no-such-dir/, but
  still exit(0) at the end. I'm mostly willing to just call Solaris crazy
  for allowing the rename (Linux returns ENOTDIR), but I do wonder if
  the index codepath could be improved (and especially to return an
  error).
 
 I think j6t has a patch for that, a8933469 (mv: let 'git mv file
 no-such-dir/' error out on Windows, too, 2014-01-08).

Ah yeah, that looks like exactly the same issue (and the fix looks sane
from my cursory investigation). Thanks for the pointer. I wasn't
planning to look further into it, but now I can do so without feeling
guilty. :)

-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 2/2] t4010: match_pathspec_depth() and trailing slash after submodule

2014-01-23 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 When you do git diff HEAD submodule/, submodule from the index is
 picked out and match_pathspec_depth() in charge of matching it with
 the pathspec submodule/.

Is ... is called or something missing at the end of this sentence?

 Unlike tree_entry_interesting(), match_pathspec_depth() has no
 knowledge about entry mode to realize submodule is a directory and
 treat the trailing slash specially. And it does not have too, mostly,

s/too/to/, I think.

 because the index only contains files, not directories (not until
 submodules come)

 I have no solutions for it (no, stripping '/' at pathspec
 preprocessing phase seems like a workaround than a solution). So let's
 mark it. Maybe I or somebody else could revisit it later.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
  t/t4010-diff-pathspec.sh | 6 ++
  1 file changed, 6 insertions(+)

 diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
 index 15a4912..b54251a 100755
 --- a/t/t4010-diff-pathspec.sh
 +++ b/t/t4010-diff-pathspec.sh
 @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash on 
 submodule path' '
   test_cmp expect actual
  '
  
 +test_expect_failure 'diff-cache ignores trailing slash on submodule path' '
 + git diff --name-only HEAD^ submod expect 
 + git diff --name-only HEAD^ submod/ actual 

I actually doubt that the second line is expecting the right
behaviour in the first place.  As far as the top-level project is
concerned, submod is the name it wants, as there is nothing
underneath it.  Even if asked to recurse infinite levels, the caller
shouldn't be feeding paths like submod/a/b/c to
match_pathspec_depth() in the first place, no?



 + test_cmp expect actual
 +'
 +
  test_done
--
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] l10n: de.po: translate 27 new messages

2014-01-23 Thread Ralf Thielow
Translate 27 new messages came from git.pot update in
df49095 (l10n: git.pot: v1.9 round 1 (27 new, 11 removed).

Signed-off-by: Ralf Thielow ralf.thie...@gmail.com
---
 po/de.po | 90 
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/po/de.po b/po/de.po
index 9c03238..e771efd 100644
--- a/po/de.po
+++ b/po/de.po
@@ -442,9 +442,9 @@ msgstr[0] vor %lu Jahr
 msgstr[1] vor %lu Jahren
 
 #: diffcore-order.c:24
-#, fuzzy, c-format
+#, c-format
 msgid failed to read orderfile '%s'
-msgstr Fehler beim Lesen des Objektes '%s'.
+msgstr Fehler beim Lesen der Reihenfolgedatei '%s'.
 
 #: diff.c:113
 #, c-format
@@ -975,21 +975,23 @@ msgid 
 There is nothing to exclude from by :(exclude) patterns.\n
 Perhaps you forgot to add either ':/' or '.' ?
 msgstr 
+Es gibt nichts, was durch :(exclude) Muster ausgeschlossen wird.\n
+Vielleicht haben Sie vergessen entweder ':/' oder '.' hinzuzufügen?
 
 #: remote.c:753
-#, fuzzy, c-format
+#, c-format
 msgid Cannot fetch both %s and %s to %s
-msgstr Kann keine Commit-Beschreibung für %s bekommen
+msgstr Kann 'fetch' nicht für %s und %s nach %s ausführen.
 
 #: remote.c:757
 #, c-format
 msgid %s usually tracks %s, not %s
-msgstr 
+msgstr %s folgt üblicherweise %s, nicht %s
 
 #: remote.c:761
 #, c-format
 msgid %s tracks both %s and %s
-msgstr 
+msgstr %s folgt %s und %s
 
 #.
 #. * This last possibility doesn't occur because
@@ -997,9 +999,8 @@ msgstr 
 #. * the end of the list.
 #.
 #: remote.c:769
-#, fuzzy
 msgid Internal error
-msgstr interner Fehler
+msgstr Interner Fehler
 
 #: remote.c:1871
 #, c-format
@@ -1575,33 +1576,28 @@ msgid both modified:
 msgstr von beiden geändert:
 
 #: wt-status.c:275
-#, fuzzy
 msgid new file
-msgstr neue Datei:   %s
+msgstr neue Datei:
 
 #: wt-status.c:277
 msgid copied
-msgstr 
+msgstr kopiert
 
 #: wt-status.c:279
-#, fuzzy
 msgid deleted
 msgstr gelöscht
 
 #: wt-status.c:285
-#, fuzzy
 msgid typechange
-msgstr Typänderung: %s
+msgstr Typänderung:
 
 #: wt-status.c:287
-#, fuzzy
 msgid unknown
-msgstr unbekannt:%s
+msgstr unbekannt:
 
 #: wt-status.c:289
-#, fuzzy
 msgid unmerged
-msgstr zusammenführen
+msgstr nicht zusammengeführt
 
 #: wt-status.c:336
 msgid new commits, 
@@ -1633,6 +1629,8 @@ msgid 
 Do not touch the line above.\n
 Everything below will be removed.
 msgstr 
+Ändern Sie nicht die Zeile oberhalb.\n
+Alles unterhalb wird entfernt.
 
 #: wt-status.c:899
 msgid You have unmerged paths.
@@ -3991,14 +3989,14 @@ msgid reference repository '%s' is not a local 
repository.
 msgstr Referenziertes Repository '%s' ist kein lokales Repository.
 
 #: builtin/clone.c:256
-#, fuzzy, c-format
+#, c-format
 msgid reference repository '%s' is shallow
-msgstr Referenziertes Repository '%s' ist kein lokales Repository.
+msgstr Referenziertes Repository '%s' ist unvollständig (shallow).
 
 #: builtin/clone.c:259
-#, fuzzy, c-format
+#, c-format
 msgid reference repository '%s' is grafted
-msgstr referenziert Repository
+msgstr Referenziertes Repository '%s' ist gesondert eingehängt (graft).
 
 #: builtin/clone.c:321
 #, c-format
@@ -4099,16 +4097,16 @@ msgstr 
 
 #: builtin/clone.c:805
 msgid source repository is shallow, ignoring --local
-msgstr 
+msgstr Quelle ist ein unvollständiges Repository (shallow), ignoriere --local
 
 #: builtin/clone.c:810
 msgid --local is ignored
 msgstr --local wird ignoriert
 
 #: builtin/clone.c:814 builtin/fetch.c:1119
-#, fuzzy, c-format
+#, c-format
 msgid depth %s is not a positive number
-msgstr Objekt %s ist kein Blob
+msgstr Tiefe %s ist keine positive Zahl
 
 #: builtin/clone.c:824
 #, c-format
@@ -5215,9 +5213,8 @@ msgid default mode for recursion
 msgstr Standard-Modus für Rekursion
 
 #: builtin/fetch.c:109
-#, fuzzy
 msgid accept refs that update .git/shallow
-msgstr Konnte aktualisierte .gitmodules-Datei nicht lesen
+msgstr akzeptiert Referenzen die .git/shallow aktualisieren
 
 #: builtin/fetch.c:347
 msgid Couldn't find remote ref HEAD
@@ -5287,7 +5284,8 @@ msgstr %s hat nicht alle erforderlichen Objekte 
gesendet\n
 #: builtin/fetch.c:579
 #, c-format
 msgid reject %s because shallow roots are not allowed to be updated
-msgstr 
+msgstr %s wurde zurückgewiesen, da Ursprungs-Commits von unvollständigen\n
+Repositories (shallow) nicht aktualisiert werden dürfen.
 
 #: builtin/fetch.c:667 builtin/fetch.c:750
 #, c-format
@@ -7181,9 +7179,8 @@ msgid git merge-base --is-ancestor commit commit
 msgstr git merge-base --is-ancestor Commit Commit
 
 #: builtin/merge-base.c:33
-#, fuzzy
 msgid git merge-base --fork-point ref [commit]
-msgstr git merge-base --is-ancestor Commit Commit
+msgstr git merge-base --fork-point Referenz [Commit]
 
 #: builtin/merge-base.c:214
 msgid output all common ancestors
@@ -7203,7 +7200,7 @@ msgstr ist der Erste ein Vorgänger-Commit von dem 
Anderen?
 
 #: builtin/merge-base.c:222
 msgid find where commit forked from reflog of ref
-msgstr 
+msgstr findet wo Commit von Reflog von Referenz 

Re: [PATCH 2/2] t7700: do not use touch -r

2014-01-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 ... The test does not care about the timestamp of the
 .keep file it creates at all, only that it exists.

Please refrain from using touch for such use cases in the first
place.  It appears that

pack-$packsha1.keep 

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


Re: [PATCH 2/2] t7700: do not use touch -r

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 01:12:55PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  ... The test does not care about the timestamp of the
  .keep file it creates at all, only that it exists.
 
 Please refrain from using touch for such use cases in the first
 place.  It appears that
 
   pack-$packsha1.keep 
 
 is what the test wants.

Agreed. I was making the minimal change, but I think there is no reason
not to fix both while we are there. Do you want to just mark up the
patch in transit?

-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


[PATCH v2 0/3] unaligned reads from .bitmap files

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 01:33:20PM -0500, Jeff King wrote:

 Here's a patch series (on top of jk/pack-bitmap, naturally) that lets
 t5310 pass there. I assume the ARM problem is the same, though seeing
 the failure in realloc() is unexpected. Can you try it on both your
 platforms with these patches?
 
   [1/2]: compat: move unaligned helpers to bswap.h
   [2/2]: ewah: support platforms that require aligned reads

Here it is again, fixing the issues we've discussed.

Instead of building on the code in read-cache, it pulls the much more
battle-tested code from block-sha1, and refactors read-cache to use that
instead. So the fix now kicks in automatically, and in theory it is a
slight bit faster (though I still doubt it would even be measurable in
this case).

  [1/3]: block-sha1: factor out get_be and put_be wrappers
  [2/3]: read-cache: use get_be32 instead of hand-rolled ntoh_l
  [3/3]: ewah: support platforms that require aligned reads

-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


[PATCH 1/3] block-sha1: factor out get_be and put_be wrappers

2014-01-23 Thread Jeff King
The BLK_SHA1 code has optimized wrappers for doing endian
conversions on memory that may not be aligned. Let's pull
them out so that we can use them elsewhere, especially the
time-tested list of platforms that prefer each strategy.

Signed-off-by: Jeff King p...@peff.net
---
These short names might not be descriptive enough now that they are
globals. However, they make sense to me. I'm open to suggestions if
somebody disagrees.

 block-sha1/sha1.c | 32 
 compat/bswap.h| 32 
 2 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
index e1a1eb6..22b125c 100644
--- a/block-sha1/sha1.c
+++ b/block-sha1/sha1.c
@@ -62,38 +62,6 @@
   #define setW(x, val) (W(x) = (val))
 #endif
 
-/*
- * Performance might be improved if the CPU architecture is OK with
- * unaligned 32-bit loads and a fast ntohl() is available.
- * Otherwise fall back to byte loads and shifts which is portable,
- * and is faster on architectures with memory alignment issues.
- */
-
-#if defined(__i386__) || defined(__x86_64__) || \
-defined(_M_IX86) || defined(_M_X64) || \
-defined(__ppc__) || defined(__ppc64__) || \
-defined(__powerpc__) || defined(__powerpc64__) || \
-defined(__s390__) || defined(__s390x__)
-
-#define get_be32(p)ntohl(*(unsigned int *)(p))
-#define put_be32(p, v) do { *(unsigned int *)(p) = htonl(v); } while (0)
-
-#else
-
-#define get_be32(p)( \
-   (*((unsigned char *)(p) + 0)  24) | \
-   (*((unsigned char *)(p) + 1)  16) | \
-   (*((unsigned char *)(p) + 2)   8) | \
-   (*((unsigned char *)(p) + 3)   0) )
-#define put_be32(p, v) do { \
-   unsigned int __v = (v); \
-   *((unsigned char *)(p) + 0) = __v  24; \
-   *((unsigned char *)(p) + 1) = __v  16; \
-   *((unsigned char *)(p) + 2) = __v   8; \
-   *((unsigned char *)(p) + 3) = __v   0; } while (0)
-
-#endif
-
 /* This rolls over the 512-bit array */
 #define W(x) (array[(x)15])
 
diff --git a/compat/bswap.h b/compat/bswap.h
index c18a78e..7d17953 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -122,3 +122,35 @@ static inline uint64_t git_bswap64(uint64_t x)
 #endif
 
 #endif
+
+/*
+ * Performance might be improved if the CPU architecture is OK with
+ * unaligned 32-bit loads and a fast ntohl() is available.
+ * Otherwise fall back to byte loads and shifts which is portable,
+ * and is faster on architectures with memory alignment issues.
+ */
+
+#if defined(__i386__) || defined(__x86_64__) || \
+defined(_M_IX86) || defined(_M_X64) || \
+defined(__ppc__) || defined(__ppc64__) || \
+defined(__powerpc__) || defined(__powerpc64__) || \
+defined(__s390__) || defined(__s390x__)
+
+#define get_be32(p)ntohl(*(unsigned int *)(p))
+#define put_be32(p, v) do { *(unsigned int *)(p) = htonl(v); } while (0)
+
+#else
+
+#define get_be32(p)( \
+   (*((unsigned char *)(p) + 0)  24) | \
+   (*((unsigned char *)(p) + 1)  16) | \
+   (*((unsigned char *)(p) + 2)   8) | \
+   (*((unsigned char *)(p) + 3)   0) )
+#define put_be32(p, v) do { \
+   unsigned int __v = (v); \
+   *((unsigned char *)(p) + 0) = __v  24; \
+   *((unsigned char *)(p) + 1) = __v  16; \
+   *((unsigned char *)(p) + 2) = __v   8; \
+   *((unsigned char *)(p) + 3) = __v   0; } while (0)
+
+#endif
-- 
1.8.5.2.500.g8060133

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


Re: [PATCH 2/2] t7700: do not use touch -r

2014-01-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 Agreed. I was making the minimal change, but I think there is no reason
 not to fix both while we are there. Do you want to just mark up the
 patch in transit?

Let's just queue this instead.

-- 8 --
From: Jeff King p...@peff.net
Date: Thu, 23 Jan 2014 14:55:18 -0500
Subject: [PATCH] t7700: do not use touch unnecessarily

Some versions of touch (such as /usr/ucb/touch on Solaris)
do not know about the -r option. This would make sense as
a feature of test-chmtime, but fortunately this fix is even
easier.

The test does not care about the timestamp of the .keep file it
creates at all, only that it exists. For such a use case, with or
without portability issues around -r, touch should not be used
in the first place.

Signed-off-by: Jeff King p...@peff.net
Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t7700-repack.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7700-repack.sh b/t/t7700-repack.sh
index d954b84..b45bd1e 100755
--- a/t/t7700-repack.sh
+++ b/t/t7700-repack.sh
@@ -17,7 +17,7 @@ test_expect_success 'objects in packs marked .keep are not 
repacked' '
# The second pack will contain the excluded object
packsha1=$(git rev-list --objects --all | grep file2 |
git pack-objects pack) 
-   touch -r pack-$packsha1.pack pack-$packsha1.keep 
+   pack-$packsha1.keep 
objsha1=$(git verify-pack -v pack-$packsha1.idx | head -n 1 |
sed -e s/^\([0-9a-f]\{40\}\).*/\1/) 
mv pack-* .git/objects/pack/ 
-- 
1.9-rc0-234-g8e6341d

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


Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule

2014-01-23 Thread Jens Lehmann
Am 23.01.2014 22:09, schrieb Junio C Hamano:
 Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:
 diff --git a/t/t4010-diff-pathspec.sh b/t/t4010-diff-pathspec.sh
 index 15a4912..b54251a 100755
 --- a/t/t4010-diff-pathspec.sh
 +++ b/t/t4010-diff-pathspec.sh
 @@ -127,4 +127,10 @@ test_expect_success 'diff-tree ignores trailing slash 
 on submodule path' '
  test_cmp expect actual
  '
  
 +test_expect_failure 'diff-cache ignores trailing slash on submodule path' '
 +git diff --name-only HEAD^ submod expect 
 +git diff --name-only HEAD^ submod/ actual 
 
 I actually doubt that the second line is expecting the right
 behaviour in the first place.  As far as the top-level project is
 concerned, submod is the name it wants, as there is nothing
 underneath it.  Even if asked to recurse infinite levels, the caller
 shouldn't be feeding paths like submod/a/b/c to
 match_pathspec_depth() in the first place, no?

Agreed, submod/a/b/c would not make sense here. But a single
trailing '/' does mark submod as a directory, which I think is
ok for a submodule. And it makes life easier for the user if we
accept that, as shell completion will add it there automatically.
--
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 2/3] read-cache: use get_be32 instead of hand-rolled ntoh_l

2014-01-23 Thread Jeff King
Commit d60c49c (read-cache.c: allow unaligned mapping of the
index file, 2012-04-03) introduced helpers to access
unaligned data. However, we already have get_be32, which has
a few advantages:

  1. It's already written, so we avoid duplication.

  2. It's probably faster, since it does the endian
 conversion and the alignment fix at the same time.

  3. The get_be32 code is well-tested, having been in
 block-sha1 for a long time. By contrast, our custom
 helpers were probably almost never used, since the user
 needed to manually define a macro to enable them.

We have to add a get_be16 implementation to the existing
get_be32, but that is very simple to do.

Signed-off-by: Jeff King p...@peff.net
---
This _might_ still suffer from the issue fixed in 5f6a112 (block-sha1:
avoid pointer conversion that violates alignment constraints,
2012-07-22), as we are taking the pointer of a uint32 in a struct. But
if that is the case, then the original did, as well. It's not clear to
me if the casting get_be32 does is sufficient, or if a sufficiently
clever compiler might make assumptions based on the original pointer
type.

I'm inclined to leave it for now, as we haven't made anything worse, and
nobody has reported a problem.

 compat/bswap.h |  4 
 read-cache.c   | 44 
 2 files changed, 16 insertions(+), 32 deletions(-)

diff --git a/compat/bswap.h b/compat/bswap.h
index 7d17953..120c6c1 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -136,11 +136,15 @@ static inline uint64_t git_bswap64(uint64_t x)
 defined(__powerpc__) || defined(__powerpc64__) || \
 defined(__s390__) || defined(__s390x__)
 
+#define get_be16(p)ntohs(*(unsigned short *)(p))
 #define get_be32(p)ntohl(*(unsigned int *)(p))
 #define put_be32(p, v) do { *(unsigned int *)(p) = htonl(v); } while (0)
 
 #else
 
+#define get_be16(p)( \
+   (*((unsigned char *)(p) + 0)  8) | \
+   (*((unsigned char *)(p) + 1)  0) )
 #define get_be32(p)( \
(*((unsigned char *)(p) + 0)  24) | \
(*((unsigned char *)(p) + 1)  16) | \
diff --git a/read-cache.c b/read-cache.c
index 33dd676..4221872 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1313,26 +1313,6 @@ int read_index(struct index_state *istate)
return read_index_from(istate, get_index_file());
 }
 
-#ifndef NEEDS_ALIGNED_ACCESS
-#define ntoh_s(var) ntohs(var)
-#define ntoh_l(var) ntohl(var)
-#else
-static inline uint16_t ntoh_s_force_align(void *p)
-{
-   uint16_t x;
-   memcpy(x, p, sizeof(x));
-   return ntohs(x);
-}
-static inline uint32_t ntoh_l_force_align(void *p)
-{
-   uint32_t x;
-   memcpy(x, p, sizeof(x));
-   return ntohl(x);
-}
-#define ntoh_s(var) ntoh_s_force_align((var))
-#define ntoh_l(var) ntoh_l_force_align((var))
-#endif
-
 static struct cache_entry *cache_entry_from_ondisk(struct ondisk_cache_entry 
*ondisk,
   unsigned int flags,
   const char *name,
@@ -1340,16 +1320,16 @@ static struct cache_entry 
*cache_entry_from_ondisk(struct ondisk_cache_entry *on
 {
struct cache_entry *ce = xmalloc(cache_entry_size(len));
 
-   ce-ce_stat_data.sd_ctime.sec = ntoh_l(ondisk-ctime.sec);
-   ce-ce_stat_data.sd_mtime.sec = ntoh_l(ondisk-mtime.sec);
-   ce-ce_stat_data.sd_ctime.nsec = ntoh_l(ondisk-ctime.nsec);
-   ce-ce_stat_data.sd_mtime.nsec = ntoh_l(ondisk-mtime.nsec);
-   ce-ce_stat_data.sd_dev   = ntoh_l(ondisk-dev);
-   ce-ce_stat_data.sd_ino   = ntoh_l(ondisk-ino);
-   ce-ce_mode  = ntoh_l(ondisk-mode);
-   ce-ce_stat_data.sd_uid   = ntoh_l(ondisk-uid);
-   ce-ce_stat_data.sd_gid   = ntoh_l(ondisk-gid);
-   ce-ce_stat_data.sd_size  = ntoh_l(ondisk-size);
+   ce-ce_stat_data.sd_ctime.sec = get_be32(ondisk-ctime.sec);
+   ce-ce_stat_data.sd_mtime.sec = get_be32(ondisk-mtime.sec);
+   ce-ce_stat_data.sd_ctime.nsec = get_be32(ondisk-ctime.nsec);
+   ce-ce_stat_data.sd_mtime.nsec = get_be32(ondisk-mtime.nsec);
+   ce-ce_stat_data.sd_dev   = get_be32(ondisk-dev);
+   ce-ce_stat_data.sd_ino   = get_be32(ondisk-ino);
+   ce-ce_mode  = get_be32(ondisk-mode);
+   ce-ce_stat_data.sd_uid   = get_be32(ondisk-uid);
+   ce-ce_stat_data.sd_gid   = get_be32(ondisk-gid);
+   ce-ce_stat_data.sd_size  = get_be32(ondisk-size);
ce-ce_flags = flags  ~CE_NAMEMASK;
ce-ce_namelen = len;
hashcpy(ce-sha1, ondisk-sha1);
@@ -1389,14 +1369,14 @@ static struct cache_entry *create_from_disk(struct 
ondisk_cache_entry *ondisk,
unsigned int flags;
 
/* On-disk flags are just 16 bits */
-   flags = ntoh_s(ondisk-flags);
+   flags = get_be16(ondisk-flags);
len = flags  CE_NAMEMASK;
 
if (flags  CE_EXTENDED) {
struct ondisk_cache_entry_extended *ondisk2;
int extended_flags;
ondisk2 = (struct 

[PATCH 3/3] ewah: support platforms that require aligned reads

2014-01-23 Thread Jeff King
From: Vicent Marti tan...@gmail.com

The caller may hand us an unaligned buffer (e.g., because it
is an mmap of a file with many ewah bitmaps). On some
platforms (like SPARC) this can cause a bus error. We can
fix it with a combination of get_be32 and moving the data
into an aligned buffer (which we would do anyway, but we can
move it before fixing the endianness).

Signed-off-by: Vicent Marti tan...@gmail.com
Signed-off-by: Jeff King p...@peff.net
---
Tested on the SPARC I have access to. Please double-check that it also
works fine on ARM.

 ewah/ewah_io.c | 33 -
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c
index aed0da6..4a7fae6 100644
--- a/ewah/ewah_io.c
+++ b/ewah/ewah_io.c
@@ -112,23 +112,38 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)
 
 int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)
 {
-   uint32_t *read32 = map;
-   eword_t *read64;
-   size_t i;
+   uint8_t *ptr = map;
+
+   self-bit_size = get_be32(ptr);
+   ptr += sizeof(uint32_t);
+
+   self-buffer_size = self-alloc_size = get_be32(ptr);
+   ptr += sizeof(uint32_t);
 
-   self-bit_size = ntohl(*read32++);
-   self-buffer_size = self-alloc_size = ntohl(*read32++);
self-buffer = ewah_realloc(self-buffer,
self-alloc_size * sizeof(eword_t));
 
if (!self-buffer)
return -1;
 
-   for (i = 0, read64 = (void *)read32; i  self-buffer_size; ++i)
-   self-buffer[i] = ntohll(*read64++);
+   /*
+* Copy the raw data for the bitmap as a whole chunk;
+* if we're in a little-endian platform, we'll perform
+* the endianness conversion in a separate pass to ensure
+* we're loading 8-byte aligned words.
+*/
+   memcpy(self-buffer, ptr, self-buffer_size * sizeof(uint64_t));
+   ptr += self-buffer_size * sizeof(uint64_t);
+
+#if __BYTE_ORDER != __BIG_ENDIAN
+   {
+   size_t i;
+   for (i = 0; i  self-buffer_size; ++i)
+   self-buffer[i] = ntohll(self-buffer[i]);
+   }
+#endif
 
-   read32 = (void *)read64;
-   self-rlw = self-buffer + ntohl(*read32++);
+   self-rlw = self-buffer + get_be32(ptr);
 
return (3 * 4) + (self-buffer_size * 8);
 }
-- 
1.8.5.2.500.g8060133
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] t7700: do not use touch -r

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 01:24:44PM -0800, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  Agreed. I was making the minimal change, but I think there is no reason
  not to fix both while we are there. Do you want to just mark up the
  patch in transit?
 
 Let's just queue this instead.

That's what I meant. :)

Thanks, looks good to me.

-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 2/2] t4010: match_pathspec_depth() and trailing slash after submodule

2014-01-23 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 ... But a single
 trailing '/' does mark submod as a directory, which I think is
 ok for a submodule. And it makes life easier for the user if we
 accept that, as shell completion will add it there automatically.

OK, that would be annoying.

Perhaps the completion is what is broken here, then?  I dunno, and
haven't thought things through.



--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread brian m. carlson
On Thu, Jan 23, 2014 at 03:26:45PM -0500, Jeff King wrote:
 Looking over the format, I think the only thing preventing 4-byte
 alignment is the 1-byte XOR-offset and 1-byte flags field for each
 bitmap. If we ever have a v2, we could pad the sum of those out to 4
 bytes. Is 4-byte alignment enough? We do treat the actual data as 64-bit
 integers. I wonder if that would have problems on Sparc64, for example.

Yes, it will.  SPARC requires all loads be naturally aligned (4-byte to
an address that's a multiple of 4, 8-byte to a multiple of 8, and so
on).  In general, architectures that do not support unaligned access
require natural alignment for all quantities.

Also, even on architectures where the kernel can fix these alignment
issues up, the cost of doing so is a two context switches (in and out of
the kernel), servicing the trap, two loads, some shifts and rotates, and
a kernel message, so many people disable alignment fixups.  I know it
made things extremely slow on Alpha.  ARM is even more fun since if you
don't take the trap, it loads the data rotated, so the load happens, it
just silently returns the wrong data.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature


Re: [PATCH v4 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 09:53:26PM +, brian m. carlson wrote:

 On Thu, Jan 23, 2014 at 03:26:45PM -0500, Jeff King wrote:
  Looking over the format, I think the only thing preventing 4-byte
  alignment is the 1-byte XOR-offset and 1-byte flags field for each
  bitmap. If we ever have a v2, we could pad the sum of those out to 4
  bytes. Is 4-byte alignment enough? We do treat the actual data as 64-bit
  integers. I wonder if that would have problems on Sparc64, for example.
 
 Yes, it will.  SPARC requires all loads be naturally aligned (4-byte to
 an address that's a multiple of 4, 8-byte to a multiple of 8, and so
 on).  In general, architectures that do not support unaligned access
 require natural alignment for all quantities.

In that case, I think we cannot even blame Shawn. The ewah serialization
format itself (which JGit inherited from the javaewah library) has 8
bytes of header and 4 bytes of trailer. So packed serialized ewahs
wouldn't be 8-byte aligned (though of course he could have added his own
padding to each when we have a sequence of them).

-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 v4 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:
 On Thu, Jan 23, 2014 at 09:53:26PM +, brian m. carlson wrote:

 Yes, it will.  SPARC requires all loads be naturally aligned (4-byte to
 an address that's a multiple of 4, 8-byte to a multiple of 8, and so
 on).  In general, architectures that do not support unaligned access
 require natural alignment for all quantities.

 In that case, I think we cannot even blame Shawn. The ewah serialization
 format itself (which JGit inherited from the javaewah library) has 8
 bytes of header and 4 bytes of trailer. So packed serialized ewahs
 wouldn't be 8-byte aligned

I don't think that's a big issue.  A pair of 4-byte reads would not be
too slow.

Even on x86, aligned reads are supposed to be faster than unaligned
reads (though I haven't looked at benchmarks recently).
--
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 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 02:17:55PM -0800, Jonathan Nieder wrote:

 Jeff King wrote:
  On Thu, Jan 23, 2014 at 09:53:26PM +, brian m. carlson wrote:
 
  Yes, it will.  SPARC requires all loads be naturally aligned (4-byte to
  an address that's a multiple of 4, 8-byte to a multiple of 8, and so
  on).  In general, architectures that do not support unaligned access
  require natural alignment for all quantities.
 
  In that case, I think we cannot even blame Shawn. The ewah serialization
  format itself (which JGit inherited from the javaewah library) has 8
  bytes of header and 4 bytes of trailer. So packed serialized ewahs
  wouldn't be 8-byte aligned
 
 I don't think that's a big issue.  A pair of 4-byte reads would not be
 too slow.

The header is actually two separate 4-byte values, so that's fine. But
between the header and trailer are a series of 8-byte data values, and
that is what we need the 8-byte alignment for. So the _first_ ewah's
data is 8-byte aligned, but then it offsets the alignment with a single
4-byte trailer. So the next ewah, if they are packed in a sequence, is
will have its data misaligned.

You could solve it by putting an empty 4-byte pad at the end of each
ewah (and of course making sure the first one is 8-byte aligned).

Anyway, this is all academic until we are designing bitmap v2, which I
do not plan on doing anytime soon.

-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 v4 08/23] ewah: compressed bitmap implementation

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:
 On Thu, Jan 23, 2014 at 02:17:55PM -0800, Jonathan Nieder wrote:

 I don't think that's a big issue.  A pair of 4-byte reads would not be
 too slow.

 The header is actually two separate 4-byte values, so that's fine. But
 between the header and trailer are a series of 8-byte data values, and
 that is what we need the 8-byte alignment for.

Sorry for the lack of clarity.  What I meant is that a 4-byte aligned
8-byte value can be read using a pair of 4-byte reads, which is less
of a performance issue than a completely unaligned value.

[...]
 Anyway, this is all academic until we are designing bitmap v2, which I
 do not plan on doing anytime soon.

Sure, fair enough. :)

Jonathan
--
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] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Jeff King
On Wed, Jan 22, 2014 at 06:38:57PM -0800, Siddharth Agarwal wrote:

 Running git-next, writing bitmap indexes fails if a keep file is
 present from an earlier pack.

Right, that's expected.

The bitmap format cannot represent objects that are not present in the
pack. So we cannot write a bitmap index if any object reachable from a
packed commit is omitted from the pack.

We could be nicer and downgrade it to a warning, though. The patch below
does that.

 In our case we have .keep files lying around from ages ago (possibly
 due to kill -9s run on the server).

We ran into that problem at GitHub, too. We just turn off
`--honor-pack-keep` during our repacks, as we never want them on anyway
(and we would prefer to ignore the .keep than to abort the bitmap).

 It also means that running repack -a with bitmap writing enabled on a
 repo becomes problematic if a fetch is run concurrently.

For the most part, no. The .keep file should generally only be set
during the period between indexing the pack and updating the refs (so
while checking connectivity and running hooks). But pack-objects starts
from the ref tips and walks backwards. Until they are updated, it will
not try to pack the objects in the .keep files, as nobody references
them. There are two loopholes, though:

  1. In some instances, a remote may send an object we already have
 (e.g., because it is a blob referenced in an old commit, but newly
 referenced again due to a revert; we do not do a full object
 difference during the protocol negotiation, for reasons of
 efficiency). If that is the case, we may omit it if pack-objects
 starts during the period that the .pack and .keep files exist.

  2. Once the fetch updates the refs, it removes the .keep file. But
 this isn't atomic. A repack which starts between the two may pick
 up the new ref values, but also see the .keep file.

These are both unlikely, but possible on a very busy repository. The
patch below will downgrade each to a warning, rather than aborting the
repack.

So this should just work out of the box with this patch.  But if bitmaps
are important to you (say, you are running a very busy site and want
to make sure you always have bitmaps turned on) and you do not otherwise
care about .keep files, you may want to disable them, too.

-Peff

-- 8 --
Subject: pack-objects: turn off bitmaps when skipping objects

The pack bitmap format requires that we have a single bit
for each object in the pack, and that each object's bitmap
represents its complete set of reachable objects. Therefore
we have no way to represent the bitmap of an object which
references objects outside the pack.

We notice this problem while generating the bitmaps, as we
try to find the offset of a particular object and realize
that we do not have it. In this case we die, and neither the
bitmap nor the pack is generated. This is correct, but
perhaps a little unfriendly. If you have bitmaps turned on
in the config, many repacks will fail which would otherwise
succeed. E.g., incremental repacks, repacks with -l when
you have alternates, .keep files.

Instead, this patch notices early that we are omitting some
objects from the pack and turns off bitmaps (with a
warning). Note that this is not strictly correct, as it's
possible that the object being omitted is not reachable from
any other object in the pack. In practice, this is almost
never the case, and there are two advantages to doing it
this way:

  1. The code is much simpler, as we do not have to cleanly
 abort the bitmap-generation process midway through.

  2. We do not waste time partially generating bitmaps only
 to find out that some object deep in the history is not
 being packed.

Signed-off-by: Jeff King p...@peff.net
---
I tried to keep the warning to an 80-character line without making it
too confusing. Suggestions welcome if it doesn't make sense to people.

 builtin/pack-objects.c  | 12 +++-
 t/t5310-pack-bitmaps.sh |  5 -
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8364fbd..76831d9 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1000,6 +1000,10 @@ static void create_object_entry(const unsigned char 
*sha1,
entry-no_try_delta = no_try_delta;
 }
 
+static const char no_closure_warning[] = N_(
+disabling bitmap writing, as some objects are not being packed
+);
+
 static int add_object_entry(const unsigned char *sha1, enum object_type type,
const char *name, int exclude)
 {
@@ -1010,8 +1014,14 @@ static int add_object_entry(const unsigned char *sha1, 
enum object_type type,
if (have_duplicate_entry(sha1, exclude, index_pos))
return 0;
 
-   if (!want_object_in_pack(sha1, exclude, found_pack, found_offset))
+   if (!want_object_in_pack(sha1, exclude, found_pack, found_offset)) {
+   /* The pack is missing an object, so it will not have 

Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Junio C Hamano
Linus Torvalds torva...@linux-foundation.org writes:

 Yes, so you'll get a warning (or, if you get a partial match, maybe
 not even that), but the important part about all these changes is that
 it DOESN'T MATTER.

 Why? Because it no longer re-writes the target branch name based on
 that match or non-match. So the pull request will be fine.

Will be fine, provided if they always use local:remote syntax, I'd
agree.

 In other words, the really fundamental change here i that the oops, I
 couldn't find things on the remote no longer affects the output. It
 only affects the warning. And I think that's important.

 It used to be that the remote matching actually changed the output of
 the request-pull, and *THAT* was the fundamental problem.

The fingers of users can be retrained to use the local:remote syntax
after we apologize for this change in the Release Notes, I see only
one issue (we seem to lose the message from the annotated/signed tag
when asking to pull it) remaining, after looking at what updates are
needed for t5150.

Thanks.

-- 8 --
Subject: [PATCH] pull-request: test updates

This illustrates behaviour changes that result from the recent
change by Linus.  Most shows good changes, but there may be
usability regressions:

 - The command continues to fail when the user forgot to push out
   before running the command, but the wording of the message has
   been slightly changed.

 - The command no longer guesses when asked to request the commit at
   the HEAD be pulled after pushing it to a branch 'for-upstream',
   even when that branch points at the correct commit.  The user
   must ask the command with the new master:for-upstream syntax.

 - The command no longer favours a tag that peels to the requested
   commit over a branch that points at the same commit.  This needs
   to be asked explicitly by specifying the tag object, not the
   commit.  But somehow this does not see to work (yet); somewhere
   the tag-ness of the requested ref seems to be lost.

The new behaviour needs to be documented in any case, but we need to
agree what the new behaviour should be before doing so, so...

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 t/t5150-request-pull.sh | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 1afa0d5..412ee4f 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -86,7 +86,7 @@ test_expect_success 'setup: two scripts for reading pull 
requests' '
s/[-0-9]\{10\} [:0-9]\{8\} [-+][0-9]\{4\}/DATE/g
s/[^ ].*/SUBJECT/g
s/  [^ ].* (DATE)/  SUBJECT (DATE)/g
-   s/for-upstream/BRANCH/g
+   s|tags/full|BRANCH|g
s/mnemonic.txt/FILENAME/g
s/^version [0-9]/VERSION/
/^ FILENAME | *[0-9]* [-+]*\$/ b diffstat
@@ -127,7 +127,7 @@ test_expect_success 'pull request when forgot to push' '
test_must_fail git request-pull initial $downstream_url \
2../err
) 
-   grep No branch of.*is at:\$ err 
+   grep No match for commit .* err 
grep Are you sure you pushed err
 
 '
@@ -141,7 +141,7 @@ test_expect_success 'pull request after push' '
git checkout initial 
git merge --ff-only master 
git push origin master:for-upstream 
-   git request-pull initial origin ../request
+   git request-pull initial origin master:for-upstream ../request
) 
sed -nf read-request.sed request digest 
cat digest 
@@ -160,7 +160,7 @@ test_expect_success 'pull request after push' '
 
 '
 
-test_expect_success 'request names an appropriate branch' '
+test_expect_success 'request asks HEAD to be pulled' '
 
rm -fr downstream.git 
git init --bare downstream.git 
@@ -179,11 +179,11 @@ test_expect_success 'request names an appropriate branch' 
'
read repository 
read branch
} digest 
-   test $branch = tags/full
+   test -z $branch
 
 '
 
-test_expect_success 'pull request format' '
+test_expect_failure 'pull request format' '
 
rm -fr downstream.git 
git init --bare downstream.git 
@@ -212,8 +212,8 @@ test_expect_success 'pull request format' '
cd local 
git checkout initial 
git merge --ff-only master 
-   git push origin master:for-upstream 
-   git request-pull initial $downstream_url ../request
+   git push origin tags/full 
+   git request-pull initial $downstream_url tags/full ../request
) 
request sed -nf fuzz.sed request.fuzzy 
test_i18ncmp expect request.fuzzy
@@ -229,7 +229,7 @@ test_expect_success 'request-pull ignores 
OPTIONS_KEEPDASHDASH poison' '
git checkout initial 
git merge --ff-only master 
git push origin master:for-upstream 
-

Re: [PATCH v2 0/3] unaligned reads from .bitmap files

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

 Here it is again, fixing the issues we've discussed.

Thanks!  Passes all tests.

Tested-by: Jonathan Nieder jrnie...@gmail.com # ARMv5
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/3] block-sha1: factor out get_be and put_be wrappers

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

 These short names might not be descriptive enough now that they are
 globals. However, they make sense to me.

Yeah, I think they're clear.  And they match the Linux kernel's
get_unaligned_be32() / put_unaligned_be32().
--
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/3] read-cache: use get_be32 instead of hand-rolled ntoh_l

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

 This _might_ still suffer from the issue fixed in 5f6a112 (block-sha1:
 avoid pointer conversion that violates alignment constraints,
 2012-07-22), as we are taking the pointer of a uint32 in a struct.

No conversion, so no issue there.

Line 1484 looks more problematic:

disk_ce = (struct ondisk_cache_entry *)((char *)mmap + 
src_offset);

In v4 indexes, src_offset doesn't have any particular alignment so
this conversion has undefined behavior.

Do you know if any tests exercise this code with paths that don't
have convenient length?

[...]
 I'm inclined to leave it for now, as we haven't made anything worse, and
 nobody has reported a problem.

Yeah, agreed.

Probably the simplest fix would be to take a char *, memcpy into a
new (aligned) buffer and then byteswap in place, but that's
orthogonal to this series.

Thanks,
Jonathan
--
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] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Siddharth Agarwal

On 01/23/2014 02:52 PM, Jeff King wrote:

Right, that's expected.

The bitmap format cannot represent objects that are not present in the
pack. So we cannot write a bitmap index if any object reachable from a
packed commit is omitted from the pack.

We could be nicer and downgrade it to a warning, though. The patch below
does that.


This makes sense.


In our case we have .keep files lying around from ages ago (possibly
due to kill -9s run on the server).

We ran into that problem at GitHub, too. We just turn off
`--honor-pack-keep` during our repacks, as we never want them on anyway
(and we would prefer to ignore the .keep than to abort the bitmap).


Yes, we'd prefer to do that too. How do you actually do this, though? I 
don't see a way to pass `--honor-pack-keep` (shouldn't I pass in its 
inverse?) down to `git-pack-objects`.



It also means that running repack -a with bitmap writing enabled on a
repo becomes problematic if a fetch is run concurrently.

For the most part, no. The .keep file should generally only be set
during the period between indexing the pack and updating the refs (so
while checking connectivity and running hooks). But pack-objects starts
from the ref tips and walks backwards. Until they are updated, it will
not try to pack the objects in the .keep files, as nobody references
them.


The worry is less certain objects not being packed and more the old 
packs being deleted by git repack, isn't it? From the man page for 
git-index-pack:


--keep
Before moving the index into its final destination create an empty .keep 
file for the associated pack file. This option is usually necessary with
--stdin to prevent a simultaneous git repack process from deleting the 
newly constructed pack and index before refs can be updated to use 
objects contained in the pack.


I could be misunderstanding things here, though. From the description in 
the man page it's not clear what the actual failure mode here is.



There are two loopholes, though:

   1. In some instances, a remote may send an object we already have
  (e.g., because it is a blob referenced in an old commit, but newly
  referenced again due to a revert; we do not do a full object
  difference during the protocol negotiation, for reasons of
  efficiency). If that is the case, we may omit it if pack-objects
  starts during the period that the .pack and .keep files exist.

   2. Once the fetch updates the refs, it removes the .keep file. But
  this isn't atomic. A repack which starts between the two may pick
  up the new ref values, but also see the .keep file.

These are both unlikely, but possible on a very busy repository. The
patch below will downgrade each to a warning, rather than aborting the
repack.

So this should just work out of the box with this patch.  But if bitmaps
are important to you (say, you are running a very busy site and want
to make sure you always have bitmaps turned on) and you do not otherwise
care about .keep files, you may want to disable them, too.


We need to make sure bitmaps are always turned on, but we need to be 
even more certain that pushes don't fail due to races.



-Peff

-- 8 --
Subject: pack-objects: turn off bitmaps when skipping objects

The pack bitmap format requires that we have a single bit
for each object in the pack, and that each object's bitmap
represents its complete set of reachable objects. Therefore
we have no way to represent the bitmap of an object which
references objects outside the pack.

We notice this problem while generating the bitmaps, as we
try to find the offset of a particular object and realize
that we do not have it. In this case we die, and neither the
bitmap nor the pack is generated. This is correct, but
perhaps a little unfriendly. If you have bitmaps turned on
in the config, many repacks will fail which would otherwise
succeed. E.g., incremental repacks, repacks with -l when
you have alternates, .keep files.

Instead, this patch notices early that we are omitting some
objects from the pack and turns off bitmaps (with a
warning). Note that this is not strictly correct, as it's
possible that the object being omitted is not reachable from
any other object in the pack. In practice, this is almost
never the case, and there are two advantages to doing it
this way:

   1. The code is much simpler, as we do not have to cleanly
  abort the bitmap-generation process midway through.

   2. We do not waste time partially generating bitmaps only
  to find out that some object deep in the history is not
  being packed.

Signed-off-by: Jeff King p...@peff.net
---
I tried to keep the warning to an 80-character line without making it
too confusing. Suggestions welcome if it doesn't make sense to people.

  builtin/pack-objects.c  | 12 +++-
  t/t5310-pack-bitmaps.sh |  5 -
  2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8364fbd..76831d9 100644
--- 

Re: [PATCH 3/3] ewah: support platforms that require aligned reads

2014-01-23 Thread Jonathan Nieder
Jeff King wrote:

 --- a/ewah/ewah_io.c
 +++ b/ewah/ewah_io.c
 @@ -112,23 +112,38 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)
[...]
 +#if __BYTE_ORDER != __BIG_ENDIAN

Is this portable?

On a platform without __BYTE_ORDER or __BIG_ENDIAN defined,
it is interpreted as

#if 0 != 0

which means that such platforms are assumed to be big endian.
Does Mingw define __BYTE_ORDER, for example?


 + {
 + size_t i;
 + for (i = 0; i  self-buffer_size; ++i)
 + self-buffer[i] = ntohll(self-buffer[i]);
 + }
 +#endif

It's tempting to guard with something like

if (ntohl(1) != 1) {
...
}

The optimizer can tell if this is true or false at compile time, so
it shouldn't slow anything down.

With that change,
Reviewed-by: Jonathan Nieder jrnie...@gmail.com

Thanks for the quick fix.

diff --git i/ewah/ewah_io.c w/ewah/ewah_io.c
index 4a7fae6..5a527a4 100644
--- i/ewah/ewah_io.c
+++ w/ewah/ewah_io.c
@@ -135,13 +135,11 @@ int ewah_read_mmap(struct ewah_bitmap *self, void *map, 
size_t len)
memcpy(self-buffer, ptr, self-buffer_size * sizeof(uint64_t));
ptr += self-buffer_size * sizeof(uint64_t);
 
-#if __BYTE_ORDER != __BIG_ENDIAN
-   {
+   if (ntohl(1) != 1) {
size_t i;
for (i = 0; i  self-buffer_size; ++i)
self-buffer[i] = ntohll(self-buffer[i]);
}
-#endif
 
self-rlw = self-buffer + get_be32(ptr);
 
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] ewah: support platforms that require aligned reads

2014-01-23 Thread Vicent Martí
On Fri, Jan 24, 2014 at 12:44 AM, Jonathan Nieder jrnie...@gmail.com wrote:
 --- a/ewah/ewah_io.c
 +++ b/ewah/ewah_io.c
 @@ -112,23 +112,38 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)
 [...]
 +#if __BYTE_ORDER != __BIG_ENDIAN

 Is this portable?

We explicitly set the __BYTE_ORDER macros in `compat/bswap.h`. In
fact, this preprocessor conditional is the same one that we use when
choosing what version of the `ntohl` macro to define, so that's why I
decided to use it here.
--
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] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Siddharth Agarwal

On 01/23/2014 03:45 PM, Siddharth Agarwal wrote:


The worry is less certain objects not being packed and more the old 
packs being deleted by git repack, isn't it? From the man page for 
git-index-pack:


This should probably be new pack and not old packs, I guess. Not 
knowing much about how this actually works, I'm assuming the scenario 
here is something like:


(1) git receive-pack receives a pack P.pack and writes it to disk
(2) git index-pack runs on P.pack
(3) git repack runs separately, finds pack P.pack with no refs pointing 
to it, and deletes it

(4) everything goes wrong

With a keep file, this would be averted because

(1) git receive-pack receives a pack P.pack and writes it to disk
(2) git index-pack writes a keep file for P.pack, called P.keep
(3) git repack runs separately, finds pack P.pack with a keep file, 
doesn't touch it
(4) git index-pack finishes, and something updates refs to point to 
P.pack and deletes P.keep

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


Re: [RFC PATCH 2/1] Make request-pull able to take a refspec of form local:remote

2014-01-23 Thread Linus Torvalds
On Thu, Jan 23, 2014 at 2:58 PM, Junio C Hamano gits...@pobox.com wrote:

 Will be fine, provided if they always use local:remote syntax, I'd
 agree.

Why? No sane user should actually need to use the local:remote syntax.

The normal situation should be that you create the correctly named
branch or tag locally, and then push it out under that name.

So I don't actually think anybody should need to be retrained, or
always use the local:remote syntax. The local:remote syntax exists
only for that special insane case where you used (the same)
local:remote syntax to push out a branch under a different name.

[ And yeah, maybe that behavior is more common than I think, but even
if it is, such behavior would always be among people who are *very*
aware of the whole local branch vs remote branch name is different
situation. ]

   Linus
--
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] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Vicent Martí
On Fri, Jan 24, 2014 at 12:45 AM, Siddharth Agarwal s...@fb.com wrote:
 Yes, we'd prefer to do that too. How do you actually do this, though? I
 don't see a way to pass `--honor-pack-keep` (shouldn't I pass in its
 inverse?) down to `git-pack-objects`.

We run with this patch in production, it may be of use to you:
https://gist.github.com/vmg/8589317

In fact, it may be worth upstreaming too. I'll kindly ask peff to do
it when he has a moment.

Apologies for not attaching the patch inline, the GMail web UI doesn't
mix well with patch workflow.

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


Re: [PATCH 3/3] ewah: support platforms that require aligned reads

2014-01-23 Thread Jonathan Nieder
Vicent Martí wrote:
 On Fri, Jan 24, 2014 at 12:44 AM, Jonathan Nieder jrnie...@gmail.com wrote:

 +#if __BYTE_ORDER != __BIG_ENDIAN

 Is this portable?

 We explicitly set the __BYTE_ORDER macros in `compat/bswap.h`. In
 fact, this preprocessor conditional is the same one that we use when
 choosing what version of the `ntohl` macro to define, so that's why I
 decided to use it here.

Ah, thanks.  Sorry I missed that.  So feel free to add my reviewed-by
to the patch without my tweak, too.
--
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 5/5] implement @{publish} shorthand

2014-01-23 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 In a triangular workflow, you may have a distinct
 @{upstream} that you pull changes from, but publish by
 default (if you typed git push) to a different remote (or
 a different branch on the remote). It may sometimes be
 useful to be able to quickly refer to that publishing point
 (e.g., to see which changes you have that have not yet been
 published).

 This patch introduces the branch@{publish} shorthand (or
 @{pu} to be even shorter). It refers to the tracking
 branch of the remote branch to which you would push if you
 were to push the named branch. That's a mouthful to explain,
 so here's an example:

   $ git checkout -b foo origin/master
   $ git config remote.pushdefault github
   $ git push

 Signed-off-by: Jeff King p...@peff.net
 ---

As there is no @{pu} in publish_mark() as far as I can see, and also
I found it is a bit unclear what the example in the last paragraph
wants to illustrate, I'll reword the above as the following before
merging it to 'next'.

This patch introduces the branch@{publish} shorthand that
refers to the tracking branch of the remote branch to which
you would push if you were to push the named branch.

That's a mouthful to explain, so here's an example:

  $ git checkout -b foo origin/master
  $ git config remote.pushdefault github
  $ git push

With this, foo@{upstream} and foo@{publish} would be origin/master
and github/foo, respectively (assuming that git fetch github is
configured to use refs/remotes/github/* remote-tracking branches).

 The implementation feels weird, like the where do we push to code
 should be factored out from somewhere else. I think what we're doing
 here is not _wrong_, but I don't like repeating what git push is doing
 elsewhere. And I just punt on simple as a result. :)

I think we can polish that in-tree.

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 v2 6/9] rebase: don't try to match -M option

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

The -M option does not exist in OPTIONS_SPEC, so there is no use to try
to find it.

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 git-rebase.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index d1835ba..3b55211 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -267,7 +267,7 @@ do
--no-fork-point)
fork_point=
;;
-   -M|-m)
+   -m)
do_merge=t
;;
-X)
-- 
1.9.rc0.1002.gd081c64.dirty

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


[PATCH v2 2/9] git-sh-setup.sh: add variable to use the stuck-long mode

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

If the variable $OPTIONS_STUCKLONG is not empty, then rev-parse
option parsing is done in --stuck-long mode.

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 contrib/examples/git-checkout.sh | 1 +
 contrib/examples/git-clean.sh| 1 +
 contrib/examples/git-clone.sh| 1 +
 contrib/examples/git-merge.sh| 1 +
 contrib/examples/git-repack.sh   | 1 +
 contrib/git-resurrect.sh | 1 +
 git-am.sh| 1 +
 git-instaweb.sh  | 1 +
 git-quiltimport.sh   | 1 +
 git-rebase.sh| 1 +
 git-request-pull.sh  | 1 +
 git-sh-setup.sh  | 2 ++
 12 files changed, 13 insertions(+)

diff --git a/contrib/examples/git-checkout.sh b/contrib/examples/git-checkout.sh
index 1a7689a..d7507ed 100755
--- a/contrib/examples/git-checkout.sh
+++ b/contrib/examples/git-checkout.sh
@@ -1,6 +1,7 @@
 #!/bin/sh
 
 OPTIONS_KEEPDASHDASH=t
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git-checkout [options] [branch] [paths...]
 --
diff --git a/contrib/examples/git-clean.sh b/contrib/examples/git-clean.sh
index 01c95e9..9d881cd 100755
--- a/contrib/examples/git-clean.sh
+++ b/contrib/examples/git-clean.sh
@@ -4,6 +4,7 @@
 #
 
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git-clean [options] paths...
 
diff --git a/contrib/examples/git-clone.sh b/contrib/examples/git-clone.sh
index 547228e..ea4757d 100755
--- a/contrib/examples/git-clone.sh
+++ b/contrib/examples/git-clone.sh
@@ -8,6 +8,7 @@
 # See git-sh-setup why.
 unset CDPATH
 
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git-clone [options] [--] repo [dir]
 --
diff --git a/contrib/examples/git-merge.sh b/contrib/examples/git-merge.sh
index a5e42a9..f0243d5 100755
--- a/contrib/examples/git-merge.sh
+++ b/contrib/examples/git-merge.sh
@@ -4,6 +4,7 @@
 #
 
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git merge [options] remote...
 git merge [options] msg HEAD remote
diff --git a/contrib/examples/git-repack.sh b/contrib/examples/git-repack.sh
index 7579331..fd5ced5 100755
--- a/contrib/examples/git-repack.sh
+++ b/contrib/examples/git-repack.sh
@@ -4,6 +4,7 @@
 #
 
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git repack [options]
 --
diff --git a/contrib/git-resurrect.sh b/contrib/git-resurrect.sh
index a4ed4c3..d7e97bb 100755
--- a/contrib/git-resurrect.sh
+++ b/contrib/git-resurrect.sh
@@ -10,6 +10,7 @@ is rather slow but allows you to resurrect other people's 
topic
 branches.
 
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git resurrect $USAGE
 --
diff --git a/git-am.sh b/git-am.sh
index bbea430..a3b6f98 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -4,6 +4,7 @@
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git am [options] [(mbox|Maildir)...]
 git am [options] (--continue | --skip | --abort)
diff --git a/git-instaweb.sh b/git-instaweb.sh
index e93a238..4aa3eb8 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -5,6 +5,7 @@
 
 PERL='@@PERL@@'
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git instaweb [options] (--start | --stop | --restart)
 --
diff --git a/git-quiltimport.sh b/git-quiltimport.sh
index 8e17525..167d79f 100755
--- a/git-quiltimport.sh
+++ b/git-quiltimport.sh
@@ -1,5 +1,6 @@
 #!/bin/sh
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git quiltimport [options]
 --
diff --git a/git-rebase.sh b/git-rebase.sh
index 8a3efa2..c1f98ae 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -5,6 +5,7 @@
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC=\
 git rebase [-i] [options] [--exec cmd] [--onto newbase] [upstream] 
[branch]
 git rebase [-i] [options] [--exec cmd] [--onto newbase] --root [branch]
diff --git a/git-request-pull.sh b/git-request-pull.sh
index fe21d5d..cf4f150 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -9,6 +9,7 @@ LONG_USAGE='Summarizes the changes between two commits to the 
standard output,
 and includes the given URL in the generated summary.'
 SUBDIRECTORY_OK='Yes'
 OPTIONS_KEEPDASHDASH=
+OPTIONS_STUCKLONG=
 OPTIONS_SPEC='git request-pull [options] start url [end]
 --
 pshow patch text as well
diff --git a/git-sh-setup.sh b/git-sh-setup.sh
index fffa3c7..5f28b32 100644
--- a/git-sh-setup.sh
+++ b/git-sh-setup.sh
@@ -72,6 +72,8 @@ if test -n $OPTIONS_SPEC; then
parseopt_extra=
[ -n $OPTIONS_KEEPDASHDASH ] 
parseopt_extra=--keep-dashdash
+   [ -n $OPTIONS_STUCKLONG ] 
+   parseopt_extra=$parseopt_extra --stuck-long
 
eval $(
echo $OPTIONS_SPEC |
-- 
1.9.rc0.1002.gd081c64.dirty

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


[PATCH v2 9/9] pull: add the --gpg-sign option.

2014-01-23 Thread brian m. carlson
git merge already allows us to sign commits, and git rebase has recently
learned how to do so as well.  Teach git pull to parse the -S/--gpg-sign
option and pass this along to merge or rebase, as appropriate.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 git-pull.sh | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/git-pull.sh b/git-pull.sh
index 0a5aa2c..4164dac 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -138,6 +138,15 @@ do
--no-verify-signatures)
verify_signatures=--no-verify-signatures
;;
+   --gpg-sign|-S)
+   gpg_sign_args=-S
+   ;;
+   --gpg-sign=*)
+   gpg_sign_args=-S${1#--gpg-sign=}
+   ;;
+   -S*)
+   gpg_sign_args=-S${1#-S}
+   ;;
--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
dry_run=--dry-run
;;
@@ -305,11 +314,13 @@ merge_name=$(git fmt-merge-msg $log_arg 
$GIT_DIR/FETCH_HEAD) || exit
 case $rebase in
 true)
eval=git-rebase $diffstat $strategy_args $merge_args $rebase_args 
$verbosity
+   eval=$eval $gpg_sign_args
eval=$eval --onto $merge_head ${oldremoteref:-$merge_head}
;;
 *)
eval=git-merge $diffstat $no_commit $verify_signatures $edit $squash 
$no_ff $ff_only
-   eval=$eval  $log_arg $strategy_args $merge_args $verbosity $progress
+   eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress
+   eval=$eval $gpg_sign_args
eval=$eval \\$merge_name\ HEAD $merge_head
;;
 esac
-- 
1.9.rc0.1002.gd081c64.dirty

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


[PATCH v2 5/9] rebase: remove useless arguments check

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

Remove a check on the number of arguments for --onto and -x options.
It is not possible for $# to be = 2 at this point :

 - if --onto or -x has an argument, git rev-parse --parseopt will
   provide something like this :
 set -- --onto 'x' --
   when parsing the --onto option, $# will be 3 or more if there are
   other options.

 - if --onto or -x doesn't have an argument, git rev-parse --parseopt
   will exit with an error and display usage information.

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 git-rebase.sh | 2 --
 1 file changed, 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index c1f98ae..d1835ba 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -238,12 +238,10 @@ do
action=${1##--}
;;
--onto)
-   test 2 -le $# || usage
onto=$2
shift
;;
-x)
-   test 2 -le $# || usage
cmd=${cmd}exec $2${LF}
shift
;;
-- 
1.9.rc0.1002.gd081c64.dirty

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


[PATCH v2 1/9] cherry-pick, revert: add the --gpg-sign option

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-cherry-pick.txt |  7 ++-
 Documentation/git-revert.txt  |  6 +-
 builtin/revert.c  |  2 ++
 sequencer.c   | 11 +++
 sequencer.h   |  2 ++
 5 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-cherry-pick.txt 
b/Documentation/git-cherry-pick.txt
index c205d23..f1e6b2f 100644
--- a/Documentation/git-cherry-pick.txt
+++ b/Documentation/git-cherry-pick.txt
@@ -8,7 +8,8 @@ git-cherry-pick - Apply the changes introduced by some existing 
commits
 SYNOPSIS
 
 [verse]
-'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff] commit...
+'git cherry-pick' [--edit] [-n] [-m parent-number] [-s] [-x] [--ff]
+ [-S[keyid]] commit...
 'git cherry-pick' --continue
 'git cherry-pick' --quit
 'git cherry-pick' --abort
@@ -100,6 +101,10 @@ effect to your index in a row.
 --signoff::
Add Signed-off-by line at the end of the commit message.
 
+-S[keyid]::
+--gpg-sign[=keyid]::
+   GPG-sign commits.
+
 --ff::
If the current HEAD is the same as the parent of the
cherry-pick'ed commit, then a fast forward to this commit will
diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt
index 2de67a5..9eb83f0 100644
--- a/Documentation/git-revert.txt
+++ b/Documentation/git-revert.txt
@@ -8,7 +8,7 @@ git-revert - Revert some existing commits
 SYNOPSIS
 
 [verse]
-'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] commit...
+'git revert' [--[no-]edit] [-n] [-m parent-number] [-s] [-S[keyid]] 
commit...
 'git revert' --continue
 'git revert' --quit
 'git revert' --abort
@@ -80,6 +80,10 @@ more details.
 This is useful when reverting more than one commits'
 effect to your index in a row.
 
+-S[keyid]::
+--gpg-sign[=keyid]::
+   GPG-sign commits.
+
 -s::
 --signoff::
Add Signed-off-by line at the end of the commit message.
diff --git a/builtin/revert.c b/builtin/revert.c
index 87659c9..065d88d 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -89,6 +89,8 @@ static void parse_args(int argc, const char **argv, struct 
replay_opts *opts)
OPT_STRING(0, strategy, opts-strategy, N_(strategy), 
N_(merge strategy)),
OPT_CALLBACK('X', strategy-option, opts, N_(option),
N_(option for merge strategy), option_parse_x),
+   { OPTION_STRING, 'S', gpg-sign, opts-gpg_sign, N_(key id),
+ N_(GPG sign commit), PARSE_OPT_OPTARG, NULL, (intptr_t)  
},
OPT_END(),
OPT_END(),
OPT_END(),
diff --git a/sequencer.c b/sequencer.c
index 90cac7b..bde5f04 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -392,11 +392,18 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
 {
struct argv_array array;
int rc;
+   char *gpg_sign;
 
argv_array_init(array);
argv_array_push(array, commit);
argv_array_push(array, -n);
 
+   if (opts-gpg_sign) {
+   gpg_sign = xmalloc(3 + strlen(opts-gpg_sign));
+   sprintf(gpg_sign, -S%s, opts-gpg_sign);
+   argv_array_push(array, gpg_sign);
+   free(gpg_sign);
+   }
if (opts-signoff)
argv_array_push(array, -s);
if (!opts-edit) {
@@ -808,6 +815,8 @@ static int populate_opts_cb(const char *key, const char 
*value, void *data)
opts-mainline = git_config_int(key, value);
else if (!strcmp(key, options.strategy))
git_config_string(opts-strategy, key, value);
+   else if (!strcmp(key, options.gpg-sign))
+   git_config_string(opts-gpg_sign, key, value);
else if (!strcmp(key, options.strategy-option)) {
ALLOC_GROW(opts-xopts, opts-xopts_nr + 1, opts-xopts_alloc);
opts-xopts[opts-xopts_nr++] = xstrdup(value);
@@ -981,6 +990,8 @@ static void save_opts(struct replay_opts *opts)
}
if (opts-strategy)
git_config_set_in_file(opts_file, options.strategy, 
opts-strategy);
+   if (opts-gpg_sign)
+   git_config_set_in_file(opts_file, options.gpg-sign, 
opts-gpg_sign);
if (opts-xopts) {
int i;
for (i = 0; i  opts-xopts_nr; i++)
diff --git a/sequencer.h b/sequencer.h
index 1fc22dc..db43e9c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -37,6 +37,8 @@ struct replay_opts {
 
int mainline;
 
+   const char *gpg_sign;
+
/* Merge strategy */
const char *strategy;
const char **xopts;
-- 
1.9.rc0.1002.gd081c64.dirty

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

[PATCH v2 7/9] rebase: parse options in stuck-long mode

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

There is no functionnal change. The reason for this change is to be able
to add a new option taking an optional argument.

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 git-rebase.sh | 50 ++
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 3b55211..842d7d4 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -5,7 +5,7 @@
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
-OPTIONS_STUCKLONG=
+OPTIONS_STUCKLONG=t
 OPTIONS_SPEC=\
 git rebase [-i] [options] [--exec cmd] [--onto newbase] [upstream] 
[branch]
 git rebase [-i] [options] [--exec cmd] [--onto newbase] --root [branch]
@@ -237,21 +237,19 @@ do
test $total_argc -eq 2 || usage
action=${1##--}
;;
-   --onto)
-   onto=$2
-   shift
+   --onto=*)
+   onto=${1#--onto=}
;;
-   -x)
-   cmd=${cmd}exec $2${LF}
-   shift
+   --exec=*)
+   cmd=${cmd}exec ${1#--exec=}${LF}
;;
-   -i)
+   --interactive)
interactive_rebase=explicit
;;
-   -k)
+   --keep-empty)
keep_empty=yes
;;
-   -p)
+   --preserve-merges)
preserve_merges=t
test -z $interactive_rebase  interactive_rebase=implied
;;
@@ -267,21 +265,19 @@ do
--no-fork-point)
fork_point=
;;
-   -m)
+   --merge)
do_merge=t
;;
-   -X)
-   shift
-   strategy_opts=$strategy_opts $(git rev-parse --sq-quote 
--$1)
+   --strategy-option=*)
+   strategy_opts=$strategy_opts $(git rev-parse --sq-quote 
--${1#--strategy-option=})
do_merge=t
test -z $strategy  strategy=recursive
;;
-   -s)
-   shift
-   strategy=$1
+   --strategy=*)
+   strategy=${1#--strategy=}
do_merge=t
;;
-   -n)
+   --no-stat)
diffstat=
;;
--stat)
@@ -290,21 +286,20 @@ do
--autostash)
autostash=true
;;
-   -v)
+   --verbose)
verbose=t
diffstat=t
GIT_QUIET=
;;
-   -q)
+   --quiet)
GIT_QUIET=t
git_am_opt=$git_am_opt -q
verbose=
diffstat=
;;
-   --whitespace)
-   shift
-   git_am_opt=$git_am_opt --whitespace=$1
-   case $1 in
+   --whitespace=*)
+   git_am_opt=$git_am_opt --whitespace=${1#--whitespace=}
+   case ${1#--whitespace=} in
fix|strip)
force_rebase=t
;;
@@ -317,14 +312,13 @@ do
git_am_opt=$git_am_opt $1
force_rebase=t
;;
-   -C)
-   shift
-   git_am_opt=$git_am_opt -C$1
+   -C*)
+   git_am_opt=$git_am_opt $1
;;
--root)
rebase_root=t
;;
-   -f|--no-ff)
+   --force-rebase|--no-ff)
force_rebase=t
;;
--rerere-autoupdate|--no-rerere-autoupdate)
-- 
1.9.rc0.1002.gd081c64.dirty

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


[PATCH v2 3/9] am: parse options in stuck-long mode

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

There is no functional change. The reason for this change is to be able
to add a new option taking an optional argument.

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 git-am.sh | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/git-am.sh b/git-am.sh
index a3b6f98..020abf6 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -4,7 +4,7 @@
 
 SUBDIRECTORY_OK=Yes
 OPTIONS_KEEPDASHDASH=
-OPTIONS_STUCKLONG=
+OPTIONS_STUCKLONG=t
 OPTIONS_SPEC=\
 git am [options] [(mbox|Maildir)...]
 git am [options] (--continue | --skip | --abort)
@@ -414,14 +414,14 @@ it will be removed. Please do not use it anymore.
abort=t ;;
--rebasing)
rebasing=t threeway=t ;;
-   --resolvemsg)
-   shift; resolvemsg=$1 ;;
-   --whitespace|--directory|--exclude|--include)
-   git_apply_opt=$git_apply_opt $(sq $1=$2); shift ;;
-   -C|-p)
-   git_apply_opt=$git_apply_opt $(sq $1$2); shift ;;
-   --patch-format)
-   shift ; patch_format=$1 ;;
+   --resolvemsg=*)
+   resolvemsg=${1#--resolvemsg=} ;;
+   --whitespace=*|--directory=*|--exclude=*|--include=*)
+   git_apply_opt=$git_apply_opt $(sq $1) ;;
+   -C*|-p*)
+   git_apply_opt=$git_apply_opt $(sq $1) ;;
+   --patch-format=*)
+   patch_format=${1#--patch-format=} ;;
--reject|--ignore-whitespace|--ignore-space-change)
git_apply_opt=$git_apply_opt $1 ;;
--committer-date-is-author-date)
-- 
1.9.rc0.1002.gd081c64.dirty

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


[PATCH v2 0/9] add --gpg-sign to rebase and pull

2014-01-23 Thread brian m. carlson
This series was posted to the list some time back, but it fell through
the cracks.  This is a re-send of Nicolas Vigier's work with an
additional patch that adds --gpg-sign to pull as well.  I added my
sign-off to his patches because SubmittingPatches (section (c)) seems to
imply that I should, although I can rebase it out if it's a problem.

I've been running with this patch set for some time now, and haven't
found any issues, although I wouldn't recommend doing large rebases
without a gpg-agent running.

I'm happy to fix any issues that come up in order to move this series as
expeditiously as possible, but seeing as I'm in San Francisco at the
moment, re-rolls might be delayed through the next few days.

Nicolas Vigier (8):
  cherry-pick, revert: add the --gpg-sign option
  git-sh-setup.sh: add variable to use the stuck-long mode
  am: parse options in stuck-long mode
  am: add the --gpg-sign option
  rebase: remove useless arguments check
  rebase: don't try to match -M option
  rebase: parse options in stuck-long mode
  rebase: add the --gpg-sign option

brian m. carlson (1):
  pull: add the --gpg-sign option.

 Documentation/git-am.txt  |  6 +++-
 Documentation/git-cherry-pick.txt |  7 -
 Documentation/git-rebase.txt  |  4 +++
 Documentation/git-revert.txt  |  6 +++-
 builtin/revert.c  |  2 ++
 contrib/examples/git-checkout.sh  |  1 +
 contrib/examples/git-clean.sh |  1 +
 contrib/examples/git-clone.sh |  1 +
 contrib/examples/git-merge.sh |  1 +
 contrib/examples/git-repack.sh|  1 +
 contrib/git-resurrect.sh  |  1 +
 git-am.sh | 26 ++--
 git-instaweb.sh   |  1 +
 git-pull.sh   | 13 +++-
 git-quiltimport.sh|  1 +
 git-rebase--am.sh |  8 +++--
 git-rebase--interactive.sh| 32 
 git-rebase--merge.sh  |  2 +-
 git-rebase.sh | 62 +--
 git-request-pull.sh   |  1 +
 git-sh-setup.sh   |  2 ++
 sequencer.c   | 11 +++
 sequencer.h   |  2 ++
 23 files changed, 134 insertions(+), 58 deletions(-)

-- 
1.9.rc0.1002.gd081c64.dirty

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


[PATCH v2 8/9] rebase: add the --gpg-sign option

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-rebase.txt |  4 
 git-rebase--am.sh|  8 +---
 git-rebase--interactive.sh   | 32 
 git-rebase--merge.sh |  2 +-
 git-rebase.sh| 11 +++
 5 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2889be6..2a93c64 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -281,6 +281,10 @@ which makes little sense.
specified, `-s recursive`.  Note the reversal of 'ours' and
'theirs' as noted above for the `-m` option.
 
+-S[keyid]::
+--gpg-sign[=keyid]::
+   GPG-sign commits.
+
 -q::
 --quiet::
Be quiet. Implies --no-stat.
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index a4f683a..df46f4c 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -6,7 +6,8 @@
 
 case $action in
 continue)
-   git am --resolved --resolvemsg=$resolvemsg 
+   git am --resolved --resolvemsg=$resolvemsg \
+   ${gpg_sign_opt:+$gpg_sign_opt} 
move_to_original_branch
return
;;
@@ -26,7 +27,7 @@ then
# empty commits and even if it didn't the format doesn't really lend
# itself well to recording empty patches.  fortunately, cherry-pick
# makes this easy
-   git cherry-pick --allow-empty $revisions
+   git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} --allow-empty 
$revisions
ret=$?
 else
rm -f $GIT_DIR/rebased-patches
@@ -60,7 +61,8 @@ else
return $?
fi
 
-   git am $git_am_opt --rebasing --resolvemsg=$resolvemsg 
$GIT_DIR/rebased-patches
+   git am $git_am_opt --rebasing --resolvemsg=$resolvemsg \
+   ${gpg_sign_opt:+$gpg_sign_opt} $GIT_DIR/rebased-patches
ret=$?
 
rm -f $GIT_DIR/rebased-patches
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 43c19e0..73d32dd 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -181,7 +181,7 @@ exit_with_patch () {
git rev-parse --verify HEAD  $amend
warn You can amend the commit now, with
warn
-   warn   git commit --amend
+   warn   git commit --amend $gpg_sign_opt
warn
warn Once you are satisfied with your changes, run
warn
@@ -248,7 +248,8 @@ pick_one () {
 
test -d $rewritten 
pick_one_preserving_merges $@  return
-   output eval git cherry-pick $strategy_args $empty_args $ff $@
+   output eval git cherry-pick ${gpg_sign_opt:+$gpg_sign_opt} \
+   $strategy_args $empty_args $ff $@
 }
 
 pick_one_preserving_merges () {
@@ -359,7 +360,8 @@ pick_one_preserving_merges () {
echo $sha1 $(git rev-parse HEAD^0)  
$rewritten_list
;;
*)
-   output eval git cherry-pick $strategy_args $@ ||
+   output eval git cherry-pick 
${gpg_sign_opt:+$gpg_sign_opt} \
+   $strategy_args $@ ||
die_with_patch $sha1 Could not pick $sha1
;;
esac
@@ -470,7 +472,8 @@ do_pick () {
   --no-post-rewrite -n -q -C $1 
pick_one -n $1 
git commit --allow-empty --allow-empty-message \
-  --amend --no-post-rewrite -n -q -C $1 ||
+  --amend --no-post-rewrite -n -q -C $1 \
+  ${gpg_sign_opt:+$gpg_sign_opt} ||
die_with_patch $1 Could not apply $1... $2
else
pick_one $1 ||
@@ -497,7 +500,7 @@ do_next () {
 
mark_action_done
do_pick $sha1 $rest
-   git commit --amend --no-post-rewrite || {
+   git commit --amend --no-post-rewrite 
${gpg_sign_opt:+$gpg_sign_opt} || {
warn Could not amend commit after successfully picking 
$sha1... $rest
warn This is most likely due to an empty commit 
message, or the pre-commit hook
warn failed. If the pre-commit hook failed, you may 
need to resolve the issue before
@@ -542,19 +545,22 @@ do_next () {
squash|s|fixup|f)
# This is an intermediate commit; its message will only 
be
# used in case of trouble.  So use the long version:
-   do_with_author output git commit --amend --no-verify -F 
$squash_msg ||
+   do_with_author output git commit --amend --no-verify -F 
$squash_msg \
+   ${gpg_sign_opt:+$gpg_sign_opt} ||

[PATCH v2 4/9] am: add the --gpg-sign option

2014-01-23 Thread brian m. carlson
From: Nicolas Vigier bo...@mars-attacks.org

Signed-off-by: Nicolas Vigier bo...@mars-attacks.org
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 Documentation/git-am.txt | 6 +-
 git-am.sh| 9 -
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 54d8461..17924d0 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -14,7 +14,7 @@ SYNOPSIS
 [--ignore-date] [--ignore-space-change | --ignore-whitespace]
 [--whitespace=option] [-Cn] [-pn] [--directory=dir]
 [--exclude=path] [--include=path] [--reject] [-q | --quiet]
-[--[no-]scissors]
+[--[no-]scissors] [-S[keyid]]
 [(mbox | Maildir)...]
 'git am' (--continue | --skip | --abort)
 
@@ -119,6 +119,10 @@ default.   You can use `--no-utf8` to override this.
Skip the current patch.  This is only meaningful when
restarting an aborted patch.
 
+-S[keyid]::
+--gpg-sign[=keyid]::
+   GPG-sign commits.
+
 --continue::
 -r::
 --resolved::
diff --git a/git-am.sh b/git-am.sh
index 020abf6..dccbae2 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -38,6 +38,7 @@ abort   restore the original branch and abort the 
patching operation.
 committer-date-is-author-datelie about committer date
 ignore-date use current timestamp for author date
 rerere-autoupdate update the index with reused conflict resolution if possible
+S,gpg-sign? GPG-sign commits
 rebasing*   (internal use for git-rebase)
 
 . git-sh-setup
@@ -375,6 +376,7 @@ git_apply_opt=
 committer_date_is_author_date=
 ignore_date=
 allow_rerere_autoupdate=
+gpg_sign_opt=
 
 if test $(git config --bool --get am.keepcr) = true
 then
@@ -436,6 +438,10 @@ it will be removed. Please do not use it anymore.
keepcr=t ;;
--no-keep-cr)
keepcr=f ;;
+   --gpg-sign)
+   gpg_sign_opt=-S ;;
+   --gpg-sign=*)
+   gpg_sign_opt=-S${1#--gpg-sign=} ;;
--)
shift; break ;;
*)
@@ -900,7 +906,8 @@ did you forget to use 'git add'?
GIT_COMMITTER_DATE=$GIT_AUTHOR_DATE
export GIT_COMMITTER_DATE
fi 
-   git commit-tree $tree ${parent:+-p} $parent 
$dotest/final-commit
+   git commit-tree ${gpg_sign_opt:+$gpg_sign_opt} $tree 
${parent:+-p} $parent \
+   $dotest/final-commit
) 
git update-ref -m $GIT_REFLOG_ACTION: $FIRSTLINE HEAD $commit $parent 
||
stop_here $this
-- 
1.9.rc0.1002.gd081c64.dirty

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


Re: [PATCH 2/3] read-cache: use get_be32 instead of hand-rolled ntoh_l

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 03:34:16PM -0800, Jonathan Nieder wrote:

 Line 1484 looks more problematic:
 
   disk_ce = (struct ondisk_cache_entry *)((char *)mmap + 
 src_offset);
 
 In v4 indexes, src_offset doesn't have any particular alignment so
 this conversion has undefined behavior.
 
 Do you know if any tests exercise this code with paths that don't
 have convenient length?

My impression was that we are not testing v4 index at all (and grepping
for `--index-version`, which I think is the only way to write it,
supports that).

-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] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Jeff King
On Fri, Jan 24, 2014 at 12:56:17AM +0100, Vicent Martí wrote:

 On Fri, Jan 24, 2014 at 12:45 AM, Siddharth Agarwal s...@fb.com wrote:
  Yes, we'd prefer to do that too. How do you actually do this, though? I
  don't see a way to pass `--honor-pack-keep` (shouldn't I pass in its
  inverse?) down to `git-pack-objects`.
 
 We run with this patch in production, it may be of use to you:
 https://gist.github.com/vmg/8589317
 
 In fact, it may be worth upstreaming too. I'll kindly ask peff to do
 it when he has a moment.

I was actually looking at it earlier when I sent this message. The
tricky thing about the patch is that it turns off --honor-pack-keep, but
does _not_ teach git-repack to clean up the .keep file.

Which I think is the right and safe thing to do, as otherwise you might
blow away a pack with .keep, even though you did not just pack its
objects (i.e., because it was written by a fetch or push which did not
yet update the refs). So the safe thing is to actually duplicate those
objects, leave the .keep pack around, and then assume it will get
cleaned up on the next repack.

If you _do_ have a stale .keep file, though, then that stale pack will
hang around forever (presumably with its objects duplicated in the
real pack).

So I think the patch is doing the right thing, but I was still figuring
out how to explain it (and I hope I just did). I'll post it with a full
commit message tomorrow.

-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] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Jeff King
On Thu, Jan 23, 2014 at 03:53:28PM -0800, Siddharth Agarwal wrote:

 On 01/23/2014 03:45 PM, Siddharth Agarwal wrote:
 
 The worry is less certain objects not being packed and more the old
 packs being deleted by git repack, isn't it? From the man page for
 git-index-pack:
 
 This should probably be new pack and not old packs, I guess. Not
 knowing much about how this actually works, I'm assuming the scenario
 here is something like:
 
 (1) git receive-pack receives a pack P.pack and writes it to disk
 (2) git index-pack runs on P.pack
 (3) git repack runs separately, finds pack P.pack with no refs
 pointing to it, and deletes it
 (4) everything goes wrong
 
 With a keep file, this would be averted because
 
 (1) git receive-pack receives a pack P.pack and writes it to disk
 (2) git index-pack writes a keep file for P.pack, called P.keep
 (3) git repack runs separately, finds pack P.pack with a keep file,
 doesn't touch it
 (4) git index-pack finishes, and something updates refs to point to
 P.pack and deletes P.keep

I think your understanding is accurate here. So we want repack to
respect keep files for deletion, but we _not_ necessarily want
pack-objects to avoid packing an object just because it's in a pack
marked by .keep (see my other email).

-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] pack-objects: turn off bitmaps when skipping objects

2014-01-23 Thread Siddharth Agarwal

On 01/23/2014 06:28 PM, Jeff King wrote:

I think your understanding is accurate here. So we want repack to
respect keep files for deletion, but we _not_ necessarily want
pack-objects to avoid packing an object just because it's in a pack
marked by .keep (see my other email).


Yes, that makes sense and sounds pretty safe.

So the right solution for us probably is to apply the patch Vicent 
posted, set repack.honorpackkeep to false, and also have a cron job that 
cleans up stale .keep files so that subsequent repacks clean it up.

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


Re: [PATCH 2/2] t4010: match_pathspec_depth() and trailing slash after submodule

2014-01-23 Thread Duy Nguyen
On Fri, Jan 24, 2014 at 4:38 AM, Junio C Hamano gits...@pobox.com wrote:
 Jens Lehmann jens.lehm...@web.de writes:

 ... But a single
 trailing '/' does mark submod as a directory, which I think is
 ok for a submodule. And it makes life easier for the user if we
 accept that, as shell completion will add it there automatically.

 OK, that would be annoying.

 Perhaps the completion is what is broken here, then?  I dunno, and
 haven't thought things through.

My reasoning is more simple minded: if git diff HEAD HEAD^ submod/
works, the user would expect git diff HEAD submod/ to work too. I
think I've got something working with a bit refactoring. Will send out
soon.
-- 
Duy
--
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