[PATCH] remote-hg: fix commit messages

2013-04-18 Thread Felipe Contreras
git fast-import expects an extra newline after the commit message data,
but we are adding it only on hg-git compat mode, which is why the
bidirectionality tests pass.

We should add it unconditionally.

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

diff --git a/contrib/remote-helpers/git-remote-hg 
b/contrib/remote-helpers/git-remote-hg
index a5f0013..5481331 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -362,6 +362,8 @@ def export_ref(repo, name, kind, head):
 else:
 modified, removed = get_filechanges(repo, c, parents[0])
 
+desc += '\n'
+
 if mode == 'hg':
 extra_msg = ''
 
@@ -385,7 +387,6 @@ def export_ref(repo, name, kind, head):
 else:
 extra_msg += extra : %s : %s\n % (key, 
urllib.quote(value))
 
-desc += '\n'
 if extra_msg:
 desc += '\n--HG--\n' + extra_msg
 
-- 
1.8.2.1.679.g509521a

--
To unsubscribe from this list: send the line 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: fix commit messages

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 1:06 AM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 git fast-import expects an extra newline after the commit message data,
 but we are adding it only on hg-git compat mode, which is why the
 bidirectionality tests pass.

 We should add it unconditionally.

This doesn't depend on any other patches, and I think it might be
worth to put it in 'maint', it's not like a *huge* deal, but it would
be nice if a mercurial repo cloned with v1.8.2.2 and one cloned with
v1.8.3 end up with the same commit ids. It would have been nicer if
v1.8.1 had this already, but hey, it's new, and it's in contrib.

Cheers.

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


Re: put THEIR commits AFTER my commits with a single rebase command

2013-04-18 Thread Johannes Sixt
Am 4/18/2013 7:18, schrieb Ilya Basin:
 desired result:
 
  A---B---C origin/master
 /
 D---E---F---G---A'---B'---C' *master
 
 
 
 Variant 1:
 
 git branch -f tmp
 git reset --hard origin/master
 git rebase tmp

Variant 1a:

   git reset --hard origin/master
   git rebase @{1}

 
 This variant is bad, because 'git reset --hard' checks out some files
 and 'git rebase' rewrites them again before applying commits. It's a
redundant job.

 Variant 2:
 
 git branch -f tmp origin/master
 git rebase --onto master master tmp
 git branch -f master
 git checkout master
 
 Too many commands. I want to do this with just one command. And I want
 to stay be on branch master in case of rebase conflicts.

Perhaps this one:

   git merge origin/master
   git rebase ORIG_HEAD

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


Re: put THEIR commits AFTER my commits with a single rebase command

2013-04-18 Thread Johan Herland
On Thu, Apr 18, 2013 at 7:18 AM, Ilya Basin basini...@gmail.com wrote:
 I asked this on stackoverflow, but no reply.
 http://stackoverflow.com/questions/15971244/git-put-their-commits-after-my-commits-with-a-single-rebase-command

 Suppose master and origin/master diverged.
 I'm on master and I want to put the commits from origin/master after my 
 commits and then push --force.

  A---B---C origin/master
 /
 D---E---F---G *master

 desired result:

  A---B---C origin/master
 /
 D---E---F---G---A'---B'---C' *master

Note that if other people are working on top of origin/master, then
what you are proposing is quite rude to them, since they must now
manually rebase their own work on top of your rebased history.
Rewriting public history is generally considered evil.

 Variant 1:

 git branch -f tmp
 git reset --hard origin/master
 git rebase tmp

 This variant is bad, because 'git reset --hard' checks out some files and 
 'git rebase' rewrites them again before applying commits. It's a redundant 
 job.
 Variant 2:

 git branch -f tmp origin/master
 git rebase --onto master master tmp
 git branch -f master
 git checkout master

 Too many commands. I want to do this with just one command. And I want
 to stay be on branch master in case of rebase conflicts.

git cherry-pick master..origin/master


...Johan

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


t6200: avoid path mangling issue on Windows

2013-04-18 Thread Johannes Sixt
From: Johannes Sixt j...@kdbg.org

MSYS bash interprets the slash in the argument core.commentchar=/
as root directory and mangles it into a Windows style path. Use a
different core.commentchar to dodge the issue.

Signed-off-by: Johannes Sixt j...@kdbg.org
---
 t/t6200-fmt-merge-msg.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index e7e945d..54b5744 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -179,8 +179,8 @@ test_expect_success '--log=5 with custom comment character' 
'
cat expected -EOF 
Merge branch ${apos}left${apos}
 
-   / By Another Author (3) and A U Thor (2)
-   / Via Another Committer
+   x By Another Author (3) and A U Thor (2)
+   x Via Another Committer
* left:
  Left #5
  Left #4
@@ -189,7 +189,7 @@ test_expect_success '--log=5 with custom comment character' 
'
  Common #1
EOF
 
-   git -c core.commentchar=/ fmt-merge-msg --log=5 .git/FETCH_HEAD 
actual 
+   git -c core.commentchar=x fmt-merge-msg --log=5 .git/FETCH_HEAD 
actual 
test_cmp expected actual
 '
 
-- 
1.8.2.1.1678.gf713add
--
To unsubscribe from this list: send the line 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[2]: put THEIR commits AFTER my commits with a single rebase command

2013-04-18 Thread Ilya Basin
JH git cherry-pick master..origin/master

Thanks Johan.

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


Re: [PATCH 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread Thomas Rast
Felipe Contreras felipe.contre...@gmail.com writes:

 The *:* refspec doesn't work, and never has, clarify the code and
 documentation to reflect that. This in effect reverts commit 9e7673e
 (gitremote-helpers(1): clarify refspec behaviour).
[...]
 -test_expect_success 'pulling with straight refspec' '
 - (cd local2 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) 
 - compare_refs local2 HEAD server HEAD
 -'
 -
 -test_expect_failure 'pushing with straight refspec' '
 - test_when_finished (cd local2  git reset --hard origin) 
 - (cd local2 
 - echo content file 
 - git commit -a -m eleven 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) 
 - compare_refs local2 HEAD server HEAD
 -'

So what's wrong with the tests?  Do they fail to test what they claim
(how?), test something that wasn't reasonable to begin with, or
something entirely different?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git rev-list --pretty=format: issue

2013-04-18 Thread Thomas Rast
Junio C Hamano gits...@pobox.com writes:

 Forrest Galloway f.gallo...@gmail.com writes:

 git 1.8.2.1 on OSX(Mountain Lion) installed with Homebrew

 I am seeing an issue when trying to format the output from rev-list command.
 git log --pretty=format:%H - %an, %ar : %s When I attempt the above
 string, instead of printing to the shell, LESS is opened and the
 output is displayed there.
[...]

 Actually, less is running in both cases.
[...]
 If you do not want pager, run it with no-pager, like this:

   git --no-pager log ...your other parameters...

Also note that the pager is automatically disabled if you redirect
stdout, so --no-pager is only required in some fringe cases, notably
when attempting to run git under GDB.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line 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] stash: tighten IS_STASH_LIKE logic

2013-04-18 Thread Ramkumar Ramachandra
Currently, 'git stash show' and 'git stash apply' can show/apply any
merge commit, as the test for setting IS_STASH_LIKE simply asserts if
the commit is a merge.  Improve the situation by asserting if the
index_commit and the worktree_commit are based off the same commit, by
checking that $REV^1 is equal to $REV^2^1.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 git-stash.sh |  3 ++-
 t/t3903-stash.sh | 11 +++
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/git-stash.sh b/git-stash.sh
index bbefdf6..d0428a8 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -366,13 +366,14 @@ parse_flags_and_rev()
}
 
i_commit=$(git rev-parse --quiet --verify $REV^2 2/dev/null) 
-   set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: 2/dev/null) 
+   set -- $(git rev-parse $REV $REV^1 $REV: $REV^1: $REV^2: $REV^2^1 
2/dev/null) 
s=$1 
w_commit=$1 
b_commit=$2 
w_tree=$3 
b_tree=$4 
i_tree=$5 
+   test $b_commit = $6 
IS_STASH_LIKE=t 
test $ref_stash = $(git rev-parse --symbolic-full-name ${REV%@*}) 

IS_STASH_REF=t
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 5dfbda7..11bcd72 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -637,4 +637,15 @@ test_expect_success 'stash where working directory 
contains HEAD file' '
test_cmp output expect
 '
 
+test_expect_success 'show refuses to show any random merge commit' '
+   git stash clear 
+   git reset --hard 
+   git checkout -b quux 
+   test_commit bar 
+   git checkout - 
+   test_commit foo 
+   git merge quux 
+   test_must_fail git stash show HEAD
+'
+
 test_done
-- 
1.8.2.1.423.g4fb5c0a.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: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Matthieu Moy
Felipe Contreras felipe.contre...@gmail.com writes:

 * How many times have you tracked regressions in transport helper's
 import/export functionality?

 Hint: zero.

The real question to make the situation non-hypothetical would actually
be how many times did you track a regression that bisected down to
*this particular commit*. Any regression that ends up on another commit
is irrelevant.

I guess you realize how stupid my argument is. But how is yours
different? You do realize that your claim that nobody is ever going to
bisect down to your commit is as hypothetical as other people's claim
(if you think it is not, then try to point us a proof that nobody is
ever going to need a good message in the future to understand what I
mean).

We're trying to make all the code and all the commits clean. It seems to
be a consensus here that review is good. I see no reason to purposely
make some commits less good than others based on the fact that they may
not be used in the future.

Search your favorite search engine for broken window principle to get
more arguments in this direction.

 * How many times has *anybody* done so?

 Hint: other than me, quite possibly zero.

If you want to be the only developer, and avoid being disturbed by
others, then why are you pushing your changes to git.git? Why are you
even discussing on this list?

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


[PATCH 0/2] Add documentation for new expiry option values

2013-04-18 Thread Michael Haggerty
The first patch changes the manpages for git gc and git reflog to
document the new expiry options made possible by Junio's date.c: add
parse_expiry_date().  Feel free to squash this patch onto yours.

The second patch changes the parse_options() API documentation to
mention that a no- prefix can also be used for non-boolean options.
It is related only by the fact that I was confused by this omission
when I was trying to understand your patch.

Michael Haggerty (2):
  git-gc.txt, git-reflog.txt: document new expiry options
  api-parse-options.txt: document no- for non-boolean options

 Documentation/git-gc.txt  | 5 +++--
 Documentation/git-reflog.txt  | 9 +++--
 Documentation/technical/api-parse-options.txt | 2 ++
 3 files changed, 12 insertions(+), 4 deletions(-)

-- 
1.8.2.1

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


[PATCH 1/2] git-gc.txt, git-reflog.txt: document new expiry options

2013-04-18 Thread Michael Haggerty
Document the new values that can be used for expiry values where their
use makes sense:

* git reflog expire --expire=[all|never]
* git reflog expire --expire-unreachable=[all|never]
* git gc --prune=all

Other combinations aren't useful and therefore no documentation is
added (even though they are allowed):

* git gc --prune=never

  is redundant with git gc --no-prune

* git prune --expire=all

  is equivalent to providing no --expire option

* git prune --expire=never

  is a NOP

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 Documentation/git-gc.txt | 5 +++--
 Documentation/git-reflog.txt | 9 +++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-gc.txt b/Documentation/git-gc.txt
index b370b02..2402ed6 100644
--- a/Documentation/git-gc.txt
+++ b/Documentation/git-gc.txt
@@ -62,8 +62,9 @@ automatic consolidation of packs.
 
 --prune=date::
Prune loose objects older than date (default is 2 weeks ago,
-   overridable by the config variable `gc.pruneExpire`).  This
-   option is on by default.
+   overridable by the config variable `gc.pruneExpire`).
+   --prune=all prunes loose objects regardless of their age.
+   --prune is on by default.
 
 --no-prune::
Do not prune any loose objects.
diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt
index fb8697e..70791b9 100644
--- a/Documentation/git-reflog.txt
+++ b/Documentation/git-reflog.txt
@@ -67,14 +67,19 @@ them.
 --expire=time::
Entries older than this time are pruned.  Without the
option it is taken from configuration `gc.reflogExpire`,
-   which in turn defaults to 90 days.
+   which in turn defaults to 90 days.  --expire=all prunes
+   entries regardless of their age; --expire=never turns off
+   pruning of reachable entries (but see --expire-unreachable).
 
 --expire-unreachable=time::
Entries older than this time and not reachable from
the current tip of the branch are pruned.  Without the
option it is taken from configuration
`gc.reflogExpireUnreachable`, which in turn defaults to
-   30 days.
+   30 days.  --expire-unreachable=all prunes unreachable
+   entries regardless of their age; --expire-unreachable=never
+   turns off early pruning of unreachable entries (but see
+   --expire).
 
 --all::
Instead of listing refs explicitly, prune all refs.
-- 
1.8.2.1

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


Re: [RFC/PATCH 0/7] Rework git core for native submodules

2013-04-18 Thread Ramkumar Ramachandra
Philip Oakley wrote:
 Would it be possible to summarise the key points and proposals of where the
 subject is now?

Sure.

If you want an update from the current approach, wait for a v2; I'm
cooking it for some time, and getting some resulting ideas merged into
upstream early (look for clone.submoduleGitDir on the list, for
instance).  When upstream is in better shape to ease in a better
fundamental design, I'll post my v2 to the list.  I'll refrain from
posting any updates now, because I don't think the resulting
discussion will generate any value.

If you want to know what this thread was about, I think [1] and [2]
summarize my arguments quite well.

[1]: http://thread.gmane.org/gmane.comp.version-control.git/220047/focus=220436
[2]: http://thread.gmane.org/gmane.comp.version-control.git/220047/focus=220495
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 2/2] api-parse-options.txt: document no- for non-boolean options

2013-04-18 Thread Michael Haggerty
Document that the no- prefix can also be used for non-boolean
options.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
---
 Documentation/technical/api-parse-options.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/technical/api-parse-options.txt 
b/Documentation/technical/api-parse-options.txt
index 32ddc1c..2ff368e 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -41,6 +41,8 @@ The parse-options API allows:
 * Boolean long options can be 'negated' (or 'unset') by prepending
   `no-`, e.g. `--no-abbrev` instead of `--abbrev`. Conversely,
   options that begin with `no-` can be 'negated' by removing it.
+  Other long options can be unset (e.g., set string to NULL, set
+  integer to 0) by prepending `no-`.
 
 * Options and non-option arguments can clearly be separated using the `--`
   option, e.g. `-a -b --option -- --this-is-a-file` indicates that
-- 
1.8.2.1

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


Re: [PATCH 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast tr...@inf.ethz.ch wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 The *:* refspec doesn't work, and never has, clarify the code and
 documentation to reflect that. This in effect reverts commit 9e7673e
 (gitremote-helpers(1): clarify refspec behaviour).
 [...]
 -test_expect_success 'pulling with straight refspec' '
 - (cd local2 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) 
 - compare_refs local2 HEAD server HEAD
 -'
 -
 -test_expect_failure 'pushing with straight refspec' '
 - test_when_finished (cd local2  git reset --hard origin) 
 - (cd local2 
 - echo content file 
 - git commit -a -m eleven 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) 
 - compare_refs local2 HEAD server HEAD
 -'

 So what's wrong with the tests?  Do they fail to test what they claim
 (how?), test something that wasn't reasonable to begin with, or
 something entirely different?

Look at the code comment, and look at the now updated documentation
that assumes that *:* was reasonable. Given the available information,
it would be reasonable to assume that *:* did work, but it didn't
work, and it's not really possible to fix it, even if we wanted to, it
would be a hack. It's better to accept that fact and stop worrying too
much about what would be the best way to do the wrong thing.

-- 
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 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread John Keeping
On Wed, Apr 17, 2013 at 11:14:28PM -0500, Felipe Contreras wrote:
 The *:* refspec doesn't work, and never has, clarify the code and
 documentation to reflect that. This in effect reverts commit 9e7673e
 (gitremote-helpers(1): clarify refspec behaviour).

In what way doesn't it work?  If I specify that refspec then I do get
output that appears sensible.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/gitremote-helpers.txt |  4 ++--
  t/t5801-remote-helpers.sh   | 15 ---
  transport-helper.c  |  2 +-
  3 files changed, 3 insertions(+), 18 deletions(-)
 
 diff --git a/Documentation/gitremote-helpers.txt 
 b/Documentation/gitremote-helpers.txt
 index f506031..0c91aba 100644
 --- a/Documentation/gitremote-helpers.txt
 +++ b/Documentation/gitremote-helpers.txt
 @@ -174,8 +174,8 @@ ref.
  This capability can be advertised multiple times.  The first
  applicable refspec takes precedence.  The left-hand of refspecs
  advertised with this capability must cover all refs reported by
 -the list command.  If a helper does not need a specific 'refspec'
 -capability then it should advertise `refspec *:*`.
 +the list command.  If no 'refspec' capability is advertised,
 +there is an implied `refspec *:*`.

This is wrong.  As your later patch makes clearer, there is no implied
refspec for push - it only works for fetch.  I found the wording you've
reverted to extremely misleading.  How about something like this:

For historical reasons, 'import' treats the absence of a 'refspec'
line as equivalent to `refspec *:*`; remote helpers should always
specify an explicit refspec.

?

  'bidi-import'::
   This modifies the 'import' capability.
 diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
 index f387027..cd1873c 100755
 --- a/t/t5801-remote-helpers.sh
 +++ b/t/t5801-remote-helpers.sh
 @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' '
   compare_refs local2 HEAD server HEAD
  '
  
 -test_expect_success 'pulling with straight refspec' '
 - (cd local2 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) 
 - compare_refs local2 HEAD server HEAD
 -'
 -
 -test_expect_failure 'pushing with straight refspec' '
 - test_when_finished (cd local2  git reset --hard origin) 
 - (cd local2 
 - echo content file 
 - git commit -a -m eleven 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) 
 - compare_refs local2 HEAD server HEAD
 -'
 -
  test_expect_success 'pulling without marks' '
   (cd local2 
   GIT_REMOTE_TESTGIT_NO_MARKS=1 git pull) 
 diff --git a/transport-helper.c b/transport-helper.c
 index dcd8d97..cea787c 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport,
* were fetching.
*
* (If no refspec capability was specified, for historical
 -  * reasons we default to *:*.)
 +  * reasons we default to the equivalent of *:*.)
*
* Store the result in to_fetch[i].old_sha1.  Callers such
* as git fetch can use the value to write feedback to the
 -- 
 1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line 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[2]: put THEIR commits AFTER my commits with a single rebase command

2013-04-18 Thread Ilya Basin

JS Perhaps this one:

JSgit merge origin/master
JSgit rebase ORIG_HEAD

JS -- Hannes

Wouldn't I have to resolve conflicts twice?


BTW, during the rebase, can I tell git to rewrite a different branch
upon rebase success or abort?

git branch -f tmp origin/master
git rebase --onto master master tmp
if [ $? -ne 0 ]; then
   # modify some file in .git/ ?
else
git branch -f master
git checkout master
fi



-- 

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


Re: What's cooking between #05 and #06

2013-04-18 Thread John Keeping
On Wed, Apr 17, 2013 at 11:25:07PM +0200, Jens Lehmann wrote:
 I like it, as it gets rid of the top-level requirement. But from
 my testing it looks like we're not quite there yet.
 
 'summary' and 'status' behave as if they were run in the toplevel
 directory, while a git status shows all filenames relative to the
 current directory. Me thinks 'summary' and 'status' (and all other
 submodule commands) should behave like status and print relative
 paths too. I'm not really sure yet how $sm_path should behave for
 'foreach', but I suspect having it relative to the current
 directory would be the way to go (which it currently isn't).

 When submodule add is run with a relative path it is relative to
 the top-level directory, which I find confusing (and won't play
 well with shell completion).

This confused me for a bit because I was sure I handled this, but I see
I missed relative submodules URLs.  So the path at which to put the
submodule is correct, but the path from which to clone is not.

 'deinit .' doesn't deinit submodules above the current directory
 (but prints the path relative to top-level) while 'init' will
 initialize all submodules known to the superproject.

I can't see how this happens.  'init' uses module_list which has been
updated to handle relative paths.  So I expect 'git submodule init .' to
work correctly here.  I would expect either of them to act on all
submodules when given no extra arguments.

 So this is a good start, but it looks like there is some work left
 to do before this can hit master.

Thanks for the feedback.
--
To unsubscribe from this list: send the line 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] t3400 (rebase): add failing test for a peculiar rev spec

2013-04-18 Thread Ramkumar Ramachandra
'git rebase' does not recognize revisions specified as :/text.  This
is because the attempts to rev-parse ${REV}^0, which fails in this
case.  Add a test to document this failure.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Okay, so I'm not sure what the right fix for this is.

 - The :/text syntax seems to be broken in the first place, as it
   can't be combined with ^ or @.  I'd like to be able to say
   {:/bardery}^1, or even {:/foomery}^{/communist mule}.  Why
   shouldn't I be allowed to do this?

 - The failure occurs in git-rebase.sh:403.  Is it using the ^0 only
   to make sure that the revision specified is a commit?  Surely,
   there'a a better way to do this?

 Can someone point me in the right direction?

 t/t3400-rebase.sh | 8 
 1 file changed, 8 insertions(+)

diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index f6cc102..42ca1e0 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -88,6 +88,14 @@ test_expect_success 'rebase fast-forward to master' '
test_i18ngrep Fast-forwarded HEAD to my-topic-branch out
 '
 
+test_expect_failure 'rebase against revision specified as :/text' '
+   git checkout my-topic-branch^ 
+   sha1=$(git rev-parse :/Add B) 
+   git rebase $sha1 
+   git checkout my-topic-branch^ 
+   git rebase :/Add B
+'
+
 test_expect_success 'the rebase operation should not have destroyed author 
information' '
! (git log | grep Author: | grep )
 '
-- 
1.8.2.1.423.g4fb5c0a.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: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 2:44 AM, Matthieu Moy
matthieu@grenoble-inp.fr wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 * How many times have you tracked regressions in transport helper's
 import/export functionality?

 Hint: zero.

 The real question to make the situation non-hypothetical would actually
 be how many times did you track a regression that bisected down to
 *this particular commit*. Any regression that ends up on another commit
 is irrelevant.

 I guess you realize how stupid my argument is. But how is yours
 different?

I did not make any argument (stupid or otherwise), I made I claim; I
won't waste my time with hypotheticals.

 You do realize that your claim that nobody is ever going to
 bisect down to your commit is as hypothetical as other people's claim
 (if you think it is not, then try to point us a proof that nobody is
 ever going to need a good message in the future to understand what I
 mean).

Yeah, they are both hypotheticals, the only difference is that your
claim is very easy to prove; all you need is *ONE* example.

But I'm very happy to withdraw my claim, as long as you withdraw your
claim as well, and we go back to the default position: we don't know
if anybody will every look at these commit messages again.

 We're trying to make all the code and all the commits clean. It seems to
 be a consensus here that review is good. I see no reason to purposely
 make some commits less good than others based on the fact that they may
 not be used in the future.

You have to prove first that they are less good, and the best way to
do that is provide commit messages of your own, if you do that, they
can be used instead, but if you don't, what do you propose to do? Drop
the patches?

 Search your favorite search engine for broken window principle to get
 more arguments in this direction.

More like broken windows hypothesis, which is not without its critics.

 * How many times has *anybody* done so?

 Hint: other than me, quite possibly zero.

 If you want to be the only developer, and avoid being disturbed by
 others, then why are you pushing your changes to git.git? Why are you
 even discussing on this list?

Doesn't matter, it's still *HYPOTHETICAL* that anybody will every hit
this in a bisect.

Now, if you agree it's all hypothetical, the next rational thing to do
is risk analysis: how likely is it to happen, and what would be the
impact if it does? The answer to both questions is: close to *ZERO*.
So, considering the nature of these patches (a remote-helper in the
contrib area that is relatively new), and the active developers (me),
I'd say it's much more important to get the fixes in, than to document
every little quirk, detail and reasoning behind them. It's the balance
I think it's best at this point, and it is my time, and it is my
decision what I do with it.

It might also help to compare oranges with oranges, and with regards
to remote-hg transport helpers, I do believe the one in
contrib/remote-helpers has the best commit messages:

msysgit's remote-hg:
---
commit 6bbd5365988d63780acc2ab407878eef8c19b47c
Author: Sverre Rabbelier srabbel...@gmail.com
Date:   Sun Aug 22 01:22:14 2010 -0500

git_remote_helpers: add fastimport library

 git_remote_helpers/fastimport/__init__.py |   0
 git_remote_helpers/fastimport/commands.py | 469
+
 git_remote_helpers/fastimport/dates.py|  79 +++
 git_remote_helpers/fastimport/errors.py   | 182 +
 git_remote_helpers/fastimport/head_tracker.py |  47 +++
 git_remote_helpers/fastimport/helpers.py  |  88 +
 git_remote_helpers/fastimport/idmapfile.py|  65 +
 git_remote_helpers/fastimport/parser.py   | 621
++
 git_remote_helpers/fastimport/processor.py| 222
+++
 git_remote_helpers/setup.py   |   3 +-
 10 files changed, 1775 insertions(+), 1 deletion(-)
---

gitifyhg:
--
commit 4b364563cd705dc5e69082e6b80d304fe50b9c9c
Author: Alex Sydell a...@dropbox.com
Date:   Sat Mar 23 23:46:33 2013 -0700

Report correct (instead of unknown) hashes when importing refs into git

 gitifyhg/gitifyhg.py   | 27 +--
 gitifyhg/hgimporter.py | 20 ++--
 gitifyhg/util.py   | 44 
 test/test_push.py  | 31 +++
 4 files changed, 94 insertions(+), 28 deletions(-)
---

And of course, the best place to discuss the lack of good commit
messages, is in the patches themselves, which are after all, sent to
the mailing list for everyone to review.

Cheers.

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


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 I think the commit message is fine, you don't. So YOU go ahead and
 write the proper one. If you don't, all you are doing is being an
 impediment to progress.

Hey Felipe.  Let's get a few things straightened out first:

- We all act in our selfish interests, and write code to scratch our
personal itches.  I don't write code or commit messages for anyone
else, and neither should you.

- However, we're not working in isolation.  We have this giant mailing
list where we all post our patches.  It's like a bazaar where we
compete against other patches for developer attention and potential
reviewers.  In other words, it's a free market, and we're selling our
product: if it fails to sell, will you blame the market or your
product?  I write clear code and beautiful commit messages exactly for
this reason: I'm fighting for attention!

- We have to learn to interoperate with others' code and conventions,
if we want to be part of the community.  That doesn't mean that we
drown out our individuality, but it means that a our patch series has
to conform to some minimal, loose, and evolving standard.  Now, you
can argue that many of the existing conventions are outdated (I do it
all the time), but it cannot change overnight.  Your influence on the
community will show up over an extended period of time.

- We are not an old enterprise who blame breakages on a few
individuals, and fire them.  We're a community where all of us are
equally responsible for all parts of the code.  I am as responsible
for the remote-hg code in master as you are, as I had every
opportunity to review it when the patch series came up on the list.  I
might have chosen not to, but that doesn't relieve me of
responsibility.

-  We don't practice division of labour.  There are no managers,
testing people, documentation people, code-writing people,
commit-message writing people etc.  Everyone has to do some portion
of all these tasks, although we try to keep the boring work/ technical
debt to a minimum.  Don't ask other people to write commit messages
for your code.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 3:24 AM, John Keeping j...@keeping.me.uk wrote:
 On Wed, Apr 17, 2013 at 11:14:28PM -0500, Felipe Contreras wrote:
 The *:* refspec doesn't work, and never has, clarify the code and
 documentation to reflect that. This in effect reverts commit 9e7673e
 (gitremote-helpers(1): clarify refspec behaviour).

 In what way doesn't it work?  If I specify that refspec then I do get
 output that appears sensible.

% git checkout origin/master
% make -C t t5801-remote-helpers.sh

not ok 15 - pushing with straight refspec # TODO known breakage

It fails for me here.


  Documentation/gitremote-helpers.txt |  4 ++--
  t/t5801-remote-helpers.sh   | 15 ---
  transport-helper.c  |  2 +-
  3 files changed, 3 insertions(+), 18 deletions(-)

 diff --git a/Documentation/gitremote-helpers.txt 
 b/Documentation/gitremote-helpers.txt
 index f506031..0c91aba 100644
 --- a/Documentation/gitremote-helpers.txt
 +++ b/Documentation/gitremote-helpers.txt
 @@ -174,8 +174,8 @@ ref.
  This capability can be advertised multiple times.  The first
  applicable refspec takes precedence.  The left-hand of refspecs
  advertised with this capability must cover all refs reported by
 -the list command.  If a helper does not need a specific 'refspec'
 -capability then it should advertise `refspec *:*`.
 +the list command.  If no 'refspec' capability is advertised,
 +there is an implied `refspec *:*`.

 This is wrong.

It has been like that since v1.7.0.

 As your later patch makes clearer, there is no implied
 refspec for push - it only works for fetch.  I found the wording you've
 reverted to extremely misleading.  How about something like this:

 For historical reasons, 'import' treats the absence of a 'refspec'
 line as equivalent to `refspec *:*`; remote helpers should always
 specify an explicit refspec.

Maybe my version was misleading because it didn't mention it was for
historical reasons, but yours is plain wrong; remote helpers might not
be using 'import' or 'export' at all, so not all remote helpers should
always specify an explicit refspec. And if the previous text It is
recommended that all importers providing the 'import' capability use
this. It's mandatory for 'export'. does not convey the idea these
remote helpers should always specify an explicit refspec, I don't know
what it does.

-- 
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: State of CVS-to-git conversion tools (Was: Re: cvsps: bad usage: invalid argument --norc)

2013-04-18 Thread Eric S. Raymond
Ilya Basin basini...@gmail.com:
 Hi Eric.
 
 I tried --fast-export. It's 2 times faster.
 The first thing that differs: in cvsps2 commits with adjacent
 timestamps were joined into one (see the attached files). Do you know
 the reason?

The cvsps guy included code to do that. Keith Packard didn't.  
Sorry I can't be more helpful, but that's about all I know.

I didn't write either analysis stage; I understand cvsps's, somewhat,
because I had to fix several nasty bugs in it.  I *don't* understand
cvs-fast-export's analysis stage very well yet, because it has no
obvious bugs that have required me to dive in.  (Keith's notes
document one major bug, which may be inherent to the mismatch between
file- and changest-orientation and not fixable in the general case,
though I will try.)

 Does this --fast-export thing support what John mentioned, the
 incremental import support? Does 'git fast-import' has it?

cvs-fast-export does not have incremental-import support.  Whether
git-cvs-import has it depend on which version you have and what
backend it it is using. I don't maintain that wrapper.

 I need it, because full import takes too long.
 The central repo of my employer is CVS, other people commit to it and
 I use git internally to be able to tidy my commit history before
 exporting to CVS.

You are out of luck. That feature was dependent on a very fragile
coupling between the old output format and a bunch of unmaintainably 
horrible Perl in the git-cvs-import wrapper script.  It didn't
work very well; frankly, I'm amazed it worked at all.

The things I had to do to fix the serious bugs in cvsps2 and make it
output a fast-import stream had the side effect of breaking that
coupling. cvsps3 won't give you that feature. Dropping back to cvsps2
to keep that feature will expose you to the cvsps2 bugs.

I'm sorry these tools are such a mess.  I'm trying to fix that, but
it's hard, slow work.  The problems are deeply ugly and the edge cases
have poisoned spikes.
-- 
a href=http://www.catb.org/~esr/;Eric S. Raymond/a
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread John Keeping
On Thu, Apr 18, 2013 at 04:27:57AM -0500, Felipe Contreras wrote:
 On Thu, Apr 18, 2013 at 3:24 AM, John Keeping j...@keeping.me.uk wrote:
  On Wed, Apr 17, 2013 at 11:14:28PM -0500, Felipe Contreras wrote:
  diff --git a/Documentation/gitremote-helpers.txt 
  b/Documentation/gitremote-helpers.txt
  index f506031..0c91aba 100644
  --- a/Documentation/gitremote-helpers.txt
  +++ b/Documentation/gitremote-helpers.txt
  @@ -174,8 +174,8 @@ ref.
   This capability can be advertised multiple times.  The first
   applicable refspec takes precedence.  The left-hand of refspecs
   advertised with this capability must cover all refs reported by
  -the list command.  If a helper does not need a specific 'refspec'
  -capability then it should advertise `refspec *:*`.
  +the list command.  If no 'refspec' capability is advertised,
  +there is an implied `refspec *:*`.
 
  This is wrong.
 
 It has been like that since v1.7.0.

That doesn't mean it's right.  I suspect that it was originally correct,
but then 'export' was added at which point this became stale.

  As your later patch makes clearer, there is no implied
  refspec for push - it only works for fetch.  I found the wording you've
  reverted to extremely misleading.  How about something like this:
 
  For historical reasons, 'import' treats the absence of a 'refspec'
  line as equivalent to `refspec *:*`; remote helpers should always
  specify an explicit refspec.
 
 Maybe my version was misleading because it didn't mention it was for
 historical reasons, but yours is plain wrong; remote helpers might not
 be using 'import' or 'export' at all, so not all remote helpers should
 always specify an explicit refspec. And if the previous text It is
 recommended that all importers providing the 'import' capability use
 this. It's mandatory for 'export'. does not convey the idea these
 remote helpers should always specify an explicit refspec, I don't know
 what it does.

I didn't say mine was correct, but there was a reason that I wanted to
change the existing text.  Just going back to what was there before is
not a good way to make things better.

In my copy of pu I don't see the text It's mandatory for 'export', it
just stops after It is recommended that all importers providing the
'import' capability use this.  That paragraph also starts with This
modifies the 'import' capability making no mention of export.

Perhaps we need a slightly more extensive re-write of the documentation
for refspec to make it very clear where it applies and when it is
needed.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Pushing/fetching from/into a shallow-cloned repository

2013-04-18 Thread Konstantin Khomoutov
The git-clone manual page, both [1] and my local copy coming with
Git for Windows 1.8.1, say about the --depth command-line option:

   --depth depth

 Create a shallow clone with a history truncated to the specified
 number of revisions. A shallow repository has a number of
 limitations (you cannot clone or fetch from it, nor push from nor
 into it), but is adequate if you are only interested in the recent
 history of a large project with a long history, and would want to
 send in fixes as patches.

But having done a shallow clone (--depth=1) of one of my repositories,
I was able to record a new commit, push it out to a reference bare
repository and then fetch back to another clone of the same repository
just fine.  I have then killed my test commit doing a forced push from
another clone and subsequently was able to do `git fetch` in my shallow
clone just fine.

So I observe pushing/fetching works OK at least for a simple case like
this one.

Hence I'd like to ask: if the manual page is wrong or I'm observing
some corner case?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 4:19 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Felipe Contreras wrote:
 I think the commit message is fine, you don't. So YOU go ahead and
 write the proper one. If you don't, all you are doing is being an
 impediment to progress.

 Hey Felipe.  Let's get a few things straightened out first:

 - We all act in our selfish interests, and write code to scratch our
 personal itches.  I don't write code or commit messages for anyone
 else, and neither should you.

 - However, we're not working in isolation.  We have this giant mailing
 list where we all post our patches.  It's like a bazaar where we
 compete against other patches for developer attention and potential
 reviewers.  In other words, it's a free market, and we're selling our
 product: if it fails to sell, will you blame the market or your
 product?  I write clear code and beautiful commit messages exactly for
 this reason: I'm fighting for attention!

Except the customers are not git developers, it's git users. Git
developers rejecting patches because of the commit message is akin to
distributors rejecting products because they don't like the
transportation packages; they are only hurting themselves, by hurting
their customers.

 - We have to learn to interoperate with others' code and conventions,
 if we want to be part of the community.  That doesn't mean that we
 drown out our individuality, but it means that a our patch series has
 to conform to some minimal, loose, and evolving standard.  Now, you
 can argue that many of the existing conventions are outdated (I do it
 all the time), but it cannot change overnight.  Your influence on the
 community will show up over an extended period of time.

And the only way it can change is by discussing.

The only one that gets bitten by fixes not getting merged are git
users, not me. So if a discussion of a commit message impedes the
merging of the commit, I don't get affected, but when we have agreed
to disagree on what constitutes a good message, and the patch is still
on hold, then there's a problem.

 - We are not an old enterprise who blame breakages on a few
 individuals, and fire them.  We're a community where all of us are
 equally responsible for all parts of the code.  I am as responsible
 for the remote-hg code in master as you are, as I had every
 opportunity to review it when the patch series came up on the list.  I
 might have chosen not to, but that doesn't relieve me of
 responsibility.

I don't think so. Unless you added your Signed-off-by, you are not.

 -  We don't practice division of labour.  There are no managers,
 testing people, documentation people, code-writing people,
 commit-message writing people etc.  Everyone has to do some portion
 of all these tasks, although we try to keep the boring work/ technical
 debt to a minimum.  Don't ask other people to write commit messages
 for your code.

I am not. Neither should they ask me to write the commit messages they
want. They can make *suggestions*, and I can reject them.

When two persons have different ideas, often times both are wrong, and
the middle-ground is best, but sometimes a person reaches the
middle-ground, and sometimes one person was right from the start.

But when everyone shares the *assumption* that there is never a commit
message that is too long, you know the wrestling mat of ideas is
rigged. I wonder if I should write a commit message as long as a book
chapter for a one-liner, only to prove a point, but I'm honestly
afraid that it would be committed as is.

And remember what started the conversation; do you think a patch with
a possibly incomplete commit message should not be merged to pu
(proposed updates), shouldn't even be mentioned in the what's
cooking mail, and thus shouldn't even be considered cooking?

Cheers.

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


Re: Pushing/fetching from/into a shallow-cloned repository

2013-04-18 Thread Konstantin Khomoutov
On Thu, 18 Apr 2013 13:52:33 +0400
Konstantin Khomoutov kostix+...@007spb.ru wrote:

 The git-clone manual page, both [1] and my local copy coming with
 Git for Windows 1.8.1, say about the --depth command-line option:

[...]

Oops, by [1] I wanted to refer to this version:
https://www.kernel.org/pub/software/scm/git/docs/git-clone.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 v2 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 4:45 AM, John Keeping j...@keeping.me.uk wrote:
 On Thu, Apr 18, 2013 at 04:27:57AM -0500, Felipe Contreras wrote:

 Maybe my version was misleading because it didn't mention it was for
 historical reasons, but yours is plain wrong; remote helpers might not
 be using 'import' or 'export' at all, so not all remote helpers should
 always specify an explicit refspec. And if the previous text It is
 recommended that all importers providing the 'import' capability use
 this. It's mandatory for 'export'. does not convey the idea these
 remote helpers should always specify an explicit refspec, I don't know
 what it does.

 I didn't say mine was correct, but there was a reason that I wanted to
 change the existing text.  Just going back to what was there before is
 not a good way to make things better.

And just because it was that way before doesn't mean it was worst.
Your patch essentially switched from it is implied, to it should be
explicit, which is wrong. Going back to the previous text is the
simplest change that restores a reasonable explanation. The next
patches in this series deal with the rest of the issues in this text.

 In my copy of pu I don't see the text It's mandatory for 'export', it
 just stops after It is recommended that all importers providing the
 'import' capability use this.  That paragraph also starts with This
 modifies the 'import' capability making no mention of export.

This is patch 1 of 6, keep going.

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


“ไฟใต้” กับพวก “สวมรอย”

2013-04-18 Thread git@vger.kernel.org
ไฟใต้ ไม่จัดการพวก 'สวมรอย' ก่อน เห็นทีจะยากที่ใต้จะกลับมาร่มเย็นอีกครั้ง...

ไทยรัฐออนไลน์พยายามค้นหาข้อมูลจากคนใน
หน่วยงานระดับปฏิบัติการในพื้นที่หลายต่อหลายคน ถึงสภาพจริงที่เกิดขึ้น 
โดยเฉพาะเรื่องของการข่าวว่ามันเหมือน 
หรือแตกต่างไปจากที่รับทราบกันแค่ไหนอย่างไร ก็ได้คำตอบมาว่า 
จริงอยู่ที่จุดเริ่มต้นของความรุนแรงในพื้นที่ 3 จ.ชายแดนใต้นั้น 
อาจเริ่มมาจากการกระทำภายใต้อุดมการณ์อย่างแท้จริง 
แต่เมื่อเวลาผ่านไปนานร่วมสิบปี กลับพบว่ามีกลุ่มคนบางกลุ่ม 
ฉกฉวยโอกาสในการเกิดความไม่สงบเหล่านี้ 
เป็นช่องทางในการกระทำสิ่งเพื่อประโยชน์ของตนเองจนสถานการณ์ขยายตัว 
ซึ่งล้วนแต่ก็เป็นเรื่องที่ผิดกฎหมายแทบทั้งนั้น 
ไม่ว่าจะเป็นเรื่องของการค้าของผิดกฎหมายเช่น น้ำมันเถื่อน ยาเสพติด 
โดยเฉพาะยาเสพติด เจ้าหน้าที่ที่คลุกคลีในพื้นที่จริงๆ ต่างพูดเหมือนกันว่า
 กลายเป็นสิ่งหนึ่งที่ ระบาด ในพื้นที่ และมีการใช้ยาเสพติดเหล่านี้ 
เป็นสิ่งจูงใจในการก่อเหตุต่างๆ ไปแล้วบอกเล่าเหล่านี้ ถูกสำทับ 
ให้หนักแน่นด้วยการเล่าต่อว่า 
ขณะลงพื้นที่ไม่ยากเลยที่จะพบเห็นถุงยา ร่วงหล่น ในสวนยาง... 
เมื่อไทยรัฐออนไลน์จี้หนักเข้าไปถึงมุมมองถึงเหตุแห่งความรุนแรงที่เกิด 
กลับถูกขยายความต่อว่า หลังๆ หลายเหตุการณ์เหมือนถูก สวมรอย 
อันเนื่องมาจากเหตุการณ์ความรุนแรงที่เกิดขึ้นในพื้นที่ 
บางครั้งตอบไม่ได้ว่าทำไปเพื่ออะไร จะแค่หวังเพียงเพื่อป่วนเมือง 
บางครั้งยังมองไม่ได้ขนาดนั้นเลย

แหล่งที่มา...http://www.thairath.co.th/content/pol/337575


Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread John Keeping
On Thu, Apr 18, 2013 at 05:02:11AM -0500, Felipe Contreras wrote:
 On Thu, Apr 18, 2013 at 4:45 AM, John Keeping j...@keeping.me.uk wrote:
  In my copy of pu I don't see the text It's mandatory for 'export', it
  just stops after It is recommended that all importers providing the
  'import' capability use this.  That paragraph also starts with This
  modifies the 'import' capability making no mention of export.
 
 This is patch 1 of 6, keep going.

Ah, I see now.  I had read all of the patches initially, but then
focused on this one for discussion.

The end result after this series does look good, but I find it slightly
strange to take a step backwards on the way.
--
To unsubscribe from this list: send the line 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 3/6] transport-helper: clarify pushing without refspecs

2013-04-18 Thread John Keeping
On Wed, Apr 17, 2013 at 11:14:30PM -0500, Felipe Contreras wrote:
 This has never worked, since it's inception the code simply skips all
 the refs, essentially telling fast-export to do nothing.
 
 Let's at least tell the user what's going on.
 
 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/gitremote-helpers.txt | 4 ++--
  t/t5801-remote-helpers.sh   | 6 +++---
  transport-helper.c  | 5 +++--
  3 files changed, 8 insertions(+), 7 deletions(-)
 
 diff --git a/Documentation/gitremote-helpers.txt 
 b/Documentation/gitremote-helpers.txt
 index ba7240c..4d26e37 100644
 --- a/Documentation/gitremote-helpers.txt
 +++ b/Documentation/gitremote-helpers.txt
 @@ -162,8 +162,8 @@ Miscellaneous capabilities
   For remote helpers that implement 'import' or 'export', this capability
   allows the refs to be constrained to a private namespace, instead of
   writing to refs/heads or refs/remotes directly.
 - It is recommended that all importers providing the 'import' or 'export'
 - capabilities use this.
 + It is recommended that all importers providing the 'import'
 + capability use this. It's mandatory for 'export'.

s/It's/It is/

  +
  A helper advertising the capability
  `refspec refs/heads/*:refs/svn/origin/branches/*`
 diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
 index cd1873c..3eeb309 100755
 --- a/t/t5801-remote-helpers.sh
 +++ b/t/t5801-remote-helpers.sh
 @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' '
   compare_refs local2 HEAD server HEAD
  '
  
 -test_expect_failure 'pushing without refspecs' '
 +test_expect_success 'pushing without refspecs' '
   test_when_finished (cd local2  git reset --hard origin) 
   (cd local2 
   echo content file 
   git commit -a -m ten 
 - GIT_REMOTE_TESTGIT_REFSPEC= git push) 
 - compare_refs local2 HEAD server HEAD
 + GIT_REMOTE_TESTGIT_REFSPEC= test_must_fail git push 2 ../error) 
 + grep remote-helper doesn.t support push; refspec needed error
  '
  
  test_expect_success 'pulling without marks' '
 diff --git a/transport-helper.c b/transport-helper.c
 index cea787c..4d98567 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport 
 *transport,
   struct string_list revlist_args = STRING_LIST_INIT_NODUP;
   struct strbuf buf = STRBUF_INIT;
  
 + if (!data-refspecs)
 + die(remote-helper doesn't support push; refspec needed);

I think the refspec needed text is likely to be confusing if an
end-user ever sees this message.  I'm not sure how we can provide useful
feedback for both remote helper authors and end-users though.

 +
   helper = get_helper(transport);
  
   write_constant(helper-in, export\n);
 @@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport 
 *transport,
   char *private;
   unsigned char sha1[20];
  
 - if (!data-refspecs)
 - continue;
   private = apply_refspecs(data-refspecs, data-refspec_nr, 
 ref-name);
   if (private  !get_sha1(private, sha1)) {
   strbuf_addf(buf, ^%s, private);
 -- 
 1.8.2.1.679.g509521a
--
To unsubscribe from this list: send the line 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 3/6] transport-helper: clarify pushing without refspecs

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 5:11 AM, John Keeping j...@keeping.me.uk wrote:
 On Wed, Apr 17, 2013 at 11:14:30PM -0500, Felipe Contreras wrote:
 This has never worked, since it's inception the code simply skips all
 the refs, essentially telling fast-export to do nothing.

 Let's at least tell the user what's going on.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/gitremote-helpers.txt | 4 ++--
  t/t5801-remote-helpers.sh   | 6 +++---
  transport-helper.c  | 5 +++--
  3 files changed, 8 insertions(+), 7 deletions(-)

 diff --git a/Documentation/gitremote-helpers.txt 
 b/Documentation/gitremote-helpers.txt
 index ba7240c..4d26e37 100644
 --- a/Documentation/gitremote-helpers.txt
 +++ b/Documentation/gitremote-helpers.txt
 @@ -162,8 +162,8 @@ Miscellaneous capabilities
   For remote helpers that implement 'import' or 'export', this capability
   allows the refs to be constrained to a private namespace, instead of
   writing to refs/heads or refs/remotes directly.
 - It is recommended that all importers providing the 'import' or 'export'
 - capabilities use this.
 + It is recommended that all importers providing the 'import'
 + capability use this. It's mandatory for 'export'.

 s/It's/It is/

What's the difference?

 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport 
 *transport,
   struct string_list revlist_args = STRING_LIST_INIT_NODUP;
   struct strbuf buf = STRBUF_INIT;

 + if (!data-refspecs)
 + die(remote-helper doesn't support push; refspec needed);

 I think the refspec needed text is likely to be confusing if an
 end-user ever sees this message.  I'm not sure how we can provide useful
 feedback for both remote helper authors and end-users though.

It doesn't have to be. They'll google it, or they'll post a bug report
with this warning verbatim, or they'll just ignore it. I don't think
there's much more we can do to help in either of these cases.

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


Bug with rev-list --reverse?

2013-04-18 Thread Felipe Contreras
Hi,

If I do these:

% git log --oneline -1 v1.8.1.5^..v1.8.1.6
% git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6

I expect to get a different output, and not both showing v1.8.1.6.
Wouldn't you agree?

Cheers.

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


Re: [PATCH v2 3/6] transport-helper: clarify pushing without refspecs

2013-04-18 Thread John Keeping
On Thu, Apr 18, 2013 at 05:14:18AM -0500, Felipe Contreras wrote:
 On Thu, Apr 18, 2013 at 5:11 AM, John Keeping j...@keeping.me.uk wrote:
  On Wed, Apr 17, 2013 at 11:14:30PM -0500, Felipe Contreras wrote:
  This has never worked, since it's inception the code simply skips all
  the refs, essentially telling fast-export to do nothing.
 
  Let's at least tell the user what's going on.
 
  Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
  ---
   Documentation/gitremote-helpers.txt | 4 ++--
   t/t5801-remote-helpers.sh   | 6 +++---
   transport-helper.c  | 5 +++--
   3 files changed, 8 insertions(+), 7 deletions(-)
 
  diff --git a/Documentation/gitremote-helpers.txt 
  b/Documentation/gitremote-helpers.txt
  index ba7240c..4d26e37 100644
  --- a/Documentation/gitremote-helpers.txt
  +++ b/Documentation/gitremote-helpers.txt
  @@ -162,8 +162,8 @@ Miscellaneous capabilities
For remote helpers that implement 'import' or 'export', this 
  capability
allows the refs to be constrained to a private namespace, instead of
writing to refs/heads or refs/remotes directly.
  - It is recommended that all importers providing the 'import' or 
  'export'
  - capabilities use this.
  + It is recommended that all importers providing the 'import'
  + capability use this. It's mandatory for 'export'.
 
  s/It's/It is/
 
 What's the difference?

It's is considered informal, while we do adopt that tone on the
mailing list and sometimes in the documentation, in general I think we
should avoid contractions in the formal documentation.

In this case it is particularly jarring because it immediately follows a
sentence where we use the full It is form.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug with rev-list --reverse?

2013-04-18 Thread John Keeping
On Thu, Apr 18, 2013 at 05:17:14AM -0500, Felipe Contreras wrote:
 If I do these:
 
 % git log --oneline -1 v1.8.1.5^..v1.8.1.6
 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6
 
 I expect to get a different output, and not both showing v1.8.1.6.
 Wouldn't you agree?

I expect to get the same output.  This is probably because I consider
--reverse to be an output filter.  So I expect to show the commits
v1.8.1.5^..v1.8.1.6 -1 which selects a single commit and then show
that in reverse order.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Ramkumar Ramachandra
Felipe Contreras wrote:
 Except the customers are not git developers, it's git users. Git
 developers rejecting patches because of the commit message is akin to
 distributors rejecting products because they don't like the
 transportation packages; they are only hurting themselves, by hurting
 their customers.

Huh?  I certainly don't develop for some git users I don't even know
or care about.  In this order of precedence, my customers are:

1. Me.
2. People who develop git.git, whom I have to cooperate with.
3. People who exercise git heavily like the linux.git community, as
opposed to some little projects that operate using pull requests on
GitHub.
...
137. People who incidentally choose to use git.
138. People who incidentally choose to use git, but aren't on Linux.

I don't know if Junio or the others share this view, but this is how I
personally operate and I'm very happy.

And nobody is hurting anyone else.  Someone wrote some code, and
failed to sell it to the community.  That's all happened.

 The only one that gets bitten by fixes not getting merged are git
 users, not me. So if a discussion of a commit message impedes the
 merging of the commit, I don't get affected, but when we have agreed
 to disagree on what constitutes a good message, and the patch is still
 on hold, then there's a problem.

I think the whole issue of whether your commit message conforms to
some transcendental standard is orthogonal to the issue.  Is your
patch getting attention?  Has it attracted reviewers, and turned up on
the latest What's cooking?

 I don't think so. Unless you added your Signed-off-by, you are not.

Okay, so your view differs.

 I am not. Neither should they ask me to write the commit messages they
 want. They can make *suggestions*, and I can reject them.

Ofcourse you have a right to reject suggestions.  The question at the
end of the day doesn't change: did you manage to get people to read
your patch?

 When two persons have different ideas, often times both are wrong, and
 the middle-ground is best, but sometimes a person reaches the
 middle-ground, and sometimes one person was right from the start.

https://yourlogicalfallacyis.com/middle-ground :)

 But when everyone shares the *assumption* that there is never a commit
 message that is too long, you know the wrestling mat of ideas is
 rigged. I wonder if I should write a commit message as long as a book
 chapter for a one-liner, only to prove a point, but I'm honestly
 afraid that it would be committed as is.

I'm with you, and don't share that assumption.  I'm not accusing you
of writing commit messages that don't conform to some transcendental
standard either: I didn't look at your patches in the first place,
because the simple Signed-off-by: one-liner in the body didn't really
make me want to read it.

 And remember what started the conversation; do you think a patch with
 a possibly incomplete commit message should not be merged to pu
 (proposed updates), shouldn't even be mentioned in the what's
 cooking mail, and thus shouldn't even be considered cooking?

It's irrelevant what I think or others think.  The point is that it
wasn't mentioned.  Now, why wasn't it mentioned?  Is it because Junio
and the community hate you, and are conspiring against getting your
code merged?  Or is it because it didn't catch anyone's eye, and Junio
was waiting for it to happen (as always)?

TL;DR version: Your goal in submitting a patch is to sell it to other
people in the community.  If enough people like your patch, it gets
merged (but that is only the second step).  Your goal is not to fix
problems for some unknown users, or argue about some transcendental
standard.  Ofcourse the community shares some view about what a patch
should look like, but you can mould those expectations gradually.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 5:27 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Felipe Contreras wrote:
 Except the customers are not git developers, it's git users. Git
 developers rejecting patches because of the commit message is akin to
 distributors rejecting products because they don't like the
 transportation packages; they are only hurting themselves, by hurting
 their customers.

 Huh?  I certainly don't develop for some git users I don't even know
 or care about.  In this order of precedence, my customers are:

 1. Me.
 2. People who develop git.git, whom I have to cooperate with.
 3. People who exercise git heavily like the linux.git community, as
 opposed to some little projects that operate using pull requests on
 GitHub.
 ...
 137. People who incidentally choose to use git.
 138. People who incidentally choose to use git, but aren't on Linux.

As they are for most open source developers on the planet, not me. I
believe a project is nothing without its users.

 And nobody is hurting anyone else.  Someone wrote some code, and
 failed to sell it to the community.  That's all happened.

If remote-hg wasn't available for users, they would be hurt; if stash
wasn't available, if rebase --interactive didn't exist, if there was
no msysgit, if it wasn't so fast, if the object model wasn't so simple
and extensible; users would be hurt. And if users didn't have all
these, there would be less users, and if there were less users, there
would be less developers, and mercurial might have been more popular,
and most repositories you have to work on would be in mercurial, and
you might be developing mercurial right now.

But I won't bother trying to convince you that no project is more
important than its users (in the words of Linus Torvalds), because
most people don't see the big picture.

 I don't think so. Unless you added your Signed-off-by, you are not.

 Okay, so your view differs.

 I am not. Neither should they ask me to write the commit messages they
 want. They can make *suggestions*, and I can reject them.

 Ofcourse you have a right to reject suggestions.  The question at the
 end of the day doesn't change: did you manage to get people to read
 your patch?

No, at the end of the day what matters is: did the users benefit from this?

The answer for this particular patch is no, and it's not my fault.

 When two persons have different ideas, often times both are wrong, and
 the middle-ground is best, but sometimes a person reaches the
 middle-ground, and sometimes one person was right from the start.

 https://yourlogicalfallacyis.com/middle-ground :)

Yeah, but I didn't claim that, I said sometimes one person was right
from the start, no middle-ground.

 But when everyone shares the *assumption* that there is never a commit
 message that is too long, you know the wrestling mat of ideas is
 rigged. I wonder if I should write a commit message as long as a book
 chapter for a one-liner, only to prove a point, but I'm honestly
 afraid that it would be committed as is.

 I'm with you, and don't share that assumption.

s/everyone/almost everyone/

 I'm not accusing you
 of writing commit messages that don't conform to some transcendental
 standard either: I didn't look at your patches in the first place,
 because the simple Signed-off-by: one-liner in the body didn't really
 make me want to read it.

 And remember what started the conversation; do you think a patch with
 a possibly incomplete commit message should not be merged to pu
 (proposed updates), shouldn't even be mentioned in the what's
 cooking mail, and thus shouldn't even be considered cooking?

 It's irrelevant what I think or others think.  The point is that it
 wasn't mentioned.  Now, why wasn't it mentioned?  Is it because Junio
 and the community hate you, and are conspiring against getting your
 code merged?  Or is it because it didn't catch anyone's eye, and Junio
 was waiting for it to happen (as always)?

My abridged version of the story is: because Jeff King pointed to an
area of improvement he wasn't even strongly attached to, I agreed to
resubmit, Junio saw that, then I changed my mind, Junio probably
didn't see that, and then he forgot about it.

Then, since it's taboo to suggest that a concise commit message is
fine, a discussion sprung.

 TL;DR version: Your goal in submitting a patch is to sell it to other
 people in the community.

I disagree.

 If enough people like your patch, it gets
 merged (but that is only the second step).  Your goal is not to fix
 problems for some unknown users, or argue about some transcendental
 standard.

I disagree.

 Ofcourse the community shares some view about what a patch
 should look like, but you can mould those expectations gradually.

If experience is any guide, doesn't look like that. But I've noticed
that after many months that a patch has been sent people realize that
it's more important to get the damn issue fixed than to have a hugely
verbose commit message...

Cheers.

-- 
Felipe 

Re: Bug with rev-list --reverse?

2013-04-18 Thread Peter Krefting

Felipe Contreras:


% git log --oneline -1 v1.8.1.5^..v1.8.1.6
% git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6

I expect to get a different output, and not both showing v1.8.1.6.
Wouldn't you agree?


 Quoting the manual page:

 Commit Limiting
   Besides specifying a range of commits that should be listed using the 
special notations explained in the description, additional commit limiting may 
be applied. Note that they are applied before commit ordering and formatting 
options, such as --reverse.

Given that, I would expect the output to be the same.

--
\\// Peter - http://www.softwolves.pp.se/
--
To unsubscribe from this list: send the line 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: put THEIR commits AFTER my commits with a single rebase command

2013-04-18 Thread Johannes Sixt
Am 4/18/2013 10:33, schrieb Ilya Basin:
 
 JS Perhaps this one:
 
 JSgit merge origin/master
 JSgit rebase ORIG_HEAD
 
 JS -- Hannes
 
 Wouldn't I have to resolve conflicts twice?

Yes. But you did run 'git config rerere.enabled true' when you started
with git, didn't you? ;-)

Anyway, Johan's idea to use git cherry-pick is much better.

 BTW, during the rebase, can I tell git to rewrite a different branch
 upon rebase success or abort?
 
 git branch -f tmp origin/master
 git rebase --onto master master tmp
 if [ $? -ne 0 ]; then
# modify some file in .git/ ?

What do you expect here? Failure of git rebase means that it is not
complete, yet. So far, nothing has been rewritten. So what? Perhaps you mean:
# never mind
git rebase --abort

 else
 git branch -f master
 git checkout master
 fi

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


Re: Bug with rev-list --reverse?

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 5:26 AM, John Keeping j...@keeping.me.uk wrote:
 On Thu, Apr 18, 2013 at 05:17:14AM -0500, Felipe Contreras wrote:
 If I do these:

 % git log --oneline -1 v1.8.1.5^..v1.8.1.6
 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6

 I expect to get a different output, and not both showing v1.8.1.6.
 Wouldn't you agree?

 I expect to get the same output.  This is probably because I consider
 --reverse to be an output filter.  So I expect to show the commits
 v1.8.1.5^..v1.8.1.6 -1 which selects a single commit and then show
 that in reverse order.

How about this:

% git log --oneline --reverse --max-count=1 v1.8.1.5^..v1.8.1.6

In this case --max-count is acting as start from the first commit
before the tip, not as output a maximum of one commit. Given that
the name is max-count, I expect it to be the later.

And if max-count doesn't select a maximum of n commits, then what does?

-- 
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: Bug with rev-list --reverse?

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 5:47 AM, Peter Krefting pe...@softwolves.pp.se wrote:

 % git log --oneline -1 v1.8.1.5^..v1.8.1.6
 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6

 I expect to get a different output, and not both showing v1.8.1.6.
 Wouldn't you agree?


  Quoting the manual page:

  Commit Limiting
Besides specifying a range of commits that should be listed using the
 special notations explained in the description, additional commit limiting
 may be applied. Note that they are applied before commit ordering and
 formatting options, such as --reverse.

 Given that, I would expect the output to be the same.

If expectations were based on documentation, all one has to do is
document bugs, and there would be no bugs anymore :)

Code can be changed to fit more appropriately user expectations (which
are independent of documentation), and the documentation updated
accordingly.

-- 
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: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Ramkumar Ramachandra
Since you disagreed with the rest, I'll only respond to this part:

Felipe Contreras wrote:
 But I won't bother trying to convince you that no project is more
 important than its users (in the words of Linus Torvalds), because
 most people don't see the big picture.

I didn't say otherwise.  What I'm saying is: my personal incentive to
write code does not prioritize the supposed benefit of some unknown
user somewhere on the planet above everything else.  My personal
incentive prioritizes me, and my immediate circle (ie. the git
community).  The benefit propagates outwards to extended circles until
it reaches the people I care least about: incidental end-users.
That's how people are connected: how can I care about distant unknown
people I'm not connected to?  The people in the outermost circles
benefit the least, because they didn't get a say in the development.
All they can do is write a rant about it on their blog, and hope that
it gets fixed someday.

You just ditched us, the inner circle of people who care about your
work the most, and are instead trying to convince us that we're
hurting some unknown hypothetical users by not merging your code
immediately.

If you think these users are more important to you than we are, then
why are you posting your code on this mailing list?  Start your own
project that's focused on satisfying these users.  It doesn't even
need to be open source or have a community of reviewers, because all
you care about are users.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Git crash in Ubuntu 12.04

2013-04-18 Thread Sivaram Kannan
Hi,

 Probably not because there are no debugging symbols. Not sure how
 ubuntu packages these symbols..

Would recompiling the source packages and debugging would give
different results?


 Any chance you could publish the repository that causes the crash?
 --
 Duy

I don't think I can publish the repo online. But I am willing to try
any steps on the repo to get more info. Most likely I would restore
the latest repo to the stanby server I tested which works with any
crash. Let me know what I can do to get more information.

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


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Ramkumar Ramachandra
Okay, one more segment needs to be responded to.

Felipe Contreras wrote:
 If remote-hg wasn't available for users, they would be hurt; if stash
 wasn't available, if rebase --interactive didn't exist, if there was
 no msysgit, if it wasn't so fast, if the object model wasn't so simple
 and extensible; users would be hurt. And if users didn't have all
 these, there would be less users, and if there were less users, there
 would be less developers, and mercurial might have been more popular,
 and most repositories you have to work on would be in mercurial, and
 you might be developing mercurial right now.

Flawed logic.

A large number of users doesn't automatically imply good software with
lots of features, or even a great development community.  A great
development community leads to great software.  And great software
leads to lots of users.  Sure, there's a feedback loop pushing users
to become developers; but it doesn't start with users of vaporware,
leading to more developers joining the effort to turn that vaporware
into a great product.

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


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 6:31 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Since you disagreed with the rest, I'll only respond to this part:

 Felipe Contreras wrote:
 But I won't bother trying to convince you that no project is more
 important than its users (in the words of Linus Torvalds), because
 most people don't see the big picture.

 I didn't say otherwise.  What I'm saying is: my personal incentive to
 write code does not prioritize the supposed benefit of some unknown
 user somewhere on the planet above everything else.  My personal
 incentive prioritizes me, and my immediate circle (ie. the git
 community).  The benefit propagates outwards to extended circles until
 it reaches the people I care least about: incidental end-users.

If the people that matter most are given the worst prioritization, it
means the prioritization is wrong.

 That's how people are connected: how can I care about distant unknown
 people I'm not connected to?

It's called empathy.

 The people in the outermost circles
 benefit the least, because they didn't get a say in the development.
 All they can do is write a rant about it on their blog, and hope that
 it gets fixed someday.

To the detriment of the project.

 You just ditched us, the inner circle of people who care about your
 work the most, and are instead trying to convince us that we're
 hurting some unknown hypothetical users by not merging your code
 immediately.

The users are real, the developers that will look retroatcively to the
commit message of this patch are not.

 If you think these users are more important to you than we are, then
 why are you posting your code on this mailing list?

What other way is there for this code to reach the users?

 Start your own
 project that's focused on satisfying these users.

Start a new project so I can include a patch that hasn't made it yet
into the what's cooking in one week? That's ridiculous.

 It doesn't even
 need to be open source or have a community of reviewers, because all
 you care about are users.

Who said *all* that matters are the users? And even if somebody did,
ultimately a closed source proprietary software doesn't benefit the
users, so either way it has to be open and active to benefit the
users.

-- 
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: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 6:46 AM, Ramkumar Ramachandra
artag...@gmail.com wrote:
 Okay, one more segment needs to be responded to.

 Felipe Contreras wrote:
 If remote-hg wasn't available for users, they would be hurt; if stash
 wasn't available, if rebase --interactive didn't exist, if there was
 no msysgit, if it wasn't so fast, if the object model wasn't so simple
 and extensible; users would be hurt. And if users didn't have all
 these, there would be less users, and if there were less users, there
 would be less developers, and mercurial might have been more popular,
 and most repositories you have to work on would be in mercurial, and
 you might be developing mercurial right now.

 Flawed logic.

 A large number of users doesn't automatically imply good software with
 lots of features, or even a great development community.  A great
 development community leads to great software.  And great software
 leads to lots of users.  Sure, there's a feedback loop pushing users
 to become developers; but it doesn't start with users of vaporware,
 leading to more developers joining the effort to turn that vaporware
 into a great product.

 Life doesn't begin with users.

Nobody knows how life began, and it doesn't matter now, what matters
is how life evolves. It doesn't matter if the chicken was first, or
the egg, what matters is that if all the chickens and eggs are gone,
there won't be more.

Plenty of projects have died because they stopped caring about their
users, and without users there's no new developers, and the old
developers eventually move on, and all the literary quality of commit
messages have no eyes to see it.

I repeat: no project is more important than its users.

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


Re: [PATCH 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread Thomas Rast
Felipe Contreras felipe.contre...@gmail.com writes:

 On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast tr...@inf.ethz.ch wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 The *:* refspec doesn't work, and never has, clarify the code and
 documentation to reflect that. This in effect reverts commit 9e7673e
 (gitremote-helpers(1): clarify refspec behaviour).
 [...]
 -test_expect_success 'pulling with straight refspec' '
 - (cd local2 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) 
 - compare_refs local2 HEAD server HEAD
 -'
 -
 -test_expect_failure 'pushing with straight refspec' '
 - test_when_finished (cd local2  git reset --hard origin) 
 - (cd local2 
 - echo content file 
 - git commit -a -m eleven 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) 
 - compare_refs local2 HEAD server HEAD
 -'

 So what's wrong with the tests?  Do they fail to test what they claim
 (how?), test something that wasn't reasonable to begin with, or
 something entirely different?

 Look at the code comment, and look at the now updated documentation
 that assumes that *:* was reasonable. Given the available information,
 it would be reasonable to assume that *:* did work, but it didn't
 work, and it's not really possible to fix it, even if we wanted to, it
 would be a hack. It's better to accept that fact and stop worrying too
 much about what would be the best way to do the wrong thing.

Ok, you say that the *failing* test set an expectation that is
unrealistic, so let's drop it.

But then what about the successful test?  Does it actually work (and by
removing the test, you are saying that we don't care if we subsequently
break that (mis)feature)?  Or did it test the wrong thing?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/6] transport-helper: clarify *:* refspec

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 7:39 AM, Thomas Rast tr...@inf.ethz.ch wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 On Thu, Apr 18, 2013 at 2:28 AM, Thomas Rast tr...@inf.ethz.ch wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 The *:* refspec doesn't work, and never has, clarify the code and
 documentation to reflect that. This in effect reverts commit 9e7673e
 (gitremote-helpers(1): clarify refspec behaviour).
 [...]
 -test_expect_success 'pulling with straight refspec' '
 - (cd local2 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) 
 - compare_refs local2 HEAD server HEAD
 -'
 -
 -test_expect_failure 'pushing with straight refspec' '
 - test_when_finished (cd local2  git reset --hard origin) 
 - (cd local2 
 - echo content file 
 - git commit -a -m eleven 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git push) 
 - compare_refs local2 HEAD server HEAD
 -'

 So what's wrong with the tests?  Do they fail to test what they claim
 (how?), test something that wasn't reasonable to begin with, or
 something entirely different?

 Look at the code comment, and look at the now updated documentation
 that assumes that *:* was reasonable. Given the available information,
 it would be reasonable to assume that *:* did work, but it didn't
 work, and it's not really possible to fix it, even if we wanted to, it
 would be a hack. It's better to accept that fact and stop worrying too
 much about what would be the best way to do the wrong thing.

 Ok, you say that the *failing* test set an expectation that is
 unrealistic, so let's drop it.

 But then what about the successful test?  Does it actually work (and by
 removing the test, you are saying that we don't care if we subsequently
 break that (mis)feature)?  Or did it test the wrong thing?

Yeah, it works, in the sense that peeing in a bottle is a solution; it
might work, but it's not recommendable. So, if suddenly working,
frankly I don't care. I added those tests, and I don't think they are
needed. In a not too distant future it should not be permitted to
work; we don't want developers to shoot themselves in the foot, and
heir users too.

-- 
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: Bug with rev-list --reverse?

2013-04-18 Thread Thomas Rast
Felipe Contreras felipe.contre...@gmail.com writes:

 On Thu, Apr 18, 2013 at 5:47 AM, Peter Krefting pe...@softwolves.pp.se 
 wrote:

 % git log --oneline -1 v1.8.1.5^..v1.8.1.6
 % git log --oneline --reverse -1 v1.8.1.5^..v1.8.1.6

 I expect to get a different output, and not both showing v1.8.1.6.
 Wouldn't you agree?


  Quoting the manual page:

  Commit Limiting
Besides specifying a range of commits that should be listed using the
 special notations explained in the description, additional commit limiting
 may be applied. Note that they are applied before commit ordering and
 formatting options, such as --reverse.

 Given that, I would expect the output to be the same.

 If expectations were based on documentation, all one has to do is
 document bugs, and there would be no bugs anymore :)

 Code can be changed to fit more appropriately user expectations (which
 are independent of documentation), and the documentation updated
 accordingly.

It's been this way forever, and applies to rev-list where we can't just
break how options work (for fear of breaking scripts).

You could come up with a patch series that first starts emitting
warnings whenever the user asks for behavior that will change, and later
flips the default and removes the warning (the latter would be merged
for 2.0 or so).

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option

2013-04-18 Thread Ramkumar Ramachandra
John Keeping wrote:
 This adds a prefix string to any filename arguments encountered after it
 has been specified.

Very nice.  I thought we'd have to resort to path mangling in shell to
fix git-submodule.sh.  Glad to see that we can go with something
cleaner.

Perhaps pull some bits from your nice Documentation into the commit message?
 diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
 index f267a1d..de894c7 100644
 --- a/builtin/rev-parse.c
 +++ b/builtin/rev-parse.c
 @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const 
 char *datestr)
 show(buffer);
  }

 -static int show_file(const char *arg)
 +static int show_file(const char *arg, int output_prefix)

Okay, so you've essentially patched show_file() to accept an
additional argument, and modified callers to call with this additional
argument.  I suppose
show_(rev|reference|default|flag|rev|with_type|datestring|abbrev)
don't need to be patched, as they are path-independent.

  {
 show_default();
 if ((filter  (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
 -   show(arg);
 +   if (output_prefix) {
 +   const char *prefix = startup_info-prefix;
 +   show(prefix_filename(prefix,
 +prefix ? strlen(prefix) : 0,
 +arg));
 +   } else
 +   show(arg);

Uh, why do you need output_prefix?  If startup_info-prefix is set,
use it.  Is startup_info-prefix set by anyone by cmd_rev_parse()?

 @@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n
  int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const 
 char *prefix)
 i++;
 continue;
 }
 +   if (!strcmp(arg, --prefix)) {
 +   prefix = argv[i+1];
 +   startup_info-prefix = prefix;
 +   output_prefix = 1;
 +   i++;
 +   continue;
 +   }

Wait, why isn't prefix filled in when run_builtin() calls this?  Oh,
right: because we didn't mark this builtin with RUN_SETUP or
RUN_SETUP_GENTLY.  Okay, now why didn't we change that?  Because it
would be a major problem (all our scripts would break) if rev-parse
did cd-to-toplevel.

Why are you setting prefix to argv[i+1], and then setting
startup_info-prefix to that?  Is anyone else in cmd_rev_parse() going
to use it?

 +prefix=$(git rev-parse --show-prefix)
 +cd $(git rev-parse --show-toplevel)
 +eval set -- $(git rev-parse --sq --prefix $prefix $@)

I'm wondering if you need such a convoluted usage though.  Will you
ever need to specify a prefix by hand that is different from what git
rev-parse --show-toplevel returns?  If not, why don't you just
rev-parse --emulate-toplevel, and get rid of specifying prefix by hand
altogether?  Then again, this is a plumbing command, so the simplicity
is probably more valuable.

 diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh
 new file mode 100755
 index 000..5ef48d2
 --- /dev/null
 +++ b/t/t1513-rev-parse-prefix.sh
 +test_expect_success 'empty prefix -- file' '
 +   git rev-parse --prefix  -- top sub1/file1 actual 
 +   cat -EOF expected 

Nit: when you're not putting in variables, you can cat -\EOF.

 +test_expect_success 'empty prefix HEAD:./path' '
 +   git rev-parse --prefix  HEAD:./top actual 
 +   git rev-parse HEAD:top expected 

Nit: why did you change ./top to top?  Your --prefix option
doesn't require you to change your arguments accordingly, does it?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/2] rev-parse: add --filename-prefix option

2013-04-18 Thread John Keeping
On Thu, Apr 18, 2013 at 07:58:25PM +0530, Ramkumar Ramachandra wrote:
 John Keeping wrote:
  This adds a prefix string to any filename arguments encountered after it
  has been specified.
 
 Very nice.  I thought we'd have to resort to path mangling in shell to
 fix git-submodule.sh.  Glad to see that we can go with something
 cleaner.
 
 Perhaps pull some bits from your nice Documentation into the commit message?

Good idea.  I intended to re-write the commit message for v2 since the
patch was completely re-written but forgot by the time I'd sorted out
patch 2 as well.  I will do for v3.

  diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
  index f267a1d..de894c7 100644
  --- a/builtin/rev-parse.c
  +++ b/builtin/rev-parse.c
  @@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const 
  char *datestr)
  show(buffer);
   }
 
  -static int show_file(const char *arg)
  +static int show_file(const char *arg, int output_prefix)
 
 Okay, so you've essentially patched show_file() to accept an
 additional argument, and modified callers to call with this additional
 argument.  I suppose
 show_(rev|reference|default|flag|rev|with_type|datestring|abbrev)
 don't need to be patched, as they are path-independent.
 
   {
  show_default();
  if ((filter  (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
  -   show(arg);
  +   if (output_prefix) {
  +   const char *prefix = startup_info-prefix;
  +   show(prefix_filename(prefix,
  +prefix ? strlen(prefix) : 0,
  +arg));
  +   } else
  +   show(arg);
 
 Uh, why do you need output_prefix?  If startup_info-prefix is set,
 use it.  Is startup_info-prefix set by anyone by cmd_rev_parse()?

output_prefix is a flag to say do we want to show the prefix.  We need
it because show_file is used for the -- argument separator as well as
file paths.  Without a separate flag we end up prefixing -- with the
prefix path.

  @@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n
   int cmd_rev_parse(int argc, const char **argv, const char *prefix)
  @@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const 
  char *prefix)
  i++;
  continue;
  }
  +   if (!strcmp(arg, --prefix)) {
  +   prefix = argv[i+1];
  +   startup_info-prefix = prefix;
  +   output_prefix = 1;
  +   i++;
  +   continue;
  +   }
 
 Wait, why isn't prefix filled in when run_builtin() calls this?  Oh,
 right: because we didn't mark this builtin with RUN_SETUP or
 RUN_SETUP_GENTLY.  Okay, now why didn't we change that?  Because it
 would be a major problem (all our scripts would break) if rev-parse
 did cd-to-toplevel.

prefix is already set, by setup_git_git_directory.  The point is that we
just change the values set in setup_git_directory so that the command
behaves as if it were run from a subdirectory.

 Why are you setting prefix to argv[i+1], and then setting
 startup_info-prefix to that?  Is anyone else in cmd_rev_parse() going
 to use it?
 
  +prefix=$(git rev-parse --show-prefix)
  +cd $(git rev-parse --show-toplevel)
  +eval set -- $(git rev-parse --sq --prefix $prefix $@)
 
 I'm wondering if you need such a convoluted usage though.  Will you
 ever need to specify a prefix by hand that is different from what git
 rev-parse --show-toplevel returns?  If not, why don't you just
 rev-parse --emulate-toplevel, and get rid of specifying prefix by hand
 altogether?  Then again, this is a plumbing command, so the simplicity
 is probably more valuable.

How does that work?  When we run rev-parse with the --prefix argument
we're no longer in the subdirectory.

While this may look convoluted here, I don't think it is in normal usage
inside a script.  If you look at the way it's used in patch 2 we're
careful not to just remap all the arguments but to extract the flags
before remapping file paths when we know that everything we have is a
file path.

  diff --git a/t/t1513-rev-parse-prefix.sh b/t/t1513-rev-parse-prefix.sh
  new file mode 100755
  index 000..5ef48d2
  --- /dev/null
  +++ b/t/t1513-rev-parse-prefix.sh
  +test_expect_success 'empty prefix -- file' '
  +   git rev-parse --prefix  -- top sub1/file1 actual 
  +   cat -EOF expected 
 
 Nit: when you're not putting in variables, you can cat -\EOF.
 
  +test_expect_success 'empty prefix HEAD:./path' '
  +   git rev-parse --prefix  HEAD:./top actual 
  +   git rev-parse HEAD:top expected 
 
 Nit: why did you change ./top to top?  Your --prefix option
 doesn't require you to change your arguments accordingly, does it?

The 

Re: [PATCH v2 2/2] submodule: drop the top-level requirement

2013-04-18 Thread John Keeping
On Thu, Apr 18, 2013 at 08:16:42PM +0530, Ramkumar Ramachandra wrote:
 John Keeping wrote:
  Use the new rev-parse --prefix option to process all paths given to the
  submodule command, dropping the requirement that it be run from the
  top-level of the repository.
 
 Yay!
 
  diff --git a/git-submodule.sh b/git-submodule.sh
  index 79bfaac..bbf7983 100755
  --- a/git-submodule.sh
  +++ b/git-submodule.sh
  @@ -112,6 +115,7 @@ resolve_relative_url ()
   #
   module_list()
   {
  +   eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)
 
 Nit: Why not use -- to disambiguate between options and arguments,
 like you showed in your intended usage?

Erm... it does.  What's before $@?

Ah, you mean after set.  In this case, rev-parse will keep the -- we
supply to it, so we get that from there and do not want to give set an
extra one.

 So, this essentially converts all the paths specified in $@ to
 toplevel-relative paths.  Beautiful, as this is practically the only
 change you require in each function.
 
  +   sm_path=$wt_prefix$sm_path
 
 Wait, isn't sm_path the value of submodule.name.path in .gitmodules?
  Why does it need to be prefixed?

I think you've lost too much context here.  This is in cmd_add and at
this point sm_path holds the argument supplied by the user, which we
must adjust as it will be relative to the current directory.

We should probably be testing for an absolute path here though.

  @@ -942,6 +948,7 @@ cmd_summary() {
  fi
 
  cd_to_toplevel
  +   eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)
 
 Um, what about other functions?  Yeah, it looks like all of them
 except this one call module_list().

Exactly.  Apart from this and cmd_add everything uses module_list.

  diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
  index ff26535..7795f21 100755
  --- a/t/t7400-submodule-basic.sh
  +++ b/t/t7400-submodule-basic.sh
  @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and // 
  in path' '
  test_cmp empty untracked
   '
 
  +test_expect_success 'submodule add in subdir' '
  +   echo refs/heads/master expect 
  +   empty 
 
 Nit: Use : empty.  It's clearer.
 
  +   (
  +   mkdir addtest/sub 
 
 Why is the mkdir inside the subshell?  The next statement (cd) onwards
 should be in the subshell, to save you cd'ing back.

  +   cd addtest/sub 
  +   git submodule add $submodurl ../realsubmod3 
  +   git submodule init
  +   ) 
  +
  +   rm -f heads head untracked 
 
 Huh?  What is this in the middle?  The next statement (call to
 inspect) write-truncates them anyway, so this is unnecessary.

I just followed the surrounding tests here.  I think it's better for
follow the pattern of the surrounding code here than get this one test
perfect.  A cleanup can follow on top if someone wants to do it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: git p4 submit failing

2013-04-18 Thread Christopher Yee Mon
The issue is caused by the line endings. I retested the problem with a
different file and in this case, the error is caused by the line
endings of the file checked out in the perforce workspace being
win-style crlf, and the line endings of the file in the git repo being
unix style lf. (In this scenario, I took out the .gitattributes,
core.autocrlf was set to false and LineEnd was set to share)

In this case, I checked out the file in perforce, ran dos2unix against
it, and submitted that, then ran git p4 submit and it worked.

I noticed that the error is caused by the git apply failing in the def
applyCommit(self, id) function at lines 1296-1305.

diffcmd = git format-patch -k --stdout \%s^\..\%s\ % (id, id)
patchcmd = diffcmd +  | git apply 
tryPatchCmd = patchcmd + --check -
applyPatchCmd = patchcmd + --check --apply -
patch_succeeded = True

if os.system(tryPatchCmd) != 0:
fixed_rcs_keywords = False
patch_succeeded = False
print Unfortunately applying the change failed!

So most likely in git apply command, it can't find the changes because
of the line endings being different between them. I couldn't find a
parameter that would magically make it work. When I added --verbose to
git apply the output only says:
error: while searching for:
and then the first lines of the first diff

Hello Simon,

I have CCed you to alert you to the possible bug. Any assistance would
be appreciated.


On Sat, Apr 13, 2013 at 5:09 PM, Christopher Yee Mon
christopher.yee...@gmail.com wrote:
 Yes this is the case.

 Many of the files have crlf endings.

 core.autocrlf was recently set to input. I can't remember the timeline
 exactly though, but in addition to this, I have a .gitattributes file
 with the default set to text=auto (* text=auto) and the php files set
 to text eol=lf (*.php text eol=lf) Also my perforce workspace's
 LineEnd setting is set to Share.

 I've experienced the behavior in both .php and .xml files though

 Before all of this started I had core.autocrlf set to false, and no
 .gitattributes file and perforce workspace's LineEnd was set to the
 default, but I got a conflict where the only difference was the line
 endings, so I changed things to the way they are now.

 Any recommendations? Should I change everything back the way it was?

 On Sat, Apr 13, 2013 at 5:51 PM, Pete Wyckoff p...@padd.com wrote:
 l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100:
 Just a thought, but check the files that are failing to see if they've
 got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all
 sorts of nasty problems.

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

 I had recently debugged a similar-looking problem
 where core.autocrlf was set to input.

 Christopher, if you have this set and/or the .xml files
 have ^M (CRLF) line endings, please let us know.

 -- Pete


 On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon
 christopher.yee...@gmail.com wrote:
  I tried running git p4 submit on a repo that I've been running as an
  interim bridge between git and perforce. Multiple people are using the
  repo as a remote and its being periodically submitted back to
  perforce.
 
  It's been working mostly fine. Then one day out of the blue I get this
  error. I can no longer push any git commits to perforce. (This is from
  the remote repo which I am pushing back to perforce)
 
  user@hostname:~/Source/code$ git p4 submit -M --export-labels
  Perforce checkout for depot path //depot/perforce/workspace/ located
  at /home/user/Source/git-p4-area/perforce/workspace/
  Synchronizing p4 checkout...
  ... - file(s) up-to-date.
  Applying ffa390f comments in config xml files
  //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened for 
  edit
  //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened for 
  edit
  //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened for 
  edit
  //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened for 
  edit
  //depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened for 
  edit
  error: patch failed: sub/folder/structure/first.xml:1
  error: sub/folder/structure/first.xml: patch does not apply
  error: patch failed: sub/folder/structure/second.xml:1
  error: sub/folder/structure/second.xml: patch does not apply
  error: patch failed: sub/folder/structure/third.xml:1
  error: sub/folder/structure/third.xml: patch does not apply
  error: patch failed: sub/folder/structure/forth.xml:1
  error: sub/folder/structure/forth.xml: patch does not apply
  error: patch failed: sub/folder/structure/fifth.xml:1
  error: sub/folder/structure/fifth.xml: patch does not apply
  Unfortunately applying the change failed!
  //depot/perforce/workspace/sub/folder/structure/first.xml#1 - was edit, 
  reverted
  //depot/perforce/workspace/sub/folder/structure/second.xml#3 - was
  edit, reverted
  

Re: [PATCH] t3400 (rebase): add failing test for a peculiar rev spec

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

 'git rebase' does not recognize revisions specified as :/text.  This
 is because the attempts to rev-parse ${REV}^0, which fails in this
 case.  Add a test to document this failure.

  - The failure occurs in git-rebase.sh:403.  Is it using the ^0 only
to make sure that the revision specified is a commit?  Surely,
there'a a better way to do this?

  Can someone point me in the right direction?

How about ${REV}^0 into

nREV=$(git rev-parse ${REV})^0

and use it where you need an object name that needs to be parsed by
get_sha1(), e.g.

git checkout -q $nREV^0

I would suggest a helper function in git-sh-setup, something like:

peel_committish () {
case $1 in
:/*)
peeltmp=$(git rev-parse --verify $1) 
git rev-parse --verify $peeltmp^0
;;
*)
git rev-parse --verify $1^0
;;
esac
}

--
To unsubscribe from this list: send the line 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] blame: handle broken commit headers gracefully

2013-04-18 Thread Jeff King
On Wed, Apr 17, 2013 at 02:55:29PM -0700, Junio C Hamano wrote:

 Or you can imagine nastier input strings, like
 
Name -email@host 123456789 -
Name ema-il@host 123456789 -
Name email@host~ 123456789 -
 
 I am afraid that at some point we should salvage as much as we
 can, which is a worthy goal, becomes a losing proposition.

Good point. In the worst cases, even if you cleaned things up, you might
even need to allocate a new string (like your middle one), which would
make calling split_ident_line a lot more annoying. Probably not worth
the effort.

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


Re: t6200: avoid path mangling issue on Windows

2013-04-18 Thread Junio C Hamano
Johannes Sixt j.s...@viscovery.net writes:

 From: Johannes Sixt j...@kdbg.org

 MSYS bash interprets the slash in the argument core.commentchar=/
 as root directory and mangles it into a Windows style path. Use a
 different core.commentchar to dodge the issue.

 Signed-off-by: Johannes Sixt j...@kdbg.org
 ...
 - git -c core.commentchar=/ fmt-merge-msg --log=5 .git/FETCH_HEAD 
 actual 
 + git -c core.commentchar=x fmt-merge-msg --log=5 .git/FETCH_HEAD 
 actual 

Sigh... Again?

Are folks working on Msys bash aware that sometimes the users may
want to say key=value on their command line without the value
getting molested in any way and giving them some escape hatch would
help them?  Perhaps they have already decided that it is not
feasible after thinking about the issue, in which case I do not have
new ideas to offer.

I'll apply the patch as-is, but this feels really painful to the
users.

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


[PATCHv2] l10n: de.po: translate 39 new messages

2013-04-18 Thread Ralf Thielow
Translate 39 new messages came from git.pot update in
c138af5 (l10n: git.pot: v1.8.3 round 1 (54 new, 15 removed)).

While at there, fix some small issues.

Signed-off-by: Ralf Thielow ralf.thie...@gmail.com
Acked-by: Thomas Rast tr...@inf.ethz.ch
---
 po/de.po | 206 +--
 1 file changed, 108 insertions(+), 98 deletions(-)

diff --git a/po/de.po b/po/de.po
index 2cd3fa9..88d7919 100644
--- a/po/de.po
+++ b/po/de.po
@@ -136,12 +136,13 @@ msgstr 
 #: branch.c:201
 #, c-format
 msgid Cannot setup tracking information; starting point '%s' is not a branch.
-msgstr 
+msgstr Kann Informationen zum Übernahmezweig nicht einrichten; 
+Startpunkt '%s' ist kein Zweig.
 
 #: branch.c:203
-#, fuzzy, c-format
+#, c-format
 msgid the requested upstream branch '%s' does not exist
-msgstr Zweig '%s' existiert nicht
+msgstr der angeforderte externe Übernahmezweig '%s' existiert nicht
 
 #: branch.c:205
 msgid 
@@ -154,6 +155,15 @@ msgid 
 will track its remote counterpart, you may want to use\n
 \git push -u\ to set the upstream config as you push.
 msgstr 
+\n
+Falls Sie vorhaben, Ihre Arbeit auf einem bereits existierenden,\n
+externen Übernahmezweig aufzubauen, sollten Sie \git fetch\\n
+ausführen, um diesen abzurufen.\n
+\n
+Falls Sie vorhaben, einen neuen lokalen Zweig zu versenden\n
+der seinem externen Gegenstück folgen soll, können Sie\n
+\git push -u\ verwenden, um den externen Übernahmezweig\n
+beim Versand zu konfigurieren.
 
 #: bundle.c:36
 #, c-format
@@ -181,22 +191,22 @@ msgid revision walk setup failed
 msgstr Einrichtung des Revisionsgangs fehlgeschlagen
 
 #: bundle.c:186
-#, fuzzy, c-format
+#, c-format
 msgid The bundle contains this ref:
 msgid_plural The bundle contains these %d refs:
-msgstr[0] Das Paket enthält %d Referenz
-msgstr[1] Das Paket enthält %d Referenzen
+msgstr[0] Das Paket enthält diese Referenz:
+msgstr[1] Das Paket enthält diese %d Referenzen:
 
 #: bundle.c:193
 msgid The bundle records a complete history.
 msgstr Das Paket speichert eine komplette Historie.
 
 #: bundle.c:195
-#, fuzzy, c-format
+#, c-format
 msgid The bundle requires this ref:
 msgid_plural The bundle requires these %d refs:
-msgstr[0] Das Paket benötigt diese Referenz
-msgstr[1] Das Paket benötigt diese %d Referenzen
+msgstr[0] Das Paket benötigt diese Referenz:
+msgstr[1] Das Paket benötigt diese %d Referenzen:
 
 #: bundle.c:294
 msgid rev-list died
@@ -731,7 +741,7 @@ msgid Unable to write index.
 msgstr Konnte Bereitstellung nicht schreiben.
 
 #: object.c:195
-#, fuzzy, c-format
+#, c-format
 msgid unable to parse object: %s
 msgstr Konnte Objekt '%s' nicht parsen.
 
@@ -933,7 +943,7 @@ msgstr Kann keine Versionsbeschreibung für %s bekommen
 #: sequencer.c:621
 #, c-format
 msgid could not revert %s... %s
-msgstr Konnte %s nicht zurücksetzen... %s
+msgstr Konnte %s nicht zurücknehmen... %s
 
 #: sequencer.c:622
 #, c-format
@@ -1056,11 +1066,11 @@ msgstr Konnte %s nicht formatieren.
 
 #: sequencer.c:1101
 msgid Can't revert as initial commit
-msgstr Kann nicht zu initialer Version zurücksetzen.
+msgstr Rücknahme-Version kann nicht initial sein.
 
 #: sequencer.c:1102
 msgid Can't cherry-pick into empty head
-msgstr Kann \cherry-pick\ nicht in einem leerem Kopf ausführen.
+msgstr Kann \cherry-pick\ nicht in einem leeren Zweig ausführen.
 
 #: sha1_name.c:1036
 msgid HEAD does not point to a branch
@@ -1376,33 +1386,29 @@ msgid   (all conflicts fixed: run \git commit\)
 msgstr   (alle Konflikte behoben: führen Sie \git commit\ aus)
 
 #: wt-status.c:972
-#, fuzzy, c-format
+#, c-format
 msgid You are currently reverting commit %s.
-msgstr Sie sind gerade bei einer binären Suche in Zweig '%s'.
+msgstr Sie nehmen gerade Version '%s' zurück.
 
 #: wt-status.c:977
-#, fuzzy
 msgid   (fix conflicts and run \git revert --continue\)
 msgstr 
-  (beheben Sie die Konflikte und führen Sie dann \git rebase --continue\ 
+  (beheben Sie die Konflikte und führen Sie dann \git revert --continue\ 
 aus)
 
 #: wt-status.c:980
-#, fuzzy
 msgid   (all conflicts fixed: run \git revert --continue\)
-msgstr   (alle Konflikte behoben: führen Sie \git rebase --continue\ aus)
+msgstr   (alle Konflikte behoben: führen Sie \git revert --continue\ aus)
 
 #: wt-status.c:982
-#, fuzzy
 msgid   (use \git revert --abort\ to cancel the revert operation)
 msgstr 
-  (benutzen Sie \git rebase --abort\ um den ursprünglichen Zweig 
-auszuchecken)
+  (benutzen Sie \git revert --abort\ um die Umkehroperation abzubrechen)
 
 #: wt-status.c:993
-#, fuzzy, c-format
+#, c-format
 msgid You are currently bisecting, started from branch '%s'.
-msgstr Sie sind gerade bei einer binären Suche in Zweig '%s'.
+msgstr Sie sind gerade bei einer binären Suche, gestartet von Zweig '%s'.
 
 #: wt-status.c:997
 msgid You are currently bisecting.
@@ -1419,13 +1425,12 @@ msgid On branch 
 msgstr Auf Zweig 
 
 #: wt-status.c:1186
-#, fuzzy
 msgid HEAD detached at 
-msgstr losgelöste Zweigspitze (HEAD)
+msgstr 

Re: Bug with rev-list --reverse?

2013-04-18 Thread Junio C Hamano
Thomas Rast tr...@inf.ethz.ch writes:

 You could come up with a patch series that first starts emitting
 warnings whenever the user asks for behavior that will change, and later
 flips the default and removes the warning (the latter would be merged
 for 2.0 or so).

Please don't.  The fact that reverse then count mode may be useful
does not mean count then show in reverse mode does not have any
use. git log --oneline --reverse -20 is a very valid way to ask
what did I do recently and get you did this, that and then... in
that order.

Adding a new option can be done anytime without any complex
transition plan.  You may want to introduce a --show-in-reverse
synonym to the current --reverse when you add the new mode of
reversing (--reverse-before-count?) so that eventually we won't have
to ask which kind of reverse an unadorned --reverse option mean?
by deprecating a plain --reverse, though.

--
To unsubscribe from this list: send the line 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/13] utf8.c: add reencode_string_len() that can handle NULs in string

2013-04-18 Thread Junio C Hamano
Nguyễn Thái Ngọc Duy  pclo...@gmail.com writes:

 diff --git a/utf8.h b/utf8.h
 index d3da96f..a43ef9a 100644
 --- a/utf8.h
 +++ b/utf8.h
 @@ -17,12 +17,25 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const 
 char *data, int len,
int indent, int indent2, int width);
  
  #ifndef NO_ICONV
 -char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv);
 -char *reencode_string(const char *in, const char *out_encoding, const char 
 *in_encoding);
 +char *reencode_string_iconv(const char *in, size_t insz,
 + iconv_t conv, int *outsz);
 +char *reencode_string_len(const char *in, int insz,
 +   const char *out_encoding,
 +   const char *in_encoding,
 +   int *outsz);
  #else
 -#define reencode_string(a,b,c) NULL
 +#define reencode_string_len(a,b,c,d,e) NULL
  #endif
  
 +static inline char *reencode_string(const char *in,
 + const char *out_encoding,
 + const char *in_encoding)
 +{
 + return reencode_string_len(in, strlen(in),
 +out_encoding, in_encoding,
 +NULL);
 +}
 +
  int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
  
  #endif

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


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Jeff King
On Wed, Apr 17, 2013 at 06:39:06PM -0700, Junio C Hamano wrote:

 Subject: [PATCH] git add: rework the logic to warn git add pathspec... 
 default change

 [...]

 Rework the logic to detect the case where the behaviour will be
 different in Git 2.0, and issue a warning only when it matters.
 Even with the code before this warning, git add subdir will have
 to traverse the directory in order to find _new_ files the index
 does not know about _anyway_, so we can do this check without adding
 an extra pass to find if pathspec matches any removed file.

Thanks, I think this is looking much better.

A few minor nits on the message itself:

 +static void warn_add_would_remove(const char *path)
 +{
 + warning(_(In Git 2.0, 'git add pathspec...' will also update the\n
 +   index for paths removed from the working tree that match\n
 +   the given pathspec. If you want to 'add' only changed\n
 +   or newly created paths, say 'git add --no-all pathspec...'
 +instead.\n\n

This wrapping looks funny in the actual output, due to the extra
warning: and the lack of newline before instead:

  warning: In Git 2.0, 'git add pathspec...' will also update the
  index for paths removed from the working tree that match
  the given pathspec. If you want to 'add' only changed
  or newly created paths, say 'git add --no-all pathspec...' instead.

Wrapping it like this ends up with a much more natural-looking
paragraph:

  warning: In Git 2.0, 'git add pathspec...' will also update the
  index for paths removed from the working tree that match the given
  pathspec. If you want to 'add' only changed or newly created paths,
  say 'git add --no-all pathspec...' instead.

 +   '%s' would be removed from the index without --no-all.),
 + path);

I think mentioning the filename is a good thing; the original message
left me scratching my head and wondering so, did you add it or not?.
I still think your would be is unnecessarily confusing, though. It is
would be in Git 2.0 without --no-all, but we did not now. Which makes
sense when you think about it, but it took me a moment to parse.

Perhaps we can be more direct with something like:

  warning: did not stage removal of 'foo'

or perhaps the present tense not staging removal of... would be
better.

I also think it makes sense to show every path that is affected, not
just the first one, to be clear about what was done (and what _would_
have been done in Git 2.0).

A patch with all of the suggestions together is below. I still think the
multi-line warning block looks ugly. I kind of like the way advise()
puts hint: on each line. I wonder if we should do the same here.

diff --git a/builtin/add.c b/builtin/add.c
index 4242bce..aae550a 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -52,15 +52,19 @@ static void warn_add_would_remove(const char *path)
return DIFF_STATUS_MODIFIED;
 }
 
+static const char *add_would_remove_warning = N_(
+/* indent for warning:  */
+ In Git 2.0, 'git add pathspec...' will also update the\n
+index for paths removed from the working tree that match the given\n
+pathspec. If you want to 'add' only changed or newly created paths,\n
+say 'git add --no-all pathspec...' instead.\n);
+
 static void warn_add_would_remove(const char *path)
 {
-   warning(_(In Git 2.0, 'git add pathspec...' will also update the\n
- index for paths removed from the working tree that match\n
- the given pathspec. If you want to 'add' only changed\n
- or newly created paths, say 'git add --no-all pathspec...'
-  instead.\n\n
- '%s' would be removed from the index without --no-all.),
-   path);
+   static int warned_once;
+   if (!warned_once++)
+   warning(_(add_would_remove_warning));
+   warning(did not stage removal of '%s', path);
 }
 
 static void update_callback(struct diff_queue_struct *q,
@@ -84,10 +88,8 @@ static void update_callback(struct diff_queue_struct *q,
}
break;
case DIFF_STATUS_DELETED:
-   if (data-warn_add_would_remove) {
+   if (data-warn_add_would_remove)
warn_add_would_remove(path);
-   data-warn_add_would_remove = 0;
-   }
if (data-flags  ADD_CACHE_IGNORE_REMOVAL)
break;
if (!(data-flags  ADD_CACHE_PRETEND))
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/6] transport-helper: clarify *:* refspec

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

 The *:* refspec doesn't work, and never has, clarify the code and
 documentation to reflect that. This in effect reverts commit 9e7673e
 (gitremote-helpers(1): clarify refspec behaviour).
 ...
  applicable refspec takes precedence.  The left-hand of refspecs
  advertised with this capability must cover all refs reported by
 -the list command.  If a helper does not need a specific 'refspec'
 -capability then it should advertise `refspec *:*`.
 +the list command.  If no 'refspec' capability is advertised,
 +there is an implied `refspec *:*`.

I presume

s/.$/, but `*:*` does not work so do not use it./

is implied?

 diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
 index f387027..cd1873c 100755
 --- a/t/t5801-remote-helpers.sh
 +++ b/t/t5801-remote-helpers.sh
 @@ -120,21 +120,6 @@ test_expect_failure 'pushing without refspecs' '
   compare_refs local2 HEAD server HEAD
  '
  
 -test_expect_success 'pulling with straight refspec' '
 - (cd local2 
 - GIT_REMOTE_TESTGIT_REFSPEC=*:* git pull) 
 - compare_refs local2 HEAD server HEAD
 -'

This one made me raise my eyebrows, first.

The only reason this test passes is because this particular
pull, due to what local2 and server happens to have before this
test runs, is an Already up-to-date and a no-op.  You are removing
this because it is not really testing anything useful, and if you
change it to test something real, e.g. by rewinding local2, it will
reveal the breakage.

Am I reading you correctly?

 diff --git a/transport-helper.c b/transport-helper.c
 index dcd8d97..cea787c 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -469,7 +469,7 @@ static int fetch_with_import(struct transport *transport,
* were fetching.
*
* (If no refspec capability was specified, for historical
 -  * reasons we default to *:*.)
 +  * reasons we default to the equivalent of *:*.)
*
* Store the result in to_fetch[i].old_sha1.  Callers such
* as git fetch can use the value to write feedback to the
--
To unsubscribe from this list: send the line 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 3/6] transport-helper: clarify pushing without refspecs

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

 This has never worked, since it's inception the code simply skips all
 the refs, essentially telling fast-export to do nothing.

Good description.


 Let's at least tell the user what's going on.

 Signed-off-by: Felipe Contreras felipe.contre...@gmail.com
 ---
  Documentation/gitremote-helpers.txt | 4 ++--
  t/t5801-remote-helpers.sh   | 6 +++---
  transport-helper.c  | 5 +++--
  3 files changed, 8 insertions(+), 7 deletions(-)

 diff --git a/Documentation/gitremote-helpers.txt 
 b/Documentation/gitremote-helpers.txt
 index ba7240c..4d26e37 100644
 --- a/Documentation/gitremote-helpers.txt
 +++ b/Documentation/gitremote-helpers.txt
 @@ -162,8 +162,8 @@ Miscellaneous capabilities
   For remote helpers that implement 'import' or 'export', this capability
   allows the refs to be constrained to a private namespace, instead of
   writing to refs/heads or refs/remotes directly.
 - It is recommended that all importers providing the 'import' or 'export'
 - capabilities use this.
 + It is recommended that all importers providing the 'import'
 + capability use this. It's mandatory for 'export'.

As [1/6] said *:* does not work and has never worked, and
especially on the push side the patch below makes it clear it was a
glorified no-op, it may be better to say a bit more than use
this. It's mandatory.  It is not like any value makes sense, right?

Perhaps the documentation will be further updated in a later step in
the series to clarify it.  Let's read on til the end of the series...

 diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
 index cd1873c..3eeb309 100755
 --- a/t/t5801-remote-helpers.sh
 +++ b/t/t5801-remote-helpers.sh
 @@ -111,13 +111,13 @@ test_expect_success 'pulling without refspecs' '
   compare_refs local2 HEAD server HEAD
  '
  
 -test_expect_failure 'pushing without refspecs' '
 +test_expect_success 'pushing without refspecs' '
   test_when_finished (cd local2  git reset --hard origin) 
   (cd local2 
   echo content file 
   git commit -a -m ten 
 - GIT_REMOTE_TESTGIT_REFSPEC= git push) 
 - compare_refs local2 HEAD server HEAD
 + GIT_REMOTE_TESTGIT_REFSPEC= test_must_fail git push 2 ../error) 
 + grep remote-helper doesn.t support push; refspec needed error
  '

I somehow like this change that turns a _failure into a _success by
expecting test_must_fail, especially because it is clear why the
tested push should fail when you look at the rest of the patch ;-)

Fun.

 diff --git a/transport-helper.c b/transport-helper.c
 index cea787c..4d98567 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport 
 *transport,
   struct string_list revlist_args = STRING_LIST_INIT_NODUP;
   struct strbuf buf = STRBUF_INIT;
  
 + if (!data-refspecs)
 + die(remote-helper doesn't support push; refspec needed);
 +
   helper = get_helper(transport);
  
   write_constant(helper-in, export\n);
 @@ -795,8 +798,6 @@ static int push_refs_with_export(struct transport 
 *transport,
   char *private;
   unsigned char sha1[20];
  
 - if (!data-refspecs)
 - continue;
   private = apply_refspecs(data-refspecs, data-refspec_nr, 
 ref-name);
   if (private  !get_sha1(private, sha1)) {
   strbuf_addf(buf, ^%s, private);
--
To unsubscribe from this list: send the line 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 3/6] transport-helper: clarify pushing without refspecs

2013-04-18 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 +It is recommended that all importers providing the 'import'
 +capability use this. It's mandatory for 'export'.

 s/It's/It is/

I personally do not care _too_ deeply either way, but I agree our
documentation tends to use the latter more and being consistent
would be good.

 diff --git a/transport-helper.c b/transport-helper.c
 index cea787c..4d98567 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport 
 *transport,
  struct string_list revlist_args = STRING_LIST_INIT_NODUP;
  struct strbuf buf = STRBUF_INIT;
  
 +if (!data-refspecs)
 +die(remote-helper doesn't support push; refspec needed);

 I think the refspec needed text is likely to be confusing if an
 end-user ever sees this message.  I'm not sure how we can provide useful
 feedback for both remote helper authors and end-users though.

This refspecs only come via the helper and not directly from the
end user, no?

If that is the case, I do not think confusing is much of an issue.
Not _(localizing) is also the right thing to do.  We may want to
say BUG:  at front to clarify it is not the end-user's fault, but
a problem they may want to report.  If we at this point know what
helper attempted export without giving refspecs, it may help to show
it here, so that developers will know with what helper the user
had problems with.
--
To unsubscribe from this list: send the line 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/6] transport-helper: warn when refspec is not used

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

 For the modes that need it. In the future we should probably error out,
 instead of providing half-assed support.

 The reason we want to do this is because if it's not present, the remote
 helper might be updating refs/heads/*, or refs/remotes/origin/*,
 directly, and in the process fetch will get confused trying to update
 refs that are already updated, or older than what they should be. We
 shouldn't be messing with the rest of git.

So that answers my question in the response to an earlier one in
this series.  We expect the ref updates to be done by the fetch or
push that drives the helper, and do not want the helper to interfere
with its ref updates.

So it is not just 'refspec' _allows_ the refs to be constrained to a
private namespace, like the earlier updates made the documentation
say; it _is_ mandatory to use refspecs to constrain them to avoid
touching refs/heads and refs/remotes namespace.

Am I reading you correctly?

Assuming I am, the patch in this message looks reasonable.

It makes the documentation updates a few patches ago look a bit
wanting, though.

Thanks.

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

 diff --git a/t/t5801-remote-helpers.sh b/t/t5801-remote-helpers.sh
 index 3eeb309..1bb7529 100755
 --- a/t/t5801-remote-helpers.sh
 +++ b/t/t5801-remote-helpers.sh
 @@ -100,14 +100,16 @@ test_expect_failure 'push new branch with old:new 
 refspec' '
  
  test_expect_success 'cloning without refspec' '
   GIT_REMOTE_TESTGIT_REFSPEC= \
 - git clone testgit::${PWD}/server local2 
 + git clone testgit::${PWD}/server local2 2 error 
 + grep This remote helper should implement refspec capability error 
   compare_refs local2 HEAD server HEAD
  '
  
  test_expect_success 'pulling without refspecs' '
   (cd local2 
   git reset --hard 
 - GIT_REMOTE_TESTGIT_REFSPEC= git pull) 
 + GIT_REMOTE_TESTGIT_REFSPEC= git pull 2 ../error) 
 + grep This remote helper should implement refspec capability error 
   compare_refs local2 HEAD server HEAD
  '
  
 diff --git a/transport-helper.c b/transport-helper.c
 index 4d98567..573eaf7 100644
 --- a/transport-helper.c
 +++ b/transport-helper.c
 @@ -215,6 +215,8 @@ static struct child_process *get_helper(struct transport 
 *transport)
   free((char *)refspecs[i]);
   }
   free(refspecs);
 + } else if (data-import || data-bidi_import || data-export) {
 + warning(This remote helper should implement refspec 
 capability.);
   }
   strbuf_release(buf);
   if (debug)
--
To unsubscribe from this list: send the line 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: fix commit messages

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

 git fast-import expects an extra newline after the commit message data,
 but we are adding it only on hg-git compat mode, which is why the
 bidirectionality tests pass.

 We should add it unconditionally.

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

Without knowing that hg-git compat mode is what is used in bidi test
(the only mode that supports bidi), which is why was ungrokkable.

This is a trivial change without downside risk so I do not mind
applying it to 'maint', as you say it is an appropriate there.

  contrib/remote-helpers/git-remote-hg | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

 diff --git a/contrib/remote-helpers/git-remote-hg 
 b/contrib/remote-helpers/git-remote-hg
 index a5f0013..5481331 100755
 --- a/contrib/remote-helpers/git-remote-hg
 +++ b/contrib/remote-helpers/git-remote-hg
 @@ -362,6 +362,8 @@ def export_ref(repo, name, kind, head):
  else:
  modified, removed = get_filechanges(repo, c, parents[0])
  
 +desc += '\n'
 +
  if mode == 'hg':
  extra_msg = ''
  
 @@ -385,7 +387,6 @@ def export_ref(repo, name, kind, head):
  else:
  extra_msg += extra : %s : %s\n % (key, 
 urllib.quote(value))
  
 -desc += '\n'
  if extra_msg:
  desc += '\n--HG--\n' + extra_msg
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

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

 +static const char *add_would_remove_warning = N_(
 +/* indent for warning:  */
 + In Git 2.0, 'git add pathspec...' will also update the\n
 +index for paths removed from the working tree that match the given\n
 +pathspec. If you want to 'add' only changed or newly created paths,\n
 +say 'git add --no-all pathspec...' instead.\n);
 +
  static void warn_add_would_remove(const char *path)
  {
 - warning(_(In Git 2.0, 'git add pathspec...' will also update the\n
 -   index for paths removed from the working tree that match\n
 -   the given pathspec. If you want to 'add' only changed\n
 -   or newly created paths, say 'git add --no-all pathspec...'
 -instead.\n\n
 -   '%s' would be removed from the index without --no-all.),
 - path);
 + static int warned_once;
 + if (!warned_once++)
 + warning(_(add_would_remove_warning));
 + warning(did not stage removal of '%s', path);
  }

Would add --dry-run say this, too?

  static void update_callback(struct diff_queue_struct *q,
 @@ -84,10 +88,8 @@ static void update_callback(struct diff_queue_struct *q,
   }
   break;
   case DIFF_STATUS_DELETED:
 - if (data-warn_add_would_remove) {
 + if (data-warn_add_would_remove)
   warn_add_would_remove(path);
 - data-warn_add_would_remove = 0;
 - }
   if (data-flags  ADD_CACHE_IGNORE_REMOVAL)
   break;
   if (!(data-flags  ADD_CACHE_PRETEND))
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Jeff King
On Thu, Apr 18, 2013 at 10:51:12AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  +static const char *add_would_remove_warning = N_(
  +/* indent for warning:  */
  + In Git 2.0, 'git add pathspec...' will also update the\n
  +index for paths removed from the working tree that match the given\n
  +pathspec. If you want to 'add' only changed or newly created paths,\n
  +say 'git add --no-all pathspec...' instead.\n);
  +
   static void warn_add_would_remove(const char *path)
   {
  -   warning(_(In Git 2.0, 'git add pathspec...' will also update the\n
  - index for paths removed from the working tree that match\n
  - the given pathspec. If you want to 'add' only changed\n
  - or newly created paths, say 'git add --no-all pathspec...'
  -  instead.\n\n
  - '%s' would be removed from the index without --no-all.),
  -   path);
  +   static int warned_once;
  +   if (!warned_once++)
  +   warning(_(add_would_remove_warning));
  +   warning(did not stage removal of '%s', path);
   }
 
 Would add --dry-run say this, too?

It probably makes sense to continue to have the warning in the dry-run
case, but it may make sense to tweak it grammatically when we are in
dry-run mode. Saying would stage removal is technically correct, but I
think it is somewhat ambiguous: would git do it if we were not in a
--dry-run, or would git do it if it were Git 2.0?

Doing it as:

  warning: not staging removal of '%s'

could work for both cases. Something like not considering (or another
synonym for considering) might be even more accurate. It is not just
that we did not stage it; it is what we did not even consider it an item
for staging under the current rules.

Note that the not staging warnings may potentially be interspersed
with the normal dry-run output. I think that's OK. But another
alternative would be to collect the paths and then print:

  warning: In Git 2.0, ...

  The following deleted paths were not considered under the current
  rule. Use git add -A to stage their removal now.

foo
bar

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


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

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

 could work for both cases. Something like not considering (or another
 synonym for considering) might be even more accurate. It is not just
 that we did not stage it; it is what we did not even consider it an item
 for staging under the current rules.

Yes, not considering is much more sensible, while side-stepping
the dryrun issue.  Or

   warning(ignoring removal of '%s')

 Note that the not staging warnings may potentially be interspersed
 with the normal dry-run output. I think that's OK.

As long as the top-text makes it clear what not considering (or
ignoring) in the following text means, I think it is fine.

But I think we are doing users a disservice by listing tons of
paths.  Where the difference of versions matters _most_ is when the
user has tons of removed paths in the working tree.  Either with one
warning per path, or a block of collected paths at the end, we are
scrolling the more important part of the message up.

That was why I originally showed one path as an example and stopped
there.  Perhaps it is a better solution to keep that behaviour and
rephrase the message to say that we ignored removal of paths like
this one '%s' you lost from the working tree but it will change in
Git 2.0 and you will better learn to use the --no-all option now.

The inter-topic conflicts between stages of three add in Git 2.0
topics is getting cumbersome even with the help from rerere, so I'd
like to merge their preparatory steps as I have them now to 'next'
and merge them down to 'master' first, and start applying tweaks
from there, or something.


--
To unsubscribe from this list: send the line 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 3/6] transport-helper: clarify pushing without refspecs

2013-04-18 Thread John Keeping
On Thu, Apr 18, 2013 at 10:29:30AM -0700, Junio C Hamano wrote:
 John Keeping j...@keeping.me.uk writes:
  diff --git a/transport-helper.c b/transport-helper.c
  index cea787c..4d98567 100644
  --- a/transport-helper.c
  +++ b/transport-helper.c
  @@ -785,6 +785,9 @@ static int push_refs_with_export(struct transport 
  *transport,
 struct string_list revlist_args = STRING_LIST_INIT_NODUP;
 struct strbuf buf = STRBUF_INIT;
   
  +  if (!data-refspecs)
  +  die(remote-helper doesn't support push; refspec needed);
 
  I think the refspec needed text is likely to be confusing if an
  end-user ever sees this message.  I'm not sure how we can provide useful
  feedback for both remote helper authors and end-users though.
 
 This refspecs only come via the helper and not directly from the
 end user, no?
 
 If that is the case, I do not think confusing is much of an issue.
 Not _(localizing) is also the right thing to do.  We may want to
 say BUG:  at front to clarify it is not the end-user's fault, but
 a problem they may want to report.  If we at this point know what
 helper attempted export without giving refspecs, it may help to show
 it here, so that developers will know with what helper the user
 had problems with.

I like this idea.  Perhaps we should say Bug in remote helper; refspec
needed with export, so that it clearly indicates to both end-users and
remote helper authors that the error is in the remote helper.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/6] transport-helper: clarify pushing without refspecs

2013-04-18 Thread Stefano Lattarini
On 04/18/2013 02:05 AM, Felipe Contreras wrote:
 This has never worked, since it's inception the code simply skips all

s/it's/its/ (sorry for nitpicking)

 the refs, essentially telling fast-export to do nothing.
 
 Let's at least tell the user what's going on.
 
 [SNIP]


Regards,
  Stefano
--
To unsubscribe from this list: send the line 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/6] transport-helper: warn when refspec is not used

2013-04-18 Thread Felipe Contreras
On Thu, Apr 18, 2013 at 12:30 PM, Junio C Hamano gits...@pobox.com wrote:
 Felipe Contreras felipe.contre...@gmail.com writes:

 For the modes that need it. In the future we should probably error out,
 instead of providing half-assed support.

 The reason we want to do this is because if it's not present, the remote
 helper might be updating refs/heads/*, or refs/remotes/origin/*,
 directly, and in the process fetch will get confused trying to update
 refs that are already updated, or older than what they should be. We
 shouldn't be messing with the rest of git.

 So that answers my question in the response to an earlier one in
 this series.  We expect the ref updates to be done by the fetch or
 push that drives the helper, and do not want the helper to interfere
 with its ref updates.

 So it is not just 'refspec' _allows_ the refs to be constrained to a
 private namespace, like the earlier updates made the documentation
 say; it _is_ mandatory to use refspecs to constrain them to avoid
 touching refs/heads and refs/remotes namespace.

Yeah, it was not stated that it was mandatory, but I think it's in
everybody's best interest that it is.

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


Git over HTTPS with basic authentication

2013-04-18 Thread Sebastian Schmidt
Why is git not working over HTTPS with basic authentication? I can clone
just fine, but when I try to push, it tells me

error: Cannot access URL https://..., return code 22
fatal: git-http-push failed

I have googled for an hour now, all I find is people that have the same
problem and the solution is always to use SSH.

Sibbo

--
To unsubscribe from this list: send the line 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 0/2] submodule: drop the top-level requirement

2013-04-18 Thread John Keeping
Thanks to Ram and Jens for the feedback on v2.  I've addressed most of
their comments in this version, but I've ignored a couple of Ram's nits
about test cases where leaving it how it is makes the added tests
consistent with those already in the same file.

Since v2, the main improvement is that the output from git-submodule now
contains paths relative to the directory in which the command is run,
not the top-level of the working tree.

John Keeping (2):
  rev-parse: add --prefix option
  submodule: drop the top-level requirement

 Documentation/git-rev-parse.txt |  16 ++
 builtin/rev-parse.c |  24 +++--
 git-submodule.sh| 113 ++--
 t/t1513-rev-parse-prefix.sh |  96 ++
 t/t7400-submodule-basic.sh  |  27 ++
 t/t7401-submodule-summary.sh|  14 +
 6 files changed, 258 insertions(+), 32 deletions(-)
 create mode 100755 t/t1513-rev-parse-prefix.sh

-- 
1.8.2.1.424.g1899e5e.dirty

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


[PATCH v3 1/2] rev-parse: add --prefix option

2013-04-18 Thread John Keeping
This makes 'git rev-parse' behave as if it were invoked from the
specified subdirectory of a repository, with the difference that any
file paths which it prints are prefixed with the full path from the top
of the working tree.

This is useful for shell scripts where we may want to cd to the top of
the working tree but need to handle relative paths given by the user on
the command line.

Signed-off-by: John Keeping j...@keeping.me.uk
---
Changes since v2:
- Rewrite commit message
- Add a new test for 'prefix ignored with HEAD:top'
- Use '-\EOF' where appropriate in t1513

 Documentation/git-rev-parse.txt | 16 +++
 builtin/rev-parse.c | 24 ---
 t/t1513-rev-parse-prefix.sh | 96 +
 3 files changed, 131 insertions(+), 5 deletions(-)
 create mode 100755 t/t1513-rev-parse-prefix.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 1f9ed6c..0ab2b77 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -59,6 +59,22 @@ OPTIONS
If there is no parameter given by the user, use `arg`
instead.
 
+--prefix arg::
+   Behave as if 'git rev-parse' was invoked from the `arg`
+   subdirectory of the working tree.  Any relative filenames are
+   resolved as if they are prefixed by `arg` and will be printed
+   in that form.
++
+This can be used to convert arguments to a command run in a subdirectory
+so that they can still be used after moving to the top-level of the
+repository.  For example:
++
+
+prefix=$(git rev-parse --show-prefix)
+cd $(git rev-parse --show-toplevel)
+eval set -- $(git rev-parse --sq --prefix $prefix $@)
+
+
 --verify::
Verify that exactly one parameter is provided, and that it
can be turned into a raw 20-byte SHA-1 that can be used to
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index f267a1d..de894c7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -212,11 +212,17 @@ static void show_datestring(const char *flag, const char 
*datestr)
show(buffer);
 }
 
-static int show_file(const char *arg)
+static int show_file(const char *arg, int output_prefix)
 {
show_default();
if ((filter  (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
-   show(arg);
+   if (output_prefix) {
+   const char *prefix = startup_info-prefix;
+   show(prefix_filename(prefix,
+prefix ? strlen(prefix) : 0,
+arg));
+   } else
+   show(arg);
return 1;
}
return 0;
@@ -470,6 +476,7 @@ N_(git rev-parse --parseopt [options] -- [args...]\n
 int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 {
int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
+   int output_prefix = 0;
unsigned char sha1[20];
const char *name = NULL;
 
@@ -503,7 +510,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
const char *arg = argv[i];
 
if (as_is) {
-   if (show_file(arg)  as_is  2)
+   if (show_file(arg, output_prefix)  as_is  2)
verify_filename(prefix, arg, 0);
continue;
}
@@ -527,7 +534,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
as_is = 2;
/* Pass on the -- if we show anything but 
files.. */
if (filter  (DO_FLAGS | DO_REVS))
-   show_file(arg);
+   show_file(arg, 0);
continue;
}
if (!strcmp(arg, --default)) {
@@ -535,6 +542,13 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
i++;
continue;
}
+   if (!strcmp(arg, --prefix)) {
+   prefix = argv[i+1];
+   startup_info-prefix = prefix;
+   output_prefix = 1;
+   i++;
+   continue;
+   }
if (!strcmp(arg, --revs-only)) {
filter = ~DO_NOREV;
continue;
@@ -754,7 +768,7 @@ int cmd_rev_parse(int argc, const char **argv, const char 
*prefix)
if (verify)
die_no_single_rev(quiet);
as_is = 1;
-   if (!show_file(arg))
+   if (!show_file(arg, output_prefix))
continue;

[PATCH v3 2/2] submodule: drop the top-level requirement

2013-04-18 Thread John Keeping
Use the new rev-parse --prefix option to process all paths given to the
submodule command, dropping the requirement that it be run from the
top-level of the repository.

Signed-off-by: John Keeping j...@keeping.me.uk
---
Changes since v2:
- Handle relative paths given to submodule add --reference=./...
- Check for absolute path before prepending $wt_prefix in cmd_add
- Export relative sm_path in cmd_foreach
- Change displayed sm_path to be relative
- Move a mkdir out of a subshell in t7400

I'm not sure if the relative_path function here is sufficient on
Windows.  We should be okay with the target argument since that comes
from git-ls-files but I think we're in trouble if git rev-parse
--show-prefix output the prefix with '\' instead of '/'.

 git-submodule.sh | 113 ---
 t/t7400-submodule-basic.sh   |  27 +++
 t/t7401-submodule-summary.sh |  14 ++
 3 files changed, 127 insertions(+), 27 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 79bfaac..0eee703 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -14,10 +14,13 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--name 
name] [--reference re
or: $dashless [--quiet] foreach [--recursive] command
or: $dashless [--quiet] sync [--recursive] [--] [path...]
 OPTIONS_SPEC=
+SUBDIRECTORY_OK=Yes
 . git-sh-setup
 . git-sh-i18n
 . git-parse-remote
 require_work_tree
+wt_prefix=$(git rev-parse --show-prefix)
+cd_to_toplevel
 
 command=
 branch=
@@ -106,12 +109,48 @@ resolve_relative_url ()
echo ${is_relative:+${up_path}}${remoteurl#./}
 }
 
+# Resolve a path to be relative to another path.  This is intended for
+# converting submodule paths when git-submodule is run in a subdirectory
+# and only handles paths where the directory separator is '/'.
+#
+# The output is the first argument as a path relative to the second argument,
+# which defaults to $wt_prefix if it is omitted.
+relative_path ()
+{
+   local target curdir result
+   target=$1
+   curdir=${2-$wt_prefix}
+   curdir=${curdir%/}
+   result=
+
+   while test -n $curdir
+   do
+   case $target in
+   $curdir/*)
+   target=${target#$curdir/}
+   break
+   ;;
+   esac
+
+   result=${result}../
+   if test $curdir = ${curdir%/*}
+   then
+   curdir=
+   else
+   curdir=${curdir%/*}
+   fi
+   done
+
+   echo $result$target
+}
+
 #
 # Get submodule info for registered submodules
 # $@ = path to limit submodule list
 #
 module_list()
 {
+   eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)
(
git ls-files --error-unmatch --stage -- $@ ||
echo unmatched pathspec exists
@@ -282,6 +321,7 @@ isnumber()
 cmd_add()
 {
# parse $args after submodule ... add.
+   reference_path=
while test $# -ne 0
do
case $1 in
@@ -298,11 +338,11 @@ cmd_add()
;;
--reference)
case $2 in '') usage ;; esac
-   reference=--reference=$2
+   reference_path=$2
shift
;;
--reference=*)
-   reference=$1
+   reference_path=${1#--reference=}
;;
--name)
case $2 in '') usage ;; esac
@@ -323,6 +363,14 @@ cmd_add()
shift
done
 
+   if test -n $reference_path
+   then
+   is_absolute_path $reference_path ||
+   reference_path=$wt_prefix$reference_path
+
+   reference=--reference=$reference_path
+   fi
+
repo=$1
sm_path=$2
 
@@ -335,6 +383,8 @@ cmd_add()
usage
fi
 
+   is_absolute_path $sm_path || sm_path=$wt_prefix$sm_path
+
# assure repo is absolute or relative to parent
case $repo in
./*|../*)
@@ -479,6 +529,7 @@ cmd_foreach()
# we make $path available to scripts ...
path=$sm_path
cd $sm_path 
+   sm_path=$(relative_path $sm_path) 
eval $@ 
if test -n $recursive
then
@@ -594,27 +645,29 @@ cmd_deinit()
die_if_unmatched $mode
name=$(module_name $sm_path) || exit
 
+   displaypath=$(relative_path $sm_path)
+
# Remove the submodule work tree (unless the user already did 
it)
if test -d $sm_path
then
# Protect submodules containing a .git directory
if test 

Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Phil Hord
On Wed, Apr 17, 2013 at 2:50 PM, Felipe Contreras
felipe.contre...@gmail.com wrote:
 On Tue, Apr 16, 2013 at 5:45 PM, Phil Hord phil.h...@gmail.com wrote:
 On Tue, Apr 16, 2013 at 3:04 PM, Felipe Contreras

 If you want to waste your time, by all means, rewrite all my commit
 messages with essays that nobody will ever read. I'm not going to do
 that for some hypothetical case that will never happen. I'm not going
 to waste my time.

 This is not a hypothetical.  Almost every time I bisect a regression
 in git.git, I find the commit message tells me exactly why the commit
 did what it did and what the expected result was.  I find this to be
 amazingly useful.  Do I need to show you real instances of that
 happening? No.  I promise it did, though.

 Yes please. Show me one of the instances where you hit a bisect with
 any of the remote-hg commits mentioned above by Thomas Rast.

I made no such claim.  In fact, I have never bisected to any
remote-hg-related commit.  I fail to see the relevance of this
qualifier, though.

P
--
To unsubscribe from this list: send the line 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: Pushing/fetching from/into a shallow-cloned repository

2013-04-18 Thread Philip Oakley

From: Konstantin Khomoutov kostix+...@007spb.ru
Sent: Thursday, April 18, 2013 10:52 AM

The git-clone manual page, both [1] and my local copy coming with
Git for Windows 1.8.1, say about the --depth command-line option:

  --depth depth

Create a shallow clone with a history truncated to the specified
number of revisions. A shallow repository has a number of
limitations (you cannot clone or fetch from it, nor push from nor
into it), but is adequate if you are only interested in the recent
history of a large project with a long history, and would want to
send in fixes as patches.

But having done a shallow clone (--depth=1) of one of my repositories,
I was able to record a new commit, push it out to a reference bare
repository and then fetch back to another clone of the same repository
just fine.  I have then killed my test commit doing a forced push from
another clone and subsequently was able to do `git fetch` in my 
shallow

clone just fine.

So I observe pushing/fetching works OK at least for a simple case like
this one.

Hence I'd like to ask: if the manual page is wrong or I'm observing
some corner case?
--

The manual is deliberately misleading.
The problem is that the depth is a movable feast that depends on how far 
the branches have progressed.
The DAG will be missing the historic merge bases, and in some scenarios 
can form disconnected runs of commits. The ref negotiation can also be a 
problem.


The git\Documentation\technical\shallow.txt has some extra details on 
issues.


Philip


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


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Jeff King
On Thu, Apr 18, 2013 at 11:16:33AM -0700, Junio C Hamano wrote:

 Jeff King p...@peff.net writes:
 
  could work for both cases. Something like not considering (or another
  synonym for considering) might be even more accurate. It is not just
  that we did not stage it; it is what we did not even consider it an item
  for staging under the current rules.
 
 Yes, not considering is much more sensible, while side-stepping
 the dryrun issue.  Or
 
warning(ignoring removal of '%s')

I like that much better than either of my suggestions.

  Note that the not staging warnings may potentially be interspersed
  with the normal dry-run output. I think that's OK.
 
 As long as the top-text makes it clear what not considering (or
 ignoring) in the following text means, I think it is fine.

Agreed, and I think the current text is fine for that (though neither of
us is the best judge at this point of how a less familiar user would
interpret it).

 But I think we are doing users a disservice by listing tons of
 paths.  Where the difference of versions matters _most_ is when the
 user has tons of removed paths in the working tree.  Either with one
 warning per path, or a block of collected paths at the end, we are
 scrolling the more important part of the message up.

I'm not sure I agree. Even with a handful, it made me wonder why one was
mentioned and not others. That _could_ be cleared up by rewording (i.e.,
making it clear that this is an example, and there may be more). But
somehow listing them is what I would expect. Perhaps because it gives
the user a clue about what to do next; they ask themselves did I want
those updated or not?.

In the orphaned-commit message when leaving a detached HEAD, we collect
the answer, say you are leaving N commits, and show the first 5 five
of them, with an ellipsis at the end if we didn't show them all.  Would
it makes sense to do that here?

Yet another alternative would be to print a warning for each path, but
hold the main warning for the end, so that it is the first thing the
user sees.  That has the added bonus that regular --dry-run output
will not scroll it away, either.

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


Re: Git over HTTPS with basic authentication

2013-04-18 Thread Jeff King
On Thu, Apr 18, 2013 at 09:54:32PM +0200, Matthieu Moy wrote:

 Sebastian Schmidt isib...@gmail.com writes:
 
  Why is git not working over HTTPS with basic authentication? I can clone
  just fine, but when I try to push, it tells me
 
 What are you using on the server? Dumb HTTP works by serving the
 repository files as static pages, which is fundamentally read-only. The
 recommended way is to use smart-HTTP (see man git-http-backend, requires
 Git on the server), and the alternative is to use webdav (much slower).

Yeah, this is definitely dumb http (since http-push is involved at all,
which the original error message showed).  Code 22 is curl's there was
an HTTP error code, but http-push annoyingly does not output the actual
HTTP error[1]. You can see more by setting GIT_CURL_VERBOSE=1 in the
environment.

Though if you know you did not set up WebDAV on the server, then we can
know that is the problem even without seeing the HTTP code. :)

-Peff

[1] The dumb-http push code is largely unloved and unmaintained at this
point. Yet another reason to move to smart http.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


What's cooking in git.git (Apr 2013, #06; Thu, 18)

2013-04-18 Thread Junio C Hamano
Here are the topics that have been cooking.  Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'.

You can find the changes described here in the integration branches
of the repositories listed at

http://git-blame.blogspot.com/p/git-public-repositories.html

--
[Graduated to master]

* fc/completion (2013-04-14) 8 commits
  (merged to 'next' on 2013-04-14 at a509746)
 + completion: small optimization
 + completion: inline __gitcomp_1 to its sole callsite
 + completion: get rid of compgen
 + completion: add __gitcomp_nl tests
 + completion: add new __gitcompadd helper
 + completion: get rid of empty COMPREPLY assignments
 + completion: trivial test improvement
 + completion: add more cherry-pick options

 In addition to a user visible change to offer more options to
 cherry-pick, generally cleans up and simplifies the code.


* fc/send-email-annotate (2013-04-14) 7 commits
  (merged to 'next' on 2013-04-14 at 4af1076)
 + rebase-am: explicitly disable cover-letter
 + format-patch: trivial cleanups
 + format-patch: add format.coverLetter configuration variable
 + log: update to OPT_BOOL
 + format-patch: refactor branch name calculation
 + format-patch: improve head calculation for cover-letter
 + send-email: make annotate configurable

 Allows format-patch --cover-letter to be configurable; the most
 notable is the auto mode to create cover-letter only for multi
 patch series.


* jc/detached-head-doc (2013-04-05) 1 commit
  (merged to 'next' on 2013-04-14 at 24b9271)
 + glossary: extend detached HEAD description

 Describe what happens when a command that operates on the current
 branch is run on a detached HEAD.


* jk/daemon-user-doc (2013-04-12) 1 commit
  (merged to 'next' on 2013-04-14 at 56c08ff)
 + doc: clarify that git daemon --user=user option does not export 
HOME=~user

 Document where the configuration is read by the git-daemon when its
 --user option is used.


* jk/http-dumb-namespaces (2013-04-09) 1 commit
  (merged to 'next' on 2013-04-15 at 4bfa834)
 + http-backend: respect GIT_NAMESPACE with dumb clients

 Allow smart-capable HTTP servers to be restricted via the
 GIT_NAMESPACE mechanism when talking with commit-walker clients
 (they already do so when talking with smart HTTP clients).


* jk/http-error-messages (2013-04-16) 1 commit
  (merged to 'next' on 2013-04-16 at 4a32517)
 + http: set curl FAILONERROR each time we select a handle

 A regression fix for the recently graduated topic.


* jk/merge-tree-added-identically (2013-04-08) 1 commit
  (merged to 'next' on 2013-04-15 at 35fd4b9)
 + merge-tree: don't print entries that match local

 The resolution of some corner cases by git merge-tree were
 inconsistent between top-of-the-tree and in a subdirectory.


* jk/test-trash (2013-04-14) 2 commits
  (merged to 'next' on 2013-04-15 at 15a6624)
 + t/test-lib.sh: drop $test variable
 + t/test-lib.sh: fix TRASH_DIRECTORY handling

 Fix longstanding issues with the test harness when used with --root=there
 option.


* kb/co-orphan-suggestion-short-sha1 (2013-04-08) 1 commit
  (merged to 'next' on 2013-04-14 at 8caf7fd)
 + checkout: abbreviate hash in suggest_reattach

 Update the informational message when git checkout leaves the
 detached head state.


* rs/empty-archive (2013-04-10) 1 commit
  (merged to 'next' on 2013-04-15 at eab39bc)
 + t5004: fix issue with empty archive test and bsdtar

 Implementations of tar of BSD descend have found to have trouble
 with reading an otherwise empty tar archive with pax headers and
 causes an unnecessary test failure.


* th/t9903-symlinked-workdir (2013-04-11) 1 commit
  (merged to 'next' on 2013-04-15 at f062dc6)
 + t9903: Don't fail when run from path accessed through symlink


* tr/packed-object-info-wo-recursion (2013-03-27) 3 commits
  (merged to 'next' on 2013-03-29 at b1c3858)
 + sha1_file: remove recursion in unpack_entry
 + Refactor parts of in_delta_base_cache/cache_or_unpack_entry
 + sha1_file: remove recursion in packed_object_info

 Attempts to reduce the stack footprint of sha1_object_info()
 and unpack_entry() codepaths.

--
[New Topics]

* jk/a-thread-only-dies-once (2013-04-16) 2 commits
  (merged to 'next' on 2013-04-18 at 3208f44)
 + run-command: use thread-aware die_is_recursing routine
 + usage: allow pluggable die-recursion checks

 A regression fix for the logic to detect die() handler triggering
 itself recursively.

 Will fast-track to 'master'.


* tr/copy-revisions-from-stdin (2013-04-16) 1 commit
  (merged to 'next' on 2013-04-16 at d882870)
 + read_revisions_from_stdin: make copies for handle_revision_arg

 A fix to a long-standing issue in the command line parser for
 revisions, which was triggered by mv/sequence-pick-error-diag topic
 (now in 'next').

 Will merge to 'master'.


* jc/prune-all (2013-04-18) 3 commits
 - api-parse-options.txt: document no- for 

Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

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

 But I think we are doing users a disservice by listing tons of
 paths.  Where the difference of versions matters _most_ is when the
 user has tons of removed paths in the working tree.  Either with one
 warning per path, or a block of collected paths at the end, we are
 scrolling the more important part of the message up.

 I'm not sure I agree. Even with a handful, it made me wonder why one was
 mentioned and not others. That _could_ be cleared up by rewording (i.e.,
 making it clear that this is an example, and there may be more). But
 somehow listing them is what I would expect. Perhaps because it gives
 the user a clue about what to do next; they ask themselves did I want
 those updated or not?.

 In the orphaned-commit message when leaving a detached HEAD, we collect
 the answer, say you are leaving N commits, and show the first 5 five
 of them, with an ellipsis at the end if we didn't show them all.  Would
 it makes sense to do that here?

Because this is to help people who are _used_ to seeing git add
not take the removals into account, I doubt that Did I want those
updated or not?  Let me see the details of them. will be the
question they will be asking [*1*].

I dunno.


[Footnote]

*1* I know I didn't want to include these removals to the index,
but I learned today that in later Git I should make myself more
clear if I want to keep doing so; thanks for letting me know., or
I've long been assuming that I have to say 'git add' and 'git rm'
separately, but I learned today that I can say 'add --all', and in
later Git I do not even have to; thanks for letting me know. are
the two reactions I expected.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

2013-04-18 Thread Jeff King
On Thu, Apr 18, 2013 at 02:37:53PM -0700, Junio C Hamano wrote:

 Because this is to help people who are _used_ to seeing git add
 not take the removals into account, I doubt that Did I want those
 updated or not?  Let me see the details of them. will be the
 question they will be asking [*1*].
 
 I dunno.
 
 
 [Footnote]
 
 *1* I know I didn't want to include these removals to the index,
 but I learned today that in later Git I should make myself more
 clear if I want to keep doing so; thanks for letting me know., or
 I've long been assuming that I have to say 'git add' and 'git rm'
 separately, but I learned today that I can say 'add --all', and in
 later Git I do not even have to; thanks for letting me know. are
 the two reactions I expected.

I am expecting a reaction more like Hmm, I never thought about it
before. Does that make sense to me or not? Let me think about which
paths it pertains to and decide.

Which I admit is no more likely than the scenarios you outlined, but it
is close to what I thought the first time I saw the warning.

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


Re: What's cooking in git.git (Apr 2013, #05; Mon, 15)

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

 I am expecting a reaction more like Hmm, I never thought about it
 before. Does that make sense to me or not? Let me think about which
 paths it pertains to and decide.

Let's step back and re-review the main text.

It currently says:

In Git 2.0, 'git add pathspec...' will also update the
index for paths removed from the working tree that match
the given pathspec. If you want to 'add' only changed
or newly created paths, say 'git add --no-all pathspec...'
instead.

This was written for the old we may want to warn logic that did
not even check if we would be omitting a removal.  The new logic
will show the text _only_ when the difference matters, we have an
opportunity to tighten it a lot, for example:

You ran 'git add' with neither '-A (--all)' or '--no-all', whose
behaviour will change in Git 2.0 with respect to paths you
removed from your working tree.

* 'git add --no-all pathspec', which is the current default,
  ignores paths you removed from your working tree.

* 'git add --all pathspec' will let you also record the
  removals.

The removed paths (e.g. '%s') are ignored with this version of Git.
Run 'git status' to remind yourself what paths you have removed
from your working tree.

or something?

--
To unsubscribe from this list: send the line 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] stash: tighten IS_STASH_LIKE logic

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

 Currently, 'git stash show' and 'git stash apply' can show/apply any
 merge commit, as the test for setting IS_STASH_LIKE simply asserts if
 the commit is a merge.  Improve the situation by asserting if the
 index_commit and the worktree_commit are based off the same commit, by
 checking that $REV^1 is equal to $REV^2^1.

When was the last time you tried to run git stash apply next?

The current code parses the ancestors and trees because the real
work needs them, and the 'does this look like a stash?' is a side
effect, but reading it^2^1 and comparing with it^1 is a pure
overhead for the sake of checking.  It looks like a futile mental
masturbation to solve theoretical non-issue to me.

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


Re: [PATCH v3 2/2] submodule: drop the top-level requirement

2013-04-18 Thread Junio C Hamano
John Keeping j...@keeping.me.uk writes:

 +relative_path ()
 +{
 + local target curdir result
 + target=$1
 + curdir=${2-$wt_prefix}
 + curdir=${curdir%/}
 + result=
 +
 + while test -n $curdir
 + do
 + case $target in
 + $curdir/*)
 + target=${target#$curdir/}
 + break
 + ;;
 + esac

Could $curdir have glob wildcard to throw this part of the logic
off?  It is OK to have limitations like you cannot have a glob
characters in your path to submodule working tree (at least until
we start rewriting these in C or Perl or Python), but we need to be
aware of them.

  module_list()
  {
 + eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)

An efficient reuse of -- ;-)

 +test_expect_success 'run summary from subdir' '
 + mkdir sub 
 + (
 + cd sub 
 + git submodule summary ../actual
 + ) 
 + cat expected -EOF 
 +* ../sm1 000...$head1 (2):
 +   Add foo2
 +
 +EOF

It somewhat looks strange to start with -EOF and then not to
indent the body nor EOF.
--
To unsubscribe from this list: send the line 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: Pushing/fetching from/into a shallow-cloned repository

2013-04-18 Thread Junio C Hamano
Philip Oakley philipoak...@iee.org writes:

 So I observe pushing/fetching works OK at least for a simple case like
 this one.

 Hence I'd like to ask: if the manual page is wrong or I'm observing
 some corner case?
 --
 The manual is deliberately misleading.

Yes, it is erring on the safe side.

It was not coded with the case where the gap widens (e.g. either
side rewinds the history) in mind as you explained; for simple cases
the code just happens to work, but the users are encouraged not to
rely on it to be safe than sorry.
--
To unsubscribe from this list: send the line 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] help.c: add a compatibility comment to cmd_version()

2013-04-18 Thread Philip Oakley

From: Junio C Hamano gits...@pobox.com
Sent: Thursday, April 18, 2013 1:13 AM

Philip Oakley philipoak...@iee.org writes:


How about
   * E.g. git gui uses the extended regular expression ^git version
[1-9]+(\.[0-9]+)+.*
   * to check for an acceptable version string.

The ERE is from git-gui.sh:L958.


That is exactly the kind of guarantee we do _not_ want to give.



So you are trying to avoid giving _any_ guarantee about what visible 
manifestation a user may obtain about a system that passes the test 
suite we have set out. I was hoping we could give a basic minimum 
indication of the expected style of the version string for a git which 
*passes* our tests. But like you say, it doesn't form a real guarantee - 
caveat emptor still applies.



... Hence my suggestion of the basic test that a passing git
would produce a consistent version string.


I have been assuming that you are trying to avoid an exchange like
this one we had in the past:


http://thread.gmane.org/gmane.comp.version-control.git/216923/focus=217007

In a sense yes. As David noted, others do attempt to trust us via our 
current version string format. I thought it worthwhile (given my earlier 
'mistake' in 216923/focus=216925) to suggest such a basic indication of 
our current string style.



I also have been assuming that you are pushing to limit the possible
versioning scheme, but I do not know what that extra limitation
would achieve in the real world.

By sticking to the pattern git gui happens to use, git gui may
be able to guess that the next version of Git says it is verison
1.8.3.  That is the _only_ thing your test buys.

But having parsed the 1.8.3 substring out of it, git gui does
not know anything about what 1.8.3 gives it.  As far as it is
concerned, it is a version whose git version output it has never
seen (unless it has been kept up to date with the development of
Git).


You are focusssing on the wrong side of issue (from my viewpoint).
If my earlier patch had gone in, it would have passsed our tests but not 
played nicely with the community tools. That would have been IMHO a 
regression, so from my viewpoint I believed it was worth having a test 
to avoid such a 'regression'.




By matching against git version [1-9]+(\.[0-9]+)+, it is accepting
that future versions may break assumptions it makes for some known
versions (which is warranted) and all future versions (which is
unwarranted) of Git.  Maybe the version 2.0 of Git adds all changes
in the directory d, including removals, when you say git add d,
but it may have assumed that with Git version 1.5.0 or newer, saying
git add d would result in added and modified inside that directory
getting updated in the index, but paths removed from the working
tree will stay in the index.


If it was a gross incompatibility then (a) we are likley to be 
signalling it for a while, and (b) other tools would need updating as 
well, and they would hope that they could 'read' the version in a 
consistent style. We would also still have the choice of changing our 
existing string style, which would explicitly signal a change via the 
test fail.




The only thing the scripts that read from the output of git
version can reliably tell is if it is interacting with a version of
Git it knows about.  If it made any unwarranted assumption on the
versions it hasn't seen, it has to be fixed in git gui _anyway_.

Of course, we would not change the output of git version
willy-nilly without good reason, but that is a different topic.
Ah. I thought it was the [original] topic. I was calibrating the 
willy-nillyness ;-)




So I do not know what you want to achieve in the real world with
that extra limitation on the git version output format.

Maybe you are proposing something else.  I dunno.

I was just using a slightly different philosophical approach.


--

Philip

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


[PATCH v4 01/13] pretty: save commit encoding from logmsg_reencode if the caller needs it

2013-04-18 Thread Nguyễn Thái Ngọc Duy
The commit encoding is parsed by logmsg_reencode, there's no need for
the caller to re-parse it again. The reencoded message now has the new
encoding, not the original one. The caller would need to read commit
object again before parsing.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 builtin/blame.c  |  2 +-
 builtin/commit.c |  2 +-
 commit.h |  1 +
 pretty.c | 16 
 revision.c   |  2 +-
 5 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 86100e9..104a948 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1425,7 +1425,7 @@ static void get_commit_info(struct commit *commit,
commit_info_init(ret);
 
encoding = get_log_output_encoding();
-   message = logmsg_reencode(commit, encoding);
+   message = logmsg_reencode(commit, NULL, encoding);
get_ac_line(message, \nauthor ,
ret-author, ret-author_mail,
ret-author_time, ret-author_tz);
diff --git a/builtin/commit.c b/builtin/commit.c
index 4620437..d2f30d9 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -955,7 +955,7 @@ static const char *read_commit_message(const char *name)
if (!commit)
die(_(could not lookup commit %s), name);
out_enc = get_commit_output_encoding();
-   return logmsg_reencode(commit, out_enc);
+   return logmsg_reencode(commit, NULL, out_enc);
 }
 
 static int parse_and_validate_options(int argc, const char *argv[],
diff --git a/commit.h b/commit.h
index 87b4b6c..ad55213 100644
--- a/commit.h
+++ b/commit.h
@@ -101,6 +101,7 @@ struct userformat_want {
 extern int has_non_ascii(const char *text);
 struct rev_info; /* in revision.h, it circularly uses enum cmit_fmt */
 extern char *logmsg_reencode(const struct commit *commit,
+char **commit_encoding,
 const char *output_encoding);
 extern void logmsg_free(char *msg, const struct commit *commit);
 extern void get_commit_format(const char *arg, struct rev_info *);
diff --git a/pretty.c b/pretty.c
index d3a82d2..c361b9b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -594,6 +594,7 @@ static char *replace_encoding_header(char *buf, const char 
*encoding)
 }
 
 char *logmsg_reencode(const struct commit *commit,
+ char **commit_encoding,
  const char *output_encoding)
 {
static const char *utf8 = UTF-8;
@@ -615,9 +616,15 @@ char *logmsg_reencode(const struct commit *commit,
sha1_to_hex(commit-object.sha1), typename(type));
}
 
-   if (!output_encoding || !*output_encoding)
+   if (!output_encoding || !*output_encoding) {
+   if (commit_encoding)
+   *commit_encoding =
+   get_header(commit, msg, encoding);
return msg;
+   }
encoding = get_header(commit, msg, encoding);
+   if (commit_encoding)
+   *commit_encoding = encoding;
use_encoding = encoding ? encoding : utf8;
if (same_encoding(use_encoding, output_encoding)) {
/*
@@ -658,7 +665,8 @@ char *logmsg_reencode(const struct commit *commit,
if (out)
out = replace_encoding_header(out, output_encoding);
 
-   free(encoding);
+   if (!commit_encoding)
+   free(encoding);
/*
 * If the re-encoding failed, out might be NULL here; in that
 * case we just return the commit message verbatim.
@@ -1288,7 +1296,7 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb-len;
-   context.message = logmsg_reencode(commit, output_enc);
+   context.message = logmsg_reencode(commit, NULL, output_enc);
 
strbuf_expand(sb, format, format_commit_item, context);
rewrap_message_tail(sb, context, 0, 0, 0);
@@ -1451,7 +1459,7 @@ void pretty_print_commit(const struct 
pretty_print_context *pp,
}
 
encoding = get_log_output_encoding();
-   msg = reencoded = logmsg_reencode(commit, encoding);
+   msg = reencoded = logmsg_reencode(commit, NULL, encoding);
 
if (pp-fmt == CMIT_FMT_ONELINE || pp-fmt == CMIT_FMT_EMAIL)
indent = 0;
diff --git a/revision.c b/revision.c
index 71e62d8..2e77397 100644
--- a/revision.c
+++ b/revision.c
@@ -2314,7 +2314,7 @@ static int commit_match(struct commit *commit, struct 
rev_info *opt)
 * in it.
 */
encoding = get_log_output_encoding();
-   message = logmsg_reencode(commit, encoding);
+   message = logmsg_reencode(commit, NULL, encoding);
 
/* Copy the commit to temporary if we are using fake headers */
if (buf.len)
-- 
1.8.2.82.gc24b958

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

[PATCH v4 02/13] pretty: get the correct encoding for --pretty:format=%e

2013-04-18 Thread Nguyễn Thái Ngọc Duy
parse_commit_header() provides the commit encoding for '%e' and it
reads it from the re-encoded message, which contains the new encoding,
not the original one in the commit object. This never happens because
--pretty=format:xxx never respects i18n.logoutputencoding. But that's
a different story.

Get the commit encoding from logmsg_reencode() instead.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 pretty.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/pretty.c b/pretty.c
index c361b9b..e59688b 100644
--- a/pretty.c
+++ b/pretty.c
@@ -776,12 +776,12 @@ struct format_commit_context {
unsigned commit_message_parsed:1;
struct signature_check signature_check;
char *message;
+   char *commit_encoding;
size_t width, indent1, indent2;
 
/* These offsets are relative to the start of the commit message. */
struct chunk author;
struct chunk committer;
-   struct chunk encoding;
size_t message_off;
size_t subject_off;
size_t body_off;
@@ -828,9 +828,6 @@ static void parse_commit_header(struct 
format_commit_context *context)
} else if (!prefixcmp(msg + i, committer )) {
context-committer.off = i + 10;
context-committer.len = eol - i - 10;
-   } else if (!prefixcmp(msg + i, encoding )) {
-   context-encoding.off = i + 9;
-   context-encoding.len = eol - i - 9;
}
i = eol;
}
@@ -1185,7 +1182,8 @@ static size_t format_commit_one(struct strbuf *sb, const 
char *placeholder,
   msg + c-committer.off, c-committer.len,
   c-pretty_ctx-date_mode);
case 'e':   /* encoding */
-   strbuf_add(sb, msg + c-encoding.off, c-encoding.len);
+   if (c-commit_encoding)
+   strbuf_addstr(sb, c-commit_encoding);
return 1;
case 'B':   /* raw body */
/* message_off is always left at the initial newline */
@@ -1296,11 +1294,14 @@ void format_commit_message(const struct commit *commit,
context.commit = commit;
context.pretty_ctx = pretty_ctx;
context.wrap_start = sb-len;
-   context.message = logmsg_reencode(commit, NULL, output_enc);
+   context.message = logmsg_reencode(commit,
+ context.commit_encoding,
+ output_enc);
 
strbuf_expand(sb, format, format_commit_item, context);
rewrap_message_tail(sb, context, 0, 0, 0);
 
+   free(context.commit_encoding);
logmsg_free(context.message, commit);
free(context.signature_check.gpg_output);
free(context.signature_check.signer);
-- 
1.8.2.82.gc24b958

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


[PATCH v4 03/13] pretty-formats.txt: wrap long lines

2013-04-18 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/pretty-formats.txt | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index afac703..6bde67e 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -106,18 +106,22 @@ The placeholders are:
 - '%P': parent hashes
 - '%p': abbreviated parent hashes
 - '%an': author name
-- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1] or 
linkgit:git-blame[1])
+- '%aN': author name (respecting .mailmap, see linkgit:git-shortlog[1]
+  or linkgit:git-blame[1])
 - '%ae': author email
-- '%aE': author email (respecting .mailmap, see linkgit:git-shortlog[1] or 
linkgit:git-blame[1])
+- '%aE': author email (respecting .mailmap, see
+  linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%ad': author date (format respects --date= option)
 - '%aD': author date, RFC2822 style
 - '%ar': author date, relative
 - '%at': author date, UNIX timestamp
 - '%ai': author date, ISO 8601 format
 - '%cn': committer name
-- '%cN': committer name (respecting .mailmap, see linkgit:git-shortlog[1] or 
linkgit:git-blame[1])
+- '%cN': committer name (respecting .mailmap, see
+  linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%ce': committer email
-- '%cE': committer email (respecting .mailmap, see linkgit:git-shortlog[1] or 
linkgit:git-blame[1])
+- '%cE': committer email (respecting .mailmap, see
+  linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%cd': committer date
 - '%cD': committer date, RFC2822 style
 - '%cr': committer date, relative
@@ -138,9 +142,11 @@ The placeholders are:
 - '%gD': reflog selector, e.g., `refs/stash@{1}`
 - '%gd': shortened reflog selector, e.g., `stash@{1}`
 - '%gn': reflog identity name
-- '%gN': reflog identity name (respecting .mailmap, see 
linkgit:git-shortlog[1] or linkgit:git-blame[1])
+- '%gN': reflog identity name (respecting .mailmap, see
+  linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%ge': reflog identity email
-- '%gE': reflog identity email (respecting .mailmap, see 
linkgit:git-shortlog[1] or linkgit:git-blame[1])
+- '%gE': reflog identity email (respecting .mailmap, see
+  linkgit:git-shortlog[1] or linkgit:git-blame[1])
 - '%gs': reflog subject
 - '%Cred': switch color to red
 - '%Cgreen': switch color to green
-- 
1.8.2.82.gc24b958

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


[PATCH v4 04/13] pretty: share code between format_decoration and show_decorations

2013-04-18 Thread Nguyễn Thái Ngọc Duy
This also adds color support to format_decorations()

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 log-tree.c   | 48 ++--
 log-tree.h   |  1 +
 pretty.c | 20 ++---
 t/t4207-log-decoration-colors.sh |  8 +++
 4 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index 7cc7d59..1946e9c 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -175,36 +175,52 @@ static void show_children(struct rev_info *opt, struct 
commit *commit, int abbre
}
 }
 
-void show_decorations(struct rev_info *opt, struct commit *commit)
+/*
+ * The caller makes sure there is no funny color before
+ * calling. format_decorations makes sure the same after return.
+ */
+void format_decorations(struct strbuf *sb,
+   const struct commit *commit,
+   int use_color)
 {
const char *prefix;
struct name_decoration *decoration;
const char *color_commit =
-   diff_get_color_opt(opt-diffopt, DIFF_COMMIT);
+   diff_get_color(use_color, DIFF_COMMIT);
const char *color_reset =
-   decorate_get_color_opt(opt-diffopt, DECORATION_NONE);
+   decorate_get_color(use_color, DECORATION_NONE);
 
-   if (opt-show_source  commit-util)
-   printf(\t%s, (char *) commit-util);
-   if (!opt-show_decorations)
-   return;
decoration = lookup_decoration(name_decoration, commit-object);
if (!decoration)
return;
prefix =  (;
while (decoration) {
-   printf(%s, prefix);
-   fputs(decorate_get_color_opt(opt-diffopt, decoration-type),
- stdout);
+   strbuf_addstr(sb, color_commit);
+   strbuf_addstr(sb, prefix);
+   strbuf_addstr(sb, decorate_get_color(use_color, 
decoration-type));
if (decoration-type == DECORATION_REF_TAG)
-   fputs(tag: , stdout);
-   printf(%s, decoration-name);
-   fputs(color_reset, stdout);
-   fputs(color_commit, stdout);
+   strbuf_addstr(sb, tag: );
+   strbuf_addstr(sb, decoration-name);
+   strbuf_addstr(sb, color_reset);
prefix = , ;
decoration = decoration-next;
}
-   putchar(')');
+   strbuf_addstr(sb, color_commit);
+   strbuf_addch(sb, ')');
+   strbuf_addstr(sb, color_reset);
+}
+
+void show_decorations(struct rev_info *opt, struct commit *commit)
+{
+   struct strbuf sb = STRBUF_INIT;
+
+   if (opt-show_source  commit-util)
+   printf(\t%s, (char *) commit-util);
+   if (!opt-show_decorations)
+   return;
+   format_decorations(sb, commit, opt-diffopt.use_color);
+   fputs(sb.buf, stdout);
+   strbuf_release(sb);
 }
 
 static unsigned int digits_in_number(unsigned int number)
@@ -540,8 +556,8 @@ void show_log(struct rev_info *opt)
printf( (from %s),
   find_unique_abbrev(parent-object.sha1,
  abbrev_commit));
+   fputs(diff_get_color_opt(opt-diffopt, DIFF_RESET), stdout);
show_decorations(opt, commit);
-   printf(%s, diff_get_color_opt(opt-diffopt, DIFF_RESET));
if (opt-commit_format == CMIT_FMT_ONELINE) {
putchar(' ');
} else {
diff --git a/log-tree.h b/log-tree.h
index 9140f48..d6ecd4d 100644
--- a/log-tree.h
+++ b/log-tree.h
@@ -13,6 +13,7 @@ int log_tree_diff_flush(struct rev_info *);
 int log_tree_commit(struct rev_info *, struct commit *);
 int log_tree_opt_parse(struct rev_info *, const char **, int);
 void show_log(struct rev_info *opt);
+void format_decorations(struct strbuf *sb, const struct commit *commit, int 
use_color);
 void show_decorations(struct rev_info *opt, struct commit *commit);
 void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 const char **subject_p,
diff --git a/pretty.c b/pretty.c
index e59688b..e0f93ba 100644
--- a/pretty.c
+++ b/pretty.c
@@ -908,23 +908,6 @@ static void parse_commit_message(struct 
format_commit_context *c)
c-commit_message_parsed = 1;
 }
 
-static void format_decoration(struct strbuf *sb, const struct commit *commit)
-{
-   struct name_decoration *d;
-   const char *prefix =  (;
-
-   load_ref_decorations(DECORATE_SHORT_REFS);
-   d = lookup_decoration(name_decoration, commit-object);
-   while (d) {
-   strbuf_addstr(sb, prefix);
-   prefix = , ;
-   strbuf_addstr(sb, d-name);
-   d = d-next;
-   }
-   if (prefix[0] == ',')
-   strbuf_addch(sb, ')');
-}
-
 static void strbuf_wrap(struct strbuf *sb, 

[PATCH v4 06/13] utf8.c: add utf8_strnwidth() with the ability to skip ansi sequences

2013-04-18 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 utf8.c | 20 ++--
 utf8.h |  1 +
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/utf8.c b/utf8.c
index 6ed93c3..e7ba33c 100644
--- a/utf8.c
+++ b/utf8.c
@@ -266,18 +266,26 @@ int utf8_width(const char **start, size_t *remainder_p)
  * string, assuming that the string is utf8.  Returns strlen() instead
  * if the string does not look like a valid utf8 string.
  */
-int utf8_strwidth(const char *string)
+int utf8_strnwidth(const char *string, int len, int skip_ansi)
 {
int width = 0;
const char *orig = string;
 
-   while (1) {
-   if (!string)
-   return strlen(orig);
-   if (!*string)
-   return width;
+   if (len == -1)
+   len = strlen(string);
+   while (string  string  orig + len) {
+   int skip;
+   while (skip_ansi 
+  (skip = display_mode_esc_sequence_len(string)) != 0)
+   string += skip;
width += utf8_width(string, NULL);
}
+   return string ? width : len;
+}
+
+int utf8_strwidth(const char *string)
+{
+   return utf8_strnwidth(string, -1, 0);
 }
 
 int is_utf8(const char *text)
diff --git a/utf8.h b/utf8.h
index 1f8ecad..d3da96f 100644
--- a/utf8.h
+++ b/utf8.h
@@ -4,6 +4,7 @@
 typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
 int utf8_width(const char **start, size_t *remainder_p);
+int utf8_strnwidth(const char *string, int len, int skip_ansi);
 int utf8_strwidth(const char *string);
 int is_utf8(const char *text);
 int is_encoding_utf8(const char *name);
-- 
1.8.2.82.gc24b958

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


[PATCH v4 07/13] utf8.c: add reencode_string_len() that can handle NULs in string

2013-04-18 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 compat/precompose_utf8.c |  2 +-
 utf8.c   | 10 +++---
 utf8.h   | 19 ---
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/compat/precompose_utf8.c b/compat/precompose_utf8.c
index 8cf5955..d9203d0 100644
--- a/compat/precompose_utf8.c
+++ b/compat/precompose_utf8.c
@@ -78,7 +78,7 @@ void precompose_argv(int argc, const char **argv)
size_t namelen;
oldarg = argv[i];
if (has_non_ascii(oldarg, (size_t)-1, namelen)) {
-   newarg = reencode_string_iconv(oldarg, namelen, 
ic_precompose);
+   newarg = reencode_string_iconv(oldarg, namelen, 
ic_precompose, NULL);
if (newarg)
argv[i] = newarg;
}
diff --git a/utf8.c b/utf8.c
index e7ba33c..7c342ff 100644
--- a/utf8.c
+++ b/utf8.c
@@ -468,7 +468,7 @@ int utf8_fprintf(FILE *stream, const char *format, ...)
 #else
typedef char * iconv_ibp;
 #endif
-char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv)
+char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv, int 
*outsz_p)
 {
size_t outsz, outalloc;
char *out, *outpos;
@@ -502,13 +502,17 @@ char *reencode_string_iconv(const char *in, size_t insz, 
iconv_t conv)
}
else {
*outpos = '\0';
+   if (outsz_p)
+   *outsz_p = outpos - out;
break;
}
}
return out;
 }
 
-char *reencode_string(const char *in, const char *out_encoding, const char 
*in_encoding)
+char *reencode_string_len(const char *in, int insz,
+ const char *out_encoding, const char *in_encoding,
+ int *outsz)
 {
iconv_t conv;
char *out;
@@ -534,7 +538,7 @@ char *reencode_string(const char *in, const char 
*out_encoding, const char *in_e
return NULL;
}
 
-   out = reencode_string_iconv(in, strlen(in), conv);
+   out = reencode_string_iconv(in, insz, conv, outsz);
iconv_close(conv);
return out;
 }
diff --git a/utf8.h b/utf8.h
index d3da96f..a43ef9a 100644
--- a/utf8.h
+++ b/utf8.h
@@ -17,12 +17,25 @@ void strbuf_add_wrapped_bytes(struct strbuf *buf, const 
char *data, int len,
 int indent, int indent2, int width);
 
 #ifndef NO_ICONV
-char *reencode_string_iconv(const char *in, size_t insz, iconv_t conv);
-char *reencode_string(const char *in, const char *out_encoding, const char 
*in_encoding);
+char *reencode_string_iconv(const char *in, size_t insz,
+   iconv_t conv, int *outsz);
+char *reencode_string_len(const char *in, int insz,
+ const char *out_encoding,
+ const char *in_encoding,
+ int *outsz);
 #else
-#define reencode_string(a,b,c) NULL
+#define reencode_string_len(a,b,c,d,e) NULL
 #endif
 
+static inline char *reencode_string(const char *in,
+   const char *out_encoding,
+   const char *in_encoding)
+{
+   return reencode_string_len(in, strlen(in),
+  out_encoding, in_encoding,
+  NULL);
+}
+
 int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
 
 #endif
-- 
1.8.2.82.gc24b958

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


[PATCH v4 08/13] pretty: two phase conversion for non utf-8 commits

2013-04-18 Thread Nguyễn Thái Ngọc Duy
Always assume format_commit_item() takes an utf-8 string for string
handling simplicity (we can handle utf-8 strings, but can't with other
encodings).

If commit message is in non-utf8, or output encoding is not, then the
commit is first converted to utf-8, processed, then output converted
to output encoding. This of course only works with encodings that are
compatible with Unicode.

This also fixes the iso8859-1 test in t6006. It's supposed to create
an iso8859-1 commit, but the commit content in t6006 is in UTF-8.
t6006 is now converted back in UTF-8 (the downside is we can't put
utf-8 strings there anymore).

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 pretty.c   | 24 ++--
 t/t6006-rev-list-format.sh | 12 ++--
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/pretty.c b/pretty.c
index e0f93ba..5947275 100644
--- a/pretty.c
+++ b/pretty.c
@@ -954,7 +954,8 @@ static int format_reflog_person(struct strbuf *sb,
return format_person_part(sb, part, ident, strlen(ident), dmode);
 }
 
-static size_t format_commit_one(struct strbuf *sb, const char *placeholder,
+static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
+   const char *placeholder,
void *context)
 {
struct format_commit_context *c = context;
@@ -1193,7 +1194,8 @@ static size_t format_commit_one(struct strbuf *sb, const 
char *placeholder,
return 0;   /* unknown placeholder */
 }
 
-static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
+static size_t format_commit_item(struct strbuf *sb, /* in UTF-8 */
+const char *placeholder,
 void *context)
 {
int consumed;
@@ -1273,6 +1275,7 @@ void format_commit_message(const struct commit *commit,
 {
struct format_commit_context context;
const char *output_enc = pretty_ctx-output_encoding;
+   const char *utf8 = UTF-8;
 
memset(context, 0, sizeof(context));
context.commit = commit;
@@ -1285,6 +1288,23 @@ void format_commit_message(const struct commit *commit,
strbuf_expand(sb, format, format_commit_item, context);
rewrap_message_tail(sb, context, 0, 0, 0);
 
+   if (output_enc) {
+   if (same_encoding(utf8, output_enc))
+   output_enc = NULL;
+   } else {
+   if (context.commit_encoding 
+   !same_encoding(context.commit_encoding, utf8))
+   output_enc = context.commit_encoding;
+   }
+
+   if (output_enc) {
+   int outsz;
+   char *out = reencode_string_len(sb-buf, sb-len,
+   output_enc, utf8, outsz);
+   if (out)
+   strbuf_attach(sb, out, outsz, outsz + 1);
+   }
+
free(context.commit_encoding);
logmsg_free(context.message, commit);
free(context.signature_check.gpg_output);
diff --git a/t/t6006-rev-list-format.sh b/t/t6006-rev-list-format.sh
index 3fc3b74..0393c9f 100755
--- a/t/t6006-rev-list-format.sh
+++ b/t/t6006-rev-list-format.sh
@@ -184,7 +184,7 @@ Test printing of complex bodies
 
 This commit message is much longer than the others,
 and it will be encoded in iso8859-1. We should therefore
-include an iso8859 character: ¡bueno!
+include an iso8859 character: �bueno!
 EOF
 test_expect_success 'setup complex body' '
 git config i18n.commitencoding iso8859-1 
@@ -192,14 +192,14 @@ git config i18n.commitencoding iso8859-1 
 '
 
 test_format complex-encoding %e 'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
+commit 1ed88da4a5b5ed8c449114ac131efc62178734c3
 iso8859-1
 commit 131a310eb913d107dd3c09a65d1651175898735d
 commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
 EOF
 
 test_format complex-subject %s 'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
+commit 1ed88da4a5b5ed8c449114ac131efc62178734c3
 Test printing of complex bodies
 commit 131a310eb913d107dd3c09a65d1651175898735d
 changed foo
@@ -208,17 +208,17 @@ added foo
 EOF
 
 test_format complex-body %b 'EOF'
-commit f58db70b055c5718631e5c61528b28b12090cdea
+commit 1ed88da4a5b5ed8c449114ac131efc62178734c3
 This commit message is much longer than the others,
 and it will be encoded in iso8859-1. We should therefore
-include an iso8859 character: ¡bueno!
+include an iso8859 character: �bueno!
 
 commit 131a310eb913d107dd3c09a65d1651175898735d
 commit 86c75cfd708a0e5868dc876ed5b8bb66c80b4873
 EOF
 
 test_expect_success '%x00 shows NUL' '
-   echo  expect commit f58db70b055c5718631e5c61528b28b12090cdea 
+   echo  expect commit 1ed88da4a5b5ed8c449114ac131efc62178734c3 
echo expect fooQbar 
git rev-list -1 --format=foo%x00bar HEAD actual.nul 
nul_to_q actual.nul actual 
-- 
1.8.2.82.gc24b958

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a 

[PATCH v4 09/13] pretty: split color parsing into a separate function

2013-04-18 Thread Nguyễn Thái Ngọc Duy

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 pretty.c | 71 +++-
 1 file changed, 39 insertions(+), 32 deletions(-)

diff --git a/pretty.c b/pretty.c
index 5947275..e0413e3 100644
--- a/pretty.c
+++ b/pretty.c
@@ -954,6 +954,44 @@ static int format_reflog_person(struct strbuf *sb,
return format_person_part(sb, part, ident, strlen(ident), dmode);
 }
 
+static size_t parse_color(struct strbuf *sb, /* in UTF-8 */
+ const char *placeholder,
+ struct format_commit_context *c)
+{
+   if (placeholder[1] == '(') {
+   const char *begin = placeholder + 2;
+   const char *end = strchr(begin, ')');
+   char color[COLOR_MAXLEN];
+
+   if (!end)
+   return 0;
+   if (!prefixcmp(begin, auto,)) {
+   if (!want_color(c-pretty_ctx-color))
+   return end - placeholder + 1;
+   begin += 5;
+   }
+   color_parse_mem(begin,
+   end - begin,
+   --pretty format, color);
+   strbuf_addstr(sb, color);
+   return end - placeholder + 1;
+   }
+   if (!prefixcmp(placeholder + 1, red)) {
+   strbuf_addstr(sb, GIT_COLOR_RED);
+   return 4;
+   } else if (!prefixcmp(placeholder + 1, green)) {
+   strbuf_addstr(sb, GIT_COLOR_GREEN);
+   return 6;
+   } else if (!prefixcmp(placeholder + 1, blue)) {
+   strbuf_addstr(sb, GIT_COLOR_BLUE);
+   return 5;
+   } else if (!prefixcmp(placeholder + 1, reset)) {
+   strbuf_addstr(sb, GIT_COLOR_RESET);
+   return 6;
+   } else
+   return 0;
+}
+
 static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -967,38 +1005,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
/* these are independent of the commit */
switch (placeholder[0]) {
case 'C':
-   if (placeholder[1] == '(') {
-   const char *begin = placeholder + 2;
-   const char *end = strchr(begin, ')');
-   char color[COLOR_MAXLEN];
-
-   if (!end)
-   return 0;
-   if (!prefixcmp(begin, auto,)) {
-   if (!want_color(c-pretty_ctx-color))
-   return end - placeholder + 1;
-   begin += 5;
-   }
-   color_parse_mem(begin,
-   end - begin,
-   --pretty format, color);
-   strbuf_addstr(sb, color);
-   return end - placeholder + 1;
-   }
-   if (!prefixcmp(placeholder + 1, red)) {
-   strbuf_addstr(sb, GIT_COLOR_RED);
-   return 4;
-   } else if (!prefixcmp(placeholder + 1, green)) {
-   strbuf_addstr(sb, GIT_COLOR_GREEN);
-   return 6;
-   } else if (!prefixcmp(placeholder + 1, blue)) {
-   strbuf_addstr(sb, GIT_COLOR_BLUE);
-   return 5;
-   } else if (!prefixcmp(placeholder + 1, reset)) {
-   strbuf_addstr(sb, GIT_COLOR_RESET);
-   return 6;
-   } else
-   return 0;
+   return parse_color(sb, placeholder, c);
case 'n':   /* newline */
strbuf_addch(sb, '\n');
return 1;
-- 
1.8.2.82.gc24b958

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


[PATCH v4 10/13] pretty: add %C(auto) for auto-coloring

2013-04-18 Thread Nguyễn Thái Ngọc Duy
This is not simply convenient over %C(auto,xxx). Some placeholders
(actually only one, %d) do multi coloring and we can't emit a multiple
colors with %C(auto,xxx).

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/pretty-formats.txt |  3 ++-
 pretty.c | 26 +++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 6bde67e..bad627a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -156,7 +156,8 @@ The placeholders are:
   adding `auto,` at the beginning will emit color only when colors are
   enabled for log output (by `color.diff`, `color.ui`, or `--color`, and
   respecting the `auto` settings of the former if we are going to a
-  terminal)
+  terminal). `auto` alone (i.e. `%C(auto)`) will turn on auto coloring
+  on the next placeholders until the color is switched again.
 - '%m': left, right or boundary mark
 - '%n': newline
 - '%%': a raw '%'
diff --git a/pretty.c b/pretty.c
index e0413e3..3612f82 100644
--- a/pretty.c
+++ b/pretty.c
@@ -778,6 +778,7 @@ struct format_commit_context {
char *message;
char *commit_encoding;
size_t width, indent1, indent2;
+   int auto_color;
 
/* These offsets are relative to the start of the commit message. */
struct chunk author;
@@ -1005,7 +1006,20 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
/* these are independent of the commit */
switch (placeholder[0]) {
case 'C':
-   return parse_color(sb, placeholder, c);
+   if (!prefixcmp(placeholder + 1, (auto))) {
+   c-auto_color = 1;
+   return 7; /* consumed 7 bytes, C(auto) */
+   } else {
+   int ret = parse_color(sb, placeholder, c);
+   if (ret)
+   c-auto_color = 0;
+   /*
+* Otherwise, we decided to treat %Cunknown
+* as a literal string, and the previous
+* %C(auto) is still valid.
+*/
+   return ret;
+   }
case 'n':   /* newline */
strbuf_addch(sb, '\n');
return 1;
@@ -1051,13 +1065,19 @@ static size_t format_commit_one(struct strbuf *sb, /* 
in UTF-8 */
 
switch (placeholder[0]) {
case 'H':   /* commit hash */
+   strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_COMMIT));
strbuf_addstr(sb, sha1_to_hex(commit-object.sha1));
+   strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_RESET));
return 1;
case 'h':   /* abbreviated commit hash */
-   if (add_again(sb, c-abbrev_commit_hash))
+   strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_COMMIT));
+   if (add_again(sb, c-abbrev_commit_hash)) {
+   strbuf_addstr(sb, diff_get_color(c-auto_color, 
DIFF_RESET));
return 1;
+   }
strbuf_addstr(sb, find_unique_abbrev(commit-object.sha1,
 c-pretty_ctx-abbrev));
+   strbuf_addstr(sb, diff_get_color(c-auto_color, DIFF_RESET));
c-abbrev_commit_hash.len = sb-len - c-abbrev_commit_hash.off;
return 1;
case 'T':   /* tree hash */
@@ -1095,7 +1115,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
return 1;
case 'd':
load_ref_decorations(DECORATE_SHORT_REFS);
-   format_decorations(sb, commit, 0);
+   format_decorations(sb, commit, c-auto_color);
return 1;
case 'g':   /* reflog info */
switch(placeholder[1]) {
-- 
1.8.2.82.gc24b958

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


[PATCH v4 11/13] pretty: support padding placeholders, % % and %

2013-04-18 Thread Nguyễn Thái Ngọc Duy
Either %, % or % standing before a placeholder specifies how many
columns (at least as the placeholder can exceed it) it takes. Each
differs on how spaces are padded:

  % pads on the right (aka left alignment)
  % pads on the left (aka right alignment)
  % pads both ways equally (aka centered)

The (N) follows them, e.g. `%(100)', to specify the number of
columns the next placeholder takes.

However, if '|' stands before (N), e.g. `%|(100)', then the number
of columns is calculated so that it reaches the Nth column on screen.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/pretty-formats.txt |   8 +++
 pretty.c | 117 -
 t/t4205-log-pretty-formats.sh| 122 +++
 3 files changed, 246 insertions(+), 1 deletion(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index bad627a..e2345d2 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -164,6 +164,14 @@ The placeholders are:
 - '%x00': print a byte from a hex code
 - '%w([w[,i1[,i2]]])': switch line wrapping, like the -w option of
   linkgit:git-shortlog[1].
+- '%(N)': make the next placeholder take at least N columns,
+  padding spaces on the right if necessary
+- '%|(N)': make the next placeholder take at least until Nth
+  columns, padding spaces on the right if necessary
+- '%(N)', '%|(N)': similar to '%(N)', '%|(N)'
+  respectively, but padding spaces on the left
+- '%(N)', '%|(N)': similar to '%(N)', '%|(N)'
+  respectively, but padding both sides (i.e. the text is centered)
 
 NOTE: Some placeholders may depend on other options given to the
 revision traversal engine. For example, the `%g*` reflog options will
diff --git a/pretty.c b/pretty.c
index 3612f82..b50ebf5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -769,16 +769,25 @@ struct chunk {
size_t len;
 };
 
+enum flush_type {
+   no_flush,
+   flush_right,
+   flush_left,
+   flush_both
+};
+
 struct format_commit_context {
const struct commit *commit;
const struct pretty_print_context *pretty_ctx;
unsigned commit_header_parsed:1;
unsigned commit_message_parsed:1;
struct signature_check signature_check;
+   enum flush_type flush_type;
char *message;
char *commit_encoding;
size_t width, indent1, indent2;
int auto_color;
+   int padding;
 
/* These offsets are relative to the start of the commit message. */
struct chunk author;
@@ -993,6 +1002,52 @@ static size_t parse_color(struct strbuf *sb, /* in UTF-8 
*/
return 0;
 }
 
+static size_t parse_padding_placeholder(struct strbuf *sb,
+   const char *placeholder,
+   struct format_commit_context *c)
+{
+   const char *ch = placeholder;
+   enum flush_type flush_type;
+   int to_column = 0;
+
+   switch (*ch++) {
+   case '':
+   flush_type = flush_right;
+   break;
+   case '':
+   if (*ch == '') {
+   flush_type = flush_both;
+   ch++;
+   } else
+   flush_type = flush_left;
+   break;
+   default:
+   return 0;
+   }
+
+   /* the next value means wide enough to that column */
+   if (*ch == '|') {
+   to_column = 1;
+   ch++;
+   }
+
+   if (*ch == '(') {
+   const char *start = ch + 1;
+   const char *end = strchr(start, ')');
+   char *next;
+   int width;
+   if (!end || end == start)
+   return 0;
+   width = strtoul(start, next, 10);
+   if (next == start || width == 0)
+   return 0;
+   c-padding = to_column ? -width : width;
+   c-flush_type = flush_type;
+   return end - placeholder + 1;
+   }
+   return 0;
+}
+
 static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
const char *placeholder,
void *context)
@@ -1057,6 +1112,10 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
return end - placeholder + 1;
} else
return 0;
+
+   case '':
+   case '':
+   return parse_padding_placeholder(sb, placeholder, c);
}
 
/* these depend on the commit */
@@ -1221,6 +1280,59 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
return 0;   /* unknown placeholder */
 }
 
+static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
+   const char *placeholder,
+   struct format_commit_context *c)
+{
+   struct 

[PATCH v4 12/13] pretty: support truncating in %, % and %

2013-04-18 Thread Nguyễn Thái Ngọc Duy
%(N,trunc) truncates the right part after N columns and replace the
last two letters with ... ltrunc does the same on the left. mtrunc
cuts the middle out.

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/pretty-formats.txt |  7 --
 pretty.c | 51 +---
 t/t4205-log-pretty-formats.sh| 39 ++
 utf8.c   | 46 
 utf8.h   |  2 ++
 5 files changed, 140 insertions(+), 5 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index e2345d2..d3450bf 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -164,8 +164,11 @@ The placeholders are:
 - '%x00': print a byte from a hex code
 - '%w([w[,i1[,i2]]])': switch line wrapping, like the -w option of
   linkgit:git-shortlog[1].
-- '%(N)': make the next placeholder take at least N columns,
-  padding spaces on the right if necessary
+- '%(N[,trunc|ltrunc|mtrunc])': make the next placeholder take at
+  least N columns, padding spaces on the right if necessary.
+  Optionally truncate at the beginning (ltrunc), the middle (mtrunc)
+  or the end (trunc) if the output is longer than N columns.
+  Note that truncating only works correctly with N = 2.
 - '%|(N)': make the next placeholder take at least until Nth
   columns, padding spaces on the right if necessary
 - '%(N)', '%|(N)': similar to '%(N)', '%|(N)'
diff --git a/pretty.c b/pretty.c
index b50ebf5..f7c6442 100644
--- a/pretty.c
+++ b/pretty.c
@@ -776,6 +776,13 @@ enum flush_type {
flush_both
 };
 
+enum trunc_type {
+   trunc_none,
+   trunc_left,
+   trunc_middle,
+   trunc_right
+};
+
 struct format_commit_context {
const struct commit *commit;
const struct pretty_print_context *pretty_ctx;
@@ -783,6 +790,7 @@ struct format_commit_context {
unsigned commit_message_parsed:1;
struct signature_check signature_check;
enum flush_type flush_type;
+   enum trunc_type truncate;
char *message;
char *commit_encoding;
size_t width, indent1, indent2;
@@ -1033,7 +1041,7 @@ static size_t parse_padding_placeholder(struct strbuf *sb,
 
if (*ch == '(') {
const char *start = ch + 1;
-   const char *end = strchr(start, ')');
+   const char *end = start + strcspn(start, ,));
char *next;
int width;
if (!end || end == start)
@@ -1043,6 +1051,23 @@ static size_t parse_padding_placeholder(struct strbuf 
*sb,
return 0;
c-padding = to_column ? -width : width;
c-flush_type = flush_type;
+
+   if (*end == ',') {
+   start = end + 1;
+   end = strchr(start, ')');
+   if (!end || end == start)
+   return 0;
+   if (!prefixcmp(start, trunc)))
+   c-truncate = trunc_right;
+   else if (!prefixcmp(start, ltrunc)))
+   c-truncate = trunc_left;
+   else if (!prefixcmp(start, mtrunc)))
+   c-truncate = trunc_middle;
+   else
+   return 0;
+   } else
+   c-truncate = trunc_none;
+
return end - placeholder + 1;
}
return 0;
@@ -1309,9 +1334,29 @@ static size_t format_and_pad_commit(struct strbuf *sb, 
/* in UTF-8 */
total_consumed++;
}
len = utf8_strnwidth(local_sb.buf, -1, 1);
-   if (len  padding)
+   if (len  padding) {
+   switch (c-truncate) {
+   case trunc_left:
+   strbuf_utf8_replace(local_sb,
+   0, len - (padding - 2),
+   ..);
+   break;
+   case trunc_middle:
+   strbuf_utf8_replace(local_sb,
+   padding / 2 - 1,
+   len - (padding - 2),
+   ..);
+   break;
+   case trunc_right:
+   strbuf_utf8_replace(local_sb,
+   padding - 2, len - (padding - 2),
+   ..);
+   break;
+   case trunc_none:
+   break;
+   }
strbuf_addstr(sb, local_sb.buf);
-   else {
+   } else {
int sb_len = sb-len, offset = 0;
if (c-flush_type == flush_left)
offset = padding - len;
diff --git 

[PATCH v4 13/13] pretty: support % that steal trailing spaces

2013-04-18 Thread Nguyễn Thái Ngọc Duy
This is pretty useful in `%(100)%s%Cred%(20)% an' where %s does not
use up all 100 columns and %an needs more than 20 columns. By
replacing %(20) with %(20), %an can steal spaces from %s.

% understands escape sequences, so %Cred does not stop it from
stealing spaces in %(100).

Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
 Documentation/pretty-formats.txt |  5 -
 pretty.c | 34 ++
 t/t4205-log-pretty-formats.sh| 14 ++
 utf8.c   |  2 +-
 utf8.h   |  1 +
 5 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index d3450bf..c96ff41 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -173,7 +173,10 @@ The placeholders are:
   columns, padding spaces on the right if necessary
 - '%(N)', '%|(N)': similar to '%(N)', '%|(N)'
   respectively, but padding spaces on the left
-- '%(N)', '%|(N)': similar to '%(N)', '%|(N)'
+- '%(N)', '%|(N)': similar to '%(N)', '%|(N)'
+  respectively, except that if the next placeholder takes more spaces
+  than given and there are spaces on its left, use those spaces
+- '%(N)', '%|(N)': similar to '% (N)', '%|(N)'
   respectively, but padding both sides (i.e. the text is centered)
 
 NOTE: Some placeholders may depend on other options given to the
diff --git a/pretty.c b/pretty.c
index f7c6442..d59579a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -773,6 +773,7 @@ enum flush_type {
no_flush,
flush_right,
flush_left,
+   flush_left_and_steal,
flush_both
 };
 
@@ -1026,6 +1027,9 @@ static size_t parse_padding_placeholder(struct strbuf *sb,
if (*ch == '') {
flush_type = flush_both;
ch++;
+   } else if (*ch == '') {
+   flush_type = flush_left_and_steal;
+   ch++;
} else
flush_type = flush_left;
break;
@@ -1334,6 +1338,36 @@ static size_t format_and_pad_commit(struct strbuf *sb, 
/* in UTF-8 */
total_consumed++;
}
len = utf8_strnwidth(local_sb.buf, -1, 1);
+
+   if (c-flush_type == flush_left_and_steal) {
+   const char *ch = sb-buf + sb-len - 1;
+   while (len  padding  ch  sb-buf) {
+   const char *p;
+   if (*ch == ' ') {
+   ch--;
+   padding++;
+   continue;
+   }
+   /* check for trailing ansi sequences */
+   if (*ch != 'm')
+   break;
+   p = ch - 1;
+   while (ch - p  10  *p != '\033')
+   p--;
+   if (*p != '\033' ||
+   ch + 1 - p != display_mode_esc_sequence_len(p))
+   break;
+   /*
+* got a good ansi sequence, put it back to
+* local_sb as we're cutting sb
+*/
+   strbuf_insert(local_sb, 0, p, ch + 1 - p);
+   ch = p - 1;
+   }
+   strbuf_setlen(sb, ch + 1 - sb-buf);
+   c-flush_type = flush_left;
+   }
+
if (len  padding) {
switch (c-truncate) {
case trunc_left:
diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh
index b8685b9..26fbfde 100755
--- a/t/t4205-log-pretty-formats.sh
+++ b/t/t4205-log-pretty-formats.sh
@@ -260,4 +260,18 @@ EOF
test_cmp expected actual
 '
 
+test_expect_success 'left/right alignment formatting with stealing' '
+   git commit --amend -m short --author long long long l...@me.com 
+   git log --pretty=format:%(10,trunc)%s%(10,ltrunc)% an actual 
+   # complete the incomplete line at the end
+   echo actual 
+   cat \EOF expected 
+short long  long long
+message ..   A U Thor
+add bar  A U Thor
+initial  A U Thor
+EOF
+   test_cmp expected actual
+'
+
 test_done
diff --git a/utf8.c b/utf8.c
index 197414a..b1e1303 100644
--- a/utf8.c
+++ b/utf8.c
@@ -9,7 +9,7 @@ struct interval {
   int last;
 };
 
-static size_t display_mode_esc_sequence_len(const char *s)
+size_t display_mode_esc_sequence_len(const char *s)
 {
const char *p = s;
if (*p++ != '\033')
diff --git a/utf8.h b/utf8.h
index edde8ee..32a7bfb 100644
--- a/utf8.h
+++ b/utf8.h
@@ -3,6 +3,7 @@
 
 typedef unsigned int ucs_char_t;  /* assuming 32bit int */
 
+size_t display_mode_esc_sequence_len(const char *s);
 int utf8_width(const char **start, size_t *remainder_p);
 int utf8_strnwidth(const char *string, int len, int skip_ansi);
 int utf8_strwidth(const char 

Re: git p4 submit failing

2013-04-18 Thread Pete Wyckoff
christopher.yee...@gmail.com wrote on Thu, 18 Apr 2013 11:24 -0500:
 The issue is caused by the line endings. I retested the problem with a
 different file and in this case, the error is caused by the line
 endings of the file checked out in the perforce workspace being
 win-style crlf, and the line endings of the file in the git repo being
 unix style lf. (In this scenario, I took out the .gitattributes,
 core.autocrlf was set to false and LineEnd was set to share)
 
 In this case, I checked out the file in perforce, ran dos2unix against
 it, and submitted that, then ran git p4 submit and it worked.
 
 I noticed that the error is caused by the git apply failing in the def
 applyCommit(self, id) function at lines 1296-1305.
 
 diffcmd = git format-patch -k --stdout \%s^\..\%s\ % (id, id)
 patchcmd = diffcmd +  | git apply 
 tryPatchCmd = patchcmd + --check -
 applyPatchCmd = patchcmd + --check --apply -
 patch_succeeded = True
 
 if os.system(tryPatchCmd) != 0:
 fixed_rcs_keywords = False
 patch_succeeded = False
 print Unfortunately applying the change failed!
 
 So most likely in git apply command, it can't find the changes because
 of the line endings being different between them. I couldn't find a
 parameter that would magically make it work. When I added --verbose to
 git apply the output only says:
 error: while searching for:
 and then the first lines of the first diff

That seems like exactly the correct diagnosis of the problem.
What to do about it, I'm not so sure.

We could suggest that people use the same line-ending conventions
in both git and p4 land.  This is easy if they are both lf.  But,
if crlf is preferred, do you know how to configure git to use
crlf line endings?  Does that fix it?  There's also the config
setting apply.ignorewhitespace; not sure if that would allow
the apply step to apply an lf-ending patch to the crlf-ending p4
workspace.

-- Pete

 Hello Simon,
 
 I have CCed you to alert you to the possible bug. Any assistance would
 be appreciated.
 
 
 On Sat, Apr 13, 2013 at 5:09 PM, Christopher Yee Mon
 christopher.yee...@gmail.com wrote:
  Yes this is the case.
 
  Many of the files have crlf endings.
 
  core.autocrlf was recently set to input. I can't remember the timeline
  exactly though, but in addition to this, I have a .gitattributes file
  with the default set to text=auto (* text=auto) and the php files set
  to text eol=lf (*.php text eol=lf) Also my perforce workspace's
  LineEnd setting is set to Share.
 
  I've experienced the behavior in both .php and .xml files though
 
  Before all of this started I had core.autocrlf set to false, and no
  .gitattributes file and perforce workspace's LineEnd was set to the
  default, but I got a conflict where the only difference was the line
  endings, so I changed things to the way they are now.
 
  Any recommendations? Should I change everything back the way it was?
 
  On Sat, Apr 13, 2013 at 5:51 PM, Pete Wyckoff p...@padd.com wrote:
  l...@diamand.org wrote on Thu, 11 Apr 2013 21:19 +0100:
  Just a thought, but check the files that are failing to see if they've
  got RCS keywords in them ($Id$, $File$, $Date$, etc). These cause all
  sorts of nasty problems.
 
  That's assuming it's definitely not a CRLF line ending problem on Windows.
 
  I had recently debugged a similar-looking problem
  where core.autocrlf was set to input.
 
  Christopher, if you have this set and/or the .xml files
  have ^M (CRLF) line endings, please let us know.
 
  -- Pete
 
 
  On Thu, Apr 11, 2013 at 8:01 PM, Christopher Yee Mon
  christopher.yee...@gmail.com wrote:
   I tried running git p4 submit on a repo that I've been running as an
   interim bridge between git and perforce. Multiple people are using the
   repo as a remote and its being periodically submitted back to
   perforce.
  
   It's been working mostly fine. Then one day out of the blue I get this
   error. I can no longer push any git commits to perforce. (This is from
   the remote repo which I am pushing back to perforce)
  
   user@hostname:~/Source/code$ git p4 submit -M --export-labels
   Perforce checkout for depot path //depot/perforce/workspace/ located
   at /home/user/Source/git-p4-area/perforce/workspace/
   Synchronizing p4 checkout...
   ... - file(s) up-to-date.
   Applying ffa390f comments in config xml files
   //depot/perforce/workspace/sub/folder/structure/first.xml#3 - opened 
   for edit
   //depot/perforce/workspace/sub/folder/structure/second.xml#3 - opened 
   for edit
   //depot/perforce/workspace/sub/folder/structure/third.xml#3 - opened 
   for edit
   //depot/perforce/workspace/sub/folder/structure/forth.xml#3 - opened 
   for edit
   //depot/perforce/workspace/sub/folder/structure/fifth.xml#1 - opened 
   for edit
   error: patch failed: sub/folder/structure/first.xml:1
   error: sub/folder/structure/first.xml: patch does not apply
   

  1   2   >