* Re: [PATCH 1/4] Suppress spurious linefeeds in git-add--interactive
From: Jeff King @ 2007-11-22 8:59 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git, gitster
In-Reply-To: <1195689773-28601-2-git-send-email-win@wincent.com>
On Thu, Nov 22, 2007 at 01:02:50AM +0100, Wincent Colaiuta wrote:
> + return undef if ($#diff == -1);
Style nit: I think the rest of the code generally uses (and I prefer)
"@diff" to get the number of elements. So:
return undef unless @diff;
or I might even have written
my @diff = ...
or return undef;
but perhaps I am the only one who finds $#array comparisons to -1 hard
on the eyes.
-Peff
^ permalink raw reply
* Re: Adding push configuration to .git/config
From: Steffen Prohaska @ 2007-11-22 8:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nico -telmich- Schottelius, git
In-Reply-To: <7vd4u28z90.fsf@gitster.siamese.dyndns.org>
On Nov 22, 2007, at 9:23 AM, Junio C Hamano wrote:
> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Nov 22, 2007, at 2:48 AM, Junio C Hamano wrote:
>> ...
>>> An alternative could be to split [remote "name"] url into two
>>> variants, fetch-url and push-url. While fetching by default
>>> from two places without telling from which one does not make any
>>> sense, pushing by default to two different places is quite a
>>> normal thing to do, and we already do support more than one url
>>> entries in [remote "name"] section used for pushing.
>>>
>>> If we were to do this, it might also make sense to rename the
>>> word 'origin' we use for the default remote name to 'default' or
>>> something. People with shared repository workflow would fetch
>>> from one repository and push back to the same repository, so the
>>> distinction would not matter, but for others who need something
>>> like you suggest, the default repository for fetching and
>>> pushing are different, and while you may still consider where
>>> you fetch from your 'origin', where you push into is not your
>>> 'origin' anymore.
>>
>> I like this idea.
>>
>> But in addition, we should have a branch.$name.push line that
>> can contain a remote head to push to.
>
> Yes, but.
>
> At that point, I think you would introduce a mismatch between
> the traditional semantics of refspec and what you are trying to
> do, unless you are careful.
>
> The traditional semantics of refspecs-tied-to-remote is strongly
> based on the assumption: "I will push to this remote when these
> local branches are all ready to be pushed out, and they will all
> go there together as an atomic update. When I am _that_ ready
> to push, it does not matter which local branch I am on. The
> branches that matter are all in good shape when I push."
>
> You are making the behaviour of push dependent on which branch
> you are on. During such a push, it is safe to assume that the
> current branch is ready to be pushed out, but other ones can be
> very much un-ready. The user needs a safety valve to prevent
> other branches from being pushed out. Otherwise the user would
> not be adding branch.$name.push to begin with.
This is not the only reason. If I'm thinking of a workflow
based on a shared repo, I see two more reasons
The user wants to limit the push to the current branch
because different branches in the remote (shared) repo may
have advanced. The user does not want to the see errors by
"git push" that complain about non-fast-forwards. The safety
valve is needed to protect from changes in the remote repo
(not for changes in the local one).
The second use case is to have multiple local branches that
refer to the same (shared) remote branch. If you want to push
the changes directly to the (shared) remote branch, you need to
"rename" the branch during the push. But you want to push only
a single branch: the current branch you're on. Hence you need
a per-branch configuration if you want "git push" to to the
work for you automatically. It would be nice if "git branch"
could be asked to set up branch.$name.push to point to the
correct remote branch (similar to "--track").
With what I have in mind in the longterm you could do
git checkout -b foo origin/master
work work work
git checkout -b bar origin/master
work work work
git checkout foo
git pull # or git fetch; git rebase
git push
git checkout bar
git branch -d foo
work work, ... and later push bar, too
Note, there are several other changes needed before this would
work:
- "git checkout" would have to set branch.$name.push.
- "-d" would needs to be accept deletion if all changes are on
shared remote branch.
> It would probably need to become a target ref or a list of <URL,
> target ref>, not a list of general refspecs like the value for
> remote.$there.push variable. For example, you would want to say
> "while on master, push it to repository A as refs/heads/master
> and to repository B as refs/remotes/satellite/master", which
> would be a typical arrangement if you are working on a satellite
> machine, A is the shared central repository and B is mothership
> to your satellite machine. The specification would talk only
> about the target ref (not just "'can contain' a remote head to
> push to"), and the source ref would always be the current
> branch.
>
> I guess you could use general refspec on branch.$name.push and
> implement the safety by requiring (1) only one refspec appears
> for such a variable, and (2) the LHS of the refspec matches the
> $name of the branch, in order to make the parsing easier and to
> keep the syntax uniform.
>
> Or maybe I am being overly cautious again not to introduce any
> more unnecessary user confusion, and just should give them even
> longer rope to hang themselves. I dunno.
Yes.
In the simplest version, branch.$name.push should only contain
the remote part of the refspec. It should be symmetric to
branch.$name.merge.
So if branch.$name.push is set, "git push" means
git push <url-from-remote> $name:<value of branch.$name.push>
This is what I proposed earlier.
Steffen
^ permalink raw reply
* Re: [PATCH] Authenticate only once in git-send-email
From: Junio C Hamano @ 2007-11-22 8:48 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git
In-Reply-To: <1195648505-21653-1-git-send-email-win@wincent.com>
Wincent Colaiuta <win@wincent.com> writes:
> This commit teaches git-send-email to authenticate once and only once at
> the beginning of the series.
Ok. What does $smtp->auth() return? Presumably a true value,
but I do not find it the best coding style to hide a call made
primarily for its effects not for its return value behind a
conditional assignment to a boolean. Eek.
> if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
> - $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> + $auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> }
Perhaps something along the lines of...
> if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
> $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> + undef $smtp_authpass;
> }
... or using a separate boolean variable "my $auth_happened"
may be more appropriate.
But I am just saying this; I do not care _too_ deeply about it.
Will apply as-is.
^ permalink raw reply
* Re: [StGit PATCH] Added test case for stg refresh
From: David Kågedal @ 2007-11-22 8:38 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Git Mailing List, Karl Hasselström
In-Reply-To: <b0943d9e0711220015r64d1a5c2y25dfa44610864c99@mail.gmail.com>
"Catalin Marinas" <catalin.marinas@gmail.com> writes:
> On 22/11/2007, David Kågedal <davidk@lysator.liu.se> wrote:
>> "Catalin Marinas" <catalin.marinas@gmail.com> writes:
>> > I noticed the weirdness few days ago and fixed it in
>> > e8813959aa3a7c41ffef61d06068b10519bd4830 (though no test caught it).
>> > Do you still see problems after this commit?
>>
>> The problem I see is that there still is no test case. That is bad and
>> means that it could break again tomorrow without anyone noticing.
>>
>> Luckily, I just wrote one for you :-)
>
> Thanks :-). We are still far from testing all the possible
> combinations. Is there a way to do code coverage in Python?
Being far from testing everything doesn't mean that you can't start
adding tests for the things you know have a tendency to break. And for
the things that you are going to refactor. And as regression tests
when you fix a bug. A test suite can be built up by adding small parts
at a time.
But no, I don't know of any useful code coverage tool for python, but
I haven't really looked.
--
David Kågedal
^ permalink raw reply
* [PATCH] Fix "quote" misconversion for rewrite diff output.
From: Junio C Hamano @ 2007-11-22 8:36 UTC (permalink / raw)
To: git
663af3422a648e87945e4d8c0cc3e13671f2bbde (Full rework of
quote_c_style and write_name_quoted.) mistakenly used puts()
when writing out a fixed string when it did not want to add a
terminating LF.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 2 +-
t/t4022-diff-rewrite.sh | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+), 1 deletions(-)
create mode 100755 t/t4022-diff-rewrite.sh
diff --git a/diff.c b/diff.c
index b08d28a..6b54959 100644
--- a/diff.c
+++ b/diff.c
@@ -2716,7 +2716,7 @@ static void diff_summary(struct diff_filepair *p)
break;
default:
if (p->score) {
- puts(" rewrite ");
+ fputs(" rewrite ", stdout);
write_name_quoted(p->two->path, stdout, ' ');
printf("(%d%%)\n", similarity_index(p));
}
diff --git a/t/t4022-diff-rewrite.sh b/t/t4022-diff-rewrite.sh
new file mode 100755
index 0000000..6de4acb
--- /dev/null
+++ b/t/t4022-diff-rewrite.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='rewrite diff'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+
+ cat ../../COPYING >test &&
+ git add test &&
+ tr 'a-zA-Z' 'n-za-mN-ZA-M' <../../COPYING >test
+
+'
+
+test_expect_success 'detect rewrite' '
+
+ actual=$(git diff-files -B --summary test) &&
+ expr "$actual" : " rewrite test ([0-9]*%)$" || {
+ echo "Eh? <<$actual>>"
+ false
+ }
+
+'
+
+test_done
+
--
1.5.3.6.1929.g388a
^ permalink raw reply related
* Re: [PATCH 3/3] Replace setenv(GIT_DIR_ENVIRONMENT, ...) with set_git_dir()
From: Steffen Prohaska @ 2007-11-22 8:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Dmitry Kakurin
In-Reply-To: <7vsl2y90pm.fsf@gitster.siamese.dyndns.org>
On Nov 22, 2007, at 8:52 AM, Junio C Hamano wrote:
> Steffen Prohaska <prohaska@zib.de> writes:
>
>> On Nov 22, 2007, at 3:34 AM, Junio C Hamano wrote:
>>
>>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>>
>>>> Does this not have a fundamental issue? When you call other git
>>>> programs
>>>> with run_command(), you _need_ GIT_DIR to be set, no?
>>>
>>> It is much worse. set_git_dir() does not just setenv() but does
>>> setup_git_env() as well.
>>
>> What do your comments mean?
>>
>> My understanding is that set_git_dir() sets the environment and
>> then calls setup_git_env() to cache all pointers. This call
>> updates dangling pointer if they have been cached earlier.
>
> Well, I was agreeing with you. "Worse" was about what the
> current code does _not_ do.
>
> If there are earlier calls that obtain locations relative to the
> earlier definition of GIT_DIR, the locations they obtained are
> not just stored in memory that is "dangling" (which was what
> your proposed log message described) but they are also
> inconsistent with the updated definition of GIT_DIR.
>
> I suspect Johannes mistook set_git_dir() was only local
> (i.e. per calling process) matter without noticing that it has
> its own setenv() when he made that comment, hence my response to
> point out that the current code only calls setenv(), but
> set_git_dir() does setup_git_env() too, which should hide the
> inconsistency problem.
>
> HOWEVER.
>
> I suspect that if there are even earlier callers than these
> early parts in the codepaths (handle_options, enter_repo, and
> setup_git_directory_gently), maybe these earlier callers are
> doing something wrong. Logically, if you are somewhere very
> early in the codepath that you can still change the value of
> GIT_DIR, you shouldn't have assumed the unknown value of GIT_DIR
> and cached locations relative to that directory, no? What are
> the problematic callers? What values do they access and why?
I thought about these questions, too. But only very briefly.
I did not analyze the code path that lead to calls of getenv().
I'm not sure if it's really necessary. Calling set_git_dir()
looks more sensible too me than the old code. I believe using
set_git_dir() is the safer choice, and should not do any harm.
So I stopped analyzing too much, and instead proposed to use
set_git_dir().
Interesting, though, is to find out if we have other potentially
dangerous calls to getenv() that are not removed by this patch.
Steffen
^ permalink raw reply
* Re: Adding push configuration to .git/config
From: Junio C Hamano @ 2007-11-22 8:28 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Steffen Prohaska, Nico -telmich- Schottelius, git
In-Reply-To: <47453551.3060502@op5.se>
Andreas Ericsson <ae@op5.se> writes:
> Steffen Prohaska wrote:
>>
>> On Nov 22, 2007, at 2:48 AM, Junio C Hamano wrote:
>> ...
>>> If we were to do this, it might also make sense to rename the
>>> word 'origin' we use for the default remote name to 'default' or
>>> something. People with shared repository workflow would fetch
>>> from one repository and push back to the same repository, so the
>>> distinction would not matter, but for others who need something
>>> like you suggest, the default repository for fetching and
>>> pushing are different, and while you may still consider where
>>> you fetch from your 'origin', where you push into is not your
>>> 'origin' anymore.
>>
>> I like this idea.
>
> I don't. It's troublesome enough to try to teach the finer points
> of git to my co-workers without different defaults between versions.
I don't like the s/origin/default/ part either, which was the
reason I said "it might". That would be what I would have done
if I were doing git from scratch right now.
^ permalink raw reply
* Re: Adding push configuration to .git/config
From: Junio C Hamano @ 2007-11-22 8:23 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Nico -telmich- Schottelius, git
In-Reply-To: <C297CFC3-8DD0-4EEE-8FD3-BF997F6E269A@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
> On Nov 22, 2007, at 2:48 AM, Junio C Hamano wrote:
> ...
>> An alternative could be to split [remote "name"] url into two
>> variants, fetch-url and push-url. While fetching by default
>> from two places without telling from which one does not make any
>> sense, pushing by default to two different places is quite a
>> normal thing to do, and we already do support more than one url
>> entries in [remote "name"] section used for pushing.
>>
>> If we were to do this, it might also make sense to rename the
>> word 'origin' we use for the default remote name to 'default' or
>> something. People with shared repository workflow would fetch
>> from one repository and push back to the same repository, so the
>> distinction would not matter, but for others who need something
>> like you suggest, the default repository for fetching and
>> pushing are different, and while you may still consider where
>> you fetch from your 'origin', where you push into is not your
>> 'origin' anymore.
>
> I like this idea.
>
> But in addition, we should have a branch.$name.push line that
> can contain a remote head to push to.
Yes, but.
At that point, I think you would introduce a mismatch between
the traditional semantics of refspec and what you are trying to
do, unless you are careful.
The traditional semantics of refspecs-tied-to-remote is strongly
based on the assumption: "I will push to this remote when these
local branches are all ready to be pushed out, and they will all
go there together as an atomic update. When I am _that_ ready
to push, it does not matter which local branch I am on. The
branches that matter are all in good shape when I push."
You are making the behaviour of push dependent on which branch
you are on. During such a push, it is safe to assume that the
current branch is ready to be pushed out, but other ones can be
very much un-ready. The user needs a safety valve to prevent
other branches from being pushed out. Otherwise the user would
not be adding branch.$name.push to begin with.
It would probably need to become a target ref or a list of <URL,
target ref>, not a list of general refspecs like the value for
remote.$there.push variable. For example, you would want to say
"while on master, push it to repository A as refs/heads/master
and to repository B as refs/remotes/satellite/master", which
would be a typical arrangement if you are working on a satellite
machine, A is the shared central repository and B is mothership
to your satellite machine. The specification would talk only
about the target ref (not just "'can contain' a remote head to
push to"), and the source ref would always be the current
branch.
I guess you could use general refspec on branch.$name.push and
implement the safety by requiring (1) only one refspec appears
for such a variable, and (2) the LHS of the refspec matches the
$name of the branch, in order to make the parsing easier and to
keep the syntax uniform.
Or maybe I am being overly cautious again not to introduce any
more unnecessary user confusion, and just should give them even
longer rope to hang themselves. I dunno.
^ permalink raw reply
* Re: Git in a Nutshell guide
From: David Kågedal @ 2007-11-22 8:22 UTC (permalink / raw)
To: git; +Cc: Jakub Narebski
In-Reply-To: <fhs33j$eg9$1@ger.gmane.org>
Jakub Narebski <jnareb@gmail.com> writes:
> A few comments (from what I checked so far):
>
> 1. It is "man git-checkout" (or "git checkout --help"),
> not "man git checkout"
Or "git help checkout", which is what I usually use (for some reason).
--
David Kågedal
^ permalink raw reply
* Re: Git Screencast ?
From: Andreas Ericsson @ 2007-11-22 8:23 UTC (permalink / raw)
To: Michael Donaghy; +Cc: git
In-Reply-To: <fi1a9f$30q$1@ger.gmane.org>
Michael Donaghy wrote:
> I am a new user of git and SCMs in general , I have learned a lot from
> the docs , irc and just using it , but there are just some concepts that
> hare hard to grasp , I am a very visual persion (probably like a lot of
> you :) ) , is there a screencast of git somewhere (for free) that can
> demonstrate some of the advanced features of git...
> (Also...I have seen both Randal (Schwartz) and Linus' (you better know
> his last name) presentations at google on git , in which they explain
> the ideas behind git more than the actual usage,
>
The thing that helped me and my co-workers the most was running through
the steps of the tutorial, but stopping every time something wasn't
absolutely crystal clear and doing gitk or qgit (I prefer qgit, since
it seems to display things more consistently and also loads faster).
That primarily helped us get branches under control. We came from a
CVS world and never had any branches, as none of us had time or energy
to figure out how to merge them back together using those crippled scm's.
I noticed Johannes Gilger already sent you the link to git-for-scientists
thing which is also a really good aid, especially when you've read the
tutorial and some of the less tech-oriented docs.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [StGit PATCH] Added test case for stg refresh
From: Catalin Marinas @ 2007-11-22 8:15 UTC (permalink / raw)
To: David Kågedal; +Cc: Karl Hasselström, Git Mailing List
In-Reply-To: <871waivhgq.fsf@virtutech.se>
On 22/11/2007, David Kågedal <davidk@lysator.liu.se> wrote:
> "Catalin Marinas" <catalin.marinas@gmail.com> writes:
> > I noticed the weirdness few days ago and fixed it in
> > e8813959aa3a7c41ffef61d06068b10519bd4830 (though no test caught it).
> > Do you still see problems after this commit?
>
> The problem I see is that there still is no test case. That is bad and
> means that it could break again tomorrow without anyone noticing.
>
> Luckily, I just wrote one for you :-)
Thanks :-). We are still far from testing all the possible
combinations. Is there a way to do code coverage in Python?
--
Catalin
^ permalink raw reply
* Re: [StGit PATCH] Added test case for stg refresh
From: David Kågedal @ 2007-11-22 7:59 UTC (permalink / raw)
To: Karl Hasselström, Catalin Marinas; +Cc: Git Mailing List
In-Reply-To: <b0943d9e0711211531v2f7b9c0bl99033c0bd58971c7@mail.gmail.com>
"Catalin Marinas" <catalin.marinas@gmail.com> writes:
> On 21/11/2007, Karl Hasselström <kha@treskal.com> wrote:
>> On 2007-11-21 11:43:00 +0100, David Kågedal wrote:
>>
>> > David Kågedal <davidk@lysator.liu.se> writes:
>> >
>> > > This test case fails on the kha/experimental branch. Using "stg
>> > > refresh -p <patch>" can cause all sorts of wieirdness, and there
>> > > is no test case for it.
>
> I noticed the weirdness few days ago and fixed it in
> e8813959aa3a7c41ffef61d06068b10519bd4830 (though no test caught it).
> Do you still see problems after this commit?
The problem I see is that there still is no test case. That is bad and
means that it could break again tomorrow without anyone noticing.
Luckily, I just wrote one for you :-)
--
David Kågedal
^ permalink raw reply
* Re: Adding push configuration to .git/config
From: Andreas Ericsson @ 2007-11-22 7:52 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Junio C Hamano, Nico -telmich- Schottelius, git
In-Reply-To: <C297CFC3-8DD0-4EEE-8FD3-BF997F6E269A@zib.de>
Steffen Prohaska wrote:
>
> On Nov 22, 2007, at 2:48 AM, Junio C Hamano wrote:
>
>> Nico -telmich- Schottelius <nico-linux-git@schottelius.org>
>> writes:
>>
>>> Nice would be
>>>
>>> [branch "master"]
>>> remote-push = origin
>>> remote-push-merge = another_branch
>>>
>>> And thus perhaps also changing the existing specs:
>>>
>>> remote = ... to remote-fetch = ...
>>> merge = ... to remote-fetch-merge =
>>
>> I do not think doing this is worth it, not because I think a
>> single branch.$name.remote should be good enough for everybody,
>> but because once you need a separate remote each for fetching
>> and pushing, there is no reason to say one per direction is
>> enough.
>>
>> An alternative could be to split [remote "name"] url into two
>> variants, fetch-url and push-url. While fetching by default
>> from two places without telling from which one does not make any
>> sense, pushing by default to two different places is quite a
>> normal thing to do, and we already do support more than one url
>> entries in [remote "name"] section used for pushing.
>>
>> If we were to do this, it might also make sense to rename the
>> word 'origin' we use for the default remote name to 'default' or
>> something. People with shared repository workflow would fetch
>> from one repository and push back to the same repository, so the
>> distinction would not matter, but for others who need something
>> like you suggest, the default repository for fetching and
>> pushing are different, and while you may still consider where
>> you fetch from your 'origin', where you push into is not your
>> 'origin' anymore.
>
> I like this idea.
>
I don't. It's troublesome enough to try to teach the finer points
of git to my co-workers without different defaults between versions.
So far we're getting around the problem by the relatively crude
expedient of forcing everyone to update to the latest stable from
master each time I say so. It works, but doesn't scale too well, and
since every major distro now ships git packages, it would be nice if
default-names at least settled down and were only changed with new
major releases (that is, from 1.x to 2.x).
On the other hand, I'm sure we'll cope whatever you decide.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH 3/3] Replace setenv(GIT_DIR_ENVIRONMENT, ...) with set_git_dir()
From: Junio C Hamano @ 2007-11-22 7:52 UTC (permalink / raw)
To: Steffen Prohaska; +Cc: Johannes Schindelin, git, Dmitry Kakurin
In-Reply-To: <C50619A0-4A67-4968-8431-D7A685F723B7@zib.de>
Steffen Prohaska <prohaska@zib.de> writes:
> On Nov 22, 2007, at 3:34 AM, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> Does this not have a fundamental issue? When you call other git
>>> programs
>>> with run_command(), you _need_ GIT_DIR to be set, no?
>>
>> It is much worse. set_git_dir() does not just setenv() but does
>> setup_git_env() as well.
>
> What do your comments mean?
>
> My understanding is that set_git_dir() sets the environment and
> then calls setup_git_env() to cache all pointers. This call
> updates dangling pointer if they have been cached earlier.
Well, I was agreeing with you. "Worse" was about what the
current code does _not_ do.
If there are earlier calls that obtain locations relative to the
earlier definition of GIT_DIR, the locations they obtained are
not just stored in memory that is "dangling" (which was what
your proposed log message described) but they are also
inconsistent with the updated definition of GIT_DIR.
I suspect Johannes mistook set_git_dir() was only local
(i.e. per calling process) matter without noticing that it has
its own setenv() when he made that comment, hence my response to
point out that the current code only calls setenv(), but
set_git_dir() does setup_git_env() too, which should hide the
inconsistency problem.
HOWEVER.
I suspect that if there are even earlier callers than these
early parts in the codepaths (handle_options, enter_repo, and
setup_git_directory_gently), maybe these earlier callers are
doing something wrong. Logically, if you are somewhere very
early in the codepath that you can still change the value of
GIT_DIR, you shouldn't have assumed the unknown value of GIT_DIR
and cached locations relative to that directory, no? What are
the problematic callers? What values do they access and why?
^ permalink raw reply
* Re: Temporary directories getting errantly added into trees
From: Eric Wong @ 2007-11-22 7:30 UTC (permalink / raw)
To: Sam Vilain; +Cc: Git Mailing List
In-Reply-To: <4744FCD9.7020102@vilain.net>
Sam Vilain <sam@vilain.net> wrote:
> I just got through a rather nasty debugging session with git-rebase,
> which relies on a .dotest directory. Turns out that .dotest was
> accidentally added to the tree in the history of the commit that was
> being rebased onto.
Hi Sam,
Ouch
> There are a lot of temporary files like that made by various scripts -
> eg, git-filter-branch makes .git-rewrite, etc.
>
> I think it would be a good thing for all if you had to work very hard to
> put files like this in the tree, or perhaps it would be better to go
> through and make all the tools that create these temporary directories
> create them under .git instead.
Polluting the working tree is definitely a bad thing to do IMNSHO.
git-rebase --merge already puts its temporary directory inside
.git/.dotest-merge. I hoped to set an example with that and
get more tools to do the same; but it hasn't happened yet...
--
Eric Wong
^ permalink raw reply
* Re: [PATCH 3/3] Replace setenv(GIT_DIR_ENVIRONMENT, ...) with set_git_dir()
From: Johannes Sixt @ 2007-11-22 7:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Steffen Prohaska, git, Dmitry Kakurin
In-Reply-To: <7v63zv9fel.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Hi,
>>
>> On Wed, 21 Nov 2007, Steffen Prohaska wrote:
>>
>>> We have a function set_git_dir(). So let's use it, instead of setting
>>> the evironment directly.
>> Does this not have a fundamental issue? When you call other git programs
>> with run_command(), you _need_ GIT_DIR to be set, no?
>
> It is much worse. set_git_dir() does not just setenv() but does
> setup_git_env() as well.
I don't see what's wrong with that. Could you please explain?
-- Hannes
^ permalink raw reply
* Re: Adding push configuration to .git/config
From: Steffen Prohaska @ 2007-11-22 7:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nico -telmich- Schottelius, git
In-Reply-To: <7vabp79hjt.fsf@gitster.siamese.dyndns.org>
On Nov 22, 2007, at 2:48 AM, Junio C Hamano wrote:
> Nico -telmich- Schottelius <nico-linux-git@schottelius.org>
> writes:
>
>> Nice would be
>>
>> [branch "master"]
>> remote-push = origin
>> remote-push-merge = another_branch
>>
>> And thus perhaps also changing the existing specs:
>>
>> remote = ... to remote-fetch = ...
>> merge = ... to remote-fetch-merge =
>
> I do not think doing this is worth it, not because I think a
> single branch.$name.remote should be good enough for everybody,
> but because once you need a separate remote each for fetching
> and pushing, there is no reason to say one per direction is
> enough.
>
> An alternative could be to split [remote "name"] url into two
> variants, fetch-url and push-url. While fetching by default
> from two places without telling from which one does not make any
> sense, pushing by default to two different places is quite a
> normal thing to do, and we already do support more than one url
> entries in [remote "name"] section used for pushing.
>
> If we were to do this, it might also make sense to rename the
> word 'origin' we use for the default remote name to 'default' or
> something. People with shared repository workflow would fetch
> from one repository and push back to the same repository, so the
> distinction would not matter, but for others who need something
> like you suggest, the default repository for fetching and
> pushing are different, and while you may still consider where
> you fetch from your 'origin', where you push into is not your
> 'origin' anymore.
I like this idea.
But in addition, we should have a branch.$name.push line that
can contain a remote head to push to. This can be used to
manage push's default on a per-branch basis. So, different
branches can have different default refspecs, even when they
refer to the same remote.
The default remote of "git push" is either origin, or it is
specified in the branch configuration. The following rules
would then be used to find the refspecs to push. The first
rule that matches wins:
1) Command line overrides (e.g. "--all", "--current").
2) Check if branch.$name.push entry is available.
(Would we allow multiple entries?)
3) Check if remote.$remotename.push entries are available.
4) Use default rule, which pushes matching branches.
Steffen
^ permalink raw reply
* Re: [PATCH 3/3] Replace setenv(GIT_DIR_ENVIRONMENT, ...) with set_git_dir()
From: Steffen Prohaska @ 2007-11-22 6:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Dmitry Kakurin
In-Reply-To: <7v63zv9fel.fsf@gitster.siamese.dyndns.org>
On Nov 22, 2007, at 3:34 AM, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Hi,
>>
>> On Wed, 21 Nov 2007, Steffen Prohaska wrote:
>>
>>> We have a function set_git_dir(). So let's use it, instead of
>>> setting
>>> the evironment directly.
>>
>> Does this not have a fundamental issue? When you call other git
>> programs
>> with run_command(), you _need_ GIT_DIR to be set, no?
>
> It is much worse. set_git_dir() does not just setenv() but does
> setup_git_env() as well.
What do your comments mean?
My understanding is that set_git_dir() sets the environment and
then calls setup_git_env() to cache all pointers. This call
updates dangling pointer if they have been cached earlier.
But maybe there is a hidden secret that I don't see.
Steffen
^ permalink raw reply
* Re: [PATCH 2/3 v3] git-svn info: implement info command
From: Eric Wong @ 2007-11-22 4:17 UTC (permalink / raw)
To: David D. Kilzer, Junio C Hamano; +Cc: git
In-Reply-To: <900537.19467.qm@web52411.mail.re2.yahoo.com>
"David D. Kilzer" <ddkilzer@kilzer.net> wrote:
> Eric Wong <normalperson@yhbt.net> wrote:
> > When running from a top-level directory with no arguments, the first
> > line of git-ls-tree was being read. This allowed the test case to pass
> > because ls-tree sorts the output and 'directory' just happened to
> > be up top; so we were getting the 040000 mode from the 'directory'
> > tree and not the top-level tree.
> >
> > The below test should fix it for the trivial case I have.
>
> Acked-by: David D. Kilzer <ddkilzer@kilzer.net>
>
> Looks good! Thanks!
Ok, I've folded that into your [2/3] and pushed everything (and an
earlier fix) out to
git://git.bogomips.org/git-svn.git
David D. Kilzer (3):
git-svn: extract reusable code into utility functions
git-svn info: implement info command
git-svn: info --url [path]
Eric Wong (2):
t9106: fix a race condition that caused svn to miss modifications
git-svn: allow `info' command to work offline
Junio, please pull, thanks.
--
Eric Wong
^ permalink raw reply
* Re: [PATCH 4/3] git-svn: allow `info' command to work offline
From: Eric Wong @ 2007-11-22 3:56 UTC (permalink / raw)
To: Adam Roben; +Cc: David D. Kilzer, git
In-Reply-To: <4744F66D.7030007@apple.com>
Adam Roben <aroben@apple.com> wrote:
> Eric Wong wrote:
> >+ my $k = "svn-remote.$self->{repo_id}.reposRoot";
> >
>
> "repoRoot" seems slightly more intuitive than "reposRoot", given that
> "repository" is normally abbreviated as "repo".
>From a git-only point of view, yes. But it's repos_root everywhere
inside git-svn because SVN uses "*_repos_root" for their API.
This is inside the hidden metadata file that users
shouldn't have to touch anyways.
On a side note:
I personally *hate* camelCase names (or worse, alllowercase), but
git config doesn't allow underscores in config keys for some
strange reason (especially strange since most of the git and Linux
source code use snake_case...)
--
Eric Wong
^ permalink raw reply
* Temporary directories getting errantly added into trees
From: Sam Vilain @ 2007-11-22 3:51 UTC (permalink / raw)
To: Git Mailing List
I just got through a rather nasty debugging session with git-rebase,
which relies on a .dotest directory. Turns out that .dotest was
accidentally added to the tree in the history of the commit that was
being rebased onto.
There are a lot of temporary files like that made by various scripts -
eg, git-filter-branch makes .git-rewrite, etc.
I think it would be a good thing for all if you had to work very hard to
put files like this in the tree, or perhaps it would be better to go
through and make all the tools that create these temporary directories
create them under .git instead.
Thoughts/comments?
Sam.
^ permalink raw reply
* Re: [PATCH 4/3] git-svn: allow `info' command to work offline
From: Adam Roben @ 2007-11-22 3:24 UTC (permalink / raw)
To: Eric Wong; +Cc: David D. Kilzer, git
In-Reply-To: <20071122022343.GA9992@soma>
Eric Wong wrote:
> + my $k = "svn-remote.$self->{repo_id}.reposRoot";
>
"repoRoot" seems slightly more intuitive than "reposRoot", given that
"repository" is normally abbreviated as "repo".
-Adam
^ permalink raw reply
* Re: [PATCH 2/3 v3] git-svn info: implement info command
From: David D. Kilzer @ 2007-11-22 3:16 UTC (permalink / raw)
To: Eric Wong; +Cc: git
In-Reply-To: <20071122014038.GA25341@soma>
Eric Wong <normalperson@yhbt.net> wrote:
> When running from a top-level directory with no arguments, the first
> line of git-ls-tree was being read. This allowed the test case to pass
> because ls-tree sorts the output and 'directory' just happened to
> be up top; so we were getting the 040000 mode from the 'directory'
> tree and not the top-level tree.
>
> The below test should fix it for the trivial case I have.
Acked-by: David D. Kilzer <ddkilzer@kilzer.net>
Looks good! Thanks!
Dave
^ permalink raw reply
* Re: [PATCH 3/3] Replace setenv(GIT_DIR_ENVIRONMENT, ...) with set_git_dir()
From: Junio C Hamano @ 2007-11-22 2:34 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Steffen Prohaska, git, Dmitry Kakurin
In-Reply-To: <Pine.LNX.4.64.0711220121560.27959@racer.site>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi,
>
> On Wed, 21 Nov 2007, Steffen Prohaska wrote:
>
>> We have a function set_git_dir(). So let's use it, instead of setting
>> the evironment directly.
>
> Does this not have a fundamental issue? When you call other git programs
> with run_command(), you _need_ GIT_DIR to be set, no?
It is much worse. set_git_dir() does not just setenv() but does
setup_git_env() as well.
^ permalink raw reply
* [PATCH 4/3] git-svn: allow `info' command to work offline
From: Eric Wong @ 2007-11-22 2:23 UTC (permalink / raw)
To: David D. Kilzer; +Cc: git
In-Reply-To: <1195675039-26746-1-git-send-email-ddkilzer@kilzer.net>
Cache the repository root whenever we connect to the repository.
This will allow us to notice URL changes if the user changes the
URL in .git/config, too.
If the repository is no longer accessible, or if `git svn info'
is the first and only command run; then '(offline)' will be
displayed for "Repository Root:" in the output.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
David:
I'll apply this once you've verified my fix to 1/3 is correct behavior,
too.
git-svn.perl | 26 +++++++++++++++++++++++---
1 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 7d86870..43e1591 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -782,9 +782,14 @@ sub cmd_info {
$result .= "Name: " . basename($path) . "\n" if $file_type ne "dir";
$result .= "URL: " . $full_url . "\n";
- my $repos_root = $gs->ra->{repos_root};
- Git::SVN::remove_username($repos_root);
- $result .= "Repository Root: $repos_root\n";
+ eval {
+ my $repos_root = $gs->repos_root;
+ Git::SVN::remove_username($repos_root);
+ $result .= "Repository Root: $repos_root\n";
+ };
+ if ($@) {
+ $result .= "Repository Root: (offline)\n";
+ }
$result .= "Repository UUID: $uuid\n" unless $diff_status eq "A";
$result .= "Revision: " . ($diff_status eq "A" ? 0 : $rev) . "\n";
@@ -1773,9 +1778,24 @@ sub ra_uuid {
$self->{ra_uuid};
}
+sub _set_repos_root {
+ my ($self, $repos_root) = @_;
+ my $k = "svn-remote.$self->{repo_id}.reposRoot";
+ $repos_root ||= $self->ra->{repos_root};
+ tmp_config($k, $repos_root);
+ $repos_root;
+}
+
+sub repos_root {
+ my ($self) = @_;
+ my $k = "svn-remote.$self->{repo_id}.reposRoot";
+ eval { tmp_config('--get', $k) } || $self->_set_repos_root;
+}
+
sub ra {
my ($self) = shift;
my $ra = Git::SVN::Ra->new($self->{url});
+ $self->_set_repos_root($ra->{repos_root});
if ($self->use_svm_props && !$self->{svm}) {
if ($self->no_metadata) {
die "Can't have both 'noMetadata' and ",
--
Eric Wong
^ permalink raw reply related
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