Git development
 help / color / mirror / Atom feed
* Re: What's cooking in git.git (Oct 2009, #02; Sun, 11)
From: Jeff King @ 2009-10-14  4:24 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git
In-Reply-To: <7vmy3w9qdd.fsf@alter.siamese.dyndns.org>

On Mon, Oct 12, 2009 at 03:52:46PM -0700, Junio C Hamano wrote:

> > Hmm. I thought you wanted to re-order some of these for to put the
> > porcelain and short formats into v1.6.6, but leave the status switchover
> > for v1.7.0.
> 
> We could build an alternate history between 3fa509d..46b77a6, revert the
> merges 9558627 and 65c8513, and merge the alternate history.  But is the
> short format support so solid that it deserves to be in 1.6.6 in the
> current shape?

Somewhere I seem to recall you saying that it would be nice to give the
--short format some wider exposure as "git status --short" before making
the "status is no longer commit --dry-run" switch-over. But now I can't
find that message anywhere.

Let's not worry about it.

-Peff

^ permalink raw reply

* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Jeff King @ 2009-10-14  4:31 UTC (permalink / raw)
  To: Johannes Schindelin
  Cc: Junio C Hamano, Thomas Rast, Euguess, Mikael Magnusson,
	Matthieu Moy, Jay Soffian, git, Johannes Sixt
In-Reply-To: <alpine.DEB.1.00.0910140117280.4985@pacific.mpi-cbg.de>

On Wed, Oct 14, 2009 at 01:22:26AM +0200, Johannes Schindelin wrote:

> At some point, trying to educate the user is not helpful but annoying.  If 
> Git already knows what I want, why does it not do it already?  _That_ is 
> the question I already hear in my ears.

I am not entirely convinced that the suggested behaviors will result in
that user response, or a different one (like "why does git keep giving
me bad advice?"). Which is why I suggested data collection.

> > So doing step (1) would be a way of collecting some of that data (will 
> > users say "stupid git, if you knew what I wanted, why didn't you just do 
> > it?" or "stupid git, your suggestion is just confusing me!").
> 
> I disagree.  It is not about collecting data.  We will not get any 
> feedback from the affected people.  You know that, I know that.

I don't agree. You are already talking about users complaining about
git's interface. Isn't that feedback? How do you hear those complaints
now?

I don't think they will come on the list and talk about it, but if we
release a version of git that has differing behavior and give it some
time to be used in the wild, we _will_ get feedback in the form of
blogs, complaints on other lists, word-of-mouth, etc.

Now maybe that is not a good idea in this instance, because that sort of
feedback may take several versions to appear, and we are talking about a
potential timetable of v1.7.0, which is probalby only two versions away.

-Peff

^ permalink raw reply

* [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Daniel Barkalow @ 2009-10-14  4:44 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

When detaching HEAD (or "browsing history"), the user has specified
the commit with some "extended SHA1" which is not a local
branch. Exactly what that string was is likely to be useful to the
user later. Also, we can detect the user putting work into the history
for the first time (such that it is no longer going to be protected as
uncommitted changes in the working tree) without a branch to hold it
by seeing that there is such a description for the current state
before the commit. (Afterwards, the description should be dropped; it
doesn't make sense to tell the user they checked out "origin/master"
or "d199fb7" when they've now diverged from that remote branch with
local changes or made a different commit.)

The upshot of the messages should be:

 $ git checkout origin/master
 Since you can't actually change "origin/master" yourself, you'll just
 be sightseeing unless you create a local branch to hold new local work.

 $ git branch
 * (not a local branch, but "origin/master")

 $ git commit
 You've been sightseeing "origin/master". The commit can't change that
 value, so your commit isn't held in any branch. If you want to create
 a branch to hold it, here's how.

"git checkout origin/master" should be similar in complexity to
"svn checkout -r 8655"; the difference is that svn won't let you
commit then and git will but you'll need to understand the
implications if you do so. If you don't commit (because you don't want
to make any changes, because you don't think it would be possible, or
because you don't want to worry about what would happen), there's no
meaningful difference, and you don't need to be told.

The messages have to be improved and made more useful.

The effects of "git checkout HEAD", "git checkout origin/master; git 
checkout HEAD^", and "git checkout origin/master; git reset --hard 
origin/next" aren't handled quite right; none of them keep a description, 
but there should always be some description of a detached HEAD unless the 
user has made a commit (and therefore gotten the message about making a 
local branch to put it on).
---
 branch.c                 |   13 +++++++++++++
 branch.h                 |    6 ++++++
 builtin-branch.c         |   13 ++++++++++++-
 builtin-checkout.c       |    8 +++++++-
 builtin-commit.c         |   10 +++++++++-
 t/t3203-branch-output.sh |    2 +-
 t/t7201-co.sh            |    6 ++----
 7 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/branch.c b/branch.c
index 05ef3f5..2c5b6d3 100644
--- a/branch.c
+++ b/branch.c
@@ -194,6 +194,18 @@ void create_branch(const char *head,
 	free(real_ref);
 }
 
+char *get_detached_head_string(void)
+{
+	char *filename = git_path("DETACH_NAME");
+	struct stat st;
+	if (stat(filename, &st) || !S_ISREG(st.st_mode))
+		return NULL;
+	struct strbuf buf = STRBUF_INIT;
+	strbuf_read_file(&buf, filename, st.st_size);
+	strbuf_trim(&buf);
+	return strbuf_detach(&buf, 0);
+}
+
 void remove_branch_state(void)
 {
 	unlink(git_path("MERGE_HEAD"));
@@ -201,4 +213,5 @@ void remove_branch_state(void)
 	unlink(git_path("MERGE_MSG"));
 	unlink(git_path("MERGE_MODE"));
 	unlink(git_path("SQUASH_MSG"));
+	unlink(git_path("DETACH_NAME"));
 }
diff --git a/branch.h b/branch.h
index eed817a..0a30c3a 100644
--- a/branch.h
+++ b/branch.h
@@ -22,6 +22,12 @@ void create_branch(const char *head, const char *name, const char *start_name,
 void remove_branch_state(void);
 
 /*
+ * Returns the string used when detaching HEAD, or NULL if HEAD is not
+ * detached.
+ */
+char *get_detached_head_string(void);
+
+/*
  * Configure local branch "local" to merge remote branch "remote"
  * taken from origin "origin".
  */
diff --git a/builtin-branch.c b/builtin-branch.c
index 9f57992..9ce4127 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -425,7 +425,18 @@ static void show_detached(struct ref_list *ref_list)
 
 	if (head_commit && is_descendant_of(head_commit, ref_list->with_commit)) {
 		struct ref_item item;
-		item.name = xstrdup("(no branch)");
+		char *literal = get_detached_head_string();
+		struct stat st;
+		if (literal) {
+			struct strbuf buf = STRBUF_INIT;
+			strbuf_addstr(&buf, "(no branch, as \"");
+			strbuf_addstr(&buf, literal);
+			strbuf_addstr(&buf, "\")");
+			free(literal);
+			item.name = strbuf_detach(&buf, 0);
+		} else {
+			item.name = xstrdup("(no branch)");
+		}
 		item.len = strlen(item.name);
 		item.kind = REF_LOCAL_BRANCH;
 		item.dest = NULL;
diff --git a/builtin-checkout.c b/builtin-checkout.c
index d050c37..448397d 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -510,11 +510,17 @@ static void update_refs_for_switch(struct checkout_opts *opts,
 			   REF_NODEREF, DIE_ON_ERR);
 		if (!opts->quiet) {
 			if (old->path)
-				fprintf(stderr, "Note: moving to '%s' which isn't a local branch\nIf you want to create a new branch from this checkout, you may do so\n(now or later) by using -b with the checkout command again. Example:\n  git checkout -b <new_branch_name>\n", new->name);
+				fprintf(stderr, "Note: moving to '%s' which isn't a local branch.\nAny commits you may make will not affect the commit with this name.\n", new->name);
 			describe_detached_head("HEAD is now at", new->commit);
 		}
 	}
 	remove_branch_state();
+	if (!new->path && strcmp(new->name, "HEAD")) {
+		FILE *detach_name;
+		detach_name = fopen(git_path("DETACH_NAME"), "w");
+		fprintf(detach_name, "%s\n", new->name);
+		fclose(detach_name);
+	}
 	strbuf_release(&msg);
 	if (!opts->quiet && (new->path || !strcmp(new->name, "HEAD")))
 		report_tracking(new);
diff --git a/builtin-commit.c b/builtin-commit.c
index 200ffda..2ceb951 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -24,6 +24,7 @@
 #include "string-list.h"
 #include "rerere.h"
 #include "unpack-trees.h"
+#include "branch.h"
 
 static const char * const builtin_commit_usage[] = {
 	"git commit [options] [--] <filepattern>...",
@@ -968,6 +969,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct ref_lock *ref_lock;
 	struct commit_list *parents = NULL, **pptr = &parents;
 	struct stat statbuf;
+	char *detached_string;
 	int allow_fast_forward = 1;
 	struct wt_status s;
 
@@ -1089,10 +1091,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		die("cannot update HEAD ref");
 	}
 
+	detached_string = get_detached_head_string();
+
 	unlink(git_path("MERGE_HEAD"));
 	unlink(git_path("MERGE_MSG"));
 	unlink(git_path("MERGE_MODE"));
 	unlink(git_path("SQUASH_MSG"));
+	unlink(git_path("DETACH_NAME"));
 
 	if (commit_index_files())
 		die ("Repository has been updated, but unable to write\n"
@@ -1101,8 +1106,11 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	rerere();
 	run_hook(get_index_file(), "post-commit", NULL);
-	if (!quiet)
+	if (!quiet) {
+		if (detached_string)
+			fprintf(stderr, "\nNote: you had checked out '%s' which isn't a local branch.\nIf you want to create a new branch with this commit, you may do so\n(now or later) by using -b with the checkout command. Example:\n  git checkout -b <new_branch_name>\n\n", detached_string);
 		print_summary(prefix, commit_sha1);
+	}
 
 	return 0;
 }
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index 809d1c4..08409cd 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -67,7 +67,7 @@ test_expect_success 'git branch -v shows branch summaries' '
 '
 
 cat >expect <<'EOF'
-* (no branch)
+* (no branch, as "HEAD^0")
   branch-one
   branch-two
   master
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index ebfd34d..0f40589 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -171,10 +171,8 @@ test_expect_success 'checkout to detach HEAD' '
 	git checkout -f renamer && git clean -f &&
 	git checkout renamer^ 2>messages &&
 	(cat >messages.expect <<EOF
-Note: moving to '\''renamer^'\'' which isn'\''t a local branch
-If you want to create a new branch from this checkout, you may do so
-(now or later) by using -b with the checkout command again. Example:
-  git checkout -b <new_branch_name>
+Note: moving to '\''renamer^'\'' which isn'\''t a local branch.
+Any commits you may make will not affect the commit with this name.
 HEAD is now at 7329388... Initial A one, A two
 EOF
 ) &&
-- 
1.6.5.9.ge994f.dirty

^ permalink raw reply related

* [PATCH] checkout: add 'pre-checkout' hook
From: Sam Vilain @ 2009-10-14  4:45 UTC (permalink / raw)
  To: git; +Cc: elliot, Sam Vilain

Add a simple hook that will run before checkouts.

Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
---
 Documentation/githooks.txt |   20 +++++++++++++++-----
 builtin-checkout.c         |   25 ++++++++++++++++++++++---
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 06e0f31..8dc3fbf 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -143,21 +143,31 @@ pre-rebase
 This hook is called by 'git-rebase' and can be used to prevent a branch
 from getting rebased.
 
+pre-checkout
+-----------
 
-post-checkout
-~~~~~~~~~~~~~
-
-This hook is invoked when a 'git-checkout' is run after having updated the
+This hook is invoked when a 'git-checkout' is run after before updating the
 worktree.  The hook is given three parameters: the ref of the previous HEAD,
 the ref of the new HEAD (which may or may not have changed), and a flag
 indicating whether the checkout was a branch checkout (changing branches,
 flag=1) or a file checkout (retrieving a file from the index, flag=0).
-This hook cannot affect the outcome of 'git-checkout'.
+This hook can prevent the checkout from proceeding by exiting with an
+error code.
 
 It is also run after 'git-clone', unless the --no-checkout (-n) option is
 used. The first parameter given to the hook is the null-ref, the second the
 ref of the new HEAD and the flag is always 1.
 
+This hook can be used to perform any clean-up deemed necessary before
+checking out the new branch/files.
+
+post-checkout
+-----------
+
+This hook is invoked when a 'git-checkout' is run after having updated the
+worktree.  It takes the same arguments as the 'pre-checkout' hook.
+This hook cannot affect the outcome of 'git-checkout'.
+
 This hook can be used to perform repository validity checks, auto-display
 differences from the previous HEAD if different, or set working dir metadata
 properties.
diff --git a/builtin-checkout.c b/builtin-checkout.c
index d050c37..b72a3cb 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -36,6 +36,17 @@ struct checkout_opts {
 	enum branch_track track;
 };
 
+static int pre_checkout_hook(struct commit *old, struct commit *new,
+			      int changed)
+{
+	return run_hook(NULL, "pre-checkout",
+			sha1_to_hex(old ? old->object.sha1 : null_sha1),
+			sha1_to_hex(new ? new->object.sha1 : null_sha1),
+			changed ? "1" : "0", NULL);
+	/* "new" can be NULL when checking out from the index before
+	   a commit exists. */
+}
+
 static int post_checkout_hook(struct commit *old, struct commit *new,
 			      int changed)
 {
@@ -256,6 +267,13 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	if (errs)
 		return 1;
 
+	/* Run the pre-checkout hook */
+	resolve_ref("HEAD", rev, 0, &flag);
+	head = lookup_commit_reference_gently(rev, 1);
+	errs = pre_checkout_hook(head, head, 0);
+	if (errs)
+		return 1;
+
 	/* Now we are committed to check them out */
 	memset(&state, 0, sizeof(state));
 	state.force = 1;
@@ -279,9 +297,6 @@ static int checkout_paths(struct tree *source_tree, const char **pathspec,
 	    commit_locked_index(lock_file))
 		die("unable to write new index file");
 
-	resolve_ref("HEAD", rev, 0, &flag);
-	head = lookup_commit_reference_gently(rev, 1);
-
 	errs |= post_checkout_hook(head, head, 0);
 	return errs;
 }
@@ -543,6 +558,10 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 		parse_commit(new->commit);
 	}
 
+	ret = pre_checkout_hook(old.commit, new->commit, 1);
+	if (ret)
+		return ret;
+
 	ret = merge_working_tree(opts, &old, new);
 	if (ret)
 		return ret;
-- 
1.6.3.3

^ permalink raw reply related

* git-commit feature request: pass editor command line options
From: Matthew Cline @ 2009-10-14  4:58 UTC (permalink / raw)
  To: git


I'd like to be able to have git-commit pass the commit-message editor command
line options which aren't passed to the editor for other usages.  Right now
I have "co" aliased to "!sh -c 'GIT_EDITOR=git-commit-editor git commit'",
where git-commit-editor is a wrapper around my editor-of-choice which passes
the editor the command line options I want, but it'd be simpler and cleaner
if I could just set "commit.editor_options=-BAR".  Or even let there be a
separate editor for commits, so I could do "core.editor=foo" and
"commit.editor=foo -BAR".
-- 
View this message in context: http://www.nabble.com/git-commit-feature-request%3A-pass-editor-command-line-options-tp25885354p25885354.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply

* Re: [RFC PATCH 3/5] stash: Use new %g/%G formats instead of sed
From: Jeff King @ 2009-10-14  5:00 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <77e591b80021efc265fea1a101ce6b7124ea832e.1255380039.git.trast@student.ethz.ch>

On Mon, Oct 12, 2009 at 11:06:05PM +0200, Thomas Rast wrote:

>  list_stash () {
>  	have_stash || return 0
> -	git log --no-color --pretty=oneline -g "$@" $ref_stash -- |
> -	sed -n -e 's/^[.0-9a-f]* refs\///p'
> +	git log --format="%g: %G" -g "$@" $ref_stash

You dropped the trailing "--" which is needed for disambiguation.

-Peff

^ permalink raw reply

* Re: [RFC PATCH 2/5] Introduce new pretty formats %g and %G for reflog information
From: Jeff King @ 2009-10-14  4:59 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <e0321a8af8d702d24ace33510daff22d02f4e116.1255380039.git.trast@student.ethz.ch>

On Mon, Oct 12, 2009 at 11:06:04PM +0200, Thomas Rast wrote:

> Unfortunately, we also need to pass down the reflog_walk_info from
> show_log(), so this commit touches a lot of (unrelated) callers to
> pretty_print_commit() and format_commit_message() to accomodate the
> extra argument.

A while back I wanted to add a feature to pretty-printing, and I ran
into the same situation (though my feature never made it to the list).
We really end up passing around the same arguments over and over.  Maybe
it makes sense instead of adding another argument to refactor into a
"pretty_print_context" struct that contains all of the arguments and
current state.

It would be an even more invasive patch, but I think it would make
things more readable, and make future changes much easier to see.

-Peff

^ permalink raw reply

* Re: [RFC PATCH 4/5] stash list: drop the default limit of 10 stashes
From: Jeff King @ 2009-10-14  5:02 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <137df1f7295576345f5aefe46c882399e107c321.1255380039.git.trast@student.ethz.ch>

On Mon, Oct 12, 2009 at 11:06:06PM +0200, Thomas Rast wrote:

> 'git stash list' had an undocumented limit of 10 stashes, unless other
> git-log arguments were specified.  This surprised at least one user,
> but possibly served to cut the output below a screenful without using
> a pager.
> 
> Since the last commit, 'git stash list' will fire up a pager according
> to the same rules as the 'git log' it calls, so we can drop the limit.

Having slept on it, I think I agree that this is a good change. It fixes
the ugly corner cases I mentioned, and it is easier to explain.

-Peff

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Junio C Hamano @ 2009-10-14  5:07 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: git
In-Reply-To: <alpine.LNX.2.00.0910140037570.32515@iabervon.org>

Daniel Barkalow <barkalow@iabervon.org> writes:

> When detaching HEAD (or "browsing history"), the user has specified
> the commit with some "extended SHA1" which is not a local
> branch....

I do not have enough mental bandwidth to think about (1) if this
information is a good thing to have, shown in your transcript, or (2) if
the name of the branch _before_ getting into the detached state may be
more interesting information tonight.

But I have one question regarding the implementation.  Why do you need a
new file in $GIT_DIR for this?  Wouldn't what is in the logs/HEAD be
enough, and if not why not?

^ permalink raw reply

* Re: [RFC PATCH 5/5] stash: change built-in ref to 'stash' instead of 'refs/stash'
From: Jeff King @ 2009-10-14  5:06 UTC (permalink / raw)
  To: Thomas Rast; +Cc: Jef Driesen, Nanako Shiraishi, git
In-Reply-To: <548bc3a41c03a049e782d5d04a34c3b26c0897d2.1255380039.git.trast@student.ethz.ch>

On Mon, Oct 12, 2009 at 11:06:07PM +0200, Thomas Rast wrote:

> 'git stash list' now always shows 'refs/stash@{...}' instead of
> 'stash@{...}', because that's what we specify for the ref.
> 
> Since git checks .git/$ref, .git/refs/$ref and only then
> .git/refs/{branches,tags,remotes}, we can drop the prefix.  This only
> affects people who have .git/stash, who were never able to refer to
> their stashes by stash@{...}.  (Sadly, now they won't be able to use
> git-stash anymore at all.)

Maybe a better solution would be a "short name" variant for pretty
format specifiers. We already have %(refname) and %(refname:short) in
for-each-ref, where the latter cuts off "refs/heads/", "refs/tags", or
"refs/remotes/" from the beginning. I'm not sure if it does just
"refs/", but probably it should. It may even check for ambiguity, but
I'd have to double-check the code.

The tricky part would be deciding on a syntax. This seems to come up a
fair bit. I think there is room for somebody to suggest a more expansive
--format syntax that can handle arbitrary arguments being given to
expansions (I think there was some discussion recently in a related
thread about body indentation, but I haven't been following it too
closely).

-Peff

^ permalink raw reply

* Re: [PATCH] Proof-of-concept patch to remember what the detached HEAD was
From: Jeff King @ 2009-10-14  5:08 UTC (permalink / raw)
  To: Daniel Barkalow; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LNX.2.00.0910140037570.32515@iabervon.org>

On Wed, Oct 14, 2009 at 12:44:34AM -0400, Daniel Barkalow wrote:

> +char *get_detached_head_string(void)
> +{
> +	char *filename = git_path("DETACH_NAME");
> +	struct stat st;
> +	if (stat(filename, &st) || !S_ISREG(st.st_mode))
> +		return NULL;
> +	struct strbuf buf = STRBUF_INIT;
> +	strbuf_read_file(&buf, filename, st.st_size);
> +	strbuf_trim(&buf);
> +	return strbuf_detach(&buf, 0);
> +}

Would it hurt to tuck this information into HEAD itself, as we already
put arbitrary text into FETCH_HEAD?

-Peff

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Jeff King @ 2009-10-14  5:13 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, elliot
In-Reply-To: <1255495525-11254-1-git-send-email-sam.vilain@catalyst.net.nz>

On Wed, Oct 14, 2009 at 05:45:25PM +1300, Sam Vilain wrote:

> Add a simple hook that will run before checkouts.

What is the use case that makes it useful as a hook, and not simply as
something people can do before running checkout?

I guess you can use it to block a checkout, but only after finding out
_what_ you are going to checkout, but an exact use case escapes me.

> -post-checkout
> -~~~~~~~~~~~~~
> -
> -This hook is invoked when a 'git-checkout' is run after having updated the
> +This hook is invoked when a 'git-checkout' is run after before updating the

Did you mean "before having" here?

>  worktree.  The hook is given three parameters: the ref of the previous HEAD,
>  the ref of the new HEAD (which may or may not have changed), and a flag
>  indicating whether the checkout was a branch checkout (changing branches,
>  flag=1) or a file checkout (retrieving a file from the index, flag=0).
> -This hook cannot affect the outcome of 'git-checkout'.
> +This hook can prevent the checkout from proceeding by exiting with an
> +error code.
>  
>  It is also run after 'git-clone', unless the --no-checkout (-n) option is
>  used. The first parameter given to the hook is the null-ref, the second the
>  ref of the new HEAD and the flag is always 1.

Should this "after" in the bottom paragraph perhaps become "during"?

-Peff

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Junio C Hamano @ 2009-10-14  5:13 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, elliot
In-Reply-To: <1255495525-11254-1-git-send-email-sam.vilain@catalyst.net.nz>

Sam Vilain <sam.vilain@catalyst.net.nz> writes:

> Add a simple hook that will run before checkouts.
>
> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
> ---
>  Documentation/githooks.txt |   20 +++++++++++++++-----
>  builtin-checkout.c         |   25 ++++++++++++++++++++++---
>  2 files changed, 37 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 06e0f31..8dc3fbf 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -143,21 +143,31 @@ pre-rebase
>  This hook is called by 'git-rebase' and can be used to prevent a branch
>  from getting rebased.
>  
> +pre-checkout
> +-----------
>  
> -post-checkout
> -~~~~~~~~~~~~~
> -
> -This hook is invoked when a 'git-checkout' is run after having updated the
> +This hook is invoked when a 'git-checkout' is run after before updating the

"after before"?

>  worktree.  The hook is given three parameters: the ref of the previous HEAD,
>  the ref of the new HEAD (which may or may not have changed), and a flag
>  indicating whether the checkout was a branch checkout (changing branches,
>  flag=1) or a file checkout (retrieving a file from the index, flag=0).
> -This hook cannot affect the outcome of 'git-checkout'.
> +This hook can prevent the checkout from proceeding by exiting with an
> +error code.
>  
>  It is also run after 'git-clone', unless the --no-checkout (-n) option is
>  used. The first parameter given to the hook is the null-ref, the second the
>  ref of the new HEAD and the flag is always 1.
>  
> +This hook can be used to perform any clean-up deemed necessary before
> +checking out the new branch/files.
> +
> +post-checkout
> +-----------

This is not about your patch, but the patch text shows that our diff
algorithm seems to have a room for improvement.  I expected to see a
straight insersion of block of text, not touching anything in the original
section on post-checkout hook.

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Sam Vilain @ 2009-10-14  5:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, elliot
In-Reply-To: <7vr5t6lfr0.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Sam Vilain <sam.vilain@catalyst.net.nz> writes:
> 
>> Add a simple hook that will run before checkouts.
>>
>> Signed-off-by: Sam Vilain <sam.vilain@catalyst.net.nz>
>> ---
>>  Documentation/githooks.txt |   20 +++++++++++++++-----
>>  builtin-checkout.c         |   25 ++++++++++++++++++++++---
>>  2 files changed, 37 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
>> index 06e0f31..8dc3fbf 100644
>> --- a/Documentation/githooks.txt
>> +++ b/Documentation/githooks.txt
>> @@ -143,21 +143,31 @@ pre-rebase
>>  This hook is called by 'git-rebase' and can be used to prevent a branch
>>  from getting rebased.
>>  
>> +pre-checkout
>> +-----------
>>  
>> -post-checkout
>> -~~~~~~~~~~~~~
>> -
>> -This hook is invoked when a 'git-checkout' is run after having updated the
>> +This hook is invoked when a 'git-checkout' is run after before updating the
> 
> "after before"?

*ahem* whoops :).  I think I got the heading style wrong too...

> This is not about your patch, but the patch text shows that our diff
> algorithm seems to have a room for improvement.  I expected to see a
> straight insersion of block of text, not touching anything in the original
> section on post-checkout hook.

Correct.  This is because the paragraph explaining when the hook runs
has been moved to the pre-checkout paragraph, which appears before the
post-checkout section.  I just compared the output to 'diff -du' and it
seems to be the same, so I wouldn't worry too much.
-- 
Sam Vilain, Perl Hacker, Catalyst IT (NZ) Ltd.
phone: +64 4 499 2267        PGP ID: 0x66B25843

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Jeff King @ 2009-10-14  5:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Sam Vilain, git, elliot
In-Reply-To: <7vr5t6lfr0.fsf@alter.siamese.dyndns.org>

On Tue, Oct 13, 2009 at 10:13:39PM -0700, Junio C Hamano wrote:

> >  worktree.  The hook is given three parameters: the ref of the previous HEAD,
> >  the ref of the new HEAD (which may or may not have changed), and a flag
> >  indicating whether the checkout was a branch checkout (changing branches,
> >  flag=1) or a file checkout (retrieving a file from the index, flag=0).
> > -This hook cannot affect the outcome of 'git-checkout'.
> > +This hook can prevent the checkout from proceeding by exiting with an
> > +error code.
> >  
> >  It is also run after 'git-clone', unless the --no-checkout (-n) option is
> >  used. The first parameter given to the hook is the null-ref, the second the
> >  ref of the new HEAD and the flag is always 1.
> >  
> > +This hook can be used to perform any clean-up deemed necessary before
> > +checking out the new branch/files.
> > +
> > +post-checkout
> > +-----------
> 
> This is not about your patch, but the patch text shows that our diff
> algorithm seems to have a room for improvement.  I expected to see a
> straight insersion of block of text, not touching anything in the original
> section on post-checkout hook.

I think it's right as-is. He changed the title of the section, made a
few tweaks in the text to make it appropriate for "pre-checkout", and
then made a new post-checkout section that says "This is just like
pre-checkout". So most of the lines were left untouched. Short of our
diff understanding the block-formatting of asciidoc, I think it's as
good as we can get.

-Peff

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Sam Vilain @ 2009-10-14  5:25 UTC (permalink / raw)
  To: Jeff King; +Cc: git, elliot
In-Reply-To: <20091014051319.GF31810@coredump.intra.peff.net>

Jeff King wrote:
>> Add a simple hook that will run before checkouts.
> 
> What is the use case that makes it useful as a hook, and not simply as
> something people can do before running checkout?
> 
> I guess you can use it to block a checkout, but only after finding out
> _what_ you are going to checkout, but an exact use case escapes me.

Right.  Yes, this could be explained more.

Actually the use case is submodules.  When switching branches, the user
wants to add a hook to remove submodules.  However post-checkout is too
late, because the index and the .gitmodules file do not record the
submodule locations.

Of course the best explanation is a test case :) I'll look at cooking
one up.
-- 
Sam Vilain, Perl Hacker, Catalyst IT (NZ) Ltd.
phone: +64 4 499 2267        PGP ID: 0x66B25843

^ permalink raw reply

* Re: Efficient cloning from svn (with multiple branches/tags subdirs)
From: Eric Wong @ 2009-10-14  6:03 UTC (permalink / raw)
  To: Bruno Harbulot; +Cc: git
In-Reply-To: <hb2fvu$8qi$1@ger.gmane.org>

Bruno Harbulot <Bruno.Harbulot@manchester.ac.uk> wrote:
> Hello,
>
> I'm trying to clone an existing subversion repository (Restlet:  
> http://restlet.tigris.org/source/browse/). I'm using Git 1.6.5. The  
> layout of the project is like this:
>   trunk/
>   branches/1.0
>   branches/1.1
>   tags/1.0/1.0b1
>   tags/1.0/1.0b2
>   ...
>   tags/1.0/1.0.1
>   ...
>   tags/1.1/1.1.0
>   tags/1.1/1.1.1
>   ...


Hi Bruno,

That looks like there's two levels of tags.  You should be able to do
this with your version of git in $GIT_CONFIG:

	[svn-remote "svn"]
		url = http://restlet.tigris.org/svn/restlet
		fetch = trunk:refs/remotes/svn/trunk
		branches = branches/*:refs/remotes/svn/*
		tags = tags/*/*:refs/remotes/svn/tags/*/*
		; note the */* to glob at multiple levels

> Therefore, I've tried to use this (with and without '-T trunk', but  
> that's a separate problem):
>
>   git init
>   git svn init --prefix=svn/ -t tags/1.0 -t tags/1.1 -t tags/1.2 -t  
> tags/2.0 -b branches/1.0 -b branches/1.1  
> http://restlet.tigris.org/svn/restlet
>   git svn fetch
>
>
> This takes a while (I've had to interrupt this) and this creates a  
> number of branches such as:
>   remotes/svn/tags/1.0b1
>   remotes/svn/tags/1.0b2
>   remotes/svn/tags/1.0b3
>   remotes/svn/tags/1.0b3@1883
>   remotes/svn/tags/1.0b3@323
>
>
> What surprises me is that it looks like it's looping over and over,  
> since sometimes it starts back from SVN revision 1 when it's trying to  
> import a new tag.

Yeah, that's an unfortunate thing about the flexibility of Subversion,
basically anything can be a "tag" or a directory and it's extremely
hard for git svn to support any uncommon cases for tags/branches
out-of-the box, so the manual config editing is needed.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] gitweb: fix esc_param
From: Giuseppe Bilotta @ 2009-10-14  6:19 UTC (permalink / raw)
  To: Stephen Boyd; +Cc: git, Jakub Narebski, Junio C Hamano
In-Reply-To: <4AD525CA.8090102@gmail.com>

On Wed, Oct 14, 2009 at 3:13 AM, Stephen Boyd <bebarino@gmail.com> wrote:
> Giuseppe Bilotta wrote:
>> The custom CGI escaping done in esc_param failed to escape UTF-8
>> properly. Fix by using CGI::escape on each sequence of matched
>> characters instead of sprintf()ing a custom escaping for each byte.
>>
>> Additionally, the space -> + escape was being escaped due to greedy
>> matching on the first substitution. Fix by adding space to the
>> list of characters not handled on the first substitution.
>>
>> Finally, remove an unnecessary escaping of the + sign.
>>
>
> Signoff?

Doh.

Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>

(If this isn't enough, let me know)

> This works great for my purposes. Thanks.

Can you also check if this fixes the branch name issue you mentioned
in the other branch? (And/or do you have a repository exposing the
problem if not?)



-- 
Giuseppe "Oblomov" Bilotta

^ permalink raw reply

* Re: [PATCH] gitweb: fix esc_param
From: Stephen Boyd @ 2009-10-14  6:29 UTC (permalink / raw)
  To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Junio C Hamano
In-Reply-To: <cb7bb73a0910132319p27b4f362sf5bffa2afe6e5849@mail.gmail.com>

Giuseppe Bilotta wrote:
> On Wed, Oct 14, 2009 at 3:13 AM, Stephen Boyd <bebarino@gmail.com> wrote:
>> This works great for my purposes. Thanks.
>
> Can you also check if this fixes the branch name issue you mentioned
> in the other branch? (And/or do you have a repository exposing the
> problem if not?)

(We're jumping back and forth between threads haha)

Sorry, it doesn't. But this diff fixes the first part of the problem.
There are still problems with the RSS/Atom feed names being mangled. The
branch name I'm using is gitwéb, but I imagine any utf8 character will fail.

I see the title and the actual text being mangled without this patch.

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4b21ad2..910c370 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1500,9 +1500,10 @@ sub format_ref_marker {
                                -href => href(
                                        action=>$dest_action,
                                        hash=>$dest
-                               )}, $name);
+                               )}, esc_html($name));
 
-                       $markers .= " <span class=\"$class\" title=\"$ref\">" .
+                       my $title_ref = esc_html($ref);
+                       $markers .= " <span class=\"$class\" title=\"$title_ref\">" .
                                $link . "</span>";
                }
        }

^ permalink raw reply related

* Re: Changing branches in supermodule does not affect submodule?
From: Peter Krefting @ 2009-10-14  6:31 UTC (permalink / raw)
  To: Git Mailing List; +Cc: Jens Lehmann
In-Reply-To: <4AD47C65.5080904@web.de>

Jens Lehmann:

> just calling "git submodule update" every time you want the submodule to 
> be updated according to the state committed in the superproject will do 
> the trick (but keep in mind that all referenced commits have to be 
> accessible in the local clone of your submodule, so you might have to do a 
> fetch there once in a while).

Is it possible to automate this from a hook or something else? Basically, I 
would like it to update all the submodules to the state recorded in the 
commit I move to, if they were in a clean state before I moved. I do not 
want it to change states if I do something like

   cd submodule
   # do some changes
   git commit
   cd ..
   git checkout -b newbranch

because there I want the commit I made to the submodule to be recorded on 
the new branch I created. But then it was in a dirty state before I created 
the branch anyway, so that shouldn't be a problem.

-- 
\\// Peter - http://www.softwolves.pp.se/

^ permalink raw reply

* Re: git svn with non-standard svn layout
From: Eric Wong @ 2009-10-14  6:33 UTC (permalink / raw)
  To: Fabian Molder; +Cc: git
In-Reply-To: <loom.20091011T205226-197@post.gmane.org>

Fabian Molder <fm122@arcor.de> wrote:
> Eric Wong <normalperson <at> yhbt.net> writes:
> > Hi Fabian,
> > 
> > Since you don't want to track the entire repo and these seem like
> > unrelated (history-wise) trees, you probably want the simplest cases:
> > 
> >   git svn clone svn://example.com/path/to/xapplication2
> >   git svn clone svn://example.com/path/to/aa/bb/cc/xapplication1
> > 
> > These commands are like doing the following with plain old svn:
> > 
> >   svn co svn://example.com/path/to/xapplication2
> >   svn co svn://example.com/path/to/aa/bb/cc/xapplication1
> > 
> > > I tried to use "git config svn-remote.svn.branches" to do this,
> > >  please see in function "do_git_svn" in bash-script - but no success
> > 
> > svn-remote.svn.branches and tags are really only for repos with
> > standard layouts.
> > 
> 
> Hello Eric,
>   hmm, understand,
>   but this just does an checkout to the working dir
> 
>   the reason for using git is:
>     - work offline, with (at least read) access to all the svn branches
>     - have some more (privat, not commit back to svn) branches for experiments
>     - all the nice git stuff ..

Sorry, not sure if I understood you the first time.  The pastie was
hard to read on my screen (big fonts, low res).

I think what you need is to match the number of globs (*) on
both the remote and local sides:

  REMOTE=branches/*/*/aa/bb/cc/zapplication1
  LOCAL=refs/remotes/svn/aa/bb/cc/zapplication1/*/*
  git config svn-remote.svn.branches "${REMOTE}:${LOCAL}"

  #---> matches all in svn repo and also creates subdirs
#
#  git config svn-remote.svn.branches \
#  "branches/r*/development/aa/bb/cc/zapplication1:refs/remotes/svn/branches/*"
#
  #--> Missing trailing '/' on left side of: 'branches/r*/development/aa/bb/cc/zapplication1' (branches/r)

The glob code is still a bit wonky, but it needs to be "/*/" (that is
"*" must have a "/" or nothing, but not "/r*/"

But it looks like the example with 2 globs works.

-- 
Eric Wong

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Bert Wesarg @ 2009-10-14  6:49 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, elliot
In-Reply-To: <1255495525-11254-1-git-send-email-sam.vilain@catalyst.net.nz>

On Wed, Oct 14, 2009 at 06:45, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 06e0f31..8dc3fbf 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -143,21 +143,31 @@ pre-rebase
>  This hook is called by 'git-rebase' and can be used to prevent a branch
>  from getting rebased.
>
> +pre-checkout
> +-----------
>
> -post-checkout
> -~~~~~~~~~~~~~
> -
Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?

> -This hook is invoked when a 'git-checkout' is run after having updated the
> +This hook is invoked when a 'git-checkout' is run after before updating the
>  worktree.  The hook is given three parameters: the ref of the previous HEAD,
>  the ref of the new HEAD (which may or may not have changed), and a flag
>  indicating whether the checkout was a branch checkout (changing branches,
>  flag=1) or a file checkout (retrieving a file from the index, flag=0).
> -This hook cannot affect the outcome of 'git-checkout'.
> +This hook can prevent the checkout from proceeding by exiting with an
> +error code.
>
>  It is also run after 'git-clone', unless the --no-checkout (-n) option is
>  used. The first parameter given to the hook is the null-ref, the second the
>  ref of the new HEAD and the flag is always 1.
>
> +This hook can be used to perform any clean-up deemed necessary before
> +checking out the new branch/files.
> +
> +post-checkout
> +-----------
Ditto.

> +
> +This hook is invoked when a 'git-checkout' is run after having updated the
> +worktree.  It takes the same arguments as the 'pre-checkout' hook.
> +This hook cannot affect the outcome of 'git-checkout'.
> +
>  This hook can be used to perform repository validity checks, auto-display
>  differences from the previous HEAD if different, or set working dir metadata
>  properties.

Bert

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Sam Vilain @ 2009-10-14  6:50 UTC (permalink / raw)
  To: Bert Wesarg; +Cc: git, elliot
In-Reply-To: <36ca99e90910132349o25322021l266124bd8b0d30b3@mail.gmail.com>

Bert Wesarg wrote:
>> +pre-checkout
>> +-----------
>>
>> -post-checkout
>> -~~~~~~~~~~~~~
>> -
> Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?

Possibly the headings changed from the version I originally patched to
the current 'master'.  More likely, I just made a simple mistake :-}
-- 
Sam Vilain, Perl Hacker, Catalyst IT (NZ) Ltd.
phone: +64 4 499 2267        PGP ID: 0x66B25843

^ permalink raw reply

* Re: [PATCH] checkout: add 'pre-checkout' hook
From: Bert Wesarg @ 2009-10-14  7:04 UTC (permalink / raw)
  To: Sam Vilain; +Cc: git, elliot
In-Reply-To: <4AD574CA.8010900@catalyst.net.nz>

On Wed, Oct 14, 2009 at 08:50, Sam Vilain <sam.vilain@catalyst.net.nz> wrote:
> Bert Wesarg wrote:
>>> +pre-checkout
>>> +-----------
>>>
>>> -post-checkout
>>> -~~~~~~~~~~~~~
>>> -
>> Why do you change the caption from subsection (Ie. ~~~) to section (Ie. ---)?
>
> Possibly the headings changed from the version I originally patched to
> the current 'master'.
That was me one month ago ;-)

^ permalink raw reply

* Re: Bad URL passed to RA lay
From: Eric Wong @ 2009-10-14  7:07 UTC (permalink / raw)
  To: m.skoric; +Cc: git
In-Reply-To: <1952825702@web.de>

m.skoric@web.de wrote:
> Hi List,
> 
> i have a problem with git-svn clone / fetch. I get following error
> while doing one of previous command -> "Bad URL passed to RA lay" This
> happens because a branch doesn't exists in svn anymore and git wants
> to retrieve data from it. Here is the complete error message
> 
> Initializing parent: Abo-Uebernahme (Bug #994)@341
> Found possible branch point: "quoted"..trunk => "quoted"...Abo-Uebernahme (Bug #994), 203
> Found branch parent: (Abo-Uebernahme (Bug #994)@341) bb831869748c98bf97d105c5894ae65331c95c08
> Bad URL passed to RA layer: Malformed URL for repository at /usr/bin/git-svn line 4311
> 
> git version 1.6.3.3
> 
> Aynone else has this Problem?

Hi,

Unlikely, not many people use URIs as weird as yours :)
The existing test case (t9118) we have was also inspired by you,
on the same branch, even.

> Can anyone help me?

What exactly is the "quoted" you refer to?  That's not an actual branch
name, is it?

Can you try it with v1.6.5?  You might need to edit your $GIT_CONFIG,
but commit 5268f9edc3c86b07a64fcc2679e5ffe39be28d97 was the last
fix for URI-escaping problems:

  Author: Eric Wong <normalperson@yhbt.net>
  Date:   Sun Aug 16 14:22:12 2009 -0700

    svn: assume URLs from the command-line are URI-encoded
    
    And then unescape them when writing to $GIT_CONFIG.
    
    SVN has different rules for repository URLs (usually the root)
    and for paths within that repository (below the HTTP layer).
    Thus, for the request URI path at the HTTP level, the URI needs
    to be encoded.  However, in the body of the HTTP request (the
    with underlying SVN XML protocol), those paths should not be
    URI-encoded[1].  For non-HTTP(S) requests, SVN appears to be
    more flexible and will except weird characters in the URL as
    well as URI-encoded ones.
    
    Since users are used to using URLs being entirely URI-encoded,
    git svn will now attempt to unescape the path portion of URLs
    while leaving the actual repository URL untouched.
    
    This change will be reflected in newly-created $GIT_CONFIG files
    only.  This allows users to switch between svn(+ssh)://, file://
    and http(s):// urls without changing the fetch/branches/tags
    config keys.  This won't affect existing imports at all (since
    things didn't work before this commit anyways), and will allow
    users to force escaping into repository paths that look like
    they're escaped (but are not).
    
    Thanks to Mike Smullin for the original bug report and Björn
    Steinbrink for summarizing it into testable cases for me.
    
    [1] Except when committing copies/renames, see
        commit 29633bb91c7bcff31ff3bb59378709e3e3ef627d


And if git v1.6.5 doesn't work, would you mind looking at (and possibly
modifying) the t9118 test in git to reproduce since it's already got your
oddly-named branch in it?   Thanks!

-- 
Eric Wong

^ 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