* Re: [PATCH 2/2] Use xmemdup in many places.
From: Junio C Hamano @ 2007-09-19 8:08 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070917161142.D3C9A344A49@madism.org>
Pierre Habouzit <madcoder@debian.org> writes:
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> attr.c | 7 +------
> builtin-add.c | 8 ++------
> builtin-apply.c | 11 ++---------
> builtin-fetch--tool.c | 6 +-----
> builtin-fmt-merge-msg.c | 17 ++++++-----------
> builtin-for-each-ref.c | 40 +++++++++-------------------------------
> builtin-log.c | 12 ++----------
> builtin-ls-files.c | 9 +--------
> builtin-mv.c | 5 +----
> builtin-revert.c | 4 +---
> builtin-shortlog.c | 11 ++---------
> commit.c | 16 ++++++----------
> connect.c | 4 +---
> convert.c | 7 +------
> diff.c | 13 ++-----------
> diffcore-order.c | 7 ++-----
> fast-import.c | 4 +---
> http-push.c | 9 ++-------
> imap-send.c | 20 +++++---------------
> merge-recursive.c | 19 ++++---------------
> refs.c | 12 ++++--------
> sha1_file.c | 12 +++---------
> tag.c | 4 +---
> 23 files changed, 60 insertions(+), 197 deletions(-)
> ...
> diff --git a/builtin-apply.c b/builtin-apply.c
> index 05011bb..900d0a7 100644
> --- a/builtin-apply.c
> +++ b/builtin-apply.c
> @@ -293,11 +293,7 @@ static char *find_name(const char *line, char *def, int p_value, int terminate)
> return def;
> }
>
> - name = xmalloc(len + 1);
> - memcpy(name, start, len);
> - name[len] = 0;
> - free(def);
> - return name;
> + return xmemdup(start, len);
> }
Did we start leaking "def" here?
> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index 0afa1c5..287d52a 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> ...
> @@ -305,46 +301,28 @@ static const char *find_wholine(const char *who, int wholen, const char *buf, un
> ...
> static const char *copy_name(const char *buf)
> {
> - const char *eol = strchr(buf, '\n');
> - const char *eoname = strstr(buf, " <");
> - char *line;
> - int len;
> - if (!(eoname && eol && eoname < eol))
> - return "";
> - len = eoname - buf;
> - line = xmalloc(len + 1);
> - memcpy(line, buf, len);
> - line[len] = 0;
> - return line;
> + const char *cp;
> + for (cp = buf; *cp != '\n'; cp++) {
> + if (!strncmp(cp, " <", 2))
> + return xmemdup(buf, cp - buf);
> + }
> + return "";
> }
At least the loop should terminate upon (!*cp); if you do not
have '\n' in the buffer what happens?
^ permalink raw reply
* Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.
From: Andreas Ericsson @ 2007-09-19 8:08 UTC (permalink / raw)
To: Pierre Habouzit, Andreas Ericsson, Junio C Hamano, git
In-Reply-To: <20070919080030.GA28205@artemis.corp>
Pierre Habouzit wrote:
> On Wed, Sep 19, 2007 at 06:37:31AM +0000, Andreas Ericsson wrote:
>> Pierre Habouzit wrote:
>>> diff --git a/builtin-blame.c b/builtin-blame.c
>>> index e364b6c..16c0ca8 100644
>>> --- a/builtin-blame.c
>>> +++ b/builtin-blame.c
>>> @@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,
>>> static void write_filename_info(const char *path)
>>> {
>>> printf("filename ");
>>> - write_name_quoted(NULL, 0, path, 1, stdout);
>>> - putchar('\n');
>>> + write_name_quoted(path, stdout, '\n');
>>> }
>>>
>> This looks like a candidate for a macro. I'm not sure if gcc optimizes
>> sibling calls in void functions with -O2, and it doesn't inline without
>> -O3.
>
> Well, there is little point. write_name_quoted behaviour changes if
> the last argument is \0 or non-\0 (see patch comment and quote.c code),
> so it does not really matter to inline the "putchar" IMHO.
>
>>> -static void diff_flush_raw(struct diff_filepair *p,
>>> - struct diff_options *options)
>>> +static void diff_flush_raw(struct diff_filepair *p, struct diff_options
>>> *opt)
>> Parameter rename? I'd have thought the patch was big enough as it is ;-)
>
> I'm anal when it comes to code: the rule of the least surprise should
> apply, and consistency is fundamental. And it happens that diff_options
> are always called `opt' in diff.c, except in that place (and it allows
> to write the prototype of the function on one line).
>
Then perhaps a separate patch for this would have been prudent? I'm not
against the change per se and I understand the reasoning behind it, but
it seems to go against Documentation/SubmittingPatches (submit one change
at a time).
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: [PATCH] Simplify strbuf uses in archive-tar.c using the proper functions.
From: Junio C Hamano @ 2007-09-19 8:06 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <11890199232128-git-send-email-madcoder@debian.org>
Pierre Habouzit <madcoder@debian.org> writes:
> This is just cleaner way to deal with strbufs, using its API rather than
> reinventing it in the module (e.g. strbuf_append_string is just the plain
> strbuf_addstr function, and it was used to perform what strbuf_addch does
> anyways).
> ---
> archive-tar.c | 65 ++++++++++++++-------------------------------------------
> 1 files changed, 16 insertions(+), 49 deletions(-)
>
> diff --git a/archive-tar.c b/archive-tar.c
> index a0763c5..c84d7c0 100644
> --- a/archive-tar.c
> +++ b/archive-tar.c
> @@ -260,28 +237,18 @@ static int write_tar_entry(const unsigned char *sha1,
> const char *base, int baselen,
> const char *filename, unsigned mode, int stage)
> {
> - static struct strbuf path;
> + static struct strbuf path = STRBUF_INIT;
> int filenamelen = strlen(filename);
> void *buffer;
> enum object_type type;
> unsigned long size;
>
> - if (!path.alloc) {
> - path.buf = xmalloc(PATH_MAX);
> - path.alloc = PATH_MAX;
> - path.len = path.eof = 0;
> - }
> - if (path.alloc < baselen + filenamelen + 1) {
> - free(path.buf);
> - path.buf = xmalloc(baselen + filenamelen + 1);
> - path.alloc = baselen + filenamelen + 1;
> - }
> - memcpy(path.buf, base, baselen);
> - memcpy(path.buf + baselen, filename, filenamelen);
> - path.len = baselen + filenamelen;
> - path.buf[path.len] = '\0';
> + strbuf_grow(&path, MAX(PATH_MAX, baselen + filenamelen + 1));
Where are you getting the MAX() macro from? On my Linux box
it appears that <sys/params.h> happens to define it but I do not
think that is something we can rely upon portably.
Moreover, isn't this allocation wrong? I thought "grow" was
about "we want this much more in addition to the existing
length", not "reserve at least this much", and this "path" is a
static that will keep the buffer and length from the previous
invocation.
> + strbuf_reset(&path);
> + strbuf_add(&path, base, baselen);
> + strbuf_add(&path, filename, filenamelen);
> if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
> - strbuf_append_string(&path, "/");
> + strbuf_addch(&path, '/');
> buffer = NULL;
> size = 0;
> } else {
Having said that, I suspect that the preallocation does not
really matter in practice. How about doing something like:
strbuf_reset(&path);
strbuf_grow(&path, PATH_MAX);
strbuf_add(&path, base, baselen);
strbuf_add(&path, filename, filenamelen);
^ permalink raw reply
* Re: [PATCH] Further strbuf re-engineering.
From: Junio C Hamano @ 2007-09-19 8:05 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <1189019923943-git-send-email-madcoder@debian.org>
Pierre Habouzit <madcoder@debian.org> writes:
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> builtin-apply.c | 29 ++++++-----------------
> builtin-blame.c | 34 +++++++++-------------------
> builtin-commit-tree.c | 59 ++++++++++--------------------------------------
> diff.c | 25 ++++++--------------
> 4 files changed, 40 insertions(+), 107 deletions(-)
> ...
> diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
> index ccbcbe3..ee74814 100644
> --- a/builtin-commit-tree.c
> +++ b/builtin-commit-tree.c
> @@ -8,42 +8,13 @@
> ...
> /*
> * FIXME! Share the code with "write-tree.c"
> */
You fixed it. Remove this comment.
^ permalink raw reply
* Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.
From: Pierre Habouzit @ 2007-09-19 8:00 UTC (permalink / raw)
To: Andreas Ericsson; +Cc: Junio C Hamano, git
In-Reply-To: <46F0C3AB.8010801@op5.se>
[-- Attachment #1: Type: text/plain, Size: 1911 bytes --]
On Wed, Sep 19, 2007 at 06:37:31AM +0000, Andreas Ericsson wrote:
> Pierre Habouzit wrote:
> > diff --git a/builtin-blame.c b/builtin-blame.c
> >index e364b6c..16c0ca8 100644
> >--- a/builtin-blame.c
> >+++ b/builtin-blame.c
> >@@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,
> > static void write_filename_info(const char *path)
> > {
> > printf("filename ");
> >- write_name_quoted(NULL, 0, path, 1, stdout);
> >- putchar('\n');
> >+ write_name_quoted(path, stdout, '\n');
> > }
> >
>
> This looks like a candidate for a macro. I'm not sure if gcc optimizes
> sibling calls in void functions with -O2, and it doesn't inline without
> -O3.
Well, there is little point. write_name_quoted behaviour changes if
the last argument is \0 or non-\0 (see patch comment and quote.c code),
so it does not really matter to inline the "putchar" IMHO.
> > -static void diff_flush_raw(struct diff_filepair *p,
> >- struct diff_options *options)
> >+static void diff_flush_raw(struct diff_filepair *p, struct diff_options
> >*opt)
>
> Parameter rename? I'd have thought the patch was big enough as it is ;-)
I'm anal when it comes to code: the rule of the least surprise should
apply, and consistency is fundamental. And it happens that diff_options
are always called `opt' in diff.c, except in that place (and it allows
to write the prototype of the function on one line).
> Other than that, the diffstat calls this a good patch, and given the
> fact that all your previous series passed all tests, I assume this one
> does too.
Yes, before submitting a series I check the testsuite passes at each
step, so that it doesn't break git-bisect in obvious ways.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] git-merge: add option --no-ff
From: Lars Hjemli @ 2007-09-19 7:09 UTC (permalink / raw)
To: Peter Baumann, Junio C Hamano; +Cc: git
In-Reply-To: <20070918225134.GA5906@xp.machine.xx>
On 9/19/07, Peter Baumann <waste.manager@gmx.de> wrote:
> This should be quoted, e.g.
> + if test "$no_ff" = 't'
>
Ouch, sorry about that. I can send an updated patch late tonight (it's
now early morning here), but I'm not sure Junio wants/needs it. Junio?
--
larsh
^ permalink raw reply
* Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.
From: Andreas Ericsson @ 2007-09-19 6:37 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Junio C Hamano, git
In-Reply-To: <20070918224122.2B55D344AB3@madism.org>
Pierre Habouzit wrote:
>
> diff --git a/builtin-blame.c b/builtin-blame.c
> index e364b6c..16c0ca8 100644
> --- a/builtin-blame.c
> +++ b/builtin-blame.c
> @@ -1430,8 +1430,7 @@ static void get_commit_info(struct commit *commit,
> static void write_filename_info(const char *path)
> {
> printf("filename ");
> - write_name_quoted(NULL, 0, path, 1, stdout);
> - putchar('\n');
> + write_name_quoted(path, stdout, '\n');
> }
>
This looks like a candidate for a macro. I'm not sure if gcc optimizes
sibling calls in void functions with -O2, and it doesn't inline without
-O3.
>
> -static void diff_flush_raw(struct diff_filepair *p,
> - struct diff_options *options)
> +static void diff_flush_raw(struct diff_filepair *p, struct diff_options *opt)
Parameter rename? I'd have thought the patch was big enough as it is ;-)
Other than that, the diffstat calls this a good patch, and given the fact that
all your previous series passed all tests, I assume this one does too.
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* Re: State of Perforce importing.
From: Simon Hausmann @ 2007-09-19 6:19 UTC (permalink / raw)
To: David Brown; +Cc: Git
In-Reply-To: <20070918233749.GA19533@old.davidb.org>
[-- Attachment #1: Type: text/plain, Size: 993 bytes --]
On Wednesday 19 September 2007 01:37:49 David Brown wrote:
> On Mon, Sep 17, 2007 at 12:30:28PM -0700, David Brown wrote:
> > I'd like to track a lot of code living in a Perforce repository, so I've
> > been playing with 'git-p4.py'. Is the one in the contrib/fast-import
> > directory the latest version, or is there a better place.
> >
> > So far, it is having a couple of problems:
> >
> > - The commit comment is empty. It doesn't seem to grab the Perforce
> > description, and the user seems to be <a@b>.
> >
> > - Every revision seems to check every file out of Perforce. This means
> > that for the directory I want, every revision is going to take about
> > 20 minutes.
>
> An additional problem:
>
> - git-p4 doesn't preserve the execute permission bit from Perforce.
Hmm, can you paste the output of
p4 fstat //path/in/depot/to/file/that/is/imported/incorrectly
? I'm interested in the type of the file that p4 reports.
FWIW it works for me ;-)
Thanks,
Simon
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] Don't create .git/branches
From: Pavel Roskin @ 2007-09-19 5:08 UTC (permalink / raw)
To: git
.git/branches was obsoleted by .git/remotes in August 2005, and the
later was obsoleted in December 2006. .git/branches was kept for
compatibility with Cogito. Since Cogito is not developed any longer,
the .git/branches directory is no longer useful, especially in newly
created repositories.
Signed-off-by: Pavel Roskin <proski@gnu.org>
---
templates/branches-- | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
delete mode 100644 templates/branches--
diff --git a/templates/branches-- b/templates/branches--
deleted file mode 100644
index fae8870..0000000
--- a/templates/branches--
+++ /dev/null
@@ -1 +0,0 @@
-: this is just to ensure the directory exists.
^ permalink raw reply related
* [PATCH 5/5] Fix memory leaks when disconnecting transport instances
From: Shawn O. Pearce @ 2007-09-19 4:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Most transport implementations tend to allocate a data buffer
in the struct transport instance during transport_get() so we
need to free that data buffer when we disconnect it.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
transport.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/transport.c b/transport.c
index a1d0a3c..4f9cddc 100644
--- a/transport.c
+++ b/transport.c
@@ -236,6 +236,7 @@ static int close_bundle(struct transport *transport)
struct bundle_transport_data *data = transport->data;
if (data->fd > 0)
close(data->fd);
+ free(data);
return 0;
}
@@ -372,6 +373,12 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const
return !!err;
}
+static int disconnect_git(struct transport *transport)
+{
+ free(transport->data);
+ return 0;
+}
+
static int is_local(const char *url)
{
const char *colon = strchr(url, ':');
@@ -419,6 +426,7 @@ struct transport *transport_get(struct remote *remote, const char *url)
ret->get_refs_list = get_refs_via_connect;
ret->fetch = fetch_refs_via_pack;
ret->push = git_transport_push;
+ ret->disconnect = disconnect_git;
data->thin = 1;
data->uploadpack = "git-upload-pack";
--
1.5.3.1.195.gadd6
^ permalink raw reply related
* [PATCH 4/5] Ensure builtin-fetch honors {fetch,transfer}.unpackLimit
From: Shawn O. Pearce @ 2007-09-19 4:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
The only way to configure the unpacking limit is currently through
the .git/config (or ~/.gitconfig) mechanism as we have no existing
command line option interface to control this threshold on a per
invocation basis. This was intentional by design as the storage
policy of the repository should be a repository-wide decision and
should not be subject to variations made on individual command
executions.
Earlier builtin-fetch was bypassing the unpacking limit chosen by
the user through the configuration file as it did not reread the
configuration options through fetch_pack_config if we called the
internal fetch_pack() API directly. We now ensure we always run the
config file through fetch_pack_config at least once in this process,
thereby setting our unpackLimit properly.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-fetch-pack.c | 19 +++++++++++++------
transport.c | 9 ---------
transport.h | 3 ---
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 77eb181..d128915 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -670,18 +670,24 @@ static int fetch_pack_config(const char *var, const char *value)
static struct lock_file lock;
-int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
+static void fetch_pack_setup()
{
- int i, ret, nr_heads;
- struct ref *ref;
- char *dest = NULL, **heads;
-
+ static int did_setup;
+ if (did_setup)
+ return;
git_config(fetch_pack_config);
-
if (0 <= transfer_unpack_limit)
unpack_limit = transfer_unpack_limit;
else if (0 <= fetch_unpack_limit)
unpack_limit = fetch_unpack_limit;
+ did_setup = 1;
+}
+
+int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
+{
+ int i, ret, nr_heads;
+ struct ref *ref;
+ char *dest = NULL, **heads;
nr_heads = 0;
heads = NULL;
@@ -760,6 +766,7 @@ struct ref *fetch_pack(struct fetch_pack_args *my_args,
struct ref *ref;
struct stat st;
+ fetch_pack_setup();
memcpy(&args, my_args, sizeof(args));
if (args.depth > 0) {
if (stat(git_path("shallow"), &st))
diff --git a/transport.c b/transport.c
index 85f5b1e..a1d0a3c 100644
--- a/transport.c
+++ b/transport.c
@@ -242,11 +242,7 @@ static int close_bundle(struct transport *transport)
struct git_transport_data {
unsigned thin : 1;
unsigned keep : 1;
-
- int unpacklimit;
-
int depth;
-
const char *uploadpack;
const char *receivepack;
};
@@ -267,9 +263,6 @@ static int set_git_option(struct transport *connection,
} else if (!strcmp(name, TRANS_OPT_KEEP)) {
data->keep = !!value;
return 0;
- } else if (!strcmp(name, TRANS_OPT_UNPACKLIMIT)) {
- data->unpacklimit = atoi(value);
- return 0;
} else if (!strcmp(name, TRANS_OPT_DEPTH)) {
if (!value)
data->depth = 0;
@@ -318,7 +311,6 @@ static int fetch_refs_via_pack(struct transport *transport,
args.uploadpack = data->uploadpack;
args.keep_pack = data->keep;
args.lock_pack = 1;
- args.unpacklimit = data->unpacklimit;
args.use_thin_pack = data->thin;
args.verbose = transport->verbose;
args.depth = data->depth;
@@ -435,7 +427,6 @@ struct transport *transport_get(struct remote *remote, const char *url)
data->receivepack = "git-receive-pack";
if (remote && remote->receivepack)
data->receivepack = remote->receivepack;
- data->unpacklimit = -1;
}
return ret;
diff --git a/transport.h b/transport.h
index 3e332ff..6e318e4 100644
--- a/transport.h
+++ b/transport.h
@@ -47,9 +47,6 @@ struct transport *transport_get(struct remote *, const char *);
/* Keep the pack that was transferred if not null */
#define TRANS_OPT_KEEP "keep"
-/* Unpack the objects if fewer than this number of objects are fetched */
-#define TRANS_OPT_UNPACKLIMIT "unpacklimit"
-
/* Limit the depth of the fetch if not null */
#define TRANS_OPT_DEPTH "depth"
--
1.5.3.1.195.gadd6
^ permalink raw reply related
* [PATCH 3/5] Always obtain fetch-pack arguments from struct fetch_pack_args
From: Shawn O. Pearce @ 2007-09-19 4:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Copying the arguments from a fetch_pack_args into static globals
within the builtin-fetch-pack module is error-prone and may lead
rise to cases where arguments supplied via the struct from the
new fetch_pack() API may not be honored by the implementation.
Here we reorganize all of the static globals into a single static
struct fetch_pack_args instance and use memcpy() to move the data
from the caller supplied structure into the globals before we
execute our pack fetching implementation. This strategy is more
robust to additions and deletions of properties.
As keep_pack is a single bit we have also introduced lock_pack to
mean not only download and store the packfile via index-pack but
also to lock it against repacking by creating a .keep file when
the packfile itself is stored. The caller must remove the .keep
file when it is safe to do so.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-fetch-pack.c | 111 +++++++++++++++++++++-----------------------------
fetch-pack.h | 9 +++-
transport.c | 9 +---
3 files changed, 55 insertions(+), 74 deletions(-)
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 2977a94..77eb181 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -8,15 +8,11 @@
#include "sideband.h"
#include "fetch-pack.h"
-static int keep_pack;
static int transfer_unpack_limit = -1;
static int fetch_unpack_limit = -1;
static int unpack_limit = 100;
-static int quiet;
-static int verbose;
-static int fetch_all;
-static int depth;
-static int no_progress;
+static struct fetch_pack_args args;
+
static const char fetch_pack_usage[] =
"git-fetch-pack [--all] [--quiet|-q] [--keep|-k] [--thin] [--upload-pack=<git-upload-pack>] [--depth=<n>] [--no-progress] [-v] [<host>:]<directory> [<refs>...]";
static const char *uploadpack = "git-upload-pack";
@@ -181,7 +177,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
(use_sideband == 2 ? " side-band-64k" : ""),
(use_sideband == 1 ? " side-band" : ""),
(use_thin_pack ? " thin-pack" : ""),
- (no_progress ? " no-progress" : ""),
+ (args.no_progress ? " no-progress" : ""),
" ofs-delta");
else
packet_write(fd[1], "want %s\n", sha1_to_hex(remote));
@@ -189,13 +185,13 @@ static int find_common(int fd[2], unsigned char *result_sha1,
}
if (is_repository_shallow())
write_shallow_commits(fd[1], 1);
- if (depth > 0)
- packet_write(fd[1], "deepen %d", depth);
+ if (args.depth > 0)
+ packet_write(fd[1], "deepen %d", args.depth);
packet_flush(fd[1]);
if (!fetching)
return 1;
- if (depth > 0) {
+ if (args.depth > 0) {
char line[1024];
unsigned char sha1[20];
int len;
@@ -226,7 +222,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
retval = -1;
while ((sha1 = get_rev())) {
packet_write(fd[1], "have %s\n", sha1_to_hex(sha1));
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "have %s\n", sha1_to_hex(sha1));
in_vain++;
if (!(31 & ++count)) {
@@ -244,7 +240,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
do {
ack = get_ack(fd[0], result_sha1);
- if (verbose && ack)
+ if (args.verbose && ack)
fprintf(stderr, "got ack %d %s\n", ack,
sha1_to_hex(result_sha1));
if (ack == 1) {
@@ -263,7 +259,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
} while (ack);
flushes--;
if (got_continue && MAX_IN_VAIN < in_vain) {
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "giving up\n");
break; /* give up */
}
@@ -271,7 +267,7 @@ static int find_common(int fd[2], unsigned char *result_sha1,
}
done:
packet_write(fd[1], "done\n");
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "done\n");
if (retval != 0) {
multi_ack = 0;
@@ -280,7 +276,7 @@ done:
while (flushes || multi_ack) {
int ack = get_ack(fd[0], result_sha1);
if (ack) {
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "got ack (%d) %s\n", ack,
sha1_to_hex(result_sha1));
if (ack == 1)
@@ -317,7 +313,7 @@ static int mark_complete(const char *path, const unsigned char *sha1, int flag,
static void mark_recent_complete_commits(unsigned long cutoff)
{
while (complete && cutoff <= complete->item->date) {
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "Marking %s as complete\n",
sha1_to_hex(complete->item->object.sha1));
pop_most_recent_commit(&complete, COMPLETE);
@@ -332,7 +328,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
struct ref *ref, *next;
struct ref *fastarray[32];
- if (nr_match && !fetch_all) {
+ if (nr_match && !args.fetch_all) {
if (ARRAY_SIZE(fastarray) < nr_match)
return_refs = xcalloc(nr_match, sizeof(struct ref *));
else {
@@ -348,8 +344,8 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
if (!memcmp(ref->name, "refs/", 5) &&
check_ref_format(ref->name + 5))
; /* trash */
- else if (fetch_all &&
- (!depth || prefixcmp(ref->name, "refs/tags/") )) {
+ else if (args.fetch_all &&
+ (!args.depth || prefixcmp(ref->name, "refs/tags/") )) {
*newtail = ref;
ref->next = NULL;
newtail = &ref->next;
@@ -365,7 +361,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
free(ref);
}
- if (!fetch_all) {
+ if (!args.fetch_all) {
int i;
for (i = 0; i < nr_match; i++) {
ref = return_refs[i];
@@ -408,7 +404,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
}
}
- if (!depth) {
+ if (!args.depth) {
for_each_ref(mark_complete, NULL);
if (cutoff)
mark_recent_complete_commits(cutoff);
@@ -442,7 +438,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
o = lookup_object(remote);
if (!o || !(o->flags & COMPLETE)) {
retval = 0;
- if (!verbose)
+ if (!args.verbose)
continue;
fprintf(stderr,
"want %s (%s)\n", sha1_to_hex(remote),
@@ -451,7 +447,7 @@ static int everything_local(struct ref **refs, int nr_match, char **match)
}
hashcpy(ref->new_sha1, local);
- if (!verbose)
+ if (!args.verbose)
continue;
fprintf(stderr,
"already have %s (%s)\n", sha1_to_hex(remote),
@@ -502,14 +498,14 @@ static int get_pack(int xd[2], char **pack_lockfile)
char keep_arg[256];
char hdr_arg[256];
const char **av;
- int do_keep = keep_pack;
+ int do_keep = args.keep_pack;
int keep_pipe[2];
side_pid = setup_sideband(fd, xd);
av = argv;
*hdr_arg = 0;
- if (unpack_limit) {
+ if (!args.keep_pack && unpack_limit) {
struct pack_header header;
if (read_pack_header(fd[0], &header))
@@ -527,11 +523,11 @@ static int get_pack(int xd[2], char **pack_lockfile)
die("fetch-pack: pipe setup failure: %s", strerror(errno));
*av++ = "index-pack";
*av++ = "--stdin";
- if (!quiet && !no_progress)
+ if (!args.quiet && !args.no_progress)
*av++ = "-v";
- if (use_thin_pack)
+ if (args.use_thin_pack)
*av++ = "--fix-thin";
- if (keep_pack > 1 || unpack_limit) {
+ if (args.lock_pack || unpack_limit) {
int s = sprintf(keep_arg,
"--keep=fetch-pack %d on ", getpid());
if (gethostname(keep_arg + s, sizeof(keep_arg) - s))
@@ -541,7 +537,7 @@ static int get_pack(int xd[2], char **pack_lockfile)
}
else {
*av++ = "unpack-objects";
- if (quiet)
+ if (args.quiet)
*av++ = "-q";
}
if (*hdr_arg)
@@ -599,17 +595,17 @@ static struct ref *do_fetch_pack(int fd[2],
if (is_repository_shallow() && !server_supports("shallow"))
die("Server does not support shallow clients");
if (server_supports("multi_ack")) {
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "Server supports multi_ack\n");
multi_ack = 1;
}
if (server_supports("side-band-64k")) {
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "Server supports side-band-64k\n");
use_sideband = 2;
}
else if (server_supports("side-band")) {
- if (verbose)
+ if (args.verbose)
fprintf(stderr, "Server supports side-band\n");
use_sideband = 1;
}
@@ -622,7 +618,7 @@ static struct ref *do_fetch_pack(int fd[2],
goto all_done;
}
if (find_common(fd, sha1, ref) < 0)
- if (keep_pack != 1)
+ if (!args.keep_pack)
/* When cloning, it is not unusual to have
* no common commit.
*/
@@ -674,22 +670,6 @@ static int fetch_pack_config(const char *var, const char *value)
static struct lock_file lock;
-void setup_fetch_pack(struct fetch_pack_args *args)
-{
- uploadpack = args->uploadpack;
- quiet = args->quiet;
- keep_pack = args->keep_pack;
- if (args->unpacklimit >= 0)
- unpack_limit = args->unpacklimit;
- if (args->keep_pack)
- unpack_limit = 0;
- use_thin_pack = args->use_thin_pack;
- fetch_all = args->fetch_all;
- verbose = args->verbose;
- depth = args->depth;
- no_progress = args->no_progress;
-}
-
int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
{
int i, ret, nr_heads;
@@ -710,40 +690,40 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
if (*arg == '-') {
if (!prefixcmp(arg, "--upload-pack=")) {
- uploadpack = arg + 14;
+ args.uploadpack = arg + 14;
continue;
}
if (!prefixcmp(arg, "--exec=")) {
- uploadpack = arg + 7;
+ args.uploadpack = arg + 7;
continue;
}
if (!strcmp("--quiet", arg) || !strcmp("-q", arg)) {
- quiet = 1;
+ args.quiet = 1;
continue;
}
if (!strcmp("--keep", arg) || !strcmp("-k", arg)) {
- keep_pack++;
- unpack_limit = 0;
+ args.lock_pack = args.keep_pack;
+ args.keep_pack = 1;
continue;
}
if (!strcmp("--thin", arg)) {
- use_thin_pack = 1;
+ args.use_thin_pack = 1;
continue;
}
if (!strcmp("--all", arg)) {
- fetch_all = 1;
+ args.fetch_all = 1;
continue;
}
if (!strcmp("-v", arg)) {
- verbose = 1;
+ args.verbose = 1;
continue;
}
if (!prefixcmp(arg, "--depth=")) {
- depth = strtol(arg + 8, NULL, 0);
+ args.depth = strtol(arg + 8, NULL, 0);
continue;
}
if (!strcmp("--no-progress", arg)) {
- no_progress = 1;
+ args.no_progress = 1;
continue;
}
usage(fetch_pack_usage);
@@ -756,8 +736,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
if (!dest)
usage(fetch_pack_usage);
- ref = fetch_pack(dest, nr_heads, heads, NULL);
-
+ ref = fetch_pack(&args, dest, nr_heads, heads, NULL);
ret = !ref;
while (ref) {
@@ -769,7 +748,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
return ret;
}
-struct ref *fetch_pack(const char *dest,
+struct ref *fetch_pack(struct fetch_pack_args *my_args,
+ const char *dest,
int nr_heads,
char **heads,
char **pack_lockfile)
@@ -780,13 +760,14 @@ struct ref *fetch_pack(const char *dest,
struct ref *ref;
struct stat st;
- if (depth > 0) {
+ memcpy(&args, my_args, sizeof(args));
+ if (args.depth > 0) {
if (stat(git_path("shallow"), &st))
st.st_mtime = 0;
}
pid = git_connect(fd, (char *)dest, uploadpack,
- verbose ? CONNECT_VERBOSE : 0);
+ args.verbose ? CONNECT_VERBOSE : 0);
if (pid < 0)
return NULL;
if (heads && nr_heads)
@@ -809,7 +790,7 @@ struct ref *fetch_pack(const char *dest,
}
}
- if (!ret && depth > 0) {
+ if (!ret && args.depth > 0) {
struct cache_time mtime;
char *shallow = git_path("shallow");
int fd;
diff --git a/fetch-pack.h b/fetch-pack.h
index ad13076..a7888ea 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -8,14 +8,17 @@ struct fetch_pack_args
int depth;
unsigned quiet:1,
keep_pack:1,
+ lock_pack:1,
use_thin_pack:1,
fetch_all:1,
verbose:1,
no_progress:1;
};
-void setup_fetch_pack(struct fetch_pack_args *args);
-
-struct ref *fetch_pack(const char *dest, int nr_heads, char **heads, char **pack_lockfile);
+struct ref *fetch_pack(struct fetch_pack_args *args,
+ const char *dest,
+ int nr_heads,
+ char **heads,
+ char **pack_lockfile);
#endif
diff --git a/transport.c b/transport.c
index d8458dc..85f5b1e 100644
--- a/transport.c
+++ b/transport.c
@@ -314,21 +314,18 @@ static int fetch_refs_via_pack(struct transport *transport,
struct fetch_pack_args args;
int i;
+ memset(&args, 0, sizeof(args));
args.uploadpack = data->uploadpack;
- args.quiet = 0;
args.keep_pack = data->keep;
+ args.lock_pack = 1;
args.unpacklimit = data->unpacklimit;
args.use_thin_pack = data->thin;
- args.fetch_all = 0;
args.verbose = transport->verbose;
args.depth = data->depth;
- args.no_progress = 0;
-
- setup_fetch_pack(&args);
for (i = 0; i < nr_heads; i++)
origh[i] = heads[i] = xstrdup(to_fetch[i]->name);
- refs = fetch_pack(dest, nr_heads, heads, &transport->pack_lockfile);
+ refs = fetch_pack(&args, dest, nr_heads, heads, &transport->pack_lockfile);
for (i = 0; i < nr_heads; i++)
free(origh[i]);
--
1.5.3.1.195.gadd6
^ permalink raw reply related
* [PATCH 2/5] Refactor struct transport_ops inlined into struct transport
From: Shawn O. Pearce @ 2007-09-19 4:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Aside from reducing the code by 20 lines this refactoring removes
a level of indirection when trying to access the operations of a
given transport "instance", making the code clearer and easier to
follow.
It also has the nice effect of giving us the benefits of C99 style
struct initialization (namely ".fetch = X") without requiring that
level of language support from our compiler. We don't need to worry
about new operation methods being added as they will now be NULL'd
out automatically by the xcalloc() we use to create the new struct
transport we supply to the caller.
This pattern already exists in struct walker, so we already have
a precedent for it in Git. We also don't really need to worry
about any sort of performance decreases that may occur as a result
of filling out 4-8 op pointers when we make a "struct transport".
The extra few CPU cycles this requires over filling in the "struct
transport_ops" is killed by the time it will take Git to actually
*use* one of those functions, as most transport operations are
going over the wire or will be copying object data locally between
two directories.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-fetch.c | 3 +-
transport.c | 62 +++++++++++++++++++++---------------------------------
transport.h | 16 ++++---------
3 files changed, 30 insertions(+), 51 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index 997a8ff..2f639cc 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -392,8 +392,7 @@ static int do_fetch(struct transport *transport,
if (transport->remote->fetch_tags == -1)
no_tags = 1;
- if (!transport->ops || !transport->ops->get_refs_list ||
- !transport->ops->fetch)
+ if (!transport->get_refs_list || !transport->fetch)
die("Don't know how to fetch from %s", transport->url);
/* if not appending, truncate FETCH_HEAD */
diff --git a/transport.c b/transport.c
index cc76e3f..d8458dc 100644
--- a/transport.c
+++ b/transport.c
@@ -44,8 +44,6 @@ static int disconnect_walker(struct transport *transport)
return 0;
}
-static const struct transport_ops rsync_transport;
-
static int curl_transport_push(struct transport *transport, int refspec_nr, const char **refspec, int flags) {
const char **argv;
int argc;
@@ -199,14 +197,6 @@ static int fetch_objs_via_curl(struct transport *transport,
#endif
-static const struct transport_ops curl_transport = {
- /* set_option */ NULL,
- /* get_refs_list */ get_refs_via_curl,
- /* fetch */ fetch_objs_via_curl,
- /* push */ curl_transport_push,
- /* disconnect */ disconnect_walker
-};
-
struct bundle_transport_data {
int fd;
struct bundle_header header;
@@ -249,14 +239,6 @@ static int close_bundle(struct transport *transport)
return 0;
}
-static const struct transport_ops bundle_transport = {
- /* set_option */ NULL,
- /* get_refs_list */ get_refs_from_bundle,
- /* fetch */ fetch_refs_from_bundle,
- /* push */ NULL,
- /* disconnect */ close_bundle
-};
-
struct git_transport_data {
unsigned thin : 1;
unsigned keep : 1;
@@ -401,13 +383,6 @@ static int git_transport_push(struct transport *transport, int refspec_nr, const
return !!err;
}
-static const struct transport_ops git_transport = {
- /* set_option */ set_git_option,
- /* get_refs_list */ get_refs_via_connect,
- /* fetch */ fetch_refs_via_pack,
- /* push */ git_transport_push
-};
-
static int is_local(const char *url)
{
const char *colon = strchr(url, ':');
@@ -431,18 +406,31 @@ struct transport *transport_get(struct remote *remote, const char *url)
ret->url = url;
if (!prefixcmp(url, "rsync://")) {
- ret->ops = &rsync_transport;
+ /* not supported; don't populate any ops */
+
} else if (!prefixcmp(url, "http://")
|| !prefixcmp(url, "https://")
|| !prefixcmp(url, "ftp://")) {
- ret->ops = &curl_transport;
+ ret->get_refs_list = get_refs_via_curl;
+ ret->fetch = fetch_objs_via_curl;
+ ret->push = curl_transport_push;
+ ret->disconnect = disconnect_walker;
+
} else if (is_local(url) && is_file(url)) {
struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
ret->data = data;
- ret->ops = &bundle_transport;
+ ret->get_refs_list = get_refs_from_bundle;
+ ret->fetch = fetch_refs_from_bundle;
+ ret->disconnect = close_bundle;
+
} else {
struct git_transport_data *data = xcalloc(1, sizeof(*data));
ret->data = data;
+ ret->set_option = set_git_option;
+ ret->get_refs_list = get_refs_via_connect;
+ ret->fetch = fetch_refs_via_pack;
+ ret->push = git_transport_push;
+
data->thin = 1;
data->uploadpack = "git-upload-pack";
if (remote && remote->uploadpack)
@@ -451,7 +439,6 @@ struct transport *transport_get(struct remote *remote, const char *url)
if (remote && remote->receivepack)
data->receivepack = remote->receivepack;
data->unpacklimit = -1;
- ret->ops = &git_transport;
}
return ret;
@@ -460,24 +447,23 @@ struct transport *transport_get(struct remote *remote, const char *url)
int transport_set_option(struct transport *transport,
const char *name, const char *value)
{
- if (transport->ops->set_option)
- return transport->ops->set_option(transport, name, value);
+ if (transport->set_option)
+ return transport->set_option(transport, name, value);
return 1;
}
int transport_push(struct transport *transport,
int refspec_nr, const char **refspec, int flags)
{
- if (!transport->ops->push)
+ if (!transport->push)
return 1;
- return transport->ops->push(transport, refspec_nr, refspec, flags);
+ return transport->push(transport, refspec_nr, refspec, flags);
}
struct ref *transport_get_remote_refs(struct transport *transport)
{
if (!transport->remote_refs)
- transport->remote_refs =
- transport->ops->get_refs_list(transport);
+ transport->remote_refs = transport->get_refs_list(transport);
return transport->remote_refs;
}
@@ -496,7 +482,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs)
heads[nr_heads++] = rm;
}
- rc = transport->ops->fetch(transport, nr_heads, heads);
+ rc = transport->fetch(transport, nr_heads, heads);
free(heads);
return rc;
}
@@ -513,8 +499,8 @@ void transport_unlock_pack(struct transport *transport)
int transport_disconnect(struct transport *transport)
{
int ret = 0;
- if (transport->ops->disconnect)
- ret = transport->ops->disconnect(transport);
+ if (transport->disconnect)
+ ret = transport->disconnect(transport);
free(transport);
return ret;
}
diff --git a/transport.h b/transport.h
index 6a95d66..3e332ff 100644
--- a/transport.h
+++ b/transport.h
@@ -5,22 +5,11 @@
#include "remote.h"
struct transport {
- unsigned verbose : 1;
struct remote *remote;
const char *url;
-
void *data;
-
struct ref *remote_refs;
- const struct transport_ops *ops;
- char *pack_lockfile;
-};
-
-#define TRANSPORT_PUSH_ALL 1
-#define TRANSPORT_PUSH_FORCE 2
-
-struct transport_ops {
/**
* Returns 0 if successful, positive if the option is not
* recognized or is inapplicable, and negative if the option
@@ -34,8 +23,13 @@ struct transport_ops {
int (*push)(struct transport *connection, int refspec_nr, const char **refspec, int flags);
int (*disconnect)(struct transport *connection);
+ char *pack_lockfile;
+ unsigned verbose : 1;
};
+#define TRANSPORT_PUSH_ALL 1
+#define TRANSPORT_PUSH_FORCE 2
+
/* Returns a transport suitable for the url */
struct transport *transport_get(struct remote *, const char *);
--
1.5.3.1.195.gadd6
^ permalink raw reply related
* [PATCH 1/5] Rename remote.uri to remote.url within remote handling internals
From: Shawn O. Pearce @ 2007-09-19 4:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Anyplace we talk about the address of a remote repository we always
refer to it as a URL, especially in the configuration file and
.git/remotes where we call it "remote.$n.url" or start the first
line with "URL:". Calling this value a uri within the internal C
code just doesn't jive well with our commonly accepted terms.
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
builtin-fetch.c | 2 +-
builtin-push.c | 8 ++++----
remote.c | 34 +++++++++++++++++-----------------
remote.h | 6 +++---
send-pack.c | 2 +-
5 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/builtin-fetch.c b/builtin-fetch.c
index b9722e5..997a8ff 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -530,7 +530,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
else
remote = remote_get(argv[i++]);
- transport = transport_get(remote, remote->uri[0]);
+ transport = transport_get(remote, remote->url[0]);
if (verbose >= 2)
transport->verbose = 1;
if (quiet)
diff --git a/builtin-push.c b/builtin-push.c
index 7d7e826..4ee36c2 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -57,9 +57,9 @@ static int do_push(const char *repo, int flags)
refspec_nr = remote->push_refspec_nr;
}
errs = 0;
- for (i = 0; i < remote->uri_nr; i++) {
+ for (i = 0; i < remote->url_nr; i++) {
struct transport *transport =
- transport_get(remote, remote->uri[i]);
+ transport_get(remote, remote->url[i]);
int err;
if (receivepack)
transport_set_option(transport,
@@ -68,14 +68,14 @@ static int do_push(const char *repo, int flags)
transport_set_option(transport, TRANS_OPT_THIN, "yes");
if (verbose)
- fprintf(stderr, "Pushing to %s\n", remote->uri[i]);
+ fprintf(stderr, "Pushing to %s\n", remote->url[i]);
err = transport_push(transport, refspec_nr, refspec, flags);
err |= transport_disconnect(transport);
if (!err)
continue;
- error("failed to push to '%s'", remote->uri[i]);
+ error("failed to push to '%s'", remote->url[i]);
errs++;
}
return !!errs;
diff --git a/remote.c b/remote.c
index 31e2b70..e3c3df5 100644
--- a/remote.c
+++ b/remote.c
@@ -32,13 +32,13 @@ static void add_fetch_refspec(struct remote *remote, const char *ref)
remote->fetch_refspec_nr = nr;
}
-static void add_uri(struct remote *remote, const char *uri)
+static void add_url(struct remote *remote, const char *url)
{
- int nr = remote->uri_nr + 1;
- remote->uri =
- xrealloc(remote->uri, nr * sizeof(char *));
- remote->uri[nr-1] = uri;
- remote->uri_nr = nr;
+ int nr = remote->url_nr + 1;
+ remote->url =
+ xrealloc(remote->url, nr * sizeof(char *));
+ remote->url[nr-1] = url;
+ remote->url_nr = nr;
}
static struct remote *make_remote(const char *name, int len)
@@ -154,7 +154,7 @@ static void read_remotes_file(struct remote *remote)
switch (value_list) {
case 0:
- add_uri(remote, xstrdup(s));
+ add_url(remote, xstrdup(s));
break;
case 1:
add_push_refspec(remote, xstrdup(s));
@@ -206,7 +206,7 @@ static void read_branches_file(struct remote *remote)
} else {
branch = "refs/heads/master";
}
- add_uri(remote, p);
+ add_url(remote, p);
add_fetch_refspec(remote, branch);
remote->fetch_tags = 1; /* always auto-follow */
}
@@ -260,7 +260,7 @@ static int handle_config(const char *key, const char *value)
return 0; /* ignore unknown booleans */
}
if (!strcmp(subkey, ".url")) {
- add_uri(remote, xstrdup(value));
+ add_url(remote, xstrdup(value));
} else if (!strcmp(subkey, ".push")) {
add_push_refspec(remote, xstrdup(value));
} else if (!strcmp(subkey, ".fetch")) {
@@ -347,14 +347,14 @@ struct remote *remote_get(const char *name)
name = default_remote_name;
ret = make_remote(name, 0);
if (name[0] != '/') {
- if (!ret->uri)
+ if (!ret->url)
read_remotes_file(ret);
- if (!ret->uri)
+ if (!ret->url)
read_branches_file(ret);
}
- if (!ret->uri)
- add_uri(ret, name);
- if (!ret->uri)
+ if (!ret->url)
+ add_url(ret, name);
+ if (!ret->url)
return NULL;
ret->fetch = parse_ref_spec(ret->fetch_refspec_nr, ret->fetch_refspec);
ret->push = parse_ref_spec(ret->push_refspec_nr, ret->push_refspec);
@@ -380,11 +380,11 @@ int for_each_remote(each_remote_fn fn, void *priv)
return result;
}
-int remote_has_uri(struct remote *remote, const char *uri)
+int remote_has_url(struct remote *remote, const char *url)
{
int i;
- for (i = 0; i < remote->uri_nr; i++) {
- if (!strcmp(remote->uri[i], uri))
+ for (i = 0; i < remote->url_nr; i++) {
+ if (!strcmp(remote->url[i], url))
return 1;
}
return 0;
diff --git a/remote.h b/remote.h
index b5b558f..05add06 100644
--- a/remote.h
+++ b/remote.h
@@ -4,8 +4,8 @@
struct remote {
const char *name;
- const char **uri;
- int uri_nr;
+ const char **url;
+ int url_nr;
const char **push_refspec;
struct refspec *push;
@@ -32,7 +32,7 @@ struct remote *remote_get(const char *name);
typedef int each_remote_fn(struct remote *remote, void *priv);
int for_each_remote(each_remote_fn fn, void *priv);
-int remote_has_uri(struct remote *remote, const char *uri);
+int remote_has_url(struct remote *remote, const char *url);
struct refspec {
unsigned force : 1;
diff --git a/send-pack.c b/send-pack.c
index f74e66a..4533d1b 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -420,7 +420,7 @@ int main(int argc, char **argv)
if (remote_name) {
remote = remote_get(remote_name);
- if (!remote_has_uri(remote, dest)) {
+ if (!remote_has_url(remote, dest)) {
die("Destination %s is not a uri for %s",
dest, remote_name);
}
--
1.5.3.1.195.gadd6
^ permalink raw reply related
* [PATCH 0/5] Yet another builtin-fetch round
From: Shawn O. Pearce @ 2007-09-19 4:49 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Another short series for db/fetch-pack, still in pu. Aside from
optimizing the pipeline on the native transport (so we only invoke
the remote process we need once vs. twice) I'm actually now quite
comfortable with this whole series and think it is ready for next.
I'm certainly running it in production, and will be until it is
merged. The performance difference is too big for me (and at least
some of my coworkers) to not be doing so. If there are any specific
reasons why this topic is not ready for next or is unsuitable for
merging please let me know so I can take the time to correct it.
1/5 Rename remote.uri to remote.url within remote handling internals
2/5 Refactor struct transport_ops inlined into struct transport
3/5 Always obtain fetch-pack arguments from struct fetch_pack_args
These three are basic code cleanups for small issues that
bothered me about the original implementation of builtin-fetch.
Now is just as good of a time as any to cleanup the code and make
it more maintainable. I think the overall total line count is
reduced by these three patches.
* Ensure builtin-fetch honors {fetch,transfer}.unpackLimit
* Fix memory leaks when disconnecting transport instances
Fixes two known (but minor) outstanding bugs. At this point
I do not know of any other bugs in builtin-fetch so I would
really appreciate testing reports from other people, especially
those whose uses cases might stray outside of my workflow. Hah,
I did not tell you my workflow. ;-)
--
Shawn.
^ permalink raw reply
* Re: [PATCH] instaweb: added support Ruby's WEBrick server
From: Johannes Schindelin @ 2007-09-19 1:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: mike dalessio, git, normalperson
In-Reply-To: <7vodfztviv.fsf@gitster.siamese.dyndns.org>
Hi,
On Tue, 18 Sep 2007, Junio C Hamano wrote:
> I wonder how popular instaweb is and how widely it is used. I've
> actually wondering if we should demote it to contrib/ somewhere, but it
> gets occasional updates so people must be using it...
I have to admit that I found it easier to install gitweb manually,
especially with recent documentation enhancements, _and_ the constraint
that I had to use an existing DocumentRoot.
So I would not be opposed to move this into contrib/.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.
From: Pierre Habouzit @ 2007-09-19 1:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vabrjtqw7.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 846 bytes --]
On Wed, Sep 19, 2007 at 12:55:52AM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
>
> > On Tue, Sep 18, 2007 at 10:00:51PM +0000, Pierre Habouzit wrote:
> >> + cp = strchr(qname.buf + qname.len + 3 - max, '/');
> >> + if (cp)
> >> + cp = qname.buf + qname.len + 3 - max;
> >
> > OMG, this is supposed to be if (!cp) of course...
> >
> > I wonder how this passed the testsuite.
>
> You would need a new test, I guess, before a huge rewrite.
OTOH this is in the code that generates the diffstats, it's not _that_
surprising that we don't have extensive tests about that, as it's not
critical in git afaict ;)
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: Git tree for old kernels from before the current tree
From: Oleg Verych @ 2007-09-19 1:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Nicolas Pitre, H. Peter Anvin, Jan Engelhardt, Paul Mundt,
Jon Smirl, Git Mailing List, lkml
In-Reply-To: <alpine.LFD.0.999.0707231057480.3607@woody.linux-foundation.org>
* Mon, 23 Jul 2007 11:02:39 -0700 (PDT)
>
> On Mon, 23 Jul 2007, Nicolas Pitre wrote:
>>
>> I started this once.
>>
>> I have (sort of) a GIT tree with all Linux revisions that I could find
>> from v0.01 up to v1.0.9. But the most interesting information and also
>> what is the most time consuming is the retrieval of announcement
>> messages for those releases in old mailing list or newsgroup archives to
>> serve as commit log data. It seems to be even arder to find for post
>> v1.0 releases.
>
> Yes, I agree. Google finds some of them, but (a) I was never very good
> about announcements anyway and (b) there's nothing really good to search
> for, so it's very hit-and-miss.
>
> Some of the really early release notes are easy to find, just because I
> made them available with the sources, but mostly I'd just have posten to
> the newsgroup/mailing lists.
Maybe this can be useful somehow: ftp://ftp.shout.net/pub/users/mec/kcs
____
^ permalink raw reply
* Re: [PATCH 4/5] Full rework of quote_c_style and write_name_quoted.
From: Junio C Hamano @ 2007-09-19 0:55 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: git
In-Reply-To: <20070919000757.GC4535@artemis.corp>
Pierre Habouzit <madcoder@debian.org> writes:
> On Tue, Sep 18, 2007 at 10:00:51PM +0000, Pierre Habouzit wrote:
>> + cp = strchr(qname.buf + qname.len + 3 - max, '/');
>> + if (cp)
>> + cp = qname.buf + qname.len + 3 - max;
>
> OMG, this is supposed to be if (!cp) of course...
>
> I wonder how this passed the testsuite.
You would need a new test, I guess, before a huge rewrite.
^ permalink raw reply
* Re: [PATCH] instaweb: added support Ruby's WEBrick server
From: David Symonds @ 2007-09-19 0:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: mike dalessio, git, normalperson
In-Reply-To: <7vodfztviv.fsf@gitster.siamese.dyndns.org>
On 19/09/2007, Junio C Hamano <gitster@pobox.com> wrote:
> - I do not like extra whitespace between she-bang #! and the
> path. Looks very ugly.
At least according to the Autoconf manual, there are some UNIXes that
require a space after the shebang. Not that I've seen such a system.
Personally, I find it easier to read with the space.
Dave.
^ permalink raw reply
* Re: State of Perforce importing.
From: David Brown @ 2007-09-19 0:27 UTC (permalink / raw)
To: Sam Vilain; +Cc: Git
In-Reply-To: <46F06C0C.8090201@vilain.net>
On Wed, Sep 19, 2007 at 12:23:40PM +1200, Sam Vilain wrote:
>David Brown wrote:
>>
>> An additional problem:
>>
>> - git-p4 doesn't preserve the execute permission bit from Perforce.
>
>FWIW I found that bit on bit 9 of the 'file type' flag in the db, which
>is the third column in the "db.rev" table. It's used to come up with
>the silly names like "text" vs "xtext" (difference? well, one's
>executable of course).
It does come back in the 'kind' field when it asks the client for the file
type. I'll look into using that information to set the execute bit in the
mode it sends off.
Dave
^ permalink raw reply
* Re: State of Perforce importing.
From: David Brown @ 2007-09-19 0:26 UTC (permalink / raw)
To: Sam Vilain; +Cc: Reece Dunn, Git
In-Reply-To: <46F06B5C.2050207@vilain.net>
On Wed, Sep 19, 2007 at 12:20:44PM +1200, Sam Vilain wrote:
>If you can get a hold of the "checkpoint" and "journal" files, you could
>probably throw the client spec data into a few Pg tables, chuck a couple
>of constraints on it to confirm that it works the way you thought, and
>then get the information on what's where using a SQL query. The file
>images themselves can come from wherever, it doesn't really matter
>because there are MD5 hashes in the data tables you can use to confirm
>you got the right file.
In my instance, I don't have an account on the P4 server, so I'm going to
have to deal with this through P4's normal client.
I don't have much confidence that P4 really knows what it is doing when it
comes to integrate, so I'm not much worried about branches. But, I do want
to accurately get the history of a particular branch.
So far, the only thing that isn't working is the execute bit doesn't get
set on files that should have it.
Dave
^ permalink raw reply
* Re: State of Perforce importing.
From: Sam Vilain @ 2007-09-19 0:23 UTC (permalink / raw)
To: Reece Dunn; +Cc: Git
In-Reply-To: <3f4fd2640709181053t70b7abcdi2c4eaf67e7b75338@mail.gmail.com>
Reece Dunn wrote:
> There is no need to create yet another Perforce importing tool, git-p4
> works well in most cases. If we focus on improving git-p4, extending
> it to support the functionality mentioned here, fix the issues that
> there are with it, then that will be more beneficial to the community
> as they will not have to learn another tool with a different set of
> bugs and issues.
I like my approach; it's clean and I think shows a tasteful level of
distrust towards the sanity and integrity of the data held by Perforce.
Actually it really helped me understand what was really going on;
because the information as displayed by for instance "p4 integrate" is
a lot more confusing than the underlying tables (IMHO).
Sam.
^ permalink raw reply
* Re: State of Perforce importing.
From: Sam Vilain @ 2007-09-19 0:23 UTC (permalink / raw)
To: David Brown; +Cc: Git
In-Reply-To: <20070918233749.GA19533@old.davidb.org>
David Brown wrote:
>
> An additional problem:
>
> - git-p4 doesn't preserve the execute permission bit from Perforce.
FWIW I found that bit on bit 9 of the 'file type' flag in the db, which
is the third column in the "db.rev" table. It's used to come up with
the silly names like "text" vs "xtext" (difference? well, one's
executable of course).
Sam.
^ permalink raw reply
* Re: State of Perforce importing.
From: Sam Vilain @ 2007-09-19 0:20 UTC (permalink / raw)
To: Reece Dunn; +Cc: Git
In-Reply-To: <20070918231921.GA17652@old.davidb.org>
David Brown wrote:
> On Tue, Sep 18, 2007 at 06:53:45PM +0100, Reece Dunn wrote:
>
>> The main issues with using client workspaces is that they require you
>> to use `p4 sync`, whereas git-p4 uses `p4 print` and that they may
>> change as the repository changes, but Perforce does not track these
>> changes.
>
> Unfortunately, we have one project that heavily abuses P4 client specs.
> For every release, someone creates a >900 line client spec and labels the
> files in it. Those are the versions that need to get checked in, and
> without rewriting much of what P4 does, I'm going to have to let P4 do the
> syncing and checking out.
If you can get a hold of the "checkpoint" and "journal" files, you could
probably throw the client spec data into a few Pg tables, chuck a couple
of constraints on it to confirm that it works the way you thought, and
then get the information on what's where using a SQL query. The file
images themselves can come from wherever, it doesn't really matter
because there are MD5 hashes in the data tables you can use to confirm
you got the right file.
I haven't looked at importing the client spec because it's not important
for the project I'm importing. But I'd be happy to provide pointers.
>> I would not do that. It is a good idea to keep the original log
>> messages, even if it does make for an uninformative shortlog. Look at
>> some of the CVS/SVN imported logs!
> I think what I want then is something to filter between 'git log' and 'git
> shortlog' that would find a summary line in the commit message and copy it
> to the top. It wouldn't change the history, but clean it up for shortlog's
> purpose.
Sounds like a job for a templating git-log porcelain.
Sam.
^ 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