* Re: git-svn error "Unable to extract revision information from commit ...~1"
From: Steven Grimm @ 2007-06-27 20:25 UTC (permalink / raw)
To: Patrick Doyle; +Cc: 'git'
In-Reply-To: <e2a1d0aa0706271031n3191a27fi41ca5ba3f14c3a51@mail.gmail.com>
Patrick Doyle wrote:
> Wouldn't the svn head be called "git-svn"?
> Should the first checkout be written as:
>
> git checkout -b tempbranch git-svn
>
> (In other words, were the "trunk" and "tempbranch" arguments swapped
> in your example?)
Yes, sorry, I swapped the arguments. Should indeed be "git checkout -b
(your new branch name) (svn head branch name)" rather than the other way
around. My apologies for not proofreading my message.
However, the name of the svn head branch depends on the version of
git-svn you're using, and how you cloned the svn repository. If you used
the -T/--trunk option, then the svn head branch will indeed be called
"trunk" (and you might also have other branches corresponding to
whatever branches you're tracking in the svn repository.) If you're not
using that option, then your svn tracking branch will be called
"git-svn" or "origin" depending on your git-svn version.
> Finally, what is the purpose of creating a new branch as part of that
> command?
You never ever want to work directly on the git-svn tracking branch. It
is considered a remote branch and while it is technically possible to
check it out and commit to it, you will almost certainly confuse git-svn
if you do.
Also, creating a new branch means you still have your original nonlinear
history easily accessible if you need it (just check out your old
branch.) You can certainly still get at the old history without creating
a new branch, but it's more awkward.
-Steve
^ permalink raw reply
* [StGIT PATCH] Display a message when the server refused some recipients.
From: Yann Dirson @ 2007-06-27 19:48 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
When the smtp server directly refuses some recipients, an exception is
only raised when no recipient got the parcel. This patch displays the
recipients which were denied.
Subsequent delivery is still done, no change here.
Signed-off-by: Yann Dirson <ydirson@altern.org>
---
stgit/commands/mail.py | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/stgit/commands/mail.py b/stgit/commands/mail.py
index 899cb1a..04bdf02 100644
--- a/stgit/commands/mail.py
+++ b/stgit/commands/mail.py
@@ -176,7 +176,9 @@ def __send_message(smtpserver, from_addr, to_addr_list, msg, sleep,
s.ehlo()
s.login(smtpuser, smtppassword)
- s.sendmail(from_addr, to_addr_list, msg)
+ result = s.sendmail(from_addr, to_addr_list, msg)
+ if len(result):
+ print "mail server refused delivery for the following recipients: %s" % result
# give recipients a chance of receiving patches in the correct order
time.sleep(sleep)
except Exception, err:
^ permalink raw reply related
* [PATCH] git-submodule: Try harder to describe the status of a submodule
From: Emil Medve @ 2007-06-27 19:13 UTC (permalink / raw)
To: git; +Cc: Emil Medve
Some repositories might not use/have annotated tags (for example repositories created with
git-cvsimport) or might not have tags at all and could cause git-submodule status to fail because
git-describe might fail.
This change makes git-submodule status try harder in displaying the status of a module by
considering lightweight tags, subsequent tags and branches.
Signed-off-by: Emil Medve <Emilian.Medve@Freescale.com>
---
git-submodule.sh | 23 +++++++++++++++++++++--
1 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/git-submodule.sh b/git-submodule.sh
index 89a3885..7429ce3 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -134,6 +134,25 @@ modules_update()
}
#
+# Identify and display the most specific name describing an object specified by a SHA1 key
+#
+# First try to locate a tag that predates the commit. If that fails try to locate a tag that comes
+# after the commit. If that fails too (e.g. undefined), try to locate the branch of the commit. If
+# that fails too just display "undefined"
+#
+get_revname()
+{
+ _revname=$(git-describe --tags "$1" 2>/dev/null || git-describe --contains "$1" 2>/dev/null)
+ if test -z "$_revname" -o "$_revname" = "undefined"
+ then
+ _revname=$(git-describe --all "$1" 2>/dev/null | cut -d / -f2-)
+ test -z "$_revname" && _revname=undefined
+ fi
+ echo $_revname
+ unset _revname
+}
+
+#
# List all submodules, prefixed with:
# - submodule not initialized
# + different revision checked out
@@ -155,7 +174,7 @@ modules_list()
say "-$sha1 $path"
continue;
fi
- revname=$(unset GIT_DIR && cd "$path" && git-describe $sha1)
+ revname=$(unset GIT_DIR && cd "$path" && get_revname $sha1)
if git diff-files --quiet -- "$path"
then
say " $sha1 $path ($revname)"
@@ -163,7 +182,7 @@ modules_list()
if test -z "$cached"
then
sha1=$(unset GIT_DIR && cd "$path" && git-rev-parse --verify HEAD)
- revname=$(unset GIT_DIR && cd "$path" && git-describe $sha1)
+ revname=$(unset GIT_DIR && cd "$path" && get_revname $sha1)
fi
say "+$sha1 $path ($revname)"
fi
--
1.5.2.2.549.gaeb59
^ permalink raw reply related
* post-apply hook or something like that
From: Nicolas Vilz @ 2007-06-27 18:17 UTC (permalink / raw)
To: git
Hello fellow gits,
I try to insert metastore into my workflow and i wonder if i could get
the following scenario working:
(1) cd /etc/ && git-init-db # start gittify etc-directory
(2) git commit -a # fire up first commit
(3) metastore -s # save current state of permissions and ownerships
(4) git commit -a # commit the changes
(5) chown nobody bash/<somefile> # change ownership
(6) git commit -a # with the pre-commit shipped with metastore this is tracked
(7) git reset --hard HEAD^1 # go back one step
(8) git reset --hard <former-sha1-sum> # go forward again
(9) metastore -a # apply the changes from last commit
what i was wondering, is it possible, to get a metastore -a
automatically after checking out something?
Greetings
Nicolas
^ permalink raw reply
* If anyone feels like hacking git-rev-list --bisect...
From: linux @ 2007-06-27 18:53 UTC (permalink / raw)
To: git
I'm trying to bisect kernel bug A, but I keep running into kernel bug
B which causes a crash on boot. (But was fixed, so I don't need to think
about it much.)
that is, my revision history is something like:
G...........G......XXXXXXXXXX.....B...............B
G = good
B = bad
X = I can't tell
While I can do things like "git-reset --hard HEAD~250" to try to get past
the problem, it's an annoying bit of guessing to find good values, and
if my problem is one some branch, I'd rather explore a different branch.
How much nicer, I couldn't help thinking, if bisection could be told
to look for a ratio other than 1/2. Then I could ask for the 1/3 or 2/3
point instead.
That would also be helpful in cases where the "good" or "bad" cases
differ significantly in testing pain. For example, if "bad" forces
an fsck, or if "good" takes a couple of hours of stress-testing
to be really sure.
I *think* it's just a tweak to builtin-rev-list.c:halfway(), but the
code that calls that is rather intricate.
So if anyone feels like attempting it...
Adding the relevant UI to git-bisect would be relatively straightforward.
A new command to re-bisect from the initial state would allow evading
unbuildable commits, and maybe a way to set a preferred split ratio.
^ permalink raw reply
* Re: parsecvs fails even on simple input
From: Keith Packard @ 2007-06-27 17:20 UTC (permalink / raw)
To: Roman Kagan; +Cc: keithp, Al Viro, git
In-Reply-To: <20070627153300.GA27933@rkagan.sw.ru>
[-- Attachment #1: Type: text/plain, Size: 565 bytes --]
On Wed, 2007-06-27 at 19:33 +0400, Roman Kagan wrote:
> OTOH git-fast-import seems to be essentially the public API for the
> parsecvs kind of tasks. It may be wiser from the maintenance POV to use
> that instead of direct libgit calls (unless parsecvs is going to land in
> the git tree). I'll try to find the time and take a look at this
> somewhere next week.
Yeah, I didn't quite understand how git-fast-import worked. Looks like
it aligns with parsecvs's structure fairly well. Let me know if you get
it working.
--
keith.packard@intel.com
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: git-svn error "Unable to extract revision information from commit ...~1"
From: Steven Grimm @ 2007-06-27 16:57 UTC (permalink / raw)
To: Victor Bogado da Silva Lins; +Cc: 'git'
In-Reply-To: <1182947236.21320.3.camel@omicron.ep.petrobras.com.br>
Victor Bogado da Silva Lins wrote:
> I will try that, but on the other hand is it possible to commit the
> diff between my head and the svn head with a new message? This would
> be good enough for me.
Yes, that's pretty easy; you just have to do a squash merge to eliminate
the nonlinear history. Assuming the svn head is called "trunk" and your
head is called "myhead":
# start with the svn head
git checkout -b trunk tempbranch
# bring in all the changes from your branch
git merge --squash myhead
# commit with whatever message you want
git commit
# and ship it up to svn land
git svn dcommit
-Steve
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Linus Torvalds @ 2007-06-27 16:49 UTC (permalink / raw)
To: Jim Meyering; +Cc: Geert Bosch, Alex Riesen, git
In-Reply-To: <87odj1mpy2.fsf@rho.meyering.net>
On Wed, 27 Jun 2007, Jim Meyering wrote:
>
> Subject: [PATCH] git-log: detect dup and fdopen failure
> git-compat-util.h (xdup, xfdopen): Define functions.
>
> Signed-off-by: Jim Meyering <jim@meyering.net>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
That said, the whole "prepend 'x' to the function name" thing is obviously
how git does things, but maybe it would be more readable in the long term
to make the prefix be 'safe_' instead of 'x', or something like that?
The 'x' thing is the fairly traditional thing to do for malloc(), and
that's where it comes from, of course. In git (and other places: google
code shows that other projects have ended up with the same kind of things,
with busybox being one example I found), it then got extended to
xread/xwrite, but now it's getting extended so much that I'd worry a bit
that in the long run the easy-to-miss 'x' part really would be better off
written out a bit more.
No really strong opinion, just throwing it out for comment.
It was brought on by the fact that I mentally parsed "xfdopen()" as "xf" +
"dopen" for some reason (but maybe that's just me).
Linus
^ permalink raw reply
* Re: Problem with Linus's git repository?
From: Linus Torvalds @ 2007-06-27 16:34 UTC (permalink / raw)
To: walt; +Cc: git
In-Reply-To: <Pine.LNX.4.64.0706270636140.7038@x2.ybpnyarg>
On Wed, 27 Jun 2007, walt wrote:
> >
> > It's not missing, it's packed. I tend to re-pack after I make a release,
> > and this time I did it after -rc6...
>
> Sometime overnight this problem disappeared. I haven't actually
> tested this idea, but I have a hunch that your commit yesterday
> of "Fix zero-object version-2 packs" is the reason.
No, I think the reason is simply that I pushed more updates to kernel.org,
so now the kernel git repo has an explicit and separate "master" branch
again.
What is your git version? The _real_ fix probably is to just upgrade. Are
you using that broken(*) Debian git package by any chance?
Linus
(*) By broken I mean "stable cannot be bothered to upgrade packages to
modern versions, and is still on some ancient 1.4-based git version"
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Jim Meyering @ 2007-06-27 16:34 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0706270857w175f2b2ey4c30bac9c0836929@mail.gmail.com>
"Alex Riesen" <raa.lkml@gmail.com> wrote:
> On 6/27/07, Jim Meyering <jim@meyering.net> wrote:
>> "Alex Riesen" <raa.lkml@gmail.com> wrote:
>>
>> > On 6/27/07, Jim Meyering <jim@meyering.net> wrote:
>> >> "Alex Riesen" <raa.lkml@gmail.com> wrote:
>> >> > Check if this works in some exotic but common
>> >> > environments (like MacOSX, Cygwin or HP-UX).
>> >>
>> >> What do you mean?
>> >> I know that dup and fdopen work in those environments.
>> >
>> > Exactly as you described?
>>
>> No, but why does that matter?
>
> Your code will need additional tweaking on this
> systems.
Why bother? The only undesirable side effect you'd see on a buggy
system would be a meaningless strerror(errno) in that very unusual
error situation: when git-format-patch (without --stdout)
runs out of file descriptors or memory.
>> > Are you sure fdopen on Cygwin
>> > sets ENOMEM?
>>
>> Are you suggesting not to use errno because
>> it won't contain useful information on Mingw32?
>
> Yes. Mingw32 is just one example.
>
>> >> It's ok, but for the fact that when dup fails, all you get
>> >> is the uninformative EINVAL from fdopen.
>> >
>> > EBADF on Cygwin, 0 on Mingw32. Can't even imagine what msvc
>>
>> EBADF is fine. Setting errno to 0 upon error is a bug.
>> Don't cater to buggy systems.
>
> It is my system at work. What should I do, throw it out of window?
Perhaps you misunderstood?
My saying "don't cater to buggy systems", means don't pollute or dumb-down
application code to accommodate them. Instead, code to some best-case
standard (Linux, POSIX, whatever) and if necessary, use wrappers that
work around the deficiencies of less-well-endowed systems. For example,
coreutils assumes that functions like fchdir and openat work properly.
Even though some really crufty systems have no fchdir syscall, and many
relatively modern systems lack openat, fstatat, etc., all of the coreutils
programs work fine in spite of that: they rely on gnulib's emulation
of those underlying functions. This lets applications keep free of
the dirty underlying portability details and concentrate on algorithms.
Then there are the all of the wrappers for functions like stat, lstat,
rename, etc. that misbehave on certain losing (but still in use) systems.
I've been "accommodating" deficient systems for a very long time, so
know from experience that it can be done cleanly.
If this little corner of portability matters to you, it wouldn't be
hard to write dup and/or fdopen wrappers that do the right thing on
whatever buggy system you find you have to use. Incidentally, one nice
(essential) thing about the gnulib framework is that full-featured systems
usually incur *absolutely no* overhead, because they don't even use the
wrapper functions.
>> > Then put file name and the line number in!
>>
>> Surely you're playing devil's advocate, now...
>
> No, I'm just thinking aloud about whether I should start preparing
> a reverting commit for your changes just to be able to continue
> using Git on that mismanaged, stupid, slow and very real
No need to go to such an extreme...
Unless that particular diagnostic really matters to you.
> system I have to work with. It is a Win2k laptop, in a Novell
> network and with Cygwin on (actually 2 cygwins at least, which
> cygwin does not support), and without a chance to _EVER_ use
> anything else.
You have my sympathy :-/
^ permalink raw reply
* Re: [PATCH] git-rev-list: give better diagnostic for failed write
From: Linus Torvalds @ 2007-06-27 16:06 UTC (permalink / raw)
To: Jim Meyering; +Cc: git
In-Reply-To: <87tzstpyck.fsf@rho.meyering.net>
On Wed, 27 Jun 2007, Jim Meyering wrote:
>
> If someone pointed out a bug in coreutils whereby a tool failed
> to give an accurate diagnostic
The thing is, I don't think stdio output is "important" enough.
In fact, by definition, if the data was really important, it had better
not use stdio.
So look at where git uses "real" IO, and where it uses stdio, and you'll
see where the "diagnostics" matter.
In other words, the diagnostics matter for:
- the actual git database (including refs).
- doing things like "git checkout" that writes the working tree
Those are the ones where we want to *guarantee* correctness. And that
means that they don't use stdio (and if any of them do, then that really
*is* a bug, and should be fixed asap)
In contrast, if you redirect stdout, that's a totally different level of
correctness, and all bets are off. Exactly because we use stdio. If we
really cared, we shouldn't use stdio.
> Be careful when throwing stones...
> I mentioned (without making a fuss) a regression you introduced,
> http://thread.gmane.org/gmane.comp.version-control.git/50742/focus=50917
> and you included similar code in a snippet in the very message to which
> I'm replying now. I do see that you omitted the troublesome ferror call
> from your eventual patch.
Right. There's a difference between an email saying "ok, this is how you
might want to do it" and then actually sitting down and thinking about it
and coding it.
If you look closer, you'll notice that my initial suggestion wouldn't even
*compile*.
By the time I sent out a patch, I had actually thought it through.
And no, I don't guarantee that I do that all the time. I try to, but hey,
mistakes happen. It's when the mistake is repeated over and over after
being pointed out that I get irritated.
> You and I have a difference of opinion. I firmly believe that it is
> unnecessary and counterproductive (it is prohibited by POSIX for many
> of the tools I maintain) to suppress an EPIPE diagnostic and to exit
> successfully in spite of a write error.
Quite frankly, the tools you maintain are mostly filters in the first
place! In other words, for you, stdout *matters*. That's the real data!
But look at something like "cp" one of these days, and ask yourself:
- do you check error returns for the IO operations that actually do the
copy (I hope yes).
I also hope to high heaven that you would never use stdio for that IO.
Because you *will* lose error information!
- do you check the error returns of informational messages like the ones
from the verbose output switch (cp -v).
If you answered "no" to the second question, you should really think about
that fact. Please realize that there's a *huge* difference in error
checking between "core data" and "random information".
For something like "cat", the core data is the actual stream. You care a
lot more about that. For git, the core data is the working tree and the
database. I care about _that_. That's the stuff that should be protected
and must never _ever_ fail silently.
The rest is really "statistics".
Let's get it right, but unlike core data, that's mostly a politeness
issue, much less important than "oops, we just corrupted your archive".
And when we talk about politeness, avoiding the _idiotic_ error messages
like "Broken pipe" is a big part of it! The "Broken pipe" message means
either:
- the receiver already got everything he wanted (and you should damn well
shut up about it)
- the receiver had some problem (and it's damn well *that* which matters,
since the sender was fine, and you shouldn't confuse the issue by
writing bogus error messages)
The fact that you have some broken POSIX spec to worry about is not git's
problem. I hold git to higher standards.
Linus
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Alex Riesen @ 2007-06-27 15:57 UTC (permalink / raw)
To: Jim Meyering; +Cc: git
In-Reply-To: <87tzstmqmi.fsf@rho.meyering.net>
On 6/27/07, Jim Meyering <jim@meyering.net> wrote:
> "Alex Riesen" <raa.lkml@gmail.com> wrote:
>
> > On 6/27/07, Jim Meyering <jim@meyering.net> wrote:
> >> "Alex Riesen" <raa.lkml@gmail.com> wrote:
> >> > Check if this works in some exotic but common
> >> > environments (like MacOSX, Cygwin or HP-UX).
> >>
> >> What do you mean?
> >> I know that dup and fdopen work in those environments.
> >
> > Exactly as you described?
>
> No, but why does that matter?
>
Your code will need additional tweaking on this
systems.
> > Are you sure fdopen on Cygwin
> > sets ENOMEM?
>
> Are you suggesting not to use errno because
> it won't contain useful information on Mingw32?
Yes. Mingw32 is just one example.
> >> It's ok, but for the fact that when dup fails, all you get
> >> is the uninformative EINVAL from fdopen.
> >
> > EBADF on Cygwin, 0 on Mingw32. Can't even imagine what msvc
>
> EBADF is fine. Setting errno to 0 upon error is a bug.
> Don't cater to buggy systems.
It is my system at work. What should I do, throw it out of window?
> >
> > Then put file name and the line number in!
>
> Surely you're playing devil's advocate, now...
No, I'm just thinking aloud about whether I should start preparing
a reverting commit for your changes just to be able to continue
using Git on that mismanaged, stupid, slow and very real
system I have to work with. It is a Win2k laptop, in a Novell
network and with Cygwin on (actually 2 cygwins at least, which
cygwin does not support), and without a chance to _EVER_ use
anything else. And I happen to depend on Git, because the
alternative is, as someone noted, "badly mismanaged" Perforce.
And I believe I am not that bad off, there are other systems on
which you didn't test either. "Silly", as you say.
^ permalink raw reply
* Re: [PATCH] Avoid perl in t1300-repo-config
From: Alex Riesen @ 2007-06-27 15:47 UTC (permalink / raw)
To: Frank Lichtenheld; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <20070627151545.GF12721@planck.djpig.de>
On 6/27/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:
> On Wed, Jun 27, 2007 at 02:45:40PM +0200, Alex Riesen wrote:
> > It fixes the test on system where ActiveState Perl is used.
> > It is also shorter.
>
> Thanks :)
>
> For future reference: What in my code broke with ActiveState Perl?
> The command line options?
>
Nothing really, the code is perfect and options are correct.
ActiveState is just clueless, and activates \r\n line endings
on stdout/stderr by default, unless the stream is binmoded
(irrespective of whether the stream is tty or not).
^ permalink raw reply
* Re: parsecvs fails even on simple input
From: Roman Kagan @ 2007-06-27 15:33 UTC (permalink / raw)
To: Keith Packard; +Cc: Al Viro, git
In-Reply-To: <1182720667.13289.41.camel@neko.keithp.com>
On Sun, Jun 24, 2007 at 10:31:07PM +0100, Keith Packard wrote:
> On Fri, 2007-06-22 at 15:36 +0400, Roman Kagan wrote:
>
> > The problem is the following: after that commit parsecvs tries to add
> > objects to the git tree on its own via calls to libgit; however, in
> > between it runs git-pack-objects. Thus objects move to pack files
> > without libgit being aware of it; this results in 'ivalid object'
> > errors.
>
> Sticking a call to reprepare_packed_git() after the pack creation fixes
> this nicely.
Ehm sort of... Except that I woudn't call that extern declaration
nice.
I'm now tracking down another problem which I didn't see before:
parsecvs apparently doesn't close .git-cvs/log-XXX files and ends up
exhausting the open file descriptor limit. I'll update when I have more
info.
> > Wouldn't it be better to teach parsecvs
> > to speak git-fast-import language instead?
>
> Avoiding fork/exec is rather important for parsecvs perforamance.
Avoiding _one_ fork/exec is certainly not.
OTOH git-fast-import seems to be essentially the public API for the
parsecvs kind of tasks. It may be wiser from the maintenance POV to use
that instead of direct libgit calls (unless parsecvs is going to land in
the git tree). I'll try to find the time and take a look at this
somewhere next week.
Roman.
^ permalink raw reply
* Re: [PATCH] Avoid perl in t1300-repo-config
From: Frank Lichtenheld @ 2007-06-27 15:15 UTC (permalink / raw)
To: Alex Riesen; +Cc: Git Mailing List, Junio C Hamano
In-Reply-To: <81b0412b0706270545w65ca2556yaafaac6ff31b5961@mail.gmail.com>
On Wed, Jun 27, 2007 at 02:45:40PM +0200, Alex Riesen wrote:
> It fixes the test on system where ActiveState Perl is used.
> It is also shorter.
Thanks :)
For future reference: What in my code broke with ActiveState Perl?
The command line options?
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: t9400-git-cvsserver-server failures
From: Frank Lichtenheld @ 2007-06-27 15:07 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git Mailing List
In-Reply-To: <DA4A2395-33EA-4209-A1DE-E6CAB55C94AD@silverinsanity.com>
On Wed, Jun 27, 2007 at 09:08:16AM -0400, Brian Gernhardt wrote:
> Running the tests again this morning after a fresh pull results in
> only the above failures occurring. I looked through git-
> cvsserver.perl and couldn't find any exit(-1) or similar. Anyone
> have any idea where to start to fix these?
Look for "die", it exits with such an error code by default.
Will try to look into the issues later.
Gruesse,
--
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Jim Meyering @ 2007-06-27 14:28 UTC (permalink / raw)
To: Geert Bosch; +Cc: Alex Riesen, git
In-Reply-To: <EF53B249-8430-4700-81AE-B97FD49FB955@adacore.com>
Geert Bosch <bosch@adacore.com> wrote:
> If we really case about catching such errors, write
> the code as:
> if (!use_stdout)
> realstdout = xfdopen(dup(1), "w");
Thanks for the suggestion.
Here's a patch doing just that (and same for dup/xdup):
Subject: [PATCH] git-log: detect dup and fdopen failure
git-compat-util.h (xdup, xfdopen): Define functions.
Signed-off-by: Jim Meyering <jim@meyering.net>
---
builtin-log.c | 2 +-
git-compat-util.h | 16 ++++++++++++++++
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/builtin-log.c b/builtin-log.c
index 073a2a1..a4186ea 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -589,7 +589,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
get_patch_ids(&rev, &ids, prefix);
if (!use_stdout)
- realstdout = fdopen(dup(1), "w");
+ realstdout = xfdopen(xdup(1), "w");
prepare_revision_walk(&rev);
while ((commit = get_revision(&rev)) != NULL) {
diff --git a/git-compat-util.h b/git-compat-util.h
index b2ab3f8..362e040 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -287,6 +287,22 @@ static inline ssize_t xwrite(int fd, const void *buf, size_t len)
}
}
+static inline int xdup(int fd)
+{
+ int ret = dup(fd);
+ if (ret < 0)
+ die("dup failed: %s", strerror(errno));
+ return ret;
+}
+
+static inline FILE *xfdopen(int fd, const char *mode)
+{
+ FILE *stream = fdopen(fd, mode);
+ if (stream == NULL)
+ die("Out of memory? fdopen failed: %s", strerror(errno));
+ return stream;
+}
+
static inline size_t xsize_t(off_t len)
{
return (size_t)len;
^ permalink raw reply related
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Jim Meyering @ 2007-06-27 14:14 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0706270704y58c9e05bh6ec10118d7ee406c@mail.gmail.com>
"Alex Riesen" <raa.lkml@gmail.com> wrote:
> On 6/27/07, Jim Meyering <jim@meyering.net> wrote:
>> "Alex Riesen" <raa.lkml@gmail.com> wrote:
>> > Check if this works in some exotic but common
>> > environments (like MacOSX, Cygwin or HP-UX).
>>
>> What do you mean?
>> I know that dup and fdopen work in those environments.
>
> Exactly as you described?
No, but why does that matter?
> Are you sure fdopen on Cygwin
> sets ENOMEM?
Are you suggesting not to use errno because
it won't contain useful information on Mingw32?
>> It's ok, but for the fact that when dup fails, all you get
>> is the uninformative EINVAL from fdopen.
>
> EBADF on Cygwin, 0 on Mingw32. Can't even imagine what msvc
EBADF is fine. Setting errno to 0 upon error is a bug.
Don't cater to buggy systems.
> (which actually isn't a developers tool, but very common) will return.
>
>> > (Yes, I do think that "duplicate standard output" is useless,
>> > except for debugging. Exactly as strerror is, but that is shorter).
>>
>> I like to include *something* in the diagnostic so that when someone
>> sees it and reports it developers have an easier time finding where it
>> comes from in the code. Especially with something as unlikely (and hard
>> to reproduce) as this, that might be useful.
>
> Then put file name and the line number in!
Surely you're playing devil's advocate, now...
If you go that route, you might as well use assert() for both conditions,
and we all know *that* is silly.
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Jim Meyering @ 2007-06-27 14:06 UTC (permalink / raw)
To: Geert Bosch; +Cc: Alex Riesen, git
In-Reply-To: <EF53B249-8430-4700-81AE-B97FD49FB955@adacore.com>
Geert Bosch <bosch@adacore.com> wrote:
> On Jun 27, 2007, at 09:02, Jim Meyering wrote:
>> - if (!use_stdout)
>> - realstdout = fdopen(dup(1), "w");
>> + if (!use_stdout) {
>> + int fd = dup(1);
>> + if (fd < 0 || (realstdout = fdopen(fd, "w")) == NULL)
>> + die("failed to duplicate standard output: %s",
>> + strerror(errno));
>> + }
>
> This makes the code unreadable! A great way to ruin
> perfectly fine code is to add tons of error checking.
> The error checking is likely wrong (detects non-errors,
> or fails to detect real ones), and for sure makes code
> untestable and unreadable.
>
> If we really case about catching such errors, write
> the code as:
> if (!use_stdout)
> realstdout = xfdopen(dup(1), "w");
Of course. That is much more readable.
Though, perhaps you meant this?
> realstdout = xfdopen(xdup(1), "w");
If so, I'll be happy to prepare a patch to do that instead.
IMHO, we should never ignore syscall errors, and when preparing a
patch for a project like git (to which I haven't contributed much yet),
I prefer to keep the initial patch small and localized.
> where xfdopen is a wrapper around fdopen that dies in
> case of an error. This follows a practice we use elsewhere,
> and only adds one character to the code and only affects
> readability very slightly.
>
>> Without this, if you ever run out of file descriptors, dup will
>> fail (silently), fdopen will return NULL, and fprintf will
>> try to dereference NULL (i.e., usually segfault).
>
> As it is unlikely the failure mode will ever occur in practice,
> any way of aborting is fine. Even SIGSEGV would do: it would be
> trivial to find that we were leaking file descriptors or are out
> of memory. Oh, wait, that means we don't need any checking code
> at all...
Aren't you presuming the problem is easily reproducible
(and encountered by someone capable of investigating/reproducing),
or maybe that the abort left a usable core dump behind?
In my experience, the hardest bugs to track down are those
that are very rare and hard to reproduce.
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Alex Riesen @ 2007-06-27 14:04 UTC (permalink / raw)
To: Jim Meyering; +Cc: git
In-Reply-To: <87d4zho74h.fsf@rho.meyering.net>
On 6/27/07, Jim Meyering <jim@meyering.net> wrote:
> "Alex Riesen" <raa.lkml@gmail.com> wrote:
> > Check if this works in some exotic but common
> > environments (like MacOSX, Cygwin or HP-UX).
>
> What do you mean?
> I know that dup and fdopen work in those environments.
Exactly as you described? Are you sure fdopen on Cygwin
sets ENOMEM?
> >
> > Kinda stuffed in here. What's wrong with plain
>
> It's ok, but for the fact that when dup fails, all you get
> is the uninformative EINVAL from fdopen.
EBADF on Cygwin, 0 on Mingw32. Can't even imagine what msvc
(which actually isn't a developers tool, but very common) will return.
> > (Yes, I do think that "duplicate standard output" is useless,
> > except for debugging. Exactly as strerror is, but that is shorter).
>
> I like to include *something* in the diagnostic so that when someone
> sees it and reports it developers have an easier time finding where it
> comes from in the code. Especially with something as unlikely (and hard
> to reproduce) as this, that might be useful.
Then put file name and the line number in!
^ permalink raw reply
* Re: Problem with Linus's git repository?
From: walt @ 2007-06-27 13:41 UTC (permalink / raw)
To: git
In-Reply-To: <alpine.LFD.0.98.0706260935440.8675@woody.linux-foundation.org>
On Tue, 26 Jun 2007, Linus Torvalds wrote:
>
>
> On Tue, 26 Jun 2007, walt wrote:
> >
> > For two days I've been unable to pull from Linus's kernel
> > git repository. The reason seems to be that HEAD is a
> > symlink to refs/heads/master, which appears to be missing.
>
> It's not missing, it's packed. I tend to re-pack after I make a release,
> and this time I did it after -rc6...
Sometime overnight this problem disappeared. I haven't actually
tested this idea, but I have a hunch that your commit yesterday
of "Fix zero-object version-2 packs" is the reason.
If the problem recurs I'll follow up on it.
Thanks.
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Geert Bosch @ 2007-06-27 13:54 UTC (permalink / raw)
To: Jim Meyering; +Cc: Alex Riesen, git
In-Reply-To: <87r6nxo8iq.fsf_-_@rho.meyering.net>
On Jun 27, 2007, at 09:02, Jim Meyering wrote:
> - if (!use_stdout)
> - realstdout = fdopen(dup(1), "w");
> + if (!use_stdout) {
> + int fd = dup(1);
> + if (fd < 0 || (realstdout = fdopen(fd, "w")) == NULL)
> + die("failed to duplicate standard output: %s",
> + strerror(errno));
> + }
This makes the code unreadable! A great way to ruin
perfectly fine code is to add tons of error checking.
The error checking is likely wrong (detects non-errors,
or fails to detect real ones), and for sure makes code
untestable and unreadable.
If we really case about catching such errors, write
the code as:
if (!use_stdout)
realstdout = xfdopen(dup(1), "w");
where xfdopen is a wrapper around fdopen that dies in
case of an error. This follows a practice we use elsewhere,
and only adds one character to the code and only affects
readability very slightly.
> Without this, if you ever run out of file descriptors, dup will
> fail (silently), fdopen will return NULL, and fprintf will
> try to dereference NULL (i.e., usually segfault).
As it is unlikely the failure mode will ever occur in practice,
any way of aborting is fine. Even SIGSEGV would do: it would be
trivial to find that we were leaking file descriptors or are out
of memory. Oh, wait, that means we don't need any checking code
at all...
-Geert
^ permalink raw reply
* Re: Is it possible to "graft" a series of commits into/onto an existing repository?
From: Johannes Schindelin @ 2007-06-27 13:47 UTC (permalink / raw)
To: Patrick Doyle; +Cc: git
In-Reply-To: <e2a1d0aa0706270625o538bff7dib81802f000592e51@mail.gmail.com>
Hi,
On Wed, 27 Jun 2007, Patrick Doyle wrote:
> I have an empty directory, managed by git and I want to graft another
> (git) repository onto it, such that that history is maintained. Is
> this possible?
Yes. See Documentation/repository-layout.txt for more (look for
info/grafts).
Hth,
Dscho
^ permalink raw reply
* Re: [PATCH] git-log: detect dup and fdopen failure
From: Jim Meyering @ 2007-06-27 13:32 UTC (permalink / raw)
To: Alex Riesen; +Cc: git
In-Reply-To: <81b0412b0706270618p850df2dy41b2b797d82e3276@mail.gmail.com>
"Alex Riesen" <raa.lkml@gmail.com> wrote:
> On 6/27/07, Jim Meyering <jim@meyering.net> wrote:
>> I didn't see that fdopen could fail with ENOMEM.
>> That'll teach me to trust the man page. I see POSIX does mention it.
s/trust/use/ :)
> I wouldn't trust Linux man pages nor POSIX, if I were you.
Ha. Normally I don't rely solely on the man pages.
> Check if this works in some exotic but common
> environments (like MacOSX, Cygwin or HP-UX).
What do you mean?
I know that dup and fdopen work in those environments.
> (And yes, they probably are broken, and no, you can't fix them,
> and no, people are not going to stop using them).
>
>> + if (!use_stdout) {
>> + int fd = dup(1);
>> + if (fd < 0 || (realstdout = fdopen(fd, "w")) == NULL)
>> + die("failed to duplicate standard output: %s",
>> + strerror(errno));
>> + }
>
> Kinda stuffed in here. What's wrong with plain
It's ok, but for the fact that when dup fails, all you get
is the uninformative EINVAL from fdopen.
> realstdout = fdopen(dup(1), "w");
> if (!realstdout)
> die("%s", strerror(errno));
>
> (Yes, I do think that "duplicate standard output" is useless,
> except for debugging. Exactly as strerror is, but that is shorter).
I like to include *something* in the diagnostic so that when someone
sees it and reports it developers have an easier time finding where it
comes from in the code. Especially with something as unlikely (and hard
to reproduce) as this, that might be useful.
^ permalink raw reply
* Is it possible to "graft" a series of commits into/onto an existing repository?
From: Patrick Doyle @ 2007-06-27 13:25 UTC (permalink / raw)
To: git
I have an empty directory, managed by git and I want to graft another
(git) repository onto it, such that that history is maintained. Is
this possible?
--wpd
^ 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