[PATCH] Fix build with LDFLAGS=-Wl,--as-needed

2014-12-01 Thread Lars Wendler
Signed-off-by: Lars Wendler polynomia...@gentoo.org
---
 contrib/svn-fe/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile
index e8651aa..b90cf87 100644
--- a/contrib/svn-fe/Makefile
+++ b/contrib/svn-fe/Makefile
@@ -74,7 +74,7 @@ endif
 endif
 
 svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(XDIFF_LIB) $(GIT_LIB)
-   $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(EXTLIBS) -o $@ svn-fe.o 
$(LIBS)
+   $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ svn-fe.o $(LIBS) 
$(EXTLIBS)
 
 svn-fe.o: svn-fe.c ../../vcs-svn/svndump.h
$(QUIET_CC)$(CC) $(CFLAGS) -I../../vcs-svn -o $*.o -c $
-- 
2.2.0

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


[PATCH] git add -i: allow list (un)selection by regexp

2014-12-01 Thread Aarni Koskela
Hello!

This patch makes it possible to select or unselect files in `git add -i`
by regular expression instead of unique prefix only.

The command syntax is `/foo` for selection and `-/foo` for unselection.
I don't think the syntax will conflict with any existing use cases, but feel
free to prove me wrong.

I'm not a Perl programmer, but I've tried to follow the style of the
existing code as much as possible. :)

Note I'm currently not on the mailing list, so please cc.

Best regards,

Aarni Koskela

From 53c12d9c9928dc93a57595e92d785ecc0b245390 Mon Sep 17 00:00:00 2001
From: Aarni Koskela a...@iki.fi
Date: Mon, 1 Dec 2014 11:06:10 +0200
Subject: [PATCH] git-add--interactive: allow list (un)selection by regular
 expression

Teach `list_and_choose` to allow `/regexp` and `-/regexp` syntax to
select items based on regular expression match.

For instance, `/jpg$` will select all options whose display name ends with
`jpg`.

Signed-off-by: Aarni Koskela a...@iki.fi
---
 git-add--interactive.perl | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index 1fadd69..34cc603 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -483,6 +483,8 @@ sub is_valid_prefix {
!($prefix =~ /[\s,]/)  # separators
!($prefix =~ /^-/) # deselection
!($prefix =~ /^\d+/)   # selection
+   !($prefix =~ /^\//)# regexp selection
+   !($prefix =~ /^-\//)   # regexp unselection
($prefix ne '*')   # all wildcard
($prefix ne '?');# prompt help
 }
@@ -585,6 +587,28 @@ sub list_and_choose {
prompt_help_cmd();
next TOPLOOP;
}
+   if ($line =~ /^(-)*\/(.+)$/) {
+   my $choose = !($1  $1 eq '-');
+   my $re = $2;
+   my $found = 0;
+   for ($i = 0; $i  @stuff; $i++) {
+   my $val = $stuff[$i];
+   my $ref = ref $val;
+   if ($ref eq 'ARRAY') {
+   $val = $val-[0];
+   }
+   elsif ($ref eq 'HASH') {
+   $val = $val-{VALUE};
+   }
+   if ($val =~ /$re/) {
+   $chosen[$i] = $choose;
+   $found = 1;
+   last if $opts-{SINGLETON};
+   }
+   }
+   last if $found  ($opts-{IMMEDIATE});
+   next TOPLOOP;
+   }
for my $choice (split(/[\s,]+/, $line)) {
my $choose = 1;
my ($bottom, $top);
@@ -635,6 +659,7 @@ sub singleton_prompt_help_cmd {
 Prompt help:
 1  - select a numbered item
 foo- select item based on unique prefix
+/regexp- select item based on regular expression
- (empty) select nothing
 EOF
 }
@@ -648,6 +673,8 @@ Prompt help:
 foo- select item based on unique prefix
 -...   - unselect specified items
 *  - choose all items
+/regexp- select items based on regular expression
+-/regexp   - unselect items based on regular expression
- (empty) finish selecting
 EOF
 }
-- 
1.9.2.msysgit.0

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


Re: [PATCH] git-svn: Support for git-svn propset

2014-12-01 Thread Eric Wong
Alfred Perlstein alf...@freebsd.org wrote:
 This change allows git-svn to support setting subversion properties.
 
 Very useful for manually setting properties when committing to a
 subversion repo that *requires* properties to be set without requiring
 moving your changeset to separate subversion checkout in order to
 set props.

Thanks Alfred, that's a good reason for supporting this feature
(something I wasn't convinced of with the original).

 This change is initially from David Fraser davidf () sjsoft ! com
 Appearing here: http://marc.info/?l=gitm=125259772625008w=2

I've added David to the Cc: (but I never heard back from him regarding
comments from his original patch).

Alfred: I had some comments on David's original patch here that
were never addressed:
  http://mid.gmane.org/20090923085812.ga20...@dcvr.yhbt.net

I suspect my concerns about .gitattributes in the source tree from
2009 are less relevant now in 2014 as git-svn seems more socially
acceptable in SVN-using projects.

Some of my other concerns about mimicking existing Perl style still
apply, and we have a Perl section in Documenting/CodingGuidelines
nowadays.

 They are now forward ported to most recent git along with fixes to
 deal with files in subdirectories.
 
 Developer's Certificate of Origin 1.1

snip no need for the whole DCO in the commit message.
just the S-o-b:

 Signed-off-by: Alfred Perlstein alf...@freebsd.org

Since this was originally written by David, his sign-off from the
original email should be here (Cc: for bigger changes)

 ---
  git-svn.perl   | 50 
 +-
  perl/Git/SVN/Editor.pm | 47 +++

We need a test case written for this new feature.

Most folks (including myself) who were ever involved with git-svn
rarely use it anymore, and this will likely be rarely-used.

  2 files changed, 96 insertions(+), 1 deletion(-)
 
 diff --git a/git-svn.perl b/git-svn.perl
 index b6e2186..91423a8 100755
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -115,7 +115,7 @@ my ($_stdin, $_help, $_edit,
   $_before, $_after,
   $_merge, $_strategy, $_preserve_merges, $_dry_run, $_parents, $_local,
   $_prefix, $_no_checkout, $_url, $_verbose,
 - $_commit_url, $_tag, $_merge_info, $_interactive);
 + $_commit_url, $_tag, $_merge_info, $_interactive, $_set_svn_props);
  
  # This is a refactoring artifact so Git::SVN can get at this git-svn switch.
  sub opt_prefix { return $_prefix || '' }
 @@ -193,6 +193,7 @@ my %cmd = (
 'dry-run|n' = \$_dry_run,
 'fetch-all|all' = \$_fetch_all,
 'commit-url=s' = \$_commit_url,
 +   'set-svn-props=s' = \$_set_svn_props,
 'revision|r=i' = \$_revision,
 'no-rebase' = \$_no_rebase,
 'mergeinfo=s' = \$_merge_info,
 @@ -228,6 +229,9 @@ my %cmd = (
  'propget' = [ \cmd_propget,
  'Print the value of a property on a file or directory',
  { 'revision|r=i' = \$_revision } ],
 +'propset' = [ \cmd_propset,
 +'Set the value of a property on a file or directory - 
 will be set on commit',
 +{} ],
  'proplist' = [ \cmd_proplist,
  'List all properties of a file or directory',
  { 'revision|r=i' = \$_revision } ],
 @@ -1376,6 +1380,50 @@ sub cmd_propget {
   print $props-{$prop} . \n;
  }
  
 +# cmd_propset (PROPNAME, PROPVAL, PATH)
 +# 
 +# Adjust the SVN property PROPNAME to PROPVAL for PATH.
 +sub cmd_propset {
 + my ($propname, $propval, $path) = @_;
 + $path = '.' if not defined $path;
 + $path = $cmd_dir_prefix . $path;
 + usage(1) if not defined $propname;
 + usage(1) if not defined $propval;
 + my $file = basename($path);
 + my $dn = dirname($path);
 + # diff has check_attr locally, so just call direct
 + #my $current_properties = check_attr( svn-properties, $path );

I prefer leaving out dead code entirely instead of leaving it commented out.

 + my $current_properties = Git::SVN::Editor::check_attr( 
 svn-properties, $path );
 + my $new_properties = ;

Since some lines are too long, local variables can be shortened
to cur_props, new_props without being any less descriptive.

 + if ($current_properties eq unset || $current_properties eq  || 
 $current_properties eq set) {
 + $new_properties = $propname=$propval;
 + } else {
 + # TODO: handle combining properties better
 + my @props = split(/;/, $current_properties);
 + my $replaced_prop = 0;
 + foreach my $prop (@props) {
 + # Parse 'name=value' syntax and set the property.
 + if ($prop =~ /([^=]+)=(.*)/) {
 + my ($n,$v) = ($1,$2);
 + 

Re: [PATCH 24/34] checkout: reject if the branch is already checked out elsewhere

2014-12-01 Thread Duy Nguyen
On Sun, Nov 30, 2014 at 12:18:40PM -0500, Mark Levedahl wrote:
 On 11/30/2014 03:24 AM, Nguyễn Thái Ngọc Duy wrote:
  One branch obviously can't be checked out at two places (but detached
  heads are ok). Give the user a choice in this case: --detach, -b
  new-branch, switch branch in the other checkout first or simply 'cd'
  and continue to work there.
 
 
 This seems too restrictive and is not obvious to me: I currently use 
 git-new-workdir to have multiple checkouts of the same branch, with no 
 ill effect. While those who do not understand what is going on 
 underneath might be confused by one checkout suddenly showing 
 uncommitted diffs, I don't accept that as a reason to outright prevent 
 such use. I suggest, at the very least, that this behavior be overridden 
 by a --force flag?

Prevention is a strong word. It's more about safety for the mere
mortals. It's certainly possible to do something like this patch
(--force can't be reused, it already carries some other meaning).
Should I add this one in the next (hic) reroll?

-- 8 --
Subject: [PATCH] checkout: add --ignore-other-wortrees

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/git-checkout.txt |  6 ++
 builtin/checkout.c | 19 +++
 t/t2025-checkout-to.sh |  7 +++
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index 0c13825..71d9e4e 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -232,6 +232,12 @@ section of linkgit:git-add[1] to learn how to operate the 
`--patch` mode.
specific files such as HEAD, index... See MULTIPLE WORKING
TREES section for more information.
 
+--ignore-other-worktrees::
+   `git checkout` refuses when the wanted ref is already checked out
+   by another worktree. This option makes it check the
+   ref out anyway. In other words, the ref is held by more than one
+   worktree.
+
 branch::
Branch to checkout; if it refers to a branch (i.e., a name that,
when prepended with refs/heads/, is a valid ref), then that
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 953b763..8b2bf20 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -37,6 +37,7 @@ struct checkout_opts {
int writeout_stage;
int overwrite_ignore;
int ignore_skipworktree;
+   int ignore_other_worktrees;
 
const char *new_branch;
const char *new_branch_force;
@@ -1079,11 +1080,12 @@ static void check_linked_checkouts(struct branch_info 
*new)
 static int parse_branchname_arg(int argc, const char **argv,
int dwim_new_local_branch_ok,
struct branch_info *new,
-   struct tree **source_tree,
-   unsigned char rev[20],
-   const char **new_branch,
-   int force_detach)
+   struct checkout_opts *opts,
+   unsigned char rev[20])
 {
+   struct tree **source_tree = opts-source_tree;
+   const char **new_branch = opts-new_branch;
+   int force_detach = opts-force_detach;
int argcount = 0;
unsigned char branch_rev[20];
const char *arg;
@@ -1209,7 +1211,8 @@ static int parse_branchname_arg(int argc, const char 
**argv,
int flag;
char *head_ref = resolve_refdup(HEAD, 0, sha1, flag);
if (head_ref 
-   (!(flag  REF_ISSYMREF) || strcmp(head_ref, new-path)))
+   (!(flag  REF_ISSYMREF) || strcmp(head_ref, new-path)) 
+   !opts-ignore_other_worktrees)
check_linked_checkouts(new);
free(head_ref);
}
@@ -1340,6 +1343,8 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
N_(second guess 'git checkout 
no-such-branch')),
OPT_FILENAME(0, to, opts.new_worktree,
   N_(check a branch out in a separate working 
directory)),
+   OPT_BOOL(0, ignore-other-worktrees, 
opts.ignore_other_worktrees,
+N_(do not check if another worktree is holding the 
given ref)),
OPT_END(),
};
 
@@ -1420,9 +1425,7 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
opts.track == BRANCH_TRACK_UNSPECIFIED 
!opts.new_branch;
int n = parse_branchname_arg(argc, argv, dwim_ok,
-new, opts.source_tree,
-rev, opts.new_branch,
-opts.force_detach);
+new, opts, rev);
argv += n;
argc -= n;
}

Re: [PATCH/RFC v2] Squashed changes for multiple worktrees vs. submodules

2014-12-01 Thread Duy Nguyen
On Mon, Dec 1, 2014 at 6:27 AM, Max Kirillov m...@max630.net wrote:
 But, while hacking the submodule init I became more convinced that the
 modules directory should be common and submodules in checkout should be
 a checkouts of the submodule. Because this is looks like concept of
 submodules, that they are unique for the lifetime of repository, even if
 they do not exist in all revisions. And if anybody want to use fully
 independent checkout they can be always checked out manually. Actually,
 after a submodule is initialized and have a proper gitlink, it can be
 updated and inquired regardless of where it points to.

Just throw something in for discussion. What about keeping
$GIT_DIR/modules like it is now (i.e. not shared) and add
$GIT_DIR/shared-modules, which is the same for all checkouts? That
would keep current submodule code happy (no name collision or
anything). New submodule code can start using $GIT_DIR/shared-modules
while still keeping an eye on $GIT_DIR/modules for old setups.
-- 
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


[BUG] Documentation: git log: --exit-code undocumented?

2014-12-01 Thread Sergey Organov
Hello,

$ git help log | grep exit-code
   problems are found. Not compatible with --exit-code.
$

What --exit-code does in git log?

-- 
Sergey.
--
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] compat: convert modes to use portable file type values

2014-12-01 Thread David Michael
On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen tbo...@web.de wrote:
 On 12/01/2014 04:40 AM, David Michael wrote:

 On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen tbo...@web.de
 wrote:
 [snip]

 Could the code be more human-readable ?
 static inline mode_t mode_native_to_git(mode_t native_mode)
 {
  int perm_bits = native_mode  0;
  if (S_ISREG(native_mode))
  return 010 | perm_bits;
  if (S_ISDIR(native_mode))
  return 004 | perm_bits;
  if (S_ISLNK(native_mode))
  return 012 | perm_bits;
  if (S_ISBLK(native_mode))
  return 006 | perm_bits;
  if (S_ISCHR(native_mode))
  return 002 | perm_bits;
  if (S_ISFIFO(native_mode))
  return 001 | perm_bits;
  /* Non-standard type bits were given. */
  /* Shouldn't we die() here ?? */
  return perm_bits;
 }

 Sure, I can send an updated patch with the new variable and without the
 elses.

 Regarding the question in the last comment:  I was assuming if this
 case was ever reached, Git would handle the returned mode the same way
 as if it encountered an unknown/non-standard file type on a normal
 operating system, which could die() if needed in the function that
 called stat().

 Should I send an updated patch that die()s there?

 David

 Not yet, please wait with a V2 patch until I finished my thinking ;-)

 I take back the suggestion with the die(). I was thinking how to handle
 unforeseen types, which may show up on the z/OS some day,
 So die() is not a good idea, it is better to ignore them, what the code
 does.

 Knowing that Git does not track block devices, nor character devices nor
 sockets,
 the above code could be simplyfied even more, by mapping everything which
 is not a directory, a file or a softlink to device type 0)

 This is just a suggestion, I want to here from others as well:

 int perm_bits = native_mode  0;
 if (S_ISREG(native_mode))
 return 010 | perm_bits;
 if (S_ISDIR(native_mode))
 return 004 | perm_bits;
 if (S_ISLNK(native_mode))
 return 012 | perm_bits;
 /* Git does not track S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK  */
 return perm_bits;

I had considered omitting those three as well at first, but in this
case it would mean that they will be unusable all together.

The z/OS S_IFMT definition (i.e. the file type bit mask) is
0xFF00, and the common/translated S_IFMT definition is 0xF000.
Since the S_ISxxx macros use the typical ((mode  S_IFMT) == S_IFxxx)
definition, they would never match a native z/OS mode after redefining
S_IFMT.

So translating those types isn't just for tracking files, it's to
support any use of S_ISxxx anywhere in the code.  It should be okay to
remove any of those types if we know that Git will never need to use
them.

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


[BUG] git rebase -X fails with merge-recursive error

2014-12-01 Thread Øystein Walle
Hi,

I discovered this while rebasing a branch after having converted files
to use LF line endings. I got around the problem by using
--ignore-whitespace but the error still seems strange to me so I'm
reporting it.

The following script is equivalent: it creates a repo with a CRLF file,
creates a feature branch, converts the file and then tries to rebase
the feature branch with git rebase -X ignore-whitespace-at-eol:

git init
echo That's some catch, that Catch-1984 file.txt
unix2dos file.txt
git add file.txt
git commit -m 'Initial commit'
git checkout -b feature
ex -sc 's/1984/22/|x' file.txt
git commit -m Fix literary confusion file.txt
git checkout master
echo '* text=auto'  .gitattributes
dos2unix file.txt
git add .
git commit -m 'CRLF to LF'
git checkout feature
gti config merge.renormalize true
git rebase -X ignore-space-at-eol master

While rebasing the following error appears:

error: addinfo_cache failed for path 'file.txt'

It comes from the git-merge-recursive invocation git-rebase performs[1]
and is printed because make_cache_entry() fails[2]. I also get this
error when using the other 'ignore-*' strategy options, as well as
'theirs'.

At this point file.txt still has its CRLF terminator, but simply:

git add file.txt
git rebase --continue

has the desired effect (as it did in my real life situation). I've
tested this on Git 2.2.0, 2.1.3 and 1.9.0.

I don't know whether this is actually related to git-rebase. It's a way
to reproduce it if nothing else. I haven't tried any experiments with
git-merge-recursive directly, I get the impression I'm not supposed to
since it doesn't have a manpage ;) Interestingly, removing the creation
of .gitattributes makes the problem go away.

Additionally, git-merge-recursive exits with status 0. This confuses
git-rebase which will continue and then complain about the state of the
index. (Interestingly, at this point my prompt thingy complains that
.git/rebase-merge/done isn't there).

Regards,
Øsse

[1]: https://github.com/git/git/blob/master/git-rebase--merge.sh#L70
[2]: https://github.com/git/git/blob/master/merge-recursive.c#L209


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


HIstory simplification limited to requested refs ?

2014-12-01 Thread ydirson
Hello,

I'm trying to get gitk to draw the relationship between a bunch of integration 
branches.
What I'm looking for is a graph that would show which of those branches is 
included by
others.  --simplify-by-decoration nearly does the job, but does not omit those 
heads
merged into the requested branches.  Did I miss a particular git-log flag that 
would
allow something like --simplify-by-commandline ?

Best regards,
--
Yann
--
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] compat: convert modes to use portable file type values

2014-12-01 Thread Duy Nguyen
On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote:
 +int git_stat(const char *path, struct stat *buf)
 +{
 +   int rc;
 +   rc = stat(path, buf);
 +   if (buf != NULL)

It's a minor thing, but maybe test !rc instead of buf != NULL?

 +   buf-st_mode = mode_native_to_git(buf-st_mode);
 +   return rc;
 +}
-- 
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


Re: [PATCH/RFC v2] Squashed changes for multiple worktrees vs. submodules

2014-12-01 Thread Max Kirillov
On Mon, Dec 01, 2014 at 05:43:16PM +0700, Duy Nguyen wrote:
 On Mon, Dec 1, 2014 at 6:27 AM, Max Kirillov m...@max630.net wrote:
 But, while hacking the submodule init I became more
 convinced that the modules directory should be common and
 submodules in checkout should be a checkouts of the
 submodule. Because this is looks like concept of
 submodules, that they are unique for the lifetime of
 repository, even if they do not exist in all revisions.
 And if anybody want to use fully independent checkout
 they can be always checked out manually. Actually, after
 a submodule is initialized and have a proper gitlink, it
 can be updated and inquired regardless of where it points
 to.
 
 Just throw something in for discussion. What about keeping
 $GIT_DIR/modules like it is now (i.e. not shared) and add
 $GIT_DIR/shared-modules, which is the same for all
 checkouts? That would keep current submodule code happy
 (no name collision or anything). New submodule code can
 start using $GIT_DIR/shared-modules while still keeping an
 eye on $GIT_DIR/modules for old setups.

I think it would be too complicated. To make fancy think
user can always manually initialize a repository or a
checkout. And all sumbodule functionality except
adding/removing/(de)initialization should work with any
repository or gitlink, regardless of where it points to.

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


Bug in reflog of length 0x2BFF

2014-12-01 Thread Christoph Mallon
Hi,

I encountered a strange bug concerning the reflog.
I suspect some kind of out-of-bounds access.
The symptom is:

%git rev-parse 'master@{52}'
warning: Log for ref refs/heads/master has gap after Thu, 1 Jan 1970
00:00:01 +.
0036

Try the following:

git init gitbug
cd gitbug
git commit --allow-empty -m a
cp ../reflog.bad .git/logs/refs/heads/master
git rev-parse 'master@{52}'

The source of cp is the attached file.
This is from a reflog of stash.
I just replaced all stuff by dummy values.
This does not seem to affect the bug.
Sorry, it must be this long.

Some observations:
* If you change the length of any line starting at line 3, the symptom
vanishes.
  (The X at the line ends are free-form text.)
* Starting at line three, there are 0x2BFF bytes till the end of file.
  Is there some dynamically growing buffer, which at some point reaches
the size 0x2C00?
* Changing the length of the first two lines has no effect.
  Is the file read backwards?
* It happens at least with git 2.1.2 (amd64) and 2.2.0 (ia32).
* 2.0.2 (amd64) and 2.1.0 (amd64) seem not to have this bug.


Any ideas?

Christoph
0037 
0036  
xxx@xxx 01 +   X
0036 
0035  
xxx@xxx 01 +   X
0035 
0034  
xxx@xxx 01 +   

0034 
0033  
xxx@xxx 01 +   XXX
0033 
0032  
xxx@xxx 01 +   
XX
0032 
0031  
xxx@xxx 01 +   
X
0031 
0030  
xxx@x 01 + 
XX
0030 
002f  
xxx@x 01 + 
XXX
002f 
002e  
xxx@x 01 + 
XXX
002e 
002d  
xxx@x 01 + 

002d 
002c  
xxx@x 01 + 
XXX
002c 
002b  
xxx@x 01 + 
XXX
002b 
002a  
xxx@x 01 + 
X
002a 
0029  
xxx@x 01 + 
XXX
0029 
0028  
xxx@x 01 + 
XX
0028 
0027  
xxx@x 01 + 
XXX
0027 
0026  
xxx@x 01 + 
XXX
0026 
0025  
xxx@x 01 + 
XXX
0025 
0024  
xxx@x 

Re: [PATCH] Fix build with LDFLAGS=-Wl,--as-needed

2014-12-01 Thread Junio C Hamano
Lars Wendler polynomia...@gentoo.org writes:

 Subject: Re: [PATCH] Fix build with LDFLAGS=-Wl,--as-needed

Please identify what area you are touching, e.g.

Subject: contrib/svn-fe: avoid early $(EXTLIBS) on linker invocation

or whatever.

Fix build does not tell us why this change is needed; it does not
say what breaks, how and most importantly why it breaks.  Please
have explanation in the commit message body, e.g.

When attempting to build svn-fe with LDFLAGS=-Wl,--as-needed,
I noticed that ... breaks in such and such way.  This is
because you must not have X before Y due to Z.

Fix this by doing A, B and C, which makes sure X comes after
Y.

or somesuch.

 Signed-off-by: Lars Wendler polynomia...@gentoo.org
 ---

  contrib/svn-fe/Makefile | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/contrib/svn-fe/Makefile b/contrib/svn-fe/Makefile
 index e8651aa..b90cf87 100644
 --- a/contrib/svn-fe/Makefile
 +++ b/contrib/svn-fe/Makefile
 @@ -74,7 +74,7 @@ endif
  endif
  
  svn-fe$X: svn-fe.o $(VCSSVN_LIB) $(XDIFF_LIB) $(GIT_LIB)
 - $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) $(EXTLIBS) -o $@ svn-fe.o 
 $(LIBS)
 + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ svn-fe.o $(LIBS) 
 $(EXTLIBS)

I do not understand the original, which has

EXTLIBS =

GIT_LIB = ../../libgit.a
VCSSVN_LIB = ../../vcs-svn/lib.a
LIBS = $(VCSSVN_LIB) $(GIT_LIB) $(EXTLIBS)

i.e. it shouldn't be necessary to explicitly list EXTLIBS, as it
already has LIBS at the end, which in turn has EXTLIBS at the end.

Which in turn means I do not understand your updated version,
either.  If having EXTLIBS before svn-fe.o is bad (and it is---a
linker invocation lists *.o files that need to be linked first, and
then libraries to find the symbols requested by those *.o files),
wouldn't a fix be to just drop $(EXTLIBS)?  Why is it necessary to
list it twice?
--
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: Bug in reflog of length 0x2BFF

2014-12-01 Thread Christoph Mallon
This commit seems to introduce the bug:
4207ed285f31ad3e04f08254237c0c1a1609642b
--
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: [BUG] Documentation: git log: --exit-code undocumented?

2014-12-01 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 Hello,

 $ git help log | grep exit-code
problems are found. Not compatible with --exit-code.
 $

 What --exit-code does in git log?

It doesn't.  That is why it is not listed.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] git add -i: allow list (un)selection by regexp

2014-12-01 Thread Junio C Hamano
Aarni Koskela aarni.kosk...@andersinnovations.com writes:

 Hello!

 This patch makes it possible to select or unselect files in `git add -i`
 by regular expression instead of unique prefix only.

 The command syntax is `/foo` for selection and `-/foo` for unselection.
 I don't think the syntax will conflict with any existing use cases, but feel
 free to prove me wrong.

Usually the responsibility to ensure correctness lies on the person
who proposes a change, not those who relies on the continued correct
operation of the existing code.

 I'm not a Perl programmer, but I've tried to follow the style of the
 existing code as much as possible. :)

 Note I'm currently not on the mailing list, so please cc.

 Best regards,

 Aarni Koskela

All of the above do not belong to the commit log message.  Please
have these commentaries after the three-dash line you have under
your Signed-off-by line.


 From 53c12d9c9928dc93a57595e92d785ecc0b245390 Mon Sep 17 00:00:00 2001
 From: Aarni Koskela a...@iki.fi
 Date: Mon, 1 Dec 2014 11:06:10 +0200
 Subject: [PATCH] git-add--interactive: allow list (un)selection by regular
  expression

Remove these four lines when sending out a patch in the e-mail.

 Teach `list_and_choose` to allow `/regexp` and `-/regexp` syntax to
 select items based on regular expression match.

 For instance, `/jpg$` will select all options whose display name ends with
 `jpg`.

It has been a long time since I wrote this code, but is this about
the selection that happens after showing you a list of filenames to
choose from?  all options together with jpg made me go Huh?
Does any of our command have jpeg related options?  We are not in
the image processing business.

Perhaps s/all options/all files/ or something.

 Signed-off-by: Aarni Koskela a...@iki.fi
 ---
  git-add--interactive.perl | 27 +++
  1 file changed, 27 insertions(+)

 diff --git a/git-add--interactive.perl b/git-add--interactive.perl
 index 1fadd69..34cc603 100755
 --- a/git-add--interactive.perl
 +++ b/git-add--interactive.perl
 @@ -483,6 +483,8 @@ sub is_valid_prefix {
   !($prefix =~ /[\s,]/)  # separators
   !($prefix =~ /^-/) # deselection
   !($prefix =~ /^\d+/)   # selection
 + !($prefix =~ /^\//)# regexp selection
 + !($prefix =~ /^-\//)   # regexp unselection
   ($prefix ne '*')   # all wildcard
   ($prefix ne '?');# prompt help
  }
 @@ -585,6 +587,28 @@ sub list_and_choose {
   prompt_help_cmd();
   next TOPLOOP;
   }
 + if ($line =~ /^(-)*\/(.+)$/) {

You want zero or one '-', not zero or arbitrary large number of '-',
as a sign to unchoose.  (-)? instead of (-)*, perhaps?

 + my $choose = !($1  $1 eq '-');

The first $1 is an attempt to catch non-negative case where it is
undef; it might be more explicit this way?

my $choose = !(defined $1);

 + my $re = $2;

Mental note.  $re is an end-user supplied random string that may or
may not be a valid regular expression.

 + my $found = 0;
 + for ($i = 0; $i  @stuff; $i++) {
 + my $val = $stuff[$i];
 + my $ref = ref $val;
 + if ($ref eq 'ARRAY') {
 + $val = $val-[0];
 + }
 + elsif ($ref eq 'HASH') {
 + $val = $val-{VALUE};
 + }
 + if ($val =~ /$re/) {

... which makes me wonder what happens when $re is a bogus regular
expression here.  Does the program die?  Does the user get an error
message about bad regexp and the condition to this if expression is
false?  Something else?

Perhaps validating and catching regexp errors early like this might
be sufficient:

my $re = eval { qr/$2/ };
if (!$re) {
print STDERR $@\n;
next TOPLOOP;
}


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: [BUG] Documentation: git log: --exit-code undocumented?

2014-12-01 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 Hello,

 $ git help log | grep exit-code
problems are found. Not compatible with --exit-code.
 $

 What --exit-code does in git log?

 It doesn't.  That is why it is not listed.

Then, how can --check possibly interfer with it?

-- 
Sergey.
--
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: [BUG] Documentation: git log: --exit-code undocumented?

2014-12-01 Thread Junio C Hamano
Sergey Organov sorga...@gmail.com writes:

 Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 Hello,

 $ git help log | grep exit-code
problems are found. Not compatible with --exit-code.
 $

 What --exit-code does in git log?

 It doesn't.  That is why it is not listed.

 Then, how can --check possibly interfer with it?

The description is shared with git diff and friends, which is
invoked via git log -p.  As log does not give the exit code
of individual diff-tree invocation for each commit, --exit-code
option is irrelevant.
--
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: [BUG] Documentation: git log: --exit-code undocumented?

2014-12-01 Thread Sergey Organov
Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 Hello,

 $ git help log | grep exit-code
problems are found. Not compatible with --exit-code.
 $

 What --exit-code does in git log?

 It doesn't.  That is why it is not listed.

 Then, how can --check possibly interfer with it?

 The description is shared with git diff and friends, which is
 invoked via git log -p.  As log does not give the exit code
 of individual diff-tree invocation for each commit, --exit-code
 option is irrelevant.

Do you agree there is some minor problem here? First, git log
silently eats the --exit-code option that does nothing. Next, git-log
manual page doesn't include --exit-log description while refers to it
elsewhere. What's proposed resulution?

-- 
Sergey.
--
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] run-command.c: retire unused run_hook_with_custom_index()

2014-12-01 Thread Junio C Hamano
This was originally meant to be used to rewrite run_commit_hook()
that only special cases the GIT_INDEX_FILE environment, but the
run_hook_ve() refactoring done earlier made the implementation of
run_commit_hook() thin and clean enough.

Nobody uses this, so retire it as an unfinished clean-up made
unnecessary.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 run-command.c | 17 -
 run-command.h |  4 
 2 files changed, 21 deletions(-)

diff --git a/run-command.c b/run-command.c
index 35a3ebf..ae36852 100644
--- a/run-command.c
+++ b/run-command.c
@@ -792,20 +792,3 @@ int run_hook_le(const char *const *env, const char *name, 
...)
 
return ret;
 }
-
-int run_hook_with_custom_index(const char *index_file, const char *name, ...)
-{
-   const char *hook_env[3] =  { NULL };
-   char index[PATH_MAX];
-   va_list args;
-   int ret;
-
-   snprintf(index, sizeof(index), GIT_INDEX_FILE=%s, index_file);
-   hook_env[0] = index;
-
-   va_start(args, name);
-   ret = run_hook_ve(hook_env, name, args);
-   va_end(args);
-
-   return ret;
-}
diff --git a/run-command.h b/run-command.h
index ea73de3..9e8cd9c 100644
--- a/run-command.h
+++ b/run-command.h
@@ -53,10 +53,6 @@ LAST_ARG_MUST_BE_NULL
 extern int run_hook_le(const char *const *env, const char *name, ...);
 extern int run_hook_ve(const char *const *env, const char *name, va_list args);
 
-LAST_ARG_MUST_BE_NULL
-__attribute__((deprecated))
-extern int run_hook_with_custom_index(const char *index_file, const char 
*name, ...);
-
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD 2  /*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
-- 
2.2.0-133-g5229807

--
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: [BUG] Documentation: git log: --exit-code undocumented?

2014-12-01 Thread David Kastrup
Junio C Hamano gits...@pobox.com writes:

 Sergey Organov sorga...@gmail.com writes:

 Hello,

 $ git help log | grep exit-code
problems are found. Not compatible with --exit-code.
 $

 What --exit-code does in git log?

 It doesn't.  That is why it is not listed.

I disagree that --exit-code does nothing: it indicates whether the
listed log is empty.  So for example

git log -1 --exit-code a..b  /dev/null

can be used to figure out whether a is a proper ancestor of b or
not.

-- 
David Kastrup
--
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 24/34] checkout: reject if the branch is already checked out elsewhere

2014-12-01 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Sun, Nov 30, 2014 at 12:18:40PM -0500, Mark Levedahl wrote:
 On 11/30/2014 03:24 AM, Nguyễn Thái Ngọc Duy wrote:
  One branch obviously can't be checked out at two places (but detached
  heads are ok). Give the user a choice in this case: --detach, -b
  new-branch, switch branch in the other checkout first or simply 'cd'
  and continue to work there.
 
 
 This seems too restrictive and is not obvious to me: I currently use 
 git-new-workdir to have multiple checkouts of the same branch, with no 
 ill effect. While those who do not understand what is going on 
 underneath might be confused by one checkout suddenly showing 
 uncommitted diffs, I don't accept that as a reason to outright prevent 
 such use. I suggest, at the very least, that this behavior be overridden 
 by a --force flag?

 Prevention is a strong word. It's more about safety for the mere
 mortals. It's certainly possible to do something like this patch
 (--force can't be reused, it already carries some other meaning).
 Should I add this one in the next (hic) reroll?

Sorry, what is a hic?

If this were an existing feature like git-new-workdir, even though
it is from contrib, making it impossible to do something that used
to be possible, even if that something is what mere mortals would
never want to to to avoid risking confusion, would be a regression
that needs an escape hatch.

But this is a new feature.  I am not sure why you need to make this
overridable in the first place.  Those who want to have multiple
checkouts of the same commit can just detach HEAD at the same commit
in multiple working trees, as the first thing they need to do would
be to run git reset --hard $branch to synchronize the HEAD and the
working tree state to work in the other out-of-sync repositories
either case anyway.
--
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] compat: convert modes to use portable file type values

2014-12-01 Thread David Michael
On Mon, Dec 1, 2014 at 7:48 AM, David Michael fedora@gmail.com wrote:
 On Mon, Dec 1, 2014 at 12:55 AM, Torsten Bögershausen tbo...@web.de wrote:
 On 12/01/2014 04:40 AM, David Michael wrote:

 On Sun, Nov 30, 2014 at 3:16 PM, Torsten Bögershausen tbo...@web.de
 wrote:
 [snip]

 Could the code be more human-readable ?
 static inline mode_t mode_native_to_git(mode_t native_mode)
 {
  int perm_bits = native_mode  0;
  if (S_ISREG(native_mode))
  return 010 | perm_bits;
  if (S_ISDIR(native_mode))
  return 004 | perm_bits;
  if (S_ISLNK(native_mode))
  return 012 | perm_bits;
  if (S_ISBLK(native_mode))
  return 006 | perm_bits;
  if (S_ISCHR(native_mode))
  return 002 | perm_bits;
  if (S_ISFIFO(native_mode))
  return 001 | perm_bits;
  /* Non-standard type bits were given. */
  /* Shouldn't we die() here ?? */
  return perm_bits;
 }

 Sure, I can send an updated patch with the new variable and without the
 elses.

 Regarding the question in the last comment:  I was assuming if this
 case was ever reached, Git would handle the returned mode the same way
 as if it encountered an unknown/non-standard file type on a normal
 operating system, which could die() if needed in the function that
 called stat().

 Should I send an updated patch that die()s there?

 David

 Not yet, please wait with a V2 patch until I finished my thinking ;-)

 I take back the suggestion with the die(). I was thinking how to handle
 unforeseen types, which may show up on the z/OS some day,
 So die() is not a good idea, it is better to ignore them, what the code
 does.

 Knowing that Git does not track block devices, nor character devices nor
 sockets,
 the above code could be simplyfied even more, by mapping everything which
 is not a directory, a file or a softlink to device type 0)

 This is just a suggestion, I want to here from others as well:

 int perm_bits = native_mode  0;
 if (S_ISREG(native_mode))
 return 010 | perm_bits;
 if (S_ISDIR(native_mode))
 return 004 | perm_bits;
 if (S_ISLNK(native_mode))
 return 012 | perm_bits;
 /* Git does not track S_IFCHR, S_IFBLK, S_IFIFO, S_IFSOCK  */
 return perm_bits;

 I had considered omitting those three as well at first, but in this
 case it would mean that they will be unusable all together.

 The z/OS S_IFMT definition (i.e. the file type bit mask) is
 0xFF00, and the common/translated S_IFMT definition is 0xF000.
 Since the S_ISxxx macros use the typical ((mode  S_IFMT) == S_IFxxx)
 definition, they would never match a native z/OS mode after redefining
 S_IFMT.

 So translating those types isn't just for tracking files, it's to
 support any use of S_ISxxx anywhere in the code.  It should be okay to
 remove any of those types if we know that Git will never need to use
 them.

Apologies, in my pre-coffee reply, I confused myself into thinking the
omitted types were going to be returned unchanged as opposed to being
changed to zero.  That second paragraph is irrelevant.

But regarding the last paragraph: a quick grep for instances of using
those file types in the Git source found S_ISFIFO and S_ISSOCK in
git.c.

I just noticed that I copied the list of standard file type macros
from SUSv2, and S_IFSOCK was added after that.  z/OS does implement
S_IFSOCK, so I think I should add it to the v2 patch to support the
test in git.c.

Another grep found no instances of testing for block or character
devices, so it should be okay to remove those from the patch if Git is
unlikely to use them in the future (unless we just want to provide all
7 types from 
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/sys/stat.h.html
).

David
--
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] compat: convert modes to use portable file type values

2014-12-01 Thread David Michael
On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote:
 On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote:
 +int git_stat(const char *path, struct stat *buf)
 +{
 +   int rc;
 +   rc = stat(path, buf);
 +   if (buf != NULL)

 It's a minor thing, but maybe test !rc instead of buf != NULL?

Okay, it makes sense to only do the conversion for a successful return code.

Should it test for both a zero return code and a non-null pointer?  I
don't know if there are any cases where passing a null pointer is
legal.  The standard doesn't seem to explicitly forbid it.  z/OS
returns -1 and sets errno to EFAULT when stat() is given NULL, but
this patch should be able to be used on any platform.

David
--
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] compat: convert modes to use portable file type values

2014-12-01 Thread Junio C Hamano
David Michael fedora@gmail.com writes:

 On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote:
 On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote:
 +int git_stat(const char *path, struct stat *buf)
 +{
 +   int rc;
 +   rc = stat(path, buf);
 +   if (buf != NULL)

 It's a minor thing, but maybe test !rc instead of buf != NULL?

 Okay, it makes sense to only do the conversion for a successful return code.

 Should it test for both a zero return code and a non-null pointer?  I
 don't know if there are any cases where passing a null pointer is
 legal.  The standard doesn't seem to explicitly forbid it.  z/OS
 returns -1 and sets errno to EFAULT when stat() is given NULL, but
 this patch should be able to be used on any platform.

Huh?  I am confused.  Since when is it legal to give NULL as statbuf
to (l)stat(2)?

Wouldn't something like this be sufficient and necessary?

int rc = stat(path, buf);
if (rc)
return rc;

That is, let the underlying stat(2) diagnose any and all problems
(and leave clues in errno) and parrot its return value to the caller
to signal the failure?
--
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: Deprecation warnings under XCode

2014-12-01 Thread Junio C Hamano
Torsten Bögershausen tbo...@web.de writes:

 On 12/01/2014 04:02 AM, Michael Blume wrote:
 I have no idea whether this should concern anyone, but my mac build of git 
 shows

  CC imap-send.o
 imap-send.c:183:36: warning: 'ERR_error_string' is deprecated: first
 deprecated in OS X 10.7 [-Wdeprecated-declarations]
  fprintf(stderr, %s: %s\n, func,
 ERR_error_string(ERR_get_error(), NULL));
^
 []
 Isn't the warning a warning ;-)
 I don't see this warnings because my openssl comes from
 /opt/local/include (Mac ports)
 Does anybody know which new functions exist in Mac OS X versions = 10.7  ?

I am not a Mac person, but is this about APPLE_COMMON_CRYPTO support
added in 4dcd7732 (Makefile: add support for Apple CommonCrypto
facility, 2013-05-19) and be4c828b (imap-send: eliminate HMAC
deprecation warnings on Mac OS X, 2013-05-19)?  Specifically, the
log message for 4dcd7732 begins like so:

Makefile: add support for Apple CommonCrypto facility

As of Mac OS X 10.7, Apple deprecated all OpenSSL functions due to
OpenSSL ABI instability, thus leading to build warnings.  As a
replacement, Apple encourages developers to migrate to its own (stable)
CommonCrypto facility.

In the Makefile we seem to have this:

# Define NO_APPLE_COMMON_CRYPTO if you are building on Darwin/Mac OS X
# and do not want to use Apple's CommonCrypto library.  This allows you
# to provide your own OpenSSL library, for example from MacPorts.

which makes it sound like using APPLE_COMMON_CRYPTO is the default
for Mac.  Perhaps those who do want to use CommonCrypto to avoid
warnings should not define that macro?
--
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: [BUG] Documentation: git log: --exit-code undocumented?

2014-12-01 Thread David Kastrup
Junio C Hamano gits...@pobox.com writes:

 David Kastrup d...@gnu.org writes:

 I disagree that --exit-code does nothing: it indicates whether the
 listed log is empty.  So for example

 git log -1 --exit-code a..b  /dev/null

 can be used to figure out whether a is a proper ancestor of b or
 not.

 Hmph.

 $ git log --exit-code master..maint /dev/null; echo $?
 0
 $ git log --exit-code maint..master /dev/null; echo $?
 1

 That is a strange way to use --exit-code.  I suspect that if you did
 this, you will get 0 from the log between HEAD~..HEAD

 $ git checkout master^0
 $ git commit --allow-empty -m empty
 $ git log --exit-code HEAD~..HEAD

 even though HEAD~ is a proper ancestor of HEAD, so it is not giving
 us anything useful.  Isn't it a mere artifact that log happens to
 share the underlying machinery with diff that --exit-code shows a
 non-zero exit when there is any single commit in the range that has
 any change?

Possibly: I haven't checked the underlying code for the details.  At any
rate, it is an option git log accepts for whatever reason.

-- 
David Kastrup
--
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: Bug in reflog of length 0x2BFF

2014-12-01 Thread Stefan Beller
So I am running a 3.13.0-40-generic x86_64 linux (so its's amd64) and
git version 2.1.2
and I cannot reproduce the bug you are describing. :(

$ git rev-parse 'master@{52}'
0035

What I noticed though is there are 2 linefeeds at the end of each
line, is that intended or did it break during transmission?

Stefan
--
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] fetch: add a config option to always use the depth argument

2014-12-01 Thread Stefan Beller
When having a repository, which deals with large amounts of data, i.e.
graphics, music, films, you may want to keep the git directory at
the smallest size possible.

The depth option helped us in achieving this goal by removing the sizable
history and just keep recent history around. In the case of having large
amounts of data around, you probably want to use the depth option at any
fetch you do, so it would be convenient to have an option for this.

Change-Id: I45a569239639f20e24fbae32fb2046bc478c5f07
Signed-off-by: Stefan Beller sbel...@google.com
---
 Documentation/config.txt| 6 ++
 Documentation/fetch-options.txt | 2 ++
 builtin/fetch.c | 5 +
 3 files changed, 13 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9220725..418e21f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1106,6 +1106,12 @@ fetch.prune::
If true, fetch will automatically behave as if the `--prune`
option was given on the command line.  See also `remote.name.prune`.
 
+fetch.depth::
+   If set, fetch will automatically behave as if the `--depth`
+   option was given on the command line.  This allows users to keep
+   the git directory at low space requirements, and thus comes in handy
+   for users with large binary files in the repository.
+
 format.attach::
Enable multipart/mixed attachments as the default for
'format-patch'.  The value can also be a double quoted string
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index b09a783..81131d0 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -12,6 +12,8 @@
`git clone` with `--depth=depth` option (see linkgit:git-clone[1])
to the specified number of commits from the tip of each remote
branch history. Tags for the deepened commits are not fetched.
+   You can configure git to always use the depth option, see
+   `fetch.depth` in linkgit:git-config[1]
 
 --unshallow::
If the source repository is complete, convert a shallow
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7b84d35..30fa15b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -68,6 +68,11 @@ static int git_fetch_config(const char *k, const char *v, 
void *cb)
fetch_prune_config = git_config_bool(k, v);
return 0;
}
+   if (!strcmp(k, fetch.depth)) {
+   if (git_config_string(depth, k, v))
+   return -1;
+   return 0;
+   }
return git_default_config(k, v, cb);
 }
 
-- 
2.1.2

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


Re: [PATCH] compat: convert modes to use portable file type values

2014-12-01 Thread David Michael
On Mon, Dec 1, 2014 at 12:57 PM, Junio C Hamano gits...@pobox.com wrote:
 David Michael fedora@gmail.com writes:

 On Mon, Dec 1, 2014 at 9:44 AM, Duy Nguyen pclo...@gmail.com wrote:
 On Sun, Nov 30, 2014 at 9:41 AM, David Michael fedora@gmail.com wrote:
 +int git_stat(const char *path, struct stat *buf)
 +{
 +   int rc;
 +   rc = stat(path, buf);
 +   if (buf != NULL)

 It's a minor thing, but maybe test !rc instead of buf != NULL?

 Okay, it makes sense to only do the conversion for a successful return code.

 Should it test for both a zero return code and a non-null pointer?  I
 don't know if there are any cases where passing a null pointer is
 legal.  The standard doesn't seem to explicitly forbid it.  z/OS
 returns -1 and sets errno to EFAULT when stat() is given NULL, but
 this patch should be able to be used on any platform.

 Huh?  I am confused.  Since when is it legal to give NULL as statbuf
 to (l)stat(2)?

 Wouldn't something like this be sufficient and necessary?

 int rc = stat(path, buf);
 if (rc)
 return rc;

 That is, let the underlying stat(2) diagnose any and all problems
 (and leave clues in errno) and parrot its return value to the caller
 to signal the failure?

Alright, it wasn't immediately clear to me from the OpenGroup page on
stat() if that would always be safe.  I will just test the return code
in v2.

Thanks.

David
--
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] tree.c: update read_tree_recursive callback to pass strbuf as base

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

 This allows the callback to use 'base' as a temporary buffer to
 quickly assemble full path without extra allocation. The callback
 has to restore it afterwards of course.

Hmph, what's the quote around 'without' doing there?

--
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] ls-tree: disable negative pathspec because it's not supported

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

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com

Hmph, that's sad.  Should the below say test_expect_failure
without test_must_fail, anticipating a fix later?

 t/t3102-ls-tree-wildcards.sh | 4 
 1 file changed, 4 insertions(+)

diff --git a/t/t3102-ls-tree-wildcards.sh b/t/t3102-ls-tree-wildcards.sh
index 83fca8d..93127a0 100755
--- a/t/t3102-ls-tree-wildcards.sh
+++ b/t/t3102-ls-tree-wildcards.sh
@@ -27,4 +27,8 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success 'ls-tree rejects negated pathspec' '
+   test_must_fail git ls-tree -r HEAD :(exclude)a a*
+'
+
 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


Re: [PATCH 00/19] Add git-list-files

2014-12-01 Thread Junio C Hamano
Does this contain a lot of borrowed code or something?  The style
violation in the patches are unusually high, even compared with your
other series.

I've tried to fix it up and will push out the result on 'pu' if
things work OK, but this does not even have tests, so it is unlikely
that it would break anything but itself ;-)

--
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] compat: convert modes to use portable file type values

2014-12-01 Thread Junio C Hamano
David Michael fedora@gmail.com writes:

 Huh?  I am confused.  Since when is it legal to give NULL as statbuf
 to (l)stat(2)?

 Wouldn't something like this be sufficient and necessary?

 int rc = stat(path, buf);
 if (rc)
 return rc;

 That is, let the underlying stat(2) diagnose any and all problems
 (and leave clues in errno) and parrot its return value to the caller
 to signal the failure?

 Alright, it wasn't immediately clear to me from the OpenGroup page on
 stat() if that would always be safe.  I will just test the return code
 in v2.

It is irrelevant if that is safe or not.  As long as we are
emulating the underlying stat(), whether it is unsafe for stat(), we
should just throw whatever the user throws at us at the underlying
stat() and let it behave as if our emulation layer were not there.

Which is what the above snippet does.
--
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] fetch: add a config option to always use the depth argument

2014-12-01 Thread Junio C Hamano
Stefan Beller sbel...@google.com writes:

 When having a repository, which deals with large amounts of data, i.e.
 graphics, music, films, you may want to keep the git directory at
 the smallest size possible.

 The depth option helped us in achieving this goal by removing the sizable
 history and just keep recent history around. In the case of having large
 amounts of data around, you probably want to use the depth option at any
 fetch you do, so it would be convenient to have an option for this.

 Change-Id: I45a569239639f20e24fbae32fb2046bc478c5f07
 Signed-off-by: Stefan Beller sbel...@google.com
 ---
  Documentation/config.txt| 6 ++
  Documentation/fetch-options.txt | 2 ++
  builtin/fetch.c | 5 +
  3 files changed, 13 insertions(+)

 diff --git a/Documentation/config.txt b/Documentation/config.txt
 index 9220725..418e21f 100644
 --- a/Documentation/config.txt
 +++ b/Documentation/config.txt
 @@ -1106,6 +1106,12 @@ fetch.prune::
   If true, fetch will automatically behave as if the `--prune`
   option was given on the command line.  See also `remote.name.prune`.
  
 +fetch.depth::
 + If set, fetch will automatically behave as if the `--depth`
 + option was given on the command line.  This allows users to keep
 + the git directory at low space requirements, and thus comes in handy
 + for users with large binary files in the repository.
 +

Hmm, is this something a user would typically want repository-wide?
I am wondering if remote.$nick.fetchDepth or something scoped to
remote is more appropriate.

  format.attach::
   Enable multipart/mixed attachments as the default for
   'format-patch'.  The value can also be a double quoted string
 diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
 index b09a783..81131d0 100644
 --- a/Documentation/fetch-options.txt
 +++ b/Documentation/fetch-options.txt
 @@ -12,6 +12,8 @@
   `git clone` with `--depth=depth` option (see linkgit:git-clone[1])
   to the specified number of commits from the tip of each remote
   branch history. Tags for the deepened commits are not fetched.
 + You can configure git to always use the depth option, see
 + `fetch.depth` in linkgit:git-config[1]
  
  --unshallow::
   If the source repository is complete, convert a shallow
 diff --git a/builtin/fetch.c b/builtin/fetch.c
 index 7b84d35..30fa15b 100644
 --- a/builtin/fetch.c
 +++ b/builtin/fetch.c
 @@ -68,6 +68,11 @@ static int git_fetch_config(const char *k, const char *v, 
 void *cb)
   fetch_prune_config = git_config_bool(k, v);
   return 0;
   }
 + if (!strcmp(k, fetch.depth)) {
 + if (git_config_string(depth, k, v))
 + return -1;
 + return 0;
 + }
   return git_default_config(k, v, cb);
  }
--
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] On watchman support

2014-12-01 Thread David Turner
On Fri, 2014-11-28 at 18:13 +0700, Duy Nguyen wrote:
 On Wed, Nov 19, 2014 at 1:12 AM, David Turner dtur...@twopensource.com 
 wrote:
  Or will you go
  with cityhash now.. I ask because you have another sse optimization
  for hashmap on your watchman branch and that could reduce init time
  for name-hash. Name-hash is used often on case-insensitive fs (less
  often on case-sensitive fs).
 
  Cityhash would be better, because it has actual engineering effort put
  into it; what I did on my branch is a hack that happens to work
  decently.  As the comment notes, I did not spend much effort on tuning
  my implementation.  Also, Cityhash doesn't require SSE, so it's more
  portable.
 
 Cityhash looks less appealing to me. For one thing it's C++ so linking
 to C can't be done. I could add a few extern C to make it work.
 But if we plan to support it eventually, cityhash must support C out
 of the box.
 
 Then cityhash does not support case-insensitive hashing. I had to make
 a CityHash32i version based on CityHash32. It's probably my bugs
 there, but performance is worse (~120ms) than original hashmap.c
 (90ms). Enabling sse4.2 helps a bit, but still worse. Using the
 case-sensitive version in place for memihash and strihash does make
 cityhash win over hashmap.c, around 50ms (with or without sse4.2). But
 that's still not as good as your version (~35ms)..

Can you post your CityHash32i?  

Have you tried this C port of Cityhash?

https://github.com/santeri-io/cityhash-c


--
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] fetch: add a config option to always use the depth argument

2014-12-01 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Stefan Beller sbel...@google.com writes:

 +fetch.depth::
 +If set, fetch will automatically behave as if the `--depth`
 +option was given on the command line.  This allows users to keep
 +the git directory at low space requirements, and thus comes in handy
 +for users with large binary files in the repository.
 +

 Hmm, is this something a user would typically want repository-wide?
 I am wondering if remote.$nick.fetchDepth or something scoped to
 remote is more appropriate.

Regardless of what configuration variable is used, I think setting
depth directly from the config will mean the user cannot defeat a
configured value by passing --unshallow because of this code
sequence in builtin/fetch.c; am I mistaken?

...
git_config(git_fetch_config, NULL);
argc = parse_options(argc, argv, prefix,
 builtin_fetch_options, builtin_fetch_usage, 0);
if (unshallow) {
if (depth)
die(_(--depth and --unshallow cannot be used 
together));
...

I think depth variable should be left alone.

The right solution may be to leave the above unshallow and depth
are incompatible check done only for the command line options as
the original code, and much later in the code path after you figure
out which remote we are talking to, only when neither --depth nor
--unshallow is given, consult the configuration system to see if
there is a default, perhaps in prepare_transport().  That approach
will let you later support remote.$nick.fetchDepth, even if you
start with a repository-wide fetch.depth option, I would think.

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


Re: Bug in reflog of length 0x2BFF

2014-12-01 Thread Christoph Mallon
Am 01.12.14 19:53, schrieb Stefan Beller:
 So I am running a 3.13.0-40-generic x86_64 linux (so its's amd64) and
 git version 2.1.2
 and I cannot reproduce the bug you are describing. :(

):

I can reproduce it with
* OS X, i386 binary, git 2.2.0
* FreeBSD, amd64, git 2.1.0 and up (bisected it there)
* FreeBSD, amd64, git 2.1.2 (different machine)

I cannot reproduce it with
* Linux, amd64, git 2.1.0

 $ git rev-parse 'master@{52}'
 0035

On a machine, where you see the bug, you get entry /0...036/.
This btw causes havoc:
git stash list shows all entries, but e.g. git stash drop drops the
wrong stash after @{52}.

 What I noticed though is there are 2 linefeeds at the end of each
 line, is that intended or did it break during transmission?

That broke.
It should be a normal reflog file.
Try this:
http://tron.yamagi.org/zeug/reflog.bad

Still 4207ed285f31ad3e04f08254237c0c1a1609642b seems a plausible cause,
because it's about reflogs.
Though I suspect the actual bug was introduced before, because this
commit only uses machinery, which was added earlier.

Christoph
--
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: Bug in reflog of length 0x2BFF

2014-12-01 Thread Jonathan Nieder
Hi Christoph,

Christoph Mallon wrote:

 % git rev-parse 'master@{52}'
 warning: Log for ref refs/heads/master has gap after Thu, 1 Jan 1970 00:00:01 
 +.
 0036

Can you say more?  What output did you expect and how does this differ
from it?

I tried, with git 2.2.0,

git init gitbug 
cd gitbug 
git commit --allow-empty -m a 
wget http://tron.yamagi.org/zeug/reflog.bad 
mv reflog.bad .git/logs/refs/heads/master 
sha1sum .git/logs/refs/heads/master 
git rev-parse 'master@{52}'

The output:

 9ffe44715d0e542a60916255f144c74e6760ffd0  .git/logs/refs/heads/master
 0035

Could you make a test script that illustrates and reproduces the
problem?  I.e., a patch to a file like t/t1410-reflog.sh, such that
if I run

cd git
make
cd t
./t1410-reflog.sh

then I can reproduce the bug?

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 v5] Add another option for receive.denyCurrentBranch

2014-12-01 Thread Junio C Hamano
Johannes Schindelin johannes.schinde...@gmx.de writes:

 +static const char *update_worktree(unsigned char *sha1)
 +{
 +...
 + const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : ..;

I overlooked this one, but is there a reason why this has to look at
an internal implementatino detail which is git_work_tree_cfg,
instead of asking get_git_work_tree()?

I am wondering if that reason is a valid rationale to fix whatever
makes get_git_work_tree() unsuitable for this purpose.

Cc'ing Duy who has been working on the part of the system to
determine and set-up work-tree and git-dir.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v5] Add another option for receive.denyCurrentBranch

2014-12-01 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Johannes Schindelin johannes.schinde...@gmx.de writes:

 +static const char *update_worktree(unsigned char *sha1)
 +{
 +...
 +const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : ..;

 I overlooked this one, but is there a reason why this has to look at
 an internal implementatino detail which is git_work_tree_cfg,
 instead of asking get_git_work_tree()?

 I am wondering if that reason is a valid rationale to fix whatever
 makes get_git_work_tree() unsuitable for this purpose.

 Cc'ing Duy who has been working on the part of the system to
 determine and set-up work-tree and git-dir.

Answering myself...

This is because you know receive-pack runs inside the $GIT_DIR,
whether it is a bare or non-bare repository, so either core.worktree
points at a directory that is otherwise unrelated to the $GIT_DIR
(but is the correct $GIT_WORK_TREE), or the top of the working tree
must be .. for a non-bare repository.  I haven't checked the code,
but we probably are not even doing the repository/working-tree
discovery to set up the data necessary for get_git_work_tree() to
give us a correct answer.

Doing an optional set-up to make get_git_work_tree() work would
involve more damage to the codebase than necessary, I would suspect,
so let's keep this part of the code as-is.

That is, unless I am over-estimating the damage necessary.

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: Bug in reflog of length 0x2BFF

2014-12-01 Thread Stefan Beller
Hi,

so I have installed git version 2.2.0 currently, because I was trying to
reproduce Bug in reflog of length 0x2BFF.

Now I was using git as a normal user, rebasing some stuff
(incidentally the reflog improvements)
and an error occurred:
$ git rebase --continue
error: Ref refs/heads/todo_sb13_ref-transactions-reflog-as-file is at
c48602d0aaa11b9099440c647ac028604fde4b14 but expected
d1bbdc6f161ae7550805d565cad1d930487dad34
fatal: update_ref failed for ref
'refs/heads/todo_sb13_ref-transactions-reflog-as-file': Cannot lock
the ref 'refs/heads/todo_sb13_ref-transactions-reflog-as-file'.

So I think we definitely have a bug in the reflog code already in version 2.2.0.

Trying to reproduce the error did not yield success.

Thanks,
Stefan
--
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 3/4] refs.c: add a transaction function to append a reflog entry

2014-12-01 Thread Stefan Beller
When performing a reflog transaction update, only write to the reflog iff
msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform
an update that only truncates but does not write. This change only affects
whether or not a reflog entry should be generated and written. If msg == NULL
then no such entry will be written.

Orthogonal to this we have a boolean flag REFLOG_TRUNCATE which is used to
tell the transaction system to truncate the reflog and thus discard all
previous users.

At the current time the only place where we use msg == NULL is also the
place, where we use REFLOG_TRUNCATE. Even though these two settings are
currently only ever used together it still makes sense to have them through
two separate knobs.

This allows future consumers of this API that may want to do things
differently. For example someone can do:
  msg=Reflog truncated by Bob because ... + REFLOG_TRUNCATE
and have it truncate the log and have it start fresh with an initial message
that explains the log was truncated. This API allows that.

During one transaction we allow to make multiple reflog updates to the
same ref. This means we only need to lock the reflog once, during the first
update that touches the reflog, and that all further updates can just write the
reflog entry since the reflog is already locked.

This allows us to write code such as:

  t = transaction_begin()
  transaction_reflog_update(t, foo, REFLOG_TRUNCATE, NULL);
  loop-over-something...
 transaction_reflog_update(t, foo, 0, message);
  transaction_commit(t)

where we first truncate the reflog and then build the new content one line
at a time.

Change-Id: I83923b2dcfa29aadb86a942826060180ac6f3d07
Signed-off-by: Stefan Beller sbel...@google.com
---
 refs.c | 112 +++--
 refs.h |  21 +
 2 files changed, 131 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 58de60c..d0d2be7 100644
--- a/refs.c
+++ b/refs.c
@@ -3557,6 +3557,7 @@ struct transaction {
struct ref_update **ref_updates;
size_t alloc;
size_t nr;
+   struct string_list reflog_updates;
enum transaction_state state;
 };
 
@@ -3564,7 +3565,10 @@ struct transaction *transaction_begin(struct strbuf *err)
 {
assert(err);
 
-   return xcalloc(1, sizeof(struct transaction));
+   struct transaction *ret = xcalloc(1, sizeof(struct transaction));
+   string_list_init(ret-reflog_updates, 1);
+
+   return ret;
 }
 
 void transaction_free(struct transaction *transaction)
@@ -3629,6 +3633,102 @@ int transaction_update_ref(struct transaction 
*transaction,
return 0;
 }
 
+/* Returns a non null fd, 0 on error. */
+static int get_reflog_updates_fd(struct transaction *transaction,
+ const char *refname)
+{
+   char *path;
+   struct lock_file *lock;
+   struct string_list_item *item = string_list_insert(
+   transaction-reflog_updates,
+   refname);
+   if (!item-util) {
+   item-util = xcalloc(1, sizeof(struct lock_file));
+   lock = item-util;
+   path = git_path(logs/locks/%s, refname);
+   if (safe_create_leading_directories(path)  0)
+   return 0;
+
+   if (hold_lock_file_for_update(lock, path, 0)  0)
+   return 0;
+   }
+
+   lock = item-util;
+
+   return lock-fd;
+}
+
+int transaction_update_reflog(struct transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ const char *email,
+ unsigned long timestamp, int tz,
+ const char *msg, int flags,
+ struct strbuf *err)
+{
+   /*
+* We cannot handle reflogs the same way, we handle refs.
+*
+* Naming conflicts in the file system
+*   If renaming a ref from foo/foo to foo or the other way round,
+*   we need to be careful as we need the basic foo/ from being a
+*   directory to be a file or vice versa. When handling the refs
+*   this can be solved easily by first recording all we want into
+*   the packed refs file and then deleting all the loose refs. By
+*   doing it that way, we always have a valid state on disk.
+*
+*   We don't have an equivalent of a packed refs file when dealing
+*   with reflog updates, but the API for updating the refs turned
+*   out to be conveniant, so let's introduce an intermediate file
+*   outside the $GIT_DIR/logs/refs/heads/ directory helping us
+*   avoiding this naming conflict for the reflogs. The intermediate
+*   lock file, in which we 

[PATCH 2/4] refs.c: rename transaction.updates to ref_updates

2014-12-01 Thread Stefan Beller
The updates are only holding refs not reflogs, so express it to the reader.

Change-Id: Ifdc87722f0e7314f3d0febb970aa7769eada6d33
Signed-off-by: Stefan Beller sbel...@google.com
---
 refs.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index f0f0d23..58de60c 100644
--- a/refs.c
+++ b/refs.c
@@ -3554,7 +3554,7 @@ enum transaction_state {
  * as atomically as possible.  This structure is opaque to callers.
  */
 struct transaction {
-   struct ref_update **updates;
+   struct ref_update **ref_updates;
size_t alloc;
size_t nr;
enum transaction_state state;
@@ -3575,10 +3575,10 @@ void transaction_free(struct transaction *transaction)
return;
 
for (i = 0; i  transaction-nr; i++) {
-   free(transaction-updates[i]-msg);
-   free(transaction-updates[i]);
+   free(transaction-ref_updates[i]-msg);
+   free(transaction-ref_updates[i]);
}
-   free(transaction-updates);
+   free(transaction-ref_updates);
free(transaction);
 }
 
@@ -3589,8 +3589,8 @@ static struct ref_update *add_update(struct transaction 
*transaction,
struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);
 
strcpy((char *)update-refname, refname);
-   ALLOC_GROW(transaction-updates, transaction-nr + 1, 
transaction-alloc);
-   transaction-updates[transaction-nr++] = update;
+   ALLOC_GROW(transaction-ref_updates, transaction-nr + 1, 
transaction-alloc);
+   transaction-ref_updates[transaction-nr++] = update;
return update;
 }
 
@@ -3712,7 +3712,7 @@ int transaction_commit(struct transaction *transaction,
int ret = 0, delnum = 0, i;
const char **delnames;
int n = transaction-nr;
-   struct ref_update **updates = transaction-updates;
+   struct ref_update **updates = transaction-ref_updates;
 
assert(err);
 
-- 
2.2.0

--
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 4/4] reflog.c: use a reflog transaction when writing during expire

2014-12-01 Thread Stefan Beller
From: Ronnie Sahlberg sahlb...@google.com

Use a transaction for all updates during expire_reflog.

Change-Id: Ieb81b2660cefeeecf0bcb3cdbc1ef3cbb86e7eb8
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Stefan Beller sbel...@google.com
---
 builtin/reflog.c | 85 
 1 file changed, 37 insertions(+), 48 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 2d85d26..6bb7454 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -32,8 +32,11 @@ struct cmd_reflog_expire_cb {
int recno;
 };
 
+static struct strbuf err = STRBUF_INIT;
+
 struct expire_reflog_cb {
-   FILE *newlog;
+   struct transaction *t;
+   const char *refname;
enum {
UE_NORMAL,
UE_ALWAYS,
@@ -316,20 +319,18 @@ static int expire_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
if (cb-cmd-recno  --(cb-cmd-recno) == 0)
goto prune;
 
-   if (cb-newlog) {
-   char sign = (tz  0) ? '-' : '+';
-   int zone = (tz  0) ? (-tz) : tz;
-   fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
-   sha1_to_hex(osha1), sha1_to_hex(nsha1),
-   email, timestamp, sign, zone,
-   message);
+   if (cb-t) {
+   if (transaction_update_reflog(cb-t, cb-refname, nsha1, osha1,
+ email, timestamp, tz, message, 0,
+ err))
+   return -1;
hashcpy(cb-last_kept_sha1, nsha1);
}
if (cb-cmd-verbose)
printf(keep %s, message);
return 0;
  prune:
-   if (!cb-newlog)
+   if (!cb-t)
printf(would prune %s, message);
else if (cb-cmd-verbose)
printf(prune %s, message);
@@ -353,29 +354,26 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
 {
struct cmd_reflog_expire_cb *cmd = cb_data;
struct expire_reflog_cb cb;
-   struct ref_lock *lock;
-   char *log_file, *newlog_path = NULL;
struct commit *tip_commit;
struct commit_list *tips;
int status = 0;
 
memset(cb, 0, sizeof(cb));
+   cb.refname = ref;
 
-   /*
-* we take the lock for the ref itself to prevent it from
-* getting updated.
-*/
-   lock = lock_any_ref_for_update(ref, sha1, 0, NULL);
-   if (!lock)
-   return error(cannot lock ref '%s', ref);
-   log_file = git_pathdup(logs/%s, ref);
if (!reflog_exists(ref))
goto finish;
-   if (!cmd-dry_run) {
-   newlog_path = git_pathdup(logs/%s.lock, ref);
-   cb.newlog = fopen(newlog_path, w);
+   cb.t = transaction_begin(err);
+   if (!cb.t) {
+   status |= error(%s, err.buf);
+   goto cleanup;
+   }
+   if (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1,
+ NULL, 0, 0, NULL, REFLOG_TRUNCATE,
+ err)) {
+   status |= error(%s, err.buf);
+   goto cleanup;
}
-
cb.cmd = cmd;
 
if (!cmd-expire_unreachable || !strcmp(ref, HEAD)) {
@@ -407,7 +405,10 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
mark_reachable(cb);
}
 
-   for_each_reflog_ent(ref, expire_reflog_ent, cb);
+   if (for_each_reflog_ent(ref, expire_reflog_ent, cb)) {
+   status |= error(%s, err.buf);
+   goto cleanup;
+   }
 
if (cb.unreachable_expire_kind != UE_ALWAYS) {
if (cb.unreachable_expire_kind == UE_HEAD) {
@@ -420,32 +421,20 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
}
}
  finish:
-   if (cb.newlog) {
-   if (fclose(cb.newlog)) {
-   status |= error(%s: %s, strerror(errno),
-   newlog_path);
-   unlink(newlog_path);
-   } else if (cmd-updateref 
-   (write_in_full(lock-lock_fd,
-   sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock-lock_fd, \n) != 1 ||
-close_ref(lock)  0)) {
-   status |= error(Couldn't write %s,
-   lock-lk-filename.buf);
-   unlink(newlog_path);
-   } else if (rename(newlog_path, log_file)) {
-   status |= error(cannot rename %s to %s,
-   newlog_path, log_file);
-   unlink(newlog_path);
-   } else if (cmd-updateref  

[PATCH 1/4] refs.c: rename the transaction functions

2014-12-01 Thread Stefan Beller
From: Ronnie Sahlberg sahlb...@google.com

Rename the transaction functions. Remove the leading ref_ from the
names and append _ref to the names for functions that create/delete/
update sha1 refs.

todo(sbeller): notes below 3 dashes
This commit can be reproduced via
find . -name *.[ch] -print | xargs sed -i ' \
s/REF_TRANSACTION_OPEN/TRANSACTION_OPEN/; \
s/REF_TRANSACTION_CLOSED/TRANSACTION_CLOSED/; \
s/ref_transaction_begin/transaction_begin/; \
s/ref_transaction_commit/transaction_commit/; \
s/ref_transaction_create/transaction_create_ref/; \
s/ref_transaction_delete/transaction_delete_ref/; \
s/ref_transaction_free/transaction_free/; \
s/ref_transaction_update/transaction_update_ref/; \
s/ref_transaction/transaction/'
modulo white space changes for alignment.

Change-Id: Iffdc56536be71c5061caa23040ce0d89efd52196
Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Jonathan Nieder jrnie...@gmail.com
Signed-off-by: Stefan Beller sbel...@google.com
---
 branch.c   | 13 +
 builtin/commit.c   | 10 +++
 builtin/fetch.c| 12 
 builtin/receive-pack.c | 13 -
 builtin/replace.c  | 10 +++
 builtin/tag.c  | 10 +++
 builtin/update-ref.c   | 26 -
 fast-import.c  | 22 +++---
 refs.c | 78 +-
 refs.h | 36 +++
 sequencer.c| 12 
 walker.c   | 10 +++
 12 files changed, 126 insertions(+), 126 deletions(-)

diff --git a/branch.c b/branch.c
index 4bab55a..c8462de 100644
--- a/branch.c
+++ b/branch.c
@@ -279,16 +279,17 @@ void create_branch(const char *head,
log_all_ref_updates = 1;
 
if (!dont_change_ref) {
-   struct ref_transaction *transaction;
+   struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
 
-   transaction = ref_transaction_begin(err);
+   transaction = transaction_begin(err);
if (!transaction ||
-   ref_transaction_update(transaction, ref.buf, sha1,
-  null_sha1, 0, !forcing, msg, err) ||
-   ref_transaction_commit(transaction, err))
+   transaction_update_ref(transaction, ref.buf, sha1,
+  null_sha1, 0, !forcing, msg,
+  err) ||
+   transaction_commit(transaction, err))
die(%s, err.buf);
-   ref_transaction_free(transaction);
+   transaction_free(transaction);
strbuf_release(err);
}
 
diff --git a/builtin/commit.c b/builtin/commit.c
index e108c53..f50b7df 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
struct stat statbuf;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
-   struct ref_transaction *transaction;
+   struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
 
if (argc == 2  !strcmp(argv[1], -h))
@@ -1804,17 +1804,17 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg));
strbuf_insert(sb, strlen(reflog_msg), : , 2);
 
-   transaction = ref_transaction_begin(err);
+   transaction = transaction_begin(err);
if (!transaction ||
-   ref_transaction_update(transaction, HEAD, sha1,
+   transaction_update_ref(transaction, HEAD, sha1,
   current_head
   ? current_head-object.sha1 : NULL,
   0, !!current_head, sb.buf, err) ||
-   ref_transaction_commit(transaction, err)) {
+   transaction_commit(transaction, err)) {
rollback_index_files();
die(%s, err.buf);
}
-   ref_transaction_free(transaction);
+   transaction_free(transaction);
 
unlink(git_path(CHERRY_PICK_HEAD));
unlink(git_path(REVERT_HEAD));
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7b84d35..0be0b09 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -404,7 +404,7 @@ static int s_update_ref(const char *action,
 {
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
-   struct ref_transaction *transaction;
+   struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
int ret, df_conflict = 0;
 
@@ -414,23 +414,23 @@ static int s_update_ref(const char *action,
rla = default_rla.buf;
snprintf(msg, sizeof(msg), %s: %s, rla, action);
 
-   transaction = ref_transaction_begin(err);
+   transaction = 

Re: [PATCH 1/4] refs.c: rename the transaction functions

2014-12-01 Thread Stefan Beller
This patch series is not really ready for public digestion, I messed
up sending it to the list anyway.
Please ignore this series.
--
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


http-protocol question

2014-12-01 Thread Bryan Turner
In Documentation/technical/http-protocol.txt, there is the following statement:

S: Parse the git-upload-pack request:

Verify all objects in `want` are directly reachable from refs.

The server MAY walk backwards through history or through
the reflog to permit slightly stale requests.

Is there actually logic somewhere in Git that does that MAY walk
backwards step? Or is that something another implementation of Git
may do but the standard Git does not?

Thanks,
Bryan Turner
--
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 09/19] Add git-list-files, a user friendly version of ls-files and more

2014-12-01 Thread Eric Sunshine
On Sunday, November 30, 2014, Nguyễn Thái Ngọc Duy pclo...@gmail.com wrote:

 This is more user friendly version of ls-files:

 * it's automatically colored and columnized
 * it refreshes the index like all porcelain commands
 * it defaults to non-recursive behavior like ls
 * :(glob) is on by default so '*.c' means a.c but not a/b.c, use
   '**/*.c' for that.
 * auto pager

 The name 'ls' is not taken. It is left for the user to make an alias
 with better default options.

I understand that your original version was named git-ls and that you
renamed it to git-list-files in order to leave 'ls' available so users
can create an 'ls' alias specifying their own default options. Would
it make sense, however, to restore the name to git-ls and allow users
to set default options via a config variable instead? Doing so would
make the short-and-sweet git-ls command work for all users
out-of-the-box, which might be well appreciated by Unix users.

More below.

 Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
 ---
 diff --git a/Documentation/git-list-files.txt 
 b/Documentation/git-list-files.txt
 new file mode 100644
 index 000..3039e1e
 --- /dev/null
 +++ b/Documentation/git-list-files.txt
 @@ -0,0 +1,80 @@
 +git-list-files(1)
 +===
 +
 +NAME
 +
 +git-list-files - List files
 +
 +SYNOPSIS
 +
 +[verse]
 +'git list-files [options] [pathspec...]
 +
 +DESCRIPTION
 +---
 +List files (by default in current working directory) that are in the
 +index. Depending on the chosen options, maybe only modified files in
 +working tree are shown, or untracked files...
 +
 +OPTIONS
 +---
 +-c::
 +--cached::
 +   Show cached files (default)

I realize that this mirrors what is in git-ls-files.txt, but:

s/$/./

 +-d::
 +--deleted::
 +   Show cached files that are deleted on working directory

s/$/./

 +-m::
 +--modified::
 +   Show cached files that have modification on working directory

s/$/./

 +-o::
 +--others::
 +   Show untracked files (and only unignored ones unless -i is

s/-i/`-i`/

 +   specified)

s/$/./

 +-i::
 +--ignored::
 +   Show only ignored files. When showing files in the index,
 +   print only those matched by an exclude pattern. When showing
 +   other files, show only those matched by an exclude pattern.
 +
 +-u::
 +--unmerged::
 +   Show unmerged files

s/$/./

 +--color[=when]::
 +--no-color::
 +   Color file names. The value must be `always`, `never`, or
 +   `auto`. `--no-color` is equivalent to
 +   `--color=never`. `--color` is equivalent to
 +   `--color=auto`. See configuration variable `color.list-files`
 +   for the default settings.
 +
 +--column[=options]::
 +--no-column::
 +   Display files in columns. See configuration variable column.ui

s/column.ui/`column.ui`/

 +   for option syntax. `--column` and `--no-column` without options
 +   are equivalent to 'always' and 'never' respectively.
 +
 +--max-depth=depth::
 +   For each pathspec given on command line, descend at most depth
 +   levels of directories. A negative value means no limit.
 +   This option is ignored if pathspec contains active wildcards.
 +   In other words if a* matches a directory named a*,
 +   * is matched literally so --max-depth is still effective.
 +   The default is `--max-depth=0`.
 +
 +pathspec::
 +   Files to show. :(glob) magic is enabled and recursion disabled
 +   by default.
 +
 +SEE ALSO
 +
 +linkgit:git-ls-files[1]
 +
 +GIT
 +---
 +Part of the linkgit:git[1] suite
--
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] Let deny.currentBranch=updateInstead ignore submodules

2014-12-01 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 And thinking about the names again, I have a feeling that
 updateInstead and mergeInstead are both probably misnomer.

 Let me take this part back.  After all, I do not think I would
 design the mechanism to implement an alternative logic that decides
 when it is safe to allow the update of the ref and to reflect the
 changes to the working tree, and that actually does the checkout to
 the working tree by using a new value like mergeInstead.  So we
 would only need a single name, and updateInstead is not too bad.
 ...
 The mechanism I would employ when doing an alternative logic,
 possibly looser one but does not necessarily so, would be to have a
 hook script push-to-checkout.  When denyCurrentBranch is set to
 updateInstead, if there is no such hook, the working tree has to be
 absolutely clean and we would do a 'read-tree -m -u $old $new'
 (which is the same as 'reset --hard $new' under the precondition)
 you implemented will be used as the logic that decides when it is
 safe, and that does the checkout to the working tree.  When the
 push-to-checkout hook exists, however, we just invoke that hook
 with $new as argument, and it can decide when it is safe in whatever
 way it chooses to, and it can checkout the $new to the working tree
 in whatever way it wants.

So here comes a two-patch series on top of your series (with the
test update I sent earlier).  As I never do push to deploy that
requires no changes to the working tree or to the index, while I
have seem myself in a situation where I have to emulate a git pull
with a git push in the opposite direction (and work it around if
the target of the 'git pull' I wanted to do were the current branch,
by first pushing into a throw-away branch, because of denyCurrent),
I could imagine myself using this variant.  Having said that, this
is primarily so that I do not want to forget and discard the brain
cycles we spent discussing this to the waste, more than that I
cannot wait to use this feature myself ;-)

The first one here is a pure refactoring.  The second one is the
real fun.

-- 8 --
Subject: [PATCH 1/2] receive-pack: refactor updateInstead codepath

Keep the there is nothing to update in a bare repository, when
the check and update process runs, here are the GIT_DIR and
GIT_WORK_TREE logic, which will be common regardless of how the
decision to update and the actual update are done, in the original
update_worktree() function, and split out the working tree and
the index must match the original HEAD exactly and use two-way
read-tree to update the working tree into a new push_to_deploy()
helper function.  This will allow customizing the logic more cleanly
and easily.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/receive-pack.c | 53 ++
 1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index c047418..11800cd 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -733,7 +733,9 @@ static int update_shallow_ref(struct command *cmd, struct 
shallow_info *si)
return 0;
 }
 
-static const char *update_worktree(unsigned char *sha1)
+static const char *push_to_deploy(unsigned char *sha1,
+ struct argv_array *env,
+ const char *work_tree)
 {
const char *update_refresh[] = {
update-index, -q, --ignore-submodules, --refresh, NULL
@@ -748,69 +750,70 @@ static const char *update_worktree(unsigned char *sha1)
const char *read_tree[] = {
read-tree, -u, -m, NULL, NULL
};
-   const char *work_tree = git_work_tree_cfg ? git_work_tree_cfg : ..;
-   struct argv_array env = ARGV_ARRAY_INIT;
struct child_process child = CHILD_PROCESS_INIT;
 
-   if (is_bare_repository())
-   return denyCurrentBranch = updateInstead needs a worktree;
-
-   argv_array_pushf(env, GIT_DIR=%s, absolute_path(get_git_dir()));
-
child.argv = update_refresh;
-   child.env = env.argv;
+   child.env = env-argv;
child.dir = work_tree;
child.no_stdin = 1;
child.stdout_to_stderr = 1;
child.git_cmd = 1;
-   if (run_command(child)) {
-   argv_array_clear(env);
+   if (run_command(child))
return Up-to-date check failed;
-   }
 
/* run_command() does not clean up completely; reinitialize */
child_process_init(child);
child.argv = diff_files;
-   child.env = env.argv;
+   child.env = env-argv;
child.dir = work_tree;
child.no_stdin = 1;
child.stdout_to_stderr = 1;
child.git_cmd = 1;
-   if (run_command(child)) {
-   argv_array_clear(env);
+   if (run_command(child))
return Working directory has unstaged changes;
-   }
 
child_process_init(child);
child.argv = 

[PATCH 2/2] receive-pack: support push-to-checkout hook

2014-12-01 Thread Junio C Hamano
When receive.denyCurrentBranch is set to updateInstead, this hook
can be used to override the built-in push-to-deploy logic, which
insists that the working tree and the index must be unchanged
relative to HEAD.  The hook receives the commit with which the
tip of the current is going to be updated, and is responsible to
make any necessary changes to the working tree and to the index to
bring them to the desired state when the tip of the current branch
is updated to the new commit.

For example, the hook can simply run git read-tree -u -m HEAD $1
to the workflow to emulate 'git fetch' going in the reverse
direction with 'git push' better than the push-to-deploy logic, as
the two-tree form of read-tree -u -m is essentially the same as
git checkout that switches branches while keeping the local
changes in the working tree that do not interfere with the
difference between the branches.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 builtin/receive-pack.c | 19 ++-
 t/t5516-fetch-push.sh  | 63 ++
 2 files changed, 81 insertions(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 11800cd..fc8ec9c 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -797,6 +797,20 @@ static const char *push_to_deploy(unsigned char *sha1,
return NULL;
 }
 
+static const char *push_to_checkout_hook = push-to-checkout;
+
+static const char *push_to_checkout(unsigned char *sha1,
+   struct argv_array *env,
+   const char *work_tree)
+{
+   argv_array_pushf(env, GIT_WORK_TREE=%s, absolute_path(work_tree));
+   if (run_hook_le(env-argv, push_to_checkout_hook,
+   sha1_to_hex(sha1), NULL))
+   return push-to-checkout hook declined;
+   else
+   return NULL;
+}
+
 static const char *update_worktree(unsigned char *sha1)
 {
const char *retval;
@@ -808,7 +822,10 @@ static const char *update_worktree(unsigned char *sha1)
 
argv_array_pushf(env, GIT_DIR=%s, absolute_path(get_git_dir()));
 
-   retval = push_to_deploy(sha1, env, work_tree);
+   if (!find_hook(push_to_checkout_hook))
+   retval = push_to_deploy(sha1, env, work_tree);
+   else
+   retval = push_to_checkout(sha1, env, work_tree);
 
argv_array_clear(env);
return retval;
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 85c7fec..e4436c1 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1434,4 +1434,67 @@ test_expect_success 'receive.denyCurrentBranch = 
updateInstead' '
 
 '
 
+test_expect_success 'updateInstead with push-to-checkout hook' '
+   rm -fr testrepo 
+   git init testrepo 
+   (
+   cd testrepo 
+   git pull .. master 
+   git reset --hard HEAD^^ 
+   git tag initial 
+   git config receive.denyCurrentBranch updateInstead 
+   write_script .git/hooks/push-to-checkout -\EOF
+   echo 2 updating from $(git rev-parse HEAD)
+   echo 2 updating to $1
+
+   git update-index -q --refresh 
+   git read-tree -u -m HEAD $1 || {
+   status=$?
+   echo 2 read-tree failed
+   exit $status
+   }
+   EOF
+   ) 
+
+   # Try pushing into a pristine
+   git push testrepo master 
+   (
+   cd testrepo 
+   git diff --quiet 
+   git diff HEAD --quiet 
+   test $(git -C .. rev-parse HEAD) = $(git rev-parse HEAD)
+   ) 
+
+   # Try pushing into a repository with conflicting change
+   (
+   cd testrepo 
+   git reset --hard initial 
+   echo conflicting path2
+   ) 
+   test_must_fail git push testrepo master 
+   (
+   cd testrepo 
+   test $(git rev-parse initial) = $(git rev-parse HEAD) 
+   test conflicting = $(cat path2) 
+   git diff-index --quiet --cached HEAD
+   ) 
+
+   # Try pushing into a repository with unrelated change
+   (
+   cd testrepo 
+   git reset --hard initial 
+   echo unrelated path1 
+   echo irrelevant path5 
+   git add path5
+   ) 
+   git push testrepo master 
+   (
+   cd testrepo 
+   test $(cat path1) = unrelated 
+   test $(cat path5) = irrelevant 
+   test $(git diff --name-only --cached HEAD) = path5 
+   test $(git -C .. rev-parse HEAD) = $(git rev-parse HEAD)
+   )
+'
+
 test_done
-- 
2.2.0-141-gd3f4719

--
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: http-protocol question

2014-12-01 Thread Jonathan Nieder
Hi Bryan,

Bryan Turner wrote:

 Is there actually logic somewhere in Git that does that MAY walk
 backwards step?

Yes.  See upload-pack.c::check_non_tip and
http://thread.gmane.org/gmane.comp.version-control.git/178814.

Hope that helps,
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: http-protocol question

2014-12-01 Thread Bryan Turner
On Tue, Dec 2, 2014 at 2:34 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi Bryan,

 Bryan Turner wrote:

 Is there actually logic somewhere in Git that does that MAY walk
 backwards step?

 Yes.  See upload-pack.c::check_non_tip and
 http://thread.gmane.org/gmane.comp.version-control.git/178814.

Jonathan,

Thanks for the reply! I realize now I didn't really cite the part of
that documentation that I'm most interested in, which is: through
history _or through the reflog_. It's the reflog bit I'm looking for.
Sorry for not being more clear. check_non_tip appears to look at
ancestry, walking back up (or down, depending on your view) the DAG to
see if the requested SHA-1 is reachable from a current ref, so it
looks like that's covering the through history part of that blurb.

The reason I ask is that there is a race condition that exists where
the ref advertisement lists refs/heads/foo at abc1234, and then foo is
deleted before the actual upload-pack request comes in. In that
situation, walking backwards through _history_ will only allow the
upload-pack to complete unless the deleted ref was merged into another
ref prior to being deleted (if I understand the code correctly). It
seems like looking at the reflogs, and seeing that refs/heads/foo did
in fact exist and did in fact refer to abc1234, is the only approach
that could allow the upload-pack to complete. The documentation hints
that such a thing is possible, but I don't see any code in Git that
seems to do that.

Does that make more sense? Does that functionality exist and I've just
overlooked it?

Thanks again,
Bryan Turner


 Hope that helps,
 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: http-protocol question

2014-12-01 Thread Bryan Turner
On Tue, Dec 2, 2014 at 3:28 PM, Bryan Turner btur...@atlassian.com wrote:
 On Tue, Dec 2, 2014 at 2:34 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi Bryan,

 Bryan Turner wrote:

 Is there actually logic somewhere in Git that does that MAY walk
 backwards step?

 Yes.  See upload-pack.c::check_non_tip and
 http://thread.gmane.org/gmane.comp.version-control.git/178814.

 Jonathan,

 Thanks for the reply! I realize now I didn't really cite the part of
 that documentation that I'm most interested in, which is: through
 history _or through the reflog_. It's the reflog bit I'm looking for.
 Sorry for not being more clear. check_non_tip appears to look at
 ancestry, walking back up (or down, depending on your view) the DAG to
 see if the requested SHA-1 is reachable from a current ref, so it
 looks like that's covering the through history part of that blurb.

 The reason I ask is that there is a race condition that exists where
 the ref advertisement lists refs/heads/foo at abc1234, and then foo is
 deleted before the actual upload-pack request comes in. In that
 situation, walking backwards through _history_ will only allow the
 upload-pack to complete unless the deleted ref was merged into another

s/unless/if, sorry. In that situation, walking backwards through
history will only allow the upload-pack to complete if the deleted ref
was merged into another ref.

 ref prior to being deleted (if I understand the code correctly). It
 seems like looking at the reflogs, and seeing that refs/heads/foo did
 in fact exist and did in fact refer to abc1234, is the only approach
 that could allow the upload-pack to complete. The documentation hints
 that such a thing is possible, but I don't see any code in Git that
 seems to do that.

 Does that make more sense? Does that functionality exist and I've just
 overlooked it?

 Thanks again,
 Bryan Turner


 Hope that helps,
 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: http-protocol question

2014-12-01 Thread Jonathan Nieder
Hi,

Bryan Turner wrote:

 The reason I ask is that there is a race condition that exists where
 the ref advertisement lists refs/heads/foo at abc1234, and then foo is
 deleted before the actual upload-pack request comes in.

Can you say more about the context?  For example, was this actually
happening, or is this for the sake of understanding the protocol
better?

I ask because knowing the context might help us give more specific
advice.

Sometimes when people delete a branch they really want those objects
to be inaccessible *right away*.  So for such people, git's behavior
of failing the request unless the objects are still accessible by
some other path is helpful.

A stateful server could theoretically cache the list of objects they
have advertised for a short time, to avoid clients having to suffer
when something becomes inaccessible during the window between the
upload-pack advertisement and upload-pack request.  Or a permissive
server can allow all wants except a specific blacklist (and some
people do that).

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: http-protocol question

2014-12-01 Thread Bryan Turner
On Tue, Dec 2, 2014 at 3:45 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 Hi,

 Bryan Turner wrote:

 The reason I ask is that there is a race condition that exists where
 the ref advertisement lists refs/heads/foo at abc1234, and then foo is
 deleted before the actual upload-pack request comes in.

 Can you say more about the context?  For example, was this actually
 happening, or is this for the sake of understanding the protocol
 better?

I ask because it's actually happening. Heavy CI load sometimes has
builds fail because git clone dies with not our ref. That's the
specific context I'm working to try and address right now. Some teams
use rebase-heavy workflows, which also evades the check_non_tip easing
and fails with not our ref, so I can't be 100% certain it's ref
deletion in specific causing it (but I guess which of those it is is
probably largely academic; as long as hosting spans multiple requests
it seems like this type of race condition is unavoidable).

I'm trying to decide if there is something I can enable or tune to
prevent it, and the type of twilighting hinted at by the http-protocol
documentation seemed like it could be just the thing.


 I ask because knowing the context might help us give more specific
 advice.

 Sometimes when people delete a branch they really want those objects
 to be inaccessible *right away*.  So for such people, git's behavior
 of failing the request unless the objects are still accessible by
 some other path is helpful.

 A stateful server could theoretically cache the list of objects they
 have advertised for a short time, to avoid clients having to suffer
 when something becomes inaccessible during the window between the
 upload-pack advertisement and upload-pack request.  Or a permissive
 server can allow all wants except a specific blacklist (and some
 people do that).

 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: http-protocol question

2014-12-01 Thread Jonathan Nieder
Bryan Turner wrote:

 I ask because it's actually happening. Heavy CI load sometimes has
 builds fail because git clone dies with not our ref. That's the
 specific context I'm working to try and address right now.

Thanks --- that makes sense.

Some teams
 use rebase-heavy workflows, which also evades the check_non_tip easing
 and fails with not our ref, so I can't be 100% certain it's ref
 deletion in specific causing it (but I guess which of those it is is
 probably largely academic; as long as hosting spans multiple requests
 it seems like this type of race condition is unavoidable).

Yeah, everything mentioned before about ref deletion also applies to
non-fast-forward updates.

 I'm trying to decide if there is something I can enable or tune to
 prevent it, and the type of twilighting hinted at by the http-protocol
 documentation seemed like it could be just the thing.

If you control the side that clones, then just cloning the single branch
you are building (with --single-branch and -b) can help.

Using a bidirectional protocol like git:// or ssh:// (where the ref
advertisement and check of wants happen in the same connection) would
avoid the problem we're talking about.

On the server side, I agree that either mining reflogs or storing
advertisements to disk would be a nice way to take care of this.
No one has implemented either of those, but it would be a nice setting
to have. :)

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: http-protocol question

2014-12-01 Thread Duy Nguyen
On Tue, Dec 2, 2014 at 12:17 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 On the server side, I agree that either mining reflogs or storing
 advertisements to disk would be a nice way to take care of this.
 No one has implemented either of those, but it would be a nice setting
 to have. :)

and could be dangerous too. If I accidentally add a password file,
then delete it (way before a clone is performed), I don't want that
part of reflog visible to the cloner. Problem is we don't know how far
we should look back in reflog unless we keep some state.
-- 
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


Re: http-protocol question

2014-12-01 Thread Jeff King
On Tue, Dec 02, 2014 at 04:04:11PM +1100, Bryan Turner wrote:

  Can you say more about the context?  For example, was this actually
  happening, or is this for the sake of understanding the protocol
  better?
 
 I ask because it's actually happening. Heavy CI load sometimes has
 builds fail because git clone dies with not our ref. That's the
 specific context I'm working to try and address right now. Some teams
 use rebase-heavy workflows, which also evades the check_non_tip easing
 and fails with not our ref, so I can't be 100% certain it's ref
 deletion in specific causing it (but I guess which of those it is is
 probably largely academic; as long as hosting spans multiple requests
 it seems like this type of race condition is unavoidable).

There is a practical reason to care. Ref deletion will also delete the
reflog, leaving no trace of the reachability. Whereas a non-fast-forward
push could be resolved by looking in the reflog.

One problem with hunting for sha1s in the reflog is that upload-pack
does not know which ref the client thinks they are requesting. So a
search would involve looking in _every_ reflog, which could be
expensive. It might not be too painful if you do the search only after
hitting a not our ref condition, which should in theory be rare. A
malicious client could convince you to grep your reflogs repeatedly, but
that is hardly the only way to convince upload-pack to chew CPU. Asking
it to make a pack comes to mind. :)

 I'm trying to decide if there is something I can enable or tune to
 prevent it, and the type of twilighting hinted at by the http-protocol
 documentation seemed like it could be just the thing.

For a public repository, it might make sense to provide a config option
to loosen the is_our_ref check completely (i.e., to just has_sha1_file).
But such an option does not yet exist.

-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: http-protocol question

2014-12-01 Thread Duy Nguyen
On Tue, Dec 2, 2014 at 12:17 PM, Jonathan Nieder jrnie...@gmail.com wrote:
 I'm trying to decide if there is something I can enable or tune to
 prevent it, and the type of twilighting hinted at by the http-protocol
 documentation seemed like it could be just the thing.

 If you control the side that clones, then just cloning the single branch
 you are building (with --single-branch and -b) can help.

Or we could extend this a bit on the server side, if one ref fail, let
the clone continue with the remaining refs (and warn loudly to the
user).
-- 
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


Re: [PATCH 00/19] Add git-list-files

2014-12-01 Thread Jeff King
On Sun, Nov 30, 2014 at 03:55:48PM +0700, Nguyễn Thái Ngọc Duy wrote:

 This is something else that's been sitting in my tree for a while now.
 It adds git list-files, intended to be aliased as ls with your
 favourite display options.

When I read the subject, I thought why isn't this called git-ls?. Then
when I read this paragraph, I thought if the point is for everybody to
make their own ls alias, why do we need list-files at all, instead of
just adding options to ls-files?

I'll let you decide which (if any) you'd like to answer. :)

My guesses:

  1. If it were git-ls, it would stomp on existing aliases people have
 constructed.

  2. If it is a wrapper around ls-files, then the options may be
 constrained; ls-files already squats on useful options like -d
 (which, if we are matching traditional ls, is more like our -t).

I somewhat feel like (1) can be mitigated by the fact that your command
is what people were probably trying to approximate with their aliases,
and that as porcelain it should be very configurable (so they should be
able to accomplish the same things as their aliases). But I dunno. I do
not have an ls alias, so I am biased. :)

As a side note, I wonder if it would be sensible to whitelist some
commands as porcelain, and allow aliases to override them (either
entirely, or just to add-in some options).

-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: http-protocol question

2014-12-01 Thread Bryan Turner
On Tue, Dec 2, 2014 at 4:33 PM, Jeff King p...@peff.net wrote:
 On Tue, Dec 02, 2014 at 04:04:11PM +1100, Bryan Turner wrote:

  Can you say more about the context?  For example, was this actually
  happening, or is this for the sake of understanding the protocol
  better?

 I ask because it's actually happening. Heavy CI load sometimes has
 builds fail because git clone dies with not our ref. That's the
 specific context I'm working to try and address right now. Some teams
 use rebase-heavy workflows, which also evades the check_non_tip easing
 and fails with not our ref, so I can't be 100% certain it's ref
 deletion in specific causing it (but I guess which of those it is is
 probably largely academic; as long as hosting spans multiple requests
 it seems like this type of race condition is unavoidable).

 There is a practical reason to care. Ref deletion will also delete the
 reflog, leaving no trace of the reachability. Whereas a non-fast-forward
 push could be resolved by looking in the reflog.

A fair point. I had mistakenly thought that reflogs would survive the
ref's deletion and be pruned as part of garbage collection, but a
quick test shows that, as I'm sure you already know, that's not true.

Thanks for correcting my mistake!
Bryan Turner
--
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: http-protocol question

2014-12-01 Thread Jeff King
On Tue, Dec 02, 2014 at 04:47:50PM +1100, Bryan Turner wrote:

  There is a practical reason to care. Ref deletion will also delete the
  reflog, leaving no trace of the reachability. Whereas a non-fast-forward
  push could be resolved by looking in the reflog.
 
 A fair point. I had mistakenly thought that reflogs would survive the
 ref's deletion and be pruned as part of garbage collection, but a
 quick test shows that, as I'm sure you already know, that's not true.

I wish it worked that way. Unfortunately there are complications with
keeping the old reflogs in place, because they sometimes cause conflicts
with new refs being created (e.g., a reflog in .git/logs/refs/heads/foo
would prevent .git/logs/refs/heads/foo/bar from being created). I had
some patches long ago to try to keep a reflog graveyard around, but
they were quite invasive, and there were some corner cases that caused
weird errors.

Handling this sort of D/F conflict more gracefully is one of the things
I'd like to experiment with once we have pluggable ref backends (I think
we'll also disallow foo/bar if foo exists, but the storage could at
least keep the reflogs around).

-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: Bug in reflog of length 0x2BFF

2014-12-01 Thread Christoph Mallon
Hi Jonathan,

Am 02.12.14 00:35, schrieb Jonathan Nieder:
 Christoph Mallon wrote:
 % git rev-parse 'master@{52}'
 warning: Log for ref refs/heads/master has gap after Thu, 1 Jan 1970 
 00:00:01 +.
 0036
 
 Can you say more?  What output did you expect and how does this differ
 from it?

sorry, I thought it is obvious that the warning should not be there.
As far as I understand the code, this warning is shown, when the old
commit id of one entry does not equal the new commit id of its predecessor.
But this reflog file does not have such a gap.
Also the correct result ist 0...035, not 0...036.
I.e. one entry is erroneously skipped.

 I tried, with git 2.2.0,
 
   git init gitbug 
   cd gitbug 
   git commit --allow-empty -m a 
   wget http://tron.yamagi.org/zeug/reflog.bad 
   mv reflog.bad .git/logs/refs/heads/master 
   sha1sum .git/logs/refs/heads/master 
   git rev-parse 'master@{52}'

These steps look right.

 The output:
 
  9ffe44715d0e542a60916255f144c74e6760ffd0  .git/logs/refs/heads/master

The checksum is fine.

  0035

You do not see the bug. |:

 
 Could you make a test script that illustrates and reproduces the
 problem?  I.e., a patch to a file like t/t1410-reflog.sh [...]

http://tron.yamagi.org/zeug/0001-t1410-Test-erroneous-skipping-of-reflog-entries.patch
(also attached)

This test works for me at v2.0.4 and fails at v2.1.0 and up (v2.2.0, the
current master).
Bisect says the symptom appears at 4207ed285f31ad3e04f08254237c0c1a1609642b.


Christoph
From 82115da194adc42143b8603063e0a419fbbf4928 Mon Sep 17 00:00:00 2001
From: Christoph Mallon christoph.mal...@gmx.de
Date: Tue, 2 Dec 2014 07:03:11 +0100
Subject: [PATCH] t1410: Test erroneous skipping of reflog entries.

---
 t/t1410-reflog.sh | 63 +++
 1 file changed, 63 insertions(+)

diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index 8cf4461..cb77c27 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -287,4 +287,67 @@ test_expect_success 'stale dirs do not cause d/f conflicts 
(reflogs off)' '
test_cmp expect actual
 '
 
+test_expect_success 'erroneous skipping of reflog entries' '
+   git checkout -b reflogskip 
+   cat  .git/logs/refs/heads/reflogskip EOF 
+0037 
0036  
xxx@xxx 01 +  X
+0036 
0035  
xxx@xxx 01 +  X
+0035 
0034  
xxx@xxx 01 +  

+0034 
0033  
xxx@xxx 01 +  XXX
+0033 
0032  
xxx@xxx 01 +  
XX
+0032 
0031  
xxx@xxx 01 +  
X
+0031 
0030  
xxx@x 01 +
XX
+0030 
002f  
xxx@x 01 +
XXX
+002f 
002e  
xxx@x 01 +
XXX
+002e 
002d  
xxx@x 01 +

+002d 
002c  
xxx@x 01 +
XXX
+002c 
002b  
xxx@x 01 +
XXX
+002b 
002a  
xxx@x 01 +

Re: git status / git diff -C not detecting file copy

2014-12-01 Thread Jeff King
On Sun, Nov 30, 2014 at 12:54:53PM +1100, Bryan Turner wrote:

 I'll let someone a little more intimately familiar with the internals
 of git status comment on why the documentation for that mentions
 copies.

I don't think there is a good reason. git-status has used renames since
mid-2005. The documentation mentioning copies was added much later,
along with the short and porcelain formats. That code handles whatever
the diff engine throws at it.  I don't think anybody considered at that
time the fact that you cannot actually provoke status to look for
copies.

Interestingly, the rename behavior dates all the way back to:

  commit 753fd78458b6d7d0e65ce0ebe7b62e1bc55f3992
  Author: Linus Torvalds torva...@ppc970.osdl.org
  Date:   Fri Jun 17 15:34:19 2005 -0700

  Use -M instead of -C for git diff and git status
  
  The C in -C may stand for Cool, but it's also pretty slow, since
  right now it leaves all unmodified files to be tested even if there are
  no new files at all.  That just ends up being unacceptably slow for big
  projects, especially if it's not all in the cache.

I suspect that the copy code may be much faster these days (it sounds
like we did not even have the find-copies-harder distinction then, and
these days we certainly take the quick return if there are no copy
destination candidates).

To get a rough sense of how much effort is entailed in the various
options, here are git log --raw timings for git.git (all timings are
warm cache, best-of-five, wall clock time):

  log --raw:   0m2.311s
  log --raw -M:0m2.362s
  log --raw -C:0m2.576s
  log --raw -C -C: 1m4.462s

You can see that rename detection adds a little, and copy detections
adds a little more.  That makes sense; it's rare for new files to appear
at the same that old files are going away (renames), so most of the time
it does nothing. Copies introduce a bit more work; we have to compare
against any changed files, and there are typically several in each
commit. find-copies-harder is...well, very expensive.

These timings are of diffs between commits and their parents, of course.
But if we assume that git status will show diffs roughly similar to
what gets committed, then this should be comparable. There are about 30K
non-merge commits we traversed there, so adding 200ms is an average of
not very much per commit. Of course the cost is disproportionately borne
by diffs which have an actual file come into being. There are ~2000
commits that introduce a file, so it's probably accurate to say that it
either adds nothing in most cases, or ~1/10th of a millisecond in
others.

Note this is also doing inexact detection, which involves actually
looking at the contents of candidate blobs (whereas exact detection can
be done by comparing sha1s, which is very fast). If you set
diff.renamelimit to 1, then we do only exact detections. Here are
timings there:

  log --raw:   0m02.311s(for reference)
  log --raw -M:0m02.337s
  log --raw -C:0m02.347s
  log --raw -C -C: 0m24.419s

That speeds things up a fair bit, even for -C (we don't have to access
the blobs anymore, so I suspect the time is going to just accessing all
of the trees; normally diff does not descend into subtrees that have the
same sha1). Of course, you probably wouldn't want to turn off inexact
renames completely. I suspect what you'd want is a --find-copies-moderately
where we look for cheap copies using -C, and then follow up with -C
-C only using exact renames.

So from these timings, I'd conclude that:

  1. It's probably fine to turn on copies for git status.

  2. It's probably even OK to use -C -C for some projects. Even though
 22s looks scary there, that's only 11ms for git.git (remember,
 spread across 2000 commits). For linux.git, it's much, much worse.
 I killed my -C -C run after 10 minutes, and it had only gone
 through 1/20th of the commits. Extrapolating, you're looking at
 500ms or so added to a git status run.

 So you'd almost certainly want this to be configurable.

Does either of you want to try your hand at a patch? Just enabling
copies should be a one-liner. Making it configurable is more involved,
but should also be pretty straightforward.

-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] introduce git root

2014-12-01 Thread Jeff King
On Mon, Dec 01, 2014 at 05:17:22AM +0100, Christian Couder wrote:

 On Mon, Dec 1, 2014 at 4:04 AM, Junio C Hamano gits...@pobox.com wrote:
 
  If I were redoing this today, I would probably nominate the git
  potty as such a kitchen synk command.  We have --man-path that
  shows the location of the manual pages, --exec-path[=path] that
  either shows or allows us to override the path to the subcommands,
  and --show-prefix, --show-toplevel, and friends may feel quite
  at home there.
 
 I wonder if we could reuse git config which is already a kitchen
 synk command to get/set a lot of parameters.
 Maybe we could dedicate a git or virtual or proc or sys (like
 /proc or /sys  in Linux) namespace for these special config parameters
 that would not necessarily reflect something in the config file.
 
 git config git.man-path would be the same as git --man-path.
 git config git.root would be the same as git rev-parse --show-toplevel.
 git config git.exec-path mypath would allow us to override the path
 to the subcommands, probably by saving something in the config file.

What would:

  git config git.root foo
  git config git.root

output? No matter what the answer is, I do not relish the thought of
trying to explain it in the documentation. :)

There is also git var, which is a catch-all for printing some deduced
environmental defaults. I'd be just as happy to see it go away, though.
Having:

  git --exec-path
  git --toplevel
  git --author-ident

all work would make sense to me (I often get confused between git
--foo and git rev-parse --foo when trying to get the exec-path and
git-dir). And I don't think it's too late to move in this direction.
We'd have to keep the old interfaces around, of course, but it would
immediately improve discoverability and consistency.

-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 4/4] reflog.c: use a reflog transaction when writing during expire

2014-12-01 Thread Stefan Beller
From: Ronnie Sahlberg sahlb...@google.com

Use a transaction for all updates during expire_reflog.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---

no changes since sending it 5 days ago.

 builtin/reflog.c | 86 
 1 file changed, 37 insertions(+), 49 deletions(-)

diff --git a/builtin/reflog.c b/builtin/reflog.c
index 2d85d26..55f3023 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -33,7 +33,8 @@ struct cmd_reflog_expire_cb {
 };
 
 struct expire_reflog_cb {
-   FILE *newlog;
+   struct transaction *t;
+   const char *refname;
enum {
UE_NORMAL,
UE_ALWAYS,
@@ -290,7 +291,7 @@ static int unreachable(struct expire_reflog_cb *cb, struct 
commit *commit, unsig
 
 static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
const char *email, unsigned long timestamp, int tz,
-   const char *message, void *cb_data)
+   const char *message, void *cb_data, struct strbuf *err)
 {
struct expire_reflog_cb *cb = cb_data;
struct commit *old, *new;
@@ -316,20 +317,18 @@ static int expire_reflog_ent(unsigned char *osha1, 
unsigned char *nsha1,
if (cb-cmd-recno  --(cb-cmd-recno) == 0)
goto prune;
 
-   if (cb-newlog) {
-   char sign = (tz  0) ? '-' : '+';
-   int zone = (tz  0) ? (-tz) : tz;
-   fprintf(cb-newlog, %s %s %s %lu %c%04d\t%s,
-   sha1_to_hex(osha1), sha1_to_hex(nsha1),
-   email, timestamp, sign, zone,
-   message);
+   if (cb-t) {
+   if (transaction_update_reflog(cb-t, cb-refname, nsha1, osha1,
+ email, timestamp, tz, message, 0,
+ err))
+   return -1;
hashcpy(cb-last_kept_sha1, nsha1);
}
if (cb-cmd-verbose)
printf(keep %s, message);
return 0;
  prune:
-   if (!cb-newlog)
+   if (!cb-t)
printf(would prune %s, message);
else if (cb-cmd-verbose)
printf(prune %s, message);
@@ -353,29 +352,27 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
 {
struct cmd_reflog_expire_cb *cmd = cb_data;
struct expire_reflog_cb cb;
-   struct ref_lock *lock;
-   char *log_file, *newlog_path = NULL;
struct commit *tip_commit;
struct commit_list *tips;
+   struct strbuf err = STRBUF_INIT;
int status = 0;
 
memset(cb, 0, sizeof(cb));
+   cb.refname = ref;
 
-   /*
-* we take the lock for the ref itself to prevent it from
-* getting updated.
-*/
-   lock = lock_any_ref_for_update(ref, sha1, 0, NULL);
-   if (!lock)
-   return error(cannot lock ref '%s', ref);
-   log_file = git_pathdup(logs/%s, ref);
if (!reflog_exists(ref))
goto finish;
-   if (!cmd-dry_run) {
-   newlog_path = git_pathdup(logs/%s.lock, ref);
-   cb.newlog = fopen(newlog_path, w);
+   cb.t = transaction_begin(err);
+   if (!cb.t) {
+   status |= error(%s, err.buf);
+   goto cleanup;
+   }
+   if (transaction_update_reflog(cb.t, cb.refname, null_sha1, null_sha1,
+ NULL, 0, 0, NULL, REFLOG_TRUNCATE,
+ err)) {
+   status |= error(%s, err.buf);
+   goto cleanup;
}
-
cb.cmd = cmd;
 
if (!cmd-expire_unreachable || !strcmp(ref, HEAD)) {
@@ -407,7 +404,10 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
mark_reachable(cb);
}
 
-   for_each_reflog_ent(ref, expire_reflog_ent, cb);
+   if (for_each_reflog_ent(ref, expire_reflog_ent, cb)) {
+   status |= error(%s, err.buf);
+   goto cleanup;
+   }
 
if (cb.unreachable_expire_kind != UE_ALWAYS) {
if (cb.unreachable_expire_kind == UE_HEAD) {
@@ -420,32 +420,20 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
}
}
  finish:
-   if (cb.newlog) {
-   if (fclose(cb.newlog)) {
-   status |= error(%s: %s, strerror(errno),
-   newlog_path);
-   unlink(newlog_path);
-   } else if (cmd-updateref 
-   (write_in_full(lock-lock_fd,
-   sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
-write_str_in_full(lock-lock_fd, \n) != 1 ||
-close_ref(lock)  0)) {
-   status |= error(Couldn't write %s,
-  

[PATCH 1/4] refs.c: rename the transaction functions

2014-12-01 Thread Stefan Beller
From: Ronnie Sahlberg sahlb...@google.com

Rename the transaction functions. Remove the leading ref_ from the
names and append _ref to the names for functions that create/delete/
update sha1 refs.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com
---
This commit can be reproduced via
find . -name *.[ch] -print | xargs sed -i ' \
s/REF_TRANSACTION_OPEN/TRANSACTION_OPEN/; \
s/REF_TRANSACTION_CLOSED/TRANSACTION_CLOSED/; \
s/ref_transaction_begin/transaction_begin/; \
s/ref_transaction_commit/transaction_commit/; \
s/ref_transaction_create/transaction_create_ref/; \
s/ref_transaction_delete/transaction_delete_ref/; \
s/ref_transaction_free/transaction_free/; \
s/ref_transaction_update/transaction_update_ref/; \
s/ref_transaction/transaction/'
modulo white space changes for alignment.

No changes since sending it 5 days ago.

 branch.c   | 13 +
 builtin/commit.c   | 10 +++
 builtin/fetch.c| 12 
 builtin/receive-pack.c | 13 -
 builtin/replace.c  | 10 +++
 builtin/tag.c  | 10 +++
 builtin/update-ref.c   | 26 -
 fast-import.c  | 22 +++---
 refs.c | 78 +-
 refs.h | 36 +++
 sequencer.c| 12 
 walker.c   | 10 +++
 12 files changed, 126 insertions(+), 126 deletions(-)

diff --git a/branch.c b/branch.c
index 4bab55a..c8462de 100644
--- a/branch.c
+++ b/branch.c
@@ -279,16 +279,17 @@ void create_branch(const char *head,
log_all_ref_updates = 1;
 
if (!dont_change_ref) {
-   struct ref_transaction *transaction;
+   struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
 
-   transaction = ref_transaction_begin(err);
+   transaction = transaction_begin(err);
if (!transaction ||
-   ref_transaction_update(transaction, ref.buf, sha1,
-  null_sha1, 0, !forcing, msg, err) ||
-   ref_transaction_commit(transaction, err))
+   transaction_update_ref(transaction, ref.buf, sha1,
+  null_sha1, 0, !forcing, msg,
+  err) ||
+   transaction_commit(transaction, err))
die(%s, err.buf);
-   ref_transaction_free(transaction);
+   transaction_free(transaction);
strbuf_release(err);
}
 
diff --git a/builtin/commit.c b/builtin/commit.c
index e108c53..f50b7df 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1673,7 +1673,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
struct stat statbuf;
struct commit *current_head = NULL;
struct commit_extra_header *extra = NULL;
-   struct ref_transaction *transaction;
+   struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
 
if (argc == 2  !strcmp(argv[1], -h))
@@ -1804,17 +1804,17 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
strbuf_insert(sb, 0, reflog_msg, strlen(reflog_msg));
strbuf_insert(sb, strlen(reflog_msg), : , 2);
 
-   transaction = ref_transaction_begin(err);
+   transaction = transaction_begin(err);
if (!transaction ||
-   ref_transaction_update(transaction, HEAD, sha1,
+   transaction_update_ref(transaction, HEAD, sha1,
   current_head
   ? current_head-object.sha1 : NULL,
   0, !!current_head, sb.buf, err) ||
-   ref_transaction_commit(transaction, err)) {
+   transaction_commit(transaction, err)) {
rollback_index_files();
die(%s, err.buf);
}
-   ref_transaction_free(transaction);
+   transaction_free(transaction);
 
unlink(git_path(CHERRY_PICK_HEAD));
unlink(git_path(REVERT_HEAD));
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7b84d35..0be0b09 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -404,7 +404,7 @@ static int s_update_ref(const char *action,
 {
char msg[1024];
char *rla = getenv(GIT_REFLOG_ACTION);
-   struct ref_transaction *transaction;
+   struct transaction *transaction;
struct strbuf err = STRBUF_INIT;
int ret, df_conflict = 0;
 
@@ -414,23 +414,23 @@ static int s_update_ref(const char *action,
rla = default_rla.buf;
snprintf(msg, sizeof(msg), %s: %s, rla, action);
 
-   transaction = ref_transaction_begin(err);
+   transaction = transaction_begin(err);
if (!transaction ||
-   ref_transaction_update(transaction, ref-name, 

[PATCHv2 0/4] refs.c: use transactions for the reflog

2014-12-01 Thread Stefan Beller
This is the core part of the refs-transactions-reflog series[1],
which was in discussion for a bit already.

The idea is to have the reflog being part of the transactions, which
the refs are already using, so the we're moving towards a database
like API in the long run. This makes git easier to maintain as well
as opening the possibility to replace the backend with a real database.

The first patch is essentially just some sed magic with reformatting
the code, so the naming convention fits better, because the transactions
will handle both the refs as well as the reflog after this series.

The second patch is also just a rename patch.

The meat and most of the lines of code are found in the 3rd patch.
It was completely rewritten from scratch using ideas provided by
Jonathan, thanks! We'll treat the refs and reflogs a bit differently
within the transaction updates data structure, as the hard part for
the refs is during the commit phase, while the hard part for the reflogs
is during the preparation phase now.

We're using a temporary file for creating the new reflog and rename it
into place afterwards utilizing the lock file API.

The last patch makes actually use of the reflog transaction API.

Any comments would be appreciated,
Thanks,
Stefan

--
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/4] refs.c: rename transaction.updates to transaction.ref_updates

2014-12-01 Thread Stefan Beller
The updates are only holding refs not reflogs, so express it to the reader.

Signed-off-by: Stefan Beller sbel...@google.com
---

* only renaming, no extra code in this patch.
* new to the series.

 refs.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index f0f0d23..58de60c 100644
--- a/refs.c
+++ b/refs.c
@@ -3554,7 +3554,7 @@ enum transaction_state {
  * as atomically as possible.  This structure is opaque to callers.
  */
 struct transaction {
-   struct ref_update **updates;
+   struct ref_update **ref_updates;
size_t alloc;
size_t nr;
enum transaction_state state;
@@ -3575,10 +3575,10 @@ void transaction_free(struct transaction *transaction)
return;
 
for (i = 0; i  transaction-nr; i++) {
-   free(transaction-updates[i]-msg);
-   free(transaction-updates[i]);
+   free(transaction-ref_updates[i]-msg);
+   free(transaction-ref_updates[i]);
}
-   free(transaction-updates);
+   free(transaction-ref_updates);
free(transaction);
 }
 
@@ -3589,8 +3589,8 @@ static struct ref_update *add_update(struct transaction 
*transaction,
struct ref_update *update = xcalloc(1, sizeof(*update) + len + 1);
 
strcpy((char *)update-refname, refname);
-   ALLOC_GROW(transaction-updates, transaction-nr + 1, 
transaction-alloc);
-   transaction-updates[transaction-nr++] = update;
+   ALLOC_GROW(transaction-ref_updates, transaction-nr + 1, 
transaction-alloc);
+   transaction-ref_updates[transaction-nr++] = update;
return update;
 }
 
@@ -3712,7 +3712,7 @@ int transaction_commit(struct transaction *transaction,
int ret = 0, delnum = 0, i;
const char **delnames;
int n = transaction-nr;
-   struct ref_update **updates = transaction-updates;
+   struct ref_update **updates = transaction-ref_updates;
 
assert(err);
 
-- 
2.2.0

--
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 3/4] refs.c: add a transaction function to append a reflog entry

2014-12-01 Thread Stefan Beller
When performing a reflog transaction update, only write to the reflog iff
msg is non-NULL. This can then be combined with REFLOG_TRUNCATE to perform
an update that only truncates but does not write. This change only affects
whether or not a reflog entry should be generated and written. If msg == NULL
then no such entry will be written.

Orthogonal to this we have a boolean flag REFLOG_TRUNCATE which is used to
tell the transaction system to truncate the reflog and thus discard all
previous users.

At the current time the only place where we use msg == NULL is also the
place, where we use REFLOG_TRUNCATE. Even though these two settings are
currently only ever used together it still makes sense to have them through
two separate knobs.

This allows future consumers of this API that may want to do things
differently. For example someone can do:
  msg=Reflog truncated by Bob because ... + REFLOG_TRUNCATE
and have it truncate the log and have it start fresh with an initial message
that explains the log was truncated. This API allows that.

During one transaction we allow to make multiple reflog updates to the
same ref. This means we only need to lock the reflog once, during the first
update that touches the reflog, and that all further updates can just write the
reflog entry since the reflog is already locked.

This allows us to write code such as:

  t = transaction_begin()
  transaction_reflog_update(t, foo, REFLOG_TRUNCATE, NULL);
  loop-over-something...
 transaction_reflog_update(t, foo, 0, message);
  transaction_commit(t)

where we first truncate the reflog and then build the new content one line
at a time.

Signed-off-by: Ronnie Sahlberg sahlb...@google.com
Signed-off-by: Stefan Beller sbel...@google.com

---
This is a complete rewrite of the patch you would have found earlier on
the list. The approach was changed to deal with the reflogs differently
and not toss them into the array containing all the ref_updates, but
keep them in a separate string list.

As I am borrowing some text for the commit message and some ideas how to 
approach
the problem, I still added Ronnies sign off.

 refs.c | 127 +++--
 refs.h |  21 +++
 2 files changed, 146 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 58de60c..2861486 100644
--- a/refs.c
+++ b/refs.c
@@ -3557,6 +3557,12 @@ struct transaction {
struct ref_update **ref_updates;
size_t alloc;
size_t nr;
+
+   /*
+* Sorted list of reflogs to be committed,
+* the util points to the lock_file
+*/
+   struct string_list reflog_updates;
enum transaction_state state;
 };
 
@@ -3564,7 +3570,10 @@ struct transaction *transaction_begin(struct strbuf *err)
 {
assert(err);
 
-   return xcalloc(1, sizeof(struct transaction));
+   struct transaction *ret = xcalloc(1, sizeof(struct transaction));
+   string_list_init(ret-reflog_updates, 1);
+
+   return ret;
 }
 
 void transaction_free(struct transaction *transaction)
@@ -3629,6 +3638,112 @@ int transaction_update_ref(struct transaction 
*transaction,
return 0;
 }
 
+/* Returns a fd, -1 on error. */
+static int get_reflog_updates_fd(struct transaction *transaction,
+const char *refname,
+struct strbuf *err)
+{
+   char *path;
+   struct lock_file *lock;
+   struct string_list_item *item = string_list_insert(
+   transaction-reflog_updates,
+   refname);
+   if (!item-util) {
+   item-util = xcalloc(1, sizeof(struct lock_file));
+   lock = item-util;
+   path = git_path(logs/locks/%s, refname);
+   if (safe_create_leading_directories(path)) {
+   strbuf_addf(err,
+   creating temporary directories %s failed.,
+   path);
+   return -1;
+   }
+
+   if (hold_lock_file_for_update(lock, path, 0)  0) {
+   strbuf_addf(err,
+   creating temporary file %s failed.,
+   path);
+   return -1;
+   }
+   }
+
+   lock = item-util;
+
+   return lock-fd;
+}
+
+int transaction_update_reflog(struct transaction *transaction,
+ const char *refname,
+ const unsigned char *new_sha1,
+ const unsigned char *old_sha1,
+ const char *email,
+ unsigned long timestamp, int tz,
+ const char *msg, int flags,
+ struct strbuf *err)
+{
+   /*
+* We cannot handle reflogs the same way we handle refs because of
+*