Re: [PATCH v3 3/4] t7800: modernize tests
Am 3/21/2013 8:41, schrieb Johannes Sixt:
Am 3/20/2013 23:59, schrieb David Aguilar:
I started digging in and the @worktree_files (aka @worktree above)
is populated from the output of git diff --raw
Seeing the output filename in diff --raw implies that one of the
tests added output to the index somehow. I do not see that
happening anywhere, though, so I do not know how it would end up in
the @worktree array if it is not reported by diff --raw.
My current understanding of how it could possibly be open twice:
1. via the output redirect
2. via the copy() perl code which is fed by @worktree
So I'm confused. Why would we get different results on Windows?
I tracked down the difference between Windows and Linux, and it is...
for my $file (@worktree) {
next if $symlinks -l $b/$file;
... this line in sub dir_diff. On Linux, we take the short-cut, but on
Windows we proceed through the rest of the loop,
And that is likely by design. From the docs:
--symlinks
--no-symlinks
git difftool's default behavior is create symlinks to the working
tree when run in --dir-diff mode.
Specifying `--no-symlinks` instructs 'git difftool' to create
copies instead. `--no-symlinks` is the default on Windows.
And indeed, we have this initialization:
my %opts = (
...
symlinks = $^O ne 'cygwin'
$^O ne 'MSWin32' $^O ne 'msys',
...
);
Can the --dir-diff tests case pass on Cygwin when neither --symlinks nor
--no-symlinks is passed?
Perhaps the right solution is this:
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index c6d6b1c..19238f6 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -328,14 +328,16 @@ test_expect_success PERL 'setup change in subdirectory' '
git commit -m modified both
'
-test_expect_success PERL 'difftool -d' '
- git difftool -d --extcmd ls branch output
+# passing --symlinks helps Cygwin, which defaults to --no-symlinks
+
+test_expect_success PERL,SYMLINKS 'difftool -d' '
+ git difftool -d --symlinks --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
'
-test_expect_success PERL 'difftool --dir-diff' '
- git difftool --dir-diff --extcmd ls branch output
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff' '
+ git difftool --dir-diff --symlinks --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
'
@@ -362,16 +364,16 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff
--symlink without unstage
test_cmp actual expect
'
-test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
- git difftool --dir-diff --prompt --extcmd ls branch output
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff ignores --prompt' '
+ git difftool --dir-diff --symlinks --prompt --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
'
-test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff from subdirectory' '
(
cd sub
- git difftool --dir-diff --extcmd ls branch output
+ git difftool --dir-diff --symlinks --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
)
(Only tested on MinGW, which skips the tests.) I leave it to you
to write --no-symlinks tests.
-- 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 v2 0/6] Support triangular workflows
Philip Oakley wrote: From: Ramkumar Ramachandra artag...@gmail.com Sent: Wednesday, March 20, 2013 12:44 PM This follows-up [1], with three important differences: 1. pushremote_get() and remote_get() share code better. Thanks Jeff. 2. All spelling mistakes have been corrected. Thanks Eric. 3. One new test for each of the new configuration variables. The extra two parts [2/6] and [3/6] preprare the file for introducing tests. However, I've not gone overboard in this preparation; I don't replicate the work done by Jonathan in [2]. Thanks for reading. [1]: http://thread.gmane.org/gmane.comp.version-control.git/218410 [2]: http://thread.gmane.org/gmane.comp.version-control.git/218451/focus=218465 Ramkumar Ramachandra (6): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description t5516 (fetch-push): introduce mk_test_with_name() remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.name.pushremote Documentation/config.txt | 23 +++--- Shouldn't Documentation/gitworkflows.txt also be updated with the triangular workflow and its configuration? Currently, Documentation/gitworkflows.txt makes no effort to explain how to set up configuration variables for centralized or distributed workflows. I don't see how I could patch the existing document to include this workflow, without changing the entire structure of the document (left as an exercise for future patches). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/6] Support triangular workflows
Tay Ray Chuan wrote: On Wed, Mar 20, 2013 at 8:44 PM, Ramkumar Ramachandra artag...@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 Actually, it's quite obvious when you think about it: remote.pushurl doesn't allow remote tracking branches, and this is a very serious limitation. If anything, the documentation for remote.pushurl should be updated to explain that it has a very narrow usecase (will do in a future patch). -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/6] Support triangular workflows
After Jonathan's review of [1], I've decided to pick two changes to apply in this iteration: 1. [1/6], [5/6] and [6/6] now use the return value of git_config_string(), hence handling configuration errors properly. I consider this a major and important improvement. 2. The commit message in [4/6] has been augmented with a small example. This is mainly a nit, as full-blown examples are already present in the next two patches in the series. Thanks. [1]: http://thread.gmane.org/gmane.comp.version-control.git/218598 Ramkumar Ramachandra (6): remote.c: simplify a bit of code using git_config_string() t5516 (fetch-push): update test description t5516 (fetch-push): introduce mk_test_with_name() remote.c: introduce a way to have different remotes for fetch/push remote.c: introduce remote.pushdefault remote.c: introduce branch.name.pushremote Documentation/config.txt | 23 +++--- builtin/push.c | 2 +- remote.c | 39 -- remote.h | 1 + t/t5516-fetch-push.sh| 63 5 files changed, 107 insertions(+), 21 deletions(-) -- 1.8.2.62.ga35d936.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/6] remote.c: simplify a bit of code using git_config_string()
A small segment where handle_config() parses the branch.remote
configuration variable can be simplified using git_config_string().
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
remote.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index e53a6eb..5bd59bb 100644
--- a/remote.c
+++ b/remote.c
@@ -356,9 +356,8 @@ 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);
+ if (git_config_string(branch-remote_name, key, value))
+ return -1;
if (branch == current_branch) {
default_remote_name = branch-remote_name;
explicit_default_remote_name = 1;
--
1.8.2.62.ga35d936.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/6] t5516 (fetch-push): update test description
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 artag...@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.62.ga35d936.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
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 artag...@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.62.ga35d936.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
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 pushremote_name. For example, you can now do the following in
handle_config():
if (!strcmp(key, remote.pushdefault))
git_config_string(pushremote_name, key, value);
Then, pushes will automatically go to the remote specified by
remote.pushdefault.
Signed-off-by: Ramkumar Ramachandra artag...@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 5bd59bb..185ac11 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;
@@ -670,17 +671,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);
@@ -699,6 +704,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.62.ga35d936.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/6] remote.c: introduce remote.pushdefault
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 artag...@gmail.com
---
Documentation/config.txt | 13 ++---
remote.c | 5 +
t/t5516-fetch-push.sh| 12
3 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index b3023b8..09a8294 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 185ac11..bdb542c 100644
--- a/remote.c
+++ b/remote.c
@@ -350,6 +350,11 @@ 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))
+ if (git_config_string(pushremote_name, key, value))
+ return -1;
+ }
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.62.ga35d936.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] rebase -i: edit versus unmerged changes
Junio C Hamano wrote: Ramkumar Ramachandra artag...@gmail.com writes: Andrew Wong wrote: On 3/19/13, Ramkumar Ramachandra artag...@gmail.com wrote: I know that this is expected behavior, but is there an easy way to get rid of this inconsistency? You can actually rely on rebase to run the appropriate command. Didn't Junio explicitly mention that this is undesirable earlier (from the point of view of `rebase -i` design)? I am sure it is my fault but my memory fails me. As Andrew mentioned, rebase --continue seemed to get this right. After digging through the list, I did manage to find Junio's original message I was referring to [1]. This, along with other discussions has resulted in a sequencer with the Right (TM) design. I know I've brought this up several times before and that it has gone nowhere, but I'd really to have that final 'git continue' in Git 2.0. Can someone attempt to break up the migration path into manageable logical steps that we can start working on? [1]: http://thread.gmane.org/gmane.comp.version-control.git/179304/focus=179383 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git merge tag behavior
Hi, Le 21.03.2013 21:31, Max Nanasy a écrit : Yann Droneaud ydroneaud at opteya.com writes: 3) Merge options can't be overridden. If I modify .git/config to set a merge option, for example forcing fast-forward merge, this option cannot be overridden on command line: (git merge --no-ff-only --no-ff) should work. The --no-ff-only overrides the --ff-only configuration setting, and the --no-ff ensures that the merge is not a fast-forward. Thanks. I wasn't aware of the --no-ff-only option and thought --no-ff would be the opposite of --ff-only, or at least disable it given the order of the options. Please find a patch to document option --no-ff-only Documentation/merge-options.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 0bcbe0a..20a31cf 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -37,6 +37,10 @@ set to `no` at the beginning of them. current `HEAD` is already up-to-date or the merge can be resolved as a fast-forward. +--no-ff-only:: + Disable `--ff-only` behavior, eg. allows creation of merge commit. + This is the default behavior. + --log[=n]:: --no-log:: In addition to branch names, populate the log message with -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] t7800: modernize tests
On Fri, Mar 22, 2013 at 08:13:46AM +0100, Johannes Sixt wrote:
Am 3/21/2013 8:41, schrieb Johannes Sixt:
Am 3/20/2013 23:59, schrieb David Aguilar:
I started digging in and the @worktree_files (aka @worktree above)
is populated from the output of git diff --raw
Seeing the output filename in diff --raw implies that one of the
tests added output to the index somehow. I do not see that
happening anywhere, though, so I do not know how it would end up in
the @worktree array if it is not reported by diff --raw.
My current understanding of how it could possibly be open twice:
1. via the output redirect
2. via the copy() perl code which is fed by @worktree
So I'm confused. Why would we get different results on Windows?
I tracked down the difference between Windows and Linux, and it is...
for my $file (@worktree) {
next if $symlinks -l $b/$file;
... this line in sub dir_diff. On Linux, we take the short-cut, but on
Windows we proceed through the rest of the loop,
And that is likely by design. From the docs:
--symlinks
--no-symlinks
git difftool's default behavior is create symlinks to the working
tree when run in --dir-diff mode.
Specifying `--no-symlinks` instructs 'git difftool' to create
copies instead. `--no-symlinks` is the default on Windows.
And indeed, we have this initialization:
my %opts = (
...
symlinks = $^O ne 'cygwin'
$^O ne 'MSWin32' $^O ne 'msys',
...
);
Can the --dir-diff tests case pass on Cygwin when neither --symlinks nor
--no-symlinks is passed?
Perhaps the right solution is this:
We already have tests that explicitly pass '--symlinks'. I wonder if it
would be better to change output to .git/output, which should avoid
the problem by moving the output file out of the working tree.
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index c6d6b1c..19238f6 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -328,14 +328,16 @@ test_expect_success PERL 'setup change in subdirectory'
'
git commit -m modified both
'
-test_expect_success PERL 'difftool -d' '
- git difftool -d --extcmd ls branch output
+# passing --symlinks helps Cygwin, which defaults to --no-symlinks
+
+test_expect_success PERL,SYMLINKS 'difftool -d' '
+ git difftool -d --symlinks --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
'
-test_expect_success PERL 'difftool --dir-diff' '
- git difftool --dir-diff --extcmd ls branch output
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff' '
+ git difftool --dir-diff --symlinks --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
'
@@ -362,16 +364,16 @@ test_expect_success PERL,SYMLINKS 'difftool --dir-diff
--symlink without unstage
test_cmp actual expect
'
-test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
- git difftool --dir-diff --prompt --extcmd ls branch output
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff ignores --prompt' '
+ git difftool --dir-diff --symlinks --prompt --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
'
-test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+test_expect_success PERL,SYMLINKS 'difftool --dir-diff from subdirectory' '
(
cd sub
- git difftool --dir-diff --extcmd ls branch output
+ git difftool --dir-diff --symlinks --extcmd ls branch output
stdin_contains sub output
stdin_contains file output
)
(Only tested on MinGW, which skips the tests.) I leave it to you
to write --no-symlinks tests.
-- 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
[PATCH] t7600: test merge configuration override
Set the configuration variable 'merge.ff' to either 'only' or 'no' and check that this configuration can be overridden on command line. Additionally, test for currently not tested option '--no-ff-only' Signed-off-by: Yann Droneaud ydrone...@opteya.com --- t/t7600-merge.sh | 26 ++ 1 file changed, 26 insertions(+) diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh index 5e19598..b524bdb 100755 --- a/t/t7600-merge.sh +++ b/t/t7600-merge.sh @@ -254,6 +254,32 @@ test_expect_success 'merges with merge.ff=only' ' verify_head $c3 ' +test_expect_success 'merges with merge.ff=only and --no-ff-only' ' + git reset --hard c1 + test_tick + test_when_finished git config --unset merge.ff + git config merge.ff only + test_must_fail git merge --no-ff c2 + git merge --no-ff-only c2 + + git reset --hard c1 + git merge --no-ff-only --no-ff c2 +' + +test_expect_success 'merges with merge.ff=no and --ff' ' + git reset --hard c0 + test_tick + test_when_finished git config --unset merge.ff + git config merge.ff no + test_must_fail git merge --ff-only c1 + git merge --ff c1 + verify_head $c1 + + git reset --hard c0 + git merge --ff --ff-only c1 + verify_head $c1 +' + test_expect_success 'merge c0 with c1 (no-commit)' ' git reset --hard c0 git merge --no-commit c1 -- 1.7.11.7 -- 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/4] t7800: modernize tests
Am 3/22/2013 11:00, schrieb John Keeping:
On Fri, Mar 22, 2013 at 08:13:46AM +0100, Johannes Sixt wrote:
Am 3/21/2013 8:41, schrieb Johannes Sixt:
Am 3/20/2013 23:59, schrieb David Aguilar:
I started digging in and the @worktree_files (aka @worktree above)
is populated from the output of git diff --raw
Seeing the output filename in diff --raw implies that one of the
tests added output to the index somehow. I do not see that
happening anywhere, though, so I do not know how it would end up in
the @worktree array if it is not reported by diff --raw.
My current understanding of how it could possibly be open twice:
1. via the output redirect
2. via the copy() perl code which is fed by @worktree
So I'm confused. Why would we get different results on Windows?
I tracked down the difference between Windows and Linux, and it is...
for my $file (@worktree) {
next if $symlinks -l $b/$file;
... this line in sub dir_diff. On Linux, we take the short-cut, but on
Windows we proceed through the rest of the loop,
And that is likely by design. From the docs:
--symlinks
--no-symlinks
git difftool's default behavior is create symlinks to the working
tree when run in --dir-diff mode.
Specifying `--no-symlinks` instructs 'git difftool' to create
copies instead. `--no-symlinks` is the default on Windows.
And indeed, we have this initialization:
my %opts = (
...
symlinks = $^O ne 'cygwin'
$^O ne 'MSWin32' $^O ne 'msys',
...
);
Can the --dir-diff tests case pass on Cygwin when neither --symlinks nor
--no-symlinks is passed?
Perhaps the right solution is this:
We already have tests that explicitly pass '--symlinks'. I wonder if it
would be better to change output to .git/output, which should avoid
the problem by moving the output file out of the working tree.
The point of using --symlinks is not to test the effect of the option, but
to test the same thing on Unix and on Cygwin, because the latter uses
--no-symlinks by default. Therefore, I think that this sketch is the right
thing to do.
But the real problem seems to be that output should not be among the
files treated in the cited pieces of code (unless I'm wrong, of course; I
know next to nothing about git-difftool). It should not matter where the
file lives. Just add --no-symlinks to the difftool invocation of test
difftool -d and watch it fail on Linux, too.
-- 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 v3 3/4] t7800: modernize tests
On Fri, Mar 22, 2013 at 12:14:27PM +0100, Johannes Sixt wrote:
Am 3/22/2013 11:00, schrieb John Keeping:
On Fri, Mar 22, 2013 at 08:13:46AM +0100, Johannes Sixt wrote:
Am 3/21/2013 8:41, schrieb Johannes Sixt:
Am 3/20/2013 23:59, schrieb David Aguilar:
I started digging in and the @worktree_files (aka @worktree above)
is populated from the output of git diff --raw
Seeing the output filename in diff --raw implies that one of the
tests added output to the index somehow. I do not see that
happening anywhere, though, so I do not know how it would end up in
the @worktree array if it is not reported by diff --raw.
My current understanding of how it could possibly be open twice:
1. via the output redirect
2. via the copy() perl code which is fed by @worktree
So I'm confused. Why would we get different results on Windows?
I tracked down the difference between Windows and Linux, and it is...
for my $file (@worktree) {
next if $symlinks -l $b/$file;
... this line in sub dir_diff. On Linux, we take the short-cut, but on
Windows we proceed through the rest of the loop,
And that is likely by design. From the docs:
--symlinks
--no-symlinks
git difftool's default behavior is create symlinks to the working
tree when run in --dir-diff mode.
Specifying `--no-symlinks` instructs 'git difftool' to create
copies instead. `--no-symlinks` is the default on Windows.
And indeed, we have this initialization:
my %opts = (
...
symlinks = $^O ne 'cygwin'
$^O ne 'MSWin32' $^O ne 'msys',
...
);
Can the --dir-diff tests case pass on Cygwin when neither --symlinks nor
--no-symlinks is passed?
Perhaps the right solution is this:
We already have tests that explicitly pass '--symlinks'. I wonder if it
would be better to change output to .git/output, which should avoid
the problem by moving the output file out of the working tree.
The point of using --symlinks is not to test the effect of the option, but
to test the same thing on Unix and on Cygwin, because the latter uses
--no-symlinks by default. Therefore, I think that this sketch is the right
thing to do.
So shouldn't we be running all of the tests with both --symlinks and
--no-symlinks (except those which explicitly check that these options do
the right thing)?
But the real problem seems to be that output should not be among the
files treated in the cited pieces of code (unless I'm wrong, of course; I
know next to nothing about git-difftool). It should not matter where the
file lives. Just add --no-symlinks to the difftool invocation of test
difftool -d and watch it fail on Linux, too.
I fired up strace and it looks like difftool sees it as a working tree
file (i.e. the working tree content matches the RHS of the diff) in the
output of git diff --raw --no-abbrev -z branch and copies it over to
the temporary directories. Then when difftool completes it copies back
the working tree files from there.
When the file is copied to the temporary directory, the diff command
hasn't yet run so output is empty. When it's copied back it is after
the ls has run and so we overwrite the output of the command with the
original empty file.
So sticking the output file under .git does solve this issue since it
then is not treated as a working tree file.
Whether the current behaviour of difftool is entirely sensible is a
different question. I think we should at the very least by refusing to
overwrite working tree files that have been modified since they were
copied to the temporary directory
If I ever end up on a system using --no-symlinks I can see myself being
bitten by this by editing the original working tree file while
inspecting the diff in a separate window. I suspect this is just as
common a workflow as people editing the file in their diff tool and
wanting the changes copied back to the working tree.
John
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Introduce pull.autostash
This series adds a pull.autostash, making 'git pull' a tad bit more usable. It is part of my bigger plan to make 'git pull' just DTRT (via configuration variables) in 90% of the cases. [1/3] and [2/3] are tiny while we're here patches. [3/3] is the main patch with tests. Thanks for reading. Ramkumar Ramachandra (3): git-pull.sh: prefer invoking git command over git-command t5521 (pull-options): use test_commit() where appropriate git-pull.sh: introduce --[no-]autostash and pull.autostash Documentation/config.txt | 5 Documentation/git-pull.txt | 7 ++ git-pull.sh| 30 +++--- t/t5521-pull-options.sh| 63 -- 4 files changed, 99 insertions(+), 6 deletions(-) -- 1.8.2.141.gad203c2.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] git-pull.sh: prefer invoking git command over git-command
14e5d40c (pull: Fix parsing of -Xoption, 2010-01-17) added the lines
containing git-push and git-merge, even though the prevelant style
at the time was to use the unhyphenated git command form. Fix
this.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
git-pull.sh | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/git-pull.sh b/git-pull.sh
index 266e682..37e1cd4 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -279,11 +279,11 @@ fi
merge_name=$(git fmt-merge-msg $log_arg $GIT_DIR/FETCH_HEAD) || exit
case $rebase in
true)
- eval=git-rebase $diffstat $strategy_args $merge_args
+ eval=git rebase $diffstat $strategy_args $merge_args
eval=$eval --onto $merge_head ${oldremoteref:-$merge_head}
;;
*)
- eval=git-merge $diffstat $no_commit $edit $squash $no_ff $ff_only
+ eval=git merge $diffstat $no_commit $edit $squash $no_ff $ff_only
eval=$eval $log_arg $strategy_args $merge_args $verbosity $progress
eval=$eval \\$merge_name\ HEAD $merge_head
;;
--
1.8.2.141.gad203c2.dirty
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] t5521 (pull-options): use test_commit() where appropriate
test_commit() is a well-defined function in test-lib-functions.sh that allows you to create commits with a terse syntax. Prefer using it over creating commits by hand. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t5521-pull-options.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index 1b06691..4a804f0 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -7,8 +7,8 @@ test_description='pull options' test_expect_success 'setup' ' mkdir parent (cd parent git init -echo one file git add file -git commit -m one) +test_commit one file one + ) ' test_expect_success 'git pull -q' ' -- 1.8.2.141.gad203c2.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash
This new configuration variable executes 'git stash' before attempting
to merge/rebase, and 'git stash pop' after a successful merge/rebase.
It makes it convenient for people to pull with dirty worktrees.
Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
Documentation/config.txt | 5
Documentation/git-pull.txt | 7 ++
git-pull.sh| 26 ++--
t/t5521-pull-options.sh| 59 ++
4 files changed, 95 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1f435f..0becafe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1786,6 +1786,11 @@ pull.rebase::
of merging the default branch from the default remote when git
pull is run. See branch.name.rebase for setting this on a
per-branch basis.
+
+pull.autostash::
+ When true, automatically stash all changes before attempting to
+ merge/rebase, and pop the stash after a successful
+ merge/rebase.
+
*NOTE*: this is a possibly dangerous operation; do *not* use
it unless you understand the implications (see linkgit:git-rebase[1]
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index c975743..bb57c86 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -94,6 +94,13 @@ must be given before the options meant for 'git fetch'.
has to be called afterwards to bring the work tree up to date with the
merge result.
+--[no-]autostash::
+ When turned on, automatically stash all changes before
+ attempting to merge/rebase, and pop the stash after a
+ successful merge/rebase. Useful for people who want to pull
+ with a dirty worktree. This option can also be set via the
+ `pull.autostash` configuration variable.
+
Options related to merging
~~
diff --git a/git-pull.sh b/git-pull.sh
index 37e1cd4..ad8e494 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -48,6 +48,7 @@ if test -z $rebase
then
rebase=$(git config --bool pull.rebase)
fi
+autostash=$(git config --bool pull.autostash)
dry_run=
while :
do
@@ -116,6 +117,12 @@ do
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
rebase=false
;;
+ --autostash)
+ autostash=true
+ ;;
+ --no-autostash)
+ autostash=false
+ ;;
--recurse-submodules)
recurse_submodules=--recurse-submodules
;;
@@ -196,7 +203,8 @@ test true = $rebase {
then
die $(gettext updating an unborn branch with changes
added to the index)
fi
- else
+ elif test $autostash = false
+ then
require_clean_work_tree pull with rebase Please commit or
stash them.
fi
oldremoteref=
@@ -213,6 +221,12 @@ test true = $rebase {
fi
done
}
+
+stash_required () {
+ ! (git diff-files --quiet --ignore-submodules
+ git diff-index --quiet --cached HEAD --ignore-submodules)
+}
+
orig_head=$(git rev-parse -q --verify HEAD)
git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok
$@ || exit 1
test -z $dry_run || exit 0
@@ -288,4 +302,12 @@ true)
eval=$eval \\$merge_name\ HEAD $merge_head
;;
esac
-eval exec $eval
+
+if test $autostash = true stash_required
+then
+ git stash
+ eval $eval
+ test $? = 0 git stash pop
+else
+ eval exec $eval
+fi
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 4a804f0..cecacbc 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -90,4 +90,63 @@ test_expect_success 'git pull --all' '
)
'
+test_expect_success 'pull --autostash with clean worktree' '
+ mkdir clonedautostash
+ (cd clonedautostash
+ git init
+ git pull --autostash ../parent
+ test_must_fail test_path_is_file .git/refs/stash
+ test_commit one
+ )
+ rm -rf clonedautostash
+'
+
+test_expect_success 'pull.autostash with clean worktree' '
+ mkdir clonedautostash
+ (cd clonedautostash
+ git init
+ test_config pull.autostash true
+ git pull ../parent
+ test_must_fail test_path_is_file .git/refs/stash
+ test_commit one
+ )
+ rm -rf clonedautostash
+'
+
+test_expect_success 'pull.autostash without conflict' '
+ mkdir clonedautostash
+ (cd clonedautostash
+ git init
+ test_commit root_commit
+ cat quux -\EOF
+ this is a non-conflicting file
+ EOF
+ git add quux
+ test_config pull.autostash true
+ git pull ../parent
+ test_must_fail test_path_is_file .git/refs/stash
+ test_path_is_file quux
+ test_commit one
+ )
+ rm -rf clonedautostash
+'
+
+test_expect_success
[PATCH 0/2] Fix a couple of minor things in pull test
I just happened to open the file when searching for the right file to put my autostash tests. Ramkumar Ramachandra (2): t5520 (pull): update test description t5520 (pull): use test_config where appropriate t/t5520-pull.sh | 24 ++-- 1 file changed, 10 insertions(+), 14 deletions(-) -- 1.8.2.141.gad203c2.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] t5520 (pull): update test description
d09e79c (git-pull: allow pulling into an empty repository, 2006-11-16) created the file with the description. At the time, there were only tests to check pulling into an empty repository. The description was never updated even after more tests, unrelated to the original, were added. Fix this now. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t5520-pull.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 35304b4..e5adee8 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -1,6 +1,6 @@ #!/bin/sh -test_description='pulling into void' +test_description='pull basic functionality' . ./test-lib.sh -- 1.8.2.141.gad203c2.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] t5520 (pull): use test_config where appropriate
Configuration from test_config does not last beyond the end of the current test assertion, making each test easier to think about in isolation. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t5520-pull.sh | 22 +- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index e5adee8..0fe935b 100755 --- a/t/t5520-pull.sh +++ b/t/t5520-pull.sh @@ -60,8 +60,8 @@ test_expect_success 'pulling into void does not overwrite untracked files' ' test_expect_success 'test . as a remote' ' git branch copy master - git config branch.copy.remote . - git config branch.copy.merge refs/heads/master + test_config branch.copy.remote . + test_config branch.copy.merge refs/heads/master echo updated file git commit -a -m updated git checkout copy @@ -96,8 +96,7 @@ test_expect_success '--rebase' ' ' test_expect_success 'pull.rebase' ' git reset --hard before-rebase - git config --bool pull.rebase true - test_when_finished git config --unset pull.rebase + test_config pull.rebase true git pull . copy test $(git rev-parse HEAD^) = $(git rev-parse copy) test new = $(git show HEAD:file2) @@ -105,8 +104,7 @@ test_expect_success 'pull.rebase' ' test_expect_success 'branch.to-rebase.rebase' ' git reset --hard before-rebase - git config --bool branch.to-rebase.rebase true - test_when_finished git config --unset branch.to-rebase.rebase + test_config branch.to-rebase.rebase true git pull . copy test $(git rev-parse HEAD^) = $(git rev-parse copy) test new = $(git show HEAD:file2) @@ -114,10 +112,8 @@ test_expect_success 'branch.to-rebase.rebase' ' test_expect_success 'branch.to-rebase.rebase should override pull.rebase' ' git reset --hard before-rebase - git config --bool pull.rebase true - test_when_finished git config --unset pull.rebase - git config --bool branch.to-rebase.rebase false - test_when_finished git config --unset branch.to-rebase.rebase + test_config pull.rebase true + test_config branch.to-rebase.rebase false git pull . copy test $(git rev-parse HEAD^) != $(git rev-parse copy) test new = $(git show HEAD:file2) @@ -171,9 +167,9 @@ test_expect_success 'pull --rebase dies early with dirty working directory' ' git update-ref refs/remotes/me/copy copy^ COPY=$(git rev-parse --verify me/copy) git rebase --onto $COPY copy - git config branch.to-rebase.remote me - git config branch.to-rebase.merge refs/heads/copy - git config branch.to-rebase.rebase true + test_config branch.to-rebase.remote me + test_config branch.to-rebase.merge refs/heads/copy + test_config branch.to-rebase.rebase true echo dirty file git add file test_must_fail git pull -- 1.8.2.141.gad203c2.dirty -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] Avoid false positives in label detection in cpp diff hunk header regex.
A C++ method start such as
void
foo::bar()
wasn't recognized by cpp diff driver as it mistakenly included foo::bar as a
label. However the colon in a label can't be followed by another colon, so
recognize this case specially to correctly detect C++ methods using this style.
Signed-off-by: Vadim Zeitlin vz-...@zeitlins.org
---
userdiff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/userdiff.c b/userdiff.c
index ea43a03..9415586 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -125,7 +125,7 @@ PATTERNS(tex,
^(((sub)*section|chapter|part)\\*{0,1}\\{.*)$,
[a-zA-Z@]+|.|[a-zA-Z0-9\x80-\xff]+),
PATTERNS(cpp,
/* Jump targets or access declarations */
-!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n
+!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:].*$|$)\n
/* C/++ functions/methods at top level */
^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[
\t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n
/* compound type at top level */
--
1.8.2.135.g7b592fa
--
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] t5516 (fetch-push): introduce mk_test_with_name()
On Fri, Mar 22, 2013 at 01:22:33PM +0530, Ramkumar Ramachandra wrote:
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().
I think this is OK, and I do not mind if it gets applied. But what I was
hinting at in my earlier mail was that we might want to do this (I have
it as a separate patch on top of your 3/6 here, but it would make more
sense squashed in):
-- 8 --
Subject: [PATCH] t5516: drop implicit arguments from helper functions
Many of the tests in t5516 look like:
mk_empty
git push testrepo ...
check_push_result $commit heads/master
It's reasonably easy to see what is being tested, with the
exception that testrepo is a magic global name (it is
implicitly used in the helpers, but we have to name it
explicitly when calling git directly). Let's make it
explicit when call the helpers, too. This is slightly more
typing, but makes the test snippets read more naturally.
It also makes it easy for future tests to use an alternate
or multiple repositories, without a proliferation of helper
functions.
Signed-off-by: Jeff King p...@peff.net
---
t/t5516-fetch-push.sh | 268 --
1 file changed, 130 insertions(+), 138 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 05579b6..d27b8d3 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -8,7 +8,6 @@ mk_empty () {
mk_empty () {
repo_name=$1
- test -z $repo_name repo_name=testrepo
rm -fr $repo_name
mkdir $repo_name
(
@@ -19,7 +18,7 @@ mk_empty () {
)
}
-mk_test_with_name () {
+mk_test () {
repo_name=$1
shift
@@ -45,14 +44,11 @@ mk_test_with_hooks() {
)
}
-mk_test () {
- mk_test_with_name testrepo $@
-}
-
mk_test_with_hooks() {
+ repo_name=$1
mk_test $@
(
- cd testrepo
+ cd $repo_name
mkdir .git/hooks
cd .git/hooks
@@ -84,11 +80,11 @@ mk_child() {
}
mk_child() {
- rm -rf $1
- git clone testrepo $1
+ rm -rf $2
+ git clone $1 $2
}
-check_push_result_with_name () {
+check_push_result () {
repo_name=$1
shift
@@ -108,10 +104,6 @@ check_push_result_with_name () {
)
}
-check_push_result () {
- check_push_result_with_name testrepo $@
-}
-
test_expect_success setup '
path1
@@ -129,7 +121,7 @@ test_expect_success 'fetch without wildcard' '
'
test_expect_success 'fetch without wildcard' '
- mk_empty
+ mk_empty testrepo
(
cd testrepo
git fetch .. refs/heads/master:refs/remotes/origin/master
@@ -142,7 +134,7 @@ test_expect_success 'fetch with wildcard' '
'
test_expect_success 'fetch with wildcard' '
- mk_empty
+ mk_empty testrepo
(
cd testrepo
git config remote.up.url ..
@@ -157,7 +149,7 @@ test_expect_success 'fetch with insteadOf' '
'
test_expect_success 'fetch with insteadOf' '
- mk_empty
+ mk_empty testrepo
(
TRASH=$(pwd)/
cd testrepo
@@ -174,7 +166,7 @@ test_expect_success 'fetch with pushInsteadOf (should not
rewrite)' '
'
test_expect_success 'fetch with pushInsteadOf (should not rewrite)' '
- mk_empty
+ mk_empty testrepo
(
TRASH=$(pwd)/
cd testrepo
@@ -191,7 +183,7 @@ test_expect_success 'push without wildcard' '
'
test_expect_success 'push without wildcard' '
- mk_empty
+ mk_empty testrepo
git push testrepo refs/heads/master:refs/remotes/origin/master
(
@@ -204,7 +196,7 @@ test_expect_success 'push with wildcard' '
'
test_expect_success 'push with wildcard' '
- mk_empty
+ mk_empty testrepo
git push testrepo refs/heads/*:refs/remotes/origin/*
(
@@ -217,7 +209,7 @@ test_expect_success 'push with insteadOf' '
'
test_expect_success 'push with insteadOf' '
- mk_empty
+ mk_empty testrepo
TRASH=$(pwd)/
git config url.$TRASH.insteadOf trash/
git push trash/testrepo refs/heads/master:refs/remotes/origin/master
@@ -231,7 +223,7 @@ test_expect_success 'push with pushInsteadOf' '
'
test_expect_success 'push with pushInsteadOf' '
- mk_empty
+ mk_empty testrepo
TRASH=$(pwd)/
git config url.$TRASH.pushInsteadOf trash/
git push trash/testrepo refs/heads/master:refs/remotes/origin/master
@@ -245,7 +237,7 @@ test_expect_success 'push with pushInsteadOf and explicit
pushurl (pushInsteadOf
'
Re: [PATCH] t7600: test merge configuration override
Yann Droneaud ydrone...@opteya.com writes: +test_expect_success 'merges with merge.ff=only and --no-ff-only' ' + git reset --hard c1 + test_tick + test_when_finished git config --unset merge.ff + git config merge.ff only I see this was copied from existing tests, but we should use test_config these days. It would be a good approach to first do a preparatory patch to convert the existing ones to use test_config and then to redo this patch using test_config on top of 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] t7600: merge tag shoud create a merge commit
y...@quest-ce.net writes: From: Yann Droneaud ydrone...@opteya.com This test ensures a merge commit is always created when merging an annotated (signed) tag without --ff-only option. Signed-off-by: Yann Droneaud ydrone...@opteya.com --- Here's a proposition for a test tath check the creation of a merge commit when merging a tag. It's not in final shape: the line EDITOR=false test_must_fail git merge signed Because test_must_fail is a shell function, single-shot environment assignment like this should not be used. -- 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] t5516 (fetch-push): introduce mk_test_with_name()
Jeff King p...@peff.net writes: I think this is OK, and I do not mind if it gets applied. But what I was hinting at in my earlier mail was that we might want to do this (I have it as a separate patch on top of your 3/6 here, but it would make more sense squashed in): I would prefer to see a preparatory patch to teach mk_test/mk_empty to _always_ take the new name (i.e. the result of your patch) and then do whatever new things on top. By the way, I am planning to _not_ look at new stuff today. I'd rather see known recent regressions addressed (and unknown ones discovered and squashed) before we move forward, and I would appreciate if regular contributors did the same. 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 3/6] t5516 (fetch-push): introduce mk_test_with_name()
Ramkumar Ramachandra artag...@gmail.com writes:
mk_empty () {
- rm -fr testrepo
- mkdir testrepo
+ repo_name=$1
+ test -z $repo_name repo_name=testrepo
+ rm -fr $repo_name
+ mkdir $repo_name
Your quoting is sloppy in this entire patch X-.
--
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] t7600: merge tag shoud create a merge commit
Le 22.03.2013 15:48, Junio C Hamano a écrit : It's not in final shape: the line EDITOR=false test_must_fail git merge signed Because test_must_fail is a shell function, single-shot environment assignment like this should not be used. It's used throughout the test. The test 'merge --no-edit tag should skip editor' is using it. Before posting my half useful test, I used EDITOR=false test_must_fail set in --verbose mode to find if EDITOR was correctly defined passed test_must_fail, and it was. So it's still not clear why it's failing at failing. And it's making me angry. Regards. -- Yann Droneaud OPTEYA -- 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 5/6] remote.c: introduce remote.pushdefault
On Fri, Mar 22, 2013 at 01:22:35PM +0530, Ramkumar Ramachandra wrote:
diff --git a/remote.c b/remote.c
index 185ac11..bdb542c 100644
--- a/remote.c
+++ b/remote.c
@@ -350,6 +350,11 @@ 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))
+ if (git_config_string(pushremote_name, key, value))
+ return -1;
+ }
if (!prefixcmp(key, branch.)) {
name = key + 7;
subkey = strrchr(name, '.');
I was going to say shouldn't we return 0 here, both on successful read
of pushdefault, but also just when we have remote.*. But the answer is
yes to the first one (we know we have handled the key), but no to
the second, because we end up parsing remote.*.* later.
So I think this should at least be:
if (!prefixcmp(key, remote.)) {
if (!strcmp(key + 7, pushdefault))
return git_config_string(pushremote_name, key, value));
/* do not return; we handle other remote.* below */
}
but also possibly just move it with the other remote parsing, like:
diff --git a/remote.c b/remote.c
index 02e6c4c..d3d740a 100644
--- a/remote.c
+++ b/remote.c
@@ -388,9 +388,16 @@ static int handle_config(const char *key, const char
*value, void *cb)
add_instead_of(rewrite, xstrdup(value));
}
}
+
if (prefixcmp(key, remote.))
return 0;
name = key + 7;
+
+ /* Handle any global remote.* variables */
+ if (!strcmp(name, pushdefault))
+ return git_config_string(pushremote_name, key, value);
+
+ /* Now handle any remote.NAME.* variables */
if (*name == '/') {
warning(Config remote shorthand cannot begin with '/': %s,
name);
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
On Fri, Mar 22, 2013 at 07:54:06AM -0700, Junio C Hamano wrote:
Ramkumar Ramachandra artag...@gmail.com writes:
mk_empty () {
- rm -fr testrepo
- mkdir testrepo
+ repo_name=$1
+ test -z $repo_name repo_name=testrepo
+ rm -fr $repo_name
+ mkdir $repo_name
Your quoting is sloppy in this entire patch X-.
I meant to mention that, too. It doesn't matter, as the intent is for it
always to be a simple name like testrepo, but that assumption is not
documented anywhere. I suspect the quoting in my patch is sloppy, too.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
On Fri, Mar 22, 2013 at 07:52:47AM -0700, Junio C Hamano wrote: Jeff King p...@peff.net writes: I think this is OK, and I do not mind if it gets applied. But what I was hinting at in my earlier mail was that we might want to do this (I have it as a separate patch on top of your 3/6 here, but it would make more sense squashed in): I would prefer to see a preparatory patch to teach mk_test/mk_empty to _always_ take the new name (i.e. the result of your patch) and then do whatever new things on top. I think that is what my patch does (it is meant to come at the point of 3/6, and then the rest would need to be rebased to just use mk_test instead of mk_test_with_name). By the way, I am planning to _not_ look at new stuff today. I'd rather see known recent regressions addressed (and unknown ones discovered and squashed) before we move forward, and I would appreciate if regular contributors did the same. Yeah, I have several to look at (the subdir/ in gitattributes is the biggest one, I think). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Avoid false positives in label detection in cpp diff hunk header regex.
Vadim Zeitlin vz-...@zeitlins.org writes:
A C++ method start such as
void
foo::bar()
wasn't recognized by cpp diff driver as it mistakenly included foo::bar as a
label. However the colon in a label can't be followed by another colon, so
recognize this case specially to correctly detect C++ methods using this
style.
Signed-off-by: Vadim Zeitlin vz-...@zeitlins.org
---
userdiff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/userdiff.c b/userdiff.c
index ea43a03..9415586 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -125,7 +125,7 @@ PATTERNS(tex,
^(((sub)*section|chapter|part)\\*{0,1}\\{.*)$,
[a-zA-Z@]+|.|[a-zA-Z0-9\x80-\xff]+),
PATTERNS(cpp,
/* Jump targets or access declarations */
-!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n
+!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:].*$|$)\n
Hmm. Wouldn't find a word (possibly after indentation), colon and
then either a non-colon or end of line be sufficient and simpler?
iow, something like...
!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:]|$)
/* C/++ functions/methods at top level */
^([A-Za-z_][A-Za-z_0-9]*([ \t*]+[A-Za-z_][A-Za-z_0-9]*([ \t]*::[
\t]*[^[:space:]]+)?){1,}[ \t]*\\([^;]*)$\n
/* compound type at top level */
--
1.8.2.135.g7b592fa
--
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] t7600: merge tag shoud create a merge commit
On Fri, Mar 22, 2013 at 03:56:15PM +0100, Yann Droneaud wrote: Le 22.03.2013 15:48, Junio C Hamano a écrit : It's not in final shape: the line EDITOR=false test_must_fail git merge signed Because test_must_fail is a shell function, single-shot environment assignment like this should not be used. It's used throughout the test. The test 'merge --no-edit tag should skip editor' is using it. It's OK to do: SINGLE_SHOT=foo some_real_command and it's OK to do: some_fun args but it's not OK to do: SINGLE_SHOT=foo some_function args Because some POSIX shells do not create a new environment for the function (and SINGLE_SHOT will persist after the call, polluting the environment). Before posting my half useful test, I used EDITOR=false test_must_fail set in --verbose mode to find if EDITOR was correctly defined passed test_must_fail, and it was. I do not think there is a shell that does not set it; it is only that some shells do not _unset_ it. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/6] Support triangular workflows
Philip Oakley philipoak...@iee.org writes: Shouldn't Documentation/gitworkflows.txt also be updated with the triangular workflow and its configuration? What is missing from gitworkflows documentation is actually a non-triangular workflow, where people pull from and push into the same central repository. The merge workflow part in the distributed workflows section teaches: * fetching others' work from remote and merging it to yours; * publishing your work to remote; and * advertising your work. and it is written for the triangular workflow. Two triangles are involved. The maintainer may pull from you but pushes to her own, which is one triangle, and you pull from the maintainer and push to your own, which is another. As to your suggestion, I do think it is reasonable to clarify these triangles with an illustration, and to even add descriptions for short-cut configurations as a side note. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git merge tag behavior
Yann Droneaud ydrone...@opteya.com writes: Thanks. I wasn't aware of the --no-ff-only option and thought --no-ff would be the opposite of --ff-only, or at least disable it given the order of the options. Please find a patch to document option --no-ff-only Documentation/merge-options.txt | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt index 0bcbe0a..20a31cf 100644 --- a/Documentation/merge-options.txt +++ b/Documentation/merge-options.txt @@ -37,6 +37,10 @@ set to `no` at the beginning of them. current `HEAD` is already up-to-date or the merge can be resolved as a fast-forward. +--no-ff-only:: + Disable `--ff-only` behavior, eg. allows creation of merge commit. + This is the default behavior. + We should follow the usual --option:: --no-option:: description for both convention for this one, before or after fixing the existing --ff/--no-ff description. --log[=n]:: --no-log:: In addition to branch names, populate the log message with -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] wt-status: fix possible use of uninitialized variable
On Thu, Mar 21, 2013 at 12:49:50PM -0700, Jonathan Nieder wrote: We could also convert the flag to an enum, which would provide a compile-time check on the function input. Unfortunately C permits out-of-bounds values for enums. True, although I would think that most compilers take the hint for switch() statements that handling all defined constants for an enum is enough (certainly gcc does it with the some enum constants not handled warning, but I did not actually check whether it does so in the uninitialized-warning control flow checker). Still, I'm happy enough with the die(BUG) that I posted, so we don't need to worry about it. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/4] drop some int x = x hacks to silence gcc warnings
On Thu, Mar 21, 2013 at 11:44:02AM -0400, Jeff King wrote: I am for dropping = x and leaving it uninitialized at the declaration site, or explicitly initializing it to some reasonable starting value (e.g. NULL if it is a pointer) and adding a comment to say that the initialization is to squelch compiler warnings. I'd be in favor of that, too. In many cases, I think the fact that gcc cannot trace the control flow is a good indication that it is hard for a human to trace it, too. And in those cases we would be better off restructuring the code slightly to make it more obvious to both types of readers. Two patches to follow. [5/4]: fast-import: clarify inline logic in file_change_m [6/4]: run-command: always set failed_errno in start_command And here are two more; with these, our code base should be free of x = x initializations (at least according to clang). [7/4]: submodule: clarify logic in show_submodule_summary [8/4]: match-trees: drop x = x initializations Not pressing, obviously, but since I had just analyzed the code yesterday, I wanted to do it while they were still fresh in my mind. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug: git web--browse doesn't recognise browser on OS X
Hi, well my use case is actually that I'm trying to use the gem 'gem-browse' which uses 'git web--browse' I'm not using Apple Terminal, I'm using iTerm2 and there doesn't seem to be a SECURITYSESSIONID set, at least echo didn't find any. But neither did I find it on Apple Terminal either. What troubles me is that this issue has only arisen recently, earlier this worked fine for me On 15 March 2013 11:19, Christian Couder christian.cou...@gmail.com wrote: Hi, On Thu, Mar 14, 2013 at 12:39 PM, Timo Sand timo.j.s...@gmail.com wrote: Hi I tried to open a website by runnin 'git web--browse http://google.com' and it replied 'No known browser available'. First git web--browse is a plumbing shell script to display documentation on a web browser when you type something like git help -w log. It is not really supposed to be used directly by the user. On OS X it might be simpler to just type open http://google.com;. That said there is the following in it to make it work on OS X: # SECURITYSESSIONID indicates an OS X GUI login session if test -n $SECURITYSESSIONID \ -o $TERM_PROGRAM = Apple_Terminal ; then browser_candidates=open $browser_candidates fi So I guess that you don't have SECURITYSESSIONID set in your terminal and you are not using Apple Terminal. As I am not using OS X, I have no idea how to improve the script in this case. I also tried with '--browser=chrome' and '--browser=google-chrome' but the responded with 'The browser chrome is not available as 'chrome'.' Could you try something like: chromium http://google.com; or chromium-browser http://google.com; If it works, then using 'git web--browse' with '--browser=chromium' or '--browser=chromium-browser' should work. Otherwise did you try chrome http://google.com; and google-chrome http://google.com;? I expected the command to open a new tab in my browser in each of the 3 tries. This has worked for my system before. OS X 10.8.2, git 1.8.2, Google Chrome 27.0.1438.7 dev Thanks, Christian. -- Timo Sand timo.j.sand+...@gmail.com -- 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 7/4] submodule: clarify logic in show_submodule_summary
There are two uses of the left and right commit
variables that make it hard to be sure what values they
have (both for the reader, and for gcc, which wrongly
complains that they might be used uninitialized).
The functions starts with a cascading if statement, checking
that the input sha1s exist, and finally working up to
preparing a revision walk. We only prepare the walk if the
cascading conditional did not find any problems, which we
check by seeing whether it set the message variable or
not. It's simpler and more obvious to just add a condition
to the end of the cascade.
Later, we check the same message variable when deciding
whether to clear commit marks on the left/right commits; if
it is set, we presumably never started the walk. This is
wrong, though; we might have started the walk and munged
commit flags, only to encounter an error afterwards. We
should always clear the flags on left/right if they exist,
whether the walk was successful or not.
Signed-off-by: Jeff King p...@peff.net
---
submodule.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/submodule.c b/submodule.c
index 9ba1496..975bc87 100644
--- a/submodule.c
+++ b/submodule.c
@@ -261,7 +261,7 @@ void show_submodule_summary(FILE *f, const char *path,
const char *del, const char *add, const char *reset)
{
struct rev_info rev;
- struct commit *left = left, *right = right;
+ struct commit *left = NULL, *right = NULL;
const char *message = NULL;
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
@@ -275,10 +275,8 @@ void show_submodule_summary(FILE *f, const char *path,
else if (!(left = lookup_commit_reference(one)) ||
!(right = lookup_commit_reference(two)))
message = (commits not present);
-
- if (!message
- prepare_submodule_summary(rev, path, left, right,
- fast_forward, fast_backward))
+ else if (prepare_submodule_summary(rev, path, left, right,
+ fast_forward, fast_backward))
message = (revision walker failed);
if (dirty_submodule DIRTY_SUBMODULE_UNTRACKED)
@@ -302,11 +300,12 @@ void show_submodule_summary(FILE *f, const char *path,
strbuf_addf(sb, %s:%s\n, fast_backward ? (rewind) : ,
reset);
fwrite(sb.buf, sb.len, 1, f);
- if (!message) {
+ if (!message) /* only NULL if we succeeded in setting up the walk */
print_submodule_summary(rev, f, del, add, reset);
+ if (left)
clear_commit_marks(left, ~0);
+ if (right)
clear_commit_marks(right, ~0);
- }
strbuf_release(sb);
}
--
1.8.2.13.g0f18d3c
--
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 8/4] match-trees: drop x = x initializations
These nonsense assignments are meant to squelch gcc warnings
that the variables might be used uninitialized. However, gcc
gets it mostly right, realizing that we will either
extract tree entries from both sides, or we will hit a
continue statement and go to the top of the loop.
However, while getting this right for the elem and path
variables, it does not do so for the mode variables. Let's
drop the nonsense initialization where modern gcc does not
need them, and just set the modes to 0, along with a
comment. These values should never be used, but it makes
both gcc, as well as any compiler which does not like the x
= x initializations, happy.
While we're in the area, let's also update the loop
condition to use logical-OR rather than bitwise-OR. They should
be equivalent in this case, and the use of the latter was
probably a typo.
Signed-off-by: Jeff King p...@peff.net
---
Of the 8 patches, this is the one I find the least satisfying, if only
because I do not think gcc's failure is because of complicated control
flow, and rearranging the code would only hurt readability. And I'm
quite curious why it complains about mode, but not about the other
variables, which are set in the exact same place (and why it would not
be able to handle such a simple control flow at all).
It makes me wonder if I am missing something, or there is some subtle
bug. But I can't see it. Other eyes appreciated.
match-trees.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/match-trees.c b/match-trees.c
index 26f7ed1..4360f10 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -71,13 +71,13 @@ static int score_trees(const unsigned char *hash1, const
unsigned char *hash2)
if (type != OBJ_TREE)
die(%s is not a tree, sha1_to_hex(hash2));
init_tree_desc(two, two_buf, size);
- while (one.size | two.size) {
- const unsigned char *elem1 = elem1;
- const unsigned char *elem2 = elem2;
- const char *path1 = path1;
- const char *path2 = path2;
- unsigned mode1 = mode1;
- unsigned mode2 = mode2;
+ while (one.size || two.size) {
+ const unsigned char *elem1;
+ const unsigned char *elem2;
+ const char *path1;
+ const char *path2;
+ unsigned mode1 = 0; /* make gcc happy */
+ unsigned mode2 = 0; /* make gcc happy */
int cmp;
if (one.size)
--
1.8.2.13.g0f18d3c
--
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
feature request - have git honor nested .gitconfig files
It'd be cool if I were able to override config settings at every nested directory. For example, I have my ~/.gitconfig that has one email address in it, but I also have multiple repos inside ~/dev which I want to use a different email address for. The only way to do that now is to edit all of these: ~/dev/*/.git/conf -- and there are lots of them, and new repos get added all the time - and I forget. If I could add ~/dev/.gitconfig and have the settings in that override ~/.gitconfig then this would be way more manageable. Obviously, individual repo's .git/config should take ultimate precedence. Thanks for the consideration! -- 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] t5521 (pull-options): use test_commit() where appropriate
Ramkumar Ramachandra artag...@gmail.com writes: test_commit() is a well-defined function in test-lib-functions.sh that allows you to create commits with a terse syntax. Prefer using it over creating commits by hand. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/t5521-pull-options.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh index 1b06691..4a804f0 100755 --- a/t/t5521-pull-options.sh +++ b/t/t5521-pull-options.sh @@ -7,8 +7,8 @@ test_description='pull options' test_expect_success 'setup' ' mkdir parent (cd parent git init - echo one file git add file - git commit -m one) + test_commit one file one + ) ' In this test script perhaps it is OK, but I'd prefer people being careful *not* to use test_commit in tests that involve refs (i.e. pushing, pulling, ls-remote, for-each-ref, describe...) and paths (i.e. ls-files, diff, ...). There is one good point in the helper: it creates a commit with a predictable timestamp. But it does a lot other *bad* things than that single good thing. It adds a new path, and adds a new tag; neither of which is not desirable in many circumstances. A better future direction would be to first make these frill features into options to test_commit helper, fix the users that depend on these additional tags and stuff to explicitly ask for them, and then start advocating it for wider use, I think. -- 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
[FEATURE-REQUEST] difftool --dir-diff: use the commit names as directory names instead of left/right
Hi. Right now, when I use difftool --dir-diff, the temp dirs are creates as e.g.: /tmp/git-difftool.QqP8x/left /tmp/git-difftool.QqP8x/right Wouldn't it be nice, if instead of left/right... the specified commit name would be used? e.g. /tmp/git-difftool.QqP8x/r1.1.1 /tmp/git-difftool.QqP8x/HEAD or /tmp/git-difftool.QqP8x/fd4e4005a4b7b3e638bc405be020b867f8881e72 /tmp/git-difftool.QqP8x/ce0747b74fccd20707b6f495068503e69e5473df Cause then, one would see in the difftool which side is what, without the need to remember how git difftool was invoked. Of course one might probably need to escape the commit names if they contain stuff like / or .., etc. Cheers, Chris. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash
Ramkumar Ramachandra artag...@gmail.com writes:
@@ -1786,6 +1786,11 @@ pull.rebase::
of merging the default branch from the default remote when git
pull is run. See branch.name.rebase for setting this on a
per-branch basis.
+
+pull.autostash::
+ When true, automatically stash all changes before attempting to
+ merge/rebase, and pop the stash after a successful
+ merge/rebase.
+
*NOTE*: this is a possibly dangerous operation; do *not* use
it unless you understand the implications (see linkgit:git-rebase[1]
Is autosquash a possibly dangerous operation?
@@ -196,7 +203,8 @@ test true = $rebase {
then
die $(gettext updating an unborn branch with changes
added to the index)
fi
- else
+ elif test $autostash = false
+ then
require_clean_work_tree pull with rebase Please commit or
stash them.
fi
A safety net, after you run git stash, to validate that the
added git stash indeed made the working tree clean, is necessary
below, but there does not seem to be any.
oldremoteref=
@@ -213,6 +221,12 @@ test true = $rebase {
fi
done
}
+
+stash_required () {
+ ! (git diff-files --quiet --ignore-submodules
+ git diff-index --quiet --cached HEAD --ignore-submodules)
+}
+
orig_head=$(git rev-parse -q --verify HEAD)
git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok
$@ || exit 1
test -z $dry_run || exit 0
@@ -288,4 +302,12 @@ true)
eval=$eval \\$merge_name\ HEAD $merge_head
;;
esac
-eval exec $eval
+
+if test $autostash = true stash_required
+then
+ git stash
+ eval $eval
+ test $? = 0 git stash pop
+else
+ eval exec $eval
+fi
Delaying to run git stash as much as possible is fine, but with
the above, can the user tell if something was stashed and she has
to stash pop once she is done helping the command to resolve the
conflicts, or she shouldn't run stash pop after she is done
(because if nothing is stashed at this point, that pop will pop an
unrelated stash)?
What protects the git stash from failing and how is its error
handled?
--
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] Avoid false positives in label detection in cpp diff hunk header regex.
Junio C Hamano gitster at pobox.com writes:
Vadim Zeitlin vz-git at zeitlins.org writes:
...
diff --git a/userdiff.c b/userdiff.c
index ea43a03..9415586 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -125,7 +125,7 @@ PATTERNS(tex,
^(((sub)*section|chapter|part)\\*{0,1}\\{.*)$,
[a-zA-Z@]+|.|[a-zA-Z0-9\x80-\xff]+),
PATTERNS(cpp,
/* Jump targets or access declarations */
-!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n
+!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:].*$|$)\n
Hmm. Wouldn't find a word (possibly after indentation), colon and
then either a non-colon or end of line be sufficient and simpler?
iow, something like...
!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:]|$)
This works too, of course. I didn't know why did the original regex
contain .*$ part so I decided to keep it but your version is indeed
how I would have written it myself if I were doing it from scratch.
Should I resubmit an updated patch or could you please just apply
your version?
TIA!
VZ
--
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 6/6] remote.c: introduce branch.name.pushremote
Ramkumar Ramachandra artag...@gmail.com writes: 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 artag...@gmail.com --- Documentation/config.txt | 18 ++ remote.c | 4 t/t5516-fetch-push.sh| 15 +++ 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 09a8294..6595cd6 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. Nice write-up. It may be easier to read if the new text is in a separate paragraph, though. +branch.name.pushremote:: + When on branch name, it overrides `branch.name.remote` + when pushing. It also overrides `remote.pushdefault` when + pushing from branch name. Perhaps s/when pushing/for pushing/; Or Specify what remote to push to when on branch name, overriding `branch.name.remote` and `remote.pushdefault`. + In a typical triangular-workflow + setup,... Is there an atypical triangular-workflow? Drop typical and explain what you mean by triangular, perhaps like When you pull from one place (e.g. your upstream) and push to another place (e.g. your own publishing repository), Then the rest of the text flows more naturally without ever introducing a new lingo triangular that is not in glossary. + ... 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. -- 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: Tag peeling peculiarities
[this email is from last week, and I think most of was responded to in other parts of the thread, but there were a few loose ends] On Sat, Mar 16, 2013 at 02:38:12PM +0100, Michael Haggerty wrote: * Change pack-refs to use the peeled information from ref_entries if it is available, rather than looking up the references again. I don't know that it matters from a performance standpoint (we don't really care how long pack-refs takes, as long as it is within reason, because it is run as part of a gc). But it would mean that any errors in the file are propagated from one run of pack-refs to the next. I don't know if we want to spend the extra time to be more robust. I thought about this argument too. Either way is OK with me. BTW would it make sense to build a consistency check of peeled references into fsck? Yeah, I do think an fsck check makes sense. It should not be expensive, and if there is a realistic corruption/health check for a repo, it makes sense to me for it to be part of fsck. I do not recall many incidents of packed-refs corruption in the history of the list, but this fairly recent one comes to mind: http://thread.gmane.org/gmane.comp.version-control.git/217065 On the other hand, if it is just as cheap to rewrite the file as it is to do the health checks, I wonder if the advice should simply be run pack-refs again (and doing so should always start from scratch, not trusting the existing version). Yuck. I really wonder if repack_without_ref should literally just be writing out the exact same file, minus the lines for the deleted ref. Is there a reason we need to do any processing at all? I guess the answer is that you want to avoid re-parsing the current file, and just write it out from memory. But don't we need to refresh the memory cache from disk under the lock anyway? That was the pack-refs race that I fixed recently. It would certainly be thinkable to rewrite the file textually without parsing it again. But I did it this way for two reasons: 1. It would be better to have one packed-refs file parser and writer rather than two. (I'm working towards unifying the two writers, and will continue once I understand their differences.) I see your point, though I also feel that with the right refactoring, they could share the parser. And the second writer would be mostly a pass-through. But much more compelling is reason 2... 2. Given how peeled references were added, it seems dangerous to read, modify, and write data that might be in a future format that we don't entirely understand. For example, suppose a new feature is added to mark Junio's favorite tags: # pack-refs with: peeled fully-peeled junios-favorites \n ce432cac30f98b291be609a0fc974042a2156f55 refs/heads/master 83b3dd7151e7eb0e5ac62ee03aca7e6bccaa8d07 refs/tags/evil-dogs ^e1d04f8aad59397cbd55e72bf8a1bd75606f5350 7ed863a85a6ce2c4ac4476848310b8f917ab41f9 refs/tags/lolcats ^990f856a62a24bfd56bac1f5e4581381369e4ede ^^^junios-favorite b0517166ae2ad92f3b17638cbdee0f04b8170d99 refs/tags/nonsense ^4baff50551545e2b6825973ec37bcaf03edb95fe Hmm. Good point. It is tempting to make a rule like extensions that are specific to a ref must come after the ref but before the next ref. And then the writer could simply drop any lines between the to-delete ref and the one that follows it. I think that would work OK in practice, but I am worried that it would paint us into a corner later on (after all, we do not know what extensions will be added in the future). The only thing I can think of that would break is something where a ref or its extension depends on previous entries. E.g., we start prefix-compressing the ref names to save space. Of course that would break backwards compatibility horribly anyway, so it's a no-go. But maybe some extension would want to refer to a prior ref. I dunno. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git branch: multiple --merged and --no-merged options?
On Fri, Mar 15, 2013 at 02:38:12PM -0500, Jed Brown wrote: I find myself frequently running commands like this $ comm -12 (git branch --no-merged master) (git branch --merged next) That's a reasonable thing to want to do. when checking for graduation candidates. Of course I first tried $ git branch --no-merged master --merged next Yeah, sadly that does not work, as we use the same slot for the flag and store only one of the two (and we also allow only one --merged head, even though you could in theory want to know merged to X, or merged to Y). I do not think there is a reason we could handle both. I think we could even do it with a single traversal, but even with two traversals, doing both in-process will be faster (because we only have to pull the commits from disk once). So I think it is something that ought to work, but it will need some code written. Patches welcome. ;) -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Memory corruption when rebasing with git version 1.8.1.5 on arch
Junio C Hamano gits...@pobox.com writes:
Jeff King p...@peff.net writes:
and so on. I haven't quite figured out what is going on. It looks like
we call update_pre_post_images with postlen==0, which causes it to just
write the fixed postimage into the existing buffer. But of course the
fixed version is bigger, because we are expanding the tabs into 8
spaces (but it _doesn't_ break if each line starts with only one tab,
which confuses me).
I used to be intimately familiar with the update_pre_post_images()
function, but the version after 86c91f91794c (git apply: option to
ignore whitespace differences, 2009-08-04), I won't vouch for it
doing anything sensible. We recently had to do 5de7166d46d2
(apply.c:update_pre_post_images(): the preimage can be truncated,
2012-10-12) to fix one of its corner cases but I would not be
surprised if there are other cases the function gets it all wrong.
I'd come back to the topic after I finish other tasks on my plate,
so if somebody is inclined please go ahead digging this a bit
further; I won't have much head start to begin with in this code
X-.
This may be sufficient. In the olden days, we relied on that all
whitespace fixing rules made the result shorter and took advantage
of it in update-pre-post-images to rewrite the images in place. The
oddball tab-in-indent (aka Python), however, can grow the result by
expanding tabs (deemed incorrect) in the input into runs of spaces
(deemed kosher).
Fortunately, we already support its more generalized form match
while ignoring whitespace differences that can lengthen the result;
as long as we correctly count the number of bytes needed to hold
rewritten postimage, the existing logic in update_pre_post_images
should be able to do the rest for us.
builtin/apply.c | 16 ++--
t/t4124-apply-ws-rule.sh | 26 ++
t/t4150-am.sh| 2 +-
t/test-lib-functions.sh | 4 ++--
4 files changed, 39 insertions(+), 9 deletions(-)
diff --git a/builtin/apply.c b/builtin/apply.c
index 080ce2e..cad4e4f 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2117,10 +2117,10 @@ static void update_pre_post_images(struct image
*preimage,
/*
* Adjust the common context lines in postimage. This can be
-* done in-place when we are just doing whitespace fixing,
-* which does not make the string grow, but needs a new buffer
-* when ignoring whitespace causes the update, since in this case
-* we could have e.g. tabs converted to multiple spaces.
+* done in-place when we are shrinking it with whitespace
+* fixing, but needs a new buffer when ignoring whitespace or
+* expanding leading tabs to spaces.
+*
* We trust the caller to tell us if the update can be done
* in place (postlen==0) or not.
*/
@@ -2185,7 +2185,7 @@ static int match_fragment(struct image *img,
int i;
char *fixed_buf, *buf, *orig, *target;
struct strbuf fixed;
- size_t fixed_len;
+ size_t fixed_len, postlen;
int preimage_limit;
if (preimage-nr + try_lno = img-nr) {
@@ -2335,6 +2335,7 @@ static int match_fragment(struct image *img,
strbuf_init(fixed, preimage-len + 1);
orig = preimage-buf;
target = img-buf + try;
+ postlen = 0;
for (i = 0; i preimage_limit; i++) {
size_t oldlen = preimage-line[i].len;
size_t tgtlen = img-line[try_lno + i].len;
@@ -2362,6 +2363,7 @@ static int match_fragment(struct image *img,
match = (tgtfix.len == fixed.len - fixstart
!memcmp(tgtfix.buf, fixed.buf + fixstart,
fixed.len - fixstart));
+ postlen += tgtfix.len;
strbuf_release(tgtfix);
if (!match)
@@ -2399,8 +2401,10 @@ static int match_fragment(struct image *img,
* hunk match. Update the context lines in the postimage.
*/
fixed_buf = strbuf_detach(fixed, fixed_len);
+ if (postlen postimage-len)
+ postlen = 0;
update_pre_post_images(preimage, postimage,
- fixed_buf, fixed_len, 0);
+ fixed_buf, fixed_len, postlen);
return 1;
unmatch_exit:
diff --git a/t/t4124-apply-ws-rule.sh b/t/t4124-apply-ws-rule.sh
index 6f6ee88..0bbcf06 100755
--- a/t/t4124-apply-ws-rule.sh
+++ b/t/t4124-apply-ws-rule.sh
@@ -486,4 +486,30 @@ test_expect_success 'same, but with CR-LF line endings
cr-at-eol unset' '
test_cmp one expect
'
+test_expect_success 'whitespace=fix to expand' '
+ qz_to_tab_space preimage -\EOF
+ QQa
+ QQb
+ QQc
+ d
+ QQe
+ QQf
+ QQg
+ EOF
+ qz_to_tab_space patch -\EOF
+ diff --git a/preimage b/preimage
+ --- a/preimage
+ +++ b/preimage
+ @@
Re: feature request - have git honor nested .gitconfig files
Hi Josh, Josh Sharpe wrote: For example, I have my ~/.gitconfig that has one email address in it, but I also have multiple repos inside ~/dev which I want to use a different email address for. The only way to do that now is to edit all of these: ~/dev/*/.git/conf -- and there are lots of them, and new repos get added all the time - and I forget. A couple of ideas using existing git features: - A wrapper script around git init can take care of setting up the shared configuration appropriately based on the repository path. - The extra configuration can be applied on a per-cwd instead of a per-repository basis. Some shells provide a PROMPT_COMMAND facility that can be used to run a command (for example set up environment) each time the prompt is displayed. A PROMPT_COMMAND could set the environment variable EMAIL or GIT_EMAIL based on the value of $PWD. Room for improvement: * A new repository can be created by git init or git clone and the path where the repository will live is not immediately obvious from the command line, so setting up thorough wrappers is not actually that easy. So this sounds like a good place to provide a hook. (It could be called new-repository or something.) * Maintaining configuration per repository to record a rather simple is more complicated than ideal. It would be easier to understand the configuration if ~/.gitconfig could spell out the rule explicitly: [include] path = cond(starts_with($GIT_DIR, ~/dev/), ~/.config/git/dev-config, ~/.config/git/nondev-config) This means supporting an extension language in the config file. It sounds hard to do right, especially considering use cases like User runs into trouble, asks a privileged sysadmin to try running a command in her untrusted repository, but it is worth thinking about how to do. * The Includes facility is annoyingly close to being helpful. An include.path setting from ~/.gitconfig cannot refer to $GIT_DIR by name. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [BUG?] rebase -i: edit versus unmerged changes
Ramkumar Ramachandra wrote: I'd really to have that final 'git continue' in Git 2.0. Can someone attempt to break up the migration path into manageable logical steps that we can start working on? Is there any deadline or migration path needed? Depending on the design, it might be possible to do without backward-incompatible changes, meaning the migration path is whatever someone feels like implementing first lands first. Hope that helps, Jonathan -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/6] remote.c: simplify a bit of code using git_config_string()
Ramkumar Ramachandra wrote: A small segment where handle_config() parses the branch.remote configuration variable can be simplified using git_config_string(). Looks 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 2/6] t5516 (fetch-push): update test description
Ramkumar Ramachandra wrote: --- 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' The description before and after are equally useless. You might as well use the following description: test_description='t5516-fetch-push.sh' (Please don't actually do that.) I gave a sketch of what I think might be a more useful description and got shot down without an explanation I could understand in reply. So, this one needs more work I guess. Hope that helps, Jonathan -- 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: feature request - have git honor nested .gitconfig files
On Fri, Mar 22, 2013 at 11:22:11AM -0700, Jonathan Nieder wrote: * Maintaining configuration per repository to record a rather simple is more complicated than ideal. It would be easier to understand the configuration if ~/.gitconfig could spell out the rule explicitly: [include] path = cond(starts_with($GIT_DIR, ~/dev/), ~/.config/git/dev-config, ~/.config/git/nondev-config) This means supporting an extension language in the config file. It sounds hard to do right, especially considering use cases like User runs into trouble, asks a privileged sysadmin to try running a command in her untrusted repository, but it is worth thinking about how to do. I'd rather not invent a new language. It will either not be featureful enough, or will end up bloated. Or both. How about something like: [include] exec = case \$GIT_DIR\ in) */dev/*) cat ~/.config/git/dev-config ;; *) cat ~/.config/git/nondev-config ;; esac It involves a shell invocation, but it's not like we parse config in a tight loop. Bonus points if git provides the name of the current config file, so exec can use relative paths like: cat $(dirname $GIT_CONFIG_FILE)/dev-config * The Includes facility is annoyingly close to being helpful. An include.path setting from ~/.gitconfig cannot refer to $GIT_DIR by name. Yeah, we do not allow variable expansion at all beyond the usual path mechanisms. I think if you had $GIT_DIR, though, it would end up annoying. You do not want one file in ~/.config/git per $GIT_DIR, so you would need some way of munging $GIT_DIR into your naming scheme. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] Fix revision walk for commits with the same dates
Logic in still_interesting function allows to stop the commits traversing if the oldest processed commit is not older then the youngest commit on the list to process and the list contains only commits marked as not interesting ones. It can be premature when dealing with a set of coequal commits. For example git rev-list A^! --not B provides wrong answer if all commits in the range A..B had the same commit time and there are more then 7 of them. To fix this problem the relevant part of the logic in still_interesting is changed to: the walk can be stopped if the oldest processed commit is younger then the youngest commit on the list to processed. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- I don't know whether the first version was overlooked or deemed as not worthy. So just in case I resend it. Changes since the first version: 1. The test has been added 2. The commit log has been rewritten revision.c | 2 +- t/t6009-rev-list-parent.sh | 13 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/revision.c b/revision.c index ef60205..cf620c6 100644 --- a/revision.c +++ b/revision.c @@ -709,7 +709,7 @@ static int still_interesting(struct commit_list *src, unsigned long date, int sl * Does the destination list contain entries with a date * before the source list? Definitely _not_ done. */ - if (date src-item-date) + if (date = src-item-date) return SLOP; /* diff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh index 3050740..66cda17 100755 --- a/t/t6009-rev-list-parent.sh +++ b/t/t6009-rev-list-parent.sh @@ -133,4 +133,17 @@ test_expect_success 'dodecapus' ' check_revlist --min-parents=13 check_revlist --min-parents=4 --max-parents=11 tetrapus ' + +test_expect_success 'ancestors with the same commit time' ' + + test_tick_keep=$test_tick + for i in 1 2 3 4 5 6 7 8; do + test_tick=$test_tick_keep + test_commit t$i + done + git rev-list t1^! --not t$i result + expect + test_cmp expect result +' + test_done -- 1.8.2 -- Kacper Kornet -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git merge heuristic
Hi all, I want to learn about how Git compares patches while doing a merge. For example, if a patch has been cherry-picked from branch A to branch B, and then downstream we do a git merge from A to B, how does Git know to skip the cherry-picked patch? It would have a different SHA-1, so what is the comparison algorithm/heuristic? What happens if the comment is different, but the actual patch is identical? I searched around, but couldn't find information on this. I would appreciate it if someone could point me in the right direction. Thanks! -Senthil -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Improve difftool --dir-diff tests
How about doing this? The first patch is a cleanup as suggested by Johannes[1], the second fixes the test failure on Windows and the third makes the test behaviour more explicit and would have helped to detect this issue earlier. [1/3] t7800: don't hide grep output [2/3] t7800: fix tests when difftool uses --no-symlinks [3/3] t7800: run --dir-diff tests with and without symlinks t/t7800-difftool.sh | 71 +++-- 1 file changed, 36 insertions(+), 35 deletions(-) -- 1.8.2.324.ga64ebd9 -- 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/3] t7800: fix tests when difftool uses --no-symlinks
When 'git difftool --dir-diff' is using --no-symlinks (either explicitly or implicitly because it's running on Windows), any working tree files that have been copied to the temporary directory are copied back after the difftool completes. This includes untracked files in the working tree. During the tests, this means that the following sequence occurs: 1) the shell opens output to redirect the difftool output 2) difftool copies the empty output to the temporary directory 3) difftool runs ls which writes to output 4) difftool copies the empty output file back over the output of the command 5) the output files doesn't contain the expected output, causing the test to fail Avoid this by writing the output into .git/ which will not be copied or overwritten. In the longer term, difftool probably needs to learn to warn the user instead of overwrite any changes that have been made to the working tree file. Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7800-difftool.sh | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index e694972..1eed439 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -319,29 +319,29 @@ test_expect_success PERL 'setup change in subdirectory' ' ' test_expect_success PERL 'difftool -d' ' - git difftool -d --extcmd ls branch output - grep sub output - grep file output + git difftool -d --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff' ' - git difftool --dir-diff --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff ignores --prompt' ' - git difftool --dir-diff --prompt --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --prompt --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff from subdirectory' ' ( cd sub - git difftool --dir-diff --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --extcmd ls branch ../.git/output + grep sub ../.git/output + grep file ../.git/output ) ' -- 1.8.2.324.ga64ebd9 -- 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/3] t7800: run --dir-diff tests with and without symlinks
Currently the difftool --dir-diff tests may or may not use symlinks
depending on the operating system on which they are run. In one case
this has caused a test failure to be noticed only on Windows when the
test also fails on Linux when difftool is invoked with --no-symlinks.
Rewrite these tests so that they do not depend on the environment but
run explicitly with both --symlinks and --no-symlinks, protecting the
--symlinks version with a SYMLINKS prerequisite.
Signed-off-by: John Keeping j...@keeping.me.uk
---
t/t7800-difftool.sh | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 1eed439..4a70508 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -318,25 +318,36 @@ test_expect_success PERL 'setup change in subdirectory' '
git commit -m modified both
'
-test_expect_success PERL 'difftool -d' '
+run_dir_diff_test () {
+ test_expect_success PERL $1 --no-symlinks
+ symlinks=--no-symlinks
+ $2
+
+ test_expect_success PERL,SYMLINKS $1 --symlinks
+ symlinks=--symlinks
+ $2
+
+}
+
+run_dir_diff_test 'difftool -d' '
git difftool -d --extcmd ls branch .git/output
grep sub .git/output
grep file .git/output
'
-test_expect_success PERL 'difftool --dir-diff' '
+run_dir_diff_test 'difftool --dir-diff' '
git difftool --dir-diff --extcmd ls branch .git/output
grep sub .git/output
grep file .git/output
'
-test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
+run_dir_diff_test 'difftool --dir-diff ignores --prompt' '
git difftool --dir-diff --prompt --extcmd ls branch .git/output
grep sub .git/output
grep file .git/output
'
-test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+run_dir_diff_test 'difftool --dir-diff from subdirectory' '
(
cd sub
git difftool --dir-diff --extcmd ls branch ../.git/output
--
1.8.2.324.ga64ebd9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Git.pm with recent File::Temp fail
git-1.8.2, perl-5.16.3, File::Temp-0.23
Without patch:
$ git svn fetch
'tempfile' can't be called as a method at
/pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.
After patch:
$ git svn fetch
M t/06virtual.t
r15506 = 6c65be7ff36ffc6fd9b960a4b470ca297103004e (refs/remotes/git-svn)
⋮
patch attached
--
H.Merijn Brand http://tux.nl Perl Monger http://amsterdam.pm.org/
using perl5.00307 .. 5.17 porting perl5 on HP-UX, AIX, and openSUSE
http://mirrors.develooper.com/hpux/http://www.test-smoke.org/
http://qa.perl.org http://www.goldmark.org/jeff/stupid-disclaimers/
From e78bf3e99deb26050f8515076db63075f6d0d171 Mon Sep 17 00:00:00 2001
From: H.Merijn Brand - Tux h.m.br...@xs4all.nl
Date: Fri, 22 Mar 2013 20:56:53 +0100
Subject: [PATCH] Syntax error in Git.pm for File::Temp-0.23
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary=1.8.2
This is a multi-part message in MIME format.
--1.8.2
Content-Type: text/plain; charset=UTF-8; format=fixed
Content-Transfer-Encoding: 8bit
Testing with perl-5.16.3 and most recent File::Temp-0.23 revealed:
$ git svn fetch
'tempfile' can't be called as a method at /pro/lib/perl5/site_perl/5.16.3/Git.pm line 1117.
---
perl/Git.pm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--1.8.2
Content-Type: text/x-patch; name=0001-Syntax-error-in-Git.pm-for-File-Temp-0.23.patch
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment; filename=0001-Syntax-error-in-Git.pm-for-File-Temp-0.23.patch
diff --git a/perl/Git.pm b/perl/Git.pm
index 96cac39..cf4f54a 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -1265,7 +1265,7 @@ sub _temp_cache {
$tmpdir = $self-repo_path();
}
- ($$temp_fd, $fname) = File::Temp-tempfile(
+ ($$temp_fd, $fname) = File::Temp::tempfile(
'Git_XX', UNLINK = 1, DIR = $tmpdir,
) or throw Error::Simple(couldn't open new temp file);
--1.8.2--
Re: [BUG?] rebase -i: edit versus unmerged changes
Jonathan Nieder jrnie...@gmail.com writes: Ramkumar Ramachandra wrote: I'd really to have that final 'git continue' in Git 2.0. Can someone attempt to break up the migration path into manageable logical steps that we can start working on? Is there any deadline or migration path needed? Depending on the design, it might be possible to do without backward-incompatible changes, meaning the migration path is whatever someone feels like implementing first lands first. FWIW, I am not convinced yet why we would even want git continue in the first place, so I won't be the one who would be suggesting a migration path. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Fix revision walk for commits with the same dates
Kacper Kornet drae...@pld-linux.org writes: Logic in still_interesting function allows to stop the commits traversing if the oldest processed commit is not older then the youngest commit on the list to process and the list contains only commits marked as not interesting ones. It can be premature when dealing with a set of coequal commits. For example git rev-list A^! --not B provides wrong answer if all commits in the range A..B had the same commit time and there are more then 7 of them. To fix this problem the relevant part of the logic in still_interesting is changed to: the walk can be stopped if the oldest processed commit is younger then the youngest commit on the list to processed. Is the made-up test case to freeze the clock even interesting? The slop logic is merely a heuristic to compensate for effects caused by skewed or non-monototic clocks, so in a different repository you may even need to fuzz the timestamp comparison further if (date - 10 src-item-date) or something silly like that. Signed-off-by: Kacper Kornet drae...@pld-linux.org --- I don't know whether the first version was overlooked or deemed as not worthy. So just in case I resend it. Changes since the first version: 1. The test has been added 2. The commit log has been rewritten revision.c | 2 +- t/t6009-rev-list-parent.sh | 13 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/revision.c b/revision.c index ef60205..cf620c6 100644 --- a/revision.c +++ b/revision.c @@ -709,7 +709,7 @@ static int still_interesting(struct commit_list *src, unsigned long date, int sl * Does the destination list contain entries with a date * before the source list? Definitely _not_ done. */ - if (date src-item-date) + if (date = src-item-date) return SLOP; /* diff --git a/t/t6009-rev-list-parent.sh b/t/t6009-rev-list-parent.sh index 3050740..66cda17 100755 --- a/t/t6009-rev-list-parent.sh +++ b/t/t6009-rev-list-parent.sh @@ -133,4 +133,17 @@ test_expect_success 'dodecapus' ' check_revlist --min-parents=13 check_revlist --min-parents=4 --max-parents=11 tetrapus ' + +test_expect_success 'ancestors with the same commit time' ' + + test_tick_keep=$test_tick + for i in 1 2 3 4 5 6 7 8; do + test_tick=$test_tick_keep + test_commit t$i + done + git rev-list t1^! --not t$i result + expect + test_cmp expect result +' + test_done -- 1.8.2 -- 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/3 v2] t7800: run --dir-diff tests with and without symlinks
Currently the difftool --dir-diff tests may or may not use symlinks
depending on the operating system on which they are run. In one case
this has caused a test failure to be noticed only on Windows when the
test also fails on Linux when difftool is invoked with --no-symlinks.
Rewrite these tests so that they do not depend on the environment but
run explicitly with both --symlinks and --no-symlinks, protecting the
--symlinks version with a SYMLINKS prerequisite.
Signed-off-by: John Keeping j...@keeping.me.uk
---
The previous version of this was missing half the intended change :-(
Sorry for the noise.
t/t7800-difftool.sh | 27 +++
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index 1eed439..bba8a9d 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh
@@ -318,28 +318,39 @@ test_expect_success PERL 'setup change in subdirectory' '
git commit -m modified both
'
-test_expect_success PERL 'difftool -d' '
- git difftool -d --extcmd ls branch .git/output
+run_dir_diff_test () {
+ test_expect_success PERL $1 --no-symlinks
+ symlinks=--no-symlinks
+ $2
+
+ test_expect_success PERL,SYMLINKS $1 --symlinks
+ symlinks=--symlinks
+ $2
+
+}
+
+run_dir_diff_test 'difftool -d' '
+ git difftool -d $symlinks --extcmd ls branch .git/output
grep sub .git/output
grep file .git/output
'
-test_expect_success PERL 'difftool --dir-diff' '
- git difftool --dir-diff --extcmd ls branch .git/output
+run_dir_diff_test 'difftool --dir-diff' '
+ git difftool --dir-diff $symlinks --extcmd ls branch .git/output
grep sub .git/output
grep file .git/output
'
-test_expect_success PERL 'difftool --dir-diff ignores --prompt' '
- git difftool --dir-diff --prompt --extcmd ls branch .git/output
+run_dir_diff_test 'difftool --dir-diff ignores --prompt' '
+ git difftool --dir-diff $symlinks --prompt --extcmd ls branch
.git/output
grep sub .git/output
grep file .git/output
'
-test_expect_success PERL 'difftool --dir-diff from subdirectory' '
+run_dir_diff_test 'difftool --dir-diff from subdirectory' '
(
cd sub
- git difftool --dir-diff --extcmd ls branch ../.git/output
+ git difftool --dir-diff $symlinks --extcmd ls branch
../.git/output
grep sub ../.git/output
grep file ../.git/output
)
--
1.8.2.324.ga64ebd9
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] Fix revision walk for commits with the same dates
On Fri, Mar 22, 2013 at 01:45:47PM -0700, Junio C Hamano wrote: Kacper Kornet drae...@pld-linux.org writes: Logic in still_interesting function allows to stop the commits traversing if the oldest processed commit is not older then the youngest commit on the list to process and the list contains only commits marked as not interesting ones. It can be premature when dealing with a set of coequal commits. For example git rev-list A^! --not B provides wrong answer if all commits in the range A..B had the same commit time and there are more then 7 of them. To fix this problem the relevant part of the logic in still_interesting is changed to: the walk can be stopped if the oldest processed commit is younger then the youngest commit on the list to processed. Is the made-up test case to freeze the clock even interesting? The slop logic is merely a heuristic to compensate for effects caused by skewed or non-monototic clocks, so in a different repository you may even need to fuzz the timestamp comparison further if (date - 10 src-item-date) or something silly like that. I don't think it is a made-up test case. For example it is easy to get a number of coequal commits by using git rebase -i. So I argue that git should treat correctly ranges of such commits. -- Kacper Kornet -- 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 7/4] submodule: clarify logic in show_submodule_summary
Jeff King p...@peff.net writes:
There are two uses of the left and right commit
variables that make it hard to be sure what values they
have (both for the reader, and for gcc, which wrongly
complains that they might be used uninitialized).
The functions starts with a cascading if statement, checking
that the input sha1s exist, and finally working up to
preparing a revision walk. We only prepare the walk if the
cascading conditional did not find any problems, which we
check by seeing whether it set the message variable or
not. It's simpler and more obvious to just add a condition
to the end of the cascade.
Later, we check the same message variable when deciding
whether to clear commit marks on the left/right commits; if
it is set, we presumably never started the walk. This is
wrong, though; we might have started the walk and munged
commit flags, only to encounter an error afterwards. We
should always clear the flags on left/right if they exist,
whether the walk was successful or not.
Signed-off-by: Jeff King p...@peff.net
---
Looks good. Thanks.
submodule.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/submodule.c b/submodule.c
index 9ba1496..975bc87 100644
--- a/submodule.c
+++ b/submodule.c
@@ -261,7 +261,7 @@ void show_submodule_summary(FILE *f, const char *path,
const char *del, const char *add, const char *reset)
{
struct rev_info rev;
- struct commit *left = left, *right = right;
+ struct commit *left = NULL, *right = NULL;
const char *message = NULL;
struct strbuf sb = STRBUF_INIT;
int fast_forward = 0, fast_backward = 0;
@@ -275,10 +275,8 @@ void show_submodule_summary(FILE *f, const char *path,
else if (!(left = lookup_commit_reference(one)) ||
!(right = lookup_commit_reference(two)))
message = (commits not present);
-
- if (!message
- prepare_submodule_summary(rev, path, left, right,
- fast_forward, fast_backward))
+ else if (prepare_submodule_summary(rev, path, left, right,
+fast_forward, fast_backward))
message = (revision walker failed);
if (dirty_submodule DIRTY_SUBMODULE_UNTRACKED)
@@ -302,11 +300,12 @@ void show_submodule_summary(FILE *f, const char *path,
strbuf_addf(sb, %s:%s\n, fast_backward ? (rewind) : ,
reset);
fwrite(sb.buf, sb.len, 1, f);
- if (!message) {
+ if (!message) /* only NULL if we succeeded in setting up the walk */
print_submodule_summary(rev, f, del, add, reset);
+ if (left)
clear_commit_marks(left, ~0);
+ if (right)
clear_commit_marks(right, ~0);
- }
strbuf_release(sb);
}
--
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] t5516 (fetch-push): introduce mk_test_with_name()
Junio C Hamano wrote: I would prefer to see a preparatory patch to teach mk_test/mk_empty to _always_ take the new name (i.e. the result of your patch) and then do whatever new things on top. Yes, that sounds like a good way to go. By the way, I am planning to _not_ look at new stuff today. I'd rather see known recent regressions addressed (and unknown ones discovered and squashed) before we move forward, and I would appreciate if regular contributors did the same. I've been flushing out my thoughts to avoid forgetting them. ;-) Agreed, though. 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 4/6] remote.c: introduce a way to have different remotes for fetch/push
Ramkumar Ramachandra wrote:
This patch has no visible impact, but
serves to enable future patches to introduce configuration variables
to set pushremote_name. For example, you can now do the following in
handle_config():
if (!strcmp(key, remote.pushdefault))
git_config_string(pushremote_name, key, value);
Thanks.
[...]
--- 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);
struct remote has url and pushurl fields. What do they mean in the
context of these two accessors? /me is confused.
Is the idea that now I should not use pushurl any more, and that I
should use pushremote_get and use url instead?
Hope that helps,
Jonathan
--
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: Memory corruption when rebasing with git version 1.8.1.5 on arch
On Fri, Mar 22, 2013 at 11:08:59AM -0700, Junio C Hamano wrote: This may be sufficient. In the olden days, we relied on that all whitespace fixing rules made the result shorter and took advantage of it in update-pre-post-images to rewrite the images in place. The oddball tab-in-indent (aka Python), however, can grow the result by expanding tabs (deemed incorrect) in the input into runs of spaces (deemed kosher). Fortunately, we already support its more generalized form match while ignoring whitespace differences that can lengthen the result; as long as we correctly count the number of bytes needed to hold rewritten postimage, the existing logic in update_pre_post_images should be able to do the rest for us. Yeah, your patch looks right to me. I do wonder if the optimization here: @@ -2399,8 +2401,10 @@ static int match_fragment(struct image *img, * hunk match. Update the context lines in the postimage. */ fixed_buf = strbuf_detach(fixed, fixed_len); + if (postlen postimage-len) + postlen = 0; update_pre_post_images(preimage, postimage, -fixed_buf, fixed_len, 0); +fixed_buf, fixed_len, postlen); should simply go into update_pre_post_images (i.e., let it decide on whether to do it inline or with a new allocation, rather than making postlen==0 special). That would let the ignore-whitespace code path use the optimization, too (when it's possible). By the way, I notice that when update_pre_post_images does allocate, the old value of postimage-buf is lost. It looks like that is not leaked, because it was pointing to a strbuf (newlines in apply_one_fragment) that we are going to release anyway afterwards. But that means nobody is freeing postimage-buf, which means that our newly malloc'd version is getting leaked. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/4] match-trees: drop x = x initializations
Jeff King p...@peff.net writes:
Of the 8 patches, this is the one I find the least satisfying, if only
because I do not think gcc's failure is because of complicated control
flow, and rearranging the code would only hurt readability. And I'm
quite curious why it complains about mode, but not about the other
variables, which are set in the exact same place (and why it would not
be able to handle such a simple control flow at all).
It makes me wonder if I am missing something, or there is some subtle
bug. But I can't see it. Other eyes appreciated.
I obviously am not qualified as other eyes to catch bugs in this
code as this is entirely mine, but I do not see any obvious reason
that would make the compiler to think mode[12] less initialized than
elem[12] or path[12] either.
These three are all updated by the same tree_entry_extract() call,
and whenever we use mode[12] we use path[12], so if it decides path1
is used or assigned, it should be able to tell mode1 is, too.
Unsatisfactory, it surely is...
match-trees.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/match-trees.c b/match-trees.c
index 26f7ed1..4360f10 100644
--- a/match-trees.c
+++ b/match-trees.c
@@ -71,13 +71,13 @@ static int score_trees(const unsigned char *hash1, const
unsigned char *hash2)
if (type != OBJ_TREE)
die(%s is not a tree, sha1_to_hex(hash2));
init_tree_desc(two, two_buf, size);
- while (one.size | two.size) {
- const unsigned char *elem1 = elem1;
- const unsigned char *elem2 = elem2;
- const char *path1 = path1;
- const char *path2 = path2;
- unsigned mode1 = mode1;
- unsigned mode2 = mode2;
+ while (one.size || two.size) {
+ const unsigned char *elem1;
+ const unsigned char *elem2;
+ const char *path1;
+ const char *path2;
+ unsigned mode1 = 0; /* make gcc happy */
+ unsigned mode2 = 0; /* make gcc happy */
int cmp;
if (one.size)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 06/45] Add parse_pathspec() that converts cmdline args to struct pathspec
Nguyễn Thái Ngọc Duy pclo...@gmail.com writes:
diff --git a/Documentation/technical/api-setup.txt
b/Documentation/technical/api-setup.txt
index 4f63a04..59a947e 100644
--- a/Documentation/technical/api-setup.txt
+++ b/Documentation/technical/api-setup.txt
@@ -8,6 +8,23 @@ Talk about
* is_inside_git_dir()
* is_inside_work_tree()
* setup_work_tree()
-* get_pathspec()
(Dscho)
+
+Pathspec
+
asciidoc: ERROR: api-setup.txt: line 15: only book doctypes can
contain level 0 sections
Just demoting this to should be sufficient, I think.
+
+See glossary-context.txt for the syntax of pathspec. In memory, a
+pathspec set is represented by struct pathspec and is prepared by
+parse_pathspec(). This function takes several arguments:
+
+- magic_mask specifies what features that are NOT supported by the
+ following code. If a user attempts to use such a feature,
+ parse_pathspec() can reject it early.
+
+- flags specifies other things that the caller wants parse_pathspec to
+ perform.
+
+- prefix and args come from cmd_* functions
+
+get_pathspec() is obsolete and should never be used in new code.
diff --git a/dir.c b/dir.c
index 97ad45b..a442467 100644
--- a/dir.c
+++ b/dir.c
@@ -338,7 +338,7 @@ int match_pathspec_depth(const struct pathspec *ps,
/*
* Return the length of the simple part of a path match limiter.
*/
-static int simple_length(const char *match)
+int simple_length(const char *match)
{
int len = -1;
@@ -350,7 +350,7 @@ static int simple_length(const char *match)
}
}
-static int no_wildcard(const char *string)
+int no_wildcard(const char *string)
{
return string[simple_length(string)] == '\0';
}
diff --git a/dir.h b/dir.h
index c3eb4b5..89427fd 100644
--- a/dir.h
+++ b/dir.h
@@ -125,6 +125,8 @@ struct dir_struct {
#define MATCHED_RECURSIVELY 1
#define MATCHED_FNMATCH 2
#define MATCHED_EXACTLY 3
+extern int simple_length(const char *match);
+extern int no_wildcard(const char *string);
extern char *common_prefix(const char **pathspec);
extern int match_pathspec(const char **pathspec, const char *name, int
namelen, int prefix, char *seen);
extern int match_pathspec_depth(const struct pathspec *pathspec,
diff --git a/pathspec.c b/pathspec.c
index ab53b8a..ebe9844 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -103,10 +103,6 @@ void die_if_path_beyond_symlink(const char *path, const
char *prefix)
/*
* Magic pathspec
*
- * NEEDSWORK: These need to be moved to dir.h or even to a new
- * pathspec.h when we restructure get_pathspec() users to use the
- * struct pathspec interface.
- *
* Possible future magic semantics include stuff like:
*
* { PATHSPEC_NOGLOB, '!', noglob },
@@ -115,7 +111,6 @@ void die_if_path_beyond_symlink(const char *path, const
char *prefix)
* { PATHSPEC_REGEXP, '\0', regexp },
*
*/
-#define PATHSPEC_FROMTOP(10)
static struct pathspec_magic {
unsigned bit;
@@ -127,7 +122,7 @@ static struct pathspec_magic {
/*
* Take an element of a pathspec and check for magic signatures.
- * Append the result to the prefix.
+ * Append the result to the prefix. Return the magic bitmap.
*
* For now, we only parse the syntax and throw out anything other than
* top magic.
@@ -138,10 +133,15 @@ static struct pathspec_magic {
* the prefix part must always match literally, and a single stupid
* string cannot express such a case.
*/
-static const char *prefix_pathspec(const char *prefix, int prefixlen, const
char *elt)
+static unsigned prefix_pathspec(struct pathspec_item *item,
+ unsigned *p_short_magic,
+ const char **raw, unsigned flags,
+ const char *prefix, int prefixlen,
+ const char *elt)
{
- unsigned magic = 0;
+ unsigned magic = 0, short_magic = 0;
const char *copyfrom = elt;
+ char *match;
int i;
if (elt[0] != ':') {
@@ -183,7 +183,7 @@ static const char *prefix_pathspec(const char *prefix,
int prefixlen, const char
break;
for (i = 0; i ARRAY_SIZE(pathspec_magic); i++)
if (pathspec_magic[i].mnemonic == ch) {
- magic |= pathspec_magic[i].bit;
+ short_magic |= pathspec_magic[i].bit;
break;
}
if (ARRAY_SIZE(pathspec_magic) = i)
@@ -194,15 +194,128 @@ static const char *prefix_pathspec(const char *prefix,
int prefixlen, const char
copyfrom++;
}
+ magic |= short_magic;
+ *p_short_magic = short_magic;
+
if (magic PATHSPEC_FROMTOP)
- return xstrdup(copyfrom);
+ match = xstrdup(copyfrom);
else
-
Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
Jonathan Nieder jrnie...@gmail.com writes:
--- 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);
struct remote has url and pushurl fields. What do they mean in the
context of these two accessors? /me is confused.
Is the idea that now I should not use pushurl any more, and that I
should use pushremote_get and use url instead?
I thought the basic idea from the user-level is:
- If you have to use different URL to push to and fetch from the
logically same location (e.g. git://k.org/pub/scm/git/git.git
used for fetch, k.org:/pub/scm/git/git.git/ used for push), use
url for fetch, pushurl for push and you don't have to bother with
per-branch pushremote at all. You are logically working with the
same remote, perhaps called 'origin'.
- If you push to and fetch from logically different repositories,
(e.g. fetch from https://github.com/gitster/git, push to
github.com:artagnon/git), you may want to call your upstream
'origin' and your publishing repository 'mine'. You set the
remote.pushdefault to 'mine', perhaps like:
[remote mine]
url = github.com:artagnon/git
(this can also be written with remote.mine.pushurl).
By splitting remote_get() used for fetch and pushremote_get() used
for push, the latter function can return 'origin' and 'mine' for
these two cases, while remote_get() will return 'origin' for both of
these cases. At the programming level, you would still ask what the
URL to be pushed to to the remote obtained here, and would use
pushurl if defined, or url otherwise.
Ram, am I following your thoughts correctly?
--
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: [regression?] trailing slash required in .gitattributes
On Tue, Mar 19, 2013 at 02:10:42PM -0400, Jeff King wrote:
The issue bisects to 94bc671 (Add directory pattern matching to
attributes, 2012-12-08). That commit actually tests not only that
subdir/ matches, but also that just subdir does not match.
[...]
So I think the regression is accidental. And we would want tests like
this on top (which currently fail):
[...]
I'm having trouble figuring out the right solution for this.
The problem is in path_matches, which used to receive just the unadorned
pathname, and now receives path/ for directories. It now looks like
this:
static int path_matches(const char *pathname, int pathlen,
const char *basename,
const struct pattern *pat,
const char *base, int baselen)
{
const char *pattern = pat-pattern;
int prefix = pat-nowildcardlen;
if ((pat-flags EXC_FLAG_MUSTBEDIR)
((!pathlen) || (pathname[pathlen-1] != '/')))
return 0;
This first stanza checks that a pattern like foo/ must be matched by a
real directory. Which is fine; that's the point of adding the / to the
pattern.
if (pat-flags EXC_FLAG_NODIR) {
return match_basename(basename,
pathlen - (basename - pathname),
pattern, prefix,
pat-patternlen, pat-flags);
}
return match_pathname(pathname, pathlen,
base, baselen,
pattern, prefix, pat-patternlen, pat-flags);
}
But then here we'll end up feeding foo/ to be compared with foo,
which we don't want. For a pattern foo, we want to match _either_
foo/ or foo. So you'd think something like:
if (pathlen pathname[pathlen-1] == '/')
pathlen--;
would work. But it seems that match_basename, despite taking the length
of all of the strings we pass it, will happily use NUL-terminated
functions like strcmp or fnmatch. Converting the former to check lengths
should be pretty straightforward. But there is no version of fnmatch
that does what we want. I wonder if we using wildmatch can get around
this limitation.
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] t7800: fix tests when difftool uses --no-symlinks
Am 22.03.2013 20:36, schrieb John Keeping: When 'git difftool --dir-diff' is using --no-symlinks (either explicitly or implicitly because it's running on Windows), any working tree files that have been copied to the temporary directory are copied back after the difftool completes. This includes untracked files in the working tree. During the tests, this means that the following sequence occurs: 1) the shell opens output to redirect the difftool output 2) difftool copies the empty output to the temporary directory But this should not happen, should it? 3) difftool runs ls which writes to output 4) difftool copies the empty output file back over the output of the command 5) the output files doesn't contain the expected output, causing the test to fail Avoid this by writing the output into .git/ which will not be copied or overwritten. Isn't this just painting over the bug that output is incorrectly copied? In the longer term, difftool probably needs to learn to warn the user instead of overwrite any changes that have been made to the working tree file. Sure, but this is an independent issue. diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index e694972..1eed439 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -319,29 +319,29 @@ test_expect_success PERL 'setup change in subdirectory' ' ' test_expect_success PERL 'difftool -d' ' - git difftool -d --extcmd ls branch output - grep sub output - grep file output + git difftool -d --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' ... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] t7800: don't hide grep output
Am 22.03.2013 20:36, schrieb John Keeping: Remove the stdin_contains and stdin_doesnt_contain helper functions which add nothing but hide the output of grep, hurting debugging. Thanks. Patch looks good. -- 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] Avoid false positives in label detection in cpp diff hunk header regex.
Johannes Sixt j...@kdbg.org writes: Am 22.03.2013 16:02, schrieb Junio C Hamano: Vadim Zeitlin vz-...@zeitlins.org writes: A C++ method start such as void foo::bar() wasn't recognized by cpp diff driver as it mistakenly included foo::bar as a label. However the colon in a label can't be followed by another colon, so recognize this case specially to correctly detect C++ methods using this style. Much appreciated! PATTERNS(cpp, /* Jump targets or access declarations */ -!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n +!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:].*$|$)\n Hmm. Wouldn't find a word (possibly after indentation), colon and then either a non-colon or end of line be sufficient and simpler? iow, something like... !^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:]|$) Yes, indeed. We don't need to match more than necessary in a negative pattern. The \n must still remain, though. ... because \n is not for matching against the text, but merely to separate the regular expressions, right? I also wonder if label : should also be caught, or is it too weird format to be worth supporting? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git merge heuristic
On Fri, Mar 22, 2013 at 11:53:04AM -0700, Senthil Natarajan wrote: I want to learn about how Git compares patches while doing a merge. For example, if a patch has been cherry-picked from branch A to branch B, and then downstream we do a git merge from A to B, how does Git know to skip the cherry-picked patch? It doesn't. Git's 3-way merge only looks at three things: where each side of the merge ended, and what their common ancestor looked like. So when you cherry-pick a commit, as long as the content in the file ended up the same, there is no conflict. And it doesn't matter if it happened by cherry-picking, or if you just happened to make a sequence of commits that ended in the same state. However, we do perform such detection during a rebase, in which we try to skip patches that have already been applied upstream. It would have a different SHA-1, so what is the comparison algorithm/heuristic? What happens if the comment is different, but the actual patch is identical? Yes, the commit will have a different sha1. For that, we use the patch-id, which is basically a sha of the contents of the diff of the commit against its parent. See the manual for git-patch-id, git-cherry, and the --cherry-* options of git log. It will not find all duplicate commits (e.g., it will miss ones where there was a conflict during cherry-pick, or even where the context is slightly different). However, in many cases, rebase can also realize while applying a patch that it has already been applied, and skip it then. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] t7800: fix tests when difftool uses --no-symlinks
John Keeping j...@keeping.me.uk writes: When 'git difftool --dir-diff' is using --no-symlinks (either explicitly or implicitly because it's running on Windows), any working tree files that have been copied to the temporary directory are copied back after the difftool completes. This includes untracked files in the working tree. Hmph. Why do we populate the temporary directory with a copy of an untracked path in the first place? I thought the point of dir-diff was to materialize only the relevant paths to two temporaries and compare these temporaries with a tool that knows how to compare two directories? Even if you had path F in HEAD that you are no longer tracking in the working tree, a normal $ git diff HEAD would report the path F to have been deleted, so I would imagine that the preimage side of the temporary directory should get a copy of HEAD:F at path F, while the postimage side of the temporary directory should not even have anything at path F, when dir-diff runs, no? Isn't that the real reason why the test fails? The path 'output' is not being tracked at any revision or in the index that is involved in the test, is it? During the tests, this means that the following sequence occurs: 1) the shell opens output to redirect the difftool output 2) difftool copies the empty output to the temporary directory 3) difftool runs ls which writes to output 4) difftool copies the empty output file back over the output of the command 5) the output files doesn't contain the expected output, causing the test to fail Avoid this by writing the output into .git/ which will not be copied or overwritten. It is a good idea to move these test output and expect test vectore files to a different place to make it easier to distinguish them from test input (e.g. sub, file, etc.) in general, but the description of the original problem sounds like it is just working around a bug to me. What am I missing? In the longer term, difftool probably needs to learn to warn the user instead of overwrite any changes that have been made to the working tree file. Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7800-difftool.sh | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index e694972..1eed439 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -319,29 +319,29 @@ test_expect_success PERL 'setup change in subdirectory' ' ' test_expect_success PERL 'difftool -d' ' - git difftool -d --extcmd ls branch output - grep sub output - grep file output + git difftool -d --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff' ' - git difftool --dir-diff --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff ignores --prompt' ' - git difftool --dir-diff --prompt --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --prompt --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff from subdirectory' ' ( cd sub - git difftool --dir-diff --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --extcmd ls branch ../.git/output + grep sub ../.git/output + grep file ../.git/output ) ' -- 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] merge/pull: verify GPG signatures of commits being merged
Sebastian Götte ja...@physik.tu-berlin.de writes:
git merge/pull:
When --verify-signatures is specified on the command-line of git-merge
or git-pull, check whether the commits being merged have good gpg
signatures and abort the merge in case they do not. This allows e.g.
auto-deployment from untrusted repo hosts.
pretty printing:
Tell about an untrusted good signature in addition to the previous
good signature and bad signature. In case of a missing signature,
expand the pretty format string %G? to N in since this eases the
wirting of anything parsing git-log output.
Signed-off-by: Sebastian Götte ja...@physik-pool.tu-berlin.de
---
I moved the commit signature verification code from pretty.c to commit.c
because it is used from pretty.c and builtin/merge.c. I include that pretty
printing change here because I needed to add the good/untrusted check for the
merge --verify-signatures option to really make sense.
Those new %G? options are implicitly tested by
t7612-merge-verify-signatures.sh
because %G? is just replaced by the passed-through output of the commit
verification function.
While I think the new --verify-signature option may be a good
addition, I wonder if you can split this patch down a bit for easier
review and validation.
Perhaps this needs to be done in at least three steps:
(1) first move the code without changing anything (most
importantly, do not add 'U' or 'N' at this step); then
(2) teach 'merge' and 'pull' to understand the new option, and
finally;
(3) introduce 'U' and 'N'.
The existing users of '%G?' placeholders are not expecting to see
'N' but turning it into an empty string, so if the third step turns
out to be problematic to these users, we can discard the third step
and still benefit from the first two, for example.
diff --git a/Documentation/pretty-formats.txt
b/Documentation/pretty-formats.txt
index 105f18a..7297b1b 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -131,7 +131,7 @@ The placeholders are:
- '%B': raw body (unwrapped subject and body)
- '%N': commit notes
- '%GG': raw verification message from GPG for a signed commit
-- '%G?': show either G for Good or B for Bad for a signed commit
+- '%G?': show G for a Good signature, B for a Bad signature, U for a
good, untrusted signature and N for no signature
Even though this is a source that is turned into html and manpages,
people do read these in the original text format (that is the whole
point of using AsciiDoc as the source format), so please see if you
can avoid this overly long line.
diff --git a/builtin/merge.c b/builtin/merge.c
index 7c8922c..37ece3d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -49,7 +49,7 @@ static const char * const builtin_merge_usage[] = {
static int show_diffstat = 1, shortlog_len = -1, squash;
static int option_commit = 1, allow_fast_forward = 1;
static int fast_forward_only, option_edit = -1;
-static int allow_trivial = 1, have_message;
+static int allow_trivial = 1, have_message, verify_signatures = 0;
Avoid initializing static variables to 0, and instead let BSS take
care of them.
@@ -199,6 +199,8 @@ static struct option builtin_merge_options[] = {
OPT_BOOLEAN(0, ff-only, fast_forward_only,
N_(abort if fast-forward is not possible)),
OPT_RERERE_AUTOUPDATE(allow_rerere_auto),
+ OPT_BOOLEAN(0, verify-signatures, verify_signatures,
+ N_(Verify that the named commit has a valid GPG signature)),
OPT_CALLBACK('s', strategy, use_strategies, N_(strategy),
N_(merge strategy to use), option_parse_strategy),
OPT_CALLBACK('X', strategy-option, xopts, N_(option=value),
@@ -1233,6 +1235,35 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
usage_with_options(builtin_merge_usage,
builtin_merge_options);
+ if (verify_signatures) {
+ //Verify the commit signatures
No // C99/C++ comments.
The rest of the patch not reviewed; expecting a split reroll.
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 2/3] t7800: fix tests when difftool uses --no-symlinks
On Fri, Mar 22, 2013 at 03:53:38PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: When 'git difftool --dir-diff' is using --no-symlinks (either explicitly or implicitly because it's running on Windows), any working tree files that have been copied to the temporary directory are copied back after the difftool completes. This includes untracked files in the working tree. Hmph. Why do we populate the temporary directory with a copy of an untracked path in the first place? I thought the point of dir-diff was to materialize only the relevant paths to two temporaries and compare these temporaries with a tool that knows how to compare two directories? Even if you had path F in HEAD that you are no longer tracking in the working tree, a normal $ git diff HEAD would report the path F to have been deleted, so I would imagine that the preimage side of the temporary directory should get a copy of HEAD:F at path F, while the postimage side of the temporary directory should not even have anything at path F, when dir-diff runs, no? Isn't that the real reason why the test fails? The path 'output' is not being tracked at any revision or in the index that is involved in the test, is it? Actually it is, which is what I missed earlier. A couple of tests before this 'setup change in subdirectory' does 'git add .' which is far more general than it needs. Perhaps this is a better change: diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index bba8a9d..561c993 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -314,7 +314,7 @@ test_expect_success PERL 'setup change in subdirectory' ' git commit -m added sub/sub echo test file echo test sub/sub - git add . + git add file sub/sub git commit -m modified both ' During the tests, this means that the following sequence occurs: 1) the shell opens output to redirect the difftool output 2) difftool copies the empty output to the temporary directory 3) difftool runs ls which writes to output 4) difftool copies the empty output file back over the output of the command 5) the output files doesn't contain the expected output, causing the test to fail Avoid this by writing the output into .git/ which will not be copied or overwritten. It is a good idea to move these test output and expect test vectore files to a different place to make it easier to distinguish them from test input (e.g. sub, file, etc.) in general, but the description of the original problem sounds like it is just working around a bug to me. What am I missing? I think there is a bug, as described in the paragraph below, and this test should be made independent of that. In light of the above I think we can drop this patch and do this with that change instead. In the longer term, difftool probably needs to learn to warn the user instead of overwrite any changes that have been made to the working tree file. Signed-off-by: John Keeping j...@keeping.me.uk --- t/t7800-difftool.sh | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index e694972..1eed439 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -319,29 +319,29 @@ test_expect_success PERL 'setup change in subdirectory' ' ' test_expect_success PERL 'difftool -d' ' - git difftool -d --extcmd ls branch output - grep sub output - grep file output + git difftool -d --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff' ' - git difftool --dir-diff --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff ignores --prompt' ' - git difftool --dir-diff --prompt --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --prompt --extcmd ls branch .git/output + grep sub .git/output + grep file .git/output ' test_expect_success PERL 'difftool --dir-diff from subdirectory' ' ( cd sub - git difftool --dir-diff --extcmd ls branch output - grep sub output - grep file output + git difftool --dir-diff --extcmd ls branch ../.git/output + grep sub ../.git/output + grep file ../.git/output ) ' -- 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 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More
Re: [regression?] trailing slash required in .gitattributes
Jeff King p...@peff.net writes: if (pathlen pathname[pathlen-1] == '/') pathlen--; would work. But it seems that match_basename, despite taking the length of all of the strings we pass it, will happily use NUL-terminated functions like strcmp or fnmatch. Converting the former to check lengths should be pretty straightforward. But there is no version of fnmatch that does what we want. I wonder if we using wildmatch can get around this limitation. Or save away pathname[pathlen], temporarily NUL terminate and call these functions? -- 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] Avoid false positives in label detection in cpp diff hunk header regex.
Am 22.03.2013 23:32, schrieb Junio C Hamano: Johannes Sixt j...@kdbg.org writes: Am 22.03.2013 16:02, schrieb Junio C Hamano: Vadim Zeitlin vz-...@zeitlins.org writes: A C++ method start such as void foo::bar() wasn't recognized by cpp diff driver as it mistakenly included foo::bar as a label. However the colon in a label can't be followed by another colon, so recognize this case specially to correctly detect C++ methods using this style. Much appreciated! PATTERNS(cpp, /* Jump targets or access declarations */ -!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n +!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:].*$|$)\n Hmm. Wouldn't find a word (possibly after indentation), colon and then either a non-colon or end of line be sufficient and simpler? iow, something like... !^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:]|$) Yes, indeed. We don't need to match more than necessary in a negative pattern. The \n must still remain, though. ... because \n is not for matching against the text, but merely to separate the regular expressions, right? Correct. I also wonder if label : should also be caught, or is it too weird format to be worth supporting? It's easy to support, by inserting another [ \t] before the first colon. So, why not? -- 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] merge/pull: verify GPG signatures of commits being merged
Hi, Sebastian Götte wrote: git merge/pull: When --verify-signatures is specified on the command-line of git-merge or git-pull, check whether the commits being merged have good gpg signatures and abort the merge in case they do not. This allows e.g. auto-deployment from untrusted repo hosts. This leaves me pretty nervous. Is there an argument to pass in to specify a keyring with public keys to trust? Without that, it is presumably using ~/.gnupg/trustdb.gpg, which is about trust of identity rather than trust to provide code to run on my machine. :( If there's a good way to avoid that, this looks like a good thing to do, though. Hope that helps, Jonathan -- 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 4/6] remote.c: introduce a way to have different remotes for fetch/push
Junio C Hamano wrote: Jonathan Nieder jrnie...@gmail.com writes: - struct remote *remote = remote_get(repo); + struct remote *remote = pushremote_get(repo); struct remote has url and pushurl fields. What do they mean in the context of these two accessors? /me is confused. Is the idea that now I should not use pushurl any more, and that I should use pushremote_get and use url instead? [...] At the programming level, you would still ask what the URL to be pushed to to the remote obtained here, and would use pushurl if defined, or url otherwise. Ah, I think I see. It might be more convenient to the caller if pushremote_get returned a remote with url set to the pushurl, but that would prevent sharing the struct with other callers that want that remote for fetching. So instead, the idea is something like remote: support a different default remote for pushing Teach remote_get() to accept an argument FOR_FETCH or FOR_PUSH that determines, when no remote is passed to it, whether to use the default remote for fetching or the default for pushing. The default remote for fetching is stored in the static var default_remote_name, while the default for pushing, if set, is in default_push_remote_name. Currently there is never a different default for pushing set but later patches will change that. If remote_get() gained a new required parameter, that would force all call sites to be examined (even any potential call sites added by new patches in flight) and there would no longer be need for the remote_get_1() function. What do you think? Jonathan -- 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: feature request - have git honor nested .gitconfig files
Jeff King wrote: On Fri, Mar 22, 2013 at 11:22:11AM -0700, Jonathan Nieder wrote: It would be easier to understand the configuration if ~/.gitconfig could spell out the rule explicitly: [...] It sounds hard to do right, especially considering use cases like User runs into trouble, asks a privileged sysadmin to try running a command in her untrusted repository, but it is worth thinking about how to do. I'd rather not invent a new language. It will either not be featureful enough, or will end up bloated. Or both. How about something like: [include] exec = case \$GIT_DIR\ in) */dev/*) cat ~/.config/git/dev-config ;; *) cat ~/.config/git/nondev-config ;; esac Yes, an existing language like shell or lua would presumably be the way to go. The scary aspect of shell is the Prankster sets up a malicious configuration, asks a sysadmin to help in her repository scenario. Of course we already have that problem with command aliases, but if the sysadmin has perfect spelling then aliases would never trip, so... Hopefully that's enough information for anyone interested to start and understand the relevant variables at play. Thanks, Jonathan -- 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] Avoid false positives in label detection in cpp diff hunk header regex.
Johannes Sixt j6t at kdbg.org writes: I also wonder if label : should also be caught, or is it too weird format to be worth supporting? It's easy to support, by inserting another [ \t] before the first colon. So, why not? This is really nitpicking, but if we do it, then it should be [ \t]*. And the * after the label should actually be a +. So the full line becomes !^[ \t]*[A-Za-z_][A-Za-z_0-9]+[ \t]*:([^:]|$)\n But then I've never actually seen git putting labels incorrectly into the hunk headers while I did see the problem this patch tries to fix, with wrong method appearing in the header because the correct one was skipped due to this ignore regex, quite a few times in the past. Regards, VZ -- 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/4] Verify GPG signatures when merging and extend %G? pretty string
As suggested by Junio I split the preivous patch into four parts. Sebastian Götte (4): Move commit GPG signature verification to commit.c merge/pull: verify GPG signatures of commits being merged merge/pull Check for untrusted good GPG signatures pretty printing: extend %G? to include 'N' and 'U' Documentation/merge-options.txt| 4 +++ Documentation/pretty-formats.txt | 3 +- builtin/merge.c| 33 +- commit.c | 55 + commit.h | 9 + git-pull.sh| 10 -- gpg-interface.h| 6 pretty.c | 69 - t/lib-gpg/pubring.gpg | Bin 1164 - 2359 bytes t/lib-gpg/random_seed | Bin 600 - 600 bytes t/lib-gpg/secring.gpg | Bin 1237 - 3734 bytes t/lib-gpg/trustdb.gpg | Bin 1280 - 1360 bytes t/t7612-merge-verify-signatures.sh | 61 13 files changed, 184 insertions(+), 66 deletions(-) create mode 100755 t/t7612-merge-verify-signatures.sh -- 1.8.1.5 -- 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 2/4] merge/pull: verify GPG signatures of commits being merged
When --verify-signatures is specified on the command-line of git-merge
or git-pull, check whether the commits being merged have good gpg
signatures and abort the merge in case they do not. This allows e.g.
auto-deployment from untrusted repo hosts.
Signed-off-by: Sebastian Götte ja...@physik-pool.tu-berlin.de
---
Documentation/merge-options.txt| 4 +++
builtin/merge.c| 31 ++-
git-pull.sh| 10 ++--
t/t7612-merge-verify-signatures.sh | 52 ++
4 files changed, 94 insertions(+), 3 deletions(-)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 0bcbe0a..2f76ab5 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -83,6 +83,10 @@ option can be used to override --squash.
Pass merge strategy specific option through to the merge
strategy.
+--verify-signatures::
+--no-verify-signatures::
+ Verify that the commits being merged have good trusted GPG signatures
+
--summary::
--no-summary::
Synonyms to --stat and --no-stat; these are deprecated and will be
diff --git a/builtin/merge.c b/builtin/merge.c
index 7c8922c..b3788aa 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -49,7 +49,7 @@ static const char * const builtin_merge_usage[] = {
static int show_diffstat = 1, shortlog_len = -1, squash;
static int option_commit = 1, allow_fast_forward = 1;
static int fast_forward_only, option_edit = -1;
-static int allow_trivial = 1, have_message;
+static int allow_trivial = 1, have_message, verify_signatures;
static int overwrite_ignore = 1;
static struct strbuf merge_msg = STRBUF_INIT;
static struct strategy **use_strategies;
@@ -199,6 +199,8 @@ static struct option builtin_merge_options[] = {
OPT_BOOLEAN(0, ff-only, fast_forward_only,
N_(abort if fast-forward is not possible)),
OPT_RERERE_AUTOUPDATE(allow_rerere_auto),
+ OPT_BOOLEAN(0, verify-signatures, verify_signatures,
+ N_(Verify that the named commit has a valid GPG signature)),
OPT_CALLBACK('s', strategy, use_strategies, N_(strategy),
N_(merge strategy to use), option_parse_strategy),
OPT_CALLBACK('X', strategy-option, xopts, N_(option=value),
@@ -1233,6 +1235,33 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
usage_with_options(builtin_merge_usage,
builtin_merge_options);
+ if (verify_signatures) {
+ /* Verify the commit signatures */
+ for (p = remoteheads; p; p = p-next) {
+ struct commit *commit = p-item;
+ struct signature signature;
+ signature.check_result = 0;
+
+ check_commit_signature(commit, signature);
+
+ char hex[41];
+ strcpy(hex, find_unique_abbrev(commit-object.sha1,
DEFAULT_ABBREV));
+ switch(signature.check_result){
+ case 'G':
+ if (verbosity = 0)
+ printf(_(Commit %s has a good
GPG signature by %s\n), hex, signature.signer);
+ break;
+ case 'B':
+ die(_(Commmit %s has a bad GPG
signature allegedly by %s.), hex, signature.signer);
+ default: /* 'N' */
+ die(_(Commmit %s does not have a good
GPG signature. In fact, commit %s does not have a GPG signature at all.), hex,
hex);
+ }
+
+ free(signature.gpg_output);
+ free(signature.signer);
+ }
+ }
+
strbuf_addstr(buf, merge);
for (p = remoteheads; p; p = p-next)
strbuf_addf(buf, %s, merge_remote_util(p-item)-name);
diff --git a/git-pull.sh b/git-pull.sh
index 266e682..705940d 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -39,7 +39,7 @@ test -z $(git ls-files -u) || die_conflict
test -f $GIT_DIR/MERGE_HEAD die_merge
strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
-log_arg= verbosity= progress= recurse_submodules=
+log_arg= verbosity= progress= recurse_submodules= verify_signatures=
merge_args= edit=
curr_branch=$(git symbolic-ref -q HEAD)
curr_branch_short=${curr_branch#refs/heads/}
@@ -125,6 +125,12 @@ do
--no-recurse-submodules)
recurse_submodules=--no-recurse-submodules
;;
+ --verify-signatures)
+ verify_signatures=--verify-signatures
+ ;;
+ --no-verify-signatures)
+ verify_signatures=--no-verify-signatures
+ ;;
--d|--dr|--dry|--dry-|--dry-r|--dry-ru|--dry-run)
dry_run=--dry-run
;;
@@
Re: git branch: multiple --merged and --no-merged options?
Jeff King p...@peff.net writes: On Fri, Mar 15, 2013 at 02:38:12PM -0500, Jed Brown wrote: $ git branch --no-merged master --merged next Yeah, sadly that does not work, as we use the same slot for the flag and store only one of the two (and we also allow only one --merged head, even though you could in theory want to know merged to X, or merged to Y). Hmm, I would have said conjunction (AND) was more natural than disjunction (OR). If we add support for multiple '--merged' and '--no-merged', do we expect to eventually have a full query grammar? -- 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] merge/pull: verify GPG signatures of commits being merged
Jonathan Nieder jrnie...@gmail.com writes: git merge/pull: When --verify-signatures is specified on the command-line of git-merge or git-pull, check whether the commits being merged have good gpg signatures and abort the merge in case they do not. This allows e.g. auto-deployment from untrusted repo hosts. This leaves me pretty nervous. Is there an argument to pass in to specify a keyring with public keys to trust? Without that, it is presumably using ~/.gnupg/trustdb.gpg, which is about trust of identity rather than trust to provide code to run on my machine. :( I think people who create a real merge via git pull and use that as auto-deployment mechanism is insane, but presumably that auto tells us some other things, like it will be done by non-human account, its $HOME/.gnupg would contain only the keyring that is for the auto deployer, or the cronscript that runs git pull can set GNUPGHOME and export it before doing so. So, I wouldn't be worried about it too much. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/45] parse_pathspec and :(glob) magic
On Thu, Mar 21, 2013 at 10:50:02AM -0700, Junio C Hamano wrote:
Why could the test pass for you without it? It doesn't look like a
bug that depended on uninitialized memory or something from the
above observation.
It depends on uninitialized memory. For absolute paths, prefix is
useless and I should have set the useful prefix length to zero, but I
did not. Later in prefix_pathspec, I rely on this value to set
nowildcard_len without checking if it's sane. The actual pathspec
after prefix_pathspec is src (length of 3) but nowildcard_len is 5.
In common_prefix_len(), I use nowildcard_len without sanity checks. So
the code examines 's', 'r', 'c', '\0', 'random'. In my case,
'random' has never been '/'. I guess yours is '/' (which leads to
wrong common prefix length).
I've added an assert() to make sure nowildcard_len and prefix have
sane values before exiting prefix_pathspec. This assert() chokes at
t7300.8 for me.
The change made to prefix_path_gently() in this series is beyond
disgusting, especially with the above fix-up.
Sometimes it uses the original len, sometimes it uses the fixed-up
*p_len (e.g. passes it down to normalize_path_copy_len()), and lets
normalize_path_copy_len() further update it, and thenit makes the
caller use the updated *p_len.
Does the caller know what the value in *p_len _mean_ after this
function returns? Can it afford to lose the original length of the
prefix it saved in a variable, without getting confused?
I think any change that turns a value-passed argument in the
existing code into modifiable pointer-to-variable in this series
should add in-code comment to describe what the variable mean upon
entry and after return, just like normalize_path_copy_len() that was
built out of the original normalize_path_copy(). I didn't look if
there are many others, or if this is the only one that is tricky. it
is tricky that even the original author of the patch got it wrong
X-.
The author of the patch totally forgot that prefix has nothing to do
with prefix. How about this? The prefix length is passed as value as
before. A separate pointer is for passing back the actual prefix
length. You can pull the actual patch from
https://github.com/pclouds/git parse-pathspec
which also includes all document bugs reported so far.
-- 8 --
diff --git a/pathspec.c b/pathspec.c
index 0771e48..126771c 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -205,7 +205,7 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
match = xstrdup(copyfrom);
prefixlen = 0;
} else {
- match = prefix_path_gently(prefix, prefixlen, copyfrom);
+ match = prefix_path_gently(prefix, prefixlen, prefixlen,
copyfrom);
if (!match)
die(%s: '%s' is outside repository, elt, copyfrom);
}
@@ -284,6 +284,10 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
no_wildcard(item-match + item-nowildcard_len + 1))
item-flags |= PATHSPEC_ONESTAR;
}
+
+ /* sanity checks, pathspec matchers assume these are sane */
+ assert(item-nowildcard_len = item-len
+ item-prefix = item-len);
return magic;
}
@@ -315,7 +319,7 @@ static void NORETURN unsupported_magic(const char *pattern,
n++;
}
/*
-* We may want to substitue this command with a command
+* We may want to substitute this command with a command
* name. E.g. when add--interactive dies when running
* checkout -p
*/
diff --git a/setup.c b/setup.c
index e59146b..6cf2bc6 100644
--- a/setup.c
+++ b/setup.c
@@ -5,24 +5,37 @@
static int inside_git_dir = -1;
static int inside_work_tree = -1;
-char *prefix_path_gently(const char *prefix, int *p_len, const char *path)
+/*
+ * Normalize path, prepending the prefix for relative paths. If
+ * remaining_prefix is not NULL, return the actual prefix still
+ * remains in the path. For example, prefix = sub1/sub2/ and path is
+ *
+ * foo - sub1/sub2/foo (full prefix)
+ * ../foo - sub1/foo (remaining prefix is sub1/)
+ * ../../bar- bar(no remaining prefix)
+ * ../../sub1/sub2/foo - sub1/sub2/foo (but no remaining prefix)
+ * `pwd`/../bar - sub1/bar (no remaining prefix)
+ */
+char *prefix_path_gently(const char *prefix, int len,
+int *remaining_prefix, const char *path)
{
const char *orig = path;
char *sanitized;
- int len = *p_len;
if (is_absolute_path(orig)) {
const char *temp = real_path(path);
sanitized = xmalloc(len + strlen(temp) + 1);
strcpy(sanitized, temp);
- if (p_len)
- *p_len = 0;
+ if (remaining_prefix)
+ *remaining_prefix = 0;
} else {
sanitized = xmalloc(len +
Re: [PATCH v2 00/45] parse_pathspec and :(glob) magic
On Sat, Mar 23, 2013 at 10:13 AM, Duy Nguyen pclo...@gmail.com wrote: which also includes all document bugs reported so far. s/all/fixes for all/ -- Duy -- 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] t7800: fix tests when difftool uses --no-symlinks
On Fri, Mar 22, 2013 at 4:05 PM, John Keeping j...@keeping.me.uk wrote: On Fri, Mar 22, 2013 at 03:53:38PM -0700, Junio C Hamano wrote: John Keeping j...@keeping.me.uk writes: When 'git difftool --dir-diff' is using --no-symlinks (either explicitly or implicitly because it's running on Windows), any working tree files that have been copied to the temporary directory are copied back after the difftool completes. This includes untracked files in the working tree. Hmph. Why do we populate the temporary directory with a copy of an untracked path in the first place? I thought the point of dir-diff was to materialize only the relevant paths to two temporaries and compare these temporaries with a tool that knows how to compare two directories? Even if you had path F in HEAD that you are no longer tracking in the working tree, a normal $ git diff HEAD would report the path F to have been deleted, so I would imagine that the preimage side of the temporary directory should get a copy of HEAD:F at path F, while the postimage side of the temporary directory should not even have anything at path F, when dir-diff runs, no? Isn't that the real reason why the test fails? The path 'output' is not being tracked at any revision or in the index that is involved in the test, is it? Actually it is, which is what I missed earlier. A couple of tests before this 'setup change in subdirectory' does 'git add .' which is far more general than it needs. Perhaps this is a better change: diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index bba8a9d..561c993 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -314,7 +314,7 @@ test_expect_success PERL 'setup change in subdirectory' ' git commit -m added sub/sub echo test file echo test sub/sub - git add . + git add file sub/sub git commit -m modified both ' During the tests, this means that the following sequence occurs: 1) the shell opens output to redirect the difftool output 2) difftool copies the empty output to the temporary directory 3) difftool runs ls which writes to output 4) difftool copies the empty output file back over the output of the command 5) the output files doesn't contain the expected output, causing the test to fail Avoid this by writing the output into .git/ which will not be copied or overwritten. It is a good idea to move these test output and expect test vectore files to a different place to make it easier to distinguish them from test input (e.g. sub, file, etc.) in general, but the description of the original problem sounds like it is just working around a bug to me. What am I missing? I think there is a bug, as described in the paragraph below, and this test should be made independent of that. In light of the above I think we can drop this patch and do this with that change instead. I, too, was scratching my head when the Windows issue was first reported. Thanks for clearing this up; fixing the blind add . certainly seems like the way to go. -- David -- 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] status, branch: fix the misleading bisecting message
The current message is bisecting %s (or bisecting branch %s).
%s is the current branch when we started bisecting. Clarify that to
avoid confusion with good and bad refs passed to bisect command.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
builtin/branch.c| 2 +-
t/t6030-bisect-porcelain.sh | 2 +-
t/t7512-status-help.sh | 2 +-
wt-status.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 99105f8..8f00203 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -562,7 +562,7 @@ static char *get_head_description(void)
strbuf_addf(desc, _((no branch, rebasing %s)),
state.branch);
else if (state.bisect_in_progress)
- strbuf_addf(desc, _((no branch, bisecting %s)),
+ strbuf_addf(desc, _((no branch, bisect started on %s)),
state.branch);
else if (state.detached_from)
strbuf_addf(desc, _((detached from %s)),
diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh
index 9b6f0d0..2fce99a 100755
--- a/t/t6030-bisect-porcelain.sh
+++ b/t/t6030-bisect-porcelain.sh
@@ -164,7 +164,7 @@ test_expect_success 'bisect start: existing
.git/BISECT_START not modified if
cp .git/BISECT_START saved
test_must_fail git bisect start $HASH4 foo --
git branch branch.output
- test_i18ngrep * (no branch, bisecting other) branch.output
/dev/null
+ test_i18ngrep * (no branch, bisect started on other) branch.output
/dev/null
test_cmp saved .git/BISECT_START
'
test_expect_success 'bisect start: no .git/BISECT_START if mistaken rev' '
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 5adba4f..c35d01d 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -578,7 +578,7 @@ test_expect_success 'status when bisecting' '
TGT=$(git rev-parse --short two_bisect)
cat expected -EOF
# HEAD detached at $TGT
- # You are currently bisecting branch '\''bisect'\''.
+ # You are currently bisecting, started from branch '\''bisect'\''.
# (use git bisect reset to get back to the original branch)
#
nothing to commit (use -u to show untracked files)
diff --git a/wt-status.c b/wt-status.c
index 32a51e1..cf3d81a 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -953,7 +953,7 @@ static void show_bisect_in_progress(struct wt_status *s,
{
if (state-branch)
status_printf_ln(s, color,
-_(You are currently bisecting branch '%s'.),
+_(You are currently bisecting, started from
branch '%s'.),
state-branch);
else
status_printf_ln(s, color,
--
1.8.2.83.gc99314b
--
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: mailmap documentation add grave accents
Any response for such a small fix? 2013/3/10 乙酸鋰 ch3co...@gmail.com: Hi, Here is the patch. Regards, ch3cooli -- 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: [regression?] trailing slash required in .gitattributes
On Fri, Mar 22, 2013 at 06:24:39PM -0400, Jeff King wrote:
I'm having trouble figuring out the right solution for this.
Thanks for looking into this. It was on my todo list, but you beat me
to it :)
But then here we'll end up feeding foo/ to be compared with foo,
which we don't want. For a pattern foo, we want to match _either_
foo/ or foo. So you'd think something like:
if (pathlen pathname[pathlen-1] == '/')
pathlen--;
would work. But it seems that match_basename, despite taking the length
of all of the strings we pass it, will happily use NUL-terminated
functions like strcmp or fnmatch. Converting the former to check lengths
should be pretty straightforward. But there is no version of fnmatch
that does what we want. I wonder if we using wildmatch can get around
this limitation.
You can use nwildmatch() from this patch. I tested it lightly with
t3070-wildmatch.sh, feeding the strings with no terminating NUL. It
seems to work ok.
-- 8 --
Subject: [PATCH] wildmatch: do not require text to be NUL-terminated
This may be helpful when we just want to match a part of text.
nwildmatch can be used for this purpose.
Signed-off-by: Nguyễn Thái Ngọc Duy pclo...@gmail.com
---
wildmatch.c | 25 +
wildmatch.h | 11 +--
2 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/wildmatch.c b/wildmatch.c
index 7192bdc..f97ae2a 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -52,7 +52,8 @@ typedef unsigned char uchar;
#define ISXDIGIT(c) (ISASCII(c) isxdigit(c))
/* Match pattern p against text */
-static int dowild(const uchar *p, const uchar *text, unsigned int flags)
+static int dowild(const uchar *p, const uchar *text,
+ const uchar *textend, unsigned int flags)
{
uchar p_ch;
const uchar *pattern = p;
@@ -60,8 +61,9 @@ static int dowild(const uchar *p, const uchar *text, unsigned
int flags)
for ( ; (p_ch = *p) != '\0'; text++, p++) {
int matched, match_slash, negated;
uchar t_ch, prev_ch;
- if ((t_ch = *text) == '\0' p_ch != '*')
+ if (text = textend p_ch != '*')
return WM_ABORT_ALL;
+ t_ch = *text;
if ((flags WM_CASEFOLD) ISUPPER(t_ch))
t_ch = tolower(t_ch);
if ((flags WM_CASEFOLD) ISUPPER(p_ch))
@@ -101,7 +103,7 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
* both foo/bar and foo/a/bar.
*/
if (p[0] == '/'
- dowild(p + 1, text, flags) ==
WM_MATCH)
+ dowild(p + 1, text, textend, flags)
== WM_MATCH)
return WM_MATCH;
match_slash = 1;
} else
@@ -130,9 +132,7 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
/* the slash is consumed by the top-level for
loop */
break;
}
- while (1) {
- if (t_ch == '\0')
- break;
+ while (text textend) {
/*
* Try to advance faster when an asterisk is
* followed by a literal. We know in this case
@@ -145,18 +145,18 @@ static int dowild(const uchar *p, const uchar *text,
unsigned int flags)
p_ch = *p;
if ((flags WM_CASEFOLD)
ISUPPER(p_ch))
p_ch = tolower(p_ch);
- while ((t_ch = *text) != '\0'
+ while (text textend
(match_slash || t_ch != '/')) {
if ((flags WM_CASEFOLD)
ISUPPER(t_ch))
t_ch = tolower(t_ch);
if (t_ch == p_ch)
break;
- text++;
+ t_ch = *++text;
}
if (t_ch != p_ch)
return WM_NOMATCH;
}
- if ((matched = dowild(p, text, flags)) !=
WM_NOMATCH) {
+ if ((matched = dowild(p, text, textend, flags))
!= WM_NOMATCH) {
if (!match_slash
Re: [regression?] trailing slash required in .gitattributes
On Sat, Mar 23, 2013 at 11:18:24AM +0700, Duy Nguyen wrote:
You can use nwildmatch() from this patch. I tested it lightly with
t3070-wildmatch.sh, feeding the strings with no terminating NUL. It
seems to work ok.
And valgrind spotted my faults, especially for using strchr. You would
need this on top:
-- 8 --
diff --git a/wildmatch.c b/wildmatch.c
index f97ae2a..939bac8 100644
--- a/wildmatch.c
+++ b/wildmatch.c
@@ -61,9 +61,13 @@ static int dowild(const uchar *p, const uchar *text,
for ( ; (p_ch = *p) != '\0'; text++, p++) {
int matched, match_slash, negated;
uchar t_ch, prev_ch;
- if (text = textend p_ch != '*')
- return WM_ABORT_ALL;
- t_ch = *text;
+ if (text = textend) {
+ if (p_ch != '*')
+ return WM_ABORT_ALL;
+ else
+ t_ch = '\0';
+ } else
+ t_ch = *text;
if ((flags WM_CASEFOLD) ISUPPER(t_ch))
t_ch = tolower(t_ch);
if ((flags WM_CASEFOLD) ISUPPER(p_ch))
@@ -115,8 +119,9 @@ static int dowild(const uchar *p, const uchar *text,
/* Trailing ** matches everything. Trailing
* matches
* only if there are no more slash characters.
*/
if (!match_slash) {
- if (strchr((char*)text, '/') != NULL)
- return WM_NOMATCH;
+ for (;text textend; text++)
+ if (*text == '/')
+ return WM_NOMATCH;
}
return WM_MATCH;
} else if (!match_slash *p == '/') {
@@ -125,10 +130,11 @@ static int dowild(const uchar *p, const uchar *text,
* with WM_PATHNAME matches the next
* directory
*/
- const char *slash = strchr((char*)text, '/');
- if (!slash)
+ for (;text textend; text++)
+ if (*text == '/')
+ break;
+ if (text == textend)
return WM_NOMATCH;
- text = (const uchar*)slash;
/* the slash is consumed by the top-level for
loop */
break;
}
@@ -151,7 +157,7 @@ static int dowild(const uchar *p, const uchar *text,
t_ch = tolower(t_ch);
if (t_ch == p_ch)
break;
- t_ch = *++text;
+ t_ch = ++text textend ? *text
: '\0';
}
if (t_ch != p_ch)
return WM_NOMATCH;
-- 8 --
--
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: [FEATURE-REQUEST] difftool --dir-diff: use the commit names as directory names instead of left/right
On Fri, Mar 22, 2013 at 9:52 AM, Christoph Anton Mitterer cales...@scientia.net wrote: Hi. Right now, when I use difftool --dir-diff, the temp dirs are creates as e.g.: /tmp/git-difftool.QqP8x/left /tmp/git-difftool.QqP8x/right Wouldn't it be nice, if instead of left/right... the specified commit name would be used? e.g. /tmp/git-difftool.QqP8x/r1.1.1 /tmp/git-difftool.QqP8x/HEAD or /tmp/git-difftool.QqP8x/fd4e4005a4b7b3e638bc405be020b867f8881e72 /tmp/git-difftool.QqP8x/ce0747b74fccd20707b6f495068503e69e5473df Cause then, one would see in the difftool which side is what, without the need to remember how git difftool was invoked. Of course one might probably need to escape the commit names if they contain stuff like / or .., etc. I can see that being pretty helpful. If we do it we should go all the way. What do you all think about something like the output of git describe --always instead of the SHA-1? BTW there are some patches in-flight around difftool so you'll probably want to apply them first if you're going to give it a try. patches very much appreciated! ;-) If no one beats me to it, I can give it a try after the weekend. -- David -- 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 v9 5/5] Speed up log -L... -M
Eric Sunshine sunsh...@sunshineco.com writes:
On Thu, Mar 21, 2013 at 8:52 AM, Thomas Rast tr...@student.ethz.ch wrote:
This is a bit hacky and should really be replaced by equivalent
support in --follow, and just using that. However, in the meantime it
s/using/use/
I'm not a native speaker, but I really think 'using' is more correct
here. But feel free to suggest a better wording. The intention is that
we should proceed in two steps: 'git log --follow' first needs to learn
to adjust its pathspec filter as it walks revisions, much like I did
here. Then this patch should be reverted in favor of just enabling
--follow.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
