* 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: [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: 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 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: [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 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
* 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
* [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
* [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
* 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
* 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
* git hang with corrupted .pack
From: Andy Isaacson @ 2009-10-14 4:22 UTC (permalink / raw)
To: git
I have a clone of torvalds/linux-2.6.git that got corrupted; many git
commands hang with a backtrace like
% git cat-file -p 60fa3f769f7651a60125a0f44e3ffe3246d7cf39
...
#0 use_pack (p=0x1dcf310, w_cursor=0x7fffb4c0e430, offset=43544957,
left=0x7fffb4c0e348) at sha1_file.c:792
#1 0x00000000004990ed in unpack_compressed_entry (p=0x1dcf310,
w_curs=0x7fffb4c0e430, curpos=43544957, size=728) at sha1_file.c:1594
#2 0x000000000049b089 in unpack_entry (p=0x1dcf310, obj_offset=43544534,
type=0x7fffb4c0e7b4, sizep=0x7fffb4c0e7a8) at sha1_file.c:1821
#3 0x000000000049b5f2 in read_packed_sha1 (
sha1=0x7fffb4c0e780 "`xxxxxxxxxxxxxxxxxxxxxxxxxxx",
type=0x7fffb4c0e7b4, size=0x7fffb4c0e7a8) at sha1_file.c:2054
#4 0x000000000049b6d6 in read_object (
sha1=0x7fffb4c0e780 "`xxxxxxxxxxxxxxxxxxxxxxxxxxx",
type=0x7fffb4c0e7b4, size=0x7fffb4c0e7a8) at sha1_file.c:2142
#5 0x000000000049b765 in read_sha1_file (sha1=0x1dcf310 "",
type=0x7fffb4c0e430, size=0x0) at sha1_file.c:2158
#6 0x00000000004128da in cmd_cat_file (argc=<value optimized out>,
argv=<value optimized out>, prefix=<value optimized out>)
at builtin-cat-file.c:125
#7 0x0000000000404293 in handle_internal_command (argc=3, argv=0x7fffb4c0ea30)
at git.c:246
#8 0x0000000000404454 in main (argc=3, argv=0x7fffb4c0ea30) at git.c:438
(I overwrote the UTF8 that gdb helpfully spewed with 'x'.)
We're looping in unpack_compressed_entry, adding a fprintf immediately
after the call to git_inflate() shows:
git_inflate returned -5 next_in=0x7f5e602bc17d curpos=43544536 avail_in=293462652 avail_out=0
git_inflate returned -5 next_in=0x7f5e602bc17d curpos=43544957 avail_in=293462652 avail_out=0
git_inflate returned -5 next_in=0x7f5e602bc17d curpos=43544957 avail_in=293462652 avail_out=0
git_inflate returned -5 next_in=0x7f5e602bc17d curpos=43544957 avail_in=293462652 avail_out=0
git_inflate returned -5 next_in=0x7f5e602bc17d curpos=43544957 avail_in=293462652 avail_out=0
The pack is corrupt:
% hd -s $((0x2986fd0)) .git/objects/pack/pack-9836f9e49a483ad95e7de546d775a4f247e6ac74.pack
02986fd0 f6 17 44 74 4e d5 98 2d 78 9c 95 51 5d 8b db 30 |..DtN..-x..Q]..0|
02986fe0 10 7c d7 af d8 1f d0 18 cb df 0e a1 04 72 77 10 |.|...........rw.|
02986ff0 b8 f4 a0 97 f7 20 4b ab 58 77 8a 64 24 f9 92 fc |..... K.Xw.d$...|
02987000 fb ca 4e 08 a5 b4 0f 7d d3 ce 8e 66 76 77 82 43 |..N....}...fvw.C|
02987010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
02987020 00 00 00 00 00 00 00 00 0f 00 00 00 00 00 00 00 |................|
02987030 0f 00 00 00 33 a4 6d db 94 35 4d 5b 51 74 59 d9 |....3.m..5M[QtY.|
02987040 b5 b4 ca 51 ca a2 2a aa 28 83 88 b9 20 6c 0c bd |...Q..*.(... l..|
02987050 75 b0 77 d6 08 d8 5d 3f 35 76 a3 0f b0 9a 81 e4 |u.w...]?5v......|
02987060 01 ac 0d 06 36 0c 09 b7 a7 ef 40 69 5d 95 6d 96 |....6.....@i].m.|
02987070 d3 0c 16 69 91 a6 24 a2 27 15 02 3a 78 55 66 f4 |...i..$.'..:xUf.|
02987080 b0 b7 ee 8b 69 e1 61 15 ee af f5 d9 5a 71 4d 74 |....i.a.....ZqMt|
02987090 6c 5f 16 d2 8e 46 b0 a0 ac 49 ac 3b de f4 2a 9a |l_...F...I.;..*.|
029870a0 15 69 13 f5 ea a8 47 7e bc bc 2f e1 45 5d 20 9c |.i....G~../.E] .|
029870b0 2d 74 e3 d1 83 32 10 7a 84 b7 c3 d3 f6 e7 f3 66 |-t...2.z.......f|
and that corruption is almost certainly a kernel bug or hardware
failure, but git shouldn't lock up.
Ilari on #git suggested the following, which does resolve the hangs in
my case.
diff --git a/sha1_file.c b/sha1_file.c
index 4ea0b18..838df82 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1594,6 +1594,8 @@ static void *unpack_compressed_entry(struct packed_git *p,
in = use_pack(p, w_curs, curpos, &stream.avail_in);
stream.next_in = in;
st = git_inflate(&stream, Z_FINISH);
+ if (stream.next_in == in)
+ break;
curpos += stream.next_in - in;
} while (st == Z_OK || st == Z_BUF_ERROR);
git_inflate_end(&stream);
If anyone has a suggestion for how 36 bytes of my .pack got overwritten,
I'm interested in that too; it's a Core 2 Duo (Thinkpad x200) running
Ubuntu Karmic beta with 2.6.31-13 (upgraded from -10 it looks like),
Intel GMA graphics, CONFIG_DMAR=n, ext4 on the internal HDD, which now
that I look at it actually does have a fair number of non-zero SMART
counters:
Device Model: FUJITSU MHZ2160BH G1
Serial Number: K60WT8C2HHRS
Firmware Version: 0084000A
User Capacity: 160,041,885,696 bytes
...
ID# ATTRIBUTE_NAME FLAG VALUE WORST THRESH TYPE UPDATED WHEN_FAILED RAW_VALUE
1 Raw_Read_Error_Rate 0x000f 100 100 046 Pre-fail Always - 219593
2 Throughput_Performance 0x0005 100 100 030 Pre-fail Offline - 27721728
3 Spin_Up_Time 0x0003 100 100 025 Pre-fail Always - 0
4 Start_Stop_Count 0x0032 099 099 000 Old_age Always - 406
5 Reallocated_Sector_Ct 0x0033 100 100 024 Pre-fail Always - 8589934592000
7 Seek_Error_Rate 0x000f 100 100 047 Pre-fail Always - 112
8 Seek_Time_Performance 0x0005 100 100 019 Pre-fail Offline - 0
9 Power_On_Hours 0x0032 097 097 000 Old_age Always - 1598
10 Spin_Retry_Count 0x0013 100 100 020 Pre-fail Always - 0
12 Power_Cycle_Count 0x0032 100 100 000 Old_age Always - 284
192 Power-Off_Retract_Count 0x0032 100 100 000 Old_age Always - 78
193 Load_Cycle_Count 0x0032 100 100 000 Old_age Always - 1216
194 Temperature_Celsius 0x0022 100 100 000 Old_age Always - 38 (Lifetime Min/Max 21/46)
195 Hardware_ECC_Recovered 0x001a 100 100 000 Old_age Always - 247
196 Reallocated_Event_Count 0x0032 100 100 000 Old_age Always - 457965568
197 Current_Pending_Sector 0x0012 100 100 000 Old_age Always - 0
198 Offline_Uncorrectable 0x0010 100 100 000 Old_age Offline - 0
199 UDMA_CRC_Error_Count 0x003e 200 253 000 Old_age Always - 0
200 Multi_Zone_Error_Rate 0x000f 100 100 060 Pre-fail Always - 10448
203 Run_Out_Cancel 0x0002 100 100 000 Old_age Always - 1529011503750
240 Head_Flying_Hours 0x003e 200 200 000 Old_age Always - 0
-andy
^ permalink raw reply related
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Junio C Hamano @ 2009-10-14 3:28 UTC (permalink / raw)
To: Jay Soffian
Cc: Johannes Schindelin, Jeff King, Thomas Rast, Euguess,
Mikael Magnusson, Matthieu Moy, git, Johannes Sixt
In-Reply-To: <76718490910131805o42e8321ama85b90b7e901dc7d@mail.gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
> This doesn't help with the original problem, which was that a user
> attempted to checkout refs/remotes/origin/<name> by just saying 'git
> checkout <name>' which I happen to think should work. A lot of what I
> keep hearing in this thread seems to be in the vein of the perfect
> being the enemy of the good.
I do not think there is "perfect" nor "good" anywhere in this. It is just
the proposals were either not well thought out, were not presented well,
or were misunderstood, or a bit of all.
When you do not have local "frotz" branch, and do have cloned/fetched from
the origin that has "frotz" branch, I am actually Ok with this
$ git checkout frotz [--]
to do an equivalent of:
$ git checkout -t -b frotz origin/frotz
I do not have problem with this _particular_ DWIMmery. It will not break
people's expectations, other than "asking to check out non-existing ref
should fail". That expectation might be logical, but I do not think it is
useful.
Another reason I won't have problem with this one is that perhaps after
creating a few more commits, the next day when the user does the same
$ git checkout frotz
what will be shown is the _local_ frotz branch. Nowhere in this sequence
there is any room to mistake that you somehow checked out a branch owned
by somebody else (namely, origin). You started by auto-creating your
local branch, worked on it, and checked it out again the next day. In
other words, this is really about a shorthand to create a new local branch
called "frotz" when the commit that the branch should start from is
clearly unambiguous.
I have trouble with yours, on the other hand, which is to make
$ git checkout origin/frotz
$ git checkout v1.5.5
into
$ git checkout -b frotz-47 origin/frotz
$ git checkout -b v1.5.5-47 v1.5.5
(replace -47 with whatever random string you would come up with to make it
unique), as it _will_ break people's expectations, and the expected
behaviour to detach without polluting the local branch namespace for
the purpose of sightseeing happens to be a useful one.
I also have issues with turning
$ git checkout origin/frotz
into
$ git checkout -b frotz origin/frotz
only when frotz does not exist locally. This will cause the "next day"
problem, and also by naming the remote tracking branch, gives a wrong
impression that this is about a remote branch. It should not be.
Perhaps without touching the "detached" case at all, if we limit the scope
of the change that comes out of this discussion to only one case, it might
result in a good trouble-free enhancement [*1*].
The new rule would be:
"git checkout $name", when all of the following holds:
- $name is a good name for a local branch (i.e. check-ref-format is
happy);
- No local branch of that name exists;
- There is exactly one remote $remote that has $name branch; and
- $name itself is not a good commit name (i.e. get_sha1() barfs)
is a request to create a local branch $name, and the branch tracks the
remote tracking branch found in the third condition [*2*].
The important point here is that this exception is _not_ about remote
tracking branch but is about a rule to allow omitting -b to create and
checkout a local branch when the user's intent is clear that (1) he wants
to create a new one named $name, and (2) he wants to create it starting at
the commit $remote/$name.
Such a change feels quite safe and I wouldn't be opposed to it.
We _could_ discuss extending the $name in the above rule to other kinds
(tags and even arbitrary committish that may not even have a direct ref
pointing at it), but I think they are much more problematic.
[Footnote]
*1* Yes, I know I won't try to come with a strawman.
*2* The fourth condition is to avoid taking "origin/frotz" when "origin"
remote has "frotz" branch _and_ "other" remote has "origin/frotz" branch.
The remote chosen by the third condition would be "other" (because
"origin" remote only has "frotz", and not "origin/frotz", the name is
unique in the sense of the third condition). The fourth condition
prevents this from happening, and forbids an explicit request to detach
HEAD at one point (i.e. "origin/frotz") from triggering.
^ permalink raw reply
* [PATCH] change throughput display units with fast links
From: Nicolas Pitre @ 2009-10-14 3:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Switch to MiB/s when the connection is fast enough (i.e. on a LAN).
Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---
diff --git a/progress.c b/progress.c
index 132ed95..3971f49 100644
--- a/progress.c
+++ b/progress.c
@@ -131,7 +131,13 @@ static void throughput_string(struct throughput *tp, off_t total,
} else {
l -= snprintf(tp->display, l, ", %u bytes", (int)total);
}
- if (rate)
+
+ if (rate > 1 << 10) {
+ int x = rate + 5; /* for rounding */
+ snprintf(tp->display + sizeof(tp->display) - l, l,
+ " | %u.%2.2u MiB/s",
+ x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
+ } else if (rate)
snprintf(tp->display + sizeof(tp->display) - l, l,
" | %u KiB/s", rate);
}
^ permalink raw reply related
* Re: [PATCH 0/2] user-manual: reorganize the configuration steps
From: J. Bruce Fields @ 2009-10-14 2:49 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Junio C Hamano, Jonathan Nieder
In-Reply-To: <1255293786-17293-1-git-send-email-felipe.contreras@gmail.com>
On Sun, Oct 11, 2009 at 11:43:04PM +0300, Felipe Contreras wrote:
> This basically introduces the "getting started" section so users get familiar
> with the configuration from the get-go, and also, most people prefer to teach
> 'git config --global' to setup the user name and email. Here are a few
> examples:
I'm not personally a big fan of starting out with a "how to use
git-config" section, because it's not that difficult or important:
questions we get on this list suggest confusion about a lot of things,
but git configuration is rarely one of them (that I've noticed).
I'd rather just point people to the git-config man page the first time
we mention any git configuration. (And improve the man page if
necessary to ensure it's up to the job.)
If we have to do this, just keep it short....
--b.
>
> git tutorial:
> http://www.kernel.org/pub/software/scm/git/docs/gittutorial.html
>
> GNOME:
> http://live.gnome.org/Git/Developers
>
> SourceForge:
> http://sourceforge.net/apps/trac/sourceforge/wiki/Git
>
> github:
> http://help.github.com/git-email-settings/
>
> Felipe Contreras (2):
> user-manual: add global config section
> user-manual: simplify the user configuration
>
> Documentation/user-manual.txt | 35 ++++++++++++++++++++++++++++++-----
> 1 files changed, 30 insertions(+), 5 deletions(-)
>
^ permalink raw reply
* [PATCH] gitweb: linkify author/committer names with search
From: Stephen Boyd @ 2009-10-14 2:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Giuseppe Bilotta, Jakub Narebski
It's nice to search for an author by merely clicking on their name in
gitweb. This is usually faster than selecting the name, copying the
selection, pasting it into the search box, selecting between
author/committer and then hitting enter.
Linkify the avatar icon in log/shortlog view because the icon is directly
adjacent to the name and thus more related. The same is not true
when in commit/tag view where the icon is farther away.
Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
This is on top of Giuseppe's fix esc_param patch.
gitweb/gitweb.css | 1 +
gitweb/gitweb.perl | 21 ++++++++++++++++-----
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/gitweb/gitweb.css b/gitweb/gitweb.css
index 8f68fe3..e2cd9d7 100644
--- a/gitweb/gitweb.css
+++ b/gitweb/gitweb.css
@@ -30,6 +30,7 @@ img.logo {
img.avatar {
vertical-align: middle;
+ border-width: 0px;
}
div.page_header {
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 4b21ad2..bdf1037 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1602,8 +1602,12 @@ sub format_author_html {
my $co = shift;
my $author = chop_and_escape_str($co->{'author_name'}, @_);
return "<$tag class=\"author\">" .
- git_get_avatar($co->{'author_email'}, -pad_after => 1) .
- $author . "</$tag>";
+ $cgi->a({-href => href(action=>"search", hash=>$hash,
+ searchtext=>$co->{'author_name'},
+ searchtype=>"author"), class=>"list"},
+ git_get_avatar($co->{'author_email'}, -pad_after => 1) .
+ $author) .
+ "</$tag>";
}
# format git diff header line, i.e. "diff --(git|combined|cc) ..."
@@ -3372,10 +3376,13 @@ sub git_print_authorship {
my $co = shift;
my %opts = @_;
my $tag = $opts{-tag} || 'div';
+ my $author = $co->{'author_name'};
my %ad = parse_date($co->{'author_epoch'}, $co->{'author_tz'});
print "<$tag class=\"author_date\">" .
- esc_html($co->{'author_name'}) .
+ $cgi->a({-href => href(action=>"search", searchtext=>$author,
+ searchtype=>"author"), class=>"list"},
+ esc_html($author)) .
" [$ad{'rfc2822'}";
print_local_time(%ad) if ($opts{-localtime});
print "]" . git_get_avatar($co->{'author_email'}, -pad_before => 1)
@@ -3394,8 +3401,12 @@ sub git_print_authorship_rows {
@people = ('author', 'committer') unless @people;
foreach my $who (@people) {
my %wd = parse_date($co->{"${who}_epoch"}, $co->{"${who}_tz"});
- print "<tr><td>$who</td><td>" . esc_html($co->{$who}) . "</td>" .
- "<td rowspan=\"2\">" .
+ print "<tr><td>$who</td><td>" .
+ $cgi->a({-href => href(action=>"search",
+ searchtext=>$co->{"${who}_name"},
+ searchtype=>$who), class=>"list"},
+ esc_html($co->{$who})) .
+ "</td><td rowspan=\"2\">" .
git_get_avatar($co->{"${who}_email"}, -size => 'double') .
"</td></tr>\n" .
"<tr>" .
--
1.6.5.1.g75846.dirty
^ permalink raw reply related
* Re: [PATCH] gitweb: fix esc_param
From: Stephen Boyd @ 2009-10-14 1:13 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git, Jakub Narebski, Junio C Hamano
In-Reply-To: <1255463496-21617-1-git-send-email-giuseppe.bilotta@gmail.com>
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?
This works great for my purposes. Thanks.
^ permalink raw reply
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Jay Soffian @ 2009-10-14 1:05 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Jeff King, Junio C Hamano, Thomas Rast, Euguess, Mikael Magnusson,
Matthieu Moy, git, Johannes Sixt
In-Reply-To: <alpine.DEB.1.00.0910140117280.4985@pacific.mpi-cbg.de>
On Tue, Oct 13, 2009 at 7:22 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> 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.
Modify checkout so that the first commit while detached automatically
creates a branch. Perhaps the name is derived from the branch point,
or the user is prompted for a name.
This doesn't help with the original problem, which was that a user
attempted to checkout refs/remotes/origin/<name> by just saying 'git
checkout <name>' which I happen to think should work. A lot of what I
keep hearing in this thread seems to be in the vein of the perfect
being the enemy of the good.
That rambled a bit. Sorry.
j.
^ permalink raw reply
* Re: [PATCH] clone: Supply the right commit hash to post-checkout when -b is used
From: Jeff King @ 2009-10-14 0:06 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: Junio C Hamano, git, ranguvar
In-Reply-To: <20091013221109.GA30972@atjola.homenet>
On Wed, Oct 14, 2009 at 12:11:09AM +0200, Björn Steinbrink wrote:
> When we use -b <branch>, we may checkout something else than what the
> remote's HEAD references, but we still used remote_head to supply the
> new ref value to the post-checkout hook, which is wrong.
>
> So instead of using remote_head to find the value to be passed to the
> post-checkout hook, we have to use our_head_points_at, which is always
> correctly setup, even if -b is not used.
>
> This also fixes a segfault when "clone -b <branch>" is used with a
> remote repo that doesn't have a valid HEAD, as in such a case
> remote_head is NULL, but we still tried to access it.
>
> Reported-by: Devin Cofer <ranguvar@archlinux.us>
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Acked-by: Jeff King <peff@peff.net>
Thanks.
When splitting remote_head versus our_head, I tried to find every use of
the remote head and pick the appropriate variable, but I think I just
missed this one. I gave the code another once-over and didn't see any
other spots that needed fixing.
-Peff
^ permalink raw reply
* Re: [PATCH v5 2/2] gitweb: append short hash ids to snapshot files
From: Jakub Narebski @ 2009-10-13 23:46 UTC (permalink / raw)
To: Mark Rada; +Cc: Junio C Hamano, git
In-Reply-To: <4AD34C93.20605@mailservices.uwaterloo.ca>
On Mon, 12 Oct 2009, Mark Rada wrote:
> On 09-10-05 6:06 AM, Jakub Narebski wrote:
>
>>> my $o_git_dir = $git_dir;
>>> my $retval = undef;
>>> $git_dir = "$projectroot/$project";
>>> - if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
>>> - my $head = <$fd>;
>>> + if (open my $fd, '-|', git_cmd(), 'rev-parse', '--verify', $hash) {
>>> + $hash = <$fd>;
>>> close $fd;
>>> - if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
>>> + if (defined $hash && $hash =~ /^([0-9a-fA-F]{40})$/) {
>>> + $retval = $1;
>>> + }
>>
>> I guess that you use "$retval = $1;" instead of just "$retval = $hash;"
>> because of similarities with git_get_short_hash, isn't it? Or it is just
>> following earlier code?
>
> Yeah, it is following earlier code, I did not change it,
Ah, that's O.K.
Although if you plan refactoring this, you might fix this bit of
inefficiency (no need for capturing).
> though the diff seems to think I added it, perhaps this is a bug with
> diff?
No, that is just diff being ambiguous, as there is more than one way
to generate diff of changes. Perhaps patience diff would produce
better results, perhaps not. It might mean that refactoring common
code is needed ;-)))))
>>> + }
>>> + if (defined $o_git_dir) {
>>> + $git_dir = $o_git_dir;
>>> + }
>>> + return $retval;
>>> +}
>>> +
>>> +# try and get a shorter hash id
>>> +sub git_get_short_hash {
>>> + my $project = shift;
>>> + my $hash = shift;
>>> + my $o_git_dir = $git_dir;
>>> + my $retval = undef;
>>> + $git_dir = "$projectroot/$project";
>>> + if (open my $fd, '-|', git_cmd(), 'rev-parse', '--short=7', $hash) {
>>> + $hash = <$fd>;
>>> + close $fd;
>>> + if (defined $hash && $hash =~ /^([0-9a-fA-F]{7,})$/) {
>>> $retval = $1;
>>> }
>>> }
>>
>> Note that git_get_full_hash (which additionally does verification) and
>> git_get_short_hash share much of code. Perhaps it might be worth to
>> avoid code duplication somehow? On the other hand it might be not worth
>> to complicate code by trying to extract common parts here...
>
> Hmm, I think it might be a good idea to just write a generic routine
> that takes a hash length as an extra parameter. Then the short and full
> hash fetching routines can just acts as wrappers.
Well, git_get_full_hash uses --verify, git_get_short_hash uses --short=7
(but perhaps it should also use --verify).
BTW. I think that checking that output of git-rev-parse is (shortened)
SHA-1 predates usage of --verify; with --verify is, I think, not
necessary:
--verify
The parameter given must be usable as a single, valid object name.
Otherwise barf and abort.
>>> @@ -5203,6 +5228,13 @@ sub git_snapshot {
>>> die_error(400, 'Object is not a tree-ish');
>>> }
>>>
>>> +
>>> + my $full_hash = git_get_full_hash($project, $hash);
>>> + if ($full_hash =~ /^$hash/) {
BTW, we can use
if (index($full_hash, $hash) == 0) {
instead. BTW, $hash could contain regexp metacharacters like '.'
('dead.beef' is a valid branch name), so it should be
if ($full_hash =~ /^\Q$hash/) {
if you want to use regexp (it might be easier to read).
Or you can encapsulate this into is_substring() subroutine, but that
might be (well, almost surely is) overkill...
>>> + $hash = git_get_short_hash($project, $hash);
>>> + } else {
>>> + $hash .= '-' . git_get_short_hash($project, $hash);
>>> + }
>>
>> I think we might want to avoid calling git_get_full_hash (and extra call
>> to "git rev-parse" command, which is extra fork) if we know in advance
>> that $full_hash =~ /^$hash/ can't be true, i.e. if $hash doesn't match
>> /^[0-9a-fA-F]+$/. That would require that we continue to use $hash
>> and not $full_hash, see comment for the chunk below.
>>
>> BTW do you think that having better name (nicer name in the case
>> when $hash is full SHA-1, or name which describes exact version as
>> in the case when $hash is branch name or just 'HEAD') is worth
>> slight extra cost of "git rev-parse --abbrev=7"?
>
> Hmm, yeah, some optimization will have to occur in that block of
> code. Though, my reason for that extra call to rev-parse to get the
> short hash is so I can get git to find the shortest unique SHA-1,
> instead of just assuming that it will always be of length 7. I think
> the cost is not too bad considering a snapshot will have to be generated
> and probably take way more time. Though, warthog9 has some caching
> patches that work, so maybe it isn't worth it. Hmm...
What I meant here that unless $hash =~ /^[0-9a-fA-F]{7,}$/ then we
always use git_get_short_hash, as $full_hash wouldn't match /^$hash/
($hash wouldn't be a prefix of $full_hash). We don't need to
calculate git_get_full_hash which wouldn't be used (see also comment
below, though).
>
>>> my $name = $project;
>>> $name =~ s,([^/])/*\.git$,$1,;
>>> $name = basename($name);
>>> @@ -5213,7 +5245,7 @@ sub git_snapshot {
>>> $cmd = quote_command(
>>> git_cmd(), 'archive',
>>> "--format=$known_snapshot_formats{$format}{'format'}",
>>> - "--prefix=$name/", $hash);
>>> + "--prefix=$name/", $full_hash);
>>
>> Why this change?
>
> Since $hash can change by becoming something like 'HEAD-43ab5f2c' due to
> the process of creating the better name we need to pass something to
> `archive' that will be valid, and $full_hash will be valid.
Errr... why it is called _$hash_ then, if it can be not hash? Wouldn't
it be better to manipulate $name here?
I think this fragment should be extracted into snapshot_name() subroutine,
which result would be used both as proposed snapshot name, and as prefix
to be used.
>
>>> +test_description='gitweb as standalone script (parsing script output).
>>> +
>>> +This test runs gitweb (git web interface) as a CGI script from the
>>> +commandline, and checks that it produces the correct output, either
>>> +in the HTTP header or the actual script output.'
>>
>> Currently all tests here are about 'snapshot' action. They are quite
>> specific, and they do require some knowledge about chosen archive format.
That is not true, as I haven't noticed at this point that you are
examining only HTTP headers... but not the HTTP status but other headers.
[...]
>>> + grep ".git-$ID.tar.gz" gitweb.output
>>
>> Here had to think a bit that gitweb.output consists both of HTTP headers,
>> and of response body, and you are grepping here in the HTTP headers part.
>> It would be better solution for gitweb_run to split gitweb.output into
>> gitweb.headers and gitweb.body (perhaps if requested by setting some
>> variable, e.g. GITWEB_SPLIT_OUTPUT).
>>
>> It can be done using the following lines:
>>
>> sed -e '/^\r$/' <gitweb.output>gitweb.headers
That was meant to be
>> sed -e '/^\r$/q' <gitweb.output >gitweb.headers
which means print (the default action) until single empty CRLF terminated
line, which ends HTTP headers.
>> sed -n -e '0,/^\r$/!p' <gitweb.output>gitweb.body
>>
>> # gitweb.headers is used to parse http headers
>> # gitweb.body is response without http headers
>>
>> But the second one uses GNU sed extension; I don't know how to write
>> it in more portable way.
>
> I like this and will try to find a way of setting this up without using
> GNU extensions.
Well, we do know that there always would be at least one header, so we
can use:
sed -n -e '1,/^\r$/!p' <gitweb.output >gitweb.body
But I'd prefer that somebody better versed in sed would come up with
solution to extract everything up to first empty CRLF terminated line,
and everything from such line till the end of file.
>> Note that to avoid ambiguities currently gitweb uses refs/heads/master
>> and refs/tags/SnapshotFileTests... but dealing with this issue should be
>> left, I think, for separate commit.
>>
>
> I do not understand what ambiguity exists, can you please explain this?
The problem I was thinking about is the following.
In commit bf901f8 (gitweb: disambiguate heads and tags withs the same
name, 2007-12-15) started to use refs/heads/<branch> and refs/tags/<tag>
instead of <branch> and <tag> because there was problem when there were
tag and branch with the same name.
The problem is that we can't use '/' in proposed snapshot file name,
and we shouldn't use '/' in git-archive prefix. So we can't simply
use (as you proposed)
$hash . '-' . git_get_short_hash($project, $hash);
as a snapshot basename suffix, because $hash can be 'refs/heads/master',
or it can be 'mr/gitweb-snapshot'. What to do, what to do...
Also if $hash is refs/tags/v1.6.0, we don't really need shortened SHA-1
suffix.
Alternative to checking for refs/tags/ prefix would be to use
git-describe output... perhaps.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Schindelin @ 2009-10-13 23:22 UTC (permalink / raw)
To: Jeff King
Cc: Junio C Hamano, Thomas Rast, Euguess, Mikael Magnusson,
Matthieu Moy, Jay Soffian, git, Johannes Sixt
In-Reply-To: <20091013220640.GB12603@coredump.intra.peff.net>
Hi,
On Tue, 13 Oct 2009, Jeff King wrote:
> On Tue, Oct 13, 2009 at 11:20:28PM +0200, Johannes Schindelin wrote:
>
> > So in my opinion, we should DWIM "git checkout $X" to mean "git checkout
> > -b $X refs/remotes/$REMOTE/$X" when there is no ref $X, refs/heads/$X and
> > no other refs/remotes/$OTHER/$X.
>
> The similar suggestion that is less magical is to say something like
> "there is no $X; maybe you meant $REMOTE/$X?".
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.
> Is there a reason not to phase in the behavior, to make sure it is not
> doing unexpected things?
Sure, I have nothing against that. But just insisting on the current
behavior, or on some behavior that is not helpful at all, well, is not
really clever.
Note that I am fully aware that my "git checkout -t origin/master" DWIMery
backfired quite badly. So I am in the same boat.
> In other words:
>
> 1. In v1.6.6, find all error-correcting candidates and print them as
> a suggestion (similar to what we do with "git foo").
>
> 2. Then, if we all agree that it seems to be producing sane results,
> the next step is to turn the unambiguous cases into a DWIM (and
> leave the ambiguous ones with the "did you mean?" message).
>
> Because right now I think there are a lot of hypothetical "maybe it
> would be less convenient or more confusing in this instance", but we
> don't have any data on how often those instances occur, or how actual
> users might react.
Oh, I do not want to spam the list with user experiences. But I do have
not only a faint idea how users react. Thankyouverymuch.
> 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.
The step (1) would help in the way that it is a smoother transition.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Johannes Schindelin @ 2009-10-13 23:16 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Daniel Barkalow, Johannes Sixt, Thomas Rast, Euguess,
Mikael Magnusson, Matthieu Moy, Jay Soffian, git
In-Reply-To: <7vhbu2syi6.fsf@alter.siamese.dyndns.org>
Hi,
On Tue, 13 Oct 2009, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Tue, Oct 13, 2009 at 05:31:46PM -0400, Daniel Barkalow wrote:
> >
> >> I personally think that the real issue is that our "detached HEAD" message
> >> is still too scary, and what we really want is to issue the scary message
> >> when using "git commit" to move a detached HEAD from what was checked out
> >> to a new commit.
> >
> > This has been discussed before (I happen to agree with you, but you
> > probably want to address other comments in the thread):
> >
> > http://thread.gmane.org/gmane.comp.version-control.git/38201/focus=38213
>
> I just re-read the discussion again (thanks for a useful pointers). I
> mostly agree with everything said in the thread and obviously agree with
> its conclusion, but one thing I noticed that everybody (who _was_ a git
> expert) in the thread was assuming bothered me somewhat.
We can of course continue to this public wanking session in our nice
little circle here on git@vger, fully aware that real users will not dare
to interrupt us.
In the alternative, we can go out into the world (you know, that thing
behind the computer screen?) and ask somebody who has _not_ been exposed
to Git for _4 years_ (like most of you!) just how hard it is to work with
Git.
Let me tell you this from my experience: the least likely answer is "the
messages are too scary". Invariably, the answer I get is "it is totally
unintuitive". Often followed by "I tell Git to do something
straight-forward, and it refuses to do it."
Maybe we should just admit that we are no user interface designers, so one
cannot expect miracles from us in that respect. And first and foremost,
we should not pretend to ourselves that we are good at user interfaces,
because we have a track record of sucking in that area. Big time.
Ciao,
Dscho
P.S.: As somebody mentioned already, it is time to fix the tools, not our
users.
^ permalink raw reply
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Junio C Hamano @ 2009-10-13 22:46 UTC (permalink / raw)
To: Jeff King
Cc: Daniel Barkalow, Johannes Sixt, Thomas Rast, Johannes Schindelin,
Euguess, Mikael Magnusson, Matthieu Moy, Jay Soffian, git
In-Reply-To: <20091013215751.GA12603@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Tue, Oct 13, 2009 at 05:31:46PM -0400, Daniel Barkalow wrote:
>
>> I personally think that the real issue is that our "detached HEAD" message
>> is still too scary, and what we really want is to issue the scary message
>> when using "git commit" to move a detached HEAD from what was checked out
>> to a new commit. So:
>
> This has been discussed before (I happen to agree with you, but you
> probably want to address other comments in the thread):
>
> http://thread.gmane.org/gmane.comp.version-control.git/38201/focus=38213
I just re-read the discussion again (thanks for a useful pointers). I
mostly agree with everything said in the thread and obviously agree with
its conclusion, but one thing I noticed that everybody (who _was_ a git
expert) in the thread was assuming bothered me somewhat.
In this sequence:
1$ git checkout $commit_name_that_is_not_a_local_branch
2$ git commit; hack; hack; hack;...
3$ git checkout $branch_name
Step #1 is where the HEAD is detached. It is correct to argue that
detached HEAD is a different state and we should inform unsuspecting
users, which we do.
Step #2 is where a commit that is not connected to any ref is made.
Step #3 is where the state built in the detached HEAD "branch" vanishes
into lost-found.
The experts argued that #3 is where it is dangerous, and while it is
technically correct, an unsuspecting non-expert would not even _know_ that
nothing dangerous is happening while in step #2.
If the commit name used in step #1 were "v1.0.0", and if the user while in
step #2 ran "gitk v1.0.0" (or "git log v1.0.0"), he will be confused by
not seeing the recent commits. The distinction between "detached HEAD"
and being on a branch needs to be understood to appreciate this (and taken
advantage of, when running e.g. "git show-branch v1.0.0 HEAD").
Way before step #3, such a user, even though technically not in any danger
yet, would be confused and panic: "I wanted to fix something in the 1.0.0
release, but where did my fix go?"
The current message in step #1 reads like this:
$ git checkout origin/next
Note: moving to 'origin/next' 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>
HEAD is now at 9ecb2a7... Merge branch 'maint'
And perhaps for people who do not understand the second point in the
four-point list [*1*] I showed earlier in the thread, "If you want to
create a new branch" may not be descriptive enough, as a sight-seer and an
occasional typofixer, the user does not know what branch is good for to
begin with, and would not be able to tell if s/he even "wants to create"
one. Perhaps it would help more if we reworded three lines after "Note:"
with something like:
To keep the history of commits you will build from now on in a branch,
you may want to do "git checkout -b <new-branch-name>" now.
and customize the "in a branch" and <new-branch-name> part if the checkout
was given a remote tracking branch and the corresponding local branch does
not yet exist, e.g. in the above example:
To keep the history of commits you will build from now on in 'next'
branch, you may want to do "git checkout -b next" now.
[Footnote]
*1* The world model in which a git user works is:
* You clone and get copies of where the other end has its branches;
* You do all your work on your local branches;
* You may incorporate what the other end further did by merging from the
tracking branch from it;
* You update the other end by pushing what you did on your local branches.
I do not think you can nor should hide them from the user [*2*].
*2* We had to repeat "don't hide but teach" many times until it finally
sank in for another essential thing in the git world model. I hope we do
not have to do the same repeating for the above four points. Luckily we
do not have to repeat "don't hide but teach" about the index anymore these
days.
^ permalink raw reply
* Re: [PATCH/RFC] builtin-checkout: suggest creating local branch when appropriate to do so
From: Björn Steinbrink @ 2009-10-13 22:38 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, Johannes Sixt, Thomas Rast, Johannes Schindelin,
Euguess, Mikael Magnusson, Matthieu Moy, Jeff King, Jay Soffian,
git
In-Reply-To: <alpine.LNX.2.00.0910131654270.32515@iabervon.org>
On 2009.10.13 17:31:46 -0400, Daniel Barkalow wrote:
> The other thing that I think would be nice is:
>
> $ git checkout origin/next
> $ git fetch origin
> $ git checkout !! (probably not a good syntax)
>
> That is, expand "!!" to the string used to detach HEAD, and expand it
> again now. (Of course, something would have to be done if you did "git
> checkout HEAD^1" before, or "git checkout !!^1".) This is related in that
> I think the scary message should happen when "git commit" sees this stored
> string and clears it.
That sounds somewhat like the "git up" hack I've shown here:
http://article.gmane.org/gmane.comp.version-control.git/130050
In #git, Dscho even suggested that "git pull" could do that kind of
DWIMmery while on a detached HEAD that waas reached by checking out a remote
tracking branch. I'm undecided about that, because real merges/rebases
could make it easier to lose work, as opposed to the "fast-forward only"
behaviour I had in mind for that "git up" thing. Though of course, the
"git pull" DWIMmery for a detached HEAD could simply refuse to do
anything but a fast-forward.
Overall, I'm starting to think that improving the "work with a detached
HEAD" area might be more worthwhile than adding DWIMmery that tries to
completely avoid a detached HEAD.
This could include DWIMmery like the "git up"/"git pull" stuff, and
improved security checks, like checking that leaving a detached HEAD
doesn't "lose" any commits to the reflog. So checkout could do
something like "git rev-list HEAD --not --all" (or does --all include
HEAD?) and complain if there's something to be "lost".
Björn
^ permalink raw reply
* Re: [msysgit? bug] crlf double-conversion on win32
From: Eric Raible @ 2009-10-13 22:17 UTC (permalink / raw)
To: git
In-Reply-To: <38cfaa83fdf80dec3a3d81ed3e0de0e2.squirrel@intranet.linagora.com>
Yann Dirson <y.dirson <at> e-sidor.com> writes:
>
> With a msysgit 1.6.4 package, I got stuck after someone copied a CRLF file
> to a Linux box and committed it.
>
> In that situation, the win32 client in autocrlf mode keeps telling that
> the files are locally modified, even after eg "git reset --hard". Without
> touching the crlf setting (which I believe should not ever be necessary),
> this can be corrected by committing the faulty files after dos2unix'ing
> them, and using "git fetch && git reset --hard origin/master" ("git pull
> --rebase" refuses to do the job since it believes there are local
> changes).
See http://thread.gmane.org/gmane.comp.version-control.git/122823/focus=122862
In which Junio suggests:
$ rm .git/index
$ git reset --hard
in order to "restore sanity to your work tree"
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox