Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Remove deprecated commands from command list and update manpages
From: Johannes Schindelin @ 2007-11-08 15:18 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: Junio C Hamano, git
In-Reply-To: <20071108145435.GA18727@diku.dk>

Hi,

On Thu, 8 Nov 2007, Jonas Fonseca wrote:

> ... and remove manpages of commands that no longer exists.
> 
> Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
> ---
>  Documentation/cmd-list.perl       |    5 ---

Maybe keep git-lost-found?

>  Documentation/git-local-fetch.txt |   66 -------------------------------------
>  Documentation/git-ssh-fetch.txt   |   52 -----------------------------
>  Documentation/git-ssh-upload.txt  |   48 ---------------------------
>  Documentation/git-tar-tree.txt    |    2 +-
>  Documentation/git-var.txt         |    3 +-

Last time I checked, git-var was still alive.

Ciao,
Dscho

^ permalink raw reply

* Re: [PATCH 4/3] t3700: avoid racy git situation
From: Johannes Schindelin @ 2007-11-08 15:16 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, krh, gitster
In-Reply-To: <47331E65.9010209@viscovery.net>

Hi,

On Thu, 8 Nov 2007, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > Wow, the builtin commit is fast.  It sometimes triggers a racy
> > situation in the test case for "git add --refresh -- foo".
> > 
> > So when that test fails, simply sleep one second and try again.
> 
> [/me looks at the calender - no, it's not April Fool's day]

No. The builtin commit is really that fast.

> Wouldn't it be better to fix git-commit (or git-add)? I would like to 
> help, but you already seem to have done the analysis, so...

The problem is that the index has the same timestamp as the file "foo".

Therefore, git cannot tell if "foo" is up-to-date in the index, since it 
could have been modified (and indeed is) just a fraction of a second later 
than the index was last updated.

And since diff-index, as called from the test script, does not generate a 
diff, but really only determines if the index information suggests that 
the files are up-to-date, there is not really much you can do.

This is our good old friend, the racy git problem.

NOTE: other scms do not have this problem, mostly because they are too 
slow to trigger it.

Ciao,
Dscho

^ permalink raw reply

* [PATCH] core-tutorial.txt: Fix git-show-branch example and its description
From: Sergei Organov @ 2007-11-08 15:10 UTC (permalink / raw)
  To: git; +Cc: gitster



Signed-off-by: Sergei Organov <osv@javad.com>
---

 The example and its description didn't match each other. In addition,
 I've added explanatory notes that hopefully will decrease the chances
 of confusion I've personally ran into after reading this part of the
 tutorial.

 Documentation/core-tutorial.txt |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/core-tutorial.txt b/Documentation/core-tutorial.txt
index 99817c5..ebd2492 100644
--- a/Documentation/core-tutorial.txt
+++ b/Documentation/core-tutorial.txt
@@ -931,12 +931,13 @@ Another useful tool, especially if you do not always work in X-Window
 environment, is `git show-branch`.
 
 ------------------------------------------------
-$ git show-branch --topo-order master mybranch
+$ git-show-branch --topo-order --more=1 master mybranch
 * [master] Merge work in mybranch
  ! [mybranch] Some work.
 --
 -  [master] Merge work in mybranch
 *+ [mybranch] Some work.
+*  [master^] Some fun.
 ------------------------------------------------
 
 The first two lines indicate that it is showing the two branches
@@ -954,10 +955,22 @@ because `mybranch` has not been merged to incorporate these
 commits from the master branch.  The string inside brackets
 before the commit log message is a short name you can use to
 name the commit.  In the above example, 'master' and 'mybranch'
-are branch heads.  'master~1' is the first parent of 'master'
+are branch heads.  'master^' is the first parent of 'master'
 branch head.  Please see 'git-rev-parse' documentation if you
 see more complex cases.
 
+[NOTE]
+Without the '--more=1' option, 'git-show-branch' would not output the
+'[master^]' commit, as '[mybranch]' commit is a common ancestor of
+both 'master' and 'mybranch' tips.  Please see 'git-show-branch'
+documentation for details.
+
+[NOTE]
+If there were more commits on the 'master' branch after the merge, the
+merge commit itself would not be shown by 'git-show-branch' by
+default.  You would need to provide '--sparse' option to make the
+merge commit visible in this case.
+
 Now, let's pretend you are the one who did all the work in
 `mybranch`, and the fruit of your hard work has finally been merged
 to the `master` branch. Let's go back to `mybranch`, and run
-- 
1.5.3.5.529.ge3d6d

^ permalink raw reply related

* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Steffen Prohaska @ 2007-11-08 15:07 UTC (permalink / raw)
  To: Benoit Sigoure
  Cc: Johannes Sixt, Andreas Ericsson, Ralf Wildenhues,
	Git Mailing List
In-Reply-To: <5498B368-053A-424E-B901-A55B66FF5801@lrde.epita.fr>


On Nov 8, 2007, at 3:51 PM, Benoit Sigoure wrote:

> On Nov 8, 2007, at 1:54 PM, Steffen Prohaska wrote:
>
>> Do you use rebase like this in real life?
>>
>> I thought of the text as background information that might
>> be helpful for users who want do decide wether to merge or
>> to rebase. The problem described may be valuable information
>> supporting a decision about a recommended workflow for a group
>> of users.
>
> You're missing the point.  Johannes suggested that you rebase  
> *only* for bisecting purpose.  Once you find the culprit commit,  
> throw away your rebased stuff.

I got this point. I also noted that it might be time consuming
if you need to resolve conflicts.

The original discussion in which Junio explained the problem
with bisecting merges was about workflows. The question then was
if users should _always_ rebase if possible to make bisecting
easier. It was really a workflow question.

Well, the context of the original discussion is no longer
present in the patch I sent. Therefore, Johannes' comments
absolutely make sense. Actually I find them really inspiring as
I have not thought before about rebasing temporarily, just for
finding a commit. Now I learnt that this could be a useful tool.


> I've never thought about doing this myself, but it's a very clever  
> way of tackling this problem.

Apparently, you haven't thought about this solution either ;)

	Steffen

^ permalink raw reply

* Re: [PATCH 1/2] mirror pushing
From: Johannes Schindelin @ 2007-11-08 15:03 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: git
In-Reply-To: <1194531890.0@pinky>

Hi,

On Thu, 8 Nov 2007, Andy Whitcroft wrote:

> -		       nr_refspec, refspec, args.send_all))
> +		       nr_refspec, refspec, args.send_all | (args.send_mirror << 1)))

This line is too long.  But it needsmore love: it's all too magic to have 
a 1 for send_all, and a 2 for mirror.  Please introduce an enum for that 
in remote.h, and use those constants, so that this hunk and the following 
one cannot get out of sync that easily.

> +++ b/remote.c
> @@ -722,10 +722,12 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
>   * without thinking.
>   */
>  int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
> -	       int nr_refspec, const char **refspec, int all)
> +	       int nr_refspec, const char **refspec, int flags)
>  {
>  	struct refspec *rs =
>  		parse_ref_spec(nr_refspec, (const char **) refspec);
> +	int send_all = flags & 01;
> +	int send_mirror = flags & 02;
>  
>  	if (match_explicit_refs(src, dst, dst_tail, rs, nr_refspec))
>  		return -1;


Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH] Remove deprecated commands from command list and update manpages
From: Andreas Ericsson @ 2007-11-08 15:00 UTC (permalink / raw)
  To: Jonas Fonseca; +Cc: Johannes Schindelin, Junio C Hamano, git
In-Reply-To: <20071108145435.GA18727@diku.dk>

Jonas Fonseca wrote:
> ... and remove manpages of commands that no longer exists.
> 
> diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
> index 8139423..73c37b0 100644
> --- a/Documentation/git-var.txt
> +++ b/Documentation/git-var.txt
> @@ -20,7 +20,8 @@ OPTIONS
>  	Cause the logical variables to be listed. In addition, all the
>  	variables of the git configuration file .git/config are listed
>  	as well. (However, the configuration variables listing functionality
> -	is deprecated in favor of `git-config -l`.)
> +	is deprecated. Use gitlink:git-config[1] with the option '-l'
> +	instead.)
>  

Skip the parentheses. It reads better without it.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Steffen Prohaska @ 2007-11-08 14:55 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: Andreas Ericsson, gitster, Ralf.Wildenhues, tsuna, git
In-Reply-To: <47330D78.3040808@viscovery.net>


On Nov 8, 2007, at 2:22 PM, Johannes Sixt wrote:

> Steffen Prohaska schrieb:
>> On Nov 8, 2007, at 10:53 AM, Johannes Sixt wrote:
>>> The text that the patch amends is about bisecting history that  
>>> reveals that a merge commit breaks, which is not helpful, and  
>>> then how to find where and what and why the breakage really was  
>>> introduce.
>>>
>>> And the answer to "how to find" is to rebase and bisect in the  
>>> rebased history.
>> Do you use rebase like this in real life?
>
> Why is this relevant?

Well, I don't want to give recommendations that are not tested
in real life. Maybe the solution turns out to be less practical
than it should be theoretically.


> You've written a superb addendum to the user manual, but IT TALKS  
> ABOUT BISECTION, and is not a guideline when to use merges and when  
> to rebase.

BTW, I only took what Junio wrote during a recent discussion
on the list, polished it a bit and sent it as a patch. I'm
only the editor, not the original author. That doesn't mean
I wouldn't care about the text. I'm willing to improve the text.
But all the cheers really should go to Junio.


> It better not be meant as such. Consider an integrator who has just  
> merged two histories, both of which are available publically.  
> Pushing out a rebased history IS NOT AN OPTION. If the poor fellow  
> for the heck of it has no choice but to find the bogus commit, then  
> your instructions are worth a thousand bucks - even if the rebased  
> history is otherwise useless -, but any guidelines how to construct  
> histories are IRRELEVANT for his case.

Ok, I see your point. I'll mention these points.


>> But now I'm wondering if your suggestions of rebasing only for
>> locating the evil commit is feasible in reality. You may need
>> to solve a lot of merge conflicts if you rebase a larger part
>> of the history. If you do not have them in your rerere cache
>> this might be time consuming. ...
>
> During the rebase you will see the same conflicts that you also had  
> during the merge, even simpler ones (because they are - hopefully -  
> broken down into smaller pieces). If your merge was clean (as was  
> suggested in the patch), then you won't see a lot of conflicts  
> during the rebase, either.

Yeah. I'll try to mention this at an appropriate place.

	Steffen

^ permalink raw reply

* [PATCH] Remove deprecated commands from command list and update manpages
From: Jonas Fonseca @ 2007-11-08 14:54 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0711080041120.4362@racer.site>

... and remove manpages of commands that no longer exists.

Signed-off-by: Jonas Fonseca <fonseca@diku.dk>
---
 Documentation/cmd-list.perl       |    5 ---
 Documentation/git-local-fetch.txt |   66 -------------------------------------
 Documentation/git-ssh-fetch.txt   |   52 -----------------------------
 Documentation/git-ssh-upload.txt  |   48 ---------------------------
 Documentation/git-tar-tree.txt    |    2 +-
 Documentation/git-var.txt         |    3 +-
 6 files changed, 3 insertions(+), 173 deletions(-)
 delete mode 100644 Documentation/git-local-fetch.txt
 delete mode 100644 Documentation/git-ssh-fetch.txt
 delete mode 100644 Documentation/git-ssh-upload.txt

 Although this new version updates the git-lost-found(1) the command is
 still listed in git(7) which I find misleading. Anyway, it made me
 investigate the commands we list in git(7) and I found a few that could
 possibly be cleaned up.

diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl
index 8d21d42..7f1f5d2 100755
--- a/Documentation/cmd-list.perl
+++ b/Documentation/cmd-list.perl
@@ -124,9 +124,7 @@ git-index-pack                          plumbingmanipulators
 git-init                                mainporcelain
 git-instaweb                            ancillaryinterrogators
 gitk                                    mainporcelain
-git-local-fetch                         synchingrepositories
 git-log                                 mainporcelain
-git-lost-found                          ancillarymanipulators
 git-ls-files                            plumbinginterrogators
 git-ls-remote                           plumbinginterrogators
 git-ls-tree                             plumbinginterrogators
@@ -178,8 +176,6 @@ git-show-branch                         ancillaryinterrogators
 git-show-index                          plumbinginterrogators
 git-show-ref                            plumbinginterrogators
 git-sh-setup                            purehelpers
-git-ssh-fetch                           synchingrepositories
-git-ssh-upload                          synchingrepositories
 git-stash                               mainporcelain
 git-status                              mainporcelain
 git-stripspace                          purehelpers
@@ -187,7 +183,6 @@ git-submodule                           mainporcelain
 git-svn                                 foreignscminterface
 git-symbolic-ref                        plumbingmanipulators
 git-tag                                 mainporcelain
-git-tar-tree                            plumbinginterrogators
 git-unpack-file                         plumbinginterrogators
 git-unpack-objects                      plumbingmanipulators
 git-update-index                        plumbingmanipulators
diff --git a/Documentation/git-local-fetch.txt b/Documentation/git-local-fetch.txt
deleted file mode 100644
index e830dee..0000000
--- a/Documentation/git-local-fetch.txt
+++ /dev/null
@@ -1,66 +0,0 @@
-git-local-fetch(1)
-==================
-
-NAME
-----
-git-local-fetch - Duplicate another git repository on a local system
-
-
-SYNOPSIS
---------
-[verse]
-'git-local-fetch' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] [-l] [-s] [-n]
-                  commit-id path
-
-DESCRIPTION
------------
-THIS COMMAND IS DEPRECATED.
-
-Duplicates another git repository on a local system.
-
-OPTIONS
--------
--c::
-	Get the commit objects.
--t::
-	Get trees associated with the commit objects.
--a::
-	Get all the objects.
--v::
-	Report what is downloaded.
--s::
-	Instead of regular file-to-file copying use symbolic links to the objects
-	in the remote repository.
--l::
-	Before attempting symlinks (if -s is specified) or file-to-file copying the
-	remote objects, try to hardlink the remote objects into the local
-	repository.
--n::
-	Never attempt to file-to-file copy remote objects.  Only useful with
-	-s or -l command-line options.
-
--w <filename>::
-        Writes the commit-id into the filename under $GIT_DIR/refs/<filename> on
-        the local end after the transfer is complete.
-
---stdin::
-	Instead of a commit id on the command line (which is not expected in this
-	case), 'git-local-fetch' expects lines on stdin in the format
-
-		<commit-id>['\t'<filename-as-in--w>]
-
---recover::
-	Verify that everything reachable from target is fetched.  Used after
-	an earlier fetch is interrupted.
-
-Author
-------
-Written by Junio C Hamano <junkio@cox.net>
-
-Documentation
---------------
-Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
-
-GIT
----
-Part of the gitlink:git[7] suite
diff --git a/Documentation/git-ssh-fetch.txt b/Documentation/git-ssh-fetch.txt
deleted file mode 100644
index 8d3e2ff..0000000
--- a/Documentation/git-ssh-fetch.txt
+++ /dev/null
@@ -1,52 +0,0 @@
-git-ssh-fetch(1)
-================
-
-NAME
-----
-git-ssh-fetch - Fetch from a remote repository over ssh connection
-
-
-
-SYNOPSIS
---------
-'git-ssh-fetch' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] commit-id url
-
-DESCRIPTION
------------
-THIS COMMAND IS DEPRECATED.
-
-Pulls from a remote repository over ssh connection, invoking
-git-ssh-upload on the other end. It functions identically to
-git-ssh-upload, aside from which end you run it on.
-
-
-OPTIONS
--------
-commit-id::
-        Either the hash or the filename under [URL]/refs/ to
-        pull.
-
--c::
-	Get the commit objects.
--t::
-	Get trees associated with the commit objects.
--a::
-	Get all the objects.
--v::
-	Report what is downloaded.
--w::
-        Writes the commit-id into the filename under $GIT_DIR/refs/ on
-        the local end after the transfer is complete.
-
-
-Author
-------
-Written by Daniel Barkalow <barkalow@iabervon.org>
-
-Documentation
---------------
-Documentation by David Greaves, Junio C Hamano and the git-list <git@vger.kernel.org>.
-
-GIT
----
-Part of the gitlink:git[7] suite
diff --git a/Documentation/git-ssh-upload.txt b/Documentation/git-ssh-upload.txt
deleted file mode 100644
index 5e2ca8d..0000000
--- a/Documentation/git-ssh-upload.txt
+++ /dev/null
@@ -1,48 +0,0 @@
-git-ssh-upload(1)
-=================
-
-NAME
-----
-git-ssh-upload - Push to a remote repository over ssh connection
-
-
-SYNOPSIS
---------
-'git-ssh-upload' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] commit-id url
-
-DESCRIPTION
------------
-THIS COMMAND IS DEPRECATED.
-
-Pushes from a remote repository over ssh connection, invoking
-git-ssh-fetch on the other end. It functions identically to
-git-ssh-fetch, aside from which end you run it on.
-
-OPTIONS
--------
-commit-id::
-        Id of commit to push.
-
--c::
-        Get the commit objects.
--t::
-        Get tree associated with the requested commit object.
--a::
-        Get all the objects.
--v::
-        Report what is uploaded.
--w::
-        Writes the commit-id into the filename under [URL]/refs/ on
-        the remote end after the transfer is complete.
-
-Author
-------
-Written by Daniel Barkalow <barkalow@iabervon.org>
-
-Documentation
---------------
-Documentation by Daniel Barkalow
-
-GIT
----
-Part of the gitlink:git[7] suite
diff --git a/Documentation/git-tar-tree.txt b/Documentation/git-tar-tree.txt
index 434607b..4aaf813 100644
--- a/Documentation/git-tar-tree.txt
+++ b/Documentation/git-tar-tree.txt
@@ -12,7 +12,7 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-THIS COMMAND IS DEPRECATED.  Use `git-archive` with `--format=tar`
+THIS COMMAND IS DEPRECATED.  Use gitlink:git-archive[1] with `--format=tar`
 option instead (and move the <base> argument to `--prefix=base/`).
 
 Creates a tar archive containing the tree structure for the named tree.
diff --git a/Documentation/git-var.txt b/Documentation/git-var.txt
index 8139423..73c37b0 100644
--- a/Documentation/git-var.txt
+++ b/Documentation/git-var.txt
@@ -20,7 +20,8 @@ OPTIONS
 	Cause the logical variables to be listed. In addition, all the
 	variables of the git configuration file .git/config are listed
 	as well. (However, the configuration variables listing functionality
-	is deprecated in favor of `git-config -l`.)
+	is deprecated. Use gitlink:git-config[1] with the option '-l'
+	instead.)
 
 EXAMPLE
 --------
-- 
1.5.3.5.1623.g4aab495-dirty

-- 
Jonas Fonseca

^ permalink raw reply related

* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Benoit Sigoure @ 2007-11-08 14:51 UTC (permalink / raw)
  To: Steffen Prohaska
  Cc: Johannes Sixt, Andreas Ericsson, Ralf Wildenhues,
	Git Mailing List
In-Reply-To: <97F64156-A457-4BC1-84BE-108369FFD18C@zib.de>

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

On Nov 8, 2007, at 1:54 PM, Steffen Prohaska wrote:

> Do you use rebase like this in real life?
>
> I thought of the text as background information that might
> be helpful for users who want do decide wether to merge or
> to rebase. The problem described may be valuable information
> supporting a decision about a recommended workflow for a group
> of users.

You're missing the point.  Johannes suggested that you rebase *only*  
for bisecting purpose.  Once you find the culprit commit, throw away  
your rebased stuff.  I've never thought about doing this myself, but  
it's a very clever way of tackling this problem.  It's slightly less  
convenient if you need to bisect a large portion of the history (that  
involves many branches and merges) because in this case we'd like to  
have a magic git-linearize-history <start-treeish> <end-treeish>.   
Unless this is already easily doable with git-rebase?

So to summarize (untested):
   git merge topic
   make check
   <omg something's broken>
   git reset --hard HEAD~1 # undo the merge
   git checkout -b wtf
   git rebase topic master
   git bisect ...
   <OK I found the culprit>
   git checkout master
   git branch -D wtf
   git merge --no-commit topic
   <fix the problem>
   git commit # merge done, semantic glitch fixed

Correct me if I'm wrong.

This has *nothing* to do with the fact that you use merge or rebase  
to do whatever you were doing.

-- 
Benoit Sigoure aka Tsuna
EPITA Research and Development Laboratory



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 186 bytes --]

^ permalink raw reply

* Re: [PATCH 4/3] t3700: avoid racy git situation
From: Johannes Sixt @ 2007-11-08 14:34 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711081414160.4362@racer.site>

Johannes Schindelin schrieb:
> Wow, the builtin commit is fast.  It sometimes triggers a racy
> situation in the test case for "git add --refresh -- foo".
> 
> So when that test fails, simply sleep one second and try again.

[/me looks at the calender - no, it's not April Fool's day]

Wouldn't it be better to fix git-commit (or git-add)? I would like to help, 
but you already seem to have done the analysis, so...

-- Hannes

^ permalink raw reply

* Re: [NEW REPLACEMENT PATCH] git-checkout: Add a test case for relative paths use.
From: Johannes Schindelin @ 2007-11-08 14:32 UTC (permalink / raw)
  To: David Symonds; +Cc: Junio C Hamano, git, Andreas Ericsson
In-Reply-To: <11945276321726-git-send-email-dsymonds@gmail.com>

Hi,

just a few nitpicks:

On Fri, 9 Nov 2007, David Symonds wrote:

> +test_expect_success setup '
> +
> +	echo base > file0 &&
> +	git add file0 &&
> +	mkdir dir1 &&
> +	echo hello > dir1/file1 &&
> +	git add dir1/file1 &&
> +	test_tick &&

please move the test_tick directly in front of the commit.  Readers might 
assume that it has an effect on mkdir otherwise.

> +	mkdir dir2 &&
> +	echo bonjour > dir2/file2 &&
> +	git add dir2/file2 &&
> +	git commit -m "populate tree"
> +
> +'

Please lose the empty line before the closing quote.  (This applies to all 
tests.)

> +test_expect_success 'remove and restore with relative path' '
> +
> +	cd dir1 &&
> +	rm ../file0 &&
> +	git checkout HEAD -- ../file0 && test -f ../file0 &&
> +	rm ../dir2/file2 &&
> +	git checkout HEAD -- ../dir2/file2 && test -f ../dir2/file2 &&
> +	rm ../file0 ./file1 &&
> +	git checkout HEAD -- .. && test -f ../file0 && test -f ./file1 &&
> +	rm file1 &&
> +	git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1
> +
> +'
> +
> +test_expect_failure 'checkout with relative path outside tree should fail (1)' \
> +	'git checkout HEAD -- ../file0'

Maybe do that with an existing file?  Since the test script lives in t/, 
and the test is run in t/trash/, we can test for "../Makefile".

Also, I would shorten the message to "relative path outside tree should 
fail".

> +test_expect_failure 'checkout with relative path outside tree should fail (2)' \
> +	'cd dir1 && git checkout HEAD -- ./file0'

I am not convinced that this should fail.

> +test_expect_failure 'checkout with relative path outside tree should fail (2)' \
> +	'cd dir1 && git checkout HEAD -- ../../file0'

Please add some other test like

test_expect_success 'checkout with empty prefix' '
	rm file0 &&
	git checkout HEAD -- file0 &&
	test base = "$(cat file0)"
'

Thanks,
Dscho

^ permalink raw reply

* Re: [PATCH] git-checkout: Handle relative paths containing "..".
From: Johannes Schindelin @ 2007-11-08 14:25 UTC (permalink / raw)
  To: David Symonds; +Cc: Junio C Hamano, git
In-Reply-To: <ee77f5c20711080510i138729b9vb0b9ad485cb2db3@mail.gmail.com>

Hi,

On Fri, 9 Nov 2007, David Symonds wrote:

> On Nov 8, 2007 7:30 PM, Junio C Hamano <junio@pobox.com> wrote:
> >
> > Have you tested this patch from the toplevel of any tree, where
> > "git-rev-parse --show-cdup" would yield an empty string?
> 
> No, I didn't. Arguably, "git-rev-parse --show-cdup" should always return 
> a path to the top-level, which would make this kind of construction much 
> simpler.

As it is, we have a convenience function for this, to make it much 
simpler: cd_to_toplevel in git-sh-setup.

> > I also wonder how this patch (with an obvious fix to address the above 
> > point) would interact with GIT_DIR and/or GIT_WORK_TREE in the 
> > environment.
> 
> No idea. I'm still learning my way around the git codebase, so I was
> hoping for some review and feedback from more experienced Gits.

It _should_ work with GIT_DIR/GIT_WORK_TREE, as the full name is relative 
to the project root, and that's exactly where cd_to_toplevel is jumping 
to.  And the first call to ls-files should make certain that the paths are 
correct, so there should be no confusion either.

But yes, I have been burnt by that work tree stuff too many times, so I'd 
appreciate tests for that (both positive and negative ones).

Ciao,
Dscho

^ permalink raw reply

* [PATCH 2/2] git-push: plumb in --mirror mode
From: Andy Whitcroft @ 2007-11-08 14:25 UTC (permalink / raw)
  To: git
In-Reply-To: <20071108121136.GG9736@shadowen.org>


Plumb in the --mirror mode for git-push.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 builtin-push.c |   14 ++++++++++++--
 transport.c    |    7 +++++++
 transport.h    |    1 +
 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/builtin-push.c b/builtin-push.c
index 2c56195..d49157c 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -10,7 +10,7 @@
 #include "parse-options.h"
 
 static const char * const push_usage[] = {
-	"git-push [--all] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]",
+	"git-push [--all | --mirror] [--dry-run] [--tags] [--receive-pack=<git-receive-pack>] [--repo=all] [-f | --force] [-v] [<repository> <refspec>...]",
 	NULL,
 };
 
@@ -91,6 +91,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 {
 	int flags = 0;
 	int all = 0;
+	int mirror = 0;
 	int dry_run = 0;
 	int force = 0;
 	int tags = 0;
@@ -100,6 +101,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		OPT__VERBOSE(&verbose),
 		OPT_STRING( 0 , "repo", &repo, "repository", "repository"),
 		OPT_BOOLEAN( 0 , "all", &all, "push all refs"),
+		OPT_BOOLEAN( 0 , "mirror", &mirror, "mirror all refs"),
 		OPT_BOOLEAN( 0 , "tags", &tags, "push tags"),
 		OPT_BOOLEAN( 0 , "dry-run", &dry_run, "dry run"),
 		OPT_BOOLEAN('f', "force", &force, "force updates"),
@@ -119,13 +121,21 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 		add_refspec("refs/tags/*");
 	if (all)
 		flags |= TRANSPORT_PUSH_ALL;
+	if (mirror)
+		flags |= (TRANSPORT_PUSH_MIRROR|TRANSPORT_PUSH_FORCE);
 
 	if (argc > 0) {
 		repo = argv[0];
 		set_refspecs(argv + 1, argc - 1);
 	}
-	if ((flags & TRANSPORT_PUSH_ALL) && refspec)
+	if ((flags & (TRANSPORT_PUSH_ALL|TRANSPORT_PUSH_MIRROR)) && refspec)
 		usage_with_options(push_usage, options);
 
+	if ((flags & (TRANSPORT_PUSH_ALL|TRANSPORT_PUSH_MIRROR)) ==
+				(TRANSPORT_PUSH_ALL|TRANSPORT_PUSH_MIRROR)) {
+		error("--all and --mirror are incompatible");
+		usage_with_options(push_usage, options);
+	}
+
 	return do_push(repo, flags);
 }
diff --git a/transport.c b/transport.c
index f4577b7..08e62b1 100644
--- a/transport.c
+++ b/transport.c
@@ -284,6 +284,9 @@ static int rsync_transport_push(struct transport *transport,
 	struct child_process rsync;
 	const char *args[10];
 
+	if (flags & TRANSPORT_PUSH_MIRROR)
+		return error("rsync transport does not support mirror mode");
+
 	/* first push the objects */
 
 	strbuf_addstr(&buf, transport->url);
@@ -386,6 +389,9 @@ static int curl_transport_push(struct transport *transport, int refspec_nr, cons
 	int argc;
 	int err;
 
+	if (flags & TRANSPORT_PUSH_MIRROR)
+		return error("http transport does not support mirror mode");
+
 	argv = xmalloc((refspec_nr + 11) * sizeof(char *));
 	argv[0] = "http-push";
 	argc = 1;
@@ -653,6 +659,7 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const
 
 	args.receivepack = data->receivepack;
 	args.send_all = !!(flags & TRANSPORT_PUSH_ALL);
+	args.send_mirror = !!(flags & TRANSPORT_PUSH_MIRROR);
 	args.force_update = !!(flags & TRANSPORT_PUSH_FORCE);
 	args.use_thin_pack = data->thin;
 	args.verbose = transport->verbose;
diff --git a/transport.h b/transport.h
index d27f562..7f337d2 100644
--- a/transport.h
+++ b/transport.h
@@ -30,6 +30,7 @@ struct transport {
 #define TRANSPORT_PUSH_ALL 1
 #define TRANSPORT_PUSH_FORCE 2
 #define TRANSPORT_PUSH_DRY_RUN 4
+#define TRANSPORT_PUSH_MIRROR 8
 
 /* Returns a transport suitable for the url */
 struct transport *transport_get(struct remote *, const char *);

^ permalink raw reply related

* [PATCH 1/2] mirror pushing
From: Andy Whitcroft @ 2007-11-08 14:24 UTC (permalink / raw)
  To: git
In-Reply-To: <20071108121136.GG9736@shadowen.org>


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

Existing "git push --all" is almost perfect for backing up to
another repository, except that "--all" only means "all
branches" in modern git, and it does not delete old branches and
tags that exist at the back-up repository that you have removed
from your local repository.

This teaches "git-send-pack" a new "--mirror" option.  The
difference from the "--all" option are that (1) it sends all
refs, not just branches, and (2) it deletes old refs you no
longer have on the local side from the remote side.

[apw@shadowen.org: rebase to next post arguments update]
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
 builtin-send-pack.c |   40 ++++++++++++++++++++++++++++------------
 remote.c            |   15 ++++++++++-----
 send-pack.h         |    1 +
 3 files changed, 39 insertions(+), 17 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 5a0f5c6..d5ead97 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -8,7 +8,7 @@
 #include "send-pack.h"
 
 static const char send_pack_usage[] =
-"git-send-pack [--all] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
+"git-send-pack [--all | --mirror] [--dry-run] [--force] [--receive-pack=<git-receive-pack>] [--verbose] [--thin] [<host>:]<directory> [<ref>...]\n"
 "  --all and explicit <ref> specification are mutually exclusive.";
 
 static struct send_pack_args args = {
@@ -242,7 +242,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 	if (!remote_tail)
 		remote_tail = &remote_refs;
 	if (match_refs(local_refs, remote_refs, &remote_tail,
-		       nr_refspec, refspec, args.send_all))
+		       nr_refspec, refspec, args.send_all | (args.send_mirror << 1)))
 		return -1;
 
 	if (!remote_refs) {
@@ -259,20 +259,28 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 		char old_hex[60], *new_hex;
 		int will_delete_ref;
 		const char *pretty_ref;
-		const char *pretty_peer;
+		const char *pretty_peer = NULL; /* only used when not deleting */
+		const unsigned char *new_sha1;
 
-		if (!ref->peer_ref)
-			continue;
+		if (!ref->peer_ref) {
+			if (!args.send_mirror)
+				continue;
+			new_sha1 = null_sha1;
+		}
+		else
+			new_sha1 = ref->peer_ref->new_sha1;
 
 		if (!shown_dest) {
 			fprintf(stderr, "To %s\n", dest);
 			shown_dest = 1;
 		}
 
+		will_delete_ref = is_null_sha1(new_sha1);
+
 		pretty_ref = prettify_ref(ref->name);
-		pretty_peer = prettify_ref(ref->peer_ref->name);
+		if (!will_delete_ref)
+			pretty_peer = prettify_ref(ref->peer_ref->name);
 
-		will_delete_ref = is_null_sha1(ref->peer_ref->new_sha1);
 		if (will_delete_ref && !allow_deleting_refs) {
 			fprintf(stderr, " ! %-*s %s (remote does not support deleting refs)\n",
 					SUMMARY_WIDTH, "[rejected]", pretty_ref);
@@ -280,7 +288,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 			continue;
 		}
 		if (!will_delete_ref &&
-		    !hashcmp(ref->old_sha1, ref->peer_ref->new_sha1)) {
+		    !hashcmp(ref->old_sha1, new_sha1)) {
 			if (args.verbose)
 				fprintf(stderr, " = %-*s %s -> %s\n",
 					SUMMARY_WIDTH, "[up to date]",
@@ -312,8 +320,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 		    !is_null_sha1(ref->old_sha1) &&
 		    !ref->force) {
 			if (!has_sha1_file(ref->old_sha1) ||
-			    !ref_newer(ref->peer_ref->new_sha1,
-				       ref->old_sha1)) {
+			    !ref_newer(new_sha1, ref->old_sha1)) {
 				/* We do not have the remote ref, or
 				 * we know that the remote ref is not
 				 * an ancestor of what we are trying to
@@ -328,7 +335,7 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
 				continue;
 			}
 		}
-		hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
+		hashcpy(ref->new_sha1, new_sha1);
 		if (!will_delete_ref)
 			new_refs++;
 		strcpy(old_hex, sha1_to_hex(ref->old_sha1));
@@ -459,6 +466,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 				args.dry_run = 1;
 				continue;
 			}
+			if (!strcmp(arg, "--mirror")) {
+				args.send_mirror = 1;
+				continue;
+			}
 			if (!strcmp(arg, "--force")) {
 				args.force_update = 1;
 				continue;
@@ -483,7 +494,12 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	}
 	if (!dest)
 		usage(send_pack_usage);
-	if (heads && args.send_all)
+	/*
+	 * --all and --mirror are incompatible; neither makes sense
+	 * with any refspecs.
+	 */
+	if ((heads && (args.send_all || args.send_mirror)) ||
+					(args.send_all && args.send_mirror))
 		usage(send_pack_usage);
 
 	if (remote_name) {
diff --git a/remote.c b/remote.c
index 59defdb..45dd59b 100644
--- a/remote.c
+++ b/remote.c
@@ -722,10 +722,12 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
  * without thinking.
  */
 int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
-	       int nr_refspec, const char **refspec, int all)
+	       int nr_refspec, const char **refspec, int flags)
 {
 	struct refspec *rs =
 		parse_ref_spec(nr_refspec, (const char **) refspec);
+	int send_all = flags & 01;
+	int send_mirror = flags & 02;
 
 	if (match_explicit_refs(src, dst, dst_tail, rs, nr_refspec))
 		return -1;
@@ -742,7 +744,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 			if (!pat)
 				continue;
 		}
-		else if (prefixcmp(src->name, "refs/heads/"))
+		else if (!send_mirror && prefixcmp(src->name, "refs/heads/"))
 			/*
 			 * "matching refs"; traditionally we pushed everything
 			 * including refs outside refs/heads/ hierarchy, but
@@ -763,10 +765,13 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 		if (dst_peer && dst_peer->peer_ref)
 			/* We're already sending something to this ref. */
 			goto free_name;
-		if (!dst_peer && !nr_refspec && !all)
-			/* Remote doesn't have it, and we have no
+
+		if (!dst_peer && !nr_refspec && !(send_all || send_mirror))
+			/*
+			 * Remote doesn't have it, and we have no
 			 * explicit pattern, and we don't have
-			 * --all. */
+			 * --all nor --mirror.
+			 */
 			goto free_name;
 		if (!dst_peer) {
 			/* Create a new one and link it */
diff --git a/send-pack.h b/send-pack.h
index 7a24f71..8ff1dc3 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -5,6 +5,7 @@ struct send_pack_args {
 	const char *receivepack;
 	unsigned verbose:1,
 		send_all:1,
+		send_mirror:1,
 		force_update:1,
 		use_thin_pack:1,
 		dry_run:1;

^ permalink raw reply related

* [PATCH 4/3] t3700: avoid racy git situation
From: Johannes Schindelin @ 2007-11-08 14:14 UTC (permalink / raw)
  To: git, krh, gitster
In-Reply-To: <Pine.LNX.4.64.0711081213580.4362@racer.site>


Wow, the builtin commit is fast.  It sometimes triggers a racy
situation in the test case for "git add --refresh -- foo".

So when that test fails, simply sleep one second and try again.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t3700-add.sh |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index a328bf5..f4950a3 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -152,7 +152,12 @@ test_expect_success 'git add --refresh' '
 	*) echo fail; (exit 1);;
 	esac &&
 	git add --refresh -- foo &&
-	test -z "`git diff-index HEAD -- foo`"
+	test -z "`git diff-index HEAD -- foo`" || {
+		echo Sleeping one second to avoid racy situation &&
+		sleep 1 &&
+		git add --refresh -- foo &&
+		test -z "`git diff-index HEAD -- foo`"
+	}
 '
 
 test_done
-- 
1.5.3.5.1634.g0fa78

^ permalink raw reply related

* [PATCH REPLACING 2/3] launch_editor(): read the file, even when EDITOR=:
From: Johannes Schindelin @ 2007-11-08 14:06 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, krh, gitster
In-Reply-To: <47330222.7000206@viscovery.net>


Earlier we just returned in case EDITOR=: but the message stored
in the file was not read back.  Fix this, at the same time
simplifying the code as suggested by Johannes Sixt.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-tag.c |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/builtin-tag.c b/builtin-tag.c
index c3b76da..86d8121 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -20,8 +20,6 @@ static char signingkey[1000];
 void launch_editor(const char *path, struct strbuf *buffer)
 {
 	const char *editor, *terminal;
-	struct child_process child;
-	const char *args[3];
 
 	editor = getenv("GIT_EDITOR");
 	if (!editor && editor_program)
@@ -42,17 +40,12 @@ void launch_editor(const char *path, struct strbuf *buffer)
 	if (!editor)
 		editor = "vi";
 
-	if (!strcmp(editor, ":"))
-		return;
+	if (strcmp(editor, ":")) {
+		const char *args[] = { editor, path, NULL };
 
-	memset(&child, 0, sizeof(child));
-	child.argv = args;
-	args[0] = editor;
-	args[1] = path;
-	args[2] = NULL;
-
-	if (run_command(&child))
-		die("There was a problem with the editor %s.", editor);
+		if (run_command_v_opt(args, 0))
+			die("There was a problem with the editor %s.", editor);
+	}
 
 	if (strbuf_read_file(buffer, path, 0) < 0)
 		die("could not read message file '%s': %s",
-- 
1.5.3.5.1634.g0fa78

^ permalink raw reply related

* [PATCH] hooks--update: fix test for properly set up project description file
From: Gerrit Pape @ 2007-11-08 14:02 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: git, Junio C Hamano
In-Reply-To: <4730726B.1000407@op5.se>

The update hook template intends to abort if the project description file
hasn't been adjusted or is empty.  This patch fixes the check for 'being
adjusted'.

Signed-off-by: Gerrit Pape <pape@smarden.org>
---

On Tue, Nov 06, 2007 at 02:55:55PM +0100, Andreas Ericsson wrote:
> Gerrit Pape wrote:
> > # check for no description
> >-projectdesc=$(sed -e '1p' "$GIT_DIR/description")
> >-if [ -z "$projectdesc" -o "$projectdesc" = "Unnamed repository; edit
> >this
> >file to name it for gitweb" ]; then
> >+projectdesc=$(sed -ne '1p' "$GIT_DIR/description")
>
> Write this as
>       projectdesc=$(sed -e 1q "$GIT_DIR/description")
> instead. It's a little shorter, a little faster and slightly more
> portable.

I made it $(head -n 1 ...).


 templates/hooks--update |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/templates/hooks--update b/templates/hooks--update
index d8c7626..fbf664f 100644
--- a/templates/hooks--update
+++ b/templates/hooks--update
@@ -34,8 +34,8 @@ fi
 allowunannotated=$(git-repo-config --bool hooks.allowunannotated)
 
 # check for no description
-projectdesc=$(sed -e '1p' "$GIT_DIR/description")
-if [ -z "$projectdesc" -o "$projectdesc" = "Unnamed repository; edit this file to name it for gitweb" ]; then
+projectdesc=$(head -n 1 "$GIT_DIR/description")
+if [ -z "$projectdesc" -o "$projectdesc" = "Unnamed repository; edit this file to name it for gitweb." ]; then
 	echo "*** Project description file hasn't been set" >&2
 	exit 1
 fi
-- 
1.5.3.5

^ permalink raw reply related

* Re: git push mirror mode
From: Andreas Ericsson @ 2007-11-08 13:48 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: Johannes Schindelin, git, Junio C Hamano
In-Reply-To: <20071108134432.GK9736@shadowen.org>

Andy Whitcroft wrote:
> On Thu, Nov 08, 2007 at 02:14:12PM +0100, Andreas Ericsson wrote:
> 
>> Barring any errors in my understanding of the matter, here's how it
>> works for git.
>>
>> git separates author from committer, so code attribution is done with
>> author, and "I verified this is sane" is done by committer. Those two
>> usually only ever differ when the user tells git commit that the author
>> was someone else than him/her self, or when rewriting history with git
>> rebase or similar. git am also maintains authorship (using the From:
>> line in emails), but sets $committer to the person running it, so when
>> you apply patches sent by email from someone else you get the code
>> attribution right by default.
>>
>> The Signed-off-by line is, in git, used as "I touched the code here and
>> agree that it may be included in the mothership repo and all future
>> releases" (the spirit of that sentence is also in
>> Documentation/SubmittingPatches).
>>
>> We also have Acked-by (as does the kernel, no? I think we inherited it
>> from there) to mean something along the lines of "I vote we include this",
>> but not always based on technical merit (ie, patches can have many acks
>> without having ever been tested).
>>
>> Suggested-by, Tested-by and Reported-by are used less often, not always
>> written in dash-form, but hopefully always self-explanatory ;-)
> 
> What that doesn't tell me is how when sending an email carrying a patch
> one ensures the attribution is correct when loaded into git.
> 

Ach damn. I had a sentence there reading "From: can also be specified in
the email body to attribute code to someone else than the sender." It's
in my clipboard, but I forgot to paste it :-/


> Having messed about with it a bit it does seem that if one wants git to
> attribute the patch to junio I have to add a From: line to the top of
> the email payload.
> 
> I'll resend so attributed.
> 

Thanks.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: git push mirror mode
From: Andy Whitcroft @ 2007-11-08 13:44 UTC (permalink / raw)
  To: Andreas Ericsson; +Cc: Johannes Schindelin, git, Junio C Hamano
In-Reply-To: <47330BA4.6030101@op5.se>

On Thu, Nov 08, 2007 at 02:14:12PM +0100, Andreas Ericsson wrote:

> Barring any errors in my understanding of the matter, here's how it
> works for git.
> 
> git separates author from committer, so code attribution is done with
> author, and "I verified this is sane" is done by committer. Those two
> usually only ever differ when the user tells git commit that the author
> was someone else than him/her self, or when rewriting history with git
> rebase or similar. git am also maintains authorship (using the From:
> line in emails), but sets $committer to the person running it, so when
> you apply patches sent by email from someone else you get the code
> attribution right by default.
> 
> The Signed-off-by line is, in git, used as "I touched the code here and
> agree that it may be included in the mothership repo and all future
> releases" (the spirit of that sentence is also in
> Documentation/SubmittingPatches).
> 
> We also have Acked-by (as does the kernel, no? I think we inherited it
> from there) to mean something along the lines of "I vote we include this",
> but not always based on technical merit (ie, patches can have many acks
> without having ever been tested).
> 
> Suggested-by, Tested-by and Reported-by are used less often, not always
> written in dash-form, but hopefully always self-explanatory ;-)

What that doesn't tell me is how when sending an email carrying a patch
one ensures the attribution is correct when loaded into git.

Having messed about with it a bit it does seem that if one wants git to
attribute the patch to junio I have to add a From: line to the top of
the email payload.

I'll resend so attributed.

-apw

^ permalink raw reply

* Re: Inconsistencies with git log
From: Andreas Ericsson @ 2007-11-08 13:40 UTC (permalink / raw)
  To: David Symonds
  Cc: Brian Gernhardt, Jon Smirl, Johannes Schindelin, Git Mailing List
In-Reply-To: <ee77f5c20711080516n4f207ba3pccc8efffa2a6ad4c@mail.gmail.com>

David Symonds wrote:
> On Nov 8, 2007 11:11 AM, Andreas Ericsson <ae@op5.se> wrote:
>> David Symonds wrote:
>>> On Nov 8, 2007 10:19 AM, Brian Gernhardt <benji@silverinsanity.com> wrote:
>>>> However, Dave's suggestion of altering git-status output to be
>>>> relative to (but not limited by) CWD has merit.  Too bad I don't have
>>>> time to work on it right now.
>>> I am happy to hack on this if there's not widespread revolt against the concept.
>>>
>> I'd definitely like that feature, but I wonder how many people will run
>> "git commit -a" in a subdir after seeing only what they want to see in the
>> output, and then accidentally committing junk somewhere else in the repo.
> 
> I never suggested path *limited*, only path *relative*. git-status
> would still show all the same files, but their paths would be relative
> to your current directory, so there'd be no confusion like you
> mentioned. This is how Johannes' patch works.
> 

Ah, that'd be a different matter entirely then. Thanks for clarifying. I
have no further objections then.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Andreas Ericsson @ 2007-11-08 13:38 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Johannes Sixt, gitster, Ralf.Wildenhues, tsuna, git
In-Reply-To: <97F64156-A457-4BC1-84BE-108369FFD18C@zib.de>

Steffen Prohaska wrote:
> 
> On Nov 8, 2007, at 10:53 AM, Johannes Sixt wrote:
> 
>> Andreas Ericsson schrieb:
>>> Johannes Sixt wrote:
>>>> Steffen Prohaska schrieb:
>>>>>
>>>>> On Nov 8, 2007, at 8:19 AM, Johannes Sixt wrote:
>>>>>
>>>>>> Steffen Prohaska schrieb:
>>>>>>> +If you linearize the history by rebasing the lower branch on
>>>>>>> +top of the upper, instead of merging, the bug becomes much 
>>>>>>> easier to
>>>>>>> +find and understand.  Your history would instead be:
>>>>>>
>>>>>> At this point I'm missing the words
>>>>>>
>>>>>>     The solution is ...
>>>>>>
>>>>>> I.e.:
>>>>>>
>>>>>> The solution is to linearize the history by rebasing the lower 
>>>>>> branch on
>>>>>> top of the upper, instead of merging. Now the bug becomes much 
>>>>>> easier to
>>>>>> find and understand.  Your history would instead be:
>>>>>
>>>>> Hmm. It might be a solution if you did not publish history.
>>>>
>>>> This is about finding the commit that introduced a bug. Once you 
>>>> found it, better: you know how to fix the bug, you are expected to 
>>>> throw away the rebased branch, not to publish it! Maybe a note along 
>>>> these lines could be appended:
>>>>
>>>> Now that you know what caused the error (and how to fix it), throw 
>>>> away the rebased branch, and commit a fix on top of D.
>>>>
>>> Well, if rebasing becomes the standard for normal development, it's 
>>> hardly
>>> right to throw it away, is it? I like Steffen's suggestion better.
>>
>> There is a big misunderstanding. The text that the patch amends is 
>> about bisecting history that reveals that a merge commit breaks, which 
>> is not helpful, and then how to find where and what and why the 
>> breakage really was introduce.
>>
>> And the answer to "how to find" is to rebase and bisect in the rebased 
>> history.
> 
> Do you use rebase like this in real life?
> 
> I thought of the text as background information that might
> be helpful for users who want do decide wether to merge or
> to rebase. The problem described may be valuable information
> supporting a decision about a recommended workflow for a group
> of users.
> 
> My personal conclusion was: I'll accept the danger of complex
> merges that might be hard to bisect. I now understand this
> risk, but I nonetheless prefer the simplicity of a merge
> based workflow. This avoids the danger that published history
> gets rewritten.
> 
> But now I'm wondering if your suggestions of rebasing only for
> locating the evil commit is feasible in reality. You may need
> to solve a lot of merge conflicts if you rebase a larger part
> of the history. If you do not have them in your rerere cache
> this might be time consuming. ...
> 

It is no great chore to put one merge-parent on top of another
and then re-run bisect on the result. git-bisect could even be
taught to do that by itself.

> 
>> My initial complaint was that in the flow of reading the instructions 
>> the pointer to "the solution" was missing. Rather, at the point where 
>> the reader is supposed to think "ah, yes, that's how to do it", there 
>> is the conditional statement "If you linearize history". My suggestion 
>> is to put a big emphasis on the solution by using the words "The 
>> solution is".
>>
>> Now, the user can *always* rebase one of the branches on top of the 
>> other, even if both histories are already published. *But* if both 
>> were indeed published, then the rebased history must be thrown away, 
>> and the only thing you learnt from it was where and what and why the 
>> breakage really was introduced.
>>
>> Of course we could include a few "ifs" and "unlesses" (about published 
>> histories), before suggesting to throw away rebased history. But once 
>> the task is accomplished (find the bogus commit), throwing away the 
>> rebased history (and continuing at commit D) is always correct, but 
>> keeping it (and continuing at D*) is not.
> 
> ... So, again, the question for me is if someone does use
> rebase in reality in the way that you suggests. Do you?


I don't, but if I'd thought a bit further I would have on at least one
occasion in the past. Instead I spent two days manually auditing every
commit of several branches.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* Re: [PATCH v2] user-manual: add advanced topic "bisecting merges"
From: Johannes Sixt @ 2007-11-08 13:22 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Andreas Ericsson, gitster, Ralf.Wildenhues, tsuna, git
In-Reply-To: <97F64156-A457-4BC1-84BE-108369FFD18C@zib.de>

Steffen Prohaska schrieb:
> 
> On Nov 8, 2007, at 10:53 AM, Johannes Sixt wrote:
>> The text that the patch amends is 
>> about bisecting history that reveals that a merge commit breaks, which 
>> is not helpful, and then how to find where and what and why the 
>> breakage really was introduce.
>>
>> And the answer to "how to find" is to rebase and bisect in the rebased 
>> history.
> 
> Do you use rebase like this in real life?

Why is this relevant?

You've written a superb addendum to the user manual, but IT TALKS ABOUT 
BISECTION, and is not a guideline when to use merges and when to rebase.

It better not be meant as such. Consider an integrator who has just merged 
two histories, both of which are available publically. Pushing out a rebased 
history IS NOT AN OPTION. If the poor fellow for the heck of it has no 
choice but to find the bogus commit, then your instructions are worth a 
thousand bucks - even if the rebased history is otherwise useless -, but any 
guidelines how to construct histories are IRRELEVANT for his case.

> But now I'm wondering if your suggestions of rebasing only for
> locating the evil commit is feasible in reality. You may need
> to solve a lot of merge conflicts if you rebase a larger part
> of the history. If you do not have them in your rerere cache
> this might be time consuming. ...

During the rebase you will see the same conflicts that you also had during 
the merge, even simpler ones (because they are - hopefully - broken down 
into smaller pieces). If your merge was clean (as was suggested in the 
patch), then you won't see a lot of conflicts during the rebase, either.

-- Hannes

^ permalink raw reply

* Re: Inconsistencies with git log
From: David Symonds @ 2007-11-08 13:16 UTC (permalink / raw)
  To: Andreas Ericsson
  Cc: Brian Gernhardt, Jon Smirl, Johannes Schindelin, Git Mailing List
In-Reply-To: <47325415.1070205@op5.se>

On Nov 8, 2007 11:11 AM, Andreas Ericsson <ae@op5.se> wrote:
>
> David Symonds wrote:
> > On Nov 8, 2007 10:19 AM, Brian Gernhardt <benji@silverinsanity.com> wrote:
> >> However, Dave's suggestion of altering git-status output to be
> >> relative to (but not limited by) CWD has merit.  Too bad I don't have
> >> time to work on it right now.
> >
> > I am happy to hack on this if there's not widespread revolt against the concept.
> >
>
> I'd definitely like that feature, but I wonder how many people will run
> "git commit -a" in a subdir after seeing only what they want to see in the
> output, and then accidentally committing junk somewhere else in the repo.

I never suggested path *limited*, only path *relative*. git-status
would still show all the same files, but their paths would be relative
to your current directory, so there'd be no confusion like you
mentioned. This is how Johannes' patch works.


Dave.

^ permalink raw reply

* Re: git push mirror mode
From: Andreas Ericsson @ 2007-11-08 13:14 UTC (permalink / raw)
  To: Andy Whitcroft; +Cc: Johannes Schindelin, git, Junio C Hamano
In-Reply-To: <20071108124435.GH9736@shadowen.org>

Andy Whitcroft wrote:
> On Thu, Nov 08, 2007 at 12:19:18PM +0000, Johannes Schindelin wrote:
>> Hi,
>>
>> On Thu, 8 Nov 2007, Andy Whitcroft wrote:
>>
>>> Ok, sometime back Junio sent out a proof-of-concept change to
>>> send-pack allowing a mirror mode.
>> You added/left his sign-off, but did not attribute the patches to him.  
>> Why?
> 
> I believe I left his signed off by from the original (first) patch, and
> added mine to indicate that what I had modified was also unecombered.
> The second patch is only signed off by me as I am the author.  In my
> world (admittedly a kernel hacker) the first Signed-off-by: indicates the
> primary authorship of that patch and the [apw@...] part tries to clarify
> the changes I made therein.
> 
> No intentional stripping of credit was intended, and I believe that the
> attribution as written states Junio is the originator of this patch.
> However that is the way I would read the meanings of these lines, if git
> has different rules or you think there is a clearer way of stating this
> I am happy to change it, and resend it so attributed.
> 

Barring any errors in my understanding of the matter, here's how it
works for git.

git separates author from committer, so code attribution is done with
author, and "I verified this is sane" is done by committer. Those two
usually only ever differ when the user tells git commit that the author
was someone else than him/her self, or when rewriting history with git
rebase or similar. git am also maintains authorship (using the From:
line in emails), but sets $committer to the person running it, so when
you apply patches sent by email from someone else you get the code
attribution right by default.

The Signed-off-by line is, in git, used as "I touched the code here and
agree that it may be included in the mothership repo and all future
releases" (the spirit of that sentence is also in
Documentation/SubmittingPatches).

We also have Acked-by (as does the kernel, no? I think we inherited it
from there) to mean something along the lines of "I vote we include this",
but not always based on technical merit (ie, patches can have many acks
without having ever been tested).

Suggested-by, Tested-by and Reported-by are used less often, not always
written in dash-form, but hopefully always self-explanatory ;-)

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

^ permalink raw reply

* [NEW REPLACEMENT PATCH] git-checkout: Add a test case for relative paths use.
From: David Symonds @ 2007-11-08 13:13 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Johannes Schindelin, Andreas Ericsson, David Symonds

Signed-off-by: David Symonds <dsymonds@gmail.com>
---
 t/t2008-checkout-subdir.sh |   47 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 47 insertions(+), 0 deletions(-)
 create mode 100755 t/t2008-checkout-subdir.sh

diff --git a/t/t2008-checkout-subdir.sh b/t/t2008-checkout-subdir.sh
new file mode 100755
index 0000000..45b9e13
--- /dev/null
+++ b/t/t2008-checkout-subdir.sh
@@ -0,0 +1,47 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 David Symonds
+
+test_description='git checkout from subdirectories'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+	echo base > file0 &&
+	git add file0 &&
+	mkdir dir1 &&
+	echo hello > dir1/file1 &&
+	git add dir1/file1 &&
+	test_tick &&
+	mkdir dir2 &&
+	echo bonjour > dir2/file2 &&
+	git add dir2/file2 &&
+	git commit -m "populate tree"
+
+'
+
+test_expect_success 'remove and restore with relative path' '
+
+	cd dir1 &&
+	rm ../file0 &&
+	git checkout HEAD -- ../file0 && test -f ../file0 &&
+	rm ../dir2/file2 &&
+	git checkout HEAD -- ../dir2/file2 && test -f ../dir2/file2 &&
+	rm ../file0 ./file1 &&
+	git checkout HEAD -- .. && test -f ../file0 && test -f ./file1 &&
+	rm file1 &&
+	git checkout HEAD -- ../dir1/../dir1/file1 && test -f ./file1
+
+'
+
+test_expect_failure 'checkout with relative path outside tree should fail (1)' \
+	'git checkout HEAD -- ../file0'
+
+test_expect_failure 'checkout with relative path outside tree should fail (2)' \
+	'cd dir1 && git checkout HEAD -- ./file0'
+
+test_expect_failure 'checkout with relative path outside tree should fail (2)' \
+	'cd dir1 && git checkout HEAD -- ../../file0'
+
+test_done
-- 
1.5.3.1

^ permalink raw reply related


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