* Re: [PATCH] Makefile: add uninstall target. Fixes elementary good cleaning manners.
From: Matthieu Moy @ 2009-11-18 13:28 UTC (permalink / raw)
To: Jan Nieuwenhuizen; +Cc: Tomas Carnecky, git
In-Reply-To: <1258547389.25909.101.camel@heerbeest>
Jan Nieuwenhuizen <janneke-list@xs4all.nl> writes:
> Sorry. Let me retry that. See below.
Please, read Documentation/SubmittingPatches
>>From f260a4dcf0b42088eb1da74aee49f49ac4b0c55b Mon Sep 17 00:00:00 2001
> From: Jan Nieuwenhuizen <janneke@gnu.org>
> Date: Wed, 11 Nov 2009 14:19:00 +0100
> Subject: [PATCH] Makefile: add uninstall target. Fixes elementary good cleaning manners.
>
> * Modified Makefile
> * Modified gitk-git/Makefile
> * Modified perl/Makefile
> * Modified templates/Makefile
Git knows better than you which files are modified by a commit, so
it's counter-productive to rewrite by hand this list in the commit
message. This place is the place to explain _why_ your change is good
(to convince the maintainer to apply it in git.git in particular).
> Makefile | 18 +++++++++++++++++-
> gitk-git/Makefile | 2 ++
> perl/Makefile | 2 +-
> templates/Makefile | 5 +++++
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: [ANNOUNCE] codeBeamer MR - Easy ACL for Git
From: Intland Software @ 2009-11-18 13:19 UTC (permalink / raw)
To: Petr Baudis; +Cc: git
In-Reply-To: <20091118120936.GL17748@machine.or.cz>
Thanks for your comments, Petr.
Petr Baudis wrote:
>> LEARN MORE & DOWNLOAD
>> More details, screenshots and downloads:
>> http://www.intland.com/products/cb-mr/overview.html
>
> Interesting, thank you for the announcement; it would be good to note
> that it's not open-source.
That's right, codeBeamer MR is not open source.
More precisely: parts of the source code are actually open, including
the wiki plugins and the remote clients, for instance. The core source
is closed, because the same core is also used in our commercial offerings, and
our commercial license doesn't (currently) allow publishing the
complete code. We have quite some large customers from the defense space
that would not be happy if we opened everything ;)
We are currently in the midst of rethinking our licensing scheme
in general, to make things more liberal or to set up some kind of a
dual license.
> I think a lot of people wonder now, how does this compare to existing
> solutions; from your announcement I thought it's something like
> Gitosis/Gitolite, but in fact it seems more similar to Gitorious or
> GitHub (if it was publicly available, of course); perhaps it would be
> good idea to present comparison to these on the project homepage.
Good point. More on this later.
---
Intland
^ permalink raw reply
* Re: Hey - A Conceptual Simplication....
From: Thomas Rast @ 2009-11-18 13:30 UTC (permalink / raw)
To: George Dennie; +Cc: git, torvalds
In-Reply-To: <005a01ca684e$71a1d710$54e58530$@com>
George Dennie wrote:
>
> Instead, Git is treating a manually maintained list of files within the
> working tree as the versioned document, this list being initialized and
> manually amended by the "Git add/rm/mv" commands, etc.
This feature is called the "index", and is not merely a list of the
files, but also their content. Please read
http://tomayko.com/writings/the-thing-about-git
for a nice explanation why this is a good and useful thing.
> "Git commit -x" -- performs a "Git add ." then a "Git commit"
> "Git checkout -x" -- that clean the working tree prior to perform a checkout
That would require supernaturally good maintenance of your .gitignore
to avoid adding or (worse) nuking files by accident.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: Hey - A Conceptual Simplication....
From: Jason Sewall @ 2009-11-18 13:31 UTC (permalink / raw)
To: George Dennie; +Cc: git, torvalds
In-Reply-To: <005a01ca684e$71a1d710$54e58530$@com>
On Wed, Nov 18, 2009 at 7:55 AM, George Dennie <gdennie@pospeople.com> wrote:
>
> In particular, why is Git not treating the entire working tree as the
> versioned document (qualified of course by the .gitignore file).
>
> Instead, Git is treating a manually maintained list of files within the
> working tree as the versioned document, this list being initialized and
> manually amended by the "Git add/rm/mv" commands, etc.
Isn't fastidiously maintaining a .gitignore file to contain everything
you *don't* want in the project more confusing than explicitly
specifying things you *do* want in the project?
> The result is conceptual complexity and rather counter-intuitive behavior.
> For example, adding and renaming files outside of Git is not considered
> editing the version until you subsequently do a "Git Add ." Contrast that
> with editing or deleting files outside of Git. Yet adding and renaming files
> and folders is a significant part of substantive projects, especially in the
> early stages and experimental branches.
>
> Granted, this is not a big deal functionally, but what is being lost is
> conceptual simplicity (and consistency, in my book) and conceptual
> simplicity is a key value point, if not THE key.
In fact, it's a big deal in functionality, but the utility is in being
able to to specify exactly what I want to be part of each commit. One
of git's great features is the ability to specify *exactly* what you
want to be part of each commit, down to the line. This means that each
commit can be extremely fine grained and represent specific bug fixes
and or features.
If you have a bunch of debugging code sitting around in your working
tree after you've tracked down a problem, you don't want to commit all
of those printfs, etc. - you want to commit the fix. This has
ramifications from making diffs of history cleaner to making git
bisect actually useful.
> Also can we augment checkout to totally CLEAN the working directory prior to
> a restore. If necessary we can augment .gitignore to stipulate those files
> or folders that should be excluded from the cleaning. This suggestion is in
> recognition of the fact that if you are not versioning the file, it is
> typically trash; which becomes the case when the entire working treat is
> treated as the versioned document.
This is even worse. It's already pretty easy to trash your working
directory by reflexively typing git checkout -f, and you want to
> Consequently, I recommend the following new commands:
> "Git commit -x" -- performs a "Git add ." then a "Git commit"
> "Git checkout -x" -- that clean the working tree prior to perform a
> checkout
I see that Jan has replied with some loaded guns, *ahem* aliases. Go
ahead and use them, but I recommend you look at the diffs in git.git
or some other repository that takes advantage of making commits as
compact as possible, and learn how to use git add -p.
Jason
^ permalink raw reply
* Re: git-svn of both trunk and tags while having no access to the 'parent' of those
From: Yaroslav Halchenko @ 2009-11-18 13:32 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4B03B7D3.8050905@drmicha.warpmail.net>
On Wed, 18 Nov 2009, Michael J Gruber wrote:
> > git svn clone --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo svnrepo.gitsvn
> Your problem description seems to match perfectly with the description
> of the "--no-minimize-url" option in git svn's man page. I'm sure it's
> worth a try.
;-) oh I did I did ;)
I've used
git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo/trunk svnrepo.gitsvn
that initialized repository, went through a lengthy list of
trace: built-in: git 'config' 'svn-remote.svn.tags-maxRev' '...'
with last command reported
trace: built-in: git 'gc' '--auto'
and resulted in nothing being cloned/checked out or even a single ref.
The only file under .git besides the ones created by git init
./svn/refs/remotes/upstream-svn/trunk/.rev_map.33fb83da-1015-0410-9b9b-96027f9a4af8
and if I omitted trunk/ from url -- the same story of needed
authentication
--
Yaroslav O. Halchenko
Postdoctoral Fellow, Department of Psychological and Brain Sciences
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
^ permalink raw reply
* [PATCH] describe: do not use unannotated tag even if exact match
From: Thomas Rast @ 2009-11-18 13:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
4d23660 (describe: when failing, tell the user about options that
work, 2009-10-28) forgot to update the shortcut path where the code
detected and used a possible exact match. This means that an
unannotated tag on HEAD would be used by 'git describe'.
Guard this code path against the new circumstances, where unannotated
tags can be present in ->util even if we're not actually planning to
use them.
While there, also add some tests for --all.
Reported by 'yashi' on IRC.
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
builtin-describe.c | 2 +-
t/t6120-describe.sh | 6 ++++++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/builtin-describe.c b/builtin-describe.c
index d4efb10..71be2a9 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -200,7 +200,7 @@ static void describe(const char *arg, int last_one)
die("%s is not a valid '%s' object", arg, commit_type);
n = cmit->util;
- if (n) {
+ if (n && (tags || all || n->prio == 2)) {
/*
* Exact match to an existing ref.
*/
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index c050f94..065dead 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -92,12 +92,18 @@ check_describe A-* HEAD^
check_describe D-* HEAD^^
check_describe A-* HEAD^^2
check_describe B HEAD^^2^
+check_describe D-* HEAD^^^
check_describe c-* --tags HEAD
check_describe c-* --tags HEAD^
check_describe e-* --tags HEAD^^
check_describe c-* --tags HEAD^^2
check_describe B --tags HEAD^^2^
+check_describe e --tags HEAD^^^
+
+check_describe heads/master --all HEAD
+check_describe tags/c-* --all HEAD^
+check_describe tags/e --all HEAD^^^
check_describe B-0-* --long HEAD^^2^
check_describe A-3-* --long HEAD^^2
--
1.6.5.3.381.gfeb7e
^ permalink raw reply related
* [PATCH RESEND] Explicitly truncate bswap operand to uint32_t
From: Benjamin Kramer @ 2009-11-18 13:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Nicolas Pitre
There are some places in git where a long is passed to htonl/ntohl. llvm
doesn't support matching operands of different bitwidths intentionally.
This patch fixes the build with llvm-gcc (and clang) on x86_64.
Signed-off-by: Benjamin Kramer <benny.kra@googlemail.com>
---
Any comments on this patch?
See also:
* http://llvm.org/bugs/show_bug.cgi?id=3373
* http://lkml.org/lkml/2009/1/23/261
compat/bswap.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/compat/bswap.h b/compat/bswap.h
index 279e0b4..f3b8c44 100644
--- a/compat/bswap.h
+++ b/compat/bswap.h
@@ -24,7 +24,7 @@ static inline uint32_t default_swab32(uint32_t val)
if (__builtin_constant_p(x)) { \
__res = default_swab32(x); \
} else { \
- __asm__("bswap %0" : "=r" (__res) : "0" (x)); \
+ __asm__("bswap %0" : "=r" (__res) : "0" ((uint32_t)(x))); \
} \
__res; })
--
1.6.5.3.149.g9aa3
^ permalink raw reply related
* Re: git-svn of both trunk and tags while having no access to the 'parent' of those
From: Michael J Gruber @ 2009-11-18 13:56 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git
In-Reply-To: <20091118133205.GB17964@onerussian.com>
Yaroslav Halchenko venit, vidit, dixit 18.11.2009 14:32:
>
> On Wed, 18 Nov 2009, Michael J Gruber wrote:
>
>>> git svn clone --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo svnrepo.gitsvn
>
>> Your problem description seems to match perfectly with the description
>> of the "--no-minimize-url" option in git svn's man page. I'm sure it's
>> worth a try.
>
> ;-) oh I did I did ;)
> I've used
>
> git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo/trunk svnrepo.gitsvn
Is the trunk really at svnrepo/trunk/trunk?
> that initialized repository, went through a lengthy list of
>
> trace: built-in: git 'config' 'svn-remote.svn.tags-maxRev' '...'
>
> with last command reported
>
> trace: built-in: git 'gc' '--auto'
>
> and resulted in nothing being cloned/checked out or even a single ref.
> The only file under .git besides the ones created by git init
>
> ./svn/refs/remotes/upstream-svn/trunk/.rev_map.33fb83da-1015-0410-9b9b-96027f9a4af8
>
> and if I omitted trunk/ from url -- the same story of needed
> authentication
I would try both
git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t
releases http://domain.com/svnrepo/ svnrepo.gitsvn
and also the seemingly equivalent
git svn clone --no-minimize-url --prefix=upstream-svn/ -T
http://domain.com/svnrepo/trunk -t http://domain.com/svnrepo/releases
svnrepo.gitsvn
Also, I assume you can svn list http://domain.com/svnrepo/trunk and
http://domain.com/svnrepo/releases ;)
Michael
^ permalink raw reply
* Re: Hey - A Conceptual Simplication....
From: Jonathan del Strother @ 2009-11-18 13:18 UTC (permalink / raw)
To: George Dennie; +Cc: git, torvalds
In-Reply-To: <005a01ca684e$71a1d710$54e58530$@com>
2009/11/18 George Dennie <gdennie@pospeople.com>:
> A Clean checkout command might be...
>
> The Git model does not seem to go far enough conceptually, for some
> unexplainable reason...
>
> In particular, why is Git not treating the entire working tree as the
> versioned document (qualified of course by the .gitignore file).
>
> Instead, Git is treating a manually maintained list of files within the
> working tree as the versioned document, this list being initialized and
> manually amended by the "Git add/rm/mv" commands, etc.
>
> The result is conceptual complexity and rather counter-intuitive behavior.
> For example, adding and renaming files outside of Git is not considered
> editing the version until you subsequently do a "Git Add ." Contrast that
> with editing or deleting files outside of Git. Yet adding and renaming files
> and folders is a significant part of substantive projects, especially in the
> early stages and experimental branches.
>
> Granted, this is not a big deal functionally, but what is being lost is
> conceptual simplicity (and consistency, in my book) and conceptual
> simplicity is a key value point, if not THE key.
>
> Also can we augment checkout to totally CLEAN the working directory prior to
> a restore. If necessary we can augment .gitignore to stipulate those files
> or folders that should be excluded from the cleaning. This suggestion is in
> recognition of the fact that if you are not versioning the file, it is
> typically trash; which becomes the case when the entire working treat is
> treated as the versioned document.
>
> Consequently, I recommend the following new commands:
> "Git commit -x" -- performs a "Git add ." then a "Git commit"
> "Git checkout -x" -- that clean the working tree prior to perform a
> checkout
>
Perhaps try 'git commit -a' and 'git checkout -f' ?
^ permalink raw reply
* Re: [PATCH] Give the hunk comment its own color
From: Jeff King @ 2009-11-18 14:23 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Junio C Hamano, git
In-Reply-To: <1258543836-799-1-git-send-email-bert.wesarg@googlemail.com>
On Wed, Nov 18, 2009 at 12:30:36PM +0100, Bert Wesarg wrote:
> Insired by the coloring of quilt.
>
> Introduce a separate color for the hunk comment part, i.e. the current
> function. Whitespace between hunk header and hunk comment is now
> printed as plain.
>
> The current default is magenta. But I'm not settled on this. My
> favorite would be bold yellow.
I don't see any reason not to add this, as it is simply introducing one
extra knob to tweak for people who care. However, after some
experimentation, I found that I don't personally really like it. I ended
up wanting it set to the same color as the hunk header.
I wonder how hard it would be to make it backwards-compatible; that is,
to inherit the color value of the hunk header (be it the original or one
set by the user) unless the func color is set by the user. But maybe
that is over-engineering. It is not like we are breaking scripts, and it
is not that hard for people to see the new behavior and then tweak their
config if they don't like it.
-Peff
PS I almost complained about your default of "magenta" as the same as
the meta color before I remembered that magenta meta is a personal
setting I use. Personally I find the bold meta color to be distractingly
ugly. Blaming it, the default seems to come from Linus, who even in his
commit message (50f575f) seems to indicate that it is somewhat arbitrary
(mostly just dropping the purple from the bold purple).
I'm not sure what is the best way to arrive at a default color for
something like this. Arguing about it really is almost the definition of
bikeshedding. Maybe next year's git survey should contain a special
section on colors, and majority should rule. :)
^ permalink raw reply
* Re: git-svn of both trunk and tags while having no access to the 'parent' of those
From: Yaroslav Halchenko @ 2009-11-18 14:23 UTC (permalink / raw)
To: Michael J Gruber; +Cc: git
In-Reply-To: <4B03FD29.3090001@drmicha.warpmail.net>
On Wed, 18 Nov 2009, Michael J Gruber wrote:
> > git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo/trunk svnrepo.gitsvn
> Is the trunk really at svnrepo/trunk/trunk?
nope... it is just svnrepo/trunk but if I set url to point to parent --
git svn seeks authentication right away
> I would try both
> git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t
> releases http://domain.com/svnrepo/ svnrepo.gitsvn
asks for authentication since there is no public access to
http://domain.com/svnrepo/
> and also the seemingly equivalent
> git svn clone --no-minimize-url --prefix=upstream-svn/ -T
> http://domain.com/svnrepo/trunk -t http://domain.com/svnrepo/releases
> svnrepo.gitsvn
seems to not work since it wants url as a parameter
Bad URL passed to RA layer: Illegal repository URL svnrepo.gitsvn at /usr/lib/git-core/git-svn line 940
> Also, I assume you can svn list http://domain.com/svnrepo/trunk and
> http://domain.com/svnrepo/releases ;)
yeap -- I can list both of those but not their parent.
--
Yaroslav O. Halchenko
Postdoctoral Fellow, Department of Psychological and Brain Sciences
Dartmouth College, 419 Moore Hall, Hinman Box 6207, Hanover, NH 03755
Phone: +1 (603) 646-9834 Fax: +1 (603) 646-1419
WWW: http://www.linkedin.com/in/yarik
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #04; Tue, 17)
From: Nguyen Thai Ngoc Duy @ 2009-11-18 14:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7hto46ce.fsf@alter.siamese.dyndns.org>
On 11/18/09, Junio C Hamano <gitster@pobox.com> wrote:
> * nd/sparse (2009-08-20) 19 commits.
> - sparse checkout: inhibit empty worktree
> - Add tests for sparse checkout
> - read-tree: add --no-sparse-checkout to disable sparse checkout support
> - unpack-trees(): ignore worktree check outside checkout area
> - unpack_trees(): apply $GIT_DIR/info/sparse-checkout to the final index
> - unpack-trees(): "enable" sparse checkout and load $GIT_DIR/info/sparse-checkout
> - unpack-trees.c: generalize verify_* functions
> - unpack-trees(): add CE_WT_REMOVE to remove on worktree alone
> - Introduce "sparse checkout"
> - dir.c: export excluded_1() and add_excludes_from_file_1()
> - excluded_1(): support exclude files in index
> - unpack-trees(): carry skip-worktree bit over in merged_entry()
> - Read .gitignore from index if it is skip-worktree
> - Avoid writing to buffer in add_excludes_from_file_1()
> - Teach Git to respect skip-worktree bit (writing part)
> - Teach Git to respect skip-worktree bit (reading part)
> - Introduce "skip-worktree" bit in index, teach Git to get/set this bit
> - Add test-index-version
> - update-index: refactor mark_valid() in preparation for new options
>
> The latest update I didn't look at very closely but I had an impression
> that it was touching very generic codepath that would affect non sparse
> cases, iow the patch looked very scary (the entire series already is).
I wonder if there is any other approach for sparse checkout? I'll see
if I can improve it, but with a series touching unpack logic, diff
core, .gitattributes/.gitignore, it's hard to get it right and
obvious.
--
Duy
^ permalink raw reply
* git-mailinfo doesn't stop parsing at the end of the header
From: Philip Hofstetter @ 2009-11-18 14:20 UTC (permalink / raw)
To: git
Hello,
today, after working on a topic branch and trying to rebase it on top
of the updated master, the rebase failed, complaining about an invalid
email address.
Some investigating revealed an interesting quirk in git-mailinfo which
seems to be a bit too eager to extract author information: Instead of
just looking at the From:-Line in a mails header (git-rebase seems to
use git-am which in turn uses git-mailinfo), it searches for "from:"
*anywhere* in the mail and uses the last found information as the
source for the author information.
In this case, git-format-patch has generated a file that looks
something like this:
--------------8<---------------
From d28f21ea8ca64681ba7756417799ceea81ad6873 Mon Sep 17 00:00:00 2001
From: Foo Bar <foo@bar.com>
Date: Tue, 17 Nov 2009 15:27:25 +0100
Subject: blah, blah, blah
from:
- this is a
- list for stuff
---
list/of/changed/files | 1 -
list/of/changed/files2 | 1 -
2 files changed, 0 insertions(+), 2 deletions(-)
the actual diff down here
--------------8<---------------
And when you feed this into mailinfo, this is what you get:
pilif@celes ~/git % git mailinfo /dev/null /dev/null < somepatch.patch
Author:
Email:
Subject: blah, blah, blah
Date: Tue, 17 Nov 2009 15:27:25 +0100
pilif@celes ~/git %
and consequently, anything that depends on the correct author being
extracted then fails.
While I know it's rude to have a line beginning with "from:" (and it's
even ruder to have a line beginning with "from "), IMHO the header
ends at the first blank line and I see no reason to extract author
information past the header.
And if this is in fact intended behavior, it should probably not be
permitted to create a commit that later on can't be rebased or applied
using git-am.
I had a look at the source of git-mailinfo to fix it myself, but this
thing does too much for my minimal knowledge in C.
Philip
--
Sensational AG
Giesshübelstrasse 62c, Postfach 1966, 8021 Zürich
Tel. +41 43 544 09 60, Mobile +41 79 341 01 99
info@sensational.ch, http://www.sensational.ch
^ permalink raw reply
* Re: [PATCHv3] Add branch management for releases to gitworkflows
From: Thomas Rast @ 2009-11-18 14:59 UTC (permalink / raw)
To: Raman Gupta; +Cc: Nanako Shiraishi, git, gitster, skillzero
In-Reply-To: <4B033D8F.1080309@fastmail.fm>
Raman Gupta wrote:
>
> I *am* a native English speaker. Sadly, its the *only* language I
> speak, read, and write. However, additional comments would
> definitely be nice.
Oh, my apologies. I just looked at the names and jumped to
conclusions from there.
> Agree. I reworded the sections to untangle the information
> somewhat. Let me know what you think.
[...]
> * `git merge --ff-only master`
> =====================================
>
[...]
> +If the merge fails because it is not a fast-forward, then it is
> +possible some fixes on 'maint' were missed in the feature release.
> +This will not happen if the content of the branches was verified as
> +described in the previous section.
Yes, I think that is nicer. It's no longer a repetition of what was
said above, but merely points out what could have gone wrong and where
to look for advice. The last sentence sounds a bit like "ha ha we
told you so!" though ;-)
FWIW, you can add my
Acked-by: Thomas Rast <trast@student.ethz.ch>
to the final (squashed) patch.
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply
* Re: git-svn of both trunk and tags while having no access to the 'parent' of those
From: Michael J Gruber @ 2009-11-18 15:07 UTC (permalink / raw)
To: Yaroslav Halchenko; +Cc: git, Eric Wong
In-Reply-To: <20091118142332.GC17964@onerussian.com>
Yaroslav Halchenko venit, vidit, dixit 18.11.2009 15:23:
>
> On Wed, 18 Nov 2009, Michael J Gruber wrote:
>>> git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t releases http://domain.com/svnrepo/trunk svnrepo.gitsvn
>> Is the trunk really at svnrepo/trunk/trunk?
> nope... it is just svnrepo/trunk but if I set url to point to parent --
> git svn seeks authentication right away
>
>> I would try both
>> git svn clone --no-minimize-url --prefix=upstream-svn/ -T trunk -t
>> releases http://domain.com/svnrepo/ svnrepo.gitsvn
>
> asks for authentication since there is no public access to
> http://domain.com/svnrepo/
>
>> and also the seemingly equivalent
>
>> git svn clone --no-minimize-url --prefix=upstream-svn/ -T
>> http://domain.com/svnrepo/trunk -t http://domain.com/svnrepo/releases
>> svnrepo.gitsvn
> seems to not work since it wants url as a parameter
>
> Bad URL passed to RA layer: Illegal repository URL svnrepo.gitsvn at /usr/lib/git-core/git-svn line 940
>
>> Also, I assume you can svn list http://domain.com/svnrepo/trunk and
>> http://domain.com/svnrepo/releases ;)
> yeap -- I can list both of those but not their parent.
>
>
OK, so the way it's implemented --no-minimize-url might be half as
useful as it could be. One last try (before asking Eric...) would be
git svn clone --no-minimize-url --prefix=upstream-svn/ -T
http://domain.com/svnrepo/trunk -t http://domain.com/svnrepo/releases
http://domain.com/svnrepo/trunk svnrepo.gitsvn
because that involves accessible URLs only and trunk and branch URLs are
absolute.
[Meanwhile I git the actual URL PMed by Yaroslov.]
So, what happens with the above is that git-svn tries to set the URL
config again from the URL part of an absolute tags argument. I don't
know how absolute URLs (which are documented) for tags etc. could
possibly work if git-svn tries to do that. Eric?
I tried also with two svn sections to circumvent this:
[svn-remote "svn"]
url = http://domain.com:/project/trunk
fetch = :refs/remotes/trunk
[svn-remote "svnr"]
url = http://domain.com:/project/releases
tags = /*:refs/remotes/tags/*
Fetching -Rsvn works fine, but fetching -Rsvnr gives the same
authentication problems. And fetch does not accept --no-minimize-url as
an option. OTOH: If the option is not used it seems to me (from the
source) that not minimizing is the default, which leaves me really
stomped. Eric?? ;)
Michael
^ permalink raw reply
* [PATCH v2] Give the hunk comment its own color
From: Bert Wesarg @ 2009-11-18 15:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Bert Wesarg
In-Reply-To: <1258543836-799-1-git-send-email-bert.wesarg@googlemail.com>
Inspired by the coloring of quilt.
Introduce a separate color for the hunk comment part, i.e. the current function.
Whitespace between hunk header and hunk comment is now printed as plain.
The current default is magenta. But I'm not settled on this. My favorite would
be bold yellow.
Now with updated test suit.
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
Documentation/config.txt | 8 +++---
combine-diff.c | 5 +++-
diff.c | 64 +++++++++++++++++++++++++++++++++++++++++++--
diff.h | 1 +
t/t4034-diff-words.sh | 3 +-
5 files changed, 72 insertions(+), 9 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index cb73d75..421cd50 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -598,10 +598,10 @@ color.diff.<slot>::
Use customized color for diff colorization. `<slot>` specifies
which part of the patch to use the specified color, and is one
of `plain` (context text), `meta` (metainformation), `frag`
- (hunk header), `old` (removed lines), `new` (added lines),
- `commit` (commit headers), or `whitespace` (highlighting
- whitespace errors). The values of these variables may be specified as
- in color.branch.<slot>.
+ (hunk header), 'func' (function in hunk header), `old` (removed lines),
+ `new` (added lines), `commit` (commit headers), or `whitespace`
+ (highlighting whitespace errors). The values of these variables may be
+ specified as in color.branch.<slot>.
color.grep::
When set to `always`, always highlight matches. When `false` (or
diff --git a/combine-diff.c b/combine-diff.c
index 5b63af1..6162691 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -524,6 +524,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
int i;
unsigned long lno = 0;
const char *c_frag = diff_get_color(use_color, DIFF_FRAGINFO);
+ const char *c_func = diff_get_color(use_color, DIFF_FUNCINFO);
const char *c_new = diff_get_color(use_color, DIFF_FILE_NEW);
const char *c_old = diff_get_color(use_color, DIFF_FILE_OLD);
const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
@@ -588,7 +589,9 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
comment_end = i;
}
if (comment_end)
- putchar(' ');
+ printf("%s%s %s%s", c_reset,
+ c_plain, c_reset,
+ c_func);
for (i = 0; i < comment_end; i++)
putchar(hunk_comment[i]);
}
diff --git a/diff.c b/diff.c
index 0d7f5ea..e210525 100644
--- a/diff.c
+++ b/diff.c
@@ -39,6 +39,7 @@ static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_GREEN, /* NEW */
GIT_COLOR_YELLOW, /* COMMIT */
GIT_COLOR_BG_RED, /* WHITESPACE */
+ GIT_COLOR_MAGENTA, /* FUNCINFO */
};
static void diff_filespec_load_driver(struct diff_filespec *one);
@@ -60,6 +61,8 @@ static int parse_diff_color_slot(const char *var, int ofs)
return DIFF_COMMIT;
if (!strcasecmp(var+ofs, "whitespace"))
return DIFF_WHITESPACE;
+ if (!strcasecmp(var+ofs, "func"))
+ return DIFF_FUNCINFO;
die("bad config variable '%s'", var);
}
@@ -344,6 +347,63 @@ static void emit_add_line(const char *reset,
}
}
+static void emit_hunk_line(struct emit_callback *ecbdata,
+ const char *line, int len)
+{
+ const char *plain = diff_get_color(ecbdata->color_diff, DIFF_PLAIN);
+ const char *frag = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);
+ const char *func = diff_get_color(ecbdata->color_diff, DIFF_FUNCINFO);
+ const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
+ const char *orig_line = line;
+ int orig_len = len;
+ const char *frag_start;
+ int frag_len;
+ const char *part_end = NULL;
+ int part_len = 0;
+
+ /* determine length of @ */
+ while (part_len < len && line[part_len] == '@')
+ part_len++;
+
+ /* find end of frag, (Ie. find second @@) */
+ part_end = memmem(line + part_len, len - part_len,
+ line, part_len);
+ if (!part_end)
+ return emit_line(ecbdata->file, frag, reset, line, len);
+ /* calculate total length of frag */
+ part_len = (part_end + part_len) - line;
+
+ /* remember frag part, we emit only if we find a space separator */
+ frag_start = line;
+ frag_len = part_len;
+
+ /* consume hunk header */
+ len -= part_len;
+ line += part_len;
+
+ /*
+ * for empty reminder or empty space sequence (exclusive any newlines
+ * or carriage returns) emit complete original line as FRAGINFO
+ */
+ if (!len || !(part_len = strspn(line, " \t")))
+ return emit_line(ecbdata->file, frag, reset,
+ orig_line, orig_len);
+
+ /* now emit the hunk header as FRAGINFO */
+ emit_line(ecbdata->file, frag, reset, frag_start, frag_len);
+
+ /* print whitespace sep as PLAIN */
+ emit_line(ecbdata->file, plain, reset, line, part_len);
+
+ /* consume whitespace sep */
+ len -= part_len;
+ line += part_len;
+
+ /* print reminder as FUNCINFO */
+ if (len)
+ emit_line(ecbdata->file, func, reset, line, len);
+}
+
static struct diff_tempfile *claim_diff_tempfile(void) {
int i;
for (i = 0; i < ARRAY_SIZE(diff_temp); i++)
@@ -781,9 +841,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
diff_words_flush(ecbdata);
len = sane_truncate_line(ecbdata, line, len);
find_lno(line, ecbdata);
- emit_line(ecbdata->file,
- diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
- reset, line, len);
+ emit_hunk_line(ecbdata, line, len);
if (line[len-1] != '\n')
putc('\n', ecbdata->file);
return;
diff --git a/diff.h b/diff.h
index 2740421..15fcecd 100644
--- a/diff.h
+++ b/diff.h
@@ -130,6 +130,7 @@ enum color_diff {
DIFF_FILE_NEW = 5,
DIFF_COMMIT = 6,
DIFF_WHITESPACE = 7,
+ DIFF_FUNCINFO = 8,
};
const char *diff_get_color(int diff_use_color, enum color_diff ix);
#define diff_get_color_opt(o, ix) \
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 21db6e9..64a7c38 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -16,6 +16,7 @@ decrypt_color () {
-e 's/.\[1m/<WHITE>/g' \
-e 's/.\[31m/<RED>/g' \
-e 's/.\[32m/<GREEN>/g' \
+ -e 's/.\[35m/<MAGENTA>/g' \
-e 's/.\[36m/<BROWN>/g' \
-e 's/.\[m/<RESET>/g'
}
@@ -70,7 +71,7 @@ cat > expect <<\EOF
<WHITE>+++ b/post<RESET>
<BROWN>@@ -1 +1 @@<RESET>
<RED>h(4)<RESET><GREEN>h(4),hh[44]<RESET>
-<BROWN>@@ -3,0 +4,4 @@ a = b + c<RESET>
+<BROWN>@@ -3,0 +4,4 @@<RESET> <RESET><MAGENTA>a = b + c<RESET>
<GREEN>aa = a<RESET>
--
tg: (785c58e..) bw/func-color (depends on: master)
^ permalink raw reply related
* Re: [PATCH] Give the hunk comment its own color
From: Bert Wesarg @ 2009-11-18 15:16 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20091118142320.GA1220@coredump.intra.peff.net>
On Wed, Nov 18, 2009 at 15:23, Jeff King <peff@peff.net> wrote:
> PS I almost complained about your default of "magenta" as the same as
> the meta color before I remembered that magenta meta is a personal
> setting I use. Personally I find the bold meta color to be distractingly
> ugly. Blaming it, the default seems to come from Linus, who even in his
> commit message (50f575f) seems to indicate that it is somewhat arbitrary
> (mostly just dropping the purple from the bold purple).
I took magenta, because it is not used as any other default color
value. I think choosing a color other than cyan would bring the
attention to this new feature.
Bert
^ permalink raw reply
* Re: [PATCH v2] Give the hunk comment its own color
From: Jason Sewall @ 2009-11-18 15:17 UTC (permalink / raw)
To: Bert Wesarg; +Cc: Junio C Hamano, git
In-Reply-To: <1258557087-31540-1-git-send-email-bert.wesarg@googlemail.com>
On Wed, Nov 18, 2009 at 10:11 AM, Bert Wesarg
<bert.wesarg@googlemail.com> wrote:
> Now with updated test suit.
Spelling this as 'suite' would be sweet.
^ permalink raw reply
* Re: [PATCH v2] Give the hunk comment its own color
From: Bert Wesarg @ 2009-11-18 15:20 UTC (permalink / raw)
To: Jason Sewall; +Cc: Junio C Hamano, git
In-Reply-To: <31e9dd080911180717i27c6ef3fp736b7f8d41e4c8be@mail.gmail.com>
On Wed, Nov 18, 2009 at 16:17, Jason Sewall <jasonsewall@gmail.com> wrote:
> On Wed, Nov 18, 2009 at 10:11 AM, Bert Wesarg
> <bert.wesarg@googlemail.com> wrote:
>
>> Now with updated test suit.
>
> Spelling this as 'suite' would be sweet.
Thanks. I re-submit only if you find a bug too.
Bert
>
^ permalink raw reply
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 15:51 UTC (permalink / raw)
To: Philip Hofstetter; +Cc: git
In-Reply-To: <aa2993680911180620g151d8a07t11144d150cd6e29e@mail.gmail.com>
On Wed, Nov 18, 2009 at 03:20:48PM +0100, Philip Hofstetter wrote:
> Some investigating revealed an interesting quirk in git-mailinfo which
> seems to be a bit too eager to extract author information: Instead of
> just looking at the From:-Line in a mails header (git-rebase seems to
> use git-am which in turn uses git-mailinfo), it searches for "from:"
> *anywhere* in the mail and uses the last found information as the
> source for the author information.
It is not quite "anywhere"; extra headers are respected at the very top
of the message body. This is intentional, to allow one to indicate that
a patch you are sending was authored by somebody else.
So the problem is slightly less severe; the body of your commit message
has to _start_ with "From:". Still, it is awfully ugly to hit a parsing
ambiguity like this when you are trying to do something as simple as
rebase.
Some solutions I can think of are:
1. Improve the header-finding heuristic to actually look for something
more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
head which other headers we handle in this position. Probably
Date, too).
2. Give mailinfo a "--strict" mode to indicate that it is directly
parsing the output of format-patch, and not some random email. Use
--strict when invoking "git am" via "git rebase".
> While I know it's rude to have a line beginning with "from:" (and it's
> even ruder to have a line beginning with "from "), IMHO the header
> ends at the first blank line and I see no reason to extract author
> information past the header.
As I explained above, there is a reason, but I don't think it's rude to
have either of those lines. You were, after all, writing a commit
message, not an email (and even if you were, it is a failure of the
storage format if it can't represent your data correctly). So I think
git is to blame here.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Nov 2009, #04; Tue, 17)
From: Johannes Sixt @ 2009-11-18 16:12 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Junio C Hamano, git
In-Reply-To: <fcaeb9bf0911180643w5e659340jd845aa202e6feca3@mail.gmail.com>
Nguyen Thai Ngoc Duy schrieb:
> On 11/18/09, Junio C Hamano <gitster@pobox.com> wrote:
>> * nd/sparse (2009-08-20) 19 commits.
>>
>> The latest update I didn't look at very closely but I had an impression
>> that it was touching very generic codepath that would affect non sparse
>> cases, iow the patch looked very scary (the entire series already is).
>
> I wonder if there is any other approach for sparse checkout? I'll see
> if I can improve it, but with a series touching unpack logic, diff
> core, .gitattributes/.gitignore, it's hard to get it right and
> obvious.
Just FYI: I run some of my installations with this series, but without
using sparse checkout to see if there are regressions. Nothing came up so
far.
-- Hannes
^ permalink raw reply
* [PATCH] unset GREP_OPTIONS in test-lib.sh
From: Bert Wesarg @ 2009-11-18 16:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Bert Wesarg
I used to set GREP_OPTIONS to exclude *.orig and *.rej files. But with this
the test t4252-am-options.sh fails because it calls grep with a .rej file:
grep "@@ -1,3 +1,3 @@" file-2.rej
Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
t/test-lib.sh | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f2ca536..6ac8dc6 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -65,6 +65,8 @@ GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
# CDPATH into the environment
unset CDPATH
+unset GREP_OPTIONS
+
case $(echo $GIT_TRACE |tr "[A-Z]" "[a-z]") in
1|2|true)
echo "* warning: Some tests will not work if GIT_TRACE" \
--
tg: (785c58e..) bw/unset-GREP_OPTIONS (depends on: master)
^ permalink raw reply related
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 16:42 UTC (permalink / raw)
To: Philip Hofstetter; +Cc: git
In-Reply-To: <20091118155154.GA15184@coredump.intra.peff.net>
On Wed, Nov 18, 2009 at 10:51:54AM -0500, Jeff King wrote:
> So the problem is slightly less severe; the body of your commit message
> has to _start_ with "From:". Still, it is awfully ugly to hit a parsing
> ambiguity like this when you are trying to do something as simple as
> rebase.
>
> Some solutions I can think of are:
>
> 1. Improve the header-finding heuristic to actually look for something
> more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
> head which other headers we handle in this position. Probably
> Date, too).
>
> 2. Give mailinfo a "--strict" mode to indicate that it is directly
> parsing the output of format-patch, and not some random email. Use
> --strict when invoking "git am" via "git rebase".
Solution (2) seemed like a lot of work, so here is the relatively small
solution (1). I think looking for <.*@.*> is too restrictive, as people
may be using:
From: bare@example.com
which should remain valid. I just look for an '@' instead.
Note that this validation also applies to actual headers. Should we turn
it off for them? As it is, it breaks t3400, which uses a bogus email
address. I suppose we should probably preserve such bogosities if they
are in the official headers.
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index c90cd31..6d69ef3 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -275,6 +275,17 @@ static inline int cmp_header(const struct strbuf *line, const char *hdr)
line->buf[len] == ':' && isspace(line->buf[len + 1]);
}
+static int validate_header(const char *header, const struct strbuf *data)
+{
+ if (!strcmp(header, "From"))
+ return !!strchr(data->buf, '@');
+ if (!strcmp(header, "Date")) {
+ char buf[50];
+ return parse_date(data->buf, buf, sizeof(buf)) >= 0;
+ }
+ return 1;
+}
+
static int check_header(const struct strbuf *line,
struct strbuf *hdr_data[], int overwrite)
{
@@ -289,8 +300,10 @@ static int check_header(const struct strbuf *line,
*/
strbuf_add(&sb, line->buf + len + 2, line->len - len - 2);
decode_header(&sb);
- handle_header(&hdr_data[i], &sb);
- ret = 1;
+ if (validate_header(header[i], &sb)) {
+ ret = 1;
+ handle_header(&hdr_data[i], &sb);
+ }
goto check_header_out;
}
}
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index 0279d07..be06e0f 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
last=`cat last` &&
echo total is $last &&
- test `cat last` = 14'
+ test `cat last` = 16'
check_mailinfo () {
mail=$1 opt=$2
diff --git a/t/t5100/info0015 b/t/t5100/info0015
new file mode 100644
index 0000000..c4d8d77
--- /dev/null
+++ b/t/t5100/info0015
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/info0016 b/t/t5100/info0016
new file mode 100644
index 0000000..f4857d4
--- /dev/null
+++ b/t/t5100/info0016
@@ -0,0 +1,5 @@
+Author: A U Thor
+Email: a.u.thor@example.com
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
diff --git a/t/t5100/msg0015 b/t/t5100/msg0015
new file mode 100644
index 0000000..be5115b
--- /dev/null
+++ b/t/t5100/msg0015
@@ -0,0 +1,3 @@
+From: bogosity
+ - a list
+ - of stuff
diff --git a/t/t5100/msg0016 b/t/t5100/msg0016
new file mode 100644
index 0000000..1063f51
--- /dev/null
+++ b/t/t5100/msg0016
@@ -0,0 +1,4 @@
+Date: bogus
+
+and some content
+
diff --git a/t/t5100/patch0015 b/t/t5100/patch0015
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0015
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/patch0016 b/t/t5100/patch0016
new file mode 100644
index 0000000..ad64848
--- /dev/null
+++ b/t/t5100/patch0016
@@ -0,0 +1,8 @@
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 13fa4ae..de10312 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -650,3 +650,36 @@ index b0b5d8f..461c47e 100644
convert_to_utf8(line, charset.buf);
--
1.6.4.1
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (from)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+From: bogosity
+ - a list
+ - of stuff
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
+From nobody Mon Sep 17 00:00:00 2001
+From: A U Thor <a.u.thor@example.com>
+Subject: check bogus body header (date)
+Date: Fri, 9 Jun 2006 00:44:16 -0700
+
+Date: bogus
+
+and some content
+
+---
+diff --git a/foo b/foo
+index e69de29..d95f3ad 100644
+--- a/foo
++++ b/foo
+@@ -0,0 +1 @@
++content
+
^ permalink raw reply related
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Philip Hofstetter @ 2009-11-18 17:11 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091118155154.GA15184@coredump.intra.peff.net>
Hello,
On Wed, Nov 18, 2009 at 4:51 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Nov 18, 2009 at 03:20:48PM +0100, Philip Hofstetter wrote:
> 1. Improve the header-finding heuristic to actually look for something
> more sane, like "From:.*<.*@.*>" (I don't recall off the top of my
> head which other headers we handle in this position. Probably
> Date, too).
or at least don't prefer obviously invalid data over valid data that
has already been seen.
> 2. Give mailinfo a "--strict" mode to indicate that it is directly
> parsing the output of format-patch, and not some random email. Use
> --strict when invoking "git am" via "git rebase".
That would solve the problem too, though it feels like adding yet
another switch to guard against one specific issue. The purpose behind
options like this tends to get forgotten over time.
> As I explained above, there is a reason, but I don't think it's rude to
> have either of those lines. You were, after all, writing a commit
> message, not an email (and even if you were, it is a failure of the
> storage format if it can't represent your data correctly). So I think
> git is to blame here.
IMHO, another workable solution would be to reject a commit that later
can't be handled. That way the current attempts at getting an email
address can remain intact and the (much more) unlikely case that
somebody begins the commit message with from: will be caught before
damage is done.
So, just check that from-line for a valid email address at commit
time. If it is, ok. If not, treat it as an error and inform the user
that an invalid email address was given in the commit message.
Also, the error message by rebase (which is actually the message
printed by am) could have been a bit more helpful. If am fails during
a rebase, rebase could explicitly tell which commit am failed at. The
output I got made me suspect the problem to be in the first commit (as
that was the last one printed) when in fact it was in the second one
(which was not printed).
But that's just nit-picking.
Philip
^ permalink raw reply
* Re: git-mailinfo doesn't stop parsing at the end of the header
From: Jeff King @ 2009-11-18 17:24 UTC (permalink / raw)
To: Philip Hofstetter; +Cc: git
In-Reply-To: <aa2993680911180911o7e3af804m4ebdc20096baa609@mail.gmail.com>
On Wed, Nov 18, 2009 at 06:11:36PM +0100, Philip Hofstetter wrote:
> > As I explained above, there is a reason, but I don't think it's rude to
> > have either of those lines. You were, after all, writing a commit
> > message, not an email (and even if you were, it is a failure of the
> > storage format if it can't represent your data correctly). So I think
> > git is to blame here.
>
> IMHO, another workable solution would be to reject a commit that later
> can't be handled. That way the current attempts at getting an email
> address can remain intact and the (much more) unlikely case that
> somebody begins the commit message with from: will be caught before
> damage is done.
I'm not sure I like that solution for a few reasons:
1. It creates a bad user experience. You are not unreasonable for
wanting to put some specific text in your commit message. Having
git come back and say "oops, I might get confused by this later"
just seems like an annoyance to the user.
2. Mailinfo has to deal with data created by older versions of git. So
in your case, the rebase was a bomb waiting to go off. If we can
fix it so that an existing bomb doesn't go off, rather than not
creating the bomb in the first place, then we are better off.
3. Commit has to know about rules for mailinfo, even versions of
mailinfo that will exist in the future. Probably the rules aren't
going to change much, but it is a weakness.
4. Commit messages can come from other places than "git commit". What
should we do with a commit message like this that is imported from
SVN? Reject the import? Munge the message?
Of course all of that presupposes that we can correctly handle the
existing data after the fact. Even with my patch, you still can't write
"From: foo@example.com" as the first line of your commit body. But that
is, IMHO, getting even more unlikely than your "From:" (which already
seems fairly unlikely).
I also think "git commit" would not be the right time for such a
feature. The problem is not that you have this text in your commit
message. The problem is that the "format-patch | am" transport is lossy.
You would do better to have format-patch say "Ah, this is going to
create a bogus email address" and somehow quote it appropriately.
-Peff
^ 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