* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Jeff King @ 2009-03-08 14:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Chris Johnsen, Miklos Vajna
In-Reply-To: <1236418251-16947-1-git-send-email-chris_johnsen@pobox.com>
On Sat, Mar 07, 2009 at 03:30:51AM -0600, Chris Johnsen wrote:
> +test_expect_code 1 'cherry-pick an empty commit' '
> +
> + git checkout master &&
> + git cherry-pick empty-branch
> +
> +'
Hmm. This test fails for me on FreeBSD. However, it seems to be related
to a shell portability issue with the test suite. The extra newline
inside the shell snippet seems to lose the last status. The following
works fine for me with bash or dash:
-- >8 --
#!/bin/sh
test_description='test that shell handles whitespace in eval'
. ./test-lib.sh
test_expect_code 1 'no newline' 'false'
test_expect_code 1 'one newline' 'false
'
test_expect_code 1 'extra newline' 'false
'
test_done
-- 8< --
but on a FreeBSD 6.1 box generates:
* ok 1: no newline
* ok 2: one newline
* FAIL 3: 1
extra newline false
With this minimal example, you can see that the problem is not some
subtle bug in the test suite:
-- >8 --
#!/bin/sh
eval 'false'
echo status is $?
eval 'false
'
echo status is $?
eval 'false
'
echo status is $?
-- 8< --
generates:
status is 1
status is 1
status is 0
which means that any tests of the form
test_expect_success description '
contents
'
are not actually having their exit code checked properly, and are just
claiming success regardless of what happened. So this definitely needs
to be addressed, I think. I'm not sure of the best course of action,
though. Our options include:
1. Declare appended newline a forbidden style, fix all existing cases
in the test suite, and be on the lookout for new ones.
The biggest problem with this option is that we have no automated
way of policing. Such tests will just silently pass on the broken
platform.
2. Have test_run_ canonicalize the snippet by removing trailing
newlines.
3. Declare FreeBSD's /bin/sh unfit for git consumption, and require
bash for the test suite.
I think (2) is the most reasonable option of those choices.
We could also try to convince FreeBSD that it's a bug, but that doesn't
change the fact that the tests are broken on every existing version.
-Peff
^ permalink raw reply
* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Jeff King @ 2009-03-08 15:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Chris Johnsen, Miklos Vajna
In-Reply-To: <20090308144240.GA30794@coredump.intra.peff.net>
On Sun, Mar 08, 2009 at 10:42:40AM -0400, Jeff King wrote:
> 2. Have test_run_ canonicalize the snippet by removing trailing
> newlines.
> [...]
> I think (2) is the most reasonable option of those choices.
And here's what that would look like:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7a847ec..276a14d 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -273,7 +273,7 @@ test_debug () {
}
test_run_ () {
- eval >&3 2>&4 "$1"
+ eval >&3 2>&4 "`echo "$1" | sed -e :a -e '/^ *\n*$/{$d;N;ba' -e '}'`"
eval_ret="$?"
return 0
}
That is a truly hideous sed expression, and I would be happy to hear
more readable suggestions (it is based on one from the "sed one-liners"
compilation).
With this applied, t3505 passes for me. However, some other random tests
are broken as a result. It looks like it might be related to an extra
round of '\' expansion.
-Peff
^ permalink raw reply related
* [EGIT PATCH] Prevent an exception if the user tries to push a non-existing ref.
From: Robin Rosenberg @ 2009-03-08 15:21 UTC (permalink / raw)
To: spearce, Daniel Cheng; +Cc: git, Robin Rosenberg
In-Reply-To: <20090307224831.GS16213@spearce.org>
Instead of a StringIndexOutOfBoundsException we now get an error telling
us that the ref could not be resolved.
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
.../src/org/spearce/jgit/transport/Transport.java | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java
index a0a2575..8a25213 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/Transport.java
@@ -255,7 +255,7 @@ else if (TransportLocal.canHandle(remote))
} else {
if (!remoteName.startsWith(Constants.R_REFS)) {
// null source is another special case (delete)
- if (srcRef != null) {
+ if (src != null) {
// assume the same type of ref at the destination
String srcPrefix = srcRef.substring(0, srcRef.indexOf('/', Constants.R_REFS.length()));
remoteName = srcPrefix + "/" + remoteName;
--
1.6.1.285.g35d8b
^ permalink raw reply related
* [ANNOUNCE] Magit 0.7
From: Marius Vollmer @ 2009-03-08 17:12 UTC (permalink / raw)
To: git; +Cc: magit
It's time for the next release of Magit, the Emacs interface to Git!
http://zagadka.vm.bytemark.co.uk/magit/
A lot of people have contributed to it. Thanks a lot!
Please report bugs and general feedback to our little mailing list:
http://groups.google.com/group/magit
or to me directly, of course.
What's new:
* Tagging, on 't' and 'T'.
* Stashing, on 'z' and 'Z'.
* Wazzup, on 'w'. Wazzup gives you an overview over how other
branches relate to the current one.
* There is more control over pushing. 'P' now takes a prefix argument
and pushing a branch without a default remote will ask for one.
* Logs have changed a bit: 'l' shows the traditional brief log, and
'L' shows a more verbose log. Use the prefix arg to specify the
range of the log.
* M-x magit-status doesn't prompt anymore for a directory when invoked
from within a Git repository. Use C-u to force a prompt.
* When you have nothing staged, 'c' will now explicitly ask whether to
commit everything instead of just going ahead and do it. This can
be customized.
* The digit keys '1', '2', '3', and '4' now show sections on the
respective level and hide everything below. With Meta, they work on
all sections; without, they work only on sections that are a parent
or child of the current section.
* Typing '+' and '-' will change the size of hunks, via the "-U"
option to git diff. '0' resets hunks to their default size.
* Typing 'k' on the "Untracked files" section title will offer to
delete all untracked files.
* Magit understands a bit of git-svn: the status buffer shows unpushed
and unpulled commits, 'N r' runs git svn rebase, and 'N c' runs git
svn dcommit.
* Magit now also works when the direcory is accessed via tramp.
* M-x magit-status can also create new repositories when given a
directory that is not a Git repository.
* Magit works better with oldish Gits that don't understand "--graph",
for example.
* The name of the Git program and common options for it can be
customized.
Enjoy!
^ permalink raw reply
* [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN
From: René Scharfe @ 2009-03-08 18:12 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit
In-Reply-To: <49B2C784.90800@lsrfire.ath.cx>
Add a parseopt flag, PARSE_OPT_KEEP_UNKNOWN, that can be used to keep
unknown options in argv, similar to the existing KEEP flags.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
parse-options.c | 12 +++++++++---
parse-options.h | 1 +
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 4c5d09d..39808ae 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -274,7 +274,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
case -1:
return parse_options_usage(usagestr, options);
case -2:
- return PARSE_OPT_UNKNOWN;
+ goto unknown;
}
if (ctx->opt)
check_typos(arg + 1, options);
@@ -292,7 +292,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
*/
ctx->argv[0] = xstrdup(ctx->opt - 1);
*(char *)ctx->argv[0] = '-';
- return PARSE_OPT_UNKNOWN;
+ goto unknown;
}
}
continue;
@@ -314,8 +314,14 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
case -1:
return parse_options_usage(usagestr, options);
case -2:
- return PARSE_OPT_UNKNOWN;
+ goto unknown;
}
+ continue;
+unknown:
+ if (!(ctx->flags & PARSE_OPT_KEEP_UNKNOWN))
+ return PARSE_OPT_UNKNOWN;
+ ctx->out[ctx->cpidx++] = ctx->argv[0];
+ ctx->opt = NULL;
}
return PARSE_OPT_DONE;
}
diff --git a/parse-options.h b/parse-options.h
index 9122905..b7d08b1 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -21,6 +21,7 @@ enum parse_opt_flags {
PARSE_OPT_KEEP_DASHDASH = 1,
PARSE_OPT_STOP_AT_NON_OPTION = 2,
PARSE_OPT_KEEP_ARGV0 = 4,
+ PARSE_OPT_KEEP_UNKNOWN = 8,
};
enum parse_opt_option_flags {
--
1.6.2
^ permalink raw reply related
* [PATCH 2/4] parseopt: add PARSE_OPT_NO_INTERNAL_HELP
From: René Scharfe @ 2009-03-08 18:15 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit
In-Reply-To: <49B2C784.90800@lsrfire.ath.cx>
Add a parseopt flag, PARSE_OPT_NO_INTERNAL_HELP, that turns off internal
handling of -h, --help and --help-all. This allows the implementation
of custom help option handlers or incremental parsers.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
parse-options.c | 10 ++++++----
parse-options.h | 1 +
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 39808ae..8b21dea 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -253,6 +253,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[])
{
+ int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);
+
/* we must reset ->opt, unknown short option leave it dangling */
ctx->opt = NULL;
@@ -268,7 +270,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (arg[1] != '-') {
ctx->opt = arg + 1;
- if (*ctx->opt == 'h')
+ if (internal_help && *ctx->opt == 'h')
return parse_options_usage(usagestr, options);
switch (parse_short_opt(ctx, options)) {
case -1:
@@ -279,7 +281,7 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (ctx->opt)
check_typos(arg + 1, options);
while (ctx->opt) {
- if (*ctx->opt == 'h')
+ if (internal_help && *ctx->opt == 'h')
return parse_options_usage(usagestr, options);
switch (parse_short_opt(ctx, options)) {
case -1:
@@ -306,9 +308,9 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
break;
}
- if (!strcmp(arg + 2, "help-all"))
+ if (internal_help && !strcmp(arg + 2, "help-all"))
return usage_with_options_internal(usagestr, options, 1);
- if (!strcmp(arg + 2, "help"))
+ if (internal_help && !strcmp(arg + 2, "help"))
return parse_options_usage(usagestr, options);
switch (parse_long_opt(ctx, arg + 2, options)) {
case -1:
diff --git a/parse-options.h b/parse-options.h
index b7d08b1..f8ef1db 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -22,6 +22,7 @@ enum parse_opt_flags {
PARSE_OPT_STOP_AT_NON_OPTION = 2,
PARSE_OPT_KEEP_ARGV0 = 4,
PARSE_OPT_KEEP_UNKNOWN = 8,
+ PARSE_OPT_NO_INTERNAL_HELP = 16,
};
enum parse_opt_option_flags {
--
1.6.2
^ permalink raw reply related
* [PATCH 3/4] parseopt: make usage optional
From: René Scharfe @ 2009-03-08 18:16 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit
In-Reply-To: <49B2C784.90800@lsrfire.ath.cx>
Allow usagestr to be NULL and don't display anything a help screen in
this case. This is useful to implement incremental parsers.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
parse-options.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 8b21dea..51e804b 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -364,6 +364,9 @@ int parse_options(int argc, const char **argv, const struct option *options,
int usage_with_options_internal(const char * const *usagestr,
const struct option *opts, int full)
{
+ if (!usagestr)
+ return PARSE_OPT_HELP;
+
fprintf(stderr, "usage: %s\n", *usagestr++);
while (*usagestr && **usagestr)
fprintf(stderr, " or: %s\n", *usagestr++);
--
1.6.2
^ permalink raw reply related
* [PATCH 4/4] archive: use parseopt for local-only options
From: René Scharfe @ 2009-03-08 18:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, carlos.duclos, Pierre Habouzit
In-Reply-To: <49B2C784.90800@lsrfire.ath.cx>
Replace the hand-rolled parsers that find and remove --remote and --exec
by a parseopt parser that also handles --output.
All three options only have a meaning if no remote server is used or on
the local side. They must be rejected by upload-archive and should not
be sent to the server by archive.
We can't use a single parser for both remote and local side because the
remote end possibly understands a different set of options than the
local side. A local parser would then wrongly accuse options valid on
the other side as being incorrect.
This patch implements a very forgiving parser that understands only the
three options mentioned above. All others are passed to the normal,
complete parser in archive.c (running either locally in archive, or
remotely in upload-archive). This normal parser definition contains
dummy entries for the three options, in order for them to appear in the
help screen.
The parseopt parser allows multiple occurrences of --remote and --exec
unlike the previous one; the one specified last wins. This looseness
is acceptable, I think.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.c | 18 +--------
builtin-archive.c | 103 +++++++++++++++++++---------------------------------
2 files changed, 40 insertions(+), 81 deletions(-)
diff --git a/archive.c b/archive.c
index c6aea83..96b62d4 100644
--- a/archive.c
+++ b/archive.c
@@ -239,19 +239,6 @@ static void parse_treeish_arg(const char **argv,
ar_args->time = archive_time;
}
-static void create_output_file(const char *output_file)
-{
- int output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
- if (output_fd < 0)
- die("could not create archive file: %s ", output_file);
- if (output_fd != 1) {
- if (dup2(output_fd, 1) < 0)
- die("could not redirect output");
- else
- close(output_fd);
- }
-}
-
#define OPT__COMPR(s, v, h, p) \
{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
@@ -306,13 +293,12 @@ static int parse_archive_args(int argc, const char **argv,
die("Unexpected option --remote");
if (exec)
die("Option --exec can only be used together with --remote");
+ if (output)
+ die("Unexpected option --output");
if (!base)
base = "";
- if (output)
- create_output_file(output);
-
if (list) {
for (i = 0; i < ARRAY_SIZE(archivers); i++)
printf("%s\n", archivers[i].name);
diff --git a/builtin-archive.c b/builtin-archive.c
index 5ceec43..60adef9 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -5,44 +5,35 @@
#include "cache.h"
#include "builtin.h"
#include "archive.h"
+#include "parse-options.h"
#include "pkt-line.h"
#include "sideband.h"
-static int run_remote_archiver(const char *remote, int argc,
- const char **argv)
+static void create_output_file(const char *output_file)
+{
+ int output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
+ if (output_fd < 0)
+ die("could not create archive file: %s ", output_file);
+ if (output_fd != 1) {
+ if (dup2(output_fd, 1) < 0)
+ die("could not redirect output");
+ else
+ close(output_fd);
+ }
+}
+
+static int run_remote_archiver(int argc, const char **argv,
+ const char *remote, const char *exec)
{
char *url, buf[LARGE_PACKET_MAX];
int fd[2], i, len, rv;
struct child_process *conn;
- const char *exec = "git-upload-archive";
- int exec_at = 0, exec_value_at = 0;
-
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
- if (!prefixcmp(arg, "--exec=")) {
- if (exec_at)
- die("multiple --exec specified");
- exec = arg + 7;
- exec_at = i;
- } else if (!strcmp(arg, "--exec")) {
- if (exec_at)
- die("multiple --exec specified");
- if (i + 1 >= argc)
- die("option --exec requires a value");
- exec = argv[i + 1];
- exec_at = i;
- exec_value_at = ++i;
- }
- }
url = xstrdup(remote);
conn = git_connect(fd, url, exec, 0);
- for (i = 1; i < argc; i++) {
- if (i == exec_at || i == exec_value_at)
- continue;
+ for (i = 1; i < argc; i++)
packet_write(fd[1], "argument %s\n", argv[i]);
- }
packet_flush(fd[1]);
len = packet_read_line(fd[0], buf, sizeof(buf));
@@ -69,51 +60,33 @@ static int run_remote_archiver(const char *remote, int argc,
return !!rv;
}
-static const char *extract_remote_arg(int *ac, const char **av)
-{
- int ix, iy, cnt = *ac;
- int no_more_options = 0;
- const char *remote = NULL;
-
- for (ix = iy = 1; ix < cnt; ix++) {
- const char *arg = av[ix];
- if (!strcmp(arg, "--"))
- no_more_options = 1;
- if (!no_more_options) {
- if (!prefixcmp(arg, "--remote=")) {
- if (remote)
- die("Multiple --remote specified");
- remote = arg + 9;
- continue;
- } else if (!strcmp(arg, "--remote")) {
- if (remote)
- die("Multiple --remote specified");
- if (++ix >= cnt)
- die("option --remote requires a value");
- remote = av[ix];
- continue;
- }
- if (arg[0] != '-')
- no_more_options = 1;
- }
- if (ix != iy)
- av[iy] = arg;
- iy++;
- }
- if (remote) {
- av[--cnt] = NULL;
- *ac = cnt;
- }
- return remote;
-}
+#define PARSE_OPT_KEEP_ALL ( PARSE_OPT_KEEP_DASHDASH | \
+ PARSE_OPT_KEEP_ARGV0 | \
+ PARSE_OPT_KEEP_UNKNOWN | \
+ PARSE_OPT_NO_INTERNAL_HELP )
int cmd_archive(int argc, const char **argv, const char *prefix)
{
+ const char *exec = "git-upload-archive";
+ const char *output = NULL;
const char *remote = NULL;
+ struct option local_opts[] = {
+ OPT_STRING(0, "output", &output, "file",
+ "write the archive to this file"),
+ OPT_STRING(0, "remote", &remote, "repo",
+ "retrieve the archive from remote repository <repo>"),
+ OPT_STRING(0, "exec", &exec, "cmd",
+ "path to the remote git-upload-archive command"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, local_opts, NULL, PARSE_OPT_KEEP_ALL);
+
+ if (output)
+ create_output_file(output);
- remote = extract_remote_arg(&argc, argv);
if (remote)
- return run_remote_archiver(remote, argc, argv);
+ return run_remote_archiver(argc, argv, remote, exec);
setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
--
1.6.2
^ permalink raw reply related
* [PATCH 1/2] fast-import: no longer compute the SHA1 data in store_object(). Instead, do it in the caller through the new sha1_object() and parse_object_data() functions.
From: Sam Hocevar @ 2009-03-08 18:35 UTC (permalink / raw)
To: git
This is necessary if we want to send deflated data to store_object() without
keeping the original data in memory.
Signed-off-by: Sam Hocevar <sam@zoy.org>
---
fast-import.c | 58 +++++++++++++++++++++++++++++++++++++++++---------------
1 files changed, 42 insertions(+), 16 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 3748ddf..6419d00 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1022,29 +1022,35 @@ static size_t encode_header(
return n;
}
-static int store_object(
+static void sha1_object(
enum object_type type,
struct strbuf *dat,
- struct last_object *last,
- unsigned char *sha1out,
- uintmax_t mark)
+ unsigned char *sha1out)
{
- void *out, *delta;
- struct object_entry *e;
unsigned char hdr[96];
- unsigned char sha1[20];
- unsigned long hdrlen, deltalen;
+ unsigned long hdrlen;
git_SHA_CTX c;
- z_stream s;
hdrlen = sprintf((char*)hdr,"%s %lu", typename(type),
(unsigned long)dat->len) + 1;
git_SHA1_Init(&c);
git_SHA1_Update(&c, hdr, hdrlen);
git_SHA1_Update(&c, dat->buf, dat->len);
- git_SHA1_Final(sha1, &c);
- if (sha1out)
- hashcpy(sha1out, sha1);
+ git_SHA1_Final(sha1out, &c);
+}
+
+static int store_object(
+ enum object_type type,
+ struct strbuf *dat,
+ struct last_object *last,
+ unsigned char *sha1,
+ uintmax_t mark)
+{
+ void *out, *delta;
+ struct object_entry *e;
+ unsigned char hdr[96];
+ unsigned long hdrlen, deltalen;
+ z_stream s;
e = insert_object(sha1);
if (mark)
@@ -1336,6 +1342,7 @@ static void store_tree(struct tree_entry *root)
}
mktree(t, 1, &new_tree);
+ sha1_object(OBJ_TREE, &new_tree, root->versions[1].sha1);
store_object(OBJ_TREE, &new_tree, &lo, root->versions[1].sha1, 0);
t->delta_depth = lo.depth;
@@ -1702,7 +1709,12 @@ static void parse_mark(void)
next_mark = 0;
}
-static void parse_data(struct strbuf *sb)
+/* This actually parses a "data" command, with the addition that if sha1out
+ * is not NULL, it will also compute the sha1 on the fly. */
+static void parse_object_data(
+ enum object_type type,
+ struct strbuf *sb,
+ unsigned char *sha1out)
{
strbuf_reset(sb);
@@ -1724,6 +1736,9 @@ static void parse_data(struct strbuf *sb)
strbuf_addch(sb, '\n');
}
free(term);
+
+ if(sha1out)
+ sha1_object(type, sb, sha1out);
}
else {
size_t n = 0, length;
@@ -1737,11 +1752,19 @@ static void parse_data(struct strbuf *sb)
(unsigned long)(length - n));
n += s;
}
+
+ if(sha1out)
+ sha1_object(type, sb, sha1out);
}
skip_optional_lf();
}
+static void parse_data(struct strbuf *sb)
+{
+ parse_object_data(OBJ_NONE, sb, NULL);
+}
+
static int validate_raw_date(const char *src, char *result, int maxlen)
{
const char *orig_src = src;
@@ -1805,12 +1828,13 @@ static char *parse_ident(const char *buf)
static void parse_new_blob(void)
{
+ unsigned char sha1[20];
static struct strbuf buf = STRBUF_INIT;
read_next_command();
parse_mark();
- parse_data(&buf);
- store_object(OBJ_BLOB, &buf, &last_blob, NULL, next_mark);
+ parse_object_data(OBJ_BLOB, &buf, sha1);
+ store_object(OBJ_BLOB, &buf, &last_blob, sha1, next_mark);
}
static void unload_one_branch(void)
@@ -1928,7 +1952,7 @@ static void file_change_m(struct branch *b)
p = uq.buf;
}
read_next_command();
- parse_data(&buf);
+ parse_object_data(OBJ_BLOB, &buf, sha1);
store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);
} else if (oe) {
if (oe->type != OBJ_BLOB)
@@ -2211,6 +2235,7 @@ static void parse_new_commit(void)
free(author);
free(committer);
+ sha1_object(OBJ_COMMIT, &new_data, b->sha1);
if (!store_object(OBJ_COMMIT, &new_data, NULL, b->sha1, next_mark))
b->pack_id = pack_id;
b->last_commit = object_count_by_type[OBJ_COMMIT];
@@ -2291,6 +2316,7 @@ static void parse_new_tag(void)
strbuf_addbuf(&new_data, &msg);
free(tagger);
+ sha1_object(OBJ_TAG, &new_data, t->sha1);
if (store_object(OBJ_TAG, &new_data, NULL, t->sha1, 0))
t->pack_id = MAX_PACK_ID;
else
--
1.6.2
^ permalink raw reply related
* Re: Git for Windows 1.6.2-preview20090308
From: Johannes Schindelin @ 2009-03-08 18:43 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, msysgit
In-Reply-To: <m363iki1ua.fsf@localhost.localdomain>
Hi,
On Sun, 8 Mar 2009, Jakub Narebski wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Known issues
>
> > - The Logitec QuickCam software can cause spurious crashes. See "Why does
> > make often crash creating a sh.exe.stackdump file when I try to compile
> > my source code?" in the MinGW FAQs
> > (http://www.mingw.org/MinGWiki/index.php/FAQ).
>
> You meant issue described here?
> http://www.mingw.org/wiki/Environment_issues
Exactly.
Fixed, committed and pushed.
Thanks,
Dscho
^ permalink raw reply
* [PATCH 2/2] fast-import: treat large blobs (> 100 MiB) specially, by deflating them on-the-fly from stdin instead of keeping an entire copy in memory.
From: Sam Hocevar @ 2009-03-08 18:40 UTC (permalink / raw)
To: git
Since deltas need no longer be computed for such files, fast-import is
now twice as fast and memory usage decreases more than threefold when
importing large files.
Signed-off-by: Sam Hocevar <sam@zoy.org>
---
I'd like to hear any suggestions on how to improve this patch. I am
not sure all the decisions I made by trying not to refactor too much
of the code were appropriate. If at least the idea is welcome, I will
also write a proper patch to make the data size threshold a config
option.
Here is a graph of memory usage against time for the current version
of fast-import and a patched version, when importing four 100 MiB files
filled with random data: http://zoy.org/~sam/git/git-faster-import.png
fast-import.c | 155 +++++++++++++++++++++++++++++++++++++++++----------------
1 files changed, 112 insertions(+), 43 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 6419d00..bdd40e7 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1044,12 +1044,14 @@ static int store_object(
struct strbuf *dat,
struct last_object *last,
unsigned char *sha1,
- uintmax_t mark)
+ uintmax_t mark,
+ int orig_bytes)
{
void *out, *delta;
struct object_entry *e;
unsigned char hdr[96];
unsigned long hdrlen, deltalen;
+ size_t outbytes;
z_stream s;
e = insert_object(sha1);
@@ -1066,7 +1068,9 @@ static int store_object(
return 1;
}
- if (last && last->data.buf && last->depth < max_depth) {
+ /* If orig_bytes is set, the object is already deflated and the
+ * caller does not want us to computes delta. */
+ if (!orig_bytes && last && last->data.buf && last->depth < max_depth) {
delta = diff_delta(last->data.buf, last->data.len,
dat->buf, dat->len,
&deltalen, 0);
@@ -1077,24 +1081,30 @@ static int store_object(
} else
delta = NULL;
- memset(&s, 0, sizeof(s));
- deflateInit(&s, pack_compression_level);
- if (delta) {
- s.next_in = delta;
- s.avail_in = deltalen;
+ if (!orig_bytes) {
+ memset(&s, 0, sizeof(s));
+ deflateInit(&s, pack_compression_level);
+ if (delta) {
+ s.next_in = delta;
+ s.avail_in = deltalen;
+ } else {
+ s.next_in = (void *)dat->buf;
+ s.avail_in = dat->len;
+ }
+ s.avail_out = deflateBound(&s, s.avail_in);
+ s.next_out = out = xmalloc(s.avail_out);
+ while (deflate(&s, Z_FINISH) == Z_OK)
+ /* nothing */;
+ deflateEnd(&s);
+ outbytes = s.total_out;
} else {
- s.next_in = (void *)dat->buf;
- s.avail_in = dat->len;
+ out = dat->buf;
+ outbytes = dat->len;
}
- s.avail_out = deflateBound(&s, s.avail_in);
- s.next_out = out = xmalloc(s.avail_out);
- while (deflate(&s, Z_FINISH) == Z_OK)
- /* nothing */;
- deflateEnd(&s);
/* Determine if we should auto-checkpoint. */
- if ((pack_size + 60 + s.total_out) > max_packsize
- || (pack_size + 60 + s.total_out) < pack_size) {
+ if ((pack_size + 60 + outbytes) > max_packsize
+ || (pack_size + 60 + outbytes) < pack_size) {
/* This new object needs to *not* have the current pack_id. */
e->pack_id = pack_id + 1;
@@ -1141,24 +1151,27 @@ static int store_object(
pack_size += sizeof(hdr) - pos;
} else {
e->depth = 0;
- hdrlen = encode_header(type, dat->len, hdr);
+ hdrlen = encode_header(type, orig_bytes ? orig_bytes
+ : dat->len, hdr);
write_or_die(pack_data->pack_fd, hdr, hdrlen);
pack_size += hdrlen;
}
- write_or_die(pack_data->pack_fd, out, s.total_out);
- pack_size += s.total_out;
+ write_or_die(pack_data->pack_fd, out, outbytes);
+ pack_size += outbytes;
- free(out);
free(delta);
- if (last) {
- if (last->no_swap) {
- last->data = *dat;
- } else {
- strbuf_swap(&last->data, dat);
+ if (!orig_bytes) {
+ free(out);
+ if (last) {
+ if (last->no_swap) {
+ last->data = *dat;
+ } else {
+ strbuf_swap(&last->data, dat);
+ }
+ last->offset = e->offset;
+ last->depth = e->depth;
}
- last->offset = e->offset;
- last->depth = e->depth;
}
return 0;
}
@@ -1343,7 +1356,7 @@ static void store_tree(struct tree_entry *root)
mktree(t, 1, &new_tree);
sha1_object(OBJ_TREE, &new_tree, root->versions[1].sha1);
- store_object(OBJ_TREE, &new_tree, &lo, root->versions[1].sha1, 0);
+ store_object(OBJ_TREE, &new_tree, &lo, root->versions[1].sha1, 0, 0);
t->delta_depth = lo.depth;
for (i = 0, j = 0, del = 0; i < t->entry_count; i++) {
@@ -1711,11 +1724,15 @@ static void parse_mark(void)
/* This actually parses a "data" command, with the addition that if sha1out
* is not NULL, it will also compute the sha1 on the fly. */
-static void parse_object_data(
+static int parse_object_data(
enum object_type type,
struct strbuf *sb,
- unsigned char *sha1out)
+ unsigned char *sha1out,
+ int candeflate)
{
+ int orig_bytes = 0;
+ size_t n = 0, length;
+
strbuf_reset(sb);
if (prefixcmp(command_buf.buf, "data "))
@@ -1737,14 +1754,63 @@ static void parse_object_data(
}
free(term);
- if(sha1out)
+ if (sha1out)
sha1_object(type, sb, sha1out);
}
- else {
- size_t n = 0, length;
+ /* TODO: make the hardcoded value a configuration option */
+ else if ((length = strtoul(command_buf.buf + 5, NULL, 10))
+ > 100 * 1024 * 1024
+ && candeflate) {
+ /* The incoming file is really big. As it is pretty unlikely
+ * it will give any interesting deltas, we immediately deflate
+ * it instead of storing the original data in memory. */
+ static struct strbuf tmp = STRBUF_INIT;
+ git_SHA_CTX c;
+ z_stream zs;
+
+ if (sha1out) {
+ unsigned char hdr[96];
+ unsigned long hdrlen;
+ hdrlen = sprintf((char*)hdr,"%s %lu", typename(type),
+ (unsigned long)length) + 1;
+ git_SHA1_Init(&c);
+ git_SHA1_Update(&c, hdr, hdrlen);
+ }
+
+ memset(&zs, 0, sizeof(zs));
+ deflateInit(&zs, pack_compression_level);
+ /* TODO: ideally, this should grow dynamically while we
+ * deflate the file. */
+ zs.avail_out = deflateBound(&zs, length);
+ strbuf_grow(sb, zs.avail_out);
+ zs.next_out = (unsigned char *)sb->buf;
- length = strtoul(command_buf.buf + 5, NULL, 10);
+ while (n < length) {
+ size_t s = strbuf_fread(&tmp, length - n < 4096 ?
+ length - n : 4096, stdin);
+ if (!s && feof(stdin))
+ die("EOF in data (%lu bytes remaining)",
+ (unsigned long)(length - n));
+ if (sha1out)
+ git_SHA1_Update(&c, tmp.buf, s);
+ zs.next_in = (unsigned char *)tmp.buf;
+ zs.avail_in = s;
+ while (deflate(&zs, Z_NO_FLUSH) == Z_OK)
+ /* nothing */;
+ strbuf_reset(&tmp);
+ n += s;
+ }
+ deflate(&zs, Z_FINISH);
+ deflateEnd(&zs);
+ strbuf_setlen(sb, zs.total_out);
+
+ if (sha1out)
+ git_SHA1_Final(sha1out, &c);
+
+ orig_bytes = length;
+ }
+ else {
while (n < length) {
size_t s = strbuf_fread(sb, length - n, stdin);
if (!s && feof(stdin))
@@ -1753,16 +1819,17 @@ static void parse_object_data(
n += s;
}
- if(sha1out)
+ if (sha1out)
sha1_object(type, sb, sha1out);
}
skip_optional_lf();
+ return orig_bytes;
}
static void parse_data(struct strbuf *sb)
{
- parse_object_data(OBJ_NONE, sb, NULL);
+ parse_object_data(OBJ_NONE, sb, NULL, 0);
}
static int validate_raw_date(const char *src, char *result, int maxlen)
@@ -1828,13 +1895,14 @@ static char *parse_ident(const char *buf)
static void parse_new_blob(void)
{
- unsigned char sha1[20];
static struct strbuf buf = STRBUF_INIT;
+ unsigned char sha1[20];
+ int orig_bytes;
read_next_command();
parse_mark();
- parse_object_data(OBJ_BLOB, &buf, sha1);
- store_object(OBJ_BLOB, &buf, &last_blob, sha1, next_mark);
+ orig_bytes = parse_object_data(OBJ_BLOB, &buf, sha1, 1);
+ store_object(OBJ_BLOB, &buf, &last_blob, sha1, next_mark, orig_bytes);
}
static void unload_one_branch(void)
@@ -1946,14 +2014,15 @@ static void file_change_m(struct branch *b)
*/
} else if (inline_data) {
static struct strbuf buf = STRBUF_INIT;
+ int orig_bytes;
if (p != uq.buf) {
strbuf_addstr(&uq, p);
p = uq.buf;
}
read_next_command();
- parse_object_data(OBJ_BLOB, &buf, sha1);
- store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0);
+ orig_bytes = parse_object_data(OBJ_BLOB, &buf, sha1, 1);
+ store_object(OBJ_BLOB, &buf, &last_blob, sha1, 0, orig_bytes);
} else if (oe) {
if (oe->type != OBJ_BLOB)
die("Not a blob (actually a %s): %s",
@@ -2236,7 +2305,7 @@ static void parse_new_commit(void)
free(committer);
sha1_object(OBJ_COMMIT, &new_data, b->sha1);
- if (!store_object(OBJ_COMMIT, &new_data, NULL, b->sha1, next_mark))
+ if (!store_object(OBJ_COMMIT, &new_data, NULL, b->sha1, next_mark, 0))
b->pack_id = pack_id;
b->last_commit = object_count_by_type[OBJ_COMMIT];
}
@@ -2317,7 +2386,7 @@ static void parse_new_tag(void)
free(tagger);
sha1_object(OBJ_TAG, &new_data, t->sha1);
- if (store_object(OBJ_TAG, &new_data, NULL, t->sha1, 0))
+ if (store_object(OBJ_TAG, &new_data, NULL, t->sha1, 0, 0))
t->pack_id = MAX_PACK_ID;
else
t->pack_id = pack_id;
--
1.6.2
^ permalink raw reply related
* Re: [PATCH 2/2] fast-import: treat large blobs (> 100 MiB) specially, by deflating them on-the-fly from stdin instead of keeping an entire copy in memory.
From: Sam Hocevar @ 2009-03-08 18:57 UTC (permalink / raw)
To: git
In-Reply-To: <20090308184057.GA9606@zoy.org>
On Sun, Mar 08, 2009, Sam Hocevar wrote:
> + int orig_bytes)
Self-comment: this variable should be size_t everywhere.
--
Sam.
^ permalink raw reply
* Re: [PATCH] http: add_fill_function checks if function has been added
From: Junio C Hamano @ 2009-03-08 19:38 UTC (permalink / raw)
To: Tay Ray Chuan; +Cc: git
In-Reply-To: <be6fef0d0903080327u551c0b4mcb86f2ba76473efc@mail.gmail.com>
Tay Ray Chuan <rctay89@gmail.com> writes:
> In the current use instances in http-push and http-walker,
> run_active_slot and finish_all_active_slots (which calls
> run_active_slot) will always be called, because they are the functions
> that actually start the http requests[1], and that means the fill
> functions will be called repeated, because
>
> run_active_slot, in its while loop, repeatedly calls
> step_active_slots calls
> fill_active_slots calls
> our fill functions.
That is not what I was worried about. There are two callsites to
add_fill_function().
http-push.c:2439: add_fill_function(NULL, fill_active_slot);
http-walker.c:936: add_fill_function(walker, (int (*)(void *)) fill_active_slot);
The caller in http-push.c always passes NULL as the callback data for the
fill_active_slot callback, which means that the callback function, as long
as it does not depend on the state other than the callback function
parameter (which is always NULL), will do the same thing whether you queue
it only once or as many times as add_fill_function() was called, which the
improvement your patch brings in.
But it is not immediately obvious in http-walker.c callchain if multiple
calls to add_fill_function(), if they were ever made, give the same
"walker" as the callback data. Your patch only looks at the function but
not the callback data for its filtering. Doesn't it mean that if a caller
made two calls to add_fill_function() with walker#1 and then walker#2 as
the callback data, you would discard the request for walker#2 and call the
callback function fill_active_slot() with walker#1 inside run_active_slot
repeatedly, without ever calling it for walker#2?
I do not know if that actually happens in the current http-walker.c
codepath, or the caller _happens to_ call the add_fill_function() only
once and making my worry a non-issue, but as a patch submitter, you would
know the answer to the question.
If it is the former, then your change is introducing a bug. If it is the
latter, your change won't break anything immediately, but still introduces
a potential bug waiting to happen, as the API looks as if you can call
add_fill_function() to ask for callback functions to be called with
different callback data and the caller can rely on both of them to be
called at appropriate times, but in reality that guarantee does not hold.
^ permalink raw reply
* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Junio C Hamano @ 2009-03-08 19:45 UTC (permalink / raw)
To: Jeff King; +Cc: git, Chris Johnsen, Miklos Vajna
In-Reply-To: <20090308144240.GA30794@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> 1. Declare appended newline a forbidden style, fix all existing cases
> in the test suite, and be on the lookout for new ones.
>
> The biggest problem with this option is that we have no automated
> way of policing. Such tests will just silently pass on the broken
> platform.
>
> 2. Have test_run_ canonicalize the snippet by removing trailing
> newlines.
>
> 3. Declare FreeBSD's /bin/sh unfit for git consumption, and require
> bash for the test suite.
>
> I think (2) is the most reasonable option of those choices.
>
> We could also try to convince FreeBSD that it's a bug, but that doesn't
> change the fact that the tests are broken on every existing version.
If this part from your analysis is true for a shell:
> eval 'false
>
> '
> echo status is $?
>
> generates:
> ...
> status is 0
I would be very tempted to declare that shell is unfit for any serious
use, not just for test suite. Removing the empty line at the end of a
scriptlet that such a broken shell misinterprets as an empty command
that is equivalent to ":" (or "true") might hide breakages in the test
suite, but
(1) eval "$string" is used outside of test suite, most notably "am" and
"bisect". I think "am"'s use is safe, but I wouldn't be surprised if
the scriptlet "bisect" internally creates has empty lines if only for
debuggability; and more importantly
(2) who knows what _other_ things may be broken in such a shell?
^ permalink raw reply
* [PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin
From: Brian Gernhardt @ 2009-03-08 20:04 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
Not all OSes use st_ctim and st_mtim in their struct stat. In
particular, it appears that OS X uses st_*timespec instead. So add a
Makefile variable and #define called USE_ST_TIMESPEC to switch the
USE_NSEC defines to use st_*timespec.
This also turns it on by default for OS X (Darwin) machines. Likely
this is a sane default for other BSD kernels as well, but I don't have
any to test that assumption on.
Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
This is on top of "next".
Now time to go debug a Bus Error in git-grep that made this hard to find.
Makefile | 7 +++++++
git-compat-util.h | 5 +++++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/Makefile b/Makefile
index 9a23aa5..4bdaad7 100644
--- a/Makefile
+++ b/Makefile
@@ -126,6 +126,9 @@ all::
# randomly break unless your underlying filesystem supports those sub-second
# times (my ext3 doesn't).
#
+# Define USE_ST_TIMESPEC if your "struct stat" uses "st_ctimespec" instead of
+# "st_ctim"
+#
# Define NO_NSEC if your "struct stat" does not have "st_ctim.tv_nsec"
# available. This automatically turns USE_NSEC off.
#
@@ -660,6 +663,7 @@ ifeq ($(uname_S),Darwin)
endif
NO_MEMMEM = YesPlease
THREADED_DELTA_SEARCH = YesPlease
+ USE_ST_TIMESPEC = YesPlease
endif
ifeq ($(uname_S),SunOS)
NEEDS_SOCKET = YesPlease
@@ -925,6 +929,9 @@ endif
ifdef NO_ST_BLOCKS_IN_STRUCT_STAT
BASIC_CFLAGS += -DNO_ST_BLOCKS_IN_STRUCT_STAT
endif
+ifdef USE_ST_TIMESPEC
+ BASIC_CFLAGS += -DUSE_ST_TIMESPEC
+endif
ifdef NO_NSEC
BASIC_CFLAGS += -DNO_NSEC
endif
diff --git a/git-compat-util.h b/git-compat-util.h
index 83d8389..1906253 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -393,8 +393,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,
#define ST_CTIME_NSEC(st) 0
#define ST_MTIME_NSEC(st) 0
#else
+#ifdef USE_ST_TIMESPEC
+#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))
+#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))
+#else
#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
#endif
+#endif
#endif
--
1.6.2.221.g2411c.dirty
^ permalink raw reply related
* [EGIT PATCH] Show diff when double-clicking on file in commit dialog
From: Robin Stocker @ 2009-03-08 20:09 UTC (permalink / raw)
To: Shawn O. Pearce, Robin Rosenberg; +Cc: git
It only compares the index version to the working tree version for now.
So if the file was already added to the index, the diff is empty. What
it should show is the diff that will be in the commit.
Signed-off-by: Robin Stocker <robin@nibor.org>
---
Hi,
An essential feature I miss in EGit at the moment (apart from the
synchronize view [1]) is seeing what changes one is about to commit. In
the Subclipse SVN plugin one can double-click a file in the commit
dialog and the diff is shown.
This patch is a first step for adding this to EGit. It only compares the
index version to the working tree version as I couldn't figure out an
easy way to get the HEAD version.
It's more a proof of concept than a final patch. What do you think?
[1] Someone is working on the synchronize view:
http://github.com/yanns/egit/commits/compare-with-index
-- Robin, a different one :)
.../egit/ui/internal/dialogs/CommitDialog.java | 40 ++++++++++++++++++++
1 files changed, 40 insertions(+), 0 deletions(-)
diff --git a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
index 8b7fe45..6999e87 100644
--- a/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
+++ b/org.spearce.egit.ui/src/org/spearce/egit/ui/internal/dialogs/CommitDialog.java
@@ -18,6 +18,8 @@
import java.util.Comparator;
import java.util.Iterator;
+import org.eclipse.compare.CompareUI;
+import org.eclipse.compare.ITypedElement;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.jface.dialogs.Dialog;
@@ -52,9 +54,17 @@
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableColumn;
import org.eclipse.swt.widgets.Text;
+import org.eclipse.team.core.RepositoryProvider;
+import org.eclipse.team.core.history.IFileHistory;
+import org.eclipse.team.core.history.IFileRevision;
+import org.eclipse.team.internal.ui.history.FileRevisionTypedElement;
import org.eclipse.ui.model.WorkbenchLabelProvider;
+import org.spearce.egit.core.GitProvider;
+import org.spearce.egit.core.internal.storage.GitFileHistoryProvider;
+import org.spearce.egit.core.internal.storage.GitFileRevision;
import org.spearce.egit.core.project.RepositoryMapping;
import org.spearce.egit.ui.UIText;
+import org.spearce.egit.ui.internal.GitCompareFileRevisionEditorInput;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.GitIndex;
import org.spearce.jgit.lib.PersonIdent;
@@ -262,6 +272,8 @@ public void modifyText(ModifyEvent e) {
resourcesTable.setLayoutData(GridDataFactory.fillDefaults().hint(600,
200).span(2,1).grab(true, true).create());
+ resourcesTable.addSelectionListener(new CommitItemSelectionListener());
+
resourcesTable.setHeaderVisible(true);
TableColumn statCol = new TableColumn(resourcesTable, SWT.LEFT);
statCol.setText(UIText.CommitDialog_Status);
@@ -505,6 +517,34 @@ public void widgetSelected(SelectionEvent e) {
}
+ class CommitItemSelectionListener extends SelectionAdapter {
+
+ public void widgetDefaultSelected(SelectionEvent e) {
+ IStructuredSelection selection = (IStructuredSelection) filesViewer.getSelection();
+
+ CommitItem commitItem = (CommitItem) selection.getFirstElement();
+ if (commitItem == null) {
+ return;
+ }
+
+ IProject project = commitItem.file.getProject();
+ GitProvider provider = (GitProvider) RepositoryProvider.getProvider(project);
+ GitFileHistoryProvider fileHistoryProvider = (GitFileHistoryProvider) provider.getFileHistoryProvider();
+
+ IFileHistory fileHistory = fileHistoryProvider.getFileHistoryFor(commitItem.file, 0, null);
+
+ IFileRevision baseFile = fileHistory.getFileRevision(GitFileRevision.INDEX);
+ IFileRevision nextFile = fileHistoryProvider.getWorkspaceFileRevision(commitItem.file);
+
+ ITypedElement base = new FileRevisionTypedElement(baseFile);
+ ITypedElement next = new FileRevisionTypedElement(nextFile);
+
+ GitCompareFileRevisionEditorInput input = new GitCompareFileRevisionEditorInput(base, next, null);
+ CompareUI.openCompareDialog(input);
+ }
+
+ }
+
@Override
protected void okPressed() {
commitMessage = commitText.getText();
--
1.6.1.2
^ permalink raw reply related
* Re: [PATCH 4/4] archive: use parseopt for local-only options
From: Junio C Hamano @ 2009-03-08 20:20 UTC (permalink / raw)
To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit
In-Reply-To: <49B40CC1.2090906@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> The parseopt parser allows multiple occurrences of --remote and --exec
> unlike the previous one; the one specified last wins. This looseness
> is acceptable, I think.
I agree.
If we care about this very deeply, we can add "this option is supposed to
be singleton" option to the parse_options infrastructure. Besides, the
"last one wins" rule is often more convenient than "there has to be at
most one" rule in real life.
^ permalink raw reply
* Re: [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN
From: Junio C Hamano @ 2009-03-08 20:24 UTC (permalink / raw)
To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit
In-Reply-To: <49B40A9F.7050408@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Add a parseopt flag, PARSE_OPT_KEEP_UNKNOWN, that can be used to keep
> unknown options in argv, similar to the existing KEEP flags.
Very nice.
The only caveat I can think of is with PARSE_OPT_STOP_AT_NON_OPTION set
(which is not default), you can correctly handle:
git cmd --known --unknown=value arg0 arg1
but cannot correctly handle:
git cmd --known --unknown value arg0 arg1
An update to Documentation/technical/api-parse-options.txt that
(1) describes this new option; and
(2) warns about this issue.
It might even make sense to diagnose PARSE_OPT_STOP_AT_NON_OPTION used
together with PARSE_OPT_KEEP_UNKNOWN as a programming error.
^ permalink raw reply
* Re: [PATCH 3/4] parseopt: make usage optional
From: Junio C Hamano @ 2009-03-08 20:25 UTC (permalink / raw)
To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit
In-Reply-To: <49B40B9A.8080202@lsrfire.ath.cx>
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> Allow usagestr to be NULL and don't display anything a help screen in
> this case. This is useful to implement incremental parsers.
Can I amend "s/anything a/any/"?
^ permalink raw reply
* Re: [PATCH 1/4] parseopt: add PARSE_OPT_KEEP_UNKNOWN
From: Junio C Hamano @ 2009-03-08 20:30 UTC (permalink / raw)
To: René Scharfe; +Cc: git, carlos.duclos, Pierre Habouzit
In-Reply-To: <7vvdqjkbka.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>
>> Add a parseopt flag, PARSE_OPT_KEEP_UNKNOWN, that can be used to keep
>> unknown options in argv, similar to the existing KEEP flags.
>
> Very nice.
>
> The only caveat I can think of is with PARSE_OPT_STOP_AT_NON_OPTION set
> (which is not default), you can correctly handle:
>
> git cmd --known --unknown=value arg0 arg1
>
> but cannot correctly handle:
>
> git cmd --known --unknown value arg0 arg1
>
> An update to Documentation/technical/api-parse-options.txt that
>
> (1) describes this new option; and
>
> (2) warns about this issue.
"is necessary" is necessary here to complete my sentence. Sorry.
> It might even make sense to diagnose PARSE_OPT_STOP_AT_NON_OPTION used
> together with PARSE_OPT_KEEP_UNKNOWN as a programming error.
^ permalink raw reply
* Re: [PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin
From: Junio C Hamano @ 2009-03-08 20:51 UTC (permalink / raw)
To: Brian Gernhardt; +Cc: Git List
In-Reply-To: <1236542668-83333-1-git-send-email-benji@silverinsanity.com>
Brian Gernhardt <benji@silverinsanity.com> writes:
> This also turns it on by default for OS X (Darwin) machines. Likely
> this is a sane default for other BSD kernels as well, but I don't have
> any to test that assumption on.
Yeah, that was my initial reaction. Any BSDers?
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 83d8389..1906253 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -393,8 +393,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,
> #define ST_CTIME_NSEC(st) 0
> #define ST_MTIME_NSEC(st) 0
> #else
> +#ifdef USE_ST_TIMESPEC
> +#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))
> +#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))
> +#else
> #define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
> #define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
> #endif
> +#endif
Thanks.
I think this patch moves things in the right direction, but there are
other uses of "st_[cm]tim.tv_nsec" that do not use the ST_[CM]TIME_NSEC
macro.
$ git grep -n -e 'st_[cm]tim\.' --cached -- '*.[ch]'
builtin-fetch-pack.c:810: || st.st_mtim.tv_nsec != mtime.nsec
git-compat-util.h:396:#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))
git-compat-util.h:397:#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))
read-cache.c:207: if (ce->ce_mtime.nsec != (unsigned int)st->st_mtim.tv_nsec)
read-cache.c:209: if (trust_ctime && ce->ce_ctime.nsec != (unsigned int)st->st_ctim.tv_nsec)
Probably we should apply the following patch as a fix, and then apply your
enhancement to support st_[cm]timespec systems?
builtin-fetch-pack.c | 2 +-
read-cache.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 59b0b0a..1d7e023 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -807,7 +807,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
die("shallow file was removed during fetch");
} else if (st.st_mtime != mtime.sec
#ifdef USE_NSEC
- || st.st_mtim.tv_nsec != mtime.nsec
+ || ST_CTIME_NSEC(st) != mtime.nsec
#endif
)
die("shallow file was changed during fetch");
diff --git a/read-cache.c b/read-cache.c
index b819abb..7f74c8d 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -204,9 +204,9 @@ static int ce_match_stat_basic(struct cache_entry *ce, struct stat *st)
changed |= CTIME_CHANGED;
#ifdef USE_NSEC
- if (ce->ce_mtime.nsec != (unsigned int)st->st_mtim.tv_nsec)
+ if (ce->ce_mtime.nsec != ST_MTIME_NSEC(*st))
changed |= MTIME_CHANGED;
- if (trust_ctime && ce->ce_ctime.nsec != (unsigned int)st->st_ctim.tv_nsec)
+ if (trust_ctime && ce->ce_ctime.nsec != ST_CTIME_NSEC(*st))
changed |= CTIME_CHANGED;
#endif
^ permalink raw reply related
* Re: git-svn and repository hierarchy?
From: Florian Mickler @ 2009-03-08 20:33 UTC (permalink / raw)
To: Josef Wolf; +Cc: git, Peter Harris
In-Reply-To: <20090306161026.GA14554@raven.wolf.lan>
[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]
On Fri, 6 Mar 2009 17:10:26 +0100
Josef Wolf <jw@raven.inka.de> wrote:
> On Thu, Mar 05, 2009 at 02:48:14PM -0500, Peter Harris wrote:
> > On Thu, Mar 5, 2009 at 1:05 PM, Josef Wolf wrote:
> > >
> > > Well, actually it allows the changes for a very limited user
> > > group (that is: only me 8-). While I agree that author/date
> > > should not be changed, I like to be able to fix silly typos in
> > > the log. After all, we all do typos now and then ;-)
> >
> > True, but in my experience it happens considerably less often with
> > git. I find and fix most of my typos when reviewing my change-set
> > before doing a "git push" or "git svn dcommit".
>
> So you are rewriting yourself but not accept rewrites by svn ;-)
the thing is: with git you don't ''rewrite history''.
you create a completely new history. that is because the
sha1-descriptions includes the meta-data.
that means, even if you want, you can't change ''published'' history.
because the history is unique'ly identified by the topmost sha-1.
if smth changes underneath the topmost sha-1 you have to rebase all
your other changes on the new sha-1 and thus altering them.
that is why the dcommitt'ed&svn-rebased changes have different sha-1s
and all your clone's work needs to be rebased onto the newly altered
committs.
Sincerely,
Florian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH/RFD] builtin-revert.c: release index lock when cherry-picking an empty commit
From: Chris Johnsen @ 2009-03-08 21:09 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Miklos Vajna
In-Reply-To: <7v63ikmz11.fsf@gitster.siamese.dyndns.org>
On 2009 Mar 7, at 22:14, Junio C Hamano wrote:
>> UNEVEN TREATMENT OF EMPTY CHANGES
>>
>> fetch, push, bundle
> They just transfer an existing history from one place to another
> without
> modifying, so it is unfortunately true that they preserve such a
> broken
> history with empty commits.
>> 'format-patch', 'send-email', 'apply', 'am', 'rebase' (automatic,
>> non-fast-forward; and --interactive).
>
> These are all about creating history afresh, and they actively
> discourage
> empty commits to be (re)created.
>
> There is no "uneven treatment".
>> 36863af16e (git-commit --allow-empty) says "This is primarily for
>> use by foreign scm interface scripts.". Is this the only case
>> where empty commits _should_ be used?
>
> If foreign scm recorded an empty commit, it would be nice to be
> able to
> recreate such a broken history _if the user wanted to_, and that is
> where
> the --allow-empty option can be used.
Thank you for the clarification. I will explain the source of my
confusion.
The current documentation "Usually recording [an empty commit] is a
mistake... This option ... is primarily for use by foreign scm
interface scripts." implied to me that there was room outside foreign
scm interface for "normal" use of empty commits.
My confusion was that I took "usually a mistake" to refer to the case
where the user meant to commit content changes but forgot to first
stage any changed content. But your clarification shows that "usually
a mistake" really means that making any empty commit, intentional or
not, is (considered to be) a fundamental misuse of SCM machinery.
Would it be acceptable to strengthen the language in the
documentation for --allow-empty? Possibly something like the
following paragraph:
Empty commits are a broken concept and should never be made during
normal usage. By default, the command prevents you
from making such a commit. This option bypasses the safety, and is
primarily for use by foreign scm interface scripts.
If such a change is acceptable, I will send a patch.
--
Chris
^ permalink raw reply
* Re: [PATCH 2/2] ls-files: fix broken --no-empty-directory
From: Miklos Vajna @ 2009-03-08 21:13 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, git
In-Reply-To: <20090308012722.GB18714@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 831 bytes --]
On Sat, Mar 07, 2009 at 08:27:22PM -0500, Jeff King <peff@peff.net> wrote:
> diff --git a/builtin-ls-files.c b/builtin-ls-files.c
> index 1742c0f..437c366 100644
> --- a/builtin-ls-files.c
> +++ b/builtin-ls-files.c
> @@ -454,8 +454,8 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
> OPT_BIT(0, "directory", &dir.flags,
> "show 'other' directories' name only",
> DIR_SHOW_OTHER_DIRECTORIES),
> - OPT_BIT(0, "empty-directory", &dir.flags,
> - "list empty directories",
> + OPT_BIT(0, "no-empty-directory", &dir.flags,
> + "don't show empty directories",
> DIR_HIDE_EMPTY_DIRECTORIES),
> OPT_BOOLEAN('u', "unmerged", &show_unmerged,
> "show unmerged files in the output"),
Thanks for catching this. But then why not using PARSE_OPT_NONEG?
That would avoid --no-no-empty-directory.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] Create USE_ST_TIMESPEC and turn it on for Darwin
From: Brian Gernhardt @ 2009-03-08 21:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <7vhc23kaay.fsf@gitster.siamese.dyndns.org>
On Mar 8, 2009, at 4:51 PM, Junio C Hamano wrote:
> I think this patch moves things in the right direction, but there are
> other uses of "st_[cm]tim.tv_nsec" that do not use the
> ST_[CM]TIME_NSEC
> macro.
>
> $ git grep -n -e 'st_[cm]tim\.' --cached -- '*.[ch]'
> builtin-fetch-pack.c:810: || st.st_mtim.tv_nsec != mtime.nsec
> git-compat-util.h:396:#define ST_CTIME_NSEC(st) ((unsigned int)
> ((st).st_ctim.tv_nsec))
> git-compat-util.h:397:#define ST_MTIME_NSEC(st) ((unsigned int)
> ((st).st_mtim.tv_nsec))
> read-cache.c:207: if (ce->ce_mtime.nsec != (unsigned int)st-
> >st_mtim.tv_nsec)
> read-cache.c:209: if (trust_ctime && ce->ce_ctime.nsec != (unsigned
> int)st->st_ctim.tv_nsec)
Interesting. I couldn't use git-grep due to other problems, but
thought any other tim/timespec issues would have stopped my
compilation. Looking at the code, this is because everything other
than the #defines in git-compat-util.h are surrounded by USE_NSEC
which is not defined on my machine.
However, I noticed another breakage entirely. Namely, that USE_NSEC
is not defined anywhere. There's a comment in the Makefile that says
"define USE_NSEC below", but there is no code that checks for USE_NSEC
and sets the appropriate compiler switch. Trivial patch to follow.
> Probably we should apply the following patch as a fix, and then
> apply your
> enhancement to support st_[cm]timespec systems?
Your patch looks sane to me.
~~ Brian
^ 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