date:20120711

On Tue, 2012-07-10 at 19:20 +0200, Matthieu Moy wrote:
 Carlos Martín Nieto c...@elego.de writes:
 
  --- a/builtin/branch.c
  +++ b/builtin/branch.c
  @@ -864,10 +864,32 @@ int cmd_branch(int argc, const char **argv, const 
  char *prefix)
 info and making sure new_upstream is correct */
  create_branch(head, branch-name, new_upstream, 0, 0, 0, quiet, 
  BRANCH_TRACK_OVERRIDE);
  } else if (argc  0  argc = 2) {
  +   struct branch *branch = branch_get(argv[0]);
  +   const char *old_upstream = NULL;
  +   int branch_existed = 0;
  +
  if (kinds != REF_LOCAL_BRANCH)
  die(_(-a and -r options to 'git branch' do not make 
  sense with a branch name));
  +
  +   /* Save what argv[0] was pointing to so we can give
  +  the --set-upstream-to hint */
 
 Multi-line comments are usually written in Git as
 
 /*
  * multi-line
  * comment
  */

I've seen this style often, but sure.

 
  +   if (branch_has_merge_config(branch))
  + old_upstream = shorten_unambiguous_ref(branch-merge[0]-dst, 
  0);
 
 Broken indentation.

Yeah, sorry. New laptop, hadn't got the default style fixed in the
config.

 
  +   if (argc == 1) {
  +   printf(If you wanted to make '%s' track '%s', do 
  this:\n, head, argv[0]);
 
 Could be marked for translation with _(...).

Done.

 
  +   if (branch_existed)
  +   printf( $ git branch --set-upstream '%s' 
  '%s'\n, argv[0], old_upstream);
 
 old_upstream may be NULL at this point. I guess you want to skip this
 line if old_upsteam is NULL.

We've just set up tracking for it, so we'd want to undo that. Which
means --unset-upstream would have to move earlier in the series so we
can suggest that.

 
 The fact that I could find this bug suggests that this lacks a few new
 tests too ;-).

Indeed :) the next round will have them.

 
  +   else
  +   printf( $ git branch -d '%s'\n, argv[0]);
  +
  +   printf( $ git branch --set-upstream-to '%s'\n, 
  argv[0]);
 
 For the 3 printf()s: we usually display commands without the $, and
 separate them from text with a blank line. See for example what git
 commit says when you didn't provide authorship:

Yeah, I was going by what Junio wrote in his mail. We should probably
have a double-LF as well, like in the message below.

 
 You can suppress this message by setting them explicitly:
 
 git config --global user.name Your Name
 git config --global user.email y...@example.com
 
 After doing this, you may fix the identity used for this commit with:
 
 git commit --amend --reset-author
 
 (the absence of $ sign avoids the temptation to cut-and-paste it)
 



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

Re: [PATCH 3/3] branch: add --unset-upstream option

On Tue, 2012-07-10 at 11:02 -0700, Junio C Hamano wrote:
 Carlos Martín Nieto c...@elego.de writes:
 
  We have ways of setting the upstream information, but if we want to
  unset it, we need to resort to modifying the configuration manually.
 
  Teach branch an --unset-upstream option that unsets this information.
 
  ---
 
  create_branch() uses install_branch_config() which may also set
  branch.foo.rebase, so this version might leave some configuration
  laying around.
 
  I wonder if deleting the whole branch.foo section would be better. Can
  we be sure that nothing else shows up there?
 
 branch.foo.$unknown may not be related to upstream at all, so that
 will not fly.  Besides, we already have unknown=description, no?

Ah yes, that exists. I've added a bit of code to also remove
branch.foo.rebase, which I'd also consider to be part of the upstream
information.

   cmn


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

Re: [PATCH 2/3] branch: suggest how to undo a --set-upstream when given one branch

On Tue, 2012-07-10 at 10:40 -0700, Junio C Hamano wrote:
 Carlos Martín Nieto c...@elego.de writes:
 
  This interface is error prone, and a better one (--set-upstream-to)
  exists. Suggest how to fix a --set-upstream invocation in case the
  user only gives one argument, which makes it likely that he meant to
  do the opposite, like with
 
  git branch --set-upstream origin/master
 
  when they meant one of
 
  git branch --set-upstream origin/master master
  git branch --set-upstream-to origin/master
 
  Signed-off-by: Carlos Martín Nieto c...@elego.de
 
 The new code does not seem to depend on the value of track (which
 is set by either -t or --set-upstream) in any way.  Shouldn't it be
 done only when it is set to track-override?

Yes, yes it should.

 
 Doesn't git branch [-f] frotz without any other argument trigger
 the warning?

It does. Oops. Fixed.

   cmn


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

Re: [PATCH 2/3] branch: suggest how to undo a --set-upstream when given one branch

On Tue, 2012-07-10 at 18:00 -0500, Jonathan Nieder wrote:
 Junio C Hamano wrote:
 
I think it
  is better to leave them emitted unconditionally to the standard
  error stream, in order to train users away from using the old option
  that has its arguments wrong (the option does not take an argument
  it should, and makes the command line to look as if it takes two
  branch arguments in the wrong order).
 
 I thought we already discussed that that is a side-issue?

The current --set-upstream is the whole reason for this series existing.

 
 The option is a mode option for the command, like -m, -d, or
 --edit-description.  I genuinely don't think the order of options it
 takes is counter-intuitive.  The second argument defaulting to HEAD
 and the behavior of creating the branch named by the first argument
 when it does not exist are quite counter-intuitive.

This is confusing. First you say that you don't think it's
counter-intuitive but then you say it is? Or is the first part about -m
and -d?

The second part of the paragraph is indeed what I'm trying to solve with
this series. If you want to create a new branch, you should be using -t.

 
 Transitioning to a different argument order seems like it would just
 make the command more complicated.  After the transition, there are
 two options to explain, and during the transition, it is easy to make
 scripts with gratuitous incompatibilities that won't work on older
 systems.
 
 Where is my thinking going wrong?

We're not transitioning to a new order as such, particularly not with
the same option name. The incompatibilities would ensue with the other
patch I send which did change the order for --set-upstream, but what
this does is _add_ --set-upstream-to=upstream such that the option
takes one argument and the command takes one optional argument, which
makes it closer to what one would expect, specially as changing the
upstream information is something you're most likely to do on the
current branch.

   cmn


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

[PATCH 3/6] Teach git remote about remote.default.

From: Marc Branchaud marcn...@xiplink.com

The rename and rm commands now handle the case where the remote being
changed is the default remote.

If the add command is used to add the repo's first remote, that remote
becomes the default remote.

Also introduce a default sub-command to get or set the default remote:

 - git remote default (with no parameter) displays the current default remote.

 - git remote default remo checks if remo is a configured remote and if
   so changes remote.default to remo.

These changes needed a couple of new helper functions in remote.c:
 - remote_get_default_name() returns default_remote_name.
 - remote_count() returns the number of remotes.

(The test in t5528 had to be changed because now with push.default=matching
a plain git push succeeds with Everything up-to-date.  It used to
fail with No configured push destination.)

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 Documentation/git-remote.txt | 32 ---
 builtin/remote.c | 29 +
 remote.c | 12 +++
 remote.h |  2 ++
 t/t5505-remote.sh| 76 
 t/t5512-ls-remote.sh |  8 -
 t/t5528-push-default.sh  |  4 +--
 7 files changed, 156 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
index a308f4c..f4de21d 100644
--- a/Documentation/git-remote.txt
+++ b/Documentation/git-remote.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 [verse]
 'git remote' [-v | --verbose]
 'git remote add' [-t branch] [-m master] [-f] [--tags|--no-tags] 
[--mirror=fetch|push] name url
+'git remote default' [name]
 'git remote rename' old new
 'git remote rm' name
 'git remote set-head' name (-a | -d | branch)
@@ -75,20 +76,43 @@ because a fetch would overwrite any local commits.
 +
 When a push mirror is created with `--mirror=push`, then `git push`
 will always behave as if `--mirror` was passed.
++
+When the first remote is added to a repository, it becomes the repository's
+default remote.
+
+'default'::
+
+Displays or sets the name of the repository's default remote.  When git needs
+to automatically choose a remote to use, it first tries to use the remote
+associated with the currently checked-out branch.  If the currently
+checked-out branch has no remote, git uses the repository's default remote.
+If the repository has no default remote, git tries to use a remote named
+origin even if there is no actual remote named origin.  (In other words,
+the default value for the default remote name is origin.)
++
+With no name parameter, displays the current default remote name.
++
+With the name parameter, sets the repository's default remote to name.
 
 'rename'::
 
-Rename the remote named old to new. All remote-tracking branches and
+Renames the remote named old to new. All remote-tracking branches and
 configuration settings for the remote are updated.
 +
 In case old and new are the same, and old is a file under
 `$GIT_DIR/remotes` or `$GIT_DIR/branches`, the remote is converted to
 the configuration file format.
++
+If old was the repository's default remote name, the default remote name
+changes to new.
 
 'rm'::
 
-Remove the remote named name. All remote-tracking branches and
-configuration settings for the remote are removed.
+Removes the remote named name. All remote-tracking branches and
+configuration settings for the remote are removed.  If the remote was
+the repository's default remote, then the repository's default remote
+name is changed to origin.  Use 'git remote default name' to set
+the repository's default remote name.
 
 'set-head'::
 
@@ -160,7 +184,7 @@ actually prune them.
 
 'update'::
 
-Fetch updates for a named set of remotes in the repository as defined by
+Fetches updates for a named set of remotes in the repository as defined by
 remotes.group.  If a named group is not specified on the command line,
 the configuration parameter remotes.default will be used; if
 remotes.default is not defined, all remotes which do not have the
diff --git a/builtin/remote.c b/builtin/remote.c
index 920262d..1fb272c 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -12,6 +12,7 @@ static const char * const builtin_remote_usage[] = {
git remote add [-t branch] [-m master] [-f] [--tags|--no-tags] 
[--mirror=fetch|push] name url,
git remote rename old new,
git remote rm name,
+   git remote default [name],
git remote set-head name (-a | -d | branch),
git remote [-v | --verbose] show [-n] name,
git remote prune [-n | --dry-run] name,
@@ -198,6 +199,9 @@ static int add(int argc, const char **argv)
if (!valid_fetch_refspec(buf2.buf))
die(_('%s' is not a valid remote name), name);
 
+   if (remote_count() == 1) /* remote_get() adds a remote if it's new */
+   git_config_set(remote.default, name);
+
strbuf_addf(buf, remote.%s.url, name);
if

[PATCH 4/6] Teach clone to set remote.default.

From: Marc Branchaud marcn...@xiplink.com

This makes git-clone consistent with the remote.default support implemented in
git-remote.  Specifically, since git remote add sets remote.default if
it's adding the first remote to the repository, when clone itself adds the
first remote it should do the same.

This also makes git clone -o work without a special case in the code (so
git clone /some/repo and git clone -o origin /some/repo produce the same
result).

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 builtin/clone.c  |  2 ++
 t/t5601-clone.sh | 10 ++
 t/t5702-clone-options.sh |  7 +--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index a4d8d25..b198456 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -770,6 +770,8 @@ int cmd_clone(int argc, const char **argv, const char 
*prefix)
git_config_set(key.buf, repo);
strbuf_reset(key);
 
+   git_config_set(remote.default, option_origin);
+
if (option_reference.nr)
setup_reference();
 
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 67869b4..046610d 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -57,6 +57,16 @@ test_expect_success 'clone checks out files' '
 
 '
 
+test_expect_success 'clone sets remote.default' '
+
+   rm -fr dst 
+   git clone src dst 
+   (
+   cd dst 
+   test $(git config --get remote.default) = origin
+   )
+'
+
 test_expect_success 'clone respects GIT_WORK_TREE' '
 
GIT_WORK_TREE=worktree git clone src bare 
diff --git a/t/t5702-clone-options.sh b/t/t5702-clone-options.sh
index 02cb024..9e573dd 100755
--- a/t/t5702-clone-options.sh
+++ b/t/t5702-clone-options.sh
@@ -15,8 +15,11 @@ test_expect_success 'setup' '
 test_expect_success 'clone -o' '
 
git clone -o foo parent clone-o 
-   (cd clone-o  git rev-parse --verify refs/remotes/foo/master)
-
+   (
+   cd clone-o 
+   git rev-parse --verify refs/remotes/foo/master 
+   test $(git config --get remote.default) = foo
+   )
 '
 
 test_expect_success 'redirected clone' '
-- 
1.7.11.1

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

[PATCH v2 0/6] Default remote

Incorporated feedback on the first version of this series[1], and also added
documentation updates.

Note that the documentation changes include 4 minor grammatical fixes (verb
tenses, added a the in a couple fo places).  

I also added Phil's git push scenario to patch #2's message, rather than
putting it in the documentation as I'd planned.  Explaining the behavior
change in the commit message felt more natural.

The specific differences from v1 are:

Patch #1 (Rename remote.c's default_remote_name static variables.):
  * Expanded the commit message to explain the choice of
effective_remote_name.

Patch #2 (Teach remote.c about the remote.default configuration setting.):
  * Added documentation updates.
  * Commit message now describes change in default git push behavior.
  * Moved new remote_get_default_name() and remote_count() functions
to patch #3.

Patch #3 (Teach git remote about remote.default.):
  * (Was patch #4 in v1 of this series.)
  * Documented changes to git remote.
  * The remote_get_default_name() and remote_count() functions are
now added to remote.[ch] here, with proper declarations.
  * Added a test to ensure that renaming the origin remote still
properly sets remote.default in repos created with an older
version of git.

Patch #4 (Teach clone to set remote.default.):
  * (Was patch #3 in v1 of this series.)
  * Commit message now justifies changes to git clone.

Patches 5  6 are unchanged.

M.

[1] http://thread.gmane.org/gmane.comp.version-control.git/201065

Marc Branchaud (6):
  Rename remote.c's default_remote_name static variables.
  Teach remote.c about the remote.default configuration setting.
  Teach git remote about remote.default.
  Teach clone to set remote.default.
  Test that plain git fetch uses remote.default when on a detached HEAD.
  Teach get_default_remote to respect remote.default.

 Documentation/config.txt   |  8 
 Documentation/git-pull.txt |  6 ++-
 Documentation/git-push.txt |  8 +++-
 Documentation/git-remote.txt   | 32 ++--
 Documentation/pull-fetch-param.txt |  6 +++
 builtin/clone.c|  2 +
 builtin/remote.c   | 29 +++
 git-parse-remote.sh|  5 +--
 remote.c   | 34 +
 remote.h   |  2 +
 t/t5505-remote.sh  | 76 ++
 t/t5510-fetch.sh   | 17 +
 t/t5512-ls-remote.sh   |  8 +++-
 t/t5528-push-default.sh|  4 +-
 t/t5601-clone.sh   | 10 +
 t/t5702-clone-options.sh   |  7 +++-
 t/t7400-submodule-basic.sh | 21 +++
 17 files changed, 253 insertions(+), 22 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/6] Test that plain git fetch uses remote.default when on a detached HEAD.

From: Marc Branchaud marcn...@xiplink.com

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 t/t5510-fetch.sh | 17 +
 1 file changed, 17 insertions(+)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index d7a19a1..8ecd996 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -69,6 +69,23 @@ test_expect_success fetch test '
test z$mine = z$his
 '
 
+test_expect_success fetch test detached HEAD uses default remote '
+   cd $D 
+   git clone -o foo . default-remote 
+   git checkout -b detached 
+   test_commit update1 
+   (
+   cd default-remote 
+   git checkout HEAD@{0} 
+   git fetch 
+   test -f .git/refs/remotes/foo/detached 
+   mine=`git rev-parse refs/remotes/foo/detached` 
+   his=`cd ..  git rev-parse refs/heads/detached` 
+   test z$mine = z$his
+   ) 
+   git checkout master
+'
+
 test_expect_success fetch test for-merge '
cd $D 
cd three 
-- 
1.7.11.1

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

[PATCH 6/6] Teach get_default_remote to respect remote.default.

From: Marc Branchaud marcn...@xiplink.com

Use git remote default instead of replicating its logic.

The unit test checks a relative-path submodule because the submodule code
is (almost) the only thing that uses get_default_remote.

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 git-parse-remote.sh|  5 +
 t/t7400-submodule-basic.sh | 21 +
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/git-parse-remote.sh b/git-parse-remote.sh
index 484b2e6..49257f0 100644
--- a/git-parse-remote.sh
+++ b/git-parse-remote.sh
@@ -5,10 +5,7 @@
 GIT_DIR=$(git rev-parse -q --git-dir) || :;
 
 get_default_remote () {
-   curr_branch=$(git symbolic-ref -q HEAD)
-   curr_branch=${curr_branch#refs/heads/}
-   origin=$(git config --get branch.$curr_branch.remote)
-   echo ${origin:-origin}
+   echo $(git remote default)
 }
 
 get_remote_merge_branch () {
diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index 81827e6..2ac2ffc 100755
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -507,6 +507,27 @@ test_expect_success 'relative path works with 
user@host:path' '
)
 '
 
+test_expect_success 'realtive path works when superproject on detached HEAD' '
+   (
+   test_create_repo detach 
+   cd detach 
+   test_create_repo sub 
+   (
+   cd sub 
+   test_commit foo
+   ) 
+   git add sub 
+   git commit -m added sub 
+   rm -rf sub 
+   git checkout HEAD@{0} 
+   git config -f .gitmodules submodule.sub.path sub 
+   git config -f .gitmodules submodule.sub.url ../subrepo 
+   git remote add awkward /path/to/awkward
+   git submodule init sub 
+   test $(git config submodule.sub.url) = /path/to/subrepo
+   )
+'
+
 test_expect_success 'moving the superproject does not break submodules' '
(
cd addtest 
-- 
1.7.11.1

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

[PATCH 1/6] Rename remote.c's default_remote_name static variables.

From: Marc Branchaud marcn...@xiplink.com

This prepares the code to handle a true remote.default configuration value.

Rename two variables:
  default_remote_name  -- effective_remote_name
  explicit_default_remote_name -- explicit_effective_remote_name

effective_remote_name is the remote name that is currently in effect.
This is the currently checked-out branch's remote (in which case
explicit_effective_remote_name=1), or origin if the branch has no remote
(or the working tree is a detached HEAD).

The next commit re-introduces default_remote_name with its new semantics.

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 remote.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/remote.c b/remote.c
index 6833538..6f371e0 100644
--- a/remote.c
+++ b/remote.c
@@ -47,8 +47,8 @@ static int branches_alloc;
 static int branches_nr;
 
 static struct branch *current_branch;
-static const char *default_remote_name;
-static int explicit_default_remote_name;
+static const char *effective_remote_name;
+static int explicit_effective_remote_name;
 
 static struct rewrites rewrites;
 static struct rewrites rewrites_push;
@@ -360,8 +360,8 @@ static int handle_config(const char *key, const char 
*value, void *cb)
return config_error_nonbool(key);
branch-remote_name = xstrdup(value);
if (branch == current_branch) {
-   default_remote_name = branch-remote_name;
-   explicit_default_remote_name = 1;
+   effective_remote_name = branch-remote_name;
+   explicit_effective_remote_name = 1;
}
} else if (!strcmp(subkey, .merge)) {
if (!value)
@@ -481,9 +481,9 @@ static void read_config(void)
unsigned char sha1[20];
const char *head_ref;
int flag;
-   if (default_remote_name) /* did this already */
+   if (effective_remote_name) /* did this already */
return;
-   default_remote_name = xstrdup(origin);
+   effective_remote_name = xstrdup(origin);
current_branch = NULL;
head_ref = resolve_ref_unsafe(HEAD, sha1, 0, flag);
if (head_ref  (flag  REF_ISSYMREF) 
@@ -680,8 +680,8 @@ struct remote *remote_get(const char *name)
if (name)
name_given = 1;
else {
-   name = default_remote_name;
-   name_given = explicit_default_remote_name;
+   name = effective_remote_name;
+   name_given = explicit_effective_remote_name;
}
 
ret = make_remote(name, 0);
-- 
1.7.11.1

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

[PATCH 2/6] Teach remote.c about the remote.default configuration setting.

From: Marc Branchaud marcn...@xiplink.com

The code now has a default_remote_name and an effective_remote_name:

 - default_remote_name is set by remote.default in the config, or is origin
   if remote.default doesn't exist (origin was the fallback value before
   this change).

 - effective_remote_name is the name of the remote tracked by the current
   branch, or is default_remote_name if the current branch doesn't have a
   remote.

This has a minor side effect on the default behavior of git push.  Consider
the following sequence of commands:

  git config remote.default foo # Set default remote
  git checkout somelocalbranch  # Does not track a remote
  git remote add origin ssh://example.com/repo  # Add a new origin
  git push

Prior to this change, the above git push would attempt to push to the new
origin repository at ssh://example.com/repo.  Now instead that git push
will attempt to push to the repository named foo.

Signed-off-by: Marc Branchaud marcn...@xiplink.com
---
 Documentation/config.txt   |  8 
 Documentation/git-pull.txt |  6 +-
 Documentation/git-push.txt |  8 +++-
 Documentation/pull-fetch-param.txt |  6 ++
 remote.c   | 14 +++---
 5 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 915cb5a..7869e1b 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1856,6 +1856,14 @@ remote.name.vcs::
Setting this to a value vcs will cause git to interact with
the remote with the git-remote-vcs helper.
 
+remote.default::
+   This value is the name of a remote.  When Git needs to automatically
+   choose a remote to use, it first tries the 'branch.branchname.remote'
+   value of the currently checked-out branch.  If the currently checked-out
+   branch has no remote, Git uses the remote named by 'remote.default', or
+   the remote named origin if no value is set (even if there is no
+   actual remote named origin).
+
 remotes.group::
The list of remotes which are fetched by git remote update
group.  See linkgit:git-remote[1].
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index defb544..2610253 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -33,7 +33,11 @@ but usually it is the name of a branch in the remote 
repository.
 
 Default values for repository and branch are read from the
 remote and merge configuration for the current branch
-as set by linkgit:git-branch[1] `--track`.
+as set by linkgit:git-branch[1] `--track`.  If the current branch
+has no remote configured, the default for repository is read
+from the remote.default configuration variable.  If that variable
+is not set, the default repository is origin even if there is no
+actual remote named origin.
 
 Assume the following history exists and the current branch is
 `master`:
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index cb97cc1..fc17d39 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -27,10 +27,16 @@ documentation for linkgit:git-receive-pack[1].
 OPTIONS[[OPTIONS]]
 --
 repository::
-   The remote repository that is destination of a push
+   The remote repository that is the destination of the push
operation.  This parameter can be either a URL
(see the section URLS,GIT URLS below) or the name
of a remote (see the section REMOTES,REMOTES below).
+   If this parameter is omitted, git tries to use the remote
+   associated with the currently checked-out branch.  If there
+   is no remote associated with the current branch, git uses
+   the remote named by the remote.default configuration variable.
+   If remote.default is not set, git uses the name origin even
+   if there is no actual remote named origin.
 
 refspec...::
The format of a refspec parameter is an optional plus
diff --git a/Documentation/pull-fetch-param.txt 
b/Documentation/pull-fetch-param.txt
index 94a9d32..696f1fb 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -3,6 +3,12 @@
or pull operation.  This parameter can be either a URL
(see the section URLS,GIT URLS below) or the name
of a remote (see the section REMOTES,REMOTES below).
+   If this parameter is omitted, git tries to use the remote
+   associated with the currently checked-out branch.  If there
+   is no remote associated with the current branch, git uses
+   the remote named by the remote.default configuration variable.
+   If remote.default is not set, git uses the name origin even
+   if there is no actual remote named origin.
 
 ifndef::git-pull[]
 group::
diff --git a/remote.c b/remote.c
index 6f371e0..2ebdbbd 100644
--- a/remote.c
+++ b/remote.c
@@ -47,6 +47,7 @@

Re: Subtree in Git

2012-07-11 Thread dag

Herman van Rink r...@initfour.nl writes:

 It's hard to tell what's what with one big diff.  Each command should
 get its own commit plus more if infrastructure work has to be done.  I
 realize it's a bit of a pain to reformulate this but git rebase -i makes
 it easy and the history will be much better long-term.

 Each command should be described briefly in the commit log.

 That would indeed be nice, but as some parts interdependent it would be
 rather complicated.

Do the interdependent parts first, then.  These should be pure
infrastructure.

 And what is the use if their not fully independently testable.

The command should be testable as soon as they are fully implemented,
no?

I'm thinking about a sequence like this:

- Infrastructure for command A (and possibly B, C, etc. if they are
  interdependent).
- Command A + tests
- Infrastructure for command B
- Command B + tests
- etc.

 If you want to fake a nice history tree then go ahead, I just don't have
 the energy to go through these commits again just for that.

Well, I can't do this either, both because it would take time to get up
to speed on the patches and because I have a million other things going
on at the moment.  So unfortunately, this is going to sit until someone
can take it up.

Unless Junio accepts your patches, of course.  :)

 Some questions/comments:

 - Is .gittrees the right solution?  I like the feature it provides but
   an external file feels a bit hacky.  I wonder if there is a better way
   to track this metadata.  Notes maybe?  Other git experts will have to
   chime in with suggestions.

 It's similar to what git submodule does. And when you add this file to
 the index you can use it on other checkouts as well.

Well, I guess I'm not strongly opposed, I was just asking the question.

 - This code seems to be repeated a lot.  Maybe it should be a utility
   function.

 Yes that's there three times...

So you agree it should be factored?

 - I removed all this stuff in favor of the test library.  Please don't
   reintroduce it.  These new tests will have to be rewritten in terms of
   the existing test infrastructure.  It's not too hard.

 I've left it in to be able to verify your new tests. Once all the new
 tests are passing we can get rid of the old one, not before.
 And as all the old tests are contained in test.sh it should not interfere...

No, I'm very strongly against putting this back in.  The new tests will
have to be updated to the upstream test infrastructure.

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

Re: [PATCH/RFC v2 1/2] Strip namelen out of ce_flags into a ce_namelen field

Junio C Hamano gits...@pobox.com writes:

 @@ -395,10 +395,8 @@ int df_name_compare(const char *name1, int len1, int 
 mode1,
  return c1 - c2;
  }
  
 -int cache_name_compare(const char *name1, int flags1, const char *name2, 
 int flags2)
 +int cache_name_stage_compare(const char *name1, int stage1, int len1, const 
 char *name2, int stage2, int len2)
  {
 -int len1 = flags1  CE_NAMEMASK;
 -int len2 = flags2  CE_NAMEMASK;
  int len = len1  len2 ? len1 : len2;
  int cmp;

 Isn't this a _BUGFIX_?  It appears to me that the original code
 would only compare the first 4k bytes and ignore the rest, if two
 cache entries, both with overlong names, are compared.  Care to come
 up with a test case to demonstrate the breakage and a bugfix without
 the remainder of this patch, to be applied to 'master' and older
 maintenance releases?

Perhaps something like this (based on v1.7.0.9 as this may deserve
to go to older maintenance releases).

-- 8 --
Subject: [PATCH] cache_name_compare(): do not truncate while comparing paths

We failed to use ce_namelen() equivalent and instead only compared
up to the CE_NAMEMASK bytes by mistake.  Adding an overlong path
that shares the same common prefix as an existing entry in the index
did not add a new entry, but instead replaced the existing one, as
the result.

Signed-off-by: Junio C Hamano gits...@pobox.com
---
 read-cache.c | 13 +
 t/t3006-ls-files-long.sh | 39 +++
 2 files changed, 48 insertions(+), 4 deletions(-)
 create mode 100755 t/t3006-ls-files-long.sh

diff --git a/read-cache.c b/read-cache.c
index f1f789b..0cd13aa 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -405,10 +405,15 @@ int df_name_compare(const char *name1, int len1, int 
mode1,
 
 int cache_name_compare(const char *name1, int flags1, const char *name2, int 
flags2)
 {
-   int len1 = flags1  CE_NAMEMASK;
-   int len2 = flags2  CE_NAMEMASK;
-   int len = len1  len2 ? len1 : len2;
-   int cmp;
+   int len1, len2, len, cmp;
+
+   len1 = flags1  CE_NAMEMASK;
+   if (CE_NAMEMASK = len1)
+   len1 = strlen(name1 + CE_NAMEMASK) + CE_NAMEMASK;
+   len2 = flags2  CE_NAMEMASK;
+   if (CE_NAMEMASK = len2)
+   len2 = strlen(name2 + CE_NAMEMASK) + CE_NAMEMASK;
+   len = len1  len2 ? len1 : len2;
 
cmp = memcmp(name1, name2, len);
if (cmp)
diff --git a/t/t3006-ls-files-long.sh b/t/t3006-ls-files-long.sh
new file mode 100755
index 000..202ad65
--- /dev/null
+++ b/t/t3006-ls-files-long.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+
+test_description='overly long paths'
+. ./test-lib.sh
+
+test_expect_success setup '
+   p=filefilefilefilefilefilefilefile 
+   p=$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p 
+   p=$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p$p 
+
+   path_a=${p}_a 
+   path_z=${p}_z 
+
+   blob_a=$(echo frotz | git hash-object -w --stdin) 
+   blob_z=$(echo nitfol | git hash-object -w --stdin) 
+
+   pat=100644 %s 0\t%s\n
+'
+
+test_expect_success 'overly-long path by itself is not a problem' '
+   printf $pat $blob_a $path_a |
+   git update-index --add --index-info 
+   echo $path_a expect 
+   git ls-files actual 
+   test_cmp expect actual
+'
+
+test_expect_success 'overly-long path does not replace another by mistake' '
+   printf $pat $blob_a $path_a $blob_z $path_z |
+   git update-index --add --index-info 
+   (
+   echo $path_a
+   echo $path_z
+   ) expect 
+   git ls-files actual 
+   test_cmp expect actual
+'
+
+test_done
-- 
1.7.11
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/3] read-cache.c: Handle long filenames correctly

Thomas Gummerer t.gumme...@gmail.com writes:

 Make git handle long file/path names ( 4096 characters) correctly.

 There is a bug in the current version, which causes very long
 file/pathnames to be handled incorrectly, or not even added to
 the index, if they share the first 4096 characters.

 Signed-off-by: Thomas Gummerer t.gumme...@gmail.com

Ahh, I guess I should have opened my mailbox before starting to look
into this myself ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 3/3] Replace strlen() with ce_namelen()

Thanks for resending; I've merged this independently to 'next' and
will merge it to 'master' soonish.  This one is trivially correct.

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

Re: [PATCH 3/3] branch: add --unset-upstream option

Carlos Martín Nieto c...@elego.de writes:

 I've added a bit of code to also remove branch.foo.rebase, which
 I'd also consider to be part of the upstream information.

If git branch -t or git branch --set-upstream took another
option --integrate-with=[rebase|merge] to set the variable, I
would agree that removing branch.$name.rebase would be the right
thing to do, but because it is not, I do not know if it is sensible
to remove it upon --unset-upstream.

I actually thought about commenting on that exact variable in my
review, saying that the patch was correct that it didn't touch it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] Allow reading svn dumps from files via file:// urls.

Dmitry Ivankov divanor...@gmail.com writes:

 Florian Achleitner florian.achleitner.2.6.31 at gmail.com writes:

 
 Especially for testing and development it's useful
 to bypass svnrdump and replay the svndump from a file
 without connecting to an svn server.
 
 Add support for file:// urls in the remote url.
 e.g. svn::file:///path/to/dump
 When the remote helper finds an url starting with
 file:// it tries to open that file instead of invoking svnrdump.

 file:// is a bad choice because file:// style repo urls are valid for svn and 
 it's for local repos rather than dumpfiles.

Thanks; I had the same reaction when I saw it.

 Maybe something like dumpfile:// instead?

If dumpfile:// pseudo URL is an established convention in the
Subversion land, that sounds like a sensible direction, but if that
is not the case, it may be cleaner if you can find some other way to
convey the information to the backend out-of-band, instead of
overloading it in the URL used to access the repository.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] Allow reading svn dumps from files via file:// urls.

2012-07-11 Thread Stephen Bash

- Original Message -
From: Junio C Hamano gits...@pobox.com
To: Dmitry Ivankov divanor...@gmail.com
Cc: git@vger.kernel.org
Sent: Wednesday, July 11, 2012 1:00:29 PM
Subject: Re: [PATCH 2/4] Allow reading svn dumps from files via file:// urls.

Dmitry Ivankov divanor...@gmail.com writes:

Florian Achleitner florian.achleitner.2.6.31 at gmail.com
writes:

Especially for testing and development it's useful to bypass
svnrdump and replay the svndump from a file without connecting to
an svn server.

Add support for file:// urls in the remote url. e.g.
svn::file:///path/to/dump When the remote helper finds an url
starting with file:// it tries to open that file instead of
invoking svnrdump.

file:// is a bad choice because file:// style repo urls are valid
for svn and it's for local repos rather than dumpfiles.

Thanks; I had the same reaction when I saw it.

Maybe something like dumpfile:// instead?

If dumpfile:// pseudo URL is an established convention in the
Subversion land, that sounds like a sensible direction, but if that is
not the case, it may be cleaner if you can find some other way to
convey the information to the backend out-of-band, instead of
overloading it in the URL used to access the repository.

Others may have a different opinion, but in my experience dump files are always
handled via stdin/stdout in Subversion land. For example:

http://svnbook.red-bean.com/en/1.7/svn.ref.svnadmin.c.dump.html
http://svnbook.red-bean.com/en/1.7/svn.ref.svnadmin.c.load.html
http://svnbook.red-bean.com/en/1.7/svn.reposadmin.maint.html#svn.reposadmin.maint.filtering

I'm not sure that helps in this scenario, but that was the convention I grew
used to.

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

[PATCH] submodules: don't stumble over symbolic links when cloning recursively

2012-07-11 Thread Jens Lehmann

Since 69c305178 (submodules: refactor computation of relative gitdir path)
cloning a submodule recursively fails for recursive submodules when a
symbolic link is part of the path to the work tree of the superproject.

This happens when module_clone() tries to find the relative paths between
work tree and git dir. When a symbolic link in current $PWD points to a
directory in a different level determining the number of ../ needed to
traverse to the superprojects work tree leads to a wrong result.

As there is no portable way to say pwd -P use cd_to_toplevel to remove
the link from the pwd, which fixes this problem.

A test for this problem has been added to t7406.

Reported-by: Bob Halley hal...@play-bow.org
Signed-off-by: Jens Lehmann jens.lehm...@web.de
---

Thanks to Bob for providing a very detailed bug report and test case!

 git-submodule.sh|  4 ++--
 t/t7406-submodule-update.sh | 13 +
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 5629d87..f73e32e 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -186,8 +186,8 @@ module_clone()
die $(eval_gettext Clone of '\$url' into submodule path 
'\$sm_path' failed)
fi

-   a=$(cd $gitdir  pwd)/
-   b=$(cd $sm_path  pwd)/
+   a=$(cd_to_toplevel  cd $gitdir  pwd)/
+   b=$(cd_to_toplevel  cd $sm_path  pwd)/
# normalize Windows-style absolute paths to POSIX-style absolute paths
case $a in [a-zA-Z]:/*) a=/${a%%:*}${a#*:} ;; esac
case $b in [a-zA-Z]:/*) b=/${b%%:*}${b#*:} ;; esac
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index dcb195b..05521de 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -636,4 +636,17 @@ test_expect_success 'submodule update properly revives a 
moved submodule' '
)
 '

+test_expect_success 'submodule update can handle symbolic links in pwd' '
+   mkdir -p linked/dir 
+   ln -s linked/dir linkto 
+   (
+   cd linkto 
+   git clone $TRASH_DIRECTORY/super_update_r2 super 
+   (
+   cd super 
+   git submodule update --init --recursive
+   )
+   )
+'
+
 test_done
-- 
1.7.11.1.166.gb8ff004
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.

marcn...@xiplink.com writes:

 From: Marc Branchaud marcn...@xiplink.com

 The code now has a default_remote_name and an effective_remote_name:

  - default_remote_name is set by remote.default in the config, or is origin
if remote.default doesn't exist (origin was the fallback value before
this change).

  - effective_remote_name is the name of the remote tracked by the current
branch, or is default_remote_name if the current branch doesn't have a
remote.

 This has a minor side effect on the default behavior of git push.  Consider
 the following sequence of commands:

   git config remote.default foo # Set default remote
   git checkout somelocalbranch  # Does not track a remote
   git remote add origin ssh://example.com/repo  # Add a new origin
   git push

 Prior to this change, the above git push would attempt to push to the new
 origin repository at ssh://example.com/repo.  Now instead that git push
 will attempt to push to the repository named foo.

Hrm, is this a _minor_ side effect?

Isn't that a natural and direct consequence of now you can set
remote.default configuration to name the remote you want to use in
place of traditional 'origin' feature?  I think changing the
behaviour of git push in such a way is the point (may not be the
only but one of the important points) of this series.

 diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
 index cb97cc1..fc17d39 100644
 --- a/Documentation/git-push.txt
 +++ b/Documentation/git-push.txt
 @@ -27,10 +27,16 @@ documentation for linkgit:git-receive-pack[1].
  OPTIONS[[OPTIONS]]
  --
  repository::
 - The remote repository that is destination of a push
 + The remote repository that is the destination of the push
   operation.  This parameter can be either a URL
   (see the section URLS,GIT URLS below) or the name
   of a remote (see the section REMOTES,REMOTES below).
 + If this parameter is omitted, git tries to use the remote
 + associated with the currently checked-out branch.  If there
 + is no remote associated with the current branch, git uses
 + the remote named by the remote.default configuration variable.
 + If remote.default is not set, git uses the name origin even
 + if there is no actual remote named origin.

This comment applies to other changes to the documentation in this
patch I didn't quote, but I think the phrasing 'even if there is no
acutual remote named origin' needs to be rethought, because we
use it even if there is no such remote determined with this logic
applies equally to branch.$name.remote, remote.default or built-in
default value of remote.default which happens to be origin.

 diff --git a/remote.c b/remote.c
 index 6f371e0..2ebdbbd 100644
 --- a/remote.c
 +++ b/remote.c
 @@ -47,6 +47,7 @@ static int branches_alloc;
  static int branches_nr;
  
  static struct branch *current_branch;
 +static const char *default_remote_name;
  static const char *effective_remote_name;

Coming from UNIX background, effective gives me an impression that
it is contrasted with real (i.e. there is real remote for the
branch, but during this particular invocation it is overridden and
effective remote is used instead).  Perhaps it is just me.

Something along the line of remote-name-to-use, use-remote, might
sound more natural.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] Teach git remote about remote.default.

marcn...@xiplink.com writes:

 From: Marc Branchaud marcn...@xiplink.com

 The rename and rm commands now handle the case where the remote being
 changed is the default remote.

handle the case is way underspecified.

Until I peeked the patch, I couldn't tell if you were now allowing
git remote rm that does not say which remote to remove the remote
named via remote.default by default, and had to wonder if you made
git remote rename frotz without any other argument mean git
remote rename remote.default frotz or the other way around.

The rename and rm commands adjusts the value of the
remote.default variable when they make the current default
remote disappear.

 If the add command is used to add the repo's first remote, that remote
 becomes the default remote.

Probably sensible, but I could easily imagine existing users to be
surprised, harmed and complain, especially now those who want this
behaviour already have a separate remote default command to set it
themselves.

 Also introduce a default sub-command to get or set the default remote:

  - git remote default (with no parameter) displays the current default 
 remote.

  - git remote default remo checks if remo is a configured remote and if
so changes remote.default to remo.

Avoid cute and not-widely-used abbreviation; in other words s/remo/remote/.

 These changes needed a couple of new helper functions in remote.c:
  - remote_get_default_name() returns default_remote_name.
  - remote_count() returns the number of remotes.

 (The test in t5528 had to be changed because now with push.default=matching
 a plain git push succeeds with Everything up-to-date.  It used to
 fail with No configured push destination.)

I would like to see people think, when their new feature breaks
existing tests and needs adjustments, like this:

Even test scripts gets upset with this unexpected behaviour
change---real users may also be hurt the same way.  Should we
really need to do this change?  What can we do to help them?

Even though I said Probably sensible, I would prefer the first
remote automatically replaces 'origin' feature not to be part of
this patch.  It is something we can queue separately on top as a
separate feature.

I personally think in the long run it is probably the right thing to
do, but at the same time, I do not think it is something we want to
throw at the users as a flag-day event without transition planning.

 +'default'::
 +
 +Displays or sets the name of the repository's default remote.  When git needs
 +to automatically choose a remote to use, it first tries to use the remote
 +associated with the currently checked-out branch.  If the currently
 +checked-out branch has no remote, git uses the repository's default remote.
 +If the repository has no default remote, git tries to use a remote named
 +origin even if there is no actual remote named origin.  (In other words,
 +the default value for the default remote name is origin.)

After saying something long-winded and convoluted that you think it
needs In other words, appended, try to re-read the sentence
without the long-winded part (and In other words,).  I often find
that the long description is unnecessary after doing so myself.

I wonder if it makes the result easier to read if you consolidated
all the duplicated explanations of what the default remote is/does
in this patch and previous patches to a single place (perhaps
glossary) and refer to it from these places, e.g.

default::

Display or sets the name of the default remote for
the repository.  See default remote in
linkgit:gitglossary[7].

  'rename'::
  
 -Rename the remote named old to new. All remote-tracking branches and
 +Renames the remote named old to new. All remote-tracking branches and

Why change the mood from imperative (which I thought was the norm
for these command descriptions) to third-person-present?

  configuration settings for the remote are updated.
  +
  In case old and new are the same, and old is a file under
  `$GIT_DIR/remotes` or `$GIT_DIR/branches`, the remote is converted to
  the configuration file format.
 ++
 +If old was the repository's default remote name, the default remote name
 +changes to new.

This is unrelated to your patch, but I wonder what should happen to
this repository:

git config branch.foo.remote origin
git remote rename origin upstream

Should branch.foo.remote be updated, and if so how (either set to
upstream or configuration removed)?

  'rm'::
  
 -Remove the remote named name. All remote-tracking branches and
 -configuration settings for the remote are removed.
 +Removes the remote named name. All remote-tracking branches and

Why change the mood from imperative (which I thought was the norm
for these command descriptions) to third-person-present?

 +configuration settings for the remote are removed.  If the remote was
 +the repository's default remote, then the

Re: [PATCH] submodules: don't stumble over symbolic links when cloning recursively

2012-07-11 Thread Johannes Sixt

Am 11.07.2012 20:11, schrieb Jens Lehmann:
 Since 69c305178 (submodules: refactor computation of relative gitdir path)
 cloning a submodule recursively fails for recursive submodules when a
 symbolic link is part of the path to the work tree of the superproject.
 
 This happens when module_clone() tries to find the relative paths between
 work tree and git dir. When a symbolic link in current $PWD points to a
 directory in a different level determining the number of ../ needed to
 traverse to the superprojects work tree leads to a wrong result.
 
 As there is no portable way to say pwd -P use cd_to_toplevel to remove
 the link from the pwd, which fixes this problem.
...
 - a=$(cd $gitdir  pwd)/
 - b=$(cd $sm_path  pwd)/
 + a=$(cd_to_toplevel  cd $gitdir  pwd)/
 + b=$(cd_to_toplevel  cd $sm_path  pwd)/

But if you cd out, how can it be correct not to cd in again if $gitdir
and/or $sm_path are relative?

And if $gitdir and/or $sm_path are absolute, how can the earlier
cd_to_toplevel make a difference?

 +test_expect_success 'submodule update can handle symbolic links in pwd' '

Please add a SYMLINKS prerequisite.

 + mkdir -p linked/dir 
 + ln -s linked/dir linkto 
 + (
 + cd linkto 
 + git clone $TRASH_DIRECTORY/super_update_r2 super 
 + (
 + cd super 
 + git submodule update --init --recursive
 + )
 + )
 +'

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

Re: [PATCH] submodules: don't stumble over symbolic links when cloning recursively

2012-07-11 Thread Jens Lehmann

Am 11.07.2012 21:10, schrieb Johannes Sixt:
 Am 11.07.2012 20:11, schrieb Jens Lehmann:
 Since 69c305178 (submodules: refactor computation of relative gitdir path)
 cloning a submodule recursively fails for recursive submodules when a
 symbolic link is part of the path to the work tree of the superproject.

 This happens when module_clone() tries to find the relative paths between
 work tree and git dir. When a symbolic link in current $PWD points to a
 directory in a different level determining the number of ../ needed to
 traverse to the superprojects work tree leads to a wrong result.

 As there is no portable way to say pwd -P use cd_to_toplevel to remove
 the link from the pwd, which fixes this problem.
 ...
 -a=$(cd $gitdir  pwd)/
 -b=$(cd $sm_path  pwd)/
 +a=$(cd_to_toplevel  cd $gitdir  pwd)/
 +b=$(cd_to_toplevel  cd $sm_path  pwd)/
 
 But if you cd out, how can it be correct not to cd in again if $gitdir
 and/or $sm_path are relative?

I'm not sure what you mean by cd out, but the two cd_to_toplevel
make sure that when $gitdir or $sm_path are relative the symbolic link
gets removed from the output of pwd. So it's rather cd into the path
where the symlink is resolved.

 And if $gitdir and/or $sm_path are absolute, how can the earlier
 cd_to_toplevel make a difference?

Then it doesn't, but $sm_path is always relative while $gitdir is
sometimes (in the superproject it returns .git). Just drop either
of the cd_to_toplevel and run the test ;-)

But it looks like the commit message could use some tuning ...

 +test_expect_success 'submodule update can handle symbolic links in pwd' '
 
 Please add a SYMLINKS prerequisite.

Oops. Thanks, will add that.

Will wait some time for other comments before preparing v2.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Mini bug report origin/pu: t1512 failed on Mac OS X (commit 957d74062c1f0e ?)

2012-07-11 Thread Torsten Bögershausen

The following tweak will make t1512 work on my Mac OS box:


--- a/t/t1512-rev-parse-disambiguation.sh
+++ b/t/t1512-rev-parse-disambiguation.sh
@@ -257,7 +257,7 @@ test_expect_success 'rev-parse --disambiguate' '
# commits created by commit-tree in earlier tests do not share
# the prefix.
git rev-parse --disambiguate=0 actual 
-   test $(wc -l actual) = 16 
+   test $(wc -l actual) -eq  16 
test $(sed -e s/^\(.\).*/\1/ actual | sort -u) = 0



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

Re: [PATCH] submodules: don't stumble over symbolic links when cloning recursively

2012-07-11 Thread Johannes Sixt

Am 11.07.2012 22:06, schrieb Jens Lehmann:
 Am 11.07.2012 21:10, schrieb Johannes Sixt:
 Am 11.07.2012 20:11, schrieb Jens Lehmann:
 Since 69c305178 (submodules: refactor computation of relative gitdir path)
 cloning a submodule recursively fails for recursive submodules when a
 symbolic link is part of the path to the work tree of the superproject.

 This happens when module_clone() tries to find the relative paths between
 work tree and git dir. When a symbolic link in current $PWD points to a
 directory in a different level determining the number of ../ needed to
 traverse to the superprojects work tree leads to a wrong result.

 As there is no portable way to say pwd -P use cd_to_toplevel to remove
 the link from the pwd, which fixes this problem.
 ...
 -   a=$(cd $gitdir  pwd)/
 -   b=$(cd $sm_path  pwd)/
 +   a=$(cd_to_toplevel  cd $gitdir  pwd)/
 +   b=$(cd_to_toplevel  cd $sm_path  pwd)/

 But if you cd out, how can it be correct not to cd in again if $gitdir
 and/or $sm_path are relative?
 
 I'm not sure what you mean by cd out, but the two cd_to_toplevel
 make sure that when $gitdir or $sm_path are relative the symbolic link
 gets removed from the output of pwd. So it's rather cd into the path
 where the symlink is resolved.

At this point we can be in a subdirectory of the worktree. With
cd_to_toplevel we move up in the directory hierarchy (cd out). Then a
relative $gitdir or $sm_path now points to the wrong directory. No?

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

Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.

2012-07-11 Thread Marc Branchaud

On 12-07-11 02:21 PM, Junio C Hamano wrote:
 marcn...@xiplink.com writes:
 
 From: Marc Branchaud marcn...@xiplink.com

 The code now has a default_remote_name and an effective_remote_name:

  - default_remote_name is set by remote.default in the config, or is origin
if remote.default doesn't exist (origin was the fallback value before
this change).

  - effective_remote_name is the name of the remote tracked by the current
branch, or is default_remote_name if the current branch doesn't have a
remote.

 This has a minor side effect on the default behavior of git push.  Consider
 the following sequence of commands:

   git config remote.default foo # Set default remote
   git checkout somelocalbranch  # Does not track a remote
   git remote add origin ssh://example.com/repo  # Add a new origin
   git push

 Prior to this change, the above git push would attempt to push to the new
 origin repository at ssh://example.com/repo.  Now instead that git push
 will attempt to push to the repository named foo.
 
 Hrm, is this a _minor_ side effect?

Well, I thought so...  :)

It's minor because of what you say:

 Isn't that a natural and direct consequence of now you can set
 remote.default configuration to name the remote you want to use in
 place of traditional 'origin' feature?  I think changing the
 behaviour of git push in such a way is the point (may not be the
 only but one of the important points) of this series.

I agree.  Phil Hord pointed out the change in behaviour and felt the commit
message should explain it.

Should I just remove minor?

 diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
 index cb97cc1..fc17d39 100644
 --- a/Documentation/git-push.txt
 +++ b/Documentation/git-push.txt
 @@ -27,10 +27,16 @@ documentation for linkgit:git-receive-pack[1].
  OPTIONS[[OPTIONS]]
  --
  repository::
 -The remote repository that is destination of a push
 +The remote repository that is the destination of the push
  operation.  This parameter can be either a URL
  (see the section URLS,GIT URLS below) or the name
  of a remote (see the section REMOTES,REMOTES below).
 +If this parameter is omitted, git tries to use the remote
 +associated with the currently checked-out branch.  If there
 +is no remote associated with the current branch, git uses
 +the remote named by the remote.default configuration variable.
 +If remote.default is not set, git uses the name origin even
 +if there is no actual remote named origin.
 
 This comment applies to other changes to the documentation in this
 patch I didn't quote, but I think the phrasing 'even if there is no
 acutual remote named origin' needs to be rethought, because we
 use it even if there is no such remote determined with this logic
 applies equally to branch.$name.remote, remote.default or built-in
 default value of remote.default which happens to be origin.

I'm sorry, but I'm having trouble understanding what you mean.  I don't see
how the because ... part of your sentence suggests what aspect needs
rephrasing.

(BTW, I'm under the impression that when git sets branch.$name.remote it
doesn't ever have to fall back to the default remote name.  That is, commands
that set the value always have the user explicitly, though perhaps
indirectly, specifying the remote.  Did I miss something?)

 diff --git a/remote.c b/remote.c
 index 6f371e0..2ebdbbd 100644
 --- a/remote.c
 +++ b/remote.c
 @@ -47,6 +47,7 @@ static int branches_alloc;
  static int branches_nr;
  
  static struct branch *current_branch;
 +static const char *default_remote_name;
  static const char *effective_remote_name;
 
 Coming from UNIX background, effective gives me an impression that
 it is contrasted with real (i.e. there is real remote for the
 branch, but during this particular invocation it is overridden and
 effective remote is used instead).  Perhaps it is just me.

I did have something like that in mind when I chose effective:  There's a
default remote name that's normally used, but in the certain circumstances
it's overridden.  Perhaps effective_default_remote_name would be better,
though that's getting to be a mouthful (plus it would mean having an
explicit_effective_default_remote_name, which IMHO is a bit much).

 Something along the line of remote-name-to-use, use-remote, might
 sound more natural.

current_remote_name?

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

Re: [PATCH 3/6] Teach git remote about remote.default.

2012-07-11 Thread Marc Branchaud

On 12-07-11 02:27 PM, Junio C Hamano wrote:
 marcn...@xiplink.com writes:
 
 From: Marc Branchaud marcn...@xiplink.com

 The rename and rm commands now handle the case where the remote being
 changed is the default remote.
 
 handle the case is way underspecified.
 
 Until I peeked the patch, I couldn't tell if you were now allowing
 git remote rm that does not say which remote to remove the remote
 named via remote.default by default, and had to wonder if you made
 git remote rename frotz without any other argument mean git
 remote rename remote.default frotz or the other way around.
 
 The rename and rm commands adjusts the value of the
 remote.default variable when they make the current default
 remote disappear.

I'll use that.

 If the add command is used to add the repo's first remote, that remote
 becomes the default remote.
 
 Probably sensible, but I could easily imagine existing users to be
 surprised, harmed and complain, especially now those who want this
 behaviour already have a separate remote default command to set it
 themselves.

As you suggest below, it makes more sense to put the
first-remote-is-the-default change in a separate patch.  I'll combine it with
the git-clone change in patch #4.

 Also introduce a default sub-command to get or set the default remote:

  - git remote default (with no parameter) displays the current default 
 remote.

  - git remote default remo checks if remo is a configured remote and if
so changes remote.default to remo.
 
 Avoid cute and not-widely-used abbreviation; in other words 
 s/remo/remote/.

OK.

 These changes needed a couple of new helper functions in remote.c:
  - remote_get_default_name() returns default_remote_name.
  - remote_count() returns the number of remotes.

 (The test in t5528 had to be changed because now with push.default=matching
 a plain git push succeeds with Everything up-to-date.  It used to
 fail with No configured push destination.)
 
 I would like to see people think, when their new feature breaks
 existing tests and needs adjustments, like this:
 
 Even test scripts gets upset with this unexpected behaviour
 change---real users may also be hurt the same way.  Should we
 really need to do this change?  What can we do to help them?

I did not think the change was that important, because the fact that the
command failed looked like a bug to me (ie. just because I used -o foo when
I cloned shouldn't make git push with push.default=matching fail).  So it
feels to me like the patch is just making things work the way they should
have in the first place.

 Even though I said Probably sensible, I would prefer the first
 remote automatically replaces 'origin' feature not to be part of
 this patch.  It is something we can queue separately on top as a
 separate feature.

Agreed.

However, this push.default=matching change isn't caused by the first remote
automatically replaces 'origin' stuff.  It's a direct result of having
remote.default set.

 I personally think in the long run it is probably the right thing to
 do, but at the same time, I do not think it is something we want to
 throw at the users as a flag-day event without transition planning.

Fair enough.

What about a warning displayed if remote.default is not set?  Something like:

This repository does not have an explicitly configured default
remote.  Selecting origin as the default remote repository.
To suppress this warning, or if you wish to have a different
default remote repository, run git remote default name.
In git X.Y, the default remote will be automatically configured
as the first remote added to the repository.

 +'default'::
 +
 +Displays or sets the name of the repository's default remote.  When git 
 needs
 +to automatically choose a remote to use, it first tries to use the remote
 +associated with the currently checked-out branch.  If the currently
 +checked-out branch has no remote, git uses the repository's default remote.
 +If the repository has no default remote, git tries to use a remote named
 +origin even if there is no actual remote named origin.  (In other words,
 +the default value for the default remote name is origin.)
 
 After saying something long-winded and convoluted that you think it
 needs In other words, appended, try to re-read the sentence
 without the long-winded part (and In other words,).  I often find
 that the long description is unnecessary after doing so myself.

I'll take another crack at it.

 I wonder if it makes the result easier to read if you consolidated
 all the duplicated explanations of what the default remote is/does
 in this patch and previous patches to a single place (perhaps
 glossary) and refer to it from these places, e.g.
 
   default::
 
   Display or sets the name of the default remote for
   the repository.  See default remote in
   linkgit:gitglossary[7].

Good idea!

  'rename'::

Re: [PATCH] submodules: don't stumble over symbolic links when cloning recursively

2012-07-11 Thread Jens Lehmann

Am 11.07.2012 22:39, schrieb Johannes Sixt:
 Am 11.07.2012 22:06, schrieb Jens Lehmann:
 Am 11.07.2012 21:10, schrieb Johannes Sixt:
 Am 11.07.2012 20:11, schrieb Jens Lehmann:
 Since 69c305178 (submodules: refactor computation of relative gitdir path)
 cloning a submodule recursively fails for recursive submodules when a
 symbolic link is part of the path to the work tree of the superproject.

 This happens when module_clone() tries to find the relative paths between
 work tree and git dir. When a symbolic link in current $PWD points to a
 directory in a different level determining the number of ../ needed to
 traverse to the superprojects work tree leads to a wrong result.

 As there is no portable way to say pwd -P use cd_to_toplevel to remove
 the link from the pwd, which fixes this problem.
 ...
 -  a=$(cd $gitdir  pwd)/
 -  b=$(cd $sm_path  pwd)/
 +  a=$(cd_to_toplevel  cd $gitdir  pwd)/
 +  b=$(cd_to_toplevel  cd $sm_path  pwd)/

 But if you cd out, how can it be correct not to cd in again if $gitdir
 and/or $sm_path are relative?

 I'm not sure what you mean by cd out, but the two cd_to_toplevel
 make sure that when $gitdir or $sm_path are relative the symbolic link
 gets removed from the output of pwd. So it's rather cd into the path
 where the symlink is resolved.
 
 At this point we can be in a subdirectory of the worktree. With
 cd_to_toplevel we move up in the directory hierarchy (cd out). Then a
 relative $gitdir or $sm_path now points to the wrong directory. No?

Nope, git submodule will refuse to run in anything but the root of
the worktree. So we already are at the toplevel and use cd_to_toplevel
only to resolve any symlinks present in $PWD. Looks like a comment
explaining that above those lines would be a good idea ... will add one.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH/RFC] git-svn: don't create master if another head exists

2012-07-11 Thread Marcin Owsiany

On Wed, Jul 11, 2012 at 01:26:17AM +, Eric Wong wrote:
 Junio C Hamano gits...@pobox.com wrote:
  Marcin Owsiany mar...@owsiany.pl writes:
  
   This makes my idea to do the same to my something else instead of
   master much less attractive. In fact I don't think such behaviour would
   be useful.
   
   I think with the suggested patch git-svn works as I would like it to:
- creates master at initial checkout - consistent with git clone
- using a different tracking-like branch is possible with dcommit
- does not re-create master on fetch - so the annoying part is gone
  
   Any comments?
  
  Not from me.  Even though I'd love to hear Eric's opinion, your I
  don't think such behaviour would be useful. gave me an impression
  that you would justify the change in a different way (i.e. a rewrite
  of proposed log message) or tweak the patch (i.e. a modified
  behaviour), or perhaps both, in your re-roll, the ball was in your
  court, and we were waiting for such a rerolled patch.
 
 Sorry, I keep forgetting this topic.  But yes, I thought you would tweak
 your patch.

Oh, I guess I got used to projects where people pay no attention to
patch comments. How about this:

From: Marcin Owsiany mar...@owsiany.pl
Date: Sun, 24 Jun 2012 22:40:05 +0100
Subject: [PATCH] git-svn: don't create master if another head exists

git-svn insists on creating the master head (unless it exists) on every
fetch. It is useful that it gets created initially, when no head exists
- users expect this git convention of having a master branch on initial
clone.

However creating it when there already is another head does not provide any
value - the ref is never updated, so it just gets stale after a while.  Also,
some users find it annoying that it gets recreated, especially when they would
like the git branch names to follow SVN repository branch names. More
background in http://thread.gmane.org/gmane.comp.version-control.git/115030

Make git-svn skip the master creation if HEAD points at a valid head. This
means master does get created on initial clone but does not get recreated
once a user deletes it.

Signed-off-by: Marcin Owsiany mar...@owsiany.pl
---
 git-svn.perl |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 0b074c4..2379a71 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1612,9 +1612,9 @@ sub post_fetch_checkout {
}
}
 
-   my $valid_head = verify_ref('HEAD^0');
+   return if verify_ref('HEAD^0');
command_noisy(qw(update-ref refs/heads/master), $gs-refname);
-   return if ($valid_head || !verify_ref('HEAD^0'));
+   return unless verify_ref('HEAD^0');
 
return if $ENV{GIT_DIR} !~ m#^(?:.*/)?\.git$#;
my $index = $ENV{GIT_INDEX_FILE} || $ENV{GIT_DIR}/index;
-- 
1.7.7.3


-- 
Marcin Owsiany mar...@owsiany.pl  http://marcin.owsiany.pl/
GnuPG: 2048R/02F946FC  35E9 1344 9F77 5F43 13DD  6423 DBF4 80C6 02F9 46FC

Every program in development at MIT expands until it can read mail.
  -- Unknown
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/6] Teach git remote about remote.default.

Marc Branchaud marcn...@xiplink.com writes:

 What about a warning displayed if remote.default is not set?  Something 
 like:

   This repository does not have an explicitly configured default
   remote.  Selecting origin as the default remote repository.
   To suppress this warning, or if you wish to have a different
   default remote repository, run git remote default name.
   In git X.Y, the default remote will be automatically configured
   as the first remote added to the repository.

When do you plan to issue the above message?  Any time we default to
'origin' without remote.default?  

I do not see a value to it---if the user has been happily using
'origin' as the default remote, there is no reason to give such a
noise.  We will keep defaulting to 'origin' even in the version of
Git with this series in it.

A warning is necessary if we plan to _later_ add the first remote
created either by 'git clone' or 'git remote add' is automatically
set as the value for remote.default configuration feature, and the
place to do so is git clone and git remote add that creates the
first remote for the repository.  If the name of the remote created
by them is 'origin', then there is no need for warning, but if the
user called that first remote with a name that is _not_ 'origin',
later versions of Git will change the behaviour.

I can see a transition plan that goes like this:

Step 0.  With this series but without the first one becomes default

$ git init
$ git remote add upstream git://over.there.xz/git.git/
hint: You may want to run git remote default upstream now so that
hint: a lazy 'git push' and 'git fetch' defaults to this 'upstream'
hint: repository, instead of 'origin' (which you do not yet have).

$ git config --global advice.settingDefaultRemote false
$ git remote rm upstream
$ git remote add upstream git://over.there.xz/git.git/
... nothing, as the user declined the advice ...

Step 1.  First one becomes default as an opt-in feature

$ git config --global --unset advice.settingDefaultRemote
$ git init
$ git remote add upstream git://over.there.xz/git.git/
hint: You may want to run git remote default upstream now so that
hint: a lazy 'git push' and 'git fetch' defaults to this 'upstream'
hint: repository, instead of 'origin' (which you do not yet have).
hint: git config --global remote.firstRemoteBecomesDefault true can be
hint: used to make the first remote added to the repository the default
hint: remote automatically.
$ git remote default
origin

$ git config --global remote.firstRemoteBecomesDefault true
$ git remote rm upstream
$ git remote add upstream git://over.there.xz/git.git/
... nothing; user already knows about remote.firstRemoteBecomesDefault
$ git remote default
upstream

Step 2.  Start warning the default change for First one becomes default

$ git config --global --unset advice.settingDefaultRemote
$ git config --global --unset remote.firstRemoteBecomesDefault
$ git init
$ git remote add upstream git://over.there.xz/git.git/
hint: You may want to run git remote default upstream now so that
hint: a lazy 'git push' and 'git fetch' defaults to this 'upstream'
hint: repository, instead of 'origin' (which you do not yet have).
hint: git config --global remote.firstRemoteBecomesDefault true can be
hint: used to make the first remote added to the repository the default
hint: remote automatically.
hint:
hint: In future versions of Git, this will happen automatically.
hint: If you do not want the first remote to become default, run
hint: git config --global remote.firstRemoteBecomesDefault false now.

Step 3. First one becomes default is now default

$ git config --global --unset advice.settingDefaultRemote
$ git config --global --unset remote.firstRemoteBecomesDefault
$ git init
$ git remote add upstream git://over.there.xz/git.git/
warning: Made 'upstream' the default remote for this repository.
$ git remote default
upstream

$ git remote rm upstream
$ git config --global remote.firstRemoteBecomesDefault true
$ git init
$ git remote add upstream git://over.there.xz/git.git/
... nothing; the user explicitly told us what to do
$ git remote default
upstream

$ git remote rm upstream
$ git config --global remote.firstRemoteBecomesDefault false
$ git init
$ git remote add upstream git://over.there.xz/git.git/
... nothing; the user explicitly told us what to do
$ git remote default
origin

 Since the cmd_ prefix is already used for the main commands, perhaps
 another prefix for subcommands?  Maybe sub_?  Some of the shell-based
 commands use the main command itself as a prefix (e.g. bisect_start()).
 Doing that here would mean remote_default() etc.  Any preference?

No strong preference for file-scope-statics.

 +{
 +   if (argc  2)
 +

Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.

Marc Branchaud marcn...@xiplink.com writes:

 On 12-07-11 02:21 PM, Junio C Hamano wrote:
 marcn...@xiplink.com writes:
 
 From: Marc Branchaud marcn...@xiplink.com

 The code now has a default_remote_name and an effective_remote_name:

  - default_remote_name is set by remote.default in the config, or is 
 origin
if remote.default doesn't exist (origin was the fallback value before
this change).

  - effective_remote_name is the name of the remote tracked by the current
branch, or is default_remote_name if the current branch doesn't have a
remote.

 This has a minor side effect on the default behavior of git push
 
 Hrm, is this a _minor_ side effect?

 Well, I thought so...  :)

 It's minor because of what you say:

 Isn't that a natural and direct consequence of now you can set
 remote.default configuration to name the remote you want to use in
 place of traditional 'origin' feature?  I think changing the
 behaviour of git push in such a way is the point (may not be the
 only but one of the important points) of this series.

 I agree.  Phil Hord pointed out the change in behaviour and felt the commit
 message should explain it.

 Should I just remove minor?

Is it even a _side effect_?  Isn't this one of the primary points of
the series?  I do not think this patch makes sense if we did not
want that change to happen.

Or am I missing something?

 diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
 index cb97cc1..fc17d39 100644
 --- a/Documentation/git-push.txt
 +++ b/Documentation/git-push.txt
 @@ -27,10 +27,16 @@ documentation for linkgit:git-receive-pack[1].
  OPTIONS[[OPTIONS]]
  --
  repository::
 -   The remote repository that is destination of a push
 +   The remote repository that is the destination of the push
 operation.  This parameter can be either a URL
 (see the section URLS,GIT URLS below) or the name
 of a remote (see the section REMOTES,REMOTES below).
 +   If this parameter is omitted, git tries to use the remote
 +   associated with the currently checked-out branch.  If there
 +   is no remote associated with the current branch, git uses
 +   the remote named by the remote.default configuration variable.
 +   If remote.default is not set, git uses the name origin even
 +   if there is no actual remote named origin.
 
 This comment applies to other changes to the documentation in this
 patch I didn't quote, but I think the phrasing 'even if there is no
 acutual remote named origin' needs to be rethought, because we
 use it even if there is no such remote determined with this logic
 applies equally to branch.$name.remote, remote.default or built-in
 default value of remote.default which happens to be origin.

 I'm sorry, but I'm having trouble understanding what you mean.  I don't see
 how the because ... part of your sentence suggests what aspect needs
 rephrasing.

You say the remote is taken from three places (branch.$name.remote,
remote.default, or 'origin').

Imagine there is remote.origin.url at all.  If remote.default is set
to 'origin', or branch.$name.remote for the current branch is set to
'origin', the repository you will try to use is 'origin'.  And you
will fail the same way if you try to push there.  It does not matter
if the hardcoded default gave you 'origin' or configuration variable
gave it to you.  The name is used regardless, even if there is no
actual remote under such name.

So If 'remote.default' is not set, git uses the name origin even
if there is no actual remote, while is not untrue, is misleading.
Even if there is no actual remote 'origin', if 'remote.default' is
set to 'origin', git will use that name, and will fail the same way.

Just saying If 'remote.default' is not set, git uses 'origin' or
even 'remote.default', if unset, defaults to 'origin' would be
sufficient.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/6] t4012: Actually quote the sed script

2012-07-11 Thread Alexander Strasser

The nested quoting is not needed in this cases, thus the previous
version did work just fine. Never the less the usage is misleading,
so just achieve nested quoting by using double quotes instead. Lower
the probability of breakage in the future and make the code easier
to read.

NOTE: Just dropping the single quotes around the sed arguments would
  have also been possible.

Signed-off-by: Alexander Strasser eclip...@gmx.net
---
 t/t4012-diff-binary.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 3c54269..60c2f6c 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -64,7 +64,7 @@ test_expect_success 'apply --numstat understands diff 
--binary format' '
 # apply needs to be able to skip the binary material correctly
 # in order to report the line number of a corrupt patch.
 test_expect_success 'apply detecting corrupt patch correctly' '
-git diff | sed -e 's/-CIT/xCIT/' broken 
+git diff | sed -e s/-CIT/xCIT/ broken 
 if git apply --stat --summary broken 2detected
 then
echo unhappy - should have detected an error
@@ -79,7 +79,7 @@ test_expect_success 'apply detecting corrupt patch correctly' 
'
 '
 
 test_expect_success 'apply detecting corrupt patch correctly' '
-git diff --binary | sed -e 's/-CIT/xCIT/' broken 
+git diff --binary | sed -e s/-CIT/xCIT/ broken 
 if git apply --stat --summary broken 2detected
 then
echo unhappy - should have detected an error
-- 
1.7.10.2.552.gaa3bb87
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/6] t4012: Break up pipe into serial redirections

2012-07-11 Thread Alexander Strasser

Do not hide possible git errors by masquerading its process
exit status.

Signed-off-by: Alexander Strasser eclip...@gmx.net
---
 t/t4012-diff-binary.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 60c2f6c..daf8234 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -64,7 +64,8 @@ test_expect_success 'apply --numstat understands diff 
--binary format' '
 # apply needs to be able to skip the binary material correctly
 # in order to report the line number of a corrupt patch.
 test_expect_success 'apply detecting corrupt patch correctly' '
-git diff | sed -e s/-CIT/xCIT/ broken 
+git diff output 
+sed -e s/-CIT/xCIT/ output broken 
 if git apply --stat --summary broken 2detected
 then
echo unhappy - should have detected an error
-- 
1.7.10.2.552.gaa3bb87
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 6/6] t4012: Make --shortstat more robust

2012-07-11 Thread Alexander Strasser

The --shortstat test depends on the same scenario as the
--stat test. Use the part of the same expected result for
the --stat test to avoid duplicating it manually.

Signed-off-by: Alexander Strasser eclip...@gmx.net
---
 t/t4012-diff-binary.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
index 81a9e8c..a3f6030 100755
--- a/t/t4012-diff-binary.sh
+++ b/t/t4012-diff-binary.sh
@@ -38,9 +38,9 @@ test_expect_success 'apply --stat output for binary file 
change' '
 '
 
 test_expect_success 'diff --shortstat output for binary file change' '
-   echo  4 files changed, 2 insertions(+), 2 deletions(-) expected 
+   tail -1 expected expect 
git diff --shortstat current 
-   test_i18ncmp expected current
+   test_i18ncmp expect current
 '
 
 test_expect_success 'diff --shortstat output for binary file change only' '
-- 
1.7.10.2.552.gaa3bb87
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[ANNOUNCE] Git v1.7.11.2

The latest maintenance release Git v1.7.11.2 is now available at
the usual places.

The release tarballs are found at:

http://code.google.com/p/git-core/downloads/list

and their SHA-1 checksums are:

f67b4f6c0277250411c6872ae7b8a872ae11d313  git-1.7.11.2.tar.gz
088996c301cca24360fd5e30ce66bfa26139fe95  git-htmldocs-1.7.11.2.tar.gz
78b46ca7b5037c61a58086879869dadeac9eea3e  git-manpages-1.7.11.2.tar.gz

Also the following public repositories all have a copy of the v1.7.11.2
tag and the maint branch that the tag points at:

  url = git://repo.or.cz/alt-git.git
  url = https://code.google.com/p/git-core/
  url = git://git.sourceforge.jp/gitroot/git-core/git.git
  url = git://git-core.git.sourceforge.net/gitroot/git-core/git-core
  url = https://github.com/gitster/git

Git v1.7.11.2 Release Notes
===

Fixes since v1.7.11.1
-

 * On Cygwin, the platform pread(2) is not thread safe, just like our
   own compat/ emulation, and cannot be used in the index-pack
   program.  Makefile variable NO_THREAD_SAFE_PREAD can be defined to
   avoid use of this function in a threaded program.

 * git add allows adding a regular file to the path where a
   submodule used to exist, but git update-index does not allow an
   equivalent operation to Porcelain writers.

 * git archive incorrectly computed the header checksum; the symptom
   was observed only when using pathnames with hi-bit set.

 * git blame did not try to make sure that the abbreviated commit
   object names in its output are unique.

 * Running git bundle verify on a bundle that records a complete
   history said it requires these 0 commits.

 * git clone --single-branch to clone a single branch did not limit
   the cloning to the specified branch.

 * git diff --no-index did not correctly handle relative paths and
   did not correctly give exit codes when run under --quiet option.

 * git diff --no-index did not work with pagers correctly.

 * git diff COPYING HEAD:COPYING gave a nonsense error message that
   claimed that the treeish HEAD did not have COPYING in it.

 * When git log gets --simplify-merges/by-decoration together with
   --first-parent, the combination of these options makes the
   simplification logic to use in-core commit objects that haven't
   been examined for relevance, either producing incorrect result or
   taking too long to produce any output.  Teach the simplification
   logic to ignore commits that the first-parent traversal logic
   ignored when both are in effect to work around the issue.

 * git ls-files --exclude=t -i did not consider anything under t/ as
   excluded, as it did not pay attention to exclusion of leading paths
   while walking the index.  Other two users of excluded() are also
   updated.

 * git request-pull $url dev when the tip of dev branch was tagged
   with ext4-for-linus used the contents from the tag in the output
   but still asked the dev branch to be pulled, not the tag.

Also contains minor typofixes and documentation updates.



Changes since v1.7.11.1 are as follows:

Carlos Martín Nieto (2):
  Documentation: --no-walk is no-op if range is specified
  git-cherry-pick.txt: clarify the use of revision range notation

Heiko Voigt (1):
  update-index: allow overwriting existing submodule index entries

Jeff King (3):
  fix pager.diff with diff --no-index
  do not run pager with diff --no-index --quiet
  diff: handle relative paths in no-index

Junio C Hamano (15):
  request-pull: really favor a matching tag
  ls-files -i: pay attention to exclusion of leading paths
  ls-files -i: micro-optimize path_excluded()
  tweak bundle verify of a complete history
  path_excluded(): update API to less cache-entry centric
  builtin/add.c: use path_excluded()
  unpack-trees.c: use path_excluded() in check_ok_to_remove()
  dir.c: make excluded() file scope static
  revision: simplify options imply topo-order sort
  revision: note the lack of free() in simplify_merges()
  archive: ustar header checksum is computed unsigned
  revision: ignore side parents while running simplify-merges
  index-pack: Disable threading on cygwin
  blame: compute abbreviation width that ensures uniqueness
  Git 1.7.11.2

Leila Muhtasib (1):
  Documentation: Fix misspellings

Matthieu Moy (2):
  sha1_name: do not trigger detailed diagnosis for file arguments
  verify_filename(): ask the caller to chose the kind of diagnosis

Michał Górny (1):
  git-submodule.sh: fix filename in comment.

Nguyễn Thái Ngọc Duy (1):
  clone: fix ref selection in --single-branch --branch=xxx

Peter Krefting (1):
  Update Swedish translation (1066t0f0u)

Thomas Badie (1):
  git-add--interactive.perl: Remove two unused variables

Tim Henigan (1):
  diff-no-index: exit(1) if 'diff --quiet repo file external file' 
finds changes

--
To

Re: [PATCH 6/6] t4012: Make --shortstat more robust

Alexander Strasser eclip...@gmx.net writes:

 The --shortstat test depends on the same scenario as the --stat
 test. Use the part of the same expected result for the --stat test
 to avoid duplicating it manually.

 diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh
 index 81a9e8c..a3f6030 100755
 --- a/t/t4012-diff-binary.sh
 +++ b/t/t4012-diff-binary.sh
 @@ -38,9 +38,9 @@ test_expect_success 'apply --stat output for binary file 
 change' '
  '
  
  test_expect_success 'diff --shortstat output for binary file change' '
 - echo  4 files changed, 2 insertions(+), 2 deletions(-) expected 
 + tail -1 expected expect 

(tail|head) -n 1 is preferred.  There are the same POSIX.1
violations in a handful of other scripts, 5526, 7502, 7800 and
9146.

   git diff --shortstat current 
 - test_i18ncmp expected current
 + test_i18ncmp expect current
  '
  
  test_expect_success 'diff --shortstat output for binary file change only' '

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

Re: [PATCH/RFC] git-svn: don't create master if another head exists

Marcin Owsiany mar...@owsiany.pl writes:

 Date: Sun, 24 Jun 2012 22:40:05 +0100
 Subject: [PATCH] git-svn: don't create master if another head exists

 git-svn insists on creating the master head (unless it exists) on every
 fetch. It is useful that it gets created initially, when no head exists
 - users expect this git convention of having a master branch on initial
 clone.

 However creating it when there already is another head does not provide any
 value - the ref is never updated, so it just gets stale after a while.  Also,
 some users find it annoying that it gets recreated, especially when they would
 like the git branch names to follow SVN repository branch names. More
 background in http://thread.gmane.org/gmane.comp.version-control.git/115030

 Make git-svn skip the master creation if HEAD points at a valid head. This
 means master does get created on initial clone but does not get recreated
 once a user deletes it.

The above description makes sense to me, but the code updated with
this patch doesn't quite make sense to me.

This is your patch with a bit wider context.

 diff --git a/git-svn.perl b/git-svn.perl
 index 0b074c4..2379a71 100755
 --- a/git-svn.perl
 +++ b/git-svn.perl
 @@ -1599,35 +1599,35 @@ sub rebase_cmd {
  sub post_fetch_checkout {
 return if $_no_checkout;
 my $gs = $Git::SVN::_head or return;
 return if verify_ref('refs/heads/master^0');

Does master matter here?

I am wondering why this is not

return if verify_ref(HEAD^0);

Moreover, since the code will check verify_ref(HEAD^0) anyway in
the place you updated, is this early return still necessary?

 # look for trunk ref if it exists
 my $remote = Git::SVN::read_all_remotes()-{$gs-{repo_id}};
 my $fetch = $remote-{fetch};
 if ($fetch) {
 foreach my $p (keys %$fetch) {
 basename($fetch-{$p}) eq 'trunk' or next;
 $gs = Git::SVN-new($fetch-{$p}, $gs-{repo_id}, $p);
 last;
 }
 }

 - my $valid_head = verify_ref('HEAD^0');
 + return if verify_ref('HEAD^0');

This one matches the description.  When post_fetch_checkout() is
called, if HEAD is already pointing at a valid commit, we do not
want to run checkout (or create a ref).

 command_noisy(qw(update-ref refs/heads/master), $gs-refname);
 - return if ($valid_head || !verify_ref('HEAD^0'));
 + return unless verify_ref('HEAD^0');

I do not understand these three lines.  Why aren't they like this?

command_noisy(qw(update-ref HEAD), $gs-refname) || return;

That is, in a fresh repository whose HEAD points at an unborn
'master', nothing changes from the current behaviour.  If a fresh
repository whose HEAD points at some other unborn branch, should the
code still want to update 'master'?  Wouldn't we rather want to
update that branch?

If the caller does not handle errors, it could be even clearer to
write it like

command_noisy(qw(update-ref HEAD), $gs-refname) ||
die Cannot update HEAD!!!;

 return if $ENV{GIT_DIR} !~ m#^(?:.*/)?\.git$#;
 my $index = $ENV{GIT_INDEX_FILE} || $ENV{GIT_DIR}/index;
 return if -f $index;

 return if command_oneline(qw/rev-parse --is-inside-work-tree/) eq 
 'false';
 return if command_oneline(qw/rev-parse --is-inside-git-dir/) eq 
 'true';
 command_noisy(qw/read-tree -m -u -v HEAD HEAD/);
 print STDERR Checked out HEAD:\n  ,
  $gs-full_url,  r, $gs-last_rev, \n;
 if (auto_create_empty_directories($gs)) {
 $gs-mkemptydirs($gs-last_rev);
 }
  }
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Mini bug report origin/pu: t1512 failed on Mac OS X (commit 957d74062c1f0e ?)

Torsten Bögershausen tbo...@web.de writes:

 The following tweak will make t1512 work on my Mac OS box:


 --- a/t/t1512-rev-parse-disambiguation.sh
 +++ b/t/t1512-rev-parse-disambiguation.sh
 @@ -257,7 +257,7 @@ test_expect_success 'rev-parse --disambiguate' '
 # commits created by commit-tree in earlier tests do not share
 # the prefix.
 git rev-parse --disambiguate=0 actual 
 -   test $(wc -l actual) = 16 
 +   test $(wc -l actual) -eq  16 

I think the other tests in t/ prefer to unquote it so that we would
ignore spaces around wc -l output, i.e.

test $(wc -l actual) = 16

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

Re: Mini bug report origin/pu: t1512 failed on Mac OS X (commit 957d74062c1f0e ?)