* Re: [PATCH] t: don't redefine test_config() in various places
From: Ramkumar Ramachandra @ 2013-03-20 19:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vsj3pewgl.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> test_config() is already a well-defined function in
>> test-lib-functions.sh. Don't duplicate it unnecessarily in:
>>
>> t4018-diff-funcname.sh
>> t7810-grep.sh
>> t7811-grep-open.sh
>>
>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
>> ---
>> Thanks, Junio.
>
> Heh, I've pushed out the original one after fixing them up myself
> last night.
Doing a fine job as the editor, as always. I need to watch `pu` more closely.
^ permalink raw reply
* Re: [PATCH] t3200 (branch): modernize style
From: Ramkumar Ramachandra @ 2013-03-20 19:57 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Git List
In-Reply-To: <CAPig+cSAzCM893kunUO=FM5Qh0-+5YON1KfbG4rUiDjZ8CvV4w@mail.gmail.com>
Eric Sunshine wrote:
> On Wed, Mar 20, 2013 at 8:30 AM, Ramkumar Ramachandra
> <artagnon@gmail.com> wrote:
>> Style is inconsistent throughout the file. Make the following
>> changes:
>>
>> 1. Indent everything with tabs.
>>
>> 2. Put the opening quote (') for the test in the same line as
>> test_expect_succcess, and the closing quote on a line by itself.
>
> s/succcess/success/
Thanks, much appreciated. I really need to set up some Flyspell hooks.
^ permalink raw reply
* Re: FW: Windows. Git, and Dedupe
From: René Scharfe @ 2013-03-20 19:54 UTC (permalink / raw)
To: Josh Rowe; +Cc: git@vger.kernel.org, msysgit@googlegroups.com
In-Reply-To: <614e174a0b4548a1bcdc6c24c2682829@BLUPR03MB019.namprd03.prod.outlook.com>
Am 19.03.2013 22:36, schrieb Josh Rowe:
> Yes, Dedup is in fact a Server-only feature.
Is there an easier way to reproduce the issue than registering and
downloading the Windows Server 2012 evaluation version? It's not that
hard, admittedly, but still.
> The reparse point could be decoded as being a non-symlink reparse
> itemusing; in those cases, treating the file as an "ordinary"
> file would be appropriate.
>
> For example, see the following. The reparse tag value for symlinks
> isIO_REPARSE_TAG_SYMLINK (0xa000000c) and for deduped files is
> (IO_REPARSE_TAG_DEDUP) 0x80000013.
That's interesting and invalidates my initial checks with mklink,
because if I read compat/mingw.c [1] correctly then git handles symlinks
on Windows in a special way, but should treat dedup reparse points as
normal files already.
Hrm, but probably st_size is set to zero for them. Do the deduped files
appear as empty? "git ls-tree -r HEAD" would show them with a hash of
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391. If true then how do we get
their real content sizes using Win32 API calls?
By the way, what does the command "git version" return for you?
Thanks,
René
[1] https://git.kernel.org/cgit/git/git.git/tree/compat/mingw.c#n427
--
--
*** Please reply-to-all at all times ***
*** (do not pretend to know who is subscribed and who is not) ***
*** Please avoid top-posting. ***
The msysGit Wiki is here: https://github.com/msysgit/msysgit/wiki - Github accounts are free.
You received this message because you are subscribed to the Google
Groups "msysGit" group.
To post to this group, send email to msysgit@googlegroups.com
To unsubscribe from this group, send email to
msysgit+unsubscribe@googlegroups.com
For more options, and view previous threads, visit this group at
http://groups.google.com/group/msysgit?hl=en_US?hl=en
---
You received this message because you are subscribed to the Google Groups "msysGit" group.
To unsubscribe from this group and stop receiving emails from it, send an email to msysgit+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.
^ permalink raw reply
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
From: Junio C Hamano @ 2013-03-20 19:52 UTC (permalink / raw)
To: Jeff King; +Cc: Jonathan Nieder, Ramkumar Ramachandra, Git List, Eric Sunshine
In-Reply-To: <20130320185844.GB30165@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... even though it is more typing, I would argue that:
>
> mk_empty testrepo &&
> git push testrepo ...
>
> is better, because the test script is more readable as a unit.
>
> None of this is that huge a deal to me (and yet I seem to have written a
> page about it :) ), but I figure while we are bikeshedding about test
> style...
;-) But the above is a good point.
^ permalink raw reply
* Re: [PATCH] t3200 (branch): modernize style
From: Eric Sunshine @ 2013-03-20 19:50 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1363782612-19260-1-git-send-email-artagnon@gmail.com>
On Wed, Mar 20, 2013 at 8:30 AM, Ramkumar Ramachandra
<artagnon@gmail.com> wrote:
> Style is inconsistent throughout the file. Make the following
> changes:
>
> 1. Indent everything with tabs.
>
> 2. Put the opening quote (') for the test in the same line as
> test_expect_succcess, and the closing quote on a line by itself.
s/succcess/success/
^ permalink raw reply
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
From: Junio C Hamano @ 2013-03-20 19:48 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <CALkWK0=29fu92A8_XQCmW86U2y-ktNAOy4qMmJAhyYHo8FgX1Q@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> ... There is no implicit fallback (like
> "origin"): it just falls back to the .remote codepath, if not
> explicitly specified.
That one sentence is enough to explain the apparent asymmetry, which
bothered me.
^ permalink raw reply
* Re: [PATCH 5/6] remote.c: introduce remote.pushdefault
From: Ramkumar Ramachandra @ 2013-03-20 19:46 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Git List, Junio C Hamano, Jeff King, Eric Sunshine
In-Reply-To: <20130320183255.GL3655@google.com>
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> This new configuration variable defines the default remote to push to,
>> and overrides `branch.<name>.remote` for all branches.
>
> Micronit: I think this would be easier to explain if it came after
> patch 6, since then you could say "In other words, it is a default for
> branch.<name>.pushremote for all branches" and readers would not have
> to wonder "Why does the more generic configuration override the more
> specific one?".
I'm sorry, but this is not worth a rebase.
^ permalink raw reply
* Re: [PATCH v2 14/45] Guard against new pathspec magic in pathspec matching code
From: Eric Sunshine @ 2013-03-20 19:45 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: Git List, Junio C Hamano
In-Reply-To: <1363781779-14947-5-git-send-email-pclouds@gmail.com>
On Wed, Mar 20, 2013 at 8:16 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> diff --git a/Documentation/technical/api-setup.txt b/Documentation/technical/api-setup.txt
> index 59a947e..f62528e 100644
> --- a/Documentation/technical/api-setup.txt
> +++ b/Documentation/technical/api-setup.txt
> @@ -28,3 +28,22 @@ parse_pathspec(). This function takes several arguments:
> - prefix and args come from cmd_* functions
>
> get_pathspec() is obsolete and should never be used in new code.
> +
> +parse_pathspec() helps catch unsupported features and reject it
s/reject it/reject them/
> +politely. At a lower level, different pathspec-related functions may
> +not support the same set of features. Such pathspec-sensitive
> +functions are guarded with GUARD_PATHSPEC(), which will die in an
> +unfriendly way when an unsupported feature is requested.
^ permalink raw reply
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
From: Ramkumar Ramachandra @ 2013-03-20 19:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <7vfvzpevwf.fsf@alter.siamese.dyndns.org>
Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> if (name)
>> name_given = 1;
>> else {
>> - name = default_remote_name;
>> - name_given = explicit_default_remote_name;
>> + if (pushremote_name) {
>> + name = pushremote_name;
>> + name_given = 1;
>> + } else {
>> + name = default_remote_name;
>> + name_given = explicit_default_remote_name;
>> + }
>> }
>
> The code to read branch.$name.remote configuration flips
> explicit_default_remote_name to one when it is used to set the
> default_remote_name, and that controls the value of name_given in
> this codepath. At this point in the series, you do not have a
> corresponding branch.$name.pushremote, but your [6/6] does not seem
> to do the same.
>
> Why isn't it necessary to add explicit_default_pushremote_name and
> do the same here in patch [6/6]?
Sorry, I'm still trying to understand your comment. Okay, yes:
branch.$name.remote does flip explicit_default_remote_name, because we
need to know if the default remote name was explicitly given. Wait,
how is explicit_default_remote_name used to set default_remote_name?
Don't you mean name_given? It controls name_give, yes. At this point
I don't have .pushremote, yes: I'm setting up for [5/6] and [6/6]. My
[6/6] doesn't seem to do the "same"? The same thing as .remote? Are
you asking why .pushremote doesn't flip explicit_default_remote_name
like .remote does? Because .pushremote can only ever be specified
explicitly: otherwise, it falls back to the .remote logic.
Okay, next paragraph. Why isn't it necessary to add
explicit_default_pushremote_name? Like I said, .pushremote can only
ever be specified explicitly. There is no implicit fallback (like
"origin"): it just falls back to the .remote codepath, if not
explicitly specified. In other words, it's just a small override on
the .remote codepath.
^ permalink raw reply
* Re: [PATCH v2 06/45] Add parse_pathspec() that converts cmdline args to struct pathspec
From: Eric Sunshine @ 2013-03-20 19:40 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: Git List, Junio C Hamano
In-Reply-To: <1363781779-14947-2-git-send-email-pclouds@gmail.com>
On Wed, Mar 20, 2013 at 8:16 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> +static void NORETURN unsupported_magic(const char *pattern,
> + unsigned magic,
> + unsigned short_magic)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + int i, n;
> + for (n = i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
> + const struct pathspec_magic *m = pathspec_magic + i;
> + if (!(magic & m->bit))
> + continue;
> + if (sb.len)
> + strbuf_addstr(&sb, " ");
> + if (short_magic & m->bit)
> + strbuf_addf(&sb, "'%c'", m->mnemonic);
> + else
> + strbuf_addf(&sb, "'%s'", m->name);
> + n++;
> + }
> + /*
> + * We may want to substitue "this command" with a command
s/substitue/substitute/
> + * name. E.g. when add--interactive dies when running
> + * "checkout -p"
> + */
> + die(_("%s: pathspec magic not supported by this command: %s"),
> + pattern, sb.buf);
> +}
^ permalink raw reply
* Re: [PATCH v2] index-pack: always zero-initialize object_entry list
From: Eric Sunshine @ 2013-03-20 19:14 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, Junio C Hamano, Stefan Zager, Git List,
Nguyễn Thái Ngọc
In-Reply-To: <20130320191327.GA31383@sigill.intra.peff.net>
On Wed, Mar 20, 2013 at 3:13 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Mar 20, 2013 at 03:12:07PM -0400, Eric Sunshine wrote:
>
>> On Tue, Mar 19, 2013 at 12:17 PM, Jeff King <peff@peff.net> wrote:
>> > To ensure that all depths start at 0, that commit changed
>> > calls to xmalloc the object_entry list into calls to
>> > xcalloc. However, it forgot that we grow the list with
>> > xrealloc later. These extra entries are used when we add an
>> > object from elsewhere pack to complete a thin pack. If we
>>
>> s/elsewhere pack/pack/
>
> I think it is supposed to be s/elsewhere pack/elsewhere/.
Sorry, yes.
-- ES
^ permalink raw reply
* Re: [PATCH v2] index-pack: always zero-initialize object_entry list
From: Jeff King @ 2013-03-20 19:13 UTC (permalink / raw)
To: Eric Sunshine
Cc: Thomas Rast, Junio C Hamano, Stefan Zager, Git List,
Nguyễn Thái Ngọc
In-Reply-To: <CAPig+cQobu8GoqSNjVw8498e8D3vEJKU+UVUqkYbwypLyPTNhQ@mail.gmail.com>
On Wed, Mar 20, 2013 at 03:12:07PM -0400, Eric Sunshine wrote:
> On Tue, Mar 19, 2013 at 12:17 PM, Jeff King <peff@peff.net> wrote:
> > To ensure that all depths start at 0, that commit changed
> > calls to xmalloc the object_entry list into calls to
> > xcalloc. However, it forgot that we grow the list with
> > xrealloc later. These extra entries are used when we add an
> > object from elsewhere pack to complete a thin pack. If we
>
> s/elsewhere pack/pack/
I think it is supposed to be s/elsewhere pack/elsewhere/.
Thanks for noticing.
-Peff
^ permalink raw reply
* Re: [PATCH v2] index-pack: always zero-initialize object_entry list
From: Eric Sunshine @ 2013-03-20 19:12 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, Junio C Hamano, Stefan Zager, Git List,
Nguyễn Thái Ngọc
In-Reply-To: <20130319161722.GA17445@sigill.intra.peff.net>
On Tue, Mar 19, 2013 at 12:17 PM, Jeff King <peff@peff.net> wrote:
> To ensure that all depths start at 0, that commit changed
> calls to xmalloc the object_entry list into calls to
> xcalloc. However, it forgot that we grow the list with
> xrealloc later. These extra entries are used when we add an
> object from elsewhere pack to complete a thin pack. If we
s/elsewhere pack/pack/
> add a non-delta object, its depth value will just be
> uninitialized heap data.
^ permalink raw reply
* Re: [PATCH] Documentation: merging a tag is a special case
From: Junio C Hamano @ 2013-03-20 19:07 UTC (permalink / raw)
To: Yann Droneaud; +Cc: Git
In-Reply-To: <1363802033-26868-1-git-send-email-ydroneaud@opteya.com>
Yann Droneaud <ydroneaud@opteya.com> writes:
> When asking Git to merge a tag (such as a signed tag or annotated tag),
> it will always create a merge commit even if fast-forward was possible.
> It's like having --no-ff present on the command line.
>
> It's a difference from the default behavior described in git-merge.txt.
> It should be documented as an exception of "FAST-FORWARD MERGE" section
> and "--ff" option description.
>
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> ---
> Documentation/git-merge.txt | 9 +++++++++
> Documentation/merge-options.txt | 2 +-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt
> index c852a26..84bc873 100644
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -170,6 +170,15 @@ happens:
> If you tried a merge which resulted in complex conflicts and
> want to start over, you can recover with `git merge --abort`.
>
> +MERGING TAG
> +-----------
> +
> +When merging a tag (annotated or signed), Git will create a merge commit
> +even if a fast-forward merge is possible (see above).
> +The commit message template will be created from the tag message.
> +Additionally, the signature check will be reported as a comment
> +if the tag was signed. See also linkgit:git-tag[1].
> +
It would make it more helpful to readers to describe how _not_ to
create such a merge commit if it is unwanted, and how the request to
merge a tag interacts with --ff-only option.
> HOW CONFLICTS ARE PRESENTED
> ---------------------------
>
> diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
> index 0bcbe0a..70d1ec0 100644
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -26,7 +26,7 @@ set to `no` at the beginning of them.
> --ff::
> When the merge resolves as a fast-forward, only update the branch
> pointer, without creating a merge commit. This is the default
> - behavior.
> + behavior (except when merging a tag).
With this update, the reader will be left wondering what would be
the default when she asks Git to merge a tag, no?
> --no-ff::
> Create a merge commit even when the merge resolves as a
^ permalink raw reply
* Re: [PATCH 4/6] remote.c: introduce a way to have different remotes for fetch/push
From: Junio C Hamano @ 2013-03-20 19:03 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Jeff King, Eric Sunshine, Jonathan Nieder
In-Reply-To: <1363783501-27981-5-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> if (name)
> name_given = 1;
> else {
> - name = default_remote_name;
> - name_given = explicit_default_remote_name;
> + if (pushremote_name) {
> + name = pushremote_name;
> + name_given = 1;
> + } else {
> + name = default_remote_name;
> + name_given = explicit_default_remote_name;
> + }
> }
The code to read branch.$name.remote configuration flips
explicit_default_remote_name to one when it is used to set the
default_remote_name, and that controls the value of name_given in
this codepath. At this point in the series, you do not have a
corresponding branch.$name.pushremote, but your [6/6] does not seem
to do the same.
Why isn't it necessary to add explicit_default_pushremote_name and
do the same here in patch [6/6]?
^ permalink raw reply
* Re: USE_NSEC bug?
From: Jeff King @ 2013-03-20 19:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andrew Rodland, git
In-Reply-To: <7vobedewer.fsf@alter.siamese.dyndns.org>
On Wed, Mar 20, 2013 at 11:52:28AM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > So I think your real complaint is "configure does not set NO_NSEC
> > properly", which is something worth fixing.
>
> Yes.
>
> I think our "Meh" was about "do we want to unify USE_NSEC and
> NO_NSEC?" and nothing else.
Exactly. Now I am hoping Andrew will volunteer to fix configure.ac, as I
will be happy to go to my grave never having had to figure out autoconf
m4 syntax ever again. :)
-Peff
^ permalink raw reply
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
From: Jeff King @ 2013-03-20 18:58 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ramkumar Ramachandra, Git List, Junio C Hamano, Eric Sunshine
In-Reply-To: <20130320184157.GO3655@google.com>
On Wed, Mar 20, 2013 at 11:41:57AM -0700, Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> > Jonathan Nieder wrote:
>
> >> I dunno. The helper functions at the top of this test are already
> >> intimidating, so I guess I am looking for a way to avoid making that
> >> problem worse.
> [...]
> > My patch does not make the situation worse in any way:
>
> Um, yes it does. It adds another function to learn to an already
> intimidating list.
Personally I do not find the set of helper functions intimidating. I
tend to read the tests in a top-down manner: a test is interesting
(usually because it fails), and then I want to see what it is doing, so
I look at any functions it calls, and so forth.
What I usually find _much_ harder to debug is when there is hidden state
leftover from other tests. So even though it is longer to write, I would
much rather see:
test_expect_success 'check that frob only affects foo' '
set_state_of foo &&
set_state_of bar &&
git frob &&
check_state_of foo &&
check_state_of bar
'
than for the test to assume the state of "foo" or "bar" prior to the
test. And I think helper functions can help make writing those sorts of
pre-conditions more reasonable (and without looking too hard, I think
t5516 does an OK job of that).
Related to that is when the helper functions operate on hidden state. In
this instance, we have tests that do things like:
mk_empty &&
git push testrepo refs/heads/master:refs/remotes/origin/master
and as a reader I say "wait, what's in testrepo?". I can follow mk_empty
and see that it hardcodes testrepo, but it is even better if the
function and its arguments are named in a way that I don't have to. So
even though it is more typing, I would argue that:
mk_empty testrepo &&
git push testrepo ...
is better, because the test script is more readable as a unit.
None of this is that huge a deal to me (and yet I seem to have written a
page about it :) ), but I figure while we are bikeshedding about test
style...
-Peff
^ permalink raw reply
* Re: [PATCH] Documentation: merging a tag is a special case
From: Jonathan Nieder @ 2013-03-20 18:54 UTC (permalink / raw)
To: Yann Droneaud; +Cc: Git, Junio C Hamano
In-Reply-To: <1363802033-26868-1-git-send-email-ydroneaud@opteya.com>
Yann Droneaud wrote:
> When asking Git to merge a tag (such as a signed tag or annotated tag),
> it will always create a merge commit even if fast-forward was possible.
> It's like having --no-ff present on the command line.
Thanks. This looks good, modulo some nitpicks.
[...]
> --- a/Documentation/git-merge.txt
> +++ b/Documentation/git-merge.txt
> @@ -170,6 +170,15 @@ happens:
> If you tried a merge which resulted in complex conflicts and
> want to start over, you can recover with `git merge --abort`.
>
> +MERGING TAG
> +-----------
> +
> +When merging a tag (annotated or signed), Git will create a merge commit
How about something like "When merging an annotated or signed tag" or
"When merging an annotated (and possibly signed) tag"? The above text
can be misread as meaning "When merging any tag, no matter whether it
is annotated or signed", which is needlessly confusing for people who
don't know about unannotated tags.
[...]
> --- a/Documentation/merge-options.txt
> +++ b/Documentation/merge-options.txt
> @@ -26,7 +26,7 @@ set to `no` at the beginning of them.
> --ff::
> When the merge resolves as a fast-forward, only update the branch
> pointer, without creating a merge commit. This is the default
> - behavior.
> + behavior (except when merging a tag).
s/a tag/an annotated tag/ here as well.
By the way, what about the possibility of dropping this implicit
--no-ff? I think Linus could get used to passing --no-ff explicitly
when responding to pull requests. I could go either way on it.
It is certainly useful to document the current state before
considering changing it, so with the tweaks mentioned above,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCH 5/6] remote.c: introduce remote.pushdefault
From: Junio C Hamano @ 2013-03-20 18:53 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Git List, Jeff King, Eric Sunshine
In-Reply-To: <20130320183255.GL3655@google.com>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Ramkumar Ramachandra wrote:
>
>> This new configuration variable defines the default remote to push to,
>> and overrides `branch.<name>.remote` for all branches.
>
> Micronit: I think this would be easier to explain if it came after
> patch 6, since then you could say "In other words, it is a default for
> branch.<name>.pushremote for all branches" and readers would not have
> to wonder "Why does the more generic configuration override the more
> specific one?".
>
> My two cents,
> Jonathan
Thanks for all good comments (not only to this one but to others in
the series). I agree with all of them.
^ permalink raw reply
* Re: USE_NSEC bug?
From: Junio C Hamano @ 2013-03-20 18:52 UTC (permalink / raw)
To: Jeff King; +Cc: Andrew Rodland, git
In-Reply-To: <20130320183049.GB29609@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So I think your real complaint is "configure does not set NO_NSEC
> properly", which is something worth fixing.
Yes.
I think our "Meh" was about "do we want to unify USE_NSEC and
NO_NSEC?" and nothing else.
^ permalink raw reply
* Re: [PATCH] t: don't redefine test_config() in various places
From: Junio C Hamano @ 2013-03-20 18:51 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1363767703-12936-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> test_config() is already a well-defined function in
> test-lib-functions.sh. Don't duplicate it unnecessarily in:
>
> t4018-diff-funcname.sh
> t7810-grep.sh
> t7811-grep-open.sh
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> Thanks, Junio.
Heh, I've pushed out the original one after fixing them up myself
last night.
^ permalink raw reply
* Re: [RFC] Add posibility to preload stat information.
From: Jeff King @ 2013-03-20 18:46 UTC (permalink / raw)
To: Fredrik Gustafsson; +Cc: Ramkumar Ramachandra, spearce, git, pclouds
In-Reply-To: <20130320183641.GB32426@paksenarrion.iveqy.com>
On Wed, Mar 20, 2013 at 07:36:41PM +0100, Fredrik Gustafsson wrote:
> > Yes, I would certainly like my git startup time to be improved. But I
> > don't want to trade my hard drive's life for it.
>
> Does this really increase disk-reads? The fs-cache would make sure that
> the disk reads is almost the same, we only do them before we usually do
> them.
It shouldn't. But if you are running "stat" on every file in the repo
for each prompt, that is going to take measurable CPU time for large
repos (e.g., WebKit).
> > What I meant by "first time" is "chpwd() into the git repository, not
> > further chpwd()s when already inside the git repository".
>
> That's a good point. I'm not sure how to solve that though. Because it's
> not a fact that you always go to the root git-dir first.
>
> The only way I see this is with a lock-file that's kept and we only run
> git status every 5 minutes when doing something inside a work dir. That
> would add a lot of meta-data (the lock files), to store. (I hope I
> successfully explained that).
How about something like:
__git_primed_toplevel=
__git_prime_dir() {
local toplevel=`git rev-parse --show-toplevel 2>/dev/null`
if test -n "$toplevel" &&
test "$toplevel" != "$git_primed_toplevel"; then
git status >/dev/null 2>&1
git_primed_toplevel=$toplevel
fi
}
that would prime the whole repo the first time you enter it, but otherwise do
nothing (and you could run it from each prompt). If you switched back and forth
between two repos a lot, you would end up priming them both a lot, but that is
not that common a mode of operation (and you could keep an list of recently
primed repos instead of a single one, if you really wanted to deal with that).
You would also prime twice if you used two different terminals, but that's OK.
The subsequent ones are much faster due to disk cache, so this is really just
about not paying the extra stat penalty on _every_ prompt.
-Peff
^ permalink raw reply
* Re: git merge <tag> behavior
From: Junio C Hamano @ 2013-03-20 18:46 UTC (permalink / raw)
To: Yann Droneaud; +Cc: Git
In-Reply-To: <1363803172.6289.49.camel@test.quest-ce.net>
Yann Droneaud <yann@droneaud.fr> writes:
> But but do not take those remarks as a feature request.
> I was just asking for clarification/comment on the behavior difference
> between merging tag/tag object-id.
If you are asking why things are as they are, the answer is simply
because "git merge $(git rev-parse v1.2.3)" was not even considered
while adding the support to pull signed tags.
We did find the use case for "git merge v1.2.3" interesting and
important enough to give it a proper support with defined semantics.
"git merge $(git rev-parse v1.2.3)" may behave differently but it
was not because we found the use case for it important and designed
a behaviour that is different from merging the tag by name that
suits that use case.
It is just we didn't even think giving the bare object name to name
an annotated or signed tag on the command line is interesting, and
the command does whatever the implementation happens to do to such
an input.
I think I sent out a "how about this" patch. Have you tried it?
^ permalink raw reply
* Re: [PATCH 3/6] t5516 (fetch-push): introduce mk_test_with_name()
From: Jonathan Nieder @ 2013-03-20 18:41 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List, Junio C Hamano, Jeff King, Eric Sunshine
In-Reply-To: <CALkWK0mCVb6N76QU+U3iTO_gmU4PmhrTOgU53DAMd5x1bCQEtA@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Jonathan Nieder wrote:
>> I dunno. The helper functions at the top of this test are already
>> intimidating, so I guess I am looking for a way to avoid making that
>> problem worse.
[...]
> My patch does not make the situation worse in any way:
Um, yes it does. It adds another function to learn to an already
intimidating list.
Hoping that clarifies,
Jonathan
^ permalink raw reply
* Re: [PATCH] t: don't redefine test_config() in various places
From: Jonathan Nieder @ 2013-03-20 18:40 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Git List
In-Reply-To: <1363767703-12936-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> Thanks, Junio.
>
> t/t4018-diff-funcname.sh | 5 -----
> t/t7810-grep.sh | 5 -----
> t/t7811-grep-open.sh | 5 -----
> 3 files changed, 15 deletions(-)
Yeah, that looks like all of them. FWIW,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
^ 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