* [PATCH 14/19] reset [--mixed]: don't write index file twice
From: Martin von Zweigbergk @ 2013-01-09 8:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
When doing a mixed reset without paths, the index is locked, read,
reset, and written back as part of the actual reset operation (in
reset_index()). Then, when showing the list of worktree modifications,
we lock the index again, refresh it, and write it.
Change this so we only write the index once, making "git reset" a
little faster. It does mean that the index lock will be held a little
longer, but the difference is small compared to the time spent
refreshing the index.
There is one minor functional difference: We used to say "Could not
write new index file." if the first write failed, and "Could not
refresh index" if the second write failed. Now, we will only use the
first message.
This speeds up "git reset" a little on the linux-2.6 repo (best of
five, warm cache):
Before After
real 0m0.239s 0m0.214s
user 0m0.160s 0m0.130s
sys 0m0.070s 0m0.080s
---
builtin/reset.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 2243b95..254afa9 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -335,6 +335,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
err = reset_index(sha1, MIXED, quiet);
if (err)
die(_("Could not reset index file to revision '%s'."), rev);
+
+ if (reset_type == MIXED) /* Report what has not been updated. */
+ update_index_refresh(
+ quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
+
if (write_cache(newfd, active_cache, active_nr) ||
commit_locked_index(lock))
die(_("Could not write new index file."));
@@ -346,15 +351,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
if (reset_type == HARD && !update_ref_status && !quiet)
print_new_head_line(commit);
- else if (reset_type == MIXED) { /* Report what has not been updated. */
- struct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file));
- int fd = hold_locked_index(index_lock, 1);
- update_index_refresh(
- quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN);
- if (write_cache(fd, active_cache, active_nr) ||
- commit_locked_index(index_lock))
- error("Could not refresh index");
- }
remove_branch_state();
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* [PATCH 02/19] reset $pathspec: exit with code 0 if successful
From: Martin von Zweigbergk @ 2013-01-09 8:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Martin von Zweigbergk
In-Reply-To: <1357719376-16406-1-git-send-email-martinvonz@gmail.com>
"git reset $pathspec" currently exits with a non-zero exit code if the
worktree is dirty after resetting, which is inconsistent with reset
without pathspec, and it makes it harder to know whether the command
really failed. Change it to exit with code 0 regardless of whether the
worktree is dirty so that non-zero indicates an error.
This makes the 4 "disambiguation" test cases in t7102 clearer since
they all used to "fail", 3 of which "failed" due to changes in the
work tree. Now only the ambiguous one fails.
---
I suppose this makes documenting the exit code unnecessary, since
"return zero iff successful" is probably understood to be the default.
The variable in refresh_index() containing the value to be returned is
called has_errors. I'm guessing I shouldn't take the name too
seriously.
builtin/reset.c | 8 +++-----
t/t2013-checkout-submodule.sh | 2 +-
t/t7102-reset.sh | 18 ++++++++++++------
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/builtin/reset.c b/builtin/reset.c
index 8cc7c72..65413d0 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit)
static int update_index_refresh(int fd, struct lock_file *index_lock, int flags)
{
- int result;
-
if (!index_lock) {
index_lock = xcalloc(1, sizeof(struct lock_file));
fd = hold_locked_index(index_lock, 1);
}
- result = refresh_index(&the_index, (flags), NULL, NULL,
- _("Unstaged changes after reset:")) ? 1 : 0;
+ refresh_index(&the_index, (flags), NULL, NULL,
+ _("Unstaged changes after reset:"));
if (write_cache(fd, active_cache, active_nr) ||
commit_locked_index(index_lock))
return error ("Could not refresh index");
- return result;
+ return 0;
}
static void update_index_from_diff(struct diff_queue_struct *q,
diff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh
index 70edbb3..06b18f8 100755
--- a/t/t2013-checkout-submodule.sh
+++ b/t/t2013-checkout-submodule.sh
@@ -23,7 +23,7 @@ test_expect_success '"reset <submodule>" updates the index' '
git update-index --refresh &&
git diff-files --quiet &&
git diff-index --quiet --cached HEAD &&
- test_must_fail git reset HEAD^ submodule &&
+ git reset HEAD^ submodule &&
test_must_fail git diff-files --quiet &&
git reset submodule &&
git diff-files --quiet
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index b096dc8..81b2570 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -388,7 +388,8 @@ test_expect_success 'test --mixed <paths>' '
echo 4 > file4 &&
echo 5 > file1 &&
git add file1 file3 file4 &&
- test_must_fail git reset HEAD -- file1 file2 file3 &&
+ git reset HEAD -- file1 file2 file3 &&
+ test_must_fail git diff --quiet &&
git diff > output &&
test_cmp output expect &&
git diff --cached > output &&
@@ -402,7 +403,8 @@ test_expect_success 'test resetting the index at give paths' '
>sub/file2 &&
git update-index --add sub/file1 sub/file2 &&
T=$(git write-tree) &&
- test_must_fail git reset HEAD sub/file2 &&
+ git reset HEAD sub/file2 &&
+ test_must_fail git diff --quiet &&
U=$(git write-tree) &&
echo "$T" &&
echo "$U" &&
@@ -440,7 +442,8 @@ test_expect_success 'resetting specific path that is unmerged' '
echo "100644 $F3 3 file2"
} | git update-index --index-info &&
git ls-files -u &&
- test_must_fail git reset HEAD file2 &&
+ git reset HEAD file2 &&
+ test_must_fail git diff --quiet &&
git diff-index --exit-code --cached HEAD
'
@@ -449,7 +452,8 @@ test_expect_success 'disambiguation (1)' '
git reset --hard &&
>secondfile &&
git add secondfile &&
- test_must_fail git reset secondfile &&
+ git reset secondfile &&
+ test_must_fail git diff --quiet -- secondfile &&
test -z "$(git diff --cached --name-only)" &&
test -f secondfile &&
test ! -s secondfile
@@ -474,7 +478,8 @@ test_expect_success 'disambiguation (3)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
- test_must_fail git reset HEAD secondfile &&
+ git reset HEAD secondfile &&
+ test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
@@ -486,7 +491,8 @@ test_expect_success 'disambiguation (4)' '
>secondfile &&
git add secondfile &&
rm -f secondfile &&
- test_must_fail git reset -- secondfile &&
+ git reset -- secondfile &&
+ test_must_fail git diff --quiet &&
test -z "$(git diff --cached --name-only)" &&
test ! -f secondfile
'
--
1.8.1.rc3.331.g1ef2165
^ permalink raw reply related
* Re: [PATCH] commit: make default of "cleanup" option configurable
From: Ralf Thielow @ 2013-01-09 8:16 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, gitster
In-Reply-To: <20130109072952.GC6503@elie.Belkin>
Hi,
2013/1/9 Jonathan Nieder <jrnieder@gmail.com>:
> Hi,
>
> Ralf Thielow wrote:
>
>> 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.
>
> Could you give an example? I'm trying to get a sense of whether these
> habitual --cleanup= passers would use --cleanup=verbatim or
> --cleanup=strip.
>
It's actually my own usecase :). The bugtracker I'm using is able
to create relationships between issues and related commits. It
expects that a part of the commit message contains the issue number
in format "#<issueId>". So I need to use a cleanup mode different
from "default" to keep the commentary. The mode I'd use is "whitespace",
"verbatim" is also possible.
> I'm a bit worried that a setting like 'commit.cleanup = strip' would
> be likely to break scripts. Yes, scripts using "git commit" instead
> of commit-tree while caring about detailed semantics are asking for
> trouble, but I'm worried about importers, for example, which are
> sometimes written by new git users. Some scripts might assume that
> "git commit" preserves the entire change description from their source
> data, even when some lines happen to start by listing the ways the
> author is #1.
>
When a user uses a script/importer which expects that the "default" option
is used without setting it explicitly, and then the user changes the default,
isn't it the users fault if that would break things?
> I don't think that necessarily rules out this change, but:
>
> 1. We need more of an explanation of the purpose than "this lets
> people type less". What workflow does setting this option fit
> into?
>
> 2. I would prefer to see a little thought about whether it's possible
> to avoid silently impacting scripts. E.g., depending on the
> answer to (1), maybe supporting "verbatim" but not "strip" would be
> ok? Or alternatively, maybe a search of public code repositories
> would reveal that new git users tend to write their importers in a
> way that this doesn't break.
>
> As a side effect, the information gathered to answer (1) and (2) could
> have the potential to make the user-level documentation more useful,
> by adding context to the description of the configuration item.
>
I'll add a sentence of my bugtracker example to it. I think many developers
are using such a tool, so it'd makes sense.
> [...]
>> --- 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;
>
> Style nit: storage class comes first, then the type. Otherwise the
> typename "const char *" is split up.
>
Thanks.
I'll send a new version of the patch including changes of your and
Junios comments.
Ralf
^ permalink raw reply
* Re: [PATCH] commit: make default of "cleanup" option configurable
From: Jonathan Nieder @ 2013-01-09 7:29 UTC (permalink / raw)
To: Ralf Thielow; +Cc: git, gitster
In-Reply-To: <1357676176-30019-1-git-send-email-ralf.thielow@gmail.com>
Hi,
Ralf Thielow wrote:
> 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.
Could you give an example? I'm trying to get a sense of whether these
habitual --cleanup= passers would use --cleanup=verbatim or
--cleanup=strip.
I'm a bit worried that a setting like 'commit.cleanup = strip' would
be likely to break scripts. Yes, scripts using "git commit" instead
of commit-tree while caring about detailed semantics are asking for
trouble, but I'm worried about importers, for example, which are
sometimes written by new git users. Some scripts might assume that
"git commit" preserves the entire change description from their source
data, even when some lines happen to start by listing the ways the
author is #1.
I don't think that necessarily rules out this change, but:
1. We need more of an explanation of the purpose than "this lets
people type less". What workflow does setting this option fit
into?
2. I would prefer to see a little thought about whether it's possible
to avoid silently impacting scripts. E.g., depending on the
answer to (1), maybe supporting "verbatim" but not "strip" would be
ok? Or alternatively, maybe a search of public code repositories
would reveal that new git users tend to write their importers in a
way that this doesn't break.
As a side effect, the information gathered to answer (1) and (2) could
have the potential to make the user-level documentation more useful,
by adding context to the description of the configuration item.
[...]
> --- 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;
Style nit: storage class comes first, then the type. Otherwise the
typename "const char *" is split up.
Hope that helps,
Jonathan
^ permalink raw reply
* Re: On --depth=funny value
From: Junio C Hamano @ 2013-01-09 5:19 UTC (permalink / raw)
To: Duy Nguyen
Cc: Stefan Beller, Jonathan Nieder, schlotter, Ralf.Wildenhues, git
In-Reply-To: <CACsJy8CA-a0=HqTY9heJBhPO4M5jyLk=tf253rRKCRuTWz5teg@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> * We would like to update "clone --depth=1" to end up with a tip
>> only repository, but let's not to touch "git fetch" (and "git
>> clone") and make them send 0 over the wire when the command line
>> tells them to use "--depth=1" (i.e. let's not do the "off-by-one"
>> thing).
>
> You can't anyway. Depth 0 on the wire is considered invalid by upload-pack.
Yes, that is a good point that we say "if (0 < opt->depth) do the
shallow thing" everywhere, so 0 is spcial in that sense.
Which suggests that if we wanted to, we could update the fetch side
to do the off-by-one thing against the current upload-pack when the
given depth is two or more, and still send 1 when depth=1. When
talking with an updated upload-pack that advertises exact-shallow
protocol extension, it can disable that off-by-one for all values of
depth. That way, the updated client gets history of wrong depth
only for --depth=1 when talking with the current upload-pack; all
other cases, it will get history of correct depth.
Hmm?
^ permalink raw reply
* Re: On --depth=funny value
From: Duy Nguyen @ 2013-01-09 4:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Stefan Beller, Jonathan Nieder, schlotter, Ralf.Wildenhues, git
In-Reply-To: <7vy5g383sy.fsf_-_@alter.siamese.dyndns.org>
On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> * First, let's *not* do "git fetch --depth=inf"; if you want to
> unplug the bottom of your shallow clone, be more explicit and
> introduce a new option, e.g. "git fetch --unshallow", or
> something.
No problem. "Something" could be --no-depth or --no-shallow. I think
any of them is better than --unshallow.
> * We would like to update "clone --depth=1" to end up with a tip
> only repository, but let's not to touch "git fetch" (and "git
> clone") and make them send 0 over the wire when the command line
> tells them to use "--depth=1" (i.e. let's not do the "off-by-one"
> thing).
You can't anyway. Depth 0 on the wire is considered invalid by upload-pack.
> Instead, fix "upload-pack" (the bug is in get_shallow_commits()
> in shallow.c, I think), so that it counts correctly. When the
> other end asks for a history with 1-commit deep, it should return
> a history that is 1-commit deep, and tell the other end about the
> parents of the returned history, instead of returning a history
> that is 2 commmits deep. So when talking with a newer server,
> clients will get correct number of commits; when talkng with an
> older server, clients will get a bit more than they asked, but
> nothing will break.
I'll need to look at get_shallow_commits() anyway for the unshallow
patch. I'll probably do this too.
--
Duy
^ permalink raw reply
* On --depth=funny value
From: Junio C Hamano @ 2013-01-09 2:53 UTC (permalink / raw)
To: Duy Nguyen
Cc: Stefan Beller, Jonathan Nieder, schlotter, Ralf.Wildenhues, git
In-Reply-To: <CACsJy8BLLTWd+cTBj1jNW=ODPy7=Kg4-TPUdZ82YCE-0RQpMZA@mail.gmail.com>
Here to outline my current thinking. Note that this is unrelated to
the "git clone --bottom=v1.2.3" to say "I do not care about anything
that happened before that version".
* First, let's *not* do "git fetch --depth=inf"; if you want to
unplug the bottom of your shallow clone, be more explicit and
introduce a new option, e.g. "git fetch --unshallow", or
something.
* Make "git fetch" and "git clone" die() when zero or negative
number is given with --depth=$N, for the following reasons:
- The --depth option is describe as:
("git clone") ... a 'shallow' clone with a history
truncated to the specified number of revisions.
("git fetch") Limit fetching to ancestor-chains not longer
than n.
It is fairly clear from the above that negative $N does not
make any sense.
Technically, fetching the commits that were explicitly asked
for and not there parents is the only possible ancestor-chain
that is not longer than -4, so "git fetch --depth=-4" ought to
behave just like "git fetch --depth=0", but you have to be very
sick to read the documentation and expect things to work that
way. Also there is no way to misread the "git clone"
documentation and expect "git clone --depth=-4" to create a
history truncated to negative number of revisions.
Which means that it is the right thing to do to die() when a
negative number is given to --depth for these commands.
- As people count from one, the natural way to ask for the tip
commit without any history ought to be "--depth=1". Let's
declare the current behaviour of "--depth=1" that gives the tip
and one commit behind it a bug.
Which means that these commands should be updated to die() when
zero is given to their --depth option. "Do not give me any
history" is inherenty incompatibe with "clone" or "fetch".
Because there is no configuration variable "fetch.depth" (or
"clone.depth") that forces all your cloned repositories to be
shallow, "git clone --depth=0" or "git fetch --depth=0"
couldn't have been ways for existing users to ask to defeat any
funny configured depth value and clone or fetch everything.
When they wanted to clone or fetch everything, they would have
just used the command without any "--depth" option instead.
Which means that nobody gets hurt if we change these commands
to die() when zero is given to their --depth option.
* We would like to update "clone --depth=1" to end up with a tip
only repository, but let's not to touch "git fetch" (and "git
clone") and make them send 0 over the wire when the command line
tells them to use "--depth=1" (i.e. let's not do the "off-by-one"
thing).
Instead, fix "upload-pack" (the bug is in get_shallow_commits()
in shallow.c, I think), so that it counts correctly. When the
other end asks for a history with 1-commit deep, it should return
a history that is 1-commit deep, and tell the other end about the
parents of the returned history, instead of returning a history
that is 2 commmits deep. So when talking with a newer server,
clients will get correct number of commits; when talkng with an
older server, clients will get a bit more than they asked, but
nothing will break.
Can people sanity check the reasoning outlined here? Anything I
missed?
The above outline identifies three concrete tasks that different
people can tackle more or less independently, each with updated
code, documentation and test:
1. "git fetch --unshallow" that gives a pretty surface on Duy's
"--depth=inf";
2. Making "git fetch" and "git clone" die on "--depth=0" or
"--depth=-4";
3 Updating "upload-pack" to count correctly.
I'll refrain from saying "Any takers?" for now.
^ permalink raw reply
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
From: Greg Troxel @ 2013-01-09 2:07 UTC (permalink / raw)
To: René Scharfe; +Cc: git discussion list
In-Reply-To: <50EC8025.8000707@lsrfire.ath.cx>
[-- Attachment #1: Type: text/plain, Size: 1051 bytes --]
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'
The bug in NetBSD's sh has been fixed in -current:
http://gnats.netbsd.org/47361
and the change will almost certainly make it to the -6 and -5 release
branches.
With the fixed sh:
414c78c (with the workaround): t1402 passes
69637e5 (without the workaround): t1402 passes
With the buggy sh,
414c78c (with the workaround): t1402 passes
69637e5 (without workaround): t1402 fails
so I can confirm that the workaround is successful on NetBSD 5.
Thanks for addressing this, and sorry I didn't mention it on this list.
Greg
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [PATCH] t1402: work around shell quoting issue on NetBSD
From: Greg Troxel @ 2013-01-09 1:27 UTC (permalink / raw)
To: René Scharfe
Cc: git discussion list, Junio C Hamano, Jonathan Nieder,
Michael Haggerty
In-Reply-To: <50EC8025.8000707@lsrfire.ath.cx>
[-- Attachment #1: Type: text/plain, Size: 518 bytes --]
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> 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:
This is indeed a bug in NetBSD's shell, which I reported after finding
this test case problem, and I think someone is working on a fix. But
because git does not intend to be a shell torture test, if it's possible
to avoid bugs in a reasonable way, I think it's nice to do so.
[-- Attachment #2: Type: application/pgp-signature, Size: 194 bytes --]
^ permalink raw reply
* Re: [PATCH] add warning for depth=0 in git clone.
From: Duy Nguyen @ 2013-01-09 0:57 UTC (permalink / raw)
To: Stefan Beller
Cc: Junio C Hamano, Jonathan Nieder, schlotter, Ralf.Wildenhues, git
In-Reply-To: <1357632422-5686-1-git-send-email-stefanbeller@googlemail.com>
On Tue, Jan 8, 2013 at 3:07 PM, Stefan Beller
<stefanbeller@googlemail.com> wrote:
> ---
> builtin/clone.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/builtin/clone.c b/builtin/clone.c
> index ec2f75b..5e91c1e 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -818,6 +818,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
> remote = remote_get(option_origin);
> transport = transport_get(remote, remote->url[0]);
>
> + if (option_depth && transport->smart_options->depth < 1)
> + die(_("--depth less or equal 0 makes no sense; read manpage."));
> +
Isn't this too early for the check? The following code is
> if (!is_local) {
> if (!transport->get_refs_list || !transport->fetch)
> die(_("Don't know how to clone %s"), transport->url);
transport_set_option(transport, TRANS_OPT_KEEP, "yes");
if (option_depth)
transport_set_option(transport, TRANS_OPT_DEPTH,
option_depth);
where transport_set_option() calls set_git_option() to initialize
transport->smart_options->depth. A check should be done after this
point.
--
Duy
^ permalink raw reply
* What's cooking (interim report)
From: Junio C Hamano @ 2013-01-09 0:53 UTC (permalink / raw)
To: git
I'll do the next issue of "What's cooking" after tomorrow's
integration cycle, but here are the highlights.
The following topics that have already graduated to the 'master'
branch have been merged to the 'maint' branch (see the last "What's
cooking" for details of individual topics):
ms/subtree-fixlets
ss/nedmalloc-compilation
jc/maint-fnmatch-old-style-definition
jc/test-portability
jc/maint-fbsd-sh-ifs-workaround
jc/mkstemp-more-careful-error-reporting
jc/test-cvs-no-init-in-existing-dir
jc/maint-test-portability
In addition, the following two patches have been directly applied to
the 'maint' branch:
t1402: work around shell quoting issue on NetBSD
remote-hg: Fix biridectionality -> bidirectionality typos
We will have other bugfix topics merged to 'maint' and hopefully can
tag v1.8.1.1 sometime next week.
The following topics that have been cooking on 'next' have been
merged to the 'master' branch:
kb/maint-bundle-doc
as/test-name-alias-uniquely
ta/remove-stale-translated-tut
tb/test-t9810-no-sed-i
tb/test-t9020-no-which
jk/maint-fast-import-doc-dedup-done
jk/pathspec-literal
Most of these will later be merged to 'maint'.
These topics have been merged to the 'next' branch:
rs/zip-with-uncompressed-size-in-the-header
rs/zip-tests
jn/xml-depends-on-asciidoc-conf
jc/comment-cygwin-win32api-in-makefile
as/api-allocation-doc
jk/unify-exit-code-by-receiving-signal
rs/leave-base-name-in-name-field-of-tar
jl/interrupt-clone-remove-separate-git-dir
jc/merge-blobs
mo/cvs-server-updates
as/dir-c-cleanup
jk/config-uname
Also several new topics are parked in 'pu' and I think they are all
ready for 'next'.
Thanks.
^ 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
* 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: 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: 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: 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 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
* 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
* 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
* 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
* [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
* 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
* 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
* 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: 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
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