* Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.
From: Junio C Hamano @ 2008-01-12 4:57 UTC (permalink / raw)
To: Stephen Sinclair; +Cc: Junio C Hamano, git
In-Reply-To: <9b3e2dc20801111825i644da598i7556ef700b870445@mail.gmail.com>
"Stephen Sinclair" <radarsat1@gmail.com> writes:
>> In a properly configured repository, telling you who git thinks
>> you are is _ALWAYS_ useless (that's the definition of "properly
>> configured"). Just admit it.
>
> Well, I'll admit that I don't really understand you here.
> Maybe I'm still too much of a git newbie on this. (Fair enough.)
> Right now the only way to make sure I'm committing as myself with my
> proper email address is to:
>
> -- remember to "git-config --list", and check that my email is listed.
> -- "git-commit; git-log", and remember to check the last entry before
> doing a "git-push".
>
> Am I missing something?
They are both valid means to make sure you did not misconfigure.
You omitted the part that matters from the part you quoted
above, but this discussion was about "showing AUTHOR if it is
different from me", which was _one of_ the two conditions I
suggested in my counterproposal, and I was saying that it is
useless to expect that you would be able to find a
misconfiguration when AUTHOR is shown for this first reason.
This part is _not_ about catching your misconfiguration.
The other part is about the misconfiguration catching.
> Especially considering the default name is taken from the hostname
> anyway -- you're taking the local hostname and then checking with a
> rule to see if it might be localhost.
Yes, and earlier you said one of the undesirable ones was
"yourname@foo.local" (and others were "yourname@foo.(none)").
IOW, "localhost" is one of the things you want to catch as
unconfigured bogosity that you want to catch, isn't it?
And that is _the other_ condition in my counterproposal to show
AUTHOR.
To rephrase, you would show AUTHOR when one of the conditions
holds true, either:
(1) "not me" (so that we can remind that other's commit is
being amended); _OR_
(2) "funny me" (so that we can catch misconfiguration.
The latter would not have to trigger once you configure your
~/.gitconfig (or .git/config) properly.
^ permalink raw reply
* Re: git-commit fatal: Out of memory? mmap failed: Bad file descriptor
From: Jeff King @ 2008-01-12 4:56 UTC (permalink / raw)
To: Charles Bailey; +Cc: Brandon Casey, Git Mailing List, drafnel
In-Reply-To: <4787EB38.7010600@hashpling.org>
On Fri, Jan 11, 2008 at 10:18:32PM +0000, Charles Bailey wrote:
> I have seen this exact type of failure (commit reports possible oom, but
> commit appears to have succeeded) with most recent gits.
This is almost certainly caused not by the commit action itself (which
uses very little memory) but by the resulting diffstat to show what
happened. So the commit has already been "committed" to disk by the time
it crashes.
This is at least the case with Brandon's problem (his stack trace shows
the diff happening).
-Peff
^ permalink raw reply
* Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.
From: Jeff King @ 2008-01-12 4:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephen Sinclair, git
In-Reply-To: <7vejcnzu5z.fsf@gitster.siamese.dyndns.org>
On Fri, Jan 11, 2008 at 05:53:12PM -0800, Junio C Hamano wrote:
> The official party line to defend the existing behaviour is that
> there is no need to configure anything, when the host and gecos
> is done properly. But I tend to agree with you that quite a lot
> of systems are not "done properly", and users cannot do much
> about it in some cases. I think most of misconfigured systems
> are personal boxes they have control over but not all.
I think there are plenty of reasons for the host/gecos information not
being useful. Is a workstation whose hostname is not a valid mailing
address really not "done properly"?
> Perhaps we could disable the code that reads from hostname and
> gecos, and instead always force the users to configure. But
> that kind of change is not something I'd want to be discussing
> right now.
This is obviously not 1.5.4 material, so I haven't given it that much
thought either. But perhaps Stephen's "author message" should simply
trigger any time the author is pulled from gecos? I suppose that would
annoy people who use this feature all the time, but they can silence the
"warning" with a simple git-config.
-Peff
^ permalink raw reply
* Re: Decompression speed: zip vs lzo
From: Junio C Hamano @ 2008-01-12 4:46 UTC (permalink / raw)
To: Sam Vilain
Cc: Linus Torvalds, Nicolas Pitre, Pierre Habouzit, Git Mailing List,
Johannes Schindelin, Marco Costalba
In-Reply-To: <47881D44.9060105@vilain.net>
Sam Vilain <sam@vilain.net> writes:
> If the uncompressed objects are clustered in the pack, then they might
> stream compress a lot better, should they be tranmitted over a http
> transport with gzip encoding.
That would only have been a sensible optimization in older
native pack protocol, where we always exploded the transferred
packfile. However, these days, we tend to keep the packfile and
re-index at the receiving end (http transport never exploded the
packfile and it still doesn't). When used that way, choosing
object layout in packfile in such a way to ignore recency order
and cluster objects by their delta chain, which you are
advocating to reduce the transfer overhead, is a bad tradeoff.
Your packs will be kept in the form you chose for transport,
which is a layout that hurts the runtime performance. And you
keep using that suboptimal packs number of times, getting hurt
every time.
> @@ -433,7 +434,7 @@ static unsigned long write_object(struct sha1file *f,
> }
> /* compress the data to store and put compressed length in datalen */
> memset(&stream, 0, sizeof(stream));
> - deflateInit(&stream, pack_compression_level);
> + deflateInit(&stream, size >= compression_min_size ? pack_compression_level : 0);
> maxsize = deflateBound(&stream, size);
> out = xmalloc(maxsize);
> /* Compress it */
I very much like the simplicity of the patch. If such a simple
approach can give us a clear performance gain, I am all for it.
Benchmarks on different repositories need to back that up,
though.
^ permalink raw reply
* Re: [EGIT PATCH] Showing abbreviated commit hash of the versions in Compare editor.
From: Roger C. Soares @ 2008-01-12 3:37 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Shawn O. Pearce
In-Reply-To: <200801110046.41786.robin.rosenberg.lists@dewire.com>
Robin Rosenberg escreveu:
> I pushed my doc-branch as pu, you can look at it. My intention is take a look again
> and then push it as master if there are no objections. Is it ok and good to flags
> public stuff without javadoc as an error (tip pu commit)?
>
Got some errors, when trying to export to a deployable plugin:
"/home/roger/eclipse/workspace/egit/org.spearce.jgit/tst" does not exist!
And on "team -> share project... -> Git", it goes to the CVS wizard.
[]s,
Roger.
^ permalink raw reply
* Re: Decompression speed: zip vs lzo
From: Sam Vilain @ 2008-01-12 3:06 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Linus Torvalds, Pierre Habouzit, Git Mailing List,
Johannes Schindelin, Marco Costalba, Junio C Hamano
In-Reply-To: <alpine.LFD.1.00.0801112125470.3054@xanadu.home>
Nicolas Pitre wrote:
> Sorry to rain on your parade, but to me 6-10% time saving is not a clear
> win at all, given the equal increase in repository size. This is simply
> not worth it.
Agree.
> And a 50% time saving on an operation, such a git log, which takes less
> than 2 seconds in absolute time, is not worth the repo size increase
> either.
Disagree. Going as much as twice as fast for many history operations
for 10% added space sounds like a clear win to me. We can easily agree
to disagree though - making it a disabled by default config option
allows the user to unroll their packs if they want.
> Going from 2 seconds down to one second doesn't make enough of
> a user experience difference.
What do you mean? 1 second waiting is far better than 2 seconds
waiting. And the mmap optimizations have not even begun yet - that
could result in boosts from zero-copy, such as a lighter VM footprint.
> If git blame was to go from 10 seconds down to 4 then I'd say this is a
> clear win. But this is not the case.
This is an awesome boost! Everything feels snappier already :)
maia:~/src/perl.clean$ time git-log | LANG=C wc
288927 894027 8860916
real 0m0.839s
user 0m0.824s
sys 0m0.144s
maia:~/src/perl.clean$ cd ../perl.clean.loose/
maia:~/src/perl.clean.loose$ time git-log | LANG=C wc
288927 894027 8860916
real 0m0.515s
user 0m0.504s
sys 0m0.136s
maia:~/src/perl.clean.loose$ du -sk .git/objects/pack/
113484 .git/objects/pack/
maia:~/src/perl.clean.loose$ cd -
/home/samv/src/perl.clean
maia:~/src/perl.clean$ du -sk .git/objects/pack/
107040 .git/objects/pack/
maia:~/src/perl.clean$
Want me to try this on kde.git?
Sam.
^ permalink raw reply
* Re: [EGIT PATCH] Showing abbreviated commit hash of the versions in Compare editor.
From: Roger C. Soares @ 2008-01-12 3:02 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git, Shawn O. Pearce
In-Reply-To: <200801110046.41786.robin.rosenberg.lists@dewire.com>
Robin Rosenberg escreveu:
> I pushed my doc-branch as pu, you can look at it. My intention is take a look again
> and then push it as master if there are no objections. Is it ok and good to flags
> public stuff without javadoc as an error (tip pu commit)?
>
> The abbr commit patch is already pushed to master.
>
Looks good to me. You actually took some time to explain things, it will
be usefull, thanks!
If we really want to document all the public methods then I think it's
nice to flag it's absence as an error. It would have made me document
GitResourceNode.getContentIdentifier(), it's now one more comment for
you to add before pushing to master ;)
[]s,
Roger.
^ permalink raw reply
* Re: Decompression speed: zip vs lzo
From: Nicolas Pitre @ 2008-01-12 2:32 UTC (permalink / raw)
To: Sam Vilain
Cc: Linus Torvalds, Pierre Habouzit, Git Mailing List,
Johannes Schindelin, Marco Costalba, Junio C Hamano
In-Reply-To: <47881D44.9060105@vilain.net>
On Sat, 12 Jan 2008, Sam Vilain wrote:
> Linus Torvalds wrote:
> >
> > On Fri, 11 Jan 2008, Sam Vilain wrote:
> >> The difference seems only barely measurable;
> >
> > Ok.
> >
> > It may be that it might help other cases, but that seems unlikely.
> >
> > The more likely answer is that it's either of:
> >
> > - yes, zlib uncompression is noticeable in profiles, but that the
> > cold-cache access is simply the bigger problem, and getting rid of zlib
> > just moves the expense to whatever other thing that needs to access it
> > (memcpy, xdelta apply, whatever)
> >
> > or
> >
> > - I don't know exactly which patch you used (did you just do the
> > "core.deltacompression=0" thing?), and maybe zlib is fairly expensive
> > even for just the setup crud, even when it doesn't really need to be.
> >
> > but who knows..
>
> Well, my figures agree with Pierre I think - 6-10% time savings for
> 'git annotate'.
>
> I think Pierre has hit the nail on the head - that skipping
> compression for small objects is a clear win. He saw the obvious
> criterion, really. I've knocked it up as a config option that doesn't
> change the default behaviour below.
Sorry to rain on your parade, but to me 6-10% time saving is not a clear
win at all, given the equal increase in repository size. This is simply
not worth it.
And a 50% time saving on an operation, such a git log, which takes less
than 2 seconds in absolute time, is not worth the repo size increase
either. Going from 2 seconds down to one second doesn't make enough of
a user experience difference.
If git blame was to go from 10 seconds down to 4 then I'd say this is a
clear win. But this is not the case.
Nicolas
^ permalink raw reply
* Re: [PATCH] git-relink.perl: warn() instead of die() on directory open failure
From: Brandon Casey @ 2008-01-12 2:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <7vabnbzt6t.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
>
>> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
>> ---
>>
>>
>> Sometimes the repository to link to is not under your control.
>> If it contains files or unreadable directories, git-relink will
>> die without this patch.
>
> I am not so sure if dying is a bad behaviour, if it is because
> you are trying to link against an object store that you may not
> be able to read. I actually think we should actively refuse to,
> in order to prevent future problems. After seeing the command
> die, you will talk to the owner of that "master" object store
> and ask him to fix permissions (or he may choose to say "please
> do not share with me").
The case for me was that the objects directory contained temporary
pack files. This is a perfectly valid state for a git repository,
but relink() currently aborts the whole effort when it encounters
a non-directory.
Stale tmp packs can remain if the user aborted a git command before
it was finished. (hmm, maybe git-gc --prune could remove these too?)
I only mentioned the unreadable directory case as another possibility.
You're probably right about that one.
-brandon
^ permalink raw reply
* Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.
From: Stephen Sinclair @ 2008-01-12 2:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vejcnzu5z.fsf@gitster.siamese.dyndns.org>
> In a properly configured repository, telling you who git thinks
> you are is _ALWAYS_ useless (that's the definition of "properly
> configured"). Just admit it.
Well, I'll admit that I don't really understand you here.
Maybe I'm still too much of a git newbie on this. (Fair enough.)
Right now the only way to make sure I'm committing as myself with my
proper email address is to:
-- remember to "git-config --list", and check that my email is listed.
-- "git-commit; git-log", and remember to check the last entry before
doing a "git-push".
Am I missing something?
If proper use of git seems to require remembering one of these two
things, that's okay with me, I'll just do my best, but it was an area
where I thought I might suggest an improvement. (As a rule, I prefer
letting the computer remember things for me.)
> ".local", "@localhost", "@<distroname>" and ".(none)" are all
> plausible red-flag raisers. There may be more, but I think we
> should be able to catch most misconfigurations with simple
> rules.
I'd have to disagree with you here. Most people name their boxes one
thing or another, and trying to catch it with some rule is pointless.
Especially considering the default name is taken from the hostname
anyway -- you're taking the local hostname and then checking with a
rule to see if it might be localhost. Personally I think the solution
is not to take the hostname in the first place, since in my experience
it's rarely equivalent to a valid email address.
Obviously though my personal experience is apparently not the same as
that of others'. I do most of my development on personal boxes, or
laptops configured by some non-professional guy in my lab at
university. Or on virtual machines, which was my recent case.
> Perhaps we could disable the code that reads from hostname and
> gecos, and instead always force the users to configure.
That would be my preference, and I do think there's a case for it. But
whether it's a strong one or not I'm not sure.
> But that kind of change is not something I'd want to be
> discussing right now.
That's okay. In the spirit of git, I'll just solve my problem in my
own branch.. ;-)
thanks for the comments,
Steve
^ permalink raw reply
* Re: [PATCH] Teach remote machinery about remotes.default config variable
From: Junio C Hamano @ 2008-01-12 2:18 UTC (permalink / raw)
To: Mark Levedahl; +Cc: git
In-Reply-To: <30e4a070801111252s4e17b9c4m62adeb9032963e66@mail.gmail.com>
"Mark Levedahl" <mlevedahl@gmail.com> writes:
>> However, it is a bit hard to judge how much of inconvenience it
>> really is in your real life that the current behaviour does not
>> allow you to.
>
> I believe I addressed this in the thread with Dscho.
Thanks.
I have to admit that I happen to agree with Dscho. I do not see
this helping to solve communication issues very much.
^ permalink raw reply
* Re: [PATCH] git-relink.perl: warn() instead of die() on directory open failure
From: Junio C Hamano @ 2008-01-12 2:14 UTC (permalink / raw)
To: Brandon Casey; +Cc: Git Mailing List
In-Reply-To: <4787B15D.3070105@nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
>
>
> Sometimes the repository to link to is not under your control.
> If it contains files or unreadable directories, git-relink will
> die without this patch.
I am not so sure if dying is a bad behaviour, if it is because
you are trying to link against an object store that you may not
be able to read. I actually think we should actively refuse to,
in order to prevent future problems. After seeing the command
die, you will talk to the owner of that "master" object store
and ask him to fix permissions (or he may choose to say "please
do not share with me").
However, if the "master" object store has pruned its loose
objects since you read @hashdirs out of it, that opendir() can
fail, and we should silently ignore the whole $subdir.
^ permalink raw reply
* Re: Decompression speed: zip vs lzo
From: Sam Vilain @ 2008-01-12 1:52 UTC (permalink / raw)
To: Linus Torvalds
Cc: Nicolas Pitre, Pierre Habouzit, Git Mailing List,
Johannes Schindelin, Marco Costalba, Junio C Hamano
In-Reply-To: <alpine.LFD.1.00.0801110759160.3148@woody.linux-foundation.org>
Linus Torvalds wrote:
>
> On Fri, 11 Jan 2008, Sam Vilain wrote:
>> The difference seems only barely measurable;
>
> Ok.
>
> It may be that it might help other cases, but that seems unlikely.
>
> The more likely answer is that it's either of:
>
> - yes, zlib uncompression is noticeable in profiles, but that the
> cold-cache access is simply the bigger problem, and getting rid of zlib
> just moves the expense to whatever other thing that needs to access it
> (memcpy, xdelta apply, whatever)
>
> or
>
> - I don't know exactly which patch you used (did you just do the
> "core.deltacompression=0" thing?), and maybe zlib is fairly expensive
> even for just the setup crud, even when it doesn't really need to be.
>
> but who knows..
Well, my figures agree with Pierre I think - 6-10% time savings for
'git annotate'.
I think Pierre has hit the nail on the head - that skipping
compression for small objects is a clear win. He saw the obvious
criterion, really. I've knocked it up as a config option that doesn't
change the default behaviour below.
I can't help but speculate what benefits having a range of one or two
of the most elite compression algorithms (eg, lzop or even lzma for
the larger blobs) available would be, in general. eg, if gzip takes a
stream longer than X kb to offer substantial benefits over lzop, lzop
the ones shorter than that.
If the uncompressed objects are clustered in the pack, then they might
stream compress a lot better, should they be tranmitted over a http
transport with gzip encoding. In packs which should be as small as
possible, with a format change they could be distributed as one
compressed resource. The ordering of the objects would ideally be
selected such that it results in optimum compression - which could add
a savings akin to bzip2 vs gzip, at the expense of having to scan the
small objects for mini-deltas and arrange them clustering objects
which share these mini-deltas.
Well, interesting ideas anyway :)
Subject: [PATCH] pack-objects: add compressionMinSize option
Objects smaller than a page don't save much space when compressed, and
cause some overhead. Allow the user to specify a minimum size for
objects before they are compressed.
Credit: Pierre Habouzit <madcoder@debian.org>
Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
---
Documentation/config.txt | 5 +++++
builtin-pack-objects.c | 7 ++++++-
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1b6d6d6..245121e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -734,6 +734,11 @@ pack.compression::
compromise between speed and compression (currently equivalent
to level 6)."
+pack.compressionMinSize::
+ Objects smaller than this are not compressed. This can make
+ operations that deal with many small objects (such as log)
+ faster.
+
pack.deltaCacheSize::
The maximum memory in bytes used for caching deltas in
linkgit:git-pack-objects[1].
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index a39cb82..316b809 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -76,6 +76,7 @@ static int num_preferred_base;
static struct progress *progress_state;
static int pack_compression_level = Z_DEFAULT_COMPRESSION;
static int pack_compression_seen;
+static int compression_min_size = 0;
static unsigned long delta_cache_size = 0;
static unsigned long max_delta_cache_size = 0;
@@ -433,7 +434,7 @@ static unsigned long write_object(struct sha1file *f,
}
/* compress the data to store and put compressed length in datalen */
memset(&stream, 0, sizeof(stream));
- deflateInit(&stream, pack_compression_level);
+ deflateInit(&stream, size >= compression_min_size ? pack_compression_level : 0);
maxsize = deflateBound(&stream, size);
out = xmalloc(maxsize);
/* Compress it */
@@ -1841,6 +1842,10 @@ static int git_pack_config(const char *k, const char *v)
pack_compression_seen = 1;
return 0;
}
+ if (!strcmp(k, "pack.compressionminsize")) {
+ compression_min_size = git_config_int(k, v);
+ return 0;
+ }
if (!strcmp(k, "pack.deltacachesize")) {
max_delta_cache_size = git_config_int(k, v);
return 0;
--
1.5.3.7.2095.gb2448-dirty
^ permalink raw reply related
* Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.
From: Junio C Hamano @ 2008-01-12 1:53 UTC (permalink / raw)
To: Stephen Sinclair; +Cc: git
In-Reply-To: <9b3e2dc20801111733o477b3aadv6ee76d3aafade54a@mail.gmail.com>
"Stephen Sinclair" <radarsat1@gmail.com> writes:
>> * If AUTHOR_NAME+EMAIL is different from AUTHOR_NAME+EMAIL that
>> I would normally get for myself, or
>
> I thought of this, however if the purpose of this is to handle a case
> where you do a commit from a new and unconfigured user account, "that
> I would normally get for myself" is undefined, since this information
> is (rightfully) not propagated by git-clone. This is why I made it
> unconditional, (or perhaps something you could could turn off, but
> would by default be on), but I figured there would be objections since
> I admit it's not always useful information.
What are you talking about?
In a properly configured repository, telling you who git thinks
you are is _ALWAYS_ useless (that's the definition of "properly
configured"). Just admit it.
The only case it is of any use is to remind people who amend
other people's change. Showing the AUTHOR for the commit being
created would add value (and the knowledge that git shows AUTHOR
in that situation would help remind you that it will be
recording your own name if you do not see that line).
>> * If AUTHOR_NAME+EMAIL contains garbage identifier commonly
>> found when misconfigured (e.g. ".(none)" at the end of
>> e-mail),
>
> That's more interesting to me. I just checked my logs and I do see
> that in at least one case, this .(none) was not appended. The
> computer in question was configured (not by me) with a domain of
> ".local", so the commit has <machinename>.local as part of the email
> address. However I would imagine this might solve most cases.
Yes, and please notice that "e.g." in my description means "I am
just giving you an example, not the exhaustive list for the
final solution but a hint to one possibly acceptable solution".
".local", "@localhost", "@<distroname>" and ".(none)" are all
plausible red-flag raisers. There may be more, but I think we
should be able to catch most misconfigurations with simple
rules.
> I still don't understand why git generates a default email address
> instead of just giving an error message; do people actually use this
> scenario?
The official party line to defend the existing behaviour is that
there is no need to configure anything, when the host and gecos
is done properly. But I tend to agree with you that quite a lot
of systems are not "done properly", and users cannot do much
about it in some cases. I think most of misconfigured systems
are personal boxes they have control over but not all.
Perhaps we could disable the code that reads from hostname and
gecos, and instead always force the users to configure. But
that kind of change is not something I'd want to be discussing
right now.
^ permalink raw reply
* Re: [PATCH] manpages: linking all mail-related commands
From: Humberto Diogenes @ 2008-01-12 1:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vy7avzwx4.fsf@gitster.siamese.dyndns.org>
Wow! I wasn't expecting that amount of reply for such a simple
patch. :-) I just found it unnecessarily hard to discover all the
email-related commands in git (even though I already used some of
them). My point-of-view is that some additional links wouldn't hurt.
More answers below.
On 11/01/2008, at 21:53, Junio C Hamano wrote:
> Humberto Diogenes <humberto@digi.com.br> writes:
>
>> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
>> index e4a6b3a..fd00fc1 100644
>> --- a/Documentation/git-am.txt
>> +++ b/Documentation/git-am.txt
>> @@ -144,8 +144,10 @@ names.
>>
>> SEE ALSO
>> --------
>> -gitlink:git-apply[1].
>> -
>> +gitlink:git-apply[1],
>> +gitlink:git-format-patch[1],
>> +gitlink:git-imap-send[1],
>> +gitlink:git-send-email[1]
>
> I do not see a point in this. "am" is a tool for people who
> accept and they do not care how the sender prepared
> (format-patch) nor sent (imap-send nor send-email).
Well, if I'm reading the docs about how a program works with some
input, I'll sure want to know which commands in the package generate
that kind of output. I think at least git-format-patch deserves a link
(am: read, format-patch: write).
And yes, I managed to find git-am and git-send-email but couldn't
remember the name of git-format-patch.
> On the other hand, as am uses mailinfo and mailsplit, it may be
> worth mentioning them (although I suspect not all the readers of
> manual page of am are interested in such a low level details).
I think it wouldn't hurt.
>> diff --git a/Documentation/git-apply.txt b/Documentation/git-
>> apply.txt
>> index c1c54bf..53fa937 100644
>> --- a/Documentation/git-apply.txt
>> +++ b/Documentation/git-apply.txt
>> @@ -189,6 +189,10 @@ If --index is not specified, then the
>> submodule commits in the patch
>> are ignored and only the absence of presence of the corresponding
>> subdirectory is checked and (if possible) updated.
>>
>> +See Also
>> +--------
>> +gitlink:git-am[1]
>
> Why? apply is not about email at all. am uses apply but not
> the other way around.
Imagine you don't know git, but know there's some command to apply
patches from mailboxes. Which one would you try first: git-am or git-
apply? "git-am" just doesn't say anything (for beginners, at least).
> This feels you are going a bit overboard, as if you are adding
> "See Also: git[7]" everywhere (even though it is not that bad).
Again: wouldn't hurt. :)
>> -See Also
>> +SEE ALSO
>> --------
>
> If you are standardizing between "SEE ALSO" and "See Also", I
> think that is a worthy thing to do independent from the
> additional links, but (1) please be consistent --- you tried to
> add "See Also" yoruself above, (2) please have a separate patch
> that does _ONLY_ the standardization to "SEE ALSO", and not
> limited to commands that has (maybe remotely) something to do
> with emailed patch workflow.
>
> Right now, I count 14 "SEE ALSO" and 17 "See Also". 127 spell
> "Author" and 5 spell "AUTHOR". Everybody says "NAME", "SYNOPSIS",
> "DESCRIPTION", and "OPTIONS".
>
> I think we should spell these in all uppercase.
Yes, I could see this one coming. Sorry.
--
Humberto Diógenes
http://humberto.digi.com.br
^ permalink raw reply
* Re: Re-casing directories on case-insensitive systems
From: Kevin Ballard @ 2008-01-12 1:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vprw7zv7s.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1766 bytes --]
On Jan 11, 2008, at 8:30 PM, Junio C Hamano wrote:
>> Apparently so. By Junio's definition, HFS+ is not a sane filesystem,
>> and as git grows more popular with OS X users, this issue is going to
>> crop up more frequently.
>
> It's not "my" definition, but you asked the reason and I gave
> the answer. We can close this issue of "is HFS+ sane" now.
> HFS+ is insane, period. And as Linus said, you cannot forgive
> its insanity using the historical baggage argument, like MS-DOS.
Fair enough, though I believe OS X has a good reason, namely it's an
OS designed for regular users rather than servers or programmers. Case-
sensitivity would confuse my mother.
> HOWEVER.
>
> It is a totally different issue if we want to refuse supporting
> insane filesystems. And the answer is no. It was not my
> intention to say that we do not intend to support them, when I
> explained the reason why the things are as they are, which was
> the original question by you.
Ok. I wasn't implying anything with that phrase there, I was just
trying to reiterate that HFS+ is case-insensitive and emphasize that
this issue will become more relevant as time goes by.
> See Robin's proposal to let us translate random names we get
> back from readdir() from the filesystem using an additional
> look-up table in the index extension section that stores mapping
> from canonicalized form to the form that the user registered to
> the index. I think that is a sane approach to tackle this issue
> on insane filesystems like HFS+.
If I knew what the index extension section was, perhaps I would think
that's a good idea ;) I have yet to dive into the gory details of how
this stuff works.
--
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2432 bytes --]
^ permalink raw reply
* Re: [PATCH] - Updated usage and simplified sub-command action invocation
From: Junio C Hamano @ 2008-01-12 1:38 UTC (permalink / raw)
To: Imran M Yousuf; +Cc: git
In-Reply-To: <7bfdc29a0801092341j60dcb081xe4bf6c22cbaf30f2@mail.gmail.com>
"Imran M Yousuf" <imyousuf@gmail.com> writes:
>> > Actually module_$command is not possible because only add's module is
>> > module_add rest are modules_$command....
>>
>> Is there a fundamental reason why you cannot rename them to be
>> more consistent?
>
> In fact it is consistent, add works on a single module only, whereas
> rest of the command works either on 1 or more. Thus having plural
> (modules) is logical.
It certainly is consistent in _that_ meaning of the word, but I
was not talking about that consistency, which is less useful in
this context.
The consistency I was talking about was "A subcommand called $foo
is always handled by a shell function called cmd_$foo". That is
also a consistency, and it is of much more useful kind in a
situation like this, namely, a command dispatcher.
If you have show_blobs() and show_commit() subroutines, former
of which takes 1 or more blobs while the latter of which can
only take 1 commit, being consistent in your meaning might help
the programmers avoiding a mistake to pass two or more commits
to a non-existent show_commits(). In that sense, your kind of
consistency is not totally useless.
However, it is not so useful in a context where there is one
call site for each of the functions, like a command dispatcher
scenario.
^ permalink raw reply
* Re: [PATCH] Add committer and author names to top of COMMIT_EDITMSG.
From: Stephen Sinclair @ 2008-01-12 1:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbq7r28qo.fsf@gitster.siamese.dyndns.org>
> * If AUTHOR_NAME+EMAIL is different from AUTHOR_NAME+EMAIL that
> I would normally get for myself, or
I thought of this, however if the purpose of this is to handle a case
where you do a commit from a new and unconfigured user account, "that
I would normally get for myself" is undefined, since this information
is (rightfully) not propagated by git-clone. This is why I made it
unconditional, (or perhaps something you could could turn off, but
would by default be on), but I figured there would be objections since
I admit it's not always useful information.
> * If AUTHOR_NAME+EMAIL contains garbage identifier commonly
> found when misconfigured (e.g. ".(none)" at the end of
> e-mail),
That's more interesting to me. I just checked my logs and I do see
that in at least one case, this .(none) was not appended. The
computer in question was configured (not by me) with a domain of
".local", so the commit has <machinename>.local as part of the email
address. However I would imagine this might solve most cases.
I still don't understand why git generates a default email address
instead of just giving an error message; do people actually use this
scenario? In my experience an email address must always be explicitly
given, but perhaps some people work on the machines that also receive
their mail. I rarely do "real" work on an actual server, but I guess
some people do. I think they must be in the minority though..
On the other hand, now that I've been thinking about it I think my
idea of simply configuring a hook in my personal central git is
probably an easier and all-round better solution to my problem. I
understand that git relies on system accounts for security, but
there's no reason I can't configure a particular repo to issue a
warning when it receives incoming commits from an unknown user/email.
Steve
^ permalink raw reply
* Re: Re-casing directories on case-insensitive systems
From: Junio C Hamano @ 2008-01-12 1:30 UTC (permalink / raw)
To: Kevin Ballard; +Cc: git
In-Reply-To: <B2968E86-D43E-4F5E-921F-DF8A15886C98@sb.org>
Kevin Ballard <kevin@sb.org> writes:
> On Jan 11, 2008, at 7:40 PM, Johannes Schindelin wrote:
>
>> On Fri, 11 Jan 2008, Junio C Hamano wrote:
>>
>>> Kevin Ballard <kevin@sb.org> writes:
>>>
>>>> Is there a reason for this? It seems like it would be trivial to end
>>>> up with misdiagnosed "untracked" files when using any language other
>>>> than English given this behaviuor.
>>>
>>> No. The assumption of the code has always been that sane filesystems
>>> would return from readdir() the names you gave from creat().
>>
>> We do not really have to rehash that whole discussion for the Nth
>> time, do
>> we?
>
> Apparently so. By Junio's definition, HFS+ is not a sane filesystem,
> and as git grows more popular with OS X users, this issue is going to
> crop up more frequently.
It's not "my" definition, but you asked the reason and I gave
the answer. We can close this issue of "is HFS+ sane" now.
HFS+ is insane, period. And as Linus said, you cannot forgive
its insanity using the historical baggage argument, like MS-DOS.
HOWEVER.
It is a totally different issue if we want to refuse supporting
insane filesystems. And the answer is no. It was not my
intention to say that we do not intend to support them, when I
explained the reason why the things are as they are, which was
the original question by you.
See Robin's proposal to let us translate random names we get
back from readdir() from the filesystem using an additional
look-up table in the index extension section that stores mapping
from canonicalized form to the form that the user registered to
the index. I think that is a sane approach to tackle this issue
on insane filesystems like HFS+.
^ permalink raw reply
* Re: Re-casing directories on case-insensitive systems
From: Kevin Ballard @ 2008-01-12 1:16 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.LSU.1.00.0801120139170.8333@wbgn129.biozentrum.uni-wuerzburg.de>
[-- Attachment #1: Type: text/plain, Size: 1600 bytes --]
On Jan 11, 2008, at 7:40 PM, Johannes Schindelin wrote:
> On Fri, 11 Jan 2008, Junio C Hamano wrote:
>
>> Kevin Ballard <kevin@sb.org> writes:
>>
>>> Is there a reason for this? It seems like it would be trivial to end
>>> up with misdiagnosed "untracked" files when using any language other
>>> than English given this behaviuor.
>>
>> No. The assumption of the code has always been that sane filesystems
>> would return from readdir() the names you gave from creat().
>
> We do not really have to rehash that whole discussion for the Nth
> time, do
> we?
Apparently so. By Junio's definition, HFS+ is not a sane filesystem,
and as git grows more popular with OS X users, this issue is going to
crop up more frequently.
According to the HFS+ Volume Format technote[1], filenames in HFS+ are
stored in normalized, canonical order. To be more specific, they're
stored in a special apple variant of Unicode Normal Form D (the
special variant is for preserving round-trip with older encodings with
certain codepoint ranges[2]).
In other words, if you hand an HFS+ filesystem a filename that
contains unicode characters, what you get back later may be in a
different format. And that's going to be a problem if git doesn't deal
with this.
[1]: http://developer.apple.com/technotes/tn/tn1150.html#UnicodeSubtleties
[2]: http://developer.apple.com/qa/qa2001/qa1173.html
Note: CC list stripped because this is a re-sent email, as the list
bounced the last one that contained a text/html alternate.
--
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2432 bytes --]
^ permalink raw reply
* Re: git-commit fatal: Out of memory? mmap failed: Bad file descriptor
From: Junio C Hamano @ 2008-01-12 1:08 UTC (permalink / raw)
To: Brandon Casey, Shawn O. Pearce; +Cc: Git Mailing List, drafnel
In-Reply-To: <47880D18.8030405@nrlssc.navy.mil>
Brandon Casey <casey@nrlssc.navy.mil> writes:
> Junio C Hamano wrote:
>> Brandon Casey <casey@nrlssc.navy.mil> writes:
>>
>>> It's reproduceable for me by amending the commit.
>>
>> Reliably reproducible? Can you build with "-O0 -g" and run
>> "commit --amend" under gdb?
>>
>
> make NO_CURL=1 CFLAGS='-O0 -g'
> Done.
>
> I also moved xmmap into commit.c, and turned the inlined definition
> in git-compat-util.h into a declaration.
>
>
> I set a breakpoint on xmmap(). This is the backtrace on the last entry
> into xmmap() before it die()'ed.
>
> The fstat message at the end is from a call to fstat that I added to
> print out the file size (to compare with mmap length). As you can
> see the fstat also fails with the 'Bad file descriptor' message.
>
> #0 xmmap (start=0x0, length=996168, prot=1, flags=2, fd=6, offset=0)
> at commit.c:680
> #1 0x080acf30 in use_pack (p=0x8150650, w_cursor=0xffffc0ac, offset=94828,
> left=0xffffc06c) at sha1_file.c:748
That's the pack window shuffling code in use_pack(). I presume
your additional fstat is inside xmmap(), so if p->pack_fd is
already closed when this xmmap() call is made, that would
explain the symptom.
while (packed_git_limit < pack_mapped
&& unuse_one_window(p, p->pack_fd))
; /* nothing */
win->base = xmmap(NULL, win->len,
PROT_READ, MAP_PRIVATE,
p->pack_fd, win->offset);
I wonder what's the best way to find out who closes file
descriptor #6 without clearing p->pack_fd that still holds #6?
My reading of unuse_one_window() is that it tried to avoid
closing p->pack_fd, so it may already have been closed when we
get to this codepath.
Shawn, does this ring a bell?
^ permalink raw reply
* Re: Re-casing directories on case-insensitive systems
From: Robin Rosenberg @ 2008-01-12 0:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Kevin Ballard, Johannes Schindelin, git
In-Reply-To: <7v3at3280v.fsf@gitster.siamese.dyndns.org>
lördagen den 12 januari 2008 skrev Junio C Hamano:
> Robin Rosenberg <robin.rosenberg@dewire.com> writes:
>
> > Could we just have a lookup table index extension for identifying the
> > duplicates (when checking is enabled using core configuration option #3324)?
> > That table would keep a mapping from a normalized form (maybe include
> > canonical encoding while we're at it) to the actual octet sequence(s) used.
>
> I would agree that the index extension, if we ever are going to
> do this, would be the right place to store this information, at
> the single repository level.
>
> However, this opens up a can of worms. What's the canonical key
> should be? If you want to protect yourself from a unicode
> normalizing filesystem, you would use one canonicalization,
> while if you want to protect from a case losing filesystem you
> would use another? Or do we at the same time downcase and NFD
> normalize at the same time and be done with it?
The worms are out already. So the question is whether there
is a way of keeping them in the can instead of having them crawl
all around. I think we could to both unicode (UTF-8 or NFD) and
downcase at the same time.
> And where should the configuration be stored? If a project
> wants to be interoperable across Linux and vfat, for example,
In the brand new ".gitconfig". It could in principle contain any config option,
but that would not be safe so I guess one should only allow "safe" options
there.
-- robin
^ permalink raw reply
* Re: [PATCH] manpages: linking all mail-related commands
From: Junio C Hamano @ 2008-01-12 0:53 UTC (permalink / raw)
To: Humberto Diogenes; +Cc: git
In-Reply-To: <1200095286-2621-1-git-send-email-humberto@digi.com.br>
Humberto Diogenes <humberto@digi.com.br> writes:
> diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
> index e4a6b3a..fd00fc1 100644
> --- a/Documentation/git-am.txt
> +++ b/Documentation/git-am.txt
> @@ -144,8 +144,10 @@ names.
>
> SEE ALSO
> --------
> -gitlink:git-apply[1].
> -
> +gitlink:git-apply[1],
> +gitlink:git-format-patch[1],
> +gitlink:git-imap-send[1],
> +gitlink:git-send-email[1]
I do not see a point in this. "am" is a tool for people who
accept and they do not care how the sender prepared
(format-patch) nor sent (imap-send nor send-email).
On the other hand, as am uses mailinfo and mailsplit, it may be
worth mentioning them (although I suspect not all the readers of
manual page of am are interested in such a low level details).
> diff --git a/Documentation/git-apply.txt b/Documentation/git-apply.txt
> index c1c54bf..53fa937 100644
> --- a/Documentation/git-apply.txt
> +++ b/Documentation/git-apply.txt
> @@ -189,6 +189,10 @@ If --index is not specified, then the submodule commits in the patch
> are ignored and only the absence of presence of the corresponding
> subdirectory is checked and (if possible) updated.
>
> +See Also
> +--------
> +gitlink:git-am[1]
Why? apply is not about email at all. am uses apply but not
the other way around.
This feels you are going a bit overboard, as if you are adding
"See Also: git[7]" everywhere (even though it is not that bad).
> -See Also
> +SEE ALSO
> --------
If you are standardizing between "SEE ALSO" and "See Also", I
think that is a worthy thing to do independent from the
additional links, but (1) please be consistent --- you tried to
add "See Also" yoruself above, (2) please have a separate patch
that does _ONLY_ the standardization to "SEE ALSO", and not
limited to commands that has (maybe remotely) something to do
with emailed patch workflow.
Right now, I count 14 "SEE ALSO" and 17 "See Also". 127 spell
"Author" and 5 spell "AUTHOR". Everybody says "NAME", "SYNOPSIS",
"DESCRIPTION", and "OPTIONS".
I think we should spell these in all uppercase.
^ permalink raw reply
* Re: git-commit fatal: Out of memory? mmap failed: Bad file descriptor
From: Brandon Casey @ 2008-01-12 0:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, drafnel
In-Reply-To: <7vve5z2abv.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> Brandon Casey <casey@nrlssc.navy.mil> writes:
>
>> It's reproduceable for me by amending the commit.
>
> Reliably reproducible? Can you build with "-O0 -g" and run
> "commit --amend" under gdb?
>
make NO_CURL=1 CFLAGS='-O0 -g'
Done.
I also moved xmmap into commit.c, and turned the inlined definition
in git-compat-util.h into a declaration.
I set a breakpoint on xmmap(). This is the backtrace on the last entry
into xmmap() before it die()'ed.
The fstat message at the end is from a call to fstat that I added to
print out the file size (to compare with mmap length). As you can
see the fstat also fails with the 'Bad file descriptor' message.
#0 xmmap (start=0x0, length=996168, prot=1, flags=2, fd=6, offset=0)
at commit.c:680
#1 0x080acf30 in use_pack (p=0x8150650, w_cursor=0xffffc0ac, offset=94828,
left=0xffffc06c) at sha1_file.c:748
#2 0x080ae169 in unpack_object_header (p=0x8150650, w_curs=0xffffc0ac,
curpos=0xffffc0a0, sizep=0xffffc1d0) at sha1_file.c:1333
#3 0x080ae8bb in unpack_entry (p=0x8150650, obj_offset=94828,
type=0xffffc1dc, sizep=0xffffc1d0) at sha1_file.c:1595
#4 0x080ae55d in cache_or_unpack_entry (p=0x8150650, base_offset=94828,
base_size=0xffffc1d0, type=0xffffc1dc, keep_cache=1) at sha1_file.c:1490
#5 0x080af057 in read_packed_sha1 (
sha1=0xffffc1b0 "��\034\f~\023\203��E=�$n~��X��@�220",
type=0xffffc1dc, size=0xffffc1d0) at sha1_file.c:1815
#6 0x080af2cf in read_sha1_file (
sha1=0xffffc1b0 "��\034\f~\023\203��E=�$n~��X��@�220",
type=0xffffc1dc, size=0xffffc1d0) at sha1_file.c:1881
#7 0x080af3a0 in read_object_with_reference (
sha1=0x853cf18 "��\034\f~\023\203��E=�$n~��40000 libapsrs",
required_type_name=0x80f8488 "tree", size=0xffffc20c,
actual_sha1_return=0x0) at sha1_file.c:1910
#8 0x080cd2d0 in diff_tree_sha1 (
old=0x853cf18 "��\034\f~\023\203��E=�$n~��40000 libapsrs",
new=0x853d108 "��\036\034_�006�\f\025\236{�220StM0�40000 libapsrs",
base=0x8534098 "aps/src/libapsnav/", opt=0xffffc694) at tree-diff.c:376
#9 0x080cc9cb in compare_tree_entry (t1=0xffffc330, t2=0xffffc310,
base=0x815a260 "aps/src/", baselen=8, opt=0xffffc694) at tree-diff.c:61
#10 0x080ccf93 in diff_tree (t1=0xffffc330, t2=0xffffc310,
base=0x815a260 "aps/src/", opt=0xffffc694) at tree-diff.c:278
#11 0x080cd371 in diff_tree_sha1 (
old=0x853cbe0 "N|\021xH/K<R�\025��\2250�00644 stamp-h.in",
new=0x853cdb0 "\0027w���}��Ƴ\213\036±|100644 stamp-h.in",
base=0x815a260 "aps/src/", opt=0xffffc694) at tree-diff.c:384
---Type <return> to continue, or q <return> to quit---
#12 0x080cc9cb in compare_tree_entry (t1=0xffffc430, t2=0xffffc410,
base=0x817d3b8 "aps/", baselen=4, opt=0xffffc694) at tree-diff.c:61
#13 0x080ccf93 in diff_tree (t1=0xffffc430, t2=0xffffc410,
base=0x817d3b8 "aps/", opt=0xffffc694) at tree-diff.c:278
#14 0x080cd371 in diff_tree_sha1 (
old=0x853c580 "\037\215��\217\200�E�b��232�03640000 avhrr",
new=0x853c848 "g\230\032a \207V�s~��r\177�23540000 avhrr",
base=0x817d3b8 "aps/", opt=0xffffc694) at tree-diff.c:384
#15 0x080cc9cb in compare_tree_entry (t1=0xffffc530, t2=0xffffc510,
base=0x80faba8 "", baselen=0, opt=0xffffc694) at tree-diff.c:61
#16 0x080ccf93 in diff_tree (t1=0xffffc530, t2=0xffffc510, base=0x80faba8 "",
opt=0xffffc694) at tree-diff.c:278
#17 0x080cd371 in diff_tree_sha1 (
old=0x813c3dc "\231�2274M�\236�\t?�\225\t��",
new=0x813c43c "*!�\200\006�\235�?t��:DR", base=0x80faba8 "",
opt=0xffffc694) at tree-diff.c:384
#18 0x080d11d0 in log_tree_diff (opt=0xffffc610, commit=0x813c438,
log=0xffffc5c0) at log-tree.c:378
#19 0x080d125a in log_tree_commit (opt=0xffffc610, commit=0x813c438)
at log-tree.c:402
#20 0x0805d26b in print_summary (prefix=0x0,
sha1=0xffffd7e0 "*!�\200\006�\235�?t��:DR") at builtin-commit.c:709
#21 0x0805dae7 in cmd_commit (argc=0, argv=0xffffd9e8, prefix=0x0)
at builtin-commit.c:898
#22 0x0804b44b in run_command (p=0x80fd468, argc=3, argv=0xffffd9e8)
at git.c:257
#23 0x0804b5f9 in handle_internal_command (argc=3, argv=0xffffd9e8)
at git.c:383
#24 0x0804b75c in main (argc=3, argv=0xffffd9e8) at git.c:447
(gdb) c
Continuing.
fstat failed: Bad file descriptor
fatal: Out of memory? mmap failed: Bad file descriptor
Created commit
Program exited with code 0200.
(gdb)
^ permalink raw reply
* Re: Re-casing directories on case-insensitive systems
From: Johannes Schindelin @ 2008-01-12 0:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Ballard, Robin Rosenberg, Linus Torvalds, git
In-Reply-To: <7v7iif28i2.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 11 Jan 2008, Junio C Hamano wrote:
> Kevin Ballard <kevin@sb.org> writes:
>
> > Is there a reason for this? It seems like it would be trivial to end
> > up with misdiagnosed "untracked" files when using any language other
> > than English given this behaviuor.
>
> No. The assumption of the code has always been that sane filesystems
> would return from readdir() the names you gave from creat().
We do not really have to rehash that whole discussion for the Nth time, do
we?
Ciao,
Dscho
^ 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