* Re: git-cache-meta -- simple file meta data caching and applying
From: Jay Soffian @ 2009-01-09 0:28 UTC (permalink / raw)
To: jidanni; +Cc: git
In-Reply-To: <76718490901081622q618c43d0t333882cbe44f6b30@mail.gmail.com>
On Thu, Jan 8, 2009 at 7:22 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> It doesn't handle paths which contain white-space. "chown" is typo'd
> as "chowm". To be useful, the contribution might also include
> instructions on how it should be used with git, and perhaps also
> reasoning for why someone would want to use it in place of etckeeper,
> metastore, setgitperms, etc.
It will also blow-up if the output of "git ls-files" exceeds
limitations on number of arguments. Also, might be worth mentioning it
requires GNU find.
j.
^ permalink raw reply
* Re: [RFC PATCH] make diff --color-words customizable
From: Thomas Rast @ 2009-01-09 0:50 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901090121432.30769@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 1604 bytes --]
Johannes Schindelin wrote:
> On Fri, 9 Jan 2009, Thomas Rast wrote:
>
> > Allows for user-configurable word splits when using --color-words. This
> > can make the diff more readable if the regex is configured according to
> > the language of the file.
> >
> > For now the (POSIX extended) regex must be set via the environment
> > GIT_DIFF_WORDS_REGEX. Each (non-overlapping) match of the regex is
> > considered a word. Anything characters not matched are considered
> > whitespace. For example, for C try
> >
> > GIT_DIFF_WORDS_REGEX='[0-9]+|[a-zA-Z_][a-zA-Z0-9_]*|(\+|-|&|\|){1,2}|\S'
[...]
> Interesting idea. However, I think it would be better to do the opposite,
> have _word_ patterns. And even better to have _one_ pattern.
I'm not sure I understand. It _is_ a single pattern. The examples
just have several cases to distinguish various semantic groups that
can occur, as a sort of "half tokenizer". (The C example isn't very
complete however.)
> BTW I think you could do what you intended to do with a _way_ smaller
> and more intuitive patch.
How?
I don't think the existing mechanism, which just replaces all
whitespace with newlines and does a line diff to find out which words
changed, can "just" be adapted. We will have to insert extra newlines
at points where the regex said to split a word, but where there was no
whitespace in the original content. If there's a significantly easier
way to do that than I hacked up, please share.
Or maybe I got your original code all wrong?
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Wrap inflateInit to retry allocation after releasing pack memory
From: Junio C Hamano @ 2009-01-09 1:43 UTC (permalink / raw)
To: Linus Torvalds
Cc: R. Tyler Ballance, Shawn O. Pearce, Nicolas Pitre,
Jan Krüger, Git ML, kb
In-Reply-To: <alpine.LFD.2.00.0901081216060.3283@localhost.localdomain>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Junio - I think we should apply this, and likely to the stable branch too.
Yeah, I didn't lose the patch.
> Add the re-trying the inflateInit() after shrinking pack windows on top of
> it.
That too.
^ permalink raw reply
* Re: git-cache-meta -- simple file meta data caching and applying
From: jidanni @ 2009-01-09 2:50 UTC (permalink / raw)
To: jaysoffian; +Cc: git
In-Reply-To: <76718490901081628x62da43bcia5cdbb160b0ff24a@mail.gmail.com>
Fixed. Manly the program aims to be tiny. Perhaps I should include a
full example of using it with git-bundle.
#!/bin/sh -e
#git-cache-meta -- file meta data caching for possible use with
#git-bundle, git-fast-export, git-archive, hooks, as a simple
#alternative to etckeeper, metastore, setgitperms. Requires GNU Find.
: ${GIT_CACHE_META_FILE=.git_cache_meta}
case $@ in
--store|--stdout)
case $1 in --store) exec > $GIT_CACHE_META_FILE; esac
git ls-files|xargs -I{} find {} \
\( -user ${USER?} -o -printf 'chown %u "%p"\n' \) \
\( -group $USER -o -printf 'chgrp %g "%p"\n' \) \
\( \( -type l -o -perm 755 -o -perm 644 \) \
-o -printf 'chmod %#m "%p"\n' \);;
--apply) sh -e $GIT_CACHE_META_FILE;;
*) 1>&2 echo "Usage: $0 --store|--stdout|--apply"; exit 1;;
esac
^ permalink raw reply
* Re: git-cache-meta -- simple file meta data caching and applying
From: Boyd Stephen Smith Jr. @ 2009-01-09 4:29 UTC (permalink / raw)
To: jidanni; +Cc: git
In-Reply-To: <87hc49jq04.fsf@jidanni.org>
[-- Attachment #1: Type: text/plain, Size: 865 bytes --]
On Thursday 08 January 2009, jidanni@jidanni.org wrote
about 'git-cache-meta -- simple file meta data caching and applying':
>Gentlemen, I have whipped up this:
>
>#!/bin/sh -e
>#git-cache-meta -- simple file meta data caching and applying.
>#Simpler than etckeeper, metastore, setgitperms, etc.
You *might* look at pristine-tar from Debian. It's specifically designed
to be able to generate the .orig.tar.gz that Debian packages are based on
from a VCS checkout, but it's code might be useful.
I do like your script though. It's simple and the output is plain text so
it is easy to have your VCS maintain it.
--
Boyd Stephen Smith Jr. ,= ,-_-. =.
bss@iguanasuicide.net ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy `-'(. .)`-'
http://iguanasuicide.net/ \_/
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* 1.5.6.5 fails to clone git.kernel.org/[...]/rostedt/linux-2.6-rt
From: Tim Shepard @ 2009-01-09 6:24 UTC (permalink / raw)
To: git
I have git 1.5.6.5 installed from the Debian/lenny package.
Poking around in http://git.kernel.org/ looking for a git repository
that might have the latest -rt development happening, I found
http://git.kernel.org/?p=linux/kernel/git/rostedt/linux-2.6-rt.git
which looked promising.
But when I tried cloning it using:
git clone rsync://rsync.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-rt.git linux-2.6-rt
it pulled several hundred megabytes through my (rather slow) DSL line
and then printed out
error: Trying to write ref refs/tags/v2.6.11 with nonexistant object 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c
fatal: Cannot update the ref 'refs/tags/v2.6.11'.
and then removed everything it had just pulled.
Looking at http://git.kernel.org/?p=linux/kernel/git/rostedt/linux-2.6-rt.git;a=tags
I see that apparently v2.6.11 and v.2.6.11-tree both point at a tree
object and not a commit.
Is this a bug in git? (Even if there is something wrong with the
git repository I was trying to clone, the fact that it removed all the
work that took a long time to pull is annoying.)
Is this problem already fixed in some newer version of git?
-Tim Shepard
shep@alum.mit.edu
^ permalink raw reply
* Re: [PATCH 0/3] Teach Git about the patience diff algorithm
From: Junio C Hamano @ 2009-01-09 6:54 UTC (permalink / raw)
To: Adeodato Simó
Cc: Linus Torvalds, Clemens Buchacher, Johannes Schindelin,
Pierre Habouzit, davidel, Francis Galiegue, Git ML
In-Reply-To: <20090108195511.GA8734@chistera.yi.org>
Adeodato Simó <dato@net.com.org.es> writes:
> * Linus Torvalds [Fri, 02 Jan 2009 08:42:04 -0800]:
>
>> Yes, this one is a real patience diff change, but it's also the same one
>> that I've seen in the google fanboi findings. What google did _not_ show
>> was any real-life examples, or anybody doing any critical analysis.
>
> This comes a bit late and maybe it's redundant, but somebody just sent
> to a Debian mailing list a patch that was hard to read, and patience
> improved it. (I realize it's quite similar in spirit to the "toy
> patience example" that google returns, but this at list is a *real*
> example where patience helped me read a patch.)
>
> I'm also attaching bzr diff output, because it's still more readable
> IMHO. (I realize that's independent of patience, as you explained, but
> I'm making a point that it'd be nice to have this addressed by somebody
> knowledgeable.)
I found thd difference between the Bzr diff and Dscho diff somewhat
interesting. It shows that sometimes it makes the results easier to read
to consider blank lines (and lines with only punctuation marks) that match
to be non-matching when they appear inside a block of other consecutive
non-matching lines, even though the result may become larger.
The part Bzr gives this result:
> +/****************************************************************************
> Write data to a fd.
> ****************************************************************************/
>
> ssize_t write_data(int fd, const char *buffer, size_t N)
> {
> - size_t total=0;
> ssize_t ret;
> - char addr[INET6_ADDRSTRLEN];
> ... all "removed"
> - while (total < N) {
> - total += ret;
> - }
> - return (ssize_t)total;
> + struct iovec iov;
> +
> + iov.iov_base = CONST_DISCARD(char *, buffer);
> ... all "added"
> +
> +
> + return -1;
> }
>
> /****************************************************************************
is shown by the Dscho git-diff like this:
> Write data to a fd.
> ****************************************************************************/
>
> ssize_t write_data(int fd, const char *buffer, size_t N)
> {
> - size_t total=0;
> ssize_t ret;
> - char addr[INET6_ADDRSTRLEN];
> + struct iovec iov;
>
> - while (total < N) {
> - ret = sys_write(fd,buffer + total,N - total);
> + iov.iov_base = CONST_DISCARD(char *, buffer);
> + iov.iov_len = N;
>
> - if (ret == -1) {
> - if (fd == get_client_fd()) {
> ... all "removed"
> -
> - if (ret == 0) {
> - return total;
> - }
> + ret = write_data_iov(fd, &iov, 1);
> + if (ret >= 0) {
> + return ret;
> + }
>
> - total += ret;
> + if (fd == get_client_fd()) {
> + char addr[INET6_ADDRSTRLEN];
> ... all "added"
> + DEBUG(0,("write_data: write failure. Error = %s\n",
> + strerror(errno) ));
> }
> - return (ssize_t)total;
> +
> + return -1;
> }
If we find the "common" context lines that have only blank and punctuation
letters in Dscho output, turn each of them into "-" and "+", and rearrange
them so that all "-" are together followed by "+", it will match Bzr
output.
^ permalink raw reply
* Re: [PATCH 2/2] Use is_pseudo_dir_name everywhere
From: Johannes Sixt @ 2009-01-09 7:03 UTC (permalink / raw)
To: Alexander Potashev; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <1231457063-29186-3-git-send-email-aspotashev@gmail.com>
Alexander Potashev schrieb:
> - if ((ent->d_name[0] == '.') &&
> - (ent->d_name[1] == 0 ||
> - ((ent->d_name[1] == '.') && (ent->d_name[2] == 0))))
> + if (is_pseudo_dir_name(ent->d_name))
Nit-pick: When I read the resulting code, then I will have to look up that
is_pseudo_dir_name() indeed only checks for "." and "..". But if it were
named is_dot_or_dotdot(), then I would have to do that.
-- Hannes
^ permalink raw reply
* Re: [PATCH 2/2] Use is_pseudo_dir_name everywhere
From: Johannes Sixt @ 2009-01-09 7:22 UTC (permalink / raw)
Cc: Alexander Potashev, Junio C Hamano, Git Mailing List
In-Reply-To: <4966F6BB.90408@viscovery.net>
Johannes Sixt schrieb:
> Alexander Potashev schrieb:
>> - if ((ent->d_name[0] == '.') &&
>> - (ent->d_name[1] == 0 ||
>> - ((ent->d_name[1] == '.') && (ent->d_name[2] == 0))))
>> + if (is_pseudo_dir_name(ent->d_name))
>
> Nit-pick: When I read the resulting code, then I will have to look up that
> is_pseudo_dir_name() indeed only checks for "." and "..". But if it were
> named is_dot_or_dotdot(), then I would have to do that.
... then I would *not* have to do that, of course.
-- Hannes
^ permalink raw reply
* Re: [PATCH] allow 8bit data in email body sent by send-email
From: Jeff King @ 2009-01-09 7:28 UTC (permalink / raw)
To: Andre Przywara; +Cc: git
In-Reply-To: <1231422657-15305-1-git-send-email-andre.przywara@amd.com>
On Thu, Jan 08, 2009 at 02:50:57PM +0100, Andre Przywara wrote:
> when sending patch files via git send-email, the perl script assumes
> 7bit characters only. If there are other bytes in the body (foreign language
> characters in names or translations), some servers (like vger.kernel.org)
> reject the mail because of th?t. This patch always adds an 8bit header line
> to each mail.
This should be done already by git-format-patch when you generate the
patch to feed to send-email. What exactly is the workflow you use to
generate this problem? Does it matter where the non-ascii characters are
(commit versus patch, etc)? What version of git are you using?
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 77ca8fe..68a462c 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -793,6 +793,7 @@ To: $to${ccline}
> Subject: $subject
> Date: $date
> Message-Id: $message_id
> +Content-Transfer-Encoding: 8bit
> X-Mailer: git-send-email $gitversion
> ";
This fix isn't right anyway. For one thing, if you're going to include
C-T-E, you should also include a MIME-Version header. But more
importantly, we are already handling encoding elsewhere. So
unconditionally adding this means that you may conflict with existing
MIME headers in the @xh variable.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] Use is_pseudo_dir_name everywhere
From: Junio C Hamano @ 2009-01-09 8:33 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Alexander Potashev, Git Mailing List
In-Reply-To: <4966FB36.2030409@viscovery.net>
Johannes Sixt <j.sixt@viscovery.net> writes:
> Johannes Sixt schrieb:
>> Alexander Potashev schrieb:
>>> - if ((ent->d_name[0] == '.') &&
>>> - (ent->d_name[1] == 0 ||
>>> - ((ent->d_name[1] == '.') && (ent->d_name[2] == 0))))
>>> + if (is_pseudo_dir_name(ent->d_name))
>>
>> Nit-pick: When I read the resulting code, then I will have to look up that
>> is_pseudo_dir_name() indeed only checks for "." and "..". But if it were
>> named is_dot_or_dotdot(), then I would have to do that.
>
> ... then I would *not* have to do that, of course.
I think the unstated motivation of this choice of the name is to keep the
door open to include lost+found and friends to the repertoire, and perhaps
to have an isolated place for customization for non-POSIX platforms and
for local conventions. It is more like is_uninteresting_dirent_name().
As long as this function is used only to detect and skip "uninteresting"
dirent, I think that is not a bad direction.
On the other hand, I am a bit worried about is_empty_dir() abused outside
its intended purpose to say "this directory does not have anything
interesting". E.g. "Oh, it's empty so we can nuke it":
if (is_empty_dir(dir))
rmdir(dir);
even though the current callers do not do something crazy like this (the
usual order we do things is rmdir() and then check for errors).
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Jeff King @ 2009-01-09 8:38 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901081601240.30769@pacific.mpi-cbg.de>
On Thu, Jan 08, 2009 at 04:07:08PM +0100, Johannes Schindelin wrote:
> Just try this with a submodule that has more changes than fit on a screen:
>
> $ git -p submodule summary
>
> In my tests, it consistently fscks up my console. I wonder if this is
> related to ea27a18(spawn pager via run_command interface).
>
> *reverts that commit* Yep, that fixes it.
Hmm. I can reproduce your problem here, like this:
$ git -p submodule summary
but when I tried to dig deeper using strace, the problem goes away:
$ strace -f -o foo.out git -p submodule summary
However, I was able to get some data by stracing _just_ less:
$ GIT_PAGER='strace -f -o foo.out less' git -p submodule summary
and that reproduces the problem. And here's the interesting bit:
open("/dev/tty", O_RDONLY|O_LARGEFILE) = 3
...
read(0, $SOME_SUBMODULE_OUTPUT, ...)
write(1, $SOME_SUBMODULE_OUTPUT, ...)
read(3, 0xbfd3442f, 1) = -1 EIO (Input/output error)
That is, after displaying the actual output, the next thing less does is
try to get input from the user on /dev/tty. But it returns EIO. At which
point less just dies, leaving your terminal in a funny state.
Hrm. Here's a theory. The new pager behavior goes something like this
for a builtin:
1. fork, child becomes pager
2. register atexit handler to wait for pager to finish
3. run builtin
4. exit, triggering handler
but for an external command (like a shell script), we exec the command,
and presumably forget about out atexit handler entirely. Which means
that our shell script writes all of its output and exits before less has
a chance to prompt and get a response from the tty.
And I'll admit to being very hazy on the details of process groups and
controlling terminals, but from what I recall, perhaps the EIO is
related to the process group leader being dead. Which means we could
paper over it by putting less in its own pgrp.
So here's a little test (in bash):
$ set +m ;# disable job control, leaving bash as the pgrp leader
$ git -p submodule summary
A-ha. That "works" in the sense that less runs fine and show the output.
So it is a pgrp issue. _But_ we still have a problem, which is that the
original process has exited, and bash thinks the command is finished. So
now annoyingly we have both bash and less trying to read from the
terminal.
So the _real_ problem is that we are not always triggering the "wait for
pager to finish" code because we exec and forget about it. Which means
this strategy of "git runs child pager" will never work properly.
Instead, we have to use three processes: git and the pager become child
processes, while the original process waits for both to exit and returns
the proper exit code from git.
Let me try to work up a patch.
-Peff
PS I believe this is related to a similar bug which I have been hunting
fruitlessly for a few weeks: if you use ^C to kill git, the pager
will sometimes do funny things. I also traced that to an EIO on
reading from the terminal, which makes sense: we are killing git
before it gets a chance to do wait_for_pager.
^ permalink raw reply
* Re: 1.5.6.5 fails to clone git.kernel.org/[...]/rostedt/linux-2.6-rt
From: Junio C Hamano @ 2009-01-09 8:58 UTC (permalink / raw)
To: Tim Shepard; +Cc: git, Johannes Schindelin, Daniel Barkalow
In-Reply-To: <E1LLAn5-0001JM-00@alva.home>
Tim Shepard <shep@alum.mit.edu> writes:
> I have git 1.5.6.5 installed from the Debian/lenny package.
>
> Poking around in http://git.kernel.org/ looking for a git repository
> that might have the latest -rt development happening, I found
>
> http://git.kernel.org/?p=linux/kernel/git/rostedt/linux-2.6-rt.git
>
> which looked promising.
>
> But when I tried cloning it using:
>
> git clone rsync://rsync.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-rt.git linux-2.6-rt
>
> it pulled several hundred megabytes through my (rather slow) DSL line
> and then printed out
>
> error: Trying to write ref refs/tags/v2.6.11 with nonexistant object 5dc01c595e6c6ec9ccda4f6f69c131c0dd945f8c
> fatal: Cannot update the ref 'refs/tags/v2.6.11'.
>
> and then removed everything it had just pulled.
>
> Looking at http://git.kernel.org/?p=linux/kernel/git/rostedt/linux-2.6-rt.git;a=tags
> I see that apparently v2.6.11 and v.2.6.11-tree both point at a tree
> object and not a commit.
>
> Is this a bug in git?
It is not a problem for the tag pointing to a tree, but linux-2.6-rt.git
tree uses (like everybody else) objects/alternates to borrow objects from
the repository of Linus.
I think we lost the alternate object store support when git-fetch was
rewritten from the original shell script (that did support fetching from
such a repository over rsync:// transport) to a reimplementation in C,
with commit b888d61 (Make fetch a builtin, 2007-09-10).
Later, cd547b4 (fetch/push: readd rsync support, 2007-10-01) attempted to
resurrect some rsync support (b888d61 lost rsync support completely for
git-fetch), but introduced these lines in transport.c:
/* NEEDSWORK: handle one level of alternates */
result = run_command(&rsync);
These have not been touched ever since, and then finally commit 8434c2f
(Build in clone, 2008-04-27) killed support of cloning from a repository
that uses alternates for rsync transport for git-clone (before that, the
shell script version had a special case to still allow such operation over
rsync).
It appears practically nobody uses rsync:// transport anymore these days;
you are unfortunately the first one who noticed it.
^ permalink raw reply
* Re: [PATCH, resend] git-commit: colored status when color.ui is set
From: Junio C Hamano @ 2009-01-09 9:00 UTC (permalink / raw)
To: markus.heidelberg; +Cc: git
In-Reply-To: <200901081953.01418.markus.heidelberg@web.de>
Markus Heidelberg <markus.heidelberg@web.de> writes:
> When using "git commit" and there was nothing to commit (the editor
> wasn't launched), the status output wasn't colored, even though color.ui
> was set. Only when setting color.status it worked.
>
> Signed-off-by: Markus Heidelberg <markus.heidelberg@web.de>
> ---
> builtin-commit.c | 3 +++
> 1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-commit.c b/builtin-commit.c
> index e88b78f..2d90f74 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -945,6 +945,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>
> git_config(git_commit_config, NULL);
>
> + if (wt_status_use_color == -1)
> + wt_status_use_color = git_use_color_default;
> +
> argc = parse_and_validate_options(argc, argv, builtin_commit_usage, prefix);
>
> index_file = prepare_index(argc, argv, prefix);
My first reaction was:
When the editor does get launched, what would the new code do with
your patch? Would we see bunch of escape codes in the editor now?
But we do disable color explicitly when we generate contents to feed the
editor in that case since bc5d248 (builtin-commit: do not color status
output shown in the message template, 2007-11-18), so that fear is
unfounded.
Thanks for a reminder, will queue.
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Jeff King @ 2009-01-09 9:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <20090109083836.GB21389@coredump.intra.peff.net>
On Fri, Jan 09, 2009 at 03:38:36AM -0500, Jeff King wrote:
> So the _real_ problem is that we are not always triggering the "wait for
> pager to finish" code because we exec and forget about it. Which means
> this strategy of "git runs child pager" will never work properly.
> Instead, we have to use three processes: git and the pager become child
> processes, while the original process waits for both to exit and returns
> the proper exit code from git.
>
> Let me try to work up a patch.
Below is a patch that uses the three-process mechanism, and it fixes the
problem. _But_ it is not satisfactory for inclusion, because it won't
work on MINGW32. Since it is actually splitting git into two processes
(one to monitor the pager and one to actually run git), it uses fork.
So I think to do things right, we have to be even more complicated. When
we spawn the pager, we keep git as a single process. We register the
atexit() handler to wait for the pager, and intercept any death signals
to do the same. Then, if we are running a builtin, it is business as
usual. But if we want to exec something, instead we have to actually
spawn into the three-process form. Meaning we have to use run_command to
start it, and then wait for it and the pager to return.
Of course, we don't know ahead of time whether exec'ing a command will
work: we find out by trying. So now we will end up creating a pipe and
fork()ing every time we want to see whether we can exec a command. But I
suppose that only happens once or twice, so maybe the performance impact
isn't relevant.
This is all getting complicated enough that I am tempted to just suggest
reverting ea27a18c. But even that won't fix everything, though, since
MINGW32 still needs to use run_command to spawn the pager. IOW, I think
the breakage you are seeing has always been broken on MINGW32.
Blah. Anyway, here is the Unix-only patch.
---
diff --git a/pager.c b/pager.c
index f19ddbc..68ae669 100644
--- a/pager.c
+++ b/pager.c
@@ -28,18 +28,10 @@ static void pager_preexec(void)
static const char *pager_argv[] = { "sh", "-c", NULL, NULL };
static struct child_process pager_process;
-static void wait_for_pager(void)
-{
- fflush(stdout);
- fflush(stderr);
- /* signal EOF to pager */
- close(1);
- close(2);
- finish_command(&pager_process);
-}
-
void setup_pager(void)
{
+ pid_t git_child;
+ int status;
const char *pager = getenv("GIT_PAGER");
if (!isatty(1))
@@ -68,14 +60,24 @@ void setup_pager(void)
if (start_command(&pager_process))
return;
- /* original process continues, but writes to the pipe */
- dup2(pager_process.in, 1);
- if (isatty(2))
- dup2(pager_process.in, 2);
- close(pager_process.in);
+ /* now spawn the actual git process */
+ git_child = fork();
+ if (git_child == -1)
+ die("unable to fork: %s", strerror(errno));
+ if (git_child == 0) {
+ dup2(pager_process.in, 1);
+ if (isatty(2))
+ dup2(pager_process.in, 2);
+ close(pager_process.in);
+ return;
+ }
- /* this makes sure that the parent terminates after the pager */
- atexit(wait_for_pager);
+ /* and the original process just waits for both to finish */
+ close(pager_process.in);
+ if (waitpid(git_child, &status, 0) < 0)
+ die("wait failure: %s", strerror(errno));
+ finish_command(&pager_process);
+ exit(status);
}
int pager_in_use(void)
^ permalink raw reply related
* Re: Funny: git -p submodule summary
From: Junio C Hamano @ 2009-01-09 9:30 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20090109083836.GB21389@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So the _real_ problem is that we are not always triggering the "wait for
> pager to finish" code because we exec and forget about it. Which means
> this strategy of "git runs child pager" will never work properly.
> Instead, we have to use three processes: git and the pager become child
> processes, while the original process waits for both to exit and returns
> the proper exit code from git.
>
> Let me try to work up a patch.
This arrangement to have the third process could even open the possibility
of having it read from git and write to pager, and not launching the pager
if there is no interesting data from git to feed it with.
I do not know if I like the performance implications associated with it,
though.
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Jeff King @ 2009-01-09 9:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7veizc25e8.fsf@gitster.siamese.dyndns.org>
On Fri, Jan 09, 2009 at 01:30:23AM -0800, Junio C Hamano wrote:
> This arrangement to have the third process could even open the possibility
> of having it read from git and write to pager, and not launching the pager
> if there is no interesting data from git to feed it with.
>
> I do not know if I like the performance implications associated with it,
> though.
Ugh. That has definitely been a requested feature, but the thought of
essentially running "cat" in our pipeline strikes me as a bit kludgey.
On the other hand, we are by definition going to the pager in that case,
so in theory performance is less of a consideration.
But see my other mail for why a third process is hard to always do on
Windows.
-Peff
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Junio C Hamano @ 2009-01-09 9:38 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20090109093307.GA2039@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Fri, Jan 09, 2009 at 01:30:23AM -0800, Junio C Hamano wrote:
>
>> This arrangement to have the third process could even open the possibility
>> of having it read from git and write to pager, and not launching the pager
>> if there is no interesting data from git to feed it with.
>>
>> I do not know if I like the performance implications associated with it,
>> though.
>
> Ugh. That has definitely been a requested feature, but the thought of
> essentially running "cat" in our pipeline strikes me as a bit kludgey.
>
> On the other hand, we are by definition going to the pager in that case,
> so in theory performance is less of a consideration.
>
> But see my other mail for why a third process is hard to always do on
> Windows.
Heh, this late at night just before going to bed, I am allowed to say that
I do not care about Windows at all ;-). More dedicated and competent
people will solve it for us while I am sleeping.
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Jeff King @ 2009-01-09 9:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, git
In-Reply-To: <20090109092250.GA1809@coredump.intra.peff.net>
On Fri, Jan 09, 2009 at 04:22:50AM -0500, Jeff King wrote:
> So I think to do things right, we have to be even more complicated. When
> we spawn the pager, we keep git as a single process. We register the
> atexit() handler to wait for the pager, and intercept any death signals
> to do the same. Then, if we are running a builtin, it is business as
> usual. But if we want to exec something, instead we have to actually
> spawn into the three-process form. Meaning we have to use run_command to
> start it, and then wait for it and the pager to return.
Actually, this turned out to be quite a small patch. It fixes the
problem you were seeing, and it should work on Windows. It incurs an
extra fork() for every dashed-external that we try.
There is a little noise in the patch, so let me highlight the three
things that are happening (a "real" patch should break this into three
patches in a series):
1. The noise is from renaming the static run_command, which conflicts
with the definition in run_command.h
2. Substituting run_command for execvp.
3. run_command needs to signal "I couldn't exec this" as opposed to
other errors, since we care about the difference here. I do this
here with exit(127), but probably we should recognize this in
wait_or_whine and hand out the same error code for posix and mingw
platforms.
As for the extra fork, we could do away with it if we scanned the path
looking for the external before just exec'ing it. I think this is better
in the long run anyway, because then we can do other setup specific to
running an external command (I don't remember the details, but I ran
afoul of this when I was doing pager stuff a while ago).
Patch is below.
---
diff --git a/git.c b/git.c
index ecc8fad..fa946b9 100644
--- a/git.c
+++ b/git.c
@@ -2,6 +2,7 @@
#include "exec_cmd.h"
#include "cache.h"
#include "quote.h"
+#include "run-command.h"
const char git_usage_string[] =
"git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate|--no-pager] [--bare] [--git-dir=GIT_DIR] [--work-tree=GIT_WORK_TREE] [--help] COMMAND [ARGS]";
@@ -219,7 +220,7 @@ struct cmd_struct {
int option;
};
-static int run_command(struct cmd_struct *p, int argc, const char **argv)
+static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
{
int status;
struct stat st;
@@ -384,7 +385,7 @@ static void handle_internal_command(int argc, const char **argv)
struct cmd_struct *p = commands+i;
if (strcmp(p->cmd, cmd))
continue;
- exit(run_command(p, argc, argv));
+ exit(run_builtin(p, argc, argv));
}
}
@@ -392,6 +393,7 @@ static void execv_dashed_external(const char **argv)
{
struct strbuf cmd = STRBUF_INIT;
const char *tmp;
+ int status;
strbuf_addf(&cmd, "git-%s", argv[0]);
@@ -406,8 +408,13 @@ static void execv_dashed_external(const char **argv)
trace_argv_printf(argv, "trace: exec:");
- /* execvp() can only ever return if it fails */
- execvp(cmd.buf, (char **)argv);
+ /*
+ * if we failed because the command was not found, it is
+ * OK to return. Otherwise, we just pass along the status code.
+ */
+ status = run_command_v_opt(argv, 0);
+ if (status != 127 && status != -ERR_RUN_COMMAND_FORK)
+ exit(-status);
trace_printf("trace: exec failed: %s\n", strerror(errno));
diff --git a/run-command.c b/run-command.c
index c90cdc5..539609e 100644
--- a/run-command.c
+++ b/run-command.c
@@ -118,7 +118,7 @@ int start_command(struct child_process *cmd)
} else {
execvp(cmd->argv[0], (char *const*) cmd->argv);
}
- die("exec %s failed.", cmd->argv[0]);
+ exit(127);
}
#else
int s0 = -1, s1 = -1, s2 = -1; /* backups of stdin, stdout, stderr */
^ permalink raw reply related
* Re: [RFC PATCH] make diff --color-words customizable
From: Jeff King @ 2009-01-09 9:53 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Johannes Schindelin
In-Reply-To: <1231459505-14395-1-git-send-email-trast@student.ethz.ch>
On Fri, Jan 09, 2009 at 01:05:05AM +0100, Thomas Rast wrote:
> Word diff becomes much more useful especially with TeX, where it is
> common to run together \sequences\of\commands\like\this that the
> current --color-words treats as a single word.
I have run into this, as well, and it would be nice to have configurable
word boundaries.
> Apart from possible bugs, the main issue is: where should I put the
> configuration for this?
It's a per-file thing, so probably in the diff driver that is triggered
via attributes. See userdiff.[ch]; you'll need to add an entry to the
userdiff_driver struct. You can look at the funcname pattern stuff as a
template, as this is very similar.
-Peff
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Johannes Sixt @ 2009-01-09 10:09 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20090109092250.GA1809@coredump.intra.peff.net>
Jeff King schrieb:
> Below is a patch that uses the three-process mechanism, and it fixes the
> problem. _But_ it is not satisfactory for inclusion, because it won't
> work on MINGW32. Since it is actually splitting git into two processes
> (one to monitor the pager and one to actually run git), it uses fork.
We have start_async()/finish_async() to replace a fork() of the sort that
we have here.
> IOW, I think
> the breakage you are seeing has always been broken on MINGW32.
Indeed. Hitting Ctrl-C while the pager was open has messed up the console
since day 1.
-- Hannes
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Jeff King @ 2009-01-09 10:13 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Johannes Schindelin, git
In-Reply-To: <49672244.80200@viscovery.net>
On Fri, Jan 09, 2009 at 11:09:08AM +0100, Johannes Sixt wrote:
> > Below is a patch that uses the three-process mechanism, and it fixes the
> > problem. _But_ it is not satisfactory for inclusion, because it won't
> > work on MINGW32. Since it is actually splitting git into two processes
> > (one to monitor the pager and one to actually run git), it uses fork.
>
> We have start_async()/finish_async() to replace a fork() of the sort that
> we have here.
It looks like start_async is implemented using threads on Windows. Will
that survive an execvp call? Because we don't know at this point whether
we are going to actually run builtin code, or if we will exec an
external.
-Peff
^ permalink raw reply
* Re: [PATCH/RFC v3 1/2] Optimised, faster, more effective symlink/directory detection
From: Pete Harlan @ 2009-01-09 10:20 UTC (permalink / raw)
To: Kjetil Barvik; +Cc: git, Linus Torvalds
In-Reply-To: <1231334689-17135-2-git-send-email-barvik@broadpark.no>
Here are some suggestions for the commit message.
Kjetil Barvik wrote:
> Changes includes the following:
>
> - The cache functionality is more effective. Previously when A/B/C/D
> was in the cache and A/B/C/E/file.c was called for, there was no
> match at all from the cache. Now we use the fact that the paths
> "A", "A/B" and "A/B/C" is already tested, and we only need to do an
is -> are
> lstat() call on "A/B/C/E".
>
> - We only cache/store the last path regardless of it's type. Since the
it's -> its
> cache functionality is always used with alphabetically sorted names
> (at least it seams so for me), there is no need to store both the
seams -> seems
> last symlink-leading path and the last real-directory path. Note
> that if the cache is not called with (mostly) alphabetically sorted
> names, neither the old, nor this new one, would be very effective.
>
> - We also can cache the fact that a directory does not exist.
> Previously we could end up doing lots of lstat() calls for a removed
> directory which previously contained lots of files. Since we
> already have simplified the cache functionality and only store the
> last path (see above), this new functionality was easy to add.
>
> - Previously, when symlink A/B/C/S was cached/stored in the
> symlink-leading path, and A/B/C/file.c was called for, it was not
> easy to use the fact that we already known that the paths "A", "A/B"
known -> knew
> and "A/B/C" is real directories. Since we now only store one single
is -> are
> path (the last one), we also get similar logic for free regarding
> the new "non-exsisting-directory-cache".
>
> - Avoid copying the first path components of the name 2 zillions times
zillions -> zillion
> when we tests new path components. Since we always cache/store the
tests -> test
> last path, we can copy each component as we test those directly into
> the cache. Previously we ended up doing a memcpy() for the full
> path/name right before each lstat() call, and when updating the
> cache for each time we have tested an new path component.
an -> a
>
> - We also use less memory, that is PATH_MAX bytes less memory on the
is -> is,
> stack and PATH_MAX bytes less memory on the heap.
Cheers,
--Pete
^ permalink raw reply
* Minimum required version of subversion for git-svn?
From: Tom G. Christensen @ 2009-01-09 10:11 UTC (permalink / raw)
To: git
Hello,
With git 1.6.0.5 I was able to run git-svn with subversion 1.1.4 on
RHEL4/i386 but with 1.6.0.6 and 1.6.1 the testsuite now fails in the new
test t9104.10:
* FAIL 10: follow-parent is atomic
(
cd wc &&
svn up &&
svn mkdir stunk &&
echo "trunk stunk" > stunk/readme &&
svn add stunk/readme &&
svn ci -m "trunk stunk" &&
echo "stunk like junk" >> stunk/readme &&
svn ci -m "really stunk" &&
echo "stink stank stunk" >> stunk/readme &&
svn ci -m "even the grinch agrees"
) &&
svn copy -m "stunk flunked" "$svnrepo"/stunk
"$svnrepo"/flunk &&
{ svn cp -m "early stunk flunked too" \
"$svnrepo"/stunk@17 "$svnrepo"/flunked ||
svn cp -m "early stunk flunked too" \
-r17 "$svnrepo"/stunk "$svnrepo"/flunked; } &&
git svn init --minimize-url -i stunk "$svnrepo"/stunk &&
git svn fetch -i stunk &&
git update-ref refs/remotes/flunk@18
refs/remotes/stunk~2 &&
git update-ref -d refs/remotes/stunk &&
git config --unset svn-remote.svn.fetch stunk &&
mkdir -p "$GIT_DIR"/svn/flunk@18 &&
rev_map=$(cd "$GIT_DIR"/svn/stunk && ls .rev_map*) &&
dd if="$GIT_DIR"/svn/stunk/$rev_map \
of="$GIT_DIR"/svn/flunk@18/$rev_map bs=24 count=1 &&
rm -rf "$GIT_DIR"/svn/stunk &&
git svn init --minimize-url -i flunk "$svnrepo"/flunk &&
git svn fetch -i flunk &&
git svn init --minimize-url -i stunk "$svnrepo"/stunk &&
git svn fetch -i stunk &&
git svn init --minimize-url -i flunked
"$svnrepo"/flunked &&
git svn fetch -i flunked
test "`git rev-parse --verify refs/remotes/flunk@18`" \
= "`git rev-parse --verify refs/remotes/stunk`" &&
test "`git rev-parse --verify refs/remotes/flunk~1`" \
= "`git rev-parse --verify refs/remotes/stunk`" &&
test "`git rev-parse --verify refs/remotes/flunked~1`" \
= "`git rev-parse --verify refs/remotes/stunk~1`"
With 1.6.1 I also see t9129.10-12 failing with subversion 1.1.4:
* FAIL 10: ISO-8859-1 should match UTF-8 in svn
(
cd ISO-8859-1 &&
compare_svn_head_with
"$TEST_DIRECTORY"/t3900/1-UTF-8.txt
)
* FAIL 11: $H should match UTF-8 in svn
(
cd $H &&
compare_svn_head_with
"$TEST_DIRECTORY"/t3900/2-UTF-8.txt
)
* FAIL 12: $H should match UTF-8 in svn
(
cd $H &&
compare_svn_head_with
"$TEST_DIRECTORY"/t3900/2-UTF-8.txt
)
* failed 3 among 12 test(s)
make[2]: Leaving directory `/builddir/build/BUILD/git-1.6.1/t'
make[2]: *** [t9129-git-svn-i18n-commitencoding.sh] Error 1
I see in git-svn.perl that only SVN::Core 1.1.0 is required. Is it still
the intention that git-svn should work with subversion 1.1.x?
If you're going to bump the minimum requirement I would ask that you
atleast keep 1.3.x as supported. This is the last release of subversion
where RHEL3 can satisfy the dependencies out of the box and I've
verified that the testsuite will pass with 1.3.2.
-tgc
^ permalink raw reply
* Re: [PATCH 2/2] Use is_pseudo_dir_name everywhere
From: Alexander Potashev @ 2009-01-09 10:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Git Mailing List
In-Reply-To: <7vy6xk280e.fsf@gitster.siamese.dyndns.org>
On 00:33 Fri 09 Jan , Junio C Hamano wrote:
> Johannes Sixt <j.sixt@viscovery.net> writes:
>
> > Johannes Sixt schrieb:
> >> Alexander Potashev schrieb:
> >>> - if ((ent->d_name[0] == '.') &&
> >>> - (ent->d_name[1] == 0 ||
> >>> - ((ent->d_name[1] == '.') && (ent->d_name[2] == 0))))
> >>> + if (is_pseudo_dir_name(ent->d_name))
> >>
> >> Nit-pick: When I read the resulting code, then I will have to look up that
> >> is_pseudo_dir_name() indeed only checks for "." and "..". But if it were
> >> named is_dot_or_dotdot(), then I would have to do that.
> >
> > ... then I would *not* have to do that, of course.
>
> I think the unstated motivation of this choice of the name is to keep the
> door open to include lost+found and friends to the repertoire, and perhaps
> to have an isolated place for customization for non-POSIX platforms and
> for local conventions. It is more like is_uninteresting_dirent_name().
I didn't think over the support of 'lost+found'. But the name like
is_uninteresting_dirent_name is more flexible, indeed. I prefer a bit
shorter name, 'is_dummy_dirent_name'.
But if you're going to support 'lost+found's, remember that a Git
repository might have its own 'lost+found' directory. It's a bit crazy,
but it's possible:
---
lost+found/file | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
create mode 100644 lost+found/file
diff --git a/lost+found/file b/lost+found/file
new file mode 100644
index 0000000..190a180
--- /dev/null
+++ b/lost+found/file
@@ -0,0 +1 @@
+123
--
Git shouldn't allow to clone at least repositories that have lost+found
directory into a directory with already existing lost+found (neither
it's a ordinary directory created using 'mkdir' nor it's an ext2's
property)
We should probably forbid cloning to a directory with lost+found,
because a 'lost+found' may appear after pulling from somebody and the
user won't be able to resolve this anyhow.
>
> As long as this function is used only to detect and skip "uninteresting"
> dirent, I think that is not a bad direction.
>
> On the other hand, I am a bit worried about is_empty_dir() abused outside
> its intended purpose to say "this directory does not have anything
> interesting". E.g. "Oh, it's empty so we can nuke it":
I propose to rename it (if it's really necessary) to is_clean_dir, which
means "There's no old crap here, we can safely clone".
>
> if (is_empty_dir(dir))
> rmdir(dir);
>
> even though the current callers do not do something crazy like this (the
> usual order we do things is rmdir() and then check for errors).
I think, it's rather early to send [PATCHES v2] (with updated function
names), will wait for your comments.
^ 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