* [PATCH 6/6] archive: remove extra arguments parsing code
From: René Scharfe @ 2008-07-14 19:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <487B92FC.5030103@lsrfire.ath.cx>
Replace the code that calls backend specific argument parsers by a
simple flag mechanism. This reduces code size and complexity.
We can add back such a mechanism (based on incremental parse_opt(),
perhaps) when we need it. The compression level parameter, though,
is going to be shared by future compressing backends like tgz.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive-zip.c | 13 -------------
archive.h | 6 +-----
builtin-archive.c | 29 ++++++++++++++++-------------
3 files changed, 17 insertions(+), 31 deletions(-)
diff --git a/archive-zip.c b/archive-zip.c
index 8131289..d56e5cf 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -282,16 +282,3 @@ int write_zip_archive(struct archiver_args *args)
return err;
}
-
-void *parse_extra_zip_args(int argc, const char **argv)
-{
- for (; argc > 0; argc--, argv++) {
- const char *arg = argv[0];
-
- if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0')
- zlib_compression_level = arg[1] - '0';
- else
- die("Unknown argument for zip format: %s", arg);
- }
- return NULL;
-}
diff --git a/archive.h b/archive.h
index 88ee3be..96bb1cd 100644
--- a/archive.h
+++ b/archive.h
@@ -13,19 +13,16 @@ struct archiver_args {
time_t time;
const char **pathspec;
unsigned int verbose : 1;
- void *extra;
};
typedef int (*write_archive_fn_t)(struct archiver_args *);
-typedef void *(*parse_extra_args_fn_t)(int argc, const char **argv);
-
typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);
struct archiver {
const char *name;
write_archive_fn_t write_archive;
- parse_extra_args_fn_t parse_extra;
+ unsigned int flags;
};
extern int parse_archive_args(int argc, const char **argv, const struct archiver **ar, struct archiver_args *args);
@@ -41,7 +38,6 @@ extern void parse_pathspec_arg(const char **pathspec,
*/
extern int write_tar_archive(struct archiver_args *);
extern int write_zip_archive(struct archiver_args *);
-extern void *parse_extra_zip_args(int argc, const char **argv);
extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
diff --git a/builtin-archive.c b/builtin-archive.c
index e7f4ec6..88204bf 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -15,9 +15,11 @@
static const char archive_usage[] = \
"git-archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
+#define USES_ZLIB_COMPRESSION 1
+
const struct archiver archivers[] = {
- { "tar", write_tar_archive, NULL },
- { "zip", write_zip_archive, parse_extra_zip_args },
+ { "tar", write_tar_archive },
+ { "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
};
static int run_remote_archiver(const char *remote, int argc,
@@ -137,10 +139,9 @@ void parse_treeish_arg(const char **argv, struct archiver_args *ar_args,
int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
struct archiver_args *args)
{
- const char *extra_argv[MAX_EXTRA_ARGS];
- int extra_argc = 0;
const char *format = "tar";
const char *base = "";
+ int compression_level = -1;
int verbose = 0;
int i;
@@ -168,12 +169,12 @@ int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
i++;
break;
}
- if (arg[0] == '-') {
- if (extra_argc > MAX_EXTRA_ARGS - 1)
- die("Too many extra options");
- extra_argv[extra_argc++] = arg;
+ if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0') {
+ compression_level = arg[1] - '0';
continue;
}
+ if (arg[0] == '-')
+ die("Unknown argument: %s", arg);
break;
}
@@ -184,11 +185,13 @@ int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
if (!*ar)
die("Unknown archive format '%s'", format);
- if (extra_argc) {
- if (!(*ar)->parse_extra)
- die("'%s' format does not handle %s",
- (*ar)->name, extra_argv[0]);
- args->extra = (*ar)->parse_extra(extra_argc, extra_argv);
+ if (compression_level != -1) {
+ if ((*ar)->flags & USES_ZLIB_COMPRESSION)
+ zlib_compression_level = compression_level;
+ else {
+ die("Argument not supported for format '%s': -%d",
+ format, compression_level);
+ }
}
args->verbose = verbose;
args->base = base;
--
1.5.6.2.212.g08b51
^ permalink raw reply related
* [PATCH 0/6] archive: refactor and cleanup
From: René Scharfe @ 2008-07-14 19:21 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
This series is a collection of cleanups for git archive that I finally need
to get out, reviewed and (hopefully!) integrated. It should make adding
more format backends easier.
Patch 2 adds a context parameter to read_tree_recursive(), thus affecting
all its callers. The rest of the patches change *archive*.[ch], only.
René
archive-tar.c | 115 ++++++++++++---------------------------
archive-zip.c | 102 +++++------------------------------
archive.c | 132 ++++++++++++++++++++++++++++++++------------
archive.h | 15 ++----
builtin-archive.c | 75 ++++++++++++--------------
builtin-checkout.c | 4 +-
builtin-log.c | 4 +-
builtin-ls-tree.c | 4 +-
builtin-merge-recursive.c | 4 +-
builtin-upload-archive.c | 11 ++--
tree.c | 12 ++--
tree.h | 4 +-
12 files changed, 208 insertions(+), 274 deletions(-)
^ permalink raw reply
* [PATCH 3/6] archive: add baselen member to struct archiver_args
From: René Scharfe @ 2008-07-14 19:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <487B92FC.5030103@lsrfire.ath.cx>
Calculate the length of base and save it in a new member of struct
archiver_args. This way we don't have to compute it in each of the
format backends.
Note: parse_archive_args() guarantees that ->base won't ever be NULL.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive-tar.c | 8 +++-----
archive-zip.c | 8 +++-----
archive.h | 1 +
builtin-archive.c | 1 +
4 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 6eaf59e..63cc2ec 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -268,19 +268,17 @@ static int write_tar_entry(const unsigned char
*sha1, const char *base,
int write_tar_archive(struct archiver_args *args)
{
- int plen = args->base ? strlen(args->base) : 0;
-
git_config(git_tar_config, NULL);
archive_time = args->time;
verbose = args->verbose;
commit = args->commit;
- base_len = args->base ? strlen(args->base) : 0;
+ base_len = args->baselen;
if (args->commit_sha1)
write_global_extended_header(args->commit_sha1);
- if (args->base && plen > 0 && args->base[plen - 1] == '/') {
+ if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
char *base = xstrdup(args->base);
int baselen = strlen(base);
@@ -290,7 +288,7 @@ int write_tar_archive(struct archiver_args *args)
0, NULL);
free(base);
}
- read_tree_recursive(args->tree, args->base, plen, 0,
+ read_tree_recursive(args->tree, args->base, args->baselen, 0,
args->pathspec, write_tar_entry, NULL);
write_trailer();
diff --git a/archive-zip.c b/archive-zip.c
index 0d24f3f..d18254c 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -316,17 +316,15 @@ static void dos_time(time_t *time, int *dos_date,
int *dos_time)
int write_zip_archive(struct archiver_args *args)
{
- int plen = strlen(args->base);
-
dos_time(&args->time, &zip_date, &zip_time);
zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
verbose = args->verbose;
commit = args->commit;
- base_len = args->base ? strlen(args->base) : 0;
+ base_len = args->baselen;
- if (args->base && plen > 0 && args->base[plen - 1] == '/') {
+ if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
char *base = xstrdup(args->base);
int baselen = strlen(base);
@@ -336,7 +334,7 @@ int write_zip_archive(struct archiver_args *args)
0, NULL);
free(base);
}
- read_tree_recursive(args->tree, args->base, plen, 0,
+ read_tree_recursive(args->tree, args->base, args->baselen, 0,
args->pathspec, write_zip_entry, NULL);
write_zip_trailer(args->commit_sha1);
diff --git a/archive.h b/archive.h
index 1b24ae3..34151f4 100644
--- a/archive.h
+++ b/archive.h
@@ -6,6 +6,7 @@
struct archiver_args {
const char *base;
+ size_t baselen;
struct tree *tree;
const unsigned char *commit_sha1;
const struct commit *commit;
diff --git a/builtin-archive.c b/builtin-archive.c
index 6ee3677..e7f4ec6 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -192,6 +192,7 @@ int parse_archive_args(int argc, const char **argv,
const struct archiver **ar,
}
args->verbose = verbose;
args->base = base;
+ args->baselen = strlen(base);
return i;
}
--
1.5.6.2.212.g08b51
^ permalink raw reply related
* [PATCH 1/6] archive: remove args member from struct archiver
From: René Scharfe @ 2008-07-14 19:23 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <487B8E81.5030402@lsrfire.ath.cx>
Pass struct archiver and struct archiver_args explicitly to
parse_archive_args
and remove the latter from the former. This allows us to get rid of struct
archiver_desc and simplifies the code a bit.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.h | 5 +---
builtin-archive.c | 51
+++++++++++++++++++---------------------------
builtin-upload-archive.c | 11 +++++----
3 files changed, 28 insertions(+), 39 deletions(-)
diff --git a/archive.h b/archive.h
index ddf004a..1b24ae3 100644
--- a/archive.h
+++ b/archive.h
@@ -21,14 +21,11 @@ typedef void *(*parse_extra_args_fn_t)(int argc,
const char **argv);
struct archiver {
const char *name;
- struct archiver_args args;
write_archive_fn_t write_archive;
parse_extra_args_fn_t parse_extra;
};
-extern int parse_archive_args(int argc,
- const char **argv,
- struct archiver *ar);
+extern int parse_archive_args(int argc, const char **argv, const struct
archiver **ar, struct archiver_args *args);
extern void parse_treeish_arg(const char **treeish,
struct archiver_args *ar_args,
diff --git a/builtin-archive.c b/builtin-archive.c
index c2e0c1e..6ee3677 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -15,12 +15,7 @@
static const char archive_usage[] = \
"git-archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>]
<tree-ish> [path...]";
-static struct archiver_desc
-{
- const char *name;
- write_archive_fn_t write_archive;
- parse_extra_args_fn_t parse_extra;
-} archivers[] = {
+const struct archiver archivers[] = {
{ "tar", write_tar_archive, NULL },
{ "zip", write_zip_archive, parse_extra_zip_args },
};
@@ -79,21 +74,15 @@ static int run_remote_archiver(const char *remote,
int argc,
return !!rv;
}
-static int init_archiver(const char *name, struct archiver *ar)
+static const struct archiver *lookup_archiver(const char *name)
{
- int rv = -1, i;
+ int i;
for (i = 0; i < ARRAY_SIZE(archivers); i++) {
- if (!strcmp(name, archivers[i].name)) {
- memset(ar, 0, sizeof(*ar));
- ar->name = archivers[i].name;
- ar->write_archive = archivers[i].write_archive;
- ar->parse_extra = archivers[i].parse_extra;
- rv = 0;
- break;
- }
+ if (!strcmp(name, archivers[i].name))
+ return &archivers[i];
}
- return rv;
+ return NULL;
}
void parse_pathspec_arg(const char **pathspec, struct archiver_args
*ar_args)
@@ -145,7 +134,8 @@ void parse_treeish_arg(const char **argv, struct
archiver_args *ar_args,
ar_args->time = archive_time;
}
-int parse_archive_args(int argc, const char **argv, struct archiver *ar)
+int parse_archive_args(int argc, const char **argv, const struct
archiver **ar,
+ struct archiver_args *args)
{
const char *extra_argv[MAX_EXTRA_ARGS];
int extra_argc = 0;
@@ -190,17 +180,18 @@ int parse_archive_args(int argc, const char
**argv, struct archiver *ar)
/* We need at least one parameter -- tree-ish */
if (argc - 1 < i)
usage(archive_usage);
- if (init_archiver(format, ar) < 0)
+ *ar = lookup_archiver(format);
+ if (!*ar)
die("Unknown archive format '%s'", format);
if (extra_argc) {
- if (!ar->parse_extra)
+ if (!(*ar)->parse_extra)
die("'%s' format does not handle %s",
- ar->name, extra_argv[0]);
- ar->args.extra = ar->parse_extra(extra_argc, extra_argv);
+ (*ar)->name, extra_argv[0]);
+ args->extra = (*ar)->parse_extra(extra_argc, extra_argv);
}
- ar->args.verbose = verbose;
- ar->args.base = base;
+ args->verbose = verbose;
+ args->base = base;
return i;
}
@@ -238,7 +229,8 @@ static const char *extract_remote_arg(int *ac, const
char **av)
int cmd_archive(int argc, const char **argv, const char *prefix)
{
- struct archiver ar;
+ const struct archiver *ar = NULL;
+ struct archiver_args args;
int tree_idx;
const char *remote = NULL;
@@ -248,14 +240,13 @@ int cmd_archive(int argc, const char **argv, const
char *prefix)
setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
- memset(&ar, 0, sizeof(ar));
- tree_idx = parse_archive_args(argc, argv, &ar);
+ tree_idx = parse_archive_args(argc, argv, &ar, &args);
if (prefix == NULL)
prefix = setup_git_directory();
argv += tree_idx;
- parse_treeish_arg(argv, &ar.args, prefix);
- parse_pathspec_arg(argv + 1, &ar.args);
+ parse_treeish_arg(argv, &args, prefix);
+ parse_pathspec_arg(argv + 1, &args);
- return ar.write_archive(&ar.args);
+ return ar->write_archive(&args);
}
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 371400d..295e24c 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -19,7 +19,8 @@ static const char lostchild[] =
static int run_upload_archive(int argc, const char **argv, const char
*prefix)
{
- struct archiver ar;
+ const struct archiver *ar;
+ struct archiver_args args;
const char *sent_argv[MAX_ARGS];
const char *arg_cmd = "argument ";
char *p, buf[4096];
@@ -65,12 +66,12 @@ static int run_upload_archive(int argc, const char
**argv, const char *prefix)
sent_argv[sent_argc] = NULL;
/* parse all options sent by the client */
- treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar);
+ treeish_idx = parse_archive_args(sent_argc, sent_argv, &ar, &args);
- parse_treeish_arg(sent_argv + treeish_idx, &ar.args, prefix);
- parse_pathspec_arg(sent_argv + treeish_idx + 1, &ar.args);
+ parse_treeish_arg(sent_argv + treeish_idx, &args, prefix);
+ parse_pathspec_arg(sent_argv + treeish_idx + 1, &args);
- return ar.write_archive(&ar.args);
+ return ar->write_archive(&args);
}
static void error_clnt(const char *fmt, ...)
--
1.5.6.2.212.g08b51
^ permalink raw reply related
* [PATCH 2/6] add context pointer to read_tree_recursive()
From: René Scharfe @ 2008-07-14 19:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <487B92FC.5030103@lsrfire.ath.cx>
Add a pointer parameter to read_tree_recursive(), which is passed to the
callback function. This allows callers of read_tree_recursive() to
share data with the callback without resorting to global variables. All
current callers pass NULL.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive-tar.c | 11 ++++++-----
archive-zip.c | 11 ++++++-----
builtin-checkout.c | 4 ++--
builtin-log.c | 4 ++--
builtin-ls-tree.c | 4 ++--
builtin-merge-recursive.c | 4 ++--
tree.c | 12 ++++++------
tree.h | 4 ++--
8 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 99db58f..6eaf59e 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -234,9 +234,9 @@ static int git_tar_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
-static int write_tar_entry(const unsigned char *sha1,
- const char *base, int baselen,
- const char *filename, unsigned mode, int stage)
+static int write_tar_entry(const unsigned char *sha1, const char *base,
+ int baselen, const char *filename, unsigned mode, int stage,
+ void *context)
{
static struct strbuf path = STRBUF_INIT;
void *buffer;
@@ -286,11 +286,12 @@ int write_tar_archive(struct archiver_args *args)
while (baselen > 0 && base[baselen - 1] == '/')
base[--baselen] = '\0';
- write_tar_entry(args->tree->object.sha1, "", 0, base, 040777, 0);
+ write_tar_entry(args->tree->object.sha1, "", 0, base, 040777,
+ 0, NULL);
free(base);
}
read_tree_recursive(args->tree, args->base, plen, 0,
- args->pathspec, write_tar_entry);
+ args->pathspec, write_tar_entry, NULL);
write_trailer();
return 0;
diff --git a/archive-zip.c b/archive-zip.c
index 5742762..0d24f3f 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -152,9 +152,9 @@ static char *construct_path(const char *base, int baselen,
return path;
}
-static int write_zip_entry(const unsigned char *sha1,
- const char *base, int baselen,
- const char *filename, unsigned mode, int stage)
+static int write_zip_entry(const unsigned char *sha1, const char *base,
+ int baselen, const char *filename, unsigned mode, int stage,
+ void *context)
{
struct zip_local_header header;
struct zip_dir_header dirent;
@@ -332,11 +332,12 @@ int write_zip_archive(struct archiver_args *args)
while (baselen > 0 && base[baselen - 1] == '/')
base[--baselen] = '\0';
- write_zip_entry(args->tree->object.sha1, "", 0, base, 040777, 0);
+ write_zip_entry(args->tree->object.sha1, "", 0, base, 040777,
+ 0, NULL);
free(base);
}
read_tree_recursive(args->tree, args->base, plen, 0,
- args->pathspec, write_zip_entry);
+ args->pathspec, write_zip_entry, NULL);
write_zip_trailer(args->commit_sha1);
free(zip_dir);
diff --git a/builtin-checkout.c b/builtin-checkout.c
index d6641c2..a542033 100644
--- a/builtin-checkout.c
+++ b/builtin-checkout.c
@@ -43,7 +43,7 @@ static int post_checkout_hook(struct commit *old, struct commit *new,
}
static int update_some(const unsigned char *sha1, const char *base, int baselen,
- const char *pathname, unsigned mode, int stage)
+ const char *pathname, unsigned mode, int stage, void * context)
{
int len;
struct cache_entry *ce;
@@ -67,7 +67,7 @@ static int update_some(const unsigned char *sha1, const char *base, int baselen,
static int read_tree_some(struct tree *tree, const char **pathspec)
{
- read_tree_recursive(tree, "", 0, 0, pathspec, update_some);
+ read_tree_recursive(tree, "", 0, 0, pathspec, update_some, NULL);
/* update the index with the given tree's info
* for all args, expanding wildcards, and exit
diff --git a/builtin-log.c b/builtin-log.c
index 430d876..617aa67 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -313,7 +313,7 @@ static int show_object(const unsigned char *sha1, int show_tag_object,
static int show_tree_object(const unsigned char *sha1,
const char *base, int baselen,
- const char *pathname, unsigned mode, int stage)
+ const char *pathname, unsigned mode, int stage, void *context)
{
printf("%s%s\n", pathname, S_ISDIR(mode) ? "/" : "");
return 0;
@@ -366,7 +366,7 @@ int cmd_show(int argc, const char **argv, const char *prefix)
name,
diff_get_color_opt(&rev.diffopt, DIFF_RESET));
read_tree_recursive((struct tree *)o, "", 0, 0, NULL,
- show_tree_object);
+ show_tree_object, NULL);
break;
case OBJ_COMMIT:
rev.pending.nr = rev.pending.alloc = 0;
diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index f4a75dd..0da047c 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -56,7 +56,7 @@ static int show_recursive(const char *base, int baselen, const char *pathname)
}
static int show_tree(const unsigned char *sha1, const char *base, int baselen,
- const char *pathname, unsigned mode, int stage)
+ const char *pathname, unsigned mode, int stage, void *context)
{
int retval = 0;
const char *type = blob_type;
@@ -189,7 +189,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
tree = parse_tree_indirect(sha1);
if (!tree)
die("not a tree object");
- read_tree_recursive(tree, "", 0, 0, pathspec, show_tree);
+ read_tree_recursive(tree, "", 0, 0, pathspec, show_tree, NULL);
return 0;
}
diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
index 43bf6aa..385e742 100644
--- a/builtin-merge-recursive.c
+++ b/builtin-merge-recursive.c
@@ -256,7 +256,7 @@ struct tree *write_tree_from_memory(void)
static int save_files_dirs(const unsigned char *sha1,
const char *base, int baselen, const char *path,
- unsigned int mode, int stage)
+ unsigned int mode, int stage, void *context)
{
int len = strlen(path);
char *newpath = xmalloc(baselen + len + 1);
@@ -276,7 +276,7 @@ static int save_files_dirs(const unsigned char *sha1,
static int get_files_dirs(struct tree *tree)
{
int n;
- if (read_tree_recursive(tree, "", 0, 0, NULL, save_files_dirs) != 0)
+ if (read_tree_recursive(tree, "", 0, 0, NULL, save_files_dirs, NULL))
return 0;
n = current_file_set.nr + current_directory_set.nr;
return n;
diff --git a/tree.c b/tree.c
index 4b1825c..03e782a 100644
--- a/tree.c
+++ b/tree.c
@@ -29,7 +29,7 @@ static int read_one_entry_opt(const unsigned char *sha1, const char *base, int b
return add_cache_entry(ce, opt);
}
-static int read_one_entry(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage)
+static int read_one_entry(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context)
{
return read_one_entry_opt(sha1, base, baselen, pathname, mode, stage,
ADD_CACHE_OK_TO_ADD|ADD_CACHE_SKIP_DFCHECK);
@@ -39,7 +39,7 @@ static int read_one_entry(const unsigned char *sha1, const char *base, int basel
* This is used when the caller knows there is no existing entries at
* the stage that will conflict with the entry being added.
*/
-static int read_one_entry_quick(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage)
+static int read_one_entry_quick(const unsigned char *sha1, const char *base, int baselen, const char *pathname, unsigned mode, int stage, void *context)
{
return read_one_entry_opt(sha1, base, baselen, pathname, mode, stage,
ADD_CACHE_JUST_APPEND);
@@ -92,7 +92,7 @@ static int match_tree_entry(const char *base, int baselen, const char *path, uns
int read_tree_recursive(struct tree *tree,
const char *base, int baselen,
int stage, const char **match,
- read_tree_fn_t fn)
+ read_tree_fn_t fn, void *context)
{
struct tree_desc desc;
struct name_entry entry;
@@ -106,7 +106,7 @@ int read_tree_recursive(struct tree *tree,
if (!match_tree_entry(base, baselen, entry.path, entry.mode, match))
continue;
- switch (fn(entry.sha1, base, baselen, entry.path, entry.mode, stage)) {
+ switch (fn(entry.sha1, base, baselen, entry.path, entry.mode, stage, context)) {
case 0:
continue;
case READ_TREE_RECURSIVE:
@@ -126,7 +126,7 @@ int read_tree_recursive(struct tree *tree,
retval = read_tree_recursive(lookup_tree(entry.sha1),
newbase,
baselen + pathlen + 1,
- stage, match, fn);
+ stage, match, fn, context);
free(newbase);
if (retval)
return -1;
@@ -174,7 +174,7 @@ int read_tree(struct tree *tree, int stage, const char **match)
if (!fn)
fn = read_one_entry_quick;
- err = read_tree_recursive(tree, "", 0, stage, match, fn);
+ err = read_tree_recursive(tree, "", 0, stage, match, fn, NULL);
if (fn == read_one_entry || err)
return err;
diff --git a/tree.h b/tree.h
index dd25c53..2ff01a4 100644
--- a/tree.h
+++ b/tree.h
@@ -21,12 +21,12 @@ int parse_tree(struct tree *tree);
struct tree *parse_tree_indirect(const unsigned char *sha1);
#define READ_TREE_RECURSIVE 1
-typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const char *, unsigned int, int);
+typedef int (*read_tree_fn_t)(const unsigned char *, const char *, int, const char *, unsigned int, int, void *);
extern int read_tree_recursive(struct tree *tree,
const char *base, int baselen,
int stage, const char **match,
- read_tree_fn_t fn);
+ read_tree_fn_t fn, void *context);
extern int read_tree(struct tree *tree, int stage, const char **paths);
--
1.5.6.2.212.g08b51
^ permalink raw reply related
* [PATCH 4/6] archive: centralize archive entry writing
From: René Scharfe @ 2008-07-14 19:22 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List
In-Reply-To: <487B92FC.5030103@lsrfire.ath.cx>
Add the exported function write_archive_entries() to archive.c, which uses
the new ability of read_tree_recursive() to pass a context pointer to its
callback in order to centralize previously duplicated code.
The new callback function write_archive_entry() does the work that every
archiver backend needs to do: loading file contents, entering subdirectories,
handling file attributes, constructing the full path of the entry. All that
done, it calls the backend specific write_archive_entry_fn_t function.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.c | 77 ++++++++++++++++++++++++++++++++++++++
archive.h | 4 ++
archive-tar.c | 116 ++++++++++++++++++--------------------------------------
archive-zip.c | 90 +++++++-------------------------------------
4 files changed, 133 insertions(+), 154 deletions(-)
diff --git a/archive.c b/archive.c
index 6502b76..58de55e 100644
--- a/archive.c
+++ b/archive.c
@@ -1,6 +1,7 @@
#include "cache.h"
#include "commit.h"
#include "attr.h"
+#include "archive.h"
static void format_subst(const struct commit *commit,
const char *src, size_t len,
@@ -95,3 +96,79 @@ int is_archive_path_ignored(const char *path)
return 0;
return ATTR_TRUE(check[0].value);
}
+
+struct archiver_context {
+ struct archiver_args *args;
+ write_archive_entry_fn_t write_entry;
+};
+
+static int write_archive_entry(const unsigned char *sha1, const char *base,
+ int baselen, const char *filename, unsigned mode, int stage,
+ void *context)
+{
+ static struct strbuf path = STRBUF_INIT;
+ struct archiver_context *c = context;
+ struct archiver_args *args = c->args;
+ write_archive_entry_fn_t write_entry = c->write_entry;
+ int err;
+ enum object_type type;
+ unsigned long size;
+ void *buffer;
+
+ strbuf_reset(&path);
+ strbuf_grow(&path, PATH_MAX);
+ strbuf_add(&path, base, baselen);
+ strbuf_addstr(&path, filename);
+
+ if (is_archive_path_ignored(path.buf + args->baselen))
+ return 0;
+
+ if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
+ strbuf_addch(&path, '/');
+ if (args->verbose)
+ fprintf(stderr, "%.*s\n", (int)path.len, path.buf);
+ err = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);
+ if (err)
+ return err;
+ return READ_TREE_RECURSIVE;
+ }
+
+ buffer = sha1_file_to_archive(path.buf + args->baselen, sha1, mode,
+ &type, &size, args->commit);
+ if (!buffer)
+ return error("cannot read %s", sha1_to_hex(sha1));
+ if (args->verbose)
+ fprintf(stderr, "%.*s\n", (int)path.len, path.buf);
+ err = write_entry(args, sha1, path.buf, path.len, mode, buffer, size);
+ free(buffer);
+ return err;
+}
+
+int write_archive_entries(struct archiver_args *args,
+ write_archive_entry_fn_t write_entry)
+{
+ struct archiver_context context;
+ int err;
+
+ if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
+ size_t len = args->baselen;
+
+ while (len > 1 && args->base[len - 2] == '/')
+ len--;
+ if (args->verbose)
+ fprintf(stderr, "%.*s\n", (int)len, args->base);
+ err = write_entry(args, args->tree->object.sha1, args->base,
+ len, 040777, NULL, 0);
+ if (err)
+ return err;
+ }
+
+ context.args = args;
+ context.write_entry = write_entry;
+
+ err = read_tree_recursive(args->tree, args->base, args->baselen, 0,
+ args->pathspec, write_archive_entry, &context);
+ if (err == READ_TREE_RECURSIVE)
+ err = 0;
+ return err;
+}
diff --git a/archive.h b/archive.h
index 34151f4..4e44549 100644
--- a/archive.h
+++ b/archive.h
@@ -20,6 +20,8 @@ typedef int (*write_archive_fn_t)(struct archiver_args *);
typedef void *(*parse_extra_args_fn_t)(int argc, const char **argv);
+typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);
+
struct archiver {
const char *name;
write_archive_fn_t write_archive;
@@ -44,4 +46,6 @@ extern void *parse_extra_zip_args(int argc, const char **argv);
extern void *sha1_file_to_archive(const char *path, const unsigned char *sha1, unsigned int mode, enum object_type *type, unsigned long *size, const struct commit *commit);
extern int is_archive_path_ignored(const char *path);
+extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
+
#endif /* ARCHIVE_H */
diff --git a/archive-tar.c b/archive-tar.c
index 63cc2ec..f9eb726 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -13,11 +13,7 @@
static char block[BLOCKSIZE];
static unsigned long offset;
-static time_t archive_time;
static int tar_umask = 002;
-static int verbose;
-static const struct commit *commit;
-static size_t base_len;
/* writes out the whole block, but only if it is full */
static void write_if_needed(void)
@@ -114,22 +110,24 @@ static unsigned int ustar_header_chksum(const struct ustar_header *header)
return chksum;
}
-static int get_path_prefix(const struct strbuf *path, int maxlen)
+static size_t get_path_prefix(const char *path, size_t pathlen, size_t maxlen)
{
- int i = path->len;
+ size_t i = pathlen;
if (i > maxlen)
i = maxlen;
do {
i--;
- } while (i > 0 && path->buf[i] != '/');
+ } while (i > 0 && path[i] != '/');
return i;
}
-static void write_entry(const unsigned char *sha1, struct strbuf *path,
- unsigned int mode, void *buffer, unsigned long size)
+static int write_tar_entry(struct archiver_args *args,
+ const unsigned char *sha1, const char *path, size_t pathlen,
+ unsigned int mode, void *buffer, unsigned long size)
{
struct ustar_header header;
struct strbuf ext_header;
+ int err = 0;
memset(&header, 0, sizeof(header));
strbuf_init(&ext_header, 0);
@@ -143,8 +141,6 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
mode = 0100666;
sprintf(header.name, "%s.paxheader", sha1_to_hex(sha1));
} else {
- if (verbose)
- fprintf(stderr, "%.*s\n", (int)path->len, path->buf);
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
*header.typeflag = TYPEFLAG_DIR;
mode = (mode | 0777) & ~tar_umask;
@@ -155,24 +151,24 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
*header.typeflag = TYPEFLAG_REG;
mode = (mode | ((mode & 0100) ? 0777 : 0666)) & ~tar_umask;
} else {
- error("unsupported file mode: 0%o (SHA1: %s)",
- mode, sha1_to_hex(sha1));
- return;
+ return error("unsupported file mode: 0%o (SHA1: %s)",
+ mode, sha1_to_hex(sha1));
}
- if (path->len > sizeof(header.name)) {
- int plen = get_path_prefix(path, sizeof(header.prefix));
- int rest = path->len - plen - 1;
+ if (pathlen > sizeof(header.name)) {
+ size_t plen = get_path_prefix(path, pathlen,
+ sizeof(header.prefix));
+ size_t rest = pathlen - plen - 1;
if (plen > 0 && rest <= sizeof(header.name)) {
- memcpy(header.prefix, path->buf, plen);
- memcpy(header.name, path->buf + plen + 1, rest);
+ memcpy(header.prefix, path, plen);
+ memcpy(header.name, path + plen + 1, rest);
} else {
sprintf(header.name, "%s.data",
sha1_to_hex(sha1));
strbuf_append_ext_header(&ext_header, "path",
- path->buf, path->len);
+ path, pathlen);
}
} else
- memcpy(header.name, path->buf, path->len);
+ memcpy(header.name, path, pathlen);
}
if (S_ISLNK(mode) && buffer) {
@@ -187,7 +183,7 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
sprintf(header.mode, "%07o", mode & 07777);
sprintf(header.size, "%011lo", S_ISREG(mode) ? size : 0);
- sprintf(header.mtime, "%011lo", archive_time);
+ sprintf(header.mtime, "%011lo", args->time);
sprintf(header.uid, "%07o", 0);
sprintf(header.gid, "%07o", 0);
@@ -202,22 +198,30 @@ static void write_entry(const unsigned char *sha1, struct strbuf *path,
sprintf(header.chksum, "%07o", ustar_header_chksum(&header));
if (ext_header.len > 0) {
- write_entry(sha1, NULL, 0, ext_header.buf, ext_header.len);
+ err = write_tar_entry(args, sha1, NULL, 0, 0, ext_header.buf,
+ ext_header.len);
+ if (err)
+ return err;
}
strbuf_release(&ext_header);
write_blocked(&header, sizeof(header));
if (S_ISREG(mode) && buffer && size > 0)
write_blocked(buffer, size);
+ return err;
}
-static void write_global_extended_header(const unsigned char *sha1)
+static int write_global_extended_header(struct archiver_args *args)
{
+ const unsigned char *sha1 = args->commit_sha1;
struct strbuf ext_header;
+ int err;
strbuf_init(&ext_header, 0);
strbuf_append_ext_header(&ext_header, "comment", sha1_to_hex(sha1), 40);
- write_entry(NULL, NULL, 0, ext_header.buf, ext_header.len);
+ err = write_tar_entry(args, NULL, NULL, 0, 0, ext_header.buf,
+ ext_header.len);
strbuf_release(&ext_header);
+ return err;
}
static int git_tar_config(const char *var, const char *value, void *cb)
@@ -234,63 +238,17 @@ static int git_tar_config(const char *var, const char *value, void *cb)
return git_default_config(var, value, cb);
}
-static int write_tar_entry(const unsigned char *sha1, const char *base,
- int baselen, const char *filename, unsigned mode, int stage,
- void *context)
-{
- static struct strbuf path = STRBUF_INIT;
- void *buffer;
- enum object_type type;
- unsigned long size;
-
- strbuf_reset(&path);
- strbuf_grow(&path, PATH_MAX);
- strbuf_add(&path, base, baselen);
- strbuf_addstr(&path, filename);
- if (is_archive_path_ignored(path.buf + base_len))
- return 0;
- if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
- strbuf_addch(&path, '/');
- buffer = NULL;
- size = 0;
- } else {
- buffer = sha1_file_to_archive(path.buf + base_len, sha1, mode,
- &type, &size, commit);
- if (!buffer)
- die("cannot read %s", sha1_to_hex(sha1));
- }
-
- write_entry(sha1, &path, mode, buffer, size);
- free(buffer);
-
- return READ_TREE_RECURSIVE;
-}
-
int write_tar_archive(struct archiver_args *args)
{
- git_config(git_tar_config, NULL);
+ int err = 0;
- archive_time = args->time;
- verbose = args->verbose;
- commit = args->commit;
- base_len = args->baselen;
+ git_config(git_tar_config, NULL);
if (args->commit_sha1)
- write_global_extended_header(args->commit_sha1);
-
- if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
- char *base = xstrdup(args->base);
- int baselen = strlen(base);
-
- while (baselen > 0 && base[baselen - 1] == '/')
- base[--baselen] = '\0';
- write_tar_entry(args->tree->object.sha1, "", 0, base, 040777,
- 0, NULL);
- free(base);
- }
- read_tree_recursive(args->tree, args->base, args->baselen, 0,
- args->pathspec, write_tar_entry, NULL);
- write_trailer();
-
- return 0;
+ err = write_global_extended_header(args);
+ if (!err)
+ err = write_archive_entries(args, write_tar_entry);
+ if (!err)
+ write_trailer();
+ return err;
}
diff --git a/archive-zip.c b/archive-zip.c
index d18254c..8131289 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -9,11 +9,8 @@
#include "builtin.h"
#include "archive.h"
-static int verbose;
static int zip_date;
static int zip_time;
-static const struct commit *commit;
-static size_t base_len;
static unsigned char *zip_dir;
static unsigned int zip_dir_size;
@@ -128,33 +125,9 @@ static void *zlib_deflate(void *data, unsigned long size,
return buffer;
}
-static char *construct_path(const char *base, int baselen,
- const char *filename, int isdir, int *pathlen)
-{
- int filenamelen = strlen(filename);
- int len = baselen + filenamelen;
- char *path, *p;
-
- if (isdir)
- len++;
- p = path = xmalloc(len + 1);
-
- memcpy(p, base, baselen);
- p += baselen;
- memcpy(p, filename, filenamelen);
- p += filenamelen;
- if (isdir)
- *p++ = '/';
- *p = '\0';
-
- *pathlen = len;
-
- return path;
-}
-
-static int write_zip_entry(const unsigned char *sha1, const char *base,
- int baselen, const char *filename, unsigned mode, int stage,
- void *context)
+static int write_zip_entry(struct archiver_args *args,
+ const unsigned char *sha1, const char *path, size_t pathlen,
+ unsigned int mode, void *buffer, unsigned long size)
{
struct zip_local_header header;
struct zip_dir_header dirent;
@@ -163,33 +136,20 @@ static int write_zip_entry(const unsigned char *sha1, const char *base,
unsigned long uncompressed_size;
unsigned long crc;
unsigned long direntsize;
- unsigned long size;
int method;
- int result = -1;
- int pathlen;
unsigned char *out;
- char *path;
- enum object_type type;
- void *buffer = NULL;
void *deflated = NULL;
crc = crc32(0, NULL, 0);
- path = construct_path(base, baselen, filename, S_ISDIR(mode), &pathlen);
- if (is_archive_path_ignored(path + base_len))
- return 0;
- if (verbose)
- fprintf(stderr, "%s\n", path);
if (pathlen > 0xffff) {
- error("path too long (%d chars, SHA1: %s): %s", pathlen,
- sha1_to_hex(sha1), path);
- goto out;
+ return error("path too long (%d chars, SHA1: %s): %s",
+ (int)pathlen, sha1_to_hex(sha1), path);
}
if (S_ISDIR(mode) || S_ISGITLINK(mode)) {
method = 0;
attr2 = 16;
- result = (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);
out = NULL;
uncompressed_size = 0;
compressed_size = 0;
@@ -199,19 +159,13 @@ static int write_zip_entry(const unsigned char *sha1, const char *base,
(mode & 0111) ? ((mode) << 16) : 0;
if (S_ISREG(mode) && zlib_compression_level != 0)
method = 8;
- result = 0;
- buffer = sha1_file_to_archive(path + base_len, sha1, mode,
- &type, &size, commit);
- if (!buffer)
- die("cannot read %s", sha1_to_hex(sha1));
crc = crc32(crc, buffer, size);
out = buffer;
uncompressed_size = size;
compressed_size = size;
} else {
- error("unsupported file mode: 0%o (SHA1: %s)", mode,
- sha1_to_hex(sha1));
- goto out;
+ return error("unsupported file mode: 0%o (SHA1: %s)", mode,
+ sha1_to_hex(sha1));
}
if (method == 8) {
@@ -278,12 +232,9 @@ static int write_zip_entry(const unsigned char *sha1, const char *base,
zip_offset += compressed_size;
}
-out:
- free(buffer);
free(deflated);
- free(path);
- return result;
+ return 0;
}
static void write_zip_trailer(const unsigned char *sha1)
@@ -316,31 +267,20 @@ static void dos_time(time_t *time, int *dos_date, int *dos_time)
int write_zip_archive(struct archiver_args *args)
{
+ int err;
+
dos_time(&args->time, &zip_date, &zip_time);
zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
- verbose = args->verbose;
- commit = args->commit;
- base_len = args->baselen;
-
- if (args->baselen > 0 && args->base[args->baselen - 1] == '/') {
- char *base = xstrdup(args->base);
- int baselen = strlen(base);
-
- while (baselen > 0 && base[baselen - 1] == '/')
- base[--baselen] = '\0';
- write_zip_entry(args->tree->object.sha1, "", 0, base, 040777,
- 0, NULL);
- free(base);
- }
- read_tree_recursive(args->tree, args->base, args->baselen, 0,
- args->pathspec, write_zip_entry, NULL);
- write_zip_trailer(args->commit_sha1);
+
+ err = write_archive_entries(args, write_zip_entry);
+ if (!err)
+ write_zip_trailer(args->commit_sha1);
free(zip_dir);
- return 0;
+ return err;
}
void *parse_extra_zip_args(int argc, const char **argv)
--
1.5.6.2.212.g08b51
^ permalink raw reply related
* Re: Syncing up to a tree with a numeric EXTRAVERSION
From: vb @ 2008-07-14 19:41 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git
In-Reply-To: <20080714192947.GS10347@genesis.frugalware.org>
On Mon, Jul 14, 2008 at 12:29 PM, Miklos Vajna <vmiklos@frugalware.org> wrote:
> On Mon, Jul 14, 2008 at 12:24:33PM -0700, vb <vb@vsbe.com> wrote:
>> VERSION = 2
>> PATCHLEVEL = 6
>> SUBLEVEL = 19
>> EXTRAVERSION = .2
>> NAME=Avast! A bilge rat!
>>
>> (...)
>>
>> so, the question is where do I find the tree with the Makefile
>> EXTRAVERSION set to .2?
>
> git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-2.6.19.y.git ?
>
> Hope this helps.
>
It certainly does, and it was quick too! Thanks a lot!
/vb
^ permalink raw reply
* Re: [PATCH] git-rebase.sh: Display error output from git-checkout when detaching HEAD fails.
From: Rob Shearman @ 2008-07-14 19:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vr6a0hvvu.fsf@gitster.siamese.dyndns.org>
[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]
2008/7/11 Junio C Hamano <gitster@pobox.com>:
> Robert Shearman <robertshearman@gmail.com> writes:
>> diff --git a/git-rebase.sh b/git-rebase.sh
>> index e2d85ee..0da2210 100755
>> --- a/git-rebase.sh
>> +++ b/git-rebase.sh
>> @@ -376,7 +376,7 @@ fi
>>
>> # Detach HEAD and reset the tree
>> echo "First, rewinding head to replay your work on top of it..."
>> -git checkout "$onto^0" >/dev/null 2>&1 ||
>
> I think this very much is done deliberately by somebody who knows the
> shell to discard everything.
Why wasn't "git checkout "$onto^0" &> /dev/null" used then? Then only
reason I can come up with would be portability, but it seems
surprising to me.
>> +git checkout "$onto^0" 2>&1 >/dev/null ||
>
> And if it is beneficial to show the error, you just do not touch fd #2,
> like this:
>
> git checkout "$onto^0" >/dev/null
Absolutely. I was just trying to fix the statement to what I thought
was the original intent.
> As I do not see any reason to send the error message to stdout like you
> did.
>
> I also suspect that this part of the script predates 6124aee (add a quiet
> option to git-checkout, 2007-02-01) where the command learned to be more
> quiet during the normal operation. Perhaps you can replace the line with
>
> git checkout -q "$onto^0"
>
> and be done with it. I haven't tested it, though.
I just tested it and it solves the original issue whilst not
displaying unnecessary messages during a rebase. For reference, the
attached script reproduces the issue that I was trying to solve.
Should I resend the patch (like the following) now that it is
effectively completely your work?
-git checkout "$onto^0" >/dev/null 2>&1 ||
+git checkout -q "$onto^0" ||
--
Rob Shearman
[-- Attachment #2: rebase_clash.sh --]
[-- Type: application/x-sh, Size: 356 bytes --]
^ permalink raw reply
* [BUG] commit walk machinery is dangerous !
From: Nicolas Pitre @ 2008-07-14 20:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
Good! I have all your attention now.
Yes, I'm kinda fscking upset about my hardware at this moment. I
apparently have git packs corrupting themselves from time to time which
prompted me to make git more robust against some kind of corruptions
recently.
However this time a corruption turned up and exposed what I think is a
major flaw in git's error checking. To demonstrate it, I created the
following test case. Turning the error() into a die() on line 772 of
commit.c makes this test pass but I don't know if this is the
appropriate fix (e.g. some attempt to parse non existing commits could
be valid usage, etc.). Note this is critical only for git versions
later than commit 8eca0b47ff15.
So here's the test. The catastrophic consequences that this can have on
one's repository is left as an exercise to the reader.
diff --git a/t/t6011-rev-list-with-bad-commit.sh b/t/t6011-rev-list-with-bad-commit.sh
new file mode 100755
index 0000000..a5fe190
--- /dev/null
+++ b/t/t6011-rev-list-with-bad-commit.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='git rev-list should notice bad commits'
+
+. ./test-lib.sh
+
+# Note:
+# - compression level is set to zero to make "corruptions" easier to perform
+# - reflog is disabled to avoid extra references which would twart the test
+
+test_expect_success 'setup' \
+ '
+ git init &&
+ git config core.compression 0 &&
+ git config core.logallrefupdates false &&
+ echo "foo" > foo &&
+ git add foo &&
+ git commit -m "first commit" &&
+ echo "bar" > bar &&
+ git add bar &&
+ git commit -m "second commit" &&
+ echo "baz" > baz &&
+ git add baz &&
+ git commit -m "third commit" &&
+ echo "foo again" >> foo &&
+ git add foo &&
+ git commit -m "fourth commit" &&
+ git repack -a -f -d
+ '
+
+test_expect_success 'verify number of revisions' \
+ '
+ revs=$(git rev-list --all | wc -l) &&
+ test $revs -eq 4 &&
+ first_commit=$(git rev-parse HEAD~3)
+ '
+
+test_expect_success 'corrupt second commit object' \
+ '
+ perl -i.bak -pe "s/second commit/socond commit/" .git/objects/pack/*.pack &&
+ test_must_fail git fsck --all
+ '
+
+test_expect_success 'rev-list should fail' \
+ '
+ test_must_fail git rev-list --all > /dev/null
+ '
+
+test_expect_success 'git repack _MUST_ fail' \
+ '
+ test_must_fail git repack -a -f -d
+ '
+
+test_expect_success 'first commit is still available' \
+ '
+ git log $first_commit
+ '
+
+test_done
+
^ permalink raw reply related
* Re: [PATCH 3/3] gitweb: use new Git::Repo API, and add optional caching
From: Jakub Narebski @ 2008-07-14 21:23 UTC (permalink / raw)
To: Lea Wiemann; +Cc: git, John Hawley, Petr Baudis
In-Reply-To: <1215738708-5212-1-git-send-email-LeWiemann@gmail.com>
On Fri, 11 July 2008, Lea Wiemann wrote:
> Gitweb now uses the Git::Repo API; this change is behavior-preserving,
> except for slightly more aggressive error handling; see below.
Good.
It was suggested to split this into separate commit from the following
change, for making it easier to test (you can check that behavior is
the same, with the exception of error handling) and review (smaller
patch to read and review).
> This patch also adds an optional caching layer for caching repository
> data in memory and (for larger cacheable items, like blobs, snapshots,
> or diffs) on disk.
As it was said, if feasible it would be good idea to put this change
into separate commit.
> Other minor changes:
>
> - Gitweb would previously accept invalid input and either (a) display
> nothing, (b) display an obscure error message, or (c) proceed as
> normal since the parameter happens to be unused in the particular
> code path used. This has changed in that gitweb will check for
> parameter correctness more aggressively, and display meaningful
> error messages. This change is only relevant if you manually edit
> gitweb's CGI parameters, since gitweb only generates valid links.
I understand that this change deals with treating invalid specifiers,
which point to either object that do not exists, are ambiguous, or point
to object of invalid type. Gitweb does check "syntactic" validity of
input (of CGI parameters) already, even those that are not used for
selected action.
BTW. such check was not feasible before implementing --batch and/or
--batch-check options to git-cat-file; I think that possibly one more
fork is not much price to pay for better error checking.
> - Empty projects:
>
> - Only display summary link for empty projects in project list to
> avoid broken links (yielding 404).
>
> - Slim down summary page for empty projects to avoid some broken
> links and unnecessary vertical space.
>
> - Sort empty projects at the bottom of the project list when sorting
> by last change.
>
> - Add test for empty projects to t9503 (the Mechanize test), now
> that there no broken links anymore.
Good. The only thing that *might* be controversial is putting empty
projects at the bottom of sorted by age (by last change) projects list,
instead of at top.
> - For HTML pages, remove the "Expires" HTTP response header, and add
> "Cache-Control: no-cache" instead. This is because pages can
> contain dynamic content (like the subject of the latest commit), so
> the Expires headers would be wrong.
>
> This makes gitweb's responsiveness slightly worse, but it will get
> much better once If-Last-Modified is implemented. It's better to be
> correct than to be convenient here, since having to press the reload
> button makes for lousy user experience (IOW, users should be able to
> always trust gitweb's output).
>
> Raw diffs and blobs still get the Expires header, where appropriate.
I don't think it is a good change.
Gitweb generates two types of views (pages): transient and immutable.
An example of transient view (transient page/action) is for example RSS
feed, or summary page. When project (repository) is updated, they can
change.
The opposite are immutable pages. They are pages/actions/views where
all specifiers are given by full SHA-1; to be more exact all specifiers
that are needed to reconstruct object are given by SHA-1. (It is
enough to have sufficient check for immutability, i.e. such that if
check succeeds, then page is immutable, but it doesn't need to be true
in reverse.)
Gitweb sets expires to '+1d' which is one day to pages it considers
immutable, while not defining expires for other pages (which results,
I think, in lack of expires header). We could have set it to "forever",
which in terms of Expires: HTTP header is half a year (from what
I remember).
Now I don't see *any* reason to not set long expires for immutable
pages; I don't know if forbidding to cache transient pages even if in
fact they are generated dynamically is a good idea... Note that if
caching is enabled, you can set expires to either time-to-expire of
cache entries (simpler), or time left to live to invalidation of item
in cache (better, but more complicated) perhaps also setting Age:
header to appropriate value.
Sidenote: we would probably want to use Expires: for HTTP/1.0 requests,
and Cache-Control: max-age=<seconds> for HTTP/1.1 requests. But that
might be left as improvement for later...
> - Add a $page_info option to display cache stats at the bottom of each
> page; the option is named generically to allow for adding non-cache
> page info there at some point (timings perhaps?).
Great idea!
> ---
> It's all documented of course :-), but for the impatient here's a
> snippet for gitweb_config.perl to activate caching:
Nice.
> use Cache::Memcached;
> $cache = Cache::Memcached->new( { servers => ['localhost:11211'],
> compress_threshold => 1000 } );
IIRC you can use any Cache::Cache compatibile (is it explained later
what it means?) cache here; IMVHO it would be nice if this info would
be also in commit message.
> $large_cache_root = '/home/lewiemann/gitweb-cache';
> $large_cache_case_sensitive = 1;
Errr... I understand that it is your _private_ configuration, just
copied here verbatim, but I don't think '/home/lewiemann/gitweb-cache'
is a good example: '/tmp/gitweb-cache' perhaps, that I can understand.
> # Invalidate cache on changes to gitweb without version number bump;
> # useful for development.
> $cache_key = (stat '/home/lewiemann/gitweb')[9] .
> (stat '/home/lewiemann/gitweb/gitweb.cgi')[9];
What should be used in production? "$cache_key = $version;"?
Besides hardcoding those paths is not a good idea. You can always
use $ENV{'SCRIPT_FILENAME'}, or dirname of it.
> # Display detailed cache info at the bottom of each page.
> $page_info = 2;
Errr... what does "$page_info = <n>;" mean?
> A live demo is here: http://odin3.kernel.org/git-lewiemann/
Nice. Thanks.
[...]
> gitweb/README | 14 +
Very good.
[Comments on patch itself in separate email, later]
--
Jakub Narebski
Poland
^ permalink raw reply
* [PATCH 1/5] Makefile: Normalize $(bindir) and $(gitexecdir) before comparing
From: Johannes Sixt @ 2008-07-14 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Sixt
In-Reply-To: <1216071689-14823-1-git-send-email-johannes.sixt@telecom.at>
The install target needs to check whether the user has opted to make
$(gitexecdir) equal to $(bindir). It did so by a straight string
comparison. Since we are going to allow a relative $(gitexecdir), we have
to normalize paths before comparison, which we do with $(cd there && pwd).
The normalized paths are stored in shell variables. These we can now
reuse in the subsequent install statements, which conveniently shortens
the lines a bit.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Makefile | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 4796565..4de9271 100644
--- a/Makefile
+++ b/Makefile
@@ -1318,18 +1318,18 @@ ifndef NO_TCLTK
$(MAKE) -C gitk-git install
$(MAKE) -C git-gui install
endif
- if test 'z$(bindir_SQ)' != 'z$(gitexecdir_SQ)'; \
- then \
- ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
- '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' || \
- cp '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
- '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X'; \
- fi
- $(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexecdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p' ;)
ifneq (,$X)
$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
endif
- ./check_bindir 'z$(bindir_SQ)' 'z$(gitexecdir_SQ)' '$(DESTDIR_SQ)$(bindir_SQ)/git-shell$X'
+ bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
+ execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
+ if test "z$$bindir" != "z$$execdir"; \
+ then \
+ ln -f "$$bindir/git$X" "$$execdir/git$X" || \
+ cp "$$bindir/git$X" "$$execdir/git$X"; \
+ fi && \
+ { $(foreach p,$(BUILT_INS), $(RM) "$$execdir/$p" && ln "$$execdir/git$X" "$$execdir/$p" ;) } && \
+ ./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-shell$X"
install-doc:
$(MAKE) -C Documentation install
--
1.5.6.3.323.g1e58
^ permalink raw reply related
* [PATCH 0/5] replacement for the part of js/more-win that is in pu
From: Johannes Sixt @ 2008-07-14 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Sixt
In-Reply-To: <1216025226.487b128a031fd@webmail.eunet.at>
The interdiff to js/more-win is below. It is mostly the changes
of 1/5.
Johannes Sixt (5):
Makefile: Normalize $(bindir) and $(gitexecdir) before comparing
Record the command invocation path early
Fix relative built-in paths to be relative to the command
invocation
Allow the built-in exec path to be relative to the command
invocation path
Allow add_path() to add non-existent directories to the path
Makefile | 33 +++++++++++++++++-----------
abspath.c | 36 ++++++++++++++++++++++++++++++
exec_cmd.c | 54 +++++++++++----------------------------------
exec_cmd.h | 3 +-
git.c | 5 +--
path.c | 36 ------------------------------
receive-pack.c | 2 +-
shell.c | 4 +-
upload-pack.c | 2 +-
9 files changed, 77 insertions(+), 98 deletions(-)
diff --git a/Makefile b/Makefile
index 3593e6f..4df6423 100644
--- a/Makefile
+++ b/Makefile
@@ -1301,14 +1301,14 @@ remove-dashes:
### Installation rules
ifeq ($(firstword $(subst /, ,$(template_dir))),..)
-template_instdir = $(shell cd '$(bindir_SQ)/$(template_dir_SQ)' && pwd)
+template_instdir = $(bindir)/$(template_dir)
else
template_instdir = $(template_dir)
endif
export template_instdir
ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
-gitexec_instdir = $(shell cd '$(bindir_SQ)/$(gitexecdir_SQ)' && pwd)
+gitexec_instdir = $(bindir)/$(gitexecdir)
else
gitexec_instdir = $(gitexecdir)
endif
@@ -1325,18 +1325,18 @@ ifndef NO_TCLTK
$(MAKE) -C gitk-git install
$(MAKE) -C git-gui install
endif
- if test 'z$(bindir_SQ)' != 'z$(gitexec_instdir_SQ)'; \
- then \
- ln -f '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
- '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/git$X' || \
- cp '$(DESTDIR_SQ)$(bindir_SQ)/git$X' \
- '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/git$X'; \
- fi
- $(foreach p,$(BUILT_INS), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' && ln '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/git$X' '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p' ;)
ifneq (,$X)
$(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
endif
- ./check_bindir 'z$(bindir_SQ)' 'z$(gitexec_instdir_SQ)' '$(DESTDIR_SQ)$(bindir_SQ)/git-shell$X'
+ bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
+ execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
+ if test "z$$bindir" != "z$$execdir"; \
+ then \
+ ln -f "$$bindir/git$X" "$$execdir/git$X" || \
+ cp "$$bindir/git$X" "$$execdir/git$X"; \
+ fi && \
+ { $(foreach p,$(BUILT_INS), $(RM) "$$execdir/$p" && ln "$$execdir/git$X" "$$execdir/$p" ;) } && \
+ ./check_bindir "z$$bindir" "z$$execdir" "$$bindir/git-shell$X"
install-doc:
$(MAKE) -C Documentation install
^ permalink raw reply related
* [PATCH 3/5] Fix relative built-in paths to be relative to the command invocation
From: Johannes Sixt @ 2008-07-14 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Sixt
In-Reply-To: <1216071689-14823-3-git-send-email-johannes.sixt@telecom.at>
$(gitexecdir) (as defined in the Makefile) has gained another path
component, but the relative paths in the MINGW section of the Makefile,
which are interpreted relative to it, do not account for it.
Instead of adding another ../ in front of the path, we change the code that
constructs the absolute paths to do it relative to the command's directory,
which is essentially $(bindir). We do it this way because we will also
allow a relative $(gitexecdir) later.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Makefile | 2 +-
exec_cmd.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index 4de9271..b9ea0ea 100644
--- a/Makefile
+++ b/Makefile
@@ -1301,7 +1301,7 @@ remove-dashes:
### Installation rules
ifeq ($(firstword $(subst /, ,$(template_dir))),..)
-template_instdir = $(gitexecdir)/$(template_dir)
+template_instdir = $(bindir)/$(template_dir)
else
template_instdir = $(template_dir)
endif
diff --git a/exec_cmd.c b/exec_cmd.c
index dedb01d..45f92eb 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -43,9 +43,9 @@ static const char *builtin_exec_path(void)
const char *system_path(const char *path)
{
- if (!is_absolute_path(path)) {
+ if (!is_absolute_path(path) && argv0_path) {
struct strbuf d = STRBUF_INIT;
- strbuf_addf(&d, "%s/%s", git_exec_path(), path);
+ strbuf_addf(&d, "%s/%s", argv0_path, path);
path = strbuf_detach(&d, NULL);
}
return path;
--
1.5.6.3.323.g1e58
^ permalink raw reply related
* [PATCH 2/5] Record the command invocation path early
From: Johannes Sixt @ 2008-07-14 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Sixt
In-Reply-To: <1216071689-14823-2-git-send-email-johannes.sixt@telecom.at>
We will need the command invocation path in system_path(). This path was
passed to setup_path(), but system_path() can be called earlier, for
example via:
main
commit_pager_choice
setup_pager
git_config
git_etc_gitconfig
system_path
Therefore, we introduce git_set_argv0_path() and call it as soon as
possible.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
exec_cmd.c | 10 ++++++++--
exec_cmd.h | 3 ++-
git.c | 5 ++---
receive-pack.c | 2 +-
shell.c | 4 ++--
upload-pack.c | 2 +-
6 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 8899e31..dedb01d 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -5,6 +5,7 @@
extern char **environ;
static const char *argv_exec_path;
+static const char *argv0_path;
static const char *builtin_exec_path(void)
{
@@ -50,6 +51,11 @@ const char *system_path(const char *path)
return path;
}
+void git_set_argv0_path(const char *path)
+{
+ argv0_path = path;
+}
+
void git_set_argv_exec_path(const char *exec_path)
{
argv_exec_path = exec_path;
@@ -84,7 +90,7 @@ static void add_path(struct strbuf *out, const char *path)
}
}
-void setup_path(const char *cmd_path)
+void setup_path(void)
{
const char *old_path = getenv("PATH");
struct strbuf new_path;
@@ -94,7 +100,7 @@ void setup_path(const char *cmd_path)
add_path(&new_path, argv_exec_path);
add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
add_path(&new_path, builtin_exec_path());
- add_path(&new_path, cmd_path);
+ add_path(&new_path, argv0_path);
if (old_path)
strbuf_addstr(&new_path, old_path);
diff --git a/exec_cmd.h b/exec_cmd.h
index 7eb94e5..0c46cd5 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -2,8 +2,9 @@
#define GIT_EXEC_CMD_H
extern void git_set_argv_exec_path(const char *exec_path);
+extern void git_set_argv0_path(const char *path);
extern const char* git_exec_path(void);
-extern void setup_path(const char *);
+extern void setup_path(void);
extern int execv_git_cmd(const char **argv); /* NULL terminated */
extern int execl_git_cmd(const char *cmd, ...);
extern const char *system_path(const char *path);
diff --git a/git.c b/git.c
index 7075533..b90c358 100644
--- a/git.c
+++ b/git.c
@@ -470,7 +470,6 @@ int main(int argc, const char **argv)
{
const char *cmd = argv[0] && *argv[0] ? argv[0] : "git-help";
char *slash = (char *)cmd + strlen(cmd);
- const char *cmd_path = NULL;
int done_alias = 0;
/*
@@ -483,7 +482,7 @@ int main(int argc, const char **argv)
while (cmd <= slash && !is_dir_sep(*slash));
if (cmd <= slash) {
*slash++ = 0;
- cmd_path = cmd;
+ git_set_argv0_path(cmd);
cmd = slash;
}
@@ -527,7 +526,7 @@ int main(int argc, const char **argv)
* environment, and the $(gitexecdir) from the Makefile at build
* time.
*/
- setup_path(cmd_path);
+ setup_path();
while (1) {
/* See if it's an internal command */
diff --git a/receive-pack.c b/receive-pack.c
index fa653b4..d44c19e 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -482,7 +482,7 @@ int main(int argc, char **argv)
if (!dir)
usage(receive_pack_usage);
- setup_path(NULL);
+ setup_path();
if (!enter_repo(dir, 0))
die("'%s': unable to chdir or not a git archive", dir);
diff --git a/shell.c b/shell.c
index 91ca7de..6a48de0 100644
--- a/shell.c
+++ b/shell.c
@@ -15,7 +15,7 @@ static int do_generic_cmd(const char *me, char *arg)
{
const char *my_argv[4];
- setup_path(NULL);
+ setup_path();
if (!arg || !(arg = sq_dequote(arg)))
die("bad argument");
if (prefixcmp(me, "git-"))
@@ -37,7 +37,7 @@ static int do_cvs_cmd(const char *me, char *arg)
if (!arg || strcmp(arg, "server"))
die("git-cvsserver only handles server: %s", arg);
- setup_path(NULL);
+ setup_path();
return execv_git_cmd(cvsserver_argv);
}
diff --git a/upload-pack.c b/upload-pack.c
index 9f82941..c911e70 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -638,7 +638,7 @@ int main(int argc, char **argv)
if (i != argc-1)
usage(upload_pack_usage);
- setup_path(NULL);
+ setup_path();
dir = argv[i];
--
1.5.6.3.323.g1e58
^ permalink raw reply related
* [PATCH 4/5] Allow the built-in exec path to be relative to the command invocation path
From: Johannes Sixt @ 2008-07-14 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Sixt
In-Reply-To: <1216071689-14823-4-git-send-email-johannes.sixt@telecom.at>
If GIT_EXEC_PATH (the macro that is defined in the Makefile) is relative,
it is interpreted relative to the command's invocation path, which usually
is $(bindir).
The Makefile rules were written with the assumption that $(gitexecdir) is
an absolute path. We introduce a separate variable that names the
(absolute) installation directory.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
Makefile | 15 +++++++++++----
exec_cmd.c | 38 ++------------------------------------
2 files changed, 13 insertions(+), 40 deletions(-)
diff --git a/Makefile b/Makefile
index b9ea0ea..4df6423 100644
--- a/Makefile
+++ b/Makefile
@@ -1307,10 +1307,17 @@ template_instdir = $(template_dir)
endif
export template_instdir
+ifeq ($(firstword $(subst /, ,$(gitexecdir))),..)
+gitexec_instdir = $(bindir)/$(gitexecdir)
+else
+gitexec_instdir = $(gitexecdir)
+endif
+gitexec_instdir_SQ = $(subst ','\'',$(gitexec_instdir))
+
install: all
$(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(bindir_SQ)'
- $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexecdir_SQ)'
- $(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexecdir_SQ)'
+ $(INSTALL) -d -m 755 '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
+ $(INSTALL) $(ALL_PROGRAMS) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)'
$(INSTALL) git$X git-upload-pack$X git-receive-pack$X git-upload-archive$X '$(DESTDIR_SQ)$(bindir_SQ)'
$(MAKE) -C templates DESTDIR='$(DESTDIR_SQ)' install
$(MAKE) -C perl prefix='$(prefix_SQ)' DESTDIR='$(DESTDIR_SQ)' install
@@ -1319,10 +1326,10 @@ ifndef NO_TCLTK
$(MAKE) -C git-gui install
endif
ifneq (,$X)
- $(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexecdir_SQ)/$p';)
+ $(foreach p,$(patsubst %$X,%,$(filter %$X,$(ALL_PROGRAMS) $(BUILT_INS) git$X)), $(RM) '$(DESTDIR_SQ)$(gitexec_instdir_SQ)/$p';)
endif
bindir=$$(cd '$(DESTDIR_SQ)$(bindir_SQ)' && pwd) && \
- execdir=$$(cd '$(DESTDIR_SQ)$(gitexecdir_SQ)' && pwd) && \
+ execdir=$$(cd '$(DESTDIR_SQ)$(gitexec_instdir_SQ)' && pwd) && \
if test "z$$bindir" != "z$$execdir"; \
then \
ln -f "$$bindir/git$X" "$$execdir/git$X" || \
diff --git a/exec_cmd.c b/exec_cmd.c
index 45f92eb..c236034 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -7,40 +7,6 @@ extern char **environ;
static const char *argv_exec_path;
static const char *argv0_path;
-static const char *builtin_exec_path(void)
-{
-#ifndef __MINGW32__
- return GIT_EXEC_PATH;
-#else
- int len;
- char *p, *q, *sl;
- static char *ep;
- if (ep)
- return ep;
-
- len = strlen(_pgmptr);
- if (len < 2)
- return ep = ".";
-
- p = ep = xmalloc(len+1);
- q = _pgmptr;
- sl = NULL;
- /* copy program name, turn '\\' into '/', skip last part */
- while ((*p = *q)) {
- if (*q == '\\' || *q == '/') {
- *p = '/';
- sl = p;
- }
- p++, q++;
- }
- if (sl)
- *sl = '\0';
- else
- ep[0] = '.', ep[1] = '\0';
- return ep;
-#endif
-}
-
const char *system_path(const char *path)
{
if (!is_absolute_path(path) && argv0_path) {
@@ -75,7 +41,7 @@ const char *git_exec_path(void)
return env;
}
- return builtin_exec_path();
+ return system_path(GIT_EXEC_PATH);
}
static void add_path(struct strbuf *out, const char *path)
@@ -99,7 +65,7 @@ void setup_path(void)
add_path(&new_path, argv_exec_path);
add_path(&new_path, getenv(EXEC_PATH_ENVIRONMENT));
- add_path(&new_path, builtin_exec_path());
+ add_path(&new_path, system_path(GIT_EXEC_PATH));
add_path(&new_path, argv0_path);
if (old_path)
--
1.5.6.3.323.g1e58
^ permalink raw reply related
* [PATCH 5/5] Allow add_path() to add non-existent directories to the path
From: Johannes Sixt @ 2008-07-14 21:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Steffen Prohaska, git, Johannes Sixt
In-Reply-To: <1216071689-14823-5-git-send-email-johannes.sixt@telecom.at>
This function had used make_absolute_path(); but this function dies if
the directory that contains the entry whose relative path was supplied in
the argument does not exist. This is a problem if the argument is, for
example, "../libexec/git-core", and that "../libexec" does not exist.
Since the resolution of symbolic links is not required for elements in
PATH, we can fall back to using make_nonrelative_path(), which simply
prepends $PWD to the path.
We have to move make_nonrelative_path() alongside make_absolute_path() in
abspath.c so that git-shell can be linked. See 5b8e6f85f.
Signed-off-by: Johannes Sixt <johannes.sixt@telecom.at>
---
abspath.c | 36 ++++++++++++++++++++++++++++++++++++
exec_cmd.c | 2 +-
path.c | 36 ------------------------------------
3 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/abspath.c b/abspath.c
index 4f95a95..0d56124 100644
--- a/abspath.c
+++ b/abspath.c
@@ -66,3 +66,39 @@ const char *make_absolute_path(const char *path)
return buf;
}
+
+static const char *get_pwd_cwd(void)
+{
+ static char cwd[PATH_MAX + 1];
+ char *pwd;
+ struct stat cwd_stat, pwd_stat;
+ if (getcwd(cwd, PATH_MAX) == NULL)
+ return NULL;
+ pwd = getenv("PWD");
+ if (pwd && strcmp(pwd, cwd)) {
+ stat(cwd, &cwd_stat);
+ if (!stat(pwd, &pwd_stat) &&
+ pwd_stat.st_dev == cwd_stat.st_dev &&
+ pwd_stat.st_ino == cwd_stat.st_ino) {
+ strlcpy(cwd, pwd, PATH_MAX);
+ }
+ }
+ return cwd;
+}
+
+const char *make_nonrelative_path(const char *path)
+{
+ static char buf[PATH_MAX + 1];
+
+ if (is_absolute_path(path)) {
+ if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
+ die("Too long path: %.*s", 60, path);
+ } else {
+ const char *cwd = get_pwd_cwd();
+ if (!cwd)
+ die("Cannot determine the current working directory");
+ if (snprintf(buf, PATH_MAX, "%s/%s", cwd, path) >= PATH_MAX)
+ die("Too long path: %.*s", 60, path);
+ }
+ return buf;
+}
diff --git a/exec_cmd.c b/exec_cmd.c
index c236034..0ed768d 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -50,7 +50,7 @@ static void add_path(struct strbuf *out, const char *path)
if (is_absolute_path(path))
strbuf_addstr(out, path);
else
- strbuf_addstr(out, make_absolute_path(path));
+ strbuf_addstr(out, make_nonrelative_path(path));
strbuf_addch(out, PATH_SEP);
}
diff --git a/path.c b/path.c
index 504eae0..9df447b 100644
--- a/path.c
+++ b/path.c
@@ -291,42 +291,6 @@ int adjust_shared_perm(const char *path)
return 0;
}
-static const char *get_pwd_cwd(void)
-{
- static char cwd[PATH_MAX + 1];
- char *pwd;
- struct stat cwd_stat, pwd_stat;
- if (getcwd(cwd, PATH_MAX) == NULL)
- return NULL;
- pwd = getenv("PWD");
- if (pwd && strcmp(pwd, cwd)) {
- stat(cwd, &cwd_stat);
- if (!stat(pwd, &pwd_stat) &&
- pwd_stat.st_dev == cwd_stat.st_dev &&
- pwd_stat.st_ino == cwd_stat.st_ino) {
- strlcpy(cwd, pwd, PATH_MAX);
- }
- }
- return cwd;
-}
-
-const char *make_nonrelative_path(const char *path)
-{
- static char buf[PATH_MAX + 1];
-
- if (is_absolute_path(path)) {
- if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)
- die ("Too long path: %.*s", 60, path);
- } else {
- const char *cwd = get_pwd_cwd();
- if (!cwd)
- die("Cannot determine the current working directory");
- if (snprintf(buf, PATH_MAX, "%s/%s", cwd, path) >= PATH_MAX)
- die ("Too long path: %.*s", 60, path);
- }
- return buf;
-}
-
const char *make_relative_path(const char *abs, const char *base)
{
static char buf[PATH_MAX + 1];
--
1.5.6.3.323.g1e58
^ permalink raw reply related
* Re: [BUG] commit walk machinery is dangerous !
From: Junio C Hamano @ 2008-07-14 21:55 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: git
In-Reply-To: <alpine.LFD.1.10.0807141641110.12484@xanadu.home>
Nicolas Pitre <nico@cam.org> writes:
> However this time a corruption turned up and exposed what I think is a
> major flaw in git's error checking. To demonstrate it, I created the
> following test case. Turning the error() into a die() on line 772 of
> commit.c makes this test pass but I don't know if this is the
> appropriate fix (e.g. some attempt to parse non existing commits could
> be valid usage, etc.). Note this is critical only for git versions
> later than commit 8eca0b47ff15.
Which probably means we should revert that commit as faulty? IIRC, before
that commit we did check and error out correctly but you loosened the
check to introduce "a major flaw" with that commit.
$ for b in maint master next pu
do
echo -n $b; git cat-file blob $b:commit.c | wc -l
done
maint 672
master 672
next 779
pu 789
Hmph...
^ permalink raw reply
* Re: [PATCH 2/3] add new Git::Repo API
From: Lea Wiemann @ 2008-07-14 22:19 UTC (permalink / raw)
To: Petr Baudis; +Cc: git, John Hawley, Jakub Narebski
In-Reply-To: <20080714014051.GK10151@machine.or.cz>
Petr Baudis wrote:
> Maybe I sound too mentoring at places; [OTOH] if something general gets into
> the Git tree, I'd like to make sure it's something we can live happily
> with for long time, not just a hack tailored for gitweb caching.
Yup, I agree (and you don't sound too mentoring ^^). Thanks for the
review and feedback.
> I really miss some more detailed writeup on the envisioned design here. [...]
> Most importantly, how is Git::Repo interacting with working copies,
Git::Repo is not interacting with working copies at all right now. Is
there anything you think should be considered for its design?
Here's a write-up about the design (I'll probably move this into
Git::Repo's man page):
----------
Git::Repo aims to provide low-level access to Git repositories. For
instance, you can resolve object names (like "HEAD~2") to SHA1s, and
inspect objects. It does not attempt to be a wrapper around the git
plumbing or porcelain commands.
Error handling is simple: On a consistent repository, the Perl interface
will never die. You can use the get_sha1 method to resolve arbitrary
object names or check the existence of SHA1 hashes; get_sha1 will return
undef if the object does not exist in the repository. Any SHA1 that is
returned by get_sha1 can be safely passed to the other Git::Repo methods.
----------
> First, I don't think it's good idea at all to put the pipe-related stuff
> to Git::Repo - this is botched up API just like the current one.
Well, they're more like helper methods. Since they don't fit into the
design goals of the Git::Repo API at all, I'd suggest we just
underscore-prefix them and take them out of the man page. (The only
reason why I hadn't done this is that gitweb uses $repo->cmd_output
extensively, so it'd end up with a lot of underscore calls. But I
suppose we can either alias _cmd_output to cmd_output in gitweb's
CachedRepo subclass, or live with $repo->_cmd_output calls.) Does
underscore-prefixing sound good to you?
If someone wants to come up with a consistent nice interface for calling
git commands, sure. I wasn't actually trying to do that.
> Here is an idea: Introduce Git::Command object [and a Git::CommandFactory]
>
> my $c = Git::Command->new(['git', '--git-dir=.', 'cat-file', \
> '-p', 'bla'], {pipe_out=>1}) [...]
> my $cf = Git::CommandFactory->new('git', '--git-dir=.');
>
> Or am I overdoing it?
Yes, I think so. ;-) All we're talking about here is a wrapper around
"open '-|'" calls (plus some workarounds for Windows I suppose).
I don't have much of a notion of a 'command' as an object in my head;
your (snipped) example makes it look like you're trying to create a
IO::Handle-compatible interface, which I think is way too much effort
(and error-prone) for simple pipes. Also, a CommandFactory class just
to catenate lists together seems like overkill to me.
Something like a command interface *may* make sense if it's tied to
repositories or working copies, in which case it could automatically set
--git-dir or --work-tree, but it's beyond the scope of what I'm trying
to create here, and I don't think it's even overly useful.
> Instead, I believe the best course is to gradually translate all the
> Git.pm functionality to the new OO model, leaving Git.pm as a
> compatibility wrapper. Now, if you believe this is a non-trivial task,
> please tell us why.
Well, as I said, the fact that there are untested parts in Git.pm
doesn't exactly make it trivial to refactor. Also, only very few parts
can be cleanly moved to Git::Repo. In "grep '^sub [^_]' Git.pm" I find
only cat_blob and perhaps hash_object to be eligible to be moved (though
hash_object should probably live in a working-copy/non-bare-repo class,
with an optional insert => 1 parameter). And even cat_blob is
non-trivial to move unless you want the whole blob to be read into memory.
That's a lot of non-trivialness for very little gain. I doubt I'd even
have enough time till the end of GSoC (minus vacation) to do this.
> It should be actually very easy to start with moving all the pipe
> functionality to Git::Command.
Creating a new (Git::Command) API is very much non-trivial, apart from
the fact that I'm not convinced that we need Git::Command, and that a
clean command interface neither falls out of Git.pm nor Git::Repo.
>> I'm working on caching for gitweb, not on implementing the
>> next great Perl API for Git. (And Git::Repo isn't great, FTR.)
>
> Wait, I can't make sense out of this paragraph. If Git.pm sucks, we can
> work on new API. But we better _make_ it great. Or someone else comes by
> next year and says "oh, but it's buggy and needs refactoring, let's
> throw it away and redesign it!"
Sorry, I wasn't clear with my parenthesed remark: I actually think that
Git::Repo is pretty good in terms of code and interface quality. It's
just not *complete*, even in its limited scope, and I'm not attempting
to make it complete.
I do think that someone who wants to extend Git::Repo (like Jakub with
Git::Config) won't have much trouble doing so with the existing design.
>> +use constant _MESSAGE => 'M';
>> +use constant _ENCODING => 'E'; [snip]
>
> if you are going to use hashes
> anyway, why not actually key by sensible name directly?
Embarrassingly premature optimization here. ^_^ I'll fix it.
>> $commit = Git::Commit->new($repo, $sha1)
>
> Are we sure we don't want hash-based arguments instead? This is badly
> extensible and inconsistent with the rest of the API.
*ponders* Every commit needs a repo and a SHA1, so those will never get
optional. We can always add hash-based options after the two mandatory
arguments, but I don't even see any such possible options at the moment.
(And if I turn out to be completely wrong, we can even move to a
hash-only argument list by checking the type of the first parameter.)
Really, I wouldn't worry.
>> [Git::Commit->new, Git::Tag->new:]
>> +Calls to this method are free, since it does not check whether $sha1
>> +exists and has the right type. However, accessing any of the commit
>> +object's properties will fail if $sha1 is not a valid commit object.
>
> This is nice idea, but I'd also provide a well-defined way for the user
> to verify the object's validity at a good moment; basically, make load()
> a public method. The user can deal with errors then and rely on
> error-free behavior later.
No, you should never pass in an invalid SHA1 in the first place. The
above piece of documentation is just a warning that bugs will show up
delayed. IOW, this is not the right place to have your error handling.
If you're getting a SHA1 through the user-interface, check its existence
with get_sha1 before passing it to the constructor.
>> +Note that $sha1 must be the SHA1 of a commit object; tag objects are
>> +not dereferenced.
>
> Why not?
Because the SHA1 might resolve to an object of the wrong type, which
means you have to do error handling in Git::Object objects; that's the
wrong place.
If tag-resolving is really needed, we can add an optional $type
parameter to get_sha1, which will cause get_sha1 to resolve the object
until a $type object is found, or return undef if the object is or
resolves to an object of the wrong type.
I have resolving code in gitweb's git_get_sha1_or_die (which I didn't
implement in Git::Repo since it uses some customized error reporting).
The resolving code could conceivably be extracted and moved to get_sha1.
I think there are a few things to ponder and maybe discuss, so I'd do
that in a separate patch (if I get around it before the end of the project).
>> +=item $commit->authors
>
> author
Fixed.
>> +Objects are loaded lazily, and hence instantiation is free. Objects
>> +stringify to their SHA1s.
>
> Maybe use the term 'Git database objects'? This way, it seems as if we
> are talking about all Git/*.pm objects.
I've replaced it with: "Objects are loaded lazily, and hence
instantiation is free. Git::Object instances stringify to their SHA1s."
>> +sub sha1 {
>> +sub stringify {
>
> Why not overload "" directly to sha1()?
Done (and removed stringify).
>> +sub assert_opts {
>> +sub assert_sha1 {
>
> Pretend names with underscores, since they are internal?
Done, and removed them from @EXPORT_OK.
>> 'directory': The directory of the repository (mandatory).
>
> I don't think making this mandatory is reasonable, since all the git
> commands can automatically figure this out by themselves too; so can
> Git::Repo easily by calling git rev-parse --git-dir.
Sure, it can be made non-mandatory if it's needed, but there are so many
possibilities for the exact time and place at which the repo directory
should be resolved using rev-parse (if at all) that I'd rather leave
this to the person who has an actual use-case for it. I'm not a fan of
designing APIs before they are needed.
>> [Snipped a lot of quoting --LW]
>> +=item $repo->repo_dir
>> +=item $repo->git_binary
>> +=item $repo->version
>> +sub _get_git_cmd {
>
> This definitely does not belong to a Git::Repo object.
Which of those methods are you referring to? I think $repo->version
might reasonably be removed (and the code re-added to gitweb); I'll do
so unless you object. _get_git_cmd is already underscored, and repo_dir
and git_binary only access attributes passed in through the constructor,
so I think those three should stay.
>> +=item $repo->cat_file($sha1)
>
> I don't think this is good combination of semantic and name. Since we
> don't do the same thing as plain git cat-file, we might as well call it
> cat_object() or even better get_object().
Yup; I like get_object (I think I was planning to rename it and then
didn't remember doing so before sending off the patch). Will rename it.
>> +=item $repo->get_path($tree_sha1, $file_sha1)
>
> Now we are quickly getting messy again. This should definitely live in
> Git::Tree.
Yup, that's true. I'll move it into gitweb until we have Git::Tree
(with a comment that it can be moved to Git::Tree once it exists).
>> +=item $repo->get_refs
>> +=item $repo->get_refs($pattern)
>
> Again, the refs should be properly integrated into the object structure.
Really? I think it's generally fine for get_refs to exist and to live
in Git::Repo.
Its return value (currently an an arrayref of [$sha1, $object_type,
$ref_name] arrayrefs) might need improvement though, and I find the
$pattern parameter pretty suspect (in that it smells like a for-each-ref
wrapper). Since get_refs is unused at the moment (gitweb ended up
needing the slightly different show-ref), I'll remove it for now. (Same
thing about me not being a fan of premature API design applies.)
I keep patches of everything I remove so other people will be able to
use them as starting points; I'll probably post them as "FYI"-patches to
the list at the end of my project, to preserve them for posterity.
^ permalink raw reply
* Re: Git describe question
From: Jean-Luc Herren @ 2008-07-14 22:29 UTC (permalink / raw)
To: Mark Burton; +Cc: Junio C Hamano, git
In-Reply-To: <20080714092040.4090046b@crow>
Mark Burton wrote:
> Ok, I understand what it's doing now - but that makes me wonder if it
> would be useful/possible to be able to specify that git describe only
> considers the commits on top of the tag for the current (or some
> specified branch). i.e. at the moment, gitk shows 8 commits on top of
> v1.5.6.3 in the master branch.
Are you saying "git describe" should output v1.5.6.3-8-g10ce020?
That would be misleading and even wrong. This would be like
saying that there are only 8 commits of difference between
v1.5.6.3 and 10ce20, which is not true. The differences between
those two commits are over 300 single commits and that's also what
"git (log|diff) v1.5.6.3..10ce20" will tell you. All 300+ commits
that have been made to the branch master since the branch maint
forked off are part of 10ce20, but not part of v1.5.6.3. It seems
fine to me that "git describe" reflects this difference.
> As the master branch is the checked out branch and the v1.5.6.3 tag
> tags a commit in that branch (via the merge) [...]
I don't think people usually say "tag X is on branch Y", excepted
maybe if Y has never been merged anywhere. Specifically, nobody
would say v1.5.6.3 is *on* branch master. But it's part of its
history. v1.5.6.3 is *on* maint, at best.
jlh
^ permalink raw reply
* Re: [PATCH 3/3] gitweb: use new Git::Repo API, and add optional caching
From: Lea Wiemann @ 2008-07-14 23:03 UTC (permalink / raw)
To: Jakub Narebski; +Cc: git, John Hawley, Petr Baudis
In-Reply-To: <200807142323.22761.jnareb@gmail.com>
Jakub Narebski wrote:
> It was suggested to split this into separate commit
Yup; I'll probably send updated patches tomorrow night (also for patch 2/3).
>> - gitweb will check for parameter correctness more aggressively,
>
> I understand that this change deals with treating invalid specifiers,
> which point to either object that do not exists, are ambiguous, or point
> to object of invalid type.
Yes, that's right. (I don't believe we have any point where ambiguity
might come up though.)
>> - Empty projects: [...]
>
> Good. The only thing that *might* be controversial is putting empty
> projects at the bottom of sorted by age (by last change) projects list,
> instead of at top.
Yup; let's see if anyone objects though. If I sort the list by "Last
Change", I usually want to see projects with recent activity, not dead
project, at the top, which is why I changed it (since I was touching
that line anyway).
>> - For HTML pages, remove the "Expires" HTTP response header, and add
>> "Cache-Control: no-cache" instead. This is because pages can
>> contain dynamic content (like the subject of the latest commit)
>
> I don't think it is a good change.
Hm; I thought transient titles could slip in (e.g. try opening the tree
of some commit and remove the hb parameter; the URL will seem cacheable,
but the page contains the title of the HEAD commit), but I can't find
any URL right now where mainline actually sets a wrong Expires header.
I'll look into it; if you don't see me posting about it again I'll
re-add the Expires header.
> Note that if caching is enabled, you can set expires to either
> time-to-expire of cache entries (simpler), or time left to live to
> invalidation of item in cache (better, but more complicated)
Gitweb's cache is actually never out-of-date, and cache invalidation
happens automatically. It uses some (long) expiry times to guard
against non-standard modification of the repository, but it's nothing
the HTTP client should be concerned with.
>> $cache = Cache::Memcached->new( { servers => ['localhost:11211'],
>
> IIRC you can use any Cache::Cache compatibile cache here;
> IMVHO it would be nice if this info would be also in commit message.
I'll add that.
>> $large_cache_root = '/home/lewiemann/gitweb-cache';
>
> Errr... I understand that it is your _private_ configuration, just
> copied here verbatim, but I don't think '/home/lewiemann/gitweb-cache'
> is a good example: '/tmp/gitweb-cache' perhaps, that I can understand.
Yup. ;-) Or /var/cache/gitweb.
>> # Invalidate cache on changes to gitweb without version number bump;
>> # useful for development.
>> $cache_key = (stat '/home/lewiemann/gitweb')[9] .
>> (stat '/home/lewiemann/gitweb/gitweb.cgi')[9];
>
> What should be used in production? "$cache_key = $version;"?
No, nothing. $version is used automatically as a cache key; I'll add
that to the documentation for $cache_key.
> You can always use $ENV{'SCRIPT_FILENAME'}, or dirname of it.
That one doesn't exist with my thttpd, or any other environment variable
that'd be usable. It's just a hack anyway, so hardcoded paths are OK.
:) I don't think gitweb should check its own mtime by default.
>> # Display detailed cache info at the bottom of each page.
>> $page_info = 2;
>
> Errr... what does "$page_info = <n>;" mean?
Display no (0) / short (1) / detailed (2) page (cache) info at the
bottom of each page. It's documented in gitweb.perl.
> [Comments on patch itself in separate email, later]
Thanks!
^ permalink raw reply
* Re: [BUG] commit walk machinery is dangerous !
From: Nicolas Pitre @ 2008-07-14 23:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vbq10f7wr.fsf@gitster.siamese.dyndns.org>
On Mon, 14 Jul 2008, Junio C Hamano wrote:
> Nicolas Pitre <nico@cam.org> writes:
>
> > However this time a corruption turned up and exposed what I think is a
> > major flaw in git's error checking. To demonstrate it, I created the
> > following test case. Turning the error() into a die() on line 772 of
> > commit.c makes this test pass but I don't know if this is the
> > appropriate fix (e.g. some attempt to parse non existing commits could
> > be valid usage, etc.). Note this is critical only for git versions
> > later than commit 8eca0b47ff15.
>
> Which probably means we should revert that commit as faulty? IIRC, before
> that commit we did check and error out correctly but you loosened the
> check to introduce "a major flaw" with that commit.
>
> $ for b in maint master next pu
> do
> echo -n $b; git cat-file blob $b:commit.c | wc -l
> done
> maint 672
> master 672
> next 779
> pu 789
>
> Hmph...
Well, most of them aren't that critical. If anything they will only
cause a segfault if ever the return value is not checked.
It is those with semantic meaning (e.g. object doesn't exist) which
should be audited, especially if used in the context of repository
modification, which pretty much limits it to the test case I produced.
Nicolas
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Lea Wiemann @ 2008-07-14 23:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vtzetjbif.fsf@gitster.siamese.dyndns.org>
Junio C Hamano wrote:
> * lw/gitweb (Fri Jul 11 03:11:48 2008 +0200) 3 commits
> - Add new Git::Repo API
>
> This does not pass t9710, at least for me X-<.
Yikes; I thought I had removed all instanced of Carp::Always (which I
had put in for development), but this one apparently slipped through.
It'll be fixed in the next version I post (which will also have the
dependency on the non-core Test::Exception package removed).
^ permalink raw reply
* Re: [PATCH 3/3] gitweb: use new Git::Repo API, and add optional caching
From: Jakub Narebski @ 2008-07-14 23:14 UTC (permalink / raw)
To: Lea Wiemann; +Cc: git, John Hawley, Petr Baudis
In-Reply-To: <487BDB34.7010002@gmail.com>
Lea Wiemann wrote:
> Jakub Narebski wrote:
> > Note that if caching is enabled, you can set expires to either
> > time-to-expire of cache entries (simpler), or time left to live to
> > invalidation of item in cache (better, but more complicated)
>
> Gitweb's cache is actually never out-of-date, and cache invalidation
> happens automatically. It uses some (long) expiry times to guard
> against non-standard modification of the repository, but it's nothing
> the HTTP client should be concerned with.
Could you explain then how gitweb cache is invalidated?
The _reasoning_ behind setting Expires:/Cache-Control: max-age= header
for gitweb with caching is that browser doesn't need to even try to
cache-validate or access page for the time we *know* that output would
not change[*1*] because it is from the cache.
[*1*] In significant way: changing relative dates/time doesn't count.
--
Jakub Narebski
Poland
^ permalink raw reply
* Re: Git describe question
From: Junio C Hamano @ 2008-07-14 23:17 UTC (permalink / raw)
To: Jean-Luc Herren; +Cc: Mark Burton, git
In-Reply-To: <487BD34F.4080201@gmx.ch>
Jean-Luc Herren <jlh@gmx.ch> writes:
> Mark Burton wrote:
> ...
> I don't think people usually say "tag X is on branch Y", excepted
> maybe if Y has never been merged anywhere. Specifically, nobody
> would say v1.5.6.3 is *on* branch master. But it's part of its
> history. v1.5.6.3 is *on* maint, at best.
Actually I am somewhat sympathetic to Mark here. Probably what he wants
is to describe 10ce020 as v1.5.6-390-10ce020.
While that probably is doable by using the first-parent-only traversal, I
do not think it is such a good idea. It is not how branches in git are
designed to work. As Merlyn always says in #git at freenode, a branch is
an illusion, and it is especially true in the presense of fast-forward
merge (aka the upstream maintainer asking a subsystem lieutenant to do a
merge for him).
^ permalink raw reply
* Re: What's cooking in git.git (topics)
From: Lea Wiemann @ 2008-07-14 23:20 UTC (permalink / raw)
To: Lea Wiemann; +Cc: Junio C Hamano, git
In-Reply-To: <487BDD56.7010505@gmail.com>
Lea Wiemann wrote:
> It'll be fixed in the next version I post
By the way Junio, how do you prefer to get reposts of patch sequences?
Should I repost the whole sequence under a new common parent message, or
can I simply post v2 of each patch in the sequence as a followup to its
respective v1?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox