* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Nicolas Pitre @ 2009-08-13 17:23 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, git
In-Reply-To: <alpine.LFD.2.01.0908130934400.28882@localhost.localdomain>
On Thu, 13 Aug 2009, Linus Torvalds wrote:
>
>
> On Thu, 13 Aug 2009, Nicolas Pitre wrote:
> >
> > In addition to X86, PowerPC and S390 are capable of unaligned memory
> > accesses.
> >
> > Signed-off-by: Nicolas Pitre <nico@cam.org>
>
> Ack on all your patches (1-3 + this). Looks fine to me.
>
> I do wonder if we should try to do basically "per-architecture hack
> header-files", and then have for each architecture a small trivial
> 'hack-x86.h' kind of thing that just does
>
> /* x86 hacks */
> #define get_be32(p) ntohl(*(unsigned int *)(p))
> #define put_be32(p, v) do { *(unsigned int *)(p) = htonl(v); } while (0)
> #define setW(x, val) (*(volatile unsigned int *)&W(x) = (val))
>
> #define SHA_ASM(op, x, n) ({ unsigned int __res; __asm__(op " %1,%0":"=r" (__res):"i" (n), "0" (x)); __res; })
> #define SHA_ROL(x,n) SHA_ASM("rol", x, n)
> #define SHA_ROR(x,n) SHA_ASM("ror", x, n)
>
> and then we'd have each architecture separated out. Add a few
> "generic helpers":
>
> - be32-generic.h:
>
> #define get_be32(p) ( \
> (*((unsigned char *)(p) + 0) << 24) | \
> (*((unsigned char *)(p) + 1) << 16) | \
> (*((unsigned char *)(p) + 2) << 8) | \
> (*((unsigned char *)(p) + 3) << 0) )
>
> #define put_be32(p, v) do { \
> unsigned int __v = (v); \
> *((unsigned char *)(p) + 0) = __v >> 24; \
> *((unsigned char *)(p) + 1) = __v >> 16; \
> *((unsigned char *)(p) + 2) = __v >> 8; \
> *((unsigned char *)(p) + 3) = __v >> 0; } while (0)
>
> - rotate-generic.h:
>
> #define SHA_ROT(X,l,r) (((X) << (l)) | ((X) >> (r)))
> #define SHA_ROL(X,n) SHA_ROT(X,n,32-(n))
> #define SHA_ROR(X,n) SHA_ROT(X,32-(n),n)
>
> that architectures could use when they want to use some particular
> portable version. Then, add a final "hack-generic.h" with the fallback
> cases that just does
>
> #include "be32-generic.h"
> #include "rotate-generic.h"
> #define setW(x,val) (W(x) = (val))
>
> and you'd have all the hacks separated out and fairly easily used by
> different architectures.. (ie the ARM version would just look like
>
> - hack-arm.h:
>
> #include "be32-generic.h"
> #include "rotate-generic.h"
> #define setW(x, val) do { W(x) = (val); __asm__("":::"memory"); } while (0)
>
> and you'd be all done.
>
> Hmm? I don't know if this kind of generalization is strictly needed, so
> I'm just throwing it out as an idea.
Well... first I don't think there are much else we can do with that
code, i.e. there is probably not so many more hacks to add if any. With
my last patch I consider this ready for general consumption.
And given that the only user is this very sha1 implementation then there
is not much value in abstracting them in header files.
And the point of patch 2/3 was to move hack variants for the same
purpose together making it easier to document and understand (and
possibly modify) them. Your suggestion would go in the opposite
direction entirely.
As it is now, I was about to suggest:
git mv block-sha1/sha1.[ch] .
rmdir block-sha1
rm -r mozilla-sha1
rm -r arm
rm -r ppc
and remove support for openssl's SHA1 usage, making this implementation
unconditional. After all it is faster, or so close to be faster than
the alternatives, that we should probably cut on the extra dependency
and simplify portability issues at the same time.
Nicolas
^ permalink raw reply
* Re: [PATCH 2/4] fast-import: define a new option command
From: Shawn O. Pearce @ 2009-08-13 17:25 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <fabb9a1e0908131009j51c54cacp3f837f9b8525061@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Thu, Aug 13, 2009 at 10:07, Johannes
> Schindelin<Johannes.Schindelin@gmx.de> wrote:
> > ... and will import the marks twice?
>
> Ah, you're right :(. What's the best way to do this? Should we dump
> any previous marks when importing new ones?
Uh, well, yes. We shouldn't define :5 if it was in the file that
appeared in the stream, but isn't in the file on the command line.
Worse, what happens if we do this:
echo "option import-marks=/not/found" \
| git fast-import --import-marks=my.marks
I want this to work, even though /not/found does not exist, but
my.marks does. So that does complicate things...
--
Shawn.
^ permalink raw reply
* Re: [PATCH 2/4] fast-import: define a new option command
From: Sverre Rabbelier @ 2009-08-13 17:28 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <20090813172508.GO1033@spearce.org>
Heya,
On Thu, Aug 13, 2009 at 10:25, Shawn O. Pearce<spearce@spearce.org> wrote:
> I want this to work, even though /not/found does not exist, but
> my.marks does. So that does complicate things...
Should we pass an option to parse_marks to make it ignore a
non-existing file, and set that option when parsing the stream
commands?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: git gc expanding packed data?
From: Hin-Tak Leung @ 2009-08-13 17:31 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.2.00.0908121118420.10633@xanadu.home>
On Wed, Aug 12, 2009 at 4:35 PM, Nicolas Pitre<nico@cam.org> wrote:
> On Wed, 12 Aug 2009, Hin-Tak Leung wrote:
>
>> On Tue, Aug 11, 2009 at 10:33 PM, Nicolas Pitre<nico@cam.org> wrote:
>> <snipped>
>>
>> > From git v1.6.3 the --aggressive switch makes for 'git repack' to be
>> > called with --window=250 --depth=250, meaning the equivalent of:
>> >
>> > git repack -a -d -f --window=250 --depth=250
>> >
>> > Do you still get a huge pack with the above?
>> >
>> >> I guess --aggressive doesn't always save space...
>> >
>> > If so that is (and was) a bug.
>>
>> I tried 'git repack -a -d -f --window=250 --depth=250' with 1.6.2.5
>> (fc11.x86_64) and it took half a day, swallowed up all the memory -
>> 3GB virtual & 1.3GB resident - and finally the kernel oom killer
>> killed it at a last message of (601460/957910). Left no temp files.
>> Would git 1.6.3 use less memory? :-(
>
> Probably not. However you should try:
>
> git config pack.deltaCacheSize 1
>
> That limits the delta cache size to one byte (effectively disabling it)
> instead of the default of 0 which means unlimited. With that I'm able
> to repack that repository using the above git repack command on an
> x86-64 system with 4GB of RAM and using 4 threads (this is a quad core).
> Resident memory usage grows to nearly 3.3GB though.
>
> If your machine is SMP and you don't have sufficient RAM then you can
> reduce the number of threads to only one:
>
> git config pack.threads 1
>
> Additionally, you can further limit memory usage with the
> --window-memory argument to 'git repack'. For example, using
> --window-memory=128M should keep a reasonable upper bound on the delta
> search memory usage although this can result in less optimal delta match
> if the repo contains lots of large files (and I think this is the case
> for the gcc repo).
>
>
> Nicolas
>
Thanks. I used the two git config pack.* commands, and
git repack -a -d -f --window=250 --depth=250
finished after 8 hours (dual core Turion, 2GB RAM + 2GB swap). The
pack directory went from 457MB to 308MB.
Thanks a lot for the advice - learned a few interesting things about
git on the way :-).
Hin-Tak
^ permalink raw reply
* Re: rebase-with-history -- a technique for rebasing without trashing your repo history
From: Bryan O'Sullivan @ 2009-08-13 17:39 UTC (permalink / raw)
To: Michael Haggerty; +Cc: Bazaar, mercurial mailing list, Git Mailing List
In-Reply-To: <4A840B0F.9060003@alum.mit.edu>
[-- Attachment #1.1: Type: text/plain, Size: 253 bytes --]
On Thu, Aug 13, 2009 at 5:46 AM, Michael Haggerty <mhagger@alum.mit.edu>wrote:
> Sorry to cross-post, but I think this might be interesting to all three
> projects...
>
Please do not cross post, no matter how interesting you think the topic
might be.
[-- Attachment #1.2: Type: text/html, Size: 526 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] fast-import: define a new option command
From: Shawn O. Pearce @ 2009-08-13 17:41 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <fabb9a1e0908131028t438509d2m180293ca95daad74@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Thu, Aug 13, 2009 at 10:25, Shawn O. Pearce<spearce@spearce.org> wrote:
> > I want this to work, even though /not/found does not exist, but
> > my.marks does. ?So that does complicate things...
>
> Should we pass an option to parse_marks to make it ignore a
> non-existing file, and set that option when parsing the stream
> commands?
Uh, no, if we have "option import-marks=..." and we can't find the
file "..." and we have no --import-marks command line flag that
would have overridden it, we need to abort with an error.
--
Shawn.
^ permalink raw reply
* Re: [PATCH 2/4] fast-import: define a new option command
From: Sverre Rabbelier @ 2009-08-13 17:44 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <20090813174119.GP1033@spearce.org>
Heya,
On Thu, Aug 13, 2009 at 10:41, Shawn O. Pearce<spearce@spearce.org> wrote:
> Uh, no, if we have "option import-marks=..." and we can't find the
> file "..." and we have no --import-marks command line flag that
> would have overridden it, we need to abort with an error.
Ah, then how about in option_import_marks() we only store the name of
the file, like in option_export_marks, and at the end, when we reach
the first non-option command (and we've parsed argv), we read the
file. That way it's only read once, and it deals with the above
scenario.
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH 2/4] fast-import: define a new option command
From: Shawn O. Pearce @ 2009-08-13 17:52 UTC (permalink / raw)
To: Sverre Rabbelier; +Cc: Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <fabb9a1e0908131044g583f126dm6a3818b4b295eaf5@mail.gmail.com>
Sverre Rabbelier <srabbelier@gmail.com> wrote:
> On Thu, Aug 13, 2009 at 10:41, Shawn O. Pearce<spearce@spearce.org> wrote:
> > Uh, no, if we have "option import-marks=..." and we can't find the
> > file "..." and we have no --import-marks command line flag that
> > would have overridden it, we need to abort with an error.
>
> Ah, then how about in option_import_marks() we only store the name of
> the file, like in option_export_marks, and at the end, when we reach
> the first non-option command (and we've parsed argv), we read the
> file. That way it's only read once, and it deals with the above
> scenario.
That's better. :-)
--
Shawn.
^ permalink raw reply
* [PATCH v3 0/3] fast-import: add a new option command
From: Sverre Rabbelier @ 2009-08-13 19:02 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
Now we delay reading the marks file till option parsing is done.
Sverre Rabbelier (3):
fast-import: put option parsing code in seperate functions
fast-import: add option command
fast-import: test the new option command
Documentation/git-fast-import.txt | 23 ++++
fast-import.c | 235 +++++++++++++++++++++++++------------
t/t9300-fast-import.sh | 58 +++++++++
3 files changed, 239 insertions(+), 77 deletions(-)
^ permalink raw reply
* [PATCH v3 1/3] fast-import: put option parsing code in seperate functions
From: Sverre Rabbelier @ 2009-08-13 19:02 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
Cc: Sverre Rabbelier
In-Reply-To: <1250190156-4752-1-git-send-email-srabbelier@gmail.com>
Putting the options in their own functions increases readability of
the option parsing block and makes it easier to reuse the option
parsing code later on.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Unchanged from v2.
fast-import.c | 115 +++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 75 insertions(+), 40 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 7ef9865..b904f20 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -291,6 +291,7 @@ static unsigned long branch_count;
static unsigned long branch_load_count;
static int failure;
static FILE *pack_edges;
+static unsigned int show_stats = 1;
/* Memory pools */
static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -2337,7 +2338,7 @@ static void parse_progress(void)
skip_optional_lf();
}
-static void import_marks(const char *input_file)
+static void option_import_marks(const char *input_file)
{
char line[512];
FILE *f = fopen(input_file, "r");
@@ -2372,6 +2373,76 @@ static void import_marks(const char *input_file)
fclose(f);
}
+static void option_date_format(const char *fmt)
+{
+ if (!strcmp(fmt, "raw"))
+ whenspec = WHENSPEC_RAW;
+ else if (!strcmp(fmt, "rfc2822"))
+ whenspec = WHENSPEC_RFC2822;
+ else if (!strcmp(fmt, "now"))
+ whenspec = WHENSPEC_NOW;
+ else
+ die("unknown --date-format argument %s", fmt);
+}
+
+static void option_max_pack_size(const char *packsize)
+{
+ max_packsize = strtoumax(packsize, NULL, 0) * 1024 * 1024;
+}
+
+static void option_depth(const char *depth)
+{
+ max_depth = strtoul(depth, NULL, 0);
+ if (max_depth > MAX_DEPTH)
+ die("--depth cannot exceed %u", MAX_DEPTH);
+}
+
+static void option_active_branches(const char *branches)
+{
+ max_active_branches = strtoul(branches, NULL, 0);
+}
+
+static void option_export_marks(const char *marks)
+{
+ mark_file = xstrdup(marks);
+}
+
+static void option_export_pack_edges(const char *edges)
+{
+ if (pack_edges)
+ fclose(pack_edges);
+ pack_edges = fopen(edges, "a");
+ if (!pack_edges)
+ die_errno("Cannot open '%s'", edges);
+}
+
+static void parse_one_option(const char *option)
+{
+ if (!prefixcmp(option, "date-format=")) {
+ option_date_format(option + 12);
+ } else if (!prefixcmp(option, "max-pack-size=")) {
+ option_max_pack_size(option + 14);
+ } else if (!prefixcmp(option, "depth=")) {
+ option_depth(option + 6);
+ } else if (!prefixcmp(option, "active-branches=")) {
+ option_active_branches(option + 16);
+ } else if (!prefixcmp(option, "import-marks=")) {
+ option_import_marks(option + 13);
+ } else if (!prefixcmp(option, "export-marks=")) {
+ option_export_marks(option + 13);
+ } else if (!prefixcmp(option, "export-pack-edges=")) {
+ option_export_pack_edges(option + 18);
+ } else if (!prefixcmp(option, "force")) {
+ force_update = 1;
+ } else if (!prefixcmp(option, "quiet")) {
+ show_stats = 0;
+ } else if (!prefixcmp(option, "stats")) {
+ show_stats = 1;
+ } else {
+ die("Unsupported option: %s", option);
+ }
+}
+
static int git_pack_config(const char *k, const char *v, void *cb)
{
if (!strcmp(k, "pack.depth")) {
@@ -2398,7 +2469,7 @@ static const char fast_import_usage[] =
int main(int argc, const char **argv)
{
- unsigned int i, show_stats = 1;
+ unsigned int i;
git_extract_argv0_path(argv[0]);
@@ -2419,44 +2490,8 @@ int main(int argc, const char **argv)
if (*a != '-' || !strcmp(a, "--"))
break;
- else if (!prefixcmp(a, "--date-format=")) {
- const char *fmt = a + 14;
- if (!strcmp(fmt, "raw"))
- whenspec = WHENSPEC_RAW;
- else if (!strcmp(fmt, "rfc2822"))
- whenspec = WHENSPEC_RFC2822;
- else if (!strcmp(fmt, "now"))
- whenspec = WHENSPEC_NOW;
- else
- die("unknown --date-format argument %s", fmt);
- }
- else if (!prefixcmp(a, "--max-pack-size="))
- max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
- else if (!prefixcmp(a, "--depth=")) {
- max_depth = strtoul(a + 8, NULL, 0);
- if (max_depth > MAX_DEPTH)
- die("--depth cannot exceed %u", MAX_DEPTH);
- }
- else if (!prefixcmp(a, "--active-branches="))
- max_active_branches = strtoul(a + 18, NULL, 0);
- else if (!prefixcmp(a, "--import-marks="))
- import_marks(a + 15);
- else if (!prefixcmp(a, "--export-marks="))
- mark_file = a + 15;
- else if (!prefixcmp(a, "--export-pack-edges=")) {
- if (pack_edges)
- fclose(pack_edges);
- pack_edges = fopen(a + 20, "a");
- if (!pack_edges)
- die_errno("Cannot open '%s'", a + 20);
- } else if (!strcmp(a, "--force"))
- force_update = 1;
- else if (!strcmp(a, "--quiet"))
- show_stats = 0;
- else if (!strcmp(a, "--stats"))
- show_stats = 1;
- else
- die("unknown option %s", a);
+
+ parse_one_option(a + 2);
}
if (i != argc)
usage(fast_import_usage);
--
1.6.4.122.g6ffd7
^ permalink raw reply related
* [PATCH v3 2/3] fast-import: add option command
From: Sverre Rabbelier @ 2009-08-13 19:02 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
Cc: Sverre Rabbelier
In-Reply-To: <1250190156-4752-2-git-send-email-srabbelier@gmail.com>
This allows the frontend to specify any of the supported options as
long as no non-option command has been given. This way the
user does not have to include any frontend-specific options, but
instead she can rely on the frontend to tell fast-import what it
needs.
Also factor out parsing of argv and have it execute when we reach the
first non-option command, or after all commands have been read and
no non-option command has been encountered.
Lastly do not read the marks file till after all options have been
parsed, instead of when receiving the option.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Now we delay reading the marks file till option parsing is done.
Documentation/git-fast-import.txt | 23 +++++++
fast-import.c | 130 +++++++++++++++++++++++++------------
2 files changed, 111 insertions(+), 42 deletions(-)
diff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt
index c2f483a..ed8bd0d 100644
--- a/Documentation/git-fast-import.txt
+++ b/Documentation/git-fast-import.txt
@@ -303,6 +303,11 @@ and control the current import process. More detailed discussion
standard output. This command is optional and is not needed
to perform an import.
+`option`::
+ Specify any of the options listed under OPTIONS to change
+ fast-import's behavior to suit the frontend's needs. This command
+ is optional and is not needed to perform an import.
+
`commit`
~~~~~~~~
Create or update a branch with a new commit, recording one logical
@@ -813,6 +818,24 @@ Placing a `progress` command immediately after a `checkpoint` will
inform the reader when the `checkpoint` has been completed and it
can safely access the refs that fast-import updated.
+`option`
+~~~~~~~~
+Processes the specified option so that git fast-import behaves in a
+way that suits the frontend's needs.
+Note that options specified by the frontend are overridden by any
+options the user may specify to git fast-import itself.
+
+....
+ 'option' SP <option> LF
+....
+
+The `<option>` part of the command may contain any of the options
+listed in the OPTIONS section, without the leading '--' and is
+treated in the same way.
+
+Option commands must be the first commands on the input, to give an
+option command after any non-option command is an error.
+
Crash Reports
-------------
If fast-import is supplied invalid input it will terminate with a
diff --git a/fast-import.c b/fast-import.c
index b904f20..dff2937 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -292,6 +292,8 @@ static unsigned long branch_load_count;
static int failure;
static FILE *pack_edges;
static unsigned int show_stats = 1;
+static int global_argc;
+static const char **global_argv;
/* Memory pools */
static size_t mem_pool_alloc = 2*1024*1024 - sizeof(struct mem_pool);
@@ -315,6 +317,7 @@ static struct object_entry_pool *blocks;
static struct object_entry *object_table[1 << 16];
static struct mark_set *marks;
static const char *mark_file;
+static const char *input_file;
/* Our last blob */
static struct last_object last_blob = { STRBUF_INIT, 0, 0, 0 };
@@ -348,6 +351,9 @@ static struct recent_command *rc_free;
static unsigned int cmd_save = 100;
static uintmax_t next_mark;
static struct strbuf new_data = STRBUF_INIT;
+static int seen_non_option_command;
+
+static void parse_argv(void);
static void write_branch_report(FILE *rpt, struct branch *b)
{
@@ -1643,6 +1649,42 @@ static void dump_marks(void)
}
}
+static void read_marks(void)
+{
+ char line[512];
+ FILE *f = fopen(input_file, "r");
+ if (!f)
+ die_errno("cannot read '%s'", input_file);
+ while (fgets(line, sizeof(line), f)) {
+ uintmax_t mark;
+ char *end;
+ unsigned char sha1[20];
+ struct object_entry *e;
+
+ end = strchr(line, '\n');
+ if (line[0] != ':' || !end)
+ die("corrupt mark line: %s", line);
+ *end = 0;
+ mark = strtoumax(line + 1, &end, 10);
+ if (!mark || end == line + 1
+ || *end != ' ' || get_sha1(end + 1, sha1))
+ die("corrupt mark line: %s", line);
+ e = find_object(sha1);
+ if (!e) {
+ enum object_type type = sha1_object_info(sha1, NULL);
+ if (type < 0)
+ die("object not found: %s", sha1_to_hex(sha1));
+ e = insert_object(sha1);
+ e->type = type;
+ e->pack_id = MAX_PACK_ID;
+ e->offset = 1; /* just not zero! */
+ }
+ insert_mark(mark, e);
+ }
+ fclose(f);
+}
+
+
static int read_next_command(void)
{
static int stdin_eof = 0;
@@ -1663,6 +1705,11 @@ static int read_next_command(void)
if (stdin_eof)
return EOF;
+ if (!seen_non_option_command
+ && prefixcmp(command_buf.buf, "option ")) {
+ parse_argv();
+ }
+
rc = rc_free;
if (rc)
rc_free = rc->next;
@@ -2338,39 +2385,9 @@ static void parse_progress(void)
skip_optional_lf();
}
-static void option_import_marks(const char *input_file)
+static void option_import_marks(const char *marks)
{
- char line[512];
- FILE *f = fopen(input_file, "r");
- if (!f)
- die_errno("cannot read '%s'", input_file);
- while (fgets(line, sizeof(line), f)) {
- uintmax_t mark;
- char *end;
- unsigned char sha1[20];
- struct object_entry *e;
-
- end = strchr(line, '\n');
- if (line[0] != ':' || !end)
- die("corrupt mark line: %s", line);
- *end = 0;
- mark = strtoumax(line + 1, &end, 10);
- if (!mark || end == line + 1
- || *end != ' ' || get_sha1(end + 1, sha1))
- die("corrupt mark line: %s", line);
- e = find_object(sha1);
- if (!e) {
- enum object_type type = sha1_object_info(sha1, NULL);
- if (type < 0)
- die("object not found: %s", sha1_to_hex(sha1));
- e = insert_object(sha1);
- e->type = type;
- e->pack_id = MAX_PACK_ID;
- e->offset = 1; /* just not zero! */
- }
- insert_mark(mark, e);
- }
- fclose(f);
+ input_file = xstrdup(marks);
}
static void option_date_format(const char *fmt)
@@ -2443,6 +2460,16 @@ static void parse_one_option(const char *option)
}
}
+static void parse_option(void)
+{
+ char* option = command_buf.buf + 7;
+
+ if (seen_non_option_command)
+ die("Got option command '%s' after non-option command", option);
+
+ parse_one_option(option);
+}
+
static int git_pack_config(const char *k, const char *v, void *cb)
{
if (!strcmp(k, "pack.depth")) {
@@ -2467,6 +2494,26 @@ static int git_pack_config(const char *k, const char *v, void *cb)
static const char fast_import_usage[] =
"git fast-import [--date-format=f] [--max-pack-size=n] [--depth=n] [--active-branches=n] [--export-marks=marks.file]";
+static void parse_argv(void)
+{
+ unsigned int i;
+
+ for (i = 1; i < global_argc; i++) {
+ const char *a = global_argv[i];
+
+ if (*a != '-' || !strcmp(a, "--"))
+ break;
+
+ parse_one_option(a + 2);
+ }
+ if (i != global_argc)
+ usage(fast_import_usage);
+
+ seen_non_option_command = 1;
+ if (input_file)
+ read_marks();
+}
+
int main(int argc, const char **argv)
{
unsigned int i;
@@ -2485,16 +2532,8 @@ int main(int argc, const char **argv)
avail_tree_table = xcalloc(avail_tree_table_sz, sizeof(struct avail_tree_content*));
marks = pool_calloc(1, sizeof(struct mark_set));
- for (i = 1; i < argc; i++) {
- const char *a = argv[i];
-
- if (*a != '-' || !strcmp(a, "--"))
- break;
-
- parse_one_option(a + 2);
- }
- if (i != argc)
- usage(fast_import_usage);
+ global_argc = argc;
+ global_argv = argv;
rc_free = pool_alloc(cmd_save * sizeof(*rc_free));
for (i = 0; i < (cmd_save - 1); i++)
@@ -2517,9 +2556,16 @@ int main(int argc, const char **argv)
parse_checkpoint();
else if (!prefixcmp(command_buf.buf, "progress "))
parse_progress();
+ else if (!prefixcmp(command_buf.buf, "option "))
+ parse_option();
else
die("Unsupported command: %s", command_buf.buf);
}
+
+ // argv hasn't been parsed yet, do so
+ if (!seen_non_option_command)
+ parse_argv();
+
end_packfile();
dump_branches();
--
1.6.4.122.g6ffd7
^ permalink raw reply related
* [PATCH v3 3/3] fast-import: test the new option command
From: Sverre Rabbelier @ 2009-08-13 19:02 UTC (permalink / raw)
To: Junio C Hamano, Shawn O. Pearce, Johannes Schindelin, Git List
Cc: Sverre Rabbelier
In-Reply-To: <1250190156-4752-3-git-send-email-srabbelier@gmail.com>
Test three options (quiet and import/export-marks) and verify that the
commandline options override these.
Signed-off-by: Sverre Rabbelier <srabbelier@gmail.com>
---
Added some tests to verify that the marks file is handled
properly as suggested by Dscho and Shawn.
t/t9300-fast-import.sh | 58 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 58 insertions(+), 0 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 821be7c..62369e5 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -1088,4 +1088,62 @@ INPUT_END
test_expect_success 'P: fail on blob mark in gitlink' '
test_must_fail git fast-import <input'
+###
+### series Q (options)
+###
+
+cat >input << EOF
+option quiet
+blob
+data 3
+hi
+
+EOF
+
+touch empty
+
+test_expect_success 'Q: quiet option results in no stats being output' '
+ cat input | git fast-import 2> output &&
+ test_cmp empty output
+'
+
+cat >input << EOF
+option export-marks=git.marks
+blob
+mark :1
+data 3
+hi
+
+EOF
+
+test_expect_success \
+ 'Q: export-marks option results in a marks file being created' \
+ 'cat input | git fast-import &&
+ grep :1 git.marks'
+
+test_expect_success \
+ 'Q: export-marks options can be overriden by commandline options' \
+ 'cat input | git fast-import --export-marks=other.marks &&
+ grep :1 other.marks'
+
+cat >input << EOF
+option import-marks=marks.out
+option export-marks=marks.new
+EOF
+
+test_expect_success \
+ 'Q: import to output marks works without any content' \
+ 'cat input | git fast-import &&
+ test_cmp marks.out marks.new'
+
+cat >input <<EOF
+option import-marks=nonexistant.marks
+option export-marks=marks.new
+EOF
+
+test_expect_success \
+ 'Q: import marks uses the commandline marks file when the stream specifies one' \
+ 'cat input | git fast-import --import-marks=marks.out &&
+ test_cmp marks.out marks.new'
+
test_done
--
1.6.4.122.g6ffd7
^ permalink raw reply related
* Re: [PATCH] git-cvsimport: add support for cvs pserver password scrambling.
From: Dirk Hörner @ 2009-08-13 19:19 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <alpine.DEB.1.00.0908131837110.7429@intel-tinevez-2-302>
[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]
Hi all,
sorry for the long delay, but I finally sat down, hacked two testcases
and amended the patch after rebasing to the most recent HEAD. Find it
attached to this mail.
Ciao,
Dirk
On Thu, Aug 13, 2009 at 6:43 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> Hi,
>
> On Sat, 11 Apr 2009, Junio C Hamano wrote:
>
> > Nanako Shiraishi <nanako3@lavabit.com> writes:
> >
> > > Quoting Dirk Hörner:
> > >
> > >> Instead of a cleartext password, the CVS pserver expects a scrambled one
> > >> in the authentication request. With this patch it is possible to import
> > >> CVS repositories only accessible via pserver and user/password.
> > >>
> > >> Signed-off-by: Dirk Hoerner <dirker@gmail.com>
> > >
> > > Junio, may I ask what happened to this patch?
> >
> > I do not use cvs emulation myself, nor pserver access, and I actually have
> > been waiting for people who do use pserver access to report breakages and
> > people pointing this patch out.
>
> I really think it would be good if this patch was amended with a simple
> and quick test. Using the stdin/stdout server method, it should not be
> hard.
>
> Ciao,
> Dscho
[-- Attachment #2: 0001-git-cvsimport-add-support-for-cvs-pserver-password-s.patch --]
[-- Type: application/octet-stream, Size: 3611 bytes --]
From 2f3deea40def04286f0483bd33a5756ac233838a Mon Sep 17 00:00:00 2001
From: Dirk Hoerner <dirker@gmail.com>
Date: Fri, 28 Nov 2008 19:11:38 +0200
Subject: [PATCH] git-cvsimport: add support for cvs pserver password scrambling.
Instead of a cleartext password, the CVS pserver expects a scrambled one
in the authentication request. With this patch it is possible to import
CVS repositories only accessible via pserver and user/password.
Signed-off-by: Dirk Hoerner <dirker@gmail.com>
---
git-cvsimport.perl | 39 ++++++++++++++++++++++++++++++++++++++-
t/t9600-cvsimport.sh | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 79 insertions(+), 1 deletions(-)
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index e439202..593832d 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -252,7 +252,8 @@ sub conn {
}
};
}
- $pass="A" unless $pass;
+
+ $pass = $self->_scramble($pass);
my ($s, $rep);
if ($proxyhost) {
@@ -484,6 +485,42 @@ sub _fetchfile {
return $res;
}
+sub _scramble {
+ my ($self, $pass) = @_;
+ my $scrambled = "A";
+
+ return $scrambled unless $pass;
+
+ my $pass_len = length($pass);
+ my @pass_arr = split("", $pass);
+ my $i;
+
+ # from cvs/src/scramble.c
+ my @shifts = (
+ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15,
+ 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31,
+ 114,120, 53, 79, 96,109, 72,108, 70, 64, 76, 67,116, 74, 68, 87,
+ 111, 52, 75,119, 49, 34, 82, 81, 95, 65,112, 86,118,110,122,105,
+ 41, 57, 83, 43, 46,102, 40, 89, 38,103, 45, 50, 42,123, 91, 35,
+ 125, 55, 54, 66,124,126, 59, 47, 92, 71,115, 78, 88,107,106, 56,
+ 36,121,117,104,101,100, 69, 73, 99, 63, 94, 93, 39, 37, 61, 48,
+ 58,113, 32, 90, 44, 98, 60, 51, 33, 97, 62, 77, 84, 80, 85,223,
+ 225,216,187,166,229,189,222,188,141,249,148,200,184,136,248,190,
+ 199,170,181,204,138,232,218,183,255,234,220,247,213,203,226,193,
+ 174,172,228,252,217,201,131,230,197,211,145,238,161,179,160,212,
+ 207,221,254,173,202,146,224,151,140,196,205,130,135,133,143,246,
+ 192,159,244,239,185,168,215,144,139,165,180,157,147,186,214,176,
+ 227,231,219,169,175,156,206,198,129,164,150,210,154,177,134,127,
+ 182,128,158,208,162,132,167,209,149,241,153,251,237,236,171,195,
+ 243,233,253,240,194,250,191,155,142,137,245,235,163,242,178,152
+ );
+
+ for ($i = 0; $i < $pass_len; $i++) {
+ $scrambled .= pack("C", $shifts[ord($pass_arr[$i])]);
+ }
+
+ return $scrambled;
+}
package main;
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 363345f..57c0eac 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -128,4 +128,45 @@ test_expect_success 'import from a CVS working tree' '
test_expect_success 'test entire HEAD' 'test_cmp_branch_tree master'
+if ! type nc >/dev/null 2>&1
+then
+ say 'skipping cvsimport pserver test, nc not found'
+ test_done
+ exit
+fi
+
+cat << EOF >expected
+BEGIN AUTH REQUEST
+/cvs
+me
+AyuhedEIc?^]'%=0:q Z,b<3!a>
+END AUTH REQUEST
+EOF
+
+test_expect_success 'connect to pserver with password' '
+
+ echo "I HATE YOU" | nc -l 2401 >actual &
+ test_must_fail git cvsimport -d \
+ :pserver:me:abcdefghijklmnopqrstuvwxyz@localhost:/cvs foo \
+ >/dev/null 2>&1 &&
+ test_cmp expected actual
+'
+
+cat << EOF >expected
+BEGIN AUTH REQUEST
+/cvs
+anonymous
+A
+END AUTH REQUEST
+EOF
+
+test_expect_success 'connect to pserver without password' '
+
+ echo "I HATE YOU" | nc -l 2401 >actual &
+ test_must_fail git cvsimport -d \
+ :pserver:anonymous@localhost:/cvs foo \
+ >/dev/null 2>&1 &&
+ test_cmp expected actual
+'
+
test_done
--
1.6.4
^ permalink raw reply related
* Re: [PATCH 2/4] fast-import: define a new option command
From: Junio C Hamano @ 2009-08-13 19:26 UTC (permalink / raw)
To: Shawn O. Pearce
Cc: Sverre Rabbelier, Johannes Schindelin, Git List, Junio C Hamano
In-Reply-To: <20090813172508.GO1033@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> writes:
> Sverre Rabbelier <srabbelier@gmail.com> wrote:
>> On Thu, Aug 13, 2009 at 10:07, Johannes
>> Schindelin<Johannes.Schindelin@gmx.de> wrote:
>> > ... and will import the marks twice?
>>
>> Ah, you're right :(. What's the best way to do this? Should we dump
>> any previous marks when importing new ones?
>
> Uh, well, yes. We shouldn't define :5 if it was in the file that
> appeared in the stream, but isn't in the file on the command line.
>
> Worse, what happens if we do this:
>
> echo "option import-marks=/not/found" \
> | git fast-import --import-marks=my.marks
>
> I want this to work, even though /not/found does not exist, but
> my.marks does. So that does complicate things...
How about making the option parser get and keep the _name_ of the file
until option parsing session (i.e. read the stream until initial run of
"option" command runs out and then parse the command line to override),
and then finally open the file and read it?
^ permalink raw reply
* Re: msysGit and SCons: broken?
From: Dirk Süsserott @ 2009-08-13 19:32 UTC (permalink / raw)
To: Dirk Süsserott; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <4A7B32EA.2080505@dirk.my1.cc>
Am 06.08.2009 21:45 schrieb Dirk Süsserott:
> Am 04.08.2009 00:13 schrieb Johannes Schindelin:
>> On Mon, 3 Aug 2009, Dirk Süsserott wrote:
>>
>> How does your SCons call relate to Git? Do you call it from the Git
>> Bash? Do you call it from cmd.exe directly? Is Git/bash in your PATH?
>
> I used to call SCons from Git-bash and it worked. After Git's upgrade
> (or some other unknown change) I did the same and it didn't work from
> Git-bash, but it still worked from cmd.exe. Git-bash ist not in my PATH
> when I run cmd.exe.
If someone had the same or a similar problem: I tracked it down and
found a solution. The problem was that I tried to run a Windows program
from git-bash. The Windows program then faces the bash's $PATH with a
different separator (':' vs. ';') and a different root directory ('/c/'
vs. 'C:/'). Scons tries to split the PATH apart to figure out which
tools are installed. ActivePython thinks ';' is the right separator and
then fails. Thus, I wrote a wrapper to call Scons after manipulating the
$PATH variable by first exchanging the separator and then exchanging
'/c/' with 'c:/'.
Funny, though, that my things worked a few weeks ago *without* this
wrapper. Dunno why. At least it hasn't anything to do with my Git
update. I proved that by installing earlier versions of Git.
Dirk
^ permalink raw reply
* [PATCH] git submodule summary: add --files option
From: Jens Lehmann @ 2009-08-13 19:32 UTC (permalink / raw)
To: git, hjemli; +Cc: gitster
git submodule summary is providing similar functionality for submodules as
git diff-index does for a git project (including the meaning of --cached).
But the analogon to git diff-files is missing, so add a --files option to
summarize the differences between the index of the super project and the
last commit checked out in the working tree of the submodule.
Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de>
---
Documentation/git-submodule.txt | 13 +++++++++++--
git-submodule.sh | 19 ++++++++++++++++---
t/t7401-submodule-summary.sh | 22 ++++++++++++++++++++++
3 files changed, 49 insertions(+), 5 deletions(-)
diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt
index 7dd73ae..145802a 100644
--- a/Documentation/git-submodule.txt
+++ b/Documentation/git-submodule.txt
@@ -15,7 +15,7 @@ SYNOPSIS
'git submodule' [--quiet] init [--] [<path>...]
'git submodule' [--quiet] update [--init] [-N|--no-fetch] [--rebase]
[--reference <repository>] [--merge] [--] [<path>...]
-'git submodule' [--quiet] summary [--cached] [--summary-limit <n>] [commit] [--] [<path>...]
+'git submodule' [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]
'git submodule' [--quiet] foreach <command>
'git submodule' [--quiet] sync [--] [<path>...]
@@ -127,7 +127,11 @@ summary::
Show commit summary between the given commit (defaults to HEAD) and
working tree/index. For a submodule in question, a series of commits
in the submodule between the given super project commit and the
- index or working tree (switched by --cached) are shown.
+ index or working tree (switched by --cached) are shown. If the option
+ --files is given, show the series of commits in the submodule between
+ the index of super project the and the working tree of the submodule
+ (this option doesn't allow to use the --cached option or to provide an
+ explicit commit).
foreach::
Evaluates an arbitrary shell command in each checked out submodule.
@@ -169,6 +173,11 @@ OPTIONS
commands typically use the commit found in the submodule HEAD, but
with this option, the commit stored in the index is used instead.
+--files::
+ This option is only valid for the summary command. This command
+ compares the commit in the index with that in the submodule HEAD
+ when this option is used.
+
-n::
--summary-limit::
This option is only valid for the summary command.
diff --git a/git-submodule.sh b/git-submodule.sh
index ebed711..9bdd6ea 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -4,7 +4,7 @@
#
# Copyright (c) 2007 Lars Hjemli
-USAGE="[--quiet] [--cached] \
+USAGE="[--quiet] [--cached|--files] \
[add [-b branch] <repo> <path>]|[status|init|update [-i|--init] [-N|--no-fetch] [--rebase|--merge]|summary [-n|--summary-limit <n>] [<commit>]] \
[--] [<path>...]|[foreach <command>]|[sync [--] [<path>...]]"
OPTIONS_SPEC=
@@ -16,6 +16,7 @@ command=
branch=
reference=
cached=
+files=
nofetch=
update=
@@ -460,6 +461,7 @@ set_name_rev () {
cmd_summary() {
summary_limit=-1
for_status=
+ diff_cmd=diff-index
# parse $args after "submodule ... summary".
while test $# -ne 0
@@ -468,6 +470,9 @@ cmd_summary() {
--cached)
cached="$1"
;;
+ --files)
+ files="$1"
+ ;;
--for-status)
for_status="$1"
;;
@@ -504,9 +509,17 @@ cmd_summary() {
head=HEAD
fi
+ if [ -n "$files" ]
+ then
+ test -n "$cached" &&
+ die "--cached cannot be used with --files"
+ diff_cmd=diff-files
+ head=
+ fi
+
cd_to_toplevel
# Get modified modules cared by user
- modules=$(git diff-index $cached --raw $head -- "$@" |
+ modules=$(git $diff_cmd $cached --raw $head -- "$@" |
egrep '^:([0-7]* )?160000' |
while read mod_src mod_dst sha1_src sha1_dst status name
do
@@ -520,7 +533,7 @@ cmd_summary() {
test -z "$modules" && return
- git diff-index $cached --raw $head -- $modules |
+ git $diff_cmd $cached --raw $head -- $modules |
egrep '^:([0-7]* )?160000' |
cut -c2- |
while read mod_src mod_dst sha1_src sha1_dst status name
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index 6149829..6cc16c3 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -56,6 +56,15 @@ test_expect_success 'modified submodule(forward)' "
EOF
"
+test_expect_success 'modified submodule(forward), --files' "
+ git submodule summary --files >actual &&
+ diff actual - <<-EOF
+* sm1 $head1...$head2 (1):
+ > Add foo3
+
+EOF
+"
+
commit_file sm1 &&
cd sm1 &&
git reset --hard HEAD~2 >/dev/null &&
@@ -114,6 +123,15 @@ test_expect_success 'typechanged submodule(submodule->blob), --cached' "
EOF
"
+test_expect_success 'typechanged submodule(submodule->blob), --files' "
+ git submodule summary --files >actual &&
+ diff actual - <<-EOF
+* sm1 $head5(blob)->$head4(submodule) (3):
+ > Add foo5
+
+EOF
+"
+
rm -rf sm1 &&
git checkout-index sm1
test_expect_success 'typechanged submodule(submodule->blob)' "
@@ -205,4 +223,8 @@ test_expect_success '--for-status' "
EOF
"
+test_expect_success 'fail when using --files together with --cached' "
+ test_must_fail git submodule summary --files --cached
+"
+
test_done
--
1.6.4.114.gefd1
^ permalink raw reply related
* [PATCH] Fix "unpack-objects --strict"
From: Junio C Hamano @ 2009-08-13 19:33 UTC (permalink / raw)
To: Frank Lichtenheld; +Cc: git, Martin Koegler
In-Reply-To: <20090813111933.GZ14475@mail-vs.djpig.de>
When unpack-objects is run under the --strict option, objects that have
pointers to other objects are verified for the reachability at the end, by
calling check_object() on each of them, and letting check_object to walk
the reachable objects from them using fsck_walk() recursively.
The function however misunderstands the semantics of fsck_walk() function
when it makes a call to it, setting itself as the callback. fsck_walk()
expects the callback function to return a non-zero value to signal an
error (negative value causes an immediate abort, positive value is still
an error but allows further checks on sibling objects) and return zero to
signal a success. The function however returned 1 on some non error
cases, and to cover up this mistake, complained only when fsck_walk() did
not detect any error.
To fix this double-bug, make the function return zero on all success
cases, and also check for non-zero return from fsck_walk() for an error.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Caused by b41860b (unpack-objects: prevent writing of inconsistent
objects, 2008-02-25), which introduced these checks and also the code to
keep unverified objects in core until check_objects() verifies their
reachability. While I think it is a good idea to check for incomplete
pack data, I do not think it is necessary to keep them in core. We can
simply error out to signal the caller not to update the refs.
We probably should write everything as they become unpackable (i.e. as
their delta bases becomes available) while keeping track of object names
(but not data) of structured objects that we received, and running only
one level of reachability check on them at the end. That would certainly
reduce the memory consumption and may simplify the complexity of the code
at the same time.
But I'll leave that to other people. Hint, hint...
builtin-unpack-objects.c | 8 ++++----
t/t5531-deep-submodule-push.sh | 32 ++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 557148a..109b7c8 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -184,7 +184,7 @@ static int check_object(struct object *obj, int type, void *data)
return 0;
if (obj->flags & FLAG_WRITTEN)
- return 1;
+ return 0;
if (type != OBJ_ANY && obj->type != type)
die("object type mismatch");
@@ -195,15 +195,15 @@ static int check_object(struct object *obj, int type, void *data)
if (type != obj->type || type <= 0)
die("object of unexpected type");
obj->flags |= FLAG_WRITTEN;
- return 1;
+ return 0;
}
if (fsck_object(obj, 1, fsck_error_function))
die("Error in object");
- if (!fsck_walk(obj, check_object, NULL))
+ if (fsck_walk(obj, check_object, NULL))
die("Error on reachable objects of %s", sha1_to_hex(obj->sha1));
write_cached_object(obj);
- return 1;
+ return 0;
}
static void write_rest(void)
diff --git a/t/t5531-deep-submodule-push.sh b/t/t5531-deep-submodule-push.sh
new file mode 100755
index 0000000..13b8e40
--- /dev/null
+++ b/t/t5531-deep-submodule-push.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+test_description='unpack-objects'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+ git init --bare pub.git &&
+ GIT_DIR=pub.git git config receive.fsckobjects true &&
+ git init work &&
+ (
+ cd work &&
+ git init gar/bage &&
+ (
+ cd gar/bage &&
+ >junk &&
+ git add junk &&
+ git commit -m "Initial junk"
+ ) &&
+ git add gar/bage &&
+ git commit -m "Initial superproject"
+ )
+'
+
+test_expect_failure push '
+ (
+ cd work &&
+ git push ../pub.git master
+ )
+'
+
+test_done
^ permalink raw reply related
* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Junio C Hamano @ 2009-08-13 19:33 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Linus Torvalds, git
In-Reply-To: <alpine.LFD.2.00.0908131304520.10633@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> As it is now, I was about to suggest:
>
> git mv block-sha1/sha1.[ch] .
> rmdir block-sha1
> rm -r mozilla-sha1
> rm -r arm
> rm -r ppc
>
> and remove support for openssl's SHA1 usage, making this implementation
> unconditional. After all it is faster, or so close to be faster than
> the alternatives, that we should probably cut on the extra dependency
> and simplify portability issues at the same time.
Wow. Is it now faster than the arm/ and ppc/ hand-tweaked assembly?
^ permalink raw reply
* Re: [PATCH 1/5] port --ignore-unmatch to "git add"
From: Junio C Hamano @ 2009-08-13 19:36 UTC (permalink / raw)
To: Luke Dashjr; +Cc: git
In-Reply-To: <1250133624-2272-1-git-send-email-luke-jr+git@utopios.org>
Luke Dashjr <luke-jr+git@utopios.org> writes:
> "git rm" has a --ignore-unmatch option that is also applicable to "git add"
> and may be useful for persons wanting to ignore unmatched arguments, but not
> all errors.
>
> Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
Chould you refresh my memory a bit?
In what circumstance is "rm --ignore-unmatch" useful to begin with?
A similar question for "add --ignore-unmatch".
Now the obligatory design level question is behind us, let's take a brief
look at the codde.
> +static int ignore_unmatch = 0;
Drop " = 0" and let the language initialize this to zero.
> static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
> {
> @@ -63,7 +64,7 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
> fill_pathspec_matches(pathspec, seen, specs);
>
> for (i = 0; i < specs; i++) {
> - if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]))
> + if (!seen[i] && pathspec[i][0] && !file_exists(pathspec[i]) && !ignore_unmatch)
> die("pathspec '%s' did not match any files",
> pathspec[i]);
> }
> @@ -108,7 +109,7 @@ static void refresh(int verbose, const char **pathspec)
> refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
> pathspec, seen);
> for (i = 0; i < specs; i++) {
> - if (!seen[i])
> + if (!seen[i] && !ignore_unmatch)
> die("pathspec '%s' did not match any files", pathspec[i]);
> }
> free(seen);
What's the point of these two loops if under ignore_unmatch everything
becomes no-op?
That is, wouldn't it be much more clear if you wrote like this?
builtin-add.c | 25 ++++++++++++++++++-------
1 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/builtin-add.c b/builtin-add.c
index 581a2a1..49576b4 100644
--- a/builtin-add.c
+++ b/builtin-add.c
@@ -41,16 +41,25 @@ static void fill_pathspec_matches(const char **pathspec, char *seen, int specs)
}
}
+static char *alloc_seen(const char **pathspec, int *specs_)
+{
+ int specs;
+
+ if (ignore_unmatch)
+ return NULL;
+ for (specs = 0; pathspec[specs]; specs++)
+ ; /* nothing */
+ *specs_ = specs;
+ return xcalloc(specs, 1);
+}
+
static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)
{
char *seen;
int i, specs;
struct dir_entry **src, **dst;
- for (specs = 0; pathspec[specs]; specs++)
- /* nothing */;
- seen = xcalloc(specs, 1);
-
+ seen = alloc_seen(pathspec, &specs);
src = dst = dir->entries;
i = dir->nr;
while (--i >= 0) {
@@ -60,6 +69,8 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p
*dst++ = entry;
}
dir->nr = dst - dir->entries;
+ if (!seen)
+ return;
fill_pathspec_matches(pathspec, seen, specs);
for (i = 0; i < specs; i++) {
@@ -102,11 +113,11 @@ static void refresh(int verbose, const char **pathspec)
char *seen;
int i, specs;
- for (specs = 0; pathspec[specs]; specs++)
- /* nothing */;
- seen = xcalloc(specs, 1);
+ seen = alloc_seen(pathspec, &specs);
refresh_index(&the_index, verbose ? REFRESH_SAY_CHANGED : REFRESH_QUIET,
pathspec, seen);
+ if (!seen)
+ return;
for (i = 0; i < specs; i++) {
if (!seen[i])
die("pathspec '%s' did not match any files", pathspec[i]);
^ permalink raw reply related
* Re: [PATCH 2/5] fix "git add --ignore-errors" to ignore pathspec errors
From: Junio C Hamano @ 2009-08-13 19:38 UTC (permalink / raw)
To: Luke Dashjr; +Cc: git
In-Reply-To: <1250133624-2272-2-git-send-email-luke-jr+git@utopios.org>
Luke Dashjr <luke-jr+git@utopios.org> writes:
> Unmatched files are errors, and should be ignored with the rest of them.
Why is this a "fix"?
I would understand if it were "Make --ignore-errors imply --ignore-unmatch
unconditionally". But then I do not think I would necessarily agree it is
a good change.
The user may know that some files in the work tree are unreadable and
cannot be indexed (hence he gives --ignore-errors) but he still may want
to catch a typo on the command line.
I do not think it is wise to make --ignore-errors imply --ignore-unmatch
unconditionally like this patch does without any escape hatch.
> Signed-off-by: Luke Dashjr <luke-jr+git@utopios.org>
> ---
> builtin-add.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-add.c b/builtin-add.c
> index 0597fb9..e3132c8 100644
> --- a/builtin-add.c
> +++ b/builtin-add.c
> @@ -280,6 +280,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)
> add_interactive = 1;
> if (add_interactive)
> exit(interactive_add(argc - 1, argv + 1, prefix));
> + if (ignore_add_errors)
> + ignore_unmatch = 1;
>
> if (edit_interactive)
> return(edit_patch(argc, argv, prefix));
> --
> 1.6.3.3
^ permalink raw reply
* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Linus Torvalds @ 2009-08-13 19:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, git
In-Reply-To: <7v63crbja2.fsf@alter.siamese.dyndns.org>
On Thu, 13 Aug 2009, Junio C Hamano wrote:
>
> Wow. Is it now faster than the arm/ and ppc/ hand-tweaked assembly?
For the good cases, yes.
For POWER, with gcc-4.4, the C code apparently outperforms the asm code on
POWER6. The asm code is scheduled for POWER4, and I think outperforms the
C code there. Also, when compiling in 64-bit mode (with "-m64"), at least
some versions of gcc seem to do some stupid things and add extra zero
extension stuff, and that performed suboptimally at least on a PPC G5.
So it's certainly not a clear case of "the C code outperforms the asm
code", but in BenH's tests, the best numbers really did come from the C
version. With some silly cases of at least some versions gcc screwing up
(not reload, but zero extension), and making it noticeably slower.
IOW, the PPC situation really isn't that different from x86.
Linus
^ permalink raw reply
* Re: [PATCH 2/4] fast-import: define a new option command
From: Sverre Rabbelier @ 2009-08-13 20:01 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Shawn O. Pearce, Johannes Schindelin, Git List
In-Reply-To: <7vzla3bjn5.fsf@alter.siamese.dyndns.org>
Heya,
On Thu, Aug 13, 2009 at 12:26, Junio C Hamano<gitster@pobox.com> wrote:
> How about making the option parser get and keep the _name_ of the file
> until option parsing session (i.e. read the stream until initial run of
> "option" command runs out and then parse the command line to override),
> and then finally open the file and read it?
On Thu, Aug 13, 2009 at 10:44, Sverre Rabbelier<srabbelier@gmail.com> wrote:
> Ah, then how about in option_import_marks() we only store the name of
> the file, like in option_export_marks, and at the end, when we reach
> the first non-option command (and we've parsed argv), we read the
> file. That way it's only read once, and it deals with the above
> scenario.
Which is exactly what the latest version does :).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] git-cvsimport: add support for cvs pserver password scrambling.
From: Sverre Rabbelier @ 2009-08-13 20:04 UTC (permalink / raw)
To: Dirk Hörner
Cc: Johannes Schindelin, Junio C Hamano, Nanako Shiraishi, git
In-Reply-To: <4da546dc0908131219q149844abi453d8429847af1cf@mail.gmail.com>
Heya,
2009/8/13 Dirk Hörner <dirker@gmail.com>:
> sorry for the long delay, but I finally sat down, hacked two testcases
> and amended the patch after rebasing to the most recent HEAD. Find it
> attached to this mail.
I think we'd rather find it inlined, as per SubmittingPatches ;).
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* [PATCH] Use "gitk: /path/to/repo" as gitk window title.
From: Zbyszek Szmek @ 2009-08-13 19:58 UTC (permalink / raw)
To: git; +Cc: zbyszek
In case of non-bare repos, the .git suffix in the path is skipped.
Previously, when run in a subdirectory, gitk would show the name
of this subdirectory as the title, which was misleading.
---
gitk-git/gitk | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/gitk-git/gitk b/gitk-git/gitk
index 4604c83..e656e81 100644
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -16,6 +16,14 @@ proc gitdir {} {
}
}
+proc reponame {} {
+ set n [file normalize [gitdir]]
+ if {[string match "*/.git" $n]} {
+ set n [string range $n 0 end-5]
+ }
+ return $n
+}
+
# A simple scheduler for compute-intensive stuff.
# The aim is to make sure that event handlers for GUI actions can
# run at least every 50-100 ms. Unfortunately fileevent handlers are
@@ -11156,6 +11164,8 @@ set nullfile "/dev/null"
set have_tk85 [expr {[package vcompare $tk_version "8.5"] >= 0}]
set git_version [join [lrange [split [lindex [exec git version] end] .] 0 2] .]
+set appname "gitk"
+
set runq {}
set history {}
set historyindex 0
@@ -11220,7 +11230,7 @@ catch {
}
# wait for the window to become visible
tkwait visibility .
-wm title . "[file tail $argv0]: [file tail [pwd]]"
+wm title . "$appname: [reponame]"
update
readrefs
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] block-sha1: more good unaligned memory access candidates
From: Nicolas Pitre @ 2009-08-13 20:13 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, git
In-Reply-To: <7v63crbja2.fsf@alter.siamese.dyndns.org>
On Thu, 13 Aug 2009, Junio C Hamano wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
> > As it is now, I was about to suggest:
> >
> > git mv block-sha1/sha1.[ch] .
> > rmdir block-sha1
> > rm -r mozilla-sha1
> > rm -r arm
> > rm -r ppc
> >
> > and remove support for openssl's SHA1 usage, making this implementation
> > unconditional. After all it is faster, or so close to be faster than
> > the alternatives, that we should probably cut on the extra dependency
> > and simplify portability issues at the same time.
>
> Wow. Is it now faster than the arm/ and ppc/ hand-tweaked assembly?
It is indeed faster than the ARM assembly version by far, and faster
than all the alternative implementations too, but with a 7x increase in
compiled code size. In the context of Git I think this is a good
compromize. Making the assembly version faster than the C version could
be possible, but that would require quite some work and I don't expect
the gain to be significant, certainly not worth the trouble.
Furthermore the C version can be used to generate ARM Thumb code while
the asm version cannot without yet more work.
Nicolas
^ 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