* [PATCH 2/6] t5516 (fetch-push): update test description
From: Ramkumar Ramachandra @ 2013-03-20 12:44 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <1363783501-27981-1-git-send-email-artagnon@gmail.com>
The file was originally created in bcdb34f (Test wildcard push/fetch,
2007-06-08), and only contained tests that exercised wildcard
functionality at the time. In subsequent commits, many other tests
unrelated to wildcards were added but the test description was never
updated. Fix this.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t5516-fetch-push.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index c31e5c1..bfeec60 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-test_description='fetching and pushing, with or without wildcard'
+test_description='fetching and pushing'
. ./test-lib.sh
--
1.8.2
^ permalink raw reply related
* [PATCH 1/6] remote.c: simplify a bit of code using git_config_string()
From: Ramkumar Ramachandra @ 2013-03-20 12:44 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <1363783501-27981-1-git-send-email-artagnon@gmail.com>
A small segment where handle_config() parses the branch.remote
configuration variable can be simplified using git_config_string().
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
remote.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index e53a6eb..45b69d6 100644
--- a/remote.c
+++ b/remote.c
@@ -356,9 +356,7 @@ static int handle_config(const char *key, const char *value, void *cb)
return 0;
branch = make_branch(name, subkey - name);
if (!strcmp(subkey, ".remote")) {
- if (!value)
- return config_error_nonbool(key);
- branch->remote_name = xstrdup(value);
+ git_config_string(&branch->remote_name, key, value);
if (branch == current_branch) {
default_remote_name = branch->remote_name;
explicit_default_remote_name = 1;
--
1.8.2
^ permalink raw reply related
* [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
From: Ramkumar Ramachandra @ 2013-03-20 12:44 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <1363783501-27981-1-git-send-email-artagnon@gmail.com>
Currently, do_push() in push.c calls remote_get(), which gets the
configured remote for fetching and pushing. Replace this call with a
call to pushremote_get() instead, a new function that will return the
remote configured specifically for pushing. This function tries to
work with the string pushremote_name, before falling back to the
codepath of remote_get(). This patch has no visible impact, but
serves to enable future patches to introduce configuration variables
to set this variable.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/push.c | 2 +-
remote.c | 25 +++++++++++++++++++++----
remote.h | 1 +
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index 42b129d..d447a80 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -322,7 +322,7 @@ static int push_with_options(struct transport *transport, int flags)
static int do_push(const char *repo, int flags)
{
int i, errs;
- struct remote *remote = remote_get(repo);
+ struct remote *remote = pushremote_get(repo);
const char **url;
int url_nr;
diff --git a/remote.c b/remote.c
index 45b69d6..cf7a65d 100644
--- a/remote.c
+++ b/remote.c
@@ -48,6 +48,7 @@ static int branches_nr;
static struct branch *current_branch;
static const char *default_remote_name;
+static const char *pushremote_name;
static int explicit_default_remote_name;
static struct rewrites rewrites;
@@ -669,17 +670,21 @@ static int valid_remote_nick(const char *name)
return !strchr(name, '/'); /* no slash */
}
-struct remote *remote_get(const char *name)
+static struct remote *remote_get_1(const char *name, const char *pushremote_name)
{
struct remote *ret;
int name_given = 0;
- read_config();
if (name)
name_given = 1;
else {
- name = default_remote_name;
- name_given = explicit_default_remote_name;
+ if (pushremote_name) {
+ name = pushremote_name;
+ name_given = 1;
+ } else {
+ name = default_remote_name;
+ name_given = explicit_default_remote_name;
+ }
}
ret = make_remote(name, 0);
@@ -698,6 +703,18 @@ struct remote *remote_get(const char *name)
return ret;
}
+struct remote *remote_get(const char *name)
+{
+ read_config();
+ return remote_get_1(name, NULL);
+}
+
+struct remote *pushremote_get(const char *name)
+{
+ read_config();
+ return remote_get_1(name, pushremote_name);
+}
+
int remote_is_configured(const char *name)
{
int i;
diff --git a/remote.h b/remote.h
index 251d8fd..99a437f 100644
--- a/remote.h
+++ b/remote.h
@@ -51,6 +51,7 @@ struct remote {
};
struct remote *remote_get(const char *name);
+struct remote *pushremote_get(const char *name);
int remote_is_configured(const char *name);
typedef int each_remote_fn(struct remote *remote, void *priv);
--
1.8.2
^ permalink raw reply related
* [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
From: Ramkumar Ramachandra @ 2013-03-20 12:44 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <1363783501-27981-1-git-send-email-artagnon@gmail.com>
mk_test() creates a repository with the constant name "testrepo", and
this may be limiting for tests that need to create more than one
repository for testing. To fix this, create a new mk_test_with_name()
which accepts the repository name as $1. Reimplement mk_test() as a
special case of this function, making sure that no tests need to be
rewritten. Do the same thing for check_push_result().
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
t/t5516-fetch-push.sh | 34 +++++++++++++++++++++++++---------
1 file changed, 25 insertions(+), 9 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index bfeec60..a546c2c 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -7,27 +7,32 @@ test_description='fetching and pushing'
D=`pwd`
mk_empty () {
- rm -fr testrepo &&
- mkdir testrepo &&
+ repo_name="$1"
+ test -z "$repo_name" && repo_name=testrepo
+ rm -fr $repo_name &&
+ mkdir $repo_name &&
(
- cd testrepo &&
+ cd $repo_name &&
git init &&
git config receive.denyCurrentBranch warn &&
mv .git/hooks .git/hooks-disabled
)
}
-mk_test () {
- mk_empty &&
+mk_test_with_name () {
+ repo_name="$1"
+ shift
+
+ mk_empty $repo_name &&
(
for ref in "$@"
do
- git push testrepo $the_first_commit:refs/$ref || {
+ git push $repo_name $the_first_commit:refs/$ref || {
echo "Oops, push refs/$ref failure"
exit 1
}
done &&
- cd testrepo &&
+ cd $repo_name &&
for ref in "$@"
do
r=$(git show-ref -s --verify refs/$ref) &&
@@ -40,6 +45,10 @@ mk_test () {
)
}
+mk_test () {
+ mk_test_with_name testrepo "$@"
+}
+
mk_test_with_hooks() {
mk_test "$@" &&
(
@@ -79,9 +88,12 @@ mk_child() {
git clone testrepo "$1"
}
-check_push_result () {
+check_push_result_with_name () {
+ repo_name="$1"
+ shift
+
(
- cd testrepo &&
+ cd $repo_name &&
it="$1" &&
shift
for ref in "$@"
@@ -96,6 +108,10 @@ check_push_result () {
)
}
+check_push_result () {
+ check_push_result_with_name testrepo "$@"
+}
+
test_expect_success setup '
>path1 &&
--
1.8.2
^ permalink raw reply related
* [PATCH 5/6] remote.c: introduce remote.pushdefault
From: Ramkumar Ramachandra @ 2013-03-20 12:45 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <1363783501-27981-1-git-send-email-artagnon@gmail.com>
This new configuration variable defines the default remote to push to,
and overrides `branch.<name>.remote` for all branches. It is useful
in the typical triangular-workflow setup, where the remote you're
fetching from is different from the remote you're pushing to.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/config.txt | 13 ++++++++++---
remote.c | 4 ++++
t/t5516-fetch-push.sh | 12 ++++++++++++
3 files changed, 26 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index bbba728..e813c33 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -723,9 +723,12 @@ branch.autosetuprebase::
This option defaults to never.
branch.<name>.remote::
- When in branch <name>, it tells 'git fetch' and 'git push' which
- remote to fetch from/push to. It defaults to `origin` if no remote is
- configured. `origin` is also used if you are not on any branch.
+ When on branch <name>, it tells 'git fetch' and 'git push'
+ which remote to fetch from/push to. The remote to push to
+ may be overridden with `remote.pushdefault` (for all branches).
+ If no remote is configured, or if you are not on any branch,
+ it defaults to `origin` for fetching and `remote.pushdefault`
+ for pushing.
branch.<name>.merge::
Defines, together with branch.<name>.remote, the upstream branch
@@ -1894,6 +1897,10 @@ receive.updateserverinfo::
If set to true, git-receive-pack will run git-update-server-info
after receiving data from git-push and updating refs.
+remote.pushdefault::
+ The remote to push to by default. Overrides
+ `branch.<name>.remote` for all branches.
+
remote.<name>.url::
The URL of a remote repository. See linkgit:git-fetch[1] or
linkgit:git-push[1].
diff --git a/remote.c b/remote.c
index cf7a65d..68056c7 100644
--- a/remote.c
+++ b/remote.c
@@ -350,6 +350,10 @@ static int handle_config(const char *key, const char *value, void *cb)
const char *subkey;
struct remote *remote;
struct branch *branch;
+ if (!prefixcmp(key, "remote.")) {
+ if (!strcmp(key + 7, "pushdefault"))
+ git_config_string(&pushremote_name, key, value);
+ }
if (!prefixcmp(key, "branch.")) {
name = key + 7;
subkey = strrchr(name, '.');
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index a546c2c..63171f1 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -517,6 +517,18 @@ test_expect_success 'push with config remote.*.push = HEAD' '
git config --remove-section remote.there
git config --remove-section branch.master
+test_expect_success 'push with remote.pushdefault' '
+ mk_test_with_name up_repo heads/frotz &&
+ mk_test_with_name down_repo heads/master &&
+ test_config remote.up.url up_repo &&
+ test_config remote.down.url down_repo &&
+ test_config branch.master.remote up &&
+ test_config remote.pushdefault down &&
+ git push &&
+ test_must_fail check_push_result_with_name up_repo $the_commit heads/master &&
+ check_push_result_with_name down_repo $the_commit heads/master
+'
+
test_expect_success 'push with config remote.*.pushurl' '
mk_test heads/master &&
--
1.8.2
^ permalink raw reply related
* [PATCH 6/6] remote.c: introduce branch.<name>.pushremote
From: Ramkumar Ramachandra @ 2013-03-20 12:45 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <1363783501-27981-1-git-send-email-artagnon@gmail.com>
This new configuration variable overrides `remote.pushdefault` and
`branch.<name>.remote` for pushes. In a typical triangular-workflow
setup, you would want to set `remote.pushdefault` to specify the
remote to push to for all branches, and use this option to override it
for a specific branch.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/config.txt | 18 ++++++++++++++----
remote.c | 3 +++
t/t5516-fetch-push.sh | 15 +++++++++++++++
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index e813c33..4b9647a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -726,9 +726,18 @@ branch.<name>.remote::
When on branch <name>, it tells 'git fetch' and 'git push'
which remote to fetch from/push to. The remote to push to
may be overridden with `remote.pushdefault` (for all branches).
- If no remote is configured, or if you are not on any branch,
- it defaults to `origin` for fetching and `remote.pushdefault`
- for pushing.
+ The remote to push to, for the current branch, may be further
+ overridden by `branch.<name>.pushremote`. If no remote is
+ configured, or if you are not on any branch, it defaults to
+ `origin` for fetching and `remote.pushdefault` for pushing.
+
+branch.<name>.pushremote::
+ When on branch <name>, it overrides `branch.<name>.remote`
+ when pushing. It also overrides `remote.pushdefault` when
+ pushing from branch <name>. In a typical triangular-workflow
+ setup, you would want to set `remote.pushdefault` to specify
+ the remote to push to for all branches, and use this option to
+ override it for a specific branch.
branch.<name>.merge::
Defines, together with branch.<name>.remote, the upstream branch
@@ -1899,7 +1908,8 @@ receive.updateserverinfo::
remote.pushdefault::
The remote to push to by default. Overrides
- `branch.<name>.remote` for all branches.
+ `branch.<name>.remote` for all branches, and is overridden by
+ `branch.<name>.pushremote` for specific branches.
remote.<name>.url::
The URL of a remote repository. See linkgit:git-fetch[1] or
diff --git a/remote.c b/remote.c
index 68056c7..c988168 100644
--- a/remote.c
+++ b/remote.c
@@ -366,6 +366,9 @@ static int handle_config(const char *key, const char *value, void *cb)
default_remote_name = branch->remote_name;
explicit_default_remote_name = 1;
}
+ } else if (!strcmp(subkey, ".pushremote")) {
+ if (branch == current_branch)
+ git_config_string(&pushremote_name, key, value);
} else if (!strcmp(subkey, ".merge")) {
if (!value)
return config_error_nonbool(key);
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 63171f1..3f91874 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -539,6 +539,21 @@ test_expect_success 'push with config remote.*.pushurl' '
check_push_result $the_commit heads/master
'
+test_expect_success 'push with config branch.*.pushremote' '
+ mk_test_with_name up_repo heads/frotz &&
+ mk_test_with_name side_repo heads/quux &&
+ mk_test_with_name down_repo heads/master &&
+ test_config remote.up.url up_repo &&
+ test_config remote.pushdefault side_repo &&
+ test_config remote.down.url down_repo &&
+ test_config branch.master.remote up &&
+ test_config branch.master.pushremote down &&
+ git push &&
+ test_must_fail check_push_result_with_name up_repo $the_commit heads/master &&
+ test_must_fail check_push_result_with_name side_repo $the_commit heads/master &&
+ check_push_result_with_name down_repo $the_commit heads/master
+'
+
# clean up the cruft left with the previous one
git config --remove-section remote.there
--
1.8.2
^ permalink raw reply related
* Re: [PATCH 6/6] remote.c: introduce branch.<name>.pushremote
From: Tay Ray Chuan @ 2013-03-20 13:03 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Jeff King, Eric Sunshine,
Jonathan Nieder
In-Reply-To: <1363783501-27981-7-git-send-email-artagnon@gmail.com>
On Wed, Mar 20, 2013 at 8:45 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> This new configuration variable overrides `remote.pushdefault` and
> `branch.<name>.remote` for pushes. In a typical triangular-workflow
> setup, you would want to set `remote.pushdefault` to specify the
> remote to push to for all branches, and use this option to override it
> for a specific branch.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> Documentation/config.txt | 18 ++++++++++++++----
> remote.c | 3 +++
> t/t5516-fetch-push.sh | 15 +++++++++++++++
> 3 files changed, 32 insertions(+), 4 deletions(-)
Shouldn't this patch be squashed into 5/6 because of...
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index e813c33..4b9647a 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -726,9 +726,18 @@ branch.<name>.remote::
> When on branch <name>, it tells 'git fetch' and 'git push'
> which remote to fetch from/push to. The remote to push to
> may be overridden with `remote.pushdefault` (for all branches).
> - If no remote is configured, or if you are not on any branch,
> - it defaults to `origin` for fetching and `remote.pushdefault`
> - for pushing.
> + The remote to push to, for the current branch, may be further
> + overridden by `branch.<name>.pushremote`. If no remote is
> + configured, or if you are not on any branch, it defaults to
> + `origin` for fetching and `remote.pushdefault` for pushing.
> +
...this? (Since this description was introduced in 5/6)
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [PATCH v2 0/6] Support triangular workflows
From: Tay Ray Chuan @ 2013-03-20 13:06 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git List, Junio C Hamano, Jeff King, Eric Sunshine,
Jonathan Nieder
In-Reply-To: <1363783501-27981-1-git-send-email-artagnon@gmail.com>
On Wed, Mar 20, 2013 at 8:44 PM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> remote.c: introduce remote.pushdefault
> remote.c: introduce branch.<name>.pushremote
Perhaps we should clarify how this differs from remote.pushurl in the
documentation for it, in git-config and/or git-push. Maybe even
include the design decisions behind it from [1]. :)
http://thread.gmane.org/gmane.comp.version-control.git/215702/focus=215717
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [PATCH 6/6] remote.c: introduce branch.<name>.pushremote
From: Ramkumar Ramachandra @ 2013-03-20 13:35 UTC (permalink / raw)
To: Tay Ray Chuan
Cc: Git List, Junio C Hamano, Jeff King, Eric Sunshine,
Jonathan Nieder
In-Reply-To: <CALUzUxobybPOqsLgEFVOCK2OLOvyqHtAiuyi8wozOSYeWzkhNg@mail.gmail.com>
Tay Ray Chuan wrote:
> On Wed, Mar 20, 2013 at 8:45 PM, Ramkumar Ramachandra
> <artagnon@gmail.com> wrote:
>> This new configuration variable overrides `remote.pushdefault` and
>> `branch.<name>.remote` for pushes. In a typical triangular-workflow
>> setup, you would want to set `remote.pushdefault` to specify the
>> remote to push to for all branches, and use this option to override it
>> for a specific branch.
>>
>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>> ---
>> Documentation/config.txt | 18 ++++++++++++++----
>> remote.c | 3 +++
>> t/t5516-fetch-push.sh | 15 +++++++++++++++
>> 3 files changed, 32 insertions(+), 4 deletions(-)
>
> Shouldn't this patch be squashed into 5/6 because of...
>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index e813c33..4b9647a 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -726,9 +726,18 @@ branch.<name>.remote::
>> When on branch <name>, it tells 'git fetch' and 'git push'
>> which remote to fetch from/push to. The remote to push to
>> may be overridden with `remote.pushdefault` (for all branches).
>> - If no remote is configured, or if you are not on any branch,
>> - it defaults to `origin` for fetching and `remote.pushdefault`
>> - for pushing.
>> + The remote to push to, for the current branch, may be further
>> + overridden by `branch.<name>.pushremote`. If no remote is
>> + configured, or if you are not on any branch, it defaults to
>> + `origin` for fetching and `remote.pushdefault` for pushing.
>> +
>
> ...this? (Since this description was introduced in 5/6)
Huh? This patch introduces branch.<name>.pushremote: the relevant
code and documentation changes. 5/6 introduces remote.pushdefault,
which is completely different.
^ permalink raw reply
* Re: git branch based hook desigh
From: Magnus Bäck @ 2013-03-20 13:46 UTC (permalink / raw)
To: Joydeep Bakshi; +Cc: git
In-Reply-To: <148C5E2E-8FD6-4686-A009-57D1F403C808@infoservices.in>
On Monday, March 18, 2013 at 00:27 EDT,
Joydeep Bakshi <joydeep.bakshi@infoservices.in> wrote:
> I have implemented git pre-received hook successfully. And it works on
> the repo level.
> Could anyone suggest how to call branch level hook please ?--
If you need to have different hook behaviors depending on the ref that's
about to be updated, just put a conditional in the hook. You get the
name(s) of ref(s) to be updated on stdin.
--
Magnus Bäck
baeck@google.com
^ permalink raw reply
* Re: [PATCH] push: Alias pushurl from push rewrites
From: Junio C Hamano @ 2013-03-20 14:35 UTC (permalink / raw)
To: Rob Hoelz; +Cc: Jonathan Nieder, git, josh
In-Reply-To: <5149AC88.9090206@hoelz.ro>
Rob Hoelz <rob@hoelz.ro> writes:
> On 3/19/13 7:08 PM, Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>
>>> Junio C Hamano wrote:
>>>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>>>> Test nits:
>>>>> ...
>>>>> Hope that helps,
>>>>>
>>>>> Jonathan Nieder (3):
>>>>> push test: use test_config where appropriate
>>>>> push test: simplify check of push result
>>>>> push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'
>>>> Are these supposed to be follow-up patches? Preparatory steps that
>>>> Rob can reroll on top? Something else?
>>> Preparatory steps.
>> OK, thanks. I'll queue these first then.
>>
> Should I apply these to my patch to move things along? What's the next
> step for me?
You would fetch from nearby git.git mirror, find the tip of
Janathan's series and redo your patch on top. Perhaps the session
would go like this:
$ git fetch git://git.kernel.org/pub/scm/git/git.git/ pu
$ git log --oneline --first-parent ..FETCH_HEAD | grep jn/push-tests
83a072a Merge branch 'jn/push-tests' into pu
$ git checkout -b rh/push-pushurl-pushinsteadof 83a072a
... redo the work, perhaps with combinations of:
$ git cherry-pick -n $your_original_branch
$ edit t/t5516-fetch-push.sh
... and then
$ make test
$ git commit
^ permalink raw reply
* Re: [PATCH v3 0/5] nd/branch-show-rebase-bisect-state updates
From: Junio C Hamano @ 2013-03-20 14:37 UTC (permalink / raw)
To: Duy Nguyen; +Cc: git, Matthieu Moy, Jonathan Niedier
In-Reply-To: <CACsJy8C93OOLWW2tdh17XA1k3Zs_kUjyZ32v4Ywk2BK83BL3wQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
>> I think the message should better say "before you started to bisect
>> you were on 'jch'" or something instead.
>
> How about
>
> * (no branch, bisect started on jch)
>
> then?
Sure.
What you are showing is used only to decide where "git bisect reset"
goes to, I think.
^ permalink raw reply
* Bug? git show behaves incorrectly when called on a stash object
From: Kirill Likhodedov @ 2013-03-20 15:08 UTC (permalink / raw)
To: git
Hi,
I experience difference in behavior between `git stash show stash@{0}` and `git show stash@{0}`:
-----------------------------------------------------
# git stash show --name-status stash@{0}
A a
M conflict.js
# git show --name-status stash@{0}
commit 05aa170746dc4e060f10710a92aed4592276f2d4
Merge: 737f544 7c2160f
Author: Kirill Likhodedov <Kirill.Likhodedov@gmail.com>
Date: Wed Mar 20 18:56:28 2013 +0400
WIP on master: 737f544 master
MM conflict.js
-----------------------------------------------------
The first gives the correct result, while the second doesn't show the addition.
Is it a bug, or I'm just missing something?
Thanks a lot!
Kirill.
^ permalink raw reply
* Re: [PATCH 4/6] add -u: only show pathless 'add -u' warning when changes exist outside cwd
From: Junio C Hamano @ 2013-03-20 15:10 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Matthieu Moy, Jeff King, git,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20130319225050.GE19014@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> A common workflow in large projects is to chdir into a subdirectory of
> interest and only do work there:
>
> cd src
> vi foo.c
> make test
> git add -u
> git commit
>
> The upcoming change to 'git add -u' behavior would not affect such a
> workflow: when the only changes present are in the current directory,
> 'git add -u' will add all changes, and whether that happens via an
> implicit "." or implicit ":/" parameter is an unimportant
> implementation detail.
>
> The warning about use of 'git add -u' with no pathspec is annoying
> because it seemingly serves no purpose in this case. So suppress the
> warning unless there are changes outside the cwd that are not being
> added.
>
> A previous version of this patch ran two I/O-intensive diff-files
> passes: one to find changes outside the cwd, and another to find
> changes to add to the index within the cwd. This version runs one
> full-tree diff and decides for each change whether to add it or warn
> and suppress it in update_callback. As a result, even on very large
> repositories "git add -u" will not be significantly slower than the
> future default behavior ("git add -u :/"), and the slowdown relative
> to "git add -u ." should be a useful clue to users of such
> repositories to get into the habit of explicitly passing '.'.
I'll queue this as-is for now, but the point Peff raised on
IMPLICIT_DOT needs to be addressed.
This is a tangent, but I would also suggest at least considering to
measure how big the working tree is relative to the area covered by
the implicit dot [*1*]. If you are running "add -u" in a project
with 30k paths but are working with only a few dozen files, you may
want more encouragement to use an explicit "." than the command
mysteriously and silently getting slower at Git version boundary.
IOW, an advice message issued only when (1) you are indeed working
in a narrow directory, (2) you did not touch anything outside, and
(3) you did not give an explicit ".". We would want to keep the
advice in place even after Git 2.0 switches the default to ":/". In
fact, it would become even more valuable after the transition. And
make it protected under advice.addUAuseexplicitdot or something.
By the way, I am not a big fan of the approach taken in [3/6]. It
may be a lot cleaner to separate the "do we need to warn?" logic
that flips a global (or a field in a callback, if we make some of
these codepaths callable from other places) and the code to issue
the warning, no?
[Footnote]
*1* The former you can get an approximation from active_nr, the
latter you can count after first finding where prefix would insert
to the active_cache with index_name_pos(). I suspect that you also
could add a new read-only API to cache-tree to see if it already
knows how many index entries a given subtree for the prefix covers.
^ permalink raw reply
* Re: [PATCH 5/6] add -A: only show pathless 'add -A' warning when changes exist outside cwd
From: Junio C Hamano @ 2013-03-20 15:30 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Matthieu Moy, Jeff King, git,
Nguyễn Thái Ngọc Duy
In-Reply-To: <20130319225134.GF19014@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> In the spirit of the recent similar change for 'git add -u', avoid
> pestering users that restrict their attention to a subdirectory and
> will not be affected by the coming change in the behavior of pathless
> 'git add -A'.
>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> As before.
Have you considered implementing this by adding a separate check
immediately after fill_directory() to inspect the paths in dir with
the same strncmp_icase() against the prefix, without touching
prune_directory() at all? I suspect that would be much cleaner and
easier to change in the version boundary.
Same comment about measuring the size of the working tree and the
area the user is working on applies to this. After Git 2.0, we
would still want to advise users who say "git add -A" without
pathspecs to see if the user would have been better off with an
explicit ".", so unless advice.addUAuseexplicitdot is set to false,
we would still want to inspect the result from fill_directory (and
in that case we won't be calling into prune_directory).
> builtin/add.c | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index 4d8d441..2493493 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -170,7 +170,9 @@ int add_files_to_cache(const char *prefix, const char **pathspec, int flags)
> return !!data.add_errors;
> }
>
> -static char *prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
> +#define WARN_IMPLICIT_DOT (1u << 0)
> +static char *prune_directory(struct dir_struct *dir, const char **pathspec,
> + int prefix, unsigned flag)
> {
> char *seen;
> int i, specs;
> @@ -187,6 +189,16 @@ static char *prune_directory(struct dir_struct *dir, const char **pathspec, int
> if (match_pathspec(pathspec, entry->name, entry->len,
> prefix, seen))
> *dst++ = entry;
> + else if (flag & WARN_IMPLICIT_DOT)
> + /*
> + * "git add -A" was run from a subdirectory with a
> + * new file outside that directory.
> + *
> + * "git add -A" will behave like "git add -A :/"
> + * instead of "git add -A ." in the future.
> + * Warn about the coming behavior change.
> + */
> + warn_pathless_add();
> }
> dir->nr = dst - dir->entries;
> add_pathspec_matches_against_index(pathspec, seen, specs);
> @@ -433,8 +445,6 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> }
> if (option_with_implicit_dot && !argc) {
> static const char *here[2] = { ".", NULL };
> - if (prefix && addremove)
> - warn_pathless_add();
> argc = 1;
> argv = here;
> implicit_dot = 1;
> @@ -475,9 +485,10 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> }
>
> /* This picks up the paths that are not tracked */
> - baselen = fill_directory(&dir, pathspec);
> + baselen = fill_directory(&dir, implicit_dot ? NULL : pathspec);
> if (pathspec)
> - seen = prune_directory(&dir, pathspec, baselen);
> + seen = prune_directory(&dir, pathspec, baselen,
> + implicit_dot ? WARN_IMPLICIT_DOT : 0);
> }
>
> if (refresh_only) {
^ permalink raw reply
* Re: Bug? git show behaves incorrectly when called on a stash object
From: Matthieu Moy @ 2013-03-20 15:56 UTC (permalink / raw)
To: Kirill Likhodedov; +Cc: git
In-Reply-To: <AF060B6D-27F7-45BE-9BC3-85FAF3487481@gmail.com>
Kirill Likhodedov <kirill.likhodedov@gmail.com> writes:
> The first gives the correct result, while the second doesn't show the addition.
> Is it a bug, or I'm just missing something?
stash objects are commits with 2 parents (ie. merge commits). One commit
is the HEAD you stashed from, and the other is the saved state of the
index. I'm not sure from the doc what the semantics of --name-status is
for merge commits, but it seems it shows only files modified in both
parents.
Use git show --first-parent --name-status, it should do what you expect
(or -m instead of --first-parent).
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [PATCH] sha1_file: remove recursion in packed_object_info
From: Junio C Hamano @ 2013-03-20 16:47 UTC (permalink / raw)
To: Thomas Rast
Cc: Stefan Zager, git, Jeff King,
Nguyễn Thái Ngọc Duy
In-Reply-To: <c5fc1d2040544965ad3cc09e7b82b6013f06b7fa.1363729774.git.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> So here's a nonrecursive version. Dijkstra is probably turning over
> in his grave as we speak.
>
> I *think* I actually got it right.
You seem to have lost the "if we cannot get delta base, this object
is BAD" check where you measure the size of a deltified object,
which would correspond to this check:
> -static int packed_delta_info(struct packed_git *p,
> - struct pack_window **w_curs,
> - off_t curpos,
> - enum object_type type,
> - off_t obj_offset,
> - unsigned long *sizep)
> -{
> - off_t base_offset;
> -
> - base_offset = get_delta_base(p, w_curs, &curpos, type, obj_offset);
> - if (!base_offset)
> - return OBJ_BAD;
The following comment is also lost but...
> - /* We choose to only get the type of the base object and
> - * ignore potentially corrupt pack file that expects the delta
> - * based on a base with a wrong size. This saves tons of
> - * inflate() calls.
> - */
> - if (sizep) {
> - *sizep = get_size_from_delta(p, w_curs, curpos);
> - if (*sizep == 0)
> - type = OBJ_BAD;
... is this check correct? There is an equivalent check at the
beginning of the new packed_object_info() to error out a deltified
result. Why is an object whose size is 0 bad?
This comes from 3d77d8774fc1 (make packed_object_info() resilient to
pack corruptions, 2008-10-29), and I tend to trust Nico more than I
do myself. I must be missing something obvious, but it appears to me
that the only thing that keeps us from triggering a false positive
is that we do not even attempt to deltify anything smaller than 50
bytes, and create_delta() refuses to create a delta to produce an
empty data. But a hand-crafted packfile could certainly record such
a delta, no?
> The task of the two functions is not all that hard to describe without
> any recursion, however. It proceeds in three steps:
>
> - determine the representation type and size, based on the outermost
> object (delta or not)
>
> - follow through the delta chain, if any
>
> - determine the object type from what is found at the end of the delta
> chain
The stack/recursion is used _only_ for error recovery, no? If we do
not care about retrying with a different copy of an object we find
in the delta chain, we can just update obj_offset with base_offset
and keep digging. It almost makes me wonder if a logical follow-up
to this patch may be to do so, and rewrite the error recovery
codepath to just mark the bad copy and jump back to the very top,
retrying everything from scratch. Eventually we would run out
bad copies of the problematic object and would report an error, or
find a good copy and return the type.
^ permalink raw reply
* Re: [RFC] Add posibility to preload stat information.
From: Jeff King @ 2013-03-20 16:48 UTC (permalink / raw)
To: Fredrik Gustafsson; +Cc: spearce, git, pclouds
In-Reply-To: <1363781732-11396-1-git-send-email-iveqy@iveqy.com>
On Wed, Mar 20, 2013 at 01:15:32PM +0100, Fredrik Gustafsson wrote:
> When entering a git working dir, optionally run a forked process that
> stat all files in the whole workdir and therefore loads stat information
> to RAM which will speedup things like git status and so on.
>
> The feature is optional and by default it's off.
Kind of gross, but I guess it is useful to some people.
> +__git_recursive_stat ()
> +{
> + if test ! -e /tmp/gitbash.lock
> + then
> + touch /tmp/gitbash.lock
This is a tmp-race security hole. E.g., do:
ln -s /etc/nologin /tmp/gitbash.lock
as a user; when root runs __git_recursive_stat, it will create
/etc/nologin. It's not quite as bad as some other holes, because we only
"touch" the file, not overwrite its contents, but you can see that it's
possible to do some mischief.
Should this maybe just be ~/.gitbash.lock or something?
> + cd $(git rev-parse --show-toplevel)
> + find . | xargs stat 2&> /dev/null
The "stat" utility is not portable. But why not use git to do the
reading? Then you can get the benefit of core.preloadindex, and you will
not recurse into untracked directories that are ignored (i.e.,
ones that git would not go into anyway).
So maybe just run "git status >/dev/null"?
-Peff
^ permalink raw reply
* Re: USE_NSEC bug?
From: Junio C Hamano @ 2013-03-20 17:04 UTC (permalink / raw)
To: Jeff King; +Cc: Andrew Rodland, git
In-Reply-To: <20130320075344.GA8159@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> But maybe there is some subtle reason I'm missing for having the two
> options separate.
The closest I found was c06ff4908bf9 (Record ns-timestamps if
possible, but do not use it without USE_NSEC, 2009-03-04).
commit c06ff4908bf9ad8bf2448439a3574321c9399b17
Author: Kjetil Barvik <barvik@broadpark.no>
Date: Wed Mar 4 18:47:40 2009 +0100
Record ns-timestamps if possible, but do not use it without USE_NSEC
Traditionally, the lack of USE_NSEC meant "do not record nor use the
nanosecond resolution part of the file timestamps". To avoid problems on
filesystems that lose the ns part when the metadata is flushed to the disk
and then later read back in, disabling USE_NSEC has been a good idea in
general.
If you are on a filesystem without such an issue, it does not hurt to read
and store them in the cached stat data in the index entries even if your
git is compiled without USE_NSEC. The index left with such a version of
git can be read by git compiled with USE_NSEC and it can make use of the
nanosecond part to optimize the check to see if the path on the filesystem
hsa been modified since we last looked at.
^ permalink raw reply
* Re: [PATCH jk/pkt-line-cleanup] t5700-clone-reference: send trace to fd 2, not 3, to please Windows git
From: Jeff King @ 2013-03-20 17:06 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <51499DE7.2030404@viscovery.net>
On Wed, Mar 20, 2013 at 12:30:47PM +0100, Johannes Sixt wrote:
> > I think that is OK, but I'm curious why this is a problem _now_, and not
> > with the code prior to 97a83fa8. The old GIT_DEBUG_SEND_PACK was also
> > just calling write() to descriptor 3.
>
> Before this change, both affected commands completed and the trace file
> was empty. Notice that in both test cases we only check for the absence of
> certain lines, which is naturally true for an empty file, so that the
> tests pass.
Hmm. The code in t5503 is similar, but before my patch used to actually
use "test -s" to make sure that some trace output was written. Did it
fail before 97a83fa8 (and does it pass now)?
We should probably be adjusting t5503, too. And possibly adding back in
the "test -s" checks (which I removed as redundant, but I guess the
double-checking might have caught a platform error in this case).
Also, though I think your use of stderr is probably OK, might it be a
little more robust against future output changes to use:
GIT_TRACE_PACKET=$PWD/$U.D git clone ...
to write directly to the file. The original GIT_DEBUG_SEND_PACK did not
support such niceties, but GIT_TRACE gives them to us for free.
> With the updated code, 'git fetch' hung, which is how I noticed the
> problem. As I said, I didn't investigate where and why this happens.
Thinking on it more, almost certainly what is happening is that fd 3 is
being used for the actual protocol, and we are jamming random bytes over
it. Since we have changed the bytes, the code is reacting in a different
way (but the actual reaction doesn't matter; we should stop the cause,
not worry about the symptom).
-Peff
^ permalink raw reply
* Re: USE_NSEC bug?
From: Jeff King @ 2013-03-20 17:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Rodland, git
In-Reply-To: <7vppyuf1f5.fsf@alter.siamese.dyndns.org>
On Wed, Mar 20, 2013 at 10:04:14AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > But maybe there is some subtle reason I'm missing for having the two
> > options separate.
>
> The closest I found was c06ff4908bf9 (Record ns-timestamps if
> possible, but do not use it without USE_NSEC, 2009-03-04).
>
> commit c06ff4908bf9ad8bf2448439a3574321c9399b17
> Author: Kjetil Barvik <barvik@broadpark.no>
> Date: Wed Mar 4 18:47:40 2009 +0100
>
> Record ns-timestamps if possible, but do not use it without USE_NSEC
>
> Traditionally, the lack of USE_NSEC meant "do not record nor use the
> nanosecond resolution part of the file timestamps". To avoid problems on
> filesystems that lose the ns part when the metadata is flushed to the disk
> and then later read back in, disabling USE_NSEC has been a good idea in
> general.
>
> If you are on a filesystem without such an issue, it does not hurt to read
> and store them in the cached stat data in the index entries even if your
> git is compiled without USE_NSEC. The index left with such a version of
> git can be read by git compiled with USE_NSEC and it can make use of the
> nanosecond part to optimize the check to see if the path on the filesystem
> hsa been modified since we last looked at.
Thanks, I suspected that might be the reason. IMHO it is not a big win,
as you would just refresh the index when using the version compiled with
USE_NSEC (so it really only helps you if you are frequently switching
between the two versions, which seems silly).
And the cost is that we have another Makefile knob people need to tweak
that would not otherwise need to be there. Which can be annoying, but is
also not that huge a cost to deal with (we might want to improve the
configure script or something, though).
I admit I don't care too much either way.
-Peff
^ permalink raw reply
* Re: [PATCH jk/checkout-attribute-lookup] t2003: work around path mangling issue on Windows
From: Junio C Hamano @ 2013-03-20 17:10 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <514977BD.6060604@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> From: Johannes Sixt <j6t@kdbg.org>
>
> MSYS bash considers the part "/g" in the sed expression "s/./=/g" as an
> absolute path after an assignment, and mangles it to a C:/something
> string. Do not attract bash's attention by avoiding the equals sign.
If this breakage is about path mangling, I suspect it may be cleaner
to work it around by not using / as the pattern separator, e.g.
sed -e s!.!=!g
Or perhaps use SHELL_PATH to point at a more reasonable
implementation of shell that does not have such an idiocy?
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
> t/t2003-checkout-cache-mkdir.sh | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t2003-checkout-cache-mkdir.sh b/t/t2003-checkout-cache-mkdir.sh
> index 4c97468..ff163cf 100755
> --- a/t/t2003-checkout-cache-mkdir.sh
> +++ b/t/t2003-checkout-cache-mkdir.sh
> @@ -94,14 +94,14 @@ test_expect_success 'apply filter from working tree .gitattributes with --prefix
> rm -fr path0 path1 path2 tmp* &&
> mkdir path1 &&
> mkdir tmp &&
> - git config filter.replace-all.smudge "sed -e s/./=/g" &&
> + git config filter.replace-all.smudge "sed -e s/./,/g" &&
> git config filter.replace-all.clean cat &&
> git config filter.replace-all.required true &&
> echo "file1 filter=replace-all" >path1/.gitattributes &&
> git checkout-index --prefix=tmp/ -f -a &&
> echo frotz >expected &&
> test_cmp expected tmp/path0 &&
> - echo ====== >expected &&
> + echo ,,,,,, >expected &&
> test_cmp expected tmp/path1/file1
> '
^ permalink raw reply
* Re: [RFC] Add posibility to preload stat information.
From: Junio C Hamano @ 2013-03-20 17:15 UTC (permalink / raw)
To: Jeff King; +Cc: Fredrik Gustafsson, spearce, git, pclouds
In-Reply-To: <20130320164806.GA10752@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So maybe just run "git status >/dev/null"?
In the background? How often would it run? I do not think a single
lockfile solves anything. It may prevent simultaneous runs of two
such "prime the well" processes, but the same user may be working in
two separate repositories.
I do not see anything that prevents it from running in the same
repository over and over again, either. "prompt" is a bad place to
do this kind of thing.
^ permalink raw reply
* Re: [RFC] Add posibility to preload stat information.
From: Ramkumar Ramachandra @ 2013-03-20 17:15 UTC (permalink / raw)
To: Fredrik Gustafsson; +Cc: spearce, git, pclouds, Jeff King
In-Reply-To: <1363781732-11396-1-git-send-email-iveqy@iveqy.com>
Fredrik Gustafsson wrote:
> When entering a git working dir, optionally run a forked process that
> stat all files in the whole workdir and therefore loads stat information
> to RAM which will speedup things like git status and so on.
This is misleading. You just execute the equivalent of `git status`
everytime I request a prompt inside a git working directory. And this
is if I'm using __git_ps1() to augment my prompt, which I'm not- I use
ZSH's vcs_info, which is arguably better. Also, you forgot to say how
to turn on the feature.
That said, this feature is extremely gross; it thrashes my filesystem
and hard drive. Modern software is written to minimize IO, not
maximize it! I'm completely against the inclusion of this patch.
However, I would not mind a feature that runs `git status` the very
first time I enter a git working directory: when I enter my clone of
linux.git, it takes my first `git status` invocation a good ten
seconds to complete, and we can fix this pretty easily.
^ permalink raw reply
* Re: [PATCH jk/pkt-line-cleanup] t5700-clone-reference: send trace to fd 2, not 3, to please Windows git
From: Jeff King @ 2013-03-20 17:26 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Git Mailing List
In-Reply-To: <20130320170607.GB10752@sigill.intra.peff.net>
On Wed, Mar 20, 2013 at 01:06:07PM -0400, Jeff King wrote:
> On Wed, Mar 20, 2013 at 12:30:47PM +0100, Johannes Sixt wrote:
>
> > > I think that is OK, but I'm curious why this is a problem _now_, and not
> > > with the code prior to 97a83fa8. The old GIT_DEBUG_SEND_PACK was also
> > > just calling write() to descriptor 3.
> >
> > Before this change, both affected commands completed and the trace file
> > was empty. Notice that in both test cases we only check for the absence of
> > certain lines, which is naturally true for an empty file, so that the
> > tests pass.
>
> Hmm. The code in t5503 is similar, but before my patch used to actually
> use "test -s" to make sure that some trace output was written. Did it
> fail before 97a83fa8 (and does it pass now)?
Ah, I see. It did fail, and it was marked with NOT_MINGW. I think we can
fix that now. Patch coming in a moment.
-Peff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox