error: Trying to write ref refs/heads/master with nonexistant object

2013-04-11 Thread Joel Thompson
Hi,

I can no longer clone my git repository from v1.7 client, i get error: Trying 
to write ref refs/heads/master with nonexistant object.

It is a remote repo that im using ssh to connect to, and i have done everything 
i can find on google from creating a new master branch, cleaning up the repo, 
removing dangling bits etc. etc.

So far I have come to the conclusion that while creating a tag on my v1.8 
client in windows, something has happened which has stopped the repo from being 
compatible with v1.7 (as i am still able to clone it with 1.8).

The full message when trying to clone is:

Initialized empty Git repository in /home/jthompso/Downloads/mtsd/.git/
remote: Counting objects: 2464, done.
remote: Compressing objects: 100% (905/905), done.
remote: Total 2464 (delta 1201), reused 2446 (delta 1186)
Receiving objects: 100% (2464/2464), 652.66 KiB, done.
Resolving deltas: 100% (1201/1201), done.
error: refs/remotes/origin/master does not point to a valid object!
error: Trying to write ref refs/heads/master with nonexistant object 
fdf19005911f07e3246649f906e0d45fa9ff884f
fatal: Cannot update the ref 'HEAD'.

Regards,

Joel Thompson
I.T. Support Officer

Macquarie Textile Solutions
211 East St, Albury NSW 2640
www.macquarietextiles.com.au
T: +61 2 6043 0302 F: +61 2 6041 1321
--
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: git-http-backend: anonymous read, authenticated write

2013-04-11 Thread Magnus Therning
On Thu, Apr 11, 2013 at 3:56 AM, Jeff King p...@peff.net wrote:
 On Thu, Apr 11, 2013 at 01:19:19AM +0200, Magnus Therning wrote:

 Nope.  I'm pretty sure this had *nothing* to do with my config.  This
 is the original config, which doesn't work:

 $HTTP[url] =~ ^/git {
 cgi.assign = (  =  )
 setenv.add-environment = (
 GIT_PROJECT_ROOT = /srv/git,
 GIT_HTTP_EXPORT_ALL = 
 )
 $HTTP[url] =~ ^/git/.*/git-receive-pack$ {
 include trac-git-auth.conf
 }
 }

 Ah, I think I see what it is.

 Did you turn on http.receivepack in the git config to enable pushing?

Nope, of course I didn't :)  Instead I did what the man-page tells me
will allow full export of a git repo *without* having to fiddle around
with the repo's config:

1. set GIT_HTTP_EXPORT_ALL in the environment
2. turn on authentication for *one* location that requires it for
pushing: ^/git/.*/git-receive-pack$

[...]
 If there is no authentication happening for the initial service-request,
 then the default http.receivepack kicks in, which is to turn pushing
 off (because there is no authenticated user).

Yes, but that only becomes clear when looking at the traffic.  In
fact, the whole design of services is not clearly mentioned in the
man-page.  I was *very* surprised to see the query strings when I
started looking at the access logs.

 The documentation should probably make the use of http.receivepack more
 clear in this situation.

I think that'd be good.  The fact that it wasn't until several mails
into the thread that anyone thought of the http.receivepack setting
also suggests that its use is a bit un-intuitive (even though it
probably makes perfect sense and is a good solution).

 So _if_ you fixed it by setting http.receivepack (which I think is the
 simplest thing under Apache, since matching the query string there is
 hard), then you would need a version of git with that fix on the
 client side to actually have git prompt for the password correctly.

Ah, so *that* is the fix that has been mentioned (I haven't bothered
reading it myself), or are there in fact two fixes that have been
referred to in the thread?

 But your fix under lighttpd is much better, as it asks for the
 credentials up front (which means the client does not go to any work
 creating a packfile just to find out that it does not have access).

Yes, I think it also helps with my particular scenario where new repos
will be added from time to time.  This way there is no second step,
after `git init`, that must be remembered.

Thank you very much for taking the time to help me out with this!
I'll also take a look at the patches you sent, as a dumb simpler user
I might have something to add, who knows?

/M

--
Magnus Therning  OpenPGP: 0xAB4DFBA4
email: mag...@therning.org   jabber: mag...@therning.org
twitter: magthe   http://therning.org/magnus
--
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] doc/http-backend: clarify half-auth repo configuration

2013-04-11 Thread Magnus Therning
On Thu, Apr 11, 2013 at 5:32 AM, Jeff King p...@peff.net wrote:
 When the http-backend is set up to allow anonymous read but
 authenticated write, the http-backend manual suggests
 catching only the /git-receive-pack POST of the packfile,
 not the initial info/refs?service=git-receive-pack GET in
 which we advertise refs.

 This does work and is secure, as we do not allow any write
 during the info/refs request, and the information in the ref
 advertisement is the same that you would get from a fetch.

 However, the configuration required by the server is
 slightly more complex. The default `http.receivepack`
 setting is to allow pushes if the webserver tells us that
 the user authenticated, and otherwise to return a 403
 (Forbidden). That works fine if authentication is turned
 on completely; the initial request requires authentication,
 and http-backend realizes it is OK to do a push.

 But for this half-auth state, no authentication has
 occurred during the initial ref advertisement. The
 http-backend CGI therefore does not think that pushing
 should be enabled, and responds with a 403. The client
 cannot continue, even though the server would have allowed
 it to run if it had provided credentials.

 It would be much better if the server responded with a 401,
 asking for credentials during the initial contact. But
 git-http-backend does not know about the server's auth
 configuration (so a 401 would be confusing in the case of a
 true anonymous server). Unfortunately, configuring Apache to
 recognize the query string and apply the auth appropriately
 to receive-pack (but not upload-pack) initial requests is
 non-trivial.

 The site admin can work around this by just turning on
 http.receivepack explicitly in its repositories. Let's
 document this workaround.

 Signed-off-by: Jeff King p...@peff.net
 ---
 My understanding is that you can do query-string matching through a
 clever use of mod-rewrite. I am not nearly clever nor interested in
 Apache enough to figure it out, but if somebody does, it would be nice
 to put the recipe here.

  Documentation/git-http-backend.txt | 9 +
  1 file changed, 9 insertions(+)

 diff --git a/Documentation/git-http-backend.txt 
 b/Documentation/git-http-backend.txt
 index 7b1e85c..f43980f 100644
 --- a/Documentation/git-http-backend.txt
 +++ b/Documentation/git-http-backend.txt
 @@ -91,6 +91,15 @@ require authorization with a LocationMatch directive:
  /LocationMatch
  
  +
 +In this mode, the server will not request authentication until the
 +client actually starts the object negotiation phase of the push, rather
 +than during the initial contact.  For this reason, you must also enable
 +the `http.receivepack` config option in any repositories that should
 +accept a push. The default behavior, if `http.receivepack` is not set,
 +is to reject any pushes by unauthenticated users; the initial request
 +will therefore report `403 Forbidden` to the client, without even giving
 +an opportunity for authentication.
 ++
  To require authentication for both reads and writes, use a Location
  directive around the repository, or one of its parent directories:
  +
 --
 1.8.2.rc0.33.gd915649


As the dumb user who started the thread that lead to this proposed
change, I do think this makes the documentation much clearer and had I
read this I most probably would have managed to set up the webserver
on my own.

/M

-- 
Magnus Therning  OpenPGP: 0xAB4DFBA4
email: mag...@therning.org   jabber: mag...@therning.org
twitter: magthe   http://therning.org/magnus
--
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 send-pack: protocol error: bad band #50

2013-04-11 Thread João Joyce

Hello,

I am not sure this is the right place to ask this. If it isn't I would 
be grateful if you could help me find the best place to do it.


I am trying to push some files to a server with git push. I have 
configured the server to push the files:

git remote set-url test ssh://u...@location.com:2200/fullpath/

but I am getting the following error:
git send-pack: protocol error: bad band #50
fatal: The remote end hung up unexpectedly

It seems that something is failing on the remote side but I can't find 
any reference to this protocol error.


Does anyone know how to solve this error? Or which logs should I search 
to better understand the problem?


Thank you very much,
João Joyce
--
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: [ITCH] Specify refspec without remote

2013-04-11 Thread Ramkumar Ramachandra
Jeff King wrote:
   $ git clone https://github.com/upstream/project.git
   $ cd project
   $ hack hack hack; commit commit commit
   $ git tag -m 'something of note' my-tag
   $ git remote add me https://github.com/me/project.git
   $ git config branch.master.remote me
   $ git tag -m 'something of note'
   $ git push master my-tag

Tags have nothing to do with branches, and it is illogical to respect
branch.* when pushing a tag.  You've illustrated a common case when
the user creates a tag on a specific branch and immediately pushes it.
 I would argue that optimizing our tools for this specific usecase
breaks the general case.  What if I create a branch on master, and
decide to push the tag after checking out implicit-push and doing some
work on it?  Does that not break user expectations?

In the I push to the same place I pull from (aka. single-remote)
case, there are never any problems and even matching works fine.
However, in the triangular workflow (aka. multiple-remote) case, you
must give git enough information about your workflow for it to DTRT.
I will argue that, in the above example, you have not configured git
for a multiple-remote case, and that you cannot expect it to DTRT
since you have supplied insufficient information.  As to how to
configure git for the general multiple-remote case:

Let us imagine that origin points to git/git.git (upstream), ram
points to artagnon/git.git and peff points to peff/git.git.  I fork
off from upstream and have various local branches that only have
corresponding refs in ram (say implicit-push).  Then, you fork off
from me, and have various local branches that only have corresponding
refs in peff (say implicit-push-next).  I have peff as a configured
remote, because I routinely review the changes you make to my fork.
In this case, I must have:

- push.default set to anything but matching, because matching makes no
sense in the multiple-remote scenario*.

- remote.default set to origin, because this is where I get new code
from for all branches.

- remote.pushdefault set to ram, because this is where I publish all
my refs to (whether branches or tags).  I might have local branches
that I will never publish, but that's a separate issue.  The point is
that all my tags will always be published here.

- branch.implicit-push.remote set to ram, because this is the correct
upstream for the implicit-push branch.

- branch.implicit-push-next.remote set to peff, because this is the
correct upstream for the implicit-push-next branch.

- branch.implicit-push-next.pushremote set to null**, because I will
never want to push this branch.

(Note that branch.implicit-push.pushremote is unnecessary because
remote.pushdefault takes care of that)

With these settings, git push will always DTRT when you specify
nothing, just a remote, or just a refspec.  Ofcourse, you can specify
both and be explicit (which is what we do now).  Does this make sense?
 Should we document this in gitworkflows.txt so that users know what
is expected of them when they move from a single-remote setup to a
multi-remote setup?

* I will definitely push for the deprecation of push.default=matching,
but I doubt we need to invent a new push.default.  current makes a lot
of sense to me personally.

** Yet to be invented.
--
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: git rebase : knowing where I am...

2013-04-11 Thread Tay Ray Chuan
On Wed, Apr 10, 2013 at 4:40 PM, Jeremy Rosen jeremy.ro...@openwide.fr wrote:
 is there some way to know how far you are within a rebase when the rebase is 
 interupted by a conflict other than the message given by git rebase when it 
 was interrupted ?

How about

  $ cat .git/rebase-merge/done

sample output:

  p 3b465bd foo2 1
  e 03f8bea foo2 2
  e 0871817 foo2 1

last line is the current commit being edited/under conflict/etc.

--
Cheers,
Ray Chuan
--
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: [ITCH] Specify refspec without remote

2013-04-11 Thread Ramkumar Ramachandra
Ramkumar Ramachandra wrote:
 - branch.implicit-push-next.pushremote set to null**, because I will
 never want to push this branch.

Currently, I have a hacky workaround: I set
branch.implicit-push-next.pushremote to a remote that I don't have
write access to (ie. origin), effectively failing all my attempts to
push this branch.
--
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: [git] regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Mike Galbraith
On Thu, 2013-04-11 at 01:42 -0400, Jeff King wrote: 
 On Thu, Apr 11, 2013 at 05:39:43AM +0200, Mike Galbraith wrote:
 
 ALLOWED_ENV=PATH HOME
 HOME=/
  
  I can work around it by changing the init script to use su - git -c bla
  bla to launch the thing, instead of using --user=git --group=daemon,
  but that's just a bandaid for the busted environment setup those
  switches were supposed to make happen, no?
 
 Yeah, I think the bug here is that git-daemon should be setting $HOME
 when it switches privileges with --user. Does this patch fix it for you?
 
 diff --git a/daemon.c b/daemon.c
 index 6aeddcb..a4451fd 100644
 --- a/daemon.c
 +++ b/daemon.c
 @@ -1091,6 +1091,7 @@ static void drop_privileges(struct credentials *cred)
   if (cred  (initgroups(cred-pass-pw_name, cred-gid) ||
   setgid (cred-gid) || setuid(cred-pass-pw_uid)))
   die(cannot drop privileges);
 + setenv(HOME, cred-pass-pw_dir, 1);
  }
  
  static struct credentials *prepare_credentials(const char *user_name,
 
 I guess that would technically break anybody who was trying to do
 something clever with HOME (i.e., point it somewhere besides --user's
 HOME where they had put some config files). But the obvious clever
 thing would be to also set the user's passwd homedir to the same place.

I did exactly that yesterday, and it didn't work, which rather surprised
me.  Let me double check that I didn't screw trivial all up...

Heh, if ya don't plunk new binary into the old oddly placed bucket :)
Yeah, that works just fine.

-Mike 


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


Re: [PATCH v2 2/3] Teach mv to move submodules using a gitfile

2013-04-11 Thread Ramkumar Ramachandra
Jens Lehmann wrote:
 When moving a submodule which uses a gitfile to point to the git directory
 stored in .git/modules/name of the superproject two changes must be made
 to make the submodule work: the .git file and the core.worktree setting
 must be adjusted to point from work tree to git directory and back.

Isn't it untrue that the git directory is stored in
.git/modules/name: it is stored in .git/modules/path/to/module.  I
thought the whole point of this complex scheme was to avoid name
conflicts with submodules with the same name in other directories.
Then why aren't you moving the object store as well?  What happens if
I try to create a submodule in the old path of this moved submodule in
the future?  How will you fail, and how will the user recover from
this?

A nit on the wording.  Perhaps: the .git file in the worktree must be
adjusted to point to $GITDIR, and core.worktree must be set to point
to the worktree.

 Achieve that by remembering which submodule uses a gitfile by storing the
 result of read_gitfile() of each submodule. If that is not NULL the new
 function connect_work_tree_and_git_dir() is called after renaming the
 submodule's work tree which updates the two settings to the new values.

Oh God.  Can't you figure out at mv-time if the submodule uses a
gitfile?  Why are you storing it?  Where are you storing it exactly?

 diff --git a/builtin/mv.c b/builtin/mv.c
 index 361028d..609bbb8 100644
 --- a/builtin/mv.c
 +++ b/builtin/mv.c
 @@ -9,6 +9,7 @@
  #include cache-tree.h
  #include string-list.h
  #include parse-options.h
 +#include submodule.h

  static const char * const builtin_mv_usage[] = {
 N_(git mv [options] source... destination),
 @@ -65,7 +66,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 OPT_BOOLEAN('k', NULL, ignore_errors, N_(skip move/rename 
 errors)),
 OPT_END(),
 };
 -   const char **source, **destination, **dest_path;
 +   const char **source, **destination, **dest_path, **submodule_gitfile;
 enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
 struct stat st;
 struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
 @@ -84,6 +85,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 source = copy_pathspec(prefix, argv, argc, 0);
 modes = xcalloc(argc, sizeof(enum update_mode));
 dest_path = copy_pathspec(prefix, argv + argc, 1, 0);
 +   submodule_gitfile = xcalloc(argc, sizeof(char *));

This is cmd_mv, and you're allocating argc number of char * pointers
to submodule_gitfile.  Why?  Are you guaranteed that there are argc
number of submodule_gitfiles?

 if (dest_path[0][0] == '\0')
 /* special case: . was normalized to  */
 @@ -119,8 +121,14 @@ int cmd_mv(int argc, const char **argv, const char 
 *prefix)
 else if (src_is_dir) {
 int first = cache_name_pos(src, length);
 if (first = 0) {
 +   struct strbuf submodule_dotgit = STRBUF_INIT;
 if 
 (!S_ISGITLINK(active_cache[first]-ce_mode))
 die (_(Huh? Directory %s is in index 
 and no submodule?), src);
 +   strbuf_addf(submodule_dotgit, %s/.git, 
 src);
 +   submodule_gitfile[i] = 
 read_gitfile(submodule_dotgit.buf);

What?!  read_gitfile() returns the path to the git directory, if it is
found.  How are you assigning the path to a $GITDIR to a variable
named submodule_gitfile?

 +   if (submodule_gitfile[i])
 +   submodule_gitfile[i] = 
 xstrdup(submodule_gitfile[i]);

Doesn't read as smoothly, but saves you an extra char *.  Okay.

 +   strbuf_release(submodule_dotgit);
 } else {
 const char *src_w_slash = add_slash(src);
 int last, len_w_slash = length + 1;
 @@ -215,9 +223,12 @@ int cmd_mv(int argc, const char **argv, const char 
 *prefix)
 int pos;
 if (show_only || verbose)
 printf(_(Renaming %s to %s\n), src, dst);
 -   if (!show_only  mode != INDEX 
 -   rename(src, dst)  0  !ignore_errors)
 -   die_errno (_(renaming '%s' failed), src);
 +   if (!show_only  mode != INDEX) {
 +   if (rename(src, dst)  0  !ignore_errors)
 +   die_errno (_(renaming '%s' failed), src);
 +   if (submodule_gitfile[i])
 +   connect_work_tree_and_git_dir(dst, 
 submodule_gitfile[i]);
 +   }

Okay, scratch my previous comment about allocating argc char *
pointers for submodule_gitfile().  Since your logic is in a loop that
loops argc 

Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees

2013-04-11 Thread Ramkumar Ramachandra
Jens Lehmann wrote:
 Currently the attempt to use git mv on a submodule errors out with:
   fatal: source directory is empty, source=src, destination=dest
 The reason is that mv searches for the submodule with a trailing slash in
 the index, which it doesn't find (because it is stored without a trailing
 slash). As it doesn't find any index entries inside the submodule it
 claims the directory would be empty even though it isn't.

Why does it search for a submodule with a trailing slash in the index?
 You make it sound like it's doing something unnatural; in reality, it
does this because it executes lstat() on the filesystem path
specified, and the stat mode matches S_ISDIR (because it _is_ a
directory on the filesystem).  It is stored in the index as an entry
(without a trailing slash) with the mode 16, gitlink.

What happens if I attempt to git mv oldpath/ newpath/ (with the
literal slashes, probably because I'm using a stupid shell
completion)?

 Fix that by searching for the name without a trailing slash and continue
 if it is a submodule.

So, the correct solution is not to search for a name without a
trailing slash, but rather to handle the gitlink entry in the index
appropriately.

 Then rename() will move the submodule work tree just
 like it moves a file.

What is this rename() function you're talking about?  I don't see it anywhere.

 diff --git a/builtin/mv.c b/builtin/mv.c
 index 034fec9..361028d 100644
 --- a/builtin/mv.c
 +++ b/builtin/mv.c
 @@ -117,55 +117,60 @@ int cmd_mv(int argc, const char **argv, const char 
 *prefix)
  lstat(dst, st) == 0)
 bad = _(cannot move directory over file);
 else if (src_is_dir) {
 -   const char *src_w_slash = add_slash(src);
 -   int len_w_slash = length + 1;
 -   int first, last;
 -
 -   modes[i] = WORKING_DIRECTORY;
 -
 -   first = cache_name_pos(src_w_slash, len_w_slash);
 -   if (first = 0)
 -   die (_(Huh? %.*s is in index?),
 -   len_w_slash, src_w_slash);
 -
 -   first = -1 - first;
 -   for (last = first; last  active_nr; last++) {
 -   const char *path = active_cache[last]-name;
 -   if (strncmp(path, src_w_slash, len_w_slash))
 -   break;
 -   }
 -   free((char *)src_w_slash);
 -
 -   if (last - first  1)
 -   bad = _(source directory is empty);
 -   else {
 -   int j, dst_len;
 -
 -   if (last - first  0) {
 -   source = xrealloc(source,
 -   (argc + last - first)
 -   * sizeof(char *));
 -   destination = xrealloc(destination,
 -   (argc + last - first)
 -   * sizeof(char *));
 -   modes = xrealloc(modes,
 -   (argc + last - first)
 -   * sizeof(enum 
 update_mode));
 +   int first = cache_name_pos(src, length);
 +   if (first = 0) {
 +   if 
 (!S_ISGITLINK(active_cache[first]-ce_mode))
 +   die (_(Huh? Directory %s is in index 
 and no submodule?), src);

I didn't understand this.  Why does it have to be a gitlink if it is
stored at index position = 0?
I'm assuming the else-case has nothing to do with the actual moving
but rather something specific to directories (enumerating entries in
it?), which is why it doesn't get executed when we find a gitlink.

 +   } else {
 +   const char *src_w_slash = add_slash(src);
 +   int last, len_w_slash = length + 1;
 +
 +   modes[i] = WORKING_DIRECTORY;
 +
 +   first = cache_name_pos(src_w_slash, 
 len_w_slash);
 +   if (first = 0)
 +   die (_(Huh? %.*s is in index?),
 +   len_w_slash, 
 src_w_slash);
 +
 +   first = -1 - first;
 +   for (last = first; last  active_nr; last++) {
 +   const char *path = 
 active_cache[last]-name;
 +   if (strncmp(path, src_w_slash, 
 

Re: git send-pack: protocol error: bad band #50

2013-04-11 Thread Konstantin Khomoutov
On Thu, 11 Apr 2013 07:49:44 +0100
João Joyce joao.jo...@netcabo.pt wrote:

[...]

 but I am getting the following error:
  git send-pack: protocol error: bad band #50
  fatal: The remote end hung up unexpectedly
 
 It seems that something is failing on the remote side but I can't
 find any reference to this protocol error.

I should add that the OP first asked this question on SO [1] and I
advised him to simply run `git --version` on the remote via SSH to
verify Git is working there, and it worked OK.

Another thing the OP failed to provide is the info on his setup.
To cite his message posted to git-users [2]:

 I am using ubuntu 12.10 in my local machine and 12.04 in the remote
 (which is a VPS hosting). The git version is 1.7.9.5 on both.

Hope this might help tracking down the issue.

1. http://stackoverflow.com/q/15921202/720999
2. https://groups.google.com/forum/?fromgroups=#!topic/git-users/eJXASG1GlDA
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] cherry-pick: make sure all input objects are commits

2013-04-11 Thread Miklos Vajna
When a single argument was a non-commit, the error message used to be:

fatal: BUG: expected exactly one commit from walk

For multiple arguments, when none of the arguments was a commit, the error was:

fatal: empty commit set passed

Finally, when some of the arguments were non-commits, we ignored those
arguments.  Instead, now make sure all arguments are commits, and for
the first non-commit, error out with:

fatal: name: Can't cherry-pick a type

Signed-off-by: Miklos Vajna vmik...@suse.cz
---

On Mon, Apr 08, 2013 at 09:56:55AM -0700, Junio C Hamano gits...@pobox.com 
wrote:
 In other words, perhaps we would want to inspect pending objects
 before running prepare_revision_walk and make sure everybody is
 commit-ish or something?

Sure, that makes sense to me.

 sequencer.c | 13 +
 t/t3508-cherry-pick-many-commits.sh |  6 ++
 2 files changed, 19 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index baa0310..eb25101 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1047,6 +1047,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 {
struct commit_list *todo_list = NULL;
unsigned char sha1[20];
+   int i;
 
if (opts-subcommand == REPLAY_NONE)
assert(opts-revs);
@@ -1067,6 +1068,18 @@ int sequencer_pick_revisions(struct replay_opts *opts)
if (opts-subcommand == REPLAY_CONTINUE)
return sequencer_continue(opts);
 
+   for (i = 0; i  opts-revs-pending.nr; i++) {
+   unsigned char sha1[20];
+   const char *name = opts-revs-pending.objects[i].name;
+
+   if (!get_sha1(name, sha1)) {
+   enum object_type type = sha1_object_info(sha1, NULL);
+
+   if (type  0  type != OBJ_COMMIT)
+   die(_(%s: can't cherry-pick a %s), name, 
typename(type));
+   }
+   }
+
/*
 * If we were called as git cherry-pick commit, just
 * cherry-pick/revert it, set CHERRY_PICK_HEAD /
diff --git a/t/t3508-cherry-pick-many-commits.sh 
b/t/t3508-cherry-pick-many-commits.sh
index 4e7136b..19c99d7 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -55,6 +55,12 @@ one
 two
 '
 
+test_expect_success 'cherry-pick three one two: fails' '
+   git checkout -f master 
+   git reset --hard first 
+   test_must_fail git cherry-pick three one two:
+'
+
 test_expect_success 'output to keep user entertained during multi-pick' '
cat -\EOF expected 
[master OBJID] second
-- 
1.8.1.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


Re: [PATCH/RFC 3/3] Teach mv to update the path entry in .gitmodules for moved submodules

2013-04-11 Thread Ramkumar Ramachandra
Jens Lehmann wrote:
 Currently using git mv on a submodule moves the submodule's work tree in
 that of the superproject.

It's not Currently: it's the result of your last two patches.  The
wording is very unclear.  Perhaps: As a result of the last two patches
in this series, a 'git mv' moves the submodule directory to a new path
in the superproject's worktree, and updates the cache entry in the
index appropriately.

 But the submodule's path setting in .gitmodules
 is left untouched, which is now inconsistent with the work tree and makes
 git commands that rely on the proper path - name mapping (like status and
 diff) behave strangely.

I'm frankly a little surprised that your previous two patches didn't
break any tests.  That's probably because we don't have enough tests
to exercise git-core with submodules.  I'm not saying that it's a bad
thing though: submodules are still immature, and tight tests would get
in the way of new patches.

 Let git mv help here by not only moving the submodule's work tree but
 also updating the submodule.submodule name.path setting from the
 .gitmodules file and stage both.

 This doesn't happen when no .gitmodules
 file is found and only issues a warning when it doesn't have a section for
 this submodule. This is because the user might just use plain gitlinks
 without the .gitmodules file

How will git-core work without a .gitmodules?  Shouldn't we create a
fresh .gitmodules here?

 or has already updated the path setting by
 hand before issuing the git mv command (in which case the warning
 reminds him that mv would have done that for him).

Shouldn't these two cases issue different warnings?

Besides, why is this explanation missing in your rm patch's commit message?

 diff --git a/builtin/mv.c b/builtin/mv.c
 index 609bbb8..36e5605 100644
 --- a/builtin/mv.c
 +++ b/builtin/mv.c
 @@ -239,6 +242,9 @@ int cmd_mv(int argc, const char **argv, const char 
 *prefix)
 rename_cache_entry_at(pos, dst);
 }

 +   if (gitmodules_modified)
 +   stage_updated_gitmodules();
 +

I'm unhappy with this, but we I'll have to live with my
dissatisfaction until we get rid of .gitmodules (if that ever
happens).  mv has a clear task: it should perform a filesystem mv,
manipulate the index, and write out the changed index.  Adding an
unrelated file to the index has nothing to do with its primary task.

 diff --git a/submodule.c b/submodule.c
 index eba9b42..fb742b4 100644
 --- a/submodule.c
 +++ b/submodule.c

All my comments in the rm review apply here too, so I won't repeat myself.

 @@ -10,6 +10,7 @@
  #include string-list.h
  #include sha1-array.h
  #include argv-array.h
 +#include blob.h

Interesting.  Let's see why you need blob.h.

  static struct string_list config_name_for_path;
  static struct string_list config_fetch_recurse_submodules_for_name;
 @@ -30,6 +31,67 @@ static struct sha1_array ref_tips_after_fetch;
   */
  static int gitmodules_is_unmerged;

 +/*
 + * Try to update the path entry in the submodule.name section of the
 + * .gitmodules file.
 + */
 +int update_path_in_gitmodules(const char *oldpath, const char *newpath)
 +{
 +   struct strbuf entry = STRBUF_INIT;
 +   struct string_list_item *path_option;
 +
 +   if (!file_exists(.gitmodules)) /* Do nothing whithout .gitmodules */
 +   return -1;

s/whithout/without.  Why are you not returning an error()?  Don't you
want to tell the user that no .gitmodules was found?

 +void stage_updated_gitmodules(void)

void return?  Don't you want the caller to know whether we were
successful or not?

Why does the name contain updated?  Why does the function care if I
updated the .gitmodules or not?  It's just staging .gitmodules, as it
is on the filesystem.

 +{
 +   struct strbuf buf = STRBUF_INIT;
 +   struct stat st;
 +   int pos;
 +   struct cache_entry *ce;
 +   int namelen = strlen(.gitmodules);
 +
 +   pos = cache_name_pos(.gitmodules, strlen(.gitmodules));

And you forgot about namelen just like that.

 +   if (pos  0) {
 +   warning(_(could not find .gitmodules in index));
 +   return;
 +   }

What could this mean?  I might have an untracked .gitmodules file in
my worktree, or I might not have a .gitmodules file at all.  Since
you're already checking the latter case in the previous function,
can't you persist the result, and return a more sensible error?

 +   ce = active_cache[pos];
 +   ce-ce_flags = namelen;
 +   if (strbuf_read_file(buf, .gitmodules, 0)  0)
 +   die(_(reading updated .gitmodules failed));

You're reading it because?  What does _updated_ .gitmodules mean here?

 +   if (lstat(.gitmodules, st)  0)
 +   die_errno(_(unable to stat updated .gitmodules));
 +   fill_stat_cache_info(ce, st);
 +   ce-ce_mode = ce_mode_from_stat(ce, st.st_mode);

I don't understand why you're taking the pains to fill out a cache_entry.

 +   if 

Re: Fixing typos

2013-04-11 Thread Ramkumar Ramachandra
Benoit Bourbie wrote:
 I apologize for being picky

There's nothing to apologize for.  These are good catches worth fixing.

 but that patch fixes 3 typos.

Please read Documentation/SubmittingPatches, particularly the parts about:
1. Sending patches inline using git send-email, as opposed to an attachment.
2. Signing off on your patch.

Cheers.
--
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: Git crash in Ubuntu 12.04

2013-04-11 Thread Sivaram Kannan
Hi,

Thanks for the reply.

 Can you tell us what command you ran, and also try to get a readable
 backtrace from your installation?


The crash is happening only when the users are trying to do a clone. I
was monitoring from the htop when triggering a clone operation, all
the cores of the processor hits 100% for some time before dropping.
The servers hw config is Intel Quad core processor with 8GB Ram.

 It seems that the paste would have contained a core dump (you snipped
 it9, but it would be pretty useless without the corresponding binary
 anyway.  Once you have the coredump in hand (as a file) you can use

   gdb $(which git) the_coredump_file

 and then in the GDB prompt, enter 'backtrace' and paste its output, to
 give us an idea what is going on.


Output of coredump gdb:

gitadmin@gitserver:/var/crash/dump$ gdb git CoreDump
GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04
Copyright (C) 2012 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later http://gnu.org/licenses/gpl.html
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type show copying
and show warranty for details.
This GDB was configured as x86_64-linux-gnu.
For bug reporting instructions, please see:
http://bugs.launchpad.net/gdb-linaro/...
Reading symbols from /usr/bin/git...(no debugging symbols found)...done.
BFD: Warning: /var/crash/dump/CoreDump is truncated: expected core
file size = 600195072, found: 1114112.
[New LWP 17584]
[New LWP 17587]
[New LWP 17586]
[New LWP 17440]
[New LWP 17585]
Cannot access memory at address 0x7f193623e2a8
Cannot access memory at address 0x7f193623e2a0
(gdb) bt
#0  0x004820f0 in ?? ()
Cannot access memory at address 0x7f191073bda8
(gdb)


=

The following I am copy pasting from the file called ProcMaps, which
gets created after I run apport-unpack crashfile.

0040-00541000 r-xp  fc:00 2098029
  /usr/lib/git-core/git
 00741000-00742000 r--p 00141000 fc:00 2098029
   /usr/lib/git-core/git
 00742000-00749000 rw-p 00142000 fc:00 2098029
   /usr/lib/git-core/git
 00749000-00799000 rw-p  00:00 0
 01c13000-01ecf000 rw-p  00:00 0  [heap]
 7f1ad57c-7f1ae000 rw-p  00:00 0
 7f1ae000-7f1ae0ed4000 rw-p  00:00 0
 7f1ae0ed4000-7f1ae400 ---p  00:00 0
 7f1ae400-7f1ae665f000 rw-p  00:00 0
 7f1ae665f000-7f1ae800 ---p  00:00 0
 7f1ae800-7f1aea91a000 rw-p  00:00 0
 7f1aea91a000-7f1aec00 ---p  00:00 0
 7f1aec00-7f1aefede000 rw-p  00:00 0
 7f1aefede000-7f1af000 ---p  00:00 0
 7f1af000-7f1af3b95000 rw-p  00:00 0
 7f1af3b95000-7f1af400 ---p  00:00 0
 7f1af414b000-7f1af5eaf000 rw-p  00:00 0
 7f1af6c4d000-7f1af6ffe000 rw-p  00:00 0
 7f1af6ffe000-7f1af6fff000 ---p  00:00 0
 7f1af6fff000-7f1af77ff000 rw-p  00:00 0
 7f1af77ff000-7f1af780 ---p  00:00 0
 7f1af780-7f1af800 rw-p  00:00 0
 7f1af800-7f1afb9cf000 rw-p  00:00 0
 7f1afb9cf000-7f1afc00 ---p  00:00 0
 7f1afc113000-7f1afc114000 ---p  00:00 0
 7f1afc114000-7f1afc914000 rw-p  00:00 0
 7f1afc914000-7f1afc915000 ---p  00:00 0
 7f1afc915000-7f1afd115000 rw-p  00:00 0
 7f1afd115000-7f1afe147000 r--p  fc:00 13371109
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-a72ab4cf996276ca527a26c6711414e97828aa01.pack
 7f1afe147000-7f1afe52d000 rw-p  00:00 0
 7f1afe8ad000-7f1afe92e000 rw-p  00:00 0
 7f1afe92e000-7f1afea57000 r--p  fc:00 13371175
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-725eb21535f4d3782dce7c1f163c535ae46dd7a3.pack
 7f1afea57000-7f1afeaab000 r--p  fc:00 13371183
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-a242e26dd75d9f94ac7cc021b79b5a23be86f772.pack
 7f1afeaab000-7f1afeb2d000 r--p  fc:00 13371188
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-8df0b1d878ce57e7b81439bfbbed75da58298d4f.pack
 7f1afeb2d000-7f1afed1c000 r--p  fc:00 13371200
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-e7279a5e263e36487664846efd3cf8e326e28b52.pack
 7f1afed1c000-7f1afee48000 r--p  fc:00 13371324
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-b9440d0950347a7f42994eecb3e4b96ac6cba2ef.pack
 7f1afee48000-7f1afef23000 r--p  fc:00 13371272
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-334b5d2e30e802e8d11095d64348c61a6fabf4ab.pack
 7f1afef23000-7f1b0028 r--p  fc:00 13371511
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-89264699af6b413cbcba76e68866a9531693dc2e.pack
 7f1b0028-7f1b00391000 r--p  fc:00 13372427
   
/home/git/repositories/sggroup/storegrid.git/objects/pack/pack-c7d9d3af8fb96dd231186fcdb08c451e0aba3cc2.pack
 

Re: [PATCH v2] cherry-pick: make sure all input objects are commits

2013-04-11 Thread Ramkumar Ramachandra
Miklos Vajna wrote:
 When a single argument was a non-commit, the error message used to be:

 fatal: BUG: expected exactly one commit from walk

 For multiple arguments, when none of the arguments was a commit, the error 
 was:

 fatal: empty commit set passed

 Finally, when some of the arguments were non-commits, we ignored those
 arguments.  Instead, now make sure all arguments are commits, and for
 the first non-commit, error out with:

 fatal: name: Can't cherry-pick a type

Thanks.  This is worth fixing.

 diff --git a/sequencer.c b/sequencer.c
 index baa0310..eb25101 100644
 --- a/sequencer.c
 +++ b/sequencer.c
 @@ -1067,6 +1068,18 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 if (opts-subcommand == REPLAY_CONTINUE)
 return sequencer_continue(opts);

 +   for (i = 0; i  opts-revs-pending.nr; i++) {
 +   unsigned char sha1[20];
 +   const char *name = opts-revs-pending.objects[i].name;
 +
 +   if (!get_sha1(name, sha1)) {
 +   enum object_type type = sha1_object_info(sha1, NULL);
 +
 +   if (type  0  type != OBJ_COMMIT)
 +   die(_(%s: can't cherry-pick a %s), name, 
 typename(type));
 +   }

else?  What happens if get_sha1() fails?

 diff --git a/t/t3508-cherry-pick-many-commits.sh 
 b/t/t3508-cherry-pick-many-commits.sh
 index 4e7136b..19c99d7 100755
 --- a/t/t3508-cherry-pick-many-commits.sh
 +++ b/t/t3508-cherry-pick-many-commits.sh
 @@ -55,6 +55,12 @@ one
  two
  '

 +test_expect_success 'cherry-pick three one two: fails' '
 +   git checkout -f master 
 +   git reset --hard first 
 +   test_must_fail git cherry-pick three one two:
 +'

So you're testing just the third case (where commit objects are mixed
with non-commit objects), which is arguably a bug.  Okay.
--
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 log -p unexpected behaviour - security risk?

2013-04-11 Thread John Tapsell
Hi,

  I noticed that code that you put in merge will not be visible by
default.  This seems like a pretty horrible security problem, no?

I made the following test tree, with just 3 commits:

https://github.com/johnflux/ExampleEvilness.git

Doing git log -p  shows all very innocent commits.  Completely
hidden is the change to add EVIL CODE MUWHAHAHA.

This seems really dangerous!

The evil code only shows up with the non-default  --cc or -m  option.

Is there a way to make --cc default?

John
--
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/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Adam Spiers
On Thu, Apr 11, 2013 at 01:31:45AM -0400, Jeff King wrote:
 On Thu, Apr 11, 2013 at 02:59:32AM +0100, Adam Spiers wrote:
 
  @@ -111,14 +110,11 @@ static int check_ignore_stdin_paths(struct 
  path_exclude_check check, const char
  die(line is badly quoted);
  strbuf_swap(buf, nbuf);
  }
  -   ALLOC_GROW(pathspec, nr + 1, alloc);
  -   pathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));
  -   strcpy(pathspec[nr++], buf.buf);
  +   pathspec[0] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));
  +   strcpy(pathspec[0], buf.buf);
  +   num_ignored += check_ignore(check, prefix, (const char 
  **)pathspec);
  +   maybe_flush_or_die(stdout, check-ignore to stdout);
 
 Now that you are not storing the whole pathspec at once, the pathspec
 buffer only needs to be valid for the length of check_ignore, right?
 That means you can drop this extra copy and just pass in buf.buf:
 
   pathspec[0] = buf.buf;
   num_ignored += check_ignore(check, prefix, pathspec);

Oops, good point - thanks.  I've made that change.

  +test_expect_success 'setup: have stdbuf?' '
  +   if which stdbuf /dev/null 21
  +   then
  +   test_set_prereq STDBUF
  +   fi
  +'
 
 Hmm. Today I learned about stdbuf. :)

Yeah, it's a relatively recent addition to coreutils.
--
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] cherry-pick: make sure all input objects are commits

2013-04-11 Thread Miklos Vajna
On Thu, Apr 11, 2013 at 03:52:44PM +0530, Ramkumar Ramachandra 
artag...@gmail.com wrote:
  +   for (i = 0; i  opts-revs-pending.nr; i++) {
  +   unsigned char sha1[20];
  +   const char *name = opts-revs-pending.objects[i].name;
  +
  +   if (!get_sha1(name, sha1)) {
  +   enum object_type type = sha1_object_info(sha1, 
  NULL);
  +
  +   if (type  0  type != OBJ_COMMIT)
  +   die(_(%s: can't cherry-pick a %s), name, 
  typename(type));
  +   }
 
 else?  What happens if get_sha1() fails?

I guess that is a should-not-happen category. parse_args() calls
setup_revisions(), and that will already die() if the argument is not a
valid object at all.

  diff --git a/t/t3508-cherry-pick-many-commits.sh 
  b/t/t3508-cherry-pick-many-commits.sh
  index 4e7136b..19c99d7 100755
  --- a/t/t3508-cherry-pick-many-commits.sh
  +++ b/t/t3508-cherry-pick-many-commits.sh
  @@ -55,6 +55,12 @@ one
   two
   '
 
  +test_expect_success 'cherry-pick three one two: fails' '
  +   git checkout -f master 
  +   git reset --hard first 
  +   test_must_fail git cherry-pick three one two:
  +'
 
 So you're testing just the third case (where commit objects are mixed
 with non-commit objects), which is arguably a bug.  Okay.

Yes. If you would want, I could of course add test cases for two other
cases when we already errored out and now the error message is just
changed, but I don't think duplicating the error message strings from
the code to the testsuite is really wanted. :-)


signature.asc
Description: Digital signature


Re: [PATCH 1/5] check-ignore: move setup into cmd_check_ignore()

2013-04-11 Thread Adam Spiers
On Thu, Apr 11, 2013 at 01:25:53AM -0400, Jeff King wrote:
 On Thu, Apr 11, 2013 at 02:59:31AM +0100, Adam Spiers wrote:
  -static int check_ignore(const char *prefix, const char **pathspec)
  +static int check_ignore(struct path_exclude_check check,
  +   const char *prefix, const char **pathspec)
 
 Did you mean to pass the struct by value here? If it is truly a per-path
 value, shouldn't it be declared and initialized inside here? Otherwise
 you risk one invocation munging things that the struct points to, but
 the caller's copy does not know about the change.
 
 In particular, I see that the struct includes a strbuf. What happens
 when one invocation of check_ignore grows the strbuf, then returns? The
 copy of the struct in the caller will not know that the buffer it is
 pointing to is now bogus.
 
  -static int check_ignore_stdin_paths(const char *prefix)
  +static int check_ignore_stdin_paths(struct path_exclude_check check, const 
  char *prefix)
 
 Ditto here.

It's not a per-path value; it's supposed to be reused across checks
for multiple paths, as explained in the comments above
last_exclude_matching_path():

...
 * A path to a directory known to be excluded is left in check-path to
 * optimize for repeated checks for files in the same excluded directory.
 */
struct exclude *last_exclude_matching_path(struct path_exclude_check *check,
...

So I think you're probably right that there is potential for
check-path to become effectively corrupted due to the caller not
seeing the reallocation.  I'll change this too.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Adam Spiers
On Thu, Apr 11, 2013 at 02:59:32AM +0100, Adam Spiers wrote:
 +test_expect_success STDBUF 'streaming support for --stdin' '
 + (
 + echo one
 + sleep 2
 + echo two
 + ) | stdbuf -oL git check-ignore -v -n --stdin out 

I just noticed that this patch precedes the one in the same series
which adds -n support.  I'll reorder them accordingly to avoid
breaking git bisect.
--
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] cherry-pick: make sure all input objects are commits

2013-04-11 Thread Ramkumar Ramachandra
Miklos Vajna wrote:
 I guess that is a should-not-happen category. parse_args() calls
 setup_revisions(), and that will already die() if the argument is not a
 valid object at all.

Then why do you have an if() guarding the code?  In my opinion, you
should have an else-clause that die()s with an appropriate message.

 Yes. If you would want, I could of course add test cases for two other
 cases when we already errored out and now the error message is just
 changed, but I don't think duplicating the error message strings from
 the code to the testsuite is really wanted. :-)

Nope, I'd never suggest that: this is fine.  What I meant is: you
should clarify that you're fixing a bug and adding a test to guard it,
in the commit message.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/5] t0008: remove duplicated test fixture data

2013-04-11 Thread Adam Spiers
The expected contents of STDOUT for the final --stdin tests can be
derived from the expected contents of STDOUT for the same tests when
--verbose is given, in the same way that test_expect_success_multi
derives this for earlier tests.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 t/t0008-ignores.sh | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 9c1bde1..314a86d 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -575,21 +575,6 @@ cat -\EOF stdin
b/globaltwo
../b/globaltwo
 EOF
-cat -\EOF expected-default
-   ../one
-   one
-   b/on
-   b/one
-   b/one one
-   b/one two
-   b/one\three
-   b/two
-   b/twooo
-   ../globaltwo
-   globaltwo
-   b/globaltwo
-   ../b/globaltwo
-EOF
 cat -EOF expected-verbose
.gitignore:1:one../one
.gitignore:1:oneone
@@ -605,6 +590,7 @@ cat -EOF expected-verbose
$global_excludes:2:!globaltwo   b/globaltwo
$global_excludes:2:!globaltwo   ../b/globaltwo
 EOF
+sed -e 's/.*   //' expected-verbose expected-default
 
 sed -e 's/^//' -e 's/\\//' -e 's/$//' stdin | \
tr \n \0 stdin0
-- 
1.8.2.1.342.gfa7285d

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


[PATCH v2 3/5] check-ignore: move setup into cmd_check_ignore()

2013-04-11 Thread Adam Spiers
Initialisation of the dir_struct and path_exclude_check structs was
previously done within check_ignore().  This was acceptable since
check_ignore() was only called once per check-ignore invocation;
however the next commit will convert it into an inner loop which is
called once per line of STDIN when --stdin is given.  Therefore moving
the initialisation code out into cmd_check_ignore() ensures that
initialisation is still only performed once per check-ignore
invocation, and consequently that the output is identical whether
pathspecs are provided as CLI arguments or via STDIN.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 builtin/check-ignore.c | 41 +
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 59acf74..e2d3006 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -63,30 +63,20 @@ static void output_exclude(const char *path, struct exclude 
*exclude)
}
 }
 
-static int check_ignore(const char *prefix, const char **pathspec)
+static int check_ignore(struct path_exclude_check *check,
+   const char *prefix, const char **pathspec)
 {
-   struct dir_struct dir;
const char *path, *full_path;
char *seen;
int num_ignored = 0, dtype = DT_UNKNOWN, i;
-   struct path_exclude_check check;
struct exclude *exclude;
 
-   /* read_cache() is only necessary so we can watch out for submodules. */
-   if (read_cache()  0)
-   die(_(index file corrupt));
-
-   memset(dir, 0, sizeof(dir));
-   dir.flags |= DIR_COLLECT_IGNORED;
-   setup_standard_excludes(dir);
-
if (!pathspec || !*pathspec) {
if (!quiet)
fprintf(stderr, no pathspec given.\n);
return 0;
}
 
-   path_exclude_check_init(check, dir);
/*
 * look for pathspecs matching entries in the index, since these
 * should not be ignored, in order to be consistent with
@@ -101,7 +91,7 @@ static int check_ignore(const char *prefix, const char 
**pathspec)
die_if_path_beyond_symlink(full_path, prefix);
exclude = NULL;
if (!seen[i]) {
-   exclude = last_exclude_matching_path(check, full_path,
+   exclude = last_exclude_matching_path(check, full_path,
 -1, dtype);
}
if (!quiet  (exclude || show_non_matching))
@@ -110,13 +100,11 @@ static int check_ignore(const char *prefix, const char 
**pathspec)
num_ignored++;
}
free(seen);
-   clear_directory(dir);
-   path_exclude_check_clear(check);
 
return num_ignored;
 }
 
-static int check_ignore_stdin_paths(const char *prefix)
+static int check_ignore_stdin_paths(struct path_exclude_check *check, const 
char *prefix)
 {
struct strbuf buf, nbuf;
char **pathspec = NULL;
@@ -139,17 +127,18 @@ static int check_ignore_stdin_paths(const char *prefix)
}
ALLOC_GROW(pathspec, nr + 1, alloc);
pathspec[nr] = NULL;
-   num_ignored = check_ignore(prefix, (const char **)pathspec);
+   num_ignored = check_ignore(check, prefix, (const char **)pathspec);
maybe_flush_or_die(stdout, attribute to stdout);
strbuf_release(buf);
strbuf_release(nbuf);
-   free(pathspec);
return num_ignored;
 }
 
 int cmd_check_ignore(int argc, const char **argv, const char *prefix)
 {
int num_ignored;
+   struct dir_struct dir;
+   struct path_exclude_check check;
 
git_config(git_default_config, NULL);
 
@@ -174,12 +163,24 @@ int cmd_check_ignore(int argc, const char **argv, const 
char *prefix)
if (show_non_matching  !verbose)
die(_(--non-matching is only valid with --verbose));
 
+   /* read_cache() is only necessary so we can watch out for submodules. */
+   if (read_cache()  0)
+   die(_(index file corrupt));
+
+   memset(dir, 0, sizeof(dir));
+   dir.flags |= DIR_COLLECT_IGNORED;
+   setup_standard_excludes(dir);
+
+   path_exclude_check_init(check, dir);
if (stdin_paths) {
-   num_ignored = check_ignore_stdin_paths(prefix);
+   num_ignored = check_ignore_stdin_paths(check, prefix);
} else {
-   num_ignored = check_ignore(prefix, argv);
+   num_ignored = check_ignore(check, prefix, argv);
maybe_flush_or_die(stdout, ignore to stdout);
}
 
+   clear_directory(dir);
+   path_exclude_check_clear(check);
+
return !num_ignored;
 }
-- 
1.8.2.1.342.gfa7285d

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


[PATCH v2 2/5] check-ignore: add -n / --non-matching option

2013-04-11 Thread Adam Spiers
If `-n` or `--non-matching` are specified, non-matching pathnames will
also be output, in which case all fields in each output record except
for pathname will be empty.  This can be useful when running
check-ignore as a background process, so that files can be
incrementally streamed to STDIN, and for each of these files, STDOUT
will indicate whether that file matched a pattern or not.  (Without
this option, it would be impossible to tell whether the absence of
output for a given file meant that it didn't match any pattern, or
that the result simply hadn't been flushed to STDOUT yet.)

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/git-check-ignore.txt |  15 +
 builtin/check-ignore.c |  46 --
 t/t0008-ignores.sh | 122 +++--
 3 files changed, 134 insertions(+), 49 deletions(-)

diff --git a/Documentation/git-check-ignore.txt 
b/Documentation/git-check-ignore.txt
index 854e4d0..7e3cabc 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -39,6 +39,12 @@ OPTIONS
below).  If `--stdin` is also given, input paths are separated
with a NUL character instead of a linefeed character.
 
+-n, --non-matching::
+   Show given paths which don't match any pattern.  This only
+   makes sense when `--verbose` is enabled, otherwise it would
+   not be possible to distinguish between paths which match a
+   pattern and those which don't.
+
 OUTPUT
 --
 
@@ -65,6 +71,15 @@ are also used instead of colons and hard tabs:
 
 source NULL linenum NULL pattern NULL pathname NULL
 
+If `-n` or `--non-matching` are specified, non-matching pathnames will
+also be output, in which case all fields in each output record except
+for pathname will be empty.  This can be useful when running
+non-interactively, so that files can be incrementally streamed to
+STDIN of a long-running check-ignore process, and for each of these
+files, STDOUT will indicate whether that file matched a pattern or
+not.  (Without this option, it would be impossible to tell whether the
+absence of output for a given file meant that it didn't match any
+pattern, or that the output hadn't been generated yet.)
 
 EXIT STATUS
 ---
diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index 0240f99..59acf74 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -5,7 +5,7 @@
 #include pathspec.h
 #include parse-options.h
 
-static int quiet, verbose, stdin_paths;
+static int quiet, verbose, stdin_paths, show_non_matching;
 static const char * const check_ignore_usage[] = {
 git check-ignore [options] pathname...,
 git check-ignore [options] --stdin  list-of-paths,
@@ -22,21 +22,28 @@ static const struct option check_ignore_options[] = {
N_(read file names from stdin)),
OPT_BOOLEAN('z', NULL, null_term_line,
N_(input paths are terminated by a null character)),
+   OPT_BOOLEAN('n', non-matching, show_non_matching,
+   N_(show non-matching input paths)),
OPT_END()
 };
 
 static void output_exclude(const char *path, struct exclude *exclude)
 {
-   char *bang  = exclude-flags  EXC_FLAG_NEGATIVE  ? ! : ;
-   char *slash = exclude-flags  EXC_FLAG_MUSTBEDIR ? / : ;
+   char *bang  = (exclude  exclude-flags  EXC_FLAG_NEGATIVE)  ? ! : 
;
+   char *slash = (exclude  exclude-flags  EXC_FLAG_MUSTBEDIR) ? / : 
;
if (!null_term_line) {
if (!verbose) {
write_name_quoted(path, stdout, '\n');
} else {
-   quote_c_style(exclude-el-src, NULL, stdout, 0);
-   printf(:%d:%s%s%s\t,
-  exclude-srcpos,
-  bang, exclude-pattern, slash);
+   if (exclude) {
+   quote_c_style(exclude-el-src, NULL, stdout, 
0);
+   printf(:%d:%s%s%s\t,
+  exclude-srcpos,
+  bang, exclude-pattern, slash);
+   }
+   else {
+   printf(::\t);
+   }
quote_c_style(path, NULL, stdout, 0);
fputc('\n', stdout);
}
@@ -44,11 +51,14 @@ static void output_exclude(const char *path, struct exclude 
*exclude)
if (!verbose) {
printf(%s%c, path, '\0');
} else {
-   printf(%s%c%d%c%s%s%s%c%s%c,
-  exclude-el-src, '\0',
-  exclude-srcpos, '\0',
-  bang, exclude-pattern, slash, '\0',
-  path, '\0');
+   if (exclude)
+   printf(%s%c%d%c%s%s%s%c%s%c,
+   

[PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Adam Spiers
Some callers, such as the git-annex web assistant, find it useful to
invoke git check-ignore as a persistent background process, which can
then have queries fed to its STDIN at any point, and the corresponding
response consumed from its STDOUT.  For this we need to invoke
check_ignore() once per line of standard input, and flush standard
output after each result.

The above use case suggests that empty STDIN is actually a reasonable
scenario (e.g. when the caller doesn't know in advance whether any
queries need to be fed to the background process until after it's
already started), so we make the minor behavioural change that no
pathspec given. is no longer emitted in when STDIN is empty.

Even though check_ignore() could now be changed to operate on a single
pathspec, we keep it operating on an array of pathspecs since that is
a more convenient way of consuming the existing pathspec API.

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 builtin/check-ignore.c | 15 +--
 t/t0008-ignores.sh | 28 +++-
 2 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
index e2d3006..c00a7d6 100644
--- a/builtin/check-ignore.c
+++ b/builtin/check-ignore.c
@@ -107,10 +107,9 @@ static int check_ignore(struct path_exclude_check *check,
 static int check_ignore_stdin_paths(struct path_exclude_check *check, const 
char *prefix)
 {
struct strbuf buf, nbuf;
-   char **pathspec = NULL;
-   size_t nr = 0, alloc = 0;
+   char *pathspec[2] = { NULL, NULL };
int line_termination = null_term_line ? 0 : '\n';
-   int num_ignored;
+   int num_ignored = 0;
 
strbuf_init(buf, 0);
strbuf_init(nbuf, 0);
@@ -121,14 +120,10 @@ static int check_ignore_stdin_paths(struct 
path_exclude_check *check, const char
die(line is badly quoted);
strbuf_swap(buf, nbuf);
}
-   ALLOC_GROW(pathspec, nr + 1, alloc);
-   pathspec[nr] = xcalloc(strlen(buf.buf) + 1, sizeof(*buf.buf));
-   strcpy(pathspec[nr++], buf.buf);
+   pathspec[0] = buf.buf;
+   num_ignored += check_ignore(check, prefix, (const char 
**)pathspec);
+   maybe_flush_or_die(stdout, check-ignore to stdout);
}
-   ALLOC_GROW(pathspec, nr + 1, alloc);
-   pathspec[nr] = NULL;
-   num_ignored = check_ignore(check, prefix, (const char **)pathspec);
-   maybe_flush_or_die(stdout, attribute to stdout);
strbuf_release(buf);
strbuf_release(nbuf);
return num_ignored;
diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
index 7af93ba..fbf12ae 100755
--- a/t/t0008-ignores.sh
+++ b/t/t0008-ignores.sh
@@ -216,11 +216,7 @@ test_expect_success_multi 'empty command line' '' '
 
 test_expect_success_multi '--stdin with empty STDIN' '' '
test_check_ignore --stdin 1 /dev/null 
-   if test -n $quiet_opt; then
-   test_stderr 
-   else
-   test_stderr no pathspec given.
-   fi
+   test_stderr 
 '
 
 test_expect_success '-q with multiple args' '
@@ -692,5 +688,27 @@ do
'
 done
 
+test_expect_success 'setup: have stdbuf?' '
+   if which stdbuf /dev/null 21
+   then
+   test_set_prereq STDBUF
+   fi
+'
+
+test_expect_success STDBUF 'streaming support for --stdin' '
+   (
+   echo one
+   sleep 2
+   echo two
+   ) | stdbuf -oL git check-ignore -v -n --stdin out 
+   pid=$! 
+   sleep 1 
+   grep ^\.gitignore:1:oneone out 
+   test $( wc -l out ) = 1 
+   sleep 2 
+   grep ^::   two out 
+   test $( wc -l out ) = 2 
+   ( wait $pid || kill $pid || : ) 2/dev/null
+'
 
 test_done
-- 
1.8.2.1.342.gfa7285d

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


[PATCH v2 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}

2013-04-11 Thread Adam Spiers
check-attr and check-ignore have the potential to deadlock callers
which do not read back the output in real-time.  For example, if a
caller writes N paths out and then reads N lines back in, it risks
becoming blocked on write() to check-*, and check-* is blocked on
write back to the caller.  Somebody has to buffer; the pipe buffers
provide some leeway, but they are limited.

Thanks to Peff for pointing this out:

http://article.gmane.org/gmane.comp.version-control.git/220534

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/git-check-attr.txt   |  5 +
 Documentation/git-check-ignore.txt |  5 +
 Documentation/git.txt  | 16 +---
 3 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 5abdbaa..a7be80d 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -56,6 +56,11 @@ being queried and info can be either:
 'set';;when the attribute is defined as true.
 value;;  when a value has been assigned to the attribute.
 
+Buffering happens as documented under the `GIT_FLUSH` option in
+linkgit:git[1].  The caller is responsible for avoiding deadlocks
+caused by overfilling an input buffer or reading from an empty output
+buffer.
+
 EXAMPLES
 
 
diff --git a/Documentation/git-check-ignore.txt 
b/Documentation/git-check-ignore.txt
index 7e3cabc..8e1f7ab 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -81,6 +81,11 @@ not.  (Without this option, it would be impossible to tell 
whether the
 absence of output for a given file meant that it didn't match any
 pattern, or that the output hadn't been generated yet.)
 
+Buffering happens as documented under the `GIT_FLUSH` option in
+linkgit:git[1].  The caller is responsible for avoiding deadlocks
+caused by overfilling an input buffer or reading from an empty output
+buffer.
+
 EXIT STATUS
 ---
 
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6a875f2..eecdb15 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -808,13 +808,15 @@ for further details.
 
 'GIT_FLUSH'::
If this environment variable is set to 1, then commands such
-   as 'git blame' (in incremental mode), 'git rev-list', 'git log',
-   and 'git whatchanged' will force a flush of the output stream
-   after each commit-oriented record have been flushed.   If this
-   variable is set to 0, the output of these commands will be done
-   using completely buffered I/O.   If this environment variable is
-   not set, Git will choose buffered or record-oriented flushing
-   based on whether stdout appears to be redirected to a file or not.
+   as 'git blame' (in incremental mode), 'git rev-list', 'git
+   log', 'git check-attr', 'git check-ignore', and 'git
+   whatchanged' will force a flush of the output stream after
+   each commit-oriented record have been flushed.  If this
+   variable is set to 0, the output of these commands will be
+   done using completely buffered I/O.  If this environment
+   variable is not set, Git will choose buffered or
+   record-oriented flushing based on whether stdout appears to be
+   redirected to a file or not.
 
 'GIT_TRACE'::
If this variable is set to 1, 2 or true (comparison
-- 
1.8.2.1.342.gfa7285d

--
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 v4 00/21] remote-hg: general updates

2013-04-11 Thread Felipe Contreras
Hi,

This is a reroll of the previous series due to a few minor issues. As the
previous version, forced pushes remain a configuration option. Also, I picked
up a fix for test regarding hg_log() that was sent to the mailing list.

Antoine Pelisse (1):
  remote-hg: activate graphlog extension for hg_log()

Dusty Phillips (2):
  remote-hg: add missing config variable in doc
  remote-hg: push to the appropriate branch

Felipe Contreras (15):
  remote-hg: trivial cleanups
  remote-hg: properly report errors on bookmark pushes
  remote-hg: make sure fake bookmarks are updated
  remote-hg: trivial test cleanups
  remote-hg: redirect buggy mercurial output
  remote-hg: split bookmark handling
  remote-hg: refactor export
  remote-hg: update remote bookmarks
  remote-hg: update tags globally
  remote-hg: force remote push
  remote-hg: show more proper errors
  remote-hg: add basic author tests
  remote-hg: add simple mail test
  remote-hg: fix bad state issue
  remote-hg: fix bad file paths

Peter van Zetten (1):
  remote-hg: fix for files with spaces

Simon Ruderich (2):
  remote-hg: add 'insecure' option
  remote-hg: document location of stored hg repository

 contrib/remote-helpers/git-remote-hg | 122 +--
 contrib/remote-helpers/test-hg-bidi.sh   |  11 ++-
 contrib/remote-helpers/test-hg-hg-git.sh |   8 +-
 contrib/remote-helpers/test-hg.sh|  36 +
 4 files changed, 148 insertions(+), 29 deletions(-)

-- 
1.8.2.1

--
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 v4 01/21] remote-hg: trivial cleanups

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 328c2dc..d0dfb1e 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -531,7 +531,6 @@ def parse_blob(parser):
 data = parser.get_data()
 blob_marks[mark] = data
 parser.next()
-return
 
 def get_merge_files(repo, p1, p2, files):
 for e in repo[p1].files():
@@ -542,7 +541,7 @@ def get_merge_files(repo, p1, p2, files):
 files[e] = f
 
 def parse_commit(parser):
-global marks, blob_marks, bmarks, parsed_refs
+global marks, blob_marks, parsed_refs
 global mode
 
 from_mark = merge_mark = None
@@ -647,10 +646,11 @@ def parse_commit(parser):
 rev = repo[node].rev()
 
 parsed_refs[ref] = node
-
 marks.new_mark(rev, commit_mark)
 
 def parse_reset(parser):
+global parsed_refs
+
 ref = parser[1]
 parser.next()
 # ugh
@@ -715,11 +715,11 @@ def do_export(parser):
 continue
 print ok %s % ref
 
-print
-
 if peer:
 parser.repo.push(peer, force=False)
 
+print
+
 def fix_path(alias, repo, orig_url):
 repo_url = util.url(repo.url())
 url = util.url(orig_url)
-- 
1.8.2.1

--
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 v4 02/21] remote-hg: add missing config variable in doc

2013-04-11 Thread Felipe Contreras
From: Dusty Phillips du...@linux.ca

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 4 
 1 file changed, 4 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index d0dfb1e..844ec50 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -23,6 +23,10 @@ import urllib
 # If you want to switch to hg-git compatibility mode:
 # git config --global remote-hg.hg-git-compat true
 #
+# If you are not in hg-git-compat mode and want to disable the tracking of
+# named branches:
+# git config --global remote-hg.track-branches false
+#
 # git:
 # Sensible defaults for git.
 # hg bookmarks are exported as git branches, hg branches are prefixed
-- 
1.8.2.1

--
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 v4 03/21] remote-hg: properly report errors on bookmark pushes

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 844ec50..19eb4db 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -710,6 +710,7 @@ def do_export(parser):
 else:
 old = ''
 if not bookmarks.pushbookmark(parser.repo, bmark, old, node):
+print error %s % ref
 continue
 elif ref.startswith('refs/tags/'):
 tag = ref[len('refs/tags/'):]
-- 
1.8.2.1

--
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 v4 04/21] remote-hg: fix for files with spaces

2013-04-11 Thread Felipe Contreras
From: Peter van Zetten peter.van.zet...@cgi.com

Set the maximum number of splits to make when dividing the diff stat
lines based on space characters.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 19eb4db..c6a1a47 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -578,7 +578,7 @@ def parse_commit(parser):
 mark = int(mark_ref[1:])
 f = { 'mode' : hgmode(m), 'data' : blob_marks[mark] }
 elif parser.check('D'):
-t, path = line.split(' ')
+t, path = line.split(' ', 1)
 f = { 'deleted' : True }
 else:
 die('Unknown file command: %s' % line)
@@ -625,7 +625,7 @@ def parse_commit(parser):
 i = data.find('\n--HG--\n')
 if i = 0:
 tmp = data[i + len('\n--HG--\n'):].strip()
-for k, v in [e.split(' : ') for e in tmp.split('\n')]:
+for k, v in [e.split(' : ', 1) for e in tmp.split('\n')]:
 if k == 'rename':
 old, new = v.split(' = ', 1)
 files[new]['rename'] = old
-- 
1.8.2.1

--
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 v4 05/21] remote-hg: make sure fake bookmarks are updated

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 7 +++
 contrib/remote-helpers/test-hg-bidi.sh   | 1 +
 contrib/remote-helpers/test-hg-hg-git.sh | 1 +
 3 files changed, 9 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index c6a1a47..b200e60 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -709,9 +709,16 @@ def do_export(parser):
 old = bmarks[bmark].hex()
 else:
 old = ''
+
+if bmark == 'master' and 'master' not in parser.repo._bookmarks:
+# fake bookmark
+print ok %s % ref
+continue
+
 if not bookmarks.pushbookmark(parser.repo, bmark, old, node):
 print error %s % ref
 continue
+
 elif ref.startswith('refs/tags/'):
 tag = ref[len('refs/tags/'):]
 parser.repo.tag([tag], node, None, True, None, {})
diff --git a/contrib/remote-helpers/test-hg-bidi.sh 
b/contrib/remote-helpers/test-hg-bidi.sh
index 1d61982..fe38e49 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -30,6 +30,7 @@ git_clone () {
 hg_clone () {
(
hg init $2 
+   hg -R $2 bookmark -i master 
cd $1 
git push -q hg::$PWD/../$2 'refs/tags/*:refs/tags/*' 
'refs/heads/*:refs/heads/*'
) 
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh 
b/contrib/remote-helpers/test-hg-hg-git.sh
index 3f253b7..e116cb0 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -35,6 +35,7 @@ git_clone_git () {
 hg_clone_git () {
(
hg init $2 
+   hg -R $2 bookmark -i master 
cd $1 
git push -q hg::$PWD/../$2 'refs/tags/*:refs/tags/*' 
'refs/heads/*:refs/heads/*'
) 
-- 
1.8.2.1

--
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 v4 06/21] remote-hg: trivial test cleanups

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/test-hg-bidi.sh   | 5 ++---
 contrib/remote-helpers/test-hg-hg-git.sh | 3 +--
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/contrib/remote-helpers/test-hg-bidi.sh 
b/contrib/remote-helpers/test-hg-bidi.sh
index fe38e49..a3c88f6 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -22,7 +22,6 @@ fi
 
 # clone to a git repo
 git_clone () {
-   hg -R $1 bookmark -f -r tip master 
git clone -q hg::$PWD/$1 $2
 }
 
@@ -201,8 +200,8 @@ test_expect_success 'hg branch' '
hg_push hgrepo gitrepo 
hg_clone gitrepo hgrepo2 
 
-   : TODO, avoid master bookmark 
-   (cd hgrepo2  hg checkout gamma) 
+   : Back to the common revision 
+   (cd hgrepo  hg checkout default) 
 
hg_log hgrepo  expected 
hg_log hgrepo2  actual 
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh 
b/contrib/remote-helpers/test-hg-hg-git.sh
index e116cb0..73ae18d 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -27,7 +27,6 @@ fi
 
 # clone to a git repo with git
 git_clone_git () {
-   hg -R $1 bookmark -f -r tip master 
git clone -q hg::$PWD/$1 $2
 }
 
@@ -48,7 +47,7 @@ git_clone_hg () {
(
git init -q $2 
cd $1 
-   hg bookmark -f -r tip master 
+   hg bookmark -i -f -r tip master 
hg -q push -r master ../$2 || true
)
 }
-- 
1.8.2.1

--
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 v4 08/21] remote-hg: split bookmark handling

2013-04-11 Thread Felipe Contreras
Will be useful for remote bookmarks.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 39 +++-
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 874ccd4..73cd812 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -685,6 +685,8 @@ def parse_tag(parser):
 def do_export(parser):
 global parsed_refs, bmarks, peer
 
+p_bmarks = []
+
 parser.next()
 
 for line in parser.each_block('done'):
@@ -706,20 +708,9 @@ def do_export(parser):
 pass
 elif ref.startswith('refs/heads/'):
 bmark = ref[len('refs/heads/'):]
-if bmark in bmarks:
-old = bmarks[bmark].hex()
-else:
-old = ''
-
-if bmark == 'master' and 'master' not in parser.repo._bookmarks:
-# fake bookmark
-print ok %s % ref
-continue
-
-if not bookmarks.pushbookmark(parser.repo, bmark, old, node):
-print error %s % ref
-continue
-
+p_bmarks.append((bmark, node))
+# handle below
+continue
 elif ref.startswith('refs/tags/'):
 tag = ref[len('refs/tags/'):]
 parser.repo.tag([tag], node, None, True, None, {})
@@ -731,6 +722,26 @@ def do_export(parser):
 if peer:
 parser.repo.push(peer, force=False)
 
+# handle bookmarks
+for bmark, node in p_bmarks:
+ref = 'refs/heads/' + bmark
+
+if bmark in bmarks:
+old = bmarks[bmark].hex()
+else:
+old = ''
+
+if bmark == 'master' and 'master' not in parser.repo._bookmarks:
+# fake bookmark
+print ok %s % ref
+continue
+
+if not bookmarks.pushbookmark(parser.repo, bmark, old, node):
+print error %s % ref
+continue
+
+print ok %s % ref
+
 print
 
 def fix_path(alias, repo, orig_url):
-- 
1.8.2.1

--
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 v4 09/21] remote-hg: refactor export

2013-04-11 Thread Felipe Contreras
No functional changes.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 73cd812..3ceec85 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -9,7 +9,7 @@
 # Then you can clone with:
 # git clone hg::/path/to/mercurial/repo/
 
-from mercurial import hg, ui, bookmarks, context, util, encoding
+from mercurial import hg, ui, bookmarks, context, util, encoding, node
 
 import re
 import sys
@@ -60,6 +60,9 @@ def hgmode(mode):
 m = { '100755': 'x', '12': 'l' }
 return m.get(mode, '')
 
+def hghex(node):
+return hg.node.hex(node)
+
 def get_config(config):
 cmd = ['git', 'config', '--get', config]
 process = subprocess.Popen(cmd, stdout=subprocess.PIPE)
@@ -705,19 +708,18 @@ def do_export(parser):
 
 for ref, node in parsed_refs.iteritems():
 if ref.startswith('refs/heads/branches'):
-pass
+print ok %s % ref
 elif ref.startswith('refs/heads/'):
 bmark = ref[len('refs/heads/'):]
 p_bmarks.append((bmark, node))
-# handle below
 continue
 elif ref.startswith('refs/tags/'):
 tag = ref[len('refs/tags/'):]
 parser.repo.tag([tag], node, None, True, None, {})
+print ok %s % ref
 else:
 # transport-helper/fast-export bugs
 continue
-print ok %s % ref
 
 if peer:
 parser.repo.push(peer, force=False)
@@ -725,6 +727,7 @@ def do_export(parser):
 # handle bookmarks
 for bmark, node in p_bmarks:
 ref = 'refs/heads/' + bmark
+new = hghex(node)
 
 if bmark in bmarks:
 old = bmarks[bmark].hex()
@@ -733,10 +736,11 @@ def do_export(parser):
 
 if bmark == 'master' and 'master' not in parser.repo._bookmarks:
 # fake bookmark
-print ok %s % ref
-continue
-
-if not bookmarks.pushbookmark(parser.repo, bmark, old, node):
+pass
+elif bookmarks.pushbookmark(parser.repo, bmark, old, new):
+# updated locally
+pass
+else:
 print error %s % ref
 continue
 
-- 
1.8.2.1

--
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 v4 10/21] remote-hg: update remote bookmarks

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 5 +
 1 file changed, 5 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 3ceec85..46cddc9 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -744,6 +744,11 @@ def do_export(parser):
 print error %s % ref
 continue
 
+if peer:
+if not peer.pushkey('bookmarks', bmark, old, new):
+print error %s % ref
+continue
+
 print ok %s % ref
 
 print
-- 
1.8.2.1

--
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 v4 11/21] remote-hg: update tags globally

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 46cddc9..fc04f81 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -715,7 +715,11 @@ def do_export(parser):
 continue
 elif ref.startswith('refs/tags/'):
 tag = ref[len('refs/tags/'):]
-parser.repo.tag([tag], node, None, True, None, {})
+if mode == 'git':
+msg = 'Added tag %s for changeset %s' % (tag, hghex(node[:6]));
+parser.repo.tag([tag], node, msg, False, None, {})
+else:
+parser.repo.tag([tag], node, None, True, None, {})
 print ok %s % ref
 else:
 # transport-helper/fast-export bugs
-- 
1.8.2.1

--
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 v4 12/21] remote-hg: push to the appropriate branch

2013-04-11 Thread Felipe Contreras
From: Dusty Phillips du...@linux.ca

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 4 
 1 file changed, 4 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index fc04f81..ec599c6 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -625,6 +625,10 @@ def parse_commit(parser):
 if merge_mark:
 get_merge_files(repo, p1, p2, files)
 
+# Check if the ref is supposed to be a named branch
+if ref.startswith('refs/heads/branches/'):
+extra['branch'] = ref[len('refs/heads/branches/'):]
+
 if mode == 'hg':
 i = data.find('\n--HG--\n')
 if i = 0:
-- 
1.8.2.1

--
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 v4 13/21] remote-hg: force remote push

2013-04-11 Thread Felipe Contreras
Ideally we shouldn't do this, as it's not recommended in mercurial
documentation, but there's no other way to push multiple bookmarks (on
the same branch), which would be the behavior most similar to git.

At the same time, add a configuration option for the people that don't
want to risk creating new remote heads.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index ec599c6..ff89725 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -27,6 +27,9 @@ import urllib
 # named branches:
 # git config --global remote-hg.track-branches false
 #
+# If you don't want to force pushes (and thus risk creating new remote heads):
+# git config --global remote-hg.force-push false
+#
 # git:
 # Sensible defaults for git.
 # hg bookmarks are exported as git branches, hg branches are prefixed
@@ -730,7 +733,7 @@ def do_export(parser):
 continue
 
 if peer:
-parser.repo.push(peer, force=False)
+parser.repo.push(peer, force=force_push)
 
 # handle bookmarks
 for bmark, node in p_bmarks:
@@ -773,7 +776,7 @@ def main(args):
 global prefix, dirname, branches, bmarks
 global marks, blob_marks, parsed_refs
 global peer, mode, bad_mail, bad_name
-global track_branches
+global track_branches, force_push
 
 alias = args[1]
 url = args[2]
@@ -781,12 +784,16 @@ def main(args):
 
 hg_git_compat = False
 track_branches = True
+force_push = True
+
 try:
 if get_config('remote-hg.hg-git-compat') == 'true\n':
 hg_git_compat = True
 track_branches = False
 if get_config('remote-hg.track-branches') == 'false\n':
 track_branches = False
+if get_config('remote-hg.force-push') == 'false\n':
+force_push = False
 except subprocess.CalledProcessError:
 pass
 
-- 
1.8.2.1

--
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 v4 14/21] remote-hg: show more proper errors

2013-04-11 Thread Felipe Contreras
When cloning or pushing fails, we don't want to show a stack-trace.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index ff89725..3ae3598 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -9,7 +9,7 @@
 # Then you can clone with:
 # git clone hg::/path/to/mercurial/repo/
 
-from mercurial import hg, ui, bookmarks, context, util, encoding, node
+from mercurial import hg, ui, bookmarks, context, util, encoding, node, error
 
 import re
 import sys
@@ -284,11 +284,17 @@ def get_repo(url, alias):
 else:
 local_path = os.path.join(dirname, 'clone')
 if not os.path.exists(local_path):
-peer, dstpeer = hg.clone(myui, {}, url, local_path, update=False, 
pull=True)
+try:
+peer, dstpeer = hg.clone(myui, {}, url, local_path, 
update=True, pull=True)
+except:
+die('Repository error')
 repo = dstpeer.local()
 else:
 repo = hg.repository(myui, local_path)
-peer = hg.peer(myui, {}, url)
+try:
+peer = hg.peer(myui, {}, url)
+except:
+die('Repository error')
 repo.pull(peer, heads=None, force=True)
 
 return repo
-- 
1.8.2.1

--
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 v4 15/21] remote-hg: add basic author tests

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/test-hg.sh | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 5f81dfa..62e3a47 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -118,4 +118,39 @@ test_expect_success 'update bookmark' '
   hg -R hgrepo bookmarks | grep devel\s\+3:
 '
 
+author_test () {
+  echo $1  content 
+  hg commit -u $2 -m add $1 
+  echo $3  ../expected
+}
+
+test_expect_success 'authors' '
+  mkdir -p tmp  cd tmp 
+  test_when_finished cd ..  rm -rf tmp 
+
+  (
+  hg init hgrepo 
+  cd hgrepo 
+
+  touch content 
+  hg add content 
+
+  author_test alpha  H G Wells we...@example.com 
+  author_test beta test test unknown 
+  author_test beta test t...@example.com (comment) test unknown 
+  author_test gamma t...@example.com Unknown t...@example.com 
+  author_test delta namet...@example.com name t...@example.com 
+  author_test epsilon name t...@example.com name unknown 
+  author_test zeta  test  test unknown 
+  author_test eta test  t...@example.com  test t...@example.com 
+  author_test theta test t...@example.com test unknown 
+  author_test iota test  test at example dot com test unknown
+  ) 
+
+  git clone hg::$PWD/hgrepo gitrepo 
+  git --git-dir=gitrepo/.git log --reverse --format=%an %ae  actual 
+
+  test_cmp expected actual
+'
+
 test_done
-- 
1.8.2.1

--
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 v4 16/21] remote-hg: add simple mail test

2013-04-11 Thread Felipe Contreras
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/test-hg.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-hg.sh 
b/contrib/remote-helpers/test-hg.sh
index 62e3a47..6a1e4b1 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -144,7 +144,8 @@ test_expect_success 'authors' '
   author_test zeta  test  test unknown 
   author_test eta test  t...@example.com  test t...@example.com 
   author_test theta test t...@example.com test unknown 
-  author_test iota test  test at example dot com test unknown
+  author_test iota test  test at example dot com test unknown 
+  author_test kappa t...@example.com t...@example.com unknown
   ) 
 
   git clone hg::$PWD/hgrepo gitrepo 
-- 
1.8.2.1

--
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 v4 18/21] remote-hg: fix bad state issue

2013-04-11 Thread Felipe Contreras
The problem reportedly happened after doing a push that fails, the abort
causes the state of remote-hg to go bad, this happens because
remote-hg's marks are not stored, but 'git fast-export' marks are.

Ensure that the marks are _always_ stored.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 3eb07dc..e3d7f77 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -18,6 +18,7 @@ import json
 import shutil
 import subprocess
 import urllib
+import atexit
 
 #
 # If you want to switch to hg-git compatibility mode:
@@ -791,7 +792,7 @@ def main(args):
 global prefix, dirname, branches, bmarks
 global marks, blob_marks, parsed_refs
 global peer, mode, bad_mail, bad_name
-global track_branches, force_push
+global track_branches, force_push, is_tmp
 
 alias = args[1]
 url = args[2]
@@ -833,6 +834,7 @@ def main(args):
 bmarks = {}
 blob_marks = {}
 parsed_refs = {}
+marks = None
 
 repo = get_repo(url, alias)
 prefix = 'refs/hg/%s' % alias
@@ -860,9 +862,13 @@ def main(args):
 die('unhandled command: %s' % line)
 sys.stdout.flush()
 
+def bye():
+if not marks:
+return
 if not is_tmp:
 marks.store()
 else:
 shutil.rmtree(dirname)
 
+atexit.register(bye)
 sys.exit(main(sys.argv))
-- 
1.8.2.1

--
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 v4 17/21] remote-hg: add 'insecure' option

2013-04-11 Thread Felipe Contreras
From: Simon Ruderich si...@ruderich.org

If set to true acts as hg's clone/pull --insecure option.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 9 +
 1 file changed, 9 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 3ae3598..3eb07dc 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -30,6 +30,9 @@ import urllib
 # If you don't want to force pushes (and thus risk creating new remote heads):
 # git config --global remote-hg.force-push false
 #
+# If you want the equivalent of hg's clone/pull--insecure option:
+# git config remote-hg.insecure true
+#
 # git:
 # Sensible defaults for git.
 # hg bookmarks are exported as git branches, hg branches are prefixed
@@ -279,6 +282,12 @@ def get_repo(url, alias):
 myui.setconfig('ui', 'interactive', 'off')
 myui.fout = sys.stderr
 
+try:
+if get_config('remote-hg.insecure') == 'true\n':
+myui.setconfig('web', 'cacerts', '')
+except subprocess.CalledProcessError:
+pass
+
 if hg.islocal(url):
 repo = hg.repository(myui, url)
 else:
-- 
1.8.2.1

--
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 v4 20/21] remote-hg: fix bad file paths

2013-04-11 Thread Felipe Contreras
Mercurial allows absolute file paths, and Git doesn't like that.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index 0db4cca..a5f0013 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -205,9 +205,15 @@ class Parser:
 tz = ((tz / 100) * 3600) + ((tz % 100) * 60)
 return (user, int(date), -tz)
 
+def fix_file_path(path):
+if not os.path.isabs(path):
+return path
+return os.path.relpath(path, '/')
+
 def export_file(fc):
 d = fc.data()
-print M %s inline %s % (gitmode(fc.flags()), fc.path())
+path = fix_file_path(fc.path())
+print M %s inline %s % (gitmode(fc.flags()), path)
 print data %d % len(d)
 print d
 
@@ -401,7 +407,7 @@ def export_ref(repo, name, kind, head):
 for f in modified:
 export_file(c.filectx(f))
 for f in removed:
-print D %s % (f)
+print D %s % (fix_file_path(f))
 print
 
 count += 1
-- 
1.8.2.1

--
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 v4 19/21] remote-hg: document location of stored hg repository

2013-04-11 Thread Felipe Contreras
From: Simon Ruderich si...@ruderich.org

Signed-off-by: Simon Ruderich si...@ruderich.org
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index e3d7f77..0db4cca 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -8,6 +8,9 @@
 # Just copy to your ~/bin, or anywhere in your $PATH.
 # Then you can clone with:
 # git clone hg::/path/to/mercurial/repo/
+#
+# For remote repositories a local clone is stored in
+# $GIT_DIR/hg/origin/clone/.hg/.
 
 from mercurial import hg, ui, bookmarks, context, util, encoding, node, error
 
-- 
1.8.2.1

--
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 v4 21/21] remote-hg: activate graphlog extension for hg_log()

2013-04-11 Thread Felipe Contreras
From: Antoine Pelisse apeli...@gmail.com

The hg_log() test helper uses the --graph parameter that is
implemented by the GraphLog extension. If the extension is not activated
by the user, the parameter is not available. Activate the extension in
setup().

Also changes the way we grep the output in hg_log(). The pipe operator
can hide the return code of hg command. As a matter of fact, if log
fails because it doesn't know about --graph, it doesn't report any
failure and let's you think everything worked.

Signed-off-by: Antoine Pelisse apeli...@gmail.com
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/test-hg-bidi.sh   | 5 -
 contrib/remote-helpers/test-hg-hg-git.sh | 4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/test-hg-bidi.sh 
b/contrib/remote-helpers/test-hg-bidi.sh
index a3c88f6..f368953 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
 }
 
 hg_log () {
-   hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+   hg -R $1 log --graph --debug log 
+   grep -v 'tag: *default/' log
 }
 
 setup () {
@@ -62,6 +63,8 @@ setup () {
echo commit = -d \0 0\
echo debugrawcommit = -d \0 0\
echo tag = -d \0 0\
+   echo [extensions]
+   echo graphlog =
)  $HOME/.hgrc 
git config --global remote-hg.hg-git-compat true
 
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh 
b/contrib/remote-helpers/test-hg-hg-git.sh
index 73ae18d..253e65a 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -78,7 +78,8 @@ hg_push_hg () {
 }
 
 hg_log () {
-   hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+   hg -R $1 log --graph --debug log 
+   grep -v 'tag: *default/' log
 }
 
 git_log () {
@@ -97,6 +98,7 @@ setup () {
echo [extensions]
echo hgext.bookmarks =
echo hggit =
+   echo graphlog =
)  $HOME/.hgrc 
git config --global receive.denycurrentbranch warn
git config --global remote-hg.hg-git-compat true
-- 
1.8.2.1

--
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 v4 07/21] remote-hg: redirect buggy mercurial output

2013-04-11 Thread Felipe Contreras
Mercurial emits messages like searching for changes, no changes
found, etc. meant for the use of its own UI layer, which break the pipe
between transport helper and remote helper.

Since there's no way to silence Mercurial, let's redirect to standard
error.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 contrib/remote-helpers/git-remote-hg | 1 +
 1 file changed, 1 insertion(+)

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index b200e60..874ccd4 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -271,6 +271,7 @@ def get_repo(url, alias):
 
 myui = ui.ui()
 myui.setconfig('ui', 'interactive', 'off')
+myui.fout = sys.stderr
 
 if hg.islocal(url):
 repo = hg.repository(myui, url)
-- 
1.8.2.1

--
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] cherry-pick: make sure all input objects are commits

2013-04-11 Thread Miklos Vajna
When a single argument was a non-commit, the error message used to be:

fatal: BUG: expected exactly one commit from walk

For multiple arguments, when none of the arguments was a commit, the error was:

fatal: empty commit set passed

Finally, when some of the arguments were non-commits, we ignored those
arguments.  Fix this bug and make sure all arguments are commits, and
for the first non-commit, error out with:

fatal: name: Can't cherry-pick a type

Signed-off-by: Miklos Vajna vmik...@suse.cz
---

On Thu, Apr 11, 2013 at 05:12:06PM +0530, Ramkumar Ramachandra 
artag...@gmail.com wrote:
 Then why do you have an if() guarding the code?  In my opinion, you
 should have an else-clause that die()s with an appropriate message.

And you were right -- I actually forgot about --stdin, where the 
else-clause is hit. Added that for now, excluding --stdin.

 Nope, I'd never suggest that: this is fine.  What I meant is: you
 should clarify that you're fixing a bug and adding a test to guard it,
 in the commit message.

Done.

 sequencer.c | 18 ++
 t/t3508-cherry-pick-many-commits.sh |  6 ++
 2 files changed, 24 insertions(+)

diff --git a/sequencer.c b/sequencer.c
index baa0310..61fdb68 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1047,6 +1047,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 {
struct commit_list *todo_list = NULL;
unsigned char sha1[20];
+   int i;
 
if (opts-subcommand == REPLAY_NONE)
assert(opts-revs);
@@ -1067,6 +1068,23 @@ int sequencer_pick_revisions(struct replay_opts *opts)
if (opts-subcommand == REPLAY_CONTINUE)
return sequencer_continue(opts);
 
+   for (i = 0; i  opts-revs-pending.nr; i++) {
+   unsigned char sha1[20];
+   const char *name = opts-revs-pending.objects[i].name;
+
+   /* This happens when using --stdin. */
+   if (!strlen(name))
+   continue;
+
+   if (!get_sha1(name, sha1)) {
+   enum object_type type = sha1_object_info(sha1, NULL);
+
+   if (type  0  type != OBJ_COMMIT)
+   die(_(%s: can't cherry-pick a %s), name, 
typename(type));
+   } else
+   die(_(%s: bad revision), name);
+   }
+
/*
 * If we were called as git cherry-pick commit, just
 * cherry-pick/revert it, set CHERRY_PICK_HEAD /
diff --git a/t/t3508-cherry-pick-many-commits.sh 
b/t/t3508-cherry-pick-many-commits.sh
index 4e7136b..19c99d7 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -55,6 +55,12 @@ one
 two
 '
 
+test_expect_success 'cherry-pick three one two: fails' '
+   git checkout -f master 
+   git reset --hard first 
+   test_must_fail git cherry-pick three one two:
+'
+
 test_expect_success 'output to keep user entertained during multi-pick' '
cat -\EOF expected 
[master OBJID] second
-- 
1.8.1.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


Re: Git crash in Ubuntu 12.04

2013-04-11 Thread Konstantin Khomoutov
On Thu, 11 Apr 2013 15:50:31 +0530
Sivaram Kannan siva.de...@gmail.com wrote:

[...]
 Output of coredump gdb:
 
 gitadmin@gitserver:/var/crash/dump$ gdb git CoreDump
 GNU gdb (Ubuntu/Linaro 7.4-2012.04-0ubuntu2.1) 7.4-2012.04
 Copyright (C) 2012 Free Software Foundation, Inc.
 License GPLv3+: GNU GPL version 3 or later
 http://gnu.org/licenses/gpl.html This is free software: you are
 free to change and redistribute it. There is NO WARRANTY, to the
 extent permitted by law.  Type show copying and show warranty for
 details. This GDB was configured as x86_64-linux-gnu.
 For bug reporting instructions, please see:
 http://bugs.launchpad.net/gdb-linaro/...
 Reading symbols from /usr/bin/git...(no debugging symbols
 found)...done. BFD: Warning: /var/crash/dump/CoreDump is truncated:
 expected core file size = 600195072, found: 1114112.

^^^ Try to issue the

$ ulimit -c unlimited

command in your shell before attempting the cloning -- this should
remove the upper limit on the core file size.  And try look for the core
file in the current directory after the crash occurs.  I'm not sure
Ubuntu's crash interceptor won't kick in, but just in case...
--
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] transport-helper: report errors properly

2013-04-11 Thread Felipe Contreras
On Wed, Apr 10, 2013 at 4:15 PM, Jeff King p...@peff.net wrote:
 From: Felipe Contreras felipe.contre...@gmail.com

 If a push fails because the remote-helper died (with
 fast-export), the user does not see any error message. We do
 correctly die with a failed exit code, as we notice that the
 helper has died while reading back the ref status from the
 helper. However, we don't print any message.  This is OK if
 the helper itself printed a useful error message, but we
 cannot count on that; let's let the user know that the
 helper failed.

This explained the same thing:
 If a push fails because the remote-helper died (with fast-export), the user 
 won't see any error message. So let's add one.

Granted, depending on the way the remote-helper died, an error might
or might not been printed, so s/won't/might not/.

The fact that an exit code was returned before is not relevant,
neither is how the exit was returned, and for that matter neither is
all the other things that are happening in this code. It's just noise.

The only thing that is relevant is this:

-   exit(128);
+   die(Reading from remote helper failed);

It's a simple change, and simple to explain.

 In the long run, it may make more sense to propagate the
 error back up to push, so that it can present the usual
 status table and give a nicer message. But this is a much
 simpler fix that can help immediately.

Yes it might, and it might make sense to rewrite much of this code,
but that's not relevant.

 While we're adding tests, let's also confirm that the
 remote-helper dying is also detect when importing refs.

That is enough explanation.

 We
 currently do so robustly when the helper uses the done
 feature (and that is what we test).  We cannot do so
 reliably when the helper does not use the done feature,
 but it is not even worth testing; the right solution is for
 the helper to start using done.

This doesn't help anyone, and it's not even accurate. I think it might
be possible enforce remote-helpers to implement the done feature,
and we might want to do that later. But of course, discussing what bad
things remote-helpers could do, and how we should test and babysit
them is not relevant here.

If it was important to explain the subtleties and reasoning behind
this change, it should be a separate patch.

 Suggested-by: Jeff King p...@peff.net
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 Signed-off-by: Jeff King p...@peff.net

I would add:

[jk: rewrote every piece of text]

 export)
 +   if test -n $GIT_REMOTE_TESTGIT_FAILURE
 +   then
 +   # consume input so fast-export doesn't get SIGPIPE;

I think this is explanation enough.

 +   # git would also notice that case, but we want
 +   # to make sure we are exercising the later
 +   # error checks

I don't understand what is being said here. What is that case?

 +   while read line; do
 +   test done = $line  break
 +   done
 +   exit

LGTM.

Cheers.

--
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] pull: fail early if we know we can't merge from upstream

2013-04-11 Thread Carlos Martín Nieto
A 'git pull' without specifying a remote is asked to take the current
branch's upstream as the branch to merge from. This cannot work
without an upstream configuration nor with HEAD detached, but we only
check for this after fetching.

Perform the check beforehand, as we already know whether we have
enough information to merge and can fail immediately otherwise.

Signed-off-by: Carlos Martín Nieto c...@elego.de
---
 git-pull.sh | 62 -
 1 file changed, 45 insertions(+), 17 deletions(-)

I can't quite decide whether the behaviour of 'git pull' with no
upstream configured but a default remote with no fetch refspecs
merging the remote's HEAD is a feature, a bug or something in between,
but it's used by t7409 so maybe someone else is using it and we
shouldn't break it.

There's another check that could be made earlier ('git pull
someremote' when that's not the branch's upstream remote), but then
you have to start figuring out what the flags to fetch are. I'll
revisit this at some point, but I wanted to get this out since it's
working.

diff --git a/git-pull.sh b/git-pull.sh
index 266e682..b62f5d3 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -43,6 +43,8 @@ log_arg= verbosity= progress= recurse_submodules=
 merge_args= edit=
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short=${curr_branch#refs/heads/}
+upstream=$(git config branch.$curr_branch_short.merge)
+remote=$(git config branch.$curr_branch_short.remote)
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 if test -z $rebase
 then
@@ -138,6 +140,47 @@ do
esac
shift
 done
+if test true = $rebase
+then
+op_type=rebase
+op_prep=against
+else
+op_type=merge
+op_prep=with
+fi
+
+check_args_against_config () {
+   # If fetch gets user-provided arguments, the user is
+   # overriding the upstream configuration, so we have to wait
+   # for fetch to do its work to know if we can merge.
+   if [ $# -gt 0 ]; then
+   return
+   fi
+
+   # Figure out what remote we're going to be fetching from
+   use_remote=origin
+   if [ -n $remote ]; then
+   use_remote=$remote
+   fi
+
+   # If the remote doesn't have a fetch refspec, then we'll merge
+   # whatever fetch marks for-merge, same as above.
+   fetch=$(git config --get-all remote.$use_remote.fetch)
+   if [ -z $fetch ]; then
+   return
+   fi
+
+   # The typical 'git pull' case where it should merge from the
+   # current branch's upstream. We can already check whether we
+   # we can do it. If HEAD is detached or there is no upstream
+   # branch, complain now.
+   if [ -z $curr_branch_short -o -z $upstream ]; then
+   . git-parse-remote
+   error_on_missing_default_upstream pull $op_type $op_prep \
+   git pull remote branch
+   exit 1
+   fi
+}
 
 error_on_no_merge_candidates () {
exec 2
@@ -151,19 +194,6 @@ error_on_no_merge_candidates () {
esac
done
 
-   if test true = $rebase
-   then
-   op_type=rebase
-   op_prep=against
-   else
-   op_type=merge
-   op_prep=with
-   fi
-
-   curr_branch=${curr_branch#refs/heads/}
-   upstream=$(git config branch.$curr_branch.merge)
-   remote=$(git config branch.$curr_branch.remote)
-
if [ $# -gt 1 ]; then
if [ $rebase = true ]; then
printf There is no candidate for rebasing against 
@@ -177,10 +207,6 @@ error_on_no_merge_candidates () {
echo You asked to pull from the remote '$1', but did not 
specify
echo a branch. Because this is not the default configured 
remote
echo for your current branch, you must specify a branch on the 
command line.
-   elif [ -z $curr_branch -o -z $upstream ]; then
-   . git-parse-remote
-   error_on_missing_default_upstream pull $op_type $op_prep \
-   git pull remote branch
else
echo Your configuration specifies to $op_type $op_prep the ref 
'${upstream#refs/heads/}'
echo from the remote, but no such ref was fetched.
@@ -213,6 +239,8 @@ test true = $rebase  {
fi
done
 }
+
+check_args_against_config $@
 orig_head=$(git rev-parse -q --verify HEAD)
 git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok 
$@ || exit 1
 test -z $dry_run || exit 0
-- 
1.8.2.524.g8f8def7

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


Re: [PATCH v3] cherry-pick: make sure all input objects are commits

2013-04-11 Thread Ramkumar Ramachandra
Miklos Vajna wrote:
 Signed-off-by: Miklos Vajna vmik...@suse.cz

This one looks good.  FWIW,

Reviewed-by: Ramkumar Ramachandra artag...@gmail.com
--
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-imap-send.txt: remove the use of sslverify=false in GMail example

2013-04-11 Thread Barbu Paul - Gheorghe
On 04/10/2013 09:44 PM, Junio C Hamano wrote:
 Thanks.

My pleasure.

 While removing that item from the configuration is a good thing to
 do in the post 1.8.2.1 era, the reason why it is does not have much
 to do with GMail is SSL capable.

Should I change the commit message in order to avoid confusion among devs that
read it?

 The configuration item is not about Do we connect over SSL when
 talking to this host?, but is about When we use SSL with this
 host, do we verify the certificate it gave us?.

If I change it, how should it sound?
It could be:

Since GMail's certificates can be sslverify-ed there is no need to set sslverify
to false, the example using it may confuse readers that it's needed since it's
also used in the previous example configurations, too.

Have a nice day!

-- 
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - 
https://github.com/paullik
--
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: git log -p unexpected behaviour - security risk?

2013-04-11 Thread Tay Ray Chuan
On Thu, Apr 11, 2013 at 6:36 PM, John Tapsell johnf...@gmail.com wrote:
   I noticed that code that you put in merge will not be visible by
 default.  This seems like a pretty horrible security problem, no?

 I made the following test tree, with just 3 commits:

 https://github.com/johnflux/ExampleEvilness.git

 Doing git log -p  shows all very innocent commits.  Completely
 hidden is the change to add EVIL CODE MUWHAHAHA.

 This seems really dangerous!

 The evil code only shows up with the non-default  --cc or -m  option.

For email-based patch workflows (eg. git, linux kernel), then this is
not a problem - the diff doesn't even show up, so nothing is applied
when git-am is run.

For github with pull-requests, a diff is shown between trees, so this
will show up.

--
Cheers,
Ray Chuan
--
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-imap-send.txt: remove the use of sslverify=false in GMail example

2013-04-11 Thread Simon Ruderich
On Wed, Apr 10, 2013 at 11:44:03AM -0700, Junio C Hamano wrote:
 The reason why we can run with sslverify=true against gmail is
 because we know imap.gmail.com gives a validly signed certificate
 that leads all the way to a root CA the user's OpenSSL installation
 is likely to trust (if your hand-rolled imap-over-ssl server uses a
 snakeoil certificate, even though the server may be SSL capable,
 you may not be able to successfully connect to it without sslverify
 turned off).

Maybe imap-send should learn imap.sslCAInfo and imap.sslCAPath
like http.* to handle custom certificates.

 diff --git a/Documentation/git-imap-send.txt 
 b/Documentation/git-imap-send.txt
 index 875d283..b15dffe 100644
 --- a/Documentation/git-imap-send.txt
 +++ b/Documentation/git-imap-send.txt
 @@ -123,7 +123,6 @@ to specify your account settings:
  host = imaps://imap.gmail.com
  user = u...@gmail.com
  port = 993
 -sslverify = false
  -
   You might need to instead use: folder = [Google Mail]/Drafts if you get 
 an error

I think we should remove sslverify = false from the other example
as well. Recommending sslverify = false is IMHO a bad idea as
SSL provides no protection without verification.

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: regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Thu, Apr 11, 2013 at 05:39:43AM +0200, Mike Galbraith wrote:

ALLOWED_ENV=PATH HOME
HOME=/
 
 I can work around it by changing the init script to use su - git -c bla
 bla to launch the thing, instead of using --user=git --group=daemon,
 but that's just a bandaid for the busted environment setup those
 switches were supposed to make happen, no?

 Yeah, I think the bug here is that git-daemon should be setting $HOME
 when it switches privileges with --user. Does this patch fix it for you?

 diff --git a/daemon.c b/daemon.c
 index 6aeddcb..a4451fd 100644
 --- a/daemon.c
 +++ b/daemon.c
 @@ -1091,6 +1091,7 @@ static void drop_privileges(struct credentials *cred)
   if (cred  (initgroups(cred-pass-pw_name, cred-gid) ||
   setgid (cred-gid) || setuid(cred-pass-pw_uid)))
   die(cannot drop privileges);
 + setenv(HOME, cred-pass-pw_dir, 1);
  }
  
  static struct credentials *prepare_credentials(const char *user_name,

Yeah, that sounds like the obvious fix 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] git-imap-send.txt: remove the use of sslverify=false in GMail example

2013-04-11 Thread Barbu Paul - Gheorghe
On 04/11/2013 06:26 PM, Simon Ruderich wrote:

 I think we should remove sslverify = false from the other example
 as well. Recommending sslverify = false is IMHO a bad idea as
 SSL provides no protection without verification.

Yep, that was why I thought there should be at least an example without it.

Should I create a new patch removing them all?

-- 
Barbu Paul - Gheorghe
Common sense is not so common - Voltaire
Visit My GitHub profile to see my open-source projects - 
https://github.com/paullik
--
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] transport-helper: report errors properly

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 08:22:26AM -0500, Felipe Contreras wrote:

  We
  currently do so robustly when the helper uses the done
  feature (and that is what we test).  We cannot do so
  reliably when the helper does not use the done feature,
  but it is not even worth testing; the right solution is for
  the helper to start using done.
 
 This doesn't help anyone, and it's not even accurate. I think it might
 be possible enforce remote-helpers to implement the done feature,
 and we might want to do that later. But of course, discussing what bad
 things remote-helpers could do, and how we should test and babysit
 them is not relevant here.
 
 If it was important to explain the subtleties and reasoning behind
 this change, it should be a separate patch.

I am OK with adding the test for import as a separate patch. What I am
not OK with (and this goes for the rest of the commit message, too) is
failing to explain any back-story at all for why the change is done in
the way it is.

_You_ may understand it _right now_, but that is not the primary
audience of the message. The primary audience is somebody else a year
from now who is wondering why this patch was done the way it was. When
they are trying to find out why git does not detect errors in a helper,
and they notice that our test for failure only check the done case,
isn't it more helpful to say we considered the other case, but it was
not worth fixing rather than leaving them to guess?

I may be more verbose than necessary in some of my commit messages, but
I would much rather err on the side of explaining too much than too
little.

  export)
  +   if test -n $GIT_REMOTE_TESTGIT_FAILURE
  +   then
  +   # consume input so fast-export doesn't get SIGPIPE;
 
 I think this is explanation enough.
 
  +   # git would also notice that case, but we want
  +   # to make sure we are exercising the later
  +   # error checks
 
 I don't understand what is being said here. What is that case?

The case that fast-export gets SIGPIPE. I was trying to explain not
just _what_ we are doing, but _why_ it is important. Perhaps a better
wording would be:

  # consume input so fast-export doesn't get SIGPIPE;
  # we do not technically need to do so in order for
  # git to notice the failure to export, as it will
  # detect problems either with fast-export or with
  # the helper failing to report ref status. But since
  # we are trying to demonstrate that the latter
  # check works, we must avoid the SIGPIPE, which would
  # trigger the former.

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


Re: [PATCH/RFC 1/3] Teach mv to move submodules together with their work trees

2013-04-11 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Then rename() will move the submodule work tree just
 like it moves a file.

 What is this rename() function you're talking about?  I don't see it anywhere.

man 2 rename; it is called from a generic part of builtin/mv.c to
rename one path to another and can move both files and directories.

 +   if (first = 0) {
 +   if 
 (!S_ISGITLINK(active_cache[first]-ce_mode))
 +   die (_(Huh? Directory %s is in 
 index and no submodule?), src);

 I didn't understand this.  Why does it have to be a gitlink if it is
 stored at index position = 0?

The path is not in the middle of the conflict, but the index records
something that is not a gitlink.  E.g. you start from A (regular file)
in the index, rm A  mkdir A and make it the top of a working
tree of a separate project.  git mv A elsewhere will say src_is_dir
but the index still thinks A should be a regular file blob.

 I'm assuming the else-case has nothing to do with the actual moving
 but rather something specific to directories (enumerating entries in
 it?), which is why it doesn't get executed when we find a gitlink.

It wants to move all the paths in the directory to a new
destination, e.g. git mv srcdir dstdir, and update 


 +   } else {
 +   const char *src_w_slash = add_slash(src);
 +   int last, len_w_slash = length + 1;
 +
 +   modes[i] = WORKING_DIRECTORY;
 +
 +   first = cache_name_pos(src_w_slash, 
 len_w_slash);
 +   if (first = 0)
 +   die (_(Huh? %.*s is in index?),
 +   len_w_slash, 
 src_w_slash);
 +
 +   first = -1 - first;
 +   for (last = first; last  active_nr; last++) 
 {
 +   const char *path = 
 active_cache[last]-name;
 +   if (strncmp(path, src_w_slash, 
 len_w_slash))
 +   break;
 }

 Mostly unchanged, but I didn't understand the line first = -1 - first
 in the original.  What is it doing?  So, I'm guessing first is the
 cache position of the directory itself, and last stores the index of
 the last entry in the cache?  What does that even mean?

cache_name_pos() either returns the position for a path at stage #0
that exists in the index, or the position the given path _would_ be
inserted if you were to add it to the index for a path that does not
exist in the index at stage #0 (I think this part of the code does
not consider that the given path is unmerged, either by being sloppy
or detecting that case much earlier---I didn't check), and when
doing the latter, it encodes the position by negating it and
offsetting it by 1 (otherwise you cannot tell it would come at the
very beginning and it is at the very beginning, because negated
zero is still zero).  The -1 - first is an idiom used everywhere
by callers of cache_name_pos() to recover the latter from the
returned value.

If you start at a position src/ would have been inserted, and
iterate over the index while the entry's path prefix-matches with
src/, you will find where the entries in the src/ directory
ends.

 +   if (last - first  1)
 +   bad = _(source directory is empty);

 This is exactly what was tripping us up earlier.  Can you explain what
 last - first  1 means?

I think the above covers it.

Asking questions to learn the basic part of Git internals on this
list, e.g. I found this existing code, and I do not understand what
it is doing. Can somebody shed a light on it?, is perfectly fine,
but can you do so outside the review discussion?

It clutters the review discussions when such a request for education
is labeled as if it were a review or mixed with a message with
genuine review comments.
--
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: git-http-backend: anonymous read, authenticated write

2013-04-11 Thread Jakub Narębski
W dniu 11.04.2013 03:56, Jeff King napisał:

 So _if_ you fixed it by setting http.receivepack (which I think is the
 simplest thing under Apache, since matching the query string there is
 hard), then you would need a version of git with that fix on the
 client side to actually have git prompt for the password correctly.
 
 But your fix under lighttpd is much better, as it asks for the
 credentials up front (which means the client does not go to any work
 creating a packfile just to find out that it does not have access).

According to http://paperlined.org/apps/git/SmartHTTP_Ubuntu.html
it is (supposedly) not that hard in Apache (though it requires mod_rewrite):


  RewriteCond %{QUERY_STRING} =service=git-receive-pack [OR]
  RewriteCond %{REQUEST_URI} /git-receive-pack$
  RewriteRule (.*) $1 [E=AUTHREQUIRED:yes]

  Location /git/
  Order Deny,Allow
  Deny from env=AUTHREQUIRED

  AuthType Basic
  AuthName Git Access
  Require group committers

  Satisfy Any
  Location

Not tested.


P.S. By the way, is there some debugger for apache config (mod_rewrite
and deny/allow)?
-- 
Jakub Narębski
--
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/3] Teach mv to move submodules together with their work trees

2013-04-11 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Why does it search for a submodule with a trailing slash in the index?
  You make it sound like it's doing something unnatural; in reality, it
 does this because it executes lstat() on the filesystem path
 specified, and the stat mode matches S_ISDIR (because it _is_ a
 directory on the filesystem).  It is stored in the index as an entry
 (without a trailing slash) with the mode 16, gitlink.

 What happens if I attempt to git mv oldpath/ newpath/ (with the
 literal slashes, probably because I'm using a stupid shell
 completion)?

I think it should work.

mkdir a  a/f  git add a/f  git mv a/ b/

 Fix that by searching for the name without a trailing slash and continue
 if it is a submodule.

 So, the correct solution is not to search for a name without a
 trailing slash, but rather to handle the gitlink entry in the index
 appropriately.

For moving an entire directory's contents, because the index is
flat, you would look for name/, because you know all of the paths
contained in it will share that prefix.

But when dealing with a submodule, you need to see if the path that
found to be a directory in the working tree is a gitlink in the
index.  And the way to do so is to look for it without trailing
slash, because there is nothing hanging under it in the index.

So the right implementation of handle appropriately is to search
without slash.  They are saying the same thing, and the latter is a
more specific way to point out in what way the existing code that
wanted to delegate moving to submodule mv and not having to worry
about gitlinks was unprepared for it, and what change is needed to
make it handle appropriately.

So I think there is no need to rephrase here, but your comment makes
me wonder if the patch covers the case where you have a submodule at
a/x and the user does git mv a/ b/.  The src_is_dir thing will
notice a/ is a directory, finds all the paths inside a/ including
a/x (and we won't see any paths inside the submodule's working
tree) to b/, and update the cache entries and the working tree.
Does the adjusting of the path for that moved submodule, which is a
theme for [PATCH 3/3], cover that case, too?

Another thing to wonder is if we correctly reject git mv a/x/f here
in the same example where a/x is a directory.  The path is beyond
the lower boundary of our working tree and should not be touched.
--
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] doc/http-backend: give some lighttpd config examples

2013-04-11 Thread Jakub Narębski
W dniu 11.04.2013 05:36, Jeff King napisał:

 +Note that unlike the similar setup with Apache, we can easily match the
 +query string for receive-pack, catching the initial request from the
 +client. This means that the server administrator does not have to worry
 +about configuring `http.receivepack` for the repositories (the default
 +value, which enables it only in the case of authentication, is
 +sufficient).

Perhaps it would be worth including for Apache2 beside basic setup that
requires http.receivepack set to true, also one like for LigHTTPd, i.e.

  RewriteCond %{QUERY_STRING} =service=git-receive-pack [OR]
  RewriteCond %{REQUEST_URI} /git-receive-pack$
  RewriteRule (.*) $1 [E=AUTHREQUIRED:yes]

  Location /gitweb/
  Order Deny,Allow
  Deny from env=AUTHREQUIRED

  AuthType Basic
  AuthName Git Access
  Require group committers

  Satisfy Any
  Location

And perhaps also adding it as test...
-- 
Jakub Narębski
--
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] transport-helper: report errors properly

2013-04-11 Thread Felipe Contreras
On Thu, Apr 11, 2013 at 11:18 AM, Jeff King p...@peff.net wrote:
 On Thu, Apr 11, 2013 at 08:22:26AM -0500, Felipe Contreras wrote:

  We
  currently do so robustly when the helper uses the done
  feature (and that is what we test).  We cannot do so
  reliably when the helper does not use the done feature,
  but it is not even worth testing; the right solution is for
  the helper to start using done.

 This doesn't help anyone, and it's not even accurate. I think it might
 be possible enforce remote-helpers to implement the done feature,
 and we might want to do that later. But of course, discussing what bad
 things remote-helpers could do, and how we should test and babysit
 them is not relevant here.

 If it was important to explain the subtleties and reasoning behind
 this change, it should be a separate patch.

 I am OK with adding the test for import as a separate patch. What I am
 not OK with (and this goes for the rest of the commit message, too) is
 failing to explain any back-story at all for why the change is done in
 the way it is.

 _You_ may understand it _right now_, but that is not the primary
 audience of the message. The primary audience is somebody else a year
 from now who is wondering why this patch was done the way it was.

Who would be this person? Somebody who wonders why this test is using
feature done? I doubt such a person would exist, as using this
feature is standard, as can be seen below this chunk. *If* the test
was *not* using this feature done, *then* sure, an explanation would
be needed.

But why is this test doing something expected is not a question
anybody would benefit from asking.

 When
 they are trying to find out why git does not detect errors in a helper,
 and they notice that our test for failure only check the done case,
 isn't it more helpful to say we considered the other case, but it was
 not worth fixing rather than leaving them to guess?

If you are worried about such hypothetical people, they would be
better served by a comment in the source code of the test, or even
better, the c file, or even better, to document that remote helpers
should use this feature. But wait:

---
Just like 'push', a batch sequence of one or more 'import' is
terminated with a blank line. For each batch of 'import', the remote
helper should produce a fast-import stream terminated by a 'done'
command.
---

So it's already explained, if somebody fails to follow this
documentation, it's dubious a commit message that introduces a test
would help. Surely, the writer of this bad remote helper would _never_
look there.

 I may be more verbose than necessary in some of my commit messages, but
 I would much rather err on the side of explaining too much than too
 little.

I wouldn't. The only thing an overload of information achieves is that
the reader would simply skip or skim it.

  export)
  +   if test -n $GIT_REMOTE_TESTGIT_FAILURE
  +   then
  +   # consume input so fast-export doesn't get SIGPIPE;

 I think this is explanation enough.

  +   # git would also notice that case, but we want
  +   # to make sure we are exercising the later
  +   # error checks

 I don't understand what is being said here. What is that case?

 The case that fast-export gets SIGPIPE.

If we are trying to avoid SIGPIPE wouldn't that imply that git notices
the SIGPIPE?

   # consume input so fast-export doesn't get SIGPIPE;
   # we do not technically need to do so in order for
   # git to notice the failure to export, as it will
   # detect problems either with fast-export or with
   # the helper failing to report ref status. But since
   # we are trying to demonstrate that the latter
   # check works, we must avoid the SIGPIPE, which would
   # trigger the former.

# consume input so fast-export doesn't get SIGPIPE; we want to test
the remote-helper's code after fast-export.

--
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 1/2] transport-helper: report errors properly

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 11:49:11AM -0500, Felipe Contreras wrote:

  I am OK with adding the test for import as a separate patch. What I am
  not OK with (and this goes for the rest of the commit message, too) is
  failing to explain any back-story at all for why the change is done in
  the way it is.
 
  _You_ may understand it _right now_, but that is not the primary
  audience of the message. The primary audience is somebody else a year
  from now who is wondering why this patch was done the way it was.
 
 Who would be this person? Somebody who wonders why this test is using
 feature done? I doubt such a person would exist, as using this
 feature is standard, as can be seen below this chunk. *If* the test
 was *not* using this feature done, *then* sure, an explanation would
 be needed.

If it was so obvious, why did your initial patch not use feature done?
If it was so obvious, why did our email discussion go back and forth so
many times before arriving at this patch?

It was certainly not obvious to me when this email thread started. So in
response to your question: *I* am that person. I was him two weeks ago,
and there is a good chance that I will be him a year from now. Much of
my work on git is spent tracking down bugs in older code, and those
commit messages are extremely valuable to me in understanding what
happened at the time.

But I give up on you. I find most of your commit messages lacking in
details and motivation, making assumptions that the reader is as
familiar with the code when reading the commit as you are when you wrote
it. I tried to help by suggesting in review that you elaborate. That
didn't work. So I tried to help by writing the text myself. But clearly
I am not going to convince you that it is valuable, even if it requires
no work at all from you, so I have nothing else to say on the matter.

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


Re: [PATCH/RFC 3/3] Teach mv to update the path entry in .gitmodules for moved submodules

2013-04-11 Thread Junio C Hamano
Jens Lehmann jens.lehm...@web.de writes:

 Currently using git mv on a submodule moves the submodule's work tree in
 that of the superproject. But the submodule's path setting in .gitmodules
 is left untouched, which is now inconsistent with the work tree and makes
 git commands that rely on the proper path - name mapping (like status and
 diff) behave strangely.

 Let git mv help here by not only moving the submodule's work tree but
 also updating the submodule.submodule name.path setting from the
 .gitmodules file and stage both. This doesn't happen when no .gitmodules
 file is found and only issues a warning when it doesn't have a section for
 this submodule.

Should it happen when the user has other changes to .gitmodules that
hasn't been updated to the index?

As this is done in the same final per-path loop as adjusting the
gitfile links, the worry I expressed in an earlier message about
git mv a/ b/ when a/x is a submodule turns out to be unfounded,
which is 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 2/2] doc/http-backend: give some lighttpd config examples

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 06:47:49PM +0200, Jakub Narębski wrote:

 W dniu 11.04.2013 05:36, Jeff King napisał:
 
  +Note that unlike the similar setup with Apache, we can easily match the
  +query string for receive-pack, catching the initial request from the
  +client. This means that the server administrator does not have to worry
  +about configuring `http.receivepack` for the repositories (the default
  +value, which enables it only in the case of authentication, is
  +sufficient).
 
 Perhaps it would be worth including for Apache2 beside basic setup that
 requires http.receivepack set to true, also one like for LigHTTPd, i.e.
 
   RewriteCond %{QUERY_STRING} =service=git-receive-pack [OR]
   RewriteCond %{REQUEST_URI} /git-receive-pack$
   RewriteRule (.*) $1 [E=AUTHREQUIRED:yes]
 
   Location /gitweb/
   Order Deny,Allow
   Deny from env=AUTHREQUIRED
 
   AuthType Basic
   AuthName Git Access
   Require group committers
 
   Satisfy Any
   Location
 
 And perhaps also adding it as test...

That was the I am not clever nor interested in Apache enough to figure
out how to do this... part that I wrote. I have no clue if the above
works, but I'd be happy if you wanted to test it out and submit it as a
patch on top (I think it could even replace my 1/2, as making it just
work is a much better solution than having to explain the extra step in
the documentation).

-Peff

 -- 
 Jakub Narębski
--
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-svn: wrong history after cloning a repository

2013-04-11 Thread Frans Fürst
Hi,

I'm currently struggeling with a strage behaviour of git-svn: After
git-svn-cloning a given repository (see attachment) in some cases the
git blame differs from the svn blame. The history looks like after a
merge all affected files have been added from scratch at this revision.

The behaviour can be observed using at least the following versions of
git:
1.7.10.2
1.8.1.4
1.8.2.GIT

Given: * svn-repository, svn-server 1.6.11, apache2 module via WEB DAV
   * repository like the one attached:
* standard svn layout
* branch ^/branches/feature1 copied from ^/trunk
* changes made and files added on 
  project1/branches/feature1 by 'testuser1'
* branch ^/branches/rc-1.0 copied from ^/trunk on 
  revX by 'mergeuser'
* ^/branches/feature1 merged to ^/trunk on 
  revY by 'mergeuser'
* ^/branches/rc-1.0 merged back to trunk revZ by 'mergeuser'

Action: git svn clone -s repo-url
git blame some_file_modified_on-feature1

Observed behaviour: Changes being made on project1/branches/feature1
being associated to 'mergeuser' who integrated feature1 but not
implemented on that branch.

 $ git blame src/file2.txt
 9846ece6 (mergeuser 2013-04-11 14:31:21 + 1) content2-1
 9846ece6 (mergeuser 2013-04-11 14:31:21 + 2) content2-2
 9846ece6 (mergeuser 2013-04-11 14:31:21 + 3) content2-3

Action: svn blame repo-url/trunk/some_file_modified_on-feature1

Observed behaviour: Changes are correctly associated to user1

 $ svn blame src/file2.txt
 4  testuser1 content2-1
 4  testuser1 content2-2
 4  testuser1 content2-3

I don't know yet which steps are really needed to re-create a repository
with this behaviour. My guess is that adding files together with a merge
(see r6 in the attached repository) has something to do with it.

Can you tell me whether this is a known issue or if there's a work
around for this? Unfortunately I'm not very into perl so I can't solve
this on my own. Please let me know if you need additional information.

Thanks

Frans


svn-example-repo.tgz
Description: application/compressed-tar


[PATCH] RelNotes: few typofixes in notes for recent releases

2013-04-11 Thread Stefano Lattarini
Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Documentation/RelNotes/1.8.2.1.txt | 2 +-
 Documentation/RelNotes/1.8.3.txt   | 8 
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/RelNotes/1.8.2.1.txt 
b/Documentation/RelNotes/1.8.2.1.txt
index 1354ad0..769a6fc 100644
--- a/Documentation/RelNotes/1.8.2.1.txt
+++ b/Documentation/RelNotes/1.8.2.1.txt
@@ -49,7 +49,7 @@ Fixes since v1.8.2
common prefix and suffix between the two filenames overlapped.
 
  * git submodule update, when recursed into sub-submodules, did not
-   acccumulate the prefix paths.
+   accumulate the prefix paths.
 
  * git am $maildir/ applied messages in an unexpected order; sort
filenames read from the maildir/ in a way that is more likely to
diff --git a/Documentation/RelNotes/1.8.3.txt b/Documentation/RelNotes/1.8.3.txt
index ca76a2b..537fc7a 100644
--- a/Documentation/RelNotes/1.8.3.txt
+++ b/Documentation/RelNotes/1.8.3.txt
@@ -41,11 +41,11 @@ UI, Workflows  Features
revert session, just like it does for a cherry-pick and a bisect
session.
 
- * The handing by git branch --set-upstream-to against various forms
-   of errorneous inputs was suboptimal and has been improved.
+ * The handling by git branch --set-upstream-to against various forms
+   of erroneous inputs was suboptimal and has been improved.
 
  * When the interactive access to git-shell is not enabled, it issues
-   a message meant to help the system admininstrator to enable it.
+   a message meant to help the system administrator to enable it.
An explicit way to help the end users who connect to the service by
issuing custom messages to refuse such an access has been added.
 
@@ -294,4 +294,4 @@ details).
alphabetical order.
 
  * git submodule update, when recursed into sub-submodules, did not
-   acccumulate the prefix paths.
+   accumulate the prefix paths.
-- 
1.8.2.373.g961c512

--
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: regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 08:35:46AM -0700, Junio C Hamano wrote:

  Yeah, I think the bug here is that git-daemon should be setting $HOME
  when it switches privileges with --user. Does this patch fix it for you?
 [...]
 Yeah, that sounds like the obvious fix to me.

Here it is with a commit message.

-- 8 --
Subject: [PATCH] daemon: set HOME when we switch to --user

If git-daemon is invoked with the --user foo option, we
setuid and setgid to the foo user. However, we do not
currently touch $HOME or any other environment variables.

This means that a git-daemon (and its git subprocesses)
invoked as root will look at ~root/.gitconfig,
~root/.config/git, etc. This is probably not what the admin
expected; it would make more sense to load user-wide config
from ~foo.

Traditionally this wasn't that big a deal, as most sites do
not put config in either homedir (they would use the
system-wide /etc/gitconfig if they wanted global config).
However, since 96b9e0e (config: treat user and xdg config
permission problems as errors, 2012-10-13), it is now an
error to try to read from an inaccessible config file (which
a file in ~root is very likely to be), meaning that
git-daemon will not run at all in such a case.

We can fix this by setting HOME appropriately when we switch
users. Note that this is a regression for any site that uses
--user but depends on putting config in the $HOME of the
user invoking git-daemon. Since the original behavior was
never documented, and the new behavior is much more
sensible, we can consider this a bugfix.

Reported-by: Mike Galbraith bitbuc...@online.de
Signed-off-by: Jeff King p...@peff.net
---
I don't have any problem calling this a bugfix and claiming that anyone
who was depending on the original behavior is stupid and wrong. But it
should probably get a prominent slot in the ReleaseNotes.

 daemon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/daemon.c b/daemon.c
index 6aeddcb..a4451fd 100644
--- a/daemon.c
+++ b/daemon.c
@@ -1091,6 +1091,7 @@ static void drop_privileges(struct credentials *cred)
if (cred  (initgroups(cred-pass-pw_name, cred-gid) ||
setgid (cred-gid) || setuid(cred-pass-pw_uid)))
die(cannot drop privileges);
+   setenv(HOME, cred-pass-pw_dir, 1);
 }
 
 static struct credentials *prepare_credentials(const char *user_name,
-- 
1.8.2.rc0.33.gd915649

--
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: Locating merge that dropped a change

2013-04-11 Thread Kevin Bracey

On 09/04/2013 21:00, Kevin Bracey wrote:


So, how to automatically find a merge that ignored a known change?


I think I've found the problem. It only doesn't work _if you specify the 
file_.


Specifically, if I was missing an addition, my first attempt to find it 
would be


  git log -p -m -Saddition file

If the addition was lost in a merge, that doesn't even show the 
addition, which is surprising, but intentional. The addition isn't part 
of the HEAD version of file, so no point going down that path of the 
merge. Fine. However, I expected this to work:


  git log --full-history -p -m -Saddition file

But it doesn't. It finds the addition, but _not_ the loss in the merge 
commit.


But this does work:

  git log -p -m -Saddition

That really feels like a bug to me. By specifying a file, I've made it 
miss the change, and I can see no way to get the change without making 
it a full-tree operation.


Looking at try_to_simplify_commit() it appears the merge that removed 
the addition is excluded because it's TREESAME to its _other_ parent. 
But with --full-history, we should only be eliminating a merge if its 
TREESAME to all parents, surely? Otherwise we have this case that we 
show a commit but not its reversion.


And the code doing this looks broken, or at least illogical - the parent 
loop sets tree_same for a same parent in --full-history, rather than 
immediately setting the TREESAME flag, so maybe previous authors were 
thinking about this. But setting tree_same guarantees that TREESAME is 
always set on exit anyway, so it's pointless (unless I'm missing something).


It does appear this is documented behaviour in the manual: full-history 
without parent rewriting .. P and M were excluded because they are 
TREESAME to _a_ parent. I would say that they should have been excluded 
due to being TREESAME to _all_ parents. I really don't want a merge 
where one version of my file was chosen over another excluded from its 
so-called full history.


Now, obviously in a sane environment, most merges won't be that 
interesting, as they'll just be propagating wanted patches from the real 
commits already in the log. But I'd like some way to find merges that 
drop code in a specified file, and surely --full-history is it?


Kevin


--
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] pull: fail early if we know we can't merge from upstream

2013-04-11 Thread Junio C Hamano
Carlos Martín Nieto c...@elego.de writes:

 I can't quite decide whether the behaviour of 'git pull' with no
 upstream configured but a default remote with no fetch refspecs
 merging the remote's HEAD is a feature, a bug or something in between,
 but it's used by t7409 so maybe someone else is using it and we
 shouldn't break it.

Isn't it the simplest works without any configuration from the
original days? 

 There's another check that could be made earlier ('git pull
 someremote' when that's not the branch's upstream remote), but then
 you have to start figuring out what the flags to fetch are.

When the user gave us explicitly the name of the remote, it does not
sound too bad to fetch from there.  git pull someremote thatbranch
can be given after seeing a failure and succeed without retransfer,
no?

I am not sure if it is worth the added complexity and potential to
introduce new bugs in general by trying to outsmart the for-merge
logic that kicks in only after we learn what the other side offers
and fetch from it, but anyway, let's see what we got here...

 diff --git a/git-pull.sh b/git-pull.sh
 index 266e682..b62f5d3 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -43,6 +43,8 @@ log_arg= verbosity= progress= recurse_submodules=
  merge_args= edit=
  curr_branch=$(git symbolic-ref -q HEAD)
  curr_branch_short=${curr_branch#refs/heads/}
 +upstream=$(git config branch.$curr_branch_short.merge)
 +remote=$(git config branch.$curr_branch_short.remote)
  rebase=$(git config --bool branch.$curr_branch_short.rebase)

Learning these upfront sounds sensible.

  if test -z $rebase
  then
 @@ -138,6 +140,47 @@ do
   esac
   shift
  done
 +if test true = $rebase
 +then
 +op_type=rebase
 +op_prep=against
 +else
 +op_type=merge
 +op_prep=with
 +fi
 +
 +check_args_against_config () {
 + # If fetch gets user-provided arguments, the user is
 + # overriding the upstream configuration, so we have to wait
 + # for fetch to do its work to know if we can merge.
 + if [ $# -gt 0 ]; then
 + return
 + fi

 + # Figure out what remote we're going to be fetching from
 + use_remote=origin
 + if [ -n $remote ]; then
 + use_remote=$remote
 + fi
 +
 + # If the remote doesn't have a fetch refspec, then we'll merge
 + # whatever fetch marks for-merge, same as above.

The above in this sentence refers to...?

I guess we have to wait, but it wasn't very clear.

 + fetch=$(git config --get-all remote.$use_remote.fetch)
 + if [ -z $fetch ]; then
 + return
 + fi

Hmm, it is probably correct to punt on this case, but it defeats
large part of the effect of your effort, doesn't it? We fetch what
is covered by remote.$name.fetch _and_ what need to complete the
merge operation (otherwise branch.$name.merge that is not covered by
remote.$there.fetch will not work).  So

[remote origin]
url = $over_there
[branch master]
remote = origin
merge = refs/heads/master

would still fetch refs/heads/master from there and merge it.

 + # The typical 'git pull' case where it should merge from the
 + # current branch's upstream. We can already check whether we
 + # we can do it. If HEAD is detached or there is no upstream
 + # branch, complain now.

Drop typical, and rephrase merge from to also cover rebase (I
often say integrate with).

To return to your original description:

A 'git pull' without specifying a remote is asked to take the
current branch's upstream as the branch to merge from. This
cannot work without an upstream configuration nor with HEAD
detached, but we only check for this after fetching.

Wouldn't it be sufficient to add something like this before fetch
happens:

if test $# != 0 || # args explicitly specified
   test -n $curr_branch || # not detached
   test -n $upstream # what to integrate with is known
then
return ;# then no problem
fi
die underspecified 'git pull'

without changing anything else?  For that matter, $upstream is
likely to be empty when detached, so the second test may not even be
necessary.

--
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 v5] transport-helper: report errors properly

2013-04-11 Thread Felipe Contreras
If a push fails because the remote-helper died (with fast-export), the
user might not see any error message. So let's add one.

Suggested-by: Jeff King p...@peff.net
Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 git-remote-testgit| 11 +++
 t/t5801-remote-helpers.sh | 10 ++
 transport-helper.c|  2 +-
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/git-remote-testgit b/git-remote-testgit
index b395c8d..472ab1a 100755
--- a/git-remote-testgit
+++ b/git-remote-testgit
@@ -67,6 +67,17 @@ do
echo done
;;
export)
+   if test -n $GIT_REMOTE_TESTGIT_FAILURE
+   then
+   # consume input so fast-export doesn't get SIGPIPE; we
+   # want to test the remote-helper's code after
+   # fast-export.
+   while read line; do
+   test done = $line  break
+   done
+   exit 1
+   fi
+
before=$(git for-each-ref --format='%(refname) %(objectname)')
 
git fast-import ${testgitmarks_args[@]} --quiet
diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
index f387027..8d61702 100755
--- a/t/t5801-remote-helpers.sh
+++ b/t/t5801-remote-helpers.sh
@@ -166,4 +166,14 @@ test_expect_success 'push ref with existing object' '
compare_refs local dup server dup
 '
 
+test_expect_success 'proper failure checks for pushing' '
+   (GIT_REMOTE_TESTGIT_FAILURE=1 
+   export GIT_REMOTE_TESTGIT_FAILURE 
+   cd local 
+   test_must_fail git push --all 2 error 
+   cat error 
+   grep -q Reading from remote helper failed error
+   )
+'
+
 test_done
diff --git a/transport-helper.c b/transport-helper.c
index cb3ef7d..96081cc 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -54,7 +54,7 @@ static int recvline_fh(FILE *helper, struct strbuf *buffer)
if (strbuf_getline(buffer, helper, '\n') == EOF) {
if (debug)
fprintf(stderr, Debug: Remote helper quit.\n);
-   exit(128);
+   die(Reading from remote helper failed);
}
 
if (debug)
-- 
1.8.2.1

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


Re: [PATCH v4 00/21] remote-hg: general updates

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

 This is a reroll of the previous series due to a few minor issues. As the
 previous version, forced pushes remain a configuration option. Also, I picked
 up a fix for test regarding hg_log() that was sent to the mailing list.

Will replace the previous round with this one.

The changes since the previous round looks like this on my end,
which all look sensible.

Thanks.

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index d45f16d..a5f0013 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -655,7 +655,7 @@ def parse_commit(parser):
 
 # Check if the ref is supposed to be a named branch
 if ref.startswith('refs/heads/branches/'):
-extra['branch'] = ref.replace('refs/heads/branches/', '')
+extra['branch'] = ref[len('refs/heads/branches/'):]
 
 if mode == 'hg':
 i = data.find('\n--HG--\n')
@@ -762,7 +762,7 @@ def do_export(parser):
 
 # handle bookmarks
 for bmark, node in p_bmarks:
-ref = 'refs/heads' + bmark
+ref = 'refs/heads/' + bmark
 new = hghex(node)
 
 if bmark in bmarks:
diff --git a/contrib/remote-helpers/test-hg-bidi.sh 
b/contrib/remote-helpers/test-hg-bidi.sh
index a3c88f6..f368953 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
 }
 
 hg_log () {
-   hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+   hg -R $1 log --graph --debug log 
+   grep -v 'tag: *default/' log
 }
 
 setup () {
@@ -62,6 +63,8 @@ setup () {
echo commit = -d \0 0\
echo debugrawcommit = -d \0 0\
echo tag = -d \0 0\
+   echo [extensions]
+   echo graphlog =
)  $HOME/.hgrc 
git config --global remote-hg.hg-git-compat true
 
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh 
b/contrib/remote-helpers/test-hg-hg-git.sh
index 8c59d8e..5daad6b 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -78,7 +78,8 @@ hg_push_hg () {
 }
 
 hg_log () {
-   hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+   hg -R $1 log --graph --debug log 
+   grep -v 'tag: *default/' log
 }
 
 git_log () {
@@ -97,6 +98,7 @@ setup () {
echo [extensions]
echo hgext.bookmarks =
echo hggit =
+   echo graphlog =
)  $HOME/.hgrc 
git config --global receive.denycurrentbranch warn
git config --global remote-hg.hg-git-compat true
--
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] transport-helper: report errors properly

2013-04-11 Thread Felipe Contreras
On Thu, Apr 11, 2013 at 11:59 AM, Jeff King p...@peff.net wrote:
 On Thu, Apr 11, 2013 at 11:49:11AM -0500, Felipe Contreras wrote:

  I am OK with adding the test for import as a separate patch. What I am
  not OK with (and this goes for the rest of the commit message, too) is
  failing to explain any back-story at all for why the change is done in
  the way it is.
 
  _You_ may understand it _right now_, but that is not the primary
  audience of the message. The primary audience is somebody else a year
  from now who is wondering why this patch was done the way it was.

 Who would be this person? Somebody who wonders why this test is using
 feature done? I doubt such a person would exist, as using this
 feature is standard, as can be seen below this chunk. *If* the test
 was *not* using this feature done, *then* sure, an explanation would
 be needed.

 If it was so obvious, why did your initial patch not use feature done?

Because I didn't want to test the obvious, I wanted to test something else.

 If it was so obvious, why did our email discussion go back and forth so
 many times before arriving at this patch?

This patch has absolutely nothing to do with that, in fact, forget
about it, such a minor check is not worth this time and effort:
http://article.gmane.org/gmane.comp.version-control.git/220899

 It was certainly not obvious to me when this email thread started. So in
 response to your question: *I* am that person. I was him two weeks ago,
 and there is a good chance that I will be him a year from now.

No, you are not. I didn't send a patch with feature done originally,
the only reason you wondered about the patch with feature done is
that you saw one without it. It will _never_ happen again.

 Much of
 my work on git is spent tracking down bugs in older code, and those
 commit messages are extremely valuable to me in understanding what
 happened at the time.

Lets make a bet. Let's push the simpler version, and when you hit this
commit message retrospectively and find that you don't understand what
is happening, I loose, and I will forever accept verbose commit
messages. It will never happen.

 But I give up on you. I find most of your commit messages lacking in
 details and motivation, making assumptions that the reader is as
 familiar with the code when reading the commit as you are when you wrote
 it. I tried to help by suggesting in review that you elaborate. That
 didn't work. So I tried to help by writing the text myself. But clearly
 I am not going to convince you that it is valuable, even if it requires
 no work at all from you, so I have nothing else to say on the matter.

Me neither. I picked your solution, but that's not enough, you
*always* want me to do EXACTLY what you want, and never argue back.

It's not going to happen. There's nothing wrong with disagreeing.

Cheers.

--
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 v2 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}

2013-04-11 Thread Junio C Hamano
Adam Spiers g...@adamspiers.org writes:

 diff --git a/Documentation/git-check-ignore.txt 
 b/Documentation/git-check-ignore.txt
 index 7e3cabc..8e1f7ab 100644
 --- a/Documentation/git-check-ignore.txt
 +++ b/Documentation/git-check-ignore.txt
 @@ -81,6 +81,11 @@ not.  (Without this option, it would be impossible to tell 
 whether the
  absence of output for a given file meant that it didn't match any
  pattern, or that the output hadn't been generated yet.)
  
 +Buffering happens as documented under the `GIT_FLUSH` option in
 +linkgit:git[1].  The caller is responsible for avoiding deadlocks
 +caused by overfilling an input buffer or reading from an empty output
 +buffer.
 +
  EXIT STATUS
  ---
  
 diff --git a/Documentation/git.txt b/Documentation/git.txt
 index 6a875f2..eecdb15 100644
 --- a/Documentation/git.txt
 +++ b/Documentation/git.txt
 @@ -808,13 +808,15 @@ for further details.
  
  'GIT_FLUSH'::
   If this environment variable is set to 1, then commands such
 - as 'git blame' (in incremental mode), 'git rev-list', 'git log',
 - and 'git whatchanged' will force a flush of the output stream
 - after each commit-oriented record have been flushed.   If this
 - variable is set to 0, the output of these commands will be done
 - using completely buffered I/O.   If this environment variable is
 - not set, Git will choose buffered or record-oriented flushing
 - based on whether stdout appears to be redirected to a file or not.
 + as 'git blame' (in incremental mode), 'git rev-list', 'git
 + log', 'git check-attr', 'git check-ignore', and 'git
 + whatchanged' will force a flush of the output stream after
 + each commit-oriented record have been flushed.  If this
 + variable is set to 0, the output of these commands will be
 + done using completely buffered I/O.  If this environment
 + variable is not set, Git will choose buffered or
 + record-oriented flushing based on whether stdout appears to be
 + redirected to a file or not.

Reflowing of the text is very much unappreciated X-.  

It took me five minutes to spot that you only added check-attr and
check-ignore and forgot to adjust that commit-oriented record to
an updated reality, where you now have commands that produce
non-commit-oriented record to the output.

It would have been far simpler to review if it were like this, don't
you think?

   If this environment variable is set to 1, then commands such
   as 'git blame' (in incremental mode), 'git rev-list', 'git log',
 - and 'git whatchanged' will force a flush of the output stream
 - after each commit-oriented record have been flushed.   If this
 + 'git check-attr', 'git check-ignore', and 'git whatchanged' will
 + force a flush of the output stream
 + after each record have been flushed.   If this
   variable is set to 0, the output of these commands will be done
   using completely buffered I/O.   If this environment variable is
   not set, Git will choose buffered or record-oriented flushing
   based on whether stdout appears to be redirected to a file or not.

--
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: regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Jonathan Nieder
Jeff King wrote:

 Here it is with a commit message.

 -- 8 --
 Subject: [PATCH] daemon: set HOME when we switch to --user

Thanks for taking care of it.  For what it's worth,

Acked-by: Jonathan Nieder jrnie...@gmail.com

I'm not sure whether to keep 96b9e0e (config: treat user and xdg
config permission problem as errors) in the long run, BTW.  There have
been multiple reports about dropping privileges and not being able to
access the old HOME, and I'm not convinced any more that the
predictability is worth the breakage for such people.  Though checking
if $HOME is inaccessible and treating that case specially would be
even worse...

Insights welcome.

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


Re: regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 11:11:03AM -0700, Jonathan Nieder wrote:

  -- 8 --
  Subject: [PATCH] daemon: set HOME when we switch to --user
 
 Thanks for taking care of it.  For what it's worth,
 
 Acked-by: Jonathan Nieder jrnie...@gmail.com
 
 I'm not sure whether to keep 96b9e0e (config: treat user and xdg
 config permission problem as errors) in the long run, BTW.  There have
 been multiple reports about dropping privileges and not being able to
 access the old HOME, and I'm not convinced any more that the
 predictability is worth the breakage for such people.  Though checking
 if $HOME is inaccessible and treating that case specially would be
 even worse...
 
 Insights welcome.

I could go either way. I think 96b9e0e is the right thing to do
conceptually, but I kind of doubt it was affecting all that many people.
And though it's _possible_ for it to be a security problem, I find it
much more likely that the site admin tries to set some config, gets
annoyed when it doesn't work, and debugs it. So from a practical
perspective, 96b9e0e may be doing more harm than good, even though it's
the right thing.

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


Re: regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Jonathan Nieder
Jeff King wrote:

 I could go either way. I think 96b9e0e is the right thing to do
 conceptually, but I kind of doubt it was affecting all that many people.
 And though it's _possible_ for it to be a security problem, I find it
 much more likely that the site admin tries to set some config, gets
 annoyed when it doesn't work, and debugs it. So from a practical
 perspective, 96b9e0e may be doing more harm than good, even though it's
 the right thing.

Ok.  By the way, another commit to blame is v1.7.12.1~2^2~4 (config:
warn on inaccessible files, 2012-08-21), which made it into a warnable
offense instead of just a strange but accepted configuration. ;-)

I'm still leaning toward keeping v1.7.12.1~2^2~4 and 96b9e0e as being
worth it, though I could be easily swayed in the other direction (for
example via a patch by an interested user with documentation that
explains how to debug and makes it unlikely for the behavior to keep
flipping in the future).  Thanks for spelling out the trade-offs.

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


Re: [PATCH 2/2] doc/http-backend: give some lighttpd config examples

2013-04-11 Thread Jakub Narębski
W dniu 11.04.2013 19:02, Jeff King napisał:
 On Thu, Apr 11, 2013 at 06:47:49PM +0200, Jakub Narębski wrote:
 W dniu 11.04.2013 05:36, Jeff King napisał:

 +Note that unlike the similar setup with Apache, we can easily match the
 +query string for receive-pack, catching the initial request from the
 +client. This means that the server administrator does not have to worry
 +about configuring `http.receivepack` for the repositories (the default
 +value, which enables it only in the case of authentication, is
 +sufficient).

 Perhaps it would be worth including for Apache2 beside basic setup that
 requires http.receivepack set to true, also one like for LigHTTPd, i.e.

   RewriteCond %{QUERY_STRING} =service=git-receive-pack [OR]
   RewriteCond %{REQUEST_URI} /git-receive-pack$
   RewriteRule (.*) $1 [E=AUTHREQUIRED:yes]
[...]
 And perhaps also adding it as test...
 
 That was the I am not clever nor interested in Apache enough to figure
 out how to do this... part that I wrote. I have no clue if the above
 works, but I'd be happy if you wanted to test it out and submit it as a
 patch on top (I think it could even replace my 1/2, as making it just
 work is a much better solution than having to explain the extra step in
 the documentation).

I don't know if short description of `http.receivepack`, suitable for
a reference documentation, tells a new user how to configure web server
for pushes.


With `http.receivepack` unset git (git-http-backed?) will refuse
unauthenthicated pushes but allow authenthicated ones (though it doesn't
handle authorization).  This makes it easy to configure web server for
fetches (read-only) access via smart HTTP (and you can make it
bulletproof by refusing pushes at all with `http.receivepack` false,
isn't it?).

But in this case (`http.receivepack` unset - the default) web server
must be configured to request authorization for both steps of push:
requesting references (for coming up with what
repositories have in common), i.e.

  GET ...?service=git-receive-pack

and actual sending of data and updating refs...

  POST .../git-receive-pack

though only second part is actually writing.


With `http.receivepack` set to true git (git-http-backend?) allows
anonymous pushes, and it is responsibility of web server configuration
to deny unauthorized pushes... but it is sufficient to do it only for
writes i.e.

  POST .../git-receive-pack


[Now to translate it to manpage or users-manual contents...]

P.S. Do I understand it correctly that `http.receivepack` is
three-state: true (allow all), unset (allow authenthicated) and false
(deny all)?

P.P.S. It would be better to accept both patches; I don't know when
I would be able to test Apache config; I remember that I had problems
with it...
-- 
Jakub Narębski
--
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/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 12:20:00PM +0100, Adam Spiers wrote:

 On Thu, Apr 11, 2013 at 02:59:32AM +0100, Adam Spiers wrote:
  +test_expect_success STDBUF 'streaming support for --stdin' '
  +   (
  +   echo one
  +   sleep 2
  +   echo two
  +   ) | stdbuf -oL git check-ignore -v -n --stdin out 
 
 I just noticed that this patch precedes the one in the same series
 which adds -n support.  I'll reorder them accordingly to avoid
 breaking git bisect.

Thanks for noticing. FWIW, I often do this:

  GIT_EDITOR='sed -i /^pick /aexec make test \
  git rebase -i origin/master

on my topics to make sure each commit compiles and passes the tests in
isolation (it will stop on a failure, at which point you can fix up and
git rebase --continue).

It's somewhat annoying, because it runs the whole test suite, even for
commits that are obviously not going to have an impact (e.g., doc
updates, or change to a single test). But it has saved me from
embarrassment many times, when I thought for sure that my commits were
obviously correct. :)

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


Re: [PATCH 1/5] check-ignore: move setup into cmd_check_ignore()

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 12:05:11PM +0100, Adam Spiers wrote:

 On Thu, Apr 11, 2013 at 01:25:53AM -0400, Jeff King wrote:
  On Thu, Apr 11, 2013 at 02:59:31AM +0100, Adam Spiers wrote:
   -static int check_ignore(const char *prefix, const char **pathspec)
   +static int check_ignore(struct path_exclude_check check,
   + const char *prefix, const char **pathspec)
  
  Did you mean to pass the struct by value here? If it is truly a per-path
  [...]

 It's not a per-path value; it's supposed to be reused across checks
 for multiple paths, as explained in the comments above
 last_exclude_matching_path():

Makes sense (I didn't look into it very far, and was just guessing based
on the pass-by-value). Passing a pointer is definitely the right fix,
then.

Thanks.

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


Re: git send-pack: protocol error: bad band #50

2013-04-11 Thread Jonathan Nieder
Hello,

João Joyce wrote:

 I am not sure this is the right place to ask this.

You're in the right place.

[...]
 I am trying to push some files to a server with git push. I have
 configured the server to push the files:
 git remote set-url test ssh://u...@location.com:2200/fullpath/

 but I am getting the following error:
 git send-pack: protocol error: bad band #50
 fatal: The remote end hung up unexpectedly

That means that where git expected to read a binary sideband number,
it instead received the byte \x32 (ASCII '2').

Without more details, it's hard to debug this further.  Can you get
a trace of the communication by setting the envvar
GIT_TRACE_PACKET=/tmp/log on the client?

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


Re: [PATCH v2 2/3] Teach mv to move submodules using a gitfile

2013-04-11 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes:

 Jens Lehmann wrote:
 When moving a submodule which uses a gitfile to point to the git directory
 stored in .git/modules/name of the superproject two changes must be made
 to make the submodule work: the .git file and the core.worktree setting
 must be adjusted to point from work tree to git directory and back.

 Isn't it untrue that the git directory is stored in
 .git/modules/name: it is stored in .git/modules/path/to/module.

 I thought the whole point of this complex scheme was to avoid name
 conflicts with submodules with the same name in other directories.

I think Jens is right on this one.  There are three things the code needs
to know about a submodule: its name, path and URL.

A canonical use case to think about is a project that builds an
appliance:

 * Its zero-th version only has the sources to the userspace for the
   appliance.

 * The first version adds the Linux kernel as a submodule bound at
   kernel/, taking it from git://k.org/linux-2.6.git/.

 * The second version adds a choice to build the appliance with the
   BSD kernel, and the project reorganizes the source tree to have
   Linux kernel at path linux/ and adds the bsd kernel at path bsd/.

 * By the time the third version is released, the URL to the Linux
   has migrated to git://k.org/linux.git/; it is still logically
   the same (i.e. continuation of) the old 2.6.git repository [*1*].

We would want to make it possible to git checkout smoothly between
these four versions.  Switching from v1 to v0 would have to remove
the submodule working tree at kernel/ but the user may want to
switch back to v1 without having to re-download the kernel
submodule, so the kernel/.git repository needs to be stashed away
somewhere.  Somewhere in $GIT_DIR of the superproject, but where?
Switching from v1 to v2 would need to move kernel/ to linux/ and
move kernel/.git to linux/.git.  The design choice made long time
ago (if you recall the collection of old threads I gave you some
time ago, this is what was called Steven's three-level thing) was
to give a stable logical name for the Linux kernel component, so
that no matter where in the working tree the version that happens to
be at the tip of the current branch has it, we know where in the
superproject's .git/modules it is found.

So at the second version when we move the submodule that used to be
at kernel/ to linux/, we move the working tree of it, adjust the
path of the submodule, but keep the name.  And that name gives an
identity to the submodule, and that is what is used as a key inside
$GIT_DIR of the superproject to decide where the repository
(together with its object store) of the submodule is stashed away.


[Footnote]

*1* I mentioned the URL thing only for completeness; it does not
come into play in the checkout scenario, but when you start
thinking about remote interactions, you need to be aware of how that
value and the one copied to the configuration upon submodule init
need to be managed. Which is a separate topic but is an integral
part of the canonical example.

Ideally, a user who has followed along the life of this project
should:

 * first encounter git://k.org/linux-2.6.git/ in v1; git submodule
   init would copy it to her .git/config in the superproject.

 * later notice that .gitmodules has git://k.org/linux.git/ location
   that she hasn't seen for the submodule, and is given a chance to
   have the URL entry updated in her .git/config.

This is becuase even when she checks out an older branch that has
2.6 in .gitmodules, git submodule update _should_ go to the new
URL, not to the defunct 2.6 URL.  We do the copy initially, but do
not do the latter offer a chance to update when seeing a new one
(at least, not yet).


--
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] transport-helper: report errors properly

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

 On Wed, Apr 10, 2013 at 4:15 PM, Jeff King p...@peff.net wrote:
 From: Felipe Contreras felipe.contre...@gmail.com

 If a push fails because the remote-helper died (with
 fast-export), the user does not see any error message. We do

I agree with you that s/does not see/may not see/ would be more
helpful here, so I'll squash it in while queuing.

 In the long run, it may make more sense to propagate the
 error back up to push, so that it can present the usual
 status table and give a nicer message. But this is a much
 simpler fix that can help immediately.

 Yes it might, and it might make sense to rewrite much of this code,
 but that's not relevant.

It is a good reminder for people who later inspect this part of the
code and wonder if it was a conscious design choice not to propagate
the error or just being simple and sufficient for now, I think.
It would help them by making it clear that it is the latter, no?

 ... I think it might
 be possible enforce remote-helpers to implement the done feature,
 and we might want to do that later.

Yes, all these are possible and I think writing it down explicitly
will serve as a reminder for our future selves, I think.

 +   if test -n $GIT_REMOTE_TESTGIT_FAILURE
 +   then
 +   # consume input so fast-export doesn't get SIGPIPE;

 I think this is explanation enough.

 +   # git would also notice that case, but we want
 +   # to make sure we are exercising the later
 +   # error checks

 I don't understand what is being said here. What is that case?

In my first reading, it felt to me that it was natural to interpret
that this is even if we didn't have this loop that avoids killing
fast-export with SIGPIPE, we would notice death of fast-export by
SIGPIPE.
--
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] transport-helper: report errors properly

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

 On Thu, Apr 11, 2013 at 11:59 AM, Jeff King p...@peff.net wrote:

 But I give up on you. I find most of your commit messages lacking in
 details and motivation, making assumptions that the reader is as
 familiar with the code when reading the commit as you are when you wrote
 it. I tried to help by suggesting in review that you elaborate. That
 didn't work. So I tried to help by writing the text myself. But clearly
 I am not going to convince you that it is valuable, even if it requires
 no work at all from you, so I have nothing else to say on the matter.

 Me neither. I picked your solution, but that's not enough, you
 *always* want me to do EXACTLY what you want, and never argue back.

 It's not going to happen. There's nothing wrong with disagreeing.

Heh, it seems that I was late for the party.

Writing only minimally sufficient in the log messages is fine for
your own project. We won't decide nor dictate the policy for your
project for you.

But _this_ project wants its log messages to be understandable by
people who you may disagree with and who may have shorter memory
span than you do.  Disagreeing with that policy is fine.  You need
to learn to disagree but accept to be part of the project.

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


git p4 submit failing

2013-04-11 Thread Christopher Yee Mon
I tried running git p4 submit on a repo that I've been running as an
interim bridge between git and perforce. Multiple people are using the
repo as a remote and its being periodically submitted back to
perforce.

It's been working mostly fine. Then one day out of the blue I get this
error. I can no longer push any git commits to perforce. (This is from
the remote repo which I am pushing back to perforce)

user@hostname:~/Source/code$ git p4 submit -M --export-labels
Perforce checkout for depot path //depot/perforce/workspace/ located
at /home/user/Source/git-p4-area/perforce/workspace/
Synchronizing p4 checkout...
... - file(s) up-to-date.
Applying ffa390f comments in config xml files
//depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for edit
//depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened for edit
//depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened for edit
//depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened for edit
//depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened for edit
error: patch failed: sub/folder/structure/first.xml:1
error: sub/folder/structure/first.xml: patch does not apply
error: patch failed: sub/folder/structure/second.xml:1
error: sub/folder/structure/second.xml: patch does not apply
error: patch failed: sub/folder/structure/third.xml:1
error: sub/folder/structure/third.xml: patch does not apply
error: patch failed: sub/folder/structure/forth.xml:1
error: sub/folder/structure/forth.xml: patch does not apply
error: patch failed: sub/folder/structure/fifth.xml:1
error: sub/folder/structure/fifth.xml: patch does not apply
Unfortunately applying the change failed!
//depot/perforce/workspace/sub/folder/structure/first.xml#1 - was edit, reverted
//depot/perforce/workspace/sub/folder/structure/second.xml#3 - was
edit, reverted
//depot/perforce/workspace/sub/folder/structure/third.xml#3 - was edit, reverted
//depot/perforce/workspace/sub/folder/structure/forth.xml#3 - was edit, reverted
//depot/perforce/workspace/sub/folder/structure/fifth.xml#3 - was edit, reverted
No commits applied.

I thought it could be the .gitattributes setting that I had which was
this at the time was this:

* text eol=lf

My global core.autocrlf setting was also false.

So I remade a new remote repo, and changed core.autocrlf to input and
changed .gitattributes to this

* text=auto

*.php text eol=lf
*.pl text eol=lf
*.pm text eol=lf
*.sh text eol=lf

*.vbs text eol=crlf
*.bat text eol=crlf
*.ps1 text eol=crlf

*.bdb binary
*.mtr binary

Then I started to realize that it could just be the files in the
initial commit that are suspect, because when i made edits to other
files in the repo then tried to push them back with git p4 submit,
those files submitted successfully  But the files in the commit where
I initially got the failure still give me this problem.

Here's what it looks like when I retested with a fresh git repo cloned
from perforce with git p4 clone and tried to do the git p4 submit with
verbose turned on on only one of the suspecting files

user@hostname:/code$ git p4 submit -M --export-labels --verbose
Reading pipe: git name-rev HEAD
Reading pipe: ['git', 'config', 'git-p4.allowSubmit']
Reading pipe: git rev-parse --symbolic --remotes
Reading pipe: git rev-parse p4/master
Reading pipe: git cat-file commit 0457c7589ea679dcc0c9114b34f8f30bc2ee08cf
Reading pipe: git cat-file commit HEAD~0
Reading pipe: git cat-file commit HEAD~1
Reading pipe: ['git', 'config', 'git-p4.conflict']
Origin branch is remotes/p4/master
Reading pipe: ['git', 'config', '--bool', 'git-p4.useclientspec']
Opening pipe: ['p4', '-G', 'where', '//depot/perforce/workspace/...']
Perforce checkout for depot path //depot/perforce/workspace/ located
at /home/user/Source/git-p4-area/perforce/workspace/
Synchronizing p4 checkout...
... - file(s) up-to-date.
Opening pipe: p4 -G opened ...
Reading pipe: ['git', 'rev-list', '--no-merges', 'remotes/p4/master..master']
Reading pipe: ['git', 'config', '--bool', 'git-p4.skipUserNameCheck']
Reading pipe: ['git', 'config', 'git-p4.detectCopies']
Reading pipe: ['git', 'config', '--bool', 'git-p4.detectCopiesHarder']
Reading pipe: ['git', 'show', '-s', '--format=format:%h %s',
'ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e']
Applying ef3b95f making test change
Opening pipe: p4 -G users
Reading pipe: ['git', 'log', '--max-count=1', '--format=%ae',
'ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e']
Reading pipe: git diff-tree -r -M
ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e^
ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e
//depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for edit
stdin:17: trailing whitespace.
!-- comment line 1 --
stdin:18: trailing whitespace.
!-- comment line 2 --
stdin:19: trailing whitespace.
!-- comment line 3 --
error: patch failed: sub/folder/structure/first.xml:1
error: sub/folder/structure/first.xml: patch does not apply
Unfortunately applying the change failed!
Reading pipe: ['git', 'config', 

Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 01:05:12PM +0100, Adam Spiers wrote:

 +test_expect_success 'setup: have stdbuf?' '
 + if which stdbuf /dev/null 21
 + then
 + test_set_prereq STDBUF
 + fi
 +'
 +
 +test_expect_success STDBUF 'streaming support for --stdin' '
 + (
 + echo one
 + sleep 2
 + echo two
 + ) | stdbuf -oL git check-ignore -v -n --stdin out 
 + pid=$! 
 + sleep 1 
 + grep ^\.gitignore:1:oneone out 
 + test $( wc -l out ) = 1 
 + sleep 2 
 + grep ^::   two out 
 + test $( wc -l out ) = 2 
 + ( wait $pid || kill $pid || : ) 2/dev/null
 +'

I always get a little nervous with sleeps in the test suite, as they are
indicative that we are trying to avoid some race condition, which means
that the test can fail when the system is under load, or when a tool
like valgrind is used which drastically alters the timing (e.g., if
check-ignore takes longer than 1 second to produce its answer, we may
fail here).

Is there a simpler way to test this?

Like:

  # Set up a long-running check-ignore connected by pipes.
  mkfifo in out 
  (git check-ignore ... in out ) 

  # We cannot just echo in because check-ignore
  # would get EOF after echo exited; instead we open
  # the descriptor in our shell, and then echo to the
  # fd. We make sure to close it at the end, so that
  # the subprocess does get EOF and dies properly.
  exec 9in 
  test_when_finished exec 9- 

  # Now we can do interactive tests
  echo 9 one 
  read response out 
  test $response = ... 
  echo 9 two 
  read response out 
  test $response = ...

Hmm. Maybe simpler wasn't the right word. :) But it avoids any sleeps or
race conditions.

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


[PATCH] t9903: Don't fail when run from path accessed through symlink

2013-04-11 Thread Torstein Hegge
When the git directory is accessed through a symlink like

  ln -s /tmp/git /tmp/git-symlink
  cd /tmp/git-symlink/t
  make -C ..  ./t9903-bash-prompt.sh

$TRASH_DIRECTORY is /tmp/git-symlink/t/trash directory.t9903-bash-prompt
and $(pwd -P) is /tmp/git/t/trash directory.t9903-bash-prompt.

When __gitdir looks up the path through 'git rev-parse --git-dir', it
will return paths similar to $(pwd -P). This behavior is already tested in
t9903 'gitdir - resulting path avoids symlinks'.

Signed-off-by: Torstein Hegge he...@resisty.net
---
 t/t9903-bash-prompt.sh |   10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh
index 2101d91..e147a8d 100755
--- a/t/t9903-bash-prompt.sh
+++ b/t/t9903-bash-prompt.sh
@@ -59,7 +59,7 @@ test_expect_success 'gitdir - .git directory in cwd' '
 '
 
 test_expect_success 'gitdir - .git directory in parent' '
-   echo $TRASH_DIRECTORY/.git  expected 
+   echo $(pwd -P)/.git  expected 
(
cd subdir/subsubdir 
__gitdir  $actual
@@ -77,7 +77,7 @@ test_expect_success 'gitdir - cwd is a .git directory' '
 '
 
 test_expect_success 'gitdir - parent is a .git directory' '
-   echo $TRASH_DIRECTORY/.git  expected 
+   echo $(pwd -P)/.git  expected 
(
cd .git/refs/heads 
__gitdir  $actual
@@ -115,7 +115,7 @@ test_expect_success 'gitdir - non-existing $GIT_DIR' '
 '
 
 test_expect_success 'gitdir - gitfile in cwd' '
-   echo $TRASH_DIRECTORY/otherrepo/.git  expected 
+   echo $(pwd -P)/otherrepo/.git  expected 
echo gitdir: $TRASH_DIRECTORY/otherrepo/.git  subdir/.git 
test_when_finished rm -f subdir/.git 
(
@@ -126,7 +126,7 @@ test_expect_success 'gitdir - gitfile in cwd' '
 '
 
 test_expect_success 'gitdir - gitfile in parent' '
-   echo $TRASH_DIRECTORY/otherrepo/.git  expected 
+   echo $(pwd -P)/otherrepo/.git  expected 
echo gitdir: $TRASH_DIRECTORY/otherrepo/.git  subdir/.git 
test_when_finished rm -f subdir/.git 
(
@@ -137,7 +137,7 @@ test_expect_success 'gitdir - gitfile in parent' '
 '
 
 test_expect_success SYMLINKS 'gitdir - resulting path avoids symlinks' '
-   echo $TRASH_DIRECTORY/otherrepo/.git  expected 
+   echo $(pwd -P)/otherrepo/.git  expected 
mkdir otherrepo/dir 
test_when_finished rm -rf otherrepo/dir 
ln -s otherrepo/dir link 
-- 
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


Re: [PATCH] t9903: Don't fail when run from path accessed through symlink

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 08:53:22PM +0200, Torstein Hegge wrote:

 When the git directory is accessed through a symlink like
 
   ln -s /tmp/git /tmp/git-symlink
   cd /tmp/git-symlink/t
   make -C ..  ./t9903-bash-prompt.sh
 
 $TRASH_DIRECTORY is /tmp/git-symlink/t/trash directory.t9903-bash-prompt
 and $(pwd -P) is /tmp/git/t/trash directory.t9903-bash-prompt.
 
 When __gitdir looks up the path through 'git rev-parse --git-dir', it
 will return paths similar to $(pwd -P). This behavior is already tested in
 t9903 'gitdir - resulting path avoids symlinks'.

Thanks, this makes sense to me, and is the same solution used in other
scripts (e.g., t2300), so I don't think we have to worry about any
portability concerns with pwd -P.

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


Re: Locating merge that dropped a change

2013-04-11 Thread Junio C Hamano
Kevin Bracey ke...@bracey.fi writes:

 I think I've found the problem. It only doesn't work _if you specify
 the file_.

 Specifically, if I was missing an addition, my first attempt to find
 it would be

   git log -p -m -Saddition file

 If the addition was lost in a merge, that doesn't even show the
 addition, which is surprising, but intentional. The addition isn't
 part of the HEAD version of file, so no point going down that path
 of the merge. Fine. However, I expected this to work:

   git log --full-history -p -m -Saddition file

 But it doesn't. It finds the addition, but _not_ the loss in the merge
 commit.

 But this does work:

   git log -p -m -Saddition

 That really feels like a bug to me. By specifying a file, I've made it
 miss the change, and I can see no way to get the change without making
 it a full-tree operation.
 ... But I'd like some way to find merges
 that drop code in a specified file, and surely --full-history is it?

Yeah, I think that is a bug.

$ echo first file
$ git add file  git commit -m initial
$ git checkout -b side
$ echo second file  git commit -a -m side
$ git checkout -  file  git add file  git commit -m lose
$ git merge -s ours -m lost side
$ git log -p -m --full-history -Ssecond -1 file

does not seem to find the commit that lost the line.


--
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: git-http-backend: anonymous read, authenticated write

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 08:52:56AM +0200, Magnus Therning wrote:

  The documentation should probably make the use of http.receivepack more
  clear in this situation.
 
 I think that'd be good.  The fact that it wasn't until several mails
 into the thread that anyone thought of the http.receivepack setting
 also suggests that its use is a bit un-intuitive (even though it
 probably makes perfect sense and is a good solution).

Yeah, I did not even think of http.receivepack because I have never had
to set it before (it was turned on in the original tests that I built
top of). I have the impression that the anonymous-read/authenticated-write
setup you are using has not been all that commonly used. The example in
the manpage dates back to 2009, but it was only in 2012 that we got a
bug report that the client-side authentication handler has problems with
it.

  So _if_ you fixed it by setting http.receivepack (which I think is the
  simplest thing under Apache, since matching the query string there is
  hard), then you would need a version of git with that fix on the
  client side to actually have git prompt for the password correctly.
 
 Ah, so *that* is the fix that has been mentioned (I haven't bothered
 reading it myself), or are there in fact two fixes that have been
 referred to in the thread?

No, there's only the one fix in git itself (not counting improving the
documentation just now). With the Apache config given in the manual,
clients older than git v1.7.11.7 will not properly handle the 401
response they get mid-way through the push process.

But you do not have to worry about that with your configuration, as you
provide the 401 up-front.

  But your fix under lighttpd is much better, as it asks for the
  credentials up front (which means the client does not go to any work
  creating a packfile just to find out that it does not have access).
 
 Yes, I think it also helps with my particular scenario where new repos
 will be added from time to time.  This way there is no second step,
 after `git init`, that must be remembered.

Yeah, avoiding setting http.receivepack at all is helpful. Though note
that you can also set it in /etc/gitconfig for the whole system at once.

 Thank you very much for taking the time to help me out with this!
 I'll also take a look at the patches you sent, as a dumb simpler user
 I might have something to add, who knows?

You're welcome. I'm glad we got it resolved, and looking over the
documentation patch is appreciated.

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


Re: regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 On Thu, Apr 11, 2013 at 11:11:03AM -0700, Jonathan Nieder wrote:

  -- 8 --
  Subject: [PATCH] daemon: set HOME when we switch to --user
 
 Thanks for taking care of it.  For what it's worth,
 
 Acked-by: Jonathan Nieder jrnie...@gmail.com
 
 I'm not sure whether to keep 96b9e0e (config: treat user and xdg
 config permission problem as errors) in the long run, BTW.  There have
 been multiple reports about dropping privileges and not being able to
 access the old HOME, and I'm not convinced any more that the
 predictability is worth the breakage for such people.  Though checking
 if $HOME is inaccessible and treating that case specially would be
 even worse...
 
 Insights welcome.

 I could go either way. I think 96b9e0e is the right thing to do
 conceptually, but I kind of doubt it was affecting all that many people.
 And though it's _possible_ for it to be a security problem, I find it
 much more likely that the site admin tries to set some config, gets
 annoyed when it doesn't work, and debugs it. So from a practical
 perspective, 96b9e0e may be doing more harm than good, even though it's
 the right thing.

Recent reports in this thread make us think so, I guess.

But reverting 96b9e0e alone would not help these people very much
though.  They will have reams of warning messages in their server
logs, and the way to fix it would be the same as the way to work
around the access_or_die(), namely, to set $HOME to point at a more
appropriate place before running git daemon.

I also have a suspicion that your patch makes things worse for
people who are more adept at these issues around running daemons
than the people who introduced this problem in the first place (eh,
that's us).  It is plausible that they may run multiple instances
of initially root but setuid() to an unprivileged user daemons,
giving each of them a separate play area by setting $HOME to
different values, just for management's ease not necessarily for
security (hence sharing the same unprivileged user), which will be
broken by the patch that unconditionally overrides $HOME.

A trade off to make things slightly easier for one sysadmin by
making another thing impossible to do for another sysadmin does not
sound like a good one.




--
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: regression: 96b9e0e3 config: treat user and xdg config permission problems as errors busted git-daemon

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 12:54:34PM -0700, Junio C Hamano wrote:

  I could go either way. I think 96b9e0e is the right thing to do
  conceptually, but I kind of doubt it was affecting all that many people.
  And though it's _possible_ for it to be a security problem, I find it
  much more likely that the site admin tries to set some config, gets
  annoyed when it doesn't work, and debugs it. So from a practical
  perspective, 96b9e0e may be doing more harm than good, even though it's
  the right thing.
 
 Recent reports in this thread make us think so, I guess.
 
 But reverting 96b9e0e alone would not help these people very much
 though.  They will have reams of warning messages in their server
 logs, and the way to fix it would be the same as the way to work
 around the access_or_die(), namely, to set $HOME to point at a more
 appropriate place before running git daemon.

Yeah, if we revert 96b9e0e, it would only make sense to revert the
warnings, too. Going halfway does not help anyone.

 I also have a suspicion that your patch makes things worse for
 people who are more adept at these issues around running daemons
 than the people who introduced this problem in the first place (eh,
 that's us).  It is plausible that they may run multiple instances
 of initially root but setuid() to an unprivileged user daemons,
 giving each of them a separate play area by setting $HOME to
 different values, just for management's ease not necessarily for
 security (hence sharing the same unprivileged user), which will be
 broken by the patch that unconditionally overrides $HOME.

Yes, we would definitely be breaking them with this patch. I don't know
how common that is. As you noted, it is a bad idea security-wise (if
everything runs as nobody, then the services are not insulated from
each other), but I can perhaps see a case where all git repos are owned
by the git user, but they may be accessed by different config
profiles, which are managed by $HOME.

You could still accomplish the same thing with git by setting
XDG_CONFIG_HOME, though that of course requires effort from the admin.
Sub-programs may not necessarily respect $XDG_CONFIG_HOME, though (e.g.,
anything run from a post-receive hook). On the other hand, people do not
generally push through git-daemon. But that feels like a weak argument.

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


[PATCH v3 5/5] Documentation: add caveats about I/O buffering for check-{attr,ignore}

2013-04-11 Thread Adam Spiers
On Thu, Apr 11, 2013 at 11:09:28AM -0700, Junio C Hamano wrote:
 Reflowing of the text is very much unappreciated X-.  

I very much appreciate the excellent job you do as maintainer; your
attention to detail results in an incredibly high quality project.
However I do occasionally find your communication style unnecessarily
abrasive.  Maybe that's just me.

 It took me five minutes to spot that you only added check-attr and
 check-ignore and forgot to adjust that commit-oriented record to
 an updated reality, where you now have commands that produce
 non-commit-oriented record to the output.

I am sorry for wasting five minutes of your time.  A non-reflowed
version is included inline below, and also at:

https://github.com/aspiers/git/compare/master...git-annex-streaming

 It would have been far simpler to review if it were like this, don't
 you think?

It would have been slightly simpler, yes.  It did occur to me not to
re-flow it, but then one of the lines ended up noticeably shorter, and
as it was a short paragraph, I estimated that you would prefer it
re-flowed.  Clearly I was wrong - not the first time, and it won't be
the last either, since I'm just a flawed human being trying to do my
best in the time available.  The question then arises: how uneven does
a paragraph's right margin have to be in order to justify re-flowing?
I could not find any guidelines in SubmittingPatches or
CodingGuidelines regarding re-flowing of documentation.  With
hindsight, I can now see that it would have been better to skip it on
this occasion, or at least keep the re-flow as a separate commit.

So I apologise again for the mistake, but don't you think it would
have been far more pleasant if instead you'd worded your email
something like this?

Thanks for the patches.  I notice that you unnecessarily re-flowed
the latter half of the GIT_FLUSH paragraph; unfortunately this
meant I had to spend a few extra minutes on the review and almost
missed that commit-oriented is no longer applicable.  In future,
please avoid re-flowing text where possible.

Fortunately I'm not the sensitive sort, but I imagine that there are
others in this community who might be discouraged from contributing
for fear of being on the receiving end of sentences which end with
phrases such as [...] is very much unappreciated X-.  Please don't
underestimate the human factor; common courtesy can make a big
difference.

Thanks,
Adam

-- 8 --
Subject: [PATCH v3 5/5] Documentation: add caveats about I/O buffering for
 check-{attr,ignore}

check-attr and check-ignore have the potential to deadlock callers
which do not read back the output in real-time.  For example, if a
caller writes N paths out and then reads N lines back in, it risks
becoming blocked on write() to check-*, and check-* is blocked on
write back to the caller.  Somebody has to buffer; the pipe buffers
provide some leeway, but they are limited.

Thanks to Peff for pointing this out:

http://article.gmane.org/gmane.comp.version-control.git/220534

Signed-off-by: Adam Spiers g...@adamspiers.org
---
 Documentation/git-check-attr.txt   | 5 +
 Documentation/git-check-ignore.txt | 5 +
 Documentation/git.txt  | 7 ---
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-check-attr.txt b/Documentation/git-check-attr.txt
index 5abdbaa..a7be80d 100644
--- a/Documentation/git-check-attr.txt
+++ b/Documentation/git-check-attr.txt
@@ -56,6 +56,11 @@ being queried and info can be either:
 'set';;when the attribute is defined as true.
 value;;  when a value has been assigned to the attribute.
 
+Buffering happens as documented under the `GIT_FLUSH` option in
+linkgit:git[1].  The caller is responsible for avoiding deadlocks
+caused by overfilling an input buffer or reading from an empty output
+buffer.
+
 EXAMPLES
 
 
diff --git a/Documentation/git-check-ignore.txt 
b/Documentation/git-check-ignore.txt
index 7e3cabc..8e1f7ab 100644
--- a/Documentation/git-check-ignore.txt
+++ b/Documentation/git-check-ignore.txt
@@ -81,6 +81,11 @@ not.  (Without this option, it would be impossible to tell 
whether the
 absence of output for a given file meant that it didn't match any
 pattern, or that the output hadn't been generated yet.)
 
+Buffering happens as documented under the `GIT_FLUSH` option in
+linkgit:git[1].  The caller is responsible for avoiding deadlocks
+caused by overfilling an input buffer or reading from an empty output
+buffer.
+
 EXIT STATUS
 ---
 
diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6a875f2..3258f2c 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -808,9 +808,10 @@ for further details.
 
 'GIT_FLUSH'::
If this environment variable is set to 1, then commands such
-   as 'git blame' (in incremental mode), 'git rev-list', 'git log',
-   and 'git whatchanged' will force a flush of the output stream
-   after each 

Re: git p4 submit failing

2013-04-11 Thread Luke Diamand
Just a thought, but check the files that are failing to see if they've
got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all
sorts of nasty problems.

That's assuming it's definitely not a CRLF line ending problem on Windows.

On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon
christopher.yee...@gmail.com wrote:
 I tried running git p4 submit on a repo that I've been running as an
 interim bridge between git and perforce. Multiple people are using the
 repo as a remote and its being periodically submitted back to
 perforce.

 It's been working mostly fine. Then one day out of the blue I get this
 error. I can no longer push any git commits to perforce. (This is from
 the remote repo which I am pushing back to perforce)

 user@hostname:~/Source/code$ git p4 submit -M --export-labels
 Perforce checkout for depot path //depot/perforce/workspace/ located
 at /home/user/Source/git-p4-area/perforce/workspace/
 Synchronizing p4 checkout...
 ... - file(s) up-to-date.
 Applying ffa390f comments in config xml files
 //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened for edit
 //depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened for edit
 error: patch failed: sub/folder/structure/first.xml:1
 error: sub/folder/structure/first.xml: patch does not apply
 error: patch failed: sub/folder/structure/second.xml:1
 error: sub/folder/structure/second.xml: patch does not apply
 error: patch failed: sub/folder/structure/third.xml:1
 error: sub/folder/structure/third.xml: patch does not apply
 error: patch failed: sub/folder/structure/forth.xml:1
 error: sub/folder/structure/forth.xml: patch does not apply
 error: patch failed: sub/folder/structure/fifth.xml:1
 error: sub/folder/structure/fifth.xml: patch does not apply
 Unfortunately applying the change failed!
 //depot/perforce/workspace/sub/folder/structure/first.xml#1 - was edit, 
 reverted
 //depot/perforce/workspace/sub/folder/structure/second.xml#3 - was
 edit, reverted
 //depot/perforce/workspace/sub/folder/structure/third.xml#3 - was edit, 
 reverted
 //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - was edit, 
 reverted
 //depot/perforce/workspace/sub/folder/structure/fifth.xml#3 - was edit, 
 reverted
 No commits applied.

 I thought it could be the .gitattributes setting that I had which was
 this at the time was this:

 * text eol=lf

 My global core.autocrlf setting was also false.

 So I remade a new remote repo, and changed core.autocrlf to input and
 changed .gitattributes to this

 * text=auto

 *.php text eol=lf
 *.pl text eol=lf
 *.pm text eol=lf
 *.sh text eol=lf

 *.vbs text eol=crlf
 *.bat text eol=crlf
 *.ps1 text eol=crlf

 *.bdb binary
 *.mtr binary

 Then I started to realize that it could just be the files in the
 initial commit that are suspect, because when i made edits to other
 files in the repo then tried to push them back with git p4 submit,
 those files submitted successfully  But the files in the commit where
 I initially got the failure still give me this problem.

 Here's what it looks like when I retested with a fresh git repo cloned
 from perforce with git p4 clone and tried to do the git p4 submit with
 verbose turned on on only one of the suspecting files

 user@hostname:/code$ git p4 submit -M --export-labels --verbose
 Reading pipe: git name-rev HEAD
 Reading pipe: ['git', 'config', 'git-p4.allowSubmit']
 Reading pipe: git rev-parse --symbolic --remotes
 Reading pipe: git rev-parse p4/master
 Reading pipe: git cat-file commit 0457c7589ea679dcc0c9114b34f8f30bc2ee08cf
 Reading pipe: git cat-file commit HEAD~0
 Reading pipe: git cat-file commit HEAD~1
 Reading pipe: ['git', 'config', 'git-p4.conflict']
 Origin branch is remotes/p4/master
 Reading pipe: ['git', 'config', '--bool', 'git-p4.useclientspec']
 Opening pipe: ['p4', '-G', 'where', '//depot/perforce/workspace/...']
 Perforce checkout for depot path //depot/perforce/workspace/ located
 at /home/user/Source/git-p4-area/perforce/workspace/
 Synchronizing p4 checkout...
 ... - file(s) up-to-date.
 Opening pipe: p4 -G opened ...
 Reading pipe: ['git', 'rev-list', '--no-merges', 'remotes/p4/master..master']
 Reading pipe: ['git', 'config', '--bool', 'git-p4.skipUserNameCheck']
 Reading pipe: ['git', 'config', 'git-p4.detectCopies']
 Reading pipe: ['git', 'config', '--bool', 'git-p4.detectCopiesHarder']
 Reading pipe: ['git', 'show', '-s', '--format=format:%h %s',
 'ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e']
 Applying ef3b95f making test change
 Opening pipe: p4 -G users
 Reading pipe: ['git', 'log', '--max-count=1', '--format=%ae',
 'ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e']
 Reading pipe: git diff-tree -r -M
 ef3b95f5fec193fe2612b28e2e3b5e7f8ba9419e^
 

Re: [PATCH v2 4/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Adam Spiers
On Thu, Apr 11, 2013 at 03:11:32PM -0400, Jeff King wrote:
 I always get a little nervous with sleeps in the test suite, as they are
 indicative that we are trying to avoid some race condition, which means
 that the test can fail when the system is under load, or when a tool
 like valgrind is used which drastically alters the timing (e.g., if
 check-ignore takes longer than 1 second to produce its answer, we may
 fail here).

Agreed, especially here where my btrfs filesystems see fit to kindly
freeze my system for a few seconds many times each day :-/

 Is there a simpler way to test this?
 
 Like:
 
   # Set up a long-running check-ignore connected by pipes.
   mkfifo in out 
   (git check-ignore ... in out ) 
 
   # We cannot just echo in because check-ignore
   # would get EOF after echo exited; instead we open
   # the descriptor in our shell, and then echo to the
   # fd. We make sure to close it at the end, so that
   # the subprocess does get EOF and dies properly.
   exec 9in 
   test_when_finished exec 9- 
 
   # Now we can do interactive tests
   echo 9 one 
   read response out 
   test $response = ... 
   echo 9 two 
   read response out 
   test $response = ...
 
 Hmm. Maybe simpler wasn't the right word. :) But it avoids any sleeps or
 race conditions.

The shell source is strong with this one ;-)

Congrats - I first tried with FIFOs (hence my other patch which moves
the PIPE test prerequisite definition into the core framework - the
original intention was to reuse it here) but failed to get it working.
I'll re-roll using your approach.
--
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 4/5] check-ignore: allow incremental streaming of queries via --stdin

2013-04-11 Thread Jeff King
On Thu, Apr 11, 2013 at 09:31:41PM +0100, Adam Spiers wrote:

 The shell source is strong with this one ;-)
 
 Congrats - I first tried with FIFOs (hence my other patch which moves
 the PIPE test prerequisite definition into the core framework - the
 original intention was to reuse it here) but failed to get it working.
 I'll re-roll using your approach.

Thanks. If it make you feel any better, it took about 20 minutes of
experimenting and at least three head-scratching wait, that _should_
have worked moments to get it right. :)

The rest of the series looked good to me, though I admit I did not think
too hard about the --non-matching patch, as it looked like you and
Junio had already given some thought to the output format, which is the
tricky part.

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


[PATCH] Various typofixes

2013-04-11 Thread Stefano Lattarini
Mostly suggested by codespell https://github.com/lucasdemarchi/codespell

Signed-off-by: Stefano Lattarini stefano.lattar...@gmail.com
---
 Documentation/git-credential.txt   |  2 +-
 Documentation/git-remote-ext.txt   |  2 +-
 Documentation/git-svn.txt  |  4 ++--
 Documentation/git-tools.txt|  2 +-
 Documentation/revisions.txt|  2 +-
 Documentation/technical/api-argv-array.txt |  2 +-
 Documentation/technical/api-credentials.txt|  2 +-
 Documentation/technical/api-ref-iteration.txt  |  2 +-
 builtin/apply.c|  6 +++---
 commit.c   |  2 +-
 commit.h   |  2 +-
 compat/nedmalloc/Readme.txt|  2 +-
 compat/nedmalloc/malloc.c.h|  6 +++---
 compat/obstack.h   |  2 +-
 compat/precompose_utf8.c   |  2 +-
 compat/regex/regcomp.c |  4 ++--
 compat/regex/regex.c   |  2 +-
 compat/regex/regex_internal.c  |  6 +++---
 contrib/mw-to-git/git-remote-mediawiki.perl|  6 +++---
 contrib/mw-to-git/t/README |  6 +++---
 contrib/mw-to-git/t/install-wiki/LocalSettings.php |  2 +-
 contrib/mw-to-git/t/t9360-mw-to-git-clone.sh   |  2 +-
 contrib/mw-to-git/t/t9362-mw-to-git-utf8.sh| 10 +-
 contrib/subtree/t/t7900-subtree.sh |  2 +-
 diff.c |  2 +-
 git-add--interactive.perl  |  2 +-
 git-cvsserver.perl |  4 ++--
 git-gui/lib/blame.tcl  |  2 +-
 git-gui/lib/index.tcl  |  2 +-
 git-gui/lib/spellcheck.tcl |  4 ++--
 git-quiltimport.sh |  2 +-
 gitweb/INSTALL |  2 +-
 gitweb/gitweb.perl |  6 +++---
 kwset.c|  4 ++--
 perl/Git.pm|  2 +-
 perl/Git/I18N.pm   |  2 +-
 perl/private-Error.pm  |  2 +-
 po/README  |  6 +++---
 sequencer.c|  2 +-
 t/t1006-cat-file.sh|  2 +-
 t/t3511-cherry-pick-x.sh   |  4 ++--
 t/t3701-add-interactive.sh |  2 +-
 t/t4014-format-patch.sh|  6 +++---
 t/t4124-apply-ws-rule.sh   |  2 +-
 t/t6030-bisect-porcelain.sh|  2 +-
 t/t7601-merge-pull-config.sh   |  2 +-
 t/t7610-mergetool.sh   |  2 +-
 t/t9001-send-email.sh  |  4 ++--
 transport-helper.c |  2 +-
 transport.h|  2 +-
 xdiff/xdiffi.c |  2 +-
 xdiff/xhistogram.c |  2 +-
 52 files changed, 79 insertions(+), 79 deletions(-)

diff --git a/Documentation/git-credential.txt b/Documentation/git-credential.txt
index 472f00f..7da0f13 100644
--- a/Documentation/git-credential.txt
+++ b/Documentation/git-credential.txt
@@ -56,7 +56,7 @@ For example, if we want a password for
 `https://example.com/foo.git`, we might generate the following
 credential description (don't forget the blank line at the end; it
 tells `git credential` that the application finished feeding all the
-infomation it has):
+information it has):
 
 protocol=https
 host=example.com
diff --git a/Documentation/git-remote-ext.txt b/Documentation/git-remote-ext.txt
index 58b7fac..8cfc748 100644
--- a/Documentation/git-remote-ext.txt
+++ b/Documentation/git-remote-ext.txt
@@ -86,7 +86,7 @@ begins with `ext::`.  Examples:
edit .ssh/config.
 
 ext::socat -t3600 - ABSTRACT-CONNECT:/git-server %G/somerepo::
-   Represents repository with path /somerepo accessable over
+   Represents repository with path /somerepo accessible over
git protocol at abstract namespace address /git-server.
 
 ext::git-server-alias foo %G/repo::
diff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt
index 1b8b649..7706d41 100644
--- a/Documentation/git-svn.txt
+++ b/Documentation/git-svn.txt
@@ -245,7 +245,7 @@ first have already been pushed into SVN.
patch), all (accept all patches), or quit.
+
'git svn dcommit' returns immediately if answer if no or quit, 
without
-   commiting anything to SVN.
+   committing anything to SVN.
 
 'branch'::
Create a branch in the SVN repository.
@@ -856,7 +856,7 @@ 

  1   2   >