Re: [PATCH v2 1/3] fast-{import,export}: use get_sha1_hex() directly

2013-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> Turns out most of the get_sha1() calls were correct; this does the trick:
>
> diff --git a/builtin/fast-export.c b/builtin/fast-export.c
> index 18fdfb3..d1d68e9 100644
> --- a/builtin/fast-export.c
> +++ b/builtin/fast-export.c
> @@ -623,7 +623,7 @@ static void import_marks(char *input_file)
>
> mark = strtoumax(line + 1, &mark_end, 10);
> if (!mark || mark_end == line + 1
> -   || *mark_end != ' ' || get_sha1(mark_end + 1, sha1))
> +   || *mark_end != ' ' || get_sha1_hex(mark_end + 1, 
> sha1))
> die("corrupt mark line: %s", line);
>
> if (last_idnum < mark)
> diff --git a/fast-import.c b/fast-import.c
> index 5f539d7..3f32149 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1822,7 +1822,7 @@ static void read_marks(void)
> *end = 0;
> mark = strtoumax(line + 1, &end, 10);
> if (!mark || end == line + 1
> -   || *end != ' ' || get_sha1(end + 1, sha1))
> +   || *end != ' ' || get_sha1_hex(end + 1, sha1))

This is where --import-marks is handled, and we should be seeing

:markid SHA-1

one per each line (according to Documentation/git-fast-import.txt).
So this one should be get_sha1_hex().

The other one in fast-export.c would be the same.

The other ones in the original patch were reading from the
fast-import stream and shouldn't have insisted on 40-hex.

Will replace the body of the change with only these two hunks and
requeue.  Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] fast-{import,export}: use get_sha1_hex() directly

2013-05-07 Thread Felipe Contreras
On Tue, May 7, 2013 at 9:38 AM, Junio C Hamano  wrote:
> Felipe Contreras  writes:
>
>> It's wrong to call get_sha1() if they should be SHA-1s, plus
>> inefficient.
>>
>> Signed-off-by: Felipe Contreras 
>> ---
>
> It appears that "they should be SHA-1s" assumption does not hold;
> this patch breaks at least 3303, 9020, and 9300.
>
> Also assuming these are always 40-hex goes directly against what is
> documented in Documentation/git-fast-import.txt (look for "Here
> committish is any of the following").  My bad while reviewing the
> earlier round.
>
> I've redone 'pu' (which was failing the test last night) after
> dropping this and keeping only patches 2 and 3 from the series.

Turns out most of the get_sha1() calls were correct; this does the trick:

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 18fdfb3..d1d68e9 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -623,7 +623,7 @@ static void import_marks(char *input_file)

mark = strtoumax(line + 1, &mark_end, 10);
if (!mark || mark_end == line + 1
-   || *mark_end != ' ' || get_sha1(mark_end + 1, sha1))
+   || *mark_end != ' ' || get_sha1_hex(mark_end + 1, sha1))
die("corrupt mark line: %s", line);

if (last_idnum < mark)
diff --git a/fast-import.c b/fast-import.c
index 5f539d7..3f32149 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1822,7 +1822,7 @@ static void read_marks(void)
*end = 0;
mark = strtoumax(line + 1, &end, 10);
if (!mark || end == line + 1
-   || *end != ' ' || get_sha1(end + 1, sha1))
+   || *end != ' ' || get_sha1_hex(end + 1, sha1))
die("corrupt mark line: %s", line);
e = find_object(sha1);
if (!e) {

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/3] fast-{import,export}: use get_sha1_hex() directly

2013-05-07 Thread Junio C Hamano
Felipe Contreras  writes:

> It's wrong to call get_sha1() if they should be SHA-1s, plus
> inefficient.
>
> Signed-off-by: Felipe Contreras 
> ---

It appears that "they should be SHA-1s" assumption does not hold;
this patch breaks at least 3303, 9020, and 9300.

Also assuming these are always 40-hex goes directly against what is
documented in Documentation/git-fast-import.txt (look for "Here
committish is any of the following").  My bad while reviewing the
earlier round.

I've redone 'pu' (which was failing the test last night) after
dropping this and keeping only patches 2 and 3 from the series.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2 1/3] fast-{import,export}: use get_sha1_hex() directly

2013-05-05 Thread Felipe Contreras
It's wrong to call get_sha1() if they should be SHA-1s, plus
inefficient.

Signed-off-by: Felipe Contreras 
---
 builtin/fast-export.c |  2 +-
 fast-import.c | 10 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index d60d675..a4dee14 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -621,7 +621,7 @@ static void import_marks(char *input_file)
 
mark = strtoumax(line + 1, &mark_end, 10);
if (!mark || mark_end == line + 1
-   || *mark_end != ' ' || get_sha1(mark_end + 1, sha1))
+   || *mark_end != ' ' || get_sha1_hex(mark_end + 1, sha1))
die("corrupt mark line: %s", line);
 
if (last_idnum < mark)
diff --git a/fast-import.c b/fast-import.c
index 5f539d7..e02f212 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1822,7 +1822,7 @@ static void read_marks(void)
*end = 0;
mark = strtoumax(line + 1, &end, 10);
if (!mark || end == line + 1
-   || *end != ' ' || get_sha1(end + 1, sha1))
+   || *end != ' ' || get_sha1_hex(end + 1, sha1))
die("corrupt mark line: %s", line);
e = find_object(sha1);
if (!e) {
@@ -2490,7 +2490,7 @@ static void note_change_n(struct branch *b, unsigned char 
*old_fanout)
if (commit_oe->type != OBJ_COMMIT)
die("Mark :%" PRIuMAX " not a commit", commit_mark);
hashcpy(commit_sha1, commit_oe->idx.sha1);
-   } else if (!get_sha1(p, commit_sha1)) {
+   } else if (!get_sha1_hex(p, commit_sha1)) {
unsigned long size;
char *buf = read_object_with_reference(commit_sha1,
commit_type, &size, commit_sha1);
@@ -2604,7 +2604,7 @@ static int parse_from(struct branch *b)
free(buf);
} else
parse_from_existing(b);
-   } else if (!get_sha1(from, b->sha1))
+   } else if (!get_sha1_hex(from, b->sha1))
parse_from_existing(b);
else
die("Invalid ref name or SHA1 expression: %s", from);
@@ -2632,7 +2632,7 @@ static struct hash_list *parse_merge(unsigned int *count)
if (oe->type != OBJ_COMMIT)
die("Mark :%" PRIuMAX " not a commit", idnum);
hashcpy(n->sha1, oe->idx.sha1);
-   } else if (!get_sha1(from, n->sha1)) {
+   } else if (!get_sha1_hex(from, n->sha1)) {
unsigned long size;
char *buf = read_object_with_reference(n->sha1,
commit_type, &size, n->sha1);
@@ -2792,7 +2792,7 @@ static void parse_new_tag(void)
oe = find_mark(from_mark);
type = oe->type;
hashcpy(sha1, oe->idx.sha1);
-   } else if (!get_sha1(from, sha1)) {
+   } else if (!get_sha1_hex(from, sha1)) {
struct object_entry *oe = find_object(sha1);
if (!oe) {
type = sha1_object_info(sha1, NULL);
-- 
1.8.3.rc0.401.g45bba44

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html