Re: [PATCH v4] clone: simplify string handling in guess_dir_name()
On Tue, Aug 04, 2015 at 06:42:46PM -0400, Jeff King wrote: On Tue, Aug 04, 2015 at 09:31:18AM +0200, Sebastian Schuberth wrote: [snip] Sadly we cannot just `strip_suffix_mem(repo, len, /.git))` in the earlier code, as we have to account for multiple directory separators. I believe the above code does the right thing, though. I haven't looked at how badly it interacts with the other guess_dir_name work from Patrick Steinhardt that has been going on, though. -Peff It shouldn't be hard rebasing my work onto this. If it's being applied I'll come up with a new version. Patrick signature.asc Description: Digital signature
Re: [PATCH v2] send-email: provide whitelist of SMTP AUTH mechanisms
Hello Eric, all,
thanks for comments, the coding style will be fixed
in the next version (I cannot find a way how to set
vim to help me with those ifSPACE( issues. I always/often
forget it when writing so I never do it to be consistent.).
Do I understand well that you are complaining about too
narrow commmit message?
I am trying to figure out how to write a test. It is
not very clear to me, what the testing suite does. My
attempt looks this way at the moment:
1657 do_smtp_auth_test() {
1658 git send-email \
1659 --from=Example nob...@example.com \
1660 --to=some...@example.com \
1661 --smtp-server=$(pwd)/fake.sendmail \
1662 --smtp-auth=$1 \
1663 -v \
1664 0001-*.patch \
1665 2errors out
1666 }
1667
1668 test_expect_success $PREREQ 'accepts SMTP AUTH mechanisms (see RFC-4422,
p. 8)' '
1669 do_smtp_auth_test PLAIN LOGIN CRAM-MD5 DIGEST-MD5 GSSAPI EXTERNAL
ANONYMOUS
1670 do_smtp_auth_test ABCDEFGHIKLMNOPQRSTUVWXYZ 0123456789_-
1671 '
1672
1673 test_expect_success $PREREQ 'does not accept non-RFC-4422 strings for SMTP
AUTH' '
1674 test_must_fail do_smtp_auth_test ../ATTACK
1675 test_must_fail do_smtp_auth_test TOO-LONG-BUT-VALID-STRING
1676 test_must_fail do_smtp_auth_test no-lower-case-sorry
1677 '
* I do not know yet, what to check after each do_smtp_auth_test call.
* Perhaps, each case should have its own test_expect_success call?
* Why send-email -v does not generate any output?
(I found a directory 'trash directory.t9001-send-email', however, the
errors file is always empty.)
* Is there any other place where the files out, errors are placed?
* I have no idea what the fake.sendmail does (I could see its contents
but still...). Is it suitable for my tests?
* Should I check the behaviour '--smtp-auth overrides
sendemail.smtpAuth'?
Regards
Jan
On Sun, 2 Aug 2015 14:57:19 -0400
Eric Sunshine sunsh...@sunshineco.com wrote:
On Sun, Aug 2, 2015 at 12:42 PM, Jan Viktorin
vikto...@rehivetech.com wrote:
When sending an e-mail, the client and server must
agree on an authentication mechanism. Some servers
(due to misconfiguration or a bug) deny valid
credentials for certain mechanisms. In this patch,
a new option --smtp-auth and configuration entry
smtpauth are introduced. If smtp_auth is defined,
it works as a whitelist of allowed mechanisms for
authentication selected from the ones supported by
the installed SASL perl library.
Nit: This would read a bit more nicely if wrapped to 70-72 columns.
Signed-off-by: Jan Viktorin vikto...@rehivetech.com
---
diff --git a/Documentation/git-send-email.txt
b/Documentation/git-send-email.txt index 7ae467b..c237c80 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -171,6 +171,14 @@ Sending
+--smtp-auth=mechs::
+ Specify allowed SMTP-AUTH mechanisms. This setting forces
using only
+ the listed mechanisms. Separate allowed mechanisms by a
whitespace.
Perhaps:
Whitespace-separated list of allowed SMTP-AUTH mechanisms.
+ Example: PLAIN LOGIN GSSAPI. If at least one of the
specified mechanisms
+ matchs those advertised by the SMTP server and it is
supported by the SASL
s/matchs/matches/
+ library we use, it is used for authentication. If neither
of 'sendemail.smtpAuth'
+ or '--smtp-auth' is specified, all mechanisms supported on
client can be used.
s/neither of/neither/
s/or/nor/
diff --git a/git-send-email.perl b/git-send-email.perl
index ae9f869..ebc1e90 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -75,6 +75,9 @@ git send-email [options] file | directory |
rev-list options Pass an empty string to disable certificate
verification.
--smtp-domain str * The domain name sent to
HELO/EHLO handshake
+--smtp-auth str * Space separated list of
allowed AUTH methods.
s/Space separated/Space-separated/
+ This setting forces to use
one of the listed methods.
+ Supported: PLAIN LOGIN
CRAM-MD5 DIGEST-MD5.
Since you're no longer checking explicitly for these mechanisms, you
probably want to drop the Supported: line.
--smtp-debug0|1 * Disable, enable Net::SMTP
debug.
Automating:
@@ -1136,6 +1141,10 @@ sub smtp_auth_maybe {
Authen::SASL-import(qw(Perl));
};
+ if($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
+ die invalid smtp auth: '${smtp_auth}';
+ }
Style: space after 'if'
# TODO: Authentication may fail not because credentials were
# invalid but due to other reasons, in which we should not
# reject credentials.
@@ -1148,6 +1157,20
[PATCH 0/2] fix clone guess_dir_name regression in v2.4.8
On Tue, Aug 04, 2015 at 06:42:46PM -0400, Jeff King wrote:
I did not intend this change in behavior, and I can confirm that
reverting my patch restores the original behavior. Thanks for bringing
this to my attention, I'll work on a patch.
I think this regression is in v2.4.8, as well. We should be able to use
a running len instead of the end pointer in the earlier part, and
then use strip_suffix_mem later (to strip from our already-reduced
length, rather than the full NUL-terminated string). Like this:
Looks like git clone --bare host:foo/.git is broken, too. I've added
some tests to cover the recently broken cases, as well as some obvious
normal cases (which the patch I sent earlier break!). And as a bonus, we
can easily cover Patrick's root-repo problems (so people will actually
run the tests, unlike the stuff in t1509. :) ).
@@ -167,14 +166,14 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
* the form remote.example.com:foo.git, i.e. no slash
* in the directory part.
*/
- start = end;
+ start = repo + len;
while (repo start !is_dir_sep(start[-1]) start[-1] != ':')
start--;
/*
* Strip .{bundle,git}.
*/
- strip_suffix(start, is_bundle ? .bundle : .git , len);
+ strip_suffix_mem(start, len, is_bundle ? .bundle : .git);
This is crap, of course. Our len variable is computed from the start
of repo, of which start is a subset. So we are indexing way out of
bounds here.
As it turns out, this actually makes things simpler. We can stop using
len entirely in the early part, and leave it as-is with pointer math
(the patch I sent earlier did not really make anything simpler, anyway).
And then we can just compute the length of start here, minus
everything we've stripped off the end (i.e., len = end - start).
Here are the patches.
[1/2]: clone: add tests for output directory
[2/2]: clone: use computed length in guess_dir_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
[PATCH 1/2] clone: add tests for output directory
When we run git clone $url, clone guesses from the $url
what to name the local output directory. We don't have any
test coverage of this, so let's add some basic tests.
This reveals a few problems:
- cloning foo.git/ does not properly remove the .git;
this is a recent regression from 7e837c6 (clone:
simplify string handling in guess_dir_name(), 2015-07-09)
- likewise, cloning foo/.git does not seem to handle the
bare case (we should end up in foo.git, but we try to
use foo/.git on the local end), which also comes from
7e837c6.
- cloning the root is not very smart about URL parsing,
and usernames and port numbers may end up in the
directory name
All of these tests are marked as failures.
Signed-off-by: Jeff King p...@peff.net
---
t/t5603-clone-dirname.sh | 69
1 file changed, 69 insertions(+)
create mode 100755 t/t5603-clone-dirname.sh
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
new file mode 100755
index 000..a0140b9
--- /dev/null
+++ b/t/t5603-clone-dirname.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+test_description='check output directory names used by git-clone'
+. ./test-lib.sh
+
+# we use a fake ssh wrapper that ignores the arguments
+# entirely; we really only care that we get _some_ repo,
+# as the real test is what clone does on the local side
+test_expect_success 'setup ssh wrapper' '
+ write_script $TRASH_DIRECTORY/ssh-wrapper -\EOF
+ git upload-pack $TRASH_DIRECTORY
+ EOF
+ GIT_SSH=$TRASH_DIRECTORY/ssh-wrapper
+ export GIT_SSH
+ export TRASH_DIRECTORY
+'
+
+# make sure that cloning $1 results in local directory $2
+test_clone_dir () {
+ url=$1; shift
+ dir=$1; shift
+ expect=success
+ bare=non-bare
+ clone_opts=
+ for i in $@; do
+ case $i in
+ fail)
+ expect=failure
+ ;;
+ bare)
+ bare=bare
+ clone_opts=--bare
+ ;;
+ esac
+ done
+ test_expect_$expect clone of $url goes to $dir ($bare)
+ rm -rf $dir
+ git clone $clone_opts $url
+ test_path_is_dir $dir
+
+}
+
+# basic syntax with bare and non-bare variants
+test_clone_dir host:foo foo
+test_clone_dir host:foo foo.git bare
+test_clone_dir host:foo.git foo
+test_clone_dir host:foo.git foo.git bare
+test_clone_dir host:foo/.git foo
+test_clone_dir host:foo/.git foo.git bare fail
+
+# similar, but using ssh URL rather than host:path syntax
+test_clone_dir ssh://host/foo foo
+test_clone_dir ssh://host/foo foo.git bare
+test_clone_dir ssh://host/foo.git foo
+test_clone_dir ssh://host/foo.git foo.git bare
+test_clone_dir ssh://host/foo/.git foo
+test_clone_dir ssh://host/foo/.git foo.git bare fail
+
+# we should remove trailing slashes
+test_clone_dir ssh://host/foo/ foo
+test_clone_dir ssh://host/foo.git/ foo fail
+test_clone_dir ssh://host/foo/.git/ foo
+
+# omitting the path should default to the hostname
+test_clone_dir ssh://host/ host
+test_clone_dir ssh://host:1234/ host fail
+test_clone_dir ssh://user@host/ host fail
+
+test_done
--
2.5.0.148.g63828c1
--
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] clone: use computed length in guess_dir_name
Commit 7e837c6 (clone: simplify string handling in
guess_dir_name(), 2015-07-09) changed clone to use
strip_suffix instead of hand-rolled pointer manipulation.
However, strip_suffix will strip from the end of a
NUL-terminated string, and we may have already stripped some
characters (like directory separators, or /.git). This
leads to commands like:
git clone host:foo.git/
failing to strip the .git.
We must instead convert our pointer arithmetic into a
computed length and feed that to strip_suffix_mem, which will
then reduce the length further for us.
It would be nicer if we could drop the pointer manipulation
entirely, and just continually strip using strip_suffix. But
that doesn't quite work for two reasons:
1. The early suffixes we're stripping are not constant; we
need to look for is_dir_sep, which could be one of
several characters.
2. Mid-way through the stripping we compute the pointer
start, which shows us the beginning of the pathname.
Which really give us two lengths to work with: the
offset from the start of the string, and from the start
of the path. By using pointers for the early part, we
can just compute the length from start when we need
it.
Signed-off-by: Jeff King p...@peff.net
---
I suspect you _could_ clean up this logic further, but I
really wanted to do the minimal fix for the regression.
Especially because Patrick is hopefully going to sweep
through and make it all more robust soon enough. :)
builtin/clone.c | 3 ++-
t/t5603-clone-dirname.sh | 6 +++---
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 303a3a7..bf45199 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -174,7 +174,8 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
/*
* Strip .{bundle,git}.
*/
- strip_suffix(start, is_bundle ? .bundle : .git , len);
+ len = end - start;
+ strip_suffix_mem(start, len, is_bundle ? .bundle : .git);
if (is_bare)
dir = xstrfmt(%.*s.git, (int)len, start);
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index a0140b9..46725b9 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -46,7 +46,7 @@ test_clone_dir host:foo foo.git bare
test_clone_dir host:foo.git foo
test_clone_dir host:foo.git foo.git bare
test_clone_dir host:foo/.git foo
-test_clone_dir host:foo/.git foo.git bare fail
+test_clone_dir host:foo/.git foo.git bare
# similar, but using ssh URL rather than host:path syntax
test_clone_dir ssh://host/foo foo
@@ -54,11 +54,11 @@ test_clone_dir ssh://host/foo foo.git bare
test_clone_dir ssh://host/foo.git foo
test_clone_dir ssh://host/foo.git foo.git bare
test_clone_dir ssh://host/foo/.git foo
-test_clone_dir ssh://host/foo/.git foo.git bare fail
+test_clone_dir ssh://host/foo/.git foo.git bare
# we should remove trailing slashes
test_clone_dir ssh://host/foo/ foo
-test_clone_dir ssh://host/foo.git/ foo fail
+test_clone_dir ssh://host/foo.git/ foo
test_clone_dir ssh://host/foo/.git/ foo
# omitting the path should default to the hostname
--
2.5.0.148.g63828c1
--
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 v4] clone: simplify string handling in guess_dir_name()
On Wed, Aug 05, 2015 at 08:08:52AM +0200, Patrick Steinhardt wrote: Sadly we cannot just `strip_suffix_mem(repo, len, /.git))` in the earlier code, as we have to account for multiple directory separators. I believe the above code does the right thing, though. I haven't looked at how badly it interacts with the other guess_dir_name work from Patrick Steinhardt that has been going on, though. It shouldn't be hard rebasing my work onto this. If it's being applied I'll come up with a new version. Thanks, it is always nice when contributors are flexible and easy to work with. :) Hopefully the new tests I've added can help you out, as well. -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/2] clone: use computed length in guess_dir_name
On 8/5/2015 10:39, Jeff King wrote: Commit 7e837c6 (clone: simplify string handling in guess_dir_name(), 2015-07-09) changed clone to use strip_suffix instead of hand-rolled pointer manipulation. However, strip_suffix will strip from the end of a NUL-terminated string, and we may have already stripped some characters (like directory separators, or /.git). This leads to commands like: git clone host:foo.git/ failing to strip the .git. Thanks a lot Peff for fixing my bugs, I should have known that you'll be able to come up with something much sooner than I would ;-) This all looks good to me! Regards, Sebastian -- 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] git_open_noatime: return with errno=0 on success
On Tue, Aug 4, 2015 at 11:03 PM, Junio C Hamano gits...@pobox.com wrote: I would agree it is a good idea to clear it after seeing the first open fail due to lack of O_NOATIME before trying open for the second time, iow, more like this? So I don't think this is _wrong_ per se, but I think the deeper issue is that somebody cares about 'errno' here in the first place. A stale 'errno' generally shouldn't matter, because we either (a) return success (and nobody should look at errno) or (b) return an error later, without setting errno for that _later_ error. and I think either of those two situations are the real bug, and this clear stale errno is just a workaround. But as mentioned, I don't think clearign errno is wrong, so I'm not objecting to the patch. I just suspect there's something else goign on too.. Linus -- 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 v4] clone: simplify string handling in guess_dir_name()
On Wed, Aug 05, 2015 at 04:41:48AM -0400, Jeff King wrote: On Wed, Aug 05, 2015 at 08:08:52AM +0200, Patrick Steinhardt wrote: Sadly we cannot just `strip_suffix_mem(repo, len, /.git))` in the earlier code, as we have to account for multiple directory separators. I believe the above code does the right thing, though. I haven't looked at how badly it interacts with the other guess_dir_name work from Patrick Steinhardt that has been going on, though. It shouldn't be hard rebasing my work onto this. If it's being applied I'll come up with a new version. Thanks, it is always nice when contributors are flexible and easy to work with. :) Hopefully the new tests I've added can help you out, as well. -Peff You're welcome. And yes, your tests help me quite a lot here. Got tedious to always set up the chroot. Guess I'll still send my fixes for the chroot-tests as a separate patch series, even though I don't require them anymore. Short question on how to proceed: should I mention that my patch series builds upon your patches or just include them in my series? Patrick signature.asc Description: Digital signature
Re: [PATCH v4] clone: simplify string handling in guess_dir_name()
On Wed, Aug 05, 2015 at 11:06:03AM +0200, Patrick Steinhardt wrote: You're welcome. And yes, your tests help me quite a lot here. Got tedious to always set up the chroot. Guess I'll still send my fixes for the chroot-tests as a separate patch series, even though I don't require them anymore. Yeah, fixes for t1509 are definitely welcome. Short question on how to proceed: should I mention that my patch series builds upon your patches or just include them in my series? I think we'll want to merge my patches separately, due to the regression, so you should not include them. So hopefully the sequence is: 1. Junio picks them up as jk/guess-repo-name-regression or similar. 2. They are merged to 'next', and then eventually to 'maint'. 3. Mention the topic branch name (whatever Junio ends up picking for it) when you post your patches. Junio can then apply on top in his repo. 4. If re-rolls keep going past step 2, then it becomes a non-issue, as 'maint' gets merged to 'master'. -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 1/2] tests: fix broken chains in t1509-root-worktree
Signed-off-by: Patrick Steinhardt p...@pks.im --- These two patches have previously been part of my patch series fixing directory guessing. As Jeff King has been posting a patch that contains tests for cloning from a server's root without requiring t1509 I now post these two fixes as separate patches. t/t1509-root-worktree.sh | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh index b6977d4..0c80129 100755 --- a/t/t1509-root-worktree.sh +++ b/t/t1509-root-worktree.sh @@ -125,7 +125,7 @@ fi ONE_SHA1=d00491fd7e5bb6fa28c517a0bb32b8b506539d4d test_expect_success 'setup' ' - rm -rf /foo + rm -rf /foo mkdir /foo mkdir /foo/bar echo 1 /foo/foome @@ -218,7 +218,7 @@ unset GIT_WORK_TREE test_expect_success 'go to /' 'cd /' test_expect_success 'setup' ' - rm -rf /.git + rm -rf /.git echo Initialized empty Git repository in /.git/ expected git init result test_cmp expected result @@ -241,8 +241,8 @@ say auto bare gitdir # DESTROY! test_expect_success 'setup' ' - rm -rf /refs /objects /info /hooks - rm /* + rm -rf /refs /objects /info /hooks + rm /* cd / echo Initialized empty Git repository in / expected git init --bare result -- 2.5.0 -- 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] tests: fix cleanup after tests in t1509-root-worktree
During cleanup we do a simple 'rm /*' to remove leftover files from previous tests. As 'rm' errors out when there is anything it cannot delete and there are directories present at '/' it will throw an error, causing the '' chain to fail. Fix this by explicitly removing the files. Signed-off-by: Patrick Steinhardt p...@pks.im --- t/t1509-root-worktree.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t1509-root-worktree.sh b/t/t1509-root-worktree.sh index 0c80129..553a3f6 100755 --- a/t/t1509-root-worktree.sh +++ b/t/t1509-root-worktree.sh @@ -242,7 +242,7 @@ say auto bare gitdir # DESTROY! test_expect_success 'setup' ' rm -rf /refs /objects /info /hooks - rm /* + rm -f /expected /ls.expected /me /result cd / echo Initialized empty Git repository in / expected git init --bare result -- 2.5.0 -- 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 v4 1/3] clone: do not include authentication data in guessed dir
If the URI contains authentication data and the URI's path
component is empty we fail to guess a sensible directory name.
E.g. cloning a repository 'ssh://user:passw...@example.com/' we
guess a directory name 'passw...@example.com' where we would want
the hostname only, e.g. 'example.com'.
Fix this by first skipping authentication data.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
builtin/clone.c | 40 ++--
t/t5603-clone-dirname.sh | 3 ++-
2 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index bf45199..3ddf8b9 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -146,30 +146,50 @@ static char *get_repo_path(const char *repo, int
*is_bundle)
static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
{
- const char *end = repo + strlen(repo), *start;
+ const char *end = repo + strlen(repo), *start, *ptr;
size_t len;
char *dir;
/*
+* Skip scheme.
+*/
+ start = strstr(repo, ://);
+ if (start == NULL)
+ start = repo;
+ else
+ start += 3;
+
+ /*
+* Skip authentication data.
+*/
+ ptr = start;
+ while (ptr end !is_dir_sep(*ptr) *ptr != '@')
+ ptr++;
+ if (*ptr == '@')
+ start = ptr + 1;
+
+ /*
* Strip trailing spaces, slashes and /.git
*/
- while (repo end (is_dir_sep(end[-1]) || isspace(end[-1])))
+ while (start end (is_dir_sep(end[-1]) || isspace(end[-1])))
end--;
- if (end - repo 5 is_dir_sep(end[-5])
+ if (end - start 5 is_dir_sep(end[-5])
!strncmp(end - 4, .git, 4)) {
end -= 5;
- while (repo end is_dir_sep(end[-1]))
+ while (start end is_dir_sep(end[-1]))
end--;
}
/*
-* Find last component, but be prepared that repo could have
-* the form remote.example.com:foo.git, i.e. no slash
-* in the directory part.
+* Find last component. To remain backwards compatible we
+* also regard colons as path separators, such that
+* cloning a repository 'foo:bar.git' would result in a
+* directory 'bar' being guessed.
*/
- start = end;
- while (repo start !is_dir_sep(start[-1]) start[-1] != ':')
- start--;
+ ptr = end;
+ while (start ptr !is_dir_sep(ptr[-1]) ptr[-1] != ':')
+ ptr--;
+ start = ptr;
/*
* Strip .{bundle,git}.
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index 46725b9..3a454f9 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -64,6 +64,7 @@ test_clone_dir ssh://host/foo/.git/ foo
# omitting the path should default to the hostname
test_clone_dir ssh://host/ host
test_clone_dir ssh://host:1234/ host fail
-test_clone_dir ssh://user@host/ host fail
+test_clone_dir ssh://user@host/ host
+test_clone_dir ssh://user:password@host/ host
test_done
--
2.5.0
--
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 v4 2/3] clone: do not use port number as dir name
If the URI contains a port number and the URI's path component is
empty we fail to guess a sensible directory name. E.g. cloning a
repository 'ssh://example.com:/' we guess a directory name
'' where we would want the hostname only, e.g. 'example.com'.
Fix this by stripping trailing port numbers.
Signed-off-by: Patrick Steinhardt p...@pks.im
---
builtin/clone.c | 17 +
t/t5603-clone-dirname.sh | 7 ++-
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 3ddf8b9..7d93e13 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -181,6 +181,23 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
}
/*
+* Strip trailing port number if we've got only a
+* hostname (that is, there is no dir separator but a
+* colon). This check is required such that we do not
+* strip URI's like '/foo/bar:.git', which should
+* result in a dir '' being guessed due to backwards
+* compatibility.
+*/
+ if (memchr(start, '/', end - start) == NULL
+memchr(start, ':', end - start) != NULL) {
+ ptr = end;
+ while (start ptr isdigit(ptr[-1]) ptr[-1] != ':')
+ ptr--;
+ if (start ptr ptr[-1] == ':')
+ end = ptr - 1;
+ }
+
+ /*
* Find last component. To remain backwards compatible we
* also regard colons as path separators, such that
* cloning a repository 'foo:bar.git' would result in a
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index 3a454f9..27dbd6c 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -63,8 +63,13 @@ test_clone_dir ssh://host/foo/.git/ foo
# omitting the path should default to the hostname
test_clone_dir ssh://host/ host
-test_clone_dir ssh://host:1234/ host fail
+test_clone_dir ssh://host:1234/ host
test_clone_dir ssh://user@host/ host
test_clone_dir ssh://user:password@host/ host
+test_clone_dir ssh://user:password@host:1234/ host
+
+# trailing port-like numbers should not be stripped for paths
+test_clone_dir ssh://user:password@host/test:1234 1234
+test_clone_dir ssh://user:password@host/test:1234.git 1234
test_done
--
2.5.0
--
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 v4 3/3] clone: abort if no dir name could be guessed
Due to various components of the URI being stripped off it may happen that we fail to guess a directory name. We currently error out with a message that it is impossible to create the working tree '' in such cases. Instead, error out early with a sensible error message hinting that a directory name should be specified manually on the command line. Signed-off-by: Patrick Steinhardt p...@pks.im --- builtin/clone.c | 4 1 file changed, 4 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index 7d93e13..5834978 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -214,6 +214,10 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) len = end - start; strip_suffix_mem(start, len, is_bundle ? .bundle : .git); + if (!len || (len == 1 *start == '/')) + die(No directory name could be guessed.\n + Please specify a directory on the command line); + if (is_bare) dir = xstrfmt(%.*s.git, (int)len, start); else -- 2.5.0 -- 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 v4 0/3] fix repo name when cloning a server's root
This is version 4 of my patch series, which aims to improve guessed directory names when we clone a server's root, that is we have empty path components. This version is still preliminary as it is based upon the patches by Peff ([PATCH 0/2] fix clone guess_dir_name regression in v2.4.8) which have not been merged yet. As his patches include newly added tests that allow for cloning from a server's root without using chroot, the patches regarding t1509 have been dropped. I've posted the fixes to them as a separate patch series instead. Previously I've included a guard such that we only try to find the last component if we've got a '/' or ':' in the path. This is not really required, though, as in the case where we ain't got one of those characters we'll simply skip to the beginning again, causing this to be a no-op, due to the port already being stripped. So I've simply dropped the guard to minimize code churn. Patrick Steinhardt (3): clone: do not include authentication data in guessed dir clone: do not use port number as dir name clone: abort if no dir name could be guessed builtin/clone.c | 61 t/t5603-clone-dirname.sh | 10 ++-- 2 files changed, 59 insertions(+), 12 deletions(-) -- 2.5.0 -- 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
[no subject]
Dear community, For some time I'm wondering why there's no --grep option to the git branch command, which would request to print only branches having specified string/regexp in their history. So for example: $ git branch -r --grep=BUG12345 should be roughly equivalent to following expression I'm using now for the same task: $ for r in `git rev-list --grep=BUG12345 --remotes=origin`; do git branch -r --list --contains=$r 'origin/*'; done | sort -u Am I missing something, is there some smarter/simpler way to do this? Thanks a lot in advance! -- Ivan -- 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 1/1] completion: offer '--edit-todo' during interactive rebase
Signed-off-by: Thomas Braun thomas.br...@virtuell-zuhause.de
Helped-by: John Keeping j...@keeping.me.uk
Helped-by: SZEDER Gábor sze...@ira.uka.de
---
Tested by:
- ensuring I'm in a bash shell
- source git-completion.bash
- git rebase -i HEAD~1, choose edit instead of pick in the editor
- on entering git rebase you should be offered --edit-todo
contrib/completion/git-completion.bash | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index c97c648..087771b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1667,7 +1667,10 @@ _git_push ()
_git_rebase ()
{
local dir=$(__gitdir)
- if [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ]; then
+ if [ -f $dir/rebase-merge/interactive ]; then
+ __gitcomp --continue --skip --abort --edit-todo
+ return
+ elif [ -d $dir/rebase-apply ] || [ -d $dir/rebase-merge ]; then
__gitcomp --continue --skip --abort
return
fi
--
2.4.3.413.ga5fe668
--
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/RFC] gitweb: Don't pass --full-history to git-log(1)
When you look at the history for a file via git log we don't show
--full-history by default, but the Gitweb UI does so, which can be very
confusing for all the reasons discussed in History Simplification in
git-log(1) and in
http://thread.gmane.org/gmane.comp.version-control.git/89400/focus=90659
We've been doing history via --full-history since pretty much forever,
but I think this is much more usable, and on a typical project with lots
of branches being merged it makes for a much less confusing view. We do
this for git log by default, why wouldn't Gitweb follow suit?
Signed-off-by: Ævar Arnfjörð Bjarmason ava...@gmail.com
---
gitweb/gitweb.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 7a5b23a..2913896 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -7387,7 +7387,7 @@ sub git_log_generic {
}
my @commitlist =
parse_commits($commit_hash, 101, (100 * $page),
- defined $file_name ? ($file_name,
--full-history) : ());
+ defined $file_name ? $file_name : ());
my $ftype;
if (!defined $file_hash defined $file_name) {
--
2.1.3
--
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] git_open_noatime: return with errno=0 on success
On Wed, Aug 05, 2015 at 10:59:09AM +0200, Linus Torvalds wrote: On Tue, Aug 4, 2015 at 11:03 PM, Junio C Hamano gits...@pobox.com wrote: I would agree it is a good idea to clear it after seeing the first open fail due to lack of O_NOATIME before trying open for the second time, iow, more like this? Looks good to me. So I don't think this is _wrong_ per se, but I think the deeper issue is that somebody cares about 'errno' here in the first place. A stale 'errno' generally shouldn't matter, because we either (a) return success (and nobody should look at errno) or (b) return an error later, without setting errno for that _later_ error. and I think either of those two situations are the real bug, and this clear stale errno is just a workaround. I agree. But I do not see how to get there easily. We are trying to read an object. We first try to read from a pack. We may encounter broken pack files, missing index files, unreadable files, but those errors are not necessarily fatal since we may still be able to read the object from the next pack file or from a sha1 file. If finally we do not find the object anywhere, in read_sha1_file_extended we try our best to die with an appropriate error message, for example by looking at errno, and otherwise we just return NULL. Most callers seem to die explicitly or they dereference the null pointer. I think we should instead output error messages closer to the source, like for example in map_sha1_file, but continue anyway. In particular we should immediately report failures due to EPERM or unexpected ENOENT. In the end we may return NULL without another message, but at least the user should have some hints about what went wrong along the way. -- 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/3] am: let command-line options override saved options
Interesting. This seems to break test under prove. cd t make T=t4153-am-resume-override-opts.sh prove does not seem to return. -- 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
[Bug] incomplete defence agains creating a branch named HEAD
So git branch doesn't like to create a branch named HEAD $ git branch HEAD fatal: it does not make sense to create 'HEAD' manually But, you can trick it into doing so anyway: $ git branch @ $ git branch -a HEAD * master At which point git status becomes a bit confused: $ git status warning: refname 'HEAD' is ambiguous. warning: refname 'HEAD' is ambiguous. On branch master nothing to commit, working directory clean Oh, and git checkout will accept either HEAD or @ to create a branch with the name HEAD anyway: $ git checkout -b HEAD Switched to a new branch 'HEAD' $ git checkout -b @ Switched to a new branch 'HEAD' imho none of these should create a branch named HEAD, but should do what 'git branch HEAD' does and fail with a sensible error message. All these shenanigans came up when trying to help an user who is mirroring a mercurial repo that has a branch named '@'. Whether or not git should allow branches named '@' I don't have an opinion on, I know '@' is pretty special when dealing with refs. -- Dennis Kaarsemaker www.kaarsemaker.net -- 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] git_open_noatime: return with errno=0 on success
Clemens Buchacher clemens.buchac...@intel.com writes: On Wed, Aug 05, 2015 at 10:59:09AM +0200, Linus Torvalds wrote: ... A stale 'errno' generally shouldn't matter, because we either (a) return success (and nobody should look at errno) or (b) return an error later, without setting errno for that _later_ error. and I think either of those two situations are the real bug, and this clear stale errno is just a workaround. I agree. But I do not see how to get there easily. We are trying to read an object. We first try to read from a pack. We may encounter broken pack files, missing index files, unreadable files, but those errors are not necessarily fatal since we may still be able to read the object from the next pack file or from a sha1 file. If finally we do not find the object anywhere, in read_sha1_file_extended we try our best to die with an appropriate error message, for example by looking at errno, and otherwise we just return NULL. Most callers seem to die explicitly or they dereference the null pointer. I think we should instead output error messages closer to the source, like for example in map_sha1_file, but continue anyway. Hmm, if we find one data source unreadable but an alternative usable, do we really want that error message? What should it say? error: cannot read from pack? Such a message, unless we later give info: but we managed to read it from elsewhere and make sure these two messages are clearly associated with each other, would make things unnecessarily alarming, wouldn't it? Perhaps we should not rely so heavily on 'errno', but explicitly pass around error code (or enough information to formulate an intelligent message at the end) in the callchain instead. Then the earlier part can notice EPERM on a pack, for example, and return to the caller, and after consulting an alternate data source (e.g. loose object file), the caller can then choose to say we managed to read the data, but FYI, you may want to check the permission bits of this pack, or choose to stay silent. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] submodule: implement `module_name` as a builtin helper
On Tue, Aug 4, 2015 at 5:58 PM, Eric Sunshine sunsh...@sunshineco.com wrote:
On Tue, Aug 4, 2015 at 8:04 PM, Stefan Beller sbel...@google.com wrote:
The goal of this series being rewriting `git submodule update`,
we don't want to call out to the shell script for config lookups.
So reimplement the lookup of the submodule name in C.
Signed-off-by: Stefan Beller sbel...@google.com
---
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index cb18ddf..dd5635f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -98,6 +100,48 @@ static int module_list(int argc, const char **argv,
const char *prefix)
+static int collect_module_names(const char *key, const char *value, void
*cb)
+{
+ size_t len;
+ struct string_list *sl = cb;
+
+ if (starts_with(key, submodule.)
+strip_suffix(key, .path, len)) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_add(sb, key + strlen(submodule.),
+ len - strlen(submodule.));
+ string_list_insert(sl, value)-util = strbuf_detach(sb,
NULL);
+ strbuf_release(sb);
Why the complexity and overhead of a strbuf when the same could be
accomplished more easily and straightforwardly with xstrndup()?
fixed.
+ }
+
+ return 0;
+}
+
+static int module_name(int argc, const char **argv, const char *prefix)
+{
+ struct string_list_item *item;
+ struct git_config_source config_source;
+ struct string_list values = STRING_LIST_INIT_DUP;
+
+ if (!argc)
Do you mean?
if (argc != 1)
doh! Yes I meant that.
+ usage(git submodule--helper module_name path\n);
+
+ memset(config_source, 0, sizeof(config_source));
+ config_source.file = .gitmodules;
+
+ if (git_config_with_options(collect_module_names, values,
+ config_source, 1) 0)
+ die(_(unknown error occured while reading the git modules
file));
+
+ item = string_list_lookup(values, argv[0]);
+ if (item)
+ printf(%s\n, (char*)item-util);
+ else
+ die(No submodule mapping found in .gitmodules for path
'%s', argv[0]);
+ return 0;
+}
+
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
{
if (argc 2)
@@ -106,6 +150,9 @@ int cmd_submodule__helper(int argc, const char **argv,
const char *prefix)
if (!strcmp(argv[1], module_list))
return module_list(argc - 1, argv + 1, prefix);
+ if (!strcmp(argv[1], module_name))
+ return module_name(argc - 2, argv + 2, prefix);
+
usage:
usage(git submodule--helper module_list\n);
}
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] gitweb: Don't pass --full-history to git-log(1)
Ævar Arnfjörð Bjarmason ava...@gmail.com writes: When you look at the history for a file via git log we don't show --full-history by default, but the Gitweb UI does so, which can be very confusing for all the reasons discussed in History Simplification in git-log(1) and in http://thread.gmane.org/gmane.comp.version-control.git/89400/focus=90659 We've been doing history via --full-history since pretty much forever, but I think this is much more usable, and on a typical project with lots of branches being merged it makes for a much less confusing view. We do this for git log by default, why wouldn't Gitweb follow suit? http://thread.gmane.org/gmane.comp.version-control.git/89400/focus=90758 seems to agree with you in principle that this would be what gitweb should do if it were written today. -- 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
Which branch(es) contain certain commits? (was Re: (unknown))
Ivan Chernyavsky campo...@yandex.ru writes: For some time I'm wondering why there's no --grep option to the git branch command, which would request to print only branches having specified string/regexp in their history. So for example: $ git branch -r --grep=BUG12345 should be roughly equivalent to following expression I'm using now for the same task: $ for r in `git rev-list --grep=BUG12345 --remotes=origin`; do git branch -r --list --contains=$r 'origin/*'; done | sort -u Am I missing something, is there some smarter/simpler way to do this? I think people do things like: git log --all --decorate --grep=... -- 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: Which branch(es) contain certain commits? (was Re: (unknown))
Junio C Hamano gits...@pobox.com writes: I think people do things like: git log --all --decorate --grep=... s/decorate/source/; sorry for the noise. -- 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/2] fix clone guess_dir_name regression in v2.4.8
Jeff King p...@peff.net writes: On Tue, Aug 04, 2015 at 06:42:46PM -0400, Jeff King wrote: I did not intend this change in behavior, and I can confirm that reverting my patch restores the original behavior. Thanks for bringing this to my attention, I'll work on a patch. I think this regression is in v2.4.8, as well. We should be able to use a running len instead of the end pointer in the earlier part, and then use strip_suffix_mem later (to strip from our already-reduced length, rather than the full NUL-terminated string). Like this: Looks like git clone --bare host:foo/.git is broken, too. I've added some tests to cover the recently broken cases, as well as some obvious normal cases (which the patch I sent earlier break!). And as a bonus, we can easily cover Patrick's root-repo problems (so people will actually run the tests, unlike the stuff in t1509. :) ). Sorry, my fault; I should have been much less trusting while queuing a patch like that offending one that was meant to be a no-op. Here are the patches. [1/2]: clone: add tests for output directory [2/2]: clone: use computed length in guess_dir_name 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
Error pushing new branch: value too great for base (error token is...
I had written the following code to check whether a push is for branch deletion: #!/bin/bash NULL= while read old_sha new_sha refname ; do echo Stdin: [$old_sha] [$new_sha] [$refname] if [[ $new_sha -eq $NULL ]]; then # Line 17 echo Skipping checks... continue fi ... ... ... done While it works fine for branch deletion, i noticed that if i create a branch and push it to remote, i get the following error due to the above-mentioned code: $ git push origin rel-a Counting objects: 5, done. Delta compression using up to 4 threads. Compressing objects: 100% (2/2), done. Writing objects: 100% (3/3), 407 bytes | 0 bytes/s, done. Total 3 (delta 0), reused 0 (delta 0) remote: Stdin: [] [9226289d2416af4cb7365d7aaa5e382bdb3d9a89] [refs/heads/rel-a] remote: remote: hooks/pre-receive: line 17: [[: 9226289d2416af4cb7365d7aaa5e382bdb3d9a89: value too great for base (error token is 922628 9d2416af4cb7365d7aaa5e382bdb3d9a89) Although the new branch gets pushed to remote but i'm not sure why i'm getting this error and how can i fix it. I checked online and i get few links where folks had similar issue but in each such case, the error token was 08 or 09. I still tried the suggestion of using 10# in front of my $new_sha variable but to no avail. Any suggestions? -- 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 0/6] fix repo name when cloning a server's root
Patrick Steinhardt p...@pks.im writes: - The naive way of just adding '@' as path separator would break cloning repositories like '/foo/b...@baz.git' (which would currently become 'bar@baz' but would become 'baz' only). - Skipping the scheme initially is required because without it we wouldn't be able to scan until the next dir separator in the host part when stripping authentication information. - First checking for '/' in the current stripped URI when we want to remove the port is required because we do not want to strip port numbers when cloning from something like '/foo/bar:.git' (which would currently become '' but would then be stripped of the ':' part and instead become 'bar'). Still, this breaks on cloning a bare repository in the current dir (e.g. cloning 'bar:.git', which should become '' because it is not a port number but would become 'bar'). This is a very good write-up. Please make sure all of the above appears in the commit log message somewhere. As you can see, there is a lot of complexity in there and I'm not convinced this is better than just exposing 'parse_connect_url()', which already handles everything for us. If the function handles everything for us, that's fine, but the primary reason I am hesitant is because parse_connect_url() was designed specifically not to have to worry about some protocols (e.g. I think feeding it a http://; would fail, and more importantly, its current callers want such a call to fail). Also it is meant to handle some non-protocols (e.g. scp style host:path that does not follow scheme://...). Also does it handle the case above? I do not think parse_connect_url() even calls get_host_and_port() to be able to tell what means in these examples. Maybe I'm just being blind for the obvious solution here, though. Patrick Steinhardt (6): tests: fix broken chains in t1509-root-worktree tests: fix cleanup after tests in t1509-root-worktree clone: do not include authentication data in guessed dir clone: do not use port number as dir name clone: abort if no dir name could be guessed clone: add tests for cloning with empty path builtin/clone.c | 67 t/t1509-root-worktree.sh | 51 +--- 2 files changed, 103 insertions(+), 15 deletions(-) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/3] clone: do not include authentication data in guessed dir
Patrick Steinhardt p...@pks.im writes:
If the URI contains authentication data and the URI's path
component is empty we fail to guess a sensible directory name.
E.g. cloning a repository 'ssh://user:passw...@example.com/' we
guess a directory name 'passw...@example.com' where we would want
the hostname only, e.g. 'example.com'.
...
+ ptr = start;
+ while (ptr end !is_dir_sep(*ptr) *ptr != '@')
+ ptr++;
Hmm
+ if (*ptr == '@')
+ start = ptr + 1;
+
+ * Find last component. To remain backwards compatible we
+ * also regard colons as path separators, such that
+ * cloning a repository 'foo:bar.git' would result in a
+ * directory 'bar' being guessed.
*/
I think this is a reasonable thing to do (besides, I think some
people cannot have colon in their filenames, so keeping this aspect
the same as before would avoid unintended regressions).
- start = end;
- while (repo start !is_dir_sep(start[-1]) start[-1] != ':')
- start--;
+ ptr = end;
+ while (start ptr !is_dir_sep(ptr[-1]) ptr[-1] != ':')
+ ptr--;
+ start = ptr;
/*
* Strip .{bundle,git}.
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index 46725b9..3a454f9 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -64,6 +64,7 @@ test_clone_dir ssh://host/foo/.git/ foo
# omitting the path should default to the hostname
test_clone_dir ssh://host/ host
test_clone_dir ssh://host:1234/ host fail
-test_clone_dir ssh://user@host/ host fail
+test_clone_dir ssh://user@host/ host
+test_clone_dir ssh://user:password@host/ host
Perhaps add
test_clone_dir ssh://user:passw@rd@host/ host
here? How is this expected to be parsed?
test_done
--
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 v4 3/3] clone: abort if no dir name could be guessed
Patrick Steinhardt p...@pks.im writes: Due to various components of the URI being stripped off it may happen that we fail to guess a directory name. We currently error out with a message that it is impossible to create the working tree '' in such cases. Instead, error out early with a sensible error message hinting that a directory name should be specified manually on the command line. Sounds like a sensible thing to do. Signed-off-by: Patrick Steinhardt p...@pks.im --- builtin/clone.c | 4 1 file changed, 4 insertions(+) diff --git a/builtin/clone.c b/builtin/clone.c index 7d93e13..5834978 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -214,6 +214,10 @@ static char *guess_dir_name(const char *repo, int is_bundle, int is_bare) len = end - start; strip_suffix_mem(start, len, is_bundle ? .bundle : .git); + if (!len || (len == 1 *start == '/')) + die(No directory name could be guessed.\n + Please specify a directory on the command line); + if (is_bare) dir = xstrfmt(%.*s.git, (int)len, start); else -- 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: Error pushing new branch: value too great for base (error token is...
On Wed, Aug 5, 2015 at 1:32 PM, Gaurav Chhabra varuag.chha...@gmail.com wrote: I had written the following code to check whether a push is for branch deletion: #!/bin/bash NULL= if [[ $new_sha -eq $NULL ]]; then # Line 17 remote: Stdin: [] [9226289d2416af4cb7365d7aaa5e382bdb3d9a89] [refs/heads/rel-a] remote: remote: hooks/pre-receive: line 17: [[: 9226289d2416af4cb7365d7aaa5e382bdb3d9a89: value too great for base (error token is 922628 9d2416af4cb7365d7aaa5e382bdb3d9a89) Although the new branch gets pushed to remote but i'm not sure why i'm getting this error and how can i fix it. I checked online and i get few links where folks had similar issue but in each such case, the error token was 08 or 09. I still tried the suggestion of using 10# in front of my $new_sha variable but to no avail. Any suggestions? Yes, try using the string comparison '=' operator rather than the numeric comparison operator '-eq'. if [[ $new_sha = $NULL ]]; then -- 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] submodule: implement `module_list` as a builtin helper
Am 05.08.2015 um 02:04 schrieb Stefan Beller:
Most of the submodule operations work on a set of submodules.
Calculating and using this set is usually done via:
module_list $@ | {
while read mode sha1 stage sm_path
do
# the actual operation
done
}
Currently the function `module_list` is implemented in the
git-submodule.sh as a shell script wrapping a perl script.
The rewrite is in C, such that it is faster and can later be
easily adapted when other functions are rewritten in C.
git-submodule.sh similar to the builtin commands will navigate
to the top most directory of the repository and keeping the
subdirectories as a variable. As the helper is called from
within the git-submodule.sh script, we are already navigated
to the root level, but the path arguments are stil relative
to the subdirectory we were in when calling git-submodule.sh.
That's why there is a `--prefix` option pointing to an alternative
path where to anchor relative path arguments.
Great you are working on this! I'll try to help, but you might
see some latency as my Git time budget is currently very limited.
I think this patch is definitely going into the right direction.
The whole test suite runs 3 seconds faster for me with this
applied: best of three is 3:16 without and 3:13 with this patch.
That's quite an improvement, especially as only parts of the test
suite deal with submodules! (And I expect Windows users to profit
even more, considered how expensive forking is there)
Acked-by: Jens Lehmann jens.lehm...@web.de
Signed-off-by: Stefan Beller sbel...@google.com
---
The same as yesterday evening, just an entry added to .gitignore.
So we'll have a git submodule--helper module_list here.
.gitignore | 1 +
Makefile| 1 +
builtin.h | 1 +
builtin/submodule--helper.c | 111
git-submodule.sh| 54 +++--
git.c | 1 +
6 files changed, 121 insertions(+), 48 deletions(-)
create mode 100644 builtin/submodule--helper.c
diff --git a/.gitignore b/.gitignore
index a685ec1..2a69ba0 100644
--- a/.gitignore
+++ b/.gitignore
@@ -155,6 +155,7 @@
/git-status
/git-stripspace
/git-submodule
+/git-submodule--helper
/git-svn
/git-symbolic-ref
/git-tag
diff --git a/Makefile b/Makefile
index 7efedbe..460d17a 100644
--- a/Makefile
+++ b/Makefile
@@ -899,6 +899,7 @@ BUILTIN_OBJS += builtin/shortlog.o
BUILTIN_OBJS += builtin/show-branch.o
BUILTIN_OBJS += builtin/show-ref.o
BUILTIN_OBJS += builtin/stripspace.o
+BUILTIN_OBJS += builtin/submodule--helper.o
BUILTIN_OBJS += builtin/symbolic-ref.o
BUILTIN_OBJS += builtin/tag.o
BUILTIN_OBJS += builtin/unpack-file.o
diff --git a/builtin.h b/builtin.h
index 839483d..924e6c4 100644
--- a/builtin.h
+++ b/builtin.h
@@ -119,6 +119,7 @@ extern int cmd_show(int argc, const char **argv, const char
*prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
extern int cmd_status(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
+extern int cmd_submodule__helper(int argc, const char **argv, const char
*prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
extern int cmd_tag(int argc, const char **argv, const char *prefix);
extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
new file mode 100644
index 000..cb18ddf
--- /dev/null
+++ b/builtin/submodule--helper.c
@@ -0,0 +1,111 @@
+#include builtin.h
+#include cache.h
+#include parse-options.h
+#include quote.h
+#include pathspec.h
+#include dir.h
+#include utf8.h
+
+static char *ps_matched;
+static const struct cache_entry **ce_entries;
+static int ce_alloc, ce_used;
+static struct pathspec pathspec;
+static const char *alternative_path;
+
+static void module_list_compute(int argc, const char **argv,
+ const char *prefix,
+ struct pathspec *pathspec)
+{
+ int i;
+ char *max_prefix;
+ int max_prefix_len;
+ parse_pathspec(pathspec, 0,
+ PATHSPEC_PREFER_FULL |
+ PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
+ prefix, argv);
+
+ /* Find common prefix for all pathspec's */
+ max_prefix = common_prefix(pathspec);
+ max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
+
+ if (pathspec-nr)
+ ps_matched = xcalloc(1, pathspec-nr);
+
+
+ if (read_cache() 0)
+ die(index file corrupt);
+
+ for (i = 0; i active_nr; i++) {
+ const struct cache_entry *ce = active_cache[i];
+
+ if (!match_pathspec(pathspec, ce-name, ce_namelen(ce),
+
[PATCH v9 03/11] ref-filter: implement an `align` atom
Implement an `align` atom which will act as a modifier atom and align
any string with or without an %(atom) appearing before a %(end) atom
to the right, left or middle.
It is followed by `:type,paddinglength`, where the `type` is
either left, right or middle and `paddinglength` is the total length
of the padding to be performed. If the atom length is more than the
padding length then no padding is performed. e.g. to pad a succeeding
atom to the middle with a total padding size of 40 we can do a
--format=%(align:middle,40)..
Add documentation and tests for the same.
Mentored-by: Christian Couder christian.cou...@gmail.com
Mentored-by: Matthieu Moy matthieu@grenoble-inp.fr
Signed-off-by: Karthik Nayak karthik@gmail.com
---
Documentation/git-for-each-ref.txt | 8
ref-filter.c | 84 +++---
ref-filter.h | 18 +++-
t/t6302-for-each-ref-filter.sh | 48 ++
4 files changed, 151 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-for-each-ref.txt
b/Documentation/git-for-each-ref.txt
index e49d578..d865f98 100644
--- a/Documentation/git-for-each-ref.txt
+++ b/Documentation/git-for-each-ref.txt
@@ -127,6 +127,14 @@ color::
Change output color. Followed by `:colorname`, where names
are described in `color.branch.*`.
+align::
+ Align any string with or without %(atom) before the %(end)
+ atom to the right, left or middle. Followed by
+ `:type,paddinglength`, where the `type` is either left,
+ right or middle and `paddinglength` is the total length of
+ the padding to be performed. If the string length is more than
+ the padding length then no padding is performed.
+
In addition to the above, for commit and tag objects, the header
field names (`tree`, `parent`, `object`, `type`, and `tag`) can
be used to specify the value in the header field.
diff --git a/ref-filter.c b/ref-filter.c
index 2c074a1..d123299 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -10,6 +10,7 @@
#include quote.h
#include ref-filter.h
#include revision.h
+#include utf8.h
typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
@@ -53,6 +54,8 @@ static struct {
{ flag },
{ HEAD },
{ color },
+ { align },
+ { end },
};
/*
@@ -620,7 +623,7 @@ static void populate_value(struct ref_array_item *ref)
const char *name = used_atom[i];
struct atom_value *v = ref-value[i];
int deref = 0;
- const char *refname;
+ const char *refname = NULL;
const char *formatp;
struct branch *branch = NULL;
@@ -687,6 +690,29 @@ static void populate_value(struct ref_array_item *ref)
else
v-s = ;
continue;
+ } else if (starts_with(name, align:)) {
+ const char *valp = NULL;
+ struct align *align = xmalloc(sizeof(struct align));
+
+ skip_prefix(name, align:, valp);
+
+ if (skip_prefix(valp, left,, valp))
+ align-align_type = ALIGN_LEFT;
+ else if (skip_prefix(valp, right,, valp))
+ align-align_type = ALIGN_RIGHT;
+ else if (skip_prefix(valp, middle,, valp))
+ align-align_type = ALIGN_MIDDLE;
+ else
+ die(_(align: improper format));
+ if (strtoul_ui(valp, 10, align-align_value))
+ die(_(align: positive value expected));
+ v-align = align;
+ v-modifier_atom = 1;
+ continue;
+ } else if (starts_with(name, end)) {
+ v-end = 1;
+ v-modifier_atom = 1;
+ continue;
} else
continue;
@@ -1251,12 +1277,48 @@ static void emit(const char *cp, const char *ep, struct
ref_formatting_state *st
static void process_formatting_state(struct atom_value *atomv, struct
ref_formatting_state *state)
{
- /* Based on the atomv values, the formatting state is set */
+ if (atomv-align) {
+ state-align = atomv-align;
+ atomv-align = NULL;
+ }
+ if (atomv-end)
+ state-end = 1;
}
static void apply_formatting_state(struct ref_formatting_state *state, struct
strbuf *final)
{
- /* More formatting options to be evetually added */
+ if (state-align state-end) {
+ struct strbuf *value = state-output;
+ int len = 0, buf_len = value-len;
+ struct align *align = state-align;
+
+ if (!value-buf)
+ return;
+
Re: [PATCH 2/4] submodule: implement `module_name` as a builtin helper
Am 05.08.2015 um 02:04 schrieb Stefan Beller:
The goal of this series being rewriting `git submodule update`,
we don't want to call out to the shell script for config lookups.
So reimplement the lookup of the submodule name in C.
Cool. This brings down the duration of the test suite from 3:13
to 3:12 for me (best of three).
You might wanna have a look into submodule.c: after initially
calling gitmodules_config() one can lookup the submodule name
in the static config_name_for_path string_list. If you'd add
a public method to submodule.c which accesses that string_list
and returns the name for the given path, you won't need your
two new functions ... or am I missing something?
Signed-off-by: Stefan Beller sbel...@google.com
---
When I started to implement git submodule add in the helper, I realized
the very first thing to be done would be module_name translated to C,
so I did that separately. Maybe we need to split this up as well into two
separate steps for processing and I/O, such that it can be reused better
from a future git submodule--helper update function
builtin/submodule--helper.c | 47 +
git-submodule.sh| 32 +++---
2 files changed, 54 insertions(+), 25 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index cb18ddf..dd5635f 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -5,6 +5,8 @@
#include pathspec.h
#include dir.h
#include utf8.h
+#include run-command.h
+#include string-list.h
static char *ps_matched;
static const struct cache_entry **ce_entries;
@@ -98,6 +100,48 @@ static int module_list(int argc, const char **argv, const
char *prefix)
return 0;
}
+
+static int collect_module_names(const char *key, const char *value, void *cb)
+{
+ size_t len;
+ struct string_list *sl = cb;
+
+ if (starts_with(key, submodule.)
+strip_suffix(key, .path, len)) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_add(sb, key + strlen(submodule.),
+ len - strlen(submodule.));
+ string_list_insert(sl, value)-util = strbuf_detach(sb, NULL);
+ strbuf_release(sb);
+ }
+
+ return 0;
+}
+
+static int module_name(int argc, const char **argv, const char *prefix)
+{
+ struct string_list_item *item;
+ struct git_config_source config_source;
+ struct string_list values = STRING_LIST_INIT_DUP;
+
+ if (!argc)
+ usage(git submodule--helper module_name path\n);
+
+ memset(config_source, 0, sizeof(config_source));
+ config_source.file = .gitmodules;
+
+ if (git_config_with_options(collect_module_names, values,
+ config_source, 1) 0)
+ die(_(unknown error occured while reading the git modules
file));
+
+ item = string_list_lookup(values, argv[0]);
+ if (item)
+ printf(%s\n, (char*)item-util);
+ else
+ die(No submodule mapping found in .gitmodules for path '%s',
argv[0]);
+ return 0;
+}
+
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
{
if (argc 2)
@@ -106,6 +150,9 @@ int cmd_submodule__helper(int argc, const char **argv,
const char *prefix)
if (!strcmp(argv[1], module_list))
return module_list(argc - 1, argv + 1, prefix);
+ if (!strcmp(argv[1], module_name))
+ return module_name(argc - 2, argv + 2, prefix);
+
usage:
usage(git submodule--helper module_list\n);
}
diff --git a/git-submodule.sh b/git-submodule.sh
index af9ecef..e6ff38d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -178,24 +178,6 @@ get_submodule_config () {
printf '%s' ${value:-$default}
}
-
-#
-# Map submodule path to submodule name
-#
-# $1 = path
-#
-module_name()
-{
- # Do we have submodule.something.path = $1 defined in .gitmodules
file?
- sm_path=$1
- re=$(printf '%s\n' $1 | sed -e 's/[].[^$\\*]/\\/g')
- name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
- sed -n -e 's|^submodule\.\(.*\)\.path '$re'$|\1|p' )
- test -z $name
- die $(eval_gettext No submodule mapping found in .gitmodules for path
'\$sm_path')
- printf '%s\n' $name
-}
-
#
# Clone a submodule
#
@@ -498,7 +480,7 @@ cmd_foreach()
then
displaypath=$(relative_path $sm_path)
say $(eval_gettext Entering '\$prefix\$displaypath')
- name=$(module_name $sm_path)
+ name=$(git submodule--helper module_name $sm_path)
(
prefix=$prefix$sm_path/
clear_local_git_env
@@ -554,7 +536,7 @@ cmd_init()
while read mode sha1 stage sm_path
do
Re: [PATCH v4 1/3] clone: do not include authentication data in guessed dir
Junio C Hamano gits...@pobox.com writes:
Perhaps add
test_clone_dir ssh://user:passw@rd@host/ host
here? How is this expected to be parsed?
For completeness, here is what I think the end result (together with
Peff's series) of the test should look like.
The first hunk is merely style. We could drop 'in $@' from there
and some people may argue that it would be more obvious, but I think
being explict is fine.
As to the second hunk:
- the first batch is for trailing slash removal for scp-like
syntax;
- the second batch is for omitting path should default to host for
the same;
- the third batch is for omitting authentication material for the
same.
Note that ssh://user:passw@rd@host:1234/ and user:passw@rd@host:/
tests fail for the same reason (finding @ should be greedy, I think).
t/t5603-clone-dirname.sh | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/t/t5603-clone-dirname.sh b/t/t5603-clone-dirname.sh
index 27dbd6c..4897ea8 100755
--- a/t/t5603-clone-dirname.sh
+++ b/t/t5603-clone-dirname.sh
@@ -22,7 +22,8 @@ test_clone_dir () {
expect=success
bare=non-bare
clone_opts=
- for i in $@; do
+ for i in $@
+ do
case $i in
fail)
expect=failure
@@ -61,12 +62,23 @@ test_clone_dir ssh://host/foo/ foo
test_clone_dir ssh://host/foo.git/ foo
test_clone_dir ssh://host/foo/.git/ foo
+test_clone_dir host:foo/ foo
+test_clone_dir host:foo.git/ foo
+test_clone_dir host:foo/.git/ foo
+
# omitting the path should default to the hostname
test_clone_dir ssh://host/ host
test_clone_dir ssh://host:1234/ host
test_clone_dir ssh://user@host/ host
+test_clone_dir host:/ host
+
+# auth materials should be redacted
test_clone_dir ssh://user:password@host/ host
test_clone_dir ssh://user:password@host:1234/ host
+test_clone_dir ssh://user:passw@rd@host:1234/ host
+test_clone_dir user@host:/ host
+test_clone_dir user:password@host:/ host
+test_clone_dir user:passw@rd@host:/ host
# trailing port-like numbers should not be stripped for paths
test_clone_dir ssh://user:password@host/test:1234 1234
--
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 v4 1/3] clone: do not include authentication data in guessed dir
Junio C Hamano gits...@pobox.com writes:
For completeness, here is what I think the end result (together with
Peff's series) of the test should look like.
...
Note that ssh://user:passw@rd@host:1234/ and user:passw@rd@host:/
tests fail for the same reason (finding @ should be greedy, I think).
And I think this should make it pass. Just remember the last
occurrence of '@' by moving the 'start' every time we see an '@'
sign.
builtin/clone.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index cae288f..5d86439 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -160,13 +160,12 @@ static char *guess_dir_name(const char *repo, int
is_bundle, int is_bare)
start += 3;
/*
-* Skip authentication data.
+* Skip authentication data, if exists.
*/
- ptr = start;
- while (ptr end !is_dir_sep(*ptr) *ptr != '@')
- ptr++;
- if (*ptr == '@')
- start = ptr + 1;
+ for (ptr = start; ptr end !is_dir_sep(*ptr); ptr++) {
+ if (*ptr == '@')
+ start = ptr + 1;
+ }
/*
* Strip trailing spaces, slashes and /.git
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] submodule: implement `module_name` as a builtin helper
On Wed, Aug 5, 2015 at 12:06 PM, Jens Lehmann jens.lehm...@web.de wrote: Am 05.08.2015 um 02:04 schrieb Stefan Beller: The goal of this series being rewriting `git submodule update`, we don't want to call out to the shell script for config lookups. So reimplement the lookup of the submodule name in C. Cool. This brings down the duration of the test suite from 3:13 to 3:12 for me (best of three). You might wanna have a look into submodule.c: after initially calling gitmodules_config() one can lookup the submodule name in the static config_name_for_path string_list. If you'd add a public method to submodule.c which accesses that string_list and returns the name for the given path, you won't need your two new functions ... or am I missing something? Yes I just realized there is already lots of submodule related code written in C, so I wanted to look at that as the next step and see what can be reused and maybe redo the patches reusing code. -- 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: Which branch(es) contain certain commits? (was Re: (unknown))
Sorry for empty subject in the original mail, somehow I've deleted it and didn't even notice. 05.08.2015, 20:05, Junio C Hamano gits...@pobox.com: Junio C Hamano gits...@pobox.com writes: I think people do things like: git log --all --decorate --grep=... s/decorate/source/; sorry for the noise. Thanks Junio! I was actually considering using --source, but for me the problem is it always returns *just one* branch for every matching commit. So the caller must then use his own knowledge to deduce all branches where this branch merged. Considering following history: * b46f30e refs/heads/zzz eee | * dc0280f refs/heads/yyy ddd |/ | * 31739da refs/heads/xxx ccc |/ * a42bd23 refs/heads/master bbb * 01a8291 refs/heads/master aaa Command git log --all --source --grep=bbb --oneline will return: a42bd23 refs/heads/master bbb While I'm expecting something like git branch --contains=a42bd23 output in terms of *all* topics being listed: master xxx yyy * zzz The most common use case is when support people need to quickly get at least rough idea which branches have specific ticket/CR mentioned. -- Ivan -- 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
Missing pages Git Book (Kindle - HTC M8)
Hi, i want to learn git. I've downloaded the book for kindle. But i've found that after page 398 follows page 405, then 412, 419, 426. So there are pages missing. Any one had a similar problem? 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 v3] remote: add get-url subcommand
Ben Boeckel maths...@gmail.com writes: Expanding `insteadOf` is a part of ls-remote --url and there is no way to expand `pushInsteadOf` as well. Add a get-url subcommand to be able to query both as well as a way to get all configured urls. Signed-off-by: Ben Boeckel maths...@gmail.com --- Documentation/git-remote.txt | 10 builtin/remote.c | 54 2 files changed, 64 insertions(+) Changes to these two files look reasonable. Don't you want to protect this feature from future breakage by others by adding a couple of tests, though, to t/t5505? 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
[bug report] Signing your work: GnuPG signing failed (gpg version 2.1)
Hello, I've tried to sign a commit following https://git-scm.com/book/tr/v2/Git-Tools-Signing-Your-Work however, this is what happened: % git commit -a -S -m signed commit testing gpg: échec de la signature : Opération annulée gpg: signing failed: Opération annulée error: gpg n'a pas pu signer les données fatal: échec de l'écriture de l'objet commit sorry for the locale, annulée means cancelled and the last two lines mean: error: gpg could not sign data fatal: failure to write the commit object However, when I tried later, nothing specific happened and the commit was done just as if I did not add the -S option: % git commit -a -S -m signed commit test [master 739cdd3] signed commit test 1 file changed, 5 deletions(-) Commit: https://github.com/hugoroy/.emacs.d/commit/b8e5b72def0c5fcc760c84d6ecd2b95b9727ae62 I notice in the git-scm.com manual that they use GnuPG version 1. I use GnuPG 2.1.6 with libgcrypt 1.6.3. Let me know if you need more information or any pointer to help me debug this more in depth. thanks, Hugo -- Hugo Roy – Free Software Foundation Europe https://fsfe.org/about/roy Please use cryptography for email: see https://emailselfdefense.fsf.org/en/ Merci d’utiliser la cryptographie pour l’email : voir https://emailselfdefense.fsf.org/fr/ signature.asc Description: PGP signature
Re: [PATCH v3 3/4] notes: add notes.merge option to select default strategy
Jacob Keller jacob.e.kel...@intel.com writes:
From: Jacob Keller jacob.kel...@gmail.com
Teach git-notes about a new configuration option notes.merge for
selecting the default notes merge strategy. Document the option in
config.txt and git-notes.txt
Add tests for use of the configuration option. Include a test to ensure
that --strategy correctly overrides the configured setting.
Signed-off-by: Jacob Keller jacob.kel...@gmail.com
Cc: Johan Herland jo...@herland.net
Cc: Michael Haggerty mhag...@alum.mit.edu
Cc: Eric Sunshine sunsh...@sunshineco.com
---
Perhaps I am biased because we do not usually use Cc: around here,
but the above looks in a somewhat strange order. Shouldn't your
sign-off be at the end?
+static enum notes_merge_strategy merge_strategy;
OK.
+static int parse_notes_strategy(const char *arg, enum notes_merge_strategy
*strategy)
+{
+ if (!strcmp(arg, manual))
+ *strategy = NOTES_MERGE_RESOLVE_MANUAL;
+ else if (!strcmp(arg, ours))
+ *strategy = NOTES_MERGE_RESOLVE_OURS;
+ else if (!strcmp(arg, theirs))
+ *strategy = NOTES_MERGE_RESOLVE_THEIRS;
+ else if (!strcmp(arg, union))
+ *strategy = NOTES_MERGE_RESOLVE_UNION;
+ else if (!strcmp(arg, cat_sort_uniq))
+ *strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
+ else
+ return -1;
+
+ return 0;
+}
OK.
static int merge(int argc, const char **argv, const char *prefix)
{
struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
@@ -795,23 +815,13 @@ static int merge(int argc, const char **argv, const
char *prefix)
expand_notes_ref(remote_ref);
o.remote_ref = remote_ref.buf;
- if (strategy) {
- if (!strcmp(strategy, manual))
- o.strategy = NOTES_MERGE_RESOLVE_MANUAL;
- else if (!strcmp(strategy, ours))
- o.strategy = NOTES_MERGE_RESOLVE_OURS;
- else if (!strcmp(strategy, theirs))
- o.strategy = NOTES_MERGE_RESOLVE_THEIRS;
- else if (!strcmp(strategy, union))
- o.strategy = NOTES_MERGE_RESOLVE_UNION;
- else if (!strcmp(strategy, cat_sort_uniq))
- o.strategy = NOTES_MERGE_RESOLVE_CAT_SORT_UNIQ;
- else {
- error(Unknown -s/--strategy: %s, strategy);
- usage_with_options(git_notes_merge_usage, options);
- }
+ if (strategy parse_notes_strategy(strategy, merge_strategy)) {
+ error(Unknown -s/--strategy: %s, strategy);
+ usage_with_options(git_notes_merge_usage, options);
}
+ o.strategy = merge_strategy;
+
This may be a minor taste thing, but initializing o.strategy with
merge_strategy at the same place as o.remote_ref is initialized,
and then passing o.merge_strategy to parse_notes_strategy(), may be
easier to follow.
Renaming the global merge_strategy to configured_merge_strategy
might make it even easier to follow. If anybody uses the variable
instead of o.strategy after this point, it would be immediately
obvious that it is either a bug or it is deliberately using the
value from the configuration file, not command line.
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 0/2] fix clone guess_dir_name regression in v2.4.8
On Wed, Aug 05, 2015 at 10:19:56AM -0700, Junio C Hamano wrote: I think this regression is in v2.4.8, as well. We should be able to use a running len instead of the end pointer in the earlier part, and then use strip_suffix_mem later (to strip from our already-reduced length, rather than the full NUL-terminated string). Like this: Looks like git clone --bare host:foo/.git is broken, too. I've added some tests to cover the recently broken cases, as well as some obvious normal cases (which the patch I sent earlier break!). And as a bonus, we can easily cover Patrick's root-repo problems (so people will actually run the tests, unlike the stuff in t1509. :) ). Sorry, my fault; I should have been much less trusting while queuing a patch like that offending one that was meant to be a no-op. I reviewed it, too. :-/ I actually did give some thought to that while working on the fix. Why did we miss what in retrospect was a pretty obvious bug? I saw two interesting bits: 1. From the diff context, it looked like a perfectly reasonable change; the shrinking of the end pointer happened further up in the function. So I guess the lesson is not to trust reading just the diff, and to really read the whole of the modified function. But that's easy to say in retrospect; most of the time the bits outside the context aren't interesting, and we can't afford to read the whole code base for each patch. It's a judgement call where to stop looking at the surrounding context of a given change (e.g., the function, the callers, their callers, etc). 2. We didn't have any test coverage in this area; when I wrote even basic tests, it caught the problem. I hate to set a rule like if you are cleaning something up, make sure there is decent test coverage. Lots of trivial-looking patches really are trivial, and it doesn't make sense to insist the submitter add a new battery of tests. So I dunno. This was definitely preventable, but that is all in retrospect. Bugs will happen, and we usually catch them while cooking. The biggest pain is that this slipped through to a release, and that may just be a measure of how few people were impacted (the cases it affected were relatively obscure). -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] submodule: implement `module_name` as a builtin helper
This implements the helper `module_name` in C instead of shell,
yielding a nice performance boost.
Before this patch, I measured a time (best out of three):
$ time ./t7400-submodule-basic.sh /dev/null
real0m11.066s
user0m3.348s
sys 0m8.534s
With this patch applied I measured (also best out of three)
$ time ./t7400-submodule-basic.sh /dev/null
real0m10.063s
user0m3.044s
sys 0m7.487s
Signed-off-by: Stefan Beller sbel...@google.com
---
Is this what you had in mind, Jens?
Jonathan pointed me to https://github.com/jlehmann/git-submod-enhancements/wiki
Does it reflect reality (i.e. as time passes code changes)?
I also noticed that you have made quite some changes to submodules on different
branches which are not upstream. Soem changes look familiar (as in I believe
this is upstream alreaday? while others look new and exciting to me).
I could not quite get the order yet, though.
builtin/submodule--helper.c | 23 +++
git-submodule.sh| 32 +++-
submodule.c | 18 +-
submodule.h | 1 +
4 files changed, 44 insertions(+), 30 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index cb18ddf..3713c4c 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -5,6 +5,8 @@
#include pathspec.h
#include dir.h
#include utf8.h
+#include submodule.h
+#include string-list.h
static char *ps_matched;
static const struct cache_entry **ce_entries;
@@ -98,6 +100,24 @@ static int module_list(int argc, const char **argv, const
char *prefix)
return 0;
}
+static int module_name(int argc, const char **argv, const char *prefix)
+{
+ const char *name;
+
+ if (argc != 1)
+ usage(git submodule--helper module_name path\n);
+
+ gitmodules_config();
+ name = submodule_name_for_path(argv[0]);
+
+ if (name)
+ printf(%s\n, name);
+ else
+ die(No submodule mapping found in .gitmodules for path '%s',
argv[0]);
+
+ return 0;
+}
+
int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
{
if (argc 2)
@@ -106,6 +126,9 @@ int cmd_submodule__helper(int argc, const char **argv,
const char *prefix)
if (!strcmp(argv[1], module_list))
return module_list(argc - 1, argv + 1, prefix);
+ if (!strcmp(argv[1], module_name))
+ return module_name(argc - 2, argv + 2, prefix);
+
usage:
usage(git submodule--helper module_list\n);
}
diff --git a/git-submodule.sh b/git-submodule.sh
index af9ecef..e6ff38d 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -178,24 +178,6 @@ get_submodule_config () {
printf '%s' ${value:-$default}
}
-
-#
-# Map submodule path to submodule name
-#
-# $1 = path
-#
-module_name()
-{
- # Do we have submodule.something.path = $1 defined in .gitmodules
file?
- sm_path=$1
- re=$(printf '%s\n' $1 | sed -e 's/[].[^$\\*]/\\/g')
- name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
- sed -n -e 's|^submodule\.\(.*\)\.path '$re'$|\1|p' )
- test -z $name
- die $(eval_gettext No submodule mapping found in .gitmodules for path
'\$sm_path')
- printf '%s\n' $name
-}
-
#
# Clone a submodule
#
@@ -498,7 +480,7 @@ cmd_foreach()
then
displaypath=$(relative_path $sm_path)
say $(eval_gettext Entering '\$prefix\$displaypath')
- name=$(module_name $sm_path)
+ name=$(git submodule--helper module_name $sm_path)
(
prefix=$prefix$sm_path/
clear_local_git_env
@@ -554,7 +536,7 @@ cmd_init()
while read mode sha1 stage sm_path
do
die_if_unmatched $mode
- name=$(module_name $sm_path) || exit
+ name=$(git submodule--helper module_name $sm_path) || exit
displaypath=$(relative_path $sm_path)
@@ -636,7 +618,7 @@ cmd_deinit()
while read mode sha1 stage sm_path
do
die_if_unmatched $mode
- name=$(module_name $sm_path) || exit
+ name=$(git submodule--helper module_name $sm_path) || exit
displaypath=$(relative_path $sm_path)
@@ -758,7 +740,7 @@ cmd_update()
echo 2 Skipping unmerged submodule $prefix$sm_path
continue
fi
- name=$(module_name $sm_path) || exit
+ name=$(git submodule--helper module_name $sm_path) || exit
url=$(git config submodule.$name.url)
branch=$(get_submodule_config $name branch master)
if ! test -z $update
@@ -1022,7 +1004,7 @@ cmd_summary() {
Re: [PATCH RFC 4/4] notes: add per-ref configuration of merge strategy
Jacob Keller jacob.e.kel...@intel.com writes:
+notes.localref.merge::
+ Which merge strategy to choose if the local ref for a notes merge
+ matches localref. Is overridden by notes.merge and takes the same
+ values. localref may be fully qualified or just under refs/notes/.
+ See NOTES MERGE STRATEGIES section in linkgit:git-notes[1] for more
+ information on each strategy.
If I have notes.refs/notes/commit.merge and notes.merge specified,
I'd expect the former overrides the latter. The second sentence may
need correcting.
I think it is a mistake to allow both notes.refs/notes/commit.merge
and notes.commit.merge. You'd end up needing to implement quite a
complicated the last one wins rule if you did so.
+notes.localref.merge::
+ Which strategy to choose when merging into localref. Uses the same
+ values as notes.merge. localref may be either a fully qualified ref
+ or the shortname under refs/notes/. See NOTES MERGE STRATEGIES
+ section above for more information about each strategy.
As a reviewer, I can tell that Uses the same values wants to say
that the set of allowed values is the same, but a casual reader is
bound to read it as notes.commit.merge must be set to the same
value as the value set to notes.merge.
diff --git a/builtin/notes.c b/builtin/notes.c
index de0caa00df1b..b0174d1024dc 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -92,6 +92,10 @@ static const char * const git_notes_get_ref_usage[] = {
static const char note_template[] =
\nWrite/edit the notes for the following object:\n;
+static struct note_ref **note_refs;
+static int note_refs_alloc;
+static int note_refs_nr;
+static struct hashmap note_refs_hash;
static enum notes_merge_strategy merge_strategy;
struct note_data {
@@ -757,12 +761,87 @@ static int parse_notes_strategy(const char *arg, enum
notes_merge_strategy *stra
return 0;
}
...
+struct note_refs_hash_key {
+ const char *str;
+ int len;
+};
+ ...
+static void set_strategy_for_ref(const char *ref)
+{
+ ...
+}
Hmmm, I do not see why you need all the complexity above.
When you come to merge(), after calling default_notes_ref(), you
know exactly which notes ref you are merging into, no? Shouldn't
then the change required for this feature just the matter of asking
the configuration system values for notes.$remote_ref.merge and
notes.merge?
IOW,
struct strbuf key = STRBUF_INIT;
char *value = NULL;
strbuf_addf(key, notes.%s.merge, remote_ref.buf);
git_config_get_string(key.buf, value) ||
git_config_get_string_const(notes.merge, value));
if (value)
parse_notes_strategy(value, configured_merge_strategy);
...
parse_options();
if (strategy)
parse_notes_strategy(value, configured_merge_strategy);
or something?
--
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 0/6] fix repo name when cloning a server's root
On Wed, Aug 05, 2015 at 10:34:34AM -0700, Junio C Hamano wrote: As you can see, there is a lot of complexity in there and I'm not convinced this is better than just exposing 'parse_connect_url()', which already handles everything for us. If the function handles everything for us, that's fine, but the primary reason I am hesitant is because parse_connect_url() was designed specifically not to have to worry about some protocols (e.g. I think feeding it a http://; would fail, and more importantly, its current callers want such a call to fail). Also it is meant to handle some non-protocols (e.g. scp style host:path that does not follow scheme://...). True, but the transport code _is_ handling that at some point. It makes me wonder if it would be possible to push the call to transport_get further up inside cmd_clone(), and then provide some way to query the remote path and hostname from the transport code. Then guess_dir_name could just go away entirely, in favor of something like: dir_name = transport_get_path(transport); if (!*dir_name) dir_name = transport_get_host(transport); That may be overly simplistic or unworkable, though. I haven't dug into the code. Also does it handle the case above? I do not think parse_connect_url() even calls get_host_and_port() to be able to tell what means in these examples. Speaking of which, has anyone tested whether the old or new code handles external remote helpers? Certainly: foo::https://host/repo.git should still use repo.git. But technically the string handed to git-remote-foo does not have to look anything like a URL. In those cases neither guess_dir_name nor the transport code have any idea what anything to the right of the :: means; we probably have to resort to blind guessing based on characters like colon and slash. -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/RFC] gitweb: Don't pass --full-history to git-log(1)
On Wed, Aug 5, 2015 at 6:54 PM, Junio C Hamano gits...@pobox.com wrote: Ævar Arnfjörð Bjarmason ava...@gmail.com writes: When you look at the history for a file via git log we don't show --full-history by default, but the Gitweb UI does so, which can be very confusing for all the reasons discussed in History Simplification in git-log(1) and in http://thread.gmane.org/gmane.comp.version-control.git/89400/focus=90659 We've been doing history via --full-history since pretty much forever, but I think this is much more usable, and on a typical project with lots of branches being merged it makes for a much less confusing view. We do this for git log by default, why wouldn't Gitweb follow suit? http://thread.gmane.org/gmane.comp.version-control.git/89400/focus=90758 seems to agree with you in principle that this would be what gitweb should do if it were written today. I'm reminded of the make(1) story about not supporting spaces instead of tabs because the guy already had a few dozen users. We could have changed this in 2008, when Git already had much fewer users, and I think we can still change it. It makes more sense as a default, especially on busy repos with lots of merges. At work where lots of merges are in flight literally 1/10 commits for any given file is relevant. Who'd be linking to gitweb's log output expecting its semantics to never change, and is use case more important than having a saner view for the vast majority of users who are just browsing around? But if there's strong objections to it a coworker who encountered this made a patch to it to add an extra full history an addition to the history view (which would change, but not the permalinks), in case there were objections to just changing 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 v3] remote: add get-url subcommand
On Wed, Aug 05, 2015 at 13:34:18 -0700, Junio C Hamano wrote: Changes to these two files look reasonable. Don't you want to protect this feature from future breakage by others by adding a couple of tests, though, to t/t5505? Thanks, I've done so locally. It actually brings up this case: $ git remote add someremote foo $ git remote get-url --push someremote fatal: no URLs configured for remote 'someremote' Is it better to use: remote = remote_get(remotename); remote-pushurl; if (remote-pushurl_nr) remote-pushurl; else remote-url; or: remote = pushremote_get(remotename); remote-pushurl; ? What is the actual difference between the two? Thanks, --Ben -- 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/10] Port branch.c to ref-filter.
Karthik Nayak karthik@gmail.com writes: There are nine patches in the series. Have put 0/10 by mistake. FYI, format-patch has --cover-letter option. -- 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] remote: add get-url subcommand
Ben Boeckel maths...@gmail.com writes:
On Wed, Aug 05, 2015 at 13:34:18 -0700, Junio C Hamano wrote:
Changes to these two files look reasonable.
Don't you want to protect this feature from future breakage by
others by adding a couple of tests, though, to t/t5505?
Thanks, I've done so locally. It actually brings up this case:
$ git remote add someremote foo
$ git remote get-url --push someremote
fatal: no URLs configured for remote 'someremote'
Is it better to use:
remote = remote_get(remotename);
remote-pushurl;
if (remote-pushurl_nr)
remote-pushurl;
else
remote-url;
or:
remote = pushremote_get(remotename);
remote-pushurl;
? What is the actual difference between the two?
You tell me ;-)
The default remote based on the current branch is computed
differently based on the direction of the transfer, I think.
struct remote *remote_get(const char *name)
{
return remote_get_1(name, remote_for_branch);
}
struct remote *pushremote_get(const char *name)
{
return remote_get_1(name, pushremote_for_branch);
}
When you are not giving name explicitly, the second parameter to _1
function is used to determine the name.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH/RFC] gitweb: Don't pass --full-history to git-log(1)
Ævar Arnfjörð Bjarmason ava...@gmail.com writes: I'm reminded of the make(1) story about not supporting spaces instead of tabs because the guy already had a few dozen users. We could have changed this in 2008, when Git already had much fewer users, Heh, in 2008 we already had more than a few dozen. I think (1) It is perfectly OK to add an UI option to let the web visitor choose between simplified and full history at runtime, optionally with a new gitweb.conf option to let the project owner choose which one is the default; (2) It is also OK to add gitweb.conf option to let the project/site choose between the two, optionally allowing the web visitor to override it with something like (1). Anything else would not give the same out-of-the-box experience and would probably not fly well. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[ANNOUNCE] Git Rev News edition 6
Dear all, I'm happy announce that the 6th edition of Git Rev News is now published: http://git.github.io/rev_news/2015/08/05/edition-6/ Big thanks to the contributors! Cheers, Thomas, Christian and Nicola -- 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] test-lib: disable trace when test is not verbose
The -x test-script option turns on the shell's -x
tracing, which can help show why a particular test is
failing. Unfortunately, this can create false negatives in
some tests if they invoke a shell function with its stderr
redirected. t5512.10 is such a test, as it does:
test_must_fail git ls-remote refs*master actual 21
test_cmp exp actual
The actual file gets the -x trace for the test_must_fail
function, which prevents it from matching the expected
output.
There's no way to avoid this without managing the
trace flag inside each sub-function, which isn't really a
workable solution. But unless you specifically care about
t5512.10, we can work around it by enabling tracing only for
the specific tests we want.
You can already do:
./t5512-ls-remote.sh -x --verbose-only=16
to see the trace only for a specific test. But that doesn't
_disable_ the tracing in the other tests; it just sends it
to /dev/null. However, there's no point in generating a
trace that the user won't see, so we can simply disable
tracing whenever it doesn't have a matching verbose flag.
The normal case of just ./t5512-ls-remote.sh -x stays the
same, as -x already implies --verbose (and
--verbose-only overrides --verbose, which is why this
works at all). And for our test, we need only check
$verbose, as maybe_setup_verbose will have already
set that flag based on the $verbose_only list).
Signed-off-by: Jeff King p...@peff.net
---
t/test-lib.sh | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 374bfcb..16c4d7b 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -531,6 +531,10 @@ maybe_setup_valgrind () {
fi
}
+want_trace () {
+ test $trace = t test $verbose = t
+}
+
# This is a separate function because some tests use
# return to end a test_expect_success block early
# (and we want to make sure we run any cleanup like
@@ -538,7 +542,7 @@ maybe_setup_valgrind () {
test_eval_inner_ () {
# Do not add anything extra (including LF) after '$*'
eval
- test \$trace\ = t set -x
+ want_trace set -x
$*
}
@@ -554,7 +558,7 @@ test_eval_ () {
{
test_eval_inner_ $@ /dev/null 3 24
test_eval_ret_=$?
- if test $trace = t
+ if want_trace
then
set +x
if test $test_eval_ret_ != 0
--
2.5.0.148.g63828c1
--
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
Error when cloning with weird local directory
Hi All, A developer at $dayjob called me over to have a look at a git error he was getting (names changed to protect the innocent). $ git --version git version 2.5.0 $ git clone ssh://example.com/repo.git Cloning into 'repo'... fatal: I don't handle protocol '/home/user/src/ssh' After a bit of head scratching we found that he had a local directory structure called 'ssh://example.com/repo.git' it wasn't a complete repo but it had some of the things one expects to find in a .git directory (info, objects, refs, etc). It had been there for a while and we suspect was created by a scp gone wrong from the last time he was dealing with repo.git. I'm wondering if it's worth catching this kind of weirdness and erroring out with a slightly more useful message. I'm also wondering what would have happened if this repo was actually a full and complete thing. I'm not sure that there is a problem worth solving here. I can provide an anonymized tarball of the directory structure in question if anyone is interested. But maybe this is useful for future mailing list searchers[1]. Thanks, Chris -- [1] - https://xkcd.com/979/ -- 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] test-lib: turn off -x tracing during chain-lint check
Now that GIT_TEST_CHAIN_LINT is on by default, running:
./t-basic.sh -x --verbose-only=1
starts with:
expecting success:
find .git/objects -type f -print should-be-empty
test_line_count = 0 should-be-empty
+ exit 117
error: last command exited with $?=117
+ find .git/objects -type f -print
+ test_line_count = 0 should-be-empty
+ test 3 != 3
+ wc -l
+ test 0 = 0
ok 1 - .git/objects should be empty after git init in an empty repo
This is confusing, as the exit 117 line and the error line
(which is printed in red, no less!) are not part of the test
at all, but are rather in the separate chain-lint test_eval.
Let's unset the trace variable when eval-ing the chain
lint check, which avoids this.
Note that we cannot just do a one-shot variable like:
trace= test_eval ...
as the behavior of one-shot variables for function calls
is not portable.
Signed-off-by: Jeff King p...@peff.net
---
t/test-lib.sh | 5 +
1 file changed, 5 insertions(+)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index cea6cda..374bfcb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -571,12 +571,17 @@ test_run_ () {
expecting_failure=$2
if test ${GIT_TEST_CHAIN_LINT:-1} != 0; then
+ # turn off tracing for this test-eval, as it simply creates
+ # confusing noise in the -x output
+ trace_tmp=$trace
+ trace=
# 117 is magic because it is unlikely to match the exit
# code of other programs
test_eval_ (exit 117) $1
if test $? != 117; then
error bug in the test script: broken -chain: $1
fi
+ trace=$trace_tmp
fi
setup_malloc_check
--
2.5.0.148.g63828c1
--
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/2] ./t5512-*.sh -x complaints
On Thu, Aug 06, 2015 at 12:55:35AM -0400, Jeff King wrote: PS I don't recall the outcome of our last discussion on the verbose test function. Here it makes debug output for the grep above more readable when it fails. But it also looks weird not to have the matching negative one for the final grep (which could be test_must_fail in this case, but we do not usually apply that to non-git commands). If you would prefer to strip out the verbose (from here and the test just below) while squashing, I am OK with that. Here's a squashable patch for that, in case it is easier (on top of the previous squash; I am happy to just send a re-rolled patch if you'd prefer): diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh index 7756100..aadaac5 100755 --- a/t/t5512-ls-remote.sh +++ b/t/t5512-ls-remote.sh @@ -150,7 +150,7 @@ do git config --add $configsection.hiderefs !refs/tags/magic git config --add $configsection.hiderefs refs/tags/magic/one git ls-remote . actual - verbose grep refs/tags/magic/two actual + grep refs/tags/magic/two actual ! grep refs/tags/magic/one actual ' @@ -160,7 +160,7 @@ test_expect_success 'overrides work between mixed transfer/upload-pack hideRefs' test_config uploadpack.hiderefs refs/tags test_config transfer.hiderefs !refs/tags/magic git ls-remote . actual - verbose grep refs/tags/magic actual + grep refs/tags/magic actual ' test_done I think the last discussion did end up with eh, now that we have -x it is simpler to just use that. Of course, then I tried to _use_ -x and found some small niggles. So here are fixes for those: [1/2]: test-lib: turn off -x tracing during chain-lint check [2/2]: test-lib: disable trace when test is not verbose -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: Error when cloning with weird local directory
On 2015-08-06 06.21, Chris Packham wrote: Hi All, A developer at $dayjob called me over to have a look at a git error he was getting (names changed to protect the innocent). $ git --version git version 2.5.0 $ git clone ssh://example.com/repo.git Cloning into 'repo'... fatal: I don't handle protocol '/home/user/src/ssh' After a bit of head scratching we found that he had a local directory structure called 'ssh://example.com/repo.git' it wasn't a complete repo but it had some of the things one expects to find in a .git directory (info, objects, refs, etc). It had been there for a while and we suspect was created by a scp gone wrong from the last time he was dealing with repo.git. I'm wondering if it's worth catching this kind of weirdness and erroring out with a slightly more useful message. I'm also wondering what would have happened if this repo was actually a full and complete thing. I'm not sure that there is a problem worth solving here. I can provide an anonymized tarball of the directory structure in question if anyone is interested. But maybe this is useful for future mailing list searchers[1]. Thanks, Chris This is indeed a bug: It looks as if static char *get_repo_path(const char *repo, int *is_bundle) in built/clone.c checks if there is a local directory structure looking like a .git directory. This is wrong. There should be a check for the scheme first. It is not the error message that is confusing, we should never get there, but invoke ssh instead. The bug is in clone.c -- 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: What's cooking in git.git
On Wed, Aug 05, 2015 at 03:55:23PM -0700, Junio C Hamano wrote: * jk/negative-hiderefs (2015-07-28) 2 commits - refs: support negative transfer.hideRefs - docs/config.txt: reorder hideRefs config Allow negative !ref entry in multi-value transfer.hideRefs configuration to say don't hide this one. An update to test coming? Thanks for reminding me. I think we just want to squash this in to the tip commit: diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh index afde495..7756100 100755 --- a/t/t5512-ls-remote.sh +++ b/t/t5512-ls-remote.sh @@ -150,7 +150,8 @@ do git config --add $configsection.hiderefs !refs/tags/magic git config --add $configsection.hiderefs refs/tags/magic/one git ls-remote . actual - verbose grep refs/tags/magic/two actual + verbose grep refs/tags/magic/two actual + ! grep refs/tags/magic/one actual ' done -Peff PS I don't recall the outcome of our last discussion on the verbose test function. Here it makes debug output for the grep above more readable when it fails. But it also looks weird not to have the matching negative one for the final grep (which could be test_must_fail in this case, but we do not usually apply that to non-git commands). If you would prefer to strip out the verbose (from here and the test just below) while squashing, I am OK with that. -- 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: What's cooking in git.git
On Wed, 2015-08-05 at 15:55 -0700, Junio C Hamano wrote: * dt/untracked-subdir (2015-08-05) 2 commits - DONTMERGE: Waiting for an Ack from Duy - untracked-cache: fix subdirectory handling (this branch uses dt/untracked-sparse.) This seems to break some tests. All tests pass for me locally. What's broken for you? -- 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: What's cooking in git.git
David Turner dtur...@twopensource.com writes: On Wed, 2015-08-05 at 15:55 -0700, Junio C Hamano wrote: * dt/untracked-subdir (2015-08-05) 2 commits - DONTMERGE: Waiting for an Ack from Duy - untracked-cache: fix subdirectory handling (this branch uses dt/untracked-sparse.) This seems to break some tests. All tests pass for me locally. What's broken for you? *** prove *** t7063-status-untracked-cache.sh .. Dubious, test returned 1 (wstat 256, 0x100) Failed 3/30 subtests Test Summary Report --- t7063-status-untracked-cache.sh (Wstat: 256 Tests: 30 Failed: 3) Failed tests: 28-30 Non-zero exit status: 1 Files=1, Tests=30, 27 wallclock secs ( 0.04 usr 0.01 sys + 0.15 cusr 0.67 csys = 0.87 CPU) Result: FAIL $ sh t7063-status-untracked-cache -i -v ends like so: ... node creation: 2 gitignore invalidation: 0 directory invalidation: 1 opendir: 3 EOF test_cmp ../trace.expect ../trace strace: invalid option -- 'k' usage: strace [-CdffhiqrtttTvVxxy] [-I n] [-e expr]... [-a column] [-o file] [-s strsize] [-P path]... -p pid... / [-D] [-E var=val]... [-u username] PROG [ARGS] or: strace -c[df] [-I n] [-e expr]... [-O overhead] [-S sortby] -p pid... / [-D] [-E var=val]... [-u username] PROG [ARGS] -c -- count time, calls, and errors for each syscall and report summary -C -- like -c but also print regular output -d -- enable debug output to stderr -D -- run tracer process as a detached grandchild, not as parent -f -- follow forks, -ff -- with output into separate files -i -- print instruction pointer at time of syscall -q -- suppress messages about attaching, detaching, etc. -r -- print relative timestamp, -t -- absolute timestamp, -tt -- with usecs -T -- print time spent in each syscall -v -- verbose mode: print unabbreviated argv, stat, termios, etc. args -x -- print non-ascii strings in hex, -xx -- print all strings in hex -y -- print paths associated with file descriptor arguments -h -- print help message, -V -- print version -a column -- alignment COLUMN for printing syscall results (default 40) -b execve -- detach on this syscall -e expr -- a qualifying expression: option=[!]all or option=[!]val1[,val2]... options: trace, abbrev, verbose, raw, signal, read, write -I interruptible -- 1: no signals are blocked 2: fatal signals are blocked while decoding syscall (default) 3: fatal signals are always blocked (default if '-o FILE PROG') 4: fatal signals and SIGTSTP (^Z) are always blocked (useful to make 'strace -o FILE PROG' not stop on ^Z) -o file -- send trace output to FILE instead of stderr -O overhead -- set overhead for tracing syscalls to OVERHEAD usecs -p pid -- trace process with process id PID, may be repeated -s strsize -- limit length of print strings to STRSIZE chars (default 32) -S sortby -- sort syscall counts by: time, calls, name, nothing (default time) -u username -- run command as username handling setuid and/or setgid -E var=val -- put var=val in the environment for command -E var -- remove var from the environment for command -P path -- trace accesses to path not ok 28 - test sparse status with untracked cache and subdir -- 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
What's cooking in git.git
This is still a draft but only to show the highlights on new topics. * bb/remote-get-url (2015-08-05) 1 commit - remote: add get-url subcommand git remote learned get-url subcommand to show the URL for a given remote name used for fetching and pushing. Expecting a reroll. * dt/untracked-subdir (2015-08-05) 2 commits - DONTMERGE: Waiting for an Ack from Duy - untracked-cache: fix subdirectory handling (this branch uses dt/untracked-sparse.) This seems to break some tests. * cb/open-noatime-clear-errno (2015-08-05) 1 commit - git_open_noatime: return with errno=0 on success Breaks build; need to tweak. * jk/guess-repo-name-regression-fix (2015-08-05) 2 commits - clone: use computed length in guess_dir_name - clone: add tests for output directory (this branch is used by ps/guess-repo-name-at-root.) git clone $URL in recent releases of Git contains a regression in the code that invents a new repository name incorrectly based on the $URL. This has been corrected. May need some tests from SQUASH??? on ps/guess-repo-name-at-root squashed into this, but otherwise looks good. * ps/guess-repo-name-at-root (2015-08-05) 4 commits - SQUASH??? - clone: abort if no dir name could be guessed - clone: do not use port number as dir name - clone: do not include authentication data in guessed dir (this branch uses jk/guess-repo-name-regression-fix.) git clone $URL, when cloning from a site whose sole purpose is to host a single repository (hence, no path after scheme://site/), tried to used the site name as the new repository name, but did not remove username or password when site part was of the form user@pass:host. The code is taught to redact these. * jk/notes-merge-config (2015-08-05) 4 commits - SQUASH??? - notes: add notes.merge option to select default strategy - notes: add tests for --commit/--abort/--strategy exclusivity - notes: document cat_sort_uniq rewriteMode git notes merge can be told with --strategy=how option how to automatically handle conflicts; this can now be configured by setting notes.merge configuration variable. The last step to add more specific notes.$ref.merge looked questionable. Waiting for reroll. * mk/submodule-gitdir-path (2015-08-05) 2 commits - path: implement common_dir handling in git_path_submodule() - submodule refactor: use git_path_submodule() in add_submodule_odb() The submodule code has been taught to work better with separate work trees created via git worktree add. * mm/pull-upload-pack (2015-07-30) 1 commit - pull.sh: quote $upload_pack when passing it to git-fetch git pull in recent releases of Git has a regression in the code that allows custom path to the --upload-pack=program. This has been corrected. Will merge to 'maint'. This should have already become irrelevant in 'master' with git pull getting rewritten in C. * ps/t1509-chroot-test-fixup (2015-08-05) 2 commits - tests: fix cleanup after tests in t1509-root-worktree - tests: fix broken chains in t1509-root-worktree t1509 test that requires a dedicated VM environment had some bitrot, which has been corrected. Will merge to 'next'. * pt/am-builtin-options (2015-08-05) 4 commits - am: let --signoff override --no-signoff - am: let command-line options override saved options - squash! test_terminal: redirect child process' stdin to a pty - test_terminal: redirect child process' stdin to a pty (this branch uses pt/am-builtin.) After git am --opt1 stops, running git am --opt2 pays attention to --opt2 only for the patch that caused the original invocation to stop. * sb/remove-get-pathspec (2015-08-03) 1 commit - builtin/mv: remove get_pathspec() Expecting a reroll ($gmane/275224). * sb/submodule-helper (2015-08-05) 1 commit - submodule: implement `module_list` as a builtin helper The beginning of git submodule rewritten in C. * tb/complete-rebase-i-edit-todo (2015-08-05) 1 commit - completion: offer '--edit-todo' during interactive rebase Comments? * jk/negative-hiderefs (2015-07-28) 2 commits - refs: support negative transfer.hideRefs - docs/config.txt: reorder hideRefs config Allow negative !ref entry in multi-value transfer.hideRefs configuration to say don't hide this one. An update to test coming? -- 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 report] Signing your work: GnuPG signing failed (gpg version 2.1)
On Wed, Aug 05, 2015 at 10:35:45PM +0200, Hugo Roy wrote: Hello, I've tried to sign a commit following https://git-scm.com/book/tr/v2/Git-Tools-Signing-Your-Work however, this is what happened: % git commit -a -S -m signed commit testing gpg: échec de la signature : Opération annulée gpg: signing failed: Opération annulée error: gpg n'a pas pu signer les données fatal: échec de l'écriture de l'objet commit sorry for the locale, annulée means cancelled and the last two lines mean: error: gpg could not sign data fatal: failure to write the commit object However, when I tried later, nothing specific happened and the commit was done just as if I did not add the -S option: % git commit -a -S -m signed commit test [master 739cdd3] signed commit test 1 file changed, 5 deletions(-) Commit: https://github.com/hugoroy/.emacs.d/commit/b8e5b72def0c5fcc760c84d6ecd2b95b9727ae62 I notice in the git-scm.com manual that they use GnuPG version 1. I use GnuPG 2.1.6 with libgcrypt 1.6.3. I believe this is a bug in GnuPG 2.1 or the relevant pinentry. Debian bug #793016[0] describes my experience with it. Killing the gpg-agent when this occurs and letting it respawn works for me. I'm very certain that this is not a bug in Git, though. GnuPG 2.1 provides the same command line interface as GnuPG 1, so it will work the same way with git. Failing to sign is a GnuPG problem, not a Git problem. [0] https://bugs.debian.org/790316 -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
