Authentication with e-mail address as user name for HTTPS remote

2013-08-31 Thread Patrick Atoon
Hello,

I run into a problem with command line git on Linux.

The remote git server I try to clone from uses HTTPS as a protocol and
requires full
fledged e-mail addresses for a user name in its authentication. In
TortoiseGit (with
winstore) or SourceTree, the user name and password are asked and
stored. But on the
command line I'm stuck, unable to provide the user name.

Here is what happens. First try cloning without specifying the user name:

---8---
$ git clone https://git.server.com/git/test.git
Initialized empty Git repository in /tmp/git/test/.git/
error: The requested URL returned error: 401 Authorization Required
while accessing
https://git.server.com/git/test.git/nfo/refs

fatal: HTTP request failed
---8---

I couldn't find a --username flag or something similar for the git
command, so my next
try was to incorporate the user name in the URL, basic auth style. The
repo URL looks
something like this:

https://user.em...@emaildomain.com@git.server.com/git/test.git

Note the double @ there, it is bound to cause trouble.
This is what happened:

---8---
$ git clone https://user.em...@emaildomain.com@git.server.com/git/test.git
Initialized empty Git repository in /tmp/git/test/.git/
Password:
error: Couldn't resolve host 'emaildomain@git.server.com' while accessing
https://user.em...@emaildomain.com@git.server.com/git/test.git/info/refs

fatal: HTTP request failed
---8---

It appears the @ was picked up as an indication that the URL
contains the username,
however the URL was split at the wrong position. The splitting
algorithm doesn't seem to
take into account that a user name might contain an @.

My request: please modify command line git to support full fledged
e-mail addresses as
user names in HTTPS requests.

I'm not a C adept, still I browsed the code a bit and bumped into
transport.c's
transport_anonymize_url method. I think that code could be modified
to do a better job
of splitting the URL, which might then avoid problems down the line.

Thanks for reading this far.

Regards,

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


Re: Officially start moving to the term 'staging area'

2013-08-31 Thread René Scharfe

Am 29.08.2013 22:36, schrieb Felipe Contreras:

On Thu, Aug 29, 2013 at 3:03 PM, René Scharfe l@web.de wrote:

If you have a --work-tree option then parseopt accepts --work as well,
unless it's ambiguous, i.e. another option starts with --work, too.  So you
can have a descriptive, extra-long option and type just a few characters at
the same time.


Right, but what do we use in the documentation? Writing --work-tree in
the 'git reset' table for example would be rather ugly. I'm fine with
--work-tree, but I think it would be weird to have short-hands in the
documentation, although not entirely bad.


I don't see what's so ugly about it.

The git command itself has a --work-tree parameter for specifying the 
location of the checked-out files, however.  It could be confusing to 
have the same parameter do different things:


$ git reset --work-tree=/some/where reset --work-tree

Perhaps a note in the documentation is enough to clear this up.


In general I think that the full long option should be mentioned in 
synopsis and description, while abbreviated parameters could be used in 
an example.  The ability to understand unambiguous shortened options is 
a general feature and doesn't have to be explained in the manpage for a 
specific command.


NB: It would be nice to have more commands use parseopt, for that 
feature alone.


René

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


Re: Officially start moving to the term 'staging area'

2013-08-31 Thread Felipe Contreras
On Sat, Aug 31, 2013 at 2:46 AM, René Scharfe l@web.de wrote:
 Am 29.08.2013 22:36, schrieb Felipe Contreras:

 On Thu, Aug 29, 2013 at 3:03 PM, René Scharfe l@web.de wrote:

 If you have a --work-tree option then parseopt accepts --work as well,
 unless it's ambiguous, i.e. another option starts with --work, too.  So
 you
 can have a descriptive, extra-long option and type just a few characters
 at
 the same time.


 Right, but what do we use in the documentation? Writing --work-tree in
 the 'git reset' table for example would be rather ugly. I'm fine with
 --work-tree, but I think it would be weird to have short-hands in the
 documentation, although not entirely bad.

 I don't see what's so ugly about it.

Maybe not so much.

 The git command itself has a --work-tree parameter for specifying the
 location of the checked-out files, however.  It could be confusing to have
 the same parameter do different things:

 $ git reset --work-tree=/some/where reset --work-tree

I don't see what's confusing about that, one option is for git core,
the other is for the subcommand.

 In general I think that the full long option should be mentioned in synopsis
 and description, while abbreviated parameters could be used in an example.
 The ability to understand unambiguous shortened options is a general feature
 and doesn't have to be explained in the manpage for a specific command.

 NB: It would be nice to have more commands use parseopt, for that feature
 alone.

Indeed.

-- 
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 6/6] pull: trivial cleanup

2013-08-31 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 10:58 PM, Junio C Hamano gits...@pobox.com wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 There's no need to remove 'refs/heads/' yet again.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  git-pull.sh | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

 diff --git a/git-pull.sh b/git-pull.sh
 index f0df41c..3bdcbfd 100755
 --- a/git-pull.sh
 +++ b/git-pull.sh
 @@ -166,7 +166,6 @@ error_on_no_merge_candidates () {
   op_prep=with
   fi

 - curr_branch=${curr_branch#refs/heads/}

 The code assumes that at this point $curr_branch has the result of
 git symbolic-ref -q HEAD it did at the beginning, before it entered
 in the command line parsing loop.  But immediately after it, the
 code sets up $curr_branch_short for the value this code computes.

   upstream=$(git config branch.$curr_branch.merge)
   remote=$(git config branch.$curr_branch.remote)

 So it appears to me that the above two lines that are not updated
 would introduce a regression.  Am I missing something trivial?

Yes, I'm not exactly sure where from which branch I got this change,
but those lines were removed. They should be updated.

-- 
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: Authentication with e-mail address as user name for HTTPS remote

2013-08-31 Thread Jeff King
On Sat, Aug 31, 2013 at 08:52:06AM +0200, Patrick Atoon wrote:

 Here is what happens. First try cloning without specifying the user name:
 
 ---8---
 $ git clone https://git.server.com/git/test.git
 Initialized empty Git repository in /tmp/git/test/.git/
 error: The requested URL returned error: 401 Authorization Required
 while accessing
 https://git.server.com/git/test.git/nfo/refs
 
 fatal: HTTP request failed

That should be prompting you. What version of git are you using?

 I couldn't find a --username flag or something similar for the git
 command, so my next try was to incorporate the user name in the URL,
 basic auth style.

Yes, that's the correct way to do it from the command line.

If you are running v1.7.9 and later, you can also put this in your
config:

  [credential https://git.server.com;]
username = user.em...@emaildomain.com

to automatically use that username for the particular domain.

 https://user.em...@emaildomain.com@git.server.com/git/test.git
 
 Note the double @ there, it is bound to cause trouble.

Yes. You probably want to escape it like:

  https://user.email%40emaildomain@git.server.com/git/test.git

In theory we could parse from the right-hand side of the hostname and
realize that only the right-most @ is the username separator (under
the assumption that hostnames can never contain an @). I don't know if
that would violate the standard or not. However, it is not even git that
does the actual parsing in this case, but rather libcurl.

-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] branch: use $curr_branch_short more

2013-08-31 Thread René Scharfe
One of the first things git-pull.sh does is setting $curr_branch to
the target of HEAD and $curr_branch_short to the same but with the
leading refs/heads/ removed.  Use $curr_branch_short in the function
error_on_no_merge_candidates instead of removing the prefix from
$curr_branch directly.

The only other use of $curr_branch in that function doesn't have to
be replaced with $curr_branch_short because it just checks if the
string is empty.  That property is the same with or without the prefix
unless HEAD points to refs/heads/ alone, which is invalid.

Noticed-by: Felipe Contreras felipe.contre...@gmail.com
Signed-off-by: Rene Scharfe l@web.de
---
 git-pull.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index f0df41c..d8b2708 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -166,9 +166,8 @@ error_on_no_merge_candidates () {
op_prep=with
fi
 
-   curr_branch=${curr_branch#refs/heads/}
-   upstream=$(git config branch.$curr_branch.merge)
-   remote=$(git config branch.$curr_branch.remote)
+   upstream=$(git config branch.$curr_branch_short.merge)
+   remote=$(git config branch.$curr_branch_short.remote)
 
if [ $# -gt 1 ]; then
if [ $rebase = true ]; then
-- 
1.8.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] branch: use $curr_branch_short more

2013-08-31 Thread Felipe Contreras
 Subject: branch: use $curr_branch_short more

Why? I don't think that summary explains the reason for being for this
patch, also, it starts with branch: instead of pull:

Subject: pull: trivial simplification

With that summary, people would have an easier time figuring out if
they need to read more about the patch or not.

-- 
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] branch: use $curr_branch_short more

2013-08-31 Thread René Scharfe

Am 31.08.2013 10:22, schrieb Felipe Contreras:

Subject: branch: use $curr_branch_short more


Why? I don't think that summary explains the reason for being for this
patch, also, it starts with branch: instead of pull:


You're right about branch vs. pull.  I'll better go back to bed. ~_~


Subject: pull: trivial simplification

With that summary, people would have an easier time figuring out if
they need to read more about the patch or not.


trivial simplification is too generic; we could have lots of them.  A 
summary should describe the change.  Its low complexity can be derived 
from it -- using an existing variable a bit more is not very exciting.


But I wouldn't call that patch trivial because its correctness depends 
on code outside of its shown context.


The reason for the patch isn't mentioned explicitly.  Perhaps it should 
be.  I felt that using something that's already there instead of 
recreating it is motivation alone.


René

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


Bug in git rebase --continue in v1.8.4

2013-08-31 Thread Christoph Mallon
Hi,

if I run rebase --continue (e.g. after a conflict resolution), then the rebase 
always ends with this error message:
It seems that there is already a rebase-apply directory, and
I wonder if you are in the middle of another rebase.  If that is the
case, please try
git rebase (--continue | --abort | --skip)
If that is not the case, please
rm -fr /home/tron/gitRebaseTest/test/.git/rebase-apply
and run me again.  I am stopping in case you still have something
valuable there.

This happens on git v1.8.4 on FreeBSD. It is fine with v1.8.3.4. It seems to be 
caused by a1549e1049439386b9fd643fae236ad3ba649650, specifically this hunk:
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -7,12 +7,12 @@ case $action in
 continue)
  git am --resolved --resolvemsg=$resolvemsg 
  move_to_original_branch
- exit
+ return
  ;;
 skip)
  git am --skip --resolvemsg=$resolvemsg 

Attached is a test script for this problem.

Regards
Christoph
#! /bin/sh
set -eux

git init test
cd test

echo a  file
git add .
git commit -m a

git branch -t test

echo b  file
git add .
git commit -m b

git checkout test
echo c  file
git add .
git commit -m c

! git rebase
git checkout --theirs .
git add .
git rebase --continue


Re: [PATCH] remote-hg: skip ill-formed references

2013-08-31 Thread Max Kirillov
Felipe Contreras felipe.contreras at 
gmail.com writes:
 Which repository triggered this?

Tha was some of the vim repositories, upstream 
https://code.google.com/p/vim/ or debian 
anonscm.debian.org/hg/pkg-vim/vim, or both. 
They contain tags with ~ symbol.

I don't have any experience with bazaar yet, so 
cannot say much about it.

Br,
-- 
Max

--
To unsubscribe from this list: send the line 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-rebase --continue eats commits

2013-08-31 Thread Madhu

Don't know if this has been resolved-by-debate here before, But if
`git-rebase' finds a hitherto untracked file in the worktree, which it
wants to create, it then aborts asking you to remove the file.  So if
you remove it and ask git to continue with `git-rebase --continue', it
then deletes the commit that was being applied from the branch. ---Madhu


(setenv TDIR /dev/shm/foo/)

mkdir -pv $TDIR  cd $TDIR  git-init
(cd $TDIR  echo a  a  git add a  git commit -m a)
(cd $TDIR  echo b  b  git add b  git commit -m b)
(cd $TDIR  echo c  c  git add c  git commit -m c)
(cd $TDIR  EDITOR=sed -i -e 's/^pick/edit/' git rebase -i 'HEAD^^')
(cd $TDIR  echo fubar  c)
(cd $TDIR  git-rebase --continue)

Rebasing (2/2)
error: The following untracked working tree files would be overwritten by merge:
c
Please move or remove them before you can merge.
Aborting
Could not apply ddd6f51...

(cd $TDIR  rm -fv c)
(cd $TDIR  git-rebase --continue)

Rebasing (2/2)
Successfully rebased and updated refs/heads/master.

(cd $TDIR  git log)

;; commit `c' is gone
--
To unsubscribe from this list: send the line 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] gitweb: Fix the author initials in blame for non-ASCII names

2013-08-31 Thread Ævar Arnfjörð Bjarmason
I did. I just clumsily sent out the wrong patch. I.e. tested it
manually on another system, and then fat-fingered $fh instead of $fd.

Should I send another patch or do you want to just fix this one up?

On Fri, Aug 30, 2013 at 8:13 PM, Junio C Hamano gits...@pobox.com wrote:
 Junio C Hamano gits...@pobox.com writes:

 Ævar Arnfjörð Bjarmason  ava...@gmail.com writes:

 Acked-by: Jakub Narębski jna...@gmail.com
 Tested-by: Ævar Arnfjörð Bjarmason ava...@gmail.com
 Tested-by: Simon Ruderich si...@ruderich.org
 ---
 +++ b/gitweb/gitweb.perl
 @@ -6631,6 +6631,7 @@ sub git_blame_common {
 ...
 +binmode $fh, ':utf8';


 [Fri Aug 30 17:48:17 2013] gitweb.perl: Global symbol $fh requires
 explicit package name at 
 /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl line 6634.
 [Fri Aug 30 17:48:17 2013] gitweb.perl: Execution of 
 /home/gitster/w/buildfarm/next/t/../gitweb/gitweb.perl aborted due to 
 compilation errors.

 I think in this function the filehandle is called $fd, not $fh.  Has
 any of you really tested this???
--
To unsubscribe from this list: send the line 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/8] refs: factor delete_ref loose ref step into a helper

2013-08-31 Thread Michael Haggerty
On 08/30/2013 08:12 PM, Brad King wrote:
 Factor loose ref deletion into helper function delete_ref_loose to allow
 later use elsewhere.
 
 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  refs.c |   22 +++---
  1 file changed, 15 insertions(+), 7 deletions(-)
 
 diff --git a/refs.c b/refs.c
 index 2e755b4..5dd86ee 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -2450,14 +2450,9 @@ static int repack_without_ref(const char *refname)
   return commit_packed_refs();
  }
  
 -int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 +static int delete_ref_loose(struct ref_lock *lock, int flag)
  {
 - struct ref_lock *lock;
 - int err, i = 0, ret = 0, flag = 0;
 -
 - lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
 - if (!lock)
 - return 1;
 + int err, i, ret = 0;
   if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
   /* loose */
   i = strlen(lock-lk-filename) - 5; /* .lock */
 @@ -2468,6 +2463,19 @@ int delete_ref(const char *refname, const unsigned 
 char *sha1, int delopt)
  
   lock-lk-filename[i] = '.';
   }
 + return ret;
 +}
 +


At first glance it is odd that delete_ref_loose() takes a (struct
ref_lock *) argument but only actually uses lock-lk-filename.  But I
guess that the function is so specific to the contents of struct
ref_lock and indeed struct lock_file that it wouldn't make sense to pass
it only the filename attribute.  So OK.

Given that ret is only returned, you could restore the filename before
the if statement and replace the ret variable with an immediate return
statement:

static int delete_ref_loose(struct ref_lock *lock, int flag)
{
if (!(flag  REF_ISPACKED) || flag  REF_ISSYMREF) {
/* loose */
int err, i = strlen(lock-lk-filename) - 5; /* .lock */

lock-lk-filename[i] = 0;
err = unlink_or_warn(lock-lk-filename);
lock-lk-filename[i] = '.';
if (err  errno != ENOENT)
return 1;
}
return 0;
}

 +int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
 +{
 + struct ref_lock *lock;
 + int ret = 0, flag = 0;
 +
 + lock = lock_ref_sha1_basic(refname, sha1, delopt, flag);
 + if (!lock)
 + return 1;
 + ret |= delete_ref_loose(lock, flag);
 +
   /* removing the loose one could have resurrected an earlier
* packed one.  Also, if it was not loose we need to repack
* without it.
 

Otherwise looks good.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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] gitweb: Fix the author initials in blame for non-ASCII names

2013-08-31 Thread Jakub Narębski
On Fri, Aug 30, 2013 at 11:39 PM, Kyle J. McKay mack...@gmail.com wrote:
 On Aug 30, 2013, at 11:13, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 Ævar Arnfjörð Bjarmason  ava...@gmail.com writes:

 +   binmode $fh, ':utf8';

 What happens if the author name is written in ISO-8859-1 instead of UTF-8 in
 the actual commit object itself?

 I'm pretty sure I've seen this where older commits have a ISO-8859-1 author
 name and then newer commits have a UTF-8 version of the same author's name.

 In fact, in the git repository itself, look at commit 0cb3f80d (UTF-8) and
 commit 7eb93c89 (ISO-8859-1) to see this in action.

Well, then you have a problem, though it is only with old history (before
introduction of encoding header in commit object).

Better and more complete solution would be to use to_utf8() function
instead of 'utf8' layer, which when finding invalid UTF-8 sequence uses
$fallback_encoding (by default latin1, i.e. ISO-8859-1) instead.


In my TODO list is creating PerlIO layer ':utf8-with-fallback' which would
replace all those to_utf8() calls...

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


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

2013-08-31 Thread brian m. carlson
On Fri, Aug 30, 2013 at 10:08:53PM +0200, Jens Lehmann wrote:
 Am 30.08.2013 21:51, schrieb Jens Lehmann:
  Am 30.08.2013 21:40, schrieb Jens Lehmann:
  Am 29.08.2013 23:23, schrieb Matthieu Moy:
  Jens Lehmann jens.lehm...@web.de writes:
 
  Am 29.08.2013 15:05, schrieb Matthieu Moy:
  Because of the missing quotes around $for_status, it seems the test is
  unconditionnaly true:
 
  $ test -n t ; echo $?
  0
  $ test -n   ; echo $?
  0
 
 Right you are, I did not notice the missing  in my review. Looks like
 we also should add one or more tests making sure that submodule summary
 and status never honor the ignore settings.

How do we want to handle this?  I can send a reroll and include some
new tests, but if this code is going away, then there's no point.

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


signature.asc
Description: Digital signature


Re: [PATCH] branch: use $curr_branch_short more

2013-08-31 Thread Felipe Contreras
On Sat, Aug 31, 2013 at 5:28 AM, René Scharfe l@web.de wrote:
 Am 31.08.2013 11:22, schrieb Felipe Contreras:

 On Sat, Aug 31, 2013 at 4:11 AM, René Scharfe l@web.de wrote:

 Subject: pull: trivial simplification

 With that summary, people would have an easier time figuring out if
 they need to read more about the patch or not.



 trivial simplification is too generic; we could have lots of them.


 No, we can have only one, otherwise it would say simplificationS.


 I was too terse again, let me rephrase that: We could have lots of commits
 that fit the same description if we used such a generic one.

Yes, but they are all trivial, and they all simply the code.

 A summary should describe the change.


 You can never fully describe the change, only the diff does that.

 For example use $curr_branch_short more does not tell me anything
 about the extent of the changes, is it used in one more place? two?
 one hundred? Moreover, how exactly is it used more? Is some
 refactoring needed?

 And it still doesn't answer the most important question any summary
 should answer: why? Why use $curr_branch_short more?

 A summary doesn't have to contain lots of details.  The what is important,
 the why can be explained in the commit message.

A summary should contain as much information that would allow me to
skip the commit message as possible.

If I can't tell from the summary if I can safely skip the commit
message, the summary is not doing a good job.

trivial simplification explains the what, and the why at the
same time, and allows most people to skip the commit message, thus is
a good summary.

 Its low complexity can be derived from
 it -- using an existing variable a bit more is not very exciting.


 You didn't say a bit more you said more. And yes, the complexity
 can be derived from the summary, but not from this one.

 But I wouldn't call that patch trivial because its correctness depends on
 code outside of its shown context.


 Correctness is a separate question from triviality, and the
 correctness can only be assessed by looking at the actual patch.

 The patch can be both trivial and wrong.


 Probably too terse again, let's say it differently: Only a patch whose
 correctness can be judged without looking outside of the three lines of
 context it includes qualifies as trivial in my book.  The patch in question
 is not trivial because you can't see the value of $curr_branch_short just by
 looking at the diff.

Again, triviality and correctness are two separate different things.
The patch is trivial even if you can't judge it's correctness.

To me, what you are describing is an obvious patch, not a trivial one.
An obvious patch is so obvious that you can judge it's correctness
easily by looking at the diff, a trivial one is of little importance.

 The reason for the patch isn't mentioned explicitly.  Perhaps it should
 be.
 I felt that using something that's already there instead of recreating it
 is
 motivation alone.


 Why? Because it simplifies the code. That's the real answer.

 I don't disagree.

Yet your commit message doesn't explain that anywhere.

-- 
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 0/2] branch: improve verbose option

2013-08-31 Thread Felipe Contreras
On Sat, Aug 31, 2013 at 9:41 AM, Philip Oakley philipoak...@iee.org wrote:
 From: Felipe Contreras felipe.contre...@gmail.com

Showing ahead/behind is
 not as important,


 I still find this useful - if it's up to date I don't need reminding of
 which remote / upstream tracking branch it's configured against ;-)

Whether or not it's up to date depends on which is the upstream
tracking branch. What if you see ahead 1, and you think it's not up
to date, but it turns out it's tracking the wrong branch?

 These two bits of information, while related are separate. If anything I
 think I'd prefer both, but then again optimisations are still good.

So you can keep using 'git branch -vv'.

-- 
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] remote-hg: skip ill-formed references

2013-08-31 Thread Felipe Contreras
On Sat, Aug 31, 2013 at 8:58 AM, Max Kirillov m...@max630.net wrote:
 Felipe Contreras felipe.contreras at
 gmail.com writes:
 Which repository triggered this?

 Tha was some of the vim repositories, upstream
 https://code.google.com/p/vim/ or debian
 anonscm.debian.org/hg/pkg-vim/vim, or both.
 They contain tags with ~ symbol.

Thanks.

 I don't have any experience with bazaar yet, so
 cannot say much about it.

That code is independent from bazaar. the ref_is_valid() method should
return True or False depending on whether Git thinks it's a valid ref
or not, it's independent from Bazaar or Mercurial, and all
remote-helpers could share the same one.

So this is what I have in mind:

--- a/git-remote-hg.py
+++ b/git-remote-hg.py
@@ -617,6 +617,9 @@ def list_head(repo, cur):
 print @refs/heads/%s HEAD % head
 g_head = (head, node)

+def ref_is_valid(name):
+return not True in [c in name for c in '~^: \\']
+
 def do_list(parser):
 repo = parser.repo
 for bmark, node in bookmarks.listbookmarks(repo).iteritems():
@@ -635,15 +638,24 @@ def do_list(parser):

 if track_branches:
 for branch in branches:
-print ? refs/heads/branches/%s % gitref(branch)
+branch = gitref(branch)
+if not ref_is_valid(branch):
+continue
+print ? refs/heads/branches/%s % branch

 for bmark in bmarks:
-print ? refs/heads/%s % gitref(bmark)
+bmark = gitref(bmark)
+if not ref_is_valid(bmark):
+continue
+print ? refs/heads/%s % bmark

 for tag, node in repo.tagslist():
 if tag == 'tip':
 continue
-print ? refs/tags/%s % gitref(tag)
+tag = gitref(tag)
+if not ref_is_valid(tag):
+continue
+print ? refs/tags/%s % tag

 print

-- 
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] remote-hg: skip ill-formed references

2013-08-31 Thread Felipe Contreras
On Sat, Aug 31, 2013 at 8:58 AM, Max Kirillov m...@max630.net wrote:
 Felipe Contreras felipe.contreras at
 gmail.com writes:
 Which repository triggered this?

 Tha was some of the vim repositories, upstream
 https://code.google.com/p/vim/ or debian
 anonscm.debian.org/hg/pkg-vim/vim, or both.
 They contain tags with ~ symbol.

I can clone both fine. This is what I get with the debian one:

error: * Ignoring funny ref 'refs/tags/debian-7.2.436+hg~e12b9d992389-1' locally
error: * Ignoring funny ref
'refs/tags/debian-7.2.436+hg~e12b9d992389-1+' locally
error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-1' locally
error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-2' locally
error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-3' locally
error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-4' locally
error: * Ignoring funny ref 'refs/tags/debian-7.2.438+hg~d44112feb815-5' locally
error: * Ignoring funny ref 'refs/tags/debian-7.2.445+hg~cb94c42c0e1a-1' locally
error: * Ignoring funny ref
'refs/tags/debian-7.3b.20100720+hg~7b7508ee56f1-1' locally
error: * Ignoring funny ref
'refs/tags/debian-7.3f.20100812+hg~20e83abf88b1-1' locally
error: * Ignoring funny ref 'refs/tags/debian-7.3.000+hg~ee53a39d5896-1' locally
error: * Ignoring funny ref 'refs/tags/debian-7.3.035+hg~8fdc1210-1' locally
error: * Ignoring funny ref 'refs/tags/debian-7.3.154+hg~74503f6ee649-1' locally
error: * Ignoring funny ref 'refs/tags/debian-7.3.154+hg~74503f6ee649-2' locally
error: * Ignoring funny ref 'refs/tags/debian-7.3.547-6~bpo60+1' locally
error: * Ignoring funny ref 'refs/tags/debian-7.3.547-7~bpo60+1' locally

Maybe you need a newer version of Git. I'm using v1.8.4.

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

2013-08-31 Thread Michael Haggerty
On 08/30/2013 08:12 PM, Brad King wrote:
 Add 'struct ref_update' to encode the information needed to update or
 delete a ref (name, new sha1, optional old sha1, no-deref flag).  Add
 function 'update_refs' accepting an array of updates to perform.  First
 sort the input array to order locks consistently everywhere and reject
 multiple updates to the same ref.  Then acquire locks on all refs with
 verified old values.  Then update or delete all refs accordingly.  Fail
 if any one lock cannot be obtained or any one old value does not match.
 
 Though the refs themeselves cannot be modified together in a single

s/themeselves/themselves/

 atomic transaction, this function does enable some useful semantics.
 For example, a caller may create a new branch starting from the head of
 another branch and rewind the original branch at the same time.  This
 transfers ownership of commits between branches without risk of losing
 commits added to the original branch by a concurrent process, or risk of
 a concurrent process creating the new branch first.
 
 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  refs.c |  121 
 
  refs.h |   14 
  2 files changed, 135 insertions(+)
 
 diff --git a/refs.c b/refs.c
 index 3bcd26e..901a38a 100644
 --- a/refs.c
 +++ b/refs.c
 @@ -3238,6 +3238,127 @@ int update_ref(const char *action, const char 
 *refname,
   return update_ref_write(action, refname, sha1, lock, onerr);
  }
  
 +static int ref_update_compare(const void *r1, const void *r2)
 +{
 + struct ref_update *u1 = (struct ref_update *)(r1);
 + struct ref_update *u2 = (struct ref_update *)(r2);

If you declare u1 and u2 to be const struct ref_update * (i.e., add
const), then you have const correctness and don't need the explicit
casts.  (And the parentheses around r1 and r2 are superfluous in any case.)

 + int ret;

Style: we usually put a blank line between variable declarations and the
first line of code.

 + ret = strcmp(u1-ref_name, u2-ref_name);
 + if (ret)
 + return ret;
 + ret = hashcmp(u1-new_sha1, u2-new_sha1);
 + if (ret)
 + return ret;
 + ret = hashcmp(u1-old_sha1, u2-old_sha1);
 + if (ret)
 + return ret;
 + ret = u1-flags - u2-flags;
 + if (ret)
 + return ret;
 + return u1-have_old - u2-have_old;
 +}

Is there a need to compare more than ref_name?  If two entries are found
with the same name, then ref_update_reject_duplicates() will error out
anyway.  So the relative order among entries with the same name is
irrelevant.  I think it would be OK to return 0 for any entries with the
same ref_name, even if they differ in other fields.

 +
 +static int ref_update_reject_duplicates(struct ref_update *updates, int n,
 + enum action_on_err onerr)
 +{
 + int i;
 + for (i = 1; i  n; ++i)
 + if (!strcmp(updates[i - 1].ref_name, updates[i].ref_name))
 + break;

The error handling code could be right here instead of the break
statement, removing the need for the if conditional.

 + if (i  n) {
 + const char *str = Multiple updates for ref '%s' not allowed.;
 + switch (onerr) {
 + case MSG_ON_ERR: error(str, updates[i].ref_name); break;
 + case DIE_ON_ERR: die(str, updates[i].ref_name); break;
 + case QUIET_ON_ERR: break;
 + }
 + return 1;
 + }
 + return 0;
 +}
 +
 +int update_refs(const char *action, const struct ref_update *updates_orig,
 + int n, enum action_on_err onerr)
 +{
 + int ret = 0, delnum = 0, i;
 + struct ref_update *updates;
 + int *types;
 + struct ref_lock **locks;
 + const char **delnames;
 +
 + if (!updates_orig || !n)
 + return 0;
 +
 + /* Allocate work space: */
 + updates = xmalloc(sizeof(struct ref_update) * n);

It seems preferred here to write

updates = xmalloc(sizeof(*updates) * n);

as this will continue to work if the type of updates is ever changed.
Similarly for the next lines.

 + types = xmalloc(sizeof(int) * n);
 + locks = xmalloc(sizeof(struct ref_lock *) * n);
 + delnames = xmalloc(sizeof(const char *) * n);

An alternative to managing separate arrays to hold types and locks would
be to include the scratch space in struct ref_update and document it
for internal use only; need not be initialized by caller.  On the one
hand it's ugly to cruft up the interface with internal implementation
details; on the other hand there is precedent for this sort of thing
(e.g., ref_lock::force_write or lock_file::on_list) and it would
simplify the code.

 +
 + /* Copy, sort, and reject duplicate refs: */
 + memcpy(updates, updates_orig, sizeof(struct ref_update) * n);
 + qsort(updates, n, sizeof(struct ref_update), ref_update_compare);

You could save some space and memory 

Re: [PATCH v2 7/8] update-ref: support multiple simultaneous updates

2013-08-31 Thread Michael Haggerty
On 08/30/2013 08:12 PM, Brad King wrote:
 Add a --stdin signature to read update instructions from standard input
 and apply multiple ref updates together.  Use an input format that
 supports any update that could be specified via the command-line,
 including object names like 'branch:path with space'.
 
 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  Documentation/git-update-ref.txt |   21 ++-
  builtin/update-ref.c |  121 
 +-
  2 files changed, 140 insertions(+), 2 deletions(-)
 
 diff --git a/Documentation/git-update-ref.txt 
 b/Documentation/git-update-ref.txt
 index 0df13ff..295d0bb 100644
 --- a/Documentation/git-update-ref.txt
 +++ b/Documentation/git-update-ref.txt
 @@ -8,7 +8,7 @@ git-update-ref - Update the object name stored in a ref safely
  SYNOPSIS
  
  [verse]
 -'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] ref 
 newvalue [oldvalue])
 +'git update-ref' [-m reason] (-d ref [oldvalue] | [--no-deref] ref 
 newvalue [oldvalue] | --stdin)
  
  DESCRIPTION
  ---
 @@ -58,6 +58,25 @@ archive by creating a symlink tree).
  With `-d` flag, it deletes the named ref after verifying it
  still contains oldvalue.
  
 +With `--stdin`, update-ref reads instructions from standard input and
 +performs all modifications together.  Empty lines are ignored.
 +Each non-empty line is parsed as whitespace-separated arguments.
 +Use single-quotes to enclose whitespace and backslashes and an
 +unquoted backslash to escape a single quote.  Specify updates with
 +lines of the form:
 +
 + [--no-deref] [--] ref newvalue [oldvalue]
 +
 +Lines of any other format or a repeated ref produce an error.
 +Specify a zero newvalue to delete a ref and/or a zero oldvalue
 +to make sure that a ref not exist.  Use either 40 0 or the
 +empty string (written as '') to specify a zero value.
 +
 +If all refs can be locked with matching oldvalues
 +simultaneously all modifications are performed.  Otherwise, no

Comma after simultaneously.

 +modifications are performed.  Note that while each individual
 +ref is updated or deleted atomically, a concurrent reader may
 +still see a subset of the modifications.
  
  Logging Updates
  ---
 diff --git a/builtin/update-ref.c b/builtin/update-ref.c
 index 51d2684..eb8db85 100644
 --- a/builtin/update-ref.c
 +++ b/builtin/update-ref.c
 @@ -6,19 +6,129 @@
  static const char * const git_update_ref_usage[] = {
   N_(git update-ref [options] -d refname [oldval]),
   N_(git update-ref [options]refname newval [oldval]),
 + N_(git update-ref [options] --stdin),
   NULL
  };
  
 +static const char blank[] =  \t\r\n;
 +
 +static int updates_size;
 +static int updates_count;
 +static struct ref_update *updates;
 +
 +static const char* update_refs_stdin_next_arg(const char* next,
 +   struct strbuf *arg)
 +{
 + /* Skip leading whitespace: */
 + while (isspace(*next))
 + ++next;
 +
 + /* Return NULL when no argument is found: */
 + if (!*next)
 + return NULL;
 +
 + /* Parse the argument: */
 + strbuf_reset(arg);
 + for (;;) {
 + char c = *next;
 + if (!c || isspace(c))
 + break;
 + ++next;
 + if (c == '\'') {
 + size_t len = strcspn(next, ');

I agree with Junio that your quoting rules are peculiar.

 + if (!next[len])
 + die(unterminated single-quote: '%s, next);
 + strbuf_add(arg, next, len);
 + next += len + 1;
 + continue;
 + }
 + if (c == '\\') {
 + if (*next == '\'')
 + c = *next++;
 + else
 + die(unquoted backslash not escaping 
 + single-quote: \\%s, next);
 + }
 + strbuf_addch(arg, c);
 + }
 + return next;
 +}
 +
 +static void update_refs_stdin(const char *line)
 +{
 + int options = 1, flags = 0, argc = 0;
 + char *argv[3] = {0, 0, 0};
 + struct strbuf arg = STRBUF_INIT;
 + struct ref_update *update;
 + const char *next = line;
 +
 + /* Skip blank lines: */
 + if (!line[0])
 + return;
 +
 + /* Parse arguments on this line: */
 + while ((next = update_refs_stdin_next_arg(next, arg)) != NULL) {
 + if (options  arg.buf[0] == '-')
 + if (!strcmp(arg.buf, --no-deref))
 + flags |= REF_NODEREF;
 + else if (!strcmp(arg.buf, --))
 + options = 0;
 + else
 + die(unknown option %s, arg.buf);
 + else if (argc = 3)
 + die(too many arguments on line: %s, line);
 + else {
 +

Re: [PATCH v2 0/8] Multiple simultaneously locked ref updates

2013-08-31 Thread Michael Haggerty
On 08/30/2013 08:11 PM, Brad King wrote:
 Here is the second revision of a series to support locking multiple
 refs at the same time to update all of them consistently.  The first
 series can be found at $gmane/233260.  This revision is ready to
 consider for integration.

I'm very interested in this area and I regret that I have been so
invisible lately.  I definitely like the way your changes are going.

I have been doing some work in the same neighborhood and our work
overlaps somewhat.  Namely, it is incorrect that delete_ref() currently
deletes the loose ref before rewriting packed-refs, because it can cause
a simultaneous reader to see the packed value for a moment in time, and
the packed value might not even point to a valid object anymore.  In
fact, the current version is even worse: it deletes the loose reference
before even locking the packed-refs file, so if the packed refs file
cannot be rewritten (e.g., because it is locked by another process) then
the reference is permanently left in a corrupt state.

On the other hand, writing the packed-refs file first would also be
incorrect, because a pack-refs process could jump in before the loose
refs file was deleted, read the loose value, and write it to the new
packed-refs file.  The result would be that the first process would
think that it had deleted the reference but it would still exist (not
such a catastrophe, but incorrect nevertheless).

The solution that I have been working on is to first lock *both* the
loose and packed refs files, then rewrite the packed-refs file *but
retain a lock on it*, then rewrite the loose-ref file and release its
lock, then release the lock on the packed-refs file.  By retaining the
lock on the packed-refs file during the whole transaction, we prevent
another process from trying to pack the refs before our reference is
completely deleted.  This requires the file-locking API to be enhanced
to allow a file to be replaced by its new version while still retaining
a lock on the file.

Your code has the same bug as the original (it's not your fault!) so I
think it will eventually have to be fixed to look something like

acquire lock on packed-refs

for reference in ref_updates:
lock reference
if old sha1 known:
verify old sha1 is still current

for reference in ref_updates:
if reference should be created/modified:
modify reference
release lock on reference

delete references from packed-refs file and activate new
version of the file *but retain a lock on the
packed-refs file*
for reference in ref_updates:
if reference should be deleted:
delete loose version of reference
release lock on reference

release lock on packed-refs file

This is really all just for your information; there is certainly no
obligation for you to fix this pre-existing problem.  And I'm working on
it anyway; if you happen to be interested you can view my current
work-in-progress on GitHub (though it still doesn't work!):

https://github.com/mhagger/git/tree/WIP-delete-ref-locking

Feedback would of course be welcome.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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


[PATCH v3 05/11] Documentation/replace: add Creating Replacement Objects section

2013-08-31 Thread Christian Couder
There were no hints in the documentation about how to create
replacement objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 16 
 1 file changed, 16 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index aa66d27..736b48c 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -64,6 +64,19 @@ OPTIONS
Typing git replace without arguments, also lists all replace
refs.
 
+CREATING REPLACEMENT OBJECTS
+
+
+linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
+linkgit:git-rebase[1], among other git commands, can be used to create
+replacement objects from existing objects.
+
+If you want to replace many blobs, trees or commits that are part of a
+string of commits, you may just want to create a replacement string of
+commits and then only replace the commit at the tip of the target
+string of commits with the commit at the tip of the replacement string
+of commits.
+
 BUGS
 
 Comparing blobs or trees that have been replaced with those that
@@ -76,6 +89,9 @@ pending objects.
 
 SEE ALSO
 
+linkgit:git-hash-object[1]
+linkgit:git-filter-branch[1]
+linkgit:git-rebase[1]
 linkgit:git-tag[1]
 linkgit:git-branch[1]
 linkgit:git[1]
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 00/11] Check replacement object type and minor updates

2013-08-31 Thread Christian Couder
In this new version of the series, the only change in the 5 first
patches, is a small change in the commit message of patch 1.

Patches from 6/11 to 11/11 are all new:
- 6/11, 7/11 and 8/11 are about bypassing the type check
if -f is used
- 9/11, 10/11 and 11/11 are about adding long option names

Christian Couder (11):
  replace: forbid replacing an object with one of a different type
  Documentation/replace: state that objects must be of the same type
  t6050-replace: test that objects are of the same type
  t6050-replace: add test to clean up all the replace refs
  Documentation/replace: add Creating Replacement Objects section
  replace: bypass the type check if -f option is used
  Documentation/replace: tell that -f option bypasses the type check
  t6050-replace: check that -f option bypasses the type check
  replace: allow long option names
  Documentation/replace: list long option names
  t6050-replace: use some long option names

 Documentation/git-replace.txt | 28 +---
 builtin/replace.c | 16 +---
 t/t6050-replace.sh| 31 ---
 3 files changed, 66 insertions(+), 9 deletions(-)

-- 
1.8.4.rc1.31.g530f5ce.dirty

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


[PATCH v3 01/11] replace: forbid replacing an object with one of a different type

2013-08-31 Thread Christian Couder
Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

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

* Annotated tags contain the type of the tagged object.

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

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

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..9a94769 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, const char 
*replace_ref,
  int force)
 {
unsigned char object[20], prev[20], repl[20];
+   enum object_type obj_type, repl_type;
char ref[PATH_MAX];
struct ref_lock *lock;
 
@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
if (check_refname_format(ref, 0))
die('%s' is not a valid ref name., ref);
 
+   obj_type = sha1_object_info(object, NULL);
+   repl_type = sha1_object_info(repl, NULL);
+   if (obj_type != repl_type)
+   die(Objects must be of the same type.\n
+   '%s' points to a replaced object of type '%s'\n
+   while '%s' points to a replacement object of type '%s'.,
+   object_ref, typename(obj_type),
+   replace_ref, typename(repl_type));
+
if (read_ref(ref, prev))
hashclr(prev);
else if (!force)
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 03/11] t6050-replace: test that objects are of the same type

2013-08-31 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 13 +
 1 file changed, 13 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index decdc33..5c352c4 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -263,4 +263,17 @@ test_expect_success 'not just commits' '
test_cmp file.replaced file
 '
 
+test_expect_success 'replaced and replacement objects must be of the same 
type' '
+   test_must_fail git replace mytag $HASH1 2err 
+   grep mytag. points to a replaced object of type .tag err 
+   grep $HASH1. points to a replacement object of type .commit err 
+   test_must_fail git replace HEAD^{tree} HEAD~1 2err 
+   grep HEAD^{tree}. points to a replaced object of type .tree err 
+   grep HEAD~1. points to a replacement object of type .commit err 
+   BLOB=$(git rev-parse :file) 
+   test_must_fail git replace HEAD^ $BLOB 2err 
+   grep HEAD^. points to a replaced object of type .commit err 
+   grep $BLOB. points to a replacement object of type .blob err
+'
+
 test_done
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 04/11] t6050-replace: add test to clean up all the replace refs

2013-08-31 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 5c352c4..05be228 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,4 +276,10 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep $BLOB. points to a replacement object of type .blob err
 '
 
+test_expect_success 'replace ref cleanup' '
+   test -n $(git replace) 
+   git replace -d $(git replace) 
+   test -z $(git replace)
+'
+
 test_done
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 02/11] Documentation/replace: state that objects must be of the same type

2013-08-31 Thread Christian Couder
A previous patch ensures that both the replaced and the replacement
objects passed to git replace must be of the same type.

While at it state that there is no other restriction on both objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index e0b4057..aa66d27 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -20,6 +20,9 @@ The name of the 'replace' reference is the SHA-1 of the 
object that is
 replaced. The content of the 'replace' reference is the SHA-1 of the
 replacement object.
 
+The replaced object and the replacement object must be of the same type.
+There is no other restriction on them.
+
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
 Replacement references will be used by default by all Git commands
@@ -69,9 +72,7 @@ go back to a replaced commit will move the branch to the 
replacement
 commit instead of the replaced commit.
 
 There may be other problems when using 'git rev-list' related to
-pending objects. And of course things may break if an object of one
-type is replaced by an object of another type (for example a blob
-replaced by a commit).
+pending objects.
 
 SEE ALSO
 
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 06/11] replace: bypass the type check if -f option is used

2013-08-31 Thread Christian Couder
If -f option, which means '--force', is used, we can allow an object
to be replaced with one of a different type, as the user should know
what (s)he is doing.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 9a94769..95736d9 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -103,7 +103,7 @@ static int replace_object(const char *object_ref, const 
char *replace_ref,
 
obj_type = sha1_object_info(object, NULL);
repl_type = sha1_object_info(repl, NULL);
-   if (obj_type != repl_type)
+   if (!force  obj_type != repl_type)
die(Objects must be of the same type.\n
'%s' points to a replaced object of type '%s'\n
while '%s' points to a replacement object of type '%s'.,
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 08/11] t6050-replace: check that -f option bypasses the type check

2013-08-31 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 05be228..0b07a0b 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -276,6 +276,12 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
grep $BLOB. points to a replacement object of type .blob err
 '
 
+test_expect_success '-f option bypasses the type check' '
+   git replace -f mytag $HASH1 2err 
+   git replace -f HEAD^{tree} HEAD~1 2err 
+   git replace -f HEAD^ $BLOB 2err
+'
+
 test_expect_success 'replace ref cleanup' '
test -n $(git replace) 
git replace -d $(git replace) 
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 10/11] Documentation/replace: list long option names

2013-08-31 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index a2bd2ee..414000e 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -54,13 +54,16 @@ achieve the same effect as the `--no-replace-objects` 
option.
 OPTIONS
 ---
 -f::
+--force::
If an existing replace ref for the same object exists, it will
be overwritten (instead of failing).
 
 -d::
+--delete::
Delete existing replace refs for the given objects.
 
 -l pattern::
+--list pattern::
List replace refs for objects that match the given pattern (or
all if no pattern is given).
Typing git replace without arguments, also lists all replace
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 11/11] t6050-replace: use some long option names

2013-08-31 Thread Christian Couder
So that they are tested a litlle bit too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 t/t6050-replace.sh | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 0b07a0b..5dc26e8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and deleting' '
  test $HASH2 = $(git replace -l) 
  test $HASH2 = $(git replace) 
  aa=${HASH2%??} 
- test $HASH2 = $(git replace -l $aa*) 
+ test $HASH2 = $(git replace --list $aa*) 
  test_must_fail git replace -d $R 
- test_must_fail git replace -d 
+ test_must_fail git replace --delete 
  test_must_fail git replace -l -d $HASH2 
  git replace -d $HASH2 
  git show $HASH2 | grep A U Thor 
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' '
  git show $HASH2 | grep O Thor 
  test_must_fail git replace $HASH2 $R 
  git replace -f $HASH2 $R 
- test_must_fail git replace -f 
+ test_must_fail git replace --force 
  test $HASH2 = $(git replace)
 '
 
@@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement objects must 
be of the same type'
 
 test_expect_success '-f option bypasses the type check' '
git replace -f mytag $HASH1 2err 
-   git replace -f HEAD^{tree} HEAD~1 2err 
+   git replace --force HEAD^{tree} HEAD~1 2err 
git replace -f HEAD^ $BLOB 2err
 '
 
-- 
1.8.4.rc1.31.g530f5ce.dirty

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


[PATCH v3 09/11] replace: allow long option names

2013-08-31 Thread Christian Couder
It is now standard practice in Git to have both short and long option
names. So let's give a long option name to the git replace options too.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 builtin/replace.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 95736d9..d4d1b75 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -128,9 +128,9 @@ int cmd_replace(int argc, const char **argv, const char 
*prefix)
 {
int list = 0, delete = 0, force = 0;
struct option options[] = {
-   OPT_BOOLEAN('l', NULL, list, N_(list replace refs)),
-   OPT_BOOLEAN('d', NULL, delete, N_(delete replace refs)),
-   OPT_BOOLEAN('f', NULL, force, N_(replace the ref if it 
exists)),
+   OPT_BOOLEAN('l', list, list, N_(list replace refs)),
+   OPT_BOOLEAN('d', delete, delete, N_(delete replace refs)),
+   OPT_BOOLEAN('f', force, force, N_(replace the ref if it 
exists)),
OPT_END()
};
 
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


[PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check

2013-08-31 Thread Christian Couder
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
 Documentation/git-replace.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-replace.txt b/Documentation/git-replace.txt
index 736b48c..a2bd2ee 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -21,10 +21,12 @@ replaced. The content of the 'replace' reference is the 
SHA-1 of the
 replacement object.
 
 The replaced object and the replacement object must be of the same type.
-There is no other restriction on them.
+This restriction can be bypassed using `-f`.
 
 Unless `-f` is given, the 'replace' reference must not yet exist.
 
+There is no other restriction on the replaced and replacement objects.
+
 Replacement references will be used by default by all Git commands
 except those doing reachability traversal (prune, pack transfer and
 fsck).
-- 
1.8.4.rc1.31.g530f5ce.dirty


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


Re: [PATCH] git send-email: include [anything]-by: signatures

2013-08-31 Thread Michael S. Tsirkin
On Mon, Aug 26, 2013 at 07:57:47PM +0300, Michael S. Tsirkin wrote:
 Consider [anything]-by: a valid signature.
 This includes Tested-by: Acked-by: Reviewed-by: etc.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

Ping.
Any opinion on whether this change is acceptable?

 ---
  git-send-email.perl | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/git-send-email.perl b/git-send-email.perl
 index ecbf56f..bb9093b 100755
 --- a/git-send-email.perl
 +++ b/git-send-email.perl
 @@ -1359,7 +1359,7 @@ foreach my $t (@files) {
   # Now parse the message body
   while($fh) {
   $message .=  $_;
 - if (/^(Signed-off-by|Cc): (.*)$/i) {
 + if (/^([A-Za-z-]*-by|Cc): (.*)$/i) {
   chomp;
   my ($what, $c) = ($1, $2);
   chomp $c;
 -- 
 MST
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line 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] revision: add --except option

2013-08-31 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 2:56 AM, Johannes Sixt j.s...@viscovery.net wrote:
 Am 8/30/2013 8:32, schrieb Junio C Hamano:
 If you have a history where

  - branches master and maint point at commit A;
  - branch next points at commit B that is a descendant of A; and
  - there are tags X and Y pointing at commits that are ahead of B
or behind A

 i.e.

   XABY

 what are the desired semantics for these?

 I think the simplest were that --except trumps everything and means
 whatever else I say, do as if I did not mention the following.

Actually, my patch is almost there, I attach the necessary changed
below to make everything work. I've added debug prints to show what
it's actually doing:

  (1) --branches --except maint

 = master next

= master next

  (2) --all --not --branches --except maint

 = X Y --not master next

= ^master ^next X Y HEAD

  (3) ^master next --except maint

 = ^master next

= ^master next

 (4) Y next --except master next --not --branches

 this = Y --not maint
 or this = Y --not maint master next

= Y

Remember that maint (or rather ^maint) is after --except.

 (5) --branches --except ^master

 this = maint next
 or this = maint master next ^master
 or error(--except does not allow negated revisions)

= maint next

Here's the diff:

--- a/revision.c
+++ b/revision.c
@@ -2578,7 +2578,11 @@ void reset_revision_walk(void)
 static int refcmp(const char *a, const char *b)
 {
a = prettify_refname(a);
+   if (*a == '^')
+   a++;
b = prettify_refname(b);
+   if (*b == '^')
+   b++;
return strcmp(a, b);
 }

@@ -2594,13 +2598,14 @@ int prepare_revision_walk(struct rev_info *revs)
revs-pending.alloc = 0;
revs-pending.objects = NULL;
while (--nr = 0) {
-   struct commit *commit = handle_commit(revs, e-item, e-name);
+   struct commit *commit;
for (i = 0; i  revs-cmdline.nr; i++) {
struct rev_cmdline_entry *ce;
ce = revs-cmdline.rev[i];
if ((ce-flags  SKIP)  !refcmp(ce-name, e-name))
goto next;
}
+   commit = handle_commit(revs, e-item, e-name);
if (commit) {
if (!(commit-object.flags  SEEN)) {
commit-object.flags |= SEEN;

-- 
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] revision: introduce --exclude=glob to tame wildcards

2013-08-31 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 6:55 PM, Junio C Hamano gits...@pobox.com wrote:
 People often find git log --branches etc. that includes _all_
 branches is cumbersome to use when they want to grab most but except
 some.  The same applies to --tags, --all and --glob.

This idea looks very familiar, from the wording of this commit message
it seems you came with the idea out of nowhere. Did you?

 Teach the revision machinery to remember patterns, and then upon the
 next such a globbing option, exclude those that match the pattern.

 With this, I can view only my integration branches (e.g. maint,
 master, etc.) without topic branches, which are named after two
 letters from primary authors' names, slash and topic name.

 git rev-list --no-walk --exclude=??/* --branches |
 git name-rev --refs refs/heads/* --stdin

My patch does the trick with:

--branches --except --glob='heads/??/*'

-- 
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] diff: add a config option to control orderfile

2013-08-31 Thread Michael S. Tsirkin
I always want my diffs to show header files first,
then .c files, then the rest. Make it possible to
set orderfile though a config option to achieve this.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 diff.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/diff.c b/diff.c
index 208094f..cca0767 100644
--- a/diff.c
+++ b/diff.c
@@ -264,6 +264,9 @@ int git_diff_basic_config(const char *var, const char 
*value, void *cb)
return 0;
}
 
+if (!strcmp(var, diff.orderfile))
+return git_config_string(default_diff_options.orderfile, var, 
value);
+
if (!prefixcmp(var, submodule.))
return parse_submodule_config_option(var, value);
 
-- 
MST
--
To unsubscribe from this list: send the line 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] remote-hg: skip ill-formed references

2013-08-31 Thread Max Kirillov
On Sat, Aug 31, 2013 at 12:57:34PM -0500, Felipe Contreras wrote:
 On Sat, Aug 31, 2013 at 8:58 AM, Max Kirillov m...@max630.net wrote:
 Tha was some of the vim repositories, upstream
 https://code.google.com/p/vim/ or debian
 anonscm.debian.org/hg/pkg-vim/vim, or both.
 They contain tags with ~ symbol.
 
 I can clone both fine. This is what I get with the debian one:
 
 error: * Ignoring funny ref 'refs/tags/debian-7.2.436+hg~e12b9d992389-1' 
 locally

Yes, it really works with the new version. I used 1.7.10
before.

Since it is fixed already, just forget it. Sorry, should
have checked it with the latest version.

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


Proposal From Mr. Gibson Mouka.

2013-08-31 Thread Mr. Gibson Mouka


Dear Friend,
 
I decided to contact you to help me actualize this business for the mutual 
benefit of both our families. I am the Auditing and Accounting section manager 
in a bank, there is one of our customers who have made fixed deposit of sum of 
($39.5)million for 7 years and upon maturity; I discovered that he died after a 
brief illness without any next of kin on his file. I am contacting you for 
joining hands with the honesty and truth to ensure that the fund is transferred 
into your bank account.
 
Please reply quickly enough to enable me decide how to proceed with further 
details.
  
Regards.
Mr. Gibson Mouka.
--
To unsubscribe from this list: send the line 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] revision: add --except option

2013-08-31 Thread Felipe Contreras
So that it's possible to remove certain refs from the list without
removing the objects that are referenced by other refs.

For example this repository:

  C (crap)
  B (test)
  A (HEAD, master)

When using '--branches --except crap':

  B (test)
  A (HEAD, master)

But when using '--branches --not crap' nothing will come out.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---

I don't like the complexity of recalculate_flag(), but it seems to be the most
straight-forward way of making --not and --except work.

 Documentation/git-rev-parse.txt|  6 
 contrib/completion/git-completion.bash |  2 +-
 revision.c | 52 -
 revision.h |  3 +-
 t/t6112-rev-list-except.sh | 60 ++
 5 files changed, 120 insertions(+), 3 deletions(-)
 create mode 100755 t/t6112-rev-list-except.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 2b126c0..fe5cc6b 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -110,6 +110,12 @@ can be used.
strip '{caret}' prefix from the object names that already have
one.
 
+--except::
+   Skip the following object names. For example:
+   '--branches --except master' will show all the branches, except master.
+   This differs from --not in that --except will still show the object, if
+   they are referenced by another object name.
+
 --symbolic::
Usually the object names are output in SHA-1 form (with
possible '{caret}' prefix); this option makes them output in a
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 5da920e..aed8c12 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1386,7 +1386,7 @@ _git_ls_tree ()
 
 # Options that go well for log, shortlog and gitk
 __git_log_common_options=
-   --not --all
+   --not --except --all
--branches --tags --remotes
--first-parent --merges --no-merges
--max-count=
diff --git a/revision.c b/revision.c
index 84ccc05..a7664dd 100644
--- a/revision.c
+++ b/revision.c
@@ -1984,6 +1984,8 @@ static int handle_revision_pseudo_opt(const char 
*submodule,
handle_reflog(revs, *flags);
} else if (!strcmp(arg, --not)) {
*flags ^= UNINTERESTING | BOTTOM;
+   } else if (!strcmp(arg, --except)) {
+   *flags |= SKIP;
} else if (!strcmp(arg, --no-walk)) {
revs-no_walk = REVISION_WALK_NO_WALK_SORTED;
} else if (!prefixcmp(arg, --no-walk=)) {
@@ -2573,24 +2575,72 @@ void reset_revision_walk(void)
clear_object_flags(SEEN | ADDED | SHOWN);
 }
 
+static int refcmp(const char *a, const char *b)
+{
+   a = prettify_refname(a);
+   if (*a == '^')
+   a++;
+   b = prettify_refname(b);
+   if (*b == '^')
+   b++;
+   return strcmp(a, b);
+}
+
+static int recalculate_flag(struct rev_info *revs, unsigned char *sha1, const 
char *name)
+{
+   int flags = 0;
+   int i;
+   for (i = 0; i  revs-cmdline.nr; i++) {
+   struct object *object;
+   struct rev_cmdline_entry *ce;
+   ce = revs-cmdline.rev[i];
+   object = ce-item;
+   while (object-type == OBJ_TAG) {
+   struct tag *tag = (struct tag *) object;
+   if (!tag-tagged)
+   continue;
+   object = parse_object(tag-tagged-sha1);
+   if (!object)
+   continue;
+   }
+   if (hashcmp(object-sha1, sha1))
+   continue;
+   if (!strcmp(ce-name, name))
+   continue;
+   flags |= ce-flags;
+   }
+   return flags;
+}
+
 int prepare_revision_walk(struct rev_info *revs)
 {
int nr = revs-pending.nr;
struct object_array_entry *e, *list;
struct commit_list **next = revs-commits;
+   int i;
 
e = list = revs-pending.objects;
revs-pending.nr = 0;
revs-pending.alloc = 0;
revs-pending.objects = NULL;
while (--nr = 0) {
-   struct commit *commit = handle_commit(revs, e-item, e-name);
+   struct commit *commit;
+   for (i = 0; i  revs-cmdline.nr; i++) {
+   struct rev_cmdline_entry *ce;
+   ce = revs-cmdline.rev[i];
+   if ((ce-flags  SKIP)  !refcmp(ce-name, e-name)) {
+   e-item-flags = recalculate_flag(revs, 
e-item-sha1, ce-name);
+   goto next;
+   }
+   }
+   commit = handle_commit(revs, e-item, e-name);
if (commit) {
if 

Re: [PATCH v3 01/11] replace: forbid replacing an object with one of a different type

2013-08-31 Thread Philip Oakley

Sorry for not replying earlier in the series.

From: Christian Couder chrisc...@tuxfamily.org

Users replacing an object with one of a different type were not
prevented to do so, even if it was obvious, and stated in the doc,
that bad things would result from doing that.

To avoid mistakes, it is better to just forbid that though.

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


Isn't this a recursion problem? Taken in that order one unravels the 
whole DAG.


However if considered in the reverse direction, one can replace an 
existing object within the DAG with a carefully crafted alternative of 
the same type, but which then wrongly references other dangling objects 
which are then replaced by objects which have the right type (this last 
replacement requires -f force).



That's because:

* Annotated tags contain the type of the tagged object.

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

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

* Blobs don't point at anything.

The doc will be updated in a later patch.

Acked-by: Philip Oakley philipoak...@iee.org
Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
builtin/replace.c | 10 ++
1 file changed, 10 insertions(+)

diff --git a/builtin/replace.c b/builtin/replace.c
index 59d3115..9a94769 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -85,6 +85,7 @@ static int replace_object(const char *object_ref, 
const char *replace_ref,

   int force)
{
 unsigned char object[20], prev[20], repl[20];
+ enum object_type obj_type, repl_type;
 char ref[PATH_MAX];
 struct ref_lock *lock;

@@ -100,6 +101,15 @@ static int replace_object(const char *object_ref, 
const char *replace_ref,

 if (check_refname_format(ref, 0))
 die('%s' is not a valid ref name., ref);

+ obj_type = sha1_object_info(object, NULL);
+ repl_type = sha1_object_info(repl, NULL);
+ if (obj_type != repl_type)
+ die(Objects must be of the same type.\n
+ '%s' points to a replaced object of type '%s'\n
+ while '%s' points to a replacement object of type '%s'.,
+ object_ref, typename(obj_type),
+ replace_ref, typename(repl_type));
+
 if (read_ref(ref, prev))
 hashclr(prev);
 else if (!force)
--
1.8.4.rc1.31.g530f5ce.dirty





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


Re: [PATCH v3 07/11] Documentation/replace: tell that -f option bypasses the type check

2013-08-31 Thread Philip Oakley

From: Christian Couder chrisc...@tuxfamily.org

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Documentation/git-replace.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-replace.txt 
b/Documentation/git-replace.txt

index 736b48c..a2bd2ee 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -21,10 +21,12 @@ replaced. The content of the 'replace' reference 
is the SHA-1 of the

replacement object.

The replaced object and the replacement object must be of the same 
type.

-There is no other restriction on them.
+This restriction can be bypassed using `-f`.

Unless `-f` is given, the 'replace' reference must not yet exist.

+There is no other restriction on the replaced and replacement 
objects.


Is this trying to allude to the fact that merge commits may be exchanged 
with non-merge commits? I strongly believe that this ability to exchange 
merge and non-merge commits should be stated _explicitly_ to counteract 
the false beliefs that are listed out on the internet.


It's probably better stated in a separate patch for that explicit 
purpose to avoid mixed messages within this commit.



+
Replacement references will be used by default by all Git commands
except those doing reachability traversal (prune, pack transfer and
fsck).
--


--
To unsubscribe from this list: send the line 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 05/11] Documentation/replace: add Creating Replacement Objects section

2013-08-31 Thread Philip Oakley

From: Christian Couder chrisc...@tuxfamily.org

There were no hints in the documentation about how to create
replacement objects.

Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
Documentation/git-replace.txt | 16 
1 file changed, 16 insertions(+)

diff --git a/Documentation/git-replace.txt 
b/Documentation/git-replace.txt

index aa66d27..736b48c 100644
--- a/Documentation/git-replace.txt
+++ b/Documentation/git-replace.txt
@@ -64,6 +64,19 @@ OPTIONS
 Typing git replace without arguments, also lists all replace
 refs.

+CREATING REPLACEMENT OBJECTS
+
+
+linkgit:git-filter-branch[1], linkgit:git-hash-object[1] and
+linkgit:git-rebase[1],


Let's not forget the obvious 'git commit' or 'git merge' on a temporary 
branch for creating a replacement commit.


In particular we need to have covered the alternate to a graft of A B 
C (i.e. A is now a merge of B  C) if we are to deprecate grafts with 
any conviction. (https://git.wiki.kernel.org/index.php/GraftPoint)



among other git commands, can be used to create
+replacement objects from existing objects.
+
+If you want to replace many blobs, trees or commits that are part of 
a
+string of commits, you may just want to create a replacement string 
of

+commits and then only replace the commit at the tip of the target
+string of commits with the commit at the tip of the replacement 
string

+of commits.
+
BUGS

Comparing blobs or trees that have been replaced with those that
@@ -76,6 +89,9 @@ pending objects.

SEE ALSO

+linkgit:git-hash-object[1]
+linkgit:git-filter-branch[1]
+linkgit:git-rebase[1]
linkgit:git-tag[1]
linkgit:git-branch[1]
linkgit:git[1]
--
1.8.4.rc1.31.g530f5ce.dirty





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


Re: [PATCH v3 11/11] t6050-replace: use some long option names

2013-08-31 Thread Philip Oakley

From: Christian Couder chrisc...@tuxfamily.org
Subject: [PATCH v3 11/11] t6050-replace: use some long option names



So that they are tested a litlle bit too.

s /litlle/little/



Signed-off-by: Christian Couder chrisc...@tuxfamily.org
---
t/t6050-replace.sh | 8 
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/t6050-replace.sh b/t/t6050-replace.sh
index 0b07a0b..5dc26e8 100755
--- a/t/t6050-replace.sh
+++ b/t/t6050-replace.sh
@@ -122,9 +122,9 @@ test_expect_success 'git replace listing and 
deleting' '

 test $HASH2 = $(git replace -l) 
 test $HASH2 = $(git replace) 
 aa=${HASH2%??} 
- test $HASH2 = $(git replace -l $aa*) 
+ test $HASH2 = $(git replace --list $aa*) 
 test_must_fail git replace -d $R 
- test_must_fail git replace -d 
+ test_must_fail git replace --delete 
 test_must_fail git replace -l -d $HASH2 
 git replace -d $HASH2 
 git show $HASH2 | grep A U Thor 
@@ -147,7 +147,7 @@ test_expect_success 'git replace resolves sha1' 
'

 git show $HASH2 | grep O Thor 
 test_must_fail git replace $HASH2 $R 
 git replace -f $HASH2 $R 
- test_must_fail git replace -f 
+ test_must_fail git replace --force 
 test $HASH2 = $(git replace)
'

@@ -278,7 +278,7 @@ test_expect_success 'replaced and replacement 
objects must be of the same type'


test_expect_success '-f option bypasses the type check' '
 git replace -f mytag $HASH1 2err 
- git replace -f HEAD^{tree} HEAD~1 2err 
+ git replace --force HEAD^{tree} HEAD~1 2err 
 git replace -f HEAD^ $BLOB 2err
'

--
1.8.4.rc1.31.g530f5ce.dirty




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


[PATCH 0/3] Reject non-ff pulls by default

2013-08-31 Thread Felipe Contreras
Junio already sent a similar patch, but I think this is simpler.

Felipe Contreras (3):
  merge: simplify ff-only option
  t: replace pulls with merges
  pull: reject non-ff pulls by default

 Documentation/git-pull.txt |  1 +
 builtin/merge.c| 20 ++--
 git-pull.sh|  9 -
 t/annotate-tests.sh|  2 +-
 t/t4200-rerere.sh  |  2 +-
 t/t5500-fetch-pack.sh  |  2 +-
 t/t5520-pull.sh| 33 +
 t/t5524-pull-msg.sh|  2 +-
 t/t5700-clone-reference.sh |  4 ++--
 t/t6022-merge-rename.sh| 20 ++--
 t/t6026-merge-attr.sh  |  2 +-
 t/t6029-merge-subtree.sh   |  4 ++--
 t/t6037-merge-ours-theirs.sh   | 10 +-
 t/t7603-merge-reduce-heads.sh  |  2 +-
 t/t9114-git-svn-dcommit-merge.sh   |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh |  2 +-
 16 files changed, 79 insertions(+), 38 deletions(-)

-- 
1.8.4-337-g7358a66-dirty

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


[PATCH 1/3] merge: simplify ff-only option

2013-08-31 Thread Felipe Contreras
No functional changes.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 builtin/merge.c | 11 ++-
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/builtin/merge.c b/builtin/merge.c
index 34a6166..da9fc08 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -186,13 +186,6 @@ static int option_parse_n(const struct option *opt,
return 0;
 }
 
-static int option_parse_ff_only(const struct option *opt,
- const char *arg, int unset)
-{
-   fast_forward = FF_ONLY;
-   return 0;
-}
-
 static struct option builtin_merge_options[] = {
{ OPTION_CALLBACK, 'n', NULL, NULL, NULL,
N_(do not show a diffstat at the end of the merge),
@@ -210,9 +203,9 @@ static struct option builtin_merge_options[] = {
OPT_BOOL('e', edit, option_edit,
N_(edit message before committing)),
OPT_SET_INT(0, ff, fast_forward, N_(allow fast-forward (default)), 
FF_ALLOW),
-   { OPTION_CALLBACK, 0, ff-only, NULL, NULL,
+   { OPTION_SET_INT, 0, ff-only, fast_forward, NULL,
N_(abort if fast-forward is not possible),
-   PARSE_OPT_NOARG | PARSE_OPT_NONEG, option_parse_ff_only },
+   PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, FF_ONLY },
OPT_RERERE_AUTOUPDATE(allow_rerere_auto),
OPT_BOOL(0, verify-signatures, verify_signatures,
N_(Verify that the named commit has a valid GPG signature)),
-- 
1.8.4-337-g7358a66-dirty

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


[PATCH 3/3] pull: reject non-ff pulls by default

2013-08-31 Thread Felipe Contreras
For the full discussion:

http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225305

The user still can specify 'git pull --merge' to restore the old
behavior, or 'git config pull.rebase false'.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 Documentation/git-pull.txt|  1 +
 builtin/merge.c   |  9 -
 git-pull.sh   |  9 -
 t/t5500-fetch-pack.sh |  2 +-
 t/t5520-pull.sh   | 33 +
 t/t5524-pull-msg.sh   |  2 +-
 t/t5700-clone-reference.sh|  4 ++--
 t/t6022-merge-rename.sh   | 20 ++--
 t/t6026-merge-attr.sh |  2 +-
 t/t6029-merge-subtree.sh  |  4 ++--
 t/t6037-merge-ours-theirs.sh  | 10 +-
 t/t7603-merge-reduce-heads.sh |  2 +-
 12 files changed, 73 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 6ef8d59..1833779 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -119,6 +119,7 @@ It rewrites history, which does not bode well when you
 published that history already.  Do *not* use this option
 unless you have read linkgit:git-rebase[1] carefully.
 
+--merge::
 --no-rebase::
Override earlier --rebase.
 
diff --git a/builtin/merge.c b/builtin/merge.c
index da9fc08..97b4205 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1437,8 +1437,15 @@ int cmd_merge(int argc, const char **argv, const char 
*prefix)
}
}
 
-   if (fast_forward == FF_ONLY)
+   if (fast_forward == FF_ONLY) {
+   const char *msg = getenv(GIT_MERGE_FF_ONLY_HELP);
+   if (msg) {
+   fprintf(stderr, %s\n, msg);
+   ret = 1;
+   goto done;
+   }
die(_(Not possible to fast-forward, aborting.));
+   }
 
/* We are going to make a new commit. */
git_committer_info(IDENT_STRICT);
diff --git a/git-pull.sh b/git-pull.sh
index f0df41c..c6b576b 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -113,7 +113,8 @@ do
-r|--r|--re|--reb|--reba|--rebas|--rebase)
rebase=true
;;
-   --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
+   --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase|\
+   -m|--m|--me|--mer|--merg|--merge)
rebase=false
;;
--recurse-submodules)
@@ -289,6 +290,12 @@ then
fi
 fi
 
+if test -z $rebase$no_ff$ff_only${squash#--no-squash}
+then
+   ff_only=--ff-only
+   export GIT_MERGE_FF_ONLY_HELP=The pull was not fast-forward, please 
either merge or rebase.
+fi
+
 merge_name=$(git fmt-merge-msg $log_arg $GIT_DIR/FETCH_HEAD) || exit
 case $rebase in
 true)
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index fd2598e..f1a068f 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -259,7 +259,7 @@ test_expect_success 'clone shallow object count' '
 test_expect_success 'pull in shallow repo with missing merge base' '
(
cd shallow 
-   test_must_fail git pull --depth 4 .. A
+   test_must_fail git pull --merge --depth 4 .. A
)
 '
 
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index ed4d9c8..c0c50a2 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -284,4 +284,37 @@ test_expect_success 'git pull --rebase against local 
branch' '
test file = $(cat file2)
 '
 
+test_expect_success 'git pull fast-forward' '
+   test_when_finished git checkout master  git branch -D other test 
+   git checkout -b other master 
+   new 
+   git add new 
+   git commit -m new 
+   git checkout -b test -t other 
+   git reset --hard master 
+   git pull
+'
+
+test_expect_success 'git pull non-fast-forward' '
+   test_when_finished git checkout master  git branch -D other test 
+   git checkout -b other master^ 
+   new 
+   git add new 
+   git commit -m new 
+   git checkout -b test -t other 
+   git reset --hard master 
+   test_must_fail git pull
+'
+
+test_expect_success 'git pull non-fast-forward (merge)' '
+   test_when_finished git checkout master  git branch -D other test 
+   git checkout -b other master^ 
+   new 
+   git add new 
+   git commit -m new 
+   git checkout -b test -t other 
+   git reset --hard master 
+   git pull --merge
+'
+
 test_done
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index 8cccecc..ec9f413 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 test_expect_success pull '
 (
cd cloned 
-   git pull --log 
+   git pull --merge --log 
git log -2 
git cat-file commit HEAD result 
grep Dollar result
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 6537911..306badf 100755
--- 

[PATCH 2/3] t: replace pulls with merges

2013-08-31 Thread Felipe Contreras
This is what the code intended.

No functional changes.

Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
---
 t/annotate-tests.sh| 2 +-
 t/t4200-rerere.sh  | 2 +-
 t/t9114-git-svn-dcommit-merge.sh   | 2 +-
 t/t9500-gitweb-standalone-no-errors.sh | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index d4e7f47..01deece 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -92,7 +92,7 @@ test_expect_success 'blame 2 authors + 1 branch2 author' '
 '
 
 test_expect_success 'merge branch1  branch2' '
-   git pull . branch1
+   git merge branch1
 '
 
 test_expect_success 'blame 2 authors + 2 merged-in authors' '
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 7ff..cf19eb7 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -172,7 +172,7 @@ test_expect_success 'first postimage wins' '
git show second^:a1 | sed s/To die: t/To die! T/ a1 
git commit -q -a -m third 
 
-   test_must_fail git pull . first 
+   test_must_fail git merge first 
# rerere kicked in
! grep ^===\$ a1 
test_cmp expect a1
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index f524d2f..d33d714 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' '
echo friend  README 
cat tmp  README 
git commit -a -m friend 
-   git pull . merge
+   git merge merge
'
 
 test_debug 'gitk --all  sleep 1'
diff --git a/t/t9500-gitweb-standalone-no-errors.sh 
b/t/t9500-gitweb-standalone-no-errors.sh
index 6fca193..3864388 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -328,7 +328,7 @@ test_expect_success \
 git add b 
 git commit -a -m On branch 
 git checkout master 
-git pull . b 
+git merge b 
 git tag merge_commit'
 
 test_expect_success \
-- 
1.8.4-337-g7358a66-dirty

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


Ṽɨᾍǧṝᾍ

2013-08-31 Thread Christen Blair
http://cyno-tavannes.ch/language/comon.php?page=4904127
--
To unsubscribe from this list: send the line 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] Turn off pathspec magic on {checkout,reset,add} -p on native Windows builds

2013-08-31 Thread Nguyễn Thái Ngọc Duy
git-add--interactive.perl rejects arguments with colons in 21e9757
(Hack git-add--interactive to make it work with ActiveState Perl -
2007-08-01). Pathspec magic starts with a colon, so it won't work if
these pathspecs are passed to git-add--interactive.perl running with
ActiveState Perl. Make sure we only pass plain paths in this case.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Johannes, can you check the test suite passes for you with this
 patch? I assume that Cygwin Perl behaves differently and does not hit
 this limit. So I keep the special case to GIT_WINDOWS_NATIVE only.
 I'll resend the patch with a few others on the same topic if it works
 for you.

 builtin/add.c  | 13 +
 builtin/checkout.c | 23 ---
 builtin/reset.c| 24 
 3 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/builtin/add.c b/builtin/add.c
index 9d52fc7..3402239 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -270,6 +270,18 @@ int interactive_add(int argc, const char **argv, const 
char *prefix, int patch)
 {
struct pathspec pathspec;
 
+#ifdef GIT_WINDOWS_NATIVE
+   /*
+* Pathspec magic is completely turned off on native Windows
+* builds because git-add-interactive.perl won't accept
+* arguments with colons in them. :/foo is an exception
+* because no colons remain after parsing.
+*/
+   parse_pathspec(pathspec, PATHSPEC_ALL_MAGIC  ~PATHSPEC_FROMTOP,
+  PATHSPEC_PREFER_FULL |
+  PATHSPEC_SYMLINK_LEADING_PATH,
+  prefix, argv);
+#else
/*
 * git-add--interactive itself does not parse pathspec. It
 * simply passes the pathspec to other builtin commands. Let's
@@ -281,6 +293,7 @@ int interactive_add(int argc, const char **argv, const char 
*prefix, int patch)
   PATHSPEC_SYMLINK_LEADING_PATH |
   PATHSPEC_PREFIX_ORIGIN,
   prefix, argv);
+#endif
 
return run_add_interactive(NULL,
   patch ? --patch : NULL,
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7ea1100..e12330f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1156,9 +1156,26 @@ int cmd_checkout(int argc, const char **argv, const char 
*prefix)
 * cannot handle. Magic mask is pretty safe to be
 * lifted for new magic when opts.patch_mode == 0.
 */
-   parse_pathspec(opts.pathspec, 0,
-  opts.patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0,
-  prefix, argv);
+   if (opts.patch_mode) {
+#ifdef GIT_WINDOWS_NATIVE
+   /*
+* Pathspec magic is completely turned off on
+* native Windows builds because
+* git-add-interactive.perl won't accept
+* arguments with colons in them. :/foo is an
+* exception because no colons remain after
+* parsing.
+*/
+   parse_pathspec(opts.pathspec,
+  PATHSPEC_ALL_MAGIC  ~PATHSPEC_FROMTOP,
+  0, prefix, argv);
+#else
+   parse_pathspec(opts.pathspec, 0,
+  PATHSPEC_PREFIX_ORIGIN,
+  prefix, argv);
+#endif
+   } else
+   parse_pathspec(opts.pathspec, 0, 0, prefix, argv);
 
if (!opts.pathspec.nr)
die(_(invalid path specification));
diff --git a/builtin/reset.c b/builtin/reset.c
index 86150d1..65f7390 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -220,10 +220,26 @@ static void parse_args(struct pathspec *pathspec,
}
}
*rev_ret = rev;
-   parse_pathspec(pathspec, 0,
-  PATHSPEC_PREFER_FULL |
-  (patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0),
-  prefix, argv);
+   if (patch_mode) {
+#ifdef GIT_WINDOWS_NATIVE
+   /*
+* Pathspec magic is completely turned off on native
+* Windows builds because git-add-interactive.perl
+* won't accept arguments with colons in them. :/foo
+* is an exception because no colons remain after
+* parsing.
+*/
+   parse_pathspec(pathspec,
+  PATHSPEC_ALL_MAGIC  ~PATHSPEC_FROMTOP,
+  PATHSPEC_PREFER_FULL, prefix, argv);
+#else
+   parse_pathspec(pathspec, 0,
+  PATHSPEC_PREFER_FULL | PATHSPEC_PREFIX_ORIGIN,
+  prefix, argv);
+#endif
+   } else
+   parse_pathspec(pathspec, 0, 

Re: [PATCH v2 8/8] update-ref: add test cases covering --stdin signature

2013-08-31 Thread Eric Sunshine
On Fri, Aug 30, 2013 at 2:12 PM, Brad King brad.k...@kitware.com wrote:
 Extend t/t1400-update-ref.sh to cover cases using the --stdin option.

 Signed-off-by: Brad King brad.k...@kitware.com
 ---
  t/t1400-update-ref.sh |  206 
 +
  1 file changed, 206 insertions(+)

 diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
 index e415ee0..9fd03fc 100755
 --- a/t/t1400-update-ref.sh
 +++ b/t/t1400-update-ref.sh
 @@ -302,4 +302,210 @@ test_expect_success \
 'git cat-file blob master@{2005-05-26 23:42}:F (expect OTHER)' \
 'test OTHER = $(git cat-file blob master@{2005-05-26 23:42}:F)'

 +a=refs/heads/a
 +b=refs/heads/b
 +c=refs/heads/c
 +z=
 +e=''
 +
 +test_expect_success 'stdin works with no input' '
 +   rm -f stdin 
 +   touch stdin 

Unless the timestamp of 'stdin' has particular significance, modern
git tests avoid 'touch' in favor of creating the empty file like this

stdin 

 +   git update-ref --stdin  stdin 

Style: Git test scripts omit whitespace following , , , and .

 +   git rev-parse --verify -q $m
 +'
 +
 +test_expect_success 'stdin fails with bad line lines' '
 +   echostdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: no ref on line:   err 
 +   echo --  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: no ref on line: -- err 
 +   echo --bad-option  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: unknown option --bad-option err 
 +   echo -\''' $a $m  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: unknown option -''' err 
 +   echo ~a $m  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: invalid ref format on line: ~a $m err 
 +   echo $a '''master  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: unterminated single-quote: '''master err 
 +   echo $a \master  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: unquoted backslash not escaping single-quote: 
 master err 
 +   echo $a $m $m $m  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: too many arguments on line: $a $m $m $m err 
 +   echo $a  stdin 
 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: missing new value on line: $a err
 +'

Despite the semantic relationship between all these cases, if there is
a regression in one case, the person reading the verbose output has to
study it carefully to determine the offending case. If you decompose
this monolith so that each case is in its own test_expect_success,
then the regressed case becomes immediately obvious.

 +test_expect_success 'stdin fails with duplicate refs' '
 +   echo $a $m  stdin 
 +   echo $b $m  stdin 
 +   echo $a $m  stdin 

These multi-line preparations of 'stdin' might be more readable with a heredoc:

cat stdin -EOF 
$a $m
$b $m
$a $m
EOF

 +   test_must_fail git update-ref --stdin  stdin 2 err 
 +   grep fatal: Multiple updates for ref '''$a''' not allowed. err
 +'
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Officially start moving to the term 'staging area'

2013-08-31 Thread David Aguilar
On Sat, Aug 31, 2013 at 12:46 AM, René Scharfe l@web.de wrote:
 Am 29.08.2013 22:36, schrieb Felipe Contreras:

 On Thu, Aug 29, 2013 at 3:03 PM, René Scharfe l@web.de wrote:

 If you have a --work-tree option then parseopt accepts --work as well,
 unless it's ambiguous, i.e. another option starts with --work, too.  So
 you
 can have a descriptive, extra-long option and type just a few characters
 at
 the same time.


 Right, but what do we use in the documentation? Writing --work-tree in
 the 'git reset' table for example would be rather ugly. I'm fine with
 --work-tree, but I think it would be weird to have short-hands in the
 documentation, although not entirely bad.


 I don't see what's so ugly about it.

 The git command itself has a --work-tree parameter for specifying the
 location of the checked-out files, however.  It could be confusing to have
 the same parameter do different things:

 $ git reset --work-tree=/some/where reset --work-tree

 Perhaps a note in the documentation is enough to clear this up.

I agree that this is confusing for people not deeply versed in Git jargon.
We also know that no one reads documentation.

Maybe a better word can be found?  How about git reset --files?
-- 
David
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Re* [PATCH 1/8] remote-bzr: fix export of utf-8 authors

2013-08-31 Thread Michael Haggerty
On 08/29/2013 11:25 PM, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
 Actually, I think not fixing it inside that 1/8 is good, as there
 are many existing cmd  file (and worse, cmd  file-$x) in these
 test-*.sh scripts.  Clean-up is better done as a follow-up patch.

 Here are two that I noticed.

 -- 8 --
 Subject: [PATCH 9/8] contrib/remote-helpers: style updates for test scripts
 
 And here is the second one.
 
 -- 8 --
 Subject: [PATCH 10/8] remote-helpers: quote variable references in 
 redirection targets
 
 Even though it is not required by POSIX to double-quote the
 redirection target in a variable, our code does so because some
 versions of bash issue a warning without the quotes.
 
 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
  contrib/remote-helpers/test-hg-hg-git.sh | 40 
 
  1 file changed, 20 insertions(+), 20 deletions(-)
 [...]

Looks good.

Reviewed-by: Michael Haggerty mhag...@alum.mit.edu

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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: Re* [PATCH 1/8] remote-bzr: fix export of utf-8 authors

2013-08-31 Thread Michael Haggerty
On 08/29/2013 11:24 PM, Junio C Hamano wrote:
 Junio C Hamano gits...@pobox.com writes:
 
 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Do I really need to quote the paragraph in CodingGuidelines?

 Existing violations are not an excuse to make things worse by adding
 more.  I think with these comments we can expect a reroll coming,
 and it should be trivial for any contributor to fix it while at it.
 
 Actually, I think not fixing it inside that 1/8 is good, as there
 are many existing cmd  file (and worse, cmd  file-$x) in these
 test-*.sh scripts.  Clean-up is better done as a follow-up patch.
 
 Here are two that I noticed.
 
 -- 8 --
 Subject: [PATCH 9/8] contrib/remote-helpers: style updates for test scripts
 
 During the review of the main series it was noticed that these test
 scripts can use updates to conform to our coding style better, but
 fixing the style should be done in a patch separate from the main
 series.
 
 This updates the test-*.sh scripts only for styles:

s/styles/style/

 
  * We do not leave SP between a redirection operator and the
filename;
 
  * We change line before then, do, etc. rather than terminating
the condition for if/while and list for for with a
semicolon;
 
  * When HERE document does not use any expansion, we quote the end
marker (e.g. cat \EOF not cat EOF) to signal the readers
that there is no funny substitution to worry about when reading
the code.
 

Please add

* We use test rather than [.

, as you made a few such changes as well.

 Signed-off-by: Junio C Hamano gits...@pobox.com
 ---
  contrib/remote-helpers/test-bzr.sh   | 112 +++---
  contrib/remote-helpers/test-hg-bidi.sh   |  52 ++-
  contrib/remote-helpers/test-hg-hg-git.sh | 156 
 +--
  contrib/remote-helpers/test-hg.sh| 154 +++---
  4 files changed, 248 insertions(+), 226 deletions(-)
 
 [...]

All of the changes in the patch look good to me.

Michael

-- 
Michael Haggerty
mhag...@alum.mit.edu
http://softwareswirl.blogspot.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] Teach git to change to a given directory using -C option

2013-08-31 Thread Eric Sunshine
On Fri, Aug 30, 2013 at 9:35 AM, Nazri Ramliy ayieh...@gmail.com wrote:
 This is similar in spirit to to make -C dir ... and tar -C dir 

 Currently it takes more effort (keypresses) to invoke git command in a
 different directory than the current one without leaving the current
 directory:

 1. (cd ~/foo  git status)
git --git-dir=~/foo/.git --work-dir=~/foo status
GIT_DIR=~/foo/.git GIT_WORK_TREE=~/foo git status
 2. (cd ../..; git grep foo)
 3. for d in d1 d2 d3; do (cd $d  git svn rebase); done

 While doable the methods shown above are arguably more suitable for
 scripting than quick command line invocations.

 With this new option, the above can be done with less keystrokes:

Grammar: s/less/fewer/

More below...

 1. git -C ~/foo status
 2. git -C ../.. grep foo
 3. for d in d1 d2 d3; do git -C $d svn rebase; done

 A new test script is added to verify the behavior of this option with
 other path-related options like --git-dir and --work-tree.

 Signed-off-by: Nazri Ramliy ayieh...@gmail.com
 ---
 This is a reroll of [1]. The only difference is the rewording of the
 commit message.  I'm resending this as I've found it to be useful in my
 daily git usage in that it helps me stay focused on what I'm doing in
 the current directory while needing to run git on another directory.

 nazri.

 [1] http://permalink.gmane.org/gmane.comp.version-control.git/221954

  Documentation/git.txt | 13 +
  git.c | 15 --
  t/t0056-git-C.sh  | 76 
 +++
  3 files changed, 102 insertions(+), 2 deletions(-)
  create mode 100755 t/t0056-git-C.sh

 diff --git a/Documentation/git.txt b/Documentation/git.txt
 index dca11cc..0d44fa2 100644
 --- a/Documentation/git.txt
 +++ b/Documentation/git.txt
 @@ -395,6 +395,19 @@ displayed. See linkgit:git-help[1] for more information,
  because `git --help ...` is converted internally into `git
  help ...`.

 +-C directory::

The synopsis at the top of git.txt mentions --git-dir and --work-tree.
For consistency, -C probably ought to be mentioned there, as well.

Other options which accept a directory, such as --git-dir and
--work-tree, are documented as accepting path, but -C is
inconsistently documented as accepting directory.

 +   Run as if git were started in directory instead of the current
 +   working directory. If multiple -C options are given, subsequent
 +   directory arguments are interpreted relative to the previous one: -C
 +   /usr -C src is equivalent to -C /usr/src. This option affects options

The fragment interpreted relative seems ambiguous when absolute
paths are involved. For instance, what happens when the user specifies
-C /foo/ -C /bar/. From the implementation I can see that the
working directory becomes /bar, but without checking the
implementation, it's not clear what the result would be. For instance,
if the implementation merely did string concatenation of the -C
arguments, then input -C /foo/ -C /bar/ might try to set the working
directory to /foo//bar/ which would be interpreted as /foo/bar/ on
Unix, but would probably fail on Windows. Perhaps rewriting might
remove the ambiguity?

[...] When multiple -C options are given, each subsequent non-absolute
-C path is interpreted relative to the preceding -C path. [...]

 +   that expect path name like --git-dir and --work-tree in that their
 +   interpretations of the path names would be made relative to the
 +   effective working directory caused by the -C option. For example the
 +   following invocations are equivalent:
 +
 +   git --git-dir=a.git --work-tree=b -C c status
 +   git --git-dir=c/a.git --work-tree=c/b status
 +
  -c name=value::
 Pass a configuration parameter to the command. The value
 given will override values from configuration files.
 diff --git a/git.c b/git.c
 index 2025f77..2207ee5 100644
 --- a/git.c
 +++ b/git.c
 @@ -7,7 +7,7 @@
  #include commit.h

  const char git_usage_string[] =
 -   git [--version] [--help] [-c name=value]\n
 +   git [--version] [--help] [-C directory] [-c name=value]\n
[--exec-path[=path]] [--html-path] [--man-path] 
 [--info-path]\n
[-p|--paginate|--no-pager] [--no-replace-objects] 
 [--bare]\n
[--git-dir=path] [--work-tree=path] 
 [--namespace=name]\n

For existing options accepting an argument, the argument is formatted
as argument. The -C option does not follow suit.

As mentioned above, all other options accepting a directory are
documented as taking path, but -C is inconsistent and is documented
as taking 'directory' instead.

 @@ -54,7 +54,18 @@ static int handle_options(const char ***argv, int *argc, 
 int *envchanged)
 /*
  * Check remaining flags.
  */
 -   if (!prefixcmp(cmd, --exec-path)) {
 +   if (!strcmp(cmd, -C)) {