* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Jeff King @ 2009-01-29 3:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, git
In-Reply-To: <7vwscej26i.fsf@gitster.siamese.dyndns.org>
On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:
> * sr/clone-empty (Fri Jan 23 01:07:32 2009 +0100) 1 commit
> + Allow cloning an empty repository
>
> Has anybody actually tried this and made sure the resulting empty clone
> works fine after the clone source gets updated with some contents?
Hmm. It sort of works:
$ mkdir parent && (cd parent && git init)
Initialized empty Git repository in /home/peff/parent/.git/
$ git clone parent child
Initialized empty Git repository in /home/peff/child/.git/
warning: You appear to have cloned an empty repository.
So far so good...
$ (cd parent && echo content >file && git add file && git commit -m one)
[normal commit output]
$ (cd child && git fetch)
[normal fetch output]
But:
$ (cd child && git pull)
You asked me to pull without telling me which branch you
want to merge with, and 'branch.master.merge' in
...
So it's not quite seamless. The problem is that we're not setting up the
branch.master.* config on the empty clone. Nor do we set up
refs/remotes/origin/HEAD.
On top of that, I get funniness between versions:
$ ssh peff.net 'git version && mkdir foo && cd foo && git init'
git version 1.5.6.5
Initialized empty Git repository in /mnt/data/home/peff/foo/.git/
$ git clone peff.net:foo
Initialized empty Git repository in /home/peff/foo/.git/
warning: You appear to have cloned an empty repository.
$ fatal: The remote end hung up unexpectedly
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Jeff King @ 2009-01-29 4:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Sverre Rabbelier, git
In-Reply-To: <20090129035138.GC11836@coredump.intra.peff.net>
On Wed, Jan 28, 2009 at 10:51:38PM -0500, Jeff King wrote:
> But:
>
> $ (cd child && git pull)
> You asked me to pull without telling me which branch you
> want to merge with, and 'branch.master.merge' in
> ...
>
> So it's not quite seamless. The problem is that we're not setting up the
> branch.master.* config on the empty clone. Nor do we set up
> refs/remotes/origin/HEAD.
Hrm. I was thinking we checked out "master" as a branch yet to be born,
but of course that doesn't work because we don't even know that the name
"master" exists on the other side (to do that, we would need an
extension to transmit symref information for a ref yet to be born).
We could always assume the remote side is going to eventually put
content on "master" (we know they aren't using another branch _now_, or
the repo wouldn't be empty, so we are just guessing they will follow the
usual convention). That feels a bit hack-ish, though.
So the current behavior is probably sane. But it is not obvious to a
user how to extend their repo one the upstream isn't empty. Maybe the
"empty repo" warning could mention "git fetch && git checkout -b master
origin/master" (which is the most obvious way I can think of)?
-Peff
^ permalink raw reply
* Re: do you recommend "git" (over svn) for a 1-person team???
From: Jeff King @ 2009-01-29 4:05 UTC (permalink / raw)
To: Greg Hauptmann; +Cc: Martin Langhoff, James Pickens, git
In-Reply-To: <d30068860901281846v65baf431mdab5fe961ef9407d@mail.gmail.com>
On Thu, Jan 29, 2009 at 12:46:56PM +1000, Greg Hauptmann wrote:
> does the concept of me setting up my own central git server as well
> make any sense from the point of view of a way to ensure I can (when
> I'm not remote) "check-in" to the central git server & thereby keep a
> separate copy of my code (i.e. effectively a backup)
Yes, that is exactly what I do, because:
- my central server is backed up, but my workstations are not
- my central server is accessible remotely, but my workstations are
not. So that is how I get code from one box to the other.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Junio C Hamano @ 2009-01-29 4:22 UTC (permalink / raw)
To: Jeff King; +Cc: Sverre Rabbelier, git
In-Reply-To: <20090129040254.GD11836@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> We could always assume the remote side is going to eventually put
> content on "master" (we know they aren't using another branch _now_, or
> the repo wouldn't be empty, so we are just guessing they will follow the
> usual convention). That feels a bit hack-ish, though.
Now, doesn't "The other end is empty" error start looking much saner than
everybody seems to have thought ;-)?
^ permalink raw reply
* Re: How to install and use a custom merge driver
From: Alec Clews @ 2009-01-29 4:47 UTC (permalink / raw)
To: git
In-Reply-To: <20090129032531.GA11836@coredump.intra.peff.net>
Problem solved.
It appears that TAB is not supported as white space in either attributes or
config files? I removed all the tabs and it works as described on the box
Many many thanks for the help Jeff
- Alec
^ permalink raw reply
* [PATCH] symbolic ref: refuse non-ref targets in HEAD
From: Jeff King @ 2009-01-29 4:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
When calling "git symbolic-ref" it is easy to forget that
the target must be a fully qualified ref. E.g., you might
accidentally do:
$ git symbolic-ref HEAD master
Unfortunately, this is very difficult to recover from,
because the bogus contents of HEAD make git believe we are
no longer in a git repository (as is_git_dir explicitly
checks for "^refs/" in the HEAD target). So immediately
trying to fix the situation doesn't work:
$ git symbolic-ref HEAD refs/heads/master
fatal: Not a git repository
and one is left editing the .git/HEAD file manually.
Furthermore, one might be tempted to use symbolic-ref to set
up a detached HEAD:
$ git symbolic-ref HEAD `git rev-parse HEAD`
which sets up an even more bogus HEAD:
$ cat .git/HEAD
ref: 1a9ace4f2ad4176148e61b5a85cd63d5604aac6d
This patch introduces a small safety valve to prevent the
specific case of anything not starting with refs/ to go into
HEAD. The scope of the safety valve is intentionally very
limited, to make sure that we are not preventing any
behavior that would otherwise be valid (like pointing a
different symref than HEAD outside of refs/).
Signed-off-by: Jeff King <peff@peff.net>
---
This is a follow-up to a patch that I posted during the last release
freeze:
http://article.gmane.org/gmane.comp.version-control.git/103445
I know that using symbolic-ref manually is rare, but both I and the
original poster have been bitten by this (and figuring out what is going
on and fixing it is quite painful). But most importantly, I don't think
this can possibly hurt anyone trying to use this legitimately, since the
exact thing it is protecting against corrupts your repo. :)
Please beware that running the test script on the current "master" will
actually hose your git repo (test 3 kills the trash directory's
.git/HEAD, which means test 4 thinks your parent .git/ is its current
repo). Maybe it makes sense to do a precautionary reset in between.
builtin-symbolic-ref.c | 2 ++
t/t1401-symbolic-ref.sh | 27 +++++++++++++++++++++++++++
2 files changed, 29 insertions(+), 0 deletions(-)
create mode 100755 t/t1401-symbolic-ref.sh
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index bfc78bb..46ea4b2 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -44,6 +44,8 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
check_symref(argv[0], quiet);
break;
case 2:
+ if (!strcmp(argv[0], "HEAD") && prefixcmp(argv[1], "refs/"))
+ die("Refusing to point HEAD outside of refs/");
create_symref(argv[0], argv[1], msg);
break;
default:
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
new file mode 100755
index 0000000..1f22009
--- /dev/null
+++ b/t/t1401-symbolic-ref.sh
@@ -0,0 +1,27 @@
+#!/bin/sh
+
+test_description='basic symbolic-ref tests'
+. ./test-lib.sh
+
+test_expect_success 'symbolic-ref writes HEAD' '
+ git symbolic-ref HEAD refs/heads/foo &&
+ echo ref: refs/heads/foo >expect &&
+ test_cmp expect .git/HEAD
+'
+
+test_expect_success 'symbolic-ref reads HEAD' '
+ echo refs/heads/foo >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'symbolic-ref refuses non-ref branch for HEAD' '
+ test_must_fail git symbolic-ref HEAD foo
+'
+
+test_expect_success 'symbolic-ref refuses bare sha1' '
+ echo content >file && git add file && git commit -m one
+ test_must_fail git symbolic-ref HEAD `git rev-parse HEAD`
+'
+
+test_done
--
1.6.1.1.424.gac728
^ permalink raw reply related
* Re: How to install and use a custom merge driver
From: Jeff King @ 2009-01-29 5:03 UTC (permalink / raw)
To: Alec Clews; +Cc: git
In-Reply-To: <loom.20090129T044425-689@post.gmane.org>
On Thu, Jan 29, 2009 at 04:47:19AM +0000, Alec Clews wrote:
> It appears that TAB is not supported as white space in either attributes or
> config files? I removed all the tabs and it works as described on the box
That's odd. It should be fine. And if I repeat the test I posted with:
printf '*\tmerge=overwrite\n' >.git/info/attributes
(printf '[merge "overwrite"]\n'
printf '\tname = overwrite using cp\n'
printf '\tdriver = cp %%B %%A\n'
) >>.git/config
it works as before.
What git version are you using? Can you post a shell snippet that breaks
reliably?
-Peff
^ permalink raw reply
* Re: "malloc failed"
From: Jeff King @ 2009-01-29 5:14 UTC (permalink / raw)
To: Pau Garcia i Quiles; +Cc: David Abrahams, git
In-Reply-To: <3af572ac0901281416x5adef0eak89bd4b40fda52c2b@mail.gmail.com>
On Wed, Jan 28, 2009 at 11:16:32PM +0100, Pau Garcia i Quiles wrote:
> My case: I have a 500 MB repository with a 1GB working tree, with
> binary files ranging from 100KB to 50MB and a few thousand source
> files.
>
> I have two branches ('master' and 'cmake') and the latter has suffered
> a huge hierarchy reorganization.
>
> When I merge 'master' in 'cmake', if I use the 'subtree' strategy, it
> works fine. If I use any other strategy, after a couple of minutes I
> receive a "malloc failed" and the tree is all messed up. As I said, on
> Linux it works fine, so maybe it's a Windows-specific problem.
Hmm. It very well might be the rename detection allocating a lot of
memory to do inexact rename detection. It does try to limit the amount
of work, but based on number of files. So if you have a lot of huge
files, that might be fooling it.
Try setting merge.renamelimit to something small (but not '0', which
means "no limit").
-Peff
^ permalink raw reply
* Re: "malloc failed"
From: Jeff King @ 2009-01-29 5:20 UTC (permalink / raw)
To: David Abrahams; +Cc: git
In-Reply-To: <87skn3rn5n.fsf@mcbain.luannocracy.com>
On Wed, Jan 28, 2009 at 07:06:28PM -0500, David Abrahams wrote:
> Well, moving the 2.6G .dar backup binary out of the fileset seems to
> have helped a little, not surprisingly :-P
Ok, that _is_ big. ;) I wouldn't be surprised if there is some corner of
the code that barfs on a single object that doesn't fit in a signed
32-bit integer; I don't think we have any test coverage for stuff that
big.
But it may also just be that we are going to try malloc'ing 2.6G, and
that's making some system limit unhappy.
> I don't know whether anyone on this list should care about that failure
> given the level of abuse I'm inflicting on Git, but keep in mind that
> the system *does* have 8G of memory. Conclude what you will from that,
> I suppose!
Well, I think you said before that you were never getting close to using
up all your memory. Which implies it's some system limit.
-Peff
^ permalink raw reply
* Re: "malloc failed"
From: Jeff King @ 2009-01-29 5:56 UTC (permalink / raw)
To: David Abrahams; +Cc: Junio C Hamano, git
In-Reply-To: <20090129052041.GB31507@coredump.intra.peff.net>
On Thu, Jan 29, 2009 at 12:20:41AM -0500, Jeff King wrote:
> Ok, that _is_ big. ;) I wouldn't be surprised if there is some corner of
> the code that barfs on a single object that doesn't fit in a signed
> 32-bit integer; I don't think we have any test coverage for stuff that
> big.
Sure enough, that is the problem. With the patch below I was able to
"git add" and commit a 3 gigabyte file of random bytes (so even the
deflated object was 3G).
I think it might be worth applying as a general cleanup, but I have no
idea if other parts of the system might barf on such an object.
-- >8 --
Subject: [PATCH] avoid 31-bit truncation in write_loose_object
The size of the content we are adding may be larger than
2.1G (i.e., "git add gigantic-file"). Most of the code-path
to do so uses size_t or unsigned long to record the size,
but write_loose_object uses a signed int.
On platforms where "int" is 32-bits (which includes x86_64
Linux platforms), we end up passing malloc a negative size.
Signed-off-by: Jeff King <peff@peff.net>
---
sha1_file.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 360f7e5..8868b80 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2340,7 +2340,8 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
static int write_loose_object(const unsigned char *sha1, char *hdr, int hdrlen,
void *buf, unsigned long len, time_t mtime)
{
- int fd, size, ret;
+ int fd, ret;
+ size_t size;
unsigned char *compressed;
z_stream stream;
char *filename;
--
1.6.1.1.259.g8712.dirty
^ permalink raw reply related
* Something weird is happening...
From: H. Peter Anvin @ 2009-01-29 6:24 UTC (permalink / raw)
To: Git Mailing List; +Cc: Ingo Molnar
I was investigating a problem that Ingo Molnar reported on the
linux-2.6-tip.git repository on kernel.org. Unfortunately I was not
able to reproduce his problem (which is a problem in itself) but I did
run into another oddity:
: hera 4 ; git fsck
[lots of dangling commits deleted]
missing blob af0e01d4c663a101f48614e40d006ed6272d5c36
: hera 5 ; git cat-file blob af0e01d4c663a101f48614e40d006ed6272d5c36
/*
* debugfs.h - a tiny little debug file system
*
* Copyright (C) 2004 Greg Kroah-Hartman <greg@kroah.com>
* Copyright (C) 2004 IBM Inc.
*
[... rest of blob deleted ...]
Okay, what is going on here?
Since git 1.6.1.1 just got installed on kernel.org, this phenomenon (and
hopefully Ingo's problem, too) maybe has something to do with this new
version?
-hpa
--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.
^ permalink raw reply
* Re: Something weird is happening...
From: Junio C Hamano @ 2009-01-29 6:56 UTC (permalink / raw)
To: H. Peter Anvin; +Cc: Git Mailing List, Ingo Molnar
In-Reply-To: <49814BA4.6030705@zytor.com>
"H. Peter Anvin" <hpa@zytor.com> writes:
> I was investigating a problem that Ingo Molnar reported on the
> linux-2.6-tip.git repository on kernel.org. Unfortunately I was not
> able to reproduce his problem (which is a problem in itself) but I did
> run into another oddity:
>
> : hera 4 ; git fsck
>
> [lots of dangling commits deleted]
> missing blob af0e01d4c663a101f48614e40d006ed6272d5c36
>
> : hera 5 ; git cat-file blob af0e01d4c663a101f48614e40d006ed6272d5c36
> /*
> * debugfs.h - a tiny little debug file system
> *
> * Copyright (C) 2004 Greg Kroah-Hartman <greg@kroah.com>
> * Copyright (C) 2004 IBM Inc.
> *
> [... rest of blob deleted ...]
>
> Okay, what is going on here?
Does "git fsck --full" report the blob missing?
^ permalink raw reply
* Re: How to install and use a custom merge driver
From: Alec Clews @ 2009-01-29 7:00 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090129050348.GA31202@coredump.intra.peff.net>
Jeff King wrote:
> On Thu, Jan 29, 2009 at 04:47:19AM +0000, Alec Clews wrote:
>
>
> What git version are you using? Can you post a shell snippet that breaks
> reliably?
>
I used 1.5.6.3 on Ubuntu and 1.6.1.1 on Cygwin.
The problem appears to be spaces around '=' in the attributes file. This
fails for me
----------8<----------------------------------
commit() {
echo $1 >file && git add file && git commit -m $1
}
rm -rf repo
mkdir repo && cd repo && git init
commit base
commit branch-master
git checkout -b other HEAD^
commit branch-other
echo '* merge = overwrite' >.git/info/attributes
cat >>.git/config <<'EOF'
[merge "overwrite"]
name = overwrite using cp
driver= cp %B %A
EOF
git merge master
----------8<---------------------------------
--
Alec Clews
Personal <alec.clews@gmail.com> Melbourne, Australia.
Jabber: alecclews@jabber.org.au PGPKey ID: 0x9BBBFC7C
Blog http://alecthegeek.wordpress.com/
^ permalink raw reply
* Re: How to install and use a custom merge driver
From: Jeff King @ 2009-01-29 7:11 UTC (permalink / raw)
To: Alec Clews; +Cc: git
In-Reply-To: <49815423.8000902@gmail.com>
On Thu, Jan 29, 2009 at 06:00:51PM +1100, Alec Clews wrote:
> The problem appears to be spaces around '=' in the attributes file. This
> fails for me
> [...]
> echo '* merge = overwrite' >.git/info/attributes
Ah, OK. That isn't supposed to work. The attributes are whitespace
separated, and some don't even have an equals at all (e.g., "merge",
"-merge", and "merge=foo" are all valid). So that parses to "please use
the ordinary text merge driver", and some attributes that nobody uses
called "=" and "overwrite".
-Peff
^ permalink raw reply
* Re: [PATCH] symbolic ref: refuse non-ref targets in HEAD
From: Junio C Hamano @ 2009-01-29 7:53 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20090129045205.GA31183@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I know that using symbolic-ref manually is rare, but both I and the
> original poster have been bitten by this (and figuring out what is going
> on and fixing it is quite painful). But most importantly, I don't think
> this can possibly hurt anyone trying to use this legitimately, since the
> exact thing it is protecting against corrupts your repo. :)
I generally do not like adding artificial limitation to plumbing like this
patch does, because the end user making silly mistake using plumbing is a
sign that there was something lacking in the Porcelain.
But for this particular case, I do not think any future usage of
symbolic-ref plubming will get inconvenienced with the change. I would
even suggest making the check tighter to insist on refs/heads/ (not just
refs/) and tighten validate_headref() in path.c to match.
> Please beware that running the test script on the current "master" will
> actually hose your git repo (test 3 kills the trash directory's
> .git/HEAD, which means test 4 thinks your parent .git/ is its current
> repo). Maybe it makes sense to do a precautionary reset in between.
In addition, perhaps it may make sense to use test_create_repo to go one
level deeper before starting to play around, so that trash directory's
repository will prevent you from going any further up.
^ permalink raw reply
* Re: "malloc failed"
From: Junio C Hamano @ 2009-01-29 7:53 UTC (permalink / raw)
To: Jeff King; +Cc: David Abrahams, git
In-Reply-To: <20090129055633.GA32609@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Subject: [PATCH] avoid 31-bit truncation in write_loose_object
>
> The size of the content we are adding may be larger than
> 2.1G (i.e., "git add gigantic-file"). Most of the code-path
> to do so uses size_t or unsigned long to record the size,
> but write_loose_object uses a signed int.
Thanks.
I wonder if some analysis tool like sparse can help us spot these...
^ permalink raw reply
* Re: [PATCH] symbolic ref: refuse non-ref targets in HEAD
From: Jeff King @ 2009-01-29 8:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vljsuh7kf.fsf@gitster.siamese.dyndns.org>
On Wed, Jan 28, 2009 at 11:53:20PM -0800, Junio C Hamano wrote:
> I generally do not like adding artificial limitation to plumbing like this
> patch does, because the end user making silly mistake using plumbing is a
> sign that there was something lacking in the Porcelain.
Nor do I. I only propose it in this case because
1. We cannot possibly be hurting somebody's workflow, since it
produces nonsensical results currently.
2. The damage it does is so annoying to recover from.
I have no problem with symbolic-ref eventually learning to handle these
situations in some more sane manner; in the meantime, I think it makes
sense to prevent nasty breakage. And I don't think we're closing any
doors for the future; the behavior will switch from "you aren't allowed
to do this" to "does something sensible".
> But for this particular case, I do not think any future usage of
> symbolic-ref plubming will get inconvenienced with the change. I would
> even suggest making the check tighter to insist on refs/heads/ (not just
> refs/) and tighten validate_headref() in path.c to match.
Reasonable. Updated series to follow.
> > Please beware that running the test script on the current "master" will
> > actually hose your git repo (test 3 kills the trash directory's
> > .git/HEAD, which means test 4 thinks your parent .git/ is its current
> > repo). Maybe it makes sense to do a precautionary reset in between.
>
> In addition, perhaps it may make sense to use test_create_repo to go one
> level deeper before starting to play around, so that trash directory's
> repository will prevent you from going any further up.
That sort of helps, but only by luck. Each test kills off one layer of
repo. So the first one kills the test_create_repo, and the second one
kills the trash directory. Adding another test would kill off the main
repo. :) So you have to do something per-test. I'll do that in the
re-roll.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Charles Bailey @ 2009-01-29 8:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vwscej26i.fsf@gitster.siamese.dyndns.org>
On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:
> * cb/mergetool (Wed Jan 21 22:57:48 2009 +0000) 1 commit
> + mergetool: respect autocrlf by using checkout-index
>
Can you hold off on merging this one? I now think that there's a
cleaner way of doing this and I would like the opportunity for a
rethink.
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* Re: [PATCH/RFC v1 5/6] combine-diff.c: remove a call to fstat() inside show_patch_diff()
From: Kjetil Barvik @ 2009-01-29 8:20 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vab9akj2p.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Kjetil Barvik <barvik@broadpark.no> writes:
>>
>>> Currently inside show_patch_diff() we have and fstat() call after an
>>> ok lstat() call. Since we before the call to fstat() have already
>>> test for the link case with S_ISLNK() the fstat() can be removed.
>>
>> Good eyes. Thanks.
>
> Heh, I noticed you will update the commit log message, so I'll dequeue
> this and wait for an update.
Yes, I am planing a v2 in 1 or 3 days (it takes some time to run long-
running tests). And then hopefully I have addressed all comments so
far. Thanks for comments!
-- kjetil
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Junio C Hamano @ 2009-01-29 8:26 UTC (permalink / raw)
To: Charles Bailey; +Cc: git
In-Reply-To: <20090129081438.GA10490@hashpling.org>
Charles Bailey <charles@hashpling.org> writes:
> On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:
>> * cb/mergetool (Wed Jan 21 22:57:48 2009 +0000) 1 commit
>> + mergetool: respect autocrlf by using checkout-index
>>
>
> Can you hold off on merging this one? I now think that there's a
> cleaner way of doing this and I would like the opportunity for a
> rethink.
Sure, it is not in 'master' yet.
But it's in 'next', so incremental updates from now on, please.
^ permalink raw reply
* [PATCH 1/2] validate_headref: tighten ref-matching to just branches
From: Jeff King @ 2009-01-29 8:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090129080145.GA777@coredump.intra.peff.net>
When we are trying to determine whether a directory contains
a git repository, one of the tests we do is to check whether
HEAD is either a symlink or a symref into the "refs/"
hierarchy, or a detached HEAD.
We can tighten this a little more, though: a non-detached
HEAD should always point to a branch (since checking out
anything else should result in detachment), so it is safe to
check for "refs/heads/".
Signed-off-by: Jeff King <peff@peff.net>
---
When I was writing the commit message, my spider sense tingled a little
bit. I don't think this is an unreasonable thing to do, but I also don't
know that it helps in any meaningful way. There is no evidence of people
having other stuff in their HEADs anyway.
path.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/path.c b/path.c
index a074aea..108d9e9 100644
--- a/path.c
+++ b/path.c
@@ -154,7 +154,7 @@ int validate_headref(const char *path)
/* Make sure it is a "refs/.." symlink */
if (S_ISLNK(st.st_mode)) {
len = readlink(path, buffer, sizeof(buffer)-1);
- if (len >= 5 && !memcmp("refs/", buffer, 5))
+ if (len >= 11 && !memcmp("refs/heads/", buffer, 11))
return 0;
return -1;
}
@@ -178,7 +178,7 @@ int validate_headref(const char *path)
len -= 4;
while (len && isspace(*buf))
buf++, len--;
- if (len >= 5 && !memcmp("refs/", buf, 5))
+ if (len >= 11 && !memcmp("refs/heads/", buf, 11))
return 0;
}
--
1.6.1.1.425.gdbb13
^ permalink raw reply related
* [PATCH 2/2] symbolic ref: refuse non-ref targets in HEAD
From: Jeff King @ 2009-01-29 8:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090129080145.GA777@coredump.intra.peff.net>
When calling "git symbolic-ref" it is easy to forget that
the target must be a fully qualified ref. E.g., you might
accidentally do:
$ git symbolic-ref HEAD master
Unfortunately, this is very difficult to recover from,
because the bogus contents of HEAD make git believe we are
no longer in a git repository (as is_git_dir explicitly
checks for "^refs/heads/" in the HEAD target). So
immediately trying to fix the situation doesn't work:
$ git symbolic-ref HEAD refs/heads/master
fatal: Not a git repository
and one is left editing the .git/HEAD file manually.
Furthermore, one might be tempted to use symbolic-ref to set
up a detached HEAD:
$ git symbolic-ref HEAD `git rev-parse HEAD`
which sets up an even more bogus HEAD:
$ cat .git/HEAD
ref: 1a9ace4f2ad4176148e61b5a85cd63d5604aac6d
This patch introduces a small safety valve to prevent the
specific case of anything not starting with refs/heads/ to
go into HEAD. The scope of the safety valve is intentionally
very limited, to make sure that we are not preventing any
behavior that would otherwise be valid (like pointing a
different symref than HEAD outside of refs/heads/).
Signed-off-by: Jeff King <peff@peff.net>
---
Changes from the original are:
- s,refs/,refs/heads/, in the code and commit message
- test non-ref and non-branch separately; under the current
implementation it is obvious the former will work if the latter
does, but I think by testing intent and not the implementation, the
tests are more future-proof
- tests make sure to restore validity of trash directory immediately
after running
builtin-symbolic-ref.c | 3 +++
t/t1401-symbolic-ref.sh | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 44 insertions(+), 0 deletions(-)
create mode 100755 t/t1401-symbolic-ref.sh
diff --git a/builtin-symbolic-ref.c b/builtin-symbolic-ref.c
index bfc78bb..cafc4eb 100644
--- a/builtin-symbolic-ref.c
+++ b/builtin-symbolic-ref.c
@@ -44,6 +44,9 @@ int cmd_symbolic_ref(int argc, const char **argv, const char *prefix)
check_symref(argv[0], quiet);
break;
case 2:
+ if (!strcmp(argv[0], "HEAD") &&
+ prefixcmp(argv[1], "refs/heads/"))
+ die("Refusing to point HEAD outside of refs/heads/");
create_symref(argv[0], argv[1], msg);
break;
default:
diff --git a/t/t1401-symbolic-ref.sh b/t/t1401-symbolic-ref.sh
new file mode 100755
index 0000000..569f341
--- /dev/null
+++ b/t/t1401-symbolic-ref.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='basic symbolic-ref tests'
+. ./test-lib.sh
+
+# If the tests munging HEAD fail, they can break detection of
+# the git repo, meaning that further tests will operate on
+# the surrounding git repo instead of the trash directory.
+reset_to_sane() {
+ echo ref: refs/heads/foo >.git/HEAD
+}
+
+test_expect_success 'symbolic-ref writes HEAD' '
+ git symbolic-ref HEAD refs/heads/foo &&
+ echo ref: refs/heads/foo >expect &&
+ test_cmp expect .git/HEAD
+'
+
+test_expect_success 'symbolic-ref reads HEAD' '
+ echo refs/heads/foo >expect &&
+ git symbolic-ref HEAD >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'symbolic-ref refuses non-ref for HEAD' '
+ test_must_fail git symbolic-ref HEAD foo
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref refuses non-branch for HEAD' '
+ test_must_fail git symbolic-ref HEAD refs/foo
+'
+reset_to_sane
+
+test_expect_success 'symbolic-ref refuses bare sha1' '
+ echo content >file && git add file && git commit -m one
+ test_must_fail git symbolic-ref HEAD `git rev-parse HEAD`
+'
+reset_to_sane
+
+test_done
--
1.6.1.1.425.gdbb13
^ permalink raw reply related
* Re: [PATCH] symbolic ref: refuse non-ref targets in HEAD
From: Jeff King @ 2009-01-29 8:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <20090129080145.GA777@coredump.intra.peff.net>
On Thu, Jan 29, 2009 at 03:01:45AM -0500, Jeff King wrote:
> > In addition, perhaps it may make sense to use test_create_repo to go one
> > level deeper before starting to play around, so that trash directory's
> > repository will prevent you from going any further up.
>
> That sort of helps, but only by luck. Each test kills off one layer of
> repo. So the first one kills the test_create_repo, and the second one
> kills the trash directory. Adding another test would kill off the main
> repo. :) So you have to do something per-test. I'll do that in the
> re-roll.
It occurs to me you perhaps did mean to do something per-test, like:
test_create_repo foo && (cd foo && do_the_actual_test)
That would work, too.
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2009, #07; Wed, 28)
From: Charles Bailey @ 2009-01-29 9:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbptqh60w.fsf@gitster.siamese.dyndns.org>
On Thu, Jan 29, 2009 at 12:26:39AM -0800, Junio C Hamano wrote:
> Charles Bailey <charles@hashpling.org> writes:
>
> > On Wed, Jan 28, 2009 at 06:06:45PM -0800, Junio C Hamano wrote:
> >> * cb/mergetool (Wed Jan 21 22:57:48 2009 +0000) 1 commit
> >> + mergetool: respect autocrlf by using checkout-index
> >>
> >
> > Can you hold off on merging this one? I now think that there's a
> > cleaner way of doing this and I would like the opportunity for a
> > rethink.
>
> Sure, it is not in 'master' yet.
>
> But it's in 'next', so incremental updates from now on, please.
>
OK, I've thought again and I still think that this patch is good.
Just so you know what I was thinking...
I felt that the new shell function that calls git checkout-index was a
bit clunky. git checkout-index --temp creates its own temporary file
and then the git mergetool renames this file to the temporary filename
that it had already decided on.
An earlier patch to mergetool was careful to ensure that mergetool
temporaries maintained the file extension of the target file in order
to help syntax highlighting merge tools. For this reason, just using
checkout-index generated filenames is not a sufficient solution.
I had two ideas, the first was that perhaps git mergetool could choose
a temporary naming scheme that could be matched by the appropriate use
of checkout-index --prefix. This would obviously preserve the file
extension but it's fairly obvious that it would have surprising
behaviour for merging files in subfolders.
My last idea would be to add an explicit --to-path= to git
checkout-index. It would make the mergetool code simpler but I'm not
sure how useful it would be in any other circumstance.
--
Charles Bailey
http://ccgi.hashpling.plus.com/blog/
^ permalink raw reply
* [PATCH] Support various HTTP authentication methods
From: Moriyoshi Koizumi @ 2009-01-29 9:32 UTC (permalink / raw)
To: git
Currently there is no way to specify the preferred authentication
method for the HTTP backend and it always ends up with the CURL's
default
settings.
This patch enables it if supported by CURL, adding a couple of new
settings
and config environment variables listed below (the names within the
parentheses indicate the latter.)
- http.auth (GIT_HTTP_AUTH)
Specifies the preferred authentication method for HTTP. This can
be a method name or the combination of those separated by comma. Valid
values are "basic", "digest", "gss" and "ntlm". You can also specify
"any" (all of the above), "anysafe" (all of the above except "basic").
Note that the strings are treated case-insensitive.
- http.proxy_auth (GIT_HTTP_PROXY_AUTH)
Specifies the preferred authentication method method for HTTP proxy.
The same thing as above applies to this setting.
Signed-off-by: Moriyoshi Koizumi <mozo@mozo.jp>
---
http.c | 105
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 105 insertions(+), 0 deletions(-)
diff --git a/http.c b/http.c
index ee58799..889135f 100644
--- a/http.c
+++ b/http.c
@@ -25,6 +25,12 @@ static long curl_low_speed_limit = -1;
static long curl_low_speed_time = -1;
static int curl_ftp_no_epsv = 0;
static const char *curl_http_proxy = NULL;
+#if LIBCURL_VERSION_NUM >= 0x070a06
+static const char *curl_http_auth = NULL;
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070a07
+static const char *curl_http_proxy_auth = NULL;
+#endif
static struct curl_slist *pragma_header;
@@ -153,11 +159,67 @@ static int http_options(const char *var, const
char *value, void *cb)
return git_config_string(&curl_http_proxy, var, value);
return 0;
}
+#if LIBCURL_VERSION_NUM >= 0x070a06
+ if (!strcmp("http.auth", var)) {
+ if (curl_http_auth == NULL)
+ return git_config_string(&curl_http_auth, var, value);
+ return 0;
+ }
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070a07
+ if (!strcmp("http.proxy_auth", var)) {
+ if (curl_http_proxy_auth == NULL)
+ return git_config_string(&curl_http_proxy_auth, var, value);
+ return 0;
+ }
+#endif
/* Fall back on the default ones */
return git_default_config(var, value, cb);
}
+#if LIBCURL_VERSION_NUM >= 0x070a06
+static long get_curl_auth_bitmask(const char* auth_method)
+{
+ char *buf = xmalloc(strlen(auth_method) + 1);
+ const unsigned char *p = (const unsigned char *)auth_method;
+ long mask = CURLAUTH_NONE;
+
+ for (;;) {
+ char *q = buf;
+ while (*p && isspace(*p))
+ ++p;
+
+ while (*p && *p != ',')
+ *q++ = tolower(*p++);
+
+ while (--q >= buf && isspace(*(unsigned char *)q));
+ ++q;
+
+ *q = '\0';
+
+ if (strcmp(buf, "basic") == 0)
+ mask |= CURLAUTH_BASIC;
+ else if (strcmp(buf, "digest") == 0)
+ mask |= CURLAUTH_DIGEST;
+ else if (strcmp(buf, "gss") == 0)
+ mask |= CURLAUTH_GSSNEGOTIATE;
+ else if (strcmp(buf, "ntlm") == 0)
+ mask |= CURLAUTH_NTLM;
+ else if (strcmp(buf, "any") == 0)
+ mask |= CURLAUTH_ANY;
+ else if (strcmp(buf, "anysafe") == 0)
+ mask |= CURLAUTH_ANYSAFE;
+
+ if (!*p)
+ break;
+ ++p;
+ }
+
+ return mask;
+}
+#endif
+
static CURL* get_curl_handle(void)
{
CURL* result = curl_easy_init();
@@ -210,6 +272,20 @@ static CURL* get_curl_handle(void)
if (curl_http_proxy)
curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+ if (curl_http_auth) {
+ long n = get_curl_auth_bitmask(curl_http_auth);
+ curl_easy_setopt(result, CURLOPT_HTTPAUTH, n);
+ }
+
+ if (curl_http_proxy) {
+ curl_easy_setopt(result, CURLOPT_PROXY, curl_http_proxy);
+
+ if (curl_http_proxy_auth) {
+ long n = get_curl_auth_bitmask(curl_http_proxy_auth);
+ curl_easy_setopt(result, CURLOPT_PROXYAUTH, n);
+ }
+ }
+
return result;
}
@@ -258,6 +334,21 @@ void http_init(struct remote *remote)
if (low_speed_time != NULL)
curl_low_speed_time = strtol(low_speed_time, NULL, 10);
+#if LIBCURL_VERSION_NUM >= 0x070a06
+ {
+ char *http_auth = getenv("GIT_HTTP_AUTH");
+ if (http_auth)
+ curl_http_auth = xstrdup(http_auth);
+ }
+#endif
+#if LIBCURL_VERSION_NUM >= 0x070a07
+ {
+ char *http_proxy_auth = getenv("GIT_HTTP_PROXY_AUTH");
+ if (http_proxy_auth)
+ curl_http_proxy_auth = xstrdup(http_proxy_auth);
+ }
+#endif
+
git_config(http_options, NULL);
if (curl_ssl_verify == -1)
@@ -309,6 +400,20 @@ void http_cleanup(void)
free((void *)curl_http_proxy);
curl_http_proxy = NULL;
}
+
+#if LIBCURL_VERSION_NUM >= 0x070a06
+ if (curl_http_auth) {
+ free((void *)curl_http_auth);
+ curl_http_auth = NULL;
+ }
+#endif
+
+#if LIBCURL_VERSION_NUM >= 0x070a07
+ if (curl_http_proxy_auth) {
+ free((void *)curl_http_proxy_auth);
+ curl_http_proxy_auth = NULL;
+ }
+#endif
}
struct active_request_slot *get_active_slot(void)
--
1.5.6.3
^ 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