* Re: git-p4 fails when cloning a p4 depo.
From: Benjamin Sergeant @ 2007-06-09 0:32 UTC (permalink / raw)
To: hanwen; +Cc: git, Scott Lamb
In-Reply-To: <4669E73F.2040702@xs4all.nl>
On 6/8/07, Han-Wen Nienhuys <hanwen@xs4all.nl> wrote:
> Benjamin Sergeant escreveu:
>
> > So are you saying that in the old days, git-p4 was importing the p4
> > depo in small slices to not overkill the process memory (in case the
> > depo is big) ?
>
> no, in the "old days" git-p4 used a separate p4 invocation for each file.
>
Anyway, in case you hit command line lenght limit here it is. That
might be interesting for the "next days" :)
Benjamin.
[bsergean@flanders fast-export]$ git format-patch -k -m --stdout origin
From 45f2dbdb9a8c0b3beb007ae892613cdc4afab80a Mon Sep 17 00:00:00 2001
From: Benjamin Sergeant <bsergean@flanders.(none)>
Date: Fri, 8 Jun 2007 09:58:57 -0700
Subject: Split p4 print call into multiple call to not exceed the
command line lenght maximum
---
git-p4 | 21 ++++++++++++++++++---
1 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/git-p4 b/git-p4
index 36fe69a..906b193 100755
--- a/git-p4
+++ b/git-p4
@@ -703,9 +703,22 @@ class P4Sync(Command):
if not files:
return
- filedata = p4CmdList('print %s' % ' '.join(['"%s#%s"' % (f['path'],
- f['rev'])
- for f in files]))
+ # We cannot put all the files on the command line
+ # OS have limitations on the max lenght of arguments
+ # POSIX says it's 4096 bytes, default for Linux seems to be 130 K.
+ # and all OS from the table below seems to be higher than POSIX.
+ # See http://www.in-ulm.de/~mascheck/various/argmax/
+ chunk = ''
+ filedata = []
+ for i in xrange(len(files)):
+ f = files[i]
+ chunk += '"%s#%s" ' % (f['path'], f['rev'])
+ if len(chunk) > 4000 or i == len(files)-1:
+ data = p4CmdList('print %s' % chunk)
+ if "p4ExitCode" in data[0]:
+ die("Problems executing p4. Error: [%d]." %
(data[0]['p4ExitCode']));
+ filedata.extend(data)
+ chunk = ''
j = 0;
contents = {}
@@ -1486,3 +1499,5 @@ def main():
if __name__ == '__main__':
main()
+
+# vim: set filetype=python sts=4 sw=4 et si :
--
1.5.0.4
>From dd9975708433efeec37b608755f54fbeaedf0f3f Mon Sep 17 00:00:00 2001
From: Benjamin Sergeant <bsergean@flanders.(none)>
Date: Fri, 8 Jun 2007 10:20:39 -0700
Subject: Use os.sysconf('SC_ARG_MAX') to retrieve the max value, and
build the string using join (faster)
---
git-p4 | 17 ++++++++++-------
1 files changed, 10 insertions(+), 7 deletions(-)
diff --git a/git-p4 b/git-p4
index 906b193..8dc1963 100755
--- a/git-p4
+++ b/git-p4
@@ -705,20 +705,23 @@ class P4Sync(Command):
# We cannot put all the files on the command line
# OS have limitations on the max lenght of arguments
- # POSIX says it's 4096 bytes, default for Linux seems to be 130 K.
- # and all OS from the table below seems to be higher than POSIX.
# See http://www.in-ulm.de/~mascheck/various/argmax/
- chunk = ''
+ chunks = []
+ chunkLenght = 0
filedata = []
+ maxlenght = max(int(os.sysconf('SC_ARG_MAX') * 0.90), 4000)
+ print maxlenght
for i in xrange(len(files)):
f = files[i]
- chunk += '"%s#%s" ' % (f['path'], f['rev'])
- if len(chunk) > 4000 or i == len(files)-1:
- data = p4CmdList('print %s' % chunk)
+ chunkLenght += len(f['path']) + len(f['rev'])
+ chunks.append('"%s#%s" ' % (f['path'], f['rev']))
+ if chunkLenght > maxlenght or i == len(files)-1:
+ data = p4CmdList('print %s' % ' '.join(chunks))
if "p4ExitCode" in data[0]:
die("Problems executing p4. Error: [%d]." %
(data[0]['p4ExitCode']));
filedata.extend(data)
- chunk = ''
+ chunks = []
+ chunkLenght = 0
j = 0;
contents = {}
--
1.5.0.4
^ permalink raw reply related
* Re: [PATCH] Port git-tag.sh to C.
From: Carlos Rica @ 2007-06-09 1:58 UTC (permalink / raw)
To: Kristian Høgsberg; +Cc: git
In-Reply-To: <11813427591137-git-send-email-krh@redhat.com>
2007/6/9, Kristian Høgsberg <krh@redhat.com>:
> Ok, here's an updated version that passes the test suite. Johannes, I
> leave it to you and jasam to merge the bits you find useful, but as
> far as I see it, this conversion is complete, and there's enough other
> shell scripts to port. My port doesn't pass jasam's test suite, it
> looks like he is expecting the -l glob to be a regexp, but the
> git-tag.sh I started from used shell globs.
That's a good proposal! I agree with your version for matching tag names,
apart from the fact that the code is more compact and readable that way.
I will review your code comparing it with mine and I will report
everything I feel. However, my desire is to get builtin-tag.c and its
tests t7400-tag.sh complete
in a few days or less, so I would appreciate every other correction
from you in order to get it ready faster.
> Anyways, it'd be nice if you or jasam could keep the list a little
> more in the loop with the SoC changes, it is where most of the
> development happens, after all. What's next on your list?
You're right, the development will be more open than now was, I
usually ask questions in the IRC channel and obviously that's not the
best way to report my work to everybody.
Feel free to choose the script which you need to get replaced first,
or, depending on your urgency, you could ask me for one of them and I
would try to concentrate my efforts on it. Why do you started with
git-tag? For me, it was enough easy to begin with, perhaps you could
have other reasons.
Looking forward to work with you on that,
Carlos
^ permalink raw reply
* Re: git-log fatal error in empty repo
From: Geert Bosch @ 2007-06-09 2:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steve Hoelzer, git
In-Reply-To: <7vps47o6eh.fsf@assigned-by-dhcp.cox.net>
On Jun 7, 2007, at 16:29, Junio C Hamano wrote:
> Maybe, but I highly doubt if it is worth to bother about it.
My first confusion with git was exactly this issue.
It's very normal for a new user to do a git init
and than try to verify that the current state is a sane
empty repository.
So, git log, git status and git fsck should all properly
handle this situation and give reasonable output in this case.
-Geert
^ permalink raw reply
* Re: [PATCH 2/2] filter-branch: subdirectory filter needs --full-history
From: Linus Torvalds @ 2007-06-09 2:40 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git, Johannes Schindelin
In-Reply-To: <200706082328.50923.johannes.sixt@telecom.at>
On Fri, 8 Jun 2007, Johannes Sixt wrote:
>
> When two branches are merged that modify a subdirectory (possibly in
> different intermediate steps) such that both end up identical, then
> rev-list chooses only one branch. But when we filter history, we want to
> keep both branches. Therefore, we must use --full-history.
--full-history needs to be fixed up for this, I think.
It leaves *too* many merges around, in particular, it leaves merges where
both parents end up (after simplification) being related to each other.
As an example, do this:
mkdir hello
cd hello/
git init
echo "Initial state" > file-A
echo "Another initial state" > file-B
git add file-A file-B
git commit -m "Initial commit"
echo "Add a line" >> file-A
echo "Add another line" >> file-B
git commit -a -m "On master branch"
git checkout -b another HEAD^
echo "Add a line" >> file-A
git commit -a -m "On another branch"
git checkout master
git merge another
and then do
gitk --full-history file-B
and notice what happens.. There was no actual developmet on branch
"another", so all the commits went away, but it left the merge (because
that's how --full-history works), which has now become pointless.
So you should do a "merge cleanup" phase after running --full-history.
Linus
^ permalink raw reply
* Re: [PATCH 03/21] Refactoring to make verify_tag() and parse_tag_buffer() more similar
From: Johannes Schindelin @ 2007-06-09 2:54 UTC (permalink / raw)
To: Johan Herland; +Cc: git, Junio C Hamano
In-Reply-To: <200706090213.40785.johan@herland.net>
Hi,
On Sat, 9 Jun 2007, Johan Herland wrote:
> diff --git a/mktag.c b/mktag.c
> index b82e377..0bc20c8 100644
> --- a/mktag.c
> +++ b/mktag.c
> @@ -32,52 +32,48 @@ static int verify_object(unsigned char *sha1, const char *expected_type)
> return ret;
> }
>
> +static int verify_tag(char *data, unsigned long size)
> +{
> #ifdef NO_C99_FORMAT
> #define PD_FMT "%d"
> #else
> #define PD_FMT "%td"
> #endif
>
> -static int verify_tag(char *buffer, unsigned long size)
Why this rename from buffer to data?
> -{
> - int typelen;
> - char type[20];
> unsigned char sha1[20];
> - const char *object, *type_line, *tag_line, *tagger_line;
> + char type[20];
> + const char *type_line, *tag_line, *tagger_line;
> + unsigned long type_len;
Why this change of order?
> if (size < 64)
> return error("wanna fool me ? you obviously got the size wrong !");
>
> - buffer[size] = 0;
Are you sure that your buffer is always NUL terminated?
> - type_line = object + 48;
> + type_line = data + 48;
Quite a lot of changes seem to do this object->data. The patch would have
been much more compact if you just had renamed buffer to object instead of
data.
> - typelen = tag_line - type_line - strlen("type \n");
> - if (typelen >= sizeof(type))
> - return error("char" PD_FMT ": type too long", type_line+5 - buffer);
> -
> - memcpy(type, type_line+5, typelen);
> - type[typelen] = 0;
> + type_len = tag_line - type_line - strlen("type \n");
> + if (type_len >= sizeof(type))
> + return error("char" PD_FMT ": type too long", type_line + 5 - data);
> + memcpy(type, type_line + 5, type_len);
> + type[type_len] = '\0';
This renaming variables has nothing to do with refactoring. In fact, I
have a hard time to find code changes (which your subject suggests, as you
want to make two functions more similar).
> /* TODO: check for committer info + blank line? */
> /* Also, the minimum length is probably + "tagger .", or 63+8=71 */
>
> /* The actual stuff afterwards we don't care about.. */
> return 0;
> -}
>
> #undef PD_FMT
> +}
Any particular reason for this?
> @@ -124,6 +120,7 @@ int main(int argc, char **argv)
> free(buffer);
> die("could not read from stdin");
> }
> + buffer[size] = 0;
Ah, so you terminate the buffer here. From the patch, it is relatively
hard to see if this line is always hit _before_ the function is called
that evidently relies on NUL termination. By moving it here, I think it is
much more likely to overlook the fact that the function, which made sure
that its assumption was met, needs this line. Whereas if you left it where
it was, the assumption would always be met.
> - if (item->object.parsed)
> - return 0;
> - item->object.parsed = 1;
> + if (item->object.parsed)
> + return 0;
> + item->object.parsed = 1;
Again, this has nothing to do with refactoring.
> @@ -57,39 +57,38 @@ int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
> if (memcmp(data, "object ", 7))
> return error("char%d: does not start with \"object \"", 0);
>
> - if (get_sha1_hex((char *) data + 7, sha1))
> + if (get_sha1_hex(data + 7, sha1))
Is this really necessary? Even if (which I doubt), it has nothing to do
with refactoring.
If you _want_ to _explicitely_ do arithmetic on a char* instead of void*,
why not DRT and change the function signature?
> - sig_line++;
> + tagger_line++;
I am really reluctant with renamings like these. IMHO they don't buy you
much, except for possible confusion. It is evident that sig means the
signer (and it is obvious in the case of an unsigned tag, who is meant,
too).
> +int parse_tag_buffer(struct tag *item, void *data, unsigned long size)
> +{
> + return parse_tag_buffer_internal(item, (const char *) data, size);
> +}
This cast (and indeed, this function, if you ask me) is unnecessary.
I reviewed only this patch out of your long series, mostly because I found
the subject line interesting. But IMHO the patch does not what the subject
line suggests.
Unfortunately, it's unlikely that I will have time until Monday night to
continue with this patch series.
Ciao,
Dscho
^ permalink raw reply
* [RFC][PATCH 10/10] Sparse: fix a "symbol 'weak_match' shadows an earlier one" warning
From: Ramsay Jones @ 2007-06-08 22:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
connect.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/connect.c b/connect.c
index da89c9c..d4051dd 100644
--- a/connect.c
+++ b/connect.c
@@ -179,7 +179,6 @@ static int count_refspec_match(const char *pattern,
for (weak_match = match = 0; refs; refs = refs->next) {
char *name = refs->name;
int namelen = strlen(name);
- int weak_match;
if (namelen < patlen ||
memcmp(name + namelen - patlen, pattern, patlen))
--
1.5.2
^ permalink raw reply related
* [RFC][PATCH 02/10] Sparse: fix some "using integer as NULL pointer" warnings
From: Ramsay Jones @ 2007-06-08 22:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
daemon.c | 12 ++++++------
fetch-pack.c | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/daemon.c b/daemon.c
index e74ecac..b79e905 100644
--- a/daemon.c
+++ b/daemon.c
@@ -60,12 +60,12 @@ static unsigned int init_timeout;
#define INTERP_SLOT_PERCENT (5)
static struct interp interp_table[] = {
- { "%H", 0},
- { "%CH", 0},
- { "%IP", 0},
- { "%P", 0},
- { "%D", 0},
- { "%%", 0},
+ { "%H" },
+ { "%CH" },
+ { "%IP" },
+ { "%P" },
+ { "%D" },
+ { "%%" },
};
diff --git a/fetch-pack.c b/fetch-pack.c
index 06f4aec..75649a6 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -642,7 +642,7 @@ static int remove_duplicates(int nr_heads, char **heads)
heads[dst] = heads[src];
dst++;
}
- heads[dst] = 0;
+ heads[dst] = NULL;
return dst;
}
--
1.5.2
^ permalink raw reply related
* [RFC][PATCH 06/10] Sparse: fix "'add_head' not declared" warning
From: Ramsay Jones @ 2007-06-08 22:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
This function was defined in builtin-diff.c and also used in
builtin-log.c. Since it has demonstrated its utility, move the
function definition to revision.c, which is a more logical place
for it, and add a declaration to revision.h.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
builtin-diff.c | 12 ------------
builtin-log.c | 3 ---
revision.c | 12 ++++++++++++
revision.h | 1 +
4 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/builtin-diff.c b/builtin-diff.c
index 7f367b6..e00a212 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -176,18 +176,6 @@ static int builtin_diff_combined(struct rev_info *revs,
return 0;
}
-void add_head(struct rev_info *revs)
-{
- unsigned char sha1[20];
- struct object *obj;
- if (get_sha1("HEAD", sha1))
- return;
- obj = parse_object(sha1);
- if (!obj)
- return;
- add_pending_object(revs, obj, "HEAD");
-}
-
int cmd_diff(int argc, const char **argv, const char *prefix)
{
int i;
diff --git a/builtin-log.c b/builtin-log.c
index 3744712..d40a0fc 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -17,9 +17,6 @@
static int default_show_root = 1;
-/* this is in builtin-diff.c */
-void add_head(struct rev_info *revs);
-
static void add_name_decoration(const char *prefix, const char *name, struct object *obj)
{
int plen = strlen(prefix);
diff --git a/revision.c b/revision.c
index 0125d41..1bfe80c 100644
--- a/revision.c
+++ b/revision.c
@@ -129,6 +129,18 @@ void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, con
(struct commit *)obj, name);
}
+void add_head(struct rev_info *revs)
+{
+ unsigned char sha1[20];
+ struct object *obj;
+ if (get_sha1("HEAD", sha1))
+ return;
+ obj = parse_object(sha1);
+ if (!obj)
+ return;
+ add_pending_object(revs, obj, "HEAD");
+}
+
static struct object *get_reference(struct rev_info *revs, const char *name, const unsigned char *sha1, unsigned int flags)
{
struct object *object;
diff --git a/revision.h b/revision.h
index 2845167..2fa6286 100644
--- a/revision.h
+++ b/revision.h
@@ -132,5 +132,6 @@ extern void add_object(struct object *obj,
extern void add_pending_object(struct rev_info *revs, struct object *obj, const char *name);
extern void add_pending_object_with_mode(struct rev_info *revs, struct object *obj, const char *name, unsigned mode);
+extern void add_head(struct rev_info *revs);
#endif
--
1.5.2
^ permalink raw reply related
* [RFC][PATCH 05/10] Sparse: fix some "symbol not declared" warnings (Part 3)
From: Ramsay Jones @ 2007-06-08 22:15 UTC (permalink / raw)
To: Junio C Hamano; +Cc: GIT Mailing-list
This commit removes this warning, in various ways, for three more
symbols, thus:
- 'common_cmds': this symbol is created by the generate-cmdlist.sh
script, so modify the script to include the static keyword in
the declaration.
- 'curl_ftp_no_epsv': add declaration to http.h header file.
(Note that it could be declared static in http.c, but so could
several other variables in that header; just being consistent)
- 'wt_status_use_color': add a declaration to the wt-status.h
header file, but change the symbol to a function at the same
time.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
builtin-runstatus.c | 6 ++----
generate-cmdlist.sh | 2 +-
http.h | 2 +-
wt-status.c | 12 +++++++++---
wt-status.h | 1 +
5 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/builtin-runstatus.c b/builtin-runstatus.c
index d7b04cb..1a9a647 100644
--- a/builtin-runstatus.c
+++ b/builtin-runstatus.c
@@ -2,8 +2,6 @@
#include "wt-status.h"
#include "builtin.h"
-extern int wt_status_use_color;
-
static const char runstatus_usage[] =
"git-runstatus [--color|--nocolor] [--amend] [--verbose] [--untracked]";
@@ -17,9 +15,9 @@ int cmd_runstatus(int argc, const char **argv, const char *prefix)
for (i = 1; i < argc; i++) {
if (!strcmp(argv[i], "--color"))
- wt_status_use_color = 1;
+ wt_status_use_color(1);
else if (!strcmp(argv[i], "--nocolor"))
- wt_status_use_color = 0;
+ wt_status_use_color(0);
else if (!strcmp(argv[i], "--amend")) {
s.amend = 1;
s.reference = "HEAD^1";
diff --git a/generate-cmdlist.sh b/generate-cmdlist.sh
index 975777f..17df47b 100755
--- a/generate-cmdlist.sh
+++ b/generate-cmdlist.sh
@@ -7,7 +7,7 @@ struct cmdname_help
char help[80];
};
-struct cmdname_help common_cmds[] = {"
+static struct cmdname_help common_cmds[] = {"
sort <<\EOF |
add
diff --git a/http.h b/http.h
index 69b6b66..9070e6f 100644
--- a/http.h
+++ b/http.h
@@ -99,9 +99,9 @@ extern char *ssl_capath;
extern char *ssl_cainfo;
extern long curl_low_speed_limit;
extern long curl_low_speed_time;
+extern int curl_ftp_no_epsv;
extern struct curl_slist *pragma_header;
-extern struct curl_slist *no_range_header;
extern struct active_request_slot *active_queue_head;
diff --git a/wt-status.c b/wt-status.c
index fd8a877..9fa9100 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -8,7 +8,7 @@
#include "revision.h"
#include "diffcore.h"
-int wt_status_use_color = 0;
+static int use_color = 0;
static char wt_status_colors[][COLOR_MAXLEN] = {
"", /* WT_STATUS_HEADER: normal */
"\033[32m", /* WT_STATUS_UPDATED: green */
@@ -39,7 +39,7 @@ static int parse_status_slot(const char *var, int offset)
static const char* color(int slot)
{
- return wt_status_use_color ? wt_status_colors[slot] : "";
+ return use_color ? wt_status_colors[slot] : "";
}
void wt_status_prepare(struct wt_status *s)
@@ -349,7 +349,7 @@ void wt_status_print(struct wt_status *s)
int git_status_config(const char *k, const char *v)
{
if (!strcmp(k, "status.color") || !strcmp(k, "color.status")) {
- wt_status_use_color = git_config_colorbool(k, v);
+ use_color = git_config_colorbool(k, v);
return 0;
}
if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
@@ -358,3 +358,9 @@ int git_status_config(const char *k, const char *v)
}
return git_default_config(k, v);
}
+
+void wt_status_use_color(int please)
+{
+ use_color = please;
+}
+
diff --git a/wt-status.h b/wt-status.h
index cfea4ae..dd4421a 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -24,5 +24,6 @@ struct wt_status {
int git_status_config(const char *var, const char *value);
void wt_status_prepare(struct wt_status *s);
void wt_status_print(struct wt_status *s);
+void wt_status_use_color(int please);
#endif /* STATUS_H */
--
1.5.2
^ permalink raw reply related
* [PATCH] t5000: silence unzip availability check
From: René Scharfe @ 2007-06-09 6:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0706061956540.4046@racer.site>
unzip -v on (at least) Ubuntu prints a screenful of version info
to stdout. Get rid of it since we only want to know if unzip is
installed or not.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 5500505..a6c5bf6 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -108,7 +108,7 @@ test_expect_success \
'git-archive --format=zip' \
'git-archive --format=zip HEAD >d.zip'
-$UNZIP -v 2>/dev/null
+$UNZIP -v >/dev/null 2>&1
if [ $? -eq 127 ]; then
echo "Skipping ZIP tests, because unzip was not found"
test_done
^ permalink raw reply related
* Re: [RFC][PATCH 00/10] Sparse: Git's "make check" target
From: Josh Triplett @ 2007-06-09 7:08 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Junio C Hamano, GIT Mailing-list, linux-sparse
In-Reply-To: <4669D2F2.90801@ramsay1.demon.co.uk>
[-- Attachment #1: Type: text/plain, Size: 7065 bytes --]
Ramsay Jones wrote:
> Since the Git Makefile has a "check" target that uses sparse, I decided to
> take a look at the sparse project to see what it was about, and what it
> has to say about the git source code.
Glad to hear it!
> Initially, I had many problems because I am using cygwin, but I was able to
> fix most of those problems. (the output from "make check" was about 16k
> lines at one point!). Git also tickled a bug in sparse 0.2, which resulted
> in some 120+ lines of bogus warnings; that was fixed in version 0.3 (commit
> 0.2-15-gef25961). As a result, sparse version 0.3 + my patches, elicits 106
> lines of output from "make check".
One note about using Sparse with Git: you almost certainly don't want to pass
-Wall to sparse, and current Git passes CFLAGS to Sparse which will do exactly
that. -Wall turns on all possible Sparse warnings, including nitpicky
warnings and warnings with a high false positive rate. You should start from
the default set of Sparse warnings, and add additional warnings as desired, or
turn off those you absolutely can't live with. Current Sparse from Git (post
0.3, after commit e18c1014449adf42520daa9d3e53f78a3d98da34) has a change to
cgcc to filter out -Wall, so you can pass -Wall to GCC but not Sparse. See
below for other reasons why you should use cgcc.
That said, this suggests that perhaps Sparse should treat -Wall differently
for compatibility with GCC; specifically, perhaps Sparse should just ignore
-Wall, as its meaning with GCC (enable a reasonable default set of warnings)
already occurs by default in Sparse. The current -Wall could become something
like -Weverything. This would make Sparse somewhat less intuitive, but
somewhat more GCC compatible.
> Naturally, I decided to "fix" the warnings produced by sparse, which resulted
> in the following patch series:
>
> [PATCH 01/10] Sparse: fix "non-ANSI function declaration" warnings
> [PATCH 02/10] Sparse: fix some "using integer as NULL pointer" warnings
> [PATCH 03/10] Sparse: fix some "symbol not declared" warnings (Part 1)
> [PATCH 04/10] Sparse: fix some "symbol not declared" warnings (Part 2)
> [PATCH 05/10] Sparse: fix some "symbol not declared" warnings (Part 3)
> [PATCH 06/10] Sparse: fix "'add_head' not declared" warning
> [PATCH 07/10] Sparse: fix "'merge_file' not declared" warning
> [PATCH 08/10] Sparse: fix an "incorrect type in argument n" warning
> [PATCH 09/10] Sparse: fix some "symbol 's' shadows an earlier one" warnings
> [PATCH 10/10] Sparse: fix a "symbol 'weak_match' shadows an earlier one" warning
Awesome. I'll take a look at that patch series.
> However, this patch series does not completely remove all warnings; the output
> is reduced to:
>
> builtin-pack-objects.c:312:31: warning: Using plain integer as NULL pointer
> csum-file.c:152:22: warning: Using plain integer as NULL pointer
> exec_cmd.c:7:40: error: undefined identifier 'GIT_EXEC_PATH'
> git.c:209:35: error: undefined identifier 'GIT_VERSION'
> http.c:203:46: error: undefined identifier 'GIT_USER_AGENT'
> index-pack.c:201:25: warning: Using plain integer as NULL pointer
> index-pack.c:538:26: warning: Using plain integer as NULL pointer
>
> The three "undefined identifier 'GIT_...'" are easy to remove, I just didn't
> get around to doing it (The GIT_... symbols are macros, defined in individual
> make rules rather than CFLAGS, an thus not passed to sparse).
You can easily fix that problem if you use cgcc as CC.
> The four "Using plain integer...", whilst equally easy to remove, arguably should
> not be ;-) If you look at the code you will find they are all of the form
> x = crc32(0, Z_NULL, 0);
> where the second parameter type is basically (unsigned char *) and the Z_NULL
> macro is defined in the zlib header file as 0. It could be said that this is
> "idiomatic zlib usage" and should remain as written. If you don't subscribe
> to that view, then the required patch is obvious :P)
[...]
> [Note: As far as the NULL pointer warnings are concerned, I don't much care either
> way. I just used that as an example (also note patch 02). Having said that, I
> do think that the "NULL is the only one true null pointer" brigade need to
> chill out a little; in fact I remember when 0 was the *only* null pointer.]
And at one point prototypes didn't exist either. :)
Other valid null pointers exist, such as (void *)0. You could also use (char
*)0 in this particular case. Sparse complains because you just use the
integer 0. I suggest just using NULL.
If you want to keep using Z_NULL rather than NULL, you could always undef it
and define it as NULL after including the zlib header files.
If you really want to turn that particular Sparse warning off, you can use
-Wno-non-pointer-null. However, I don't think you should do that.
> If you are on Linux and want to play along, then the official version 0.3
> release of sparse should work for you, along with a minor change to the
> Makefile thus:
>
> --->8---
> diff --git a/Makefile b/Makefile
> index 19b6da1..ac3e2af 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -184,7 +184,7 @@ export TCL_PATH TCLTK_PATH
>
> # sparse is architecture-neutral, which means that we need to tell it
> # explicitly what architecture to check for. Fix this up for yours..
> -SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powerpc__
> +SPARSE_FLAGS = -D__STDC__=1 $(shell cat .sparse_flags)
>
> --->8---
>
> where the .sparse_flags file is created with a script (gen-sparse-flags.sh)
> as follows:
>
> --->8---
> #/bin/sh
>
> rm -f /tmp/foo.h .macros; touch /tmp/foo.h
>
> gcc -E -dM /tmp/foo.h >.macros
> sed -e "s/^#define /-D'/" -e "s/ /=/" -e "s/$/'/" <.macros >.sparse_flags
>
> rm -f /tmp/foo.h
>
> --->8---
Note that you could do that much more simply by using:
gcc -E -dM -x c /dev/null | sed ...
However, see below about using cgcc instead.
> Note: setting __STDC__ in the SPARSE_FLAGS should not be necessary (I thought
> I needed it at one point...) but I didn't get around to removing it.
Sparse has defined __STDC__ since 2003, well before even version 0.1.
> As an alternative, you could clear the SPARSE_FLAGS and change the "check" target
> to call "cgcc -no-compile" in place of sparse.
Please go with that option. In addition to providing an easy way to use
sparse and GCC together (make CC=cgcc), cgcc defines arch-specific flags that
sparse currently does not. Ideally sparse should define these flags, but that
would add some architecture-specific logic to sparse, which would then require
sparse to know the desired machine target. That may need to happen in the
future, but I don't have a good plan for how to do it yet. In the meantime,
please run sparse through cgcc.
Also, you might consider just using cgcc to run both GCC and Sparse. That
would handle the issue of target-specific CFLAGS, by ensuring that Sparse and
GCC always see the same CFLAGS.
- Josh Triplett
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]
^ permalink raw reply
* Re: [RFC][PATCH 10/10] Sparse: fix a "symbol 'weak_match' shadows an earlier one" warning
From: Junio C Hamano @ 2007-06-09 8:07 UTC (permalink / raw)
To: Ramsay Jones; +Cc: GIT Mailing-list
In-Reply-To: <4669D7BC.3060607@ramsay1.demon.co.uk>
Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
> connect.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/connect.c b/connect.c
> index da89c9c..d4051dd 100644
> --- a/connect.c
> +++ b/connect.c
> @@ -179,7 +179,6 @@ static int count_refspec_match(const char *pattern,
> for (weak_match = match = 0; refs; refs = refs->next) {
> char *name = refs->name;
> int namelen = strlen(name);
> - int weak_match;
>
> if (namelen < patlen ||
> memcmp(name + namelen - patlen, pattern, patlen))
This one is an obvious bug. Essentially, it makes weak matches
ignored. Unfortunately this has been hiding a larger bug in the
caller of this function. I am refactoring the mess right now.
^ permalink raw reply
* [PATCH 1/5] remote.c: refactor match_explicit_refs()
From: Junio C Hamano @ 2007-06-09 9:21 UTC (permalink / raw)
To: git
This does not change functionality; just splits one block that
is deeply nested and indented out of a huge loop into a separate
function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
remote.c | 159 +++++++++++++++++++++++++++++++++----------------------------
1 files changed, 86 insertions(+), 73 deletions(-)
diff --git a/remote.c b/remote.c
index 33c8e50..b53130f 100644
--- a/remote.c
+++ b/remote.c
@@ -406,90 +406,98 @@ static struct ref *try_explicit_object_name(const char *name)
return ref;
}
-static int match_explicit_refs(struct ref *src, struct ref *dst,
- struct ref ***dst_tail, struct refspec *rs,
- int rs_nr)
+static int match_explicit(struct ref *src, struct ref *dst,
+ struct ref ***dst_tail,
+ struct refspec *rs,
+ int errs)
{
- int i, errs;
- for (i = errs = 0; i < rs_nr; i++) {
- struct ref *matched_src, *matched_dst;
+ struct ref *matched_src, *matched_dst;
- const char *dst_value = rs[i].dst;
+ const char *dst_value = rs->dst;
- if (rs[i].pattern)
- continue;
+ if (rs->pattern)
+ return errs;
- if (dst_value == NULL)
- dst_value = rs[i].src;
+ if (dst_value == NULL)
+ dst_value = rs->src;
- matched_src = matched_dst = NULL;
- switch (count_refspec_match(rs[i].src, src, &matched_src)) {
- case 1:
- break;
- case 0:
- /* The source could be in the get_sha1() format
- * not a reference name. :refs/other is a
- * way to delete 'other' ref at the remote end.
- */
- matched_src = try_explicit_object_name(rs[i].src);
- if (matched_src)
- break;
- errs = 1;
- error("src refspec %s does not match any.",
- rs[i].src);
- break;
- default:
- errs = 1;
- error("src refspec %s matches more than one.",
- rs[i].src);
- break;
- }
- switch (count_refspec_match(dst_value, dst, &matched_dst)) {
- case 1:
- break;
- case 0:
- if (!memcmp(dst_value, "refs/", 5)) {
- int len = strlen(dst_value) + 1;
- matched_dst = xcalloc(1, sizeof(*dst) + len);
- memcpy(matched_dst->name, dst_value, len);
- link_dst_tail(matched_dst, dst_tail);
- }
- else if (!strcmp(rs[i].src, dst_value) &&
- matched_src) {
- /* pushing "master:master" when
- * remote does not have master yet.
- */
- int len = strlen(matched_src->name) + 1;
- matched_dst = xcalloc(1, sizeof(*dst) + len);
- memcpy(matched_dst->name, matched_src->name,
- len);
- link_dst_tail(matched_dst, dst_tail);
- }
- else {
- errs = 1;
- error("dst refspec %s does not match any "
- "existing ref on the remote and does "
- "not start with refs/.", dst_value);
- }
- break;
- default:
- errs = 1;
- error("dst refspec %s matches more than one.",
- dst_value);
+ matched_src = matched_dst = NULL;
+ switch (count_refspec_match(rs->src, src, &matched_src)) {
+ case 1:
+ break;
+ case 0:
+ /* The source could be in the get_sha1() format
+ * not a reference name. :refs/other is a
+ * way to delete 'other' ref at the remote end.
+ */
+ matched_src = try_explicit_object_name(rs->src);
+ if (matched_src)
break;
+ errs = 1;
+ error("src refspec %s does not match any.",
+ rs->src);
+ break;
+ default:
+ errs = 1;
+ error("src refspec %s matches more than one.",
+ rs->src);
+ break;
+ }
+ switch (count_refspec_match(dst_value, dst, &matched_dst)) {
+ case 1:
+ break;
+ case 0:
+ if (!memcmp(dst_value, "refs/", 5)) {
+ int len = strlen(dst_value) + 1;
+ matched_dst = xcalloc(1, sizeof(*dst) + len);
+ memcpy(matched_dst->name, dst_value, len);
+ link_dst_tail(matched_dst, dst_tail);
}
- if (errs)
- continue;
- if (matched_dst->peer_ref) {
- errs = 1;
- error("dst ref %s receives from more than one src.",
- matched_dst->name);
+ else if (!strcmp(rs->src, dst_value) &&
+ matched_src) {
+ /* pushing "master:master" when
+ * remote does not have master yet.
+ */
+ int len = strlen(matched_src->name) + 1;
+ matched_dst = xcalloc(1, sizeof(*dst) + len);
+ memcpy(matched_dst->name, matched_src->name,
+ len);
+ link_dst_tail(matched_dst, dst_tail);
}
else {
- matched_dst->peer_ref = matched_src;
- matched_dst->force = rs[i].force;
+ errs = 1;
+ error("dst refspec %s does not match any "
+ "existing ref on the remote and does "
+ "not start with refs/.", dst_value);
}
+ break;
+ default:
+ errs = 1;
+ error("dst refspec %s matches more than one.",
+ dst_value);
+ break;
+ }
+ if (errs)
+ return errs;
+ if (matched_dst->peer_ref) {
+ errs = 1;
+ error("dst ref %s receives from more than one src.",
+ matched_dst->name);
}
+ else {
+ matched_dst->peer_ref = matched_src;
+ matched_dst->force = rs->force;
+ }
+ return errs;
+}
+
+static int match_explicit_refs(struct ref *src, struct ref *dst,
+ struct ref ***dst_tail, struct refspec *rs,
+ int rs_nr)
+{
+ int i, errs;
+ for (i = errs = 0; i < rs_nr; i++)
+ errs |= match_explicit(src, dst, dst_tail, &rs[i], errs);
return -errs;
}
@@ -513,6 +521,11 @@ static const struct refspec *check_pattern_match(const struct refspec *rs,
return NULL;
}
+/*
+ * Note. This is used only by "push"; refspec matching rules for
+ * push and fetch are subtly different, so do not try to reuse it
+ * without thinking.
+ */
int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
int nr_refspec, char **refspec, int all)
{
--
1.5.2.1.144.gabc40
^ permalink raw reply related
* [PATCH 3/5] remote.c: minor clean-up of match_explicit()
From: Junio C Hamano @ 2007-06-09 9:21 UTC (permalink / raw)
To: git
In-Reply-To: <11813808962261-git-send-email-gitster@pobox.com>
When checking what ref the source refspec matches, we have no
business setting the default for the destination, so move that
code lower. Also simplify the result from the code block that
matches the source side by making it set matched_src only upon
unambiguous match.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
remote.c | 23 ++++++++++++-----------
1 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/remote.c b/remote.c
index f469fb3..30abdbb 100644
--- a/remote.c
+++ b/remote.c
@@ -430,9 +430,6 @@ static int match_explicit(struct ref *src, struct ref *dst,
if (rs->pattern)
return errs;
- if (dst_value == NULL)
- dst_value = rs->src;
-
matched_src = matched_dst = NULL;
switch (count_refspec_match(rs->src, src, &matched_src)) {
case 1:
@@ -445,16 +442,22 @@ static int match_explicit(struct ref *src, struct ref *dst,
matched_src = try_explicit_object_name(rs->src);
if (matched_src)
break;
- errs = 1;
error("src refspec %s does not match any.",
rs->src);
break;
default:
- errs = 1;
+ matched_src = NULL;
error("src refspec %s matches more than one.",
rs->src);
break;
}
+
+ if (!matched_src)
+ errs = 1;
+
+ if (dst_value == NULL)
+ dst_value = rs->src;
+
switch (count_refspec_match(dst_value, dst, &matched_dst)) {
case 1:
break;
@@ -466,21 +469,19 @@ static int match_explicit(struct ref *src, struct ref *dst,
* remote does not have master yet.
*/
matched_dst = make_dst(matched_src->name, dst_tail);
- else {
- errs = 1;
+ else
error("dst refspec %s does not match any "
"existing ref on the remote and does "
"not start with refs/.", dst_value);
- }
break;
default:
- errs = 1;
+ matched_dst = NULL;
error("dst refspec %s matches more than one.",
dst_value);
break;
}
- if (errs)
- return errs;
+ if (errs || matched_dst == NULL)
+ return 1;
if (matched_dst->peer_ref) {
errs = 1;
error("dst ref %s receives from more than one src.",
--
1.5.2.1.144.gabc40
^ permalink raw reply related
* [PATCH 2/5] remote.c: refactor creation of new dst ref
From: Junio C Hamano @ 2007-06-09 9:21 UTC (permalink / raw)
To: git
In-Reply-To: <11813808962261-git-send-email-gitster@pobox.com>
This refactors open-coded sequence to create a new "struct ref"
and link it to the tail of dst list into a new function.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
remote.c | 35 +++++++++++++++++------------------
1 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/remote.c b/remote.c
index b53130f..f469fb3 100644
--- a/remote.c
+++ b/remote.c
@@ -406,6 +406,18 @@ static struct ref *try_explicit_object_name(const char *name)
return ref;
}
+static struct ref *make_dst(const char *name, struct ref ***dst_tail)
+{
+ struct ref *dst;
+ size_t len;
+
+ len = strlen(name) + 1;
+ dst = xcalloc(1, sizeof(*dst) + len);
+ memcpy(dst->name, name, len);
+ link_dst_tail(dst, dst_tail);
+ return dst;
+}
+
static int match_explicit(struct ref *src, struct ref *dst,
struct ref ***dst_tail,
struct refspec *rs,
@@ -447,23 +459,13 @@ static int match_explicit(struct ref *src, struct ref *dst,
case 1:
break;
case 0:
- if (!memcmp(dst_value, "refs/", 5)) {
- int len = strlen(dst_value) + 1;
- matched_dst = xcalloc(1, sizeof(*dst) + len);
- memcpy(matched_dst->name, dst_value, len);
- link_dst_tail(matched_dst, dst_tail);
- }
- else if (!strcmp(rs->src, dst_value) &&
- matched_src) {
+ if (!memcmp(dst_value, "refs/", 5))
+ matched_dst = make_dst(dst_value, dst_tail);
+ else if (!strcmp(rs->src, dst_value) && matched_src)
/* pushing "master:master" when
* remote does not have master yet.
*/
- int len = strlen(matched_src->name) + 1;
- matched_dst = xcalloc(1, sizeof(*dst) + len);
- memcpy(matched_dst->name, matched_src->name,
- len);
- link_dst_tail(matched_dst, dst_tail);
- }
+ matched_dst = make_dst(matched_src->name, dst_tail);
else {
errs = 1;
error("dst refspec %s does not match any "
@@ -567,11 +569,8 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
goto free_name;
if (!dst_peer) {
/* Create a new one and link it */
- int len = strlen(dst_name) + 1;
- dst_peer = xcalloc(1, sizeof(*dst_peer) + len);
- memcpy(dst_peer->name, dst_name, len);
+ dst_peer = make_dst(dst_name, dst_tail);
hashcpy(dst_peer->new_sha1, src->new_sha1);
- link_dst_tail(dst_peer, dst_tail);
}
dst_peer->peer_ref = src;
free_name:
--
1.5.2.1.144.gabc40
^ permalink raw reply related
* [PATCH 5/5] remote.c: "git-push frotz" should update what matches at the source.
From: Junio C Hamano @ 2007-06-09 9:21 UTC (permalink / raw)
To: git
In-Reply-To: <11813808962261-git-send-email-gitster@pobox.com>
Earlier, when the local repository has a branch "frotz" and the
remote repository has a tag "frotz" (but not branch "frotz"),
"git-push frotz" mistakenly updated the tag at the remote side.
This was because the partial refname matching code was applied
independently on both source and destination side.
With this fix, when a colon-less refspec is given to git-push,
we first match it with the refs in the source repository, and
update the matching ref in the destination repository.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
remote.c | 7 +-----
t/t5516-fetch-push.sh | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/remote.c b/remote.c
index 120df36..754d513 100644
--- a/remote.c
+++ b/remote.c
@@ -455,7 +455,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
errs = 1;
if (dst_value == NULL)
- dst_value = rs->src;
+ dst_value = matched_src->name;
switch (count_refspec_match(dst_value, dst, &matched_dst)) {
case 1:
@@ -463,11 +463,6 @@ static int match_explicit(struct ref *src, struct ref *dst,
case 0:
if (!memcmp(dst_value, "refs/", 5))
matched_dst = make_dst(dst_value, dst_tail);
- else if (!strcmp(rs->src, dst_value) && matched_src)
- /* pushing "master:master" when
- * remote does not have master yet.
- */
- matched_dst = make_dst(matched_src->name, dst_tail);
else
error("dst refspec %s does not match any "
"existing ref on the remote and does "
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b3b57fa..08d58e1 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -189,6 +189,58 @@ test_expect_success 'push with ambiguity (2)' '
else
check_push_result $the_first_commit heads/frotz tags/frotz
fi
+
+'
+
+test_expect_success 'push with colon-less refspec (1)' '
+
+ mk_test heads/frotz tags/frotz &&
+ git branch -f frotz master &&
+ git push testrepo frotz &&
+ check_push_result $the_commit heads/frotz &&
+ check_push_result $the_first_commit tags/frotz
+
+'
+
+test_expect_success 'push with colon-less refspec (2)' '
+
+ mk_test heads/frotz tags/frotz &&
+ if git show-ref --verify -q refs/heads/frotz
+ then
+ git branch -D frotz
+ fi &&
+ git tag -f frotz &&
+ git push testrepo frotz &&
+ check_push_result $the_commit tags/frotz &&
+ check_push_result $the_first_commit heads/frotz
+
+'
+
+test_expect_success 'push with colon-less refspec (3)' '
+
+ mk_test &&
+ if git show-ref --verify -q refs/tags/frotz
+ then
+ git tag -d frotz
+ fi &&
+ git branch -f frotz master &&
+ git push testrepo frotz &&
+ check_push_result $the_commit heads/frotz &&
+ test "$( cd testrepo && git show-ref | wc -l )" = 1
+'
+
+test_expect_success 'push with colon-less refspec (4)' '
+
+ mk_test &&
+ if git show-ref --verify -q refs/heads/frotz
+ then
+ git branch -D frotz
+ fi &&
+ git tag -f frotz &&
+ git push testrepo frotz &&
+ check_push_result $the_commit tags/frotz &&
+ test "$( cd testrepo && git show-ref | wc -l )" = 1
+
'
test_done
--
1.5.2.1.144.gabc40
^ permalink raw reply related
* [PATCH 4/5] remote.c: fix "git push" weak match disambiguation
From: Junio C Hamano @ 2007-06-09 9:21 UTC (permalink / raw)
To: git
In-Reply-To: <11813808962261-git-send-email-gitster@pobox.com>
When "git push A:B" is given, and A (or B) is not a full refname
that begins with refs/, we require an unambiguous match with an
existing ref. For this purpose, a match with a local branch or
a tag (i.e. refs/heads/A and refs/tags/A) is called a "strong
match", and any other match is called a "weak match". A partial
refname is unambiguous when there is only one strong match with
any number of weak matches, or when there is only one weak match
and no other match.
However, as reported by Sparse with Ramsay Jones recently,
count_refspec_match() function had a bug where a variable in an
inner block masked a different variable of the same name, which
caused the weak matches to be ignored.
This fixes it, and adds tests for the fix.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
remote.c | 1 -
t/t5516-fetch-push.sh | 112 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 112 insertions(+), 1 deletions(-)
diff --git a/remote.c b/remote.c
index 30abdbb..120df36 100644
--- a/remote.c
+++ b/remote.c
@@ -333,7 +333,6 @@ static int count_refspec_match(const char *pattern,
for (weak_match = match = 0; refs; refs = refs->next) {
char *name = refs->name;
int namelen = strlen(name);
- int weak_match;
if (namelen < patlen ||
memcmp(name + namelen - patlen, pattern, patlen))
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index dba018f..b3b57fa 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -15,12 +15,58 @@ mk_empty () {
)
}
+mk_test () {
+ mk_empty &&
+ (
+ for ref in "$@"
+ do
+ git push testrepo $the_first_commit:refs/$ref || {
+ echo "Oops, push refs/$ref failure"
+ exit 1
+ }
+ done &&
+ cd testrepo &&
+ for ref in "$@"
+ do
+ r=$(git show-ref -s --verify refs/$ref) &&
+ test "z$r" = "z$the_first_commit" || {
+ echo "Oops, refs/$ref is wrong"
+ exit 1
+ }
+ done &&
+ git fsck --full
+ )
+}
+
+check_push_result () {
+ (
+ cd testrepo &&
+ it="$1" &&
+ shift
+ for ref in "$@"
+ do
+ r=$(git show-ref -s --verify refs/$ref) &&
+ test "z$r" = "z$it" || {
+ echo "Oops, refs/$ref is wrong"
+ exit 1
+ }
+ done &&
+ git fsck --full
+ )
+}
+
test_expect_success setup '
: >path1 &&
git add path1 &&
test_tick &&
git commit -a -m repo &&
+ the_first_commit=$(git show-ref -s --verify refs/heads/master) &&
+
+ : >path2 &&
+ git add path2 &&
+ test_tick &&
+ git commit -a -m second &&
the_commit=$(git show-ref -s --verify refs/heads/master)
'
@@ -79,4 +125,70 @@ test_expect_success 'push with wildcard' '
)
'
+test_expect_success 'push with matching heads' '
+
+ mk_test heads/master &&
+ git push testrepo &&
+ check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with no ambiguity (1)' '
+
+ mk_test heads/master &&
+ git push testrepo master:master &&
+ check_push_result $the_commit heads/master
+
+'
+
+test_expect_success 'push with no ambiguity (2)' '
+
+ mk_test remotes/origin/master &&
+ git push testrepo master:master &&
+ check_push_result $the_commit remotes/origin/master
+
+'
+
+test_expect_success 'push with weak ambiguity (1)' '
+
+ mk_test heads/master remotes/origin/master &&
+ git push testrepo master:master &&
+ check_push_result $the_commit heads/master &&
+ check_push_result $the_first_commit remotes/origin/master
+
+'
+
+test_expect_success 'push with weak ambiguity (2)' '
+
+ mk_test heads/master remotes/origin/master remotes/another/master &&
+ git push testrepo master:master &&
+ check_push_result $the_commit heads/master &&
+ check_push_result $the_first_commit remotes/origin/master remotes/another/master
+
+'
+
+test_expect_success 'push with ambiguity (1)' '
+
+ mk_test remotes/origin/master remotes/frotz/master &&
+ if git push testrepo master:master
+ then
+ echo "Oops, should have failed"
+ false
+ else
+ check_push_result $the_first_commit remotes/origin/master remotes/frotz/master
+ fi
+'
+
+test_expect_success 'push with ambiguity (2)' '
+
+ mk_test heads/frotz tags/frotz &&
+ if git push testrepo master:frotz
+ then
+ echo "Oops, should have failed"
+ false
+ else
+ check_push_result $the_first_commit heads/frotz tags/frotz
+ fi
+'
+
test_done
--
1.5.2.1.144.gabc40
^ permalink raw reply related
* Re: [PATCH] Add autoconf-based build infrastructure for tig
From: Jonas Fonseca @ 2007-06-09 9:31 UTC (permalink / raw)
To: Steven Grimm; +Cc: git
In-Reply-To: <20070603193521.GA10161@midwinter.com>
Hi Steven,
Steven Grimm <koreth@midwinter.com> wrote Sun, Jun 03, 2007:
> ---
> This is a first cut at building tig using autoconf. I'm including
> a script to run the various autoconf tools rather than packaging
> up a finished configure script. With this patch, tig configures
> and builds on both Linux (FC4) and OS X. I left a lot of the code
> from the original Makefile intact so as to (hopefully) not mess
> up building release tarballs, etc.
First of all, thank you for starting this work!
I've only played little with this patch, but overall I like most of the
changes. I would, however, want to look into making the dependency on
autoconf optional (like it is for git) and avoid using automake at all.
It would make the autoconf.sh bootstrap script obsolete, since the
Makefile can just take care of it, and it would keep the build system
simple.
So the idea is for configure to also generate a Makefile.config that can
be sourced by the Makefile. Then of course inclusion of the config.h
file should depend on some -DHAVE_CONFIG_H flag for the compiler.
What do you think? You are of course welcome to come up with a patch for
this proposal, but else I would like to get your permission/sign-off to
include the configure.ac script and the tig.c changes you made.
--
Jonas Fonseca
^ permalink raw reply
* Re: [PATCH 03/21] Refactoring to make verify_tag() and parse_tag_buffer() more similar
From: Johan Herland @ 2007-06-09 10:49 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Junio C Hamano
In-Reply-To: <Pine.LNX.4.64.0706090339280.4059@racer.site>
On Saturday 09 June 2007, Johannes Schindelin wrote:
> Hi,
Hi. Thanks for taking the time to look at (some of) my patch. Most of your
questions below can be answered with a single answer:
The main purpose of the patch is (as the subject line says) to bring the
two functions more in line with eachother. At the time I made the patch,
I had made the observation that these function were trying to do much the
same thing, albeit in a slightly different form. This patch is therefore
about applying a series of (mostly non-functional) refactorings to make
their diff as small as possible. This involves "stupid" changes such as
renaming variables, tweaking whitespace, reordering the declaration of
variables, etc. It's all to make the functions similar to the point where
I can diff them, get a small and meaningful result, see the remaining
_real_ differences, and in the end, _merge_ them (see patches 7-9).
If this whole exercise didn't end up with merging the two functions into
one, I would _totally_ agree with you that all this refactoring is more
harmful than beneficial.
> On Sat, 9 Jun 2007, Johan Herland wrote:
> > if (size < 64)
> > return error("wanna fool me ? you obviously got the size wrong !");
> >
> > - buffer[size] = 0;
>
> Are you sure that your buffer is always NUL terminated?
First, (and you'll see this in the commit message) I'm _moving_ (not
removing) the NUL termination out of verify_tag() and into main() (which I
can be sure is the only caller of verify_tag(), since verify_tag is
declared static, and there is no other call in that file). Two reasons for
doing this:
1. Make verify_tag more similar to parse_tag_buffer() (because
parse_tag_buffer() does not NUL terminate)
2. Do the NUL termination as close to the code that actually populated the
buffer with data (the read_pipe() in main())
So now you can ask: Why doesn't parse_tag_buffer() NUL terminate its
input? It _that_ safe? And I ran around checking all the callers of
parse_tag_buffer, and found that all of them use data (most of which
originates from read_sha1_file()) that's already NUL terminated.
In the end, I also put in a comment on the resulting function
(parse_and_verify_tag_buffer()), explicitly saying the given data _must_
be NUL terminated.
Side note: At first I actually thought the manual NUL termination
could cause a buffer overflow (i.e. if the given size was the same as the
allocated size), so I actually have a version of all of this where I
_don't_ assume the buffer is NUL-terminated at all, and put in lots of
bounds checking, replace strchr() with memchr(), etc.
I then took a hard look at read_pipe(), and discovered that if you
use it to fill a 4096-byte buffer with 4096 bytes of data, it actually
_will_ reallocate to 8192 bytes and leave room for the NUL termination
(and much more) (I believe this should have been documented in read_pipe).
Thus the NUL termination was safe all along.
> > - type_line = object + 48;
> > + type_line = data + 48;
>
> Quite a lot of changes seem to do this object->data. The patch would have
> been much more compact if you just had renamed buffer to object instead of
> data.
Yes, but it would have made the aforementioned diff to parse_tag_buffer()
larger.
> > /* TODO: check for committer info + blank line? */
> > /* Also, the minimum length is probably + "tagger .", or 63+8=71 */
> >
> > /* The actual stuff afterwards we don't care about.. */
> > return 0;
> > -}
> >
> > #undef PD_FMT
> > +}
>
> Any particular reason for this?
Well, PD_FMT is only used inside the function, so I found it easier to
move the #definition of PD_FMT inside the function to indicate the scope
(_perceived_ scope; I know it hasn't any effect on the compiled code).
But since the whole function is going away in a few patches anyway,
I should have probably left it out of this patch entirely.
> > @@ -124,6 +120,7 @@ int main(int argc, char **argv)
> > free(buffer);
> > die("could not read from stdin");
> > }
> > + buffer[size] = 0;
>
> Ah, so you terminate the buffer here. From the patch, it is relatively
> hard to see if this line is always hit _before_ the function is called
> that evidently relies on NUL termination. By moving it here, I think it is
> much more likely to overlook the fact that the function, which made sure
> that its assumption was met, needs this line. Whereas if you left it where
> it was, the assumption would always be met.
But if I leave the NUL termination within the function I would have to
backtrack out of the function to all of its potential callers and check
whether it's safe to write to index size. Since the word "size" could
easily mean "allocated size" I would have the initial feeling that this
might be a buffer overflow, i.e. _not_ safe.
In the end, I think the best solution is to make sure NUL termination
happens before calling the function, and then documenting explicitly
that the function assumes NUL terminated input. Which is exactly what
I end up with at the end of the patch series.
> > - sig_line++;
> > + tagger_line++;
>
> I am really reluctant with renamings like these. IMHO they don't buy you
> much, except for possible confusion. It is evident that sig means the
> signer (and it is obvious in the case of an unsigned tag, who is meant,
> too).
Hmm. The "type" line is found in the variable type_line, the "tag" line is
found in the variable tag_line, and the "tagger" line is found in the
variable ... sig_line? Nope, I don't buy it.
...Johan
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [RFC] git integrated bugtracking
From: Pierre Habouzit @ 2007-06-09 12:12 UTC (permalink / raw)
To: git
In-Reply-To: <20070603114843.GA14336@artemis>
[-- Attachment #1: Type: text/plain, Size: 405 bytes --]
FWIW I've begun to work on this (for real). I've called the tool
"grit". You can follow the developpement on:
* gitweb: http://git.madism.org/?p=grit.git;a=summary
* git: git://git.madism.org/grit.git/
Cheers,
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] git-branch: cleanup config file when deleting branches
From: Gerrit Pape @ 2007-06-09 12:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vzm3akf6g.fsf@assigned-by-dhcp.cox.net>
When deleting branches, remove the sections referring to these branches
from the config file.
Signed-off-by: Gerrit Pape <pape@smarden.org>
---
On Fri, Jun 08, 2007 at 01:45:27AM -0700, Junio C Hamano wrote:
> Hmph. Makes sense and might even be a maint material, but it
> probably is safe not to do a behaviour change in maint.
>
> Tests?
Sure, two tests added. Regards, Gerrit.
builtin-branch.c | 9 +++++++--
t/t3200-branch.sh | 9 +++++++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/builtin-branch.c b/builtin-branch.c
index 67f46c1..9addd1a 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -85,6 +85,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
unsigned char sha1[20];
char *name = NULL;
const char *fmt, *remote;
+ char section[PATH_MAX];
int i;
int ret = 0;
@@ -152,9 +153,13 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
error("Error deleting %sbranch '%s'", remote,
argv[i]);
ret = 1;
- } else
+ } else {
printf("Deleted %sbranch %s.\n", remote, argv[i]);
-
+ snprintf(section, sizeof(section), "branch.%s",
+ argv[i]);
+ if (git_config_rename_section(section, NULL) < 0)
+ warning("Update of config-file failed");
+ }
}
if (name)
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 6f6d884..1322bfb 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -171,6 +171,15 @@ test_expect_success 'test tracking setup via --track but deeper' \
test "$(git-config branch.my7.remote)" = local &&
test "$(git-config branch.my7.merge)" = refs/heads/o/o'
+test_expect_success 'test deleting branch deletes branch config' \
+ 'git-branch -d my7 &&
+ test "$(git-config branch.my7.remote)" = "" &&
+ test "$(git-config branch.my7.merge)" = ""'
+
+test_expect_success 'test deleting branch without config' \
+ 'git-branch my7 s &&
+ test "$(git-branch -d my7)" = "Deleted branch my7."'
+
# Keep this test last, as it changes the current branch
cat >expect <<EOF
0000000000000000000000000000000000000000 $HEAD $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 1117150200 +0000 branch: Created from master
--
1.5.2.1
^ permalink raw reply related
* Attributes for commits
From: Paul Franz @ 2007-06-09 13:11 UTC (permalink / raw)
To: git
I am a ClearCase administrator and one of the things that I love
about it is the ability to assign attributes to versions of files. Is
there anybody thinking of adding the ability to assign attributes to a
commit? I ask because I was thinking about how I would move from
ClearCase to git and part of our process is to assign the BugNum
attribute to every checkin so that we can see what bugs have been fixed.
We also track the files checked in the bug tracking software to. Thus
giving us a two way linkage between repository and the bug tracking
system. This is VERY useful. And I was wondering if there are any
thought to this for commits.
Now, I will be honest it is possible that this has already been done (I
have not read all the documentation yet) and I am justing wasting
bandwidth. In which, please excuse my ignorance and tell me to just RTFM.
Paul Franz
--
-------------------------------------------
There are seven sins in the world.
Wealth without work.
Pleasure without conscience.
Knowledge without character.
Commerce without morality.
Science without humanity.
Worship without sacrifice.
Politics without principle.
-- Mohandas Gandhi
-------------------------------------------
^ permalink raw reply
* Re: Attributes for commits
From: Johan Herland @ 2007-06-09 13:49 UTC (permalink / raw)
To: git; +Cc: Paul Franz
In-Reply-To: <466AA714.4050500@comcast.net>
On Saturday 09 June 2007, Paul Franz wrote:
> I am a ClearCase administrator and one of the things that I love
> about it is the ability to assign attributes to versions of files. Is
> there anybody thinking of adding the ability to assign attributes to a
> commit? I ask because I was thinking about how I would move from
> ClearCase to git and part of our process is to assign the BugNum
> attribute to every checkin so that we can see what bugs have been fixed.
> We also track the files checked in the bug tracking software to. Thus
> giving us a two way linkage between repository and the bug tracking
> system. This is VERY useful. And I was wondering if there are any
> thought to this for commits.
>
> Now, I will be honest it is possible that this has already been done (I
> have not read all the documentation yet) and I am justing wasting
> bandwidth. In which, please excuse my ignorance and tell me to just RTFM.
Well, we're sort of in the process of getting there. I posted an initial
proof-of-concept implementation of git 'notes' a while back. Git notes is
a general mechanism for providing free-form annotations on git objects
after they've been created. My initial posting sparked a discussion [1],
and I'm currently in the process of reimplementing the notes concept
based on the outcome of that discussion.
So the thing you want will hopefully be in a future release.
Have fun!
...Johan
[1]: http://thread.gmane.org/gmane.comp.version-control.git/46770/focus=48540
--
Johan Herland, <johan@herland.net>
www.herland.net
^ permalink raw reply
* Re: [PATCH] t5000: silence unzip availability check
From: Johannes Schindelin @ 2007-06-09 14:37 UTC (permalink / raw)
To: René Scharfe; +Cc: Junio C Hamano, Git Mailing List
In-Reply-To: <466A4930.2020602@lsrfire.ath.cx>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 237 bytes --]
Hi,
On Sat, 9 Jun 2007, René Scharfe wrote:
> unzip -v on (at least) Ubuntu prints a screenful of version info
> to stdout. Get rid of it since we only want to know if unzip is
> installed or not.
Makes sense, absolutely.
ACK,
Dscho
^ permalink raw reply
* [PATCH 1/2] Show html help with git-help --html
From: Nguyen Thai Ngoc Duy @ 2007-06-09 15:03 UTC (permalink / raw)
To: git; +Cc: Frank Lichtenheld, junkio
In-Reply-To: <20070605183420.GA8450@localhost>
This patch was inspired by ClearCase command 'ct man', which
opens an html help file on Windows. I at first attempted to
implement it for MinGW port only but found it so useful that I
wanted to have it even in Linux.
A new option '--html' is added to git. When git-help is called
with --html, it will try to open an html file located at
$(docdir)/html using xdg-open. HTML files are not installed
by default so users have to install them manually or have their
distributors to do that.
There are two new config options introduced in this patch. The
first is core.help. It has one of three values: html, auto or
man. 'html' has the same effect as git-help --html. 'auto'
will in addition fall back to man pages if possible. 'man'
is 'I hate html, give me my man pages'. The second option is
core.htmlviewer, used to specify the program you want to
open html files instead of xdg-open. You can override the default
program by appending HTML_VIEWER=blah when calling make.
core.htmlviewer can contain %p, %f or %b. If none is given,
%p will be appended.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
This patch changes core.htmlprogram to core.htmlviewer and
mentions 'man' as default value for core.help as suggested
by Frank.
It also fixes a bug ignoring the remaining string after the
last %x in html command
Documentation/config.txt | 16 +++++
Documentation/git.txt | 5 +-
Makefile | 5 +-
cache.h | 2 +
config.c | 17 +++++
config.mak.in | 2 +
environment.c | 2 +
git.c | 2 +-
help.c | 161 +++++++++++++++++++++++++++++++++++++++++++++-
9 files changed, 207 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index de408b6..2489b8e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -262,6 +262,22 @@ core.excludeFile::
of files which are not meant to be tracked. See
gitlink:gitignore[5].
+core.help::
+ If 'html', it is equivalent to 'git-help' with option --html.
+ If 'auto', it tries to open html files first. If that attempt fails
+ (the html program does not exist or the program return non-zero
+ value), then it will fall back to man pages. If 'man', always use
+ man pages as usual. Default is 'man'.
+
+core.htmlviewer::
+ Specify the program used to open html help files when 'git-help'
+ is called with option --html or core.help is other than 'man'.
+ By default, xdg-open will be used.
+ Special strings '%p', '%f' and '%b' will be replaced with html
+ full path, file name and git command (without .html suffix)
+ respectively. If none is given, '%p' will be automatically appended
+ to the command line.
+
alias.*::
Command aliases for the gitlink:git[1] command wrapper - e.g.
after defining "alias.last = cat-file commit HEAD", the invocation
diff --git a/Documentation/git.txt b/Documentation/git.txt
index ba077c3..f1df402 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -10,7 +10,7 @@ SYNOPSIS
--------
[verse]
'git' [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate]
- [--bare] [--git-dir=GIT_DIR] [--help] COMMAND [ARGS]
+ [--bare] [--git-dir=GIT_DIR] [--help [--html]] COMMAND [ARGS]
DESCRIPTION
-----------
@@ -87,6 +87,9 @@ OPTIONS
commands. If a git command is named this option will bring up
the man-page for that command. If the option '--all' or '-a' is
given then all available commands are printed.
+ If option '--html' is given, try opening html files instead of
+ using 'man'. The default program to open html files is xdg-open.
+ See 'git-config' to know how to change html program.
--exec-path::
Path to wherever your core git programs are installed.
diff --git a/Makefile b/Makefile
index 0f75955..7b3180a 100644
--- a/Makefile
+++ b/Makefile
@@ -186,6 +186,7 @@ export TCL_PATH TCLTK_PATH
# explicitly what architecture to check for. Fix this up for yours..
SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powerpc__
+HTML_VIEWER=xdg-open
### --- END CONFIGURATION SECTION ---
@@ -692,6 +693,7 @@ ETC_GITCONFIG_SQ = $(subst ','\'',$(ETC_GITCONFIG))
DESTDIR_SQ = $(subst ','\'',$(DESTDIR))
bindir_SQ = $(subst ','\'',$(bindir))
gitexecdir_SQ = $(subst ','\'',$(gitexecdir))
+html_dir_SQ = $(subst ','\'',$(html_dir))
template_dir_SQ = $(subst ','\'',$(template_dir))
prefix_SQ = $(subst ','\'',$(prefix))
@@ -740,7 +742,8 @@ git$X: git.c common-cmds.h $(BUILTIN_OBJS) $(GITLIBS) GIT-CFLAGS
$(ALL_CFLAGS) -o $@ $(filter %.c,$^) \
$(BUILTIN_OBJS) $(ALL_LDFLAGS) $(LIBS)
-help.o: common-cmds.h
+help.o: help.c common-cmds.h
+ $(QUIET_CC)$(CC) -o $*.o -c $(ALL_CFLAGS) -DHTML_DIR='"$(html_dir_SQ)"' -DHTML_VIEWER='"$(HTML_VIEWER)"' $<
git-merge-subtree$X: git-merge-recursive$X
$(QUIET_BUILT_IN)rm -f $@ && ln git-merge-recursive$X $@
diff --git a/cache.h b/cache.h
index 5e7381e..60e586c 100644
--- a/cache.h
+++ b/cache.h
@@ -288,6 +288,8 @@ extern size_t packed_git_window_size;
extern size_t packed_git_limit;
extern size_t delta_base_cache_limit;
extern int auto_crlf;
+extern int show_html_help;
+extern const char *html_help_program;
#define GIT_REPO_VERSION 0
extern int repository_format_version;
diff --git a/config.c b/config.c
index 58d3ed5..9d91a06 100644
--- a/config.c
+++ b/config.c
@@ -382,6 +382,23 @@ int git_default_config(const char *var, const char *value)
return 0;
}
+ if (!strcmp(var, "core.help")) {
+ if (!strcmp(value, "auto"))
+ show_html_help = 2;
+ else if (!strcmp(value, "html"))
+ show_html_help = 1;
+ else if (!strcmp(value, "man"))
+ show_html_help = 0;
+ else
+ return 1;
+ return 0;
+ }
+
+ if (!strcmp(var, "core.htmlviewer")) {
+ html_help_program = xstrdup(value);
+ return 0;
+ }
+
/* Add other config variables here and to Documentation/config.txt. */
return 0;
}
diff --git a/config.mak.in b/config.mak.in
index a3032e3..c3e410d 100644
--- a/config.mak.in
+++ b/config.mak.in
@@ -8,12 +8,14 @@ TAR = @TAR@
#INSTALL = @INSTALL@ # needs install-sh or install.sh in sources
TCLTK_PATH = @TCLTK_PATH@
+PACKAGE_TARNAME=@PACKAGE_TARNAME@
prefix = @prefix@
exec_prefix = @exec_prefix@
bindir = @bindir@
#gitexecdir = @libexecdir@/git-core/
datarootdir = @datarootdir@
template_dir = @datadir@/git-core/templates/
+html_dir = @docdir@/html/
mandir=@mandir@
diff --git a/environment.c b/environment.c
index 8b9b89d..aa22c68 100644
--- a/environment.c
+++ b/environment.c
@@ -32,6 +32,8 @@ size_t delta_base_cache_limit = 16 * 1024 * 1024;
int pager_in_use;
int pager_use_color = 1;
int auto_crlf = 0; /* 1: both ways, -1: only when adding git objects */
+int show_html_help = 0;
+const char *html_help_program = NULL;
static const char *git_dir;
static char *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file;
diff --git a/git.c b/git.c
index 29b55a1..c3c0fe8 100644
--- a/git.c
+++ b/git.c
@@ -4,7 +4,7 @@
#include "quote.h"
const char git_usage_string[] =
- "git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate] [--bare] [--git-dir=GIT_DIR] [--help] COMMAND [ARGS]";
+ "git [--version] [--exec-path[=GIT_EXEC_PATH]] [-p|--paginate] [--bare] [--git-dir=GIT_DIR] [--help [--html]] COMMAND [ARGS]";
static void prepend_to_path(const char *dir, int len)
{
diff --git a/help.c b/help.c
index 1cd33ec..52474e9 100644
--- a/help.c
+++ b/help.c
@@ -9,6 +9,14 @@
#include "common-cmds.h"
#include <sys/ioctl.h>
+#ifndef HTML_DIR
+#define HTML_DIR "/usr/share/html/"
+#endif
+
+#ifndef HTML_VIEWER
+#define HTML_VIEWER "xdg-open"
+#endif
+
/* most GUI terminals set COLUMNS (although some don't export it) */
static int term_columns(void)
{
@@ -183,6 +191,140 @@ static void show_man_page(const char *git_cmd)
execlp("man", "man", page, NULL);
}
+static void show_html_page(const char *git_cmd)
+{
+ const char *html_dir;
+ int len, ret, nr_quotes;
+ char *p, *p2;
+ const char *cp, *cp2;
+ struct stat st;
+ char *quoted_git_cmd;
+ const char *command;
+
+ html_dir = HTML_DIR;
+ command = html_help_program ? html_help_program : HTML_VIEWER;
+
+ nr_quotes = 0;
+ for (cp = git_cmd; *cp; cp++)
+ if (*cp == '\'') nr_quotes ++;
+
+ len = strlen(git_cmd) + nr_quotes*2 + 2 + 4; /* two quotes and git- */
+ quoted_git_cmd = p2 = xmalloc(len + 1);
+ *p2++ = '\'';
+ if (prefixcmp(git_cmd, "git")) {
+ strcpy(p2,"git-");
+ p2 += 4;
+ }
+ for (cp = git_cmd; *cp; cp ++) {
+ if (*cp == '\'')
+ *p2++ = '\\';
+ *p2++ = *cp;
+ }
+ *p2++ = '\'';
+ *p2 = 0;
+
+ /* first pass, calculate command length */
+ cp = command;
+ len = 0;
+ while (*cp && (cp2 = strchr(cp, '%'))) {
+ len += cp2 - cp;
+
+ if (!cp2[1]) break;
+
+ switch (cp2[1]) {
+ case 'p':
+ len += strlen(html_dir) + strlen(quoted_git_cmd) + 5; /* .html */
+ break;
+
+ case 'f':
+ len += strlen(quoted_git_cmd) + 5; /* .html */
+ break;
+
+ case 'b':
+ len += strlen(quoted_git_cmd);
+ break;
+
+ default:
+ len += 2;
+ }
+ cp = cp2 + 2;
+ }
+ if (!len) /* no expansion, append %p */
+ len += 1 + strlen(html_dir) + strlen(quoted_git_cmd) + 5;
+ if (*cp)
+ len += strlen(cp);
+
+ /* second pass */
+ cp = command;
+ p = p2 = xmalloc(len + 1);
+ while (*cp && (cp2 = strchr(cp, '%'))) {
+ len = cp2 - cp;
+ memcpy(p2, cp, len);
+ p2 += len;
+
+ if (!cp2[1]) break;
+
+ switch (cp2[1]) {
+ case 'p':
+ len = strlen(html_dir);
+ memcpy(p2, html_dir, len);
+ p2 += len;
+
+ len = strlen(quoted_git_cmd);
+ memcpy(p2, quoted_git_cmd, len);
+ p2 += len;
+
+ memcpy(p2, ".html", 5);
+ p2 += 5;
+ break;
+
+ case 'f':
+ len = strlen(quoted_git_cmd);
+ memcpy(p2, quoted_git_cmd, len);
+ p2 += len;
+
+ memcpy(p2, ".html", 5);
+ p2 += 5;
+ break;
+
+ case 'b':
+ len = strlen(quoted_git_cmd);
+ memcpy(p2, quoted_git_cmd, len);
+ p2 += len;
+ break;
+
+ default:
+ *p2++ = cp2[0];
+ *p2++ = cp2[1];
+ }
+ cp = cp2+2;
+ }
+ if (*cp)
+ strcpy(p2,cp);
+ else
+ *p2 = 0;
+ if (p == p2) { /* no expansion, append %p */
+ strcat(p2, " ");
+ strcat(p2, html_dir);
+ strcat(p2, quoted_git_cmd);
+ strcat(p2, ".html");
+ }
+
+ free(quoted_git_cmd);
+
+ ret = system(p);
+
+ if (ret == -1)
+ error("Failed to run %s", p);
+
+ free(p);
+
+ /* fallback to man pages */
+ if (show_html_help > 1 && (ret == -1 || ret > 0))
+ show_man_page(git_cmd);
+ exit(ret);
+}
+
void help_unknown_cmd(const char *cmd)
{
printf("git: '%s' is not a git-command\n\n", cmd);
@@ -214,8 +356,23 @@ int cmd_help(int argc, const char **argv, const char *prefix)
exit(1);
}
- else
- show_man_page(help_cmd);
+ else {
+ git_config(git_default_config);
+ if (!strcmp(help_cmd, "--html")) {
+ help_cmd = argc > 2 ? argv[2] : NULL;
+ if (!help_cmd) {
+ printf("usage: %s\n\n", git_usage_string);
+ list_common_cmds_help();
+ exit(1);
+ }
+ show_html_help = 1;
+ }
+
+ if (show_html_help)
+ show_html_page(help_cmd);
+ else
+ show_man_page(help_cmd);
+ }
return 0;
}
--
1.5.2
^ permalink raw reply related
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