* [PATCH] Teach the --all option to 'git fetch'
From: Björn Gustavsson @ 2009-11-08 8:34 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
'git remote' is meant for managing remotes and 'git fetch' is meant
for actually fetching data from remote repositories. Therefore, it is
not logical that you must use 'git remote update' to fetch from
several repositories at once. (Junio called 'git remote update'
a "half-baked UI experiment that failed" in topic 130891 in Gmane.)
Add the --all option to 'git fetch', to tell it to attempt to fetch
from all remotes. (The configuration variable skipDefaultUpdate for
the remote will NOT be consulted.)
Other options except -v and -q are silently ignored.
Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
See
http://thread.gmane.org/gmane.comp.version-control.git/130819/focus=130891
for a previous discussion.
My implementation is deliberately minimal:
* There is no way to configure that certain remotes should be skipped by
the --all option. I have never used skipDefaultUpdate with 'git remote
update'. If there is a real need for that feature, it can easily be added.
(But it should not use the existing skipDefaultUpdate configuration
variable, as the name does not make sense for 'git fetch --all'.)
* All options except -q and -v are silently ignored. It might be useful
to support some more options if there is a real need for them.
(Perhaps --keep or --no-tags?)
Documentation/git-fetch.txt | 5 +++
builtin-fetch.c | 80 +++++++++++++++++++++++++++++++++++++-----
t/t5514-fetch-all.sh | 76 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 151 insertions(+), 10 deletions(-)
create mode 100755 t/t5514-fetch-all.sh
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index f2483d6..9172454 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -10,6 +10,8 @@ SYNOPSIS
--------
'git fetch' <options> <repository> <refspec>...
+'git fetch' --all <options>
+
DESCRIPTION
-----------
@@ -31,6 +33,9 @@ branches you are not interested in, you will not get them.
OPTIONS
-------
+--all::
+ Fetch all remotes.
+
include::fetch-options.txt[]
include::pull-fetch-param.txt[]
diff --git a/builtin-fetch.c b/builtin-fetch.c
index a35a6f8..c1c3c46 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -14,6 +14,7 @@
static const char * const builtin_fetch_usage[] = {
"git fetch [options] [<repository> <refspec>...]",
+ "git fetch --all [options]",
NULL
};
@@ -23,7 +24,7 @@ enum {
TAGS_SET = 2
};
-static int append, force, keep, update_head_ok, verbosity;
+static int all, append, force, keep, update_head_ok, verbosity;
static int tags = TAGS_DEFAULT;
static const char *depth;
static const char *upload_pack;
@@ -32,6 +33,8 @@ static struct transport *transport;
static struct option builtin_fetch_options[] = {
OPT__VERBOSITY(&verbosity),
+ OPT_BOOLEAN(0, "all", &all,
+ "fetch from all remotes"),
OPT_BOOLEAN('a', "append", &append,
"append to .git/FETCH_HEAD instead of overwriting"),
OPT_STRING(0, "upload-pack", &upload_pack, "PATH",
@@ -680,7 +683,53 @@ static void set_option(const char *name, const char *value)
name, transport->url);
}
-int cmd_fetch(int argc, const char **argv, const char *prefix)
+static int get_one_remote_for_fetch(struct remote *remote, void *priv)
+{
+ struct string_list *list = priv;
+ string_list_append(remote->name, list);
+ return 0;
+}
+
+static int fetch_all(int argc)
+{
+ int i, result = 0;
+ struct string_list list = { NULL, 0, 0, 0 };
+ const char *argv[] = { "fetch", NULL, NULL, NULL, NULL };
+
+ if (argc == 1)
+ die("fetch --all does not take a repository argument");
+ else if (argc > 1)
+ die("fetch --all does not make sense with refspecs");
+
+ argc = 1;
+ if (verbosity >= 2)
+ argv[argc++] = "-v";
+ if (verbosity >= 1)
+ argv[argc++] = "-v";
+ else if (verbosity < 0)
+ argv[argc++] = "-q";
+
+ result = for_each_remote(get_one_remote_for_fetch, &list);
+
+ for (i = 0; i < list.nr; i++) {
+ const char *name = list.items[i].string;
+ argv[argc] = name;
+ if (verbosity >= 0)
+ printf("Fetching %s\n", name);
+ if (run_command_v_opt(argv, RUN_GIT_CMD)) {
+ error("Could not fetch %s", name);
+ result = 1;
+ }
+ }
+
+ /* all names were strdup()ed or strndup()ed */
+ list.strdup_strings = 1;
+ string_list_clear(&list, 0);
+
+ return result;
+}
+
+static int fetch_one(int argc, const char **argv)
{
struct remote *remote;
int i;
@@ -688,14 +737,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
int ref_nr = 0;
int exit_code;
- /* Record the command line for the reflog */
- strbuf_addstr(&default_rla, "fetch");
- for (i = 1; i < argc; i++)
- strbuf_addf(&default_rla, " %s", argv[i]);
-
- argc = parse_options(argc, argv, prefix,
- builtin_fetch_options, builtin_fetch_usage, 0);
-
if (argc == 0)
remote = remote_get(NULL);
else
@@ -746,3 +787,22 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
transport = NULL;
return exit_code;
}
+
+int cmd_fetch(int argc, const char **argv, const char *prefix)
+{
+ int i;
+
+ /* Record the command line for the reflog */
+ strbuf_addstr(&default_rla, "fetch");
+ for (i = 1; i < argc; i++)
+ strbuf_addf(&default_rla, " %s", argv[i]);
+
+ argc = parse_options(argc, argv, prefix,
+ builtin_fetch_options, builtin_fetch_usage, 0);
+
+ if (all) {
+ return fetch_all(argc);
+ } else {
+ return fetch_one(argc, argv);
+ }
+}
diff --git a/t/t5514-fetch-all.sh b/t/t5514-fetch-all.sh
new file mode 100755
index 0000000..25244bf
--- /dev/null
+++ b/t/t5514-fetch-all.sh
@@ -0,0 +1,76 @@
+#!/bin/sh
+
+test_description='fetch --all works correctly'
+
+. ./test-lib.sh
+
+setup_repository () {
+ mkdir "$1" && (
+ cd "$1" &&
+ git init &&
+ >file &&
+ git add file &&
+ test_tick &&
+ git commit -m "Initial" &&
+ git checkout -b side &&
+ >elif &&
+ git add elif &&
+ test_tick &&
+ git commit -m "Second" &&
+ git checkout master
+ )
+}
+
+test_expect_success setup '
+ setup_repository one &&
+ setup_repository two &&
+ (
+ cd two && git branch another
+ ) &&
+ git clone --mirror two three
+ git clone one test
+'
+
+cat > test/expect << EOF
+ one/master
+ one/side
+ origin/HEAD -> origin/master
+ origin/master
+ origin/side
+ three/another
+ three/master
+ three/side
+ two/another
+ two/master
+ two/side
+EOF
+
+test_expect_success 'git fetch --all' '
+ (cd test &&
+ git remote add one ../one &&
+ git remote add two ../two &&
+ git remote add three ../three &&
+ git fetch --all &&
+ git branch -r > output &&
+ test_cmp expect output)
+'
+
+test_expect_success 'git fetch --all should continue if a remote has errors' '
+ (git clone one test2 &&
+ cd test2 &&
+ git remote add bad ../non-existing &&
+ git remote add one ../one &&
+ git remote add two ../two &&
+ git remote add three ../three &&
+ test_must_fail git fetch --all &&
+ git branch -r > output &&
+ test_cmp ../test/expect output)
+'
+
+test_expect_success 'git fetch --all does not allow non-option arguments' '
+ (cd test &&
+ test_must_fail git fetch --all origin &&
+ test_must_fail git fetch --all origin master)
+'
+
+test_done
--
1.6.5.1.69.g36942
^ permalink raw reply related
* [PATCH] Show usage string for 'git http-push -h'
From: Jonathan Nieder @ 2009-11-08 7:26 UTC (permalink / raw)
To: git; +Cc: Tay Ray Chuan
In-Reply-To: <20091108071152.GA20741@progeny.tock>
git http-push already knows how to dump usage if it is given no
options, but it interprets '-h' as the URL to a remote repository:
$ git http-push -h
error: Cannot access URL -h/, return code 6
Dump usage on -h, instead. Humans wanting to pass the URL -h/ to
curl for some reason can use 'git http-push -h/' explicitly.
Scripts expecting to access an HTTP repository at URL '-h' will
break, though.
Cc: Tay Ray Chuan <rctay89@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Jonathan Nieder wrote:
> Some commands
> do not show usage with '-h' and have been left unchanged.
Like this one. Full list of non-builtin commands in C like this:
http-push, fast-import, imap-send, remote-curl, show-index.
http-push.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/http-push.c b/http-push.c
index 00e83dc..2e0782a 100644
--- a/http-push.c
+++ b/http-push.c
@@ -13,8 +13,8 @@
#include <expat.h>
-static const char http_push_usage[] =
-"git http-push [--all] [--dry-run] [--force] [--verbose] <remote> [<head>...]\n";
+static const char http_push_usage[] = "git http-push "
+"[-h] [--all] [--dry-run] [--force] [--verbose] <remote> [<head>...]\n";
#ifndef XML_STATUS_OK
enum XML_Status {
@@ -1792,6 +1792,11 @@ int main(int argc, char **argv)
git_extract_argv0_path(argv[0]);
+ if (argc == 2 && !strcmp(argv[1], "-h")) {
+ fprintf(stderr, "usage: %s\n", http_push_usage);
+ return 0;
+ }
+
setup_git_directory();
repo = xcalloc(sizeof(*repo), 1);
--
1.6.5.2
^ permalink raw reply related
* [PATCH] Let 'git <command> -h' show usage without a git dir
From: Jonathan Nieder @ 2009-11-08 7:11 UTC (permalink / raw)
To: git; +Cc: Gerfried Fuchs, 462557
In-Reply-To: <20080125173149.GA10287@edna.gwendoline.at>
Hi,
Gerfried Fuchs wrote:
> I really wonder why "git <command> -h" depends on being inside a
> repository. I noticed it with "git diff -h" (add, branch does that, too):
>
> #v+
> ~/git> git tag -h
> usage: git-tag [-n [<num>]] -l [<pattern>] | [-a | -s | -u <key-id>] [-f | -d | -v] [-m <msg> | -F <file>] <tagname> [<head>]
> ~/git> cd
> ~> git tag -h
> fatal: Not a git repository
> ~>
> #v-
This is a nuisance, I agree.
So how about something like this patch? This just avoids looking for
a .git directory if the only option to a subcommand is '-h'.
-- %< --
Subject: [PATCH] Let 'git <command> -h' show usage without a git dir
There is no need for "git <command> -h" to depend on being inside
a repository.
Reported by Gerfried Fuchs through http://bugs.debian.org/462557
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Tested with all builtins and non-builtins written in C. Some commands
do not show usage with '-h' and have been left unchanged.
git.c | 48 ++++++++++++++++++++++++++++++------------------
http-fetch.c | 13 ++++++++++++-
index-pack.c | 5 +++++
pack-redundant.c | 5 +++++
4 files changed, 52 insertions(+), 19 deletions(-)
diff --git a/git.c b/git.c
index bd2c5fe..bfa9518 100644
--- a/git.c
+++ b/git.c
@@ -220,6 +220,11 @@ const char git_version_string[] = GIT_VERSION;
* RUN_SETUP for reading from the configuration file.
*/
#define NEED_WORK_TREE (1<<2)
+/*
+ * Let RUN_SETUP, USE_PAGER, and NEED_WORK_TREE take effect even if
+ * passed the -h option.
+ */
+#define H_IS_NOT_HELP (1<<3)
struct cmd_struct {
const char *cmd;
@@ -229,21 +234,25 @@ struct cmd_struct {
static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
{
- int status;
+ int status, help;
struct stat st;
const char *prefix;
prefix = NULL;
- if (p->option & RUN_SETUP)
- prefix = setup_git_directory();
-
- if (use_pager == -1 && p->option & RUN_SETUP)
- use_pager = check_pager_config(p->cmd);
- if (use_pager == -1 && p->option & USE_PAGER)
- use_pager = 1;
+ help = argc == 2 && !(p->option & H_IS_NOT_HELP) &&
+ !strcmp(argv[1], "-h");
+ if (!help) {
+ if (p->option & RUN_SETUP && !help)
+ prefix = setup_git_directory();
+
+ if (use_pager == -1 && p->option & RUN_SETUP)
+ use_pager = check_pager_config(p->cmd);
+ if (use_pager == -1 && p->option & USE_PAGER)
+ use_pager = 1;
+ }
commit_pager_choice();
- if (p->option & NEED_WORK_TREE)
+ if (!help && p->option & NEED_WORK_TREE)
setup_work_tree();
trace_argv_printf(argv, "trace: built-in: git");
@@ -278,7 +287,8 @@ static void handle_internal_command(int argc, const char **argv)
{ "annotate", cmd_annotate, RUN_SETUP },
{ "apply", cmd_apply },
{ "archive", cmd_archive },
- { "bisect--helper", cmd_bisect__helper, RUN_SETUP | NEED_WORK_TREE },
+ { "bisect--helper", cmd_bisect__helper,
+ RUN_SETUP | NEED_WORK_TREE },
{ "blame", cmd_blame, RUN_SETUP },
{ "branch", cmd_branch, RUN_SETUP },
{ "bundle", cmd_bundle },
@@ -288,12 +298,12 @@ static void handle_internal_command(int argc, const char **argv)
RUN_SETUP | NEED_WORK_TREE},
{ "check-ref-format", cmd_check_ref_format },
{ "check-attr", cmd_check_attr, RUN_SETUP },
- { "cherry", cmd_cherry, RUN_SETUP },
+ { "cherry", cmd_cherry, RUN_SETUP | H_IS_NOT_HELP },
{ "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
{ "clone", cmd_clone },
{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
- { "commit-tree", cmd_commit_tree, RUN_SETUP },
+ { "commit-tree", cmd_commit_tree, RUN_SETUP | H_IS_NOT_HELP },
{ "config", cmd_config },
{ "count-objects", cmd_count_objects, RUN_SETUP },
{ "describe", cmd_describe, RUN_SETUP },
@@ -304,7 +314,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "fast-export", cmd_fast_export, RUN_SETUP },
{ "fetch", cmd_fetch, RUN_SETUP },
{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
- { "fetch--tool", cmd_fetch__tool, RUN_SETUP },
+ { "fetch--tool", cmd_fetch__tool, RUN_SETUP | H_IS_NOT_HELP },
{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
{ "format-patch", cmd_format_patch, RUN_SETUP },
@@ -312,7 +322,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "fsck-objects", cmd_fsck, RUN_SETUP },
{ "gc", cmd_gc, RUN_SETUP },
{ "get-tar-commit-id", cmd_get_tar_commit_id },
- { "grep", cmd_grep, RUN_SETUP | USE_PAGER },
+ { "grep", cmd_grep, RUN_SETUP | USE_PAGER | H_IS_NOT_HELP },
{ "help", cmd_help },
{ "init", cmd_init_db },
{ "init-db", cmd_init_db },
@@ -325,9 +335,11 @@ static void handle_internal_command(int argc, const char **argv)
{ "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
{ "merge-base", cmd_merge_base, RUN_SETUP },
{ "merge-file", cmd_merge_file },
- { "merge-ours", cmd_merge_ours, RUN_SETUP },
- { "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
- { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
+ { "merge-ours", cmd_merge_ours, RUN_SETUP | H_IS_NOT_HELP },
+ { "merge-recursive", cmd_merge_recursive,
+ RUN_SETUP | NEED_WORK_TREE | H_IS_NOT_HELP },
+ { "merge-subtree", cmd_merge_recursive,
+ RUN_SETUP | NEED_WORK_TREE | H_IS_NOT_HELP },
{ "mktree", cmd_mktree, RUN_SETUP },
{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
{ "name-rev", cmd_name_rev, RUN_SETUP },
@@ -368,7 +380,7 @@ static void handle_internal_command(int argc, const char **argv)
{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
{ "write-tree", cmd_write_tree, RUN_SETUP },
{ "verify-pack", cmd_verify_pack },
- { "show-ref", cmd_show_ref, RUN_SETUP },
+ { "show-ref", cmd_show_ref, RUN_SETUP | H_IS_NOT_HELP },
{ "pack-refs", cmd_pack_refs, RUN_SETUP },
};
int i;
diff --git a/http-fetch.c b/http-fetch.c
index e8f44ba..85f5338 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -1,6 +1,10 @@
#include "cache.h"
+#include "exec_cmd.h"
#include "walker.h"
+static const char http_fetch_usage[] = "git http-fetch "
+ "[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin] commit-id url";
+
int main(int argc, const char **argv)
{
const char *prefix;
@@ -19,6 +23,13 @@ int main(int argc, const char **argv)
int get_verbosely = 0;
int get_recover = 0;
+ git_extract_argv0_path(argv[0]);
+
+ if (argc == 2 && !strcmp(argv[1], "-h")) {
+ fprintf(stderr, "%s\n", http_fetch_usage);
+ return 0;
+ }
+
prefix = setup_git_directory();
git_config(git_default_config, NULL);
@@ -45,7 +56,7 @@ int main(int argc, const char **argv)
arg++;
}
if (argc < arg + 2 - commits_on_stdin) {
- usage("git http-fetch [-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin] commit-id url");
+ usage(http_fetch_usage);
return 1;
}
if (commits_on_stdin) {
diff --git a/index-pack.c b/index-pack.c
index b4f8278..4a7d405 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -882,6 +882,11 @@ int main(int argc, char **argv)
git_extract_argv0_path(argv[0]);
+ if (argc == 2 && !strcmp(argv[1], "-h")) {
+ fprintf(stderr, "usage: %s\n", index_pack_usage);
+ return 0;
+ }
+
/*
* We wish to read the repository's config file if any, and
* for that it is necessary to call setup_git_directory_gently().
diff --git a/pack-redundant.c b/pack-redundant.c
index 69a7ab2..24d59f9 100644
--- a/pack-redundant.c
+++ b/pack-redundant.c
@@ -603,6 +603,11 @@ int main(int argc, char **argv)
git_extract_argv0_path(argv[0]);
+ if (argc == 2 && !strcmp(argv[1], "-h")) {
+ fprintf(stderr, "usage: %s\n", pack_redundant_usage);
+ return 0;
+ }
+
setup_git_directory();
for (i = 1; i < argc; i++) {
--
1.6.5.2
^ permalink raw reply related
* Re: performance issue: git clone compression
From: Nicolas Pitre @ 2009-11-08 3:04 UTC (permalink / raw)
To: Tim Webster; +Cc: git
In-Reply-To: <72877ab10911071845q1c60e2b0kfe6f0509b5728912@mail.gmail.com>
On Sat, 7 Nov 2009, Tim Webster wrote:
> On servers with limit memory it is not piratical to run git gc.
> I can run git gc on a bare mirror and rsync back to the server.
That is fine. As long as you end up with a single pack, then a clone
would basically be a sinple file transfer.
Also you might want to investigate the following config variable for a
low-memory server:
core.packedGitWindowSize
core.packedGitLimit
pack.windowMemory
pack.deltaCacheSize
pack.deltaCacheLimit
Nicolas
^ permalink raw reply
* Re: performance issue: git clone compression
From: Tim Webster @ 2009-11-08 2:45 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.2.00.0911072115270.16711@xanadu.home>
On servers with limit memory it is not piratical to run git gc.
I can run git gc on a bare mirror and rsync back to the server.
I would prefer to for go compress packing entirely for the large git
repositories containing media files.
Can I disable compression by file attribute via .gitattributes?
I am trying to use git for all repositories to avoid a mix of version
control methods.
For media repositories I am using
[core]
repositoryformatversion = 0
filemode = false
bare = false
logallrefupdates = true
compression = 0
loosecompression = 0
[pack]
# disable delta-based packing
depth = 1
# disable compression
compression = 0
There is no problem with git pull and push
Just git clone swaps the server to death
> Make sure the remote repository is fully packed. To do so, just go into
> the remote repository and run 'git gc'.
>
>
> Nicolas
>
^ permalink raw reply
* Re: performance issue: git clone compression
From: Nicolas Pitre @ 2009-11-08 2:17 UTC (permalink / raw)
To: Tim Webster; +Cc: git
In-Reply-To: <72877ab10911071709s200696d4mf12dc797da20be18@mail.gmail.com>
On Sat, 7 Nov 2009, Tim Webster wrote:
> git clone gitosis@gitserver:code.git
>
> After remote counting objects, remote compressing objects
> transfer begins...
> This compression operation results in excess of swapage on the remote server.
>
> --------------------
>
> I need to avoiding the clone compression step for servers with limited memory.
Make sure the remote repository is fully packed. To do so, just go into
the remote repository and run 'git gc'.
Nicolas
^ permalink raw reply
* performance issue: git clone compression
From: Tim Webster @ 2009-11-08 1:09 UTC (permalink / raw)
To: git
In-Reply-To: <72877ab10911071657p568b3b98v6fd90e84e098c107@mail.gmail.com>
git clone gitosis@gitserver:code.git
After remote counting objects, remote compressing objects
transfer begins...
This compression operation results in excess of swapage on the remote server.
--------------------
I need to avoiding the clone compression step for servers with limited memory.
I am doing the following to avoid this object compression. Is there a
better way?
-------------------
scp -r gitserver:/srv/gitosis/repositories/code.git .
git clone /tmp/junk/code.git --no-hardlinks code
# IS there a better way of converting the bare git repository to
working git repository?
edit .git/config
url = gitosis@gitserver:code.git
^ permalink raw reply
* [PATCH][next] log --format: don't ignore %w() at the start of format string
From: René Scharfe @ 2009-11-08 1:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
This fixes e.g. --format='%w(72)%s'.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
pretty.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/pretty.c b/pretty.c
index 5c3b47b..2e031e6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -619,7 +619,7 @@ static void rewrap_message_tail(struct strbuf *sb,
if (c->width == new_width && c->indent1 == new_indent1 &&
c->indent2 == new_indent2)
return;
- if (c->wrap_start && c->wrap_start < sb->len)
+ if (c->wrap_start < sb->len)
strbuf_wrap(sb, c->wrap_start, c->width, c->indent1, c->indent2);
c->wrap_start = sb->len;
c->width = new_width;
^ permalink raw reply related
* Re: [PATCH 0/3] Generalized "string function" syntax
From: René Scharfe @ 2009-11-08 1:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vd44jx9at.fsf@alter.siamese.dyndns.org>
Junio C Hamano schrieb:
> René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
>> I'm more in favour of adding ways to customize the shape of the elements
>> rather than adding string functions. %S(width=76,indent=4) over
>> %[wrap(76,4)%s%].
>
> Yeah, %X(some modifier) that can apply to any 'X' looks much simpler and
> easier to look at. The way the code is structured currently it might be
> more work and risk to break things, though.
Here's something along this line. It's an experiment that tries to
explore named parameters and extending individual place holders instead of
adding a generic modifier (%w).
The patch implements a way to pass named parameters to %s and %b. Those
parameters are width, indent1 and indent2, which are then passed to
strbuf_add_wrapped_text() to indent and wrap the subject or body text,
respectively.
Handling wrapping for the individual place holders avoids having to deal
with terminal colour codes. The patch that implements %w() currently in
next ignores this issue.
It also allows to avoid copying the results around -- no temporary strbuf
is needed for %b(). However, quick tests showed no measurable performance
improvement.
While indent1 and indent2 are numerical parameters in this patch, they
really should be strings, in order to allow indenting using tabs etc..
For that to work, strbuf_add_wrapped_text() would need to be changed
first, though.
The parser parse_params() supports strings and numerical values, but only
the latter are used in this patch. String parameters are intended to be
fed to unquote_c_style(). It never complains about syntax errors. It
aborts if the string ends prematurely, but otherwise ignores invalid input
and just keeps going. That's how the % place holder parser has always
worked, but perhaps the introduction of named parameters justifies a more
strict approach?
My main motivation for this experiment was to avoid extra copies in the
hope to speed things up. However, my basic timing tests show that it's
not that much of an improvement.
The remaining reason is the handling of colour codes. I think we can keep
ignoring this issue -- the only impact is that lines with colour codes and
wrapping combined (e.g. "%w(72)%Cred%s") shorter than they should be,
because the colour code is considered (incorrectly) to have a non-zero
width. I think we can get away with mentioning that fact in the docs.
After all, one can simple use "%Cred%w(72)%s".
Anyway, here's the patch.
pretty.c | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 150 insertions(+), 4 deletions(-)
diff --git a/pretty.c b/pretty.c
index da15cf2..09272ec 100644
--- a/pretty.c
+++ b/pretty.c
@@ -596,6 +596,117 @@ static void format_decoration(struct strbuf *sb, const struct commit *commit)
strbuf_addch(sb, ')');
}
+struct param {
+ const char *name;
+ long *numeric_value;
+ const char *value;
+ size_t value_len;
+};
+
+static size_t prefixlen(const char *s, const char *prefix)
+{
+ size_t len = 0;
+
+ while (*prefix) {
+ if (*s != *prefix)
+ return 0;
+ s++;
+ prefix++;
+ len++;
+ }
+ return len;
+}
+
+static size_t parse_params(const char *s, struct param *params, int count,
+ int end)
+{
+ const char *start = s;
+ int i;
+
+ for (;;) {
+ while (isspace(*s))
+ s++;
+again:
+ if (*s == end)
+ return s - start + 1;
+ if (*s == '\0')
+ return 0;
+
+ for (i = 0; i < count; i++) {
+ size_t len = prefixlen(s, params[i].name);
+ if (len) {
+ if (s[len] == end)
+ return s - start + 1;
+ if (s[len] == '\0')
+ return 0;
+ if (isspace(s[len])) {
+ s += len + 1;
+ while (isspace(*s))
+ s++;
+ if (*s != '=')
+ goto again;
+ s++;
+ break;
+ } else if (s[len] == '=') {
+ s += len + 1;
+ break;
+ }
+ }
+ }
+
+ if (i < count) {
+ while (isspace(*s))
+ s++;
+ if (*s == end)
+ return s - start + 1;
+ if (*s == '\0')
+ return 0;
+
+ params[i].value = s;
+ if (*s == '"') {
+ for (;;) {
+ int quoted = 0;
+ const char *q;
+
+ s = strchr(s + 1, '"');
+ if (!s)
+ return 0;
+
+ for (q = s - 1; *q == '\\'; q--)
+ quoted = !quoted;
+ if (!quoted)
+ break;
+ }
+ s++;
+ } else {
+ char *endp;
+ long num = strtol(s, &endp, 10);
+
+ s = endp;
+ if (isspace(*s) || *s == end) {
+ if (params[i].numeric_value)
+ *params[i].numeric_value = num;
+ } else {
+ while (!isspace(*s) && *s != end) {
+ if (*s == '\0')
+ return 0;
+ s++;
+ }
+ }
+ }
+ params[i].value_len = s - params[i].value;
+ } else {
+ while (!isspace(*s)) {
+ if (*s == end)
+ return s - start + 1;
+ if (*s == '\0')
+ return 0;
+ s++;
+ }
+ }
+ }
+}
+
static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
void *context)
{
@@ -744,14 +855,49 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
switch (placeholder[0]) {
case 's': /* subject */
- format_subject(sb, msg + c->subject_off, " ");
- return 1;
+ if (placeholder[1] == '(') {
+ struct strbuf subject = STRBUF_INIT;
+ long indent1 = 0, indent2 = 0, width = 0;
+ struct param params[] = {
+ { "indent1", &indent1 },
+ { "indent2", &indent2 },
+ { "width", &width },
+ };
+ size_t consumed = parse_params(placeholder + 2, params,
+ ARRAY_SIZE(params), ')');
+ if (!consumed)
+ return 0;
+ format_subject(&subject, msg + c->subject_off, " ");
+ strbuf_add_wrapped_text(sb, subject.buf, indent1,
+ indent2, width);
+ strbuf_release(&subject);
+ return consumed + 2;
+ } else {
+ format_subject(sb, msg + c->subject_off, " ");
+ return 1;
+ }
case 'f': /* sanitized subject */
format_sanitized_subject(sb, msg + c->subject_off);
return 1;
case 'b': /* body */
- strbuf_addstr(sb, msg + c->body_off);
- return 1;
+ if (placeholder[1] == '(') {
+ long indent1 = 0, indent2 = 0, width = 0;
+ struct param params[] = {
+ { "indent1", &indent1 },
+ { "indent2", &indent2 },
+ { "width", &width },
+ };
+ size_t consumed = parse_params(placeholder + 2, params,
+ ARRAY_SIZE(params), ')');
+ if (!consumed)
+ return 0;
+ strbuf_add_wrapped_text(sb, msg + c->body_off, indent1,
+ indent2, width);
+ return consumed + 2;
+ } else {
+ strbuf_addstr(sb, msg + c->body_off);
+ return 1;
+ }
}
return 0; /* unknown placeholder */
}
^ permalink raw reply related
* custom attributes per file
From: Daniel Poelzleithner @ 2009-11-07 23:55 UTC (permalink / raw)
To: git
Hi git devs,
I have seen on your TODO list, that you plan to add tree attributes. We
want to use git as a backend to store the wikipedia content in a
distributed manner. The exporter [1] is already quite stable it seems.
Today I started a frontend project [2] and have a feature request :-)
* Make attributes apply to trees, not just working tree.
What we need are versioned attributes per file and per tree. Currently
we export each article into a file, but there is no way we could save
custom attributes directly bound to it, which distributes the data which
should be together. If the maximum value per attribute should be quite
large, this way we could save reports of vote results bound to the
article directly.
I think unicode data to an ascii key would be sufficient, but real types
could provide great features in the future. Changes to these values
should be signable by gpg signed commit messages like a normal checkin.
Maybe in the long run, there could be some sort of export filter
mechanism. For cloning repositories, for example a copy of the wiki
database based on a quality key of an article could be exported for a
embedded device.
[1] http://github.com/scy/levitation
[2] http://github.com/poelzi/communiki
kindly regards
Daniel Poelzleithner
^ permalink raw reply
* A bug in gitview
From: Rafał Mużyło @ 2009-11-07 20:50 UTC (permalink / raw)
To: git
When I modified a html file, that was not utf-8
encoded and commited the changes, if I select
that particular commit in gitview, the content
isn't shown, an error is printed instead:
Traceback (most recent call last):
File "/usr/bin/gitview", line 1002, in _treeview_cursor_cb
self.message_buffer.set_text(unicode(message, self.encoding).encode('utf-8'))
UnicodeDecodeError: 'utf8' codec can't decode byte 0xa4 in position 4723:
unexpected code byte:
That's of course correct, it's not utf8.
'git diff' simply shows that as hex values,
there's probably a way to do the same for
gitview.
^ permalink raw reply
* [PATCH v2 4/4] Add explicit Cygwin check to guard WIN32 header inclusion
From: Ramsay Jones @ 2009-11-07 20:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list
Since commit 435bdf8c ("Make usage of windows.h lean and mean",
16-9-2009), the amount of code potentially including the WIN32
API header files has greatly increased. In particular, the Cygwin
build is at greater risk of inadvertently including WIN32 code
within preprocessor sections protected by the WIN32 or _WIN32
macros.
The previous commit message, along with comments elsewhere, assert
that the WIN32 macro is not defined on Cygwin. Currently, this is
true for the cygwin build. However, the cygwin platform can be
used to develop WIN32 GUI, WIN32 console, and POSIX applications.
Indeed it is possible to create applications which use a mix of
the WIN32 API and POSIX code (eg git!).
Unlike native WIN32 compilers, gcc on cygwin does not automatically
define the _WIN32 macro. However, as soon as you include the
<windows.h> header file, the _WIN32 and WIN32 macros are defined.
In order to reduce the risk of problems in the future, we protect
the inclusion of the windows header with an explicit check for
__CYGWIN__. Also, we move the other use of the <windows.h> header
from compat/win32.h to compat/cygwin.c
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Changes since v1:
- removed the definition of the WIN32_API macro.
- removed all usage of WIN32_API as a replacement for the use
of the _WIN32 and WIN32 macros in #if(n)def sections.
I wanted to separate these (remaining) changes from the WIN32_API
issue, since that is likely to require further discussion (along
with some bike-shedding), and I didn't want these to get held up.
compat/cygwin.c | 1 +
compat/win32.h | 3 ---
git-compat-util.h | 8 ++++----
3 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/compat/cygwin.c b/compat/cygwin.c
index b4a51b9..6695515 100644
--- a/compat/cygwin.c
+++ b/compat/cygwin.c
@@ -1,5 +1,6 @@
#define WIN32_LEAN_AND_MEAN
#include "../git-compat-util.h"
+#include <windows.h>
#include "win32.h"
#include "../cache.h" /* to read configuration */
diff --git a/compat/win32.h b/compat/win32.h
index 8ce9104..a7ed72b 100644
--- a/compat/win32.h
+++ b/compat/win32.h
@@ -2,9 +2,6 @@
#define WIN32_H
/* common Win32 functions for MinGW and Cygwin */
-#ifndef WIN32 /* Not defined by Cygwin */
-#include <windows.h>
-#endif
static inline int file_attr_to_st_mode (DWORD attr)
{
diff --git a/git-compat-util.h b/git-compat-util.h
index ef60803..c4b9e5a 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -65,10 +65,10 @@
#define _NETBSD_SOURCE 1
#define _SGI_SOURCE 1
-#ifdef WIN32 /* Both MinGW and MSVC */
-#define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
-#include <winsock2.h>
-#include <windows.h>
+#if defined(_WIN32) && !defined(__CYGWIN__) /* Both MinGW and MSVC */
+# define WIN32_LEAN_AND_MEAN /* stops windows.h including winsock.h */
+# include <winsock2.h>
+# include <windows.h>
#endif
#include <unistd.h>
--
1.6.5
^ permalink raw reply related
* [PATCH v2 2/4] Makefile: keep MSVC and Cygwin configuration separate
From: Ramsay Jones @ 2009-11-07 20:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Changes from v1:
- moved the "ifdef MSVC" section up to just after the block
which sets up uname_S, uname_O, etc.
- use simple assignment ':=' rather than recursive assignment
in the uname override block
Makefile | 11 ++++++++---
1 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 8e1cfc5..306ca86 100644
--- a/Makefile
+++ b/Makefile
@@ -212,6 +212,12 @@ uname_R := $(shell sh -c 'uname -r 2>/dev/null || echo not')
uname_P := $(shell sh -c 'uname -p 2>/dev/null || echo not')
uname_V := $(shell sh -c 'uname -v 2>/dev/null || echo not')
+ifdef MSVC
+ # avoid the MingW and Cygwin configuration sections
+ uname_S := Windows
+ uname_O := Windows
+endif
+
# CFLAGS and LDFLAGS are for the users to override from the command line.
CFLAGS = -g -O2 -Wall
@@ -893,7 +899,7 @@ ifeq ($(uname_S),HP-UX)
NO_SYS_SELECT_H = YesPlease
SNPRINTF_RETURNS_BOGUS = YesPlease
endif
-ifdef MSVC
+ifeq ($(uname_S),Windows)
GIT_VERSION := $(GIT_VERSION).MSVC
pathsep = ;
NO_PREAD = YesPlease
@@ -945,7 +951,7 @@ else
BASIC_CFLAGS += -Zi -MTd
endif
X = .exe
-else
+endif
ifneq (,$(findstring MINGW,$(uname_S)))
pathsep = ;
NO_PREAD = YesPlease
@@ -994,7 +1000,6 @@ else
NO_PTHREADS = YesPlease
endif
endif
-endif
-include config.mak.autogen
-include config.mak
--
1.6.5
^ permalink raw reply related
* [PATCH v2 3/4] MSVC: Add support for building with NO_MMAP
From: Ramsay Jones @ 2009-11-07 20:10 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list
When the NO_MMAP build variable is set, the msvc linker complains:
error LNK2001: unresolved external symbol _getpagesize
The msvc libraries do not define the getpagesize() function,
so we move the mingw_getpagesize() implementation from the
conditionally built win32mmap.c file to mingw.c.
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Changes since v1:
- re-worded the subject and commit message to, hopefully,
make it a little clearer.
compat/mingw.c | 12 ++++++++++++
compat/mingw.h | 2 +-
compat/win32mmap.c | 12 ------------
3 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/compat/mingw.c b/compat/mingw.c
index 6b5b5b2..15fe33e 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1000,6 +1000,18 @@ repeat:
return -1;
}
+/*
+ * Note that this doesn't return the actual pagesize, but
+ * the allocation granularity. If future Windows specific git code
+ * needs the real getpagesize function, we need to find another solution.
+ */
+int mingw_getpagesize(void)
+{
+ SYSTEM_INFO si;
+ GetSystemInfo(&si);
+ return si.dwAllocationGranularity;
+}
+
struct passwd *getpwuid(int uid)
{
static char user_name[100];
diff --git a/compat/mingw.h b/compat/mingw.h
index 5b5258b..26c4027 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -166,7 +166,7 @@ int mingw_connect(int sockfd, struct sockaddr *sa, size_t sz);
int mingw_rename(const char*, const char*);
#define rename mingw_rename
-#ifdef USE_WIN32_MMAP
+#if defined(USE_WIN32_MMAP) || defined(_MSC_VER)
int mingw_getpagesize(void);
#define getpagesize mingw_getpagesize
#endif
diff --git a/compat/win32mmap.c b/compat/win32mmap.c
index 779d796..1c5a149 100644
--- a/compat/win32mmap.c
+++ b/compat/win32mmap.c
@@ -1,17 +1,5 @@
#include "../git-compat-util.h"
-/*
- * Note that this doesn't return the actual pagesize, but
- * the allocation granularity. If future Windows specific git code
- * needs the real getpagesize function, we need to find another solution.
- */
-int mingw_getpagesize(void)
-{
- SYSTEM_INFO si;
- GetSystemInfo(&si);
- return si.dwAllocationGranularity;
-}
-
void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset)
{
HANDLE hmap;
--
1.6.5
^ permalink raw reply related
* [PATCH v2 1/4] Makefile: merge two Cygwin configuration sections into one
From: Ramsay Jones @ 2009-11-07 20:02 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list
Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---
Unchanged from v1.
Makefile | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/Makefile b/Makefile
index fea237b..8e1cfc5 100644
--- a/Makefile
+++ b/Makefile
@@ -782,6 +782,8 @@ ifeq ($(uname_O),Cygwin)
NO_MMAP = YesPlease
NO_IPV6 = YesPlease
X = .exe
+ COMPAT_OBJS += compat/cygwin.o
+ UNRELIABLE_FSTAT = UnfortunatelyYes
endif
ifeq ($(uname_S),FreeBSD)
NEEDS_LIBICONV = YesPlease
@@ -891,10 +893,6 @@ ifeq ($(uname_S),HP-UX)
NO_SYS_SELECT_H = YesPlease
SNPRINTF_RETURNS_BOGUS = YesPlease
endif
-ifneq (,$(findstring CYGWIN,$(uname_S)))
- COMPAT_OBJS += compat/cygwin.o
- UNRELIABLE_FSTAT = UnfortunatelyYes
-endif
ifdef MSVC
GIT_VERSION := $(GIT_VERSION).MSVC
pathsep = ;
--
1.6.5
^ permalink raw reply related
* [PATCH v2 0/4] Cygwin MSVC patches
From: Ramsay Jones @ 2009-11-07 20:00 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Marius Storm-Olsen, Johannes Sixt, GIT Mailing-list
Hi Junio,
Sorry for the delay in sending these, something came up.
I will be sending:
[PATCH 1/4] Makefile: merge two Cygwin configuration sections into one
[PATCH 2/4] Makefile: keep MSVC and Cygwin configuration separate
[PATCH 3/4] MSVC: Add support for building with NO_MMAP
[PATCH 4/4] Add explicit Cygwin check to guard WIN32 header inclusion
Note that I didn't know if I should send patch #1, since it is already
in pu, but I'm sure it's easy for you to drop if you don't want it. ;-)
As requested, I've moved the old patch #1 to come after the old patch #3
(which is now patch #2). I also amended to commit subject and message.
Patch #4 has been cut down and the commit subject and message changed
to reflect the new scope of the patch.
I hope this covers all comments!
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [cgit PATCH] Close file descriptor on error in readfile()
From: Lars Hjemli @ 2009-11-07 17:15 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Rys Sommefeldt, git, steven
In-Reply-To: <m2ocneb9cc.fsf@igel.home>
On Sat, Nov 7, 2009 at 17:14, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Lars Hjemli <hjemli@gmail.com> writes:
>
>> diff --git a/shared.c b/shared.c
>> index d7b2d5a..a27ab30 100644
>> --- a/shared.c
>> +++ b/shared.c
>> @@ -406,12 +406,17 @@ int readfile(const char *path, char **buf, size_t *size)
>> fd = open(path, O_RDONLY);
>> if (fd == -1)
>> return errno;
>> - if (fstat(fd, &st))
>> + if (fstat(fd, &st)) {
>> + close(fd);
>> return errno;
>
> The close call can clobber errno.
>
>> - if (!S_ISREG(st.st_mode))
>> + }
>> + if (!S_ISREG(st.st_mode)) {
>> + close(fd);
>> return EISDIR;
>> + }
>> *buf = xmalloc(st.st_size + 1);
>> *size = read_in_full(fd, *buf, st.st_size);
>> (*buf)[*size] = '\0';
>> + close(fd);
>> return (*size == st.st_size ? 0 : errno);
>
> Likewise.
Thanks for noticing. I've applied the following patch on top of the bad one:
From 21f67e7d82986135922aece6b4ebf410a98705bc Mon Sep 17 00:00:00 2001
From: Lars Hjemli <hjemli@gmail.com>
Date: Sat, 7 Nov 2009 18:08:30 +0100
Subject: [PATCH] shared.c: return original errno
Noticed-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Lars Hjemli <hjemli@gmail.com>
---
shared.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/shared.c b/shared.c
index a27ab30..9362d21 100644
--- a/shared.c
+++ b/shared.c
@@ -400,15 +400,16 @@ int cgit_close_filter(struct cgit_filter *filter)
*/
int readfile(const char *path, char **buf, size_t *size)
{
- int fd;
+ int fd, e;
struct stat st;
fd = open(path, O_RDONLY);
if (fd == -1)
return errno;
if (fstat(fd, &st)) {
+ e = errno;
close(fd);
- return errno;
+ return e;
}
if (!S_ISREG(st.st_mode)) {
close(fd);
@@ -416,7 +417,8 @@ int readfile(const char *path, char **buf, size_t *size)
}
*buf = xmalloc(st.st_size + 1);
*size = read_in_full(fd, *buf, st.st_size);
+ e = errno;
(*buf)[*size] = '\0';
close(fd);
- return (*size == st.st_size ? 0 : errno);
+ return (*size == st.st_size ? 0 : e);
}
--
larsh
^ permalink raw reply related
* Re: [PATCH 2/4] format-patch documentation: Remove diff options that are not useful
From: Sebastian Pipping @ 2009-11-07 16:55 UTC (permalink / raw)
To: Björn Gustavsson; +Cc: git, Junio C Hamano
In-Reply-To: <4AF5A605.9020008@hartwork.org>
Sebastian Pipping wrote:
> Björn Gustavsson wrote:
>> To simplify reading the documentation for format-patch, remove the
>> description of common diff options that are not useful for the
>> purpose of the command (i.e. "Prepare patches for e-mail submission").
>>
>> Specifically, this removes the description of the following options:
>
>> --color-words
>
> I disagree on at least this one.
Nevermind, I mixed something up. Sorry.
Sebastian
^ permalink raw reply
* Re: [PATCH 2/4] format-patch documentation: Remove diff options that are not useful
From: Sebastian Pipping @ 2009-11-07 16:53 UTC (permalink / raw)
To: Björn Gustavsson; +Cc: git, Junio C Hamano
In-Reply-To: <4AF5435D.9050702@gmail.com>
Björn Gustavsson wrote:
> To simplify reading the documentation for format-patch, remove the
> description of common diff options that are not useful for the
> purpose of the command (i.e. "Prepare patches for e-mail submission").
>
> Specifically, this removes the description of the following options:
> --color-words
I disagree on at least this one.
Sebastian
^ permalink raw reply
* Re: [cgit PATCH] Close file descriptor on error in readfile()
From: Andreas Schwab @ 2009-11-07 16:14 UTC (permalink / raw)
To: Lars Hjemli; +Cc: Rys Sommefeldt, git, steven
In-Reply-To: <8c5c35580911070659h35c44421q713ddba97318e2b8@mail.gmail.com>
Lars Hjemli <hjemli@gmail.com> writes:
> diff --git a/shared.c b/shared.c
> index d7b2d5a..a27ab30 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -406,12 +406,17 @@ int readfile(const char *path, char **buf, size_t *size)
> fd = open(path, O_RDONLY);
> if (fd == -1)
> return errno;
> - if (fstat(fd, &st))
> + if (fstat(fd, &st)) {
> + close(fd);
> return errno;
The close call can clobber errno.
> - if (!S_ISREG(st.st_mode))
> + }
> + if (!S_ISREG(st.st_mode)) {
> + close(fd);
> return EISDIR;
> + }
> *buf = xmalloc(st.st_size + 1);
> *size = read_in_full(fd, *buf, st.st_size);
> (*buf)[*size] = '\0';
> + close(fd);
> return (*size == st.st_size ? 0 : errno);
Likewise.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* [PATCH 2/3] gitweb: Document current snapshot rules via new tests
From: Jakub Narebski @ 2009-11-07 15:13 UTC (permalink / raw)
To: git; +Cc: Mark Rada, Jakub Narebski
In-Reply-To: <1257606809-23287-1-git-send-email-jnareb@gmail.com>
Add t9502-gitweb-standalone-parse-output test script, which runs
gitweb as a CGI script from the commandline and checks that it
produces the correct output.
Currently this test script contains only tests of snapshot naming
(proposed name of snapshot file) and snapshot prefix (prefix of files
in the archive / snapshot). It defines and uses 'tar' snapshot
format, without compression, for easy checking of snapshot prefix.
Testing is done using check_snapshot function.
Gitweb uses the following format for snapshot filenames:
<sanitized project name>-<hash parameter><snapshot suffix>
where <sanitized project name> is project name with '.git' or '/.git'
suffix stripped, unless '.git' is the whole project name. For
snapshot prefix it uses simply:
<sanitized project name>/
Disadvantages of current snapshot rules:
* There exists convention that <basename>.<suffix> archive unpacks to
<basename>/ directory (<basename>/ is prefix of archive). Gitweb
does not respect it
* Snapshot links generated by gitweb use full SHA-1 id as a value of
'h' / $hash parameter. With current rules it leads to long file
names like e.g. repo-1005c80cc11c531d327b12195027cbbb4ff9e3cb.tgz
* For handcrafted URLs, where 'h' / $hash parameter is a symbolic
'volatile' revision name such as "HEAD" or "next" snapshot name
doesn't tell us what exact version it was created from
* Proposed filename in Content-Disposition header should not contain
any directory path information, which means that it should not
contain '/' (see RFC2183)... which means that snapshot naming is
broken for $hash being e.g. hirearchical branch name such as
'xx/test'
This would be improved in next commit.
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This is a new commit in this series. It wasn't present in previous
version of "smarter snapshot names" series.
t/t9502-gitweb-standalone-parse-output.sh | 87 +++++++++++++++++++++++++++++
1 files changed, 87 insertions(+), 0 deletions(-)
create mode 100755 t/t9502-gitweb-standalone-parse-output.sh
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
new file mode 100755
index 0000000..741187b
--- /dev/null
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -0,0 +1,87 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Mark Rada
+#
+
+test_description='gitweb as standalone script (parsing script output).
+
+This test runs gitweb (git web interface) as a CGI script from the
+commandline, and checks that it produces the correct output, either
+in the HTTP header or the actual script output.'
+
+
+. ./gitweb-lib.sh
+
+# ----------------------------------------------------------------------
+# snapshot file name and prefix
+
+cat >>gitweb_config.perl <<\EOF
+
+$known_snapshot_formats{'tar'} = {
+ 'display' => 'tar',
+ 'type' => 'application/x-tar',
+ 'suffix' => '.tar',
+ 'format' => 'tar',
+};
+
+$feature{'snapshot'}{'default'} = ['tar'];
+EOF
+
+# Call check_snapshot with the arguments "<basename> [<prefix>]"
+#
+# This will check that gitweb HTTP header contains proposed filename
+# as <basename> with '.tar' suffix added, and that generated tarfile
+# (gitweb message body) has <prefix> as prefix for al files in tarfile
+#
+# <prefix> default to <basename>
+check_snapshot () {
+ basename=$1
+ prefix=${2:-"$1"}
+ echo "basename=$basename"
+ grep "filename=.*$basename.tar" gitweb.headers >/dev/null 2>&1 &&
+ "$TAR" tf gitweb.body >file_list &&
+ ! grep -v "^$prefix/" file_list
+}
+
+test_expect_success setup '
+ test_commit first foo &&
+ git branch xx/test &&
+ FULL_ID=$(git rev-parse --verify HEAD) &&
+ SHORT_ID=$(git rev-parse --verify --short=7 HEAD)
+'
+test_debug '
+ echo "FULL_ID = $FULL_ID"
+ echo "SHORT_ID = $SHORT_ID"
+'
+
+test_expect_success 'snapshot: full sha1' '
+ gitweb_run "p=.git;a=snapshot;h=$FULL_ID;sf=tar" &&
+ check_snapshot ".git-$FULL_ID" ".git"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: shortened sha1' '
+ gitweb_run "p=.git;a=snapshot;h=$SHORT_ID;sf=tar" &&
+ check_snapshot ".git-$SHORT_ID" ".git"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: HEAD' '
+ gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
+ check_snapshot ".git-HEAD" ".git"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: short branch name (master)' '
+ gitweb_run "p=.git;a=snapshot;h=master;sf=tar" &&
+ check_snapshot ".git-master" ".git"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_failure 'snapshot: hierarchical branch name (xx/test)' '
+ gitweb_run "p=.git;a=snapshot;h=xx/test;sf=tar" &&
+ ! grep "filename=.*/" gitweb.headers
+'
+test_debug 'cat gitweb.headers'
+
+test_done
--
1.6.5
^ permalink raw reply related
* [PATCHv7 3/3] gitweb: Smarter snapshot names
From: Jakub Narebski @ 2009-11-07 15:13 UTC (permalink / raw)
To: git; +Cc: Mark Rada, Shawn O. Pearce, Jakub Narebski
In-Reply-To: <1257606809-23287-1-git-send-email-jnareb@gmail.com>
From: Mark Rada <marada@uwaterloo.ca>
Teach gitweb how to produce nicer snapshot names by only using the
short hash id. If clients make requests using a tree-ish that is not
a partial or full SHA-1 hash, then the short hash will also be appended
to whatever they asked for. If clients request snapshot of a tag
(which means that $hash ('h') parameter has 'refs/tags/' prefix),
use only tag name.
Update tests cases in t9502-gitweb-standalone-parse-output.
Gitweb uses the following format for snapshot filenames:
<sanitized project name>-<version info>.<snapshot suffix>
where <sanitized project name> is project name with '.git' or '/.git'
suffix stripped, unless '.git' is the whole project name. For
snapshot prefix it uses:
<sanitized project name>-<version info>/
as compared to <sanitized project name>/ before (without version info).
Current rules for <version info>:
* if 'h' / $hash parameter is SHA-1 or shortened SHA-1, use SHA-1
shortened to to 7 characters
* otherwise if 'h' / $hash parameter is tag name (it begins with
'refs/tags/' prefix, use tag name (with 'refs/tags/' stripped
* otherwise if 'h' / $hash parameter starts with 'refs/heads/' prefix,
strip this prefix, convert '/' into '.', and append shortened SHA-1
after '-', i.e. use <sanitized hash>-<shortened sha1>
Signed-off-by: Mark Rada <marada@uwaterloo.ca>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Changes since v6:
- use check_snapshot function to test snapshots; therefore separate
test checking archive prefix is not needed any more
- fixed checking that hierarchical branch names work correctly
gitweb/gitweb.perl | 76 +++++++++++++++++++++++------
t/t9502-gitweb-standalone-parse-output.sh | 38 ++++++++++++--
2 files changed, 93 insertions(+), 21 deletions(-)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8d4a2ae..d8dfd95 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1983,16 +1983,27 @@ sub quote_command {
# get HEAD ref of given project as hash
sub git_get_head_hash {
- my $project = shift;
+ return git_get_full_hash(shift, 'HEAD');
+}
+
+sub git_get_full_hash {
+ return git_get_hash(@_);
+}
+
+sub git_get_short_hash {
+ return git_get_hash(@_, '--short=7');
+}
+
+sub git_get_hash {
+ my ($project, $hash, @options) = @_;
my $o_git_dir = $git_dir;
my $retval = undef;
$git_dir = "$projectroot/$project";
- if (open my $fd, "-|", git_cmd(), "rev-parse", "--verify", "HEAD") {
- my $head = <$fd>;
+ if (open my $fd, '-|', git_cmd(), 'rev-parse',
+ '--verify', '-q', @options, $hash) {
+ $retval = <$fd>;
+ chomp $retval if defined $retval;
close $fd;
- if (defined $head && $head =~ /^([0-9a-fA-F]{40})$/) {
- $retval = $1;
- }
}
if (defined $o_git_dir) {
$git_dir = $o_git_dir;
@@ -5179,6 +5190,43 @@ sub git_tree {
git_footer_html();
}
+sub snapshot_name {
+ my ($project, $hash) = @_;
+
+ # path/to/project.git -> project
+ # path/to/project/.git -> project
+ my $name = to_utf8($project);
+ $name =~ s,([^/])/*\.git$,$1,;
+ $name = basename($name);
+ # sanitize name
+ $name =~ s/[[:cntrl:]]/?/g;
+
+ my $ver = $hash;
+ if ($hash =~ /^[0-9a-fA-F]+$/) {
+ # shorten SHA-1 hash
+ my $full_hash = git_get_full_hash($project, $hash);
+ if ($full_hash =~ /^$hash/ && length($hash) > 7) {
+ $ver = git_get_short_hash($project, $hash);
+ }
+ } elsif ($hash =~ m!^refs/tags/(.*)$!) {
+ # tags don't need shortened SHA-1 hash
+ $ver = $1;
+ } else {
+ # branches and other need shortened SHA-1 hash
+ if ($hash =~ m!^refs/(?:heads|remotes)/(.*)$!) {
+ $ver = $1;
+ }
+ $ver .= '-' . git_get_short_hash($project, $hash);
+ }
+ # in case of hierarchical branch names
+ $ver =~ s!/!.!g;
+
+ # name = project-version_string
+ $name = "$name-$ver";
+
+ return wantarray ? ($name, $name) : $name;
+}
+
sub git_snapshot {
my $format = $input_params{'snapshot_format'};
if (!@snapshot_fmts) {
@@ -5203,24 +5251,20 @@ sub git_snapshot {
die_error(400, 'Object is not a tree-ish');
}
- my $name = $project;
- $name =~ s,([^/])/*\.git$,$1,;
- $name = basename($name);
- my $filename = to_utf8($name);
- $name =~ s/\047/\047\\\047\047/g;
- my $cmd;
- $filename .= "-$hash$known_snapshot_formats{$format}{'suffix'}";
- $cmd = quote_command(
+ my ($name, $prefix) = snapshot_name($project, $hash);
+ my $filename = "$name$known_snapshot_formats{$format}{'suffix'}";
+ my $cmd = quote_command(
git_cmd(), 'archive',
"--format=$known_snapshot_formats{$format}{'format'}",
- "--prefix=$name/", $hash);
+ "--prefix=$prefix/", $hash);
if (exists $known_snapshot_formats{$format}{'compressor'}) {
$cmd .= ' | ' . quote_command(@{$known_snapshot_formats{$format}{'compressor'}});
}
+ $filename =~ s/(["\\])/\\$1/g;
print $cgi->header(
-type => $known_snapshot_formats{$format}{'type'},
- -content_disposition => 'inline; filename="' . "$filename" . '"',
+ -content_disposition => 'inline; filename="' . $filename . '"',
-status => '200 OK');
open my $fd, "-|", $cmd
diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh
index 741187b..dd83890 100755
--- a/t/t9502-gitweb-standalone-parse-output.sh
+++ b/t/t9502-gitweb-standalone-parse-output.sh
@@ -56,29 +56,57 @@ test_debug '
test_expect_success 'snapshot: full sha1' '
gitweb_run "p=.git;a=snapshot;h=$FULL_ID;sf=tar" &&
- check_snapshot ".git-$FULL_ID" ".git"
+ check_snapshot ".git-$SHORT_ID"
'
test_debug 'cat gitweb.headers && cat file_list'
test_expect_success 'snapshot: shortened sha1' '
gitweb_run "p=.git;a=snapshot;h=$SHORT_ID;sf=tar" &&
- check_snapshot ".git-$SHORT_ID" ".git"
+ check_snapshot ".git-$SHORT_ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: almost full sha1' '
+ ID=$(git rev-parse --short=30 HEAD) &&
+ gitweb_run "p=.git;a=snapshot;h=$ID;sf=tar" &&
+ check_snapshot ".git-$SHORT_ID"
'
test_debug 'cat gitweb.headers && cat file_list'
test_expect_success 'snapshot: HEAD' '
gitweb_run "p=.git;a=snapshot;h=HEAD;sf=tar" &&
- check_snapshot ".git-HEAD" ".git"
+ check_snapshot ".git-HEAD-$SHORT_ID"
'
test_debug 'cat gitweb.headers && cat file_list'
test_expect_success 'snapshot: short branch name (master)' '
gitweb_run "p=.git;a=snapshot;h=master;sf=tar" &&
- check_snapshot ".git-master" ".git"
+ ID=$(git rev-parse --verify --short=7 master) &&
+ check_snapshot ".git-master-$ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: short tag name (first)' '
+ gitweb_run "p=.git;a=snapshot;h=first;sf=tar" &&
+ ID=$(git rev-parse --verify --short=7 first) &&
+ check_snapshot ".git-first-$ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: full branch name (refs/heads/master)' '
+ gitweb_run "p=.git;a=snapshot;h=refs/heads/master;sf=tar" &&
+ ID=$(git rev-parse --verify --short=7 master) &&
+ check_snapshot ".git-master-$ID"
+'
+test_debug 'cat gitweb.headers && cat file_list'
+
+test_expect_success 'snapshot: full tag name (refs/tags/first)' '
+ gitweb_run "p=.git;a=snapshot;h=refs/tags/first;sf=tar" &&
+ check_snapshot ".git-first"
'
test_debug 'cat gitweb.headers && cat file_list'
-test_expect_failure 'snapshot: hierarchical branch name (xx/test)' '
+test_expect_success 'snapshot: hierarchical branch name (xx/test)' '
gitweb_run "p=.git;a=snapshot;h=xx/test;sf=tar" &&
! grep "filename=.*/" gitweb.headers
'
--
1.6.5
^ permalink raw reply related
* [PATCH 1/3] t/gitweb-lib.sh: Split gitweb output into headers and body
From: Jakub Narebski @ 2009-11-07 15:13 UTC (permalink / raw)
To: git; +Cc: Mark Rada, Jakub Narebski
In-Reply-To: <1257606809-23287-1-git-send-email-jnareb@gmail.com>
Save HTTP headers into gitweb.headers, and the body of message into
gitweb.body in gitweb_run()
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This version is identical to the one in previous version of this
series.
I hope that this use of sed is portable enough.
Please take into account that HTTP headers part contains at least one
line: HTTP status.
t/gitweb-lib.sh | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
index 8452532..32b841d
--- a/t/gitweb-lib.sh
+++ b/t/gitweb-lib.sh
@@ -52,10 +52,14 @@ gitweb_run () {
rm -f gitweb.log &&
perl -- "$SCRIPT_NAME" \
>gitweb.output 2>gitweb.log &&
+ sed -e '/^\r$/q' <gitweb.output >gitweb.headers &&
+ sed -e '1,/^\r$/d' <gitweb.output >gitweb.body &&
if grep '^[[]' gitweb.log >/dev/null 2>&1; then false; else true; fi
# gitweb.log is left for debugging
- # gitweb.output is used to parse http output
+ # gitweb.output is used to parse HTTP output
+ # gitweb.headers contains only HTTP headers
+ # gitweb.body contains body of message, without headers
}
. ./test-lib.sh
--
1.6.5
^ permalink raw reply related
* [PATCHv2 0/3] gitweb: Smarter snapshot names
From: Jakub Narebski @ 2009-11-07 15:13 UTC (permalink / raw)
To: git; +Cc: Mark Rada, Jakub Narebski
This is the replacement for 'mr/gitweb-snapshot' branch (aba1076),
currently in 'pu'.
Compared to previous series the t9502-gitweb-standalone-parse-output
test is created in earlier commit, to better show changes in snapshot
behaviour in "gitweb: Smarter snapshot names".
The first commit in series is identical to the one in previous version
of this series. Changes in last commit in series are only about
tests.
Shortlog:
~~~~~~~~~
Jakub Narebski (2):
t/gitweb-lib.sh: Split gitweb output into headers and body
gitweb: Document current snapshot rules via new tests
Mark Rada (1):
gitweb: Smarter snapshot names
Table of contents:
~~~~~~~~~~~~~~~~~~
[PATCH 1/3] t/gitweb-lib.sh: Split gitweb output into headers and body
[PATCH 2/3] gitweb: Document current snapshot rules via new tests
[PATCH 3/3] gitweb: Smarter snapshot names
Diffstat:
~~~~~~~~~
gitweb/gitweb.perl | 76 +++++++++++++++----
t/gitweb-lib.sh | 6 +-
t/t9502-gitweb-standalone-parse-output.sh | 115 +++++++++++++++++++++++++++++
3 files changed, 180 insertions(+), 17 deletions(-)
create mode 100755 t/t9502-gitweb-standalone-parse-output.sh
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: [cgit PATCH] Close file descriptor on error in readfile()
From: Lars Hjemli @ 2009-11-07 14:59 UTC (permalink / raw)
To: Rys Sommefeldt; +Cc: git, steven
In-Reply-To: <4AF566C9.5090106@pixeltards.com>
On Sat, Nov 7, 2009 at 13:23, Rys Sommefeldt <rys@pixeltards.com> wrote:
> Sorry for the earlier HTML email, I'd misconfigured my mail client so accept
> my apologies for that (and thanks Steven). Here's the reworked patch:
Thanks. I've applied the following to my stable branch:
diff --git a/shared.c b/shared.c
index d7b2d5a..a27ab30 100644
--- a/shared.c
+++ b/shared.c
@@ -406,12 +406,17 @@ int readfile(const char *path, char **buf, size_t *size)
fd = open(path, O_RDONLY);
if (fd == -1)
return errno;
- if (fstat(fd, &st))
+ if (fstat(fd, &st)) {
+ close(fd);
return errno;
- if (!S_ISREG(st.st_mode))
+ }
+ if (!S_ISREG(st.st_mode)) {
+ close(fd);
return EISDIR;
+ }
*buf = xmalloc(st.st_size + 1);
*size = read_in_full(fd, *buf, st.st_size);
(*buf)[*size] = '\0';
+ close(fd);
return (*size == st.st_size ? 0 : errno);
}
--
larsh
^ 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