* Re: [PATCH] git-cvsserver: run post-update hook *after* update.
From: Junio C Hamano @ 2009-01-29 22:48 UTC (permalink / raw)
To: Stefan Karpinski; +Cc: Andy Parkins, Michael Witten, git
In-Reply-To: <1233266282-8010-1-git-send-email-stefan.karpinski@gmail.com>
Stefan Karpinski <stefan.karpinski@gmail.com> writes:
> CVS server was running the hook before the update action was
> actually done. This performs the update before the hook is called.
>
> The original commit that introduced the current incorrect behavior
> was 394d66d "git-cvsserver runs hooks/post-update". The error in
> ordering of the hook call appears to have gone unnoticed, but since
> git-cvsserver is supposed to emulate receive-pack, it stands to
> reason that the hook should be run *after* the update. Since this
> behavior is inconsistent with recieve-pack, users are either:
>
> 1) not using post-update hooks with git-cvsserver;
> 2) using post-update hooks that don't care whether they are
> called before or after the actual update occurs;
> 3) using post-update hooks *only* with git-cvsserver, and
> relying on the hook being called just before the update.
>
> This patch would affect only users in case 3. These users are
> depending on fairly obviously wrong behavior, and moreover they can
> simply change their current post-update into post-recieve hooks,
> and their systems will work correctly again.
>
> Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>
> ---
> I'm CCing Andy Parkins, Michael Witten, and Junio Hamano, who
> authored the other three commits implementing or affecting hooks in
> git-cvsserver (394d66d, cdf6328, b2741f6). If you could please take
> a look at this patch and comment on if it's harmful or not, it
> would be much appreciated.
I think I've seen this one before and I thought it was a sensible thing to
do (and perhaps I even said so here).
Is this a resend? If so, let's queue it in at least 'next' and see if
anybody screams ;-). For a program near the fringe like cvsserver, not
many people run it but the small number of people who run it gets hurt
rather quickly if the updated behaviour breaks their existing practice,
and sometimes breaking things for them would be the only way to extract
any response. Yes, it is very unfortunate.
> git-cvsserver.perl | 4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index c1e09ea..d2e6003 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -1413,14 +1413,14 @@ sub req_ci
> close $pipe || die "bad pipe: $! $?";
> }
>
> + $updater->update();
> +
> ### Then hooks/post-update
> $hook = $ENV{GIT_DIR}.'hooks/post-update';
> if (-x $hook) {
> system($hook, "refs/heads/$state->{module}");
> }
>
> - $updater->update();
> -
> # foreach file specified on the command line ...
> foreach my $filename ( @committedfiles )
> {
> --
> 1.6.0.3.3.g08dd8
^ permalink raw reply
* Re: [PATCH] git-cvsserver: run post-update hook *after* update.
From: Andy Parkins @ 2009-01-29 22:56 UTC (permalink / raw)
To: Stefan Karpinski; +Cc: Michael Witten, Junio C Hamano, git
In-Reply-To: <1233266282-8010-1-git-send-email-stefan.karpinski@gmail.com>
On Thursday 29 January 2009 21:58:02 Stefan Karpinski wrote:
> This patch would affect only users in case 3. These users are
> depending on fairly obviously wrong behavior, and moreover they can
> simply change their current post-update into post-recieve hooks,
> and their systems will work correctly again.
Quite right.
>
> Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>
Acked-By: Andy Parkins <andyparkins@gmail.com>
--
Dr Andy Parkins
andyparkins@gmail.com
^ permalink raw reply
* Re: Something weird is happening...
From: Junio C Hamano @ 2009-01-29 22:57 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Ingo Molnar, Git Mailing List
In-Reply-To: <498231EA.3030801@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> Ingo Molnar wrote:
>>
>> So the repo itself is fine, correct?
>>
>
> Looks like it.
Correct. It looks like "fsck" without --full is misreporting, which is
something we should find a fix on our end.
I am not sure how old this bug is (nor if I can make Linus admit it is
even a bug---I suspect the code that is involved in this did not change
that much over time).
^ permalink raw reply
* Re: Something weird is happening...
From: Daniel Barkalow @ 2009-01-29 23:01 UTC (permalink / raw)
To: René Scharfe
Cc: Ingo Molnar, Junio C Hamano, H. Peter Anvin, Git Mailing List
In-Reply-To: <49822B91.6070303@lsrfire.ath.cx>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 700 bytes --]
On Thu, 29 Jan 2009, René Scharfe wrote:
> Current master (a34a9dbbc) with the following patch should not
> segfault anymore (it doesn't here), but I don't know where the
> magic number six is coming from. :-/
Looks like octopus is using multiple merge bases directly, rather than
combining them using merge-recursive. The merge-recursive concept turned
out to be superior, and we kind of left multiple merge base support
hanging. We should probably remove the old support and switch octopus to
the better way.
My guess is that 6 is just that there happen to be 3 merge bases for this
particular merge (plus the merge results and the two sides).
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: [PATCH] push: Learn to set up branch tracking with '--track'
From: Jeff King @ 2009-01-29 23:17 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <20090129223308.GB12871@coredump.intra.peff.net>
On Thu, Jan 29, 2009 at 05:33:08PM -0500, Jeff King wrote:
> So I think this patch is going about it the wrong way. Instead of
> parsing the refspec, I think you actually want to look at what we _do_
> push (or at least try to push -- probably even uptodate refs should also
> have tracking established), and use that. Then you will have wildcards
> expanded, --all handled, etc. And I suspect all you have to do is
> iterate over the result of match_refs (which we call later), which
> should be even easier (because you don't have to parse the refspecs
> yourself). But I haven't looked carefully.
Something like the patch below (which is obviously missing all of the
infrastructure for doing this optionally, but is meant to illustrate
what I'm talking about).
The downside of this is that it only works for the git protocol
transport, making dumb push even more of a second class citizen (it
looks like this is already the case for updating tracking refs). But I
think this is the right place to do it, since we have detailed
information on the matched refs. If other transports want to do the same
thing, we should abstract setup_tracking (and update_tracking_ref while
we're at it) and call them from those transports.
---
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index d65d019..23b326a 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -247,6 +247,31 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)
}
}
+static void setup_tracking(const char *remote, struct ref *ref)
+{
+ const char *name;
+ struct strbuf key = STRBUF_INIT;
+
+ if (ref->status != REF_STATUS_OK && ref->status != REF_STATUS_UPTODATE)
+ return;
+ if (!ref->peer_ref)
+ return;
+
+ name = ref->peer_ref->name;
+ if (prefixcmp(name, "refs/heads/"))
+ return;
+ name += 11;
+
+ strbuf_addf(&key, "branch.%s.remote", name);
+ git_config_set(key.buf, remote);
+
+ strbuf_reset(&key);
+ strbuf_addf(&key, "branch.%s.merge", name);
+ git_config_set(key.buf, ref->name);
+
+ strbuf_release(&key);
+}
+
static const char *prettify_ref(const struct ref *ref)
{
const char *name = ref->name;
@@ -523,6 +548,10 @@ static int do_send_pack(int in, int out, struct remote *remote, const char *dest
for (ref = remote_refs; ref; ref = ref->next)
update_tracking_ref(remote, ref);
}
+ if (/* args.track && */ !args.dry_run) {
+ for (ref = remote_refs; ref; ref = ref->next)
+ setup_tracking(remote ? remote->name : dest, ref);
+ }
if (!refs_pushed(remote_refs))
fprintf(stderr, "Everything up-to-date\n");
^ permalink raw reply related
* Re: Something weird is happening...
From: Linus Torvalds @ 2009-01-29 23:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: H. Peter Anvin, Ingo Molnar, Git Mailing List
In-Reply-To: <7vvdrxd8jz.fsf@gitster.siamese.dyndns.org>
On Thu, 29 Jan 2009, Junio C Hamano wrote:
>
> I am not sure how old this bug is (nor if I can make Linus admit it is
> even a bug---I suspect the code that is involved in this did not change
> that much over time).
I'll happily admit that it's a bug.
I think our "fsck" logic kind of sucks - the default version really skips
too damn much in the name of being fast and only look at loose objects,
and --full is _so_ anal that it's silly and overkill for almost
everything.
We probably should have a middle ground (that should be the default),
which does at least somewhat proper reachability analysis, checks
alternates, and make the current "only check local loose objects" be an
option like "git fsck --loose" (or "--local").
Out current "git fsck --full" is even mis-documented. The built-in help
says that it means "also consider alternate objects", but that's just a
small part of what it does. It _also_ verifies the pack-files, and that's
actually the really expensive part.
So "check_full" turns on _everything_, but some things that it turns on
should be turned on already by default (the "alternates" directory check),
while other parts of --full are just too expensive to ever be default (the
pack-file check).
Linus
^ permalink raw reply
* Re: Security and permissions in git
From: Jeff King @ 2009-01-29 23:23 UTC (permalink / raw)
To: Daniel Barkalow; +Cc: Jay Renbaum, git
In-Reply-To: <alpine.LNX.1.00.0901291645300.19665@iabervon.org>
On Thu, Jan 29, 2009 at 05:13:41PM -0500, Daniel Barkalow wrote:
> Every commit contains a version of the complete contents, so people have
> to be able to read the contents of all directories and write some
> directory contents. However, you can prohibit users from pushing content
> to the repository unless particular directories (or files) match what's
> there already. This is generally what people want when they have this
> requirement.
If this is what you want, see Shawn's contrib/hooks/update-paranoid,
which lets you specify ACLs in the config.
-Peff
^ permalink raw reply
* Re: [PATCH] git-cvsserver: run post-update hook *after* update.
From: Stefan Karpinski @ 2009-01-29 23:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Andy Parkins, Michael Witten, git
In-Reply-To: <7v3af1enkq.fsf@gitster.siamese.dyndns.org>
On Thu, Jan 29, 2009 at 2:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I think I've seen this one before and I thought it was a sensible thing to
> do (and perhaps I even said so here).
You said it looked sane but that I should resend CCing knowledgable parties.
> Is this a resend? If so, let's queue it in at least 'next' and see if
> anybody screams ;-). For a program near the fringe like cvsserver, not
> many people run it but the small number of people who run it gets hurt
> rather quickly if the updated behaviour breaks their existing practice,
> and sometimes breaking things for them would be the only way to extract
> any response. Yes, it is very unfortunate.
Yes, it is a resend, but I expanded on the commit message, including
my analysis of the potential impact.
^ permalink raw reply
* Re: [PATCH] git-cvsserver: handle CVS 'noop' command.
From: Stefan Karpinski @ 2009-01-29 23:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7v7i4denpg.fsf@gitster.siamese.dyndns.org>
On Thu, Jan 29, 2009 at 2:45 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Imagine what you would do when you discover another request a random other
> client sends that you would want to ignore just like you did for 'noop'.
> Viewed in this light, your patch is a very short sighted one that has a
> big negative impact on maintainability.
Fair enough. I was trying to change the minimal amount that I could
and still fix the breakage. Your patch is much better. Not to mention
terser ;-)
> A true no-brainer that has no negative impact would have been something
> like the attached patch, that adds a method that does not do anything.
>
> Even then, between req_CATCHALL and req_EMPTY, I am not sure which one is
> expected by the clients, without consulting to the protocol documentation
> for cvs server/client communication. In the attached patch, I am guessing
> from your patch that at least Tortoise does not expect any response to
> it.
I have consulted the CVS protocol documentation (found at
http://www.wandisco.com/techpubs/cvs-protocol.pdf), which states the
following about the "noop" command:
"Response expected: yes. This request is a null command in the sense
that it doesn't do anything, but
merely (as with any other requests expecting a response) sends back
any responses pertaining to
pending errors, pending Notified responses, etc."
So apparently a response *is* expected. I'm not really familiar enough
with CVS or git-cvsserver to determine what that means it should do,
but I suspect from perusing the code that req_EMPTY is the appropriate
action.
Moreover, I've moved on from using git-cvsserver myself, having
instead convinced my Windows-using compatriots to use msysgit instead.
So if you feel that this change is unwarranted, feel free to just drop
it.
^ permalink raw reply
* Re: [PATCH] git-cvsserver: handle CVS 'noop' command.
From: Junio C Hamano @ 2009-01-29 23:46 UTC (permalink / raw)
To: Stefan Karpinski; +Cc: git
In-Reply-To: <d4bc1a2a0901291539m636f0fc8s5d9280ce9b7d22b2@mail.gmail.com>
Stefan Karpinski <stefan.karpinski@gmail.com> writes:
> So apparently a response *is* expected. I'm not really familiar enough
> with CVS or git-cvsserver to determine what that means it should do,
> but I suspect from perusing the code that req_EMPTY is the appropriate
> action.
>
> Moreover, I've moved on from using git-cvsserver myself, having
> instead convinced my Windows-using compatriots to use msysgit instead.
> So if you feel that this change is unwarranted, feel free to just drop
> it.
Because the issue currently has our attention, and we think we know that
the code does not do the right thing currently, and that we are fairly
sure that the right thing is to do req_EMPTY, I'd rather see a tested fix
applied so that we can forget about it ;-)
It's good that you moved your people to native git environment, but if you
have an environment where you can test the fix still lying around, I'd
appreciate a quick test and resubmit.
^ permalink raw reply
* question re dcommit problem
From: David Mann @ 2009-01-30 0:14 UTC (permalink / raw)
To: git
I created a git repository from my svn repository, later uploaded it to
GitHub, and after some branching and merging, can no longer dcommit back
to the svn repository. I notice that somewhere along the way the
git-svn-id: messages no longer appear on my master branch. When I try
to git svn dcommit I get weird messages like this:
mannd@SuperSluggo:~/dev/git/epsimulator> git svn dcommit
Committing to
https://epsimulator.svn.sourceforge.net/svnroot/epsimulator/epsimulator/trunk
...
Filesystem has no item: File not found: transaction '435-24', path
'/epsimulator/trunk/epsimulator.kdevses' at
/home/mannd/libexec/git-core/git-svn line 480
Any way to fix this? If not, I will just abandon SVN and stick with
git, which I really enjoy using.
Thanks,
David Mann
^ permalink raw reply
* Re: [PATCH] push: Learn to set up branch tracking with '--track'
From: Johannes Schindelin @ 2009-01-30 0:28 UTC (permalink / raw)
To: Jeff King; +Cc: git, gitster
In-Reply-To: <20090129231715.GA17399@coredump.intra.peff.net>
Hi,
On Thu, 29 Jan 2009, Jeff King wrote:
> On Thu, Jan 29, 2009 at 05:33:08PM -0500, Jeff King wrote:
>
> > So I think this patch is going about it the wrong way. Instead of
> > parsing the refspec, I think you actually want to look at what we _do_
> > push (or at least try to push -- probably even uptodate refs should also
> > have tracking established), and use that. Then you will have wildcards
> > expanded, --all handled, etc. And I suspect all you have to do is
> > iterate over the result of match_refs (which we call later), which
> > should be even easier (because you don't have to parse the refspecs
> > yourself). But I haven't looked carefully.
>
> Something like the patch below (which is obviously missing all of the
> infrastructure for doing this optionally, but is meant to illustrate
> what I'm talking about).
Except that you miss http:// and rsync:// protocols. Those were the
reasons I did not touch send-pack.
Ciao,
Dscho
^ permalink raw reply
* [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Johannes Schindelin @ 2009-01-30 0:34 UTC (permalink / raw)
To: git; +Cc: gitster
In-Reply-To: <cover.1233275583u.git.johannes.schindelin@gmx.de>
Many, many users set up non-bare repositories on their server, and are
confused that the working directory is not updated.
The reason may be that they did not read the manual, or that they found
"helpful" walk-throughs via Google, or that they did not understand the
concepts behind Git.
Or, the reason could be that we made a design mistake, and that the
number of puzzled new users should tell us something.
Granted, we wanted to have a longer grace period for old-timers, but
let's face it:
- old-timers will have to edit their configs at some stage anyway,
- for old-timers, it will be a matter of less than a minute,
- new-timers will not spend frustrated hours, and
- since there are many more new-timers now than old-timers, we should
cater for them anyway.
To make it easier for old-timers, the error message was enhanced to
suggest how to allow updating the current branch easily.
For the tests relying on the old behavior, receive.denyCurrentBranch
was set to false, to avoid breaking them.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Let's be honest here, I have not much respect for users who fail
to read up enough to understand what they are doing.
But hearing from those users constantly is really unnerving. And
it would be a one-time cost to old-timers.
builtin-receive-pack.c | 9 ++++++---
t/t5400-send-pack.sh | 5 ++++-
t/t5401-update-hooks.sh | 1 +
t/t5405-send-pack-rewind.sh | 1 +
t/t5516-fetch-push.sh | 1 +
t/t5517-push-mirror.sh | 3 ++-
t/t5521-pull-symlink.sh | 1 +
t/t5701-clone-local.sh | 2 +-
8 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 6564a97..e9510fc 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -19,7 +19,7 @@ enum deny_action {
static int deny_deletes = 0;
static int deny_non_fast_forwards = 0;
-static enum deny_action deny_current_branch = DENY_WARN;
+static enum deny_action deny_current_branch = DENY_REFUSE;
static int receive_fsck_objects;
static int receive_unpack_limit = -1;
static int transfer_unpack_limit = -1;
@@ -239,9 +239,12 @@ static const char *update(struct command *cmd)
" that are now in HEAD.");
break;
case DENY_REFUSE:
- if (!is_ref_checked_out(name))
+ if (is_bare_repository() || !is_ref_checked_out(name))
break;
- error("refusing to update checked out branch: %s", name);
+ error("refusing to update checked out branch: %s\n"
+ "if you know what you are doing, you can allow it by "
+ "setting\n\n"
+ "\tgit config receive.denyCurrentBranch true\n", name);
return "branch is currently checked out";
}
diff --git a/t/t5400-send-pack.sh b/t/t5400-send-pack.sh
index b21317d..240380c 100755
--- a/t/t5400-send-pack.sh
+++ b/t/t5400-send-pack.sh
@@ -33,6 +33,7 @@ test_expect_success setup '
git update-ref HEAD "$commit" &&
git clone ./. victim &&
cd victim &&
+ git config receive.denyCurrentBranch false &&
git log &&
cd .. &&
git update-ref HEAD "$zero" &&
@@ -131,13 +132,15 @@ test_expect_success \
cd .. &&
git clone parent child && cd child && git push --all &&
cd ../parent &&
- git branch -a >branches && ! grep origin/master branches
+ git branch -a >branches && ! grep origin/master branches &&
+ cd ..
'
rewound_push_setup() {
rm -rf parent child &&
mkdir parent && cd parent &&
git init && echo one >file && git add file && git commit -m one &&
+ git config receive.denyCurrentBranch false &&
echo two >file && git commit -a -m two &&
cd .. &&
git clone parent child && cd child && git reset --hard HEAD^
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 64f66c9..7f04b64 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -18,6 +18,7 @@ test_expect_success setup '
git update-ref refs/heads/master $commit0 &&
git update-ref refs/heads/tofail $commit1 &&
git clone ./. victim &&
+ GIT_DIR=victim/.git git config receive.denyCurrentBranch false &&
GIT_DIR=victim/.git git update-ref refs/heads/tofail $commit1 &&
git update-ref refs/heads/master $commit1 &&
git update-ref refs/heads/tofail $commit0
diff --git a/t/t5405-send-pack-rewind.sh b/t/t5405-send-pack-rewind.sh
index cb9aacc..37c1f23 100755
--- a/t/t5405-send-pack-rewind.sh
+++ b/t/t5405-send-pack-rewind.sh
@@ -6,6 +6,7 @@ test_description='forced push to replace commit we do not have'
test_expect_success setup '
+ git config receive.denyCurrentBranch false &&
>file1 && git add file1 && test_tick &&
git commit -m Initial &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 4426df9..9ca2730 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -12,6 +12,7 @@ mk_empty () {
(
cd testrepo &&
git init &&
+ git config receive.denyCurrentBranch false &&
mv .git/hooks .git/hooks-disabled
)
}
diff --git a/t/t5517-push-mirror.sh b/t/t5517-push-mirror.sh
index ea49ded..0a31a4e 100755
--- a/t/t5517-push-mirror.sh
+++ b/t/t5517-push-mirror.sh
@@ -19,7 +19,8 @@ mk_repo_pair () {
mkdir mirror &&
(
cd mirror &&
- git init
+ git init &&
+ git config receive.denyCurrentBranch false
) &&
mkdir master &&
(
diff --git a/t/t5521-pull-symlink.sh b/t/t5521-pull-symlink.sh
index 5672b51..736c24e 100755
--- a/t/t5521-pull-symlink.sh
+++ b/t/t5521-pull-symlink.sh
@@ -14,6 +14,7 @@ test_description='pulling from symlinked subdir'
#
# The working directory is subdir-link.
+git config receive.denyCurrentBranch false
mkdir subdir
echo file >subdir/file
git add subdir/file
diff --git a/t/t5701-clone-local.sh b/t/t5701-clone-local.sh
index 3559d17..06b2f13 100755
--- a/t/t5701-clone-local.sh
+++ b/t/t5701-clone-local.sh
@@ -119,7 +119,7 @@ test_expect_success 'bundle clone with nonexistent HEAD' '
test_expect_success 'clone empty repository' '
cd "$D" &&
mkdir empty &&
- (cd empty && git init) &&
+ (cd empty && git init && git config receive.denyCurrentBranch false) &&
git clone empty empty-clone &&
test_tick &&
(cd empty-clone
--
1.6.1.2.531.g6f52
^ permalink raw reply related
* [PATCH] git-cvsserver: handle CVS 'noop' command.
From: Stefan Karpinski @ 2009-01-30 1:12 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Stefan Karpinski
In-Reply-To: <7vhc3hd6ba.fsf@gitster.siamese.dyndns.org>
The CVS protocol documentation, found at
http://www.wandisco.com/techpubs/cvs-protocol.pdf
states the following about the 'noop' command:
Response expected: yes. This request is a null command
in the sense that it doesn't do anything, but merely
(as with any other requests expecting a response) sends
back any responses pertaining to pending errors, pending
Notified responses, etc.
In accordance with this, the correct way to handle the 'noop'
command, when issued by a client, is to call req_EMPTY.
The 'noop' command is called by some CVS clients, notably
TortoiseCVS, thus making it desirable for git-cvsserver to
respond to the command rather than choking on it as unknown.
Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>
---
On Thu, Jan 29, 2009 at 3:46 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> Because the issue currently has our attention, and we think we know that
> the code does not do the right thing currently, and that we are fairly
> sure that the right thing is to do req_EMPTY, I'd rather see a tested fix
> applied so that we can forget about it ;-)
>
> It's good that you moved your people to native git environment, but if you
> have an environment where you can test the fix still lying around, I'd
> appreciate a quick test and resubmit.
I've done the best testing I could do under the circumstances. What
that means is that the only windows machine I have access to test
this on right now is running Vista, which is only partially (read
poorly) supported by TortoiseCVS. So things seem to work well enough,
but TortoiseCVS keeps crapping out for Vista-related reasons rather
than git-cvsserver-related reasons. But I did manage to coax it into
successfully checking out a complete working repository without the
"noop" errors that it used to give.
git-cvsserver.perl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index fef7faf..277ee4e 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -76,6 +76,7 @@ my $methods = {
'history' => \&req_CATCHALL,
'watchers' => \&req_EMPTY,
'editors' => \&req_EMPTY,
+ 'noop' => \&req_EMPTY,
'annotate' => \&req_annotate,
'Global_option' => \&req_Globaloption,
#'annotate' => \&req_CATCHALL,
--
1.6.0.3.3.g08dd8
^ permalink raw reply related
* Status of my valgrind branch
From: Johannes Schindelin @ 2009-01-30 1:19 UTC (permalink / raw)
To: git
Hi,
the valgrind branch is not ready yet. And it grew to 8 patches in the
meantime:
http://repo.or.cz/w/git/dscho.git?a=log;h=c9486d38d070fefe39f90512e2e12d3a64eba1b3
There is one big problem I only realized today, when trying to catch that
valgrind bug [*1*]: in the suppression file, I introduced the "..."
notation, which is only available with valgrind 3.4.0. Which was
published this month.
So I have to think about the issue a bit more, and maybe just claim that
valgrind 3.4.0 is required. But maybe there are people who disagree with
me saying that older valgrind should be supported.
Ciao,
Dscho
[*1*] Turns out that the issue I was reporting happens with Ubuntu's zlib,
which I erroneously thought would be pretty vanilla. It is not: it was
compiled with "-O3 -DUNALIGNED_OK", both of which are required to hit the
bug.
Still, it is a bug in valgrind, I am pretty sure by now, as the byte is
not uninitialized as valgrind claims.
Oh, and I wasted a _lot_ of time on this today.
^ permalink raw reply
* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Jay Soffian @ 2009-01-30 1:28 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de>
On Thu, Jan 29, 2009 at 7:34 PM, Johannes Schindelin
<johannes.schindelin@gmx.de> wrote:
> Or, the reason could be that we made a design mistake, and that the
> number of puzzled new users should tell us something.
I happen to have spent some time looking at Mercurial the other day
since I was curious how it's evolved since I last played with it, and
so w/that perspective, I think that git did make a small design
mistake. With mercurial, pull and push are symmetric opposites.
Neither, by default, updates the working copy.
(Confusingly for users of both mercurial and git, the mercurial
equivalent of "git pull" is "hg fetch". Doh.)
Anyway, I think that this may be what leads to confusion. git pull
updates the working copy, and beginners I think expect that push,
which sounds like the opposite of pull, ought do the same thing.
j.
^ permalink raw reply
* Re: [PATCH] git-cvsserver: handle CVS 'noop' command.
From: Martin Langhoff @ 2009-01-30 1:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Karpinski, git
In-Reply-To: <7v7i4denpg.fsf@gitster.siamese.dyndns.org>
On Fri, Jan 30, 2009 at 11:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Not a no-brainer at all, sorry.
+1 on Junio's notes and patch.
Can someone with a real TortoiseCVS and a real cvs server sniff the
connection and catch the noop? (Can TortoiseCVS write debug logs of
the conversation with the server?)
Hysterical note: the original implementation of cvsserver was done
reading the output of `cvs -t $opts $cmd`, and ocassionally sniffing
the traffic on the wire or ssh connection.
Probably not a major issue for 'noop' though :-)
cheers,
m
--
martin.langhoff@gmail.com
martin@laptop.org -- School Server Architect
- ask interesting questions
- don't get distracted with shiny stuff - working code first
- http://wiki.laptop.org/go/User:Martinlanghoff
^ permalink raw reply
* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Asheesh Laroia @ 2009-01-30 1:32 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de>
On Fri, 30 Jan 2009, Johannes Schindelin wrote:
> case DENY_REFUSE:
> - if (!is_ref_checked_out(name))
> + if (is_bare_repository() || !is_ref_checked_out(name))
> break;
> - error("refusing to update checked out branch: %s", name);
> + error("refusing to update checked out branch: %s\n"
> + "if you know what you are doing, you can allow it by "
> + "setting\n\n"
> + "\tgit config receive.denyCurrentBranch true\n", name);
It seems like those new users you're trying to protect could use an
additional sentence, like:
"A bare repository would not have this issue."
or
"You may prefer to have a bare repository instead."
Being told how to do it right is even better than being told that you're
doing it wrong. (-:
-- Asheesh.
--
Fame is a vapor; popularity an accident; the only earthly certainty is
oblivion.
-- Mark Twain
^ permalink raw reply
* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Junio C Hamano @ 2009-01-30 2:18 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <7v4ozhd1wp.fsf@gitster.siamese.dyndns.org>
I do not think this improves anything.
@@ -239,9 +239,12 @@ static const char *update(struct command *cmd)
" that are now in HEAD.");
break;
case DENY_REFUSE:
- if (!is_ref_checked_out(name))
+ if (is_bare_repository() || !is_ref_checked_out(name))
break;
- error("refusing to update checked out branch: %s", name);
+ error("refusing to update checked out branch: %s\n"
+ "if you know what you are doing, you can allow it by "
+ "setting\n\n"
+ "\tgit config receive.denyCurrentBranch true\n", name);
return "branch is currently checked out";
}
As the message I am currently getting from such a push is:
$ git push ../victim-010 next:master
Total 0 (delta 0), reused 0 (delta 0)
warning: updating the currently checked out branch; this may cause confusion,
as the index and working tree do not reflect changes that are now in HEAD.
To ../victim-010
a34a9db..d79e69c next -> master
which is much better than what you did. It at least tries to explain why
it is warning, even though I think it has a huge room for improvement.
Saying "If you know what you are doing" never works in practice. It can
serve as an excuse for you to later say, "See, I told you so", but that is
the only usefulness of the expression, and everybody, especially the most
clueless people, *think* they know what they are doing.
You alluded that we wanted to make grace period much longer, but you want
to cut it short. I think it is a huge mistake. The warning has only been
there for the last two months, and only can be seen from v1.6.1-rc1 or
newer software. These new people even haven't a chance to learn from the
existing warning.
I think what would work much better would be a patch that keeps the
warn-but-allow as the default, but clarifies the warning message. Say
these things in separate paragraphs, perhaps in red blinking letters:
(1) what symptoms, that are easily observable by the most novice users,
are caused by "index and work tree going out of sync" the warning
talks about, and why that would not be what they want;
(2) if the user did not mean to do it (and the user can tell by observing
the symptom described in the previous point), describe what needs to
be done to recover from the fallout this push has caused (we do not
need a recipe; pointing at a URL or manpage is fine), and what switch
to flip to prevent herself from doing it again in the future;
(3) if the user did mean it, and finds the above two big warning
annoying, what switch to flip to squelch the warning for future
pushes.
The goal of the warning should be to *force* people *choose*, either to
silently-allow (aka DENY_IGNORE) or refuse (DENY_REFUSE), and give enough
information for them to make an informed decision. We can afford to be
annoyingly long, loud and verbose there.
On the other hand, you cannot make the message for DENY_REFUSE annoyingly
long, as people may have already chosen to say "please refuse my push into
a live branch".
If you are making "refuse" the default, an annoyingly long message is even
worse. "Yeah, thanks for stopping me, but you do not have to remind me
every time that I made a mistake in large red letters. I perfectly well
know what I am doing, I perfectly well know that I did not want to push
into that branch, I just made a mistake---you do not have to be so loud".
I suspect that you cannot even be long enough to be informative, not to
annoy people.
^ permalink raw reply
* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Miklos Vajna @ 2009-01-30 2:30 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de>
[-- Attachment #1: Type: text/plain, Size: 445 bytes --]
On Fri, Jan 30, 2009 at 01:34:28AM +0100, Johannes Schindelin <johannes.schindelin@gmx.de> wrote:
> - error("refusing to update checked out branch: %s", name);
> + error("refusing to update checked out branch: %s\n"
> + "if you know what you are doing, you can allow it by "
> + "setting\n\n"
> + "\tgit config receive.denyCurrentBranch true\n", name);
Shouldn't this be
git config receive.denyCurrentBranch ignore
instead of "true"?
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Switch receive.denyCurrentBranch to "refuse"
From: Jeff King @ 2009-01-30 2:55 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901300133070.3586@pacific.mpi-cbg.de>
On Fri, Jan 30, 2009 at 01:34:28AM +0100, Johannes Schindelin wrote:
> Let's be honest here, I have not much respect for users who fail
> to read up enough to understand what they are doing.
>
> But hearing from those users constantly is really unnerving. And
> it would be a one-time cost to old-timers.
I am not personally opposed to changing this default. I seem to
recall some opposition when this was brought up initially, but I don't
recall any specific reason besides "change is bad". Maybe those who
oppose want to summarize their arguments here.
I was hoping that introducing the warning would cause new users to "get
it". But since this warning was put in place, I think we have still
gotten a few questions on the list about this. I don't know if it simply
because they are on older versions, or if the warning is insufficient.
If the former, then perhaps that argues for leaving it a little longer.
> case DENY_REFUSE:
> - if (!is_ref_checked_out(name))
> + if (is_bare_repository() || !is_ref_checked_out(name))
Now what is this change about?
> --- a/t/t5701-clone-local.sh
> +++ b/t/t5701-clone-local.sh
> @@ -119,7 +119,7 @@ test_expect_success 'bundle clone with nonexistent HEAD' '
> test_expect_success 'clone empty repository' '
> cd "$D" &&
> mkdir empty &&
> - (cd empty && git init) &&
> + (cd empty && git init && git config receive.denyCurrentBranch false) &&
> git clone empty empty-clone &&
> test_tick &&
> (cd empty-clone
Perhaps some of these tests would do better to actually just use a bare
repository. That would better match the expected workflow for cloning
empty, anyway.
-Peff
^ permalink raw reply
* [PATCH] Add --ff-only flag to git-merge (take 2)
From: Yuval Kogman @ 2009-01-30 4:32 UTC (permalink / raw)
To: git
In-Reply-To: <7vbptpeoge.fsf@gitster.siamese.dyndns.org>
Hi,
This attempt fixes the various issues raised, except for the case of octopus
ending in 1 or more fast forwards. I'm not sure what the best way to fix this
is. I see three obvious options, none of which seem optimal:
1. git-merge-octopus will take a --ff-only flag as well, adding a branch for that
2. ff-only could be a merge strategy, and octopus is forbidden in
--ff-only, while this new strategy will behave just like octopus when
the merge can be fast forward only.
3. the logic from git-merge-octopus could be duplicated into builtin-merge,
to handle only the fast forward case.
Please advise.
Regards,
Yuval
^ permalink raw reply
* [PATCH] Add --ff-only flag to git-merge
From: Yuval Kogman @ 2009-01-30 4:32 UTC (permalink / raw)
To: git; +Cc: Yuval Kogman
In-Reply-To: <1233289965-19538-1-git-send-email-nothingmuch@woobling.org>
When specified git-merge will exit with an error unless the merge is
resolved as a fast-forward.
This is similar to the default behavior of git push.
---
Documentation/merge-options.txt | 4 +++
builtin-merge.c | 18 +++++++++++++++
git-pull.sh | 4 ++-
t/t7600-merge.sh | 45 +++++++++++++++++++++++++++++++++++++++
4 files changed, 70 insertions(+), 1 deletions(-)
diff --git a/Documentation/merge-options.txt b/Documentation/merge-options.txt
index 637b53f..ac742f8 100644
--- a/Documentation/merge-options.txt
+++ b/Documentation/merge-options.txt
@@ -59,6 +59,10 @@
a fast-forward, only update the branch pointer. This is
the default behavior of git-merge.
+--ff-only::
+ Refuse to merge unless the merge is resolved as a
+ fast-forward.
+
-s <strategy>::
--strategy=<strategy>::
Use the given merge strategy; can be supplied more than
diff --git a/builtin-merge.c b/builtin-merge.c
index e4555b0..dae53fe 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -44,6 +44,7 @@ static const char * const builtin_merge_usage[] = {
static int show_diffstat = 1, option_log, squash;
static int option_commit = 1, allow_fast_forward = 1;
static int allow_trivial = 1, have_message;
+static int only_fast_forward;
static struct strbuf merge_msg;
static struct commit_list *remoteheads;
static unsigned char head[20], stash[20];
@@ -167,6 +168,8 @@ static struct option builtin_merge_options[] = {
"perform a commit if the merge succeeds (default)"),
OPT_BOOLEAN(0, "ff", &allow_fast_forward,
"allow fast forward (default)"),
+ OPT_BOOLEAN(0, "ff-only", &only_fast_forward,
+ "allow only fast forward"),
OPT_CALLBACK('s', "strategy", &use_strategies, "strategy",
"merge strategy to use", option_parse_strategy),
OPT_CALLBACK('m', "message", &merge_msg, "message",
@@ -858,6 +861,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
option_commit = 0;
}
+ if (only_fast_forward && !allow_fast_forward)
+ die("You cannot combine --ff-only with --no-ff.");
+
if (!argc)
usage_with_options(builtin_merge_usage,
builtin_merge_options);
@@ -959,6 +965,11 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
allow_trivial = 0;
}
+ if (only_fast_forward && !allow_fast_forward)
+ die("You cannot combine --ff-only with the selected"
+ " merge strategies.");
+
+
if (!remoteheads->next)
common = get_merge_bases(lookup_commit(head),
remoteheads->item, 1);
@@ -1023,6 +1034,10 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* We are not doing octopus, not fast forward, and have
* only one common.
*/
+
+ if (only_fast_forward)
+ die("Merge is non fast forward, aborting.");
+
refresh_cache(REFRESH_QUIET);
if (allow_trivial) {
/* See if it is really trivial. */
@@ -1063,6 +1078,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
}
+ if (only_fast_forward)
+ die("Merge is non fast forward, aborting.");
+
/* We are going to make a new commit. */
git_committer_info(IDENT_ERROR_ON_NO_NAME);
diff --git a/git-pull.sh b/git-pull.sh
index 2c7f432..242d36f 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -45,6 +45,8 @@ do
no_ff=--ff ;;
--no-ff)
no_ff=--no-ff ;;
+ --ff-only)
+ ff-only=--ff-only ;;
-s=*|--s=*|--st=*|--str=*|--stra=*|--strat=*|--strate=*|\
--strateg=*|--strategy=*|\
-s|--s|--st|--str|--stra|--strat|--strate|--strateg|--strategy)
@@ -185,5 +187,5 @@ merge_name=$(git fmt-merge-msg $log_arg <"$GIT_DIR/FETCH_HEAD") || exit
test true = "$rebase" &&
exec git-rebase $strategy_args --onto $merge_head \
${oldremoteref:-$merge_head}
-exec git-merge $no_stat $no_commit $squash $no_ff $log_arg $strategy_args \
+exec git-merge $no_stat $no_commit $squash $no_ff $ff_only $log_arg $strategy_args \
"$merge_name" HEAD $merge_head $verbosity
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index e5b210b..6c2febc 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -234,6 +234,8 @@ test_expect_success 'reject non-strategy with a git-merge-foo name' '
test_must_fail git merge -s index c1
'
+test_debug 'gitk --all'
+
test_expect_success 'merge c0 with c1' '
git reset --hard c0 &&
git merge c1 &&
@@ -243,6 +245,15 @@ test_expect_success 'merge c0 with c1' '
test_debug 'gitk --all'
+test_expect_success 'merge c0 with c1 (fast forward only)' '
+ git reset --hard c0 &&
+ git merge --ff-only c1 &&
+ verify_merge file result.1 &&
+ verify_head "$c1"
+'
+
+test_debug 'gitk --all'
+
test_expect_success 'merge c1 with c2' '
git reset --hard c1 &&
test_tick &&
@@ -253,6 +264,14 @@ test_expect_success 'merge c1 with c2' '
test_debug 'gitk --all'
+test_expect_success 'merge c1 with c2' '
+ git reset --hard c1 &&
+ test_tick &&
+ test_must_fail git merge --ff-only c2
+'
+
+test_debug 'gitk --all'
+
test_expect_success 'merge c1 with c2 and c3' '
git reset --hard c1 &&
test_tick &&
@@ -263,6 +282,14 @@ test_expect_success 'merge c1 with c2 and c3' '
test_debug 'gitk --all'
+test_expect_success 'merge c1 with c2 and c3 (fast forward only' '
+ git reset --hard c1 &&
+ test_tick &&
+ test_must_fail git merge --ff-only c2 c3
+'
+
+test_debug 'gitk --all'
+
test_expect_success 'merge c0 with c1 (no-commit)' '
git reset --hard c0 &&
git merge --no-commit c1 &&
@@ -470,6 +497,15 @@ test_expect_success 'merge c1 with c0, c2, c0, and c1' '
test_debug 'gitk --all'
+test_expect_success 'merge fast forward only' '
+ git reset --hard c1 &&
+ git config branch.master.mergeoptions "" &&
+ test_tick &&
+ test_must_fail git merge --ff-only c0 c2 c0 c1
+'
+
+test_debug 'gitk --all'
+
test_expect_success 'merge c1 with c0, c2, c0, and c1' '
git reset --hard c1 &&
git config branch.master.mergeoptions "" &&
@@ -481,6 +517,15 @@ test_expect_success 'merge c1 with c0, c2, c0, and c1' '
test_debug 'gitk --all'
+test_expect_success 'merge fast forward only' '
+ git reset --hard c1 &&
+ git config branch.master.mergeoptions "" &&
+ test_tick &&
+ test_must_fail git merge --ff-only c1 c2
+'
+
+test_debug 'gitk --all'
+
test_expect_success 'merge c1 with c1 and c2' '
git reset --hard c1 &&
git config branch.master.mergeoptions "" &&
--
1.6.1
^ permalink raw reply related
* Re: "malloc failed"
From: Jeff King @ 2009-01-30 4:49 UTC (permalink / raw)
To: David Abrahams; +Cc: Junio C Hamano, git
In-Reply-To: <87pri6qmvm.fsf@mcbain.luannocracy.com>
On Thu, Jan 29, 2009 at 08:10:05AM -0500, David Abrahams wrote:
> Good work. I don't know if this matters to you, but I think on a 32-bit
> platform you'll find that size_t, which is supposed to be able to hold
> the size of the largest representable *memory block*, is only 4 bytes
> large:
That should be fine; 32-bit systems can't deal with such large files
anyway, since we want to address the whole thing. Getting around that
would, as Andreas mentioned, involve dealing with large files in chunks,
something that would make the code a lot more complex.
So I think the answer is "tough, if you want files >4G get a 64-bit
machine". Which is unreasonable for a file system to say, but I think is
fine for git.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Jeff King @ 2009-01-30 4:51 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Pieter de Bie, Junio C Hamano, git
In-Reply-To: <bd6139dc0901290420x1216a399w656e4d1622178a06@mail.gmail.com>
On Thu, Jan 29, 2009 at 01:20:20PM +0100, Sverre Rabbelier wrote:
> > It wouldn't help the case of "somebody
> > else pushed some content that you want to pull", but like you said, I
> > think the primary workflow is that you immediately push after cloning
> > the empty repo.
>
> Also, the only way to support the "somebody else pushed already"
> workflow would be to assume the user wants to name the branch
> 'master', which might not be the case at all.
You could make a guess that they will use "master", and if you are
wrong, it behaves as now. But if you are right, "git pull" pulls down
master automatically.
But that is getting a little confusing. So let's push this "git push
--track" idea to completion and see how people like it.
-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