* [PATCH] git-cvsserver: run post-update hook *after* update.
From: Stefan Karpinski @ 2009-01-29 21:58 UTC (permalink / raw)
To: Andy Parkins, Michael Witten, Junio C Hamano, git; +Cc: Stefan Karpinski
In-Reply-To: <7viqo61mfq.fsf@gitster.siamese.dyndns.org>
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.
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 related
* Re: Security and permissions in git
From: Daniel Barkalow @ 2009-01-29 22:13 UTC (permalink / raw)
To: Jay Renbaum; +Cc: git
In-Reply-To: <loom.20090129T203050-865@post.gmane.org>
On Thu, 29 Jan 2009, Jay Renbaum wrote:
> When setting up a public repository is there a way to control who has access
> to various directories within the repository or is everything equal once you
> are in?
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.
It's also possible to use submodules to have a project contain parts that
some users don't have any access to, while still being able to produce
commits to the main project (in combination with the above check so that
try can't replace the submodule entirely with content they do have access
to, or submodule commits they've seen referenced previously)
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: Something weird is happening...
From: H. Peter Anvin @ 2009-01-29 22:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Ingo Molnar
In-Reply-To: <7vfxj1eqh6.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
>
> Is the blob borrowed from one of its alternate object store, and not in a
> pack? That would explain (note that I am not saying "justify" here, just
> "explain") the symptom.
>
Yes, it is.
> But it still does the connectivity check fully from your refs. And if it
> finds an object that ought to be reachable from your refs that it hasn't
> validated, it gives the "missing" warning above. There is a check to
> exclude objects in packs when running "git fsck" without --full from this
> warning but I do not see a corresponding exclusion of objects that were
> borrowed from your neighbour which we didn't check either.
>
> But that is just a conjecture from a cursory looking at the current code.
> I do not remember (or did not know from the beginning) some details of it.
> And that is why I asked you if "git fsck --full" reports it missing in my
> earlier response to you.
Just tried it, it doesn't. So I guess things are OK.
-hpa
^ permalink raw reply
* Re: Something weird is happening...
From: René Scharfe @ 2009-01-29 22:20 UTC (permalink / raw)
To: Ingo Molnar
Cc: Junio C Hamano, H. Peter Anvin, Git Mailing List, Daniel Barkalow
In-Reply-To: <20090129120539.GA26975@elte.hu>
Ingo Molnar schrieb:
> * Ingo Molnar <mingo@elte.hu> wrote:
>
>> a simple 'git fsck' still reports a ton of dangling and missing objects.
>> (see it below)
>>
>> amongst them is the object above:
>>
>> missing blob af0e01d4c663a101f48614e40d006ed6272d5c36
>>
>> but this does not seem to cause problems with git version 1.6.0.6 that
>> i'm using now.
>
> hm - maybe it's unrelated, but yesterday, a few hours before the incident
> i did a higher-order Octopus merge with 1.6.1, that segfaulted.
>
> I didnt think much of it - git often crashes with our crazy -tip tree when
> i get above the magic 20 branches limit. The crash left a .git/index.lock
> file around which i removed - and then forgot about the incident. I wanted
> to report those crashes before but procrastinated it.
>
> I used git/maint snapshots because new versions of Git are much better at
> doing Octopus merges.
>
> I've written a reproducer for git-1.6.1.1-1.fc11.i386. See the crash log
> below.
>
> To reproduce, pick up the -tip tree as a remote:
>
> http://people.redhat.com/mingo/tip.git/README
>
> (you can run the README file as a script)
>
> Then do this:
>
> git checkout -b tmp.crash v2.6.29-rc3
>
> git merge tip/x86/asm tip/x86/cleanups tip/x86/cpudetect \
> tip/x86/debug tip/x86/doc tip/x86/header-fixes \
> tip/x86/mm tip/x86/paravirt tip/x86/pat \
> tip/x86/setup-v2 tip/x86/subarch tip/x86/uaccess \
> tip/x86/urgent tip/core/percpu
>
> and you should see the segfault.
>
> interestingly i did not reproduce with the sha1's hardcoded:
>
> git checkout -b tmp.crash 18e352e4a73465349711a9324767e1b2453383e2
>
> git merge 2d4d57db692ea790e185656516e6ebe8791f1788 a448720ca3248e8a7a426336885549d6e923fd8e b38b0665905538e76e26f2a4c686179abb1f69f6 d5e397cb49b53381e4c99a064ca733c665646de8 e56d0cfe7790fd3218ae4f6aae1335547fea8763 dbca1df48e89d8aa59254fdc10ef16c16e73d94e fb746d0e1365b7472ccc4c3d5b0672b34a092d0b 6522869c34664dd5f05a0a327e93915b1281c90d d639bab8da86d330493487e8c0fea8ca31f53427 042cbaf88ab48e11afb725541e3c2cbf5b483680 5662a2f8e7313f78d6b17ab383f3e4f04971c335 3b4b75700a245d0d48fc52a4d2f67d3155812aba bf3647c44bc76c43c4b2ebb4c37a559e899ac70e 4369f1fb7cd4cf777312f43e1cb9aa5504fc4125
>
> Ingo
>
> -------------------->
> earth4:~/tip> git merge x86/asm x86/cleanups x86/cpudetect x86/debug
> x86/doc x86/header-fixes x86/mm x86/paravirt x86/pat x86/setup-v2
> x86/subarch x86/uaccess x86/urgent core/percpu
> Trying simple merge with 2d4d57db692ea790e185656516e6ebe8791f1788
> Trying simple merge with a448720ca3248e8a7a426336885549d6e923fd8e
> Simple merge did not work, trying automatic merge.
> Auto-merging arch/x86/include/asm/io.h
> Auto-merging arch/x86/include/asm/spinlock.h
> Auto-merging arch/x86/kernel/mpparse.c
> Auto-merging arch/x86/kernel/setup_percpu.c
> Auto-merging arch/x86/mm/init_32.c
> Trying simple merge with b38b0665905538e76e26f2a4c686179abb1f69f6
> Simple merge did not work, trying automatic merge.
> Auto-merging arch/x86/kernel/cpu/common.c
> Auto-merging arch/x86/kernel/cpu/intel.c
> Auto-merging arch/x86/mm/pat.c
> Trying simple merge with d5e397cb49b53381e4c99a064ca733c665646de8
> Trying simple merge with e56d0cfe7790fd3218ae4f6aae1335547fea8763
> Trying simple merge with dbca1df48e89d8aa59254fdc10ef16c16e73d94e
> Trying simple merge with fb746d0e1365b7472ccc4c3d5b0672b34a092d0b
> Trying simple merge with 6522869c34664dd5f05a0a327e93915b1281c90d
> Simple merge did not work, trying automatic merge.
> Auto-merging arch/x86/include/asm/paravirt.h
> Trying simple merge with d639bab8da86d330493487e8c0fea8ca31f53427
> Simple merge did not work, trying automatic merge.
> Auto-merging arch/x86/include/asm/io.h
> Auto-merging arch/x86/mm/ioremap.c
> Trying simple merge with 042cbaf88ab48e11afb725541e3c2cbf5b483680
> Trying simple merge with 5662a2f8e7313f78d6b17ab383f3e4f04971c335
> Trying simple merge with 3b4b75700a245d0d48fc52a4d2f67d3155812aba
> Simple merge did not work, trying automatic merge.
> Auto-merging arch/x86/kernel/signal.c
> Trying simple merge with bf3647c44bc76c43c4b2ebb4c37a559e899ac70e
> Simple merge did not work, trying automatic merge.
> Auto-merging arch/x86/kernel/cpu/intel.c
> Trying simple merge with 4369f1fb7cd4cf777312f43e1cb9aa5504fc4125
> /usr/libexec/git-core/git-merge-octopus: line 52: 26758 Segmentation fault
> git read-tree -u -m --aggressive $common $MRT $SHA1
> Merge with strategy octopus failed.
> earth4:~/tip>
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. :-/
diff --git a/unpack-trees.c b/unpack-trees.c
index 16bc2ca..4b02fbf 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -8,6 +8,8 @@
#include "progress.h"
#include "refs.h"
+#define STAGES (5 + 1)
+
/*
* Error messages expected by scripts out of plumbing commands such as
* read-tree. Non-scripted Porcelain is not required to use these messages
@@ -143,7 +145,7 @@ static inline int call_unpack_fn(struct cache_entry **src, struct unpack_trees_o
static int unpack_index_entry(struct cache_entry *ce, struct unpack_trees_options *o)
{
- struct cache_entry *src[5] = { ce, };
+ struct cache_entry *src[STAGES] = { ce, };
o->pos++;
if (ce_stage(ce)) {
@@ -240,7 +242,7 @@ static struct cache_entry *create_ce_entry(const struct traverse_info *info, con
return ce;
}
-static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmask, struct cache_entry *src[5],
+static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmask, struct cache_entry *src[STAGES],
const struct name_entry *names, const struct traverse_info *info)
{
int i;
@@ -291,7 +293,7 @@ static int unpack_nondirectories(int n, unsigned long mask, unsigned long dirmas
static int unpack_callback(int n, unsigned long mask, unsigned long dirmask, struct name_entry *names, struct traverse_info *info)
{
- struct cache_entry *src[5] = { NULL, };
+ struct cache_entry *src[STAGES] = { NULL, };
struct unpack_trees_options *o = info->data;
const struct name_entry *p = names;
@@ -728,7 +730,7 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)
{
struct cache_entry *index;
struct cache_entry *head;
- struct cache_entry *remote = stages[o->head_idx + 1];
+ struct cache_entry *remote;
int count;
int head_match = 0;
int remote_match = 0;
@@ -755,6 +757,9 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)
head = NULL;
}
+ if (o->head_idx + 1 >= STAGES)
+ die("Too many stages (internal error)");
+ remote = stages[o->head_idx + 1];
if (remote == o->df_conflict_entry) {
df_conflict_remote = 1;
remote = NULL;
^ permalink raw reply related
* Newbie question regarding 3way merge order.
From: Raimund Berger @ 2009-01-29 22:25 UTC (permalink / raw)
To: git
Hi guys
I got a dumb question for you to wholeheartedly laugh at, and to which
the answer seems to be so self evident. I'd still like a possibly
authoritative statement though, just for the books.
The question is whether a (3way) merge is commutative, purely in terms
of content (i.e. disregarding commit history for now). Iow if no matter
in which order I merge A and B, i.e. A into B or B into A, I'd be
guaranteed to arrive at the same content.
If yes, a followup question would be if the merge machinery sitting
beneath rebase is exactly the same as that of a standard merge.
The reason I ask is obvious I guess. What basically interests me is if I
gave a bunch of topic branches exposure on a test branch and, after
resolving issues, applied them to stable, that I could be 100% sure to
not introduce new issues content wise just by applying merges in a
different order or form (rebase, patch set).
Thanks for feedback, Raimund.
^ permalink raw reply
* Re: [PATCH] Add --ff-only flag to git-merge
From: Junio C Hamano @ 2009-01-29 22:29 UTC (permalink / raw)
To: Yuval Kogman; +Cc: git
In-Reply-To: <a891e1bd0901291257t774af061s84497cde8f4bf61c@mail.gmail.com>
Yuval Kogman <nothingmuch@woobling.org> writes:
> I started incorperating your feedback but before I send a new patch I
> have several questions about the trickier bits:
>
> 2009/1/28 Junio C Hamano <gitster@pobox.com>:
>
>> * The placement of this misses the case where a merge of two unrelated
>> histories is attempted. You would need to also have a check at "No
>> common ancestors found. We need a real merge." part.
>
> Won't that fall through? The if (!common) is above, and this is
> eventually an else if for it (see line 978)
When "if (!common)" is true, the empty statement ";" is executed, and all
its "else if" conditional arms will be skipped. Is that what you want to
happen?
>> The octopus
>> codepath could also end up with a fast forward or up-to-date.
>
> So this case is obviously more convoluted... If an octopus merge is
> chosen should it just pass --ff-only to git-merge-octopus? Or maybe it
> should always pass --ff-only and the various different strategies
> would just die unconditionally?
I was referring to this part:
} else {
/*
* An octopus. If we can reach all the remote we are up
* to date.
*/
int up_to_date = 1;
...
if (up_to_date) {
finish_up_to_date("Already up-to-date. Yeeah!");
return 0;
}
}
You do not want to fail this case, where you tried to merge others that
have already been merged, when --ff-only is given, do you? After all, all
that you are interested in is "do not create a new merge commit".
If you scroll down a bit from there, you will see:
/* We are going to make a new commit. */
git_committer_info(IDENT_ERROR_ON_NO_NAME);
/*
* At this point, we need a real merge. No matter what strategy
* we use, it would operate on the index, possibly affecting the
This is where "if (!common) ;" will fall through to.
If your goal is to prevent the user from creating a new merge commit,
the logical place to do so would be immediately before that comment,
independent from all the if..elseif..fi conditional arms that come before
it, I think.
You also need to disable allow_trivial when ff-only is given, but I think
that goes without saying. If you do not want to allow creating a new
merge commit, you do not want even a trivial merge to happen.
^ permalink raw reply
* Re: [PATCH v2] http-push: refactor request url creation
From: Tay Ray Chuan @ 2009-01-29 22:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vk58ef2il.fsf@gitster.siamese.dyndns.org>
On Fri, Jan 30, 2009 at 1:25 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> On Thu, 29 Jan 2009, Tay Ray Chuan wrote:
>>
>>> Currently, functions that deal with objects on the remote repository have to
>>> allocate and
>>> do strcpys to generate the URL.
>>
>> That is a funny way to wrap the commit message :-)
>
> That's format=flowed in action, isn't it?
Ok, I've turned that off.
Eh, no I didn't wrap anything, my message looks fine here:
http://permalink.gmane.org/gmane.comp.version-control.git/107691
...turning off format=flowed would fix that?
--
Cheers,
Ray Chuan
^ permalink raw reply
* Re: [PATCH] push: Learn to set up branch tracking with '--track'
From: Jeff King @ 2009-01-29 22:33 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, gitster
In-Reply-To: <alpine.DEB.1.00.0901291438030.3586@pacific.mpi-cbg.de>
On Thu, Jan 29, 2009 at 02:38:43PM +0100, Johannes Schindelin wrote:
> When pushing a branch to a remote repository that the remote side did
> not know beforehand, it is often handy to set up the branch tracking
> such that
>
> $ git checkout xyz
> $ git push --track origin xyz:abc
> $ git pull
>
> will pull the branch 'abc' from the remote 'origin' into the branch
> 'xyz'.
What if "xyz" is a tag? Right now, the patch generates a bogus config
section. Yes, you can say "don't use --track, then". But I can think two
reasons why that isn't acceptable:
1. You might be pushing multiple things, one of which is a branch
and one of which is not.
2. This is likely to grow a related config variable, similar to
branch.autosetupmerge.
How about "git push --track origin HEAD"? This also generates bogus
config. How about "git push --track origin refs/heads/*:refs/heads/*"?
Also bogus.
It looks like you catch "git push --track --all". So at least there is
no bogus config generated. But it would be nice to translate this to
"create a tracking branch for everything I pushed".
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.
> + if (!err && track)
> + setup_tracking(transport->url);
Wouldn't the remote name (if we have one) be preferable to the URL? As
it is, doing "git push --track origin master" results in
[branch "master"]
remote = $YOUR_URL_FOR_ORIGIN
merge = refs/heads/master
which means that:
1. If you ever change remote.origin.url, it will not propagate to your
branch section.
2. When you fetch, you are fetching into FETCH_HEAD, meaning your
tracking refs are not updated.
-Peff
^ permalink raw reply
* Re: Something weird is happening...
From: Ingo Molnar @ 2009-01-29 22:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: H. Peter Anvin, Git Mailing List
In-Reply-To: <7vfxj1eqh6.fsf@gitster.siamese.dyndns.org>
* Junio C Hamano <gitster@pobox.com> wrote:
> But that is just a conjecture from a cursory looking at the current
> code. I do not remember (or did not know from the beginning) some
> details of it. And that is why I asked you if "git fsck --full" reports
> it missing in my earlier response to you.
sorry i did not follow up on that detail. I ran --full yesterday already
(before we reported this), and besides taking a lot longer to finish it
reported the same screenful of problems.
Should i send the output of it? (i started it, it will take some time to
finish)
Ingo
^ permalink raw reply
* Re: Something weird is happening...
From: Ingo Molnar @ 2009-01-29 22:35 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <49822944.8000103@zytor.com>
* H. Peter Anvin <hpa@zytor.com> wrote:
>> But it still does the connectivity check fully from your refs. And if
>> it finds an object that ought to be reachable from your refs that it
>> hasn't validated, it gives the "missing" warning above. There is a
>> check to exclude objects in packs when running "git fsck" without
>> --full from this warning but I do not see a corresponding exclusion of
>> objects that were borrowed from your neighbour which we didn't check
>> either.
>>
>> But that is just a conjecture from a cursory looking at the current
>> code. I do not remember (or did not know from the beginning) some
>> details of it. And that is why I asked you if "git fsck --full" reports
>> it missing in my earlier response to you.
>
> Just tried it, it doesn't. So I guess things are OK.
hm, it reported missing items here - but maybe i misremembered that.
Re-running it currently. (takes quite a while)
Ingo
^ permalink raw reply
* Re: Something weird is happening...
From: H. Peter Anvin @ 2009-01-29 22:36 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20090129223410.GA1465@elte.hu>
Ingo Molnar wrote:
> * Junio C Hamano <gitster@pobox.com> wrote:
>
>> But that is just a conjecture from a cursory looking at the current
>> code. I do not remember (or did not know from the beginning) some
>> details of it. And that is why I asked you if "git fsck --full" reports
>> it missing in my earlier response to you.
>
> sorry i did not follow up on that detail. I ran --full yesterday already
> (before we reported this), and besides taking a lot longer to finish it
> reported the same screenful of problems.
>
> Should i send the output of it? (i started it, it will take some time to
> finish)
I didn't get the "missing" message with git fsck --full. I get the
"dangling" messages, still, but that is more or less expected.
-hpa
^ permalink raw reply
* Re: [PATCH] git-cvsserver: handle CVS 'noop' command.
From: Junio C Hamano @ 2009-01-29 22:45 UTC (permalink / raw)
To: Stefan Karpinski; +Cc: git
In-Reply-To: <1233264914-7798-1-git-send-email-stefan.karpinski@gmail.com>
Stefan Karpinski <stefan.karpinski@gmail.com> writes:
> The implementation is trivial: ignore the 'noop' command
> if it is sent. This command is issued by some CVS clients,
> notably TortoiseCVS. Without this patch, TortoiseCVS will
> choke when git-cvsserver complains about the unsupported
> command.
>
> Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>
> ---
>
> Since this change has no negative impact, is too simple to
> be wrong, and improves interaction with some clients, it
> seem to me like a no-brainer to apply it.
>
> git-cvsserver.perl | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index fef7faf..c1e09ea 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -188,7 +188,7 @@ while (<STDIN>)
> # use the $methods hash to call the appropriate sub for this command
> #$log->info("Method : $1");
> &{$methods->{$1}}($1,$2);
> - } else {
> + } elsif ($1 ne 'noop') {
> # log fatal because we don't understand this function. If this happens
> # we're fairly screwed because we don't know if the client is expecting
> # a response. If it is, the client will hang, we'll hang, and the whole
> --
> 1.6.0.3.3.g08dd8
Not a no-brainer at all, sorry.
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.
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.
git-cvsserver.perl | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git i/git-cvsserver.perl w/git-cvsserver.perl
index fef7faf..ca47e08 100755
--- i/git-cvsserver.perl
+++ w/git-cvsserver.perl
@@ -71,6 +71,7 @@ my $methods = {
'log' => \&req_log,
'rlog' => \&req_log,
'tag' => \&req_CATCHALL,
+ 'noop' => \&req_CATCHALL,
'status' => \&req_status,
'admin' => \&req_CATCHALL,
'history' => \&req_CATCHALL,
^ permalink raw reply related
* Re: Something weird is happening...
From: H. Peter Anvin @ 2009-01-29 22:47 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <20090129224357.GA18471@elte.hu>
Ingo Molnar wrote:
>
> So the repo itself is fine, correct?
>
Looks like it.
-hpa
^ permalink raw reply
* 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
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