* Re: Fwd: git status options feature suggestion
From: Junio C Hamano @ 2008-10-26 1:47 UTC (permalink / raw)
To: Jeff King; +Cc: Michael J Gruber, Johannes Schindelin, Caleb Cushing, git
In-Reply-To: <7v1vymgrom.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> I remember a long time ago you started on a parallel diff walker that
>> could diff the working tree, the index, and a tree at once. Do you
>> remember the issues with it?
>
> Sorry, I don't.
>
>> I think that would be the right tool here to show each file only once,
>> but with multiple status flags. Something like:
>>
>> A M foo
>>
>> to show that "foo" has been added since the last commit, but there are
>> modifications in the working tree that have not yet been staged.
>
> One thing to keep in mind is what to do when you would want to detect
> renames. The parallel walk would be Ok but between HEAD and index there
> could be renames involved, and at that point it would get quite tricky.
> Once the index is in-core, I think it hurts us much to walk HEAD vs index
> and index vs working tree in separate passes.
>
> I think it is perfectly fine to run the diff-index first, and keep the
> result from it in a string_list, and then run "diff-files" and annotate
> the string_list with the output from it.
>
> Something like...
Because I was bored thinking about what to talk about in Gittogether and
lacked enough concentration to do anything productive, I did this that:
(1) introduces the "find and summarize changes in a single string list"
infrastructure;
(2) rewrites wt_status_print_{updated,changed} to use it; and
(3) adds "git shortstatus" that does not take any parameter (so it is not
about "preview of commit with the same paths arguments" anymore) to
give the status in:
XsssY PATH1 -> PATH2
format, where X is the diff status between HEAD and the index, sss is the
rename/copy score of the change (if X is rename or copy --- otherwise
it is blank), Y is the diff status between the index and the worktree.
PATH1 is the path in the HEAD, and " -> PATH2" part is shown only when
PATH1 corresponds to a different path in the index/worktree.
This was done primarily for fun and killing-time, so I won't be committing
it to my tree, but it seems to pass all the existing tests.
If you apply this patch with "git apply" (no --index) and then
$ git mv COPYING RENAMING
then you would see:
$ ./git-shortstatus
M Makefile
R100 COPYING -> RENAMING
M builtin-commit.c
M builtin-revert.c
M builtin.h
M git.c
M wt-status.c
M wt-status.h
It is very much welcomed if somebody wants to build on top of this. A few
obvious things, aside from bikeshedding to drop the score value (which I
just did as a sanity check measure and for nothing else --- I won't feel
hurt if we lost that field from the output) and such are:
* We can also rewrite wt_status_print_untracked() using the collected
data by making the collector pay attention to untracked files quite
easily;
* I did not bouther touching wt_status_print_initial() but I think it
should be straightforward to produce its output from the collected
data, as the collector already knows how to handle the initial commit.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Makefile | 1 +
builtin-commit.c | 45 ++++++++++-
builtin-revert.c | 1 +
builtin.h | 1 +
git.c | 1 +
wt-status.c | 240 +++++++++++++++++++++++++++++++++++++++++-------------
wt-status.h | 9 ++
7 files changed, 239 insertions(+), 59 deletions(-)
diff --git c/Makefile w/Makefile
index d6f3695..36afaa3 100644
--- c/Makefile
+++ w/Makefile
@@ -316,6 +316,7 @@ BUILT_INS += git-merge-subtree$X
BUILT_INS += git-peek-remote$X
BUILT_INS += git-repo-config$X
BUILT_INS += git-show$X
+BUILT_INS += git-shortstatus$X
BUILT_INS += git-status$X
BUILT_INS += git-whatchanged$X
diff --git c/builtin-commit.c w/builtin-commit.c
index 93ca496..99c6409 100644
--- c/builtin-commit.c
+++ w/builtin-commit.c
@@ -14,6 +14,7 @@
#include "diffcore.h"
#include "commit.h"
#include "revision.h"
+#include "string-list.h"
#include "wt-status.h"
#include "run-command.h"
#include "refs.h"
@@ -21,7 +22,6 @@
#include "strbuf.h"
#include "utf8.h"
#include "parse-options.h"
-#include "string-list.h"
#include "rerere.h"
#include "unpack-trees.h"
@@ -856,6 +856,49 @@ static int parse_and_validate_options(int argc, const char *argv[],
return argc;
}
+int cmd_shortstatus(int argc, const char **argv, const char *prefix)
+{
+ struct wt_status s;
+ int i;
+
+ read_cache();
+ refresh_cache(REFRESH_QUIET);
+ wt_status_prepare(&s);
+ wt_status_collect_changes(&s);
+ for (i = 0; i < s.change.nr; i++) {
+ struct wt_status_change_data *d;
+ struct string_list_item *it;
+ char pfx[1 + 3 + 1 + 1];
+
+ it = &(s.change.items[i]);
+ d = it->util;
+ switch (d->index_status) {
+ case DIFF_STATUS_COPIED:
+ case DIFF_STATUS_RENAMED:
+ sprintf(pfx, "%c%3d",
+ d->index_status,
+ (int)(d->index_score * 100 / MAX_SCORE));
+ break;
+ case 0:
+ memcpy(pfx, " ", 4);
+ break;
+ default:
+ sprintf(pfx, "%c ", d->index_status);
+ break;
+ }
+ if (!d->worktree_status)
+ pfx[4] = ' ';
+ else
+ pfx[4] = d->worktree_status;
+ pfx[5] = '\0';
+ printf("%s ", pfx);
+ if (d->head_path)
+ printf("%s -> ", d->head_path);
+ printf("%s\n", it->string);
+ }
+ return 0;
+}
+
int cmd_status(int argc, const char **argv, const char *prefix)
{
const char *index_file;
diff --git c/builtin-revert.c w/builtin-revert.c
index 4725540..060453f 100644
--- c/builtin-revert.c
+++ w/builtin-revert.c
@@ -3,6 +3,7 @@
#include "object.h"
#include "commit.h"
#include "tag.h"
+#include "string-list.h"
#include "wt-status.h"
#include "run-command.h"
#include "exec_cmd.h"
diff --git c/builtin.h w/builtin.h
index 1495cf6..f054fc7 100644
--- c/builtin.h
+++ w/builtin.h
@@ -94,6 +94,7 @@ extern int cmd_shortlog(int argc, const char **argv, const char *prefix);
extern int cmd_show(int argc, const char **argv, const char *prefix);
extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
extern int cmd_status(int argc, const char **argv, const char *prefix);
+extern int cmd_shortstatus(int argc, const char **argv, const char *prefix);
extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
extern int cmd_tag(int argc, const char **argv, const char *prefix);
diff --git c/git.c w/git.c
index 89feb0b..55e6cc9 100644
--- c/git.c
+++ w/git.c
@@ -342,6 +342,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "rm", cmd_rm, RUN_SETUP },
{ "send-pack", cmd_send_pack, RUN_SETUP },
{ "shortlog", cmd_shortlog, USE_PAGER },
+ { "shortstatus", cmd_shortstatus, RUN_SETUP | NEED_WORK_TREE },
{ "show-branch", cmd_show_branch, RUN_SETUP },
{ "show", cmd_show, RUN_SETUP | USE_PAGER },
{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
diff --git c/wt-status.c w/wt-status.c
index c3a9cab..3d2287b 100644
--- c/wt-status.c
+++ w/wt-status.c
@@ -1,4 +1,5 @@
#include "cache.h"
+#include "string-list.h"
#include "wt-status.h"
#include "color.h"
#include "object.h"
@@ -56,6 +57,7 @@ void wt_status_prepare(struct wt_status *s)
s->reference = "HEAD";
s->fp = stdout;
s->index_file = get_index_file();
+ s->change.strdup_strings = 1;
}
static void wt_status_print_cached_header(struct wt_status *s)
@@ -98,18 +100,22 @@ static void wt_status_print_trailer(struct wt_status *s)
#define quote_path quote_path_relative
-static void wt_status_print_filepair(struct wt_status *s,
- int t, struct diff_filepair *p)
+static void wt_status_print_change_data(struct wt_status *s,
+ int t,
+ int status,
+ char *one_name,
+ char *two_name,
+ int score)
{
const char *c = color(t);
const char *one, *two;
struct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;
- one = quote_path(p->one->path, -1, &onebuf, s->prefix);
- two = quote_path(p->two->path, -1, &twobuf, s->prefix);
+ one = quote_path(one_name, -1, &onebuf, s->prefix);
+ two = quote_path(two_name, -1, &twobuf, s->prefix);
color_fprintf(s->fp, color(WT_STATUS_HEADER), "#\t");
- switch (p->status) {
+ switch (status) {
case DIFF_STATUS_ADDED:
color_fprintf(s->fp, c, "new file: %s", one);
break;
@@ -135,56 +141,13 @@ static void wt_status_print_filepair(struct wt_status *s,
color_fprintf(s->fp, c, "unmerged: %s", one);
break;
default:
- die("bug: unhandled diff status %c", p->status);
+ die("bug: unhandled diff status %c", status);
}
fprintf(s->fp, "\n");
strbuf_release(&onebuf);
strbuf_release(&twobuf);
}
-static void wt_status_print_updated_cb(struct diff_queue_struct *q,
- struct diff_options *options,
- void *data)
-{
- struct wt_status *s = data;
- int shown_header = 0;
- int i;
- for (i = 0; i < q->nr; i++) {
- if (q->queue[i]->status == 'U')
- continue;
- if (!shown_header) {
- wt_status_print_cached_header(s);
- s->commitable = 1;
- shown_header = 1;
- }
- wt_status_print_filepair(s, WT_STATUS_UPDATED, q->queue[i]);
- }
- if (shown_header)
- wt_status_print_trailer(s);
-}
-
-static void wt_status_print_changed_cb(struct diff_queue_struct *q,
- struct diff_options *options,
- void *data)
-{
- struct wt_status *s = data;
- int i;
- if (q->nr) {
- int has_deleted = 0;
- s->workdir_dirty = 1;
- for (i = 0; i < q->nr; i++)
- if (q->queue[i]->status == DIFF_STATUS_DELETED) {
- has_deleted = 1;
- break;
- }
- wt_status_print_dirty_header(s, has_deleted);
- }
- for (i = 0; i < q->nr; i++)
- wt_status_print_filepair(s, WT_STATUS_CHANGED, q->queue[i]);
- if (q->nr)
- wt_status_print_trailer(s);
-}
-
static void wt_status_print_initial(struct wt_status *s)
{
int i;
@@ -205,13 +168,80 @@ static void wt_status_print_initial(struct wt_status *s)
strbuf_release(&buf);
}
-static void wt_status_print_updated(struct wt_status *s)
+static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
+ struct diff_options *options,
+ void *data)
+{
+ struct wt_status *s = data;
+ int i;
+
+ if (!q->nr)
+ return;
+ s->workdir_dirty = 1;
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p;
+ struct string_list_item *it;
+ struct wt_status_change_data *d;
+
+ p = q->queue[i];
+
+ d = xcalloc(1, sizeof(*d));
+ d->worktree_status = p->status;
+ it = string_list_insert(p->one->path, &s->change);
+ it->util = d;
+ }
+}
+
+static void wt_status_collect_updated_cb(struct diff_queue_struct *q,
+ struct diff_options *options,
+ void *data)
+{
+ struct wt_status *s = data;
+ int i;
+
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p;
+ struct string_list_item *it;
+ struct wt_status_change_data *d;
+
+ p = q->queue[i];
+ it = string_list_insert(p->two->path, &s->change);
+ d = it->util;
+ if (!d) {
+ d = xcalloc(1, sizeof(*d));
+ it->util = d;
+ }
+ d->index_status = p->status;
+ switch (p->status) {
+ case DIFF_STATUS_COPIED:
+ case DIFF_STATUS_RENAMED:
+ d->head_path = xstrdup(p->one->path);
+ d->index_score = p->score;
+ break;
+ }
+ }
+}
+
+static void wt_status_collect_changes_worktree(struct wt_status *s)
+{
+ struct rev_info rev;
+
+ init_revisions(&rev, NULL);
+ setup_revisions(0, NULL, &rev, NULL);
+ rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
+ rev.diffopt.format_callback = wt_status_collect_changed_cb;
+ rev.diffopt.format_callback_data = s;
+ run_diff_files(&rev, 0);
+}
+
+static void wt_status_collect_changes_index(struct wt_status *s)
{
struct rev_info rev;
+
init_revisions(&rev, NULL);
setup_revisions(0, NULL, &rev, s->reference);
rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
- rev.diffopt.format_callback = wt_status_print_updated_cb;
+ rev.diffopt.format_callback = wt_status_collect_updated_cb;
rev.diffopt.format_callback_data = s;
rev.diffopt.detect_rename = 1;
rev.diffopt.rename_limit = 200;
@@ -219,15 +249,107 @@ static void wt_status_print_updated(struct wt_status *s)
run_diff_index(&rev, 1);
}
+static void wt_status_collect_changes_initial(struct wt_status *s)
+{
+ int i;
+
+ for (i = 0; i < active_nr; i++) {
+ struct string_list_item *it;
+ struct wt_status_change_data *d;
+
+ it = string_list_insert(active_cache[i]->name, &s->change);
+ d = it->util;
+ if (!d) {
+ d = xcalloc(1, sizeof(*d));
+ it->util = d;
+ }
+ d->index_status = DIFF_STATUS_ADDED;
+ }
+}
+
+void wt_status_collect_changes(struct wt_status *s)
+{
+ wt_status_collect_changes_worktree(s);
+
+ if (s->is_initial)
+ wt_status_collect_changes_initial(s);
+ else
+ wt_status_collect_changes_index(s);
+}
+
+static void wt_status_print_updated(struct wt_status *s)
+{
+ int shown_header = 0;
+ int i;
+
+ for (i = 0; i < s->change.nr; i++) {
+ struct wt_status_change_data *d;
+ struct string_list_item *it;
+ it = &(s->change.items[i]);
+ d = it->util;
+ if (!d->index_status)
+ continue;
+ if (!shown_header) {
+ wt_status_print_cached_header(s);
+ s->commitable = 1;
+ shown_header = 1;
+ }
+ wt_status_print_change_data(s, WT_STATUS_UPDATED,
+ d->index_status,
+ d->head_path ? d->head_path : it->string,
+ it->string,
+ d->index_score);
+ }
+ if (shown_header)
+ wt_status_print_trailer(s);
+}
+
+/*
+ * -1 : has delete
+ * 0 : no change
+ * 1 : some change but no delete
+ */
+static int wt_status_check_worktree_changes(struct wt_status *s)
+{
+ int i;
+ int changes = 0;
+
+ for (i = 0; i < s->change.nr; i++) {
+ struct wt_status_change_data *d;
+ d = s->change.items[i].util;
+ if (!d->worktree_status)
+ continue;
+ changes = 1;
+ if (d->worktree_status == DIFF_STATUS_DELETED)
+ return -1;
+ }
+ return changes;
+}
+
static void wt_status_print_changed(struct wt_status *s)
{
- struct rev_info rev;
- init_revisions(&rev, "");
- setup_revisions(0, NULL, &rev, NULL);
- rev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;
- rev.diffopt.format_callback = wt_status_print_changed_cb;
- rev.diffopt.format_callback_data = s;
- run_diff_files(&rev, 0);
+ int i;
+ int worktree_changes = wt_status_check_worktree_changes(s);
+
+ if (!worktree_changes)
+ return;
+
+ wt_status_print_dirty_header(s, worktree_changes < 0);
+
+ for (i = 0; i < s->change.nr; i++) {
+ struct wt_status_change_data *d;
+ struct string_list_item *it;
+ it = &(s->change.items[i]);
+ d = it->util;
+ if (!d->worktree_status)
+ continue;
+ wt_status_print_change_data(s, WT_STATUS_CHANGED,
+ d->worktree_status,
+ it->string,
+ it->string,
+ 0);
+ }
+ wt_status_print_trailer(s);
}
static void wt_status_print_submodule_summary(struct wt_status *s)
@@ -347,6 +469,8 @@ void wt_status_print(struct wt_status *s)
wt_status_print_tracking(s);
}
+ wt_status_collect_changes(s);
+
if (s->is_initial) {
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "#");
color_fprintf_ln(s->fp, color(WT_STATUS_HEADER), "# Initial commit");
diff --git c/wt-status.h w/wt-status.h
index 78add09..00508c3 100644
--- c/wt-status.h
+++ w/wt-status.h
@@ -18,6 +18,13 @@ enum untracked_status_type {
};
extern enum untracked_status_type show_untracked_files;
+struct wt_status_change_data {
+ int worktree_status;
+ int index_status;
+ int index_score;
+ char *head_path;
+};
+
struct wt_status {
int is_initial;
char *branch;
@@ -33,6 +40,7 @@ struct wt_status {
const char *index_file;
FILE *fp;
const char *prefix;
+ struct string_list change;
};
int git_status_config(const char *var, const char *value, void *cb);
@@ -40,5 +48,6 @@ extern int wt_status_use_color;
extern int wt_status_relative_paths;
void wt_status_prepare(struct wt_status *s);
void wt_status_print(struct wt_status *s);
+void wt_status_collect_changes(struct wt_status *s);
#endif /* STATUS_H */
^ permalink raw reply related
* [PATCH 3/3] Fix git update-ref --no-deref -d.
From: Miklos Vajna @ 2008-10-26 2:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Brandon Casey, git
In-Reply-To: <cover.1224987944.git.vmiklos@frugalware.org>
Till now --no-deref was just ignored when deleting refs, fix this.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
builtin-update-ref.c | 8 +++++---
t/t1400-update-ref.sh | 7 +++++++
2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index d8f3142..378dc1b 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -13,7 +13,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
{
const char *refname, *oldval, *msg=NULL;
unsigned char sha1[20], oldsha1[20];
- int delete = 0, no_deref = 0;
+ int delete = 0, no_deref = 0, flags = 0;
struct option options[] = {
OPT_STRING( 'm', NULL, &msg, "reason", "reason of the update"),
OPT_BOOLEAN('d', NULL, &delete, "deletes the reference"),
@@ -47,9 +47,11 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
if (oldval && *oldval && get_sha1(oldval, oldsha1))
die("%s: not a valid old SHA1", oldval);
+ if (no_deref)
+ flags = REF_NODEREF;
if (delete)
- return delete_ref(refname, oldval ? oldsha1 : NULL, 0);
+ return delete_ref(refname, oldval ? oldsha1 : NULL, flags);
else
return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
- no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
+ flags, DIE_ON_ERR);
}
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index 04c2b16..8139cd6 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -75,6 +75,13 @@ test_expect_success "delete $m (by HEAD)" '
'
rm -f .git/$m
+cp -f .git/HEAD .git/HEAD.orig
+test_expect_success "delete symref without dereference" '
+ git update-ref --no-deref -d HEAD &&
+ ! test -f .git/HEAD
+'
+cp -f .git/HEAD.orig .git/HEAD
+
test_expect_success '(not) create HEAD with old sha1' "
test_must_fail git update-ref HEAD $A $B
"
--
1.6.0.2
^ permalink raw reply related
* [PATCH 2/3] rename_ref(): handle the case when the reflog of a ref does not exist
From: Miklos Vajna @ 2008-10-26 2:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Brandon Casey, git
In-Reply-To: <cover.1224987944.git.vmiklos@frugalware.org>
We tried to check if a reflog of a ref is a symlink without first
checking if it exists, which is a bug.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
refs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/refs.c b/refs.c
index 70c0967..3c39a31 100644
--- a/refs.c
+++ b/refs.c
@@ -975,7 +975,7 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
const char *symref = NULL;
int is_symref = 0;
- if (S_ISLNK(loginfo.st_mode))
+ if (log && S_ISLNK(loginfo.st_mode))
return error("reflog for %s is a symlink", oldref);
symref = resolve_ref(oldref, orig_sha1, 1, &flag);
--
1.6.0.2
^ permalink raw reply related
* [PATCH 0/3] symref rename/delete fixes
From: Miklos Vajna @ 2008-10-26 2:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Brandon Casey, git
In-Reply-To: <7v8wsca5ne.fsf@gitster.siamese.dyndns.org>
On Sat, Oct 25, 2008 at 11:31:01AM -0700, Junio C Hamano <gitster@pobox.com> wrote:
> > +int delete_ref(const char *refname, const unsigned char *sha1, int
> > flags)
> > {
> > struct ref_lock *lock;
> > - int err, i, ret = 0, flag = 0;
> > + int err, i = 0, ret = 0, flag = 0;
> > + char *path;
> Two variables flag vs flags is a bit confusing, isn't it? How about
> naming the new one "delopt" or something?
Renamed.
> The new variable "char *path" at the toplevel can be confined in the
> scope
> of this if () {} block and probably can become "const char *", right?
Yes - moved.
> > int log = !lstat(git_path("logs/%s", oldref), &loginfo);
> > + const char *symref = NULL;
> > + int is_symref = 0;
> >
> > if (S_ISLNK(loginfo.st_mode))
> > return error("reflog for %s is a symlink", oldref);
>
> Possible bug in the context. When there is no reflog for the ref
> being
> renamed, lstat would fail; it doesn't feel right to have this
> S_ISLNK()
> before checking the result of the lstat which is in "log".
OK, this went to a separate patch.
> > + symref = resolve_ref(oldref, orig_sha1, 0, &flag);
> > + if (flag & REF_ISSYMREF)
> > + is_symref = 1;
> > if (!resolve_ref(oldref, orig_sha1, 1, &flag))
> > return error("refname %s not found", oldref);
>
> Do we really need two calls to resolve_ref()? Your new call calls it
> without must-exist bit --- why? Immediately after that, the existing
> call
> will barf if it does not exist anyway.
Just having
if (!symref)
return error("refname %s not found", oldref);
first looks weird, given that the error message is not "refname %s is
not a symref", but you are right, changed.
> I agree it is good to have symref aware delete_ref(), but I am not
> sure
> supporting symref in rename_ref() is either needed or necessarily a
> good
> idea. You also need to worry about a symref pointing at a branch yet
> to
> be born.
That is currently not supported and the error message of 'git branch -m'
is (in case foo points to refs/heads/bar and bar is not yet born):
error: refname refs/heads/foo not found
fatal: Branch rename failed
which is quite acceptable, IMHO[1].
> In the meantime, I think we should just check (flag & REF_ISSYMREF)
> after
> the existing resolve_ref() we can see in the context above, and error
> out
> saying you cannot rename a symref, and do nothing else.
A symref-aware rename_ref() is needed by git remove rename, since it
typically does origin/HEAD -> upstream/HEAD symref renames there.
Of course you can say that this should be handled by git-remote itself,
without using rename_ref() but that not seem to be a good solution to
me. (Workaround in the wrong layer, instead of a solution in a good
one.)
[1] I mean, I have a real-world scenario (git remove rename) for "why
renaming a symref is a good idea", but I don't think renaming a ref
pointing to a yet-to-be-born ref has any real world users.
Miklos Vajna (3):
Fix git branch -m for symrefs.
rename_ref(): handle the case when the reflog of a ref does not exist
Fix git update-ref --no-deref -d.
builtin-branch.c | 2 +-
builtin-receive-pack.c | 2 +-
builtin-remote.c | 4 +-
builtin-reset.c | 2 +-
builtin-send-pack.c | 2 +-
builtin-tag.c | 2 +-
builtin-update-ref.c | 8 ++++--
cache.h | 2 +-
refs.c | 61 +++++++++++++++++++++++++++++------------------
t/t1400-update-ref.sh | 7 +++++
t/t3200-branch.sh | 9 +++++++
11 files changed, 67 insertions(+), 34 deletions(-)
Also available in the 'symref-mv' branch of 'git://repo.or.cz/git/vmiklos.git'.
^ permalink raw reply
* [PATCH 1/3] Fix git branch -m for symrefs.
From: Miklos Vajna @ 2008-10-26 2:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Brandon Casey, git
In-Reply-To: <cover.1224987944.git.vmiklos@frugalware.org>
This had two problems with symrefs. First, it copied the actual sha1
instead of the "pointer", second it failed to remove the old ref after a
successful rename.
Given that till now delete_ref() always dereferenced symrefs, a new
parameters has been introduced to delete_ref() to allow deleting refs
without a dereference.
Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
builtin-branch.c | 2 +-
builtin-receive-pack.c | 2 +-
builtin-remote.c | 4 +-
builtin-reset.c | 2 +-
builtin-send-pack.c | 2 +-
builtin-tag.c | 2 +-
builtin-update-ref.c | 2 +-
cache.h | 2 +-
refs.c | 59 ++++++++++++++++++++++++++++++------------------
t/t3200-branch.sh | 9 +++++++
10 files changed, 55 insertions(+), 31 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c
index 8d634ff..2b3613f 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -160,7 +160,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
continue;
}
- if (delete_ref(name, sha1)) {
+ if (delete_ref(name, sha1, 0)) {
error("Error deleting %sbranch '%s'", remote,
argv[i]);
ret = 1;
diff --git a/builtin-receive-pack.c b/builtin-receive-pack.c
index 45e3cd9..ab5fa1c 100644
--- a/builtin-receive-pack.c
+++ b/builtin-receive-pack.c
@@ -224,7 +224,7 @@ static const char *update(struct command *cmd)
warning ("Allowing deletion of corrupt ref.");
old_sha1 = NULL;
}
- if (delete_ref(name, old_sha1)) {
+ if (delete_ref(name, old_sha1, 0)) {
error("failed to delete %s", name);
return "failed to delete";
}
diff --git a/builtin-remote.c b/builtin-remote.c
index df2be06..e396a3a 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -337,7 +337,7 @@ static int remove_branches(struct string_list *branches)
const char *refname = item->string;
unsigned char *sha1 = item->util;
- if (delete_ref(refname, sha1))
+ if (delete_ref(refname, sha1, 0))
result |= error("Could not remove branch %s", refname);
}
return result;
@@ -565,7 +565,7 @@ static int prune(int argc, const char **argv)
const char *refname = states.stale.items[i].util;
if (!dry_run)
- result |= delete_ref(refname, NULL);
+ result |= delete_ref(refname, NULL, 0);
printf(" * [%s] %s\n", dry_run ? "would prune" : "pruned",
abbrev_ref(refname, "refs/remotes/"));
diff --git a/builtin-reset.c b/builtin-reset.c
index 16e6bb2..9514b77 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -279,7 +279,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
update_ref(msg, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR);
}
else if (old_orig)
- delete_ref("ORIG_HEAD", old_orig);
+ delete_ref("ORIG_HEAD", old_orig, 0);
prepend_reflog_action("updating HEAD", msg, sizeof(msg));
update_ref_status = update_ref(msg, "HEAD", sha1, orig, 0, MSG_ON_ERR);
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 910db92..bbf6e0a 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -234,7 +234,7 @@ static void update_tracking_ref(struct remote *remote, struct ref *ref)
if (args.verbose)
fprintf(stderr, "updating local tracking ref '%s'\n", rs.dst);
if (ref->deletion) {
- delete_ref(rs.dst, NULL);
+ delete_ref(rs.dst, NULL, 0);
} else
update_ref("update by push", rs.dst,
ref->new_sha1, NULL, 0, 0);
diff --git a/builtin-tag.c b/builtin-tag.c
index b13fa34..1ff7b37 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -125,7 +125,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
static int delete_tag(const char *name, const char *ref,
const unsigned char *sha1)
{
- if (delete_ref(ref, sha1))
+ if (delete_ref(ref, sha1, 0))
return 1;
printf("Deleted tag '%s'\n", name);
return 0;
diff --git a/builtin-update-ref.c b/builtin-update-ref.c
index 56a0b1b..d8f3142 100644
--- a/builtin-update-ref.c
+++ b/builtin-update-ref.c
@@ -48,7 +48,7 @@ int cmd_update_ref(int argc, const char **argv, const char *prefix)
die("%s: not a valid old SHA1", oldval);
if (delete)
- return delete_ref(refname, oldval ? oldsha1 : NULL);
+ return delete_ref(refname, oldval ? oldsha1 : NULL, 0);
else
return update_ref(msg, refname, sha1, oldval ? oldsha1 : NULL,
no_deref ? REF_NODEREF : 0, DIE_ON_ERR);
diff --git a/cache.h b/cache.h
index b0edbf9..a3c77f0 100644
--- a/cache.h
+++ b/cache.h
@@ -434,7 +434,7 @@ extern int commit_locked_index(struct lock_file *);
extern void set_alternate_index_output(const char *);
extern int close_lock_file(struct lock_file *);
extern void rollback_lock_file(struct lock_file *);
-extern int delete_ref(const char *, const unsigned char *sha1);
+extern int delete_ref(const char *, const unsigned char *sha1, int delopt);
/* Environment bits from configuration mechanism */
extern int trust_executable_bit;
diff --git a/refs.c b/refs.c
index 0a126fa..70c0967 100644
--- a/refs.c
+++ b/refs.c
@@ -921,25 +921,33 @@ static int repack_without_ref(const char *refname)
return commit_lock_file(&packlock);
}
-int delete_ref(const char *refname, const unsigned char *sha1)
+int delete_ref(const char *refname, const unsigned char *sha1, int delopt)
{
struct ref_lock *lock;
- int err, i, ret = 0, flag = 0;
+ int err, i = 0, ret = 0, flag = 0;
lock = lock_ref_sha1_basic(refname, sha1, 0, &flag);
if (!lock)
return 1;
if (!(flag & REF_ISPACKED)) {
/* loose */
- i = strlen(lock->lk->filename) - 5; /* .lock */
- lock->lk->filename[i] = 0;
- err = unlink(lock->lk->filename);
+ const char *path;
+
+ if (!(delopt & REF_NODEREF)) {
+ i = strlen(lock->lk->filename) - 5; /* .lock */
+ lock->lk->filename[i] = 0;
+ path = lock->lk->filename;
+ } else {
+ path = git_path(refname);
+ }
+ err = unlink(path);
if (err && errno != ENOENT) {
ret = 1;
error("unlink(%s) failed: %s",
- lock->lk->filename, strerror(errno));
+ path, strerror(errno));
}
- lock->lk->filename[i] = '.';
+ if (!(delopt & REF_NODEREF))
+ lock->lk->filename[i] = '.';
}
/* removing the loose one could have resurrected an earlier
* packed one. Also, if it was not loose we need to repack
@@ -964,11 +972,16 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
struct ref_lock *lock;
struct stat loginfo;
int log = !lstat(git_path("logs/%s", oldref), &loginfo);
+ const char *symref = NULL;
+ int is_symref = 0;
if (S_ISLNK(loginfo.st_mode))
return error("reflog for %s is a symlink", oldref);
- if (!resolve_ref(oldref, orig_sha1, 1, &flag))
+ symref = resolve_ref(oldref, orig_sha1, 1, &flag);
+ if (flag & REF_ISSYMREF)
+ is_symref = 1;
+ if (!symref)
return error("refname %s not found", oldref);
if (!is_refname_available(newref, oldref, get_packed_refs(), 0))
@@ -988,12 +1001,12 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
return error("unable to move logfile logs/%s to tmp-renamed-log: %s",
oldref, strerror(errno));
- if (delete_ref(oldref, orig_sha1)) {
+ if (delete_ref(oldref, orig_sha1, REF_NODEREF)) {
error("unable to delete old %s", oldref);
goto rollback;
}
- if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1)) {
+ if (resolve_ref(newref, sha1, 1, &flag) && delete_ref(newref, sha1, REF_NODEREF)) {
if (errno==EISDIR) {
if (remove_empty_directories(git_path("%s", newref))) {
error("Directory not empty: %s", newref);
@@ -1031,18 +1044,20 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
}
logmoved = log;
- lock = lock_ref_sha1_basic(newref, NULL, 0, NULL);
- if (!lock) {
- error("unable to lock %s for update", newref);
- goto rollback;
- }
-
- lock->force_write = 1;
- hashcpy(lock->old_sha1, orig_sha1);
- if (write_ref_sha1(lock, orig_sha1, logmsg)) {
- error("unable to write current sha1 into %s", newref);
- goto rollback;
- }
+ if (!is_symref) {
+ lock = lock_ref_sha1_basic(newref, NULL, 0, NULL);
+ if (!lock) {
+ error("unable to lock %s for update", newref);
+ goto rollback;
+ }
+ lock->force_write = 1;
+ hashcpy(lock->old_sha1, orig_sha1);
+ if (write_ref_sha1(lock, orig_sha1, logmsg)) {
+ error("unable to write current sha1 into %s", newref);
+ goto rollback;
+ }
+ } else
+ create_symref(newref, symref, logmsg);
return 0;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 2147eac..fdeb1f5 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -112,6 +112,15 @@ test_expect_success 'config information was renamed, too' \
"test $(git config branch.s.dummy) = Hello &&
test_must_fail git config branch.s/s/dummy"
+test_expect_success 'renaming a symref' \
+'
+ git symbolic-ref refs/heads/master2 refs/heads/master &&
+ git branch -m master2 master3 &&
+ git symbolic-ref refs/heads/master3 &&
+ test -f .git/refs/heads/master &&
+ ! test -f .git/refs/heads/master2
+'
+
test_expect_success \
'git branch -m u v should fail when the reflog for u is a symlink' '
git branch -l u &&
--
1.6.0.2
^ permalink raw reply related
* [VOTE] git versus mercurial
From: walt @ 2008-10-26 4:28 UTC (permalink / raw)
To: git
No, no, I'm not the one calling for a vote. You old-timers here
will know the name Matt Dillon, who is leading the dragonflybsd
project (www.dragonflybsd.org).
Matt is the one who is calling for the vote in his thread "Vote
for your source control system" in the dragonfly.kernel group,
accessible via nntp://nntp.dragonflybsd.org.
I've already cast my vote for git, which I confess is not very
honest because I've never even tried mercurial. I would truly
be grateful to anyone here who knows both git and mercurial who
could contribute verifiable facts to that debate.
Thanks.
^ permalink raw reply
* Re: [PATCH 4/7] textconv: don't convert for every operation
From: Jeff King @ 2008-10-26 4:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081025193554.GA27457@coredump.intra.peff.net>
On Sat, Oct 25, 2008 at 03:35:54PM -0400, Jeff King wrote:
> I will re-roll my latest series according to your recommendation
> (unless you are set on reverting the first part; if so, please tell me
> asap so I can work under that assumption).
And here it is. 8 patches this time, and I tried to reorder with
"obviously correct" at the beginning and "I'm not so sure" at the end.
1/8: diff: add missing static declaration
2/8: document the diff driver textconv feature
3/8: add userdiff textconv tests
4/8: refactor userdiff textconv code
5/8: userdiff: require explicitly allowing textconv
6/8: only textconv regular files
7/8: wt-status: load diff ui config
8/8: enable textconv for diff in verbose status/commit
-Peff
^ permalink raw reply
* [PATCH v3 1/8] diff: add missing static declaration
From: Jeff King @ 2008-10-26 4:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
This function isn't used outside of diff.c; the 'static' was
simply overlooked in the original writing.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as before.
diff.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index e368fef..d1fd594 100644
--- a/diff.c
+++ b/diff.c
@@ -1282,7 +1282,7 @@ static void emit_binary_diff(FILE *file, mmfile_t *one, mmfile_t *two)
emit_binary_diff_body(file, two, one);
}
-void diff_filespec_load_driver(struct diff_filespec *one)
+static void diff_filespec_load_driver(struct diff_filespec *one)
{
if (!one->driver)
one->driver = userdiff_find_by_path(one->path);
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* [PATCH v3 2/8] document the diff driver textconv feature
From: Jeff King @ 2008-10-26 4:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
This patch also changes the term "custom diff driver" to
"external diff driver"; now that there are more facets of a
"custom driver" than just external diffing, it makes sense
to refer to the configuration of "diff.foo.*" as the "foo
diff driver", with "diff.foo.command" as the "external
driver for foo".
Signed-off-by: Jeff King <peff@peff.net>
---
Same as before, but re-ordered to beginning since it is
non-controversial.
Documentation/gitattributes.txt | 66 +++++++++++++++++++++++++++++++--------
1 files changed, 53 insertions(+), 13 deletions(-)
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 2694559..314e2d3 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -213,10 +213,12 @@ with `crlf`, and then `ident` and fed to `filter`.
Generating diff text
~~~~~~~~~~~~~~~~~~~~
-The attribute `diff` affects if 'git-diff' generates textual
-patch for the path or just says `Binary files differ`. It also
-can affect what line is shown on the hunk header `@@ -k,l +n,m @@`
-line.
+The attribute `diff` affects how 'git' generates diffs for particular
+files. It can tell git whether to generate a textual patch for the path
+or to treat the path as a binary file. It can also affect what line is
+shown on the hunk header `@@ -k,l +n,m @@` line, tell git to use an
+external command to generate the diff, or ask git to convert binary
+files to a text format before generating the diff.
Set::
@@ -227,7 +229,8 @@ Set::
Unset::
A path to which the `diff` attribute is unset will
- generate `Binary files differ`.
+ generate `Binary files differ` (or a binary patch, if
+ binary patches are enabled).
Unspecified::
@@ -238,21 +241,21 @@ Unspecified::
String::
- Diff is shown using the specified custom diff driver.
- The driver program is given its input using the same
- calling convention as used for GIT_EXTERNAL_DIFF
- program. This name is also used for custom hunk header
- selection.
+ Diff is shown using the specified diff driver. Each driver may
+ specify one or more options, as described in the following
+ section. The options for the diff driver "foo" are defined
+ by the configuration variables in the "diff.foo" section of the
+ git config file.
-Defining a custom diff driver
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Defining an external diff driver
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
The definition of a diff driver is done in `gitconfig`, not
`gitattributes` file, so strictly speaking this manual page is a
wrong place to talk about it. However...
-To define a custom diff driver `jcdiff`, add a section to your
+To define an external diff driver `jcdiff`, add a section to your
`$GIT_DIR/config` file (or `$HOME/.gitconfig` file) like this:
----------------------------------------------------------------
@@ -328,6 +331,43 @@ patterns are available:
- `tex` suitable for source code for LaTeX documents.
+Performing text diffs of binary files
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Sometimes it is desirable to see the diff of a text-converted
+version of some binary files. For example, a word processor
+document can be converted to an ASCII text representation, and
+the diff of the text shown. Even though this conversion loses
+some information, the resulting diff is useful for human
+viewing (but cannot be applied directly).
+
+The `textconv` config option is used to define a program for
+performing such a conversion. The program should take a single
+argument, the name of a file to convert, and produce the
+resulting text on stdout.
+
+For example, to show the diff of the exif information of a
+file instead of the binary information (assuming you have the
+exif tool installed):
+
+------------------------
+[diff "jpg"]
+ textconv = exif
+------------------------
+
+NOTE: The text conversion is generally a one-way conversion;
+in this example, we lose the actual image contents and focus
+just on the text data. This means that diffs generated by
+textconv are _not_ suitable for applying. For this reason,
+only `git diff` and the `git log` family of commands (i.e.,
+log, whatchanged, show) will perform text conversion. `git
+format-patch` will never generate this output. If you want to
+send somebody a text-converted diff of a binary file (e.g.,
+because it quickly conveys the changes you have made), you
+should generate it separately and send it as a comment _in
+addition to_ the usual binary diff that you might send.
+
+
Performing a three-way merge
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* [PATCH v3 3/8] add userdiff textconv tests
From: Jeff King @ 2008-10-26 4:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
These tests provide a basic sanity check that textconv'd
files work. The tests try to describe how this configuration
_should_ work; thus some of the tests are marked to expect
failure.
In particular, we fail to actually textconv anything because
the 'diff.foo.binary' config option is not set, which will
be fixed in the next patch.
This also means that some "expect_failure" tests actually
seem to be fixed; in reality, this is just because textconv
is broken and its failure mode happens to make these tests
work.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as before.
t/t4030-diff-textconv.sh | 118 ++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 118 insertions(+), 0 deletions(-)
create mode 100755 t/t4030-diff-textconv.sh
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
new file mode 100755
index 0000000..1b09648
--- /dev/null
+++ b/t/t4030-diff-textconv.sh
@@ -0,0 +1,118 @@
+#!/bin/sh
+
+test_description='diff.*.textconv tests'
+. ./test-lib.sh
+
+find_diff() {
+ sed '1,/^index /d' | sed '/^-- $/,$d'
+}
+
+cat >expect.binary <<'EOF'
+Binary files a/file and b/file differ
+EOF
+
+cat >expect.text <<'EOF'
+--- a/file
++++ b/file
+@@ -1 +1,2 @@
+ 0
++1
+EOF
+
+cat >hexdump <<'EOF'
+#!/bin/sh
+perl -e '$/ = undef; $_ = <>; s/./ord($&)/ge; print $_' "$1"
+EOF
+chmod +x hexdump
+
+test_expect_success 'setup binary file with history' '
+ printf "\\0\\n" >file &&
+ git add file &&
+ git commit -m one &&
+ printf "\\1\\n" >>file &&
+ git add file &&
+ git commit -m two
+'
+
+test_expect_success 'file is considered binary by porcelain' '
+ git diff HEAD^ HEAD >diff &&
+ find_diff <diff >actual &&
+ test_cmp expect.binary actual
+'
+
+test_expect_success 'file is considered binary by plumbing' '
+ git diff-tree -p HEAD^ HEAD >diff &&
+ find_diff <diff >actual &&
+ test_cmp expect.binary actual
+'
+
+test_expect_success 'setup textconv filters' '
+ echo file diff=foo >.gitattributes &&
+ git config diff.foo.textconv "$PWD"/hexdump &&
+ git config diff.fail.textconv false
+'
+
+test_expect_failure 'diff produces text' '
+ git diff HEAD^ HEAD >diff &&
+ find_diff <diff >actual &&
+ test_cmp expect.text actual
+'
+
+test_expect_success 'diff-tree produces binary' '
+ git diff-tree -p HEAD^ HEAD >diff &&
+ find_diff <diff >actual &&
+ test_cmp expect.binary actual
+'
+
+test_expect_failure 'log produces text' '
+ git log -1 -p >log &&
+ find_diff <log >actual &&
+ test_cmp expect.text actual
+'
+
+test_expect_failure 'format-patch produces binary' '
+ git format-patch --no-binary --stdout HEAD^ >patch &&
+ find_diff <patch >actual &&
+ test_cmp expect.binary actual
+'
+
+cat >expect.stat <<'EOF'
+ file | Bin 2 -> 4 bytes
+ 1 files changed, 0 insertions(+), 0 deletions(-)
+EOF
+test_expect_failure 'diffstat does not run textconv' '
+ echo file diff=fail >.gitattributes &&
+ git diff --stat HEAD^ HEAD >actual &&
+ test_cmp expect.stat actual
+'
+# restore working setup
+echo file diff=foo >.gitattributes
+
+cat >expect.typechange <<'EOF'
+--- a/file
++++ /dev/null
+@@ -1,2 +0,0 @@
+-0
+-1
+diff --git a/file b/file
+new file mode 120000
+index ad8b3d2..67be421
+--- /dev/null
++++ b/file
+@@ -0,0 +1 @@
++frotz
+\ No newline at end of file
+EOF
+# make a symlink the hard way that works on symlink-challenged file systems
+test_expect_failure 'textconv does not act on symlinks' '
+ echo -n frotz > file &&
+ git add file &&
+ git ls-files -s | sed -e s/100644/120000/ |
+ git update-index --index-info &&
+ git commit -m typechange &&
+ git show >diff &&
+ find_diff <diff >actual &&
+ test_cmp expect.typechange actual
+'
+
+test_done
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* [PATCH v3 4/8] refactor userdiff textconv code
From: Jeff King @ 2008-10-26 4:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
The original implementation of textconv put the conversion
into fill_mmfile. This was a bad idea for a number of
reasons:
- it made the semantics of fill_mmfile unclear. In some
cases, it was allocating data (if a text conversion
occurred), and in some cases not (if we could use the
data directly from the filespec). But the caller had
no idea which had happened, and so didn't know whether
the memory should be freed
- similarly, the caller had no idea if a text conversion
had occurred, and so didn't know whether the contents
should be treated as binary or not. This meant that we
incorrectly guessed that text-converted content was
binary and didn't actually show it (unless the user
overrode us with "diff.foo.binary = false", which then
created problems in plumbing where the text conversion
did _not_ occur)
- not all callers of fill_mmfile want the text contents. In
particular, we don't really want diffstat, whitespace
checks, patch id generation, etc, to look at the
converted contents.
This patch pulls the conversion code directly into
builtin_diff, so that we only see the conversion when
generating an actual patch. We also then know whether we are
doing a conversion, so we can check the binary-ness and free
the data from the mmfile appropriately (the previous version
leaked quite badly when text conversion was used)
Signed-off-by: Jeff King <peff@peff.net>
---
This patch is the interesting one. I think your suggestion turned out
fairly clean (and this reverts fill_mmfile back to its state before the
original textconv series).
diff.c | 48 +++++++++++++++++++++++++++++++++------------
t/t4030-diff-textconv.sh | 6 ++--
2 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/diff.c b/diff.c
index d1fd594..6f01595 100644
--- a/diff.c
+++ b/diff.c
@@ -294,18 +294,8 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
else if (diff_populate_filespec(one, 0))
return -1;
- diff_filespec_load_driver(one);
- if (one->driver->textconv) {
- size_t size;
- mf->ptr = run_textconv(one->driver->textconv, one, &size);
- if (!mf->ptr)
- return -1;
- mf->size = size;
- }
- else {
- mf->ptr = one->data;
- mf->size = one->size;
- }
+ mf->ptr = one->data;
+ mf->size = one->size;
return 0;
}
@@ -1323,6 +1313,14 @@ void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const
options->b_prefix = b;
}
+static const char *get_textconv(struct diff_filespec *one)
+{
+ if (!DIFF_FILE_VALID(one))
+ return NULL;
+ diff_filespec_load_driver(one);
+ return one->driver->textconv;
+}
+
static void builtin_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
@@ -1337,6 +1335,7 @@ static void builtin_diff(const char *name_a,
const char *set = diff_get_color_opt(o, DIFF_METAINFO);
const char *reset = diff_get_color_opt(o, DIFF_RESET);
const char *a_prefix, *b_prefix;
+ const char *textconv_one, *textconv_two;
diff_set_mnemonic_prefix(o, "a/", "b/");
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
@@ -1390,8 +1389,12 @@ static void builtin_diff(const char *name_a,
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
+ textconv_one = get_textconv(one);
+ textconv_two = get_textconv(two);
+
if (!DIFF_OPT_TST(o, TEXT) &&
- (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {
+ ( (diff_filespec_is_binary(one) && !textconv_one) ||
+ (diff_filespec_is_binary(two) && !textconv_two) )) {
/* Quite common confusing case */
if (mf1.size == mf2.size &&
!memcmp(mf1.ptr, mf2.ptr, mf1.size))
@@ -1412,6 +1415,21 @@ static void builtin_diff(const char *name_a,
struct emit_callback ecbdata;
const struct userdiff_funcname *pe;
+ if (textconv_one) {
+ size_t size;
+ mf1.ptr = run_textconv(textconv_one, one, &size);
+ if (!mf1.ptr)
+ die("unable to read files to diff");
+ mf1.size = size;
+ }
+ if (textconv_two) {
+ size_t size;
+ mf2.ptr = run_textconv(textconv_two, two, &size);
+ if (!mf2.ptr)
+ die("unable to read files to diff");
+ mf2.size = size;
+ }
+
pe = diff_funcname_pattern(one);
if (!pe)
pe = diff_funcname_pattern(two);
@@ -1443,6 +1461,10 @@ static void builtin_diff(const char *name_a,
&xpp, &xecfg, &ecb);
if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
free_diff_words_data(&ecbdata);
+ if (textconv_one)
+ free(mf1.ptr);
+ if (textconv_two)
+ free(mf2.ptr);
}
free_ab_and_return:
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 1b09648..090a21d 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -52,7 +52,7 @@ test_expect_success 'setup textconv filters' '
git config diff.fail.textconv false
'
-test_expect_failure 'diff produces text' '
+test_expect_success 'diff produces text' '
git diff HEAD^ HEAD >diff &&
find_diff <diff >actual &&
test_cmp expect.text actual
@@ -64,7 +64,7 @@ test_expect_success 'diff-tree produces binary' '
test_cmp expect.binary actual
'
-test_expect_failure 'log produces text' '
+test_expect_success 'log produces text' '
git log -1 -p >log &&
find_diff <log >actual &&
test_cmp expect.text actual
@@ -80,7 +80,7 @@ cat >expect.stat <<'EOF'
file | Bin 2 -> 4 bytes
1 files changed, 0 insertions(+), 0 deletions(-)
EOF
-test_expect_failure 'diffstat does not run textconv' '
+test_expect_success 'diffstat does not run textconv' '
echo file diff=fail >.gitattributes &&
git diff --stat HEAD^ HEAD >actual &&
test_cmp expect.stat actual
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* [PATCH v3 5/8] userdiff: require explicitly allowing textconv
From: Jeff King @ 2008-10-26 4:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
Diffs that have been produced with textconv almost certainly
cannot be applied, so we want to be careful not to generate
them in things like format-patch.
This introduces a new diff options, ALLOW_TEXTCONV, which
controls this behavior. It is off by default, but is
explicitly turned on for the "log" family of commands, as
well as the "diff" porcelain (but not diff-* plumbing).
Because both text conversion and external diffing are
controlled by these diff options, we can get rid of the
"plumbing versus porcelain" distinction when reading the
config. This was an attempt to control the same thing, but
suffered from being too coarse-grained.
Signed-off-by: Jeff King <peff@peff.net>
---
The new changes in 4/8 make this even cleaner than before.
builtin-diff.c | 1 +
builtin-log.c | 1 +
diff.c | 26 +++++++++++---------------
diff.h | 1 +
t/t4030-diff-textconv.sh | 2 +-
userdiff.c | 10 +---------
userdiff.h | 3 +--
7 files changed, 17 insertions(+), 27 deletions(-)
diff --git a/builtin-diff.c b/builtin-diff.c
index 9c8c295..2de5834 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -300,6 +300,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
}
DIFF_OPT_SET(&rev.diffopt, ALLOW_EXTERNAL);
DIFF_OPT_SET(&rev.diffopt, RECURSIVE);
+ DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
/*
* If the user asked for our exit code then don't start a
diff --git a/builtin-log.c b/builtin-log.c
index a0944f7..75d698f 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -59,6 +59,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
} else
die("unrecognized argument: %s", arg);
}
+ DIFF_OPT_SET(&rev->diffopt, ALLOW_TEXTCONV);
}
/*
diff --git a/diff.c b/diff.c
index 6f01595..608223a 100644
--- a/diff.c
+++ b/diff.c
@@ -93,12 +93,6 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
if (!strcmp(var, "diff.external"))
return git_config_string(&external_diff_cmd_cfg, var, value);
- switch (userdiff_config_porcelain(var, value)) {
- case 0: break;
- case -1: return -1;
- default: return 0;
- }
-
return git_diff_basic_config(var, value, cb);
}
@@ -109,6 +103,12 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
return 0;
}
+ switch (userdiff_config(var, value)) {
+ case 0: break;
+ case -1: return -1;
+ default: return 0;
+ }
+
if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
int slot = parse_diff_color_slot(var, 11);
if (!value)
@@ -123,12 +123,6 @@ int git_diff_basic_config(const char *var, const char *value, void *cb)
return 0;
}
- switch (userdiff_config_basic(var, value)) {
- case 0: break;
- case -1: return -1;
- default: return 0;
- }
-
return git_color_default_config(var, value, cb);
}
@@ -1335,7 +1329,7 @@ static void builtin_diff(const char *name_a,
const char *set = diff_get_color_opt(o, DIFF_METAINFO);
const char *reset = diff_get_color_opt(o, DIFF_RESET);
const char *a_prefix, *b_prefix;
- const char *textconv_one, *textconv_two;
+ const char *textconv_one = NULL, *textconv_two = NULL;
diff_set_mnemonic_prefix(o, "a/", "b/");
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
@@ -1389,8 +1383,10 @@ static void builtin_diff(const char *name_a,
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
- textconv_one = get_textconv(one);
- textconv_two = get_textconv(two);
+ if (DIFF_OPT_TST(o, ALLOW_TEXTCONV)) {
+ textconv_one = get_textconv(one);
+ textconv_two = get_textconv(two);
+ }
if (!DIFF_OPT_TST(o, TEXT) &&
( (diff_filespec_is_binary(one) && !textconv_one) ||
diff --git a/diff.h b/diff.h
index a49d865..42582ed 100644
--- a/diff.h
+++ b/diff.h
@@ -65,6 +65,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_IGNORE_SUBMODULES (1 << 18)
#define DIFF_OPT_DIRSTAT_CUMULATIVE (1 << 19)
#define DIFF_OPT_DIRSTAT_BY_FILE (1 << 20)
+#define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
#define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 090a21d..1df48ae 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -70,7 +70,7 @@ test_expect_success 'log produces text' '
test_cmp expect.text actual
'
-test_expect_failure 'format-patch produces binary' '
+test_expect_success 'format-patch produces binary' '
git format-patch --no-binary --stdout HEAD^ >patch &&
find_diff <patch >actual &&
test_cmp expect.binary actual
diff --git a/userdiff.c b/userdiff.c
index d95257a..3681062 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -120,7 +120,7 @@ static int parse_tristate(int *b, const char *k, const char *v)
return 1;
}
-int userdiff_config_basic(const char *k, const char *v)
+int userdiff_config(const char *k, const char *v)
{
struct userdiff_driver *drv;
@@ -130,14 +130,6 @@ int userdiff_config_basic(const char *k, const char *v)
return parse_funcname(&drv->funcname, k, v, REG_EXTENDED);
if ((drv = parse_driver(k, v, "binary")))
return parse_tristate(&drv->binary, k, v);
-
- return 0;
-}
-
-int userdiff_config_porcelain(const char *k, const char *v)
-{
- struct userdiff_driver *drv;
-
if ((drv = parse_driver(k, v, "command")))
return parse_string(&drv->external, k, v);
if ((drv = parse_driver(k, v, "textconv")))
diff --git a/userdiff.h b/userdiff.h
index f29c18f..ba29457 100644
--- a/userdiff.h
+++ b/userdiff.h
@@ -14,8 +14,7 @@ struct userdiff_driver {
const char *textconv;
};
-int userdiff_config_basic(const char *k, const char *v);
-int userdiff_config_porcelain(const char *k, const char *v);
+int userdiff_config(const char *k, const char *v);
struct userdiff_driver *userdiff_find_by_name(const char *name);
struct userdiff_driver *userdiff_find_by_path(const char *path);
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* [PATCH v3 6/8] only textconv regular files
From: Jeff King @ 2008-10-26 4:46 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
We treat symlinks as text containing the results of the
symlink, so it doesn't make much sense to text-convert them.
Similarly gitlink components just end up as the text
"Subproject commit $sha1", which we should leave intact.
Note that a typechange may be broken into two parts: the
removal of the old part and the addition of the new. In that
case, we _do_ show the textconv for any part which is the
addition or removal of a file we would ordinarily textconv,
since it is purely acting on the file contents.
Signed-off-by: Jeff King <peff@peff.net>
---
Same as before.
diff.c | 2 ++
t/t4030-diff-textconv.sh | 2 +-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index 608223a..23d454e 100644
--- a/diff.c
+++ b/diff.c
@@ -1311,6 +1311,8 @@ static const char *get_textconv(struct diff_filespec *one)
{
if (!DIFF_FILE_VALID(one))
return NULL;
+ if (!S_ISREG(one->mode))
+ return NULL;
diff_filespec_load_driver(one);
return one->driver->textconv;
}
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 1df48ae..3945731 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -104,7 +104,7 @@ index ad8b3d2..67be421
\ No newline at end of file
EOF
# make a symlink the hard way that works on symlink-challenged file systems
-test_expect_failure 'textconv does not act on symlinks' '
+test_expect_success 'textconv does not act on symlinks' '
echo -n frotz > file &&
git add file &&
git ls-files -s | sed -e s/100644/120000/ |
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* [PATCH v3 7/8] wt-status: load diff ui config
From: Jeff King @ 2008-10-26 4:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
When "git status -v" shows a diff, we did not respect the
user's usual diff preferences at all. Loading just
git_diff_basic_config would give us things like rename
limits and diff drivers. But it makes even more sense to
load git_diff_ui_config, which gives us colorization if the
user has requested it.
Note that we need to take special care to cancel
colorization when writing to the commit template file, as
described in the code comments.
Signed-off-by: Jeff King <peff@peff.net>
---
This is necessary to do textconvs in the "status -v" diff (which will
come in the next patch).
But it makes me a little nervous. On one hand, I think it is definitely
the right thing for "status -v" to respect user options. But we do
several _other_ diffs in addition, and those are more "plumbing" diffs.
I think they should probably at least have diff_basic_config (e.g., for
rename limits). But we are applying the diff_ui_config options to all
diffs. Looking over the available options, I _think_ there are no nasty
surprises. But you never know.
t/t7502-commit.sh | 8 ++++++++
wt-status.c | 10 +++++++++-
2 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/t/t7502-commit.sh b/t/t7502-commit.sh
index 3eb9fae..ad42c78 100755
--- a/t/t7502-commit.sh
+++ b/t/t7502-commit.sh
@@ -89,6 +89,14 @@ test_expect_success 'verbose' '
'
+test_expect_success 'verbose respects diff config' '
+
+ git config color.diff always &&
+ git status -v >actual &&
+ grep "\[1mdiff --git" actual &&
+ git config --unset color.diff
+'
+
test_expect_success 'cleanup commit messages (verbatim,-t)' '
echo >>negative &&
diff --git a/wt-status.c b/wt-status.c
index c3a9cab..54d2f58 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -303,6 +303,14 @@ static void wt_status_print_verbose(struct wt_status *s)
rev.diffopt.detect_rename = 1;
rev.diffopt.file = s->fp;
rev.diffopt.close_file = 0;
+ /*
+ * If we're not going to stdout, then we definitely don't
+ * want color, since we are going to the commit message
+ * file (and even the "auto" setting won't work, since it
+ * will have checked isatty on stdout).
+ */
+ if (s->fp != stdout)
+ DIFF_OPT_CLR(&rev.diffopt, COLOR_DIFF);
run_diff_index(&rev, 1);
}
@@ -422,5 +430,5 @@ int git_status_config(const char *k, const char *v, void *cb)
return error("Invalid untracked files mode '%s'", v);
return 0;
}
- return git_color_default_config(k, v, cb);
+ return git_diff_ui_config(k, v, cb);
}
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* [PATCH v3 8/8] enable textconv for diff in verbose status/commit
From: Jeff King @ 2008-10-26 4:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <20081026043802.GA14530@coredump.intra.peff.net>
This diff is meant for human consumption, so it makes sense
to apply text conversion here, as we would for the regular
diff porcelain.
Signed-off-by: Jeff King <peff@peff.net>
---
Pretty straightforward, but depends on whether we find 7/8 acceptable.
t/t4030-diff-textconv.sh | 8 ++++++++
wt-status.c | 1 +
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/t/t4030-diff-textconv.sh b/t/t4030-diff-textconv.sh
index 3945731..09967f5 100755
--- a/t/t4030-diff-textconv.sh
+++ b/t/t4030-diff-textconv.sh
@@ -76,6 +76,14 @@ test_expect_success 'format-patch produces binary' '
test_cmp expect.binary actual
'
+test_expect_success 'status -v produces text' '
+ git reset --soft HEAD^ &&
+ git status -v >diff &&
+ find_diff <diff >actual &&
+ test_cmp expect.text actual &&
+ git reset --soft HEAD@{1}
+'
+
cat >expect.stat <<'EOF'
file | Bin 2 -> 4 bytes
1 files changed, 0 insertions(+), 0 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 54d2f58..6a7645e 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -301,6 +301,7 @@ static void wt_status_print_verbose(struct wt_status *s)
setup_revisions(0, NULL, &rev, s->reference);
rev.diffopt.output_format |= DIFF_FORMAT_PATCH;
rev.diffopt.detect_rename = 1;
+ DIFF_OPT_SET(&rev.diffopt, ALLOW_TEXTCONV);
rev.diffopt.file = s->fp;
rev.diffopt.close_file = 0;
/*
--
1.6.0.3.524.ge8b2e.dirty
^ permalink raw reply related
* Re: [PATCH 4/7] textconv: don't convert for every operation
From: Jeff King @ 2008-10-26 4:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Sixt, Matthieu Moy, git
In-Reply-To: <7vvdvg6xto.fsf@gitster.siamese.dyndns.org>
On Sat, Oct 25, 2008 at 04:48:19PM -0700, Junio C Hamano wrote:
> Another place that would benefit from ALLOW_TEXTCONV is the function
> wt_status_print_verbose() in wt-status.c, where we generate preview diff
> for "git commit -v". The output from that function is purely for human
> consumption and does not have to be applicable.
I agree, but it turned out not to be as straightforward as I had hoped,
since status doesn't even parse diff.* config. See patches 7 and 8 in
the series I just posted.
-Peff
^ permalink raw reply
* Re: Fwd: git status options feature suggestion
From: Jeff King @ 2008-10-26 4:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Michael J Gruber, Johannes Schindelin, Caleb Cushing, git
In-Reply-To: <7vej246sb4.fsf@gitster.siamese.dyndns.org>
On Sat, Oct 25, 2008 at 06:47:27PM -0700, Junio C Hamano wrote:
> This was done primarily for fun and killing-time, so I won't be committing
> it to my tree, but it seems to pass all the existing tests.
I just skimmed the code, but it looks reasonably done. I don't have time
to work on this at the moment, but I think you have done most of the
work that requires a lot of git knowledge. Maybe somebody new can pick
this up as a nice starter project to get more involved in git.
-Peff
^ permalink raw reply
* Re: Problem with git filter-branch
From: Jeff King @ 2008-10-26 5:07 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Pascal Obry, git list
In-Reply-To: <alpine.DEB.1.00.0810252235040.22125@pacific.mpi-cbg.de.mpi-cbg.de>
On Sat, Oct 25, 2008 at 10:36:26PM +0200, Johannes Schindelin wrote:
> It is a (maybe ill-conceived) feature. When branches are rewritten, their
> original versions are stored in the refs/original/ namespace. You can
> force overwriting with "-f".
>
> I wonder if people would like to have this feature removed; reflogs should
> be enough.
>
> Thoughts?
I have always been annoyed by the refs/original namespace, and it
certainly makes explaining the common task of "how do I get this blob
out of my repo" a bit more confusing.
So I would be happy to see it go, and the reflog mentioned in its place:
diff --git a/Documentation/git-filter-branch.txt b/Documentation/git-filter-branch.txt
index fed6de6..1e8e7b4 100644
--- a/Documentation/git-filter-branch.txt
+++ b/Documentation/git-filter-branch.txt
@@ -43,4 +43,4 @@ rewriting published history.)
Always verify that the rewritten version is correct: The original refs,
-if different from the rewritten ones, will be stored in the namespace
-'refs/original/'.
+if different from the rewritten ones, will be available through the
+reflog as <branch>@{1}.
^ permalink raw reply related
* Re: [PATCH] add -p: warn if only binary changes present
From: Jeff King @ 2008-10-26 5:10 UTC (permalink / raw)
To: Thomas Rast; +Cc: git, Junio C Hamano
In-Reply-To: <1224884916-20369-1-git-send-email-trast@student.ethz.ch>
On Fri, Oct 24, 2008 at 11:48:36PM +0200, Thomas Rast wrote:
> Current 'git add -p' will say "No changes." if there are no changes to
> text files, which can be confusing if there _are_ changes to binary
> files. Add some code to distinguish the two cases, and give a
> different message in the latter one.
Having just looked at this code (for potentially handling "git add -p"
of new files), I think this change is sane.
> + print STDERR "No changes except to binary files.\n";
This wording seems a little awkward to me, though. Maybe
No changed text files.
?
-Peff
^ permalink raw reply
* Re: [PATCH 3/7] gitk: Allow starting gui blame for a specific line.
From: Paul Mackerras @ 2008-10-26 3:58 UTC (permalink / raw)
To: Alexander Gavrilov; +Cc: git
In-Reply-To: <200810252045.54010.angavrilov@gmail.com>
Alexander Gavrilov writes:
> Ugh. I guess I'll have to install docs from Tcl 8.4...
Yes, although I strongly encourage people to move to 8.5, I haven't
made it a requirement (yet :).
Paul.
^ permalink raw reply
* Re: git export to svn
From: Björn Steinbrink @ 2008-10-26 9:15 UTC (permalink / raw)
To: Warren Harris; +Cc: J.H., git
In-Reply-To: <2F1954CC-E013-4861-87F8-F89CF37CE53C@gmail.com>
On 2008.10.25 13:29:50 -0700, Warren Harris wrote:
> I tried a fetch, but still no luck:
>
> $ git svn fetch
> W: Ignoring error from SVN, path probably does not exist: (175002): RA
> layer request failed: REPORT of '/svn/!svn/bc/100': Could not read chunk
> size: Secure connection truncated (https://svn)
> W: Do not be alarmed at the above message git-svn is just searching
> aggressively for old history.
> This may take a while on large repositories
> r58084 = c01dadf89b552077da132273485e7569d8694518 (trunk)
> A ...
> r58088 = 7916f3a02ad6c759985bd9fb886423c373a72125 (trunk)
>
> $ git svn rebase
> Unable to determine upstream SVN information from working tree history
Means that your current branch is not based on what git-svn has fetched,
which is expected when you use "svn init" + "svn fetch" after you
already started working.
What's the actual relationship between your local history and the
history you fetched from svn?
If your local stuff started from revision X, which you manually imported
from svn, you can just rebase it:
git rebase --onto <svn-revision-X> <your-revision-X>
If you have a bunch of merges in your local history, you might want to
merge your stuff into the svn-based branch instead. When you dcommit,
the svn repo will only see one big "do it all" commit though.
For that, you would create a graft, so that your first "real" local
commit gets the svn revision X commit as its parent. That is, from:
S---S---SX---S---S---S (svn)
LX--------L---L---L---L (local)
You want to go to:
S---S---SX---S---S---S (svn)
\
LX L---L---L---L (local)
Where 'S' means that the commit came from SVN, and L means that it is a
"local" commit. SX and LX are the commits that have the same tree
attached (same directories/files), but have a different hash due to how
they were created. The graft overrides the parent-child relation for the
first "L" commit, so that it actually appears as being branched off of SX.
And then, you'd merge local into svn, so you get:
S---S---SX---S---S---S--M (svn)
\ /
LX L---L---L---L (local)
If possible, go with the rebase though. That at least gives a somewhat
reasonable history in the svn repo as well. Also note that when you go
with the merge way, make sure that the svn branch is totally uptodate
before you merge and that the merge commit is the only one to be
dcommitted. Otherwise, funny stuff might happen, and rebase might kick
in anyway, I don't exactly remember what git-svn does, but it wasn't
pleasant :-)
Björn
^ permalink raw reply
* Re: [PATCH] add -p: warn if only binary changes present
From: Thomas Rast @ 2008-10-26 10:28 UTC (permalink / raw)
To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20081026051013.GD21178@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 763 bytes --]
Jeff King wrote:
> > + print STDERR "No changes except to binary files.\n";
>
> This wording seems a little awkward to me, though. Maybe
>
> No changed text files.
>
> ?
I tried to make a more precise statement. "No changed text files"
also holds if no files at all were changed. A user can only infer
that there _are_ binary changes if he knows that the message in the
latter case would have been "No changes".
That being said, it is somewhat awkward, I just couldn't come up with
a better message with this meaning. I considered some other options,
such as giving a hint to use git-add or running git-status for the
user, but these would have been far more verbose.
- Thomas
--
Thomas Rast
trast@{inf,student}.ethz.ch
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] add -p: warn if only binary changes present
From: SZEDER Gábor @ 2008-10-26 10:40 UTC (permalink / raw)
To: Thomas Rast; +Cc: Jeff King, git, Junio C Hamano
In-Reply-To: <200810261128.14735.trast@student.ethz.ch>
On Sun, Oct 26, 2008 at 12:28:09PM +0200, Thomas Rast wrote:
> Jeff King wrote:
> > > + print STDERR "No changes except to binary files.\n";
> > No changed text files.
> I tried to make a more precise statement. "No changed text files"
> also holds if no files at all were changed. A user can only infer
> that there _are_ binary changes if he knows that the message in the
> latter case would have been "No changes".
>
> That being said, it is somewhat awkward, I just couldn't come up with
> a better message with this meaning.
What about
Only binary files changed.
or something of the sort?
Gábor
^ permalink raw reply
* [PATCH] autoconf: Add link tests to each AC_CHECK_FUNC() test
From: David M. Syzdek @ 2008-10-26 11:52 UTC (permalink / raw)
To: git; +Cc: jnareb, David M. Syzdek
Update configure.ac to test libraries for getaddrinfo, strcasestr, memmem,
strlcpy, strtoumax, setenv, unsetenv, and mkdtemp. The default compilers
on FreeBSD 4.9-SECURITY and FreeBSD 6.2-RELEASE-p4 do not generate warnings
for missing prototypes unless `-Wall' is used. This behavior renders the
results of AC_CHECK_FUNC() void on these platforms. The test AC_SEARCH_LIBS()
verifies a function is valid by linking to symbol within the system libraries.
Signed-off-by: David M. Syzdek <david.syzdek@acsalaska.net>
---
configure.ac | 57 ++++++++++++++++++++++++++++++++++++++++-----------------
1 files changed, 40 insertions(+), 17 deletions(-)
diff --git a/configure.ac b/configure.ac
index 7c2856e..d3b8bc3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -293,9 +293,11 @@ AC_SUBST(NO_SOCKADDR_STORAGE)
#
# Define NO_IPV6 if you lack IPv6 support and getaddrinfo().
AC_CHECK_TYPE([struct addrinfo],[
- AC_CHECK_FUNC([getaddrinfo],
- [NO_IPV6=],
- [NO_IPV6=YesPlease])
+ AC_CHECK_FUNC([getaddrinfo],[
+ AC_SEARCH_LIBS([getaddrinfo],,
+ [NO_IPV6=],
+ [NO_IPV6=YesPlease])
+ ],[NO_IPV6=YesPlease])
],[NO_IPV6=YesPlease],[
#include <sys/types.h>
#include <sys/socket.h>
@@ -387,44 +389,65 @@ AC_SUBST(SNPRINTF_RETURNS_BOGUS)
AC_MSG_NOTICE([CHECKS for library functions])
#
# Define NO_STRCASESTR if you don't have strcasestr.
-AC_CHECK_FUNC(strcasestr,
-[NO_STRCASESTR=],
+AC_CHECK_FUNC(strcasestr,[
+ AC_SEARCH_LIBS(strcasestr,,
+ [NO_STRCASESTR=],
+ [NO_STRCASESTR=YesPlease])
+],
[NO_STRCASESTR=YesPlease])
AC_SUBST(NO_STRCASESTR)
#
# Define NO_MEMMEM if you don't have memmem.
-AC_CHECK_FUNC(memmem,
-[NO_MEMMEM=],
+AC_CHECK_FUNC(memmem,[
+ AC_SEARCH_LIBS(memmem,,
+ [NO_MEMMEM=],
+ [NO_MEMMEM=YesPlease])
+],
[NO_MEMMEM=YesPlease])
AC_SUBST(NO_MEMMEM)
#
# Define NO_STRLCPY if you don't have strlcpy.
-AC_CHECK_FUNC(strlcpy,
-[NO_STRLCPY=],
+AC_CHECK_FUNC(strlcpy,[
+ AC_SEARCH_LIBS(strlcpy,,
+ [NO_STRLCPY=],
+ [NO_STRLCPY=YesPlease])
+],
[NO_STRLCPY=YesPlease])
AC_SUBST(NO_STRLCPY)
#
# Define NO_STRTOUMAX if you don't have strtoumax in the C library.
-AC_CHECK_FUNC(strtoumax,
-[NO_STRTOUMAX=],
+AC_CHECK_FUNC(strtoumax,[
+ AC_SEARCH_LIBS(strtoumax,,
+ [NO_STRTOUMAX=],
+ [NO_STRTOUMAX=YesPlease])
+],
[NO_STRTOUMAX=YesPlease])
AC_SUBST(NO_STRTOUMAX)
#
# Define NO_SETENV if you don't have setenv in the C library.
-AC_CHECK_FUNC(setenv,
-[NO_SETENV=],
+AC_CHECK_FUNC(setenv,[
+ AC_SEARCH_LIBS(setenv,,
+ [NO_SETENV=],
+ [NO_SETENV=YesPlease])
+],
[NO_SETENV=YesPlease])
AC_SUBST(NO_SETENV)
#
# Define NO_UNSETENV if you don't have unsetenv in the C library.
-AC_CHECK_FUNC(unsetenv,
-[NO_UNSETENV=],
+AC_CHECK_FUNC(unsetenv,[
+ AC_SEARCH_LIBS(unsetenv,,
+ [NO_UNSETENV=],
+ [NO_UNSETENV=YesPlease])
+],
[NO_UNSETENV=YesPlease])
AC_SUBST(NO_UNSETENV)
#
# Define NO_MKDTEMP if you don't have mkdtemp in the C library.
-AC_CHECK_FUNC(mkdtemp,
-[NO_MKDTEMP=],
+AC_CHECK_FUNC(mkdtemp,[
+ AC_SEARCH_LIBS(mkdtemp,,
+ [NO_MKDTEMP=],
+ [NO_MKDTEMP=YesPlease])
+],
[NO_MKDTEMP=YesPlease])
AC_SUBST(NO_MKDTEMP)
#
--
1.6.0.2.GIT
^ permalink raw reply related
* [PATCH] Add Makefile check for FreeBSD 4.9-SECURITY
From: David M. Syzdek @ 2008-10-26 11:52 UTC (permalink / raw)
To: git; +Cc: David M. Syzdek
If the system is FreeBSD 4.9, then NO_UINTMAX_T and NO_STRTOUMAX is defined.
Signed-off-by: David M. Syzdek <david.syzdek@acsalaska.net>
---
Makefile | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index bf6a6dc..c568314 100644
--- a/Makefile
+++ b/Makefile
@@ -679,6 +679,10 @@ ifeq ($(uname_S),FreeBSD)
DIR_HAS_BSD_GROUP_SEMANTICS = YesPlease
COMPAT_CFLAGS += -Icompat/regex
COMPAT_OBJS += compat/regex/regex.o
+ ifeq ($(uname_R),4.9-SECURITY)
+ NO_UINTMAX_T = YesPlease
+ NO_STRTOUMAX = YesPlease
+ endif
endif
ifeq ($(uname_S),OpenBSD)
NO_STRCASESTR = YesPlease
--
1.6.0.2.GIT
^ permalink raw reply related
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