Git development
 help / color / mirror / Atom feed
* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 17:14 UTC (permalink / raw)
  To: Jeff King
  Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
	Jonathan Nieder
In-Reply-To: <CALkWK0=Mv_tzNw-hN_9fAr+vABappndEK5iSWQHDk8Yk6Z-stw@mail.gmail.com>

Ramkumar Ramachandra wrote:
> My current worktree (WIP):
> [...]

Classic whitespace breakage.  How many times am I going to fall for
the same joke?

diff --git a/builtin/revert.c b/builtin/revert.c
index 0d8020c..47da41b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -228,6 +228,7 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)
 		opts->revs = xmalloc(sizeof(*opts->revs));
 		init_revisions(opts->revs, NULL);
 		opts->revs->no_walk = 1;
+		opts->revs->literal_order = 1;
 		if (argc < 2)
 			usage_with_options(usage_str, options);
 		argc = setup_revisions(argc, argv, opts->revs, NULL);
diff --git a/revision.c b/revision.c
index 064e351..301ef58 100644
--- a/revision.c
+++ b/revision.c
@@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
 		if (commit) {
 			if (!(commit->object.flags & SEEN)) {
 				commit->object.flags |= SEEN;
-				commit_list_insert_by_date(commit, &revs->commits);
+				if (revs->literal_order)
+					commit_list_insert(commit, &revs->commits);
+				else
+					commit_list_insert_by_date(commit, &revs->commits);
 			}
 		}
 		e++;
diff --git a/revision.h b/revision.h
index b8e9223..65c3dc3 100644
--- a/revision.h
+++ b/revision.h
@@ -67,6 +67,7 @@ struct rev_info {
 			remove_empty_trees:1,
 			simplify_history:1,
 			lifo:1,
+			literal_order:1,
 			topo_order:1,
 			simplify_merges:1,
 			simplify_by_decoration:1,

^ permalink raw reply related

* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Jeff King @ 2012-01-12 17:15 UTC (permalink / raw)
  To: Ramkumar Ramachandra
  Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
	Jonathan Nieder
In-Reply-To: <CALkWK0=Mv_tzNw-hN_9fAr+vABappndEK5iSWQHDk8Yk6Z-stw@mail.gmail.com>

On Thu, Jan 12, 2012 at 10:39:48PM +0530, Ramkumar Ramachandra wrote:

> Jeff King wrote:
> >                  I agree it would be nice to make:
> >  git cherry-pick commit1 commit3 commit2
> >
> > work in the order specified, but how does that interact with existing
> > cases that provide more traditional revision arguments?
> 
> What are your thoughts on making it a flag in the revision API to be
> activated with "cherry-pick --literal-order commit1 commit3 commit2"
> or similar?  I'm not sure how to get it to reconcile with the more
> traditional revision arguments yet. My current worktree (WIP):

I think that is a sensible first-cut. It may even be possible to use
heuristics to identify when --literal-order is needed, and eventually it
could go away. But that is a much riskier feature that can be built on
top of the much safer proposal you are making.

> diff --git a/revision.c b/revision.c
> index 064e351..301ef58 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
>                 if (commit) {
>                         if (!(commit->object.flags & SEEN)) {
>                                 commit->object.flags |= SEEN;
> -                               commit_list_insert_by_date(commit,
> &revs->commits
> +                               if (revs->literal_order)
> +                                       commit_list_insert(commit,
> &revs->commits
> +                               else
> +
> commit_list_insert_by_date(commit, &revs-

My only concern is that there are other parts of the revision machinery
that depend on the date-ordering of the commit list. What would happen,
for example, with:

  git rev-list --literal-order --do-walk foo

It probably doesn't make sense to allow literal-order without no-walk,
anyway (which of course is the default in cherry-pick anyway, so it's
not a big deal here).

I'm also not sure what:

  git rev-list --literal-order foo..bar

would or should do.

-Peff

^ permalink raw reply

* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 17:09 UTC (permalink / raw)
  To: Jeff King
  Cc: SZEDER Gábor, Christian Couder, Christian Couder, git,
	Jonathan Nieder
In-Reply-To: <20120112165329.GA17173@sigill.intra.peff.net>

Hi Peff,

Jeff King wrote:
>                  I agree it would be nice to make:
>  git cherry-pick commit1 commit3 commit2
>
> work in the order specified, but how does that interact with existing
> cases that provide more traditional revision arguments?

What are your thoughts on making it a flag in the revision API to be
activated with "cherry-pick --literal-order commit1 commit3 commit2"
or similar?  I'm not sure how to get it to reconcile with the more
traditional revision arguments yet. My current worktree (WIP):

diff --git a/builtin/revert.c b/builtin/revert.c
index 0d8020c..47da41b 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -228,6 +228,7 @@ static void parse_args(int argc, const char
**argv, struct re
                opts->revs = xmalloc(sizeof(*opts->revs));
                init_revisions(opts->revs, NULL);
                opts->revs->no_walk = 1;
+               opts->revs->literal_order = 1;
                if (argc < 2)
                        usage_with_options(usage_str, options);
                argc = setup_revisions(argc, argv, opts->revs, NULL);
diff --git a/revision.c b/revision.c
index 064e351..301ef58 100644
--- a/revision.c
+++ b/revision.c
@@ -2054,7 +2054,10 @@ int prepare_revision_walk(struct rev_info *revs)
                if (commit) {
                        if (!(commit->object.flags & SEEN)) {
                                commit->object.flags |= SEEN;
-                               commit_list_insert_by_date(commit,
&revs->commits
+                               if (revs->literal_order)
+                                       commit_list_insert(commit,
&revs->commits
+                               else
+
commit_list_insert_by_date(commit, &revs-
                        }
                }
                e++;
diff --git a/revision.h b/revision.h
index b8e9223..65c3dc3 100644
--- a/revision.h
+++ b/revision.h
@@ -67,6 +67,7 @@ struct rev_info {
                        remove_empty_trees:1,
                        simplify_history:1,
                        lifo:1,
+                       literal_order:1,
                        topo_order:1,
                        simplify_merges:1,
                        simplify_by_decoration:1,

^ permalink raw reply related

* Re: [PATCH] git-blame.el: Fix compilation warnings.
From: Rüdiger Sonderfeld @ 2012-01-12 17:08 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: git, davidk, Sergei Organov, Kevin Ryde
In-Reply-To: <20120112162617.GA2479@burratino>

Hi,

On Thursday 12 January 2012 10:26:41 Jonathan Nieder wrote:
> These lines should be left out [*].

Sorry, I wasn't sure whether to remove them or not. I followed the description 
in git-format-patch(1) on how to send patches with kmail. I'll remove them in 
the future. Thanks for the advice.
 
> I assume this was prompted by warning messages like this one:
> 
> 	In git-blame-cleanup:
> 	git-blame.el:306:6:Warning: `mapcar' called for effect; use `mapc' or
> `dolist' instead
> 
> Looks reasonable to my very much untrained eyes, and it's consistent
> with the hints Kevin gave at [1].

Yes. I think the warnings are correct and should be addressed. E.g. Using 
mapcar compared to mapc is slower due to the required accumulation of the 
results and the additional garbage collection costs. It's not very dramatic 
but there is no reason not to fix it imho.

Regards,
Rüdiger

^ permalink raw reply

* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Jeff King @ 2012-01-12 16:53 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Christian Couder, Christian Couder, git, Ramkumar Ramachandra,
	Jonathan Nieder
In-Reply-To: <20120112144409.GV30469@goldbirke>

On Thu, Jan 12, 2012 at 03:44:09PM +0100, SZEDER Gábor wrote:

> > Thanks for the very detailed report!
> > 
> > I didn't test nor even compiled anything but maybe this can be fixed
> > by adding something like:
> > 
> > opts->revs->topo_order = 1;
> > 
> > in parse_args() or in prepare_revs()
> > 
> > I will try to have a look tonight.
> 
> [Beware, I'm mostly clueless about git internals.]
> 
> I don't think that any commit reordering, whether it's based on
> committer date, topology, or whatever, is acceptable.  Commits must be
> picked in the exact order they are specified on the command line.

I thought the multi-commit cherry-pick was supposed to take arbitrary
revision arguments, so you can do:

  git cherry-pick master..topic

and likewise you can spell it:

  git cherry-pick topic ^master

or:

  git cherry-pick ^master topic

So the order of arguments isn't relevant in those cases; the graph
ordering is. I agree it would be nice to make:

  git cherry-pick commit1 commit3 commit2

work in the order specified, but how does that interact with existing
cases that provide more traditional revision arguments?

-Peff

^ permalink raw reply

* Re: [PATCH] diff --no-index: support more than one file pair
From: Neal Kreitzinger @ 2012-01-12 16:37 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326359371-13528-1-git-send-email-pclouds@gmail.com>

On 1/12/2012 3:09 AM, Nguyễn Thái Ngọc Duy wrote:
> This allows you to do
>
> git diff --no-index file1.old file1.new file2.old file2.new...
>
> It could be seen as an abuse of "git --no-index", but it's very
> tempting considering many bells and whistles git's diff machinery
> provides.
>
I see that git-diff can be used in place of linux diff for totally 
untracked file pairs (which is kind of neat, I guess, if you're partial 
to git like I am and would probably prefer to use it as your primary 
file-system interface if you could).  I assume this new syntax implies 
manual usage since scripting this input is less straightforward than 
iterating thru a single pair via xargs, etc.  In that context, I also 
see that git-difftool doesn't bring up kdiff3 (or whatever) but just 
does a text diff (git 1.7.1) which is mildly disappointing for mere 
mortals like myself who prefer to read side-by-side gui diffs over text 
diffS.  This, of course, is also preference for someone like me who 
wouldn't mind prefixing all of my commands with "git " ;-)

v/r,
neal

^ permalink raw reply

* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Ramkumar Ramachandra @ 2012-01-12 16:35 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Christian Couder, Christian Couder, git, Jonathan Nieder
In-Reply-To: <20120112144409.GV30469@goldbirke>

Hi Gábor,

SZEDER Gábor wrote:
> I don't think that any commit reordering, whether it's based on
> committer date, topology, or whatever, is acceptable.  Commits must be
> picked in the exact order they are specified on the command line.

Thanks for the excellent report.  I'm trying to figure out how to get
the revision API to do no ordering.

-- Ram

^ permalink raw reply

* Re: [PATCH] git-blame.el: Fix compilation warnings.
From: Jonathan Nieder @ 2012-01-12 16:26 UTC (permalink / raw)
  To: Rüdiger Sonderfeld; +Cc: git, davidk, Sergei Organov, Kevin Ryde
In-Reply-To: <2608010.fNV39qBMLu@descartes>

(+cc: Sergei, Kevin)
Hi,

Rüdiger Sonderfeld wrote:

> From 4958c1b43d7a66654e15c92cbb878b38533d626e Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?R=C3=BCdiger=20Sonderfeld?= <ruediger@c-plusplus.de>
[...]

These lines should be left out [*].

> Replace mapcar with mapc because accumulation of the results was not
> needed. (git-blame-cleanup)
>
> Replace two occurrences of (save-excursion (set-buffer buf) ...)
> with (with-current-buffer buf ...). (git-blame-filter and
> git-blame-create-overlay)
>
> Replace goto-line with (goto-char (point-min)) (forward-line (1-
> start-line)). According to the documentation of goto-line it should
> not be called from elisp code. (git-blame-create-overlay)
>
> Signed-off-by: Rüdiger Sonderfeld <ruediger@c-plusplus.de>

I assume this was prompted by warning messages like this one:

	In git-blame-cleanup:
	git-blame.el:306:6:Warning: `mapcar' called for effect; use `mapc' or `dolist' instead

Looks reasonable to my very much untrained eyes, and it's consistent
with the hints Kevin gave at [1].

Thanks,
Jonathan

[1] http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=63;bug=611931
[*] The "From " line and following lines are for your mailer and can
be omited unless they differ from the mail header when reading your
patch into an email body.  See the DISCUSSION sections of
git-format-patch(1) and git-am(1) for more on this.

(patch left unsnipped for Sergei and Kevin's convenience)

> ---
>  contrib/emacs/git-blame.el |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
> index d351cfb..2e53fc6 100644
> --- a/contrib/emacs/git-blame.el
> +++ b/contrib/emacs/git-blame.el
> @@ -304,7 +304,7 @@ See also function `git-blame-mode'."
>  
>  (defun git-blame-cleanup ()
>    "Remove all blame properties"
> -    (mapcar 'delete-overlay git-blame-overlays)
> +    (mapc 'delete-overlay git-blame-overlays)
>      (setq git-blame-overlays nil)
>      (remove-git-blame-text-properties (point-min) (point-max)))
>  
> @@ -337,8 +337,7 @@ See also function `git-blame-mode'."
>  (defvar in-blame-filter nil)
>  
>  (defun git-blame-filter (proc str)
> -  (save-excursion
> -    (set-buffer (process-buffer proc))
> +  (with-current-buffer (process-buffer proc)
>      (goto-char (process-mark proc))
>      (insert-before-markers str)
>      (goto-char 0)
> @@ -385,11 +384,10 @@ See also function `git-blame-mode'."
>            info))))
>  
>  (defun git-blame-create-overlay (info start-line num-lines)
> -  (save-excursion
> -    (set-buffer git-blame-file)
> +  (with-current-buffer git-blame-file
>      (let ((inhibit-point-motion-hooks t)
>            (inhibit-modification-hooks t))
> -      (goto-line start-line)
> +      (goto-char (point-min)) (forward-line (1- start-line))
>        (let* ((start (point))
>               (end (progn (forward-line num-lines) (point)))
>               (ovl (make-overlay start end))

^ permalink raw reply

* [PATCH] git-blame.el: Fix compilation warnings.
From: Rüdiger Sonderfeld @ 2012-01-12 15:44 UTC (permalink / raw)
  To: git; +Cc: davidk

From 4958c1b43d7a66654e15c92cbb878b38533d626e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=BCdiger=20Sonderfeld?= <ruediger@c-plusplus.de>
Date: Thu, 12 Jan 2012 16:37:06 +0100
Subject: [PATCH] git-blame.el: Fix compilation warnings.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace mapcar with mapc because accumulation of the results was not
needed. (git-blame-cleanup)

Replace two occurrences of (save-excursion (set-buffer buf) ...)
with (with-current-buffer buf ...). (git-blame-filter and
git-blame-create-overlay)

Replace goto-line with (goto-char (point-min)) (forward-line (1-
start-line)). According to the documentation of goto-line it should
not be called from elisp code. (git-blame-create-overlay)

Signed-off-by: Rüdiger Sonderfeld <ruediger@c-plusplus.de>
---
 contrib/emacs/git-blame.el |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/contrib/emacs/git-blame.el b/contrib/emacs/git-blame.el
index d351cfb..2e53fc6 100644
--- a/contrib/emacs/git-blame.el
+++ b/contrib/emacs/git-blame.el
@@ -304,7 +304,7 @@ See also function `git-blame-mode'."
 
 (defun git-blame-cleanup ()
   "Remove all blame properties"
-    (mapcar 'delete-overlay git-blame-overlays)
+    (mapc 'delete-overlay git-blame-overlays)
     (setq git-blame-overlays nil)
     (remove-git-blame-text-properties (point-min) (point-max)))
 
@@ -337,8 +337,7 @@ See also function `git-blame-mode'."
 (defvar in-blame-filter nil)
 
 (defun git-blame-filter (proc str)
-  (save-excursion
-    (set-buffer (process-buffer proc))
+  (with-current-buffer (process-buffer proc)
     (goto-char (process-mark proc))
     (insert-before-markers str)
     (goto-char 0)
@@ -385,11 +384,10 @@ See also function `git-blame-mode'."
           info))))
 
 (defun git-blame-create-overlay (info start-line num-lines)
-  (save-excursion
-    (set-buffer git-blame-file)
+  (with-current-buffer git-blame-file
     (let ((inhibit-point-motion-hooks t)
           (inhibit-modification-hooks t))
-      (goto-line start-line)
+      (goto-char (point-min)) (forward-line (1- start-line))
       (let* ((start (point))
              (end (progn (forward-line num-lines) (point)))
              (ovl (make-overlay start end))
-- 
1.7.8.3

^ permalink raw reply related

* Re: Zsh completion regression
From: Matthieu Moy @ 2012-01-12 14:56 UTC (permalink / raw)
  To: Stefan Haller; +Cc: git, SZEDER Gábor
In-Reply-To: <1kdr5xk.1sopzul1hygnbrM%lists@haller-berlin.de>

lists@haller-berlin.de (Stefan Haller) writes:

> I'm using zsh   4.3.11.
>
> When I type "git log mas<TAB>", it completes to "git log master\ " (a
> backslash, a space, and then the cursor).

Same here (although I've been too lazy to bisect myself).

The following patch makes the situation better, but is not really a fix:

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -525,7 +525,7 @@ __gitcomp ()
 __gitcomp_nl ()
 {
        local s=$'\n' IFS=' '$'\t'$'\n'
-       local cur_="$cur" suffix=" "
+       local cur_="$cur" suffix=""
 
        if [ $# -gt 2 ]; then
                cur_="$3"

With this, the trailing space isn't added, but e.g. "git checkout
master<TAB>" does not add the trailing space, at all.

The problem is a little bit below:

	IFS=$s
	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))

The -S "$suffix" adds a space to the completion, but ZSH escapes the
space (which sounds sensible in general, but is not at all what we
expect). My completion-fu isn't good enough to get any further either
unfortunately.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: SZEDER Gábor @ 2012-01-12 14:44 UTC (permalink / raw)
  To: Christian Couder
  Cc: Christian Couder, git, Ramkumar Ramachandra, Jonathan Nieder
In-Reply-To: <CAP8UFD2uLoqzXRxssjwwW1Vk8RuNF_5OT1d7Z7hiRQ+Rq=UM1A@mail.gmail.com>

Hi,


On Thu, Jan 12, 2012 at 02:31:30PM +0100, Christian Couder wrote:
> Hi all,
> 
> 2012/1/11 SZEDER Gábor <szeder@ira.uka.de>:
> >
> > As far as I can tell, this buggy behavior is as old as multi-commit
> > cherry-pick itself, i.e. 7e2bfd3f (revert: allow cherry-picking more
> > than one commit, 2010-06-02).
> 
> Thanks for the very detailed report!
> 
> I didn't test nor even compiled anything but maybe this can be fixed
> by adding something like:
> 
> opts->revs->topo_order = 1;
> 
> in parse_args() or in prepare_revs()
> 
> I will try to have a look tonight.

[Beware, I'm mostly clueless about git internals.]

I don't think that any commit reordering, whether it's based on
committer date, topology, or whatever, is acceptable.  Commits must be
picked in the exact order they are specified on the command line.

Besides, AFAICT, parse_args() sets opts->revs->no_walk = 1, which will
cause prepare_revision_walk() to return before it would reach the
topo_order condition, so opts->revs->topo_order = 1 wouldn't have any
effect.


Best,
Gábor

^ permalink raw reply

* Re: clone bug
From: Andreas Schwab @ 2012-01-12 14:36 UTC (permalink / raw)
  To: Alexey Kuznetsov; +Cc: git
In-Reply-To: <CAO1Zr+pSLwRbsEZ_0LCeE2qLn+S=iMKVcMjqtYrmiBoQmjac_A@mail.gmail.com>

Alexey Kuznetsov <kuznetsov.alexey@gmail.com> writes:

> axet-laptop:123 axet$ git branch
> * master
> # git remote add common https://github.com/axet/common-bin.git
> # git fetch common
> From https://github.com/axet/common-bin
>  * [new branch]      master     -> common/master
>
> ?? already strange master (local) to the remote common/master

The message means: the remote ref 'refs/heads/master' is stored locally
in 'refs/remotes/common/master'.

> axet-laptop:123 axet$ git push
> To https://github.com/axet/common-bin.git
>  ! [rejected]        master -> master (non-fast-forward)

"git push" is the same as "git push common" ('common' is the current
branch's remote).  Since branch.master.push is not defined this then
uses the push.default config option to determine the action.  The
default is 'matching', which means that local branch names are matched
against remote branch names.  Local branch master matches remote branch
master.  Note that this disregards the setting for
branch.master.upstream.  If you do not want that you should set
push.default to 'upstream'.

See the examples in git-push(1) for more details.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply

* Re: clone bug
From: Thomas Rast @ 2012-01-12 14:32 UTC (permalink / raw)
  To: Alexey Kuznetsov; +Cc: git
In-Reply-To: <CAO1Zr+pSLwRbsEZ_0LCeE2qLn+S=iMKVcMjqtYrmiBoQmjac_A@mail.gmail.com>

Alexey Kuznetsov <kuznetsov.alexey@gmail.com> writes:
>
> [branch "common"]
> 	remote = common
> 	merge = refs/heads/master
>
> ?? correct
>
> axet-laptop:123 axet$ git pull
> Already up-to-date.
> axet-laptop:123 axet$ git push
> To https://github.com/axet/common-bin.git
>  ! [rejected]        master -> master (non-fast-forward)
> error: failed to push some refs to 'https://github.com/axet/common-bin.git'
> To prevent you from losing history, non-fast-forward updates were rejected
> Merge the remote changes (e.g. 'git pull') before pushing again.  See the
> 'Note about fast-forwards' section of 'git push --help' for details.
> axet-laptop:123 axet$
>
>
> it tries to push local master to remote common/master which is not correct.

This is controlled by the 'push.default' variable, the docs for which
are as follows:

  push.default::
  	Defines the action git push should take if no refspec is given
  	on the command line, no refspec is configured in the remote, and
  	no refspec is implied by any of the options given on the command
  	line. Possible values are:
  +
  * `nothing` - do not push anything.
  * `matching` - push all matching branches.
    All branches having the same name in both ends are considered to be
    matching. This is the default.
  * `upstream` - push the current branch to its upstream branch.
  * `tracking` - deprecated synonym for `upstream`.
  * `current` - push the current branch to a branch of the same name.

The default of 'matching' pushes X to remotename/X, which is what you
are seeing.  You can either set it to 'upstream' or specify the
target explicitly, as in

  git push common common:master

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* clone bug
From: Alexey Kuznetsov @ 2012-01-12 13:43 UTC (permalink / raw)
  To: git

Hello!

Seems like I found a bug in the clone / push logic. I'm trying to
clone remote branch master into local branch called common and unable
to push back common to master. Git trying to push local master from
different origin to common/master instead.

Here is a simple example:

# mkdir 123
# git init
# > 123
# git add .
# git commit -m "initial"
# git branch
axet-laptop:123 axet$ git branch
* master
# git remote add common https://github.com/axet/common-bin.git
# git fetch common
From https://github.com/axet/common-bin
 * [new branch]      master     -> common/master

?? already strange master (local) to the remote common/master

# axet-laptop:123 axet$ git checkout -b common common/master
Branch common set up to track remote branch master from common.
Switched to a new branch 'common'

# axet-laptop:123 axet$ git branch
* common
  master
axet-laptop:123 axet$

# cat .git/config
[...]

[branch "common"]
	remote = common
	merge = refs/heads/master

?? correct

axet-laptop:123 axet$ git pull
Already up-to-date.
axet-laptop:123 axet$ git push
To https://github.com/axet/common-bin.git
 ! [rejected]        master -> master (non-fast-forward)
error: failed to push some refs to 'https://github.com/axet/common-bin.git'
To prevent you from losing history, non-fast-forward updates were rejected
Merge the remote changes (e.g. 'git pull') before pushing again.  See the
'Note about fast-forwards' section of 'git push --help' for details.
axet-laptop:123 axet$


it tries to push local master to remote common/master which is not correct.

-- AK

^ permalink raw reply

* Re: [BUG] multi-commit cherry-pick messes up the order of commits
From: Christian Couder @ 2012-01-12 13:31 UTC (permalink / raw)
  To: SZEDER Gábor
  Cc: Christian Couder, git, Ramkumar Ramachandra, Jonathan Nieder
In-Reply-To: <20120111173101.GQ30469@goldbirke>

Hi all,

2012/1/11 SZEDER Gábor <szeder@ira.uka.de>:
>
> As far as I can tell, this buggy behavior is as old as multi-commit
> cherry-pick itself, i.e. 7e2bfd3f (revert: allow cherry-picking more
> than one commit, 2010-06-02).

Thanks for the very detailed report!

I didn't test nor even compiled anything but maybe this can be fixed
by adding something like:

opts->revs->topo_order = 1;

in parse_args() or in prepare_revs()

I will try to have a look tonight.

Thanks again,
Christian.

^ permalink raw reply

* Zsh completion regression
From: Stefan Haller @ 2012-01-12 11:52 UTC (permalink / raw)
  To: git; +Cc: SZEDER Gábor

I'm using zsh   4.3.11.

When I type "git log mas<TAB>", it completes to "git log master\ " (a
backslash, a space, and then the cursor).

Bisecting points to "a31e626 completion: optimize refs completion."
Before this commit, I get "git log master" (with no space at the end).

My completion-fu is not strong enough to dig into this myself, but I'll
be happy to help test, or do whatever else helps.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

^ permalink raw reply

* [PATCH] word-diff: ignore '\ No newline at eof' marker
From: Thomas Rast @ 2012-01-12 11:15 UTC (permalink / raw)
  To: Ivan Shirokoff; +Cc: git, Junio C Hamano
In-Reply-To: <4F0EA23D.3010603@yandex-team.ru>

The word-diff logic accumulates + and - lines until another line type
appears (normally [ @\]), at which point it generates the word diff.
This is usually correct, but it breaks when the preimage does not have
a newline at EOF:

  $ printf "%s" "a a a" >a
  $ printf "%s\n" "a ab a" >b
  $ git diff --no-index --word-diff a b
  diff --git 1/a 2/b
  index 9f68e94..6a7c02f 100644
  --- 1/a
  +++ 2/b
  @@ -1 +1 @@
  [-a a a-]
   No newline at end of file
  {+a ab a+}

Because of the order of the lines in a unified diff

  @@ -1 +1 @@
  -a a a
  \ No newline at end of file
  +a ab a

the '\' line flushed the buffers, and the - and + lines were never
matched with each other.

A proper fix would defer such markers until the end of the hunk.
However, word-diff is inherently whitespace-ignoring, so as a cheap
fix simply ignore the marker (and hide it from the output).

We use a prefix match for '\ ' to parallel the logic in
apply.c:parse_fragment().  We currently do not localize this string
(just accept other variants of it in git-apply), but this should be
future-proof.

Noticed-by: Ivan Shirokoff <shirokoff@yandex-team.ru>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 diff.c                |    8 ++++++++
 t/t4034-diff-words.sh |   14 ++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/diff.c b/diff.c
index a65223a..996cc60 100644
--- a/diff.c
+++ b/diff.c
@@ -1113,6 +1113,14 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
 			diff_words_append(line, len,
 					  &ecbdata->diff_words->plus);
 			return;
+		} else if (!prefixcmp(line, "\\ ")) {
+			/*
+			 * Silently eat the "no newline at eof" marker
+			 * (we are diffing without regard to
+			 * whitespace anyway), and defer processing:
+			 * more '+' lines could be after it.
+			 */
+			return;
 		}
 		diff_words_flush(ecbdata);
 		if (ecbdata->diff_words->type == DIFF_WORDS_PORCELAIN) {
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 6f1e5a2..5c20121 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -334,4 +334,18 @@ test_expect_success 'word-diff with diff.sbe' '
 	word_diff --word-diff=plain
 '
 
+test_expect_success 'word-diff with no newline at EOF' '
+	cat >expect <<-\EOF &&
+	diff --git a/pre b/post
+	index 7bf316e..3dd0303 100644
+	--- a/pre
+	+++ b/post
+	@@ -1 +1 @@
+	a a [-a-]{+ab+} a a
+	EOF
+	printf "%s" "a a a a a" >pre &&
+	printf "%s" "a a ab a a" >post &&
+	word_diff --word-diff=plain
+'
+
 test_done
-- 
1.7.9.rc0.168.g3847c

^ permalink raw reply related

* Re: git diff <file> HEAD^:<file> error message
From: Carlos Martín Nieto @ 2012-01-12 10:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr4z54pwp.fsf@alter.siamese.dyndns.org>

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On Wed, Jan 11, 2012 at 06:26:30PM -0800, Junio C Hamano wrote:
> Carlos Martín Nieto <cmn@elego.de> writes:
> 
> > I was trying to figure out why running
> >
> >    git diff HEAD^:RelNotes RelNotes
> >
> > gives the expected output (on maint it tells me that the stable
> > version changed from 1.7.8.3 to 1.7.8.4) but swapping the arguments
> > doesn't.
> >
> >    git diff RelNotes HEAD^:RelNotes
> >
> > doesn't show the opposite patch ...
> 
> That comes from the general argument parsing rules of Git, namely, global
> options (e.g. --paginate) first, then subcommand name, followed by dashed
> options, revs and finally the paths. Once you give "RelNotes", which
> cannot be a rev, you cannot give a rev.
> 
> We _could_ special case the rule for "diff", but we simply didn't bother,
> as the resulting code (and the implications of special casing) would be
> too ugly to live to support such a corner case usage, especially when you
> could always say "-R" to reverse the output.

The rule "non-rev stops rev parsing" is fair enough. The error message
is still very misleading, as it lies about RelNotes not being in HEAD^
and gives the impression that it was parsed as a rev (which I guess it
was, but only to show the message).

   cmn


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* Re: [PATCH] diff --no-index: support more than one file pair
From: Matthieu Moy @ 2012-01-12  9:30 UTC (permalink / raw)
  To: Nguyen Thai Ngoc Duy; +Cc: git
In-Reply-To: <CACsJy8BvA_o1+xrOx4hYhmwNWpsRnh5+mftb471h3yFW2b6vhA@mail.gmail.com>

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Single operation has its advantages:
>
>  - one pager
>  - one stat and summary

I buy these, and I understand why they apply to "git diff" and not GNU
diff.

>  - might be easier to script (just throw them all to xargs)

I don't see a use-case where a command produces old1 new1 old2 new2, but
if there is one, then "| xargs -n 2 diff" is the solution. You don't
need your patch.

>  - hell, i might even benefit from git copy/modify detection

I don't see how, if you specify explicitely the pairs (old, new). You
may have such benefit if you let the command-line express "here's a
bunch of old files, and a bunch of new ones", but not with your proposed
syntax.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* Re: [PATCH 2/2] diff --word-diff: use non-whitespace regex by default
From: Thomas Rast @ 2012-01-12  9:22 UTC (permalink / raw)
  To: Tay Ray Chuan; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <CALUzUxo3DcKqC6sQFQ1Oi0vgASFSHCcmOgHAj2_4c3vEjy663w@mail.gmail.com>

Tay Ray Chuan <rctay89@gmail.com> writes:

> On Thu, Jan 12, 2012 at 4:05 AM, Thomas Rast <trast@student.ethz.ch> wrote:
>> Tay Ray Chuan <rctay89@gmail.com> writes:
>>
>>> Factor out the comprehensive non-whitespace regex in use by PATTERNS and
>>> IPATTERN and use it as the word-diff regex for the default diff driver.
>>
>> Why?

Sorry for distracting you with the performance argument; it was mostly
the first thing that came to my mind that I could use to ask for the
motivation, and evaluation of tradeoffs, that both were missing from the
proposed commit message.

> But I think it's worthwhile to trade-off performance for a sensible
> default. Something like
>
>   matrix[a,b,c]
>   matrix[d,b,c]
>
> gives
>
>   matrix[[-a-]{+d+},b,c]
>
> and when we have
>
>   ImagineALanguageLikeFoo
>   ImagineALanguageLikeBar
>
> we get
>
>   ImagineALanguageLike[-Foo-]{+Bar+}

In that case (and I should have read the original patch), I am
definitely against this change.  It turns the default word-diff into
character-diff, which is something entirely different, and frequently
useless precisely for the reason you state:

> (But I cheated. Foo and Bar have no common characters in common; if
> they did, the word diff would be messy.)

Case in point, consider my patch sent out yesterday

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

It consists of a one-hunk doc update.  word-diff is not brilliant:

  -k::
          Usually the program [-'cleans up'-]{+removes email cruft from+} the Subject:
          header line to extract the title line for the commit log
          [-message,-]
  [-      among which (1) remove 'Re:' or 're:', (2) leading-]
  [-      whitespaces, (3) '[' up to ']', typically '[PATCH]', and-]
  [-      then prepends "[PATCH] ".-]{+message.+}  This [-flag forbids-]{+option prevents+} this munging, and is most
          useful when used to read back 'git format-patch -k' output.
[snip the rest as it's only {+}]

But character-diff tries too hard to find common subsequences:

  $ g show HEAD^^ --word-diff-regex='[^[:space:]]' | xsel
  -k::
          Usually the program [-'cl-]{+remov+}e[-an-]s {+email cr+}u[-p'-]{+ft from+} the Subject:
          header line to extract the title line for the commit log
          message[-,-]
  [-      among which (1) remove 'Re:' or 're:', (2) leading-]
  [-      w-]{+.  T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[-ically '[PATCH]', and-]t[-he-]{+io+}n pre[-p-]{+v+}en[-ds "[PATCH] ".  This flag forbid-]{+t+}s this munging, and is most
          useful when used to read back 'git format-patch -k' output.
[snip]

Wouldn't you agree that

  w-]{+.  T+}hi[-te-]s[-paces, (3) '[' up t-] o[-']', ty-]p[

is just line noise?  The colors don't even help as most of it is removed
(red).

Regarding your examples

> [1] http://article.gmane.org/gmane.comp.version-control.git/105896
> [2] http://article.gmane.org/gmane.comp.version-control.git/105237

first please notice that both of them were written before (and actually
discussing) the introduction of the wordRegex feature.  At this point,
we were trying to make up our minds w.r.t. how powerful the feature
needs to be.  Nowadays (or in fact, starting a few days after those
emails) the user can easily achieve everything discussed here by setting
the wordRegex to taste.

That being said, I can see some arguments for changing the default to
split punctuation into a separate word.  That is, whereas the current
default is semantically equivalent to a wordRegex of

  [^[:space:]]*

(but has a faster code path) and your proposal is equivalent to

  [^[:space:]]|UTF_8_GUARD

I think there is a case to be made for a default of

  [^[:space:]]|([[:alnum:]]|UTF_8_GUARD)+

or some such.  There's a lot of bikeshedding lurking in the (non)extent
of the [[:alnum:]] here, however.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply

* Re: [PATCH] diff --no-index: support more than one file pair
From: Nguyen Thai Ngoc Duy @ 2012-01-12  9:17 UTC (permalink / raw)
  To: Matthieu Moy; +Cc: git
In-Reply-To: <vpq39bll1ua.fsf@bauges.imag.fr>

2012/1/12 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> This allows you to do
>>
>> git diff --no-index file1.old file1.new file2.old file2.new...
>>
>> It could be seen as an abuse of "git --no-index", but it's very
>> tempting considering many bells and whistles git's diff machinery
>> provides.
>
> I find this very, very unintuitive. I tried with GNU diff:
>
> diff 1 2 3 4
>
> and it complained with "diff: extra operand `3'". I find
>
> git diff --no-index file1.old file1.new
> git diff --no-index file2.old file2.new
>
> far more intuitive, less error prone (when you start having a
> non-trivial list of arguments, it's hard to tell which is the new and
> which is the old visually), ... So I'm curious why you prefer your
> syntax.

Single operation has its advantages:

 - one pager
 - one stat and summary
 - might be easier to script (just throw them all to xargs)
 - hell, i might even benefit from git copy/modify detection
-- 
Duy

^ permalink raw reply

* Re: [PATCH] diff --no-index: support more than one file pair
From: Matthieu Moy @ 2012-01-12  9:14 UTC (permalink / raw)
  To: Nguyễn Thái Ngọc Duy; +Cc: git
In-Reply-To: <1326359371-13528-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> This allows you to do
>
> git diff --no-index file1.old file1.new file2.old file2.new...
>
> It could be seen as an abuse of "git --no-index", but it's very
> tempting considering many bells and whistles git's diff machinery
> provides.

I find this very, very unintuitive. I tried with GNU diff:

diff 1 2 3 4

and it complained with "diff: extra operand `3'". I find

git diff --no-index file1.old file1.new
git diff --no-index file2.old file2.new

far more intuitive, less error prone (when you start having a
non-trivial list of arguments, it's hard to tell which is the new and
which is the old visually), ... So I'm curious why you prefer your
syntax.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

^ permalink raw reply

* git diff --word-diff problem
From: Ivan Shirokoff @ 2012-01-12  9:05 UTC (permalink / raw)
  To: git

Hello.
I've got a couple of generated files with obfuscated code.
I want to word-diff them just to make sure that everything is right.

The thing is when word-diff gets oneline file without newline at the end 
it compares it with regular line by line diff.

Here is an example. Two one line files generated with perl -e "print 'a 
'x10" > file

git diff --word-diff=plain file1 file2
diff --git a/file1 b/file2
index 3526254..0515a63 100644
--- a/file1
+++ b/file2
@@ -1 +1 @@
[- a a a a a a a a a a-]
  No newline at end of file
  {+a a a a a ab a a a a+}

Git shows that the whole line is different.
And if I add newlines to that files everything works just as expected

git diff --word-diff=plain file1 file2
diff --git a/file1 b/file2
index 1756d83..1ec45b9 100644
--- a/file1
+++ b/file2
@@ -1,2 +1,2 @@
  a a a a a [-a -]{+ab +}a a a a

Is that a bug or I've missed explanation in docs?

-- 
Ivan Shirokoff

^ permalink raw reply related

* [PATCH] diff --no-index: support more than one file pair
From: Nguyễn Thái Ngọc Duy @ 2012-01-12  9:09 UTC (permalink / raw)
  To: git; +Cc: Nguyễn Thái Ngọc Duy

This allows you to do

git diff --no-index file1.old file1.new file2.old file2.new...

It could be seen as an abuse of "git --no-index", but it's very
tempting considering many bells and whistles git's diff machinery
provides.

Signed-off-by: A Clearcase user who has had enough with "ct diff"
---
 Sorry I used git@vger as a personal archive, but this might benefit
 others as well, I think.

 diff-no-index.c |   38 +++++++++++++++++++++-----------------
 1 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 3a36144..b4f6d06 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -199,12 +199,8 @@ void diff_no_index(struct rev_info *revs,
 		     !path_outside_repo(argv[i+1])))
 			return;
 	}
-	if (argc != i + 2)
-		usagef("git diff %s <path> <path>",
-		       no_index ? "--no-index" : "[--no-index]");
-
 	diff_setup(&revs->diffopt);
-	for (i = 1; i < argc - 2; ) {
+	for (i = 1; i < argc; ) {
 		int j;
 		if (!strcmp(argv[i], "--no-index"))
 			i++;
@@ -214,13 +210,19 @@ void diff_no_index(struct rev_info *revs,
 		}
 		else if (!strcmp(argv[i], "--"))
 			i++;
-		else {
+		else if (argv[i][0] == '-') {
 			j = diff_opt_parse(&revs->diffopt, argv + i, argc - i);
 			if (!j)
 				die("invalid diff option/value: %s", argv[i]);
 			i += j;
 		}
+		else
+			break;
 	}
+	if ((argc - i) % 2)
+		usagef("git diff %s <path> <path>%s",
+		       no_index ? "--no-index" : "[--no-index]",
+		       no_index ? "[ <path> <path>...]" : "");
 
 	/*
 	 * If the user asked for our exit code then don't start a
@@ -229,13 +231,15 @@ void diff_no_index(struct rev_info *revs,
 	if (!DIFF_OPT_TST(&revs->diffopt, EXIT_WITH_STATUS))
 		setup_pager();
 
+	/* argv now only contains paths */
+	argv += i;
+	argc -= i;
+
 	if (prefix) {
 		int len = strlen(prefix);
-		const char *paths[3];
-		memset(paths, 0, sizeof(paths));
 
-		for (i = 0; i < 2; i++) {
-			const char *p = argv[argc - 2 + i];
+		for (i = 0; i < argc; i++) {
+			const char *p = argv[i];
 			/*
 			 * stdin should be spelled as '-'; if you have
 			 * path that is '-', spell it as ./-.
@@ -243,12 +247,10 @@ void diff_no_index(struct rev_info *revs,
 			p = (strcmp(p, "-")
 			     ? xstrdup(prefix_filename(prefix, len, p))
 			     : p);
-			paths[i] = p;
+			argv[i] = p;
 		}
-		diff_tree_setup_paths(paths, &revs->diffopt);
 	}
-	else
-		diff_tree_setup_paths(argv + argc - 2, &revs->diffopt);
+
 	revs->diffopt.skip_stat_unmatch = 1;
 	if (!revs->diffopt.output_format)
 		revs->diffopt.output_format = DIFF_FORMAT_PATCH;
@@ -260,9 +262,11 @@ void diff_no_index(struct rev_info *revs,
 	if (diff_setup_done(&revs->diffopt) < 0)
 		die("diff_setup_done failed");
 
-	if (queue_diff(&revs->diffopt, revs->diffopt.pathspec.raw[0],
-		       revs->diffopt.pathspec.raw[1]))
-		exit(1);
+	while (argv[0] && argv[1]) {
+		if (queue_diff(&revs->diffopt, argv[0], argv[1]))
+			exit(1);
+		argv += 2;
+	}
 	diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
-- 
1.7.3.1.256.g2539c.dirty

^ permalink raw reply related

* Re: [PATCH 2/3] am: learn passing -b to mailinfo
From: Thomas Rast @ 2012-01-12  8:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzkdt4s9l.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Rast <trast@student.ethz.ch> writes:
>
>> @@ -571,8 +574,8 @@ then
>>  else
>>  	utf8=-n
>>  fi
>> -if test "$(cat "$dotest/keep")" = t
>> -then
>> +keep=$(cat "$dotest/keep")
>> +if test "$keep" = t
>>  	keep=-k
>>  fi
>
> Curious.
>
> Who writes 't' to $dotest/keep after this patch is applied?

Nobody; like the commit message says, I was just trying to help users
upgrading from one version to the next in the middle of an 'am', which
almost worked except for

> I suspect that this patch was not tested in a way to exercise this
> codepath; shell would have barfed when seeing the lack of "then" here, no?

(ouch)

> I also do not want to worry about "echo" portability issues that may come
> from an existing
>
> 	echo "$keep" >"$dotest/keep"
>
> that this patch does not touch.

Good point, thanks.  I'll reroll with printf.  Should I keep the
upgrade path compatibility?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox