git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: www.kernel.org/git
From: Martin Langhoff @ 2007-01-02  7:25 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: git, warthog19
In-Reply-To: <459A05F7.4060405@gmail.com>

On 1/2/07, Aneesh Kumar K.V <aneesh.kumar@gmail.com> wrote:
> The gitweb summary page at www.kernel.org/git have wrong git:// links.

I have just checked, and the URLs were correct. For example, I took
the git-tools url and successfully did

 git-clone git://git.kernel.org/pub/scm/git/git-tools.git

Which URLs are failing for you?

>Also the search feature is not working properly. It generates the search
> results but then immediately loads a wrong page.

This does look broken... I'll see if I can find out more.

cheers,


martin

^ permalink raw reply

* [PATCH] Detached HEAD (experimental)
From: Junio C Hamano @ 2007-01-02  7:45 UTC (permalink / raw)
  To: git

This allows "git checkout -d v1.4.3" to detach the HEAD from any
branch but point directly at the named commit.  After this, "git
branch" starts reporting that you are not on any branch.  You
can merge into "current branch" although there is not even such
a thing.

You can go back the normal state by switching to an existing
branch, say, "git checkout master" for example.  Another way to
get out of this is "git checkout -b newbranch".

This is still experimental.  While I think it makes sense to
allow commits on top of detached HEAD, it is rather dangerous
unless you are careful and know what you are doing.  Next "git
checkout master" will obviously lose what you have done, so we
might want to require "git checkout -f" out of a detached HEAD
if we find that the HEAD commit is not an ancestor of any other
branches.

On the other hand, the reason the user did not start the ad-hoc
work on a new branch with "git checkout -b" was probably because
the work was of a throw-away nature, so the convenience of not
having that safety valve might be even better.  We'll see.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-branch.c |   36 ++++++++++++++++++++++++++----------
 cache.h          |    2 +-
 git-checkout.sh  |   22 +++++++++++++++++++---
 path.c           |   26 ++++++++++++++++++--------
 setup.c          |    5 +++--
 5 files changed, 67 insertions(+), 24 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 745ee04..71f88f2 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -299,7 +299,8 @@ static void print_ref_list(int kinds, int verbose, int abbrev)
 	free_ref_list(&ref_list);
 }
 
-static void create_branch(const char *name, const char *start,
+static void create_branch(const char *name, const char *start_name,
+			  unsigned char *start_sha1,
 			  int force, int reflog)
 {
 	struct ref_lock *lock;
@@ -318,9 +319,14 @@ static void create_branch(const char *name, const char *start,
 			die("Cannot force update the current branch.");
 	}
 
-	if (get_sha1(start, sha1) ||
-	    (commit = lookup_commit_reference(sha1)) == NULL)
-		die("Not a valid branch point: '%s'.", start);
+	if (start_sha1)
+		/* detached HEAD */
+		hashcpy(sha1, start_sha1);
+	else if (get_sha1(start_name, sha1))
+		die("Not a valid object name: '%s'.", start_name);
+
+	if ((commit = lookup_commit_reference(sha1)) == NULL)
+		die("Not a valid branch point: '%s'.", start_name);
 	hashcpy(sha1, commit->object.sha1);
 
 	lock = lock_any_ref_for_update(ref, NULL);
@@ -329,7 +335,8 @@ static void create_branch(const char *name, const char *start,
 
 	if (reflog) {
 		log_all_ref_updates = 1;
-		snprintf(msg, sizeof msg, "branch: Created from %s", start);
+		snprintf(msg, sizeof msg, "branch: Created from %s",
+			 start_name);
 	}
 
 	if (write_ref_sha1(lock, sha1, msg) < 0)
@@ -341,6 +348,9 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	char oldref[PATH_MAX], newref[PATH_MAX], logmsg[PATH_MAX*2 + 100];
 	unsigned char sha1[20];
 
+	if (!oldname)
+		die("cannot rename the curren branch while not on any.");
+
 	if (snprintf(oldref, sizeof(oldref), "refs/heads/%s", oldname) > sizeof(oldref))
 		die("Old branchname too long");
 
@@ -447,9 +457,15 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	head = xstrdup(resolve_ref("HEAD", head_sha1, 0, NULL));
 	if (!head)
 		die("Failed to resolve HEAD as a valid ref.");
-	if (strncmp(head, "refs/heads/", 11))
-		die("HEAD not found below refs/heads!");
-	head += 11;
+	if (!strcmp(head, "HEAD")) {
+		/* detached HEAD */
+		;
+	}
+	else {
+		if (strncmp(head, "refs/heads/", 11))
+			die("HEAD not found below refs/heads!");
+		head += 11;
+	}
 
 	if (delete)
 		return delete_branches(argc - i, argv + i, force_delete, kinds);
@@ -460,9 +476,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	else if (rename && (i == argc - 2))
 		rename_branch(argv[i], argv[i + 1], force_rename);
 	else if (i == argc - 1)
-		create_branch(argv[i], head, force_create, reflog);
+		create_branch(argv[i], head, head_sha1, force_create, reflog);
 	else if (i == argc - 2)
-		create_branch(argv[i], argv[i + 1], force_create, reflog);
+		create_branch(argv[i], argv[i+1], NULL, force_create, reflog);
 	else
 		usage(builtin_branch_usage);
 
diff --git a/cache.h b/cache.h
index 29dd290..891045c 100644
--- a/cache.h
+++ b/cache.h
@@ -296,7 +296,7 @@ extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern int read_ref(const char *filename, unsigned char *sha1);
 extern const char *resolve_ref(const char *path, unsigned char *sha1, int, int *);
 extern int create_symref(const char *ref, const char *refs_heads_master);
-extern int validate_symref(const char *ref);
+extern int validate_headref(const char *ref);
 
 extern int base_name_compare(const char *name1, int len1, int mode1, const char *name2, int len2, int mode2);
 extern int cache_name_compare(const char *name1, int len1, const char *name2, int len2);
diff --git a/git-checkout.sh b/git-checkout.sh
index 92ec069..c50df28 100755
--- a/git-checkout.sh
+++ b/git-checkout.sh
@@ -1,6 +1,6 @@
 #!/bin/sh
 
-USAGE='[-f] [-b <new_branch>] [-m] [<branch>] [<paths>...]'
+USAGE='[-f] [-b <new_branch>] [-d] [-m] [<branch>] [<paths>...]'
 SUBDIRECTORY_OK=Sometimes
 . git-sh-setup
 
@@ -12,6 +12,7 @@ force=
 branch=
 newbranch=
 newbranch_log=
+detached=
 merge=
 while [ "$#" != "0" ]; do
     arg="$1"
@@ -27,6 +28,9 @@ while [ "$#" != "0" ]; do
 		git-check-ref-format "heads/$newbranch" ||
 			die "git checkout: we do not like '$newbranch' as a branch name."
 		;;
+	-d)
+		detached=1
+		;;
 	"-l")
 		newbranch_log=1
 		;;
@@ -144,13 +148,25 @@ fi
 # are switching to, then we'd better just be checking out
 # what we already had
 
-[ -z "$branch$newbranch" ] &&
-	[ "$new" != "$old" ] &&
+if test -z "$branch$newbranch" && test "$new" != "$old"
+then
+	case "$detached" in
+	'')
 	die "git checkout: provided reference cannot be checked out directly
 
   You need -b to associate a new branch with the wanted checkout. Example:
   git checkout -b <new_branch_name> $arg
 "
+		;;
+	1)
+		# NEEDSWORK: we would want to have this command here
+		# that allows us to detach the HEAD atomically.
+		# git update-ref --detach HEAD "$new"
+		rm -f "$GIT_DIR/HEAD"
+		echo "$new" >"$GIT_DIR/HEAD"
+		;;
+	esac
+fi
 
 if [ "X$old" = X ]
 then
diff --git a/path.c b/path.c
index 066f621..94ddd7e 100644
--- a/path.c
+++ b/path.c
@@ -90,10 +90,11 @@ int git_mkstemp(char *path, size_t len, const char *template)
 }
 
 
-int validate_symref(const char *path)
+int validate_headref(const char *path)
 {
 	struct stat st;
 	char *buf, buffer[256];
+	unsigned char sha1[20];
 	int len, fd;
 
 	if (lstat(path, &st) < 0)
@@ -119,14 +120,23 @@ int validate_symref(const char *path)
 	/*
 	 * Is it a symbolic ref?
 	 */
-	if (len < 4 || memcmp("ref:", buffer, 4))
+	if (len < 4)
 		return -1;
-	buf = buffer + 4;
-	len -= 4;
-	while (len && isspace(*buf))
-		buf++, len--;
-	if (len >= 5 && !memcmp("refs/", buf, 5))
+	if (!memcmp("ref:", buffer, 4)) {
+		buf = buffer + 4;
+		len -= 4;
+		while (len && isspace(*buf))
+			buf++, len--;
+		if (len >= 5 && !memcmp("refs/", buf, 5))
+			return 0;
+	}
+
+	/*
+	 * Is this a detached HEAD?
+	 */
+	if (!get_sha1_hex(buffer, sha1))
 		return 0;
+
 	return -1;
 }
 
@@ -241,7 +251,7 @@ char *enter_repo(char *path, int strict)
 		return NULL;
 
 	if (access("objects", X_OK) == 0 && access("refs", X_OK) == 0 &&
-	    validate_symref("HEAD") == 0) {
+	    validate_headref("HEAD") == 0) {
 		putenv("GIT_DIR=.");
 		check_repository_format();
 		return path;
diff --git a/setup.c b/setup.c
index 2ae57f7..cc97f9f 100644
--- a/setup.c
+++ b/setup.c
@@ -138,7 +138,8 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
  *    GIT_OBJECT_DIRECTORY environment variable
  *  - a refs/ directory
  *  - either a HEAD symlink or a HEAD file that is formatted as
- *    a proper "ref:".
+ *    a proper "ref:", or a regular file HEAD that has a properly
+ *    formatted sha1 object name.
  */
 static int is_git_directory(const char *suspect)
 {
@@ -161,7 +162,7 @@ static int is_git_directory(const char *suspect)
 		return 0;
 
 	strcpy(path + len, "/HEAD");
-	if (validate_symref(path))
+	if (validate_headref(path))
 		return 0;
 
 	return 1;
-- 
1.5.0.rc0.gab5a

^ permalink raw reply related

* [PATCH] Fix infinite loop when deleting multiple packed refs.
From: Shawn O. Pearce @ 2007-01-02  8:17 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Nicholas Miell
In-Reply-To: <b566b20c0701012244l21f85472k83970c0c573ce105@mail.gmail.com>

Nicholas Miell reported that `git branch -D A B` failed if both refs
A and B were packed into .git/packed-refs.  This happens because the
same pack_lock instance was being enqueued into the lock list twice,
causing the linked list to change from a singly linked list with
a NULL at the end to a circularly linked list with no termination.
This resulted in an infinite loop traversing the list during exit.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---

 Nicholas Miell <nmiell@gmail.com> wrote:
 > # this is with 1.4.4.2, spearce says master is also affected.
 > # (not subscribed, please Cc:)
 > 
 > mkdir test
 > cd test
 > git init-db
 > touch blah
 > git add blah
 > git commit -m "blah"
 > git checkout -b A
 > git checkout -b B
 > git checkout master
 > git pack-refs --all --prune
 > git branch -D A B # --> infinite loop in lockfile.c:remove_lock_file()

 Fixed.  ;-)

 Junio, this applies to master, but hopefully could also apply to
 maint, as the bug also shows up there.

 refs.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/refs.c b/refs.c
index e88ed8b..f1d1a5d 100644
--- a/refs.c
+++ b/refs.c
@@ -709,10 +709,9 @@ struct ref_lock *lock_any_ref_for_update(const char *ref, const unsigned char *o
 	return lock_ref_sha1_basic(ref, old_sha1, NULL);
 }
 
-static struct lock_file packlock;
-
 static int repack_without_ref(const char *refname)
 {
+	struct lock_file *packlock;
 	struct ref_list *list, *packed_ref_list;
 	int fd;
 	int found = 0;
@@ -726,8 +725,8 @@ static int repack_without_ref(const char *refname)
 	}
 	if (!found)
 		return 0;
-	memset(&packlock, 0, sizeof(packlock));
-	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
+	packlock = calloc(1, sizeof(*packlock));
+	fd = hold_lock_file_for_update(packlock, git_path("packed-refs"), 0);
 	if (fd < 0)
 		return error("cannot delete '%s' from packed refs", refname);
 
@@ -744,7 +743,7 @@ static int repack_without_ref(const char *refname)
 			die("too long a refname '%s'", list->name);
 		write_or_die(fd, line, len);
 	}
-	return commit_lock_file(&packlock);
+	return commit_lock_file(packlock);
 }
 
 int delete_ref(const char *refname, unsigned char *sha1)
-- 
1.5.0.rc0.gab5a

^ permalink raw reply related

* Re: Draft v1.5.0 release notes
From: David Kågedal @ 2007-01-02  7:22 UTC (permalink / raw)
  To: git
In-Reply-To: <7vlkkm47eg.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> writes:

> * Foreign SCM interfaces
> 
>   - git-svn now requires the Perl SVN:: libraries, the
>     command-line backend was too slow and limited.
> 
>   - the 'commit' command has been renamed to 'set-tree', and
>     'dcommit' is the recommended replacement for day-to-day
>     work.

The second item in this list is misleading.  It states that the
"comit" subcommand has been renamed.  But I don't believe it's the
"commit" subcommand of the "git" command.  So what is it a subcommand
to?

-- 
David Kågedal

^ permalink raw reply

* Re: Draft v1.5.0 release notes
From: Shawn O. Pearce @ 2007-01-02  8:23 UTC (permalink / raw)
  To: David Kågedal; +Cc: git, Junio C Hamano
In-Reply-To: <87vejp7v0m.fsf@morpheus.local>

David K?gedal <davidk@lysator.liu.se> wrote:
> Junio C Hamano <junkio@cox.net> writes:
> 
> > * Foreign SCM interfaces
> > 
> >   - git-svn now requires the Perl SVN:: libraries, the
> >     command-line backend was too slow and limited.
> > 
> >   - the 'commit' command has been renamed to 'set-tree', and
> >     'dcommit' is the recommended replacement for day-to-day
> >     work.
> 
> The second item in this list is misleading.  It states that the
> "comit" subcommand has been renamed.  But I don't believe it's the
> "commit" subcommand of the "git" command.  So what is it a subcommand
> to?

git-svn.  As in `git svn commit` is now `git svn set-tree`.

I agree with your point though.  My early readings of the above
quoted section just assumed git-svn, as I knew that was what Junio
was talking about.  Now that I read it again, I have to agree that
it is not quite as clear as I had originally thought.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Fix infinite loop when deleting multiple packed refs.
From: Junio C Hamano @ 2007-01-02  8:44 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070102081709.GA28779@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

>  Fixed.  ;-)
>
>  Junio, this applies to master, but hopefully could also apply to
>  maint, as the bug also shows up there.

I see a few instances of single static lock_file variable in our
code, but all of them seem to be locking the index and only
once, so they should be safe.

Thanks.

^ permalink raw reply

* Re: [PATCH] Fix infinite loop when deleting multiple packed refs.
From: Shawn O. Pearce @ 2007-01-02  8:47 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtzz9x1fo.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano <junkio@cox.net> wrote:
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> 
> >  Fixed.  ;-)
> >
> >  Junio, this applies to master, but hopefully could also apply to
> >  maint, as the bug also shows up there.
> 
> I see a few instances of single static lock_file variable in our
> code, but all of them seem to be locking the index and only
> once, so they should be safe.
> 
> Thanks.

I just realized that my patch used 'calloc' and not 'xcalloc'.
Would you mind correcting it for me?  ;-)

-- 
Shawn.

^ permalink raw reply

* [PATCH] instaweb: load Apache mime and dir modules if they are needed
From: Eric Wong @ 2007-01-02  8:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

I've noticed that Apache 2.2 on a Debian etch machine has
these compiled as modules.

Also set ServerName to avoid a warning at startup.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 git-instaweb.sh |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/git-instaweb.sh b/git-instaweb.sh
index 16cd351..08362f4 100755
--- a/git-instaweb.sh
+++ b/git-instaweb.sh
@@ -160,10 +160,20 @@ apache2_conf () {
 	test "$local" = true && bind='127.0.0.1:'
 	echo 'text/css css' > $fqgitdir/mime.types
 	cat > "$conf" <<EOF
+ServerName "git-instaweb"
 ServerRoot "$fqgitdir/gitweb"
 DocumentRoot "$fqgitdir/gitweb"
 PidFile "$fqgitdir/pid"
 Listen $bind$port
+EOF
+
+	for mod in mime dir; do
+		if test -e $module_path/mod_${mod}.so; then
+			echo "LoadModule ${mod}_module " \
+			     "$module_path/mod_${mod}.so" >> "$conf"
+		fi
+	done
+	cat >> "$conf" <<EOF
 TypesConfig $fqgitdir/mime.types
 DirectoryIndex gitweb.cgi
 EOF
-- 
Eric Wong

^ permalink raw reply related

* Re: git not tracking changes
From: Deepak Barua @ 2007-01-02  9:54 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git
In-Reply-To: <20070102055937.GD27690@spearce.org>

Thank you shawn.

Regards
Deepak

On 1/2/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Deepak Barua <dbbarua@gmail.com> wrote:
> > Hi,
> >    When i do a git add of a config.h file then make some changes and
> > then do git commit it does not reflect the changes..
> > eg
> > dep@zion:~/programs/elinks/elinks-0.11-20061220$ git add config.h
>
> Here you told Git to take the current contents of config.h and
> stage it into the index.  That content will be in the next
> commit.
>
> > dep@zion:~/programs/elinks/elinks-0.11-20061220$ vi config.h
>
> Then you modify it.  Git doesn't know about those changes to
> config.h, nor does it care at this point.
>
> > dep@zion:~/programs/elinks/elinks-0.11-20061220$ git commit
> > nothing to commit
>
> This is occuring because the content staged in the index does not
> differ from the content in HEAD (the last commit on this branch).
> You need to run `git add config.h` again now that you have modified
> it to restage the modified file.
>
> Basically I'm assuming that when you ran `git add config.h` the
> first time the content must have matched HEAD, which meant you
> didn't actually stage anything.
>
> --
> Shawn.
>


-- 
Code Code Code Away

^ permalink raw reply

* Re: [PATCH] Documentation: update git-pull.txt for clone's new default behavior
From: Jakub Narebski @ 2007-01-02 11:31 UTC (permalink / raw)
  To: git
In-Reply-To: <7vy7omyuaf.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:

> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
>> Questions:
>>
>> What is the reasonining of defining branch.<name>.merge to point
>> to the "remote's setup"?
> 
> See list archives.  
> 
> Because you are not required to use remote tracking branches.
> By the way, I think we allow the name of the remote tracking
> branch as well, but we do not advertise it -- always using
> remote's name consistently is much less confusing.

If  remember correctly there were added some magic which makes
git search on pull first remote branches (to allow to pull
without tracking branches), then tracking branches. This I think
supercedes alternate proposal of using branch.<name>.mergeLocal.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply

* Re: [PATCH] xdl_merge(): fix a segmentation fault when refining conflicts
From: Jakub Narebski @ 2007-01-02 13:18 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Shawn Pearce, git
In-Reply-To: <Pine.LNX.4.63.0612310208460.25709@wbgn013.biozentrum.uni-wuerzburg.de>

On Sun, 31 Dec 2006, Johannes Schindelin wrote:

> On Sat, 30 Dec 2006, Jakub Narebski wrote:
> 
>> Johannes Schindelin wrote:
>> 
>>> Of course, you can hit mismerges like the illustrated one _without_ 
>>> being marked as conflict (e.g. if the chunk of identical code is _not_ 
>>> added, but only the increments), but we should at least avoid them 
>>> where possible.
>> 
>> Perhaps you could make it even more conservating merge conflicts option 
>> (to tighten merge conflicts even more) to xdl_merge, but not used by 
>> default because as it removes accidental conflicts it increases 
>> mismerges (falsely not conflicted).
> 
> There is no way to do this sanely. If you want to catch these mismerges, 
> you have to mark _all_ modifications as conflicting.

Currently you have:
 - a level value of 0 means that all overlapping changes are treated
   as conflicts,
 - a value of 1 means that if the overlapping changes are identical,
   it is not treated as a conflict.
 - If you set level to 2, overlapping changes will be analyzed, so that
   almost identical changes will not result in huge conflicts. Rather,
   only the conflicting lines will be shown inside conflict markers.

I was thinking about:
 - If you set level to 3, if one part after overlapping changes analysis
   in level 2 has empty conflict region, resolve this conflict as second
   side. WARNING: this reduces number of merge conflicts, but might give
   mismerges!

Or something like that.
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [PATCH/RFH] send-pack: fix pipeline.
From: Andy Whitcroft @ 2007-01-02 14:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7vlkkql0na.fsf@assigned-by-dhcp.cox.net>

Junio C Hamano wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> 
>> On Fri, 29 Dec 2006, Junio C Hamano wrote:
>>> I really need a sanity checking on this one.  I think I got the
>>> botched pipeline fixed with the patch I am replying to, but I do
>>> not understand the waitpid() business.  Care to enlighten me?
>> I think it was a beginning of a half-hearted attempt to check the exit 
>> status of the rev-list in case something went wrong.
>>
>> Which we simply don't do, so if git-rev-list ends up with some problem 
>> (due to a corrupt git repo or something), it will just send a partial 
>> pack.
>>
>> For some reason I thought we had fixed that by just generating the object 
>> list internally, but I guess we don't do that. That's just stupid. We 
>> should make "send-pack.c" use
>>
>> 	list-heads | git pack-objects --revs
>>
>> 	list-heads | git-rev-list --stdin | git-pack-objects
>>
>> because as it is now, I think send-pack is more fragile than it needs to 
>> be.
>>
>> Or maybe I'm just confused.
> 
> Dont' worry, you are no more confused than I am ;-).
> 
> "I thought we've done the 'pack-objects --revs' for the
> upload-pack side but haven't done so on the send-pack side." was
> what I initially wrote, but apparently we haven't.  On the other
> hand, I think upload-pack gets error termination from rev-list
> right.
> 
> It seems that repack is the only thing that uses the internal
> rev-list.

>From what I can see in next/pu (by the time I stopped stuffing food and
booze into myself and remembered how to turn on the computer) you have
ripped all this code out and started using the builtin rev-list
functions.  So what I can see in there now looks sane, and seems to work
in some limited testing here.

Reading the code does highlight a weakness in the face of incomplete
writes in the ref list send, which has always been in there.  Now we may
never see these on Linux, but as we do not know what OS is under us and
the relevant standards say they can occur we should cope me thinks.

I have just been testing a patch for that which I will post in follow up
to this post.

-apw

^ permalink raw reply

* [PATCH] send pack check for failure to send revisions list
From: Andy Whitcroft @ 2007-01-02 14:12 UTC (permalink / raw)
  To: git
In-Reply-To: <459A66D2.3000804@shadowen.org>

When passing the revisions list to pack-objects we do not check for
errors nor short writes.  Introduce a new write_in_full which will
handle short writes and report errors to the caller.  Use this to
short cut the send on failure, allowing us to wait for and report
the child in case the failure is its fault.

Signed-off-by: Andy Whitcroft <apw@shadowen.org>
---
diff --git a/send-pack.c b/send-pack.c
index eaa6efb..c195d08 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -65,12 +65,16 @@ static int pack_objects(int fd, struct ref *refs)
 			memcpy(buf + 1, sha1_to_hex(refs->old_sha1), 40);
 			buf[0] = '^';
 			buf[41] = '\n';
-			write(pipe_fd[1], buf, 42);
+			if (!write_in_full(pipe_fd[1], buf, 42,
+						"send-pack: send refs"))
+				break;
 		}
 		if (!is_null_sha1(refs->new_sha1)) {
 			memcpy(buf, sha1_to_hex(refs->new_sha1), 40);
 			buf[40] = '\n';
-			write(pipe_fd[1], buf, 41);
+			if (!write_in_full(pipe_fd[1], buf, 41,
+						"send-pack: send refs"))
+				break;
 		}
 		refs = refs->next;
 	}
diff --git a/write_or_die.c b/write_or_die.c
index 8cf6486..6db1d31 100644
--- a/write_or_die.c
+++ b/write_or_die.c
@@ -59,3 +59,26 @@ int write_or_whine(int fd, const void *buf, size_t count, const char *msg)
 
 	return 1;
 }
+
+int write_in_full(int fd, const void *buf, size_t count, const char *msg)
+{
+	const char *p = buf;
+	ssize_t written;
+
+	while (count > 0) {
+		written = xwrite(fd, p, count);
+		if (written == 0) {
+			fprintf(stderr, "%s: disk full?\n", msg);
+			return 0;
+		}
+		else if (written < 0) {
+			fprintf(stderr, "%s: write error (%s)\n",
+				msg, strerror(errno));
+			return 0;
+		}
+		count -= written;
+		p += written;
+	}
+
+	return 1;
+}

^ permalink raw reply related

* Re: confusion over the new branch and merge config
From: Jeff King @ 2007-01-02 14:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vbqlvrldk.fsf@assigned-by-dhcp.cox.net>

On Sat, Dec 23, 2006 at 01:51:03AM -0800, Junio C Hamano wrote:

> If you (or other people) use branch.*.merge, with its value set
> to remote name _and_ local name, and actually verify that either
> form works without confusion, please report back and I'll apply.

This [using tracking branches in branch.*.merge] seems to be working for
me, but it is possible to get some confusing results with it. Try this
config:

[remote "origin"]
  url = /my/other/git/repo
  fetch = refs/heads/master:refs/heads/origin
  fetch = refs/heads/origin:refs/heads/junio
[branch "master"]
  remote = origin
  merge = refs/heads/origin

That is, we have a local tracking branch 'X' which has the same name as
a remote branch 'X'. When we fetch, both will be marked for merge in
FETCH_HEAD, and git-pull will attempt to do an octopus.

Is this too convoluted a config to worry about (no, I don't actually do
this in my repository -- I just constructed the most plausible reason I
could think of for having conflicting names). I actually think having a
branch.*.mergelocal would make just as much sense and would be more
robust (plus, it should be safe and sensible for "git-checkout -b foo
bar" to point branch.foo.mergelocal to refs/heads/bar).

-Peff

^ permalink raw reply

* 1.5.0.rc0.gf4bf2: "git --bare gc" in bare repo deletes all!
From: Horst H. von Brand @ 2007-01-02 15:17 UTC (permalink / raw)
  To: git

I did:

  mkdir M
  cd M
  git --bare init-db
  git --bare fetch git+ssh://localhost/SomeRepo

This got the (correct) 21 (loose) objects. OK, cleanup:

  git --bare gc

this gives:

  Generating pack...
  Done counting 0 objects.
  Nothing new to pack.

and the objects are gone, no pack there.

Same thing happens with plain "git gc". Not very nice...


Besides, you can "git --bare pull ...", and get a mishmash of repo +
checked out files, this should be forbidden. Also, "git status" reports a
bunch of repo files as untracked, "git ls-files" shows nothing at all.
I.e., handling of bare repos needs a workover.
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239
Casilla 110-V, Valparaiso, Chile               Fax:  +56 32 2797513

^ permalink raw reply

* Re: [PATCH 0/17] Sliding window mmap for packfiles.
From: Andy Whitcroft @ 2007-01-02 15:28 UTC (permalink / raw)
  To: Shawn Pearce; +Cc: Francis Moreau, Junio C Hamano, git
In-Reply-To: <20061224094959.GA7814@spearce.org>

Shawn Pearce wrote:
> Francis Moreau <francis.moro@gmail.com> wrote:
>> On 12/24/06, Shawn Pearce <spearce@spearce.org> wrote:
>>> However with this series even a 32 bit OS which only permits
>>> processes to have at most 2 GiB of address space (2 GiB split
>>> between kernel space and userspace) can access packfiles up
>>> to 4 GiB in size.  That seems to be the split most OSes wind
>>> up using, if they didn't push it out to 3.2 GiB like Linux
>>> and Solaris have done.
>>>
>> Does it still needed for 64 bit OS ?
> 
> Not really.  Almost any reasonable 64 bit OS which is also running
> a Git compiled for 64 bit userspace would be able to mmap multiple
> 4 GiB packfiles without this series.
>  
>> if not, can the overhead (if there is a significant one) implied by
>> your rework be avoid for such cases ?
> 
> The overhead is rather low.  I did try hard to make it only a handful
> of machine instructions worth of additional work, and even then I
> tried to ammortize those over relatively large blocks of data to
> reduce the impact.  But yes, there is an overhead over the current
> shipping version of Git.
> 
> However at least some of the overhead can be avoided by setting
> core.packedGitWindowSize and core.packedGitLimit to higher values.
> This will allow the implementation to mmap() larger windows of the
> packfiles and retain a greater number of windows in memory at once.
> 
> If core.packedGitWindowSize is larger than your largest packfile
> then most of the code will just "shutoff" and won't get in the way.
> Its default is 32 MiB (see Documentation/config.txt).
> 
> I think the additional overhead added by this series is neglible
> and worth the more graceful degredation it allows when virtual
> address space becomes limited.

You now change the default size based on NO_MMAP, could you not just
bump the window size to 4GiB on 64 bit?

-apw

^ permalink raw reply

* Re: confusion over the new branch and merge config
From: Junio C Hamano @ 2007-01-02 17:32 UTC (permalink / raw)
  To: Jeff King; +Cc: Nicolas Pitre, git
In-Reply-To: <20070102144940.GA23932@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Sat, Dec 23, 2006 at 01:51:03AM -0800, Junio C Hamano wrote:
>
>> If you (or other people) use branch.*.merge, with its value set
>> to remote name _and_ local name, and actually verify that either
>> form works without confusion, please report back and I'll apply.
>
> This [using tracking branches in branch.*.merge] seems to be working for
> me, but it is possible to get some confusing results with it. Try this
> config:
>
> [remote "origin"]
>   url = /my/other/git/repo
>   fetch = refs/heads/master:refs/heads/origin
>   fetch = refs/heads/origin:refs/heads/junio
> [branch "master"]
>   remote = origin
>   merge = refs/heads/origin
>
> That is, we have a local tracking branch 'X' which has the same name as
> a remote branch 'X'. When we fetch, both will be marked for merge in
> FETCH_HEAD, and git-pull will attempt to do an octopus.
>
> Is this too convoluted a config to worry about (no, I don't actually do
> this in my repository -- I just constructed the most plausible reason I
> could think of for having conflicting names). I actually think having a
> branch.*.mergelocal would make just as much sense and would be more
> robust (plus, it should be safe and sensible for "git-checkout -b foo
> bar" to point branch.foo.mergelocal to refs/heads/bar).

If we are to worry about, and I think we might have to, I think
not worrying about mergelocal and not accepting the name of
local tracking branch is the only sensible thing to do.

Is there a problem if we did that?  I do not think of any.

^ permalink raw reply

* Re: confusion over the new branch and merge config
From: Jeff King @ 2007-01-02 17:34 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7vps9xwd01.fsf@assigned-by-dhcp.cox.net>

On Tue, Jan 02, 2007 at 09:32:30AM -0800, Junio C Hamano wrote:

> If we are to worry about, and I think we might have to, I think
> not worrying about mergelocal and not accepting the name of
> local tracking branch is the only sensible thing to do.

Sorry, I don't see the problem with mergelocal. Can you elaborate?

-Peff

^ permalink raw reply

* [RFC] git-svn: make git-svn commit-diff able to work without explicit arguments
From: Steve Frécinaux @ 2007-01-02 18:23 UTC (permalink / raw)
  To: git

Hello,

When using git-svn to access a SVN repo, the commit policy may vary. 
While git makes you commit small patches often, svn users tend to prefer 
bigger patches that implement a functionnality at once.

So at the end you have a SVN commit which corresponds to several git ones.

What you can do in this case is :

   git-svn commit-diff --edit -r$REV remotes/git-svn HEAD

Which effect is that it commits (at once) all the commits between the 
latest svn fetch and HEAD.

What I'm proposing here is this:

  - use the latest fetched rev the default for the -r argument.
  - use remotes/git-svn and HEAD the defaults for the treeish objects.

A smarter way to take these defaults would be to take the last revision 
in the current branch (which can be something else than git-svn if it 
wasn't rebased/merged recently) and the relevant commit in the current 
branch.

Additionnaly, --edit could be enabled by default if -m is not set and it 
is used interactively, eventually using an option in repo-config.

Any comment ?

^ permalink raw reply

* Re: [PATCH] Documentation: update git-pull.txt for clone's new default behavior
From: Luben Tuikov @ 2007-01-02 18:39 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Theodore Tso, git, Shawn Pearce, Johannes Schindelin,
	J. Bruce Fields
In-Reply-To: <7vodpiytue.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > It is in this sense that I do "cd <branch>; git-pull . <branch>"
> > in a sequence, and I'd rather do "cd <branch>; git-pull <symbolic-ref>"
> > to define which branch is the merge coming from given the current branch
> > _and_ the symbolic ref.
> 
> If I am reading you correctly, you have multiple directories,
> each with its own .git/ directory but major parts of these .git/
> directories are shared (namely, objects/ and refs/).  You would
> not be able to have separate checkout in these directories if
> you shared .git/HEAD and .git/index, so at least each of these
> directories has these two files for its own.
> 
> Is that what you are doing?

Yes, but the _ONLY_ thing which is NOT shared is .git/HEAD and
.git/index.  EVERYTHING else is the _same_ in all branch-per-directory.

> If that is the case, I think you do not even have to have the
> "branch spec" to express the patchflow among them.  Essentially
> you are using "one branch, one directory, one repository"
> workflow (my understanding is that this is how BK worked but I
> haven't seen it) but with your own improvements.  The reason
> this is an improvement is because such a shared .git/refs/
> allows you to do diff and log across branches this way, so if
> you have a separate .git/config just like you already have
> separate .git/HEAD and .git/index in these directories, you can
> use [remote "xyz"] sections in each of them to achieve what you
> called 'symbolic'.

Yeah, but I do not want to introduce yet another non-shared
file -- namely the config.

I want the config to be shared as well, since the repo is
exactly the same.  Firthermore, the branches are branches of
the same repo, not of different one.  config should be one
and the same -- per repo.

   Luben

^ permalink raw reply

* Re: [PATCH] Documentation: update git-pull.txt for clone's new default behavior
From: Luben Tuikov @ 2007-01-02 18:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, J. Bruce Fields
In-Reply-To: <7vy7omyuaf.fsf@assigned-by-dhcp.cox.net>

--- Junio C Hamano <junkio@cox.net> wrote:
> Luben Tuikov <ltuikov@yahoo.com> writes:
> 
> > Questions:
> >
> > What is the reasonining of defining branch.<name>.merge to point
> > to the "remote's setup"?
> 
> See list archives.  
> 
> Because you are not required to use remote tracking branches.

Then why does it point to the _remote_ mapping?  One shouldn't
care what it is, and how it looks in the remote repo.  That is
handled by [remote].  In [branch] I shouldn't have to have any
absolute references, i.e. branch.<name>.remote points to [remote],
and branch.<name>.merge should only give a _branch_ name,
whose remote-to-local mapping (which preserves the branch name)
can be found by dereferencing branch.<name>.remote to get to
remote.<rname>.fetch.

Think of it as DB schema normalization.

     Luben


> By the way, I think we allow the name of the remote tracking
> branch as well, but we do not advertise it -- always using
> remote's name consistently is much less confusing.
> 
> > The reasoning is that the remote's setup should only leak into
> > [remote] and no further.  I.e. [remote] is the only one concerned
> > with the mapping between the remote repo and the local repo.
> 
> No.  Remote is not about mapping -- if mapping is there you can
> talk about it, but that is optional.
> 
> 

^ permalink raw reply

* [RFC] Re: git-svn: make git-svn commit-diff able to work without explicit  arguments
From: Pierre Habouzit @ 2007-01-02 18:40 UTC (permalink / raw)
  To: Steve Frécinaux; +Cc: git
In-Reply-To: <459AA31E.5070705@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3490 bytes --]

On Tue, Jan 02, 2007 at 07:23:26PM +0100, Steve Frécinaux wrote:
> Hello,
> 
> When using git-svn to access a SVN repo, the commit policy may vary. 
> While git makes you commit small patches often, svn users tend to prefer 
> bigger patches that implement a functionnality at once.
> 
> So at the end you have a SVN commit which corresponds to several git 
> ones.
> 
> What you can do in this case is :
> 
>   git-svn commit-diff --edit -r$REV remotes/git-svn HEAD
> 
> Which effect is that it commits (at once) all the commits between the 
> latest svn fetch and HEAD.
> 
> What I'm proposing here is this:
> 
>  - use the latest fetched rev the default for the -r argument.
>  - use remotes/git-svn and HEAD the defaults for the treeish objects.
> 
> A smarter way to take these defaults would be to take the last revision 
> in the current branch (which can be something else than git-svn if it 
> wasn't rebased/merged recently) and the relevant commit in the current 
> branch.
> 
> Additionnaly, --edit could be enabled by default if -m is not set and it 
> is used interactively, eventually using an option in repo-config.
> 
> Any comment ?

  of course a git svn subcommand that in fact allow you to cherry pick
patches from `git rev-list remotes/git-svn` would be *really* good, but
here is what I do:

  1. be up2date:
    $ git svn fetch
    $ git rebase remotes/git-svn

  2. create a local branch to cherry pick the hunk you want to combine:
    $ git branch -f svn-tmp remotes/git-svn
    # cherry pick the commits you want using any method you like, eg:
    $ git cherry-pick <....>
     or
    $ git am [some previously built mailbox of changes]

    I happen to prefer the later since I do git format-patch
    remotes/git-svn from my master branch, and use my MUA as a
    poor-man's interactive way to select patches I want. Though
    sometimes you miss some dependant patches, and I suppose git
    cherry-pick would be better at that game, YMMV.

  3. git svn commit HEAD (not dcommit) to force merging of all your
     local patches in one svn changeset.

     note that you may need a step (1) update if anything changed since,
     if you don't want to see git-svn undo the commits other user may
     have done since your last (1) update, commit your change, and
     commit the previously undone commits back. I did that once when I
     was a beginner with git-svn, and it generated a huge pile of
     globally indempotend commits, and flooded the commit mail list,
     that was kind of funny ;)

  4. go back to your branch as usual, and resync to the new svn state:
     git checkout master
     git svn fetch
     git rebase remotes/git-svn # should merge things happily

     you can then go back to (2) and iterate until your
     `git rev-list remotes/git-svn` is empty.

  This is highly unpretty, and relies in the fact that nobody commited
into the svn between your set 2 and 3. I suppose you can achieve the
same by creating combined changeset from git, instead of a cherry
picking, in some kind of "combining" branch, and then just use
`git svn dcommit` from there, but I've found no convenient (I mean no
way *I* find convenient) way to do that. Maybe someone here has a better
idea :)

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* Re: [RFC] git-svn: make git-svn commit-diff able to work without explicit arguments
From: Eric Wong @ 2007-01-02 19:18 UTC (permalink / raw)
  To: Steve Fr?cinaux; +Cc: git
In-Reply-To: <459AA31E.5070705@gmail.com>

Steve Fr?cinaux <nudrema@gmail.com> wrote:
> Hello,
> 
> When using git-svn to access a SVN repo, the commit policy may vary. 
> While git makes you commit small patches often, svn users tend to prefer 
> bigger patches that implement a functionnality at once.
> 
> So at the end you have a SVN commit which corresponds to several git ones.
> 
> What you can do in this case is :
> 
>   git-svn commit-diff --edit -r$REV remotes/git-svn HEAD
> 
> Which effect is that it commits (at once) all the commits between the 
> latest svn fetch and HEAD.
> 
> What I'm proposing here is this:
> 
>  - use the latest fetched rev the default for the -r argument.

Yes, this is very important.

>  - use remotes/git-svn and HEAD the defaults for the treeish objects.
> 
> A smarter way to take these defaults would be to take the last revision 
> in the current branch (which can be something else than git-svn if it 
> wasn't rebased/merged recently) and the relevant commit in the current 
> branch.
> 
> Additionnaly, --edit could be enabled by default if -m is not set and it 
> is used interactively, eventually using an option in repo-config.

This sounds useful.  This is basically what 'set-tree' (the command
formerly known as 'commit') was meant to do originally.  Unlike 
set-tree (or perhaps with modifying set-tree), this should
rebase or reset afterwards to linearize history like 'dcommit'.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] Fix infinite loop when deleting multiple packed refs.
From: Junio C Hamano @ 2007-01-02 19:19 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git, Nicholas Miell
In-Reply-To: <20070102081709.GA28779@spearce.org>

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Nicholas Miell reported that `git branch -D A B` failed if both refs
> A and B were packed into .git/packed-refs.  This happens because the
> same pack_lock instance was being enqueued into the lock list twice,

I do not like this, actually.

It was stupid to link the same element twice to lock_file_list
and end up in a loop, so we certainly need a fix.

But it is not like we are taking a lock on multiple files in
this case.  It is just that we leave the linked list element on
the list even after commit_lock_file() successfully removes the
cruft.

We obviously cannot remove the list element commit_lock_file();
if we are interrupted in the middle of list manipulation, the
call to remove_lock_file_on_signal will happen with a broken
lock_file_list, which would cause the cruft to remain, so not
unlinking is the right thing to do.  Instead we should be
reusing the element already on the list.

I notice that there is already a code for that in lock_file()
function in lockfile.c.  Notice that lk->next is checked and the
element is linked only when it is not already on the list?  I
think the check is wrong for the last element on the list which
has NULL in next but is on the list, but if you read the check
as "is this element already on the list?" it actually makes
sense.  We do not want to link it on the list again, nor we
would want to set up signal/atexit over and over.

In other words, I am suspecting this might be a cleaner fix.

-- >8 --

diff --git a/cache.h b/cache.h
index a5fc232..4dbf658 100644
--- a/cache.h
+++ b/cache.h
@@ -179,6 +179,7 @@ extern int refresh_cache(unsigned int flags);
 
 struct lock_file {
 	struct lock_file *next;
+	char on_list;
 	char filename[PATH_MAX];
 };
 extern int hold_lock_file_for_update(struct lock_file *, const char *path, int);
diff --git a/lockfile.c b/lockfile.c
index 261baff..731bbf3 100644
--- a/lockfile.c
+++ b/lockfile.c
@@ -27,9 +27,12 @@ static int lock_file(struct lock_file *lk, const char *path)
 	sprintf(lk->filename, "%s.lock", path);
 	fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
 	if (0 <= fd) {
-		if (!lk->next) {
+		if (!lk->on_list) {
 			lk->next = lock_file_list;
 			lock_file_list = lk;
+			lk->on_list = 1;
+		}
+		if (lock_file_list) {
 			signal(SIGINT, remove_lock_file_on_signal);
 			atexit(remove_lock_file);
 		}
@@ -37,6 +40,8 @@ static int lock_file(struct lock_file *lk, const char *path)
 			return error("cannot fix permission bits on %s",
 				     lk->filename);
 	}
+	else
+		lk->filename[0] = 0;
 	return fd;
 }
 
diff --git a/refs.c b/refs.c
index 121774c..5205745 100644
--- a/refs.c
+++ b/refs.c
@@ -726,7 +726,6 @@ static int repack_without_ref(const char *refname)
 	}
 	if (!found)
 		return 0;
-	memset(&packlock, 0, sizeof(packlock));
 	fd = hold_lock_file_for_update(&packlock, git_path("packed-refs"), 0);
 	if (fd < 0)
 		return error("cannot delete '%s' from packed refs", refname);

^ permalink raw reply related

* Re: [PATCH] Documentation: update git-pull.txt for clone's new default behavior
From: Jakub Narebski @ 2007-01-02 19:22 UTC (permalink / raw)
  To: git
In-Reply-To: <560316.34562.qm@web31812.mail.mud.yahoo.com>

Luben Tuikov wrote:

> --- Junio C Hamano <junkio@cox.net> wrote:
>> Luben Tuikov <ltuikov@yahoo.com> writes:
>> 
>>> Questions:
>>>
>>> What is the reasonining of defining branch.<name>.merge to point
>>> to the "remote's setup"?
>> 
>> See list archives.  
>> 
>> Because you are not required to use remote tracking branches.
> 
> Then why does it point to the _remote_ mapping?  One shouldn't
> care what it is, and how it looks in the remote repo.  That is
> handled by [remote].  In [branch] I shouldn't have to have any
> absolute references, i.e. branch.<name>.remote points to [remote],
> and branch.<name>.merge should only give a _branch_ name,
> whose remote-to-local mapping (which preserves the branch name)
> can be found by dereferencing branch.<name>.remote to get to
> remote.<rname>.fetch.

Once again: if you _don't_ have _local_ tracking branch, so you _must_
use _remote_ name.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).