Git development
 help / color / mirror / Atom feed
* Re: [PATCH] Merge non-first refs that match first refspec
From: Daniel Barkalow @ 2007-09-28  4:40 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070928041509.GU3099@spearce.org>

On Fri, 28 Sep 2007, Shawn O. Pearce wrote:

> My understanding of the old code was that it should do what Junio
> was reporting as broken:

Agreed; that's why I thought there must be something wrong with it.

>   - when i == 0 this is the first remote.$foo.fetch;
>   - when has_merge == 0 we have no branch.$name.merge;
>   - when ref_map != NULL we have at least one ref from this fetch spec;
>   - when fetch[0].src == ref_map->name it wasn't a wildcard spec;
> 
> The if conditional above was ordered the way it is so we can skip
> the more expensive operation (the strcmp) most of them time through
> the loop iteration.
> 
> The only way I can see the old code was failing was if ref_map
> as returned by get_fetch_map() pointed to more than one refspec.
> But for that to be true then fetch[1].src must have been a wildcard,
> in which case the strcmp() would fail.  So we should only ever
> get one entry, it should be the first entry, and dammit it should
> have matched.

That is the analysis that the original code is based on, yes. But it's not 
the easiest thing to follow, and I misunderstood it earlier this evening 
(prompted by the claim that it wasn't working, mostly). It's probably 
worth checking remote->fetch[0].pattern instead of the strcmp, anyway, 
since that's clearer and cheaper anyway.

> How/why are we getting cases where fetch[0].src isn't in the first
> entry in ref_map?  What are those other entries?  Are they possibly
> going to also match fetch[0].src and cause more than one branch
> to merge?

Beats me; Junio, what's your test case?

> BTW, thanks for looking at this.  I didn't have time to get to it
> this week and now I'm really unlikely to be able to do so until
> after I get back from San Jose.  I have too many things crammed
> into this next week. :-\

No problem. You fixed plenty of my other bugs in this code, and it's 
getting so close to done.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: [PATCH] Merge non-first refs that match first refspec
From: Shawn O. Pearce @ 2007-09-28  4:48 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709280026240.5926@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> wrote:
> Beats me; Junio, what's your test case?

If I understood him correctly it is this:

	mkdir foo; cd foo; git init
	git config remote.origin.url git://repo.or.cz/alt-git.git
	git config remote.origin.fetch refs/heads/master:refs/remotes/origin/master
	git config --add remote.origin.fetch refs/heads/maint:refs/remotes/origin/maint
	git pull

We should see "master" listed in .git/FETCH_HEAD as a "for-merge"
and "maint" listed as a "not-for-merge"...

But if that remote.origin.fetch was a wildcard spec this shouldn't
happen as the results are unpredictable.  But above the user
explicitly put master first, so it should be defaulted to.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] alloc_ref(): allow for trailing NUL
From: Daniel Barkalow @ 2007-09-28  5:03 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, git
In-Reply-To: <Pine.LNX.4.64.0709280356550.28395@racer.site>

On Fri, 28 Sep 2007, Johannes Schindelin wrote:

> The parameter name "namelen" suggests that you pass the equivalent of
> strlen() to the function alloc_ref().  However, this function did not
> allocate enough space to put a NUL after the name.
> 
> Since struct ref does not have any member to describe the length of the
> string, this just does not make sense.
> 
> So make space for the NUL.

Good point, but shouldn't you then fix call sites that use strlen(name) + 
1?

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* [PATCH 0/2] rsync support
From: Johannes Schindelin @ 2007-09-28  5:06 UTC (permalink / raw)
  To: gitster, Daniel Barkalow, git

Hi,

I worked too long on this... So here it is, a 2-patch series to readd the 
rsync support.

One thing I did not do: fetch alternates.  My rationale is this: just as 
with HTTP, the rsync protocol does allow for having a different root than 
the file system root.  So, just as with HTTP, the alternates are likely to 
break.

But hey, if somebody feels like it, there is a NEEDSWORK pointer from 
which to start.

Ciao,
Dscho

^ permalink raw reply

* [PATCH 1/2] Introduce remove_dir_recursively()
From: Johannes Schindelin @ 2007-09-28  5:06 UTC (permalink / raw)
  To: gitster, Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280602580.28395@racer.site>


There was a function called remove_empty_dir_recursive() buried
in refs.c.  Expose a slightly enhanced version in dir.h: it can now
optionally remove a non-empty directory.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 dir.c  |   45 +++++++++++++++++++++++++++++++++++++++++++++
 dir.h  |    2 ++
 refs.c |   36 ++----------------------------------
 3 files changed, 49 insertions(+), 34 deletions(-)

diff --git a/dir.c b/dir.c
index eb6c3ab..0fae400 100644
--- a/dir.c
+++ b/dir.c
@@ -685,3 +685,48 @@ int is_inside_dir(const char *dir)
 	char buffer[PATH_MAX];
 	return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
 }
+
+int remove_dir_recursively(char *path, int len, int only_empty)
+{
+	DIR *dir = opendir(path);
+	struct dirent *e;
+	int ret = 0;
+
+	if (!dir)
+		return -1;
+	if (path[len-1] != '/')
+		path[len++] = '/';
+	while ((e = readdir(dir)) != NULL) {
+		struct stat st;
+		int namlen;
+		if ((e->d_name[0] == '.') &&
+		    ((e->d_name[1] == 0) ||
+		     ((e->d_name[1] == '.') && e->d_name[2] == 0)))
+			continue; /* "." and ".." */
+
+		namlen = strlen(e->d_name);
+		if (len + namlen > PATH_MAX ||
+				!memcpy(path + len, e->d_name, namlen) ||
+				(path[len + namlen] = '\0') ||
+				lstat(path, &st))
+			; /* fall thru */
+		else if (S_ISDIR(st.st_mode)) {
+			if (!remove_dir_recursively(path, len + namlen,
+						only_empty))
+				continue; /* happy */
+		} else if (!only_empty &&
+				len + namlen + 1 < PATH_MAX &&
+				!unlink(path))
+			continue; /* happy, too */
+
+		/* path too long, stat fails, or non-directory still exists */
+		ret = -1;
+		break;
+	}
+	closedir(dir);
+	if (!ret) {
+		path[len] = 0;
+		ret = rmdir(path);
+	}
+	return ret;
+}
diff --git a/dir.h b/dir.h
index f55a87b..f52eea8 100644
--- a/dir.h
+++ b/dir.h
@@ -64,4 +64,6 @@ extern struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathna
 extern char *get_relative_cwd(char *buffer, int size, const char *dir);
 extern int is_inside_dir(const char *dir);
 
+extern int remove_dir_recursively(char *dir, int len, int only_empty);
+
 #endif
diff --git a/refs.c b/refs.c
index 07e260c..f0336ee 100644
--- a/refs.c
+++ b/refs.c
@@ -2,6 +2,7 @@
 #include "refs.h"
 #include "object.h"
 #include "tag.h"
+#include "dir.h"
 
 /* ISSYMREF=01 and ISPACKED=02 are public interfaces */
 #define REF_KNOWS_PEELED 04
@@ -673,40 +674,7 @@ static struct ref_lock *verify_lock(struct ref_lock *lock,
 
 static int remove_empty_dir_recursive(char *path, int len)
 {
-	DIR *dir = opendir(path);
-	struct dirent *e;
-	int ret = 0;
-
-	if (!dir)
-		return -1;
-	if (path[len-1] != '/')
-		path[len++] = '/';
-	while ((e = readdir(dir)) != NULL) {
-		struct stat st;
-		int namlen;
-		if ((e->d_name[0] == '.') &&
-		    ((e->d_name[1] == 0) ||
-		     ((e->d_name[1] == '.') && e->d_name[2] == 0)))
-			continue; /* "." and ".." */
-
-		namlen = strlen(e->d_name);
-		if ((len + namlen < PATH_MAX) &&
-		    strcpy(path + len, e->d_name) &&
-		    !lstat(path, &st) &&
-		    S_ISDIR(st.st_mode) &&
-		    !remove_empty_dir_recursive(path, len + namlen))
-			continue; /* happy */
-
-		/* path too long, stat fails, or non-directory still exists */
-		ret = -1;
-		break;
-	}
-	closedir(dir);
-	if (!ret) {
-		path[len] = 0;
-		ret = rmdir(path);
-	}
-	return ret;
+	return remove_dir_recursively(path, len, 1);
 }
 
 static int remove_empty_directories(char *file)
-- 
1.5.3.2.1102.g9487

^ permalink raw reply related

* [PATCH 2/2] fetch/push: readd rsync support
From: Johannes Schindelin @ 2007-09-28  5:07 UTC (permalink / raw)
  To: gitster, Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280602580.28395@racer.site>


We lost rsync support when transitioning from shell to C.  Support it
again (even if the transport is technically deprecated, some people just
do not have any chance to use anything else).

Also, add a test to t5510.  Since rsync transport is not configured by
default on most machines, and especially not such that you can
write to rsync://127.0.0.1$(pwd)/, it is disabled by default; you can
enable it by setting the environment variable TEST_RSYNC.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Maybe this should be moved into transport-rsync.c...

 t/t5510-fetch.sh |   35 ++++++
 transport.c      |  333 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 367 insertions(+), 1 deletions(-)

diff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh
index 439430f..73a4e3c 100755
--- a/t/t5510-fetch.sh
+++ b/t/t5510-fetch.sh
@@ -153,4 +153,39 @@ test_expect_success 'bundle should be able to create a full history' '
 
 '
 
+test "$TEST_RSYNC" && {
+test_expect_success 'fetch via rsync' '
+	git pack-refs &&
+	mkdir rsynced &&
+	cd rsynced &&
+	git init &&
+	git fetch rsync://127.0.0.1$(pwd)/../.git master:refs/heads/master &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	mkdir ../rsynced2 &&
+	(cd ../rsynced2 &&
+	 git init) &&
+	git push rsync://127.0.0.1$(pwd)/../rsynced2/.git master &&
+	cd ../rsynced2 &&
+	git gc --prune &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+
+test_expect_success 'push via rsync' '
+	cd .. &&
+	mkdir rsynced3 &&
+	(cd rsynced3 &&
+	 git init) &&
+	git push --all rsync://127.0.0.1$(pwd)/rsynced3/.git &&
+	cd rsynced3 &&
+	test $(git rev-parse master) = $(cd .. && git rev-parse master) &&
+	git fsck --full
+'
+}
+
 test_done
diff --git a/transport.c b/transport.c
index 4f9cddc..d50a311 100644
--- a/transport.c
+++ b/transport.c
@@ -6,6 +6,334 @@
 #include "fetch-pack.h"
 #include "walker.h"
 #include "bundle.h"
+#include "dir.h"
+#include "refs.h"
+
+/* rsync support */
+
+/*
+ * We copy packed-refs and refs/ into a temporary file, then read the
+ * loose refs recursively (sorting whenever possible), and then inserting
+ * those packed refs that are not yet in the list (not validating, but
+ * assuming that the file is sorted).
+ *
+ * Appears refactoring this from refs.c is too cumbersome.
+ */
+
+static int direntry_cmp(const void *a, const void *b)
+{
+	const struct dirent *d1 = a;
+	const struct dirent *d2 = b;
+
+	return strcmp(d1->d_name, d2->d_name);
+}
+
+/*
+ * path is assumed to point to a buffer of PATH_MAX bytes, and
+ * path + name_offset is expected to point to "refs/".
+ */
+
+static int read_loose_refs(char *path, int name_offset, struct ref **tail)
+{
+	DIR *dir = opendir(path);
+	struct dirent *de;
+	struct {
+		struct dirent *entries;
+		int nr, alloc;
+	} list;
+	int i, pathlen;
+
+	if (!dir)
+		return -1;
+
+	memset (&list, 0, sizeof(list));
+
+	while ((de = readdir(dir))) {
+		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
+				(de->d_name[1] == '.' &&
+				 de->d_name[2] == '\0')))
+			continue;
+		if (list.nr >= list.alloc) {
+			list.alloc = alloc_nr(list.nr);
+			list.entries = xrealloc(list.entries,
+				list.alloc * sizeof(*de));
+		}
+		list.entries[list.nr++] = *de;
+	}
+	closedir(dir);
+
+	/* sort the list */
+
+	qsort(list.entries, list.nr, sizeof(*de), direntry_cmp);
+
+	pathlen = strlen(path);
+	path[pathlen] = '/';
+
+	for (i = 0; i < list.nr; i++) {
+		strlcpy(path + pathlen + 1, list.entries[i].d_name,
+				PATH_MAX - pathlen - 1);
+		if (read_loose_refs(path, name_offset, tail)) {
+			int fd = open(path, O_RDONLY);
+			char buffer[40];
+			struct ref *next;
+
+			if (fd < 0)
+				continue;
+			next = alloc_ref(strlen(path + name_offset));
+			if (read_in_full(fd, buffer, 40) != 40 ||
+					get_sha1_hex(buffer, next->old_sha1)) {
+				free(next);
+				continue;
+			}
+			close(fd);
+			strcpy(next->name, path + name_offset);
+			(*tail)->next = next;
+			*tail = next;
+		}
+	}
+
+	path[pathlen] = '\0';
+
+	free(list.entries);
+	return 0;
+}
+
+/* insert the packed refs for which no loose refs were found */
+
+static void insert_packed_refs(const char *packed_refs, struct ref **list)
+{
+	FILE *f = fopen(packed_refs, "r");
+	static char buffer[PATH_MAX];
+
+	if (!f)
+		return;
+
+	for (;;) {
+		int cmp, len;
+
+		if (!fgets(buffer, sizeof(buffer), f)) {
+			fclose(f);
+			return;
+		}
+
+		if (hexval(buffer[0]) & 0x10)
+			continue;
+		len = strlen(buffer);
+		if (buffer[len - 1] == '\n')
+			buffer[--len] = '\0';
+		if (len < 41)
+			continue;
+		while ((*list)->next &&
+				(cmp = strcmp(buffer + 41,
+				      (*list)->next->name)) > 0)
+			list = &(*list)->next;
+		if (!(*list)->next || cmp < 0) {
+			struct ref *next = alloc_ref(len - 40);
+			buffer[40] = '\0';
+			if (get_sha1_hex(buffer, next->old_sha1)) {
+				warning ("invalid SHA-1: %s", buffer);
+				free(next);
+				continue;
+			}
+			strcpy(next->name, buffer + 41);
+			next->next = (*list)->next;
+			(*list)->next = next;
+			list = &(*list)->next;
+		}
+	}
+}
+
+static struct ref *get_refs_via_rsync(const struct transport *transport)
+{
+	char buffer[PATH_MAX];
+	struct strbuf buf = STRBUF_INIT;
+	struct ref dummy, *result = &dummy;
+	struct child_process rsync;
+	const char *args[5];
+	char *temp_dir;
+
+	/* copy the refs to the temporary directory */
+
+	temp_dir = mkdtemp(git_path("rsync-refs-XXXXXX"));
+	if (temp_dir == NULL)
+		die ("Could not make temporary directory");
+	temp_dir = strcpy(buffer, temp_dir);
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/refs");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = buf.buf;
+	args[3] = temp_dir;
+	args[4] = NULL;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/packed-refs");
+
+	args[2] = buf.buf;
+
+	if (run_command(&rsync))
+		die ("Could not run rsync to get refs");
+
+	/* read the copied refs */
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, temp_dir);
+	strbuf_addstr(&buf, "/refs");
+	read_loose_refs(buf.buf, strlen(buf.buf) - 4, &result);
+
+	result = &dummy;
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, temp_dir);
+	strbuf_addstr(&buf, "/packed-refs");
+	insert_packed_refs(buf.buf, &result);
+
+	if (remove_dir_recursively(temp_dir, strlen(temp_dir), 0))
+		warning ("Could not remove temporary directory %s.", temp_dir);
+
+	return dummy.next;
+}
+
+static int fetch_objs_via_rsync(struct transport *transport,
+				 int nr_objs, struct ref **to_fetch)
+{
+	struct strbuf buf = STRBUF_INIT;
+	struct child_process rsync;
+	const char *args[8];
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addstr(&buf, "/objects/");
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-rv" : "-r";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = buf.buf;
+	args[6] = get_object_directory();
+	args[7] = NULL;
+
+	/* NEEDSWORK: handle one level of alternates */
+	return run_command(&rsync);
+}
+
+static int write_one_ref(const char *name, const unsigned char *sha1,
+		int flags, void *data)
+{
+	struct strbuf *buf = data;
+	int len = buf->len;
+	FILE *f;
+
+	if (flags && prefixcmp(name, "refs/heads/") &&
+			prefixcmp(name, "refs/tags/"))
+		return 0;
+
+	strbuf_addstr(buf, name);
+	if (safe_create_leading_directories(buf->buf) ||
+			!(f = fopen(buf->buf, "w")) ||
+			!fwrite(sha1_to_hex(sha1), 40, 1, f) ||
+			fputc('\n', f) == EOF ||
+			fclose(f))
+		return error("problems writing temporary file %s", buf->buf);
+	strbuf_setlen(buf, len);
+	return 0;
+}
+
+static int write_refs_to_temp_dir(struct strbuf *temp_dir,
+		int refspec_nr, const char **refspec)
+{
+	int i;
+
+	for (i = 0; i < refspec_nr; i++) {
+		unsigned char sha1[20];
+		char *ref;
+
+		if (dwim_ref(refspec[i], strlen(refspec[i]), sha1, &ref) != 1)
+			return error("Could not get ref %s", refspec[i]);
+
+		if (write_one_ref(ref, sha1, 0, temp_dir)) {
+			free(ref);
+			return -1;
+		}
+		free(ref);
+	}
+	return 0;
+}
+
+static int rsync_transport_push(struct transport *transport,
+		int refspec_nr, const char **refspec, int flags) {
+	char buffer[PATH_MAX];
+	struct strbuf buf = STRBUF_INIT;
+	int result = 0, i;
+	struct child_process rsync;
+	const char *args[8];
+	char *temp_dir;
+
+	/* first push the objects */
+
+	strbuf_addstr(&buf, transport->url);
+	strbuf_addch(&buf, '/');
+
+	memset(&rsync, 0, sizeof(rsync));
+	rsync.argv = args;
+	rsync.stdout_to_stderr = 1;
+	args[0] = "rsync";
+	args[1] = transport->verbose ? "-av" : "-a";
+	args[2] = "--ignore-existing";
+	args[3] = "--exclude";
+	args[4] = "info";
+	args[5] = get_object_directory();;
+	args[6] = buf.buf;
+	args[7] = NULL;
+
+	if (run_command(&rsync))
+		return error("Could not push objects to %s", transport->url);
+
+	/* copy the refs to the temporary directory; they could be packed. */
+
+	temp_dir = mkdtemp(git_path("rsync-refs-XXXXXX"));
+	if (temp_dir == NULL)
+		die ("Could not make temporary directory");
+	temp_dir = strcpy(buffer, temp_dir);
+
+	strbuf_reset(&buf);
+	strbuf_addstr(&buf, temp_dir);
+	strbuf_addch(&buf, '/');
+
+	if (flags & TRANSPORT_PUSH_ALL) {
+		if (for_each_ref(write_one_ref, &buf))
+			return -1;
+	} else if (write_refs_to_temp_dir(&buf, refspec_nr, refspec))
+		return -1;
+
+	i = (flags & TRANSPORT_PUSH_FORCE) ? 2 : 3;
+	args[i++] = buf.buf;
+	args[i++] = transport->url;
+	args[i++] = NULL;
+	if (run_command(&rsync))
+		result = error("Could not push to %s", transport->url);
+
+	if (remove_dir_recursively(temp_dir, strlen(temp_dir), 0))
+		warning ("Could not remove temporary directory %s.", temp_dir);
+
+	return result;
+}
+
+static int disconnect_rsync(struct transport *transport)
+{
+	return 0;
+}
 
 /* Generic functions for using commit walkers */
 
@@ -402,7 +730,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
 	ret->url = url;
 
 	if (!prefixcmp(url, "rsync://")) {
-		/* not supported; don't populate any ops */
+		ret->get_refs_list = get_refs_via_rsync;
+		ret->fetch = fetch_objs_via_rsync;
+		ret->push = rsync_transport_push;
+		ret->disconnect = disconnect_rsync;
 
 	} else if (!prefixcmp(url, "http://")
 	        || !prefixcmp(url, "https://")
-- 
1.5.3.2.1102.g9487

^ permalink raw reply related

* Re: [PATCH] Merge non-first refs that match first refspec
From: Daniel Barkalow @ 2007-09-28  5:12 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: Junio C Hamano, git
In-Reply-To: <20070928044824.GV3099@spearce.org>

On Fri, 28 Sep 2007, Shawn O. Pearce wrote:

> Daniel Barkalow <barkalow@iabervon.org> wrote:
> > Beats me; Junio, what's your test case?
> 
> If I understood him correctly it is this:
> 
> 	mkdir foo; cd foo; git init
> 	git config remote.origin.url git://repo.or.cz/alt-git.git
> 	git config remote.origin.fetch refs/heads/master:refs/remotes/origin/master
> 	git config --add remote.origin.fetch refs/heads/maint:refs/remotes/origin/maint
> 	git pull
> 
> We should see "master" listed in .git/FETCH_HEAD as a "for-merge"
> and "maint" listed as a "not-for-merge"...
> 
> But if that remote.origin.fetch was a wildcard spec this shouldn't
> happen as the results are unpredictable.  But above the user
> explicitly put master first, so it should be defaulted to.

But after that sequence, the right thing does happen. So I'm guessing that 
he has some different sequence that triggers a bug.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Peter Baumann @ 2007-09-28  5:15 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: git, Russ Brown, Kelvie Wong
In-Reply-To: <20070927222825.GG12427@artemis.corp>

On Fri, Sep 28, 2007 at 12:28:25AM +0200, Pierre Habouzit wrote:
> On Thu, Sep 27, 2007 at 10:23:26PM +0000, Theodore Tso wrote:
> > On Thu, Sep 27, 2007 at 09:11:25PM +0200, Pierre Habouzit wrote:
> > > > >   And as none of the other merge tool that are supported are able to
> > > > > either do 3way merges, or have a decent UI (that definitely seems to be
> > > > > exclusive features) I've given up on git-mergetool (and to be fair, it
> > > > > sucks, because it could be _sooo_ useful sometimes).
> > > > > 
> > > > 
> > > > What about meld? That does 3-way merge, and the UI is fine.
> > > 
> > >   Indeed, it seems that since the last time I tested it, it now does
> > > diff3 merging. I should reevaluate it :)
> > 
> > Pierre,
> > 
> > FYI, kdiff3, meld, xxdiff, opendiff (on MacOSX), and emerge all
> > support 3-way merge.
> 
>   I know, but:
>   * kdiff3 often take decisions behind your back, and results in broken
>     merges, so it's a no-go ;
>   * xxdiff has (IMHO) a very bad and non-intuitive UI, I never get to
>     make it work ;
>   * I don't use macos (opendiff) ;
>   * emerge is emacs right ? :)
> 
>   Though I gave meld another chance, and it works really better than it
> used to, so I may give it a try :) Let's hope I won't be disappointed by
> meld :)
> 

FWIW, xxdiff has support to handle halfway merged files, so that if git
could merge some hunks already for you (e.g. rerere kicked in), you
don't have to redo the _whole_ merge by hand, just call

	xxdiff -U file/with/mergemarkers/inside

and it will do the right thing. Not sure if the other tools could handle
it, but any pointers appreciated, because it often happens to me that
only one hunk out of several wasn't merged automatically by git. And
mergetool wants to always redo the whole merge, which isn't the best it
can do.

-Peter

^ permalink raw reply

* Re: [PATCH 2/2] fetch/push: readd rsync support
From: Shawn O. Pearce @ 2007-09-28  5:20 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280606530.28395@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> +static int disconnect_rsync(struct transport *transport)
> +{
> +	return 0;
> +}
>  
>  /* Generic functions for using commit walkers */
>  
> @@ -402,7 +730,10 @@ struct transport *transport_get(struct remote *remote, const char *url)
>  	ret->url = url;
>  
>  	if (!prefixcmp(url, "rsync://")) {
> -		/* not supported; don't populate any ops */
> +		ret->get_refs_list = get_refs_via_rsync;
> +		ret->fetch = fetch_objs_via_rsync;
> +		ret->push = rsync_transport_push;
> +		ret->disconnect = disconnect_rsync;
>  
>  	} else if (!prefixcmp(url, "http://")
>  	        || !prefixcmp(url, "https://")

For what it's worth disconnect is an optional operation.  You did
not need to implement it if you don't allocate a data member in
the struct transport.  So removing disconnect_rsync() could save
you 6 lines or so.

I see push is now supported again.  Didn't we remove rsync push
support a long time ago?  Like say in:

  commit c485104741ccdf32dd0c96fcb886c38a0b5badbd
  Author: c.shoemaker@cox.net <c.shoemaker@cox.net>
  Date:   Sat Oct 29 00:16:33 2005 -0400

    Add usage help to git-push.sh
    
    Also clarify failure to push to read-only remote.  Especially,
    state why rsync:// is not used for pushing.
    
    [jc: ideally rsync should not be used for anything]
    
    Signed-off-by: Chris Shoemaker <c.shoemaker at cox.net>
    Signed-off-by: Junio C Hamano <junkio@cox.net>

I guess it is nice to see that you can't kill rsync.  Like Windows
it always finds it way back into your life.

-- 
Shawn.

^ permalink raw reply

* Re: [PATCH] Merge non-first refs that match first refspec
From: Shawn O. Pearce @ 2007-09-28  5:25 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <Pine.LNX.4.64.0709280108060.5926@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Fri, 28 Sep 2007, Shawn O. Pearce wrote:
> 
> > Daniel Barkalow <barkalow@iabervon.org> wrote:
> > > Beats me; Junio, what's your test case?
> > 
> > If I understood him correctly it is this:
> > 
> > 	mkdir foo; cd foo; git init
> > 	git config remote.origin.url git://repo.or.cz/alt-git.git
> > 	git config remote.origin.fetch refs/heads/master:refs/remotes/origin/master
> > 	git config --add remote.origin.fetch refs/heads/maint:refs/remotes/origin/maint
> > 	git pull
> > 
> > We should see "master" listed in .git/FETCH_HEAD as a "for-merge"
> > and "maint" listed as a "not-for-merge"...
> > 
> > But if that remote.origin.fetch was a wildcard spec this shouldn't
> > happen as the results are unpredictable.  But above the user
> > explicitly put master first, so it should be defaulted to.
> 
> But after that sequence, the right thing does happen. So I'm guessing that 
> he has some different sequence that triggers a bug.

OK.  Then I don't understand what issue Junio identified.  And it
makes me feel better knowing that the current code does do the
right thing in the above case, because that's what I meant for it
to do when I was last in there...

-- 
Shawn.

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: David Kastrup @ 2007-09-28  6:19 UTC (permalink / raw)
  To: Kelvie Wong; +Cc: Theodore Tso, Junio C Hamano, git
In-Reply-To: <94ccbe710709272117s6dee1a8jad6edf71dfb13c81@mail.gmail.com>

"Kelvie Wong" <kelvie@ieee.org> writes:

> On 9/27/07, Theodore Tso <tytso@mit.edu> wrote:
>
>> It's not that emacs sets $PWD via its first argument, but the output
>> file is passed from emerge-files*-command to stashed in the per-buffer
>> variable emerge-file-out, which in turn gets passed to the emacs lisp
>> file write-file, which is what gets run when you run C-x C-w --- and
>> write-file interprets a relative pathname based on the containing
>> directory of the existing buffer.
>>                                                 - Ted
>>
>
> Ah yes, I just started reading up on elisp a little while ago :)
>
> I'd always assumed that emacs kept an internal "pwd" variable (i.e.
> what's displayed with M-x pwd), but I guess my way of thinking is
> archaic and deprecated :(

It does.  For every buffer.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply

* Re: backup or mirror a repository
From: Junio C Hamano @ 2007-09-28  6:27 UTC (permalink / raw)
  To: Dan Farina; +Cc: Johannes Schindelin, git
In-Reply-To: <1190947063.2263.46.camel@Tenacity>

Dan Farina <drfarina@gmail.com> writes:

> I did look at prune and update, but my problem is the opposite: I want
> something that will remove branches from the remote repo when they no
> longer exist locally. As-is over time I will proliferate little local
> branches unless I occasionally sit down and delete branches by operating
> directly on the bare backup repository. (and then use prune on the
> remote nodes)

The "git remote add --mirror" setup is about setting up the
local repository _AS_ the backup of the remote.  In other words,
the contents come from the remote by fetching from it and safely
kept away from disaster on the local side.  And for that,
"remote prune" is a perfect thing to do.

I think what you are asking for is an opposite, a backup remote
site you would push into.  That is not what "remote add --mirror"
is about.

You can almost do it with

	git push --all $remote

except there is no way to automagically remove the branch you
removed from the local repository.  For that, we would need a
new --mirror option to "git-push".

I think it is trivial to do for native transports, as we first
get the list of all refs from the remote side before starting
the transfer.  You need to change the last parameter called
'all' to remote.c::match_refs() into an enum ('push_all' being
one of choices), introduce another enum 'push_mirror', and teach
it to "match" the remote (i.e. dst) ref that does not have
corresponding entry on our side (i.e. src) with an empty object
name to mark it removed.  Then the part marked as "Finally, tell
the other end!"  in send-pack.c::send_pack() will take care of
the actual removal.

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: Pierre Habouzit @ 2007-09-28  6:35 UTC (permalink / raw)
  To: Peter Baumann; +Cc: git, Russ Brown, Kelvie Wong
In-Reply-To: <20070928051503.GA19815@xp.machine.xx>

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

On Fri, Sep 28, 2007 at 05:15:03AM +0000, Peter Baumann wrote:
> FWIW, xxdiff has support to handle halfway merged files, so that if git
> could merge some hunks already for you (e.g. rerere kicked in), you
> don't have to redo the _whole_ merge by hand, just call
> 
> 	xxdiff -U file/with/mergemarkers/inside
> 
> and it will do the right thing. Not sure if the other tools could handle
> it, but any pointers appreciated, because it often happens to me that
> only one hunk out of several wasn't merged automatically by git. And
> mergetool wants to always redo the whole merge, which isn't the best it
> can do.

  For what I've seen, it's how meld works, and I find it nice too. Meld
is quite slow (python + gnomeish doesn't help) but well, I don't merge
things that often, so like said I think I'll give it a try for a while
and see if it has what it takes :)

-- 
·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: git push (mis ?)behavior
From: Steffen Prohaska @ 2007-09-28  6:52 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7v3awzvrpr.fsf@gitster.siamese.dyndns.org>


On Sep 27, 2007, at 9:22 PM, Junio C Hamano wrote:

>
> So what's the desired semantics?
>
> The current semantics is:
>
>    "git push" says "you do not say to which repository?" and
>    consults "branch.<current>.remote" but defaults to 'origin'
>    if unconfigured.
>
>    "git push <name>" (or using the <name> determined as above)
>    says "you do not say which branches?" and consults
>    "remote.<name>.push" to find branches to push out, but
>    defaults to 'matching branches' if unconfigured.
>
> What you would want to change is the fallback behaviour for
> unconfigured "remote.<name>.push".  I think it is sensible to
> have an option to make it push only the current branch.

I'm not sure that changing the fallback behaviour for unconfigured
"remote.<name>.push" is sufficient.

When "remote.<name>.push" is set I'd expect "git push" to
choose only the 'right' remote.<name>.push lines, that is
the lines that have the current branch as the local ref.
"git push" would only push the current branch, which could be pushed
to 0 or more branches on the remote side. If no "remote.<name>.push"
contains the current branch as a local ref nothing would happen
(maybe a warning?). If several "remote.<name>.push" have the current
branch as the local ref the branch would be pushed to several
remote branches. But other branches than the current branch
would _never_ be pushed if no argument is given to 'git push'.

	Steffen

^ permalink raw reply

* Re: git push (mis ?)behavior
From: Pierre Habouzit @ 2007-09-28  6:58 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Junio C Hamano, git
In-Reply-To: <9D61974D-E08D-49F6-9C88-6BE446D53C74@zib.de>

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

On Fri, Sep 28, 2007 at 06:52:47AM +0000, Steffen Prohaska wrote:
> 
> On Sep 27, 2007, at 9:22 PM, Junio C Hamano wrote:
> 
> >
> >So what's the desired semantics?
> >
> >The current semantics is:
> >
> >   "git push" says "you do not say to which repository?" and
> >   consults "branch.<current>.remote" but defaults to 'origin'
> >   if unconfigured.
> >
> >   "git push <name>" (or using the <name> determined as above)
> >   says "you do not say which branches?" and consults
> >   "remote.<name>.push" to find branches to push out, but
> >   defaults to 'matching branches' if unconfigured.
> >
> >What you would want to change is the fallback behaviour for
> >unconfigured "remote.<name>.push".  I think it is sensible to
> >have an option to make it push only the current branch.
> 
> I'm not sure that changing the fallback behaviour for unconfigured
> "remote.<name>.push" is sufficient.
> 
> When "remote.<name>.push" is set I'd expect "git push" to
> choose only the 'right' remote.<name>.push lines, that is
> the lines that have the current branch as the local ref.
> "git push" would only push the current branch, which could be pushed
> to 0 or more branches on the remote side. If no "remote.<name>.push"
> contains the current branch as a local ref nothing would happen
> (maybe a warning?). If several "remote.<name>.push" have the current
> branch as the local ref the branch would be pushed to several
> remote branches. But other branches than the current branch
> would _never_ be pushed if no argument is given to 'git push'.

  I'm not really sure that it makes sense, as by default, git push won't
create a new remote ref. So unless you have a branch that matches some
remote ref, but that you never want to push, even when on it and typing
git push... there is nothing that could happen by mistake.

  So if you don't want to push such a branch, ever, then you should IMHO
not name it in a way that it matches a refspec in the first place, and
be safe.

  So I do not believe we need that extra complexity.

-- 
·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: git push (mis ?)behavior
From: Junio C Hamano @ 2007-09-28  7:07 UTC (permalink / raw)
  To: Steffen Prohaska; +Cc: Pierre Habouzit, git
In-Reply-To: <9D61974D-E08D-49F6-9C88-6BE446D53C74@zib.de>

Steffen Prohaska <prohaska@zib.de> writes:

> When "remote.<name>.push" is set I'd expect "git push" to
> choose only the 'right' remote.<name>.push lines, that is
> the lines that have the current branch as the local ref.

That would break the existing setup so it would not fly as a
default, although it could be added as an option.

^ permalink raw reply

* Re: backup or mirror a repository
From: Dan Farina @ 2007-09-28  7:20 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Johannes Schindelin, git
In-Reply-To: <7vsl4zqp8l.fsf@gitster.siamese.dyndns.org>

On Thu, 2007-09-27 at 23:27 -0700, Junio C Hamano wrote:
> You can almost do it with
> 
> 	git push --all $remote
> 
> except there is no way to automagically remove the branch you
> removed from the local repository.  For that, we would need a
> new --mirror option to "git-push".

Yup! I actually was looking for such a thing in the git-push man page,
but was unsuccessful. I was living with git-push --all for a little
while before I thought I'd ask. Unfortunately, the preferred setup
(having the backup machine actively perform fetch and prune) is not very
nice for me due to firewalls.

> 
> I think it is trivial to do for native transports, as we first
> get the list of all refs from the remote side before starting
> the transfer.  You need to change the last parameter called
> 'all' to remote.c::match_refs() into an enum ('push_all' being
> one of choices), introduce another enum 'push_mirror', and teach
> it to "match" the remote (i.e. dst) ref that does not have
> corresponding entry on our side (i.e. src) with an empty object
> name to mark it removed.  Then the part marked as "Finally, tell
> the other end!"  in send-pack.c::send_pack() will take care of
> the actual removal.

I should have a look, but if someone else wants to work on this they
shouldn't block on me: work already made me get hit by a bus on one
project (luckily I wasn't important), and the storm is not over.

Thanks,
fdr

^ permalink raw reply

* Re: Mergetool generating blank files (1.5.3)
From: David Kågedal @ 2007-09-28  8:43 UTC (permalink / raw)
  To: git
In-Reply-To: <94ccbe710709271224rc65b6f4k8b68419629ed5b45@mail.gmail.com>

"Kelvie Wong" <kelvie@ieee.org> writes:

> I've tried all of the ones that were supported, the result is the same
> -- blank files in all three windows.
>
> It is because git mergetool fails to generate these files for whatever
> reason (the filebasename.{REMOTE,LOCAL,BASE}.* files).  I don't know
> why this happens.
>
> As for merge utilities, all I need is something that looks for the
> first <<<<<, and lets me choose which version I want (either top or
> bottom), plain and simple :/  I don't even need/want a gui.

Since you are already using Emacs, let me suggest you use smerge
(together with ediff).

Use M-x smerge-mode and go over the conflicts and select which one you
want with C-c ^ m (for "mine") or C-c ^ o (for "other") or one of the
commands.  Or press C-c ^ E to run a three-window merge.

Or use the last directly by running M-x smerge-ediff.

-- 
David Kågedal

^ permalink raw reply

* Re: [PATCH] alloc_ref(): allow for trailing NUL
From: Junio C Hamano @ 2007-09-28  8:46 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Johannes Schindelin, gitster, git
In-Reply-To: <Pine.LNX.4.64.0709280046241.5926@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> On Fri, 28 Sep 2007, Johannes Schindelin wrote:
>
>> The parameter name "namelen" suggests that you pass the equivalent of
>> strlen() to the function alloc_ref().  However, this function did not
>> allocate enough space to put a NUL after the name.
>> 
>> Since struct ref does not have any member to describe the length of the
>> string, this just does not make sense.
>> 
>> So make space for the NUL.
>
> Good point, but shouldn't you then fix call sites that use strlen(name) + 
> 1?

Good point.

I audited "git grep -A2 -B4 -e alloc_ref next master" output,
and it appears almost everybody knows alloc_ref() wants the
caller to count the terminating NUL.

There however are a few gotchas.

 * There is one overallocation in connect.c, which would not
   hurt but is wasteful;

 * next:transport.c has alloc_ref(strlen(e->name)) which is a
   no-no;

Discarding Johannes's patch, the following would fix it.

---

 connect.c   |    4 ++--
 transport.c |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/connect.c b/connect.c
index 06d279e..3d5c4ab 100644
--- a/connect.c
+++ b/connect.c
@@ -72,9 +72,9 @@ struct ref **get_remote_heads(int in, struct ref **list,
 			continue;
 		if (nr_match && !path_match(name, nr_match, match))
 			continue;
-		ref = alloc_ref(len - 40);
+		ref = alloc_ref(name_len + 1);
 		hashcpy(ref->old_sha1, old_sha1);
-		memcpy(ref->name, buffer + 41, len - 40);
+		memcpy(ref->name, buffer + 41, name_len + 1);
 		*list = ref;
 		list = &ref->next;
 	}
diff --git a/transport.c b/transport.c
index 4f9cddc..3475cca 100644
--- a/transport.c
+++ b/transport.c
@@ -215,7 +215,7 @@ static struct ref *get_refs_from_bundle(const struct transport *transport)
 		die ("Could not read bundle '%s'.", transport->url);
 	for (i = 0; i < data->header.references.nr; i++) {
 		struct ref_list_entry *e = data->header.references.list + i;
-		struct ref *ref = alloc_ref(strlen(e->name));
+		struct ref *ref = alloc_ref(strlen(e->name) + 1);
 		hashcpy(ref->old_sha1, e->sha1);
 		strcpy(ref->name, e->name);
 		ref->next = result;

^ permalink raw reply related

* Re: [PATCH 1/2] Introduce remove_dir_recursively()
From: Junio C Hamano @ 2007-09-28  9:05 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: gitster, Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280606350.28395@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +int remove_dir_recursively(char *path, int len, int only_empty)
> +{
> ...
> +		namlen = strlen(e->d_name);
> +		if (len + namlen > PATH_MAX ||
> +				!memcpy(path + len, e->d_name, namlen) ||
> +				(path[len + namlen] = '\0') ||
> +				lstat(path, &st))
> +			; /* fall thru */
> +		else if (S_ISDIR(st.st_mode)) {
> +			if (!remove_dir_recursively(path, len + namlen,
> +						only_empty))
> +				continue; /* happy */
> +		} else if (!only_empty &&
> +				len + namlen + 1 < PATH_MAX &&
> +				!unlink(path))
> +			continue; /* happy, too */
> +
> +		/* path too long, stat fails, or non-directory still exists */
> +		ret = -1;
> +		break;

Is it only me who finds the first if () condition way too
convoluted and needs to read three times to convince oneself
that it is doing a sane thing?

Please, especially...

 * For $DEITY's sake, memcpy() returns pointer to dst which you
   know is not NULL. so !memcpy() is always false here, which
   might be _convenient_ for you and the compiler but not for
   a human reader of the code who needs to blink twice wondering
   if you meant !memcmp().

 * Same for (path[] = '\0'), wondering if it is misspelled
   (path[] == '\0').

^ permalink raw reply

* Re: git push (mis ?)behavior
From: Steffen Prohaska @ 2007-09-28  9:11 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Pierre Habouzit, git
In-Reply-To: <7vodfnqndc.fsf@gitster.siamese.dyndns.org>


On Sep 28, 2007, at 9:07 AM, Junio C Hamano wrote:

> Steffen Prohaska <prohaska@zib.de> writes:
>
>> When "remote.<name>.push" is set I'd expect "git push" to
>> choose only the 'right' remote.<name>.push lines, that is
>> the lines that have the current branch as the local ref.
>
> That would break the existing setup so it would not fly as a
> default, although it could be added as an option.

Do you mean 'not now' or never, i.e. not in git 1.6?

What does 'break the existing setup' means? Pushing all branches
that are configured in "remote.<name>.push" could be done by
"git push <name>". This would be in the line with the idea that
"git push" should only operate on the current branch and operations
involving other local refs should be explicitly stated.

	Steffen

^ permalink raw reply

* git-mergetool
From: Mattias Ulbrich @ 2007-09-28  8:56 UTC (permalink / raw)
  To: git


Hello everybody,

There might by a tiny typo-bug in the git-mergetool:

-----------
 case "$merge_tool" in
	kdiff3)
	    if base_present ; then
		(kdiff3 --auto --L1 "$path (Base)" -L2 "$path (Local)" --L3 "$path (Remote)" 
\
		    -o "$path" -- "$BASE" "$LOCAL" "$REMOTE" > /dev/null 2>&1)
	    else
		(kdiff3 --auto -L1 "$path (Local)" --L2 "$path (Remote)" \
		    -o "$path" -- "$LOCAL" "$REMOTE" > /dev/null 2>&1)
	    fi
-----------

In the first call to kdiff3 the argument is only -L2 but should be --L2 (two 
minus signs) like in the second call - where -L1 misses its second "-".

Thank you for a very mighty set of tool

Sincerely

	Mattias

-- 
Mattias Ulbrich
Forschungsgruppe Prof. Dr. P.H. Schmitt
Institut für Theoretische Informatik
Universität Karlsruhe

^ permalink raw reply

* Re: [PATCH 2/2] fetch/push: readd rsync support
From: Junio C Hamano @ 2007-09-28  9:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Daniel Barkalow, git
In-Reply-To: <Pine.LNX.4.64.0709280606530.28395@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> +/*
> + * path is assumed to point to a buffer of PATH_MAX bytes, and
> + * path + name_offset is expected to point to "refs/".
> + */
> +
> +static int read_loose_refs(char *path, int name_offset, struct ref **tail)
> +{
> +	DIR *dir = opendir(path);
> +	struct dirent *de;
> +	struct {
> +		struct dirent *entries;
> +		int nr, alloc;
> +	} list;
> +	int i, pathlen;
> +
> +	if (!dir)
> +		return -1;
> +
> +	memset (&list, 0, sizeof(list));
> +
> +	while ((de = readdir(dir))) {
> +		if (de->d_name[0] == '.' && (de->d_name[1] == '\0' ||
> +				(de->d_name[1] == '.' &&
> +				 de->d_name[2] == '\0')))
> +			continue;
> +		if (list.nr >= list.alloc) {
> +			list.alloc = alloc_nr(list.nr);
> +			list.entries = xrealloc(list.entries,
> +				list.alloc * sizeof(*de));
> +		}

ALLOC_GROW() not applicable here?

> +		list.entries[list.nr++] = *de;

Are you sure about this?

The last paragraph in Rationale section, in

    http://www.opengroup.org/onlinepubs/000095399/basedefs/dirent.h.html

suggests that the d_name[] member in struct dirent could be
declared at the very end of the structure as length of 1 (the
traditional trick to implement a flex-array); your assignment
from *de into entries[] would not work as expected on such an
implementation.

On Linux with glibc it appears bits/dirent.h defines dirent with
"char d_name[256]", so you may not see a breakage there, though.

You only use a list of strings (char **), don't you?

> ...
> +			if (fd < 0)
> +				continue;
> +			next = alloc_ref(strlen(path + name_offset));

And as we discussed earlier you would need one more byte here ;-).

^ permalink raw reply

* Re: git push (mis ?)behavior
From: Steffen Prohaska @ 2007-09-28  9:26 UTC (permalink / raw)
  To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20070928065823.GB19299@artemis.corp>


On Sep 28, 2007, at 8:58 AM, Pierre Habouzit wrote:

> On Fri, Sep 28, 2007 at 06:52:47AM +0000, Steffen Prohaska wrote:
>>
>> On Sep 27, 2007, at 9:22 PM, Junio C Hamano wrote:
>>
>>>
>>> So what's the desired semantics?
>>>
>>> The current semantics is:
>>>
>>>   "git push" says "you do not say to which repository?" and
>>>   consults "branch.<current>.remote" but defaults to 'origin'
>>>   if unconfigured.
>>>
>>>   "git push <name>" (or using the <name> determined as above)
>>>   says "you do not say which branches?" and consults
>>>   "remote.<name>.push" to find branches to push out, but
>>>   defaults to 'matching branches' if unconfigured.
>>>
>>> What you would want to change is the fallback behaviour for
>>> unconfigured "remote.<name>.push".  I think it is sensible to
>>> have an option to make it push only the current branch.
>>
>> I'm not sure that changing the fallback behaviour for unconfigured
>> "remote.<name>.push" is sufficient.
>>
>> When "remote.<name>.push" is set I'd expect "git push" to
>> choose only the 'right' remote.<name>.push lines, that is
>> the lines that have the current branch as the local ref.
>> "git push" would only push the current branch, which could be pushed
>> to 0 or more branches on the remote side. If no "remote.<name>.push"
>> contains the current branch as a local ref nothing would happen
>> (maybe a warning?). If several "remote.<name>.push" have the current
>> branch as the local ref the branch would be pushed to several
>> remote branches. But other branches than the current branch
>> would _never_ be pushed if no argument is given to 'git push'.
>
>   I'm not really sure that it makes sense, as by default, git push  
> won't
> create a new remote ref. So unless you have a branch that matches some
> remote ref, but that you never want to push, even when on it and  
> typing
> git push... there is nothing that could happen by mistake.
>
>   So if you don't want to push such a branch, ever, then you should  
> IMHO
> not name it in a way that it matches a refspec in the first place, and
> be safe.

I could have a couple of local branches typically pushed to a couple of
remote branches. "git push origin" would update all remote refs.

But I may also be interested to push only the current branch I'm working
on. I may also have some pending fixes on another branch that should not
be pushed now. Currently I need to do "git push origin  
<current>:<someremote>".

Let me put it as a question: How can I push changes from the current
branch to all remote refs it is configured to push to via  
"remote.<name>.push"
without pushing anything else at the same time?

	Steffen

^ permalink raw reply

* Re: [PATCH] Merge non-first refs that match first refspec
From: Junio C Hamano @ 2007-09-28  9:34 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Shawn O. Pearce, git
In-Reply-To: <Pine.LNX.4.64.0709280026240.5926@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> Beats me; Junio, what's your test case?

I can paste tomorrow (it is a clone of git.git at work).  I do
not use .git/config but .git/remotes/origin and explicit four
separate Pull: lines and going over http.

^ 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