* Re: [PATCH 1/2] parse-options: introduce OPT_SUBCOMMAND
From: Ramkumar Ramachandra @ 2011-12-08 16:43 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <20111208163049.GA2116@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>> , and create a new
>> OPT_SUBCOMMAND; this is built on top of OPTION_BIT to allow for the
>> detection of more than one subcommand.
>
> This part I am not convinced about. Usually each subcommand takes its
> own options, so I cannot see this OPT_SUBCOMMAND being actually useful
> for commands like "git stash" or "git remote".
Hm, what difference does that make? We still have to parse the
subcommand, and subsequently use an if-else construct to parse more
options depending on the subcommand, no?
-- Ram
^ permalink raw reply
* Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options
From: Ramkumar Ramachandra @ 2011-12-08 16:47 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <20111208163946.GB2394@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> - if (argc < 3)
>> - usage(builtin_bundle_usage);
>> -
>> - cmd = argv[1];
>> - bundle_file = argv[2];
>> - argc -= 2;
>> - argv += 2;
>> + argc = parse_options(argc, argv, NULL,
>> + options, builtin_bundle_usage,
>> + PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
>
> Doesn't this make the usage completely confusing? Before, if I wanted
> to create bundle named "verify", I could write
>
> git bundle create verify
>
> Afterwards, not only does that not work, but if I make a typo and
> write
>
> git bundle ceate verify
>
> then it will act like "git bundle verify ceate".
No it won't -- it'll just print out the usage because "verify" and
"create" are mutually exclusive options. Sure, you can't create a
bundle named "verify", but that's the compromise you'll have to make
if you don't want to type out "--" with each option, no?
> I am starting to suspect the first half of patch 1/2 was not such a
> great idea, either. :) Do you have other examples of how it would be
> used?
Hehe, my lack of foresight is disturbing as usual. I'm not yet sure
it'll be useful.
-- Ram
^ permalink raw reply
* Re: [PATCH 3/6] t1006 (cat-file): use test_cmp
From: Jonathan Nieder @ 2011-12-08 16:55 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <1323349817-15737-6-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Use test_cmp in preference to repeatedly comparing command outputs by
> hand.
That could mean one of several things.
It could mean:
1. Use test_cmp instead of open-coding it.
2. Use test_cmp instead of using our knowledge of the underlying
filesystem to retrieve the files from the block device, instead
of relying on the perfectly good operating system facilities
that could take care of it for us
3. Use test_cmp instead of calling a human over to compare command
outputs by eye, which idiomatically might be described as "by
hand".
What I mean is, I actually don't have much of a clue what you mean by
"by hand". Usually it means "not automated sufficiently", but I think
that is not the entire problem here (since
test "$expect" = "$actual"
looks no less automatic than
printf '%s\n' "$expect" >expect &&
printf '%s\n' "$actual" >actual &&
test_cmp expect actual
to me).
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
[...]
> --- a/t/t1006-cat-file.sh
> +++ b/t/t1006-cat-file.sh
> @@ -36,66 +36,41 @@ $content"
[...]
> - expect="$(maybe_remove_timestamp "$content" $no_ts)"
> - actual="$(maybe_remove_timestamp "$(git cat-file $type $sha1)" $no_ts)"
> -
> - if test "z$expect" = "z$actual"
> - then
> - : happy
> - else
> - echo "Oops: expected $expect"
> - echo "but got $actual"
> - false
> - fi
> + maybe_remove_timestamp "$content" $no_ts >expect &&
> + maybe_remove_timestamp "$(git cat-file $type $sha1)" $no_ts >actual &&
> + test_cmp expect actual
Most of the early part of patch proper looks sane from a quick glance.
Wow, the whitespace is a little strange in the original.
[...]
> @@ -175,30 +153,41 @@ do
> done
>
> test_expect_success "--batch-check for a non-existent named object" '
> - test "foobar42 missing
> -foobar84 missing" = \
> - "$( ( echo foobar42; echo_without_newline foobar84; ) | git cat-file --batch-check)"
> + cat >expect <<\-EOF &&
> +foobar42 missing
> +foobar84 missing
> +EOF
> + $(echo foobar42; echo_without_newline foobar84) \
> + | git cat-file --batch-check >actual &&
> + test_cmp expect actual
Style: the | character goes at the end of the first line (think of it
as a way to save backslashes until they're needed).
How could this $(...) command substitution possibly work?
Later tests have the same problem, so I'm stopping here.
Ciao,
Jonathan
^ permalink raw reply
* Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options
From: Jonathan Nieder @ 2011-12-08 17:03 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <CALkWK0mmjKSzSbxq2i7=JvcB4LTro-MYDCwQLUUwqcf8qS0zPA@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Sure, you can't create a
> bundle named "verify", but that's the compromise you'll have to make
> if you don't want to type out "--" with each option, no?
No, that's not a compromise I'll have to make. I'm not making it today.
Having to type "--" or prefix with "./" to escape ordinary filenames
that do not start with "-" would be completely weird. This is
important to me: I want you to see it, rather than relying on me as an
authority figure to say it (after all, I'm not such a great authority
anyway --- I make plenty of mistakes).
^ permalink raw reply
* Re: [PATCH 4/6] t3200 (branch): fix '&&' chaining
From: Jonathan Nieder @ 2011-12-08 17:07 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <1323349817-15737-7-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Breaks in a test assertion's && chain can potentially hide failures
> from earlier commands in the chain. Fix these breaks.
>
> Additionally, note that 'git branch --help' will fail when git
This is not "Additionally, while we're here" but rather "In order to
do so".
> manpages aren't already installed: guard the line with a
> 'test_might_fail'.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> t/t3200-branch.sh | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
For what it's worth,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCH 5/6] t1510 (worktree): fix '&&' chaining
From: Jonathan Nieder @ 2011-12-08 17:12 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <1323349817-15737-8-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Breaks in a test assertion's && chain can potentially hide failures
> from earlier commands in the chain. Fix these breaks.
>
> Additionally, note that 'unset' returns non-zero status when the
> variable passed was already unset on some shells: change these
> instances to 'safe_unset'.
This is also not "Additionally", as in "As a separate change that
maybe should have been another patch but I am too lazy". Rather, it
is a necessary change that is part of the same task. So I would
write:
Breaks in a test assertion's && chain can potentially hide failures
from earlier commands in the chain. Fix these breaks.
'unset' returns non-zero status when the variable passed was already
unset on some shells, so now that the status is tested we need to
change these instances to 'safe_unset'.
Erm, sane_unset, not safe_unset. Did you even test this?
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
> t/t1501-worktree.sh | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
For what it's worth, after those changes,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCH 6/6] test: fix '&&' chaining
From: Jonathan Nieder @ 2011-12-08 17:18 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <1323349817-15737-9-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra wrote:
> Breaks in a test assertion's && chain can potentially hide failures
> from earlier commands in the chain. Fix instances of this in the
> following tests:
>
> t3419 (rebase-patch-id)
> t3310 (notes-merge-manual-resolve)
[...]
The reader can read the diffstat, so I am not sure this list is very
useful.
With the space gained, it might be helpful to mention that this patch
only adds " &&" to the ends of lines and that any other kind of change
here would be unintentional, to put the reader's mind at ease.
[...]
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
The patch proper looks good, so if this is tested,
Acked-by: Jonathan Nieder <jrnieder@gmail.com>
^ permalink raw reply
* Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options
From: Ramkumar Ramachandra @ 2011-12-08 17:38 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <20111208170319.GD2394@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
>
>> Sure, you can't create a
>> bundle named "verify", but that's the compromise you'll have to make
>> if you don't want to type out "--" with each option, no?
>
> Having to type "--" or prefix with "./" to escape ordinary filenames
> that do not start with "-" would be completely weird.
Hm, do you have any suggestions to work around this? Can we use
something like a parse-stopper after the the first subcommand is
encountered, and treat the next argument as a non-subcommand (filename
or whatever else)?
-- Ram
^ permalink raw reply
* Re: Query on git commit amend
From: Junio C Hamano @ 2011-12-08 17:52 UTC (permalink / raw)
To: Viresh Kumar
Cc: Björn Steinbrink, Vijay Lakshminarayanan,
git@vger.kernel.org, Shiraz HASHIM, Vipin KUMAR
In-Reply-To: <4EDEFD66.4020404@st.com>
Viresh Kumar <viresh.kumar@st.com> writes:
> GIT_EDITOR=cat git commit --amend
>
> over
>
> git commit --amend -C HEAD
>
> ?
>
> ...
> But for single commit probably second one looks easier. Isn't it?
I saw "--amend -C HEAD" mentioned in some newbie-guide webpages, and I
think you just copy/learned from one of them, so it is not entirely your
fault, but the combination of --amend and "-C HEAD" is an idiotic thing
that happens to work, if you think about what exactly you are telling to
the command.
The point of --amend is twofold. One is to let you tweak the contents of
what is committed, which is not the topic of this thread, and the other is
to allow the user to reuse the log message from the commit being amended,
instead of typing the message from scratch.
The "-c <commit>" and its cousin "-C <commit>" options are about telling
Git that the user does _not_ want the other usual logic to come up with
the initial commit template (e.g. when committing anew, it may read the
log template file, when committing a merge, it may read the MERGE_MSG
prepared by fmt-merge-msg, and most importantly in this context, when
amending, the one that is prepared by the --amend logic is used) kick in
at all, and instead wants to start from the log message of the named
commit.
So by saying "--amend -C HEAD" you are saying "I want to reuse the log
message of the commit I am amending,... eh, scratch that, I instead want
to use the log message of the HEAD commit", as if the commit you are
amending and HEAD are two different things. That is idiotic.
And you say that only for the side effect that capital "-C" stops the
editor.
Compared to that idiotic statement, "EDITOR=: git commit --amend" is a lot
saner way to say the same thing in a more direct and straightforward
way. "I want to reuse the log message of the commit I am amending, and the
editor to use it while running that commit is the command 'true', i.e. the
one that does not really touch any line in the text file and successfully
exits, because I am not going to change anything".
Of course, if "git commit --amend" honoured "--no-edit", that is even more
direct, straightforward and intuitive way to say so ;-)
^ permalink raw reply
* Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options
From: Jonathan Nieder @ 2011-12-08 17:59 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <CALkWK0m89D02aku8J0OXbpyrovHCOpsYS4Qpx2jH-pFG4rOG2A@mail.gmail.com>
Ramkumar Ramachandra wrote:
> Jonathan Nieder wrote:
>> Having to type "--" or prefix with "./" to escape ordinary filenames
>> that do not start with "-" would be completely weird.
>
> Hm, do you have any suggestions to work around this? Can we use
> something like a parse-stopper after the the first subcommand is
> encountered, and treat the next argument as a non-subcommand (filename
> or whatever else)?
What's the desired behavior? Then we can talk about how to implement it.
If the goal is "use parse-options for a command that has subcommands",
see builtin/notes.c.
^ permalink raw reply
* Disabling Delta Compression on a fetch
From: Todd Rinaldo @ 2011-12-08 17:34 UTC (permalink / raw)
To: git
We run an internal gitorious server with many repos on it. Due to legacy issues, some of the repos are rather bloated with large binaries. When several changes happen and are pushed to the gitorious repo and then are later pulled back (fetch and pull) to other clones, we get a broken connection while the gitorious side is generating Delta compression.
All of the git communication happens on 1 subnet all connected by a single gigabit switch. As I see it, the Delta Compression is actually a performance degradation in our environment.
The solution I've come up with is to set pack.window=0 in /etc/gitconfig on the gitorious server.
I realize our need is uncommon. I realize by doing this, I am trading increased bandwidth in exchange for starting the send sooner. When I read the documentation for pack.window, it seems more a side effect than the original intent that this variable disables Delta Compression when set to 0.
My question is: Are there are any unintended consequences of this approach anyone can think of?
Thanks,
Todd Rinaldo
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-08 18:19 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: git, Jonathan Nieder
In-Reply-To: <CALkWK0nNtvrLHxQv17jfrFQ=BcwLfgh7eE9X-nDCCYY0nsOskg@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> * On "revert: make commit subjects in insn sheet optional"
>>
>> After finding the verb and advancing the pointer "bol" at the beginning of
>> the object name, end_of_object_name variable points at the first SP or LF
>> using strcspn(bol, " \n"), but I wonder why we are not grabbing a run of
>> hexdigits instead, i.e. strspn(bol, "0123456789abcdef"). Is this so that
>> we can allow something like "pick rr/revert-cherry-pick~3"?
>
> Yes, it is exactly for that reason :)
That is not explained in the commit message, which in itself is a problem
that needs to be addressed, but ...
>> I also wonder if this should be (sorry for pcre) "(pick|revert)\s+(\S+)\s"
>> instead, i.e. allow users with fat fingers to use one or more SP or even HT
>> to separate the verb and the operand.
>
> Hm, I'm not too enthusiastic about this change, because we don't
> advertise hand-editing the instruction sheet yet:...
... is inconsistent with the above.
If you plan to later allow editing and if you know what kind of editing
you are going to allow, it would be better to prepare the parser to accept
input with a reasonable slack, especially because you are already touching
it in this series anyway.
On the other hand, if you do not want to think about (or do not want us to
get distracted by thinking about) allowing editing in this series, which
is also a sensible stance to take, the parser should be made to accept
only what the mechanism in the series would produce and error out
otherwise, leaving the entire "the user now can edit and here are the rules"
part for a separate series that comes after this series is perfected.
>> * On "revert: allow mixed pick and revert instructions"
>>
>> Reporting what we did not understand from parse_insn_line() is a good
>> idea, but I think the line number should be reported at the beginning
>> of the same line.
>
> Makes sense. Do you like this?
Not particularly.
> - return error(_("Unrecognized action: %.*s"), (int)len, bol);
> + return error(_("Line %d: Unrecognized action: %.*s"),
It may be just me, but personally I prefer to see which file of what line
had the problem, i.e. "%s:%d: Unrecognized action:%s".
But if there is not much point in reporting the filename because it always
is the same, "Unrecognized action (line %d): %s" would also be fine.
^ permalink raw reply
* Re: [bug?] checkout -m doesn't work without a base version
From: Junio C Hamano @ 2011-12-08 18:27 UTC (permalink / raw)
To: Pete Harlan; +Cc: git
In-Reply-To: <4EDF1631.5090906@pcharlan.com>
Pete Harlan <pgit@pcharlan.com> writes:
> to "ours" and "theirs". (The same thing happens in the 3-way merge
> case.)
That is entirely expected. "checkout -m" does not know or care _how_ the
index came to the conflicted state and reproduces the three-way conflicted
state in the file in the working tree solely from the contents of the
index which records only what the common thing looked like (stage #1),
what one side looked like (stage #2) and what the other side looked like
(stage #3) before the mergy operation began, so there is no way for it to
say "the other side came from foo/blah branch". There are even cases where
the conflict originally came not from any branch (think "am -3").
^ permalink raw reply
* Re: [PATCH 0/5] Re-roll rr/revert-cherry-pick
From: Junio C Hamano @ 2011-12-08 18:41 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Ramkumar Ramachandra, Git List
In-Reply-To: <20111207081734.GG11737@elie.hsd1.il.comcast.net>
Jonathan Nieder <jrnieder@gmail.com> writes:
> Ramkumar Ramachandra wrote:
>
>> This is a re-roll of rr/revert-cherry-pick, with Junio's suggestions
>> ($gname/186365) implemented.
>
> Thanks for this. I am worried that some of my comments in previous
> reviews (especially about change descriptions) were not addressed,
> which could mean one of a few things:
>
> - you didn't notice them
> - they were wrong
> - you noticed them and they were describing real problems, but it
> wasn't worth the time to fix them
>
> Fine. But I would like to know which case they fell into, so I can
> learn in order to do a better job reviewing the future and know my
> time is well spent.
Another thing I often notice after raising an issue during a review (this
is not limited to Ram) is that I do so in the form of a question "why is
this function done this way?" expecting either "ah, doing it that way
instead is simpler and easier to read" or "because of such and such, which
you may have missed. after all this is a tricky codepath with this and
that constraints" as a response. The former should and often does result
in an update of the code in the re-rolled series, but the latter often
results in a frustrating nothing in a re-roll, when the fact that a
reviewer needed to ask the question was a sure sign that the code needs
explanation either in the log message or in-code comment.
> The series makes various changes, all essentially good, which leaves
> me all the more motivated to figure out how to get this sort of thing
> in smoothly without making life difficult for people in the future
> tracking down regressions.
I looked the series over with your comments and tend to agree with many of
them. Perhaps I should wait for another round before picking it up again
(I've already dropped the old series I kept before 1.7.8).
^ permalink raw reply
* Re: [PATCH] git-send-email: Add auto-cc to all body signatures
From: Junio C Hamano @ 2011-12-08 19:37 UTC (permalink / raw)
To: Joe Perches; +Cc: git, jeffrey.t.kirsher
In-Reply-To: <1323313119.1762.58.camel@joe2Laptop>
Joe Perches <joe@perches.com> writes:
> Many types of signatures are used by various projects.
>
> The most common type is formatted:
> "[some_signature_type]-by: First Last <email@domain.tld>"
> e.g:
> "Reported-by: First Last <email@domain.tld>" (no quotes are used)
This is just a phrasing issue, but I am a bit reluctant about the name
"signature". "Acked-by:", "Tested-by:" and "Reviewed-by:" are originally
written in the message sent to the author by the person who is giving an
Ack, a successful test report, and a review comment, and the author (at
least in spirit) copies & pastes them to the final text used in the commit
log message, so it would not be incorrect to call them "signatures". But
other "Random-by:" would not fall into that pattern.
"Reported-by:" certainly does not. It is almost always added by the
author of the patch that is different from the reporter, and the reporter
wouldn't have written "Reported-by: me" in the original bug report that
triggered the discussion and resulted in the commit to fix the bug. Such a
line is certainly not a signature of/by the reporter. Same can be said for
"Helped-by:" for the author to share credits.
Also I've seen these "Random-by:", especially the ones that the author
adds on his own initiative like "Reported-by:", followed by just a name
but not an addresses [*1*].
Does your change do the right thing on such an address-less entry? The
answer to this question must start with the definition of "the right thing
to do is X", of course.
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 327233c..17ea825 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -246,8 +246,9 @@ Automating
> patch body (commit message) except for self (use 'self' for that).
> - 'sob' will avoid including anyone mentioned in Signed-off-by lines except
> for self (use 'self' for that).
> +- 'signatures' will avoid including anyone mentioned in any "<foo>-by:" lines.
> - 'cccmd' will avoid running the --cc-cmd.
> -- 'body' is equivalent to 'sob' + 'bodycc'
> +- 'body' is equivalent to 'sob' + 'bodycc + signatures'
The quotes do not match quite well.
[Footnote]
*1* This seems to be done deliberately so; I understand that this is to
avoid running afoul of EU privacy legislation or something.
^ permalink raw reply
* Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options
From: Ramkumar Ramachandra @ 2011-12-08 19:39 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, Matthieu Moy, Git List
In-Reply-To: <20111208175913.GK2394@elie.hsd1.il.comcast.net>
Hi,
Jonathan Nieder wrote:
> What's the desired behavior? Then we can talk about how to implement it.
>
> If the goal is "use parse-options for a command that has subcommands",
> see builtin/notes.c.
Uses strcmp() to match argv[0]. And you can't specify the options for
a certain subcommand before the subcommand itself on the command-line,
although I don't consider this a serious limitation. I was going for
something prettier with subcommand-specific help text, albeit a
serious limitation. I'll try working towards this for a few more
hours to see if anything useful comes out of it -- otherwise, I'll
just drop this patch and focus on eliminating the ugliness in
builtin/revert.c around '--continue', '--quit' parsing.
That being said, do you see value in lifting the restriction on
opts->long_name and PARSE_OPTS_NODASH not allowed together? The
restriction seems quite arbitrary, but I can't justify lifting it
unless I can show some valid usecase.
Thanks.
-- Ram
^ permalink raw reply
* [PATCH] mingw: give waitpid the correct signature
From: Erik Faye-Lund @ 2011-12-08 19:39 UTC (permalink / raw)
To: git; +Cc: msysgit
POSIX says that last parameter to waitpid should be 'int',
so let's make it so.
Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>
---
Just a small clean-up I stumbled over...
compat/mingw.c | 2 +-
compat/mingw.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index efdc703..a0ac487 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1712,7 +1712,7 @@ char *getpass(const char *prompt)
return strbuf_detach(&buf, NULL);
}
-pid_t waitpid(pid_t pid, int *status, unsigned options)
+pid_t waitpid(pid_t pid, int *status, int options)
{
HANDLE h = OpenProcess(SYNCHRONIZE | PROCESS_QUERY_INFORMATION,
FALSE, pid);
diff --git a/compat/mingw.h b/compat/mingw.h
index fecf0d0..0ff1e04 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -120,7 +120,7 @@ static inline int mingw_mkdir(const char *path, int mode)
#define mkdir mingw_mkdir
#define WNOHANG 1
-pid_t waitpid(pid_t pid, int *status, unsigned options);
+pid_t waitpid(pid_t pid, int *status, int options);
#define kill mingw_kill
int mingw_kill(pid_t pid, int sig);
--
1.7.7.1.msysgit.0.272.g9e47e
^ permalink raw reply related
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Erik Faye-Lund @ 2011-12-08 19:44 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Git Mailing List, Johannes Sixt, Stephan Beyer
In-Reply-To: <20111206185218.GB9492@sigill.intra.peff.net>
On Tue, Dec 6, 2011 at 7:52 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Dec 06, 2011 at 12:22:06PM +0100, Erik Faye-Lund wrote:
>
>> >> * jk/upload-archive-use-start-command (2011-11-21) 1 commit
>> >> - upload-archive: use start_command instead of fork
>> >>
>> >> What's the status of this one?
>> >
>> > I think what you have in pu is good, but of course I didn't actually
>> > test it on Windows. Erik?
>> >
>>
>> I tried to check out ee27ca4 and build, and got hit by a wall of warnings:
>
> I think you are on the wrong topic. ee27ca4 is the maint topic for
> "don't let remote clients get unreachable commits", and is based on the
> ancient 1.6.2. Which is why you are getting all of those warnings.
>
You are indeed right. Thanks for spotting :)
> The topic in question is jk/upload-archive-use-start-command, which is
> at 1bc01ef, and should be based on recent-ish master.
t5000-tar-tree pass for that one as well. No warnings this time :P
^ permalink raw reply
* Re: [PATCH 0/2] Parsing a subcommand using parse-options
From: Junio C Hamano @ 2011-12-08 19:51 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
In-Reply-To: <1323346028-9201-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> git stash show
> ^^ -- The subcommand "show" to the git-stash builtin
Contrasting this with "git tag --list", one uses subcommand verb while the
other uses operating mode option and one could say they are inconsistent.
But it does not bother me that much, and for a good reason other than
inertia.
When a command whose primary purpose is very clear (e.g. "git tag" and
"git branch" are primarily to create these things, just like "git commit"
is to create a commit), it is more natural to give the primary mode the
main interface so that you do not have to say "git tag create v1.0"; hence
operating mode option makes sense than subcommand.
On the other hand, when a command does not have a clear primary mode
(e.g. if you save a stash you must be able to use (i.e. apply or pop) the
saved one and both are equally important feature), but its primary purpose
is to dispatch various different operation, it makes more sense to name
them as subcommands. You _could_ make all of them into operating mode
options, but that only requires more typing, i.e. "git stash --save"/"git
stash --pop", without adding much value to the command. In addition, it
invites unnecessary confusion "what is the default mode of operation, and
is it really that important to be the default?", because not requiring an
explicit "subcommand" but merely allowing "operation mode option" implies
that you can say "git cmd" without anything, i.e. there must be some
default.
For "stash", "save" has been the default merely by historical accident,
but that has been rectified (it now requires you do not have any message
for the quick stash "git satsh<ENTER>" to work). There really isn't any
"default" operating mode to the command, and the command is a dispatcher
to its subcommands.
^ permalink raw reply
* Re: [PATCH v2 0/6] Fix '&&' chaining in tests
From: Junio C Hamano @ 2011-12-08 19:55 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Jonathan Nieder, Matthieu Moy, Git List
In-Reply-To: <1323349817-15737-1-git-send-email-artagnon@gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> This follows-up $gmane/186481.
I take that you meant "replaces". It was confusing especially because you
seem to have included a few unrelated patches in the thread.
^ permalink raw reply
* Re: Disabling Delta Compression on a fetch
From: Junio C Hamano @ 2011-12-08 20:10 UTC (permalink / raw)
To: Todd Rinaldo; +Cc: git
In-Reply-To: <070681D4-F87B-435E-8A3B-144E59DE722B@cpanel.net>
Todd Rinaldo <toddr@cpanel.net> writes:
> The solution I've come up with is to set pack.window=0 in /etc/gitconfig on the gitorious server.
Wouldn't setting core.bigfilethreshold be a more appropriate solution?
^ permalink raw reply
* Re: Disabling Delta Compression on a fetch
From: Jeff King @ 2011-12-08 20:14 UTC (permalink / raw)
To: Todd Rinaldo; +Cc: git
In-Reply-To: <070681D4-F87B-435E-8A3B-144E59DE722B@cpanel.net>
On Thu, Dec 08, 2011 at 11:34:26AM -0600, Todd Rinaldo wrote:
> All of the git communication happens on 1 subnet all connected by a
> single gigabit switch. As I see it, the Delta Compression is actually
> a performance degradation in our environment.
>
> The solution I've come up with is to set pack.window=0 in
> /etc/gitconfig on the gitorious server.
An alternative is to mark the binary files as "-delta" with
gitattributes on the server. Then you will get the benefits of delta
compression for other files without bothering to try the binary files.
Note that git won't read the gitattributes file out of the tree in a
bare repository, so these attributes should go either in
$REPO/info/attributes (if they are repo-specific), or in
/etc/gitattributes (if they are in many repos).
I.e., something like:
echo '*.bin -delta' >/etc/gitattributes
Also, before any of that, make sure that the upstream repos are fully
packed. Git will not try to delta two objects coming from the same pack
(since it will already have tried when they were entering the pack).
That by itself might be enough to solve your problem without any other
configuration.
> My question is: Are there are any unintended consequences of this
> approach anyone can think of?
Other than trading bandwidth, you are also trading space on the client
side, since each client will store the resulting pack.
-Peff
^ permalink raw reply
* Re: [PATCH] git-send-email: Add auto-cc to all body signatures
From: Joe Perches @ 2011-12-08 20:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, jeffrey.t.kirsher
In-Reply-To: <7v8vmmj1ng.fsf@alter.siamese.dyndns.org>
On Thu, 2011-12-08 at 11:37 -0800, Junio C Hamano wrote:
> Joe Perches <joe@perches.com> writes:
> > Many types of signatures are used by various projects.
> > The most common type is formatted:
> > "[some_signature_type]-by: First Last <email@domain.tld>"
> > e.g:
> > "Reported-by: First Last <email@domain.tld>" (no quotes are used)
> This is just a phrasing issue, but I am a bit reluctant about the name
> "signature".
I've called all these markings signatures.
Maybe email-address-tags or another name could be used.
I'm not bothered one way or another by any chosen name.
> Does your change do the right thing on such an address-less entry? The
> answer to this question must start with the definition of "the right thing
> to do is X", of course.
All addresses go through "extract_valid_address".
Invalid addresses are not used.
^ permalink raw reply
* Re: What's cooking in git.git (Dec 2011, #02; Mon, 5)
From: Junio C Hamano @ 2011-12-08 21:25 UTC (permalink / raw)
To: kusmabite; +Cc: Jeff King, Git Mailing List, Johannes Sixt, Stephan Beyer
In-Reply-To: <CABPQNSYQ=nt9LYzXpQgfwV00e9AxOV3LKj6VCCO8xkMAXb-Lfg@mail.gmail.com>
Erik Faye-Lund <kusmabite@gmail.com> writes:
>> The topic in question is jk/upload-archive-use-start-command, which is
>> at 1bc01ef, and should be based on recent-ish master.
>
> t5000-tar-tree pass for that one as well. No warnings this time :P
Thanks; let's move it forward, then.
^ permalink raw reply
* Re: [PATCHv2 0/13] credential helpers
From: Junio C Hamano @ 2011-12-08 21:34 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20111207064231.GA499@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Because the pattern takes 0 or more lines and no terminator, we can't
> distinguish between empty or truncated input and the empty pattern.
I agree that such a positive "Ok here is the end of specification" marker
is a good idea, even if we do not worry about "an empty set".
When the requestor wants to specify the credentials with host and user,
but the wire is cut after host is communicated but before user is, we do
want to notice the communication error, instead of silently erasing all
the credentials on the host regardless of the user.
Thanks.
^ 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