Junio C Hamano <gitster@xxxxxxxxx> sez:
> Phil Hord <phil.hord@xxxxxxxxx> writes:
>
>> On Tue, Oct 11, 2011 at 7:45 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> After looking at this patch and the way the other caller in transport.c
>>> uses it, I am more and more convinced that "is_gitfile()" is a stupid and
>>> horrible mistake.
>>
>> I think I misunderstood your objection before. Now I think I
>> understand. Tell me if I am right.
>>
>>
>> I think you mean that instead of this:
>> } else if (is_local(url) && is_file(url) && !is_gitfile(url)) {
>>
>> you would like to see this:
>> } else if (is_local(url) && is_file(url) && is_bundle(url)) {
>>
>> Or maybe even this:
>> } else if (is_bundle(url)) {
>
> Exactly.
I tried to refactor read_bundle_header, but it was too ugly and I don't quite
understand all the error paths. Plus, the whole is_bundle part can be just
10 lines of code. Maybe read_bundle_header() should be shortened slightly to
use is_bundle() for code duplication avoidance.
-- >8 --
Subject: [PATCH] transport: Add/use is_bundle() to identify a url
Transport currently decides that any local url which is a file
must be a bundle. This is wrong now if the local url is a
gitfile, and it will be wrong in the future when some other
exception shows up. Teach transport to verify the file is
really a bundle instead of just assuming it is so.
---
bundle.c | 16 ++++++++++++++++
bundle.h | 1 +
transport.c | 10 +---------
3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/bundle.c b/bundle.c
index f82baae..3a64a43 100644
--- a/bundle.c
+++ b/bundle.c
@@ -23,6 +23,22 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,
list->nr++;
}
+int is_bundle(const char *path)
+{
+ char buffer[100];
+ FILE *ffd = fopen(path, "rb");
+ int ret=1;
+
+ if (!ffd)
+ return 0;
+
+ if (!fgets(buffer, sizeof(buffer), ffd) ||
+ strcmp(buffer, bundle_signature))
+ ret=0;
+ fclose(ffd);
+ return ret;
+}
+
/* returns an fd */
int read_bundle_header(const char *path, struct bundle_header *header)
{
diff --git a/bundle.h b/bundle.h
index c5a22c8..35aa0eb 100644
--- a/bundle.h
+++ b/bundle.h
@@ -14,6 +14,7 @@ struct bundle_header {
struct ref_list references;
};
+int is_bundle(const char *path);
int read_bundle_header(const char *path, struct bundle_header *header);
int create_bundle(struct bundle_header *header, const char *path,
int argc, const char **argv);
diff --git a/transport.c b/transport.c
index f3195c0..bcd9b74 100644
--- a/transport.c
+++ b/transport.c
@@ -881,14 +881,6 @@ static int is_gitfile(const char *url)
return !prefixcmp(buf, "gitdir: ");
}
-static int is_file(const char *url)
-{
- struct stat buf;
- if (stat(url, &buf))
- return 0;
- return S_ISREG(buf.st_mode);
-}
-
static int external_specification_len(const char *url)
{
return strchr(url, ':') - url;
@@ -929,7 +921,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
ret->fetch = fetch_objs_via_rsync;
ret->push = rsync_transport_push;
ret->smart_options = NULL;
- } else if (is_local(url) && is_file(url) && !is_gitfile(url)) {
+ } else if (is_local(url) && is_bundle(url)) {
struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
ret->data = data;
ret->get_refs_list = get_refs_from_bundle;
--
1.7.7.334.g311c9.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html