Git development
 help / color / mirror / Atom feed
* [PATCH 6/6] builtin-reset.c: use reset_index_and_worktree()
From: Junio C Hamano @ 2008-12-06  1:54 UTC (permalink / raw)
  To: git
In-Reply-To: <1228528455-3089-6-git-send-email-gitster@pobox.com>

This makes "git reset --merge" to use the same underlying mechanism "git
checkout" uses to update the index and the work tree.

It is possible to make it use the 3-way merge fallback "git checkout -m"
allows, but this commit does not go there yet.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |    1 +
 builtin-reset.c    |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 reset.h            |   11 ++++++++
 3 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 reset.h

diff --git a/builtin-checkout.c b/builtin-checkout.c
index a08941a..d196521 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -16,6 +16,7 @@
 #include "blob.h"
 #include "xdiff-interface.h"
 #include "ll-merge.h"
+#include "reset.h"
 
 static const char * const checkout_usage[] = {
 	"git checkout [options] <branch>",
diff --git a/builtin-reset.c b/builtin-reset.c
index c0cb915..481a1cc 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -18,6 +18,7 @@
 #include "tree.h"
 #include "branch.h"
 #include "parse-options.h"
+#include "reset.h"
 
 static const char * const git_reset_usage[] = {
 	"git reset [--mixed | --soft | --hard | --merge] [-q] [<commit>]",
@@ -52,7 +53,7 @@ static inline int is_merge(void)
 	return !access(git_path("MERGE_HEAD"), F_OK);
 }
 
-static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet)
+static int reset_index_file_via_read_tree(const unsigned char *sha1, int reset_type, int quiet)
 {
 	int i = 0;
 	const char *args[6];
@@ -77,6 +78,67 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet
 	return run_command_v_opt(args, RUN_GIT_CMD);
 }
 
+static int reset_index_file(struct commit *new, int reset_type, int quiet)
+{
+	/*
+	 * SOFT reset won't even touch index nor work tree so
+	 * this function is not called.
+	 * MIXED updates the index only (should have been called
+	 * --cached), and we let "git read-tree" to do so.
+	 * HARD and MERGE corresponds to "checkout -f" and "checkout [-m]"
+	 */
+	int merge, wt_error, ret;
+	struct commit *old;
+	unsigned char head_sha1[20];
+	unsigned char *new_sha1 = new->object.sha1;
+	struct lock_file *lock_file;
+	int newfd;
+
+	/*
+	 * We play lazy and let read-tree complain if HEAD is not
+	 * readable.  Also on hard reset, HEAD does not have to be
+	 * readable.
+	 */
+	if (reset_type == MIXED ||
+	    reset_type == HARD ||
+	    get_sha1("HEAD", head_sha1) ||
+	    !(old = lookup_commit_reference_gently(head_sha1, 1)))
+		return reset_index_file_via_read_tree(new_sha1, reset_type,
+						      quiet);
+
+	lock_file = xcalloc(1, sizeof(struct lock_file));
+	newfd = hold_locked_index(lock_file, 1);
+	if (read_cache() < 0) {
+		rollback_lock_file(lock_file);
+		return reset_index_file_via_read_tree(new_sha1, reset_type,
+						      quiet);
+	}
+
+	/*
+	 * If we want "checkout -m" behaviour of falling back to
+	 * the 3-way content level merges, we could use
+	 *
+	 *	 merge = (reset_type == MERGE);
+	 *
+	 * here.
+	 */
+	merge = 0;
+
+	wt_error = 0;
+	ret = reset_index_and_worktree(0, merge, quiet, &wt_error,
+				       old, "local",
+				       new, "reset");
+	if (ret || wt_error) {
+		rollback_lock_file(lock_file);
+		return -1;
+	}
+
+	if (write_cache(newfd, active_cache, active_nr) ||
+	    commit_locked_index(lock_file))
+		return error("unable to write new index file");
+	return 0;
+}
+
 static void print_new_head_line(struct commit *commit)
 {
 	const char *hex, *body;
@@ -276,7 +338,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		if (is_merge() || read_cache() < 0 || unmerged_cache())
 			die("Cannot do a soft reset in the middle of a merge.");
 	}
-	else if (reset_index_file(sha1, reset_type, quiet))
+	else if (reset_index_file(commit, reset_type, quiet))
 		die("Could not reset index file to revision '%s'.", rev);
 
 	/* Any resets update HEAD to the head being switched to,
diff --git a/reset.h b/reset.h
new file mode 100644
index 0000000..9c42838
--- /dev/null
+++ b/reset.h
@@ -0,0 +1,11 @@
+#ifndef RESET_H
+#define RESET_H
+
+extern int reset_index_and_worktree(int force, int merge, int quiet,
+				    int *wt_error,
+				    struct commit *old_commit,
+				    const char *old_label,
+				    struct commit *new_commit,
+				    const char *new_label);
+
+#endif
-- 
1.6.1.rc1.72.ga5ce6

^ permalink raw reply related

* [PATCH 4/6] builtin-checkout.c: refactor merge_working_tree()
From: Junio C Hamano @ 2008-12-06  1:54 UTC (permalink / raw)
  To: git
In-Reply-To: <1228528455-3089-4-git-send-email-gitster@pobox.com>

The logic to bring the index and the working tree from one commit to
another is a nontrivial amount of code in this function.  Separate it out
into its own function, so that other callers can call it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |  172 +++++++++++++++++++++++++++-------------------------
 1 files changed, 89 insertions(+), 83 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index d88fce2..9c45c49 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -352,6 +352,87 @@ static int reset_tree(struct tree *tree, int quiet, int worktree, int *wt_error)
 	}
 }
 
+static int switch_trees(int merge, int quiet, int *wt_error,
+			struct commit *old_commit, const char *old_label,
+			struct commit *new_commit, const char *new_label)
+{
+	int ret;
+	struct tree_desc trees[2];
+	struct tree *tree;
+	struct unpack_trees_options topts;
+
+	memset(&topts, 0, sizeof(topts));
+	topts.head_idx = -1;
+	topts.src_index = &the_index;
+	topts.dst_index = &the_index;
+
+	topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches.";
+
+	refresh_cache(REFRESH_QUIET);
+
+	if (unmerged_cache()) {
+		error("you need to resolve your current index first");
+		return 1;
+	}
+
+	/* 2-way merge to the new branch */
+	topts.initial_checkout = is_cache_unborn();
+	topts.update = 1;
+	topts.merge = 1;
+	topts.gently = merge;
+	topts.verbose_update = !quiet;
+	topts.fn = twoway_merge;
+	topts.dir = xcalloc(1, sizeof(*topts.dir));
+	topts.dir->show_ignored = 1;
+	topts.dir->exclude_per_dir = ".gitignore";
+	tree = parse_tree_indirect(old_commit->object.sha1);
+	init_tree_desc(&trees[0], tree->buffer, tree->size);
+	tree = parse_tree_indirect(new_commit->object.sha1);
+	init_tree_desc(&trees[1], tree->buffer, tree->size);
+
+	ret = unpack_trees(2, trees, &topts);
+	if (ret == -1) {
+		/*
+		 * Unpack couldn't do a trivial merge; either give up
+		 * or do a real merge, depending on whether the merge
+		 * flag was used.
+		 */
+		struct tree *result;
+		struct tree *work;
+		struct merge_options o;
+		if (!merge)
+			return 1;
+		parse_commit(old_commit);
+
+		/* Do more real merge */
+
+		/*
+		 * We update the index fully, then write the tree from
+		 * the index, then merge the new branch with the
+		 * current tree, with the old branch as the base. Then
+		 * we reset the index (but not the working tree) to
+		 * the new branch, leaving the working tree as the
+		 * merged version, but skipping unmerged entries in
+		 * the index.
+		 */
+
+		add_files_to_cache(NULL, NULL, 0);
+		init_merge_options(&o);
+		o.verbosity = 0;
+		work = write_tree_from_memory(&o);
+
+		ret = reset_tree(new_commit->tree, quiet, 1, wt_error);
+		if (ret)
+			return ret;
+		o.branch1 = new_label;
+		o.branch2 = old_label;
+		merge_trees(&o, new_commit->tree, work,
+			    old_commit->tree, &result);
+		ret = reset_tree(new_commit->tree, quiet, 0, wt_error);
+	}
+	return ret;
+}
+
 struct branch_info {
 	const char *name; /* The short name used */
 	const char *path; /* The full name of a real branch */
@@ -376,91 +457,16 @@ static int merge_working_tree(struct checkout_opts *opts,
 	if (read_cache() < 0)
 		return error("corrupt index file");
 
-	if (opts->force) {
+	if (opts->force)
 		ret = reset_tree(new->commit->tree, opts->quiet, 1,
 				 &opts->writeout_error);
-		if (ret)
-			return ret;
-	} else {
-		struct tree_desc trees[2];
-		struct tree *tree;
-		struct unpack_trees_options topts;
-
-		memset(&topts, 0, sizeof(topts));
-		topts.head_idx = -1;
-		topts.src_index = &the_index;
-		topts.dst_index = &the_index;
-
-		topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches.";
-
-		refresh_cache(REFRESH_QUIET);
-
-		if (unmerged_cache()) {
-			error("you need to resolve your current index first");
-			return 1;
-		}
-
-		/* 2-way merge to the new branch */
-		topts.initial_checkout = is_cache_unborn();
-		topts.update = 1;
-		topts.merge = 1;
-		topts.gently = opts->merge;
-		topts.verbose_update = !opts->quiet;
-		topts.fn = twoway_merge;
-		topts.dir = xcalloc(1, sizeof(*topts.dir));
-		topts.dir->show_ignored = 1;
-		topts.dir->exclude_per_dir = ".gitignore";
-		tree = parse_tree_indirect(old->commit->object.sha1);
-		init_tree_desc(&trees[0], tree->buffer, tree->size);
-		tree = parse_tree_indirect(new->commit->object.sha1);
-		init_tree_desc(&trees[1], tree->buffer, tree->size);
-
-		ret = unpack_trees(2, trees, &topts);
-		if (ret == -1) {
-			/*
-			 * Unpack couldn't do a trivial merge; either
-			 * give up or do a real merge, depending on
-			 * whether the merge flag was used.
-			 */
-			struct tree *result;
-			struct tree *work;
-			struct merge_options o;
-			if (!opts->merge)
-				return 1;
-			parse_commit(old->commit);
-
-			/* Do more real merge */
-
-			/*
-			 * We update the index fully, then write the
-			 * tree from the index, then merge the new
-			 * branch with the current tree, with the old
-			 * branch as the base. Then we reset the index
-			 * (but not the working tree) to the new
-			 * branch, leaving the working tree as the
-			 * merged version, but skipping unmerged
-			 * entries in the index.
-			 */
-
-			add_files_to_cache(NULL, NULL, 0);
-			init_merge_options(&o);
-			o.verbosity = 0;
-			work = write_tree_from_memory(&o);
-
-			ret = reset_tree(new->commit->tree, opts->quiet, 1,
-					 &opts->writeout_error);
-			if (ret)
-				return ret;
-			o.branch1 = new->name;
-			o.branch2 = "local";
-			merge_trees(&o, new->commit->tree, work,
-				old->commit->tree, &result);
-			ret = reset_tree(new->commit->tree, opts->quiet, 0,
-					 &opts->writeout_error);
-			if (ret)
-				return ret;
-		}
-	}
+	else
+		ret = switch_trees(opts->merge, opts->quiet,
+				   &opts->writeout_error,
+				   old->commit, "local",
+				   new->commit, new->name);
+	if (ret)
+		return ret;
 
 	if (write_cache(newfd, active_cache, active_nr) ||
 	    commit_locked_index(lock_file))
-- 
1.6.1.rc1.72.ga5ce6

^ permalink raw reply related

* [PATCH 5/6] builtin-commit.c: further refactor branch switching
From: Junio C Hamano @ 2008-12-06  1:54 UTC (permalink / raw)
  To: git
In-Reply-To: <1228528455-3089-5-git-send-email-gitster@pobox.com>

This adds reset_index_and_worktree() to further factor out the logic for
switching the index and the work tree state from one commit to another to
shrink merge_working_tree() function.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |   24 ++++++++++++++++--------
 1 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 9c45c49..a08941a 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -433,6 +433,18 @@ static int switch_trees(int merge, int quiet, int *wt_error,
 	return ret;
 }
 
+int reset_index_and_worktree(int force, int merge, int quiet, int *wt_error,
+			     struct commit *old_commit, const char *old_label,
+			     struct commit *new_commit, const char *new_label)
+{
+	if (force)
+		return reset_tree(new_commit->tree, quiet, 1, wt_error);
+	else
+		return switch_trees(merge, quiet, wt_error,
+				    old_commit, old_label,
+				    new_commit, new_label);
+}
+
 struct branch_info {
 	const char *name; /* The short name used */
 	const char *path; /* The full name of a real branch */
@@ -457,14 +469,10 @@ static int merge_working_tree(struct checkout_opts *opts,
 	if (read_cache() < 0)
 		return error("corrupt index file");
 
-	if (opts->force)
-		ret = reset_tree(new->commit->tree, opts->quiet, 1,
-				 &opts->writeout_error);
-	else
-		ret = switch_trees(opts->merge, opts->quiet,
-				   &opts->writeout_error,
-				   old->commit, "local",
-				   new->commit, new->name);
+	ret = reset_index_and_worktree(opts->force, opts->merge, opts->quiet,
+				       &opts->writeout_error,
+				       old->commit, "local",
+				       new->commit, new->name);
 	if (ret)
 		return ret;
 
-- 
1.6.1.rc1.72.ga5ce6

^ permalink raw reply related

* [PATCH 2/6] read-cache.c: typofix in comment
From: Junio C Hamano @ 2008-12-06  1:54 UTC (permalink / raw)
  To: git
In-Reply-To: <1228528455-3089-2-git-send-email-gitster@pobox.com>

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 read-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index 22a8143..58d3f88 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1486,7 +1486,7 @@ int write_index(const struct index_state *istate, int newfd)
 
 /*
  * Read the index file that is potentially unmerged into given
- * index_state, dropping any unmerged entries.  Returns true is
+ * index_state, dropping any unmerged entries.  Returns true if
  * the index is unmerged.  Callers who want to refuse to work
  * from an unmerged state can call this and check its return value,
  * instead of calling read_cache().
-- 
1.6.1.rc1.72.ga5ce6

^ permalink raw reply related

* [PATCH 1/6] builtin-checkout.c: check error return from read_cache()
From: Junio C Hamano @ 2008-12-06  1:54 UTC (permalink / raw)
  To: git
In-Reply-To: <1228528455-3089-1-git-send-email-gitster@pobox.com>

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index 7f3bd7b..c2c0561 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -228,7 +228,8 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
 	newfd = hold_locked_index(lock_file, 1);
-	read_cache();
+	if (read_cache() < 0)
+		return error("corrupt index file");
 
 	if (source_tree)
 		read_tree_some(source_tree, pathspec);
@@ -371,7 +372,9 @@ static int merge_working_tree(struct checkout_opts *opts,
 	int ret;
 	struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 	int newfd = hold_locked_index(lock_file, 1);
-	read_cache();
+
+	if (read_cache() < 0)
+		return error("corrupt index file");
 
 	if (opts->force) {
 		ret = reset_tree(new->commit->tree, opts, 1);
-- 
1.6.1.rc1.72.ga5ce6

^ permalink raw reply related

* [PATCH 3/6] Make reset_tree() in builtin-checkout.c a bit more library-ish
From: Junio C Hamano @ 2008-12-06  1:54 UTC (permalink / raw)
  To: git
In-Reply-To: <1228528455-3089-3-git-send-email-gitster@pobox.com>

The function demanded an internal structure "struct checkout_opts" as its
argument, but we'd want to split this (and one of its callers) out into a
separate library.  This makes what the function wants a bit more explicit
and much less dependent on the rest of builtin-checkout.c

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-checkout.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/builtin-checkout.c b/builtin-checkout.c
index c2c0561..d88fce2 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -319,7 +319,7 @@ static void describe_detached_head(char *msg, struct commit *commit)
 	strbuf_release(&sb);
 }
 
-static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree)
+static int reset_tree(struct tree *tree, int quiet, int worktree, int *wt_error)
 {
 	struct unpack_trees_options opts;
 	struct tree_desc tree_desc;
@@ -331,14 +331,14 @@ static int reset_tree(struct tree *tree, struct checkout_opts *o, int worktree)
 	opts.reset = 1;
 	opts.merge = 1;
 	opts.fn = oneway_merge;
-	opts.verbose_update = !o->quiet;
+	opts.verbose_update = !quiet;
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 	parse_tree(tree);
 	init_tree_desc(&tree_desc, tree->buffer, tree->size);
 	switch (unpack_trees(1, &tree_desc, &opts)) {
 	case -2:
-		o->writeout_error = 1;
+		*wt_error = 1;
 		/*
 		 * We return 0 nevertheless, as the index is all right
 		 * and more importantly we have made best efforts to
@@ -377,7 +377,8 @@ static int merge_working_tree(struct checkout_opts *opts,
 		return error("corrupt index file");
 
 	if (opts->force) {
-		ret = reset_tree(new->commit->tree, opts, 1);
+		ret = reset_tree(new->commit->tree, opts->quiet, 1,
+				 &opts->writeout_error);
 		if (ret)
 			return ret;
 	} else {
@@ -446,14 +447,16 @@ static int merge_working_tree(struct checkout_opts *opts,
 			o.verbosity = 0;
 			work = write_tree_from_memory(&o);
 
-			ret = reset_tree(new->commit->tree, opts, 1);
+			ret = reset_tree(new->commit->tree, opts->quiet, 1,
+					 &opts->writeout_error);
 			if (ret)
 				return ret;
 			o.branch1 = new->name;
 			o.branch2 = "local";
 			merge_trees(&o, new->commit->tree, work,
 				old->commit->tree, &result);
-			ret = reset_tree(new->commit->tree, opts, 0);
+			ret = reset_tree(new->commit->tree, opts->quiet, 0,
+					 &opts->writeout_error);
 			if (ret)
 				return ret;
 		}
-- 
1.6.1.rc1.72.ga5ce6

^ permalink raw reply related

* [PATCH 0/6] Reusing "checkout [-m]" logic for "reset --merge"
From: Junio C Hamano @ 2008-12-06  1:54 UTC (permalink / raw)
  To: git; +Cc: Linus Torvalds

"checkout" that switches the branches, and "reset" that updates the HEAD,
have some similarities.  Their major difference is that "checkout" affects
which branch HEAD symref points at, while "reset" affects what commit such
a branch pointed at by the HEAD symref points at.  Interestingly, when
your HEAD is detached, they operate on the same thing.

They both change the commit pointed at by your HEAD from one commit to
another.  And what "checkout" and "reset --merge" do to the index and the
work tree while doing so are exactly the same.  Keep your local changes,
while updating everything else from what is in the current commit to the
one we are switching to.

There is one difference, though.

"reset --merge" does not have an equivalent of "checkout -m" that falls
back to three-way merge to resolve the conflicts at the content level.
This series, which is still a WIP, is an attempt to do so.

There are few things to notice that this series doesn't do:

 - What "reset --hard" and "checkout -f HEAD" do to the index and work
   tree (at least in naive thinking) ought to be the same.  This series
   does not attempt to unify these.

 - "reset --merge" is left as posted by Linus, and it does only "git
   checkout" equivalent, without "-m" (yet).

The latter can be enabled by changing a single line (see comment in
reset_index_file() in [Patch 6/6]), but I haven't done so yet.

While investigating for the former possibility, I noticed one interesting
thing what "checkout -f" does *NOT* do.  Starting with an index with
conflicts, "git checkout -f HEAD" keeps higher stage entries.

For example, you can insert "exit" before the "D/F resolve" test in t1004,
run the test (this leaves a handful higher stages in the index), go to the
trash directory and say "git checkout -f HEAD".

It leaves a single stage #1 entry (subdir/file2).  It probably is a bug in
unpack-trees, but I didn't take a very deep look at it.

Junio C Hamano (6):
  builtin-checkout.c: check error return from read_cache()
  read-cache.c: typofix in comment
  Make reset_tree() in builtin-checkout.c a bit more library-ish
  builtin-checkout.c: refactor merge_working_tree()
  builtin-commit.c: further refactor branch switching
  builtin-reset.c: use reset_index_and_worktree()

 builtin-checkout.c |  193 +++++++++++++++++++++++++++++-----------------------
 builtin-reset.c    |   66 +++++++++++++++++-
 read-cache.c       |    2 +-
 reset.h            |   11 +++
 4 files changed, 183 insertions(+), 89 deletions(-)
 create mode 100644 reset.h

^ permalink raw reply

* [PATCH] t9129: Prevent test failure if no UTF-8 locale
From: applehq @ 2008-12-06  1:31 UTC (permalink / raw)
  To: git

Commit 16fc08e2d86dad152194829d21bc55b2ef0c8fb1 introduced a
test that failed if the en_US.UTF-8 locale was not installed.

Make the test find a UTF-8 locale, and expect failure.

Signed-off-by: applehq <theappleman@gmail.com>
---
 t/t9129-git-svn-i18n-commitencoding.sh |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/t/t9129-git-svn-i18n-commitencoding.sh b/t/t9129-git-svn-i18n-commitencoding.sh
index 938b7fe..87ecdd0 100755
--- a/t/t9129-git-svn-i18n-commitencoding.sh
+++ b/t/t9129-git-svn-i18n-commitencoding.sh
@@ -15,8 +15,9 @@ compare_git_head_with () {
 }
 
 compare_svn_head_with () {
-	LC_ALL=en_US.UTF-8 svn log --limit 1 `git svn info --url` | \
-		sed -e 1,3d -e "/^-\{1,\}\$/d" >current &&
+	LC_ALL=`locale -a | grep -i utf | head -1` \
+		svn log --limit 1 `git svn info --url` | \
+			sed -e 1,3d -e "/^-\{1,\}\$/d" >current &&
 	test_cmp current "$1"
 }
 
@@ -60,7 +61,7 @@ do
 	'
 done
 
-test_expect_success 'ISO-8859-1 should match UTF-8 in svn' '
+test_expect_failure 'ISO-8859-1 should match UTF-8 in svn' '
 (
 	cd ISO-8859-1 &&
 	compare_svn_head_with "$TEST_DIRECTORY"/t3900/1-UTF-8.txt
@@ -69,7 +70,7 @@ test_expect_success 'ISO-8859-1 should match UTF-8 in svn' '
 
 for H in EUCJP ISO-2022-JP
 do
-	test_expect_success '$H should match UTF-8 in svn' '
+	test_expect_failure '$H should match UTF-8 in svn' '
 	(
 		cd $H &&
 		compare_svn_head_with "$TEST_DIRECTORY"/t3900/2-UTF-8.txt
-- 
1.6.1.rc1.53.g455b

^ permalink raw reply related

* Re: why not preserve file permissions?
From: Jeff King @ 2008-12-06  1:29 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: jidanni, git
In-Reply-To: <7vljuuxn66.fsf@gitster.siamese.dyndns.org>

On Fri, Dec 05, 2008 at 02:38:41PM -0800, Junio C Hamano wrote:

> Actually in a very early days, git used to record the full (mode & 0777)
> for blobs.
> 
> Once people started using git, everybody realized that it had a very
> unpleasant side effect that the resulting tree depended on user's umasks,

Note, of course that "everybody" in your sentence is "everybody who was
using git as a source control mechanism." I think as git has grown,
people have wanted to use it to version other things, such as dot files
in their home directory or the contents of /etc, two situations where
file metadata might be of as much interest as the file content.

And I don't see a real problem in a config switch or two to handle a
vastly different workflow like that, if it can be done with minimal
intrusion. For example, now we throw away most of the mode bits; it
would probably only be a few lines of code difference to keep those mode
bits, people who turned on that config option would presumably know what
they are doing, and performance for the usual workflow would be
unaffected.

_But_ I think people who ask for just permission bits aren't really
thinking things through. Those bits are only one part of the metadata.
There's file owner and group. There are timestamps. There are
non-regular files with their own associated metadata (like major/minor
device numbers). There are extended attributes, which covers things like
ACLs (and I don't even know if there's a standard representation that
can cover the ACLs for all platforms we support).

And those things _aren't_ as easy as flipping a config switch. They mean
changes to the fundamental data structures of git, and all of the pain
that entails: a lot of code, interoperability annoyances, and probably
performance impacts for unrelated workflows.

So I am sympathetic to somebody who, after thinking it through, has a
use case that just requires tracking permissions bits and wants a config
option for git to do that. But the commonly given examples seem much
better served by a _full_ metadata solution that lives on top of git,
like metastore. I haven't seen a compelling argument for just handling
permission bits.

-Peff

^ permalink raw reply

* Re: [RFCv3 1/2] gitweb: add patch view
From: Junio C Hamano @ 2008-12-06  1:26 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Giuseppe Bilotta, git, Petr Baudis
In-Reply-To: <200812060209.42906.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

> But we have also 'snapshot' feature, which like 'patches' is not simply
> on/off but is configurable feature, like 'patches' adds new action and
> does modify existing actions only by adding extra links... and which is
> enabled by default.

If my "git log" is telling the true story, "snapshot" was introduced in
cb9c6e5 (gitweb: Support for snapshot, 2006-08-17) and it was disabled by
default.

The "feature" mechanism was introduced after that, with ddb8d90 (gitweb:
Make blame and snapshot a feature., 2006-08-20).  To avoid surprises,
"blame" were marked disabled by default to match the then-current default.
"snapshot" somehow ended up enabled with that change, though, which might
have been a mistake.

But "we erred before" is not a good excuse to deliberately err again, is
it?

^ permalink raw reply

* Re: [RFCv3 1/2] gitweb: add patch view
From: Jakub Narebski @ 2008-12-06  1:09 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Giuseppe Bilotta, git, Petr Baudis
In-Reply-To: <7vabbaxh8y.fsf@gitster.siamese.dyndns.org>

Dnia sobota 6. grudnia 2008 01:46, Junio C Hamano napisał:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
>>> +	# The maximum number of patches in a patchset generated in patch
>>> +	# view. Set this to 0 or undef to disable patch view, or to a
>>> +	# negative number to remove any limit.
>>> +	'patches' => {
>>> +		'override' => 1,
>>> +		'default' => [16]},
>>>  );
> >
> > [...]  Also features are usually not overridable
> > by default, which reduces load a tiny bit (by _possibly_ not reading
> > config, although that shouldn't matter much now with reading whole
> > commit using single call to git-config, and not one call per variable).
> > And I think the default might be set larger: 'log' view generates
> > as big if not bigger load, and it is split into 100 commits long
> > pages.
> 
> I do not think defaulting to 'no' for overridability nor defaulting a new
> feature to 'disabled' have much to do with the load, but they are more
> about the principle of least surprise.  Somebody who runs gitweb in the
> playpen he was given on the server shouldn't be getting a phone call from
> his users late at night complaining that the page his gitweb serves look
> different and has one extra link per each line, only because the sysadmin
> of the server decided to update git to 1.6.1 without telling him.
> 
> Once a new version capable of serving a new feature is introduced, he can
> plan, announce and deploy by switching the feature on in his gitweb
> configuration file.
> 
> Some things, like sitewide default css changes, cannot be made disabled
> by default.  But a new feature can easily be kept disabled by default not
> to cause needless surprises.

Well, 'search', 'grep' and 'pickaxe' features are enabled by default,
but I think it is cause by the fact that they predate %features.

But we have also 'snapshot' feature, which like 'patches' is not simply
on/off but is configurable feature, like 'patches' adds new action and
does modify existing actions only by adding extra links... and which is
enabled by default.

So there... ;-)
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [RFCv3 2/2] gitweb: links to patch action in commitdiff and shortlog view
From: Jakub Narebski @ 2008-12-06  0:53 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <1228345188-15125-3-git-send-email-giuseppe.bilotta@gmail.com>

On Wed, 3 Dec 2008, Giuseppe Bilotta wrote:

> In shortlog view, a link to the patchset is only offered when the number
> of commits shown is less than the allowed maximum number of patches.
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  gitweb/gitweb.perl |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index c9abfcf..ec8fc7d 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -5083,6 +5083,11 @@ sub git_commit {
>  			} @$parents ) .
>  			')';
>  	}
> +	if (gitweb_check_feature('patches')) {
> +		$formats_nav .= " | " .
> +			$cgi->a({-href => href(action=>"patch", -replay=>1)},
> +				"patch");
> +	}
>  
>  	if (!defined $parent) {
>  		$parent = "--root";
> @@ -5415,6 +5420,11 @@ sub git_commitdiff {
>  		$formats_nav =
>  			$cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
>  			        "raw");
> +		if ($patch_max) {
> +			$formats_nav .= " | " .
> +				$cgi->a({-href => href(action=>"patch", -replay=>1)},
> +					"patch");
> +		}
>  
>  		if (defined $hash_parent &&
>  		    $hash_parent ne '-c' && $hash_parent ne '--cc') {

In the above two hunks 'patch' view functions as "git show --pretty=email"
text/plain equivalent, but this duplicates a bit 'commitdiff_plain'
functionality.  Well, 'commitdiff_plain' has currently some errors,
but...

> @@ -5949,6 +5959,14 @@ sub git_shortlog {
>  			$cgi->a({-href => href(-replay=>1, page=>$page+1),
>  			         -accesskey => "n", -title => "Alt-n"}, "next");
>  	}
> +	my $patch_max = gitweb_check_feature('patches');
> +	if ($patch_max) {
> +		if ($patch_max < 0 || @commitlist <= $patch_max) {
> +			$paging_nav .= " &sdot; " .
> +				$cgi->a({-href => href(action=>"patch", -replay=>1)},
> +					@commitlist > 1 ? "patchset" : "patch");
> +		}
> +	}

Here 'patch' view functions as "git format-patch", able to be downloaded
and fed to git-am.  Perhaps the action should also be named 'patches'
here?; it could lead to the same function.

By the way, there is subtle bug in above link. If shortlog view is less
than $patch_max commits long, but it is because the history for a given
branch (or starting from given commit) is so short, and not because
there is cutoff $hash_parent set, the 'patchset' view wouldn't display
plain text equivalent view, but only patch for top commit.

I assume that the link is only for 'shortlog' view, and not also for
'log' and 'history' views because 'shortlog' is the only log-like view
which support $hash_parent?

>  
>  	git_header_html();
>  	git_print_page_nav('shortlog','', $hash,$hash,$hash, $paging_nav);
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [RFCv3 1/2] gitweb: add patch view
From: Junio C Hamano @ 2008-12-06  0:46 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Giuseppe Bilotta, git, Petr Baudis
In-Reply-To: <200812060134.22959.jnareb@gmail.com>

Jakub Narebski <jnareb@gmail.com> writes:

>> +	# The maximum number of patches in a patchset generated in patch
>> +	# view. Set this to 0 or undef to disable patch view, or to a
>> +	# negative number to remove any limit.
>> +	'patches' => {
>> +		'override' => 1,
>> +		'default' => [16]},
>>  );
>
> You need to set "'sub' => \&feature_patches" for feature to be 
> override-able at all.  Also features are usually not overridable
> by default, which reduces load a tiny bit (by _possibly_ not reading
> config, although that shouldn't matter much now with reading whole
> commit using single call to git-config, and not one call per variable).
> And I think the default might be set larger: 'log' view generates
> as big if not bigger load, and it is split into 100 commits long
> pages.

I do not think defaulting to 'no' for overridability nor defaulting a new
feature to 'disabled' have much to do with the load, but they are more
about the principle of least surprise.  Somebody who runs gitweb in the
playpen he was given on the server shouldn't be getting a phone call from
his users late at night complaining that the page his gitweb serves look
different and has one extra link per each line, only because the sysadmin
of the server decided to update git to 1.6.1 without telling him.

Once a new version capable of serving a new feature is introduced, he can
plan, announce and deploy by switching the feature on in his gitweb
configuration file.

Some things, like sitewide default css changes, cannot be made disabled
by default.  But a new feature can easily be kept disabled by default not
to cause needless surprises.

^ permalink raw reply

* Re: [RFCv3 1/2] gitweb: add patch view
From: Jakub Narebski @ 2008-12-06  0:34 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Petr Baudis, Junio C Hamano
In-Reply-To: <1228345188-15125-2-git-send-email-giuseppe.bilotta@gmail.com>

I'm sorry for the delay reviewing this patch series...

On Wed, 3 Dec 2008, Giuseppe Bilotta wrote:

> The manually-built email format in commitdiff_plain output is not
> appropriate for feeding git-am, because of two limitations:
>  * when a range of commits is specified, commitdiff_plain publishes a
>    single patch with the message from the first commit, instead of a
>    patchset,

This is because 'commitdiff_plain' wasn't _meant_ as patch series view,
to be fed to git-am. Actually it is a bit cross between "git show"
result with '--pretty=email' format, and "git diff" between two commits,
to be fed to git-apply or GNU patch.

Nevertheless the above reasoning doesn't need to be put in a commit
message. But it explains why new 'patch' / 'patchset' view is needed:
because there was no equivalent.

>  * in either case, the patch summary is replicated both as email subject
>    and as first line of the email itself, resulting in a doubled summary
>    if the output is fed to git-am.

This is independent issue which is I think worth correcting anyway,
unless we decide to scrap 'commitdiff_plain' view altogether.
But I think we would want some text/plain patch view to be applied
by GNU patch (for example in RPM .spec file).

> 
> We thus create a new view that can be fed to git-am directly by exposing
> the output of git format-patch directly. This allows patch exchange and
> submission via gitweb.

Which is in my opinion a very good idea.

> 
> A configurable limit is imposed on the number of commits which will be
> included in a patchset, to prevent DoS attacks on the server. Setting
> the limit to 0 will disable the patch view, setting it to a negative
> number will remove the limit.

Note that this limit doesn't need to be lower than page length limit,
i.e. currently 100 commits, as new 'patch' view doesn't generate
greater load than 'log' view (which is split into 100 commits long
pages).

> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  gitweb/gitweb.perl |   41 ++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 40 insertions(+), 1 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 2738643..c9abfcf 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -329,6 +329,13 @@ our %feature = (
>  	'ctags' => {
>  		'override' => 0,
>  		'default' => [0]},
> +
> +	# The maximum number of patches in a patchset generated in patch
> +	# view. Set this to 0 or undef to disable patch view, or to a
> +	# negative number to remove any limit.
> +	'patches' => {
> +		'override' => 1,
> +		'default' => [16]},
>  );

You need to set "'sub' => \&feature_patches" for feature to be 
override-able at all.  Also features are usually not overridable
by default, which reduces load a tiny bit (by _possibly_ not reading
config, although that shouldn't matter much now with reading whole
commit using single call to git-config, and not one call per variable).
And I think the default might be set larger: 'log' view generates
as big if not bigger load, and it is split into 100 commits long
pages.

>  
>  sub gitweb_get_feature {
> @@ -503,6 +510,7 @@ our %actions = (
>  	"heads" => \&git_heads,
>  	"history" => \&git_history,
>  	"log" => \&git_log,
> +	"patch" => \&git_patch,
>  	"rss" => \&git_rss,
>  	"atom" => \&git_atom,
>  	"search" => \&git_search,
> @@ -5386,6 +5394,12 @@ sub git_blobdiff_plain {
>  
>  sub git_commitdiff {
>  	my $format = shift || 'html';
> +
> +	my $patch_max = gitweb_check_feature('patches');

Wouldn't it be better to name this variable $max_patchset_size, or
something like that? I'm not saying that this name is bad, but I'm
wondering if it could be better...

> +	if ($format eq 'patch') {
> +		die_error(403, "Patch view not allowed") unless $patch_max;

So undef and '' means "not allowed", beside '0'? I think it is a good
idea.

And it would be better (although now it is not as much performance hit)
to move "gitweb_check_feature('patches');" inside conditional:

+	my $patch_max;
+	if ($format eq 'patch') {
+		$patch_max = gitweb_check_feature('patches');
+		die_error(403, "Patch view not allowed") unless $patch_max;
+	}
+

> +	}
> +
>  	$hash ||= $hash_base || "HEAD";
>  	my %co = parse_commit($hash)
>  	    or die_error(404, "Unknown commit object");
> @@ -5396,6 +5410,7 @@ sub git_commitdiff {
>  	}
>  	# we need to prepare $formats_nav before almost any parameter munging
>  	my $formats_nav;
> +

Hmmm... some accidental change, it looks like.

>  	if ($format eq 'html') {
>  		$formats_nav =
>  			$cgi->a({-href => href(action=>"commitdiff_plain", -replay=>1)},
> @@ -5483,7 +5498,12 @@ sub git_commitdiff {
>  		open $fd, "-|", git_cmd(), "diff-tree", '-r', @diff_opts,
>  			'-p', $hash_parent_param, $hash, "--"
>  			or die_error(500, "Open git-diff-tree failed");
> -
> +	} elsif ($format eq 'patch') {
> +		open $fd, "-|", git_cmd(), "format-patch", '--encoding=utf8',
> +			'--stdout', $patch_max > 0 ? "-$patch_max" : (),

> +			$hash_parent ? ('-n', "$hash_parent..$hash") :
> +			('--root', '-1', $hash)

This bit makes 'patch' view behave bit differently than git-format-patch,
which I think is a good idea: namely it shows single patch if there is
no cutoff.  This should be mentioned in commit message, and perhaps
even in a comment in code.

Beside, if you show only single patch because $hash_parent is not set,
you don't need to examine $patch_max nor set limit, because you use '-1'.
Currently if $patch_max > 0 and !$hash_parent, you pass limit for the
number of patches twice.  This I think is harmless but...

> +			or die_error(500, "Open git-format-patch failed");
>  	} else {
>  		die_error(400, "Unknown commitdiff format");
>  	}
> @@ -5532,6 +5552,15 @@ sub git_commitdiff {
>  			print to_utf8($line) . "\n";
>  		}
>  		print "---\n\n";
> +	} elsif ($format eq 'patch') {
> +		my $filename = basename($project) . "-$hash.patch";
> +
> +		print $cgi->header(
> +			-type => 'text/plain',
> +			-charset => 'utf-8',
> +			-expires => $expires,
> +			-content_disposition => 'inline; filename="' . "$filename" . '"');

Good.

> +		# TODO add X-Git-Tag/X-Git-Url headers in a sensible way

Sensible way would mean modifying git-format-patch to be able to add
extra headers via command option, just like it is now possible via
config variable format.headers, I think.  Because there are no surefire
markers of where one patch ends and another begins: commit message is
free text, and can contain diff... although if it contains diff
separator '---' then git-am would have problem with patch; or at least
even assuming sane commit messages it is not easy.

Also I think that only X-Git-Url makes sense, and it is per whole
patchset (whole 'patch' view output) and not for each individual
patch.

>  	}
>  
>  	# write patch
> @@ -5553,6 +5582,11 @@ sub git_commitdiff {
>  		print <$fd>;
>  		close $fd
>  			or print "Reading git-diff-tree failed\n";
> +	} elsif ($format eq 'patch') {
> +		local $/ = undef;
> +		print <$fd>;
> +		close $fd
> +			or print "Reading git-format-patch failed\n";
>  	}
>  }
>  
> @@ -5560,6 +5594,11 @@ sub git_commitdiff_plain {
>  	git_commitdiff('plain');
>  }
>  
> +# format-patch-style patches
> +sub git_patch {
> +	git_commitdiff('patch');
> +}
> +

O.K.

>  sub git_history {
>  	if (!defined $hash_base) {
>  		$hash_base = git_get_head_hash($project);
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: git-svn feature request: exclude certain subpaths on clone
From: Eric Wong @ 2008-12-06  0:13 UTC (permalink / raw)
  To: Wade Berrier; +Cc: git
In-Reply-To: <bbbeeccd0812040843p3e5547c4tac88b0d01562a37f@mail.gmail.com>

Wade Berrier <wberrier@gmail.com> wrote:
> Hi,
> 
> Consider the following example layout:
> 
> trunk/src
> trunk/big_fat_binary_blobs
> trunk/doc
> 
> I think it would be really nice to be able to tell git-svn to ignore
> 'big_fat_binary_blobs' while keeping 'src' and 'doc'.
> 
> I know someone is thinking, "Why did you check in
> 'big_fat_binary_blobs' in the first place?"  In this case, the
> repository is out of my control.  For the svn users, it's not that big
> of a deal since they only get one HEAD version of the binary_blobs.
> But when trying to clone with git-svn, I repeatedly get out of memory
> and packing errors (every 1000 commits) when packing several revisions
> of these binary_blobs.  (Now, that may be a bug in of itself... which
> can reproduced by creating an svn repo with several revisions of
> KNOPPIX at the same path, followed by a git svn clone )
> 
> Anyway, I still think it may be useful to be able to ignore certain
> paths on a clone.  In thinking about the implementation details, I
> figure probably the best approach would be to manually purge the
> unwanted path after it has been fetched, but before it is committed.
> That way, if a commit contains changes in paths that are both wanted
> and unwanted, the commit could be 'pruned'.
> 
> I've looked at the git-svn script a little, but wanted to solicit
> feedback and ideas before continuing further.

Maybe... What about git-filter-branch?

I realize that doing it at the git-svn level can save bandwidth; but it
might not be possible with the way SVN deltas work...

I'll try to get around to splitting git-svn.perl out to separate source
files this weekend so it's easier to navigate.

-- 
Eric Wong

^ permalink raw reply

* Re: why not preserve file permissions?
From: Daniel Barkalow @ 2008-12-06  0:05 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <87tz9igzbr.fsf@jidanni.org>

On Sat, 6 Dec 2008, jidanni@jidanni.org wrote:

> Why not preserve permissions the way you find them, instead of just
> using 644 and 755? It certainly couldn't be more complicated than what
> you are doing now, and that way one could do things like use git to
> update system administration files across a sneakernet containing e.g.,
> # dlocate -lsconf exim4-config|sed 's/ .*//'|sort -u
> -rw-r-----
> -rw-r--r--
> -rwxr-xr-x
> 
> > git was made for tracking source code, not 640 files.
> 
> On the sneakernet no public patches would be sent, and the
> administrator would remember to make the sensitive .git directories
> 700. And sure, git should enforce umask or no set-uid or whatever when
> doing a checkout etc. The deluxe edition of git could even print a
> warning: "you are trying to track a 640 file but your .git permissions
> are less restrictive." However I recommend no premium or deluxe
> editions for now.

The fundamental issue is that git is intended to support development, 
which is to say that users have to be able to create trees that reflect 
the content that they intend the project to have, rather than only content 
that they can represent directly in their working tree. In order to do 
development on an exim configuration, you may want to be able to set 
permissions and ownership that will only make sense in the production 
environment. So it's actually better to have some setup where you've got a 
metadata file (that you can make arbitrary changes to without any 
particular hassle) in addition to the content files, and a script for 
putting the files in place with the metadata applied on the live server.

That is, preserving metadata would allow you to use git as a really 
complicated version of tar, but wouldn't give you the advantages of using 
git.

	-Daniel
*This .sig left intentionally blank*

^ permalink raw reply

* Re: Git weekly news: 2008-49
From: Junio C Hamano @ 2008-12-05 22:42 UTC (permalink / raw)
  To: Nanako Shiraishi
  Cc: Junio C Hamano, Felipe Contreras, Jakub Narebski, git list
In-Reply-To: <20081206071822.6117@nanako3.lavabit.com>

Nanako Shiraishi <nanako3@lavabit.com> writes:

> Quoting "Junio C Hamano" <gitster@pobox.com>:
>
>> Expecting people to apply for an account and write for that page would not
>> fly.  As Felipe said himself, many people already have their own blog.
>>
>> Having said all that, I am not sure "Planet" would work for the git
>> community as well as it would for others.  I do not know of many core-ish
>> people write on git on their blogs (and I know at least two core-ish
>> people who flatly say "blogging is a waste of time").
>
> But are you not "core-ish", with your own pages?
>
>   http://gitster.livejournal.com/tag/git

Eh, yes but I do not write about git very often.  It is more useful to
mine the mailing list archive than following the above URL if somebody is
interested in my thoughts on git.

^ permalink raw reply

* Re: why not preserve file permissions?
From: Junio C Hamano @ 2008-12-05 22:38 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <87tz9igzbr.fsf@jidanni.org>

jidanni@jidanni.org writes:

> Why not preserve permissions the way you find them, instead of just
> using 644 and 755? It certainly couldn't be more complicated than what
> you are doing now, and that way one could do things like use git to
> update system administration files across a sneakernet containing e.g.,
> # dlocate -lsconf exim4-config|sed 's/ .*//'|sort -u
> -rw-r-----
> -rw-r--r--
> -rwxr-xr-x

Actually in a very early days, git used to record the full (mode & 0777)
for blobs.

Once people started using git, everybody realized that it had a very
unpleasant side effect that the resulting tree depended on user's umasks,
because one person records a blob with mode 664 and the next person who
modifies the file would record with mode 644, and it made it very hard to
keep track of meaningful changes to the source code.  This issue was fixed
long time ago with commit e447947 (Be much more liberal about the file
mode bits., 2005-04-16).

^ permalink raw reply

* Re: Git weekly news: 2008-49
From: Jakub Narebski @ 2008-12-05 22:33 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Junio C Hamano, git list
In-Reply-To: <94a0d4530812051349tff33aaan76e29402215dcae2@mail.gmail.com>

On Fri, 5 Dec 2008, Felipe Contreras wrote:
> On Fri, Dec 5, 2008 at 10:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> > Jakub Narebski <jnareb@gmail.com> writes:

> > Felipe's page currently is a random collection of links, and other than
> > their titles, there is no indication for readers to judge which link is
> > worth clicking and reading.  It does not even mention who wrote each
> > piece, let alone editorial comments (e.g. "This is worth reading") like
> > you added.  When you click one of them in order to read it, you leave the
> > "list of links".  That is not how navigation (the click and thought flow
> > for the readers) usually works in a "Planet".
> 
> Lets remember that this is the first try, and there's many more links
> that what would fit in any given week, but I just didn't want to leave
> them out.
> 
> Maybe for the next weeks I'll do a bit of explanation about each link,
> lets see. 

Not necessary a bit of explanation about _each_ link, but at least put
them in rough categories (as a kind of you did), separating praise,
explanation/documentation, web and Ruby stuff and solving, and solving
specific issue (like those on BlogPosts wiki page).
 
 
> I actually propose two things:

[...]
> b) git blog
> 
> A blog can be shared by a bunch of people, much line online news
> sites. Junio could write a post once for each release, for example,
> without having to setup his own blog, maybe somebody else can
> copy-paste "What's cooking in git.git", or any kind of semi-official
> announcement.

Junio has its own blog: http://gitster.livejournal.com

And there is RSS feed for [ANNOUNCE] posts at http://gitrss.q42.co.uk
and "What's cooking in git.git" and the like.

> And I'm sure there will be one or two developers who wouldn't mind
> sharing their frustrations and/or visions.

Would they want to write blog posts? Git is very much email driven
community...

[...]
> For b) I just need interested people to send me their emails.

To apply for accounts, isn't it?
-- 
Jakub Narebski
Poland

^ permalink raw reply

* Re: [StGit] what happened to stg diff -r /{bottom|top}?
From: Catalin Marinas @ 2008-12-05 22:26 UTC (permalink / raw)
  To: Dan Williams; +Cc: git
In-Reply-To: <49395CC9.2010107@intel.com>

2008/12/5 Dan Williams <dan.j.williams@intel.com>:
> I noticed that with the latest StGit:
> # stg diff -r /bottom
> stg diff: /bottom: Unknown patch or revision name
>
> Before I bisect to find where this disappeared, is there a different syntax
> I should be using?

In the development (master) branch, we changed the syntax to make it
cleaner. A patch (its top) can be identified as [<branch>:]<patch>. It
also supports standard Git suffixes like ^, ~{...} etc. To access the
bottom of a patch, just use [<branch>:]<patch>^.

The bottom of the top patch could be accessed as HEAD^ (I've been
thinking about only allowing the caret for this, without the HEAD but
I didn't have time to try it).

-- 
Catalin

^ permalink raw reply

* Re: why not preserve file permissions?
From: Jakub Narebski @ 2008-12-05 22:23 UTC (permalink / raw)
  To: jidanni; +Cc: git
In-Reply-To: <87tz9igzbr.fsf@jidanni.org>

jidanni@jidanni.org writes:

> Why not preserve permissions the way you find them, instead of just
> using 644 and 755? It certainly couldn't be more complicated than what
> you are doing now, and that way one could do things like use git to
> update system administration files across a sneakernet containing e.g.,
> # dlocate -lsconf exim4-config|sed 's/ .*//'|sort -u
> -rw-r-----
> -rw-r--r--
> -rwxr-xr-x
> 
> > git was made for tracking source code, not 640 files.
> 
> On the sneakernet no public patches would be sent, and the
> administrator would remember to make the sensitive .git directories
> 700. And sure, git should enforce umask or no set-uid or whatever when
> doing a checkout etc. The deluxe edition of git could even print a
> warning: "you are trying to track a 640 file but your .git permissions
> are less restrictive." However I recommend no premium or deluxe
> editions for now.

Git was made for tracking _source code_ among _different_ machines, as
a way to collaborate in development.  This means that different users
of the same repository can have and usually have different user,
different user-id, different group, and perhaps different group
permission settings on a files.  Git supports only ordinary files,
executable ordinary files, directories and symbolic links for
a reason: otherwise it would be very easy to propagate spurious
permission changes because some contributor has different setup.

Side note: git can be used on operating systems / with filesystems
which do not have UNIX notion of permissions (think MS Windows).


Also changes _now_ to the repository format (and keeping full
permissions would require changing format of 'tree' objects) would
have to be extremely well substantiated.  And using git to store
system administration files, as a kind of cross between (amost) single
machine version control and backup system is possible even now: see
Etckeeper and IsiSetup tools (you can find them at git wiki at
http://git.or.cz/gitwiki/InterfacesFrontendsAndTools page).  Or you
can use tools like CFEngine...

HTH.
-- 
Jakub Narebski
Poland
ShadeHawk on #git

^ permalink raw reply

* Re: Git weekly news: 2008-49
From: Nanako Shiraishi @ 2008-12-05 22:18 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Felipe Contreras, Jakub Narebski, git list
In-Reply-To: <7v3ah2z6jh.fsf@gitster.siamese.dyndns.org>

Quoting "Junio C Hamano" <gitster@pobox.com>:

> Expecting people to apply for an account and write for that page would not
> fly.  As Felipe said himself, many people already have their own blog.
>
> Having said all that, I am not sure "Planet" would work for the git
> community as well as it would for others.  I do not know of many core-ish
> people write on git on their blogs (and I know at least two core-ish
> people who flatly say "blogging is a waste of time").

But are you not "core-ish", with your own pages?

  http://gitster.livejournal.com/tag/git

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

^ permalink raw reply

* Re: Git weekly news: 2008-49
From: Santi Béjar @ 2008-12-05 22:11 UTC (permalink / raw)
  To: Felipe Contreras; +Cc: Jakub Narebski, git list
In-Reply-To: <94a0d4530812051357m4f0bc591n802c41a78e698b3b@mail.gmail.com>

2008/12/5 Felipe Contreras <felipe.contreras@gmail.com>:
> On Fri, Dec 5, 2008 at 11:44 PM, Santi Béjar <santi@agolina.net> wrote:
>> 2008/12/5 Felipe Contreras <felipe.contreras@gmail.com>:
>>> On Fri, Dec 5, 2008 at 6:02 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>>
>> [...]
>>
>>>>> But here are the links anyway. The order is rather random.
>>>>
>>>> Moreover the _quality_ of those links is very random.
>>>
>>> Exactly, I didn't choose them, that's what people have been tagging as
>>> "git" in delicious.com. I'm subscribed to the RSS feed and saving the
>>> ones that appear a lot.
>>>
>>> In fact I don't like some of them, but that's what the "public" finds
>>> interesting.
>>
>> So I don't see the value of such a list. You can go to delicious and
>> get it.
>
> Try it. You can't see which ones are new, which are completely
> irrelevant. There are duplicates and you can't see the popularity /
> freshness ratio, or "hotness", never mind the most popular this week.
>
> Apparently some people already found interesting links they haven't
> seen before, so at lest there's value for them.

Maybe they should. Sorry for my ignorance.

>
>> Another thing that could be great is filtering this list to
>> those that pass a certain criteria (mainly quality, up to date, ...)
>> and present it in an attractive way, with summaries, categorized by
>> type (trick, tutorial, comparison,...), ...
>

If this view is more or less what you have in mind I'll try to help.

Santi

^ permalink raw reply

* Re: Git weekly news: 2008-49
From: Felipe Contreras @ 2008-12-05 21:57 UTC (permalink / raw)
  To: Santi Béjar; +Cc: Jakub Narebski, git list
In-Reply-To: <adf1fd3d0812051344x79fe2c10i77b2abb8e7e19fe@mail.gmail.com>

On Fri, Dec 5, 2008 at 11:44 PM, Santi Béjar <santi@agolina.net> wrote:
> 2008/12/5 Felipe Contreras <felipe.contreras@gmail.com>:
>> On Fri, Dec 5, 2008 at 6:02 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>
> [...]
>
>>>> But here are the links anyway. The order is rather random.
>>>
>>> Moreover the _quality_ of those links is very random.
>>
>> Exactly, I didn't choose them, that's what people have been tagging as
>> "git" in delicious.com. I'm subscribed to the RSS feed and saving the
>> ones that appear a lot.
>>
>> In fact I don't like some of them, but that's what the "public" finds
>> interesting.
>
> So I don't see the value of such a list. You can go to delicious and
> get it.

Try it. You can't see which ones are new, which are completely
irrelevant. There are duplicates and you can't see the popularity /
freshness ratio, or "hotness", never mind the most popular this week.

Apparently some people already found interesting links they haven't
seen before, so at lest there's value for them.

> Another thing that could be great is filtering this list to
> those that pass a certain criteria (mainly quality, up to date, ...)
> and present it in an attractive way, with summaries, categorized by
> type (trick, tutorial, comparison,...), ...

Yes, I'll probably improve the presentation.

-- 
Felipe Contreras

^ permalink raw reply

* Re: Git weekly news: 2008-49
From: Felipe Contreras @ 2008-12-05 21:49 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narebski, git list
In-Reply-To: <7v3ah2z6jh.fsf@gitster.siamese.dyndns.org>

On Fri, Dec 5, 2008 at 10:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>
>> [1] It would be nice if somebody resurrected GitTraffic, offshot of
>> now defunct KernelTraffic, or at least helped to write Git articles
>> for KernelTrap (which currently is in a bit of hiatus).
>
> While I 100% agree with you that "Git Traffic" was a terrific attempt, I
> do not think expecting or asking Felipe to duplicate it is realistic.
>
> I searched for Felipe's proposal on the list archive, and its title was
> "Planet Git".  That shows why the focus could be different.
>
> "Git Traffic" was great because it attempted to directly address the issue
> that the traffic on the mailing list was simply too high (and still is) to
> follow for casual observers.  It did so by giving a comprehensive summary
> of what important topics were discussed recently on the list from the
> viewpoint of one dedicated person who followed many, perhaps not all,
> important threads carefully, who very well knew what was going on, and who
> had a good taste on what is important and what is not.
>
> But "Planet Git" is quite different from "Git Traffic".  For the latter,
> somebody needs to do a real work, continuously.  But more importantly, I
> think they would serve different purposes.  A "Planet" could be valuable
> to have with or without "Traffic".
>
> I think what was presented as "Official Git blog", however, is also
> different from what people expect a "Planet" to be.  I do not think it is
> unreasonable to expect or ask Felipe to improve on making his service more
> "Planet" like.
>
> A "Planet", as I understand it, is an aggregator of (selected) people's
> blogs, and even though I am not currently involved in any Planet nor
> follow any Planet myself, I can imagine that it could be a valuable
> resource to have a "Planet Git" that subscribes to and aggregates what
> influential figures write on git in their blogs.

Yes, a planet is an aggregation of blogs. I've been involved in some
planets and they are not exactly great.

One disadvantage of planets is usually there's no control of the topic
of the posts. For example in planet GNOME there's some people that
mostly blog only about GNOME, but other people blog about everything,
including what they did in the morning, ate at lunch, and discussed at
the water cooler. Some people have their blogs properly categorized
and the right feeds go to the right planets, but they are the
minority.

Planets are great if you want people to "get intimate" with the
community, probably a small one. Not for git IMHO.

There's a single "planet" I know that doesn't follow the norm;
beagleboard news[1]. There one guy basically uses Google Reader to
share whatever he thinks is somehow interesting for the beagleboard
community. The advantage is that the feed is much more focused than a
planet, but still there's a lot of bias in the links, which sometimes
have nothing to do with the beagleboard.

> Felipe's page currently is a random collection of links, and other than
> their titles, there is no indication for readers to judge which link is
> worth clicking and reading.  It does not even mention who wrote each
> piece, let alone editorial comments (e.g. "This is worth reading") like
> you added.  When you click one of them in order to read it, you leave the
> "list of links".  That is not how navigation (the click and thought flow
> for the readers) usually works in a "Planet".

Lets remember that this is the first try, and there's many more links
that what would fit in any given week, but I just didn't want to leave
them out.

Maybe for the next weeks I'll do a bit of explanation about each link, lets see.

> If this wants to be a "Planet Git", I do not think there is any need for
> Felipe to ask "who wants accounts?"  It would go the other way.  Instead,
> Felipe, as the coordinator of the "Planet", would find people who writes
> noteworthy things on git on their own blogs, would ask for permission to
> slurp and aggregate what they wrote, and produce the page by aggregating
> their writings.  That would make a good "Planet Git".
>
> Expecting people to apply for an account and write for that page would not
> fly.  As Felipe said himself, many people already have their own blog.
>
> Having said all that, I am not sure "Planet" would work for the git
> community as well as it would for others.  I do not know of many core-ish
> people write on git on their blogs (and I know at least two core-ish
> people who flatly say "blogging is a waste of time").

I actually propose two things:

a) planet git

This would be a collection of blogs, probably not developers, which
might consider blogging to be a wast of time, but git enthusiasts. I
don't expect the list of dedicated bloggers to be that big, hence my
second proposal:

b) git blog

A blog can be shared by a bunch of people, much line online news
sites. Junio could write a post once for each release, for example,
without having to setup his own blog, maybe somebody else can
copy-paste "What's cooking in git.git", or any kind of semi-official
announcement.

And I'm sure there will be one or two developers who wouldn't mind
sharing their frustrations and/or visions.

These two are not exclusive, of course.

For a) I need a server (there's a few already offered), setup the
planet software, and people to send me their RSS feeds, which
hopefully are categorized for git-specific posts. I don't have much
faith on this, nor interest.

For b) I just need interested people to send me their emails.

[1] http://beagleboard.org/news

-- 
Felipe Contreras

^ 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