* Re: [PATCH] bash: complete more options for 'git rebase'
From: Shawn O. Pearce @ 2009-10-28 0:32 UTC (permalink / raw)
To: Bj??rn Gustavsson; +Cc: git
In-Reply-To: <4AD98F72.1060901@gmail.com>
Bj??rn Gustavsson <bgustavsson@gmail.com> wrote:
> Complete all long options for 'git rebase' except --no-verify
> (probably used very seldom) and the long options corresponding
> to -v, -q, and -f.
>
> Signed-off-by: Bj??rn Gustavsson <bgustavsson@gmail.com>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
> I am primarily interested in having the --whitespace= option completed,
> but while at it I have added completion for the other potentially useful
> long options.
Indeed, this would be nice. :-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH 3/3] add smart HTTP tests
From: Shawn O. Pearce @ 2009-10-28 0:17 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Mark Lodato, git
In-Reply-To: <1256472380-924-4-git-send-email-drizzd@aon.at>
Clemens Buchacher <drizzd@aon.at> wrote:
> Set LIB_HTTPD_GIT to enable smart HTTP tests.
My concern here is we have to run the test suite twice in order to
test HTTP support. We should only run it once, with GIT_TEST_HTTPD=1
set and have it all run at once.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 2/3] remote-helpers: return successfully if everything up-to-date
From: Shawn O. Pearce @ 2009-10-28 0:11 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Mark Lodato, git
In-Reply-To: <1256472380-924-3-git-send-email-drizzd@aon.at>
Clemens Buchacher <drizzd@aon.at> wrote:
>
> Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
> t/t5540-http-push.sh | 2 +-
> transport-helper.c | 2 ++
> 2 files changed, 3 insertions(+), 1 deletions(-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH 1/3] update http tests according to remote-curl capabilities
From: Shawn O. Pearce @ 2009-10-28 0:10 UTC (permalink / raw)
To: Clemens Buchacher; +Cc: Mark Lodato, git
In-Reply-To: <1256472380-924-2-git-send-email-drizzd@aon.at>
Clemens Buchacher <drizzd@aon.at> wrote:
> o Pushing packed refs is now fixed.
>
> o The transport helper fails if refs are already up-to-date. Add a
> test for that.
>
> o The transport helper will notice if refs are already up-to-date. We
> therefore need to update server info in the unpacked-refs test.
>
> o The transport helper will purge deleted branches automatically.
>
> Signed-off-by: Clemens Buchacher <drizzd@aon.at>
Acked-by: Shawn O. Pearce <spearce@spearce.org>
> t/t5540-http-push.sh | 14 +++++++++-----
> 1 files changed, 9 insertions(+), 5 deletions(-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH] commit: More generous accepting of RFC-2822 footer lines.
From: Shawn O. Pearce @ 2009-10-28 0:05 UTC (permalink / raw)
To: David Brown; +Cc: git
In-Reply-To: <20091027234520.GA11433@quaoar.codeaurora.org>
David Brown <davidb@codeaurora.org> wrote:
> From: David Brown <davidb@quicinc.com>
>
> 'git commit -s' will insert a blank line before the Signed-off-by
> line at the end of the message, unless this last line is a
> Signed-off-by line itself. Common use has other trailing lines
> at the ends of commit text, in the style of RFC2822 headers.
>
> Be more generous in considering lines to be part of this footer.
> This may occasionally leave out the blank line for cases where
> the commit text happens to start with a word ending in a colon,
> but this results in less fixups than the extra blank lines with
> Acked-by, or other custom footers.
The nasty perl I use in Gerrit's commit-msg hook is a bit more
expressive. Basically the rule is we insert a blank line before
the new footer unless all lines in the last paragraph (so all text
after the last "\n\n" sequence) match the regex "^[a-zA-Z0-9-]+:".
> +test_expect_success 'signoff gap' '
> +
> + echo 3 >positive &&
> + git add positive &&
> + alt="Alt-RFC-822-Header: Value" &&
> + git commit -s -m "welcome
> +
> +$alt" &&
I wonder if we shouldn't also have a test case for the message:
msg="test
this is a test that
fixes: 42.
"
as the result would be expected to be:
exp="test
this is a test that
fixes: 42.
Signed-off-by A. U. Thor <...>
"
But:
msg="test
this is a test
fixes: 42
"
would produce:
exp="test
this is a test
fixes: 42
Signed-off-by A. U. Thor <...>
"
--
Shawn.
^ permalink raw reply
* Re: [PATCH] push: support remote branches in guess_ref DWIM
From: Jeff King @ 2009-10-28 0:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vaazc31sj.fsf@alter.siamese.dyndns.org>
On Tue, Oct 27, 2009 at 03:33:16PM -0700, Junio C Hamano wrote:
> > $ git fetch ;# presumably gets origin/branch
> > $ git push origin/branch:renamed-branch
> >
> > which is much nicer than exposing clueless users to
> > ":refs/heads/renamed-branch".
>
> You would need to expose ":refs/heads/branch" to make this a rename, not a
> copy, wouldn't you?
Yeah, you're right. This was based on an actual user request, and I
didn't think too closely about the other steps. But since the deletion
is of an existing branch, you should be able to do that without
refs/heads. So:
$ git push origin origin/branch:renamed-branch
$ git push origin :branch
Which of course you could do in one command if you wanted to live
(more) dangerously.
> > Am I missing some part of your argument?
>
> I do not see much point (other than your "rename" example) in pushing what
> you got from the other end without changing anything yourself back to the
> same remote.
I don't either; my hope was that we can make that case a little bit
easier without creating undue hardship for anybody else.
> There was a thread in which people argued that the primary thing that is
> dangerous in this sequence
>
> $ git checkout origin/next; work-commit; work-commit; ...
>
> is when you leave the detached HEAD state without saving it to a local
> branch. I think what is more dangerous is that it will not give the user
> a solid understanding that these commits do _not_ change origin/next in
> any way. After doing the above, it is understandable that a novice would
> mistakenly think: "I started from origin/next and built some, let's push
> the result".
I suppose it's possible. I don't have any evidence that users actually
think that way.
> With such a misconception, you will see the likely mistake here, too:
>
> $ git push origin origin/next:refs/heads/next
>
> If "rename" is the _only_ valid reason you might want to push what you got
> from there back to the same remote, _and_ if "rename" is something very
> often needed, I think we would prefer to have a way to support that
> operation directly, instead of more general DWIM that would risk passing
> mistakes like the above unwarned.
OK, I can buy that. It would be much nicer even to support explicit
renaming (in fact, the user request started with that, and I just didn't
want to give them an answer that involved refs/heads/, which I think is
unnecessarily scary to users).
> IOW, it's between "prevent push with dubious $src from happening in the
> first place" vs "give them rope". Historically I always sided with the
> latter camp, but I am trying to play a devil's advocate for a change ;-).
Heh. Thanks for explaining your thinking.
Let's scrap this for now, then. Remote rename doesn't actually come up
very often, and I agree that it would be nice to have an actual atomic
movement, which is what people really want.
-Peff
^ permalink raw reply
* [PATCH] commit: More generous accepting of RFC-2822 footer lines.
From: David Brown @ 2009-10-27 23:45 UTC (permalink / raw)
To: git
From: David Brown <davidb@quicinc.com>
'git commit -s' will insert a blank line before the Signed-off-by
line at the end of the message, unless this last line is a
Signed-off-by line itself. Common use has other trailing lines
at the ends of commit text, in the style of RFC2822 headers.
Be more generous in considering lines to be part of this footer.
This may occasionally leave out the blank line for cases where
the commit text happens to start with a word ending in a colon,
but this results in less fixups than the extra blank lines with
Acked-by, or other custom footers.
Signed-off-by: David Brown <davidb@quicinc.com>
---
builtin-commit.c | 17 ++++++++++++++++-
t/t7501-commit.sh | 19 +++++++++++++++++++
2 files changed, 35 insertions(+), 1 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index 200ffda..f081e80 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -414,6 +414,21 @@ static void determine_author_info(void)
author_date = date;
}
+static int is_rfc2822_footer(const char *line)
+{
+ int ch;
+
+ while ((ch = *line++)) {
+ if (ch == ':')
+ return 1;
+ if ((33 <= ch && ch <= 57) ||
+ (59 <= ch && ch <= 126))
+ continue;
+ break;
+ }
+ return 0;
+}
+
static int prepare_to_commit(const char *index_file, const char *prefix,
struct wt_status *s)
{
@@ -489,7 +504,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
for (i = sb.len - 1; i > 0 && sb.buf[i - 1] != '\n'; i--)
; /* do nothing */
if (prefixcmp(sb.buf + i, sob.buf)) {
- if (prefixcmp(sb.buf + i, sign_off_header))
+ if (!is_rfc2822_footer(sb.buf + i))
strbuf_addch(&sb, '\n');
strbuf_addbuf(&sb, &sob);
}
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index e2ef532..05542b4 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -247,6 +247,25 @@ $existing" &&
'
+test_expect_success 'signoff gap' '
+
+ echo 3 >positive &&
+ git add positive &&
+ alt="Alt-RFC-822-Header: Value" &&
+ git commit -s -m "welcome
+
+$alt" &&
+ git cat-file commit HEAD | sed -e "1,/^\$/d" > actual &&
+ (
+ echo welcome
+ echo
+ echo $alt
+ git var GIT_COMMITTER_IDENT |
+ sed -e "s/>.*/>/" -e "s/^/Signed-off-by: /"
+ ) >expected &&
+ test_cmp expected actual
+'
+
test_expect_success 'multiple -m' '
>negative &&
--
1.6.5.1
^ permalink raw reply related
* Re: git update --prune issue
From: Björn Steinbrink @ 2009-10-27 23:30 UTC (permalink / raw)
To: Jeff King; +Cc: Clemens Buchacher, Jeffrey Middleton, Michael J Gruber, git
In-Reply-To: <20091027184627.GA19292@sigill.intra.peff.net>
On 2009.10.27 14:46:27 -0400, Jeff King wrote:
> On Tue, Oct 27, 2009 at 12:50:55PM -0400, Jeff King wrote:
>
> > > Actually, it's beta_gc_dev_old and beta_gc_dev, not the same refs.
> >
> > OK, I'm a bit blind. Thanks for pointing it out. I'll still see if I can
> > replicate it.
>
> I'm not having any luck reproducing here with a simple case. :( I'm
> trying:
You need more parent repos, and you need packs. What I guess what
happens is:
The first remote gets fetched from.
The first remote gets pruned. This initializes the pack stuff.
The second remote gets fetched from, and creates a _new_ _pack_.
The second remote gets pruned (*)
The third remote gets fetched from.
The third remote gets pruned.
Now, this prune step calls for_each_ref, which looks through the new
refs for the second remote. This fails, because the new pack was not
present when the pack "system" got initialzed.
(*) I have no idea why this step doesn't fail... But hey, I have no idea
about the whole pack handling either. I just recall that there's some
initialization step...
This reproduces the issue here:
#!/bin/sh
rm -rf parent* child
commit() {
echo $1 >file && git add file && git commit -m $1
}
mkdir parent0 && (
cd parent0 &&
git init &&
commit one
) &&
mkdir parent1 && (
cd parent1 &&
git init &&
commit two &&
commit three &&
git checkout -b other
) &&
mkdir parent2 && (
cd parent2 &&
git init &&
commit four
) &&
mkdir child && (
cd child &&
git init &&
git config fetch.unpackLimit 1
git remote add parent0 file://$PWD/../parent0 &&
git remote add parent1 file://$PWD/../parent1 &&
git remote add parent2 file://$PWD/../parent2 &&
git remote update
) && (
cd parent1 &&
git checkout master &&
git branch -d other &&
git reset --hard HEAD^ &&
commit five
) && (
cd child &&
git remote update --prune
)
^ permalink raw reply
* Re: git svn branch tracking + ignore paths
From: Avery Pennarun @ 2009-10-27 23:16 UTC (permalink / raw)
To: Lachlan Deck; +Cc: git list
In-Reply-To: <41F0F1D6-4F99-4828-9259-1B2BDC689747@gmail.com>
On Tue, Oct 27, 2009 at 7:00 PM, Lachlan Deck <lachlan.deck@gmail.com> wrote:
> I'm wondering if it's possible to create a branch (starting as local but
> later on pushed to svn) that essentially stays in sync with the main branch
> (svn trunk) but both (a) ignores pulling in certain paths from trunk and (b)
> provides a few extra paths of its own (some of which overlap with those
> ignored from trunk) and (c) only pushes to trunk paths that are relevant for
> trunk (i.e., not specifically ignored)?
>
> If someone's able to share how they'd go about setting this up that'd be
> great.
This sounds like a generally scary idea. Perhaps if you described
your problem at a higher level (what are you really trying to
achieve?) there would be a less scary way to do it.
That said, if you *really* need this... one option that comes to mind is:
1. extract the history from svn into git using 'git svn' as usual.
2. extract the subtree of svn that you're interested in (if you're
lucky enough that you only need one subtree) by using 'git subtree
split'. This creates a new branch or new git repo, depending how you
do it.
3. Create a third project that will host your new work with extra
subtrees that you don't want. Use 'git subtree add' and 'git subtree
merge' to keep this up to date with the stuff you extracted in step 2.
(Repeat steps 1-3 as necessary to keep your project up to date with
the svn project.)
4. When you want to merge your own changes back into svn, first
extract them from your own project (step 3) using 'git subtree split'.
Then merge those changes into the main project (step 1) using 'git
subtree merge'. Then use git-svn to upload them to the main svn repo.
You can save yourself some steps if you import the *entire* svn
project into your own project, rather than trying to trim it on
import. That way you only have to split when going from #3 to #1, not
in the other direction, and you don't need repository #2.
If all this sounds crazy, it probably is. Maybe see if you can come
up with a way to avoid trying to do this altogether.
Good luck... :)
Avery
[1] http://github.com/apenwarr/git-subtree
^ permalink raw reply
* Re: [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option
From: Charles Bailey @ 2009-10-27 23:00 UTC (permalink / raw)
To: Scott Chacon; +Cc: git list
In-Reply-To: <d411cc4a0910271536u5817802at43f7477dd8ccabc7@mail.gmail.com>
On Tue, Oct 27, 2009 at 03:36:49PM -0700, Scott Chacon wrote:
> p4merge is now a built-in diff/merge tool.
> This adds p4merge to git-completion and updates
> the documentation to mention p4merge.
> ---
I approve (but haven't had a chance to test this). p4merge is a
good mergetool, but now I'll have to find something else as an example
that you need to use custom mergetool support for.
I'm just wondering, does this work well with unixes and Mac OS X? I
think it's recommended install practice to symlink p4v as p4merge on
*nix, but Mac OS X needs some sort of 'launchp4merge' to be called
IIRC, or is this something that users can just configure with
mergetool.p4diff.path?
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* git svn branch tracking + ignore paths
From: Lachlan Deck @ 2009-10-27 23:00 UTC (permalink / raw)
To: git list
Hi there,
(Note: relative newby)
I'm wondering if it's possible to create a branch (starting as local
but later on pushed to svn) that essentially stays in sync with the
main branch (svn trunk) but both (a) ignores pulling in certain paths
from trunk and (b) provides a few extra paths of its own (some of
which overlap with those ignored from trunk) and (c) only pushes to
trunk paths that are relevant for trunk (i.e., not specifically
ignored)?
If someone's able to share how they'd go about setting this up that'd
be great.
Thanks!
with regards,
--
Lachlan Deck
^ permalink raw reply
* [PATCH] mergetool--lib: add p4merge as a pre-configured mergetool option
From: Scott Chacon @ 2009-10-27 22:36 UTC (permalink / raw)
To: git list
p4merge is now a built-in diff/merge tool.
This adds p4merge to git-completion and updates
the documentation to mention p4merge.
---
Documentation/git-difftool.txt | 2 +-
Documentation/git-mergetool.txt | 2 +-
Documentation/merge-config.txt | 2 +-
contrib/completion/git-completion.bash | 2 +-
git-mergetool--lib.sh | 17 +++++++++++++++--
5 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/Documentation/git-difftool.txt b/Documentation/git-difftool.txt
index 96a6c51..8e9aed6 100644
--- a/Documentation/git-difftool.txt
+++ b/Documentation/git-difftool.txt
@@ -31,7 +31,7 @@ OPTIONS
Use the diff tool specified by <tool>.
Valid merge tools are:
kdiff3, kompare, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff,
- ecmerge, diffuse, opendiff and araxis.
+ ecmerge, diffuse, opendiff, p4merge and araxis.
+
If a diff tool is not specified, 'git-difftool'
will use the configuration variable `diff.tool`. If the
diff --git a/Documentation/git-mergetool.txt b/Documentation/git-mergetool.txt
index 68ed6c0..4a6f7f3 100644
--- a/Documentation/git-mergetool.txt
+++ b/Documentation/git-mergetool.txt
@@ -27,7 +27,7 @@ OPTIONS
Use the merge resolution program specified by <tool>.
Valid merge tools are:
kdiff3, tkdiff, meld, xxdiff, emerge, vimdiff, gvimdiff, ecmerge,
- diffuse, tortoisemerge, opendiff and araxis.
+ diffuse, tortoisemerge, opendiff, p4merge and araxis.
+
If a merge resolution program is not specified, 'git-mergetool'
will use the configuration variable `merge.tool`. If the
diff --git a/Documentation/merge-config.txt b/Documentation/merge-config.txt
index c0f96e7..a403155 100644
--- a/Documentation/merge-config.txt
+++ b/Documentation/merge-config.txt
@@ -23,7 +23,7 @@ merge.tool::
Controls which merge resolution program is used by
linkgit:git-mergetool[1]. Valid built-in values are: "kdiff3",
"tkdiff", "meld", "xxdiff", "emerge", "vimdiff", "gvimdiff",
- "diffuse", "ecmerge", "tortoisemerge", "araxis", and
+ "diffuse", "ecmerge", "tortoisemerge", "p4merge", "araxis" and
"opendiff". Any other value is treated is custom merge tool
and there must be a corresponding mergetool.<tool>.cmd option.
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index d3fec32..5fb6017 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -953,7 +953,7 @@ _git_diff ()
}
__git_mergetools_common="diffuse ecmerge emerge kdiff3 meld opendiff
- tkdiff vimdiff gvimdiff xxdiff araxis
+ tkdiff vimdiff gvimdiff xxdiff araxis p4merge
"
_git_difftool ()
diff --git a/git-mergetool--lib.sh b/git-mergetool--lib.sh
index bfb01f7..f7c571e 100644
--- a/git-mergetool--lib.sh
+++ b/git-mergetool--lib.sh
@@ -46,7 +46,7 @@ check_unchanged () {
valid_tool () {
case "$1" in
kdiff3 | tkdiff | xxdiff | meld | opendiff | \
- emerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis)
+ emerge | vimdiff | gvimdiff | ecmerge | diffuse | araxis | p4merge)
;; # happy
tortoisemerge)
if ! merge_mode; then
@@ -130,6 +130,19 @@ run_merge_tool () {
"$merge_tool_path" "$LOCAL" "$REMOTE"
fi
;;
+ p4merge)
+ if merge_mode; then
+ touch "$BACKUP"
+ if $base_present; then
+ "$merge_tool_path" "$BASE" "$LOCAL" "$REMOTE" "$MERGED"
+ else
+ "$merge_tool_path" "$LOCAL" "$LOCAL" "$REMOTE" "$MERGED"
+ fi
+ check_unchanged
+ else
+ "$merge_tool_path" "$LOCAL" "$REMOTE"
+ fi
+ ;;
meld)
if merge_mode; then
touch "$BACKUP"
@@ -323,7 +336,7 @@ guess_merge_tool () {
else
tools="opendiff kdiff3 tkdiff xxdiff meld $tools"
fi
- tools="$tools gvimdiff diffuse ecmerge araxis"
+ tools="$tools gvimdiff diffuse ecmerge p4merge araxis"
fi
if echo "${VISUAL:-$EDITOR}" | grep emacs > /dev/null 2>&1; then
# $EDITOR is emacs so add emerge as a candidate
--
1.6.5.2.75.g16dea.dirty
^ permalink raw reply related
* Re: [PATCH] push: support remote branches in guess_ref DWIM
From: Junio C Hamano @ 2009-10-27 22:33 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20091027014525.GA29583@sigio.peff.net>
Jeff King <peff@peff.net> writes:
> On Mon, Oct 26, 2009 at 04:31:57PM -0700, Junio C Hamano wrote:
>
>> As _our_ origin can never be _their_ origin if we are clone of them, I do
>> not think anybody sane would expect it to push into refs/remotes/origin/
>> to begin with.
>
> OK, I agree.
>
>> But "not in refs/remotes/" does not automatically mean "the only sensible
>> place is refs/heads", does it? "We do not know what kind of mistake the
>> user is trying to make" could be more plausible answer, and in that case,
>> "complain and die" may be a more valid course of action.
>
> The thing is that I can't think of another sensible place. And this
> sensible place is useful for one particular action: renaming a remote
> branch, like this:
>
> $ git fetch ;# presumably gets origin/branch
> $ git push origin/branch:renamed-branch
>
> which is much nicer than exposing clueless users to
> ":refs/heads/renamed-branch".
You would need to expose ":refs/heads/branch" to make this a rename, not a
copy, wouldn't you?
>> For example,
>>
>> git push origin origin/master:refs/heads/master
>>
>> is most likely to be a mistake. The only situation something similar to
>> this makes sense is where you pushed out a bogus commit earlier and are
>> trying to correct it perhaps with
>
> I'm not sure why it's likely to be a mistake.
> ...
> Am I missing some part of your argument?
I do not see much point (other than your "rename" example) in pushing what
you got from the other end without changing anything yourself back to the
same remote.
There was a thread in which people argued that the primary thing that is
dangerous in this sequence
$ git checkout origin/next; work-commit; work-commit; ...
is when you leave the detached HEAD state without saving it to a local
branch. I think what is more dangerous is that it will not give the user
a solid understanding that these commits do _not_ change origin/next in
any way. After doing the above, it is understandable that a novice would
mistakenly think: "I started from origin/next and built some, let's push
the result".
Side note: This is the reason why I think Dscho's "git checkout
next" that dwims to "-t -b next origin/next" is OK (perhaps on the
right side of the borderline), because it is more clear that this
is about creating and using your own "next", compared to the
existing "-t origin/next" shorthand. The latter risks imprinting
a misconception on an uninitiated that we are somehow working on
origin/next.
With such a misconception, you will see the likely mistake here, too:
$ git push origin origin/next:refs/heads/next
If "rename" is the _only_ valid reason you might want to push what you got
from there back to the same remote, _and_ if "rename" is something very
often needed, I think we would prefer to have a way to support that
operation directly, instead of more general DWIM that would risk passing
mistakes like the above unwarned.
> ... But I'm not sure why
> you would accidentally provide something in refs/remotes, or not want to
> be pushing to refs/heads. Where _else_ do you push, except for tags?
The above "checking out origin/next" illustrates the mistake on the $src
side, not on the $dst side. That's why my alternative solution in the
previous message was to see the type of $src and push commits to local
branches, exactly because "where else do you push".
IOW, it's between "prevent push with dubious $src from happening in the
first place" vs "give them rope". Historically I always sided with the
latter camp, but I am trying to play a devil's advocate for a change ;-).
^ permalink raw reply
* Re: [PATCH 5/5] http-backend: more explict LocationMatch
From: Shawn O. Pearce @ 2009-10-27 22:24 UTC (permalink / raw)
To: Mark Lodato; +Cc: git, Junio C Hamano
In-Reply-To: <1256493935-8680-6-git-send-email-lodatom@gmail.com>
Mark Lodato <lodatom@gmail.com> wrote:
> In the git-http-backend examples, only match git-receive-pack within
> /git/.
All 5 patches:
Acked-by: Shawn O. Pearce <spearce@spearce.org>
Junio, add these to my smart-http topic please. :-)
Thanks Mark!
--
Shawn.
^ permalink raw reply
* Re: My custom cccmd
From: Junio C Hamano @ 2009-10-27 21:53 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git
In-Reply-To: <94a0d4530910250804w3a7da36eke10710eb1cbb03c1@mail.gmail.com>
Felipe Contreras <felipe.contreras@gmail.com> writes:
> I explored this a bit more and I've come to the conclusion that we
> actually don't wand to discard the previous commits in the patch
> series. Let's think about this example:
> 0001 <- John
> 0002 <- Me overriding some changes from John
>
> In this case we want John to appear in the CC list of 0002, because we
> are changing his code.
There are two cases: your patch partially overrides his code, and your
patch completely rewrites/removes his code.
Obviously in the former case you do want to Cc, but there are parts of his
change that remain in the final result, so this case does not matter in
this discussion. You will Cc him because of these remaining lines anyway.
In the latter case, the only contribution that remains from him is a
commit with his log message that does not help describing anything in the
end result, cluttering the history.
In such a case, I would imagine that the reviewers would want to see a
cleaned up series that does not have his patch that is irrelevant for
understanding the final result. John might want to know about it, if only
to raise objection to the way how you arranged your series. For that
reason, I think it makes sense to Cc him.
But it is a rather a convoluted logic, if you ask me. You find John and
Cc him, primarily so that he can point out that you should be redoing the
series not to have his patch as an intermediate state in the series to
begin with, because his commit does not contribute to the end result.
It might make more sense if your tool told you about such a case directly,
rather than helping you find John so that he can tell you ;-).
^ permalink raw reply
* Re: [PATCH 7/6 (v4)] support for commit grafts, slight change to general mechanism
From: Nick Edelen @ 2009-10-27 21:11 UTC (permalink / raw)
To: Thomas Rast
Cc: Junio C Hamano, Nicolas Pitre, Johannes Schindelin, Sam Vilain,
Michael J Gruber, Jeff King, Shawn O. Pearce, Andreas Ericsson,
Christian Couder, git@vger.kernel.org
In-Reply-To: <200910211115.25017.trast@student.ethz.ch>
> I felt adventurous and merged the topic into my local build, but I get
> "error: duplicate graft data ..." in repositories with only a single
> line in .git/info/grafts, which bisects to this commit (1c0a666 in
> today's pu).
Oops, it looks like there's a bug in my bugfix... I had
parse_commit() accidentally calling itself through
register_commit_graft(); fixed now though. Thanks for giving it a try
though!
^ permalink raw reply
* Re: [PATCH 2/4] Add I18N-wrappers for low-level IO-routines
From: Jeff King @ 2009-10-27 21:08 UTC (permalink / raw)
To: Timur Sufiev; +Cc: git
In-Reply-To: <1256651643-18382-2-git-send-email-timur@iris-comp.ru>
On Tue, Oct 27, 2009 at 04:54:01PM +0300, Timur Sufiev wrote:
> Signed-off-by: Timur Sufiev <timur@iris-comp.ru>
Hmm. Two questions about this series:
1. Patch 3/4 didn't seem to make it to the list. Presumably that is
where you actually use these routines in git? Or are they just for
mingw?
2. I seem to recall that Linus added a filename translation layer for
doing much more, like handling unicode normalizations (but I
confess I haven't looked closely at that code). Should this be part
of that system?
-Peff
^ permalink raw reply
* Re: possible usability issue in rebase -i?
From: Baz @ 2009-10-27 21:05 UTC (permalink / raw)
To: kusmabite; +Cc: Git Mailing List
In-Reply-To: <40aa078e0910270850u6ffec41cj372da11d9df533f@mail.gmail.com>
2009/10/27 Erik Faye-Lund <kusmabite@googlemail.com>:
> On Tue, Oct 27, 2009 at 4:17 PM, Baz <brian.ewins@gmail.com> wrote:
>> I've got a somewhat related minor usability issue with rebase -i. I
>> accidentally typed something like 'git rebase -i -z' and got this
>> message:
>>
>> error: unknown switch `z'
>> usage: git-rebase [-i] [options] [--] <upstream> [<branch>]
>> or: git-rebase [-i] (--continue | --abort | --skip)
>>
>> Available options are
>> -v, --verbose display a diffstat of what changed upstream
>> --onto ... rebase onto given branch instead of upstream
>> -p, --preserve-merges
>> try to recreate merges instead of ignoring them
>> -s, --strategy ... use the given merge strategy
>> -m, --merge always used (no-op)
>> -i, --interactive always used (no-op)
>>
>> The last two lines were the surprise. It suggested to me that '-i' and
>> '-m' were now the defaults for git-rebase - which of course they're
>> not. A user would not know that this is actually reporting the flags
>> that work for git-rebase--interactive, especially since that's not
>> what the command calls itself. I wasn't sure about the best approach
>> to fixing this - the only comparable commands that pass arbitrary
>> flags down to an exec'd program make it clear what program is going to
>> be called (usually git merge) and so interpreting errors is easier.
>>
>> It seems the intent here was to signal that the flags are different
>> once a rebase is in progress, but this usage message is shown when
>> rebase -i -z is called in any state.
>
> If that is the case, my instinct tells me that this information should
> be reflected in the usage-string (instead of the parameter
> description). Something like this?
I'm fine with this way of fixing it, but I'd make a few more changes...
>
> --->8---
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 23ded48..3ed5f94 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -13,15 +13,15 @@
> OPTIONS_KEEPDASHDASH=
> OPTIONS_SPEC="\
> git-rebase [-i] [options] [--] <upstream> [<branch>]
Use the dashless form and be more consistent with the help - and
mention '--root' here, it appears in the
help below:
-git-rebase [-i] [options] [--] <upstream> [<branch>]
+git rebase [--interactive | -i] [options] [--onto <newbase>] [--]
<upstream> [<branch>]
+git rebase [--interactive | -i] [options] --onto <newbase> --root
[--] [<branch>]
> -git-rebase [-i] (--continue | --abort | --skip)
> +git-rebase [-i] [-m] (--continue | --abort | --skip)
Again, dashless. And I'd not mention the useless -i here, the man page
doesn't either:
-git-rebase [-i] (--continue | --abort | --skip)
+git rebase (--continue | --abort | --skip)
> --
> Available options are
> v,verbose display a diffstat of what changed upstream
> onto= rebase onto given branch instead of upstream
> p,preserve-merges try to recreate merges instead of ignoring them
> s,strategy= use the given merge strategy
> -m,merge always used (no-op)
> -i,interactive always used (no-op)
> +m,merge use merging strategies
> +i,interactive interactively edit commits
These two items are misplaced in the help (I think). They're not like
abort, continue, skip, but then, the man page doesn't group those
separately either.
+no-verify override pre-rebase hook from stopping the operation
+root rebase all reachable commmits up to the root(s)
> Actions:
> continue continue rebasing process
> abort abort rebasing process and restore original branch
As above, remove the next two lines after your patch:
-no-verify override pre-rebase hook from stopping the operation
-root rebase all reachable commmits up to the root(s)
-Baz
>
> --
> Erik "kusma" Faye-Lund
>
^ permalink raw reply
* DOCBOOK_SUPPRESS_SP with docbook-sxl 1.71.0
From: Ramsay Jones @ 2009-10-27 19:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, chris_johnsen
Hi Junio,
Whenever I install a new git, I always re-build the documentation
(on Linux) rather than use the pre-built html and manpages.
(BTW, I actually don't know why; it's just a habit now:)
>From the Documentation/Makefile, it seems there may be some
speculation if DOCBOOK_SUPPRESS_SP should be set when using the
docbook-xsl version 1.71.0.
I am using asciidoc version 7.1.2 along with docbook-xsl version
1.71.0, and I can confirm that setting DOCBOOK_SUPPRESS_SP is
required. (The manpages are *unusable* otherwise)
[I'm sorry, but I don't know the xmlto version number and I can't
get to my linux box ATM]
ATB,
Ramsay Jones
^ permalink raw reply
* [PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code
From: Ramsay Jones @ 2009-10-27 19:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, Marius Storm-Olsen
After experimenting with several win32 compilers, it is clear
that the _WIN32 macro should be used to guard the inclusion
of the main WIN32 API header files, particularly in the main
git-compat-util.h header. In addition, the cygwin build should
restrict the use of the WIN32 API to compat/cygwin.c
In order to avoid mistakes with the use of WIN32 and _WIN32
macros, define a new WIN32_API macro, with a single point of
definition in git-compat-util.h, to isolate WIN32 specific
code.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
compat/cygwin.c | 1 +
compat/snprintf.c | 2 +-
compat/win32.h | 3 ---
diff-no-index.c | 2 +-
git-compat-util.h | 9 +++++----
help.c | 2 +-
| 4 ++--
run-command.c | 8 ++++----
run-command.h | 2 +-
setup.c | 2 +-
test-chmtime.c | 2 +-
thread-utils.c | 2 +-
12 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/compat/cygwin.c b/compat/cygwin.c
index b4a51b9..6695515 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,5 +1,6 @@
#define WIN32_LEAN_AND_MEAN
#include "../git-compat-util.h"
+#include <windows.h>
#include "win32.h"
#include "../cache.h" /* to read configuration */
diff --git a/compat/snprintf.c b/compat/snprintf.c
index e1e0e75..373692a 100644
--- a/compat/snprintf.c
+++ b/compat/snprintf.c
@@ -9,7 +9,7 @@
* always have room for a trailing NUL byte.
*/
#ifndef SNPRINTF_SIZE_CORR
-#if defined(WIN32) && (!defined(__GNUC__) || __GNUC__ < 4)
+#if defined(WIN32_API) && (!defined(__GNUC__) || __GNUC__ < 4)
#define SNPRINTF_SIZE_CORR 1
#else
#define SNPRINTF_SIZE_CORR 0
diff --git a/compat/win32.h b/compat/win32.h
index 8ce9104..a7ed72b 100644
--- a/compat/win32.h
+++ b/compat/win32.h
@@ -2,9 +2,6 @@
#define WIN32_H
/* common Win32 functions for MinGW and Cygwin */
-#ifndef WIN32 /* Not defined by Cygwin */
-#include <windows.h>
-#endif
static inline int file_attr_to_st_mode (DWORD attr)
{
diff --git a/diff-no-index.c b/diff-no-index.c
index 4ebc1db..fca8d5d 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -38,7 +38,7 @@ static int get_mode(const char *path, int *mode)
if (!path || !strcmp(path, "/dev/null"))
*mode = 0;
-#ifdef _WIN32
+#ifdef WIN32_API
else if (!strcasecmp(path, "nul"))
*mode = 0;
#endif
diff --git a/git-compat-util.h b/git-compat-util.h
index ef60803..5792f71 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -65,10 +65,11 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
-#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
-#include <winsock2.h>
-#include <windows.h>
+#if defined(_WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
+# define WIN32_API
+# define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
+# include <winsock2.h>
+# include <windows.h>
#endif
#include <unistd.h>
diff --git a/help.c b/help.c
index e8db31f..9fc9e69 100644
--- a/help.c
+++ b/help.c
@@ -126,7 +126,7 @@ static int is_executable(const char *name)
!S_ISREG(st.st_mode))
return 0;
-#ifdef WIN32
+#ifdef WIN32_API
{ /* cannot trust the executable bit, peek into the file instead */
char buf[3] = { 0 };
int n;
--git a/pager.c b/pager.c
index 86facec..d7aed0c 100644
--- a/pager.c
+++ b/pager.c
@@ -9,7 +9,7 @@
static int spawned_pager;
-#ifndef WIN32
+#ifndef WIN32_API
static void pager_preexec(void)
{
/*
@@ -72,7 +72,7 @@ void setup_pager(void)
static const char *env[] = { "LESS=FRSX", NULL };
pager_process.env = env;
}
-#ifndef WIN32
+#ifndef WIN32_API
pager_process.preexec_cb = pager_preexec;
#endif
if (start_command(&pager_process))
diff --git a/run-command.c b/run-command.c
index cf2d8f7..7872109 100644
--- a/run-command.c
+++ b/run-command.c
@@ -75,7 +75,7 @@ fail_pipe:
trace_argv_printf(cmd->argv, "trace: run_command:");
-#ifndef WIN32
+#ifndef WIN32_API
fflush(NULL);
cmd->pid = fork();
if (!cmd->pid) {
@@ -315,7 +315,7 @@ int run_command_v_opt_cd_env(const char **argv, int opt, const char *dir, const
return run_command(&cmd);
}
-#ifdef WIN32
+#ifdef WIN32_API
static unsigned __stdcall run_thread(void *data)
{
struct async *async = data;
@@ -331,7 +331,7 @@ int start_async(struct async *async)
return error("cannot create pipe: %s", strerror(errno));
async->out = pipe_out[0];
-#ifndef WIN32
+#ifndef WIN32_API
/* Flush stdio before fork() to avoid cloning buffers */
fflush(NULL);
@@ -360,7 +360,7 @@ int start_async(struct async *async)
int finish_async(struct async *async)
{
-#ifndef WIN32
+#ifndef WIN32_API
int ret = wait_or_whine(async->pid, "child process", 0);
#else
DWORD ret = 0;
diff --git a/run-command.h b/run-command.h
index fb34209..508568d 100644
--- a/run-command.h
+++ b/run-command.h
@@ -70,7 +70,7 @@ struct async {
int (*proc)(int fd, void *data);
void *data;
int out; /* caller reads from here and closes it */
-#ifndef WIN32
+#ifndef WIN32_API
pid_t pid;
#else
HANDLE tid;
diff --git a/setup.c b/setup.c
index 029371e..08f8891 100644
--- a/setup.c
+++ b/setup.c
@@ -41,7 +41,7 @@ const char *prefix_path(const char *prefix, int len, const char *path)
const char *prefix_filename(const char *pfx, int pfx_len, const char *arg)
{
static char path[PATH_MAX];
-#ifndef WIN32
+#ifndef WIN32_API
if (!pfx || !*pfx || is_absolute_path(arg))
return arg;
memcpy(path, pfx, pfx_len);
diff --git a/test-chmtime.c b/test-chmtime.c
index fe476cb..a779f7f 100644
--- a/test-chmtime.c
+++ b/test-chmtime.c
@@ -87,7 +87,7 @@ int main(int argc, const char *argv[])
return -1;
}
-#ifdef WIN32
+#ifdef WIN32_API
if (!(sb.st_mode & S_IWUSR) &&
chmod(argv[i], sb.st_mode | S_IWUSR)) {
fprintf(stderr, "Could not make user-writable %s: %s",
diff --git a/thread-utils.c b/thread-utils.c
index 4f9c829..c99273e 100644
--- a/thread-utils.c
+++ b/thread-utils.c
@@ -23,7 +23,7 @@ int online_cpus(void)
long ncpus;
#endif
-#ifdef _WIN32
+#ifdef WIN32_API
SYSTEM_INFO info;
GetSystemInfo(&info);
--
1.6.5
^ permalink raw reply related
* [PATCH 2/4] Makefile: merge two Cygwin configuration sections into one
From: Ramsay Jones @ 2009-10-27 19:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, Marius Storm-Olsen, dpotapov
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
I can't think of any potential problems with this... but I've
added Dmitry to the cc-list, since he appears to have added
the second cygwin section in adbc0b6b, just in case... ;-)
ATB,
Ramsay Jones
Makefile | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index fea237b..8e1cfc5 100644
--- a/Makefile
+++ b/Makefile
@@ -782,6 +782,8 @@ ifeq ($(uname_O),Cygwin)
NO_MMAP = YesPlease
NO_IPV6 = YesPlease
X = .exe
+ COMPAT_OBJS += compat/cygwin.o
+ UNRELIABLE_FSTAT = UnfortunatelyYes
endif
ifeq ($(uname_S),FreeBSD)
NEEDS_LIBICONV = YesPlease
@@ -891,10 +893,6 @@ ifeq ($(uname_S),HP-UX)
NO_SYS_SELECT_H = YesPlease
SNPRINTF_RETURNS_BOGUS = YesPlease
endif
-ifneq (,$(findstring CYGWIN,$(uname_S)))
- COMPAT_OBJS += compat/cygwin.o
- UNRELIABLE_FSTAT = UnfortunatelyYes
-endif
ifdef MSVC
GIT_VERSION := $(GIT_VERSION).MSVC
pathsep = ;
--
1.6.5
^ permalink raw reply related
* [PATCH 3/4] Makefile: keep MSVC and Cygwin configuration separate
From: Ramsay Jones @ 2009-10-27 19:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, Marius Storm-Olsen
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Makefile | 13 ++++++++++---
1 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 8e1cfc5..12c8249 100644
--- a/Makefile
+++ b/Makefile
@@ -658,6 +658,14 @@ EXTLIBS =
# Platform specific tweaks
#
+ifdef MSVC
+ # When building with msvc, on MinGW or Cygwin, we
+ # override the uname settings to make it easier to
+ # keep the configuration sections separate
+ uname_S = Windows
+ uname_O = Windows
+endif
+
# We choose to avoid "if .. else if .. else .. endif endif"
# because maintaining the nesting to match is a pain. If
# we had "elif" things would have been much nicer...
@@ -893,7 +901,7 @@ ifeq ($(uname_S),HP-UX)
NO_SYS_SELECT_H = YesPlease
SNPRINTF_RETURNS_BOGUS = YesPlease
endif
-ifdef MSVC
+ifeq ($(uname_S),Windows)
GIT_VERSION := $(GIT_VERSION).MSVC
pathsep = ;
NO_PREAD = YesPlease
@@ -945,7 +953,7 @@ else
BASIC_CFLAGS += -Zi -MTd
endif
X = .exe
-else
+endif
ifneq (,$(findstring MINGW,$(uname_S)))
pathsep = ;
NO_PREAD = YesPlease
@@ -994,7 +1002,6 @@ else
NO_PTHREADS = YesPlease
endif
endif
-endif
-include config.mak.autogen
-include config.mak
--
1.6.5
^ permalink raw reply related
* [PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
From: Ramsay Jones @ 2009-10-27 19:03 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, Marius Storm-Olsen
When the NO_MMAP build variable is set, which is the case by
default on cygwin, the msvc linker complains:
error LNK2001: unresolved external symbol _getpagesize
The mingw build has a version of the getpagesize() function
defined in the libgcc.a library. In addition, this function
is replaced, if USE_WIN32_MMAP is defined, by a version more
suitable for use with the native mmap.
The msvc libraries do not define any getpagesize() function,
so we move the mingw_getpagesize() implementation from the
conditionally built win32mmap.c file to mingw.c, which will
always be included in the MSVC build.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
This alone won't make the MSVC build work on cygwin; I have to
$ make MSVC=1 NEEDS_LIBICONV=
otherwise the linker complains about the absence of the iconv.lib.
(I have downloaded the win32 version of the library, but I haven't
got around to installing it! see http://gnuwin32.sourceforge.net/
packages/libiconv.htm)
The real problem, of course, is that the cygwin and MSVC configuration
sections are not mutually exclusive. (see patch 3/4)
ATB,
Ramsay Jones
compat/mingw.c | 12 ++++++++++++
compat/mingw.h | 2 +-
compat/win32mmap.c | 12 ------------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 6b5b5b2..15fe33e 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1000,6 +1000,18 @@ repeat:
return -1;
}
+/*
+ * Note that this doesn't return the actual pagesize, but
+ * the allocation granularity. If future Windows specific git code
+ * needs the real getpagesize function, we need to find another solution.
+ */
+int mingw_getpagesize(void)
+{
+ SYSTEM_INFO si;
+ GetSystemInfo(&si);
+ return si.dwAllocationGranularity;
+}
+
struct passwd *getpwuid(int uid)
{
static char user_name[100];
diff --git a/compat/mingw.h b/compat/mingw.h
index 5b5258b..26c4027 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -166,7 +166,7 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
int mingw_rename(const char*, const char*);
#define rename mingw_rename
-#ifdef USE_WIN32_MMAP
+#if defined(USE_WIN32_MMAP) || defined(_MSC_VER)
int mingw_getpagesize(void);
#define getpagesize mingw_getpagesize
#endif
diff --git a/compat/win32mmap.c b/compat/win32mmap.c
index 779d796..1c5a149 100644
--- a/compat/win32mmap.c
+++ b/compat/win32mmap.c
@@ -1,17 +1,5 @@
#include "../git-compat-util.h"
-/*
- * Note that this doesn't return the actual pagesize, but
- * the allocation granularity. If future Windows specific git code
- * needs the real getpagesize function, we need to find another solution.
- */
-int mingw_getpagesize(void)
-{
- SYSTEM_INFO si;
- GetSystemInfo(&si);
- return si.dwAllocationGranularity;
-}
-
void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset)
{
HANDLE hmap;
--
1.6.5
^ permalink raw reply related
* [PATCH 0/4] Cygwin MSVC patches
From: Ramsay Jones @ 2009-10-27 18:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list, mstormo
Hi Junio,
I've had these patches "hanging around" in my queue, for a few
weeks, with every intention of adding several more to fix up
some problems... Hmm, well I haven't got to those yet, so I
thought I may as well pass these on.
[PATCH 1/4] MSVC: Fix an "unresolved symbol" linker error on cygwin
[PATCH 2/4] Makefile: merge two Cygwin configuration sections into one
[PATCH 3/4] Makefile: keep MSVC and Cygwin configuration separate
[PATCH 4/4] win32: Improve the conditional inclusion of WIN32 API code
I think they are all pretty safe (famous last words), but it would be
a good idea for someone with an MSYS/MinGW installation to test them
(particularly patch #4; it's really the only one I'm slightly worried
about :D).
With these patches, the msvc build on cygwin seems to be working fine.
At first it looked bad; an ./git-status showed a shed-load of "Changed
but not updated" files along with many "Untracked files" which should
have been ignored (eg editor backup files).
In order to fix the ignored files problem, I had to make a change to
~/.gitconfig. The core.excludesfile was set to /home/ramsay/.gitignore,
which (being a cygwin path) the msvc build could not read. However,
setting this to C:/cygwin/home/ramsay/.gitignore fixed the problem
since both the cygwin and msvc builds can read that path.
When ./git-diff showed that most of the "Changed" files differed only
in the mode, in particular only the execute bit, it was easy to fix
that also; set core.filemode to false.
This left only the symlink RelNotes, which was shown as deleted and an
RelNotes.lnk file as untracked. This is to be expected on a cygwin
git repo (and is an example of the "white-lies" that cygwin tells you
when it iterates over the filesystem).
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [PATCH] Fix memory leak in transport-helper
From: Daniel Barkalow @ 2009-10-27 19:05 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Shawn O. Pearce, Junio C Hamano, Johan Herland, Nanako Shiraishi,
Sverre Rabbelier, git
In-Reply-To: <alpine.DEB.1.00.0910271954020.11562@felix-maschine>
On Tue, 27 Oct 2009, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 27 Oct 2009, Daniel Barkalow wrote:
>
> > On Tue, 27 Oct 2009, Johannes Schindelin wrote:
> >
> > > Hi,
> > >
> > > On Tue, 27 Oct 2009, Daniel Barkalow wrote:
> > >
> > > > diff --git a/transport-helper.c b/transport-helper.c
> > > > index f57e84c..479539d 100644
> > > > --- a/transport-helper.c
> > > > +++ b/transport-helper.c
> > > > @@ -67,6 +67,13 @@ static int disconnect_helper(struct transport *transport)
> > > > return 0;
> > > > }
> > > >
> > > > +static int close_helper(struct transport *transport)
> > > > +{
> > > > + disconnect_helper(transport);
> > > > + free(transport->data);
> > > > + return 0;
> > > > +}
> > >
> > > Why did you not leech the transport->data = NULL; part from Peff/Sverre's
> > > patch?
> >
> > Because this code is only called just before transport itself is freed by
> > the caller, and, in general, a transport with these methods is invalid
> > without a valid transport->data. I expect that's also why Peff called it a
> > hack not appropriate for actual application.
>
> So you mean to imply that this method is not about closing, but about
> releasing the structure. Right?
Yes, that's the word I was failing to come up with last night, thanks.
Junio, "s/close/release/g" on that patch should improve comprehensibility
greatly. (And changing the transport method name would probably also
improve matters)
-Daniel
*This .sig left intentionally blank*
^ 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