Re: [PATCH v2 1/5] replace: forbid replacing an object with one of a different type

2013-08-29 Thread Christian Couder
From: Junio C Hamano gits...@pobox.com

 Thomas Rast tr...@inf.ethz.ch writes:
 
 Hrm, you're right, that's a flaw in my logic.  You could do the same in
 all other cases too, e.g. replace a tree so that an entry is of a
 different type and at the same time change the type of the object
 itself.  You however have to carefully go through all objects that refer
 to the one that was replaced, and fix the type in all of them.

 It still seems an extremely unsafe thing to do with trees...
  ...
 Should we add a --force flag of some sort to allow the user to do this,
 while keeping the normal safety checks?
 
 As long as we do not forbid such an unusual replacement on the
 reading side, we won't break people who are more inventive than we
 are (I am not convinced that we know people's workflow well enough
 to definitively say that no sane workflow, which benefits from being
 able to replace an object with another from a different type,
 exists).
 
 Preventing git replace wrapper from creating such a replacement by
 default will make it harder to do and may reduce mistakes, without
 breaking them too much, I think.

I agree. It is always possible to create replacement refs using git
update-ref if one really wants to.

What about using the following in the commit message or in the
documentation:

--

DISCUSSION

If one object is replaced with one of a different type, the only way
to keep the history valid is to also replace all the other objects
that point to the replaced object. That's because:

* Annotated tags contain the type of the tagged object.

* The tree/parent lines in commits must be a tree and commits, resp.

* The object types referred to by trees are specified in the 'mode'
  field:
100644 and 100755blob
16   commit
04   tree
  (these are the only valid modes)

* Blobs don't point at anything.

But if all the objects that point to an object, called O, are to be
replaced, then in most cases object O probably doesn't need to be
replaced. It's probably sufficient to create the new object, called
O2, that would replace object O and to replace all the objects
pointing to object O with objects pointing to O2.

The only case where someone might really want to replace object 0,
with an object O2 of a different type, and all the objects pointing to
it, is if it's really important, perhaps for external reasons, to have
object O's SHA1 point to O2.

And anyway, if one really wants to do that, it can still be done using
git update-ref.

--

Thanks,
Christian.
--
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


nd/magic-pathspec exposes breakage in git-add--interactive on Windows

2013-08-29 Thread Johannes Sixt
With nd/magic-pathspec I get the following failure on Windows in
t2016-checkout-patch.sh:

expecting success:
set_state dir/foo work head 
# the third n is to get out in case it mistakenly does not apply
(echo y; echo n; echo n) | (cd dir  git checkout -p foo) 
verify_saved_state bar 
verify_state dir/foo head head

 xx.
 1d.
 10e.
msys does not support: :(prefix:4)dir/foo
not ok 13 - path limiting works: foo inside dir

The error message 'msys does not support...' originates from this function
in git-add--interactive.perl (which is invoked from checkout -p):

  sub run_cmd_pipe {
if ($^O eq 'MSWin32' || $^O eq 'msys') {
my @invalid = grep {m/[:*]/} @_;
die $^O does not support: @invalid\n if @invalid;
my @args = map { m/ /o ? \$_\: $_ } @_;
return qx{@args};
} else {
my $fh = undef;
open($fh, '-|', @_) or die;
return $fh;
}
  }

It looks like on Windows we disallow arguments that contain double-quote,
colon, or asterisk, and otherwise wrap arguments in double-quotes if they
contain space. Then pass them through qx{}, which I can only guess what it
does.

This code was introduced in 21e9757e (Hack git-add--interactive to make it
work with ActiveState Perl, 2007-08-01). The commit message has the
general statement It wont work for arguments with special characters
(like , : or *)), which I do not know how to interpret: Does ActiveState
Perl not work with special charactoers, or Windows? Because the latter is
definitely not true.

Can we be more permissive in the 'my @invalid' check without breaking
ActiveState Perl?

BTW, there is a similar failure in t7105-reset-patch.sh, which invokes
'git reset -p', but I haven't investigate further.

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


Re: [RFC/PATCH v2 3/3] status: introduce status.displayCommentChar to disable display of #

2013-08-29 Thread Matthieu Moy
David Aguilar dav...@gmail.com writes:

 I have a poor imagination and cannot imagine why it needs to be
 switchable.

I could not either, but I found the reason in the commit message:
eff80a9fd990

Some users do want to write a line that begin with a pound sign, #,
in their commit log message.  Many tracking system recognise
a token of #bugid form, for example.

The support we offer these use cases is not very friendly to the end
users.  They have a choice between

 - Don't do it.  Avoid such a line by rewrapping or indenting; and

 - Use --cleanup=whitespace but remove all the hint lines we add.

Give them a way to set a custom comment char, e.g.

$ git -c core.commentchar=% commit

so that they do not have to do either of the two workarounds.

I personnally think allowing an escape scheme (\#) would have been
better.

But as Junio said, it's too late. My change is not about commentchar
customizability, but about disabling the comment in git status.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC/PATCH v2 2/3] submodule: introduce --[no-]display-comment-char

2013-08-29 Thread Matthieu Moy
Jens Lehmann jens.lehm...@web.de writes:

 But I'm not terribly happy about having the --for-status option in the
 submodule script in the first place, as I believe it should rather be
 handled by wt-status.c itself (reading the output of submodule summary
 using the start_command()/finish_command() combo instead of the simple
 run_command() currently used and prepending the comment char by using
 status_printf()  friends for each line).

I went the --for-status way because it's easier to do this in shell than
in C. But I'm close to be convinced that implementing this on the
wt-status.c side is better. I'll try a re-roll doing this when I get
time.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Aug 2013, #07; Wed, 28)

2013-08-29 Thread Matthieu Moy
Junio C Hamano gits...@pobox.com writes:

 * sb/repack-in-c (2013-08-22) 3 commits
  - repack: rewrite the shell script in C (squashing proposal)
  - repack: retain the return value of pack-objects
  - repack: rewrite the shell script in C

Just a ping to make sure the series is not forgotten. Stefan, I guess
it's time to send the squashed and hopefully final version.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Aug 2013, #07; Wed, 28)

2013-08-29 Thread Stefan Beller
On 08/29/2013 09:20 AM, Matthieu Moy wrote:
 it's time to send the squashed and hopefully final version.

I will do so tonight.

Thanks,
Stefan



signature.asc
Description: OpenPGP digital signature


[BUGLET] Short form of options with an optional parameter are not very clear

2013-08-29 Thread Yann Dirson
I just stumbled over the git status -uno form, and it took me some
time to realize that no was a parameter to -u, rather than aggregated
(and undocumented) -n and -o.

Whereas the manpage does document the -u[mode] syntax, which dissipate
the misunderstanding, --help output does not, listing -u in a way that
appears not to take any option.

Looks like the --help output would gain from having the optional parameter
displayed in the short form...

-- 
Yann Dirson - Bertin Technologies
--
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: Configure a prompt callback for gnome_keyring.

2013-08-29 Thread Matthijs Kooijman
Hi folks,

any chance this patch can be merged?

Gr.

Matthijs

On Tue, Jun 18, 2013 at 06:38:10PM +0200, Matthijs Kooijman wrote:
 This allows git-svn to prompt for a keyring unlock password, when a
 the needed gnome keyring is locked.
 
 This requires changes in the subversion perl bindings which have been
 committed to svn trunk (r1241554 and some followup commits) and are
 first available in the 1.8.0 release.
 ---
  perl/Git/SVN/Prompt.pm |  5 +
  perl/Git/SVN/Ra.pm | 13 +
  2 files changed, 18 insertions(+)
 
 diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
 index e940b08..faeda01 100644
 --- a/perl/Git/SVN/Prompt.pm
 +++ b/perl/Git/SVN/Prompt.pm
 @@ -23,6 +23,11 @@ sub simple {
   $SVN::_Core::SVN_NO_ERROR;
  }
  
 +sub gnome_keyring_unlock {
 + my ($keyring, $pool) = @_;
 + _read_password(Password for '$keyring' GNOME keyring: , undef);
 +}
 +
  sub ssl_server_trust {
   my ($cred, $realm, $failures, $cert_info, $may_save, $pool) = @_;
   $may_save = undef if $_no_auth_cache;
 diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
 index 75ecc42..38ed0cb 100644
 --- a/perl/Git/SVN/Ra.pm
 +++ b/perl/Git/SVN/Ra.pm
 @@ -104,6 +104,19 @@ sub new {
   }
   } # no warnings 'once'
  
 + # Allow git-svn to show a prompt for opening up a gnome-keyring, if 
 needed.
 + if (defined(SVN::Core::auth_set_gnome_keyring_unlock_prompt_func)) {
 + my $keyring_callback = 
 SVN::Core::auth_set_gnome_keyring_unlock_prompt_func(
 + $baton,
 + \Git::SVN::Prompt::gnome_keyring_unlock
 + );
 + # Keep a reference to this callback, to prevent the function
 + # (reference) from being garbage collected.  We just add it to
 + # the callbacks value, which are also used only to prevent the
 + # garbage collector from eating stuff.
 + $callbacks = [$callbacks, $keyring_callback]
 + }
 +
   my $self = SVN::Ra-new(url = $url, auth = $baton,
 config = $config,
 pool = SVN::Pool-new,
 -- 
 1.8.3.rc1
 
--
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: nd/magic-pathspec exposes breakage in git-add--interactive on Windows

2013-08-29 Thread Duy Nguyen
On Thu, Aug 29, 2013 at 1:54 PM, Johannes Sixt j.s...@viscovery.net wrote:
 With nd/magic-pathspec I get the following failure on Windows in
 t2016-checkout-patch.sh:

 expecting success:
 set_state dir/foo work head 
 # the third n is to get out in case it mistakenly does not apply
 (echo y; echo n; echo n) | (cd dir  git checkout -p foo) 
 verify_saved_state bar 
 verify_state dir/foo head head

  xx.
  1d.
  10e.
 msys does not support: :(prefix:4)dir/foo
 not ok 13 - path limiting works: foo inside dir

 The error message 'msys does not support...' originates from this function
 in git-add--interactive.perl (which is invoked from checkout -p):

   sub run_cmd_pipe {
 if ($^O eq 'MSWin32' || $^O eq 'msys') {
 my @invalid = grep {m/[:*]/} @_;
 die $^O does not support: @invalid\n if @invalid;
 my @args = map { m/ /o ? \$_\: $_ } @_;
 return qx{@args};
 } else {
 my $fh = undef;
 open($fh, '-|', @_) or die;
 return $fh;
 }
   }

 It looks like on Windows we disallow arguments that contain double-quote,
 colon, or asterisk, and otherwise wrap arguments in double-quotes if they
 contain space. Then pass them through qx{}, which I can only guess what it
 does.

 This code was introduced in 21e9757e (Hack git-add--interactive to make it
 work with ActiveState Perl, 2007-08-01). The commit message has the
 general statement It wont work for arguments with special characters
 (like , : or *)), which I do not know how to interpret: Does ActiveState
 Perl not work with special charactoers, or Windows? Because the latter is
 definitely not true.

If it indeed does not work with ActiveState Perl, we still have a
chance to make it :(prefix=4)dir/foo, assuming '=' does not fall to
the same category as ':'


 Can we be more permissive in the 'my @invalid' check without breaking
 ActiveState Perl?

 BTW, there is a similar failure in t7105-reset-patch.sh, which invokes
 'git reset -p', but I haven't investigate further.

 -- Hannes
-- 
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 4/6] upload-pack: delegate rev walking in shallow fetch to pack-objects

2013-08-29 Thread Duy Nguyen
On Wed, Aug 28, 2013 at 9:52 PM, Matthijs Kooijman matth...@stdin.nl wrote:
 Hi Nguy,

 On Fri, Aug 16, 2013 at 04:52:05PM +0700, Nguyễn Thái Ngọc Duy wrote:
 upload-pack has a special rev walking code for shallow recipients. It
 works almost like the similar code in pack-objects except:

 1. in upload-pack, graft points could be added for deepening

 2. also when the repository is deepened, the shallow point will be
moved further away from the tip, but the old shallow point will be
marked as edge to produce more efficient packs. See 6523078 (make
shallow repository deepening more network efficient - 2009-09-03)

 pass the file to pack-objects via --shallow-file. This will override
 $GIT_DIR/shallow and give pack-objects the exact repository shape that
 upload-pack has.

 mark edge commits by revision command arguments. Even if old shallow
 points are passed as --not revisions as in this patch, they will not
 be picked up by mark_edges_uninteresting() because this function looks
 up to parents for edges, while in this case the edge is the children,
 in the opposite direction. This will be fixed in the next patch when
 all given uninteresting commits are marked as edges.
 This says the next patch but it really refers to 6/6, not 5/6. Patch
 6/6 has the same problem (it says previous patch). Perhaps patches 4
 and 5 should just be swapped?

Yeah. I guess I reordered the patches before sending out and forgot
that the commit message needs a special order. Wil do.
-- 
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] Add testcase for needless objects during a shallow fetch

2013-08-29 Thread Duy Nguyen
On Wed, Aug 28, 2013 at 11:02 PM, Matthijs Kooijman matth...@stdin.nl wrote:
 This is a testcase that checks for a problem where, during a specific
 shallow fetch where the client does not have any commits that are a
 successor of the new shallow root (i.e., the fetch creates a new
 detached piece of history), the server would simply send over _all_
 objects, instead of taking into account the objects already present in
 the client.

Thanks. This reminds me I should add a test case in the 4/6 to
demonstrate the regression and let it verify again in 6/6 that the
temporary regression is gone. Will reroll the series with your patch
included.


 The actual problem was fixed by a recent patch series by Nguyễn Thái
 Ngọc Duy already.

 Signed-off-by: Matthijs Kooijman matth...@stdin.nl
 ---
  t/t5500-fetch-pack.sh | 11 +++
  1 file changed, 11 insertions(+)

 diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
 index fd2598e..a022d65 100755
 --- a/t/t5500-fetch-pack.sh
 +++ b/t/t5500-fetch-pack.sh
 @@ -393,6 +393,17 @@ test_expect_success 'fetch in shallow repo unreachable 
 shallow objects' '
 git fsck --no-dangling
 )
  '
 +test_expect_success 'fetch creating new shallow root' '
 +   (
 +   git clone file://$(pwd)/. shallow10 
 +   git commit --allow-empty -m empty 
 +   cd shallow10 
 +   git fetch --depth=1 --progress 2 actual 
 +   # This should fetch only the empty commit, no tree or
 +   # blob objects
 +   grep remote: Total 1 actual
 +   )
 +'

  test_expect_success 'setup tests for the --stdin parameter' '
 for head in C D E F
 --
 1.8.4.rc1




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


GIT pre-commit hook problem with git commit -m 'text' file.c syntax

2013-08-29 Thread Michal Dudek
Hi!
I am writing a pre-hook script, which will be responsible for checking style 
(formatting/indenting) of the C source code. The script at the beginning 
executes git diff --name-only -z --cached HEAD in order to get all the 
committed changes, then it makes a two copies of the staged file, one is kept 
as an original and the second one is formatted by astyle program. Then I am 
creating patch using git diff between the original file and formatted one. 
After that I am applying patch (asking user some questions before doing that) 
to the index or to the file in the working tree (behaviour depends on the 
situation, if the file was staged and modified after that (git status returns 
file as staged and modified but NOT staged) I am applying changes to index 
using git apply -v --cached patch, otherwise if the file was staged and no 
changes to it were made afterwards (git status returns file as staged only) I 
am modifying file in the working tree using git apply -v patch and adding 
modified file to stage using git add my_file.c). Everything works fine till 
the moment when user modifies a file and commits it using command git commit 
-m 'comment' my_file.c. After that the committed file looks ok (also in git 
log etc.) but when I type git status or use gitk I see some leftovers. Gitk 
shows a local changes checked to index but not committed -- this is a raw 
file before applying a patch -- and local uncommitted changes not checked into 
index -- this is a file after patch was applied. The manual says that listing 
files as arguments to the commit command, make the git ignore changes staged in 
the index, and instead record the current content of the listed files. I would 
like to ask you if there is a way to force commit to ALWAYS add files before 
commit? Can I distinguish, that the commit was executed using syntax git 
commit -m 'text' file.c? Is there any other solution for my problem?

Thank you for your time!
Michal Dudek

--
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-init does not respect existing separate-git-dir

2013-08-29 Thread Ximin Luo
It should not be necessary to re-specify --separate-git-dir when 
re-initialising a git repo.

$ git init --separate-git-dir ../repo
Initialized empty Git repository in /home/infinity0/tmp/repo/

$ git init
/home/infinity0/tmp/wtree/.git/refs: Not a directory
1

One big motivation is so git init can be a good fire-and-forget invocation 
that should work anywhere. Currently, one has to do git init 
--separate-git-dir $(git rev-parse --git-dir) which is a lot less elegant.

(Please CC me as I am not subscribed.)

-- 
GPG: 4096R/1318EFAC5FBBDBCE
git://github.com/infinity0/pubkeys.git



signature.asc
Description: OpenPGP digital signature


Re: [BUG] git-init does not respect existing separate-git-dir

2013-08-29 Thread Duy Nguyen
On Thu, Aug 29, 2013 at 01:39:02PM +0100, Ximin Luo wrote:
 It should not be necessary to re-specify --separate-git-dir when 
 re-initialising a git repo.
 
 $ git init --separate-git-dir ../repo
 Initialized empty Git repository in /home/infinity0/tmp/repo/
 
 $ git init
 /home/infinity0/tmp/wtree/.git/refs: Not a directory
 1

This patch seems to work. Lightly tested.

-- 8 --
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 78aa387..d0e5b2e 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -192,6 +192,15 @@ static int create_default_files(const char *template_path)
die(_(insane git directory %s), git_dir);
memcpy(path, git_dir, len);
 
+   if (!lstat(path, st1)  S_ISREG(st1.st_mode)) {
+   git_dir = read_gitfile(git_dir);
+   len = strlen(git_dir);
+   if (len  sizeof(path)-50)
+   die(_(insane git directory %s), git_dir);
+   set_git_dir(git_dir);
+   memcpy(path, git_dir, len);
+   }
+
if (len  path[len-1] != '/')
path[len++] = '/';
-- 8 -- 

 One big motivation is so git init can be a good fire-and-forget
 invocation that should work anywhere. Currently, one has to do git
 init --separate-git-dir $(git rev-parse --git-dir) which is a lot
 less elegant.
--
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: nd/magic-pathspec exposes breakage in git-add--interactive on Windows

2013-08-29 Thread Alex Riesen
On Thu, Aug 29, 2013 at 8:54 AM, Johannes Sixt j.s...@viscovery.net wrote:
 It looks like on Windows we disallow arguments that contain double-quote,
 colon, or asterisk, and otherwise wrap arguments in double-quotes if they
 contain space. Then pass them through qx{}, which I can only guess what it
 does.

Well, the command interpreter of the platform implementation caused
this, I believe.
Quoting of the double quotes for it was too cumbersome for rarely used
combination
(windows, ActiveState Perl, and Git). I guess it is more popular now?
--
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


[RFC/PATCH v3 0/4] Disable git status comment prefix

2013-08-29 Thread Matthieu Moy
So, here's a reroll that makes the code cleaner.

First patches are cleanups without behavioral changes, only the last
one does something new.

I'm waiting for more comments to decide what to do with the
configuration option. Right now, my preference would be to call it
status.oldStyle and default it to false (i.e. change the behavior, but
allow old-timers to get back the old one).

Matthieu Moy (4):
  builtin/stripspace.c: fix broken indentation
  wt-status: use argv_array API
  get rid of git submodule summary --for-status
  status: introduce status.displayCommentChar to disable display of #

 Documentation/config.txt |  5 +++
 builtin/commit.c |  9 +
 builtin/stripspace.c |  8 ++---
 git-submodule.sh | 17 +
 t/t7401-submodule-summary.sh | 13 ---
 t/t7508-status.sh| 43 +++
 wt-status.c  | 82 +---
 wt-status.h  |  1 +
 8 files changed, 125 insertions(+), 53 deletions(-)

-- 
1.8.4.12.gf9d53a3.dirty

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


[PATCH v3 1/4] builtin/stripspace.c: fix broken indentation

2013-08-29 Thread Matthieu Moy
Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 builtin/stripspace.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/builtin/stripspace.c b/builtin/stripspace.c
index e981dfb..1259ed7 100644
--- a/builtin/stripspace.c
+++ b/builtin/stripspace.c
@@ -89,11 +89,11 @@ int cmd_stripspace(int argc, const char **argv, const char 
*prefix)
 
if (argc == 2) {
if (!strcmp(argv[1], -s) ||
-   !strcmp(argv[1], --strip-comments)) {
-strip_comments = 1;
+   !strcmp(argv[1], --strip-comments)) {
+   strip_comments = 1;
} else if (!strcmp(argv[1], -c) ||
-!strcmp(argv[1], --comment-lines)) {
-mode = COMMENT_LINES;
+  !strcmp(argv[1], --comment-lines)) {
+   mode = COMMENT_LINES;
} else {
mode = INVAL;
}
-- 
1.8.4.12.gf9d53a3.dirty

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


[PATCH v3 2/4] wt-status: use argv_array API

2013-08-29 Thread Matthieu Moy
No behavior change, but two slight code reorganization: argv_array_push
doesn't accept NULL strings, and duplicates its argument hence
summary_limit must be written to before being inserted into argv.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 wt-status.c | 26 ++
 1 file changed, 14 insertions(+), 12 deletions(-)

diff --git a/wt-status.c b/wt-status.c
index cb24f1f..958a53c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -8,6 +8,7 @@
 #include diffcore.h
 #include quote.h
 #include run-command.h
+#include argv-array.h
 #include remote.h
 #include refs.h
 #include submodule.h
@@ -663,29 +664,30 @@ static void wt_status_print_submodule_summary(struct 
wt_status *s, int uncommitt
char summary_limit[64];
char index[PATH_MAX];
const char *env[] = { NULL, NULL };
-   const char *argv[8];
-
-   env[0] =index;
-   argv[0] =   submodule;
-   argv[1] =   summary;
-   argv[2] =   uncommitted ? --files : --cached;
-   argv[3] =   --for-status;
-   argv[4] =   --summary-limit;
-   argv[5] =   summary_limit;
-   argv[6] =   uncommitted ? NULL : (s-amend ? HEAD^ : HEAD);
-   argv[7] =   NULL;
+   struct argv_array argv = ARGV_ARRAY_INIT;
 
sprintf(summary_limit, %d, s-submodule_summary);
snprintf(index, sizeof(index), GIT_INDEX_FILE=%s, s-index_file);
 
+   env[0] = index;
+   argv_array_push(argv, submodule);
+   argv_array_push(argv, summary);
+   argv_array_push(argv, uncommitted ? --files : --cached);
+   argv_array_push(argv, --for-status);
+   argv_array_push(argv, --summary-limit);
+   argv_array_push(argv, summary_limit);
+   if (!uncommitted)
+   argv_array_push(argv, s-amend ? HEAD^ : HEAD);
+
memset(sm_summary, 0, sizeof(sm_summary));
-   sm_summary.argv = argv;
+   sm_summary.argv = argv.argv;
sm_summary.env = env;
sm_summary.git_cmd = 1;
sm_summary.no_stdin = 1;
fflush(s-fp);
sm_summary.out = dup(fileno(s-fp));/* run_command closes it */
run_command(sm_summary);
+   argv_array_clear(argv);
 }
 
 static void wt_status_print_other(struct wt_status *s,
-- 
1.8.4.12.gf9d53a3.dirty

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


[PATCH v3 3/4] get rid of git submodule summary --for-status

2013-08-29 Thread Matthieu Moy
The --for-status option was an undocumented option used only by
wt-status.c, which inserted a header and commented out the output. We can
achieve the same result within wt-status.c, without polluting the
submodule command-line options.

This will make it easier to disable the comments from wt-status.c later.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 git-submodule.sh | 17 +
 t/t7401-submodule-summary.sh | 13 -
 wt-status.c  | 29 +++--
 3 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..fccdec9 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -965,7 +965,6 @@ set_name_rev () {
 #
 cmd_summary() {
summary_limit=-1
-   for_status=
diff_cmd=diff-index
 
# parse $args after submodule ... summary.
@@ -978,9 +977,6 @@ cmd_summary() {
--files)
files=$1
;;
-   --for-status)
-   for_status=$1
-   ;;
-n|--summary-limit)
summary_limit=$2
isnumber $summary_limit || usage
@@ -1149,18 +1145,7 @@ cmd_summary() {
echo
fi
echo
-   done |
-   if test -n $for_status; then
-   if [ -n $files ]; then
-   gettextln Submodules changed but not updated: | git 
stripspace -c
-   else
-   gettextln Submodule changes to be committed: | git 
stripspace -c
-   fi
-   printf \n | git stripspace -c
-   git stripspace -c
-   else
-   cat
-   fi
+   done
 }
 #
 # List all submodules, prefixed with:
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index ac2434c..b435d03 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -262,19 +262,6 @@ EOF
test_cmp expected actual
 
 
-test_expect_success '--for-status' 
-   git submodule summary --for-status HEAD^ actual 
-   test_i18ncmp actual - EOF
-# Submodule changes to be committed:
-#
-# * sm1 $head6...000:
-#
-# * sm2 000...$head7 (2):
-#Add foo9
-#
-EOF
-
-
 test_expect_success 'fail when using --files together with --cached' 
test_must_fail git submodule summary --files --cached
 
diff --git a/wt-status.c b/wt-status.c
index 958a53c..d91661d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -665,6 +665,10 @@ static void wt_status_print_submodule_summary(struct 
wt_status *s, int uncommitt
char index[PATH_MAX];
const char *env[] = { NULL, NULL };
struct argv_array argv = ARGV_ARRAY_INIT;
+   struct strbuf cmd_stdout = STRBUF_INIT;
+   struct strbuf summary = STRBUF_INIT;
+   char *summary_content;
+   size_t len;
 
sprintf(summary_limit, %d, s-submodule_summary);
snprintf(index, sizeof(index), GIT_INDEX_FILE=%s, s-index_file);
@@ -673,7 +677,6 @@ static void wt_status_print_submodule_summary(struct 
wt_status *s, int uncommitt
argv_array_push(argv, submodule);
argv_array_push(argv, summary);
argv_array_push(argv, uncommitted ? --files : --cached);
-   argv_array_push(argv, --for-status);
argv_array_push(argv, --summary-limit);
argv_array_push(argv, summary_limit);
if (!uncommitted)
@@ -685,9 +688,31 @@ static void wt_status_print_submodule_summary(struct 
wt_status *s, int uncommitt
sm_summary.git_cmd = 1;
sm_summary.no_stdin = 1;
fflush(s-fp);
-   sm_summary.out = dup(fileno(s-fp));/* run_command closes it */
+   sm_summary.out = -1;
+
run_command(sm_summary);
argv_array_clear(argv);
+
+   len = strbuf_read(cmd_stdout, sm_summary.out, 1024);
+
+   /* prepend header, only if there's an actual output */
+   if (len) {
+   if (uncommitted)
+   strbuf_addstr(summary, _(Submodules changed but not 
updated:));
+   else
+   strbuf_addstr(summary, _(Submodule changes to be 
committed:));
+   strbuf_addstr(summary, \n\n);
+   }
+   strbuf_addbuf(summary, cmd_stdout);
+   strbuf_release(cmd_stdout);
+
+   summary_content = strbuf_detach(summary, len);
+   strbuf_add_commented_lines(summary, summary_content, len);
+   free(summary_content);
+
+   summary_content = strbuf_detach(summary, len);
+   fprintf(s-fp, summary_content);
+   free(summary_content);
 }
 
 static void wt_status_print_other(struct wt_status *s,
-- 
1.8.4.12.gf9d53a3.dirty

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


[PATCH v3 4/4] status: introduce status.displayCommentChar to disable display of #

2013-08-29 Thread Matthieu Moy
Historically, git status needed to prefix each output line with '#' so
that the output could be added as comment to the commit message. This
prefix comment has no real purpose when git status is ran from the
command-line, and this may distract users from the real content.

Allow the user to disable this prefix comment. In the long run, if users
like the non-prefix output, it may make sense to flip the default value
to true.

Obviously, status.displayCommentChar applies to git status but is
ignored by git commit, so the status is still commented in
COMMIT_EDITMSG.

Signed-off-by: Matthieu Moy matthieu@imag.fr
---
 Documentation/config.txt |  5 +
 builtin/commit.c |  9 +
 t/t7508-status.sh| 43 +++
 wt-status.c  | 35 +--
 wt-status.h  |  1 +
 5 files changed, 83 insertions(+), 10 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ec57a15..0afa531 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2118,6 +2118,11 @@ status.branch::
Set to true to enable --branch by default in linkgit:git-status[1].
The option --no-branch takes precedence over this variable.
 
+status.displayCommentChar::
+   If set to false, linkgit:git-status[1] will not prefix each
+   line with the comment char (`core.commentChar`, i.e. `#` by
+   default). Defaults to true.
+
 status.showUntrackedFiles::
By default, linkgit:git-status[1] and linkgit:git-commit[1] show
files which are not currently tracked by Git. Directories which
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..d4cfced 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1182,6 +1182,10 @@ static int git_status_config(const char *k, const char 
*v, void *cb)
s-use_color = git_config_colorbool(k, v);
return 0;
}
+   if (!strcmp(k, status.displaycommentchar)) {
+   s-display_comment_char = git_config_bool(k, v);
+   return 0;
+   }
if (!prefixcmp(k, status.color.) || !prefixcmp(k, color.status.)) {
int slot = parse_status_slot(k, 13);
if (slot  0)
@@ -1496,6 +1500,11 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
gitmodules_config();
git_config(git_commit_config, s);
status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
+   /*
+* Ignore status.displayCommentChar: we do need comments in
+* COMMIT_EDITMSG.
+*/
+   s.display_comment_char = 1;
determine_whence(s);
s.colopts = 0;
 
diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index ac3d0fe..d0d7c4a 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh
@@ -113,6 +113,39 @@ test_expect_success 'status (2)' '
test_i18ncmp expect output
 '
 
+strip_comments () {
+   sed s/^\# //; s/^\#$//; s/^#\t/\t/ $1 $1.tmp 
+   rm $1  mv $1.tmp $1
+}
+
+test_expect_success 'status with status.displayCommentChar=false' '
+   strip_comments expect 
+   git -c status.displayCommentChar=false status output 
+   test_i18ncmp expect output
+'
+
+test_expect_success 'setup fake editor' '
+   cat .git/editor -\EOF 
+   #! /bin/sh
+   cp $1 output
+EOF
+   chmod 755 .git/editor
+'
+
+commit_template_commented () {
+   (
+   EDITOR=.git/editor 
+   export EDITOR 
+   # Fails due to empty message
+   test_must_fail git commit
+   ) 
+   ! grep '^[^#]' output
+}
+
+test_expect_success 'commit ignores status.displayCommentChar=false in 
COMMIT_EDITMSG' '
+   commit_template_commented
+'
+
 cat expect \EOF
 # On branch master
 # Changes to be committed:
@@ -872,6 +905,16 @@ test_expect_success 'status submodule summary' '
test_i18ncmp expect output
 '
 
+test_expect_success 'status submodule summary with 
status.displayCommentChar=false' '
+   strip_comments expect 
+   git -c status.displayCommentChar=false status output 
+   test_i18ncmp expect output
+'
+
+test_expect_success 'commit with submodule summary ignores displayCommentChar' 
'
+   commit_template_commented
+'
+
 cat expect EOF
  M dir1/modified
 A  dir2/added
diff --git a/wt-status.c b/wt-status.c
index d91661d..cdbcd6f 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -46,9 +46,11 @@ static void status_vprintf(struct wt_status *s, int at_bol, 
const char *color,
 
strbuf_vaddf(sb, fmt, ap);
if (!sb.len) {
-   strbuf_addch(sb, comment_line_char);
-   if (!trail)
-   strbuf_addch(sb, ' ');
+   if (s-display_comment_char) {
+   strbuf_addch(sb, comment_line_char);
+   if (!trail)
+   strbuf_addch(sb, ' ');
+   }
color_print_strbuf(s-fp, color, sb);
 

[PATCH/RFC 7/7] update-ref: support multiple simultaneous updates

2013-08-29 Thread Brad King
Add a --stdin signature to read update instructions from standard input
and apply multiple ref updates and deletes together.

Signed-off-by: Brad King brad.k...@kitware.com
---
 Documentation/git-update-ref.txt |   19 +++-
 builtin/update-ref.c |   93 +-
 2 files changed, 110 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
index 0df13ff..a79afe8 100644
--- a/Documentation/git-update-ref.txt
+++ b/Documentation/git-update-ref.txt
@@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely
 SYNOPSIS
 
 [verse]
-'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] ref 
newvalue [oldvalue])
+'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] ref 
newvalue [oldvalue] | --stdin)
 
 DESCRIPTION
 ---
@@ -58,6 +58,23 @@ archive by creating a symlink tree).
 With `-d` flag, it deletes the named ref after verifying it
 still contains oldvalue.
 
+With `--stdin`, update-ref reads instructions from standard input
+and performs all modifications together.  Specify updates with
+lines of the form:
+
+   [ --no-deref SP ] ref SP newvalue [ SP oldvalue ] LF
+
+and deletes with lines of the form:
+
+   [ --no-deref SP ] -d SP ref [ SP oldvalue ] LF
+
+or as updates with 40 0 as newvalue.  Blank lines are ignored.
+Lines of any other format or a repeated ref produce an error.
+If all refs can be locked with matching oldvalues
+simultaneously all modifications are performed.  Otherwise, no
+modifications are performed.  Note that while each individual
+ref is updated or deleted atomically, a concurrent reader may
+still see a subset of the modifications.
 
 Logging Updates
 ---
diff --git a/builtin/update-ref.c b/builtin/update-ref.c
index 51d2684..2f0d34c 100644
--- a/builtin/update-ref.c
+++ b/builtin/update-ref.c
@@ -6,19 +6,102 @@
 static const char * const git_update_ref_usage[] = {
N_(git update-ref [options] -d refname [oldval]),
N_(git update-ref [options]refname newval [oldval]),
+   N_(git update-ref [options] --stdin),
NULL
 };
 
+static const char blank[] =  \t\r\n;
+
+static int updates_size;
+static int updates_count;
+static struct ref_update *updates;
+
+static void update_refs_stdin(const char *line)
+{
+   int delete = 0, i;
+   const char *c, *s, *oldvalue, *value[2] = {0,0};
+   struct ref_update *update;
+   c = line;
+
+   /* Skip blank lines: */
+   if (*c == '\n')
+   return;
+
+   /* Allocate a ref_update struct: */
+   if (updates_count == updates_size) {
+   updates_size = updates_size? updates_size*2 : 16;
+   updates = xrealloc(updates, sizeof(*updates)*updates_size);
+   memset(updates + updates_count, 0,
+  sizeof(*updates)*(updates_size-updates_count));
+   }
+   update = updates[updates_count++];
+
+   /* --no-deref SP */
+   if (strncmp(c, --no-deref , 11) == 0) {
+   c += 11;
+   update-flags |= REF_NODEREF;
+   }
+
+   /* -d SP */
+   if (strncmp(c, -d , 3) == 0) {
+   c += 3;
+   delete = 1;
+   }
+
+   /* ref */
+   s = c;
+   c = s + strcspn(s, blank);
+   update-ref_name = xstrndup(s, c-s);
+
+   /* [ SP value ]... */
+   for (i=0; i  2; ++i) {
+   if (*c != ' ')
+   break;
+   ++c;
+   s = c;
+   c = s + strcspn(s, blank);
+   value[i] = xstrndup(s, c-s);
+   }
+
+   if (*c  *c != '\n')
+   die(unrecognized input line: %s, line);
+
+   if (check_refname_format(update-ref_name, REFNAME_ALLOW_ONELEVEL))
+   die(invalid ref format on input line: %s, line);
+
+   if (delete) {
+   hashclr(update-new_sha1);
+   oldvalue = value[0];
+   if (value[1])
+   die(both newvalue and oldvalue on delete line: %s,
+   line);
+   } else {
+   if (!value[0])
+   die(missing newvalue on update line: %s, line);
+   if (get_sha1(value[0], update-new_sha1))
+   die(invalid newvalue on update line: %s, line);
+   oldvalue = value[1];
+   }
+   if (oldvalue) {
+   update-old_sha1 = xmalloc(20);
+   if (get_sha1(oldvalue, update-old_sha1))
+   die(invalid oldvalue on %s line: %s,
+   delete? delete:update, line);
+   }
+}
+
 int cmd_update_ref(int argc, const char **argv, const char *prefix)
 {
const char *refname, *oldval, *msg = NULL;
unsigned char sha1[20], oldsha1[20];
-   int delete = 0, no_deref = 0, flags = 0;
+   int delete = 0, no_deref = 0, read_stdin = 0, flags = 0;
+   char line[1000];

[PATCH/RFC 6/7] refs: add update_refs for multiple simultaneous updates

2013-08-29 Thread Brad King
Add 'struct ref_update' to encode the information needed to update or
delete a ref (name, new sha1, optional old sha1, no-deref flag).  Add
function 'update_refs' accepting an array of updates to perform.  First
acquire locks on all refs with verified old values.  Then update or
delete all refs accordingly.  Fail if any one lock cannot be obtained or
any one old value does not match.

Though the refs themeselves cannot be modified together in a single
atomic transaction, this function does enable some useful semantics.
For example, a caller may create a new branch starting from the head of
another branch and rewind the original branch at the same time.  This
transfers ownership of commits between branches without risk of losing
commits added to the original branch by a concurrent process, or risk of
a concurrent process creating the new branch first.

Signed-off-by: Brad King brad.k...@kitware.com
---
 refs.c |   66 
 refs.h |   11 +++
 2 files changed, 77 insertions(+)

diff --git a/refs.c b/refs.c
index 5a6c14e..0a0c19e 100644
--- a/refs.c
+++ b/refs.c
@@ -3238,6 +3238,72 @@ int update_ref(const char *action, const char *refname,
return update_ref_write(action, refname, sha1, lock, onerr);
 }
 
+int update_refs(const char *action, struct ref_update *updates,
+   int n, enum action_on_err onerr)
+{
+   int ret = 0, delnum = 0, i;
+   int *types;
+   struct ref_lock **locks;
+   const char **delnames;
+
+   if (!updates || !n)
+   return 0;
+
+   /* Allocate work space: */
+   types = xmalloc(sizeof(int)*n);
+   locks = xmalloc(sizeof(struct ref_lock*)*n);
+   delnames = xmalloc(sizeof(const char*)*n);
+
+   /* Acquire all locks while verifying old values: */
+   for (i=0; i  n; ++i) {
+   locks[i] = update_ref_lock(updates[i].ref_name,
+  updates[i].old_sha1,
+  updates[i].flags,
+  types[i], onerr);
+   if (!locks[i])
+   break;
+   }
+
+   /* Abort if we did not get all locks: */
+   if (i  n) {
+   while (--i = 0)
+   unlock_ref(locks[i]);
+   free(types);
+   free(locks);
+   free(delnames);
+   return 1;
+   }
+
+   /* Perform updates first so live commits remain referenced: */
+   for (i=0; i  n; ++i)
+   if (!is_null_sha1(updates[i].new_sha1)) {
+   ret |= update_ref_write(action,
+   updates[i].ref_name,
+   updates[i].new_sha1,
+   locks[i], onerr);
+   locks[i] = 0; /* freed by update_ref_write */
+   }
+
+   /* Perform deletes now that updates are safely completed: */
+   for (i=0; i  n; ++i)
+   if (locks[i]) {
+   delnames[delnum++] = locks[i]-ref_name;
+   ret |= delete_ref_loose(locks[i], types[i]);
+   }
+   ret |= repack_without_refs(delnames, delnum);
+   for (i=0; i  delnum; ++i)
+   unlink_or_warn(git_path(logs/%s, delnames[i]));
+   clear_loose_ref_cache(ref_cache);
+   for (i=0; i  n; ++i)
+   if (locks[i])
+   unlock_ref(locks[i]);
+
+   free(types);
+   free(locks);
+   free(delnames);
+   return ret;
+}
+
 struct ref *find_ref_by_name(const struct ref *list, const char *name)
 {
for ( ; list; list = list-next)
diff --git a/refs.h b/refs.h
index 2cd307a..5763f3a 100644
--- a/refs.h
+++ b/refs.h
@@ -214,6 +214,17 @@ int update_ref(const char *action, const char *refname,
const unsigned char *sha1, const unsigned char *oldval,
int flags, enum action_on_err onerr);
 
+struct ref_update {
+   const char *ref_name;
+   unsigned char new_sha1[20];
+   unsigned char *old_sha1;
+   int flags;
+};
+
+/** lock all refs and then write all of them */
+int update_refs(const char *action, struct ref_update *updates,
+   int n, enum action_on_err onerr);
+
 extern int parse_hide_refs_config(const char *var, const char *value, const 
char *);
 extern int ref_is_hidden(const char *);
 
-- 
1.7.10.4

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


[PATCH/RFC 1/7] reset: rename update_refs to reset_refs

2013-08-29 Thread Brad King
Get it out of the way for a future refs.h function.

Signed-off-by: Brad King brad.k...@kitware.com
---
 builtin/reset.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/reset.c b/builtin/reset.c
index afa6e02..789ee48 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -219,7 +219,7 @@ static const char **parse_args(const char **argv, const 
char *prefix, const char
return argv[0] ? get_pathspec(prefix, argv) : NULL;
 }
 
-static int update_refs(const char *rev, const unsigned char *sha1)
+static int reset_refs(const char *rev, const unsigned char *sha1)
 {
int update_ref_status;
struct strbuf msg = STRBUF_INIT;
@@ -350,7 +350,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
if (!pathspec  !unborn) {
/* Any resets without paths update HEAD to the head being
 * switched to, saving the previous head in ORIG_HEAD before. */
-   update_ref_status = update_refs(rev, sha1);
+   update_ref_status = reset_refs(rev, sha1);
 
if (reset_type == HARD  !update_ref_status  !quiet)
print_new_head_line(lookup_commit_reference(sha1));
-- 
1.7.10.4

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


[PATCH/RFC 4/7] refs: factor delete_ref loose ref step into a helper

2013-08-29 Thread Brad King
Factor loose ref deletion into helper function delete_ref_loose to allow
later use elsewhere.  While at it, rename local names 'flag = type' and
'delopt = flags' for consistency with callers and called functions.

Signed-off-by: Brad King brad.k...@kitware.com
---
 refs.c |   24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 2e755b4..5908648 100644
--- a/refs.c
+++ b/refs.c
@@ -2450,15 +2450,10 @@ static int repack_without_ref(const char *refname)
return commit_packed_refs();
 }
 
-int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
+static int delete_ref_loose(struct ref_lock *lock, int type)
 {
-   struct ref_lock *lock;
-   int err, i = 0, ret = 0, flag = 0;
-
-   lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
-   if (!lock)
-   return 1;
-   if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
+   int err, i, ret = 0;
+   if (!(type  REF_ISPACKED) || type  REF_ISSYMREF) {
/* loose */
i = strlen(lock-lk-filename) - 5; /* .lock */
lock-lk-filename[i] = 0;
@@ -2468,6 +2463,19 @@ int delete_ref(const char *refname, const unsigned char 
*sha1, int delopt)
 
lock-lk-filename[i] = '.';
}
+   return ret;
+}
+
+int delete_ref(const char *refname, const unsigned char *sha1, int flags)
+{
+   struct ref_lock *lock;
+   int ret = 0, type = 0;
+
+   lock = lock_ref_sha1_basic(refname, sha1, flags, type);
+   if (!lock)
+   return 1;
+   ret |= delete_ref_loose(lock, type);
+
/* removing the loose one could have resurrected an earlier
 * packed one.  Also, if it was not loose we need to repack
 * without it.
-- 
1.7.10.4

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


[PATCH/RFC 5/7] refs: add function to repack without multiple refs

2013-08-29 Thread Brad King
Generalize repack_without_ref as repack_without_refs to support a list
of refs and implement the former in terms of the latter.

Signed-off-by: Brad King brad.k...@kitware.com
---
 refs.c |   29 ++---
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index 5908648..5a6c14e 100644
--- a/refs.c
+++ b/refs.c
@@ -2414,25 +2414,35 @@ static int curate_packed_ref_fn(struct ref_entry 
*entry, void *cb_data)
return 0;
 }
 
-static int repack_without_ref(const char *refname)
+static int repack_without_refs(const char **refnames, int n)
 {
struct ref_dir *packed;
struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
struct string_list_item *ref_to_delete;
+   int i, removed = 0;
+
+   /* Look for a packed ref: */
+   for (i = 0; i  n; ++i)
+   if (get_packed_ref(refnames[i]))
+   break;
 
-   if (!get_packed_ref(refname))
-   return 0; /* refname does not exist in packed refs */
+   /* Avoid locking if we have nothing to do: */
+   if(i == n)
+   return 0; /* no refname exists in packed refs */
 
if (lock_packed_refs(0)) {
unable_to_lock_error(git_path(packed-refs), errno);
-   return error(cannot delete '%s' from packed refs, refname);
+   return error(cannot delete '%s' from packed refs, 
refnames[i]);
}
packed = get_packed_refs(ref_cache);
 
-   /* Remove refname from the cache: */
-   if (remove_entry(packed, refname) == -1) {
+   /* Remove refnames from the cache: */
+   for (i = 0; i  n; ++i)
+   if (remove_entry(packed, refnames[i]) != -1)
+   removed = 1;
+   if (!removed) {
/*
-* The packed entry disappeared while we were
+* All packed entries disappeared while we were
 * acquiring the lock.
 */
rollback_packed_refs();
@@ -2450,6 +2460,11 @@ static int repack_without_ref(const char *refname)
return commit_packed_refs();
 }
 
+static int repack_without_ref(const char *refname)
+{
+   return repack_without_refs(refname, 1);
+}
+
 static int delete_ref_loose(struct ref_lock *lock, int type)
 {
int err, i, ret = 0;
-- 
1.7.10.4

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


[PATCH/RFC 2/7] refs: report ref type from lock_any_ref_for_update

2013-08-29 Thread Brad King
Expose lock_ref_sha1_basic's type_p argument to callers of
lock_any_ref_for_update.  Update all call sites to ignore it; we will
use it later.

Signed-off-by: Brad King brad.k...@kitware.com
---
 branch.c   |2 +-
 builtin/commit.c   |2 +-
 builtin/fetch.c|2 +-
 builtin/receive-pack.c |2 +-
 builtin/reflog.c   |2 +-
 builtin/replace.c  |2 +-
 builtin/tag.c  |2 +-
 fast-import.c  |2 +-
 refs.c |7 ---
 refs.h |2 +-
 sequencer.c|2 +-
 11 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/branch.c b/branch.c
index c5c6984..c244483 100644
--- a/branch.c
+++ b/branch.c
@@ -291,7 +291,7 @@ void create_branch(const char *head,
hashcpy(sha1, commit-object.sha1);
 
if (!dont_change_ref) {
-   lock = lock_any_ref_for_update(ref.buf, NULL, 0);
+   lock = lock_any_ref_for_update(ref.buf, NULL, 0, 0);
if (!lock)
die_errno(_(Failed to lock ref for update));
}
diff --git a/builtin/commit.c b/builtin/commit.c
index 10acc53..78d773f 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1618,7 +1618,7 @@ int cmd_commit(int argc, const char **argv, const char 
*prefix)
   !current_head
   ? NULL
   : current_head-object.sha1,
-  0);
+  0, 0);
 
nl = strchr(sb.buf, '\n');
if (nl)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index d784b2e..34903ef 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -246,7 +246,7 @@ static int s_update_ref(const char *action,
rla = default_rla.buf;
snprintf(msg, sizeof(msg), %s: %s, rla, action);
lock = lock_any_ref_for_update(ref-name,
-  check_old ? ref-old_sha1 : NULL, 0);
+  check_old ? ref-old_sha1 : NULL, 0, 0);
if (!lock)
return errno == ENOTDIR ? STORE_REF_ERROR_DF_CONFLICT :
  STORE_REF_ERROR_OTHER;
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e3eb5fc..dd61234 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -524,7 +524,7 @@ static const char *update(struct command *cmd)
return NULL; /* good */
}
else {
-   lock = lock_any_ref_for_update(namespaced_name, old_sha1, 0);
+   lock = lock_any_ref_for_update(namespaced_name, old_sha1, 0, 0);
if (!lock) {
rp_error(failed to lock %s, name);
return failed to lock;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 54184b3..11b30f9 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -366,7 +366,7 @@ static int expire_reflog(const char *ref, const unsigned 
char *sha1, int unused,
 * we take the lock for the ref itself to prevent it from
 * getting updated.
 */
-   lock = lock_any_ref_for_update(ref, sha1, 0);
+   lock = lock_any_ref_for_update(ref, sha1, 0, 0);
if (!lock)
return error(cannot lock ref '%s', ref);
log_file = git_pathdup(logs/%s, ref);
diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..e2e2002 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -105,7 +105,7 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
else if (!force)
die(replace ref '%s' already exists, ref);
 
-   lock = lock_any_ref_for_update(ref, prev, 0);
+   lock = lock_any_ref_for_update(ref, prev, 0, 0);
if (!lock)
die(%s: cannot lock the ref, ref);
if (write_ref_sha1(lock, repl, NULL)  0)
diff --git a/builtin/tag.c b/builtin/tag.c
index af3af3f..c261469 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -577,7 +577,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
if (annotate)
create_tag(object, tag, buf, opt, prev, object);
 
-   lock = lock_any_ref_for_update(ref.buf, prev, 0);
+   lock = lock_any_ref_for_update(ref.buf, prev, 0, 0);
if (!lock)
die(_(%s: cannot lock the ref), ref.buf);
if (write_ref_sha1(lock, object, NULL)  0)
diff --git a/fast-import.c b/fast-import.c
index 23f625f..5f7ef82 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1678,7 +1678,7 @@ static int update_branch(struct branch *b)
return 0;
if (read_ref(b-name, old_sha1))
hashclr(old_sha1);
-   lock = lock_any_ref_for_update(b-name, old_sha1, 0);
+   lock = lock_any_ref_for_update(b-name, old_sha1, 0, 0);
if (!lock)
return error(Unable to lock %s, b-name);
if (!force_update  

[PATCH/RFC 0/7] Multiple simultaneously locked ref updates

2013-08-29 Thread Brad King
Hi Folks,

While thinking about some how some server-side branch management
services might work, I came across a need to update multiple refs
locked with verified old values simultaneously.  For example, to
transfer ownership of some commits by rewinding a branch and creating
a new branch at the original head, one must lock both refs.
Otherwise, depending on the order of updates another process could
create the new branch after we've rewound the original, or add commits
to the original after we've created the new branch.

This series teaches update-ref a new --stdin option to read update and
delete instructions from lines of standard input, lock all refs up
front with verified old values, and then perform the modifications.
This is still work in progress, but it is ready for comments and
feedback.  The series is based on master as of v1.8.4.

Notable unfinished work:

* I propose a format for stdin lines in the last commit of the series
  as a proof-of-concept but I invite suggestions of better formats.
  The format must be able to specify updates and deletes with optional
  old values and optional no-deref.

* No tests for new features, though existing tests pass for me.

* No check for duplicate refs in input.  Currently a duplicate ref
  will result in a failure message like:

   fatal: Unable to create 'lock': File exists.
   If no other git process is currently running, this probably means a
   git process crashed in this repository earlier. Make sure no other git
   process is running and remove the file manually to continue.

  Instead we should reject duplicate ref names up front.  I would
  appreciate suggestions about an efficient data structure already
  available in Git to perform this lookup.

I welcome feedback on the approach, interface, and implementation.

Thanks,
-Brad

Brad King (7):
  reset: rename update_refs to reset_refs
  refs: report ref type from lock_any_ref_for_update
  refs: factor update_ref steps into helpers
  refs: factor delete_ref loose ref step into a helper
  refs: add function to repack without multiple refs
  refs: add update_refs for multiple simultaneous updates
  update-ref: support multiple simultaneous updates

 Documentation/git-update-ref.txt |   19 -
 branch.c |2 +-
 builtin/commit.c |2 +-
 builtin/fetch.c  |2 +-
 builtin/receive-pack.c   |2 +-
 builtin/reflog.c |2 +-
 builtin/replace.c|2 +-
 builtin/reset.c  |4 +-
 builtin/tag.c|2 +-
 builtin/update-ref.c |   93 ++-
 fast-import.c|2 +-
 refs.c   |  150 --
 refs.h   |   13 +++-
 sequencer.c  |2 +-
 14 files changed, 262 insertions(+), 35 deletions(-)

-- 
1.7.10.4

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


[PATCH/RFC 3/7] refs: factor update_ref steps into helpers

2013-08-29 Thread Brad King
Factor the lock and write steps and error handling into helper functions
update_ref_lock and update_ref_write to allow later use elsewhere.
Expose lock_any_ref_for_update's type_p to update_ref_lock callers.

Signed-off-by: Brad King brad.k...@kitware.com
---
 refs.c |   28 +++-
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index 3a7b597..2e755b4 100644
--- a/refs.c
+++ b/refs.c
@@ -3170,12 +3170,13 @@ int for_each_reflog(each_ref_fn fn, void *cb_data)
return retval;
 }
 
-int update_ref(const char *action, const char *refname,
-   const unsigned char *sha1, const unsigned char *oldval,
-   int flags, enum action_on_err onerr)
+static struct ref_lock *update_ref_lock(const char *refname,
+   const unsigned char *oldval,
+   int flags, int *type_p,
+   enum action_on_err onerr)
 {
static struct ref_lock *lock;
-   lock = lock_any_ref_for_update(refname, oldval, flags, 0);
+   lock = lock_any_ref_for_update(refname, oldval, flags, type_p);
if (!lock) {
const char *str = Cannot lock the ref '%s'.;
switch (onerr) {
@@ -3183,8 +3184,14 @@ int update_ref(const char *action, const char *refname,
case DIE_ON_ERR: die(str, refname); break;
case QUIET_ON_ERR: break;
}
-   return 1;
}
+   return lock;
+}
+
+static int update_ref_write(const char *action, const char *refname,
+   const unsigned char *sha1, struct ref_lock *lock,
+   enum action_on_err onerr)
+{
if (write_ref_sha1(lock, sha1, action)  0) {
const char *str = Cannot update the ref '%s'.;
switch (onerr) {
@@ -3197,6 +3204,17 @@ int update_ref(const char *action, const char *refname,
return 0;
 }
 
+int update_ref(const char *action, const char *refname,
+  const unsigned char *sha1, const unsigned char *oldval,
+  int flags, enum action_on_err onerr)
+{
+   static struct ref_lock *lock;
+   lock = update_ref_lock(refname, oldval, flags, 0, onerr);
+   if (!lock)
+   return 1;
+   return update_ref_write(action, refname, sha1, lock, onerr);
+}
+
 struct ref *find_ref_by_name(const struct ref *list, const char *name)
 {
for ( ; list; list = list-next)
-- 
1.7.10.4

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


The gitweb author initials feature from a36817b doesn't work with i18n names

2013-08-29 Thread Ævar Arnfjörð Bjarmason
The @author_initials feature Jakub added in a36817b claims to use a
i18n regexp (/\b([[:upper:]])\B/g), but in Perl this doesn't actually
do anything unless the string being matched against has the UTF8 flag.

So as a result it abbreviates me to AB not ÆAB. Here's something
that demonstrates the issue:

$ cat author-initials.pl
#!/usr/bin/env perl
use strict;
use warnings;

#binmode STDOUT, ':utf8';
open my $fd, -|, git, blame, --incremental, --, Makefile
or die Can't open: $!;
#binmode $fd, :utf8;
while (my $line = $fd) {
next unless my ($author) = $line =~ /^author (.*)/;
my @author_initials = ($author =~ /\b([[:upper:]])\B/g);
printf %s (%s)\n,  join(, @author_initials), $author;
}

With those two binmode commands commented out:

$ perl author-initials.pl |sort|uniq -c|sort -nr|head -n 5
 99 JH (Junio C Hamano)
 35 JN (Jonathan Nieder)
 35 JK (Jeff King)
 20 JS (Johannes Schindelin)
 16 AB (Ævar Arnfjörð Bjarmason)

And uncommented:

$ perl author-initials.pl |sort|uniq -c|sort -nr|head -n 5
 99 JH (Junio C Hamano)
 35 JN (Jonathan Nieder)
 35 JK (Jeff King)
 20 JS (Johannes Schindelin)
 16 ÆAB (Ævar Arnfjörð Bjarmason)

Jakub, do you see a reason not to just apply this:

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f429f75..29b3fb5 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6631,6 +6631,7 @@ sub git_blame_common {
$hash_base, '--', $file_name
or die_error(500, Open git-blame --porcelain failed);
}
+   binmode $fd, :utf8;

# incremental blame data returns early
if ($format eq 'data') {

I haven't gotten an env where I can test gitweb running, but that
looks like it should work to me.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: The gitweb author initials feature from a36817b doesn't work with i18n names

2013-08-29 Thread Jakub Narębski
[Ævar, sorry for duplication but I accidentally send HTML email; stupid Gmail]

On Thu, Aug 29, 2013 at 4:26 PM, Ævar Arnfjörð Bjarmason
ava...@gmail.com wrote:

 The @author_initials feature Jakub added in a36817b claims to use a
 i18n regexp (/\b([[:upper:]])\B/g), but in Perl this doesn't actually
 do anything unless the string being matched against has the UTF8 flag.

 So as a result it abbreviates me to AB not ÆAB.
[...]

Thanks for catching this, and for an analysis.

 Jakub, do you see a reason not to just apply this:

 diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
 index f429f75..29b3fb5 100755
 --- a/gitweb/gitweb.perl
 +++ b/gitweb/gitweb.perl
 @@ -6631,6 +6631,7 @@ sub git_blame_common {
 $hash_base, '--', $file_name
 or die_error(500, Open git-blame --porcelain 
 failed);
 }
 +   binmode $fd, :utf8;

 # incremental blame data returns early
 if ($format eq 'data') {

 I haven't gotten an env where I can test gitweb running, but that
 looks like it should work to me.

Unfortunetly I cannot check this either, but it looks obviously correct.
ACK.

-- 
Jakub Narebski
--
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/9] transport-helper: check for 'forced update' message

2013-08-29 Thread Felipe Contreras
So the remote-helpers can tell us when a forced push was needed.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 transport-helper.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/transport-helper.c b/transport-helper.c
index 62051a6..95dd72e 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -630,7 +630,7 @@ static int push_update_ref_status(struct strbuf *buf,
   struct ref *remote_refs)
 {
char *refname, *msg;
-   int status;
+   int status, forced = 0;
 
if (!prefixcmp(buf-buf, ok )) {
status = REF_STATUS_OK;
@@ -683,6 +683,11 @@ static int push_update_ref_status(struct strbuf *buf,
free(msg);
msg = NULL;
}
+   else if (!strcmp(msg, forced update)) {
+   forced = 1;
+   free(msg);
+   msg = NULL;
+   }
}
 
if (*ref)
@@ -704,6 +709,7 @@ static int push_update_ref_status(struct strbuf *buf,
}
 
(*ref)-status = status;
+   (*ref)-forced_update = forced;
(*ref)-remote_status = msg;
return !(status == REF_STATUS_OK);
 }
-- 
1.8.4-fc

--
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/9] fast-export: improve argument parsing

2013-08-29 Thread Felipe Contreras
We don't want to pass arguments specific to fast-export to
setup_revisions.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 builtin/fast-export.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 8e19058..91114f4 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -694,8 +694,9 @@ int cmd_fast_export(int argc, const char **argv, const char 
*prefix)
revs.topo_order = 1;
revs.show_source = 1;
revs.rewrite_parents = 1;
+   argc = parse_options(argc, argv, prefix, options, fast_export_usage,
+   PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
argc = setup_revisions(argc, argv, revs, NULL);
-   argc = parse_options(argc, argv, prefix, options, fast_export_usage, 0);
if (argc  1)
usage_with_options (fast_export_usage, options);
 
-- 
1.8.4-fc

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


[PATCH 0/9] transport-helper: updates

2013-08-29 Thread Felipe Contreras
Hi,

Here are the patches that allow transport helpers to be completely transparent;
renaming branches, deleting them, custom refspecs, --force, --dry-run,
reporting forced update, everything works.

Some of these were were sent before and rejected without a reason, but here
they are again in case anybody is interested.

Felipe Contreras (9):
  transport-helper: add 'force' to 'export' helpers
  transport-helper: check for 'forced update' message
  fast-export: improve argument parsing
  fast-export: add new --refspec option
  transport-helper: add support for old:new refspec
  fast-import: add support to delete refs
  fast-export: add support to delete refs
  transport-helper: add support to delete branches
  transport-helper: don't update refs in dry-run

 Documentation/git-fast-export.txt |  4 
 Documentation/git-fast-import.txt |  3 +++
 builtin/fast-export.c | 47 ++-
 fast-import.c | 13 ---
 t/t5801-remote-helpers.sh | 10 -
 t/t9300-fast-import.sh| 18 +++
 t/t9350-fast-export.sh| 18 +++
 transport-helper.c| 42 --
 8 files changed, 138 insertions(+), 17 deletions(-)

-- 
1.8.4-fc

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


[PATCH 1/9] transport-helper: add 'force' to 'export' helpers

2013-08-29 Thread Felipe Contreras
Otherwise they cannot know when to force the push or not (other than
hacks).

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 transport-helper.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/transport-helper.c b/transport-helper.c
index 63cabc3..62051a6 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -814,6 +814,9 @@ static int push_refs_with_export(struct transport 
*transport,
die(helper %s does not support dry-run, data-name);
}
 
+   if (flags  TRANSPORT_PUSH_FORCE)
+   set_helper_option(transport, force, true);
+
helper = get_helper(transport);
 
write_constant(helper-in, export\n);
-- 
1.8.4-fc

--
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/9] fast-export: add new --refspec option

2013-08-29 Thread Felipe Contreras
So that we can covert the exported ref names.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-fast-export.txt |  4 
 builtin/fast-export.c | 30 ++
 t/t9350-fast-export.sh|  7 +++
 3 files changed, 41 insertions(+)

diff --git a/Documentation/git-fast-export.txt 
b/Documentation/git-fast-export.txt
index 85f1f30..221506b 100644
--- a/Documentation/git-fast-export.txt
+++ b/Documentation/git-fast-export.txt
@@ -105,6 +105,10 @@ marks the same across runs.
in the commit (as opposed to just listing the files which are
different from the commit's first parent).
 
+--refspec::
+   Apply the specified refspec to each ref exported. Multiple of them can
+   be specified.
+
 [git-rev-list-args...]::
A list of arguments, acceptable to 'git rev-parse' and
'git rev-list', that specifies the specific objects and references
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 91114f4..7f314f0 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -17,6 +17,7 @@
 #include utf8.h
 #include parse-options.h
 #include quote.h
+#include remote.h
 
 static const char *fast_export_usage[] = {
N_(git fast-export [rev-list-opts]),
@@ -30,6 +31,8 @@ static int fake_missing_tagger;
 static int use_done_feature;
 static int no_data;
 static int full_tree;
+static struct refspec *refspecs;
+static int refspecs_nr;
 
 static int parse_opt_signed_tag_mode(const struct option *opt,
 const char *arg, int unset)
@@ -502,6 +505,15 @@ static void get_tags_and_duplicates(struct 
rev_cmdline_info *info,
if (dwim_ref(e-name, strlen(e-name), sha1, full_name) != 1)
continue;
 
+   if (refspecs) {
+   char *private;
+   private = apply_refspecs(refspecs, refspecs_nr, 
full_name);
+   if (private) {
+   free(full_name);
+   full_name = private;
+   }
+   }
+
switch (e-item-type) {
case OBJ_COMMIT:
commit = (struct commit *)e-item;
@@ -661,6 +673,7 @@ int cmd_fast_export(int argc, const char **argv, const char 
*prefix)
struct commit *commit;
char *export_filename = NULL, *import_filename = NULL;
uint32_t lastimportid;
+   struct string_list refspecs_list;
struct option options[] = {
OPT_INTEGER(0, progress, progress,
N_(show progress after n objects)),
@@ -681,6 +694,8 @@ int cmd_fast_export(int argc, const char **argv, const char 
*prefix)
OPT_BOOLEAN(0, use-done-feature, use_done_feature,
 N_(Use the done feature to terminate the 
stream)),
OPT_BOOL(0, no-data, no_data, N_(Skip output of blob 
data)),
+   OPT_STRING_LIST(0, refspec, refspecs_list, N_(refspec),
+N_(Apply refspec to exported refs)),
OPT_END()
};
 
@@ -700,6 +715,19 @@ int cmd_fast_export(int argc, const char **argv, const 
char *prefix)
if (argc  1)
usage_with_options (fast_export_usage, options);
 
+   if (refspecs_list.nr) {
+   const char *refspecs_str[refspecs_list.nr];
+   int i;
+
+   for (i = 0; i  refspecs_list.nr; i++)
+   refspecs_str[i] = refspecs_list.items[i].string;
+
+   refspecs_nr = refspecs_list.nr;
+   refspecs = parse_fetch_refspec(refspecs_nr, refspecs_str);
+
+   string_list_clear(refspecs_list, 1);
+   }
+
if (use_done_feature)
printf(feature done\n);
 
@@ -734,5 +762,7 @@ int cmd_fast_export(int argc, const char **argv, const char 
*prefix)
if (use_done_feature)
printf(done\n);
 
+   free_refspec(refspecs_nr, refspecs);
+
return 0;
 }
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index 34c2d8f..dcf 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -504,4 +504,11 @@ test_expect_success 'refs are updated even if no commits 
need to be exported' '
test_cmp expected actual
 '
 
+test_expect_success 'use refspec' '
+   git fast-export --refspec refs/heads/master:refs/heads/foobar master | \
+   grep ^commit  | sort | uniq  actual 
+   echo commit refs/heads/foobar  expected 
+   test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc

--
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 6/9] fast-import: add support to delete refs

2013-08-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-fast-import.txt |  3 +++
 fast-import.c | 13 ++---
 t/t9300-fast-import.sh| 18 ++
 3 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-fast-import.txt 
b/Documentation/git-fast-import.txt
index bf1a02a..fe5c952 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -483,6 +483,9 @@ Marks must be declared (via `mark`) before they can be used.
 * Any valid Git SHA-1 expression that resolves to a commit.  See
   ``SPECIFYING REVISIONS'' in linkgit:gitrevisions[7] for details.
 
+* The special null SHA-1 (40 zeros) specifices that the branch is to be
+  removed.
+
 The special case of restarting an incremental import from the
 current branch value should be written as:
 
diff --git a/fast-import.c b/fast-import.c
index 23f625f..b6be7a7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -248,6 +248,7 @@ struct branch {
uintmax_t last_commit;
uintmax_t num_notes;
unsigned active : 1;
+   unsigned delete : 1;
unsigned pack_id : PACK_ID_BITS;
unsigned char sha1[20];
 };
@@ -1674,10 +1675,13 @@ static int update_branch(struct branch *b)
struct ref_lock *lock;
unsigned char old_sha1[20];
 
-   if (is_null_sha1(b-sha1))
-   return 0;
if (read_ref(b-name, old_sha1))
hashclr(old_sha1);
+   if (is_null_sha1(b-sha1)) {
+   if (b-delete)
+   delete_ref(b-name, old_sha1, 0);
+   return 0;
+   }
lock = lock_any_ref_for_update(b-name, old_sha1, 0);
if (!lock)
return error(Unable to lock %s, b-name);
@@ -2604,8 +2608,11 @@ static int parse_from(struct branch *b)
free(buf);
} else
parse_from_existing(b);
-   } else if (!get_sha1(from, b-sha1))
+   } else if (!get_sha1(from, b-sha1)) {
parse_from_existing(b);
+   if (is_null_sha1(b-sha1))
+   b-delete = 1;
+   }
else
die(Invalid ref name or SHA1 expression: %s, from);
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index ac6f3b6..0150aa6 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2934,4 +2934,22 @@ test_expect_success 'S: ls with garbage after sha1 must 
fail' '
test_i18ngrep space after tree-ish err
 '
 
+test_expect_success 'T: delete branch' '
+   git branch to-delete 
+   git fast-import -EOF 
+   reset refs/heads/to-delete
+   from 
+   EOF
+   test_must_fail git rev-parse --verify refs/heads/to-delete
+'
+
+test_expect_success 'T: empty reset doesnt delete branch' '
+   git branch not-to-delete 
+   git fast-import -EOF 
+   reset refs/heads/not-to-delete
+   EOF
+   git show-ref 
+   git rev-parse --verify refs/heads/not-to-delete
+'
+
 test_done
-- 
1.8.4-fc

--
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 5/9] transport-helper: add support for old:new refspec

2013-08-29 Thread Felipe Contreras
By using fast-export's new --refspec option.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 t/t5801-remote-helpers.sh |  2 +-
 transport-helper.c| 13 ++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8c4c539..8e2dd9f 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -87,7 +87,7 @@ test_expect_success 'push new branch by name' '
compare_refs local HEAD server refs/heads/new-name
 '
 
-test_expect_failure 'push new branch with old:new refspec' '
+test_expect_success 'push new branch with old:new refspec' '
(cd local 
 git push origin new-name:new-refspec
) 
diff --git a/transport-helper.c b/transport-helper.c
index 95dd72e..c7135ef 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -809,7 +809,7 @@ static int push_refs_with_export(struct transport 
*transport,
struct ref *ref;
struct child_process *helper, exporter;
struct helper_data *data = transport-data;
-   struct string_list revlist_args = STRING_LIST_INIT_NODUP;
+   struct string_list revlist_args = STRING_LIST_INIT_DUP;
struct strbuf buf = STRBUF_INIT;
 
if (!data-refspecs)
@@ -848,8 +848,13 @@ static int push_refs_with_export(struct transport 
*transport,
die(remote-helpers do not support ref deletion);
 
if (ref-peer_ref) {
-   if (strcmp(ref-peer_ref-name, ref-name))
-   die(remote-helpers do not support old:new 
syntax);
+   if (strcmp(ref-name, ref-peer_ref-name)) {
+   struct strbuf buf = STRBUF_INIT;
+   strbuf_addf(buf, %s:%s, ref-peer_ref-name, 
ref-name);
+   string_list_append(revlist_args, --refspec);
+   string_list_append(revlist_args, buf.buf);
+   strbuf_release(buf);
+   }
string_list_append(revlist_args, ref-peer_ref-name);
}
}
@@ -857,6 +862,8 @@ static int push_refs_with_export(struct transport 
*transport,
if (get_exporter(transport, exporter, revlist_args))
die(Couldn't run fast-export);
 
+   string_list_clear(revlist_args, 1);
+
if (finish_command(exporter))
die(Error while running fast-export);
push_update_refs_status(data, remote_refs);
-- 
1.8.4-fc

--
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 8/9] transport-helper: add support to delete branches

2013-08-29 Thread Felipe Contreras
For remote-helpers that use 'export' to push.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 t/t5801-remote-helpers.sh |  8 
 transport-helper.c| 11 ++-
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index 8e2dd9f..a66a4e3 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -94,6 +94,14 @@ test_expect_success 'push new branch with old:new refspec' '
compare_refs local HEAD server refs/heads/new-refspec
 '
 
+test_expect_success 'push delete branch' '
+   (cd local 
+git push origin :new-name
+   ) 
+   test_must_fail git --git-dir=server/.git \
+rev-parse --verify refs/heads/new-name
+'
+
 test_expect_success 'cloning without refspec' '
GIT_REMOTE_TESTGIT_REFSPEC= \
git clone testgit::${PWD}/server local2 2error 
diff --git a/transport-helper.c b/transport-helper.c
index c7135ef..5490796 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -844,18 +844,19 @@ static int push_refs_with_export(struct transport 
*transport,
}
free(private);
 
-   if (ref-deletion)
-   die(remote-helpers do not support ref deletion);
-
if (ref-peer_ref) {
if (strcmp(ref-name, ref-peer_ref-name)) {
struct strbuf buf = STRBUF_INIT;
-   strbuf_addf(buf, %s:%s, ref-peer_ref-name, 
ref-name);
+   if (!ref-deletion)
+   strbuf_addf(buf, %s:%s, 
ref-peer_ref-name, ref-name);
+   else
+   strbuf_addf(buf, :%s, ref-name);
string_list_append(revlist_args, --refspec);
string_list_append(revlist_args, buf.buf);
strbuf_release(buf);
}
-   string_list_append(revlist_args, ref-peer_ref-name);
+   if (!ref-deletion)
+   string_list_append(revlist_args, 
ref-peer_ref-name);
}
}
 
-- 
1.8.4-fc

--
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 9/9] transport-helper: don't update refs in dry-run

2013-08-29 Thread Felipe Contreras
The remote helper namespace should not be updated.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 transport-helper.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index 5490796..9bbf209 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -715,7 +715,8 @@ static int push_update_ref_status(struct strbuf *buf,
 }
 
 static void push_update_refs_status(struct helper_data *data,
-   struct ref *remote_refs)
+   struct ref *remote_refs,
+   int flags)
 {
struct strbuf buf = STRBUF_INIT;
struct ref *ref = remote_refs;
@@ -729,7 +730,7 @@ static void push_update_refs_status(struct helper_data 
*data,
if (push_update_ref_status(buf, ref, remote_refs))
continue;
 
-   if (!data-refspecs)
+   if (flags  TRANSPORT_PUSH_DRY_RUN || !data-refspecs)
continue;
 
/* propagate back the update to the remote namespace */
@@ -799,7 +800,7 @@ static int push_refs_with_push(struct transport *transport,
sendline(data, buf);
strbuf_release(buf);
 
-   push_update_refs_status(data, remote_refs);
+   push_update_refs_status(data, remote_refs, flags);
return 0;
 }
 
@@ -867,7 +868,7 @@ static int push_refs_with_export(struct transport 
*transport,
 
if (finish_command(exporter))
die(Error while running fast-export);
-   push_update_refs_status(data, remote_refs);
+   push_update_refs_status(data, remote_refs, flags);
return 0;
 }
 
-- 
1.8.4-fc

--
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 7/9] fast-export: add support to delete refs

2013-08-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 builtin/fast-export.c  | 14 ++
 t/t9350-fast-export.sh | 11 +++
 2 files changed, 25 insertions(+)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 7f314f0..9b728ca 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -665,6 +665,19 @@ static void import_marks(char *input_file)
fclose(f);
 }
 
+static void handle_deletes(void)
+{
+   int i;
+   for (i = 0; i  refspecs_nr; i++) {
+   struct refspec *refspec = refspecs[i];
+   if (*refspec-src)
+   continue;
+
+   printf(reset %s\nfrom %s\n\n,
+   refspec-dst, sha1_to_hex(null_sha1));
+   }
+}
+
 int cmd_fast_export(int argc, const char **argv, const char *prefix)
 {
struct rev_info revs;
@@ -755,6 +768,7 @@ int cmd_fast_export(int argc, const char **argv, const char 
*prefix)
}
 
handle_tags_and_duplicates(extra_refs);
+   handle_deletes();
 
if (export_filename  lastimportid != last_idnum)
export_marks(export_filename);
diff --git a/t/t9350-fast-export.sh b/t/t9350-fast-export.sh
index dcf..ea6c96c 100755
--- a/t/t9350-fast-export.sh
+++ b/t/t9350-fast-export.sh
@@ -511,4 +511,15 @@ test_expect_success 'use refspec' '
test_cmp expected actual
 '
 
+test_expect_success 'delete refspec' '
+   git branch to-delete 
+   git fast-export --refspec :refs/heads/to-delete to-delete ^to-delete  
actual 
+   cat  expected -EOF 
+   reset refs/heads/to-delete
+   from 
+
+   EOF
+   test_cmp expected actual
+'
+
 test_done
-- 
1.8.4-fc

--
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 0/7] Multiple simultaneously locked ref updates

2013-08-29 Thread Martin Fick
On Thursday, August 29, 2013 08:11:48 am Brad King wrote:
 
fatal: Unable to create 'lock': File exists.
If no other git process is currently running, this
 probably means a git process crashed in this repository
 earlier. Make sure no other git process is running and
 remove the file manually to continue.

I don't believe git currently tries to do any form of stale 
lock recovery since it is racy and unreliable (both single 
server or on a multi-server shared repo),


-Martin
--
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: the pager

2013-08-29 Thread Dale R. Worley
So I set out to verify in the code that the order of priority of pager
specification is

GIT_PAGER  core.pager  PAGER  default

I discovered that there is also a pager.command configuration
variable.

I was expecting the code to be simple, uniform (with regard to the 5
sources), and reasonably well documented.  The relevant parts of the
code that I have located so far include:

in environment.c:

const char *pager_program;

in config.c:

int git_config_with_options(config_fn_t fn, void *data,
const char *filename,
const char *blob_ref,
int respect_includes)
{
char *repo_config = NULL;
int ret;
struct config_include_data inc = CONFIG_INCLUDE_INIT;

if (respect_includes) {
inc.fn = fn;
inc.data = data;
fn = git_config_include;
data = inc;
}

/*
 * If we have a specific filename, use it. Otherwise, follow the
 * regular lookup sequence.
 */
if (filename)
return git_config_from_file(fn, filename, data);
else if (blob_ref)
return git_config_from_blob_ref(fn, blob_ref, data);

repo_config = git_pathdup(config);
ret = git_config_early(fn, data, repo_config);
if (repo_config)
free(repo_config);
return ret;
}

in pager.c:

/* returns 0 for no pager, 1 for use pager, and -1 for not specified 
*/
int check_pager_config(const char *cmd)
{
struct pager_config c;
c.cmd = cmd;
c.want = -1;
c.value = NULL;
git_config(pager_command_config, c);
if (c.value)
pager_program = c.value;
return c.want;
}

const char *git_pager(int stdout_is_tty)
{
const char *pager;

if (!stdout_is_tty)
return NULL;

pager = getenv(GIT_PAGER);
if (!pager) {
if (!pager_program)
git_config(git_default_config, NULL);
pager = pager_program;
}
if (!pager)
pager = getenv(PAGER);
if (!pager)
pager = DEFAULT_PAGER;
else if (!*pager || !strcmp(pager, cat))
pager = NULL;

return pager;
}

What's with the code?  It's not simple, it's not uniform (e.g.,
setting env. var. PAGER to cat will cause git_pager() to return
NULL, but setting preprocessor var. DEFAULT_PAGER to cat will cause
it to return cat), and it's barely got any comments at all (a global
variable has *no description whatsoever*).

I'd like to clean up the manual pages at least, but it would take me
hours to figure out what the code *does*.

I know I'm griping here, but I thought that part of the reward for
contributing to an open-source project was as a showcase of one's
work.  Commenting your code is what you learn first in programming.

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


Re: [PATCHv2] git-diff: Clarify operation when not inside a repository.

2013-08-29 Thread Dale R. Worley
 From: Junio C Hamano gits...@pobox.com

 diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
 index b1630ba..33fbd8c 100644
 --- a/Documentation/git-diff.txt
 +++ b/Documentation/git-diff.txt
 @@ -28,11 +28,15 @@ two blob objects, or changes between two files on disk.
   words, the differences are what you _could_ tell Git to
   further add to the index but you still haven't.  You can
   stage these changes by using linkgit:git-add[1].
 -+
 -If exactly two paths are given and at least one points outside
 -the current repository, 'git diff' will compare the two files /
 -directories. This behavior can be forced by --no-index or by
 -executing 'git diff' outside of a working tree.
 +
 +'git diff' --no-index [--options] [--] [path...]::
 +
 + This form is to compare the given two paths on the
 + filesystem.  You can omit the `--no-index` option when
 + running the command in a working tree controlled by Git and
 + at least one of the paths points outside the working tree,
 + or when running the command outside a working tree
 + controlled by Git.

That does break out the --no-index case in a clearer way.

Dale
--
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 0/7] Multiple simultaneously locked ref updates

2013-08-29 Thread Brad King
On 08/29/2013 11:32 AM, Martin Fick wrote:
 On Thursday, August 29, 2013 08:11:48 am Brad King wrote:

fatal: Unable to create 'lock': File exists.
If no other git process is currently running, this
 probably means a git process crashed in this repository
 earlier. Make sure no other git process is running and
 remove the file manually to continue.
 
 I don't believe git currently tries to do any form of stale 
 lock recovery since it is racy and unreliable (both single 
 server or on a multi-server shared repo),

Nor should it in this case.  I was saying that the front-end
needs to reject duplicate ref names from the stdin lines before
trying to lock the ref twice to avoid this message.  I'm asking
for a suggestion for existing data structure capabilities in
Git's source to efficiently detect the duplicate name.

-Brad
--
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: the pager

2013-08-29 Thread Matthieu Moy
wor...@alum.mit.edu (Dale R. Worley) writes:

 const char *git_pager(int stdout_is_tty)
 {
 const char *pager;

 if (!stdout_is_tty)
 return NULL;

 pager = getenv(GIT_PAGER);
 if (!pager) {
 if (!pager_program)
 git_config(git_default_config, NULL);
 pager = pager_program;
 }
 if (!pager)
 pager = getenv(PAGER);
 if (!pager)
 pager = DEFAULT_PAGER;
 else if (!*pager || !strcmp(pager, cat))
 pager = NULL;

I guess the else could and should be dropped. If you do so (and
possibly insert a blank line between the DEFAULT_PAGER case and the
pager = NULL case), you get a nice pattern

if (!pager)
try_something();
if (!pager)
try_next_option();
...

 Commenting your code is what you learn first in programming.

Not commenting too much is the second thing you learn ;-).

I agree that a comment like this would help, though:

--- a/cache.h
+++ b/cache.h
@@ -1266,7 +1266,7 @@ static inline ssize_t write_str_in_full(int fd, const 
char *str)
 
 /* pager.c */
 extern void setup_pager(void);
-extern const char *pager_program;
+extern const char *pager_program; /* value read from git_config() */
 extern int pager_in_use(void);
 extern int pager_use_color;
 extern int term_columns(void);


-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/5] replace: forbid replacing an object with one of a different type

2013-08-29 Thread Junio C Hamano
Christian Couder chrisc...@tuxfamily.org writes:

 But if all the objects that point to an object, called O, are to be
 replaced, then in most cases object O probably doesn't need to be
 replaced. It's probably sufficient to create the new object, called
 O2, that would replace object O and to replace all the objects
 pointing to object O with objects pointing to O2.

H.

What the above says, with probably and most cases, can easily
inferred by anybody remotely intelligent, and the only reason to
have something like the above paragraph would be if it assures that
the statement holds without these qualifications to weaken it, which
it doesn't.  I am not sure this paragraph adds much value.

 The only case where someone might really want to replace object 0,
 with an object O2 of a different type, and all the objects pointing to
 it, is if it's really important, perhaps for external reasons, to have
 object O's SHA1 point to O2.

The same comment applies here.

 And anyway, if one really wants to do that, it can still be done using
 git update-ref.

And I really do not think this sentence is right---you can justify
with the same sentence to remove git replace wrapper.

The earlier suggestion to bypass the new hand-holding with --force
is more sensible, I 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: [RFC/PATCH v3 0/4] Disable git status comment prefix

2013-08-29 Thread Junio C Hamano
Matthieu Moy matthieu@imag.fr writes:

 ... Right now, my preference would be to call it
 status.oldStyle and default it to false (i.e. change the behavior, but
 allow old-timers to get back the old one).

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


Re: [PATCH/RFC 0/7] Multiple simultaneously locked ref updates

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 Nor should it in this case.  I was saying that the front-end
 needs to reject duplicate ref names from the stdin lines before
 trying to lock the ref twice to avoid this message.

How about trying not to feed duplicates?
--
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: The gitweb author initials feature from a36817b doesn't work with i18n names

2013-08-29 Thread Simon Ruderich
On Thu, Aug 29, 2013 at 04:26:29PM +0200, Ævar Arnfjörð Bjarmason wrote:
 I haven't gotten an env where I can test gitweb running, but that
 looks like it should work to me.

I've tested the patch and it works fine.

Tested-by: Simon Ruderich si...@ruderich.org

Regards
Simon
-- 
+ privacy is necessary
+ using gnupg http://gnupg.org
+ public key id: 0x92FEFDB7E44C32F9
--
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] git-init does not respect existing separate-git-dir

2013-08-29 Thread Junio C Hamano
Duy Nguyen pclo...@gmail.com writes:

 On Thu, Aug 29, 2013 at 01:39:02PM +0100, Ximin Luo wrote:
 It should not be necessary to re-specify --separate-git-dir when 
 re-initialising a git repo.
 
 $ git init --separate-git-dir ../repo
 Initialized empty Git repository in /home/infinity0/tmp/repo/
 
 $ git init
 /home/infinity0/tmp/wtree/.git/refs: Not a directory
 1

 This patch seems to work. Lightly tested.

 -- 8 --
 diff --git a/builtin/init-db.c b/builtin/init-db.c
 index 78aa387..d0e5b2e 100644
 --- a/builtin/init-db.c
 +++ b/builtin/init-db.c
 @@ -192,6 +192,15 @@ static int create_default_files(const char 
 *template_path)
   die(_(insane git directory %s), git_dir);
   memcpy(path, git_dir, len);
  
 + if (!lstat(path, st1)  S_ISREG(st1.st_mode)) {
 + git_dir = read_gitfile(git_dir);
 + len = strlen(git_dir);
 + if (len  sizeof(path)-50)
 + die(_(insane git directory %s), git_dir);
 + set_git_dir(git_dir);

This repetition from the pre-context of the patch makes me wonder if
it may be a better solution to make sure we have already resolved
the gitfile way before we get here, so that get_git_dir() gives the
real location.

The codepaths in init and clone are both special in that they may be
dealing with a new repository and because of that, they may need to
call set_git_dir() themselves, but in the codeflow for normal (read:
those who deal with an existing repositories) programs, discovery of
the real .git directory is done by environment.c::get_git_dir(),
which to calls setup_git_env(), which in turn read_gitfile()'s on
.git when using the default .git, like so:

static void setup_git_env(void)
{
git_dir = getenv(GIT_DIR_ENVIRONMENT);
git_dir = git_dir ? xstrdup(git_dir) : NULL;
if (!git_dir) {
git_dir = read_gitfile(DEFAULT_GIT_DIR_ENVIRONMENT);
git_dir = git_dir ? xstrdup(git_dir) : NULL;
}
if (!git_dir)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;

And after this sequence, git_object_dir and git_index_file are
computed off of git_dir, unless they are specified to use an
alternate location via environment variables.

I wonder if the last use DEFAULT_GIT_DIR_ENVIRONMENT (which is
.git) should also do the read_gitfile() thing, perhaps like this
(totally untested):

 environment.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/environment.c b/environment.c
index 5398c36..944e10e 100644
--- a/environment.c
+++ b/environment.c
@@ -123,14 +123,16 @@ static char *expand_namespace(const char *raw_namespace)
 
 static void setup_git_env(void)
 {
+   const char *gitfile;
+
git_dir = getenv(GIT_DIR_ENVIRONMENT);
-   git_dir = git_dir ? xstrdup(git_dir) : NULL;
-   if (!git_dir) {
-   git_dir = read_gitfile(DEFAULT_GIT_DIR_ENVIRONMENT);
-   git_dir = git_dir ? xstrdup(git_dir) : NULL;
-   }
if (!git_dir)
git_dir = DEFAULT_GIT_DIR_ENVIRONMENT;
+   gitfile = read_gitfile(git_dir);
+   if (!gitfile)
+   git_dir = xstrdup(git_dir);
+   else
+   git_dir = gitfile;
git_object_dir = getenv(DB_ENVIRONMENT);
if (!git_object_dir) {
git_object_dir = xmalloc(strlen(git_dir) + 9);
--
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 0/7] Multiple simultaneously locked ref updates

2013-08-29 Thread Brad King
On 08/29/2013 12:21 PM, Junio C Hamano wrote:
 Brad King brad.k...@kitware.com writes:
 needs to reject duplicate ref names from the stdin lines before
 trying to lock the ref twice to avoid this message.
 
 How about trying not to feed duplicates?

Sure, perhaps it is simplest to push the responsibility on the user
to avoid duplicates.  However, the error message will need to be
re-worded to distinguish this case from a stale lock or competing
process since both locks may come from the same update-ref process.

Without checking the input for duplicates ourselves we cannot
distinguish these cases to provide a more informative error message.
However, such a check would add runtime overhead even for valid input.
If we prefer to avoid input validation then here is proposed new
wording for the lock failure message:


fatal: Unable to create 'lock': File exists.

The lock file may exist because:
- another running git process already has the lock, or
- this process already has the lock because it was asked to
  update the same file multiple times simultaneously, or
- a stale lock is left from a git process that crashed earlier.
In the last case, make sure no other git process is running and
remove the file manually to continue.


IIUC the message cannot say anything about a 'ref' because it is
used for other file type lock failures too.

Comments?
-Brad
--
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 1/7] reset: rename update_refs to reset_refs

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 Get it out of the way for a future refs.h function.

Readers do not know if update_refs() is a good name for that
future refs.h function at this point, so evict squatter is not a
very good justification by itself.  I do agree that this static
function is resetting a ref, not doing an arbitrary update, and the
new name is a better match than the old one for what it does, though.

 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  builtin/reset.c |4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 diff --git a/builtin/reset.c b/builtin/reset.c
 index afa6e02..789ee48 100644
 --- a/builtin/reset.c
 +++ b/builtin/reset.c
 @@ -219,7 +219,7 @@ static const char **parse_args(const char **argv, const 
 char *prefix, const char
   return argv[0] ? get_pathspec(prefix, argv) : NULL;
  }
  
 -static int update_refs(const char *rev, const unsigned char *sha1)
 +static int reset_refs(const char *rev, const unsigned char *sha1)
  {
   int update_ref_status;
   struct strbuf msg = STRBUF_INIT;
 @@ -350,7 +350,7 @@ int cmd_reset(int argc, const char **argv, const char 
 *prefix)
   if (!pathspec  !unborn) {
   /* Any resets without paths update HEAD to the head being
* switched to, saving the previous head in ORIG_HEAD before. */
 - update_ref_status = update_refs(rev, sha1);
 + update_ref_status = reset_refs(rev, sha1);
  
   if (reset_type == HARD  !update_ref_status  !quiet)
   print_new_head_line(lookup_commit_reference(sha1));
--
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 2/7] refs: report ref type from lock_any_ref_for_update

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 Expose lock_ref_sha1_basic's type_p argument to callers of
 lock_any_ref_for_update.  Update all call sites to ignore it; we will
 use it later.
 ...
 diff --git a/branch.c b/branch.c
 index c5c6984..c244483 100644
 --- a/branch.c
 +++ b/branch.c
 @@ -291,7 +291,7 @@ void create_branch(const char *head,
   hashcpy(sha1, commit-object.sha1);
  
   if (!dont_change_ref) {
 - lock = lock_any_ref_for_update(ref.buf, NULL, 0);
 + lock = lock_any_ref_for_update(ref.buf, NULL, 0, 0);

If you are passing an NULL as a new parameter, please spell it
NULL, not 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/RFC 4/7] refs: factor delete_ref loose ref step into a helper

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 Factor loose ref deletion into helper function delete_ref_loose to allow
 later use elsewhere.  While at it, rename local names 'flag = type' and
 'delopt = flags' for consistency with callers and called functions.

 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  refs.c |   24 
  1 file changed, 16 insertions(+), 8 deletions(-)

 diff --git a/refs.c b/refs.c
 index 2e755b4..5908648 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -2450,15 +2450,10 @@ static int repack_without_ref(const char *refname)
   return commit_packed_refs();
  }
  
 -int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 +static int delete_ref_loose(struct ref_lock *lock, int type)
  {
 - struct ref_lock *lock;
 - int err, i = 0, ret = 0, flag = 0;
 -
 - lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
 - if (!lock)
 - return 1;
 - if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
 + int err, i, ret = 0;
 + if (!(type  REF_ISPACKED) || type  REF_ISSYMREF) {

Hits from git grep REF_IS tell me that all users of REF_IS* symbol
that check if a bit is on in a word does so against flag (either a
variable called flag, flags, or a structure member .flag).

This change is making things less consistent, not more, I am afraid.

   /* loose */
   i = strlen(lock-lk-filename) - 5; /* .lock */
   lock-lk-filename[i] = 0;
 @@ -2468,6 +2463,19 @@ int delete_ref(const char *refname, const unsigned 
 char *sha1, int delopt)
  
   lock-lk-filename[i] = '.';
   }
 + return ret;
 +}
 +
 +int delete_ref(const char *refname, const unsigned char *sha1, int flags)
 +{
 + struct ref_lock *lock;
 + int ret = 0, type = 0;
 +
 + lock = lock_ref_sha1_basic(refname, sha1, flags, type);
 + if (!lock)
 + return 1;
 + ret |= delete_ref_loose(lock, type);
 +
   /* removing the loose one could have resurrected an earlier
* packed one.  Also, if it was not loose we need to repack
* without it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 5/7] refs: add function to repack without multiple refs

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 Generalize repack_without_ref as repack_without_refs to support a list
 of refs and implement the former in terms of the latter.

 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  refs.c |   29 ++---
  1 file changed, 22 insertions(+), 7 deletions(-)

 diff --git a/refs.c b/refs.c
 index 5908648..5a6c14e 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -2414,25 +2414,35 @@ static int curate_packed_ref_fn(struct ref_entry 
 *entry, void *cb_data)
   return 0;
  }
  
 -static int repack_without_ref(const char *refname)
 +static int repack_without_refs(const char **refnames, int n)
  {
   struct ref_dir *packed;
   struct string_list refs_to_delete = STRING_LIST_INIT_DUP;
   struct string_list_item *ref_to_delete;
 + int i, removed = 0;
 +
 + /* Look for a packed ref: */
 + for (i = 0; i  n; ++i)
 + if (get_packed_ref(refnames[i]))
 + break;
  
 - if (!get_packed_ref(refname))
 - return 0; /* refname does not exist in packed refs */
 + /* Avoid locking if we have nothing to do: */
 + if(i == n)

Style:
if (i == n)

 + return 0; /* no refname exists in packed refs */
  
   if (lock_packed_refs(0)) {
   unable_to_lock_error(git_path(packed-refs), errno);
 - return error(cannot delete '%s' from packed refs, refname);
 + return error(cannot delete '%s' from packed refs, 
 refnames[i]);
   }
   packed = get_packed_refs(ref_cache);
  
 - /* Remove refname from the cache: */
 - if (remove_entry(packed, refname) == -1) {
 + /* Remove refnames from the cache: */
 + for (i = 0; i  n; ++i)
 + if (remove_entry(packed, refnames[i]) != -1)
 + removed = 1;
 + if (!removed) {
   /*
 -  * The packed entry disappeared while we were
 +  * All packed entries disappeared while we were
* acquiring the lock.
*/
   rollback_packed_refs();

... and this is not an error; somebody else did the work we wanted
to do for us, which is good ;-)

 @@ -2450,6 +2460,11 @@ static int repack_without_ref(const char *refname)
   return commit_packed_refs();
  }
  
 +static int repack_without_ref(const char *refname)
 +{
 + return repack_without_refs(refname, 1);
 +}
 +
  static int delete_ref_loose(struct ref_lock *lock, int type)
  {
   int err, i, ret = 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/RFC 6/7] refs: add update_refs for multiple simultaneous updates

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 Add 'struct ref_update' to encode the information needed to update or
 delete a ref (name, new sha1, optional old sha1, no-deref flag).  Add
 function 'update_refs' accepting an array of updates to perform.  First
 acquire locks on all refs with verified old values.  Then update or
 delete all refs accordingly.  Fail if any one lock cannot be obtained or
 any one old value does not match.

 Though the refs themeselves cannot be modified together in a single
 atomic transaction, this function does enable some useful semantics.
 For example, a caller may create a new branch starting from the head of
 another branch and rewind the original branch at the same time.  This
 transfers ownership of commits between branches without risk of losing
 commits added to the original branch by a concurrent process, or risk of
 a concurrent process creating the new branch first.

 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  refs.c |   66 
 
  refs.h |   11 +++
  2 files changed, 77 insertions(+)

 diff --git a/refs.c b/refs.c
 index 5a6c14e..0a0c19e 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -3238,6 +3238,72 @@ int update_ref(const char *action, const char *refname,
   return update_ref_write(action, refname, sha1, lock, onerr);
  }
  
 +int update_refs(const char *action, struct ref_update *updates,
 + int n, enum action_on_err onerr)
 +{
 + int ret = 0, delnum = 0, i;
 + int *types;
 + struct ref_lock **locks;
 + const char **delnames;
 +
 + if (!updates || !n)
 + return 0;
 +
 + /* Allocate work space: */
 + types = xmalloc(sizeof(int)*n);
 + locks = xmalloc(sizeof(struct ref_lock*)*n);
 + delnames = xmalloc(sizeof(const char*)*n);
 +
 + /* Acquire all locks while verifying old values: */
 + for (i=0; i  n; ++i) {

Style:

for (i = 0; i  n; i++) {

 + locks[i] = update_ref_lock(updates[i].ref_name,
 +updates[i].old_sha1,
 +updates[i].flags,
 +types[i], onerr);
 + if (!locks[i])
 + break;
 + }

Is it asking for AB-BA deadlock?  If so, is the caller responsible
for avoiding it?

 + /* Abort if we did not get all locks: */
 + if (i  n) {
 + while (--i = 0)
 + unlock_ref(locks[i]);
 + free(types);
 + free(locks);
 + free(delnames);
 + return 1;
 + }
 +
 + /* Perform updates first so live commits remain referenced: */
 + for (i=0; i  n; ++i)
 + if (!is_null_sha1(updates[i].new_sha1)) {
 + ret |= update_ref_write(action,
 + updates[i].ref_name,
 + updates[i].new_sha1,
 + locks[i], onerr);
 + locks[i] = 0; /* freed by update_ref_write */
 + }
 +
 + /* Perform deletes now that updates are safely completed: */
 + for (i=0; i  n; ++i)
 + if (locks[i]) {
 + delnames[delnum++] = locks[i]-ref_name;
 + ret |= delete_ref_loose(locks[i], types[i]);
 + }
 + ret |= repack_without_refs(delnames, delnum);
 + for (i=0; i  delnum; ++i)
 + unlink_or_warn(git_path(logs/%s, delnames[i]));
 + clear_loose_ref_cache(ref_cache);
 + for (i=0; i  n; ++i)
 + if (locks[i])
 + unlock_ref(locks[i]);
 +
 + free(types);
 + free(locks);
 + free(delnames);
 + return ret;
 +}
 +
  struct ref *find_ref_by_name(const struct ref *list, const char *name)
  {
   for ( ; list; list = list-next)
 diff --git a/refs.h b/refs.h
 index 2cd307a..5763f3a 100644
 --- a/refs.h
 +++ b/refs.h
 @@ -214,6 +214,17 @@ int update_ref(const char *action, const char *refname,
   const unsigned char *sha1, const unsigned char *oldval,
   int flags, enum action_on_err onerr);
  
 +struct ref_update {
 + const char *ref_name;
 + unsigned char new_sha1[20];
 + unsigned char *old_sha1;

This asymmetry is rather curious and will later become problematic
when we have more users of this API.  Maybe your callers happen have
static storage to keep old_sha1[] outside this structure but do not
have it for new_sha1[] for some unknown reason, which may not apply
to later callers we may want to add.

 + int flags;
 +};
 +
 +/** lock all refs and then write all of them */
 +int update_refs(const char *action, struct ref_update *updates,
 + int n, enum action_on_err onerr);
 +
  extern int parse_hide_refs_config(const char *var, const char *value, const 
 char *);
  extern int ref_is_hidden(const char *);
--
To unsubscribe from this list: send the 

Re: [PATCH] git-svn: Configure a prompt callback for gnome_keyring.

2013-08-29 Thread Eric Wong
Matthijs Kooijman matth...@stdin.nl wrote:
 Hi folks,
 
 any chance this patch can be merged?

It's probably fine.  Does anybody else have testing/feedback?  I haven't
used git-svn/SVN in years, and I don't use GNOME (nor much GUI).
--
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: Configure a prompt callback for gnome_keyring.

2013-08-29 Thread Eric Wong
Edward Rudd ur...@outoforder.cc wrote:
 Where is a link to the latest patch? I can give it a quick once-over
 with one of my git-svn'ed game ports I'm working on. (due for
 launching on the 10th.. WOOHOO!)

http://mid.gmane.org/1371573490-21973-1-git-send-email-matth...@stdin.nl
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


Officially start moving to the term 'staging area'

2013-08-29 Thread Felipe Contreras
Hi,

It has been discussed many times in the past that 'index' is not an
appropriate description for what the high-level user does with it, and
it has been agreed that 'staging area' is the best term.

The term 'staging area' is more intuitive for newcomers which are more
familiar with English than with Git, and it seems to be a
straightforward mental notion for people with different mother tongues.

In fact it is so intuitive that it's used already in a lot online
documentation, and the people that do teach Git professionally use this
term, because it's easier for many kinds of audiences to grasp.

The meaning of the words 'cache' and 'index' doesn't represent correctly
the mental model of the high-level user:

cache: a 'cache' is a place for easier access; a squirrel caches nuts
so it doesn't have to go looking for them in the future when it might
be much more difficult. Git porcelain is not using the staging area
for easier future access; it's not a cache.

index: an 'index' is a guide of pointers to something else; a book
index has a list of entries so the reader can locate information
easily without having to go through the whole book. Git porcelain is
not using the staging area to find out entries quicker; it's not an
index.

stage: a 'stage' is a special area designated for convenience in order
for some activity to take place; an orator would prepare a stage in
order for her speak to be successful, otherwise many people might not
be able to hear, or see her. Git porcelain is using the staging area
precisely as a special area to be separated from the working directory
for convenience.

The term 'stage' is a good noun itself, but also 'staging area', it
has a good verb; 'to stage', and a nice past-participle; 'staged'.

The first step in moving Git towards this term, is first to add --stage
options for every command that uses --index or --cache. However, there's
a problem with the 'git apply' command, because it treats --index and
--cache differently. Different solutions were proposed, including a
special --stage-only option, however, I think the best solution is a
--[no-]work option to specify if the working directory should be touched
or not, so --index becomes --staged, and --cached becomes --staged
--no-work.

In addition, the 'git stage' command can be extended so the staging area
can be brought closer to the user, like other important Git concepts,
like 'git branch, 'git tag', and 'git remote'. For example, the command
'git stage edit' (which allows the user to edit directly the diff from
HEAD to the staging area) can have a home, where previously there was no
place. It would become natural then to do 'git stage diff', and then
'git stage edit' (to edit the previous diff).

After adding the new --stage options and making sure no functionality is
lost, they can become the recommended ones in the documentation,
eventually, the old ones get deprecated, and eventually obsoleted.

Also, the documentation would need to be updated to replace many
instances of 'the index', with 'the staging area' in porcelain commands.

Moreover, the --stage and --work options also make sense for 'git
reset', and after these options are added, the complicated table to
explain the different behaviors between --soft, --mixed, and --hard
becomes so simple it's not needed any more:

  working stage HEAD target working stage HEAD
  
   A   B CD --no-stage  A   B D
--stage A   D D
--work  D   D D

  working stage HEAD target working stage HEAD
  
   A   B CC --no-stage  A   B C
--stage A   C C
--work  C   C C

  working stage HEAD target working stage HEAD
  
   B   B CD --no-stage  B   B D
--stage B   D D
--work  D   D D

  working stage HEAD target working stage HEAD
  
   B   B CC --no-stage  B   B C
--stage B   C C
--work  C   C C

  working stage HEAD target working stage HEAD
  
   B   C CD --no-stage  B   C D
--stage B   D D
--work  D   D D

  working stage HEAD target working stage HEAD
  
   B   C CC 

Re: [PATCH/RFC 1/7] reset: rename update_refs to reset_refs

2013-08-29 Thread Brad King
On 08/29/2013 01:17 PM, Junio C Hamano wrote:
 Brad King brad.k...@kitware.com writes:
 
 Get it out of the way for a future refs.h function.
 
 Readers do not know if update_refs() is a good name for that
 future refs.h function at this point, so evict squatter is not a
 very good justification by itself.  I do agree that this static
 function is resetting a ref, not doing an arbitrary update, and the
 new name is a better match than the old one for what it does, though.

Okay, I've revised the commit message for the next revision.

-Brad
--
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 2/7] refs: report ref type from lock_any_ref_for_update

2013-08-29 Thread Brad King
On 08/29/2013 01:22 PM, Junio C Hamano wrote:
 If you are passing an NULL as a new parameter, please spell it
 NULL, not 0.

Fixed at all updated call sites.

-Brad
--
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 4/7] refs: factor delete_ref loose ref step into a helper

2013-08-29 Thread Brad King
On 08/29/2013 01:28 PM, Junio C Hamano wrote:
 Brad King brad.k...@kitware.com writes:
 -if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
 +if (!(type  REF_ISPACKED) || type  REF_ISSYMREF) {
 
 Hits from git grep REF_IS tell me that all users of REF_IS* symbol
 that check if a bit is on in a word does so against flag (either a
 variable called flag, flags, or a structure member .flag).
 
 This change is making things less consistent, not more, I am afraid.

Okay, I removed this part of the change.  It makes the commit
simpler anyway.

-Brad
--
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 5/7] refs: add function to repack without multiple refs

2013-08-29 Thread Brad King
On 08/29/2013 01:34 PM, Junio C Hamano wrote:
 Brad King brad.k...@kitware.com writes:
 +if(i == n)
 
 Style:
   if (i == n)

Fixed in next revision.

-Brad
--
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 01/11] Call it Git User Manual and remove reference to very old Git version

2013-08-29 Thread Junio C Hamano
Thomas Ackermann th.ac...@arcor.de writes:

 Reviewed-by: Jonathan Nieder jrnie...@gmail.com
 Signed-off-by: Thomas Ackermann th.ac...@arcor.de
 ---
  Documentation/user-manual.txt | 5 ++---
  1 file changed, 2 insertions(+), 3 deletions(-)

 diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
 index fe723e4..103ec9a 100644
 --- a/Documentation/user-manual.txt
 +++ b/Documentation/user-manual.txt
 @@ -1,6 +1,5 @@
 -Git User's Manual (for version 1.5.3 or newer)
 -__
 -
 +Git User Manual
 +___

The removal of the parenthesized part is good, but it should be
User's Manual, I 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: [PATCH 02/11] Use current detached HEAD message

2013-08-29 Thread Junio C Hamano
Looks good; thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] stage: add edit command

2013-08-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-stage.txt|  5 +++
 builtin/stage.c| 74 ++
 contrib/completion/git-completion.bash |  4 +-
 3 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-stage.txt b/Documentation/git-stage.txt
index 318bf45..3e52a66 100644
--- a/Documentation/git-stage.txt
+++ b/Documentation/git-stage.txt
@@ -15,6 +15,7 @@ SYNOPSIS
 'git stage diff' [options] [commit] [--] [paths...]
 'git stage rm' [options] [--] [paths...]
 'git stage apply' [options] [--] [paths...]
+'git stage edit'
 
 DESCRIPTION
 ---
@@ -45,6 +46,10 @@ Remove files from the staging area only. See 
linkgit:git-rm[1] --staged.
 
 Apply a patch to the staging area. See linkgit:git-rm[1] --staged.
 
+'edit'::
+
+Manually edit the staging area (as a diff).
+
 SEE ALSO
 
 linkgit:git-add[1]
diff --git a/builtin/stage.c b/builtin/stage.c
index 3023d17..d3c58d5 100644
--- a/builtin/stage.c
+++ b/builtin/stage.c
@@ -6,6 +6,9 @@
 
 #include builtin.h
 #include parse-options.h
+#include diff.h
+#include diffcore.h
+#include revision.h
 
 static const char *const stage_usage[] = {
N_(git stage [options] [--] paths...),
@@ -16,6 +19,74 @@ static const char *const stage_usage[] = {
NULL
 };
 
+static int do_reset(const char *prefix)
+{
+   const char *argv[] = { reset, --quiet, NULL };
+   return cmd_reset(2, argv, prefix);
+}
+
+static int do_apply(const char *file, const char *prefix)
+{
+   const char *argv[] = { apply, --recount, --cached, file, NULL };
+   return cmd_apply(4, argv, prefix);
+}
+
+static int edit(int argc, const char **argv, const char *prefix)
+{
+   char *file = git_pathdup(STAGE_EDIT.patch);
+   int out;
+   struct rev_info rev;
+   int ret = 0;
+   struct stat st;
+
+   read_cache();
+
+   init_revisions(rev, prefix);
+   rev.diffopt.context = 7;
+
+   argc = setup_revisions(argc, argv, rev, NULL);
+   add_head_to_pending(rev);
+   if (!rev.pending.nr) {
+   struct tree *tree;
+   tree = lookup_tree(EMPTY_TREE_SHA1_BIN);
+   add_pending_object(rev, tree-object, HEAD);
+   }
+
+   rev.diffopt.output_format = DIFF_FORMAT_PATCH;
+   rev.diffopt.use_color = 0;
+   DIFF_OPT_SET(rev.diffopt, IGNORE_DIRTY_SUBMODULES);
+
+   out = open(file, O_CREAT | O_WRONLY, 0666);
+   if (out  0)
+   die(_(Could not open '%s' for writing.), file);
+   rev.diffopt.file = xfdopen(out, w);
+   rev.diffopt.close_file = 1;
+
+   if (run_diff_index(rev, 1))
+   die(_(Could not write patch));
+   if (launch_editor(file, NULL, NULL))
+   exit(1);
+
+   if (stat(file, st))
+   die_errno(_(Could not stat '%s'), file);
+
+   ret = do_reset(prefix);
+   if (ret)
+   goto leave;
+
+   if (!st.st_size)
+   goto leave;
+
+   ret = do_apply(file, prefix);
+   if (ret)
+   goto leave;
+
+leave:
+   unlink(file);
+   free(file);
+   return ret;
+}
+
 int cmd_stage(int argc, const char **argv, const char *prefix)
 {
struct option options[] = { OPT_END() };
@@ -46,6 +117,9 @@ int cmd_stage(int argc, const char **argv, const char 
*prefix)
 
return cmd_apply(argc, argv, prefix);
}
+   if (!strcmp(argv[1], edit)) {
+   return edit(argc - 1, argv + 1, prefix);
+   }
}
 
return cmd_add(argc, argv, prefix);
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 8cf26e2..2b81e78 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1693,7 +1693,7 @@ _git_stage ()
 {
__git_has_doubledash  return
 
-   local subcommands=add reset diff rm apply
+   local subcommands=add reset diff rm apply edit
local subcommand=$(__git_find_on_cmdline $subcommands)
if [ -z $subcommand ]; then
__gitcomp $subcommands
@@ -1711,6 +1711,8 @@ _git_stage ()
_git_rm;;
apply)
_git_apply;;
+   edit)
+   ;;
*)
_git_add;
esac
-- 
1.8.4-fc

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


[PATCH 1/2] Add proper 'stage' command

2013-08-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-stage.txt| 45 +
 Makefile   |  2 +-
 builtin.h  |  1 +
 builtin/stage.c| 52 ++
 contrib/completion/git-completion.bash | 24 +++-
 git.c  |  2 +-
 6 files changed, 118 insertions(+), 8 deletions(-)
 create mode 100644 builtin/stage.c

diff --git a/Documentation/git-stage.txt b/Documentation/git-stage.txt
index ba3fe0d..318bf45 100644
--- a/Documentation/git-stage.txt
+++ b/Documentation/git-stage.txt
@@ -3,20 +3,55 @@ git-stage(1)
 
 NAME
 
-git-stage - Add file contents to the staging area
+git-stage - manage the staging area
 
 
 SYNOPSIS
 
 [verse]
-'git stage' args...
-
+'git stage' [options] [--] [paths...]
+'git stage add' [options] [--] [paths...]
+'git stage reset' [-q|--patch] [--] [paths...]
+'git stage diff' [options] [commit] [--] [paths...]
+'git stage rm' [options] [--] [paths...]
+'git stage apply' [options] [--] [paths...]
 
 DESCRIPTION
 ---
 
-This is a synonym for linkgit:git-add[1].  Please refer to the
-documentation of that command.
+
+COMMANDS
+
+
+With no arguments, it's a synonym for linkgit:git-add[1].
+
+'add'::
+
+Adds file contents to the staging area. See linkgit:git-add[1].
+
+'reset'::
+
+Resets the staging area. See linkgit:git-reset[1].
+
+'diff'::
+
+View the changes you staged for the next commit. See linkgit:git-diff[1] 
--staged.
+
+'rm'::
+
+Remove files from the staging area only. See linkgit:git-rm[1] --staged.
+
+'apply'::
+
+Apply a patch to the staging area. See linkgit:git-rm[1] --staged.
+
+SEE ALSO
+
+linkgit:git-add[1]
+linkgit:git-reset[1]
+linkgit:git-diff[1]
+linkgit:git-rm[1]
+linkgit:git-apply[1]
 
 GIT
 ---
diff --git a/Makefile b/Makefile
index 3588ca1..1f7ddf3 100644
--- a/Makefile
+++ b/Makefile
@@ -598,7 +598,6 @@ BUILT_INS += git-merge-subtree$X
 BUILT_INS += git-peek-remote$X
 BUILT_INS += git-repo-config$X
 BUILT_INS += git-show$X
-BUILT_INS += git-stage$X
 BUILT_INS += git-status$X
 BUILT_INS += git-whatchanged$X
 
@@ -982,6 +981,7 @@ BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-ref.o
+BUILTIN_OBJS += builtin/stage.o
 BUILTIN_OBJS += builtin/stripspace.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
 BUILTIN_OBJS += builtin/tag.o
diff --git a/builtin.h b/builtin.h
index 8afa2de..baf3a0f 100644
--- a/builtin.h
+++ b/builtin.h
@@ -113,6 +113,7 @@ extern int cmd_send_pack(int argc, const char **argv, const 
char *prefix);
 extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
+extern int cmd_stage(int argc, const char **argv, const char *prefix);
 extern int cmd_status(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/builtin/stage.c b/builtin/stage.c
new file mode 100644
index 000..3023d17
--- /dev/null
+++ b/builtin/stage.c
@@ -0,0 +1,52 @@
+/*
+ * 'git stage' builtin command
+ *
+ * Copyright (C) 2013 Felipe Contreras
+ */
+
+#include builtin.h
+#include parse-options.h
+
+static const char *const stage_usage[] = {
+   N_(git stage [options] [--] paths...),
+   N_(git stage add [options] [--] paths...),
+   N_(git stage reset [-q|--patch] [--] paths...),
+   N_(git stage diff [options] [commit] [--] paths...),
+   N_(git stage rm [options] [--] paths...),
+   NULL
+};
+
+int cmd_stage(int argc, const char **argv, const char *prefix)
+{
+   struct option options[] = { OPT_END() };
+
+   argc = parse_options(argc, argv, prefix, options, stage_usage,
+   PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN | 
PARSE_OPT_KEEP_DASHDASH);
+
+   if (argc  1) {
+   if (!strcmp(argv[1], add))
+   return cmd_add(argc - 1, argv + 1, prefix);
+   if (!strcmp(argv[1], reset))
+   return cmd_reset(argc - 1, argv + 1, prefix);
+   if (!strcmp(argv[1], diff)) {
+   argv[0] = diff;
+   argv[1] = --staged;
+
+   return cmd_diff(argc, argv, prefix);
+   }
+   if (!strcmp(argv[1], rm)) {
+   argv[0] = rm;
+   argv[1] = --cached;
+
+   return cmd_rm(argc, argv, prefix);
+   }
+   if (!strcmp(argv[1], apply)) {
+   argv[0] = apply;
+   argv[1] = --cached;
+
+   return cmd_apply(argc, argv, 

Re: [PATCH 03/11] Use current output for git repack

2013-08-29 Thread Junio C Hamano
Thomas Ackermann th.ac...@arcor.de writes:

 Signed-off-by: Thomas Ackermann th.ac...@arcor.de
 ---
  Documentation/user-manual.txt | 16 +++-
  1 file changed, 7 insertions(+), 9 deletions(-)

 diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
 index bdefd9a..3f44ca0 100644
 --- a/Documentation/user-manual.txt
 +++ b/Documentation/user-manual.txt
 @@ -3203,17 +3203,15 @@ To put the loose objects into a pack, just run git 
 repack:
  
  
  $ git repack
 -Generating pack...
 -Done counting 6020 objects.
 -Deltifying 6020 objects.
 - 100% (6020/6020) done
 -Writing 6020 objects.
 - 100% (6020/6020) done
 -Total 6020, written 6020 (delta 4070), reused 0 (delta 0)
 -Pack pack-3e54ad29d5b2e05838c75df582c65257b8d08e1c created.
 +Counting objects: 6020, done.
 +Delta compression using up to 4 threads.
 +Compressing objects: 100% (6020/6020), done.
 +Writing objects: 100% (6020/6020), done.
 +Total 6020 (delta 4070), reused 0 (delta 0)
  
  
 -You can then run
 +This creates a single pack file in .git/objects/pack/ 
 +containing all currently unpacked objects.  You can then run
  
  
  $ git prune

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


Re: [PATCH 05/11] Fix some typos and improve wording

2013-08-29 Thread Junio C Hamano
Thomas Ackermann th.ac...@arcor.de writes:

 Signed-off-by: Thomas Ackermann th.ac...@arcor.de
 ---
  Documentation/user-manual.txt | 20 ++--
  1 file changed, 10 insertions(+), 10 deletions(-)

 diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
 index 6241a43..465d9cb 100644
 --- a/Documentation/user-manual.txt
 +++ b/Documentation/user-manual.txt
 @@ -219,7 +219,7 @@ of development leading to that point.
  
  The best way to see how this works is using the linkgit:gitk[1]
  command; running gitk now on a Git repository and looking for merge
 -commits will help understand how the Git organizes history.
 +commits will help understand how Git organizes history.
  
  In the following, we say that commit X is reachable from commit Y
  if commit X is an ancestor of commit Y.  Equivalently, you could say
 @@ -793,7 +793,7 @@ e05db0fd4f31dde7005f075a84f96b360d05984b
  -
  
  Or you could recall that the `...` operator selects all commits
 -contained reachable from either one reference or the other but not
 +reachable from either one reference or the other but not
  both; so
  
  -
 @@ -820,7 +820,7 @@ You could just visually inspect the commits since 
 e05db0fd:
  $ gitk e05db0fd..
  -
  
 -Or you can use linkgit:git-name-rev[1], which will give the commit a
 +or you can use linkgit:git-name-rev[1], which will give the commit a

As raised during the initial review, the previous sentence has ended
and this begins a new sentence, I think, hence Or, not or.

  name based on any tag it finds pointing to one of the commit's
  descendants:
  
 @@ -864,8 +864,8 @@ because it outputs only commits that are not reachable 
 from v1.5.0-rc1.
  
  As yet another alternative, the linkgit:git-show-branch[1] command lists
  the commits reachable from its arguments with a display on the left-hand
 -side that indicates which arguments that commit is reachable from.  So,
 -you can run something like
 +side that indicates which arguments that commit is reachable from.  
 +So, if you run something like
  -
  $ git show-branch e05db0fd v1.5.0-rc0 v1.5.0-rc1 v1.5.0-rc2
 @@ -877,15 +877,15 @@ available
  ...
  -
  
 -then search for a line that looks like
 +then a line like
  
  -
  + ++ [e05db0fd] Fix warnings in sha1_file.c - use C99 printf format if
  available
  -
  
 -Which shows that e05db0fd is reachable from itself, from v1.5.0-rc1, and
 -from v1.5.0-rc2, but not from v1.5.0-rc0.
 +shows that e05db0fd is reachable from itself, from v1.5.0-rc1,
 +and from v1.5.0-rc2, and not from v1.5.0-rc0.

This is easier to read. Good.

--
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 06/11] Simplify How to make a commit

2013-08-29 Thread Junio C Hamano
Looks good; thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 0/9] Add --stage and --work options

2013-08-29 Thread Felipe Contreras
Hi,

Some commands (git diff) already have the --staged alias, this patch series
document them, and do the same for the rest.

Also, add a --work (and --no-work) option, so that in addition to --stage, we
can replace --cached in 'git apply'.

The old options remain unchanged.

Felipe Contreras (9):
  diff: document --staged
  grep: add --staged option
  rm: add --staged option
  stash: add --stage option to save
  stash: add --stage to pop and apply
  submodule: add --staged options
  apply: add --stage option
  apply: add --work, --no-work options
  completion: update --staged options

 Documentation/git-apply.txt| 11 +--
 Documentation/git-diff.txt |  4 ++--
 Documentation/git-grep.txt |  5 -
 Documentation/git-rm.txt   |  5 -
 Documentation/git-stash.txt| 14 +++---
 Documentation/git-submodule.txt|  8 ++--
 builtin/apply.c|  7 +++
 builtin/grep.c |  2 ++
 builtin/rm.c   |  1 +
 contrib/completion/git-completion.bash | 10 +-
 git-stash.sh   | 12 +---
 git-submodule.sh   | 10 +-
 12 files changed, 61 insertions(+), 28 deletions(-)

-- 
1.8.4-fc

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


[PATCH 1/9] diff: document --staged

2013-08-29 Thread Felipe Contreras
Synonym for --cached.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-diff.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 78d6d50..646e5cd 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -10,7 +10,7 @@ SYNOPSIS
 
 [verse]
 'git diff' [options] [commit] [--] [path...]
-'git diff' [options] --cached [commit] [--] [path...]
+'git diff' [options] [--cached|--staged] [commit] [--] [path...]
 'git diff' [options] commit commit [--] [path...]
 'git diff' [options] blob blob
 'git diff' [options] [--no-index] [--] path path
@@ -33,7 +33,7 @@ If exactly two paths are given and at least one points outside
 the current repository, 'git diff' will compare the two files /
 directories. This behavior can be forced by --no-index.
 
-'git diff' [--options] --cached [commit] [--] [path...]::
+'git diff' [--options] [--cached|--staged] [commit] [--] [path...]::
 
This form is to view the changes you staged for the next
commit relative to the named commit.  Typically you
-- 
1.8.4-fc

--
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/9] grep: add --staged option

2013-08-29 Thread Felipe Contreras
Synonym for --cached.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-grep.txt | 5 -
 builtin/grep.c | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 8497aa4..9f7899c 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -25,7 +25,7 @@ SYNOPSIS
   [-W | --function-context]
   [-f file] [-e] pattern
   [--and|--or|--not|(|)|-e pattern...]
-  [ [--[no-]exclude-standard] [--cached | --no-index | --untracked] | 
tree...]
+  [ [--[no-]exclude-standard] [--cached | --staged | --no-index | 
--untracked] | tree...]
   [--] [pathspec...]
 
 DESCRIPTION
@@ -60,6 +60,9 @@ OPTIONS
Instead of searching tracked files in the working tree, search
blobs registered in the index file.
 
+--staged::
+   Synonym for `--cached`.
+
 --no-index::
Search files in the current directory that is not managed by Git.
 
diff --git a/builtin/grep.c b/builtin/grep.c
index d3b3b1d..b953911 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -640,6 +640,8 @@ int cmd_grep(int argc, const char **argv, const char 
*prefix)
struct option options[] = {
OPT_BOOLEAN(0, cached, cached,
N_(search in index instead of in the work tree)),
+   OPT_BOOLEAN(0, staged, cached,
+   N_(search in index instead of in the work tree)),
OPT_NEGBIT(0, no-index, use_index,
 N_(find in contents not managed by git), 1),
OPT_BOOLEAN(0, untracked, untracked,
-- 
1.8.4-fc

--
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/9] rm: add --staged option

2013-08-29 Thread Felipe Contreras
Synonym for --cached.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-rm.txt | 5 -
 builtin/rm.c | 1 +
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 1d876c2..156b40d 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -8,7 +8,7 @@ git-rm - Remove files from the working tree and from the index
 SYNOPSIS
 
 [verse]
-'git rm' [-f | --force] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] 
file...
+'git rm' [-f | --force] [-n] [-r] [--cached | --staged] [--ignore-unmatch] 
[--quiet] [--] file...
 
 DESCRIPTION
 ---
@@ -60,6 +60,9 @@ OPTIONS
Working tree files, whether modified or not, will be
left alone.
 
+--staged::
+   Synonym for --cached.
+
 --ignore-unmatch::
Exit with a zero status even if no files matched.
 
diff --git a/builtin/rm.c b/builtin/rm.c
index 0df0b4d..919911f 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -268,6 +268,7 @@ static struct option builtin_rm_options[] = {
OPT__DRY_RUN(show_only, N_(dry run)),
OPT__QUIET(quiet, N_(do not list removed files)),
OPT_BOOLEAN( 0 , cached, index_only, N_(only remove from 
the index)),
+   OPT_BOOLEAN( 0 , staged, index_only, N_(only remove from 
the index)),
OPT__FORCE(force, N_(override the up-to-date check)),
OPT_BOOLEAN('r', NULL, recursive,  N_(allow recursive 
removal)),
OPT_BOOLEAN( 0 , ignore-unmatch, ignore_unmatch,
-- 
1.8.4-fc

--
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/9] stash: add --stage option to save

2013-08-29 Thread Felipe Contreras
--no-stage is synonym for --keep-index.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-stash.txt | 6 +++---
 git-stash.sh| 8 +++-
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index db7e803..75b4cc6 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,7 +13,7 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [stash]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [stash]
 'git stash' branch branchname [stash]
-'git stash' [save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
+'git stash' [save [-p|--patch] [-k|--[no-]keep-index|--[no-]stage] [-q|--quiet]
 [-u|--include-untracked] [-a|--all] [message]]
 'git stash' clear
 'git stash' create [message]
@@ -44,7 +44,7 @@ is also possible).
 OPTIONS
 ---
 
-save [-p|--patch] [--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] [message]::
+save [-p|--patch] [--[no-]keep-index|--[no-]stage] [-u|--include-untracked] 
[-a|--all] [-q|--quiet] [message]::
 
Save your local modifications to a new 'stash', and run `git reset
--hard` to revert them.  The message part is optional and gives
@@ -54,7 +54,7 @@ save [-p|--patch] [--[no-]keep-index] 
[-u|--include-untracked] [-a|--all] [-q|--
subcommand from making an unwanted stash.
 +
 If the `--keep-index` option is used, all changes already added to the
-index are left intact.
+index are left intact. Same with `--no-stage`, which is a snynonym.
 +
 If the `--include-untracked` option is used, all untracked files are also
 stashed and then cleaned up with `git clean`, leaving the working directory
diff --git a/git-stash.sh b/git-stash.sh
index 1e541a2..47220d0 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -7,7 +7,7 @@ USAGE=list [options]
or: $dashless drop [-q|--quiet] [stash]
or: $dashless ( pop | apply ) [--index] [-q|--quiet] [stash]
or: $dashless branch branchname [stash]
-   or: $dashless [save [--patch] [-k|--[no-]keep-index] [-q|--quiet]
+   or: $dashless [save [--patch] [-k|--[no-]keep-index|--[no-]stage] 
[-q|--quiet]
   [-u|--include-untracked] [-a|--all] [message]]
or: $dashless clear
 
@@ -204,6 +204,12 @@ save_stash () {
--no-keep-index)
keep_index=n
;;
+   --stage)
+   keep_index=n
+   ;;
+   --no-stage)
+   keep_index=t
+   ;;
-p|--patch)
patch_mode=t
# only default to keep if we don't already have an 
override
-- 
1.8.4-fc

--
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 6/9] submodule: add --staged options

2013-08-29 Thread Felipe Contreras
Synonym for --cached.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-submodule.txt |  8 ++--
 git-submodule.sh| 10 +-
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index bfef8a0..904e007 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -11,13 +11,13 @@ SYNOPSIS
 [verse]
 'git submodule' [--quiet] add [-b branch] [-f|--force] [--name name]
  [--reference repository] [--depth depth] [--] repository 
[path]
-'git submodule' [--quiet] status [--cached] [--recursive] [--] [path...]
+'git submodule' [--quiet] status [--cached|--staged] [--recursive] [--] 
[path...]
 'git submodule' [--quiet] init [--] [path...]
 'git submodule' [--quiet] deinit [-f|--force] [--] path...
 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]
  [-f|--force] [--rebase] [--reference repository] [--depth 
depth]
  [--merge] [--recursive] [--] [path...]
-'git submodule' [--quiet] summary [--cached|--files] [(-n|--summary-limit) n]
+'git submodule' [--quiet] summary [--cached|--staged|--files] 
[(-n|--summary-limit) n]
  [commit] [--] [path...]
 'git submodule' [--quiet] foreach [--recursive] command
 'git submodule' [--quiet] sync [--] [path...]
@@ -248,6 +248,10 @@ OPTIONS
commands typically use the commit found in the submodule HEAD, but
with this option, the commit stored in the index is used instead.
 
+
+--staged::
+   Synonym for `--cached`.
+
 --files::
This option is only valid for the summary command. This command
compares the commit in the index with that in the submodule HEAD
diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..823b783 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -6,11 +6,11 @@
 
 dashless=$(basename $0 | sed -e 's/-/ /')
 USAGE=[--quiet] add [-b branch] [-f|--force] [--name name] [--reference 
repository] [--] repository [path]
-   or: $dashless [--quiet] status [--cached] [--recursive] [--] [path...]
+   or: $dashless [--quiet] status [--cached|--staged] [--recursive] [--] 
[path...]
or: $dashless [--quiet] init [--] [path...]
or: $dashless [--quiet] deinit [-f|--force] [--] path...
or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] 
[-f|--force] [--rebase] [--reference repository] [--merge] [--recursive] [--] 
[path...]
-   or: $dashless [--quiet] summary [--cached|--files] [--summary-limit n] 
[commit] [--] [path...]
+   or: $dashless [--quiet] summary [--cached|--staged|--files] 
[--summary-limit n] [commit] [--] [path...]
or: $dashless [--quiet] foreach [--recursive] command
or: $dashless [--quiet] sync [--recursive] [--] [path...]
 OPTIONS_SPEC=
@@ -972,7 +972,7 @@ cmd_summary() {
while test $# -ne 0
do
case $1 in
-   --cached)
+   --cached|--staged)
cached=$1
;;
--files)
@@ -1181,7 +1181,7 @@ cmd_status()
-q|--quiet)
GIT_QUIET=1
;;
-   --cached)
+   --cached|--staged)
cached=1
;;
--recursive)
@@ -1348,7 +1348,7 @@ do
esac
branch=$2; shift
;;
-   --cached)
+   --cached|--staged)
cached=$1
;;
--)
-- 
1.8.4-fc

--
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 5/9] stash: add --stage to pop and apply

2013-08-29 Thread Felipe Contreras
Synonym of --index.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-stash.txt | 8 
 git-stash.sh| 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 75b4cc6..b4066fd 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git stash' list [options]
 'git stash' show [stash]
 'git stash' drop [-q|--quiet] [stash]
-'git stash' ( pop | apply ) [--index] [-q|--quiet] [stash]
+'git stash' ( pop | apply ) [--index|--stage] [-q|--quiet] [stash]
 'git stash' branch branchname [stash]
 'git stash' [save [-p|--patch] [-k|--[no-]keep-index|--[no-]stage] [-q|--quiet]
 [-u|--include-untracked] [-a|--all] [message]]
@@ -96,7 +96,7 @@ show [stash]::
it will accept any format known to 'git diff' (e.g., `git stash show
-p stash@{1}` to view the second most recent stash in patch form).
 
-pop [--index] [-q|--quiet] [stash]::
+pop [--index|--stage] [-q|--quiet] [stash]::
 
Remove a single stashed state from the stash list and apply it
on top of the current working tree state, i.e., do the inverse
@@ -110,12 +110,12 @@ and call `git stash drop` manually afterwards.
 If the `--index` option is used, then tries to reinstate not only the working
 tree's changes, but also the index's ones. However, this can fail, when you
 have conflicts (which are stored in the index, where you therefore can no
-longer apply the changes as they were originally).
+longer apply the changes as they were originally). `--stage` is a synonym.
 +
 When no `stash` is given, `stash@{0}` is assumed, otherwise `stash` must
 be a reference of the form `stash@{revision}`.
 
-apply [--index] [-q|--quiet] [stash]::
+apply [--index|--stage] [-q|--quiet] [stash]::
 
Like `pop`, but do not remove the state from the stash list. Unlike 
`pop`,
`stash` may be any commit that looks like a commit created by
diff --git a/git-stash.sh b/git-stash.sh
index 47220d0..e2eb8dc 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -5,7 +5,7 @@ dashless=$(basename $0 | sed -e 's/-/ /')
 USAGE=list [options]
or: $dashless show [stash]
or: $dashless drop [-q|--quiet] [stash]
-   or: $dashless ( pop | apply ) [--index] [-q|--quiet] [stash]
+   or: $dashless ( pop | apply ) [--index|--stage] [-q|--quiet] [stash]
or: $dashless branch branchname [stash]
or: $dashless [save [--patch] [-k|--[no-]keep-index|--[no-]stage] 
[-q|--quiet]
   [-u|--include-untracked] [-a|--all] [message]]
@@ -373,7 +373,7 @@ parse_flags_and_rev()
-q|--quiet)
GIT_QUIET=-t
;;
-   --index)
+   --index|--stage)
INDEX_OPTION=--index
;;
-*)
-- 
1.8.4-fc

--
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 7/9] apply: add --stage option

2013-08-29 Thread Felipe Contreras
Synonym for --index.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-apply.txt | 5 -
 builtin/apply.c | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index f605327..ce44327 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 'git apply' [--stat] [--numstat] [--summary] [--check] [--index] [--3way]
  [--apply] [--no-add] [--build-fake-ancestor=file] [-R | --reverse]
  [--allow-binary-replacement | --binary] [--reject] [-z]
- [-pn] [-Cn] [--inaccurate-eof] [--recount] [--cached]
+ [-pn] [-Cn] [--inaccurate-eof] [--recount] [--cached|--staged]
  [--ignore-space-change | --ignore-whitespace ]
  [--whitespace=(nowarn|warn|fix|error|error-all)]
  [--exclude=path] [--include=path] [--directory=root]
@@ -67,6 +67,9 @@ OPTIONS
up-to-date, it is flagged as an error.  This flag also
causes the index file to be updated.
 
+--staged::
+   Synonym for --index.
+
 --cached::
Apply a patch without touching the working tree. Instead take the
cached data, apply the patch, and store the result in the index
diff --git a/builtin/apply.c b/builtin/apply.c
index 50912c9..42b5a4b 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4377,6 +4377,8 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
N_(instead of applying the patch, see if the patch is 
applicable)),
OPT_BOOLEAN(0, index, check_index,
N_(make sure the patch is applicable to the current 
index)),
+   OPT_BOOLEAN(0, stage, check_index,
+   N_(make sure the patch is applicable to the current 
index)),
OPT_BOOLEAN(0, cached, cached,
N_(apply a patch without touching the working tree)),
OPT_BOOLEAN(0, apply, force_apply,
-- 
1.8.4-fc

--
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 9/9] completion: update --staged options

2013-08-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/completion/git-completion.bash | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 5da920e..4adc4ed 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -881,7 +881,7 @@ _git_apply ()
__gitcomp 
--stat --numstat --summary --check --index
--cached --index-info --reverse --reject --unidiff-zero
-   --apply --no-add --exclude=
+   --apply --no-add --exclude= --staged
--ignore-whitespace --ignore-space-change
--whitespace= --inaccurate-eof --verbose

@@ -1294,7 +1294,7 @@ _git_grep ()
case $cur in
--*)
__gitcomp 
-   --cached
+   --cached --staged
--text --ignore-case --word-regexp --invert-match
--full-name --line-number
--extended-regexp --basic-regexp --fixed-strings
@@ -2229,7 +2229,7 @@ _git_rm ()
 {
case $cur in
--*)
-   __gitcomp --cached --dry-run --ignore-unmatch --quiet
+   __gitcomp --cached --staged --dry-run --ignore-unmatch --quiet
return
;;
esac
@@ -2296,7 +2296,7 @@ _git_show_branch ()
 
 _git_stash ()
 {
-   local save_opts='--keep-index --no-keep-index --quiet --patch'
+   local save_opts='--keep-index --no-keep-index --stage --no-stage 
--quiet --patch'
local subcommands='save list show apply clear drop pop create branch'
local subcommand=$(__git_find_on_cmdline $subcommands)
if [ -z $subcommand ]; then
@@ -2316,7 +2316,7 @@ _git_stash ()
__gitcomp $save_opts
;;
apply,--*|pop,--*)
-   __gitcomp --index --quiet
+   __gitcomp --index --stage --quiet
;;
show,--*|drop,--*|branch,--*)
;;
-- 
1.8.4-fc

--
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 8/9] apply: add --work, --no-work options

2013-08-29 Thread Felipe Contreras
'git apply', 'git apply --index', 'git apply --cached' do different
things, but what they do is not precisely clear, specially since no
other commands has similar distinctions.

With --no-work (--work being the default), it's clear what the option
would do; modify, or not, the working directory.

So, --work (the default), doesn't cause any changes, and --no-work
enables the current --cache if used with --index.

Eventually --work might replace --cache, if these options are
standarized in the whole git toolset.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-apply.txt | 6 +-
 builtin/apply.c | 5 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
index ce44327..6167061 100644
--- a/Documentation/git-apply.txt
+++ b/Documentation/git-apply.txt
@@ -16,7 +16,7 @@ SYNOPSIS
  [--ignore-space-change | --ignore-whitespace ]
  [--whitespace=(nowarn|warn|fix|error|error-all)]
  [--exclude=path] [--include=path] [--directory=root]
- [--verbose] [patch...]
+ [--verbose] [--no-work] [patch...]
 
 DESCRIPTION
 ---
@@ -75,6 +75,10 @@ OPTIONS
cached data, apply the patch, and store the result in the index
without using the working tree. This implies `--index`.
 
+--[no-]work::
+   Apply a patch with or without touching the working tree, essentially
+   `--no-work` plus `--index` are the equivalent of `--cached`.
+
 -3::
 --3way::
When the patch does not apply cleanly, fall back on 3-way merge if
diff --git a/builtin/apply.c b/builtin/apply.c
index 42b5a4b..a3dd89d 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -4350,6 +4350,7 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
int errs = 0;
int is_not_gitdir = !startup_info-have_repository;
int force_apply = 0;
+   int work = 1;
 
const char *whitespace_option = NULL;
 
@@ -4381,6 +4382,8 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
N_(make sure the patch is applicable to the current 
index)),
OPT_BOOLEAN(0, cached, cached,
N_(apply a patch without touching the working tree)),
+   OPT_BOOLEAN(0, work, work,
+   N_(modify the working tree)),
OPT_BOOLEAN(0, apply, force_apply,
N_(also apply the patch (use with 
--stat/--summary/--check))),
OPT_BOOL('3', 3way, threeway,
@@ -4433,6 +4436,8 @@ int cmd_apply(int argc, const char **argv, const char 
*prefix_)
argc = parse_options(argc, argv, prefix, builtin_apply_options,
apply_usage, 0);
 
+   if (check_index  !work)
+   cached = 1;
if (apply_with_reject  threeway)
die(--reject and --3way cannot be used together.);
if (cached  threeway)
-- 
1.8.4-fc

--
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 6/7] refs: add update_refs for multiple simultaneous updates

2013-08-29 Thread Brad King
On 08/29/2013 01:39 PM, Junio C Hamano wrote:
 Brad King brad.k...@kitware.com writes:
 +for (i=0; i  n; ++i) {
 
 Style:
 
   for (i = 0; i  n; i++) {

Fixed.

 Is it asking for AB-BA deadlock?  If so, is the caller responsible
 for avoiding it?

Since we don't actually block waiting for locks we won't really
deadlock.  However, if two competing callers keep repeating they
might never finish.  In order to avoid this one must sort the refs
so that locks are always acquired in a consistent order.

For Git's internal API I think we can document this in a comment so
that update_refs does not have to sort.  Then we can add a new
ref_update_sort function to sort an array of struct ref_update.
The user-facing update-ref --stdin can then use ref_update_sort.

The sort logic may subsume duplicate ref update detection too.
After sorting a simple linear-time scan can detect duplicates.

 +unsigned char new_sha1[20];
 +unsigned char *old_sha1;
 
 This asymmetry is rather curious and will later become problematic
 when we have more users of this API.  Maybe your callers happen have
 static storage to keep old_sha1[] outside this structure but do not
 have it for new_sha1[] for some unknown reason, which may not apply
 to later callers we may want to add.

I wasn't happy with the asymmetry either but forgot to raise it in
the cover letter.  We need a way to represent old value not given
as different from old value is_null_sha1.

One approach is to use a bit in the flags member that already
stores REF_NODEREF.  However, that will be inconsistent with the
other ref update APIs that check whether old_sha1 is NULL.  We could
still do it and document the bit to mean ignore old_sha1 member of
struct ref_update.

Another approach is to add a dedicated member to struct ref_update.

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


[PATCH 1/3] reset: add --stage and --work options

2013-08-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-reset.txt |  8 
 builtin/reset.c | 20 
 2 files changed, 28 insertions(+)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index f445cb3..5cd75a8 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 'git reset' [-q] [tree-ish] [--] paths...
 'git reset' (--patch | -p) [tree-ish] [--] [paths...]
 'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [commit]
+'git reset' [--stage | --work] [-q] [commit]
 
 DESCRIPTION
 ---
@@ -81,6 +82,13 @@ but carries forward unmerged index entries.
different between commit and HEAD.
If a file that is different between commit and HEAD has local changes,
reset is aborted.
+
+--stage::
+   Reset the index, basically `--mixed`. `--no-stage` is the equivalent of
+   `--soft`.
+
+--work::
+   Resets the working tree, basically `--hard`.
 --
 
 If you want to undo a commit other than the latest on a branch,
diff --git a/builtin/reset.c b/builtin/reset.c
index afa6e02..fbc1abc 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -23,6 +23,7 @@
 
 static const char * const git_reset_usage[] = {
N_(git reset [--mixed | --soft | --hard | --merge | --keep] [-q] 
[commit]),
+   N_(git reset [--stage | --work] [-q] [commit]),
N_(git reset [-q] tree-ish [--] paths...),
N_(git reset --patch [tree-ish] [--] [paths...]),
NULL
@@ -243,6 +244,7 @@ static int update_refs(const char *rev, const unsigned char 
*sha1)
 int cmd_reset(int argc, const char **argv, const char *prefix)
 {
int reset_type = NONE, update_ref_status = 0, quiet = 0;
+   int stage = -1, working_tree = -1;
int patch_mode = 0, unborn;
const char *rev;
unsigned char sha1[20];
@@ -258,6 +260,8 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
N_(reset HEAD, index and working tree), 
MERGE),
OPT_SET_INT(0, keep, reset_type,
N_(reset HEAD but keep local changes), KEEP),
+   OPT_BOOL(0, stage, stage, N_(reset index)),
+   OPT_BOOL(0, work, working_tree, N_(reset working tree)),
OPT_BOOLEAN('p', patch, patch_mode, N_(select hunks 
interactively)),
OPT_END()
};
@@ -290,6 +294,22 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
hashcpy(sha1, tree-object.sha1);
}
 
+   if (stage = 0 || working_tree = 0) {
+   if (reset_type != NONE)
+   die(_(--{stage,work} are incompatible with 
--{hard,mixed,soft,merge}));
+
+   if (working_tree == 1) {
+   if (stage == 0)
+   die(_(--no-stage doesn't make sense with 
--work));
+   reset_type = HARD;
+   } else {
+   if (stage == 1)
+   reset_type = NONE;
+   else
+   reset_type = SOFT;
+   }
+   }
+
if (patch_mode) {
if (reset_type != NONE)
die(_(--patch is incompatible with 
--{hard,mixed,soft}));
-- 
1.8.4-fc

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


[PATCH 0/3] reset: refactor into --stage and --work

2013-08-29 Thread Felipe Contreras
Hi,

This patch series is not really necessary for the whole --stage series, but it
makes sense while we are at it.

Felipe Contreras (3):
  reset: add --stage and --work options
  reset: allow --keep with --stage
  completion: update 'git reset' new stage options

 Documentation/git-reset.txt|  8 
 builtin/reset.c| 27 +++
 contrib/completion/git-completion.bash |  3 ++-
 3 files changed, 37 insertions(+), 1 deletion(-)

-- 
1.8.4-fc

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


[PATCH 2/3] reset: allow --keep with --stage

2013-08-29 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-reset.txt |  2 +-
 builtin/reset.c | 13 ++---
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt
index 5cd75a8..a1419c9 100644
--- a/Documentation/git-reset.txt
+++ b/Documentation/git-reset.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git reset' [-q] [tree-ish] [--] paths...
 'git reset' (--patch | -p) [tree-ish] [--] [paths...]
 'git reset' [--soft | --mixed | --hard | --merge | --keep] [-q] [commit]
-'git reset' [--stage | --work] [-q] [commit]
+'git reset' [--stage | --work | --keep] [-q] [commit]
 
 DESCRIPTION
 ---
diff --git a/builtin/reset.c b/builtin/reset.c
index fbc1abc..dde03a7 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -23,7 +23,7 @@
 
 static const char * const git_reset_usage[] = {
N_(git reset [--mixed | --soft | --hard | --merge | --keep] [-q] 
[commit]),
-   N_(git reset [--stage | --work] [-q] [commit]),
+   N_(git reset [--stage | --work | --keep] [-q] [commit]),
N_(git reset [-q] tree-ish [--] paths...),
N_(git reset --patch [tree-ish] [--] [paths...]),
NULL
@@ -295,8 +295,15 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
}
 
if (stage = 0 || working_tree = 0) {
-   if (reset_type != NONE)
+   int keep = 0;
+
+   if (reset_type == KEEP) {
+   if (working_tree == 1)
+   die(_(--keep is incompatible with --work));
+   keep = 1;
+   } else if (reset_type != NONE) {
die(_(--{stage,work} are incompatible with 
--{hard,mixed,soft,merge}));
+   }
 
if (working_tree == 1) {
if (stage == 0)
@@ -304,7 +311,7 @@ int cmd_reset(int argc, const char **argv, const char 
*prefix)
reset_type = HARD;
} else {
if (stage == 1)
-   reset_type = NONE;
+   reset_type = keep ? KEEP : NONE;
else
reset_type = SOFT;
}
-- 
1.8.4-fc

--
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 0/7] Multiple simultaneously locked ref updates

2013-08-29 Thread Brad King
On 08/29/2013 02:07 PM, Junio C Hamano wrote:
 I didn't mean to force the caller of new update-ref --stdin; the
 new code you wrote for it is what feeds the input to update_refs()
 function, and that is one place you can make sure you do not lock
 yourself out.
 
 Besides, if you get two updates to the same ref from --stdin, you
 would need to verify these are identical (i.e. updating to the same
 new object name from the same old object name; otherwise the requests
 are conflicting and do not make sense), so the code to collect the
 requests from stdin needs to match potential duplicates anyway.
 
 One clean way to do so may be to put an update request (old and new
 sha1) in a structure, and use a string_list to hold list of refs,
 with the util field pointing at the update request data.
 
 - this process already has the lock because it was asked to
   update the same file multiple times simultaneously, or
 
 The second case is like left hand not knowing what right hand is
 doing, no?  It should not happen if we code it right.

Yes.  All of the above is what I originally intended when asking
the question in the cover letter.  Using string_list and its util
field may be useful.  However, see also my response at
$gmane/233260 about how it may fold into sorting.

Thanks for the reviews!
-Brad
--
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 6/7] refs: add update_refs for multiple simultaneous updates

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 On 08/29/2013 01:39 PM, Junio C Hamano wrote:
 Brad King brad.k...@kitware.com writes:
 +   for (i=0; i  n; ++i) {
 
 Style:
 
  for (i = 0; i  n; i++) {

 Fixed.

 Is it asking for AB-BA deadlock?  If so, is the caller responsible
 for avoiding it?

 Since we don't actually block waiting for locks we won't really
 deadlock.

Ahh, OK.

 For Git's internal API I think we can document this in a comment so
 that update_refs does not have to sort.  Then we can add a new
 ref_update_sort function to sort an array of struct ref_update.
 The user-facing update-ref --stdin can then use ref_update_sort.

My immediate reaction was is there a case where the caller knows
that it already has a sorted collection?.  The single caller you
are envisioning could collect the proposed updates to a string list
and dedup, I think, and the resulting list would then be already
sorted.

But it may not be a bad idea to keep the callers dumb and have this
function always sort, dedup, *and* fail inconsistent request.  Then
your original caller that just collects --stdin input can pass
possibly unsorted, duplicated and/or inconsistent request to the
function and have it do the sanity check.
--
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 7/7] update-ref: support multiple simultaneous updates

2013-08-29 Thread Junio C Hamano
Brad King brad.k...@kitware.com writes:

 + const char *c, *s, *oldvalue, *value[2] = {0,0};

This patch has many style issues of the same kind, lack of a SP at
places where there should be between operators and after comma.
--
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: Officially start moving to the term 'staging area'

2013-08-29 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 It has been discussed many times in the past that 'index' is not an
 appropriate description for what the high-level user does with it, and
 it has been agreed that 'staging area' is the best term.

add is the verb, not index (which is a noun that refers
to the thing that keeps track of what will be written as a tree to
be committed next).

And it will stay that way.

IIRC, when this was discussed, many non-native speakers had trouble
with the verb to stage, not just from i18n/l10n point of view.




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


Re: [PATCH 1/2] Add proper 'stage' command

2013-08-29 Thread Matthieu Moy
Felipe Contreras felipe.contre...@gmail.com writes:

 +COMMANDS
 +
 +
 +With no arguments, it's a synonym for linkgit:git-add[1].

This would not be very useful since git add errors out when called
without arguments ;-).

The accurate description of your code would be closer to When the first
argument is not a subcommand, it's a synonym for linkgit:git-add[1]..
I'm not sure I like it, as it creates ambiguities (e.g. need to spell
git stage -- diff to add a file called diff. Not a strong objection
though, as we already have refs Vs filename ambiguities in many places.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 4/9] stash: add --stage option to save

2013-08-29 Thread Matthieu Moy
Felipe Contreras felipe.contre...@gmail.com writes:

 +index are left intact. Same with `--no-stage`, which is a snynonym.

s/snynonym/synonym/

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC 6/7] refs: add update_refs for multiple simultaneous updates

2013-08-29 Thread Brad King
On 08/29/2013 02:32 PM, Junio C Hamano wrote:
 But it may not be a bad idea to keep the callers dumb and have this
 function always sort, dedup, *and* fail inconsistent request.

I agree.  I was just starting to write the comment for update_refs
and it basically would have said always use ref_update_sort and
check for duplicates first.  We might was well build it into the
function.  If some call site in the future wants to optimize a case
known to be sorted it can be refactored later.

Thanks,
-Brad
--
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 8/8] remote-hg: support for notes

2013-08-29 Thread Junio C Hamano
Felipe Contreras felipe.contre...@gmail.com writes:

 @@ -629,6 +657,7 @@ def do_import(parser):
  print feature import-marks=%s % path
  print feature export-marks=%s % path
  print feature force
 +print feature notes

The remaining patches in the series seem reasonable, but this patch
seems to rely on an undefined feature notes, whose design has not
been discussed here.  Will queue without this one for now.
--
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] stage: add edit command

2013-08-29 Thread Felipe Contreras
Matthieu Moy wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  +'edit'::
  +
  +Manually edit the staging area (as a diff).
  +
 
 That sounds interesting. It reminds me git add --edit, but they are
 different ('stage edit' edits the patch with HEAD, 'add --edit' edits
 the patch with the worktree).

That's not the only difference; 'add --edit' is incremental.

 Can we find a consistent user-interface where git add --edit and git
 stage edit have a closer syntax? Maybe git stage edit --work as a
 synonym for git add --edit?

Well, the action is adding changes to the staging area. To me, I'm not editing
the stage, I'm editing the change I'm adding to the stage, so 'git stage
--edit' is perfectly fine.

-- 
Felipe Contreras
--
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: Officially start moving to the term 'staging area'

2013-08-29 Thread Matthieu Moy
Felipe Contreras felipe.contre...@gmail.com writes:

 It has been discussed many times in the past that 'index' is not an
 appropriate description for what the high-level user does with it, and
 it has been agreed that 'staging area' is the best term.

Thanks for working on this. No time for a really detailed review, but a
few remarks.

 The term 'staging area' is more intuitive [...]

 The first step in moving Git towards this term, is first to add --stage
 options for every command that uses --index or --cache.

These explanations make sense. I think it would be better to put part of
it in commit messages, so that future contributors can git blame the
doc/implem of these --stage and find them (i.e. avoid the
misunderstanding that occured with git stage command which was
proposed for removal).

 After adding the new --stage options and making sure no functionality is
 lost, they can become the recommended ones in the documentation,
 eventually, the old ones get deprecated, and eventually obsoleted.

Same: putting this in the commit message would cast in stone that we
want to obsolete the old ones.

(But that was nice to have this cover-letter)

 Moreover, the --stage and --work

--work alone sounds weird. At least to me, it does not immediately imply
working tree. It is tempting to call the option --work-tree, but git
already has a global option with that name (git --work-tree=foo bar).

Perhaps --worktree to limit the confusion?

 reset', and after these options are added, the complicated table to
 explain the different behaviors between --soft, --mixed, and --hard
 becomes so simple it's not needed any more:

I didn't understand the table, but yes, the --soft, --mixed, and --hard
is terrible, I need to read the doc whenever I do something non-trivial
with reset :-(.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] Add proper 'stage' command

2013-08-29 Thread Felipe Contreras
Matthieu Moy wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  +COMMANDS
  +
  +
  +With no arguments, it's a synonym for linkgit:git-add[1].
 
 This would not be very useful since git add errors out when called
 without arguments ;-).

Right.

 The accurate description of your code would be closer to When the first
 argument is not a subcommand, it's a synonym for linkgit:git-add[1]..
 I'm not sure I like it, as it creates ambiguities (e.g. need to spell
 git stage -- diff to add a file called diff. Not a strong objection
 though, as we already have refs Vs filename ambiguities in many places.

Yes, I thought really hard about this, but I think it's the best alternative.
In adition, we could stat to see if there's a file with the same name as the
assumed subcommand, and warn about the ambiguity, and the recommended way to
add the file.

-- 
Felipe Contreras
--
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] git-remote-mediawiki: use dont-update-private capability on dumb push

2013-08-29 Thread Matthieu Moy
Signed-off-by: Matthieu Moy matthieu@imag.fr
---

I prefer not to bother with compatibility of git-remote-mediawiki with
older Git versions. The recommanded way is to install Git and
git-remote-mediawiki from the same source tree. People who want to
keep an old Git version can still use old versions of
git-remote-mediawiki.

 contrib/mw-to-git/git-remote-mediawiki.perl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl 
b/contrib/mw-to-git/git-remote-mediawiki.perl
index f8d7d2c..cdc278c 100755
--- a/contrib/mw-to-git/git-remote-mediawiki.perl
+++ b/contrib/mw-to-git/git-remote-mediawiki.perl
@@ -590,6 +590,9 @@ sub mw_capabilities {
print {*STDOUT} import\n;
print {*STDOUT} list\n;
print {*STDOUT} push\n;
+   if ($dumb_push) {
+   print {*STDOUT} dont-update-private\n;
+   }
print {*STDOUT} \n;
return;
 }
-- 
1.8.4.12.g98a4f55.dirty

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


Re: [PATCH 4/4] git-remote-mediawiki: no need to update private ref in non-dumb push

2013-08-29 Thread Junio C Hamano
Matthieu Moy matthieu@imag.fr writes:

 We used to update the private ref ourselves, but this update is now done
 by default (since 664059fb62).

 Signed-off-by: Matthieu Moy matthieu@imag.fr
 ---

Thanks; will queue all four.
--
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: Officially start moving to the term 'staging area'

2013-08-29 Thread Felipe Contreras
Matthieu Moy wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:
 
  It has been discussed many times in the past that 'index' is not an
  appropriate description for what the high-level user does with it, and
  it has been agreed that 'staging area' is the best term.
 
 Thanks for working on this. No time for a really detailed review, but a
 few remarks.
 
  The term 'staging area' is more intuitive [...]
 
  The first step in moving Git towards this term, is first to add --stage
  options for every command that uses --index or --cache.
 
 These explanations make sense. I think it would be better to put part of
 it in commit messages, so that future contributors can git blame the
 doc/implem of these --stage and find them (i.e. avoid the
 misunderstanding that occured with git stage command which was
 proposed for removal).

Yes, but which commit? All of them? Perhaps it would make sense to add a link
to the cover e-mail, or add an explanation in Documentation/gitstagingarea.txt
or something.

  Moreover, the --stage and --work
 
 --work alone sounds weird. At least to me, it does not immediately imply
 working tree. It is tempting to call the option --work-tree, but git
 already has a global option with that name (git --work-tree=foo bar).

Yes, --work sounds weird, but so does --cherry. I thought about --wt, but I
felt --work was more understandable, and --work-tree doesn't really give much
more value, except more characters to type =/

  reset', and after these options are added, the complicated table to
  explain the different behaviors between --soft, --mixed, and --hard
  becomes so simple it's not needed any more:
 
 I didn't understand the table, but yes, the --soft, --mixed, and --hard
 is terrible, I need to read the doc whenever I do something non-trivial
 with reset :-(.

Me too, I always got 'git reset --hard', now I get 'git reset', but that's
about it, for the rest I have to read the documentation. Also, 'git stage
reset' makes it even easier.

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


Re: [PATCH 2/4] transport-helper: add dont-update-private capability

2013-08-29 Thread Felipe Contreras
On Thu, Aug 29, 2013 at 1:58 PM, Matthieu Moy matthieu@imag.fr wrote:
 Since 664059fb62 (Felipe Contreras, Apr 17 2013, transport-helper: update
 remote helper namespace), a 'push' operation on a remote helper updates
 the private ref by default. This is often a good thing, but it can also
 be desirable to disable this update to force the next 'pull' to re-import
 the pushed revisions.

 Allow remote-helpers to disable the automatic update by introducing a new
 capability.

Looks good to me, but how about 'no-private-update'?

-- 
Felipe Contreras
--
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: Officially start moving to the term 'staging area'

2013-08-29 Thread Matthieu Moy
Felipe Contreras felipe.contre...@gmail.com writes:

 Matthieu Moy wrote:
 These explanations make sense. I think it would be better to put part of
 it in commit messages, so that future contributors can git blame the
 doc/implem of these --stage and find them (i.e. avoid the
 misunderstanding that occured with git stage command which was
 proposed for removal).

 Yes, but which commit? All of them? Perhaps it would make sense to add a link
 to the cover e-mail, or add an explanation in Documentation/gitstagingarea.txt
 or something.

Putting the explanation about --stage in the first commit about it, and
then saying something like continue the work on --stage in the next
messages would make sense to me.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


  1   2   >