* 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
* Re: Funny: git -p submodule summary
From: Johannes Sixt @ 2009-01-09 10:36 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20090109101335.GA4346@coredump.intra.peff.net>
Jeff King schrieb:
> 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.
Ah, no, it would not survive.
But there's a more serious problem why we cannot use start_async() in its
current form: It expects that there is a *function* that produces the
output; but here we don't have a function - output is produced by
*returning* (from setup_pager).
I'll test your other patch (that replaces the execvp in git.c by run_command).
-- Hannes
^ permalink raw reply
* Re: Funny: git -p submodule summary
From: Jeff King @ 2009-01-09 10:47 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Johannes Schindelin, git
In-Reply-To: <496728B9.7090200@viscovery.net>
On Fri, Jan 09, 2009 at 11:36:41AM +0100, Johannes Sixt wrote:
> Ah, no, it would not survive.
>
> But there's a more serious problem why we cannot use start_async() in its
> current form: It expects that there is a *function* that produces the
> output; but here we don't have a function - output is produced by
> *returning* (from setup_pager).
Good point.
> I'll test your other patch (that replaces the execvp in git.c by run_command).
Note that it only covers the case of external commands. Hitting ctrl-c
to interrupt git will still cause funniness. For that we need to
intercept signals to call wait_for_pager(). But there is a slight snag:
we also intercept signals elsewhere (e.g., for tempfile cleanup). So we
need to start remembering the old signal handlers everywhere and
chaining to them.
-Peff
^ permalink raw reply
* Re: [PATCH, resend] git-commit: colored status when color.ui is set
From: Johannes Schindelin @ 2009-01-09 10:56 UTC (permalink / raw)
To: Junio C Hamano; +Cc: markus.heidelberg, git
In-Reply-To: <7viqoo26rq.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 9 Jan 2009, Junio C Hamano wrote:
> 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.
I had the same reaction, so I would like to see this reasoning in the
commit message.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC PATCH] make diff --color-words customizable
From: Johannes Schindelin @ 2009-01-09 11:15 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <200901090151.10880.trast@student.ethz.ch>
Hi,
On Fri, 9 Jan 2009, Thomas Rast wrote:
> 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.)
Oh, I was fooled by your use of an array of enums whose purpose I did not
understand at all.
> > BTW I think you could do what you intended to do with a _way_ smaller
> > and more intuitive patch.
>
> How?
Intuitively, all you would have to do is to replace this part in
diff_words_show()
for (i = 0; i < minus.size; i++)
if (isspace(minus.ptr[i]))
minus.ptr[i] = '\n';
by a loop finding the next word boundary. I would suggest making that a
function, say,
int find_word_boundary(struct diff_words_data *data, char *minus);
This function would also be responsible to initialize the regexp.
However, as I said, I think it would be much more intuitive to
characterize the _words_ instead of the _word boundaries_.
And I would like to keep the default as-is (together _with_ the
performance. IOW if the user did not specify a regexp, it should fall
back to what it does now, which is slow enough).
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC PATCH] make diff --color-words customizable
From: Johannes Schindelin @ 2009-01-09 11:18 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Rast, git
In-Reply-To: <20090109095300.GA4099@coredump.intra.peff.net>
Hi,
On Fri, 9 Jan 2009, Jeff King wrote:
> On Fri, Jan 09, 2009 at 01:05:05AM +0100, Thomas Rast wrote:
>
> > 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.
I am not sure I would want that in the config or the attributes. For me,
it always has been a question of "oh, that LaTeX diff looks ugly, let's
see what words actually changed".
Only rarely did I wish for a different word boundary detection algorithm.
So I'd rather have an alias than a config/attribute setting.
Ciao,
Dscho
^ permalink raw reply
* Re: [RFC PATCH] make diff --color-words customizable
From: Jeff King @ 2009-01-09 11:22 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Thomas Rast, git
In-Reply-To: <alpine.DEB.1.00.0901091215590.30769@pacific.mpi-cbg.de>
On Fri, Jan 09, 2009 at 12:18:37PM +0100, Johannes Schindelin wrote:
> > 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.
>
> I am not sure I would want that in the config or the attributes. For me,
> it always has been a question of "oh, that LaTeX diff looks ugly, let's
> see what words actually changed".
>
> Only rarely did I wish for a different word boundary detection algorithm.
>
> So I'd rather have an alias than a config/attribute setting.
I am not sure what you are saying.
If it is "I do not want color-words on by default for LaTeX", then I
agree. I meant merely that _if_ color-words is enabled, the word
boundaries would be taken from the diff driver config (just like we do
for matching the funcname header).
If it is "I want to specify the color-words boundary on a per-run basis
rather than a per-file basis", then I want the opposite. However, there
is no reason that both cannot be supported (with command line or
environment taking precedence over what's in the config).
-Peff
^ permalink raw reply
* [ILLUSTRATION PATCH] color-words: take an optional regular expression describing words
From: Johannes Schindelin @ 2009-01-09 11:59 UTC (permalink / raw)
To: Thomas Rast; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901091202250.30769@pacific.mpi-cbg.de>
In some applications, words are not delimited by white space. To
allow for that, you can specify a regular expression describing
what makes a word with
git diff --color-words='^[A-Za-z0-9]*'
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
On Fri, 9 Jan 2009, Johannes Schindelin wrote:
> Intuitively, all you would have to do is to replace this part in
> diff_words_show()
>
> for (i = 0; i < minus.size; i++)
> if (isspace(minus.ptr[i]))
> minus.ptr[i] = '\n';
>
> by a loop finding the next word boundary. I would suggest making that a
> function, say,
>
> int find_word_boundary(struct diff_words_data *data, char *minus);
>
> This function would also be responsible to initialize the regexp.
>
> However, as I said, I think it would be much more intuitive to
> characterize the _words_ instead of the _word boundaries_.
>
> And I would like to keep the default as-is (together _with_ the
> performance. IOW if the user did not specify a regexp, it should fall
> back to what it does now, which is slow enough).
And this patch does all that, and it _is_ substantially more
compact, as promised.
It lacks testing, a test script and documentation, as well as
configurability via config and/or attributes, but that's your
job, as I am not really _that_ interested in the feature myself.
diff.c | 45 +++++++++++++++++++++++++++++++++++++++------
diff.h | 1 +
2 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index 4643ffc..c7ddb60 100644
--- a/diff.c
+++ b/diff.c
@@ -339,6 +339,7 @@ static void diff_words_append(char *line, unsigned long len,
struct diff_words_data {
struct diff_words_buffer minus, plus;
FILE *file;
+ regex_t *word_regex;
};
static void print_word(FILE *file, struct diff_words_buffer *buffer, int len, int color,
@@ -398,6 +399,25 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
}
}
+static int find_word_boundary(struct diff_words_data *diff_words,
+ mmfile_t *buffer, int i)
+{
+ if (i >= buffer->size)
+ return i;
+
+ if (diff_words->word_regex) {
+ regmatch_t match[1];
+ if (!regexec(diff_words->word_regex, buffer->ptr + i,
+ 1, match, 0))
+ i += match[0].rm_eo;
+ }
+ else
+ while (i < buffer->size && !isspace(i))
+ i++;
+
+ return i;
+}
+
/* this executes the word diff on the accumulated buffers */
static void diff_words_show(struct diff_words_data *diff_words)
{
@@ -412,17 +432,17 @@ static void diff_words_show(struct diff_words_data *diff_words)
minus.size = diff_words->minus.text.size;
minus.ptr = xmalloc(minus.size);
memcpy(minus.ptr, diff_words->minus.text.ptr, minus.size);
- for (i = 0; i < minus.size; i++)
- if (isspace(minus.ptr[i]))
- minus.ptr[i] = '\n';
+ for (i = 0; (i = find_word_boundary(diff_words, &minus, i))
+ < minus.size; i++)
+ minus.ptr[i] = '\n';
diff_words->minus.current = 0;
plus.size = diff_words->plus.text.size;
plus.ptr = xmalloc(plus.size);
memcpy(plus.ptr, diff_words->plus.text.ptr, plus.size);
- for (i = 0; i < plus.size; i++)
- if (isspace(plus.ptr[i]))
- plus.ptr[i] = '\n';
+ for (i = 0; (i = find_word_boundary(diff_words, &plus, i))
+ < plus.size; i++)
+ plus.ptr[i] = '\n';
diff_words->plus.current = 0;
xpp.flags = XDF_NEED_MINIMAL;
@@ -461,6 +481,7 @@ static void free_diff_words_data(struct emit_callback *ecbdata)
free (ecbdata->diff_words->minus.text.ptr);
free (ecbdata->diff_words->plus.text.ptr);
+ free(ecbdata->diff_words->word_regex);
free(ecbdata->diff_words);
ecbdata->diff_words = NULL;
}
@@ -1483,6 +1504,14 @@ static void builtin_diff(const char *name_a,
ecbdata.diff_words =
xcalloc(1, sizeof(struct diff_words_data));
ecbdata.diff_words->file = o->file;
+ if (o->word_regex) {
+ ecbdata.diff_words->word_regex = (regex_t *)
+ xmalloc(sizeof(regex_t));
+ if (regcomp(ecbdata.diff_words->word_regex,
+ o->word_regex, REG_EXTENDED))
+ die ("Invalid regular expression: %s",
+ o->word_regex);
+ }
}
xdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,
&xpp, &xecfg, &ecb);
@@ -2496,6 +2525,10 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
DIFF_OPT_CLR(options, COLOR_DIFF);
else if (!strcmp(arg, "--color-words"))
options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
+ else if (!prefixcmp(arg, "--color-words=")) {
+ options->flags |= DIFF_OPT_COLOR_DIFF | DIFF_OPT_COLOR_DIFF_WORDS;
+ options->word_regex = arg + 14;
+ }
else if (!strcmp(arg, "--exit-code"))
DIFF_OPT_SET(options, EXIT_WITH_STATUS);
else if (!strcmp(arg, "--quiet"))
diff --git a/diff.h b/diff.h
index 4d5a327..23cd90c 100644
--- a/diff.h
+++ b/diff.h
@@ -98,6 +98,7 @@ struct diff_options {
int stat_width;
int stat_name_width;
+ const char *word_regex;
/* this is set by diffcore for DIFF_FORMAT_PATCH */
int found_changes;
--
1.6.1.203.gc8be3
^ permalink raw reply related
* Re: [ILLUSTRATION PATCH] color-words: take an optional regular expression describing words
From: Thomas Rast @ 2009-01-09 12:24 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.1.00.0901091255230.30769@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]
Johannes Schindelin wrote:
>
> In some applications, words are not delimited by white space. To
> allow for that, you can specify a regular expression describing
> what makes a word with
>
> git diff --color-words='^[A-Za-z0-9]*'
[...]
> > Intuitively, all you would have to do is to replace this part in
> > diff_words_show()
> >
> > for (i = 0; i < minus.size; i++)
> > if (isspace(minus.ptr[i]))
> > minus.ptr[i] = '\n';
> >
> > by a loop finding the next word boundary.
[...]
> > However, as I said, I think it would be much more intuitive to
> > characterize the _words_ instead of the _word boundaries_.
That doesn't work. You cannot overwrite actual content in the strings
to be diffed with newlines. The current --color-words exploits the
fact that we don't care about spaces anyway, so we might as well
replace them with newlines, but we _do_ care about the words and in
the regexed version, you have no guarantees about where they might start.
To wit:
thomas@thomas:~/tmp/foo(master)$ cat >foo
foo_bar_baz
quux
thomas@thomas:~/tmp/foo(master)$ git add foo
thomas@thomas:~/tmp/foo(master)$ git ci -m initial
[master (root-commit)]: created f110c6c: "initial"
1 files changed, 2 insertions(+), 0 deletions(-)
create mode 100644 foo
thomas@thomas:~/tmp/foo(master)$ cat >foo
foo_
ar_
az
quux
thomas@thomas:~/tmp/foo(master)$ git diff
diff --git i/foo w/foo
index 5b34f11..a2762c6 100644
--- i/foo
+++ w/foo
@@ -1,2 +1,4 @@
-foo_bar_baz
+foo_
+ar_
+az
quux
thomas@thomas:~/tmp/foo(master)$ git diff --color-words
diff --git i/foo w/foo
index 5b34f11..a2762c6 100644
--- i/foo
+++ w/foo
@@ -1,2 +1,4 @@
foo_bar_bafoo_
ar_
az
quux
thomas@thomas:~/tmp/foo(master)$ git diff --color-words='[a-zA-Z]+_?'
diff --git i/foo w/foo
index 5b34f11..a2762c6 100644
--- i/foo
+++ w/foo
@@ -1,2 +1,4 @@
quux
Even without the colours, you can see that it has a blind spot for
changes around a newline. Perhaps there is an easier way to remember
them, but we definitely cannot *forget* about the word boundaries.
That being said, even though my patch correctly sees the changes, the
above test case also exposes some sort of string overrun :-(
> > And I would like to keep the default as-is (together _with_ the
> > performance. IOW if the user did not specify a regexp, it should fall
> > back to what it does now, which is slow enough).
That's definitely a valid request.
I'll come up with a fixed patch, and probably make it both
funcname-like (Jeff's idea) and command line configurable.
--
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: [ILLUSTRATION PATCH] color-words: take an optional regular expression describing words
From: Teemu Likonen @ 2009-01-09 13:05 UTC (permalink / raw)
To: Thomas Rast; +Cc: Johannes Schindelin, git
In-Reply-To: <200901091324.40583.trast@student.ethz.ch>
Thomas Rast (2009-01-09 13:24 +0100) wrote:
> Johannes Schindelin wrote:
>> > And I would like to keep the default as-is (together _with_ the
>> > performance. IOW if the user did not specify a regexp, it should
>> > fall back to what it does now, which is slow enough).
>
> That's definitely a valid request.
I agree with that too. A good thing about the current --color-words is
that it automatically works with UTF-8 encoded text. This is _very_
important as --color-words is usually the best diff tool for
human-language texts.
^ permalink raw reply
* Re: [PATCH 0/3] Teach Git about the patience diff algorithm
From: Johannes Schindelin @ 2009-01-09 13:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: Adeodato Simó, Linus Torvalds, Clemens Buchacher,
Pierre Habouzit, davidel, Francis Galiegue, Git ML
In-Reply-To: <7v7i552clz.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 8 Jan 2009, Junio C Hamano wrote:
> 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.
So we'd need something like this (I still think we should treat curly
brackets the same as punctuation, and for good measure I just handled
everything that is not alphanumerical the same):
-- snipsnap --
[TOY PATCH] Add diff option '--collapse-non-alnums'
With the option --collapse-non-alnums, there will be no interhunks
consisting solely of non-alphanumerical letters.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
diff.c | 2 ++
xdiff/xdiff.h | 1 +
xdiff/xdiffi.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
3 files changed, 50 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index c7ddb60..4b387fb 100644
--- a/diff.c
+++ b/diff.c
@@ -2503,6 +2503,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
options->xdl_opts |= XDF_IGNORE_WHITESPACE_AT_EOL;
else if (!strcmp(arg, "--patience"))
options->xdl_opts |= XDF_PATIENCE_DIFF;
+ else if (!strcmp(arg, "--collapse-non-alnums"))
+ options->xdl_opts |= XDF_COLLAPSE_NON_ALNUMS;
/* flags options */
else if (!strcmp(arg, "--binary")) {
diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
index 4da052a..a444f9a 100644
--- a/xdiff/xdiff.h
+++ b/xdiff/xdiff.h
@@ -33,6 +33,7 @@ extern "C" {
#define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3)
#define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4)
#define XDF_PATIENCE_DIFF (1 << 5)
+#define XDF_COLLAPSE_NON_ALNUMS (1 << 6)
#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL)
#define XDL_PATCH_NORMAL '-'
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 3e97462..b8e7ee8 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -396,6 +396,50 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
return xch;
}
+static int xdl_record_contains_alnum(xrecord_t *record)
+{
+ long i;
+ for (i = 0; i < record->size; i++)
+ if (isalnum(record->ptr[i]))
+ return 1;
+ return 0;
+}
+
+static int xdl_collapse_non_alnum(xdfile_t *xdf, xdfile_t *xdfo)
+{
+ long ix, ixo, len = 0;
+
+ /*
+ * Collapse all interhunk parts consisting solely of non-alnum
+ * characters into the hunks.
+ */
+ for (ix = 0, ixo = 0; ix < xdf->nrec && ixo < xdfo->nrec; ix++, ixo++) {
+ if (xdf->rchg[ix] == 1 || xdfo->rchg[ixo] == 1) {
+ /* collapse non-alnum interhunks */
+ while (len > 0) {
+ xdf->rchg[ix - len] = 1;
+ xdfo->rchg[ixo - len] = 1;
+ len--;
+ }
+
+ /* look for end of hunk */
+ while (ix < xdf->nrec && xdf->rchg[ix] == 1)
+ ix++;
+ while (ixo < xdfo->nrec && xdfo->rchg[ixo] == 1)
+ ixo++;
+ if (ix >= xdf->nrec)
+ return 0;
+ len = !xdl_record_contains_alnum(xdf->recs[ix]);
+ }
+ else if (len > 0) {
+ if (xdl_record_contains_alnum(xdf->recs[ix]))
+ len = 0;
+ else
+ len++;
+ }
+ }
+ return 0;
+}
int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
long ix, ixo, ixs, ixref, grpsiz, nrec = xdf->nrec;
@@ -548,7 +592,9 @@ int xdl_diff(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp,
return -1;
}
- if (xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
+ if (((xpp->flags & XDF_COLLAPSE_NON_ALNUMS) &&
+ xdl_collapse_non_alnum(&xe.xdf1, &xe.xdf2)) ||
+ xdl_change_compact(&xe.xdf1, &xe.xdf2, xpp->flags) < 0 ||
xdl_change_compact(&xe.xdf2, &xe.xdf1, xpp->flags) < 0 ||
xdl_build_script(&xe, &xscr) < 0) {
--
1.6.1.203.gc8be3
^ permalink raw reply related
* Re: Comments on Presentation Notes Request.
From: Jakub Narebski @ 2009-01-09 13:50 UTC (permalink / raw)
To: Tim Visher; +Cc: git
In-Reply-To: <c115fd3c0901061433i78bf3b26v77e5981aada6728e@mail.gmail.com>
"Tim Visher" <tim.visher@gmail.com> writes:
> Hello Everyone,
>
> I'm putting together a little 15 minute presentation for my company
> regarding SCMSes in an attempt to convince them to at the very least
> use a Distributed SCMS and at best to use git. I put together all my
> notes, although I didn't put together the actual presentation yet. I
> figured I'd post them here and maybe get some feedback about it. Let
> me know what you think.
>
> Thanks in advance!
Take a look at the following links:
* "Understanding Version-Control Systems (DRAFT)" by Eric Raymond
http://www.catb.org/esr/writings/version-control/version-control.html
* "Version Control Habits of Effective Developers" at The Daily Build
http://blog.bstpierre.org/version-control-habits
Note that the first one is DRAFT; on the other hand it explains
lock-edit, merge-then-commit, and commit-then-merge workflows quite
well, and has a host of links.
--
Jakub Narebski
Poland
ShadeHawk on #git
^ permalink raw reply
* Re: 1.5.6.5 fails to clone git.kernel.org/[...]/rostedt/linux-2.6-rt
From: Miklos Vajna @ 2009-01-09 14:08 UTC (permalink / raw)
To: Tim Shepard; +Cc: git
In-Reply-To: <E1LLAn5-0001JM-00@alva.home>
[-- Attachment #1: Type: text/plain, Size: 596 bytes --]
On Fri, Jan 09, 2009 at 01:24:19AM -0500, Tim Shepard <shep@alum.mit.edu> wrote:
>
>
> 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
I would use the git:// link from gitweb.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] allow 8bit data in email body sent by send-email
From: Andre Przywara @ 2009-01-09 14:16 UTC (permalink / raw)
To: Jeff King; +Cc: git, Andre Przywara
In-Reply-To: <20090109072814.GA21180@coredump.intra.peff.net>
Jeff King 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.
Well, this could be discussed, after all the problem lies in the actual
transportation, which should be the responsibility of git-send-email.
But I am OK with putting this into format-patch.
> What exactly is the workflow you use to generate this problem?
I use git format-patch to generate a patch file for a single-mail patch
(not a patch series). Then I edit this file manually to add questions
and comments and include my signature. During this step the umlauts came
in. If you have a suggestion to improve this workflow, I am all ears, I
am fairly new to git.
> Does it matter where the non-ascii characters are
> (commit versus patch, etc)?
Oh, right you are. If there are 8bit characters in the commit message,
git-format-patch adds the appropriate headers.
> What version of git are you using?
Version 1.5.5 on one machine and 1.5.2.2 on another. I know, i know ;-)
but I haven't had time to compile a newer one, yet.
>
>> 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.
Ok, so what about adding a flag to git-format-patch that forces the 8bit
headers on? I think a workaround would be to add a --subject-prefix with
a special character and later remove this, but this is not really a
long-term solution ;-)
Thanks and regards,
Andre.
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 277-84917
----to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Jochen Polster; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632
^ 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