* Re: [PATCH 3/7] contrib/subtree: Add --unannotate
From: Junio C Hamano @ 2013-01-08 18:45 UTC (permalink / raw)
To: David A. Greene; +Cc: git, James Nylen
In-Reply-To: <1357646997-28675-4-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
> index c5bce41..75aa690 100644
> --- a/contrib/subtree/git-subtree.txt
> +++ b/contrib/subtree/git-subtree.txt
> @@ -198,6 +198,21 @@ OPTIONS FOR split
> git subtree tries to make it work anyway, particularly
> if you use --rejoin, but it may not always be effective.
>
> +--unannotate=<annotation>::
> + This option is only valid for the split command.
> +
> + When generating synthetic history, try to remove the prefix
> + <annotation> from each commit message (using bash's "strip
> + shortest match from beginning" command, which supports
> + globbing). This makes sense if you format library commits
> + like "library: Change something or other" when you're working
> + in your project's repository, but you want to remove this
> + prefix when pushing back to the library's upstream repository.
> + (In this case --unannotate='*: ' would work well.)
> +
> + Like --annotate, you need to use the same <annotation>
> + whenever you split, or you may run into problems.
I think this paragraph inherits existing breakage from the beginning
of time, but I do not think the above will format the second and
subsequent paragraphs correctly.
I've applied all seven patches in the series with minor fix-ups, and
will merge it to 'pu'.
Thanks.
^ permalink raw reply
* Re: [PATCH 4/7] contrib/subtree: Better Error Handling for add
From: Junio C Hamano @ 2013-01-08 18:45 UTC (permalink / raw)
To: David A. Greene; +Cc: git
In-Reply-To: <1357646997-28675-5-git-send-email-greened@obbligato.org>
"David A. Greene" <greened@obbligato.org> writes:
> From: "David A. Greene" <greened@obbligato.org>
>
> Check refspecs for validity before passing them on to other commands.
> This lets us generate more helpful error messages.
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
> contrib/subtree/git-subtree.sh | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index cac0680..d53eaee 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -508,12 +508,18 @@ cmd_add()
> ensure_clean
>
> if [ $# -eq 1 ]; then
> - "cmd_add_commit" "$@"
> + git rev-parse -q --verify "$1^{commit}" >/dev/null ||
> + die "'$1' does not refer to a commit"
Where do these uneven indentation come from? Is it mimicking
existing breakage in the script?
> +
> + "cmd_add_commit" "$@"
> elif [ $# -eq 2 ]; then
> - "cmd_add_repository" "$@"
> + git rev-parse -q --verify "$2^{commit}" >/dev/null ||
> + die "'$2' does not refer to a commit"
> +
> + "cmd_add_repository" "$@"
> else
> say "error: parameters were '$@'"
> - die "Provide either a refspec or a repository and refspec."
> + die "Provide either a commit or a repository and commit."
> fi
> }
^ permalink raw reply
* Re: [RFH] NetBSD 6?
From: Greg Troxel @ 2013-01-08 18:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcbew895.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]
Junio C Hamano <gitster@pobox.com> writes:
>> [OLD_ICONV]
> It refers to the type of the second parameter to iconv(); OLD_ICONV
> makes it take "const char *", as opposed to "char *", the latter of
> which matches
>
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/iconv.html
I just wanted to follow up on this. It turns out that the old POSIX
standard was buggy (header file and function spec were different), and
they resolved it in favor of non-const. NetBSD followed the const way,
and just now documented that with links to the standards email archives.
Interestingly, GNU iconv 1.14 seems to define it as const also:
https://www.gnu.org/savannah-checkouts/gnu/libiconv/documentation/libiconv-1.14/iconv.3.html
(which matches man/iconv.3 in the tarball).
When I build libiconv-1.14, it produces a .h with const. But it has a
configure test to check if there is a host include file with const, and
puts the const in the built header file or not to match!
In include/iconv.h.in, there is:
extern size_t iconv (iconv_t cd,
@ICONV_CONST@ char* * inbuf, size_t *inbytesleft,
char* * outbuf, size_t *outbytesleft);
Someday, it would be nice to have the configure test not fail an iconv
implementation just because of the const, unless the presence of const
is causing a real problem. But I can understand that no one thinks
that's important enough to get around to.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: troublesome branch name in remote repo causes .git/config inconsistency in cloned repo
From: Junio C Hamano @ 2013-01-08 18:59 UTC (permalink / raw)
To: Pavel Pospíšil; +Cc: git
In-Reply-To: <CADDfn-L_VWk5Rkn_P8aTf3pwBcbbYT=PZTrG=pFvJpNjgRg-5A@mail.gmail.com>
Pavel Pospíšil <pospispa@gmail.com> writes:
> I think that the problem may be with the branch name length. I think
> that git branch allows to created branches with very long names,
> however, such long name are not allowed in .git/config ...
I think we lifted this limit back in 1.8.0.1 timeframe.
^ permalink raw reply
* Re: [RFH] NetBSD 6?
From: Junio C Hamano @ 2013-01-08 19:03 UTC (permalink / raw)
To: Greg Troxel; +Cc: git
In-Reply-To: <rmiobgz4icr.fsf@fnord.ir.bbn.com>
Greg Troxel <gdt@ir.bbn.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>>> [OLD_ICONV]
>
>> It refers to the type of the second parameter to iconv(); OLD_ICONV
>> makes it take "const char *", as opposed to "char *", the latter of
>> which matches
>>
>> http://pubs.opengroup.org/onlinepubs/9699919799/functions/iconv.html
>
> I just wanted to follow up on this. It turns out that the old POSIX
> standard was buggy (header file and function spec were different), and
> they resolved it in favor of non-const. NetBSD followed the const way,
> and just now documented that with links to the standards email archives.
>
> Interestingly, GNU iconv 1.14 seems to define it as const also:
>
> https://www.gnu.org/savannah-checkouts/gnu/libiconv/documentation/libiconv-1.14/iconv.3.html
>
> (which matches man/iconv.3 in the tarball).
>
> When I build libiconv-1.14, it produces a .h with const. But it has a
> configure test to check if there is a host include file with const, and
> puts the const in the built header file or not to match!
> In include/iconv.h.in, there is:
>
> extern size_t iconv (iconv_t cd,
> @ICONV_CONST@ char* * inbuf, size_t *inbytesleft,
> char* * outbuf, size_t *outbytesleft);
>
> Someday, it would be nice to have the configure test not fail an iconv
> implementation just because of the const, unless the presence of const
> is causing a real problem. But I can understand that no one thinks
> that's important enough to get around to.
Interesting.
Don't get too offended by the "OLD_" prefix to that symbol, by the
way. I do not think "old" means "old and broken hence fixed in
newer version and you are low life if you live on a platform that
has to define it" ;-).
We just needed to have a boolean to tell which variant it is to let
the compiler build objects without complaining, and we named that
switch as OLD_ICONV.
^ permalink raw reply
* Re: [RFH] NetBSD 6?
From: Greg Troxel @ 2013-01-08 19:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy5g3cx9v.fsf@alter.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 841 bytes --]
Junio C Hamano <gitster@pobox.com> writes:
> Don't get too offended by the "OLD_" prefix to that symbol, by the
> way. I do not think "old" means "old and broken hence fixed in
> newer version and you are low life if you live on a platform that
> has to define it" ;-).
Thanks - it did throw me at the beginning, because I expected that it
lead to using a copy of GNU iconv and not using the native one.
But it will probably confuse few enough people that changing to
CONST_ICONV is not warranted...
> We just needed to have a boolean to tell which variant it is to let
> the compiler build objects without complaining, and we named that
> switch as OLD_ICONV.
I get that, now that I read utf8.c. It's amusing that git's own
function is const, and on non-OLD_ICONV platforms has to cast away the
const for standards-compliant iconv.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* [PATCH v3] git-fast-import(1): remove duplicate '--done' option
From: John Keeping @ 2013-01-08 19:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jonathan Nieder, Eric S. Raymond, Sverre Rabbelier, git
In-Reply-To: <20130105160652.GE6440@serenity.lan>
The '--done' option to git-fast-import is documented twice in its manual
page. Combine the best bits of each description, keeping the location
of the instance that was added first.
Signed-off-by: John Keeping <john@keeping.me.uk>
---
The commit description gained some noise in v2; this version should be
the really correct, final version.
Sorry for the noise.
Documentation/git-fast-import.txt | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index 68bca1a..4ef5721 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -39,10 +39,6 @@ OPTIONS
See ``Date Formats'' below for details about which formats
are supported, and their syntax.
--- done::
- Terminate with error if there is no 'done' command at the
- end of the stream.
-
--force::
Force updating modified existing branches, even if doing
so would cause commits to be lost (as the new commit does
@@ -108,7 +104,8 @@ OPTIONS
output.
--done::
- Require a `done` command at the end of the stream.
+ Terminate with error if there is no `done` command at the
+ end of the stream.
This option might be useful for detecting errors that
cause the frontend to terminate before it has started to
write a stream.
--
1.8.0.2
^ permalink raw reply related
* Re: [PATCH] wincred: improve compatibility with windows versions
From: Erik Faye-Lund @ 2013-01-08 20:13 UTC (permalink / raw)
To: blees; +Cc: git, msysgit, Jeff King
In-Reply-To: <50EC473A.6060203@gmail.com>
On Tue, Jan 8, 2013 at 5:20 PM, Karsten Blees <karsten.blees@gmail.com> wrote:
> Am 04.01.2013 22:57, schrieb Erik Faye-Lund:
>> The only reason why I used Cred[Un]PackAuthenticationBuffer, were that
>> I wasn't aware that it was possible any other way. I didn't even know
>> there was a Windows Credential Manager in Windows XP.
>>
>
> I believe the Cred* API was introduced in Win2k. The XP control panel applet supports domain credentials only, but cmdkey.exe from Windows Server 2003 can be used on XP to manage generic credentials.
>
Thanks for the background-info.
>> The credential attributes were because they were convenient, and I'm
>> not sure I understand what you mean about the Win7 credential manager
>> tools. I did test my code with it - in fact, it was a very useful tool
>> for debugging the helper.
>>
>> Are you referring to the credentials not *looking* like normal
>> HTTP-credentials?
>
> No, I was referring to creating / editing git credentials with Windows tools manually. For example, changing your password in control panel removes all credential attributes, so the current wincred won't find them any longer...same for git credentials created e.g. via 'cmdkey /generic:git:http://me@example.com /user:me /pass:secret'.
>
> The 'puzzling' part is that those credentials *look* exactly the same as if created by wincred, but they don't work. And wincred isn't exactly verbose in its error messages :-)
>
Right, thanks for clearing that up.
>> But, if we do any of these changes, does this mean I will lose my
>> existing credentials? It's probably not a big deal, but it's worth a
>> mention, isn't it?
>>
>
> Yes, existing stored credentials are lost after this patch. Will add a note to the commit message.
>
> We _could_ try to detect the old format, but I don't think it's worth the trouble.
>
Nah, I don't think it's worth the trouble. It's a bit unfortunate that
people might get stale credentials clogging up the system, but I don't
really thing this is a big deal.
>>> static int match_cred(const CREDENTIALW *cred)
>>> {
>>> - return (!wusername || !wcscmp(wusername, cred->UserName)) &&
>>> - match_attr(cred, L"git_protocol", protocol) &&
>>> - match_attr(cred, L"git_host", host) &&
>>> - match_attr(cred, L"git_path", path);
>>> + LPCWSTR target = cred->TargetName;
>>> + if (wusername && wcscmp(wusername, cred->UserName))
>>> + return 0;
>>> +
>>> + return match_part(&target, L"git", L":") &&
>>> + match_part(&target, protocol, L"://") &&
>>> + match_part(&target, wusername, L"@") &&
>>> + match_part(&target, host, L"/") &&
>>> + match_part(&target, path, L"");
>>> }
>>>
>>
>> Ugh, it feels a bit wrong to store and verify the username twice. Do
>> we really have to?
>>
>> The target needs to be unique, even if two different usernames are
>> stored for the same site under the same conditions. So probably. It
>> just doesn't feel quite right.
>>
>
> I don't really see why you would need several usernames to connect to the same target. I can imagine different credentials for reading / writing, but then the protocol would usually be different as well, wouldn't it? (e.g. http vs. ssh)
>
I can kind of make up some theoretical reasons, but they are a bit exotic ;)
> One of my interim solutions was to remove the username part from the URL entirely. That enabled me to change credentials in control panel (including the username), and wincred would use them. However, that version failed the 'helper can store multiple users' test, so I ended up with storing / checking username twice.
>
I don't think breaking this is a good idea. It just feels a bit silly,
but I see now that other applications does the same duplication. So
let's just stick to it, even if it's a bit icky ;)
^ permalink raw reply
* [PATCH] commit: make default of "cleanup" option configurable
From: Ralf Thielow @ 2013-01-08 20:16 UTC (permalink / raw)
To: git; +Cc: gitster, Ralf Thielow
The default of the "cleanup" option in "git commit"
is not configurable. Users who don't want to use the
default have to pass this option on every commit since
there's no way to configure it. This commit introduces
a new config option "commit.cleanup" which can be used
to change the default of the "cleanup" option in
"git commit".
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
Documentation/config.txt | 4 ++++
builtin/commit.c | 29 ++++++++++++++++++-----------
2 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 53c4ca1..3f76cd1 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -917,6 +917,10 @@ column.tag::
Specify whether to output tag listing in `git tag` in columns.
See `column.ui` for details.
+commit.cleanup::
+ This setting overrides the default of the `--cleanup` option in
+ `git commit`. See linkgit:git-commit[1] for details.
+
commit.status::
A boolean to enable/disable inclusion of status information in the
commit message template when using an editor to prepare the commit
diff --git a/builtin/commit.c b/builtin/commit.c
index d6dd3df..42acde7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -103,7 +103,7 @@ static enum {
CLEANUP_NONE,
CLEANUP_ALL
} cleanup_mode;
-static char *cleanup_arg;
+const static char *cleanup_arg;
static enum commit_whence whence;
static int use_editor = 1, include_status = 1;
@@ -966,6 +966,20 @@ static const char *read_commit_message(const char *name)
return out;
}
+static void parse_cleanup_arg()
+{
+ if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
+ cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
+ else if (!strcmp(cleanup_arg, "verbatim"))
+ cleanup_mode = CLEANUP_NONE;
+ else if (!strcmp(cleanup_arg, "whitespace"))
+ cleanup_mode = CLEANUP_SPACE;
+ else if (!strcmp(cleanup_arg, "strip"))
+ cleanup_mode = CLEANUP_ALL;
+ else
+ die(_("Invalid cleanup mode %s"), cleanup_arg);
+}
+
static int parse_and_validate_options(int argc, const char *argv[],
const struct option *options,
const char * const usage[],
@@ -1044,18 +1058,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
only_include_assumed = _("Clever... amending the last one with dirty index.");
if (argc > 0 && !also && !only)
only_include_assumed = _("Explicit paths specified without -i nor -o; assuming --only paths...");
- if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
- cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
- else if (!strcmp(cleanup_arg, "verbatim"))
- cleanup_mode = CLEANUP_NONE;
- else if (!strcmp(cleanup_arg, "whitespace"))
- cleanup_mode = CLEANUP_SPACE;
- else if (!strcmp(cleanup_arg, "strip"))
- cleanup_mode = CLEANUP_ALL;
- else
- die(_("Invalid cleanup mode %s"), cleanup_arg);
handle_untracked_files_arg(s);
+ parse_cleanup_arg();
if (all && argc > 0)
die(_("Paths with -a does not make sense."));
@@ -1320,6 +1325,8 @@ static int git_commit_config(const char *k, const char *v, void *cb)
include_status = git_config_bool(k, v);
return 0;
}
+ if (!strcmp(k, "commit.cleanup"))
+ return git_config_string(&cleanup_arg, k, v);
status = git_gpg_config(k, v, NULL);
if (status)
--
1.8.1.165.gd94bd4e.dirty
^ permalink raw reply related
* [PATCH] t1402: work around shell quoting issue on NetBSD
From: René Scharfe @ 2013-01-08 20:23 UTC (permalink / raw)
To: git discussion list; +Cc: Junio C Hamano, Jonathan Nieder, Michael Haggerty
The test fails for me on NetBSD 6.0.1 and reports:
ok 1 - ref name '' is invalid
ok 2 - ref name '/' is invalid
ok 3 - ref name '/' is invalid with options --allow-onelevel
ok 4 - ref name '/' is invalid with options --normalize
error: bug in the test script: not 2 or 3 parameters to test-expect-success
The alleged bug is in this line:
invalid_ref NOT_MINGW '/' '--allow-onelevel --normalize'
invalid_ref() constructs a test case description using its last argument,
but the shell seems to split it up into two pieces if it contains a
space. Minimal test case:
# on NetBSD with /bin/sh
$ a() { echo $#-$1-$2; }
$ t="x"; a "${t:+$t}"
1-x-
$ t="x y"; a "${t:+$t}"
2-x-y
$ t="x y"; a "${t:+x y}"
1-x y-
# and with bash
$ t="x y"; a "${t:+$t}"
1-x y-
$ t="x y"; a "${t:+x y}"
1-x y-
This may be a bug in the shell, but here's a simple workaround: Construct
the description string first and store it in a variable, and then use
that to call test_expect_success().
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
t/t1402-check-ref-format.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index 1ae4d87..1a5a5f3 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -11,7 +11,8 @@ valid_ref() {
prereq=$1
shift
esac
- test_expect_success $prereq "ref name '$1' is valid${2:+ with options $2}" "
+ desc="ref name '$1' is valid${2:+ with options $2}"
+ test_expect_success $prereq "$desc" "
git check-ref-format $2 '$1'
"
}
@@ -22,7 +23,8 @@ invalid_ref() {
prereq=$1
shift
esac
- test_expect_success $prereq "ref name '$1' is invalid${2:+ with options $2}" "
+ desc="ref name '$1' is invalid${2:+ with options $2}"
+ test_expect_success $prereq "$desc" "
test_must_fail git check-ref-format $2 '$1'
"
}
--
1.7.12
^ permalink raw reply related
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
From: Junio C Hamano @ 2013-01-08 20:39 UTC (permalink / raw)
To: René Scharfe; +Cc: git discussion list, Jonathan Nieder, Michael Haggerty
In-Reply-To: <50EC8025.8000707@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> The test fails for me on NetBSD 6.0.1 and reports:
>
> ok 1 - ref name '' is invalid
> ok 2 - ref name '/' is invalid
> ok 3 - ref name '/' is invalid with options --allow-onelevel
> ok 4 - ref name '/' is invalid with options --normalize
> error: bug in the test script: not 2 or 3 parameters to test-expect-success
>
> The alleged bug is in this line:
>
> invalid_ref NOT_MINGW '/' '--allow-onelevel --normalize'
>
> invalid_ref() constructs a test case description using its last argument,
> but the shell seems to split it up into two pieces if it contains a
> space. Minimal test case:
>
> # on NetBSD with /bin/sh
> $ a() { echo $#-$1-$2; }
> $ t="x"; a "${t:+$t}"
> 1-x-
> $ t="x y"; a "${t:+$t}"
> 2-x-y
> $ t="x y"; a "${t:+x y}"
> 1-x y-
>
> # and with bash
> $ t="x y"; a "${t:+$t}"
> 1-x y-
> $ t="x y"; a "${t:+x y}"
> 1-x y-
>
> This may be a bug in the shell, but here's a simple workaround: Construct
> the description string first and store it in a variable, and then use
> that to call test_expect_success().
Interesting. I notice that t0008 added recently to 'pu' has the
same construct.
>
> Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
> ---
> t/t1402-check-ref-format.sh | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
> index 1ae4d87..1a5a5f3 100755
> --- a/t/t1402-check-ref-format.sh
> +++ b/t/t1402-check-ref-format.sh
> @@ -11,7 +11,8 @@ valid_ref() {
> prereq=$1
> shift
> esac
> - test_expect_success $prereq "ref name '$1' is valid${2:+ with options $2}" "
> + desc="ref name '$1' is valid${2:+ with options $2}"
> + test_expect_success $prereq "$desc" "
> git check-ref-format $2 '$1'
> "
> }
> @@ -22,7 +23,8 @@ invalid_ref() {
> prereq=$1
> shift
> esac
> - test_expect_success $prereq "ref name '$1' is invalid${2:+ with options $2}" "
> + desc="ref name '$1' is invalid${2:+ with options $2}"
> + test_expect_success $prereq "$desc" "
> test_must_fail git check-ref-format $2 '$1'
> "
> }
^ permalink raw reply
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
From: René Scharfe @ 2013-01-08 21:13 UTC (permalink / raw)
To: Junio C Hamano
Cc: git discussion list, Jonathan Nieder, Michael Haggerty,
Adam Spiers
In-Reply-To: <7vr4lvcstt.fsf@alter.siamese.dyndns.org>
Am 08.01.2013 21:39, schrieb Junio C Hamano:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>> # on NetBSD with /bin/sh
>> $ a() { echo $#-$1-$2; }
>> $ t="x"; a "${t:+$t}"
>> 1-x-
>> $ t="x y"; a "${t:+$t}"
>> 2-x-y
>> $ t="x y"; a "${t:+x y}"
>> 1-x y-
>>
>> # and with bash
>> $ t="x y"; a "${t:+$t}"
>> 1-x y-
>> $ t="x y"; a "${t:+x y}"
>> 1-x y-
>>
>> This may be a bug in the shell, but here's a simple workaround: Construct
>> the description string first and store it in a variable, and then use
>> that to call test_expect_success().
>
> Interesting. I notice that t0008 added recently to 'pu' has the
> same construct.
A quick check shows that subtests 64-68 and 89-93 of t0008 fail for me
on Debian (10 in total) and subtests 64 and 89 fail on NetBSD (2 in
total). Unlike t1402 they don't report "bug in the test script".
t0008 only uses ${:+} substitution on variables that don't contain
spaces. With the test changed to store the description in a variable
first I still get the same 2 failures.
There must be something else going on here. The different results are
interesting, especially the higher number of failures on Debian.
René
^ permalink raw reply
* Re: [PATCH] commit: make default of "cleanup" option configurable
From: Junio C Hamano @ 2013-01-08 21:18 UTC (permalink / raw)
To: Ralf Thielow; +Cc: git
In-Reply-To: <1357676176-30019-1-git-send-email-ralf.thielow@gmail.com>
Ralf Thielow <ralf.thielow@gmail.com> writes:
> The default of the "cleanup" option in "git commit"
> is not configurable. Users who don't want to use the
> default have to pass this option on every commit since
> there's no way to configure it. This commit introduces
> a new config option "commit.cleanup" which can be used
> to change the default of the "cleanup" option in
> "git commit".
>
> Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
> ---
The idea makes sense, but I am not sure the implementation is
correct. Does the code already know the final value of use_editor
at the point where it calls git_commit_config? I think you do not
know at least until you call parse_and_validate_options().
Once you got the implementation right, we would want to make sure
that future changes will not break it by adding some tests that
verify:
- Without configuration and without option, the command keeps the
built-in default;
- Without configuration but with option, the command uses the
command line option (we may already have this test, I didn't
check);
- With configuration and without option, the command uses the
configured default (what this patch adds); and
- With configuration and with option, the command uses the command
line option.
The latter two probably needs a few variants, one with --edit (with
EDITOR set to something like "true"), another with --no-edit, one
without --edit nor --no-edit but with -m "$msg" to implicitly turn
use_editor off, and one without --edit, --no-edit, nor -m but with
EDITOR set to a scriptlet that writes a message to the file to
implicitly turn use_editor on.
Also, the readers of the documentation for "commit --cleanup" will
wonder the same thing you wondered before you wrote this patch;
mentioning the configuration variable in its documentation will help
them.
Thanks.
> Documentation/config.txt | 4 ++++
> builtin/commit.c | 29 ++++++++++++++++++-----------
> 2 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 53c4ca1..3f76cd1 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -917,6 +917,10 @@ column.tag::
> Specify whether to output tag listing in `git tag` in columns.
> See `column.ui` for details.
>
> +commit.cleanup::
> + This setting overrides the default of the `--cleanup` option in
> + `git commit`. See linkgit:git-commit[1] for details.
> +
> commit.status::
> A boolean to enable/disable inclusion of status information in the
> commit message template when using an editor to prepare the commit
> diff --git a/builtin/commit.c b/builtin/commit.c
> index d6dd3df..42acde7 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -103,7 +103,7 @@ static enum {
> CLEANUP_NONE,
> CLEANUP_ALL
> } cleanup_mode;
> -static char *cleanup_arg;
> +const static char *cleanup_arg;
>
> static enum commit_whence whence;
> static int use_editor = 1, include_status = 1;
> @@ -966,6 +966,20 @@ static const char *read_commit_message(const char *name)
> return out;
> }
>
> +static void parse_cleanup_arg()
> +{
> + if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
> + cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
> + else if (!strcmp(cleanup_arg, "verbatim"))
> + cleanup_mode = CLEANUP_NONE;
> + else if (!strcmp(cleanup_arg, "whitespace"))
> + cleanup_mode = CLEANUP_SPACE;
> + else if (!strcmp(cleanup_arg, "strip"))
> + cleanup_mode = CLEANUP_ALL;
> + else
> + die(_("Invalid cleanup mode %s"), cleanup_arg);
> +}
> +
> static int parse_and_validate_options(int argc, const char *argv[],
> const struct option *options,
> const char * const usage[],
> @@ -1044,18 +1058,9 @@ static int parse_and_validate_options(int argc, const char *argv[],
> only_include_assumed = _("Clever... amending the last one with dirty index.");
> if (argc > 0 && !also && !only)
> only_include_assumed = _("Explicit paths specified without -i nor -o; assuming --only paths...");
> - if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
> - cleanup_mode = use_editor ? CLEANUP_ALL : CLEANUP_SPACE;
> - else if (!strcmp(cleanup_arg, "verbatim"))
> - cleanup_mode = CLEANUP_NONE;
> - else if (!strcmp(cleanup_arg, "whitespace"))
> - cleanup_mode = CLEANUP_SPACE;
> - else if (!strcmp(cleanup_arg, "strip"))
> - cleanup_mode = CLEANUP_ALL;
> - else
> - die(_("Invalid cleanup mode %s"), cleanup_arg);
>
> handle_untracked_files_arg(s);
> + parse_cleanup_arg();
>
> if (all && argc > 0)
> die(_("Paths with -a does not make sense."));
> @@ -1320,6 +1325,8 @@ static int git_commit_config(const char *k, const char *v, void *cb)
> include_status = git_config_bool(k, v);
> return 0;
> }
> + if (!strcmp(k, "commit.cleanup"))
> + return git_config_string(&cleanup_arg, k, v);
>
> status = git_gpg_config(k, v, NULL);
> if (status)
^ permalink raw reply
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
From: Junio C Hamano @ 2013-01-08 21:37 UTC (permalink / raw)
To: René Scharfe
Cc: git discussion list, Jonathan Nieder, Michael Haggerty,
Adam Spiers
In-Reply-To: <50EC8BE7.2010508@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> A quick check shows that subtests 64-68 and 89-93 of t0008 fail for me
> on Debian (10 in total) and subtests 64 and 89 fail on NetBSD (2 in
> total). Unlike t1402 they don't report "bug in the test script".
>
> t0008 only uses ${:+} substitution on variables that don't contain
> spaces. With the test changed to store the description in a variable
> first I still get the same 2 failures.
>
> There must be something else going on here. The different results are
> interesting, especially the higher number of failures on Debian.
I forgot to mention that some of them seem to be broken under dash
but pass under bash.
^ permalink raw reply
* Enabling scissors by default?
From: Phillip Susi @ 2013-01-08 21:42 UTC (permalink / raw)
To: git
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
I was wondering why am's scissors option is not enabled by default.
It seems a very handy feature, but I'm reluctant to use it when
sending patches because the recipient has to notice the scissors and
remember to pass --scissors to git am.
Could this be made the default?
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (MingW32)
Comment: Using GnuPG with undefined - http://www.enigmail.net/
iQEcBAEBAgAGBQJQ7JLGAAoJEJrBOlT6nu75iDYIANFiiH50RlL9WKEfaoybeA5K
ZLodBze1TcAYIx2/ad6qY+XCoq98+nVXTkv2IAleDiNlfeIhKD24UTWNCysT8p1J
5KeFfR4paxLJLJKkmSL5s3DJbyjLlJWcxD7vGku6F4k35NmY3VYR4rJ/CVv0YRrs
p4nNG/EXWBo3/ngiL9QS4E65N0CfcOOjn48RQUmk1DGXSFNHP4L1KuJ4dA9cs9BC
5KmNwh5X6OOal0Lf+ezbxzvoGMwQmhBAxx3t8JQR3E22dLQlUq7stlPl5LDd+Cis
XWfNk3B4NuFTum9LqWnM5TN89WCCFh4/pskdRd5ONF51G0jbuF/hBFbwU05qL/4=
=Qd94
-----END PGP SIGNATURE-----
^ permalink raw reply
* [PATCH 11/10] doc: push.default is no longer "matching"
From: Junio C Hamano @ 2013-01-08 22:28 UTC (permalink / raw)
To: git
In-Reply-To: <1357368788-28035-1-git-send-email-gitster@pobox.com>
The documentation for the command said that `push.default` is the
default without referring to the releavant manual page.
Now `simple` is the default behaviour. Document it right there where
we say we take the default value from `push.default`, and remove the
description of old default being 'matching'.
Also reword cryptic desription of `--all`.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* I skimmed our two tutorials, just to be sure, but they do not
discuss 'git push', so there was nothing to update there.
I *think* we are minimally ready to switch the default behaviour
now, but there may be other places that need similar adjustment.
An independent audit is highly appreciated.
Documentation/git-push.txt | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 8b637d3..f326afb 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -36,10 +36,14 @@ OPTIONS[[OPTIONS]]
The format of a <refspec> parameter is an optional plus
`+`, followed by the source ref <src>, followed
by a colon `:`, followed by the destination ref <dst>.
- It is used to specify with what <src> object the <dst> ref
- in the remote repository is to be updated. If not specified,
+ It is used to specify what <src> object the <dst> ref
+ in the remote repository is to be updated to. If no
+ <refspec> is specified on the command line, and if no
+ <refspec> is configured for the <repository>,
the behavior of the command is controlled by the `push.default`
- configuration variable.
+ configuration variable, and if it is unset, the `simple`
+ behaviour is used (see lingit:git-config[1] and look
+ for `push.default`).
+
The <src> is often the name of the branch you would want to push, but
it can be any arbitrary "SHA-1 expression", such as `master~4` or
@@ -65,14 +69,11 @@ the remote repository.
The special refspec `:` (or `+:` to allow non-fast-forward updates)
directs git to push "matching" branches: for every branch that exists on
the local side, the remote side is updated if a branch of the same name
-already exists on the remote side. This is the default operation mode
-if no explicit refspec is found (that is neither on the command line
-nor in any Push line of the corresponding remotes file---see below) and
-no `push.default` configuration variable is set.
+already exists on the remote side.
--all::
- Instead of naming each ref to push, specifies that all
- refs under `refs/heads/` be pushed.
+ Push all branches (i.e. refs under `refs/heads/`); cannot be
+ used with other <refspec>.
--prune::
Remove remote branches that don't have a local counterpart. For example
--
1.8.1.317.ga1ba510
^ permalink raw reply related
* Re: Enabling scissors by default?
From: Junio C Hamano @ 2013-01-08 22:42 UTC (permalink / raw)
To: Phillip Susi; +Cc: git
In-Reply-To: <50EC92C6.7090509@ubuntu.com>
Phillip Susi <psusi@ubuntu.com> writes:
> I was wondering why am's scissors option is not enabled by default.
It is very easy to miss misidentification of scissors line; as a
dangerous, potentially information losing option, I do not think it
should be on by default.
Another reason (and this is the original one) why it is not enabled
is to discourage the contributors from overusing scissors -- >8 --
line. If you always have to write too much stuff before the proper
explanation of your patch, so that the integrator has to use -c
option all the time, you are explaining your patches wrong.
^ permalink raw reply
* trouble using cherry pick
From: Pyeron, Jason J CTR (US) @ 2013-01-08 22:43 UTC (permalink / raw)
To: git@vger.kernel.org
[-- Attachment #1.1: Type: text/plain, Size: 686 bytes --]
I am trying to accomplish the diagram below:
* ??????? Merge (or cherry-pick) all changes after 68fb8df from the task1prep branch?
|\
|* ??????? Change ...
|* ??????? Change 3
|* ??????? Change 2
|* ??????? Change 1
|* ??????? Merge branch 'task1' into task1prep
||\
|| * 5e51e26 v1.01 - fixed a typo
|| * 208296f v1.00
|| * 3393bd6 git commit -m 'start of work, created task1widget'
|| * 45dd30d Task 1 Branch (from commit 68fb8df) [orphan]
|* 68fb8df file name cleanup
|* 6f21852 redacted the sensitiveplan.txt for task1
|/ [task1prep branching]
* 9ea54ff the widget template for the widget team to use
* c0b4454 a plan was made
* aa61f73 first commit: readme.txt
Any suggestions?
[-- Attachment #1.2: primary-2013-01-08_17-35-23-pre-task1-merge-back.tgz --]
[-- Type: application/x-compressed, Size: 11544 bytes --]
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5615 bytes --]
^ permalink raw reply
* Git and Large Binaries: A Proposed Solution - current situation?
From: Christoph Buchner @ 2013-01-08 22:51 UTC (permalink / raw)
To: git
Hello git devs!
We are currently trying to deal with the large-binaries-in-git problem,
and I found this conversation from 2011 on this mailing list:
http://git.661346.n2.nabble.com/Fwd-Git-and-Large-Binaries-A-Proposed-Solution-td5948908.html
I was also motivated by finding this git GSoC 2012 proposal:
> Git copies and stores every object from a remote repository when
> cloning. For large objects, this can consume a lot of bandwidth and
> disk space, especially for older versions of large objects which are
> unlikely to be accessed. Git could learn a new alternate repository
> format where these seldom-used objects are stored on a remote server
> and only accessed on demand.
What both this proposal and the email discussion proposed (among
others), i.e. storing large binaries outside of git, and especially
fetching them from somewhere else only on demand, sounds like it would
solve our problem pretty well.
My question (I asked first in the git-devel IRC channel) is if there was
any more activity on this, and/or if this is on a roadmap or similar?
sparse clone sounds like something similar, but this has been pushed far
back, right?
I am aware of external mechanisms (e.g. git-annex, git-media), but we
would prefer something git-internal: Our userbase is heavily
cross-platform (including windows), but there's no windows support for
git-annex (which otherwise sounds like we could use it).
thank you for any input,
all the best,
Christoph
^ permalink raw reply
* Re: [PATCH 2/7] contrib/subtree: Use %B for Split Subject/Body
From: 郑文辉(Techlive Zheng) @ 2013-01-08 23:21 UTC (permalink / raw)
To: David A. Greene; +Cc: git
In-Reply-To: <1357646997-28675-3-git-send-email-greened@obbligato.org>
2013/1/8 David A. Greene <greened@obbligato.org>:
> From: Techlive Zheng <techlivezheng@gmail.com>
>
> Use %B to format the commit message and body to avoid an extra newline
> if a commit only has a subject line.
>
> Signed-off-by: Techlive Zheng <techlivezheng@gmail.com>
>
> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
> contrib/subtree/git-subtree.sh | 6 +++++-
> contrib/subtree/t/t7900-subtree.sh | 15 +++++++++++++++
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
> index 920c664..5341b36 100755
> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -296,7 +296,11 @@ copy_commit()
> # We're going to set some environment vars here, so
> # do it in a subshell to get rid of them safely later
> debug copy_commit "{$1}" "{$2}" "{$3}"
> - git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%s%n%n%b' "$1" |
> + # Use %B rather than %s%n%n%b to handle the special case of a
> + # commit that only has a subject line. We don't want to
> + # introduce a newline after the subject, causing generation of
> + # a new hash.
> + git log -1 --pretty=format:'%an%n%ae%n%ad%n%cn%n%ce%n%cd%n%B' "$1" |
> (
> read GIT_AUTHOR_NAME
> read GIT_AUTHOR_EMAIL
> diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
> index 6cf9fb9..3f17f55 100755
> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -74,6 +74,10 @@ test_expect_success 'add sub1' '
> git branch -m master subproj
> '
>
> +# Save this hash for testing later.
> +
> +subdir_hash=`git rev-parse HEAD`
> +
> test_expect_success 'add sub2' '
> create sub2 &&
> git commit -m "sub2" &&
> @@ -211,6 +215,17 @@ test_expect_success 'check split with --branch' '
> check_equal ''"$(git rev-parse splitbr1)"'' "$spl1"
> '
>
> +test_expect_success 'check hash of split' '
> + spl1=$(git subtree split --prefix subdir) &&
> + undo &&
> + git subtree split --prefix subdir --branch splitbr1test &&
> + check_equal ''"$(git rev-parse splitbr1test)"'' "$spl1"
> + git checkout splitbr1test &&
> + new_hash=$(git rev-parse HEAD~2) &&
> + git checkout mainline &&
> + check_equal ''"$new_hash"'' "$subdir_hash"
> +'
> +
This test is not test the correct thing, I am currently working on it.
> test_expect_success 'check split with --branch for an existing branch' '
> spl1=''"$(git subtree split --annotate='"'*'"' --prefix subdir --onto FETCH_HEAD --message "Split & rejoin" --rejoin)"'' &&
> undo &&
> --
> 1.7.10.4
>
^ permalink raw reply
* Re: Enabling scissors by default?
From: Phillip Susi @ 2013-01-08 23:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vvcb7b8lc.fsf@alter.siamese.dyndns.org>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 01/08/2013 05:42 PM, Junio C Hamano wrote:
> It is very easy to miss misidentification of scissors line; as a
> dangerous, potentially information losing option, I do not think
> it should be on by default.
I suppose if it only requires one instance of >8 or <8 and one -, it
might be *slightly* dangerous, but if it required a slightly longer
minimum line length, it would be pretty darn unlikely to get triggered
by accident, and of course, is easily disabled.
> Another reason (and this is the original one) why it is not
> enabled is to discourage the contributors from overusing scissors
> -- >8 -- line. If you always have to write too much stuff before
> the proper explanation of your patch, so that the integrator has to
> use -c option all the time, you are explaining your patches wrong.
I often see patches being tweaked in response to feedback and
resubmitted, usually with a description of what has changed since the
previous version. Such descriptions don't need to be in the change
log when it is finally applied and seem a perfect use of scissors.
Usually such version to version descriptions are put in a cover
letter, but if you are only submitting a single patch instead of an
entire series, using a cover letter seems silly when you could just
put the comments in one email and clearly mark them as not needing to
go into the final changelog.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/
iQEcBAEBAgAGBQJQ7KriAAoJEJrBOlT6nu755UkIALIT3T5yHH5i+0HOrXLlXzQR
+S2jJfFZ8Kcc+kleiEJ3uLFVGTLMpRyjJFKceOuB4/TdJFUivrYJHWJxcKmW8WzK
BJKZOjt/jv9r8Qt/AB7KA45S7awfQnOWkg6KQlJa1IM0nUPbo4upgMlWar9l7vjz
Hkr7geuHY4fsVUJ7R0rYPcT3pue8ywsT4a9o/ocstfXmC05IrLKQtzO4TuvfiaTb
yBG+rAPKz36zfxCN5NyKExZO6v/LnCKym/PH4a6wYIeTUz1EvuaPy5lQOo6ORQ4h
xbSyBRDPN4yiVgNXfSQmGKwd9XPqs6h8Z0q3X5mGZyOXurw0JFRJlJ3v8hHIvqg=
=Rn7z
-----END PGP SIGNATURE-----
^ permalink raw reply
* Re: [PATCH] clone: forbid --bare --separate-git-dir <dir>
From: Duy Nguyen @ 2013-01-08 23:34 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jens Lehmann, Jonathan Nieder, git, Heiko Voigt, Manlio Perillo,
W. Trevor King
In-Reply-To: <7v4nirfu1p.fsf@alter.siamese.dyndns.org>
On Wed, Jan 9, 2013 at 12:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Jens Lehmann <Jens.Lehmann@web.de> writes:
>
>> Am 08.01.2013 15:16, schrieb Duy Nguyen:
>>> On Sun, Jan 06, 2013 at 02:19:48AM -0800, Jonathan Nieder wrote:
>>>> Unfortunately we forgot to forbid the --bare
>>>> --separate-git-dir combination. In practice, we know no one
>>>> could be using --bare with --separate-git-dir because it is
>>>> broken in the following way: <explanation here>. So it is
>>>> safe to make good on our mistake and forbid the combination,
>>>> making the command easier to explain.
>>>>
>>>> I don't know what would go in the <explanation here> blank above,
>>>> though. Is it possible that some people are relying on this option
>>>> combination?
>>>
>>> I can't say it's broken in what way. Maybe it's easier to just support
>>> this case, it's not much work anyway. Jens, maybe squash this to your
>>> original patch?
>>
>> I'd be fine with that, though my gut feeling is that this should
>> be a patch of its own (My patch handles the git dir, your's handles
>> a work tree issue).
>
> I agree that these are totally unrelated issues.
>
> After all, Jonathan's suggestion to forbid it was because the
> combination does not make sense and does not have practical uses,
> and forbidding it would make the command easier to explain than
> leaving it accepted from the command line. If you choose to go in
> the opposite direction and make "clone --bare --separate-git-dir" do
> something useful, it should be explained very well in the
> documentation part of the patch why such a combination is a good
> idea, and in what situation the behaviour is useful and the user may
> want to consider using it, I think.
It is more like postponing the usefulness evaluation of the
combination until later (maybe someone will come up with an actual use
case). As of now, --separate-git-dir --bare is a valid combination.
Jens' patch fixes one case but leave the other case broken, which is
why I think it should be in one patch. It's rather ducking head in the
sand than actually declaring that the combination is useful.
--
Duy
^ permalink raw reply
* Re: Enabling scissors by default?
From: Junio C Hamano @ 2013-01-08 23:36 UTC (permalink / raw)
To: Phillip Susi; +Cc: git
In-Reply-To: <50ECAAE2.2020507@ubuntu.com>
Phillip Susi <psusi@ubuntu.com> writes:
> On 01/08/2013 05:42 PM, Junio C Hamano wrote:
>> It is very easy to miss misidentification of scissors line; as a
>> dangerous, potentially information losing option, I do not think
>> it should be on by default.
>
> I suppose if it only requires one instance of >8 or <8 and one -, it
> might be *slightly* dangerous, but if it required a slightly longer
> minimum line length, it would be pretty darn unlikely to get triggered
> by accident, and of course, is easily disabled.
"Easily disabled" is never a good enough reason to change the long
established default of not doing anything funky unless the user
explicitly asks it to do things differently.
You could introduce a new configuration variable "am.scissors" and
personally turn it on, though. Setting that variable *does* count
as the user explicitly asking for it.
> I often see patches being tweaked in response to feedback and
> resubmitted, usually with a description of what has changed since the
> previous version. Such descriptions don't need to be in the change
> log when it is finally applied and seem a perfect use of scissors.
Putting such small logs under "---" line is the accepted practice.
^ permalink raw reply
* Re: [PATCH] clone: forbid --bare --separate-git-dir <dir>
From: Junio C Hamano @ 2013-01-08 23:42 UTC (permalink / raw)
To: Duy Nguyen
Cc: Jens Lehmann, Jonathan Nieder, git, Heiko Voigt, Manlio Perillo,
W. Trevor King
In-Reply-To: <CACsJy8B=h04QAeb0D-PWvT=0n_+QfW27NuUg3KEFUN3C4MOJVQ@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
>> After all, Jonathan's suggestion to forbid it was because the
>> combination does not make sense and does not have practical uses,
>> and forbidding it would make the command easier to explain than
>> leaving it accepted from the command line. If you choose to go in
>> the opposite direction and make "clone --bare --separate-git-dir" do
>> something useful, it should be explained very well in the
>> documentation part of the patch why such a combination is a good
>> idea, and in what situation the behaviour is useful and the user may
>> want to consider using it, I think.
>
> It is more like postponing the usefulness evaluation of the
> combination until later (maybe someone will come up with an actual use
> case). As of now, --separate-git-dir --bare is a valid combination.
> Jens' patch fixes one case but leave the other case broken, which is
> why I think it should be in one patch. It's rather ducking head in the
> sand than actually declaring that the combination is useful.
When a user comes and asks how "git clone --bare --separate-git-dir"
is meant to be used, you are saying that your answer will be "Eh, it
does something random that I cannot explain, and I cannot even
suggest a good use case for it, but somebody may find it useful."?
If we get rid of it, we do not have to explain what such a useless
combination would/should do, no?
^ permalink raw reply
* Re: [PATCH 2/7] contrib/subtree: Use %B for Split Subject/Body
From: Junio C Hamano @ 2013-01-09 0:41 UTC (permalink / raw)
To: 郑文辉(Techlive Zheng); +Cc: David A. Greene, git
In-Reply-To: <CAPYzjrQ1ngfOwBuzq+Da1Ynd18Vwt8=LCyu2yhE6dX8vivwReg@mail.gmail.com>
"郑文辉(Techlive Zheng)" <techlivezheng@gmail.com> writes:
>> +test_expect_success 'check hash of split' '
>> + spl1=$(git subtree split --prefix subdir) &&
>> + undo &&
>> + git subtree split --prefix subdir --branch splitbr1test &&
>> + check_equal ''"$(git rev-parse splitbr1test)"'' "$spl1"
>> + git checkout splitbr1test &&
>> + new_hash=$(git rev-parse HEAD~2) &&
>> + git checkout mainline &&
>> + check_equal ''"$new_hash"'' "$subdir_hash"
>> +'
>> +
> This test is not test the correct thing, I am currently working on it.
Will keep the topic branch out of 'next' for now.
David, how would you like to handle a reroll of this piece?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox