* Re: [PATCH] index-pack: correctly initialize appended objects
From: Nicolas Pitre @ 2008-07-25 12:24 UTC (permalink / raw)
To: Björn Steinbrink; +Cc: Johannes Schindelin, Junio C Hamano, spearce, git
In-Reply-To: <20080725120102.GB32487@atjola.homenet>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1618 bytes --]
On Fri, 25 Jul 2008, Björn Steinbrink wrote:
> On 2008.07.25 07:54:49 -0400, Nicolas Pitre wrote:
> > On Fri, 25 Jul 2008, Johannes Schindelin wrote:
> >
> > > Hi,
> > >
> > > On Thu, 24 Jul 2008, Junio C Hamano wrote:
> > >
> > > > The function does not seem to use type (which the patch is also setting)
> > > > nor real_type (which the patch does not set).
> > > >
> > > > However, the code checks objects[nth].real_type all over the place in
> > > > the code. Doesn't the lack of real_type assignment in
> > > > append_obj_to_pack() affect them in any way?
> > >
> > > >From staring at the code, I thought that real_type was set in
> > > resolve_delta(), but I may be wrong.
> > >
> > > The safer thing would be to set it, but I am not quite sure if we can use
> > > "type" directly, or if type can be "delta" for an object that is used to
> > > complete the pack, and therefore stored as a non-delta.
> >
> > Objects to complete the pack are always non delta, so the type and
> > real_type should be the same. However that shouldn't matter since at
> > that point the object array is not walked anymore, at least not for
> > appended objects, and therefore initializing the type at that point is
> > redundant.
>
> Is that still true when the object has been pruned due to memory
> constraints set by deltaBaseCacheLimit? AFAICT when reloading the data
> for the object, we end up in get_base_data, which at least checks
> obj->type.
yeah, true. I don't really have this new code path in my head yet.
In any case, appended objects should have type = real_type = non delta
type.
Nicolas
^ permalink raw reply
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Björn Steinbrink @ 2008-07-25 12:01 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Johannes Schindelin, Junio C Hamano, spearce, git
In-Reply-To: <alpine.LFD.1.10.0807250751220.9968@xanadu.home>
On 2008.07.25 07:54:49 -0400, Nicolas Pitre wrote:
> On Fri, 25 Jul 2008, Johannes Schindelin wrote:
>
> > Hi,
> >
> > On Thu, 24 Jul 2008, Junio C Hamano wrote:
> >
> > > The function does not seem to use type (which the patch is also setting)
> > > nor real_type (which the patch does not set).
> > >
> > > However, the code checks objects[nth].real_type all over the place in
> > > the code. Doesn't the lack of real_type assignment in
> > > append_obj_to_pack() affect them in any way?
> >
> > >From staring at the code, I thought that real_type was set in
> > resolve_delta(), but I may be wrong.
> >
> > The safer thing would be to set it, but I am not quite sure if we can use
> > "type" directly, or if type can be "delta" for an object that is used to
> > complete the pack, and therefore stored as a non-delta.
>
> Objects to complete the pack are always non delta, so the type and
> real_type should be the same. However that shouldn't matter since at
> that point the object array is not walked anymore, at least not for
> appended objects, and therefore initializing the type at that point is
> redundant.
Is that still true when the object has been pruned due to memory
constraints set by deltaBaseCacheLimit? AFAICT when reloading the data
for the object, we end up in get_base_data, which at least checks
obj->type.
Björn
^ permalink raw reply
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Björn Steinbrink @ 2008-07-25 11:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Nicolas Pitre, spearce, git
In-Reply-To: <7vy73q4jzp.fsf@gitster.siamese.dyndns.org>
On 2008.07.24 22:21:14 -0700, Junio C Hamano wrote:
> Reading get_data_from_pack(), it does rely on hdr_size, idx.offset and
> idx.offset of the next entry to be set correctly. The function does not
> seem to use type (which the patch is also setting) nor real_type (which
> the patch does not set).
type is used in get_base_data().
> However, the code checks objects[nth].real_type all over the place in the
> code. Doesn't the lack of real_type assignment in append_obj_to_pack()
> affect them in any way?
I had thought that resolve_delta() would set that, but it seems that we
never call that function like that. Hm...
Björn
^ permalink raw reply
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Nicolas Pitre @ 2008-07-25 11:54 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Junio C Hamano, spearce, git
In-Reply-To: <alpine.DEB.1.00.0807251219530.11976@eeepc-johanness>
On Fri, 25 Jul 2008, Johannes Schindelin wrote:
> Hi,
>
> On Thu, 24 Jul 2008, Junio C Hamano wrote:
>
> > The function does not seem to use type (which the patch is also setting)
> > nor real_type (which the patch does not set).
> >
> > However, the code checks objects[nth].real_type all over the place in
> > the code. Doesn't the lack of real_type assignment in
> > append_obj_to_pack() affect them in any way?
>
> >From staring at the code, I thought that real_type was set in
> resolve_delta(), but I may be wrong.
>
> The safer thing would be to set it, but I am not quite sure if we can use
> "type" directly, or if type can be "delta" for an object that is used to
> complete the pack, and therefore stored as a non-delta.
Objects to complete the pack are always non delta, so the type and
real_type should be the same. However that shouldn't matter since at
that point the object array is not walked anymore, at least not for
appended objects, and therefore initializing the type at that point is
redundant.
Nicolas
^ permalink raw reply
* Re: statistics
From: Rene Herman @ 2008-07-25 11:55 UTC (permalink / raw)
To: Johannes Sixt; +Cc: git
In-Reply-To: <4889B912.9040006@viscovery.net>
On 25-07-08 13:29, Johannes Sixt wrote:
> Rene Herman schrieb:
>> Is there a (non-depressing) way of getting "which files did not change
>> since <rev>" out of git?
>
> What is "non-depressing"?
>
> Try this if you are using bash:
>
> comm -13 <(git diff --name-only your-rev-here) <(git ls-files)
That classifies as non-depressing, thank you. --name-only, process
substitution _and_ comm -13 hadn't featured in my attempts yet.
Rene.
^ permalink raw reply
* Re: [RFC] custom strategies in builtin-merge
From: Sverre Rabbelier @ 2008-07-25 11:50 UTC (permalink / raw)
To: Miklos Vajna; +Cc: git, Johannes Schindelin
In-Reply-To: <20080725113316.GF32057@genesis.frugalware.org>
On Fri, Jul 25, 2008 at 13:33, Miklos Vajna <vmiklos@frugalware.org> wrote:
> 1) Maintain a list of commands that has a git-merge- prefix, but not a
> strategy. This list would currently contain "base, file, index,
> one-file and tree".
Sounds a bit error prone, and could lead to unexpected results if/when
someone creates a new command ('git merge status' anyone?) which is
then suddenly treated as a merge strategy.
> 2) Require custom strategies to have a different naming scheme, like
> if "foo" is a custom strategy, then it would have to be named
> git-merge-custom-foo, _not_ git-merge-foo.
I think this is cleaner, what would be even nicer is to change the
current names too, so name them all "git-merge-stragegy-foo".
--
Cheers,
Sverre Rabbelier
^ permalink raw reply
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Nicolas Pitre @ 2008-07-25 11:48 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: spearce, git, gitster
In-Reply-To: <alpine.DEB.1.00.0807241821440.8986@racer>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1140 bytes --]
On Thu, 24 Jul 2008, Johannes Schindelin wrote:
>
> From: Björn Steinbrink <B.Steinbrink@gmx.de>
>
> When index-pack completes a thin pack it appends objects to the pack.
> Since the commit 92392b4(index-pack: Honor core.deltaBaseCacheLimit when
> resolving deltas) such an object can be pruned in case of memory
> pressure.
>
> To be able to re-read the object later, a few more fields have to be set.
>
> Noticed by Pierre Habouzit.
>
> Hopefully-signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> Hopefully-reviewed-and-signed-off-by: Nicolas Pitre <nico@cam.org>,
>
> --
>
> This was probably missed in the flurry of patches, scratched
> patches, and new patches.
>
> Nico could you have a quick look? (I would ask Shawn, but I know
> that he is pretty busy with real world issues.)
sorry, I have intermitant connectivity this week, and I'll be off the
net for two weeks after that.
Yes, this looks fine, although I'd add a comment mentioning that those
extra fields are uninitialized in the thin pack case when objects are
appended to the pack since they're already initialized otherwise.
ACK.
Nicolas
^ permalink raw reply
* [RFC] custom strategies in builtin-merge
From: Miklos Vajna @ 2008-07-25 11:33 UTC (permalink / raw)
To: git; +Cc: Johannes Schindelin
[-- Attachment #1: Type: text/plain, Size: 760 bytes --]
Hi,
We talked with Dscho about how would it be possible to allow custom
strategies in builtin-merge and I have a question here.
The problem is that currently merge strategies are always named
git-merge-foo, but not all git-merge-foo is a merge strategy.
So we talked about two solutions here:
1) Maintain a list of commands that has a git-merge- prefix, but not a
strategy. This list would currently contain "base, file, index,
one-file and tree".
2) Require custom strategies to have a different naming scheme, like
if "foo" is a custom strategy, then it would have to be named
git-merge-custom-foo, _not_ git-merge-foo.
Both are doable (I prefer 1) a bit), but I thought it's better to ask
first before I implement any of them.
So, comments?
Thanks.
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: statistics
From: Johannes Sixt @ 2008-07-25 11:29 UTC (permalink / raw)
To: Rene Herman; +Cc: git
In-Reply-To: <4889B66D.4020306@keyaccess.nl>
Rene Herman schrieb:
> Hi.
>
> Is there a (non-depressing) way of getting "which files did not change
> since <rev>" out of git?
What is "non-depressing"?
Try this if you are using bash:
comm -13 <(git diff --name-only your-rev-here) <(git ls-files)
-- Hannes
^ permalink raw reply
* statistics
From: Rene Herman @ 2008-07-25 11:18 UTC (permalink / raw)
To: git
Hi.
Is there a (non-depressing) way of getting "which files did not change
since <rev>" out of git?
Rene.
^ permalink raw reply
* Re: [PATCH 5/6] archive: allow --exec and --remote without equal sign
From: René Scharfe @ 2008-07-25 10:54 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1216982486-5887-6-git-send-email-rene.scharfe@lsrfire.ath.cx>
Rene Scharfe schrieb:
> Convert git archive to parse_options(). The parameters --remote and --exec
> are still handled by their special parser. Define them anyway in order for
> them to show up in the usage notice.
Hmpf, the _real_ subject of this email is "[PATCH 6/6] archive: convert
to parse_options()"..
René
^ permalink raw reply
* [PATCH 4/6] archive: declare struct archiver where it's needed
From: Rene Scharfe @ 2008-07-25 10:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1216982486-5887-3-git-send-email-rene.scharfe@lsrfire.ath.cx>
Move the declaration of struct archiver to archive.c, as this is the only
file left where it is used.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.c | 6 +++++-
archive.h | 6 ------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/archive.c b/archive.c
index c4662a2..f834b5f 100644
--- a/archive.c
+++ b/archive.c
@@ -9,7 +9,11 @@ static const char archive_usage[] = \
#define USES_ZLIB_COMPRESSION 1
-const struct archiver archivers[] = {
+const struct archiver {
+ const char *name;
+ write_archive_fn_t write_archive;
+ unsigned int flags;
+} archivers[] = {
{ "tar", write_tar_archive },
{ "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
};
diff --git a/archive.h b/archive.h
index 929368d..0b15b35 100644
--- a/archive.h
+++ b/archive.h
@@ -17,12 +17,6 @@ typedef int (*write_archive_fn_t)(struct archiver_args *);
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;
- unsigned int flags;
-};
-
/*
* Archive-format specific backends.
*/
--
1.6.0.rc0.42.g186458
^ permalink raw reply related
* [PATCH 5/6] archive: allow --exec and --remote without equal sign
From: Rene Scharfe @ 2008-07-25 10:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1216982486-5887-5-git-send-email-rene.scharfe@lsrfire.ath.cx>
Convert git archive to parse_options(). The parameters --remote and --exec
are still handled by their special parser. Define them anyway in order for
them to show up in the usage notice.
Note: in a command like "git archive --prefix --remote=a/ HEAD", the string
"--remote=a/" will be interpreted as a remote option, not a prefix, because
that special parser sees it first. If one needs such a strange prefix, it
needs to be specified like this: "git archive --prefix=--remote=a/ HEAD"
(with an equal sign).
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.c | 110 +++++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 68 insertions(+), 42 deletions(-)
diff --git a/archive.c b/archive.c
index f834b5f..5b40e26 100644
--- a/archive.c
+++ b/archive.c
@@ -3,9 +3,15 @@
#include "tree-walk.h"
#include "attr.h"
#include "archive.h"
-
-static const char archive_usage[] = \
-"git archive --format=<fmt> [--prefix=<prefix>/] [--verbose] [<extra>] <tree-ish> [path...]";
+#include "parse-options.h"
+
+static char const * const archive_usage[] = {
+ "git archive [options] <tree-ish> [path...]",
+ "git archive --list",
+ "git archive --remote <repo> [--exec <cmd>] [options] <tree-ish> [path...]",
+ "git archive --remote <repo> [--exec <cmd>] --list",
+ NULL
+};
#define USES_ZLIB_COMPRESSION 1
@@ -175,6 +181,9 @@ static const struct archiver *lookup_archiver(const char *name)
{
int i;
+ if (!name)
+ return NULL;
+
for (i = 0; i < ARRAY_SIZE(archivers); i++) {
if (!strcmp(name, archivers[i].name))
return &archivers[i];
@@ -232,51 +241,70 @@ static void parse_treeish_arg(const char **argv,
ar_args->time = archive_time;
}
+#define OPT__COMPR(s, v, h, p) \
+ { OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
+#define OPT__COMPR_HIDDEN(s, v, p) \
+ { OPTION_SET_INT, (s), NULL, (v), NULL, "", \
+ PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }
+
static int parse_archive_args(int argc, const char **argv,
const struct archiver **ar, struct archiver_args *args)
{
const char *format = "tar";
- const char *base = "";
+ const char *base = NULL;
+ const char *remote = NULL;
+ const char *exec = NULL;
int compression_level = -1;
int verbose = 0;
int i;
-
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
-
- if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
- for (i = 0; i < ARRAY_SIZE(archivers); i++)
- printf("%s\n", archivers[i].name);
- exit(0);
- }
- if (!strcmp(arg, "--verbose") || !strcmp(arg, "-v")) {
- verbose = 1;
- continue;
- }
- if (!prefixcmp(arg, "--format=")) {
- format = arg + 9;
- continue;
- }
- if (!prefixcmp(arg, "--prefix=")) {
- base = arg + 9;
- continue;
- }
- if (!strcmp(arg, "--")) {
- i++;
- break;
- }
- if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0') {
- compression_level = arg[1] - '0';
- continue;
- }
- if (arg[0] == '-')
- die("Unknown argument: %s", arg);
- break;
+ int list = 0;
+ struct option opts[] = {
+ OPT_GROUP(""),
+ OPT_STRING(0, "format", &format, "fmt", "archive format"),
+ OPT_STRING(0, "prefix", &base, "prefix",
+ "prepend prefix to each pathname in the archive"),
+ OPT__VERBOSE(&verbose),
+ OPT__COMPR('0', &compression_level, "store only", 0),
+ OPT__COMPR('1', &compression_level, "compress faster", 1),
+ OPT__COMPR_HIDDEN('2', &compression_level, 2),
+ OPT__COMPR_HIDDEN('3', &compression_level, 3),
+ OPT__COMPR_HIDDEN('4', &compression_level, 4),
+ OPT__COMPR_HIDDEN('5', &compression_level, 5),
+ OPT__COMPR_HIDDEN('6', &compression_level, 6),
+ OPT__COMPR_HIDDEN('7', &compression_level, 7),
+ OPT__COMPR_HIDDEN('8', &compression_level, 8),
+ OPT__COMPR('9', &compression_level, "compress better", 9),
+ OPT_GROUP(""),
+ OPT_BOOLEAN('l', "list", &list,
+ "list supported archive formats"),
+ OPT_GROUP(""),
+ OPT_STRING(0, "remote", &remote, "repo",
+ "retrieve the archive from remote repository <repo>"),
+ OPT_STRING(0, "exec", &exec, "cmd",
+ "path to the remote git-upload-archive command"),
+ OPT_END()
+ };
+
+ argc = parse_options(argc, argv, opts, archive_usage, 0);
+
+ if (remote)
+ die("Unexpected option --remote");
+ if (exec)
+ die("Option --exec can only be used together with --remote");
+
+ if (!base)
+ base = "";
+
+ if (list) {
+ for (i = 0; i < ARRAY_SIZE(archivers); i++)
+ printf("%s\n", archivers[i].name);
+ exit(0);
}
/* We need at least one parameter -- tree-ish */
- if (argc - 1 < i)
- usage(archive_usage);
+ if (argc < 1)
+ usage_with_options(archive_usage, opts);
*ar = lookup_archiver(format);
if (!*ar)
die("Unknown archive format '%s'", format);
@@ -294,7 +322,7 @@ static int parse_archive_args(int argc, const char **argv,
args->base = base;
args->baselen = strlen(base);
- return i;
+ return argc;
}
int write_archive(int argc, const char **argv, const char *prefix,
@@ -302,13 +330,11 @@ int write_archive(int argc, const char **argv, const char *prefix,
{
const struct archiver *ar = NULL;
struct archiver_args args;
- int tree_idx;
- tree_idx = parse_archive_args(argc, argv, &ar, &args);
+ argc = parse_archive_args(argc, argv, &ar, &args);
if (setup_prefix && prefix == NULL)
prefix = setup_git_directory();
- argv += tree_idx;
parse_treeish_arg(argv, &args, prefix);
parse_pathspec_arg(argv + 1, &args);
--
1.6.0.rc0.42.g186458
^ permalink raw reply related
* [PATCH 2/6] archive: move parameter parsing code to archive.c
From: Rene Scharfe @ 2008-07-25 10:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Stephan Beyer
In-Reply-To: <1216982486-5887-1-git-send-email-rene.scharfe@lsrfire.ath.cx>
write_archive() in archive.c is the only callsite for the command line
parsing functions located in builtin-archive.c. Move them to the place
where they are used, un-export them and make them static, as hinted at
by Stephan.
Cc: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.c | 137 +++++++++++++++++++++++++++++++++++++++++++++++++++++
archive.h | 8 ---
builtin-archive.c | 137 -----------------------------------------------------
3 files changed, 137 insertions(+), 145 deletions(-)
diff --git a/archive.c b/archive.c
index 75eb257..c4662a2 100644
--- a/archive.c
+++ b/archive.c
@@ -1,8 +1,19 @@
#include "cache.h"
#include "commit.h"
+#include "tree-walk.h"
#include "attr.h"
#include "archive.h"
+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 },
+ { "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
+};
+
static void format_subst(const struct commit *commit,
const char *src, size_t len,
struct strbuf *buf)
@@ -156,6 +167,132 @@ int write_archive_entries(struct archiver_args *args,
return err;
}
+static const struct archiver *lookup_archiver(const char *name)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(archivers); i++) {
+ if (!strcmp(name, archivers[i].name))
+ return &archivers[i];
+ }
+ return NULL;
+}
+
+static void parse_pathspec_arg(const char **pathspec,
+ struct archiver_args *ar_args)
+{
+ ar_args->pathspec = get_pathspec(ar_args->base, pathspec);
+}
+
+static void parse_treeish_arg(const char **argv,
+ struct archiver_args *ar_args, const char *prefix)
+{
+ const char *name = argv[0];
+ const unsigned char *commit_sha1;
+ time_t archive_time;
+ struct tree *tree;
+ const struct commit *commit;
+ unsigned char sha1[20];
+
+ if (get_sha1(name, sha1))
+ die("Not a valid object name");
+
+ commit = lookup_commit_reference_gently(sha1, 1);
+ if (commit) {
+ commit_sha1 = commit->object.sha1;
+ archive_time = commit->date;
+ } else {
+ commit_sha1 = NULL;
+ archive_time = time(NULL);
+ }
+
+ tree = parse_tree_indirect(sha1);
+ if (tree == NULL)
+ die("not a tree object");
+
+ if (prefix) {
+ unsigned char tree_sha1[20];
+ unsigned int mode;
+ int err;
+
+ err = get_tree_entry(tree->object.sha1, prefix,
+ tree_sha1, &mode);
+ if (err || !S_ISDIR(mode))
+ die("current working directory is untracked");
+
+ tree = parse_tree_indirect(tree_sha1);
+ }
+ ar_args->tree = tree;
+ ar_args->commit_sha1 = commit_sha1;
+ ar_args->commit = commit;
+ ar_args->time = archive_time;
+}
+
+static int parse_archive_args(int argc, const char **argv,
+ const struct archiver **ar, struct archiver_args *args)
+{
+ const char *format = "tar";
+ const char *base = "";
+ int compression_level = -1;
+ int verbose = 0;
+ int i;
+
+ for (i = 1; i < argc; i++) {
+ const char *arg = argv[i];
+
+ if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
+ for (i = 0; i < ARRAY_SIZE(archivers); i++)
+ printf("%s\n", archivers[i].name);
+ exit(0);
+ }
+ if (!strcmp(arg, "--verbose") || !strcmp(arg, "-v")) {
+ verbose = 1;
+ continue;
+ }
+ if (!prefixcmp(arg, "--format=")) {
+ format = arg + 9;
+ continue;
+ }
+ if (!prefixcmp(arg, "--prefix=")) {
+ base = arg + 9;
+ continue;
+ }
+ if (!strcmp(arg, "--")) {
+ i++;
+ break;
+ }
+ if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0') {
+ compression_level = arg[1] - '0';
+ continue;
+ }
+ if (arg[0] == '-')
+ die("Unknown argument: %s", arg);
+ break;
+ }
+
+ /* We need at least one parameter -- tree-ish */
+ if (argc - 1 < i)
+ usage(archive_usage);
+ *ar = lookup_archiver(format);
+ if (!*ar)
+ die("Unknown archive format '%s'", format);
+
+ args->compression_level = Z_DEFAULT_COMPRESSION;
+ if (compression_level != -1) {
+ if ((*ar)->flags & USES_ZLIB_COMPRESSION)
+ args->compression_level = compression_level;
+ else {
+ die("Argument not supported for format '%s': -%d",
+ format, compression_level);
+ }
+ }
+ args->verbose = verbose;
+ args->base = base;
+ args->baselen = strlen(base);
+
+ return i;
+}
+
int write_archive(int argc, const char **argv, const char *prefix,
int setup_prefix)
{
diff --git a/archive.h b/archive.h
index 6b5fe5a..f6ceaeb 100644
--- a/archive.h
+++ b/archive.h
@@ -26,14 +26,6 @@ struct archiver {
unsigned int flags;
};
-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,
- const char *prefix);
-
-extern void parse_pathspec_arg(const char **pathspec,
- struct archiver_args *args);
/*
* Archive-format specific backends.
*/
diff --git a/builtin-archive.c b/builtin-archive.c
index 502b339..4dd2716 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -5,21 +5,9 @@
#include "cache.h"
#include "builtin.h"
#include "archive.h"
-#include "commit.h"
-#include "tree-walk.h"
#include "pkt-line.h"
#include "sideband.h"
-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 },
- { "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
-};
-
static int run_remote_archiver(const char *remote, int argc,
const char **argv)
{
@@ -74,131 +62,6 @@ static int run_remote_archiver(const char *remote, int argc,
return !!rv;
}
-static const struct archiver *lookup_archiver(const char *name)
-{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(archivers); i++) {
- if (!strcmp(name, archivers[i].name))
- return &archivers[i];
- }
- return NULL;
-}
-
-void parse_pathspec_arg(const char **pathspec, struct archiver_args *ar_args)
-{
- ar_args->pathspec = get_pathspec(ar_args->base, pathspec);
-}
-
-void parse_treeish_arg(const char **argv, struct archiver_args *ar_args,
- const char *prefix)
-{
- const char *name = argv[0];
- const unsigned char *commit_sha1;
- time_t archive_time;
- struct tree *tree;
- const struct commit *commit;
- unsigned char sha1[20];
-
- if (get_sha1(name, sha1))
- die("Not a valid object name");
-
- commit = lookup_commit_reference_gently(sha1, 1);
- if (commit) {
- commit_sha1 = commit->object.sha1;
- archive_time = commit->date;
- } else {
- commit_sha1 = NULL;
- archive_time = time(NULL);
- }
-
- tree = parse_tree_indirect(sha1);
- if (tree == NULL)
- die("not a tree object");
-
- if (prefix) {
- unsigned char tree_sha1[20];
- unsigned int mode;
- int err;
-
- err = get_tree_entry(tree->object.sha1, prefix,
- tree_sha1, &mode);
- if (err || !S_ISDIR(mode))
- die("current working directory is untracked");
-
- tree = parse_tree_indirect(tree_sha1);
- }
- ar_args->tree = tree;
- ar_args->commit_sha1 = commit_sha1;
- ar_args->commit = commit;
- ar_args->time = archive_time;
-}
-
-int parse_archive_args(int argc, const char **argv, const struct archiver **ar,
- struct archiver_args *args)
-{
- const char *format = "tar";
- const char *base = "";
- int compression_level = -1;
- int verbose = 0;
- int i;
-
- for (i = 1; i < argc; i++) {
- const char *arg = argv[i];
-
- if (!strcmp(arg, "--list") || !strcmp(arg, "-l")) {
- for (i = 0; i < ARRAY_SIZE(archivers); i++)
- printf("%s\n", archivers[i].name);
- exit(0);
- }
- if (!strcmp(arg, "--verbose") || !strcmp(arg, "-v")) {
- verbose = 1;
- continue;
- }
- if (!prefixcmp(arg, "--format=")) {
- format = arg + 9;
- continue;
- }
- if (!prefixcmp(arg, "--prefix=")) {
- base = arg + 9;
- continue;
- }
- if (!strcmp(arg, "--")) {
- i++;
- break;
- }
- if (arg[0] == '-' && isdigit(arg[1]) && arg[2] == '\0') {
- compression_level = arg[1] - '0';
- continue;
- }
- if (arg[0] == '-')
- die("Unknown argument: %s", arg);
- break;
- }
-
- /* We need at least one parameter -- tree-ish */
- if (argc - 1 < i)
- usage(archive_usage);
- *ar = lookup_archiver(format);
- if (!*ar)
- die("Unknown archive format '%s'", format);
-
- args->compression_level = Z_DEFAULT_COMPRESSION;
- if (compression_level != -1) {
- if ((*ar)->flags & USES_ZLIB_COMPRESSION)
- args->compression_level = compression_level;
- else {
- die("Argument not supported for format '%s': -%d",
- format, compression_level);
- }
- }
- args->verbose = verbose;
- args->base = base;
- args->baselen = strlen(base);
-
- return i;
-}
-
static const char *extract_remote_arg(int *ac, const char **av)
{
int ix, iy, cnt = *ac;
--
1.6.0.rc0.42.g186458
^ permalink raw reply related
* [PATCH 5/6] archive: allow --exec and --remote without equal sign
From: Rene Scharfe @ 2008-07-25 10:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1216982486-5887-4-git-send-email-rene.scharfe@lsrfire.ath.cx>
Allow "--remote repo" and "--exec cmd" in addition to "--remote=repo" and
"--exec=cmd" to make their usage consistent with parameters handled by
parse_options().
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
builtin-archive.c | 20 +++++++++++++++++---
1 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index 4dd2716..22445ac 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -15,7 +15,7 @@ static int run_remote_archiver(const char *remote, int argc,
int fd[2], i, len, rv;
struct child_process *conn;
const char *exec = "git-upload-archive";
- int exec_at = 0;
+ int exec_at = 0, exec_value_at = 0;
for (i = 1; i < argc; i++) {
const char *arg = argv[i];
@@ -24,7 +24,14 @@ static int run_remote_archiver(const char *remote, int argc,
die("multiple --exec specified");
exec = arg + 7;
exec_at = i;
- break;
+ } else if (!strcmp(arg, "--exec")) {
+ if (exec_at)
+ die("multiple --exec specified");
+ if (i + 1 >= argc)
+ die("option --exec requires a value");
+ exec = argv[i + 1];
+ exec_at = i;
+ exec_value_at = ++i;
}
}
@@ -32,7 +39,7 @@ static int run_remote_archiver(const char *remote, int argc,
conn = git_connect(fd, url, exec, 0);
for (i = 1; i < argc; i++) {
- if (i == exec_at)
+ if (i == exec_at || i == exec_value_at)
continue;
packet_write(fd[1], "argument %s\n", argv[i]);
}
@@ -78,6 +85,13 @@ static const char *extract_remote_arg(int *ac, const char **av)
die("Multiple --remote specified");
remote = arg + 9;
continue;
+ } else if (!strcmp(arg, "--remote")) {
+ if (remote)
+ die("Multiple --remote specified");
+ if (++ix >= cnt)
+ die("option --remote requires a value");
+ remote = av[ix];
+ continue;
}
if (arg[0] != '-')
no_more_options = 1;
--
1.6.0.rc0.42.g186458
^ permalink raw reply related
* [PATCH 3/6] archive: define MAX_ARGS where it's needed
From: Rene Scharfe @ 2008-07-25 10:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
In-Reply-To: <1216982486-5887-2-git-send-email-rene.scharfe@lsrfire.ath.cx>
MAX_EXTRA_ARGS is not used anymore, so remove it. MAX_ARGS is used only
in builtin-upload-archive.c, so define it there. Also report the actual
value we're comparing against when the number of args is too big.
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.h | 3 ---
builtin-upload-archive.c | 3 ++-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/archive.h b/archive.h
index f6ceaeb..929368d 100644
--- a/archive.h
+++ b/archive.h
@@ -1,9 +1,6 @@
#ifndef ARCHIVE_H
#define ARCHIVE_H
-#define MAX_EXTRA_ARGS 32
-#define MAX_ARGS (MAX_EXTRA_ARGS + 32)
-
struct archiver_args {
const char *base;
size_t baselen;
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index cc37b36..a9b02fa 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -16,6 +16,7 @@ static const char deadchild[] =
static const char lostchild[] =
"git upload-archive: archiver process was lost";
+#define MAX_ARGS (64)
static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
@@ -45,7 +46,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)
if (len == 0)
break; /* got a flush */
if (sent_argc > MAX_ARGS - 2)
- die("Too many options (>29)");
+ die("Too many options (>%d)", MAX_ARGS - 2);
if (p[len-1] == '\n') {
p[--len] = 0;
--
1.6.0.rc0.42.g186458
^ permalink raw reply related
* [PATCH 1/6] archive: add write_archive()
From: Rene Scharfe @ 2008-07-25 10:41 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano
Both archive and upload-archive have to parse command line arguments and
then call the archiver specific write function. Move the duplicate code
to a new function, write_archive().
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
archive.c | 18 ++++++++++++++++++
archive.h | 1 +
builtin-archive.c | 13 +------------
builtin-upload-archive.c | 10 +---------
4 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/archive.c b/archive.c
index b8b45ba..75eb257 100644
--- a/archive.c
+++ b/archive.c
@@ -155,3 +155,21 @@ int write_archive_entries(struct archiver_args *args,
err = 0;
return err;
}
+
+int write_archive(int argc, const char **argv, const char *prefix,
+ int setup_prefix)
+{
+ const struct archiver *ar = NULL;
+ struct archiver_args args;
+ int tree_idx;
+
+ tree_idx = parse_archive_args(argc, argv, &ar, &args);
+ if (setup_prefix && prefix == NULL)
+ prefix = setup_git_directory();
+
+ argv += tree_idx;
+ parse_treeish_arg(argv, &args, prefix);
+ parse_pathspec_arg(argv + 1, &args);
+
+ return ar->write_archive(&args);
+}
diff --git a/archive.h b/archive.h
index 4a02371..6b5fe5a 100644
--- a/archive.h
+++ b/archive.h
@@ -41,5 +41,6 @@ extern int write_tar_archive(struct archiver_args *);
extern int write_zip_archive(struct archiver_args *);
extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
+extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);
#endif /* ARCHIVE_H */
diff --git a/builtin-archive.c b/builtin-archive.c
index df97724..502b339 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -232,9 +232,6 @@ static const char *extract_remote_arg(int *ac, const char **av)
int cmd_archive(int argc, const char **argv, const char *prefix)
{
- const struct archiver *ar = NULL;
- struct archiver_args args;
- int tree_idx;
const char *remote = NULL;
remote = extract_remote_arg(&argc, argv);
@@ -243,13 +240,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
- tree_idx = parse_archive_args(argc, argv, &ar, &args);
- if (prefix == NULL)
- prefix = setup_git_directory();
-
- argv += tree_idx;
- parse_treeish_arg(argv, &args, prefix);
- parse_pathspec_arg(argv + 1, &args);
-
- return ar->write_archive(&args);
+ return write_archive(argc, argv, prefix, 1);
}
diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index 13a6c62..cc37b36 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -19,12 +19,9 @@ static const char lostchild[] =
static int run_upload_archive(int argc, const char **argv, const char *prefix)
{
- const struct archiver *ar;
- struct archiver_args args;
const char *sent_argv[MAX_ARGS];
const char *arg_cmd = "argument ";
char *p, buf[4096];
- int treeish_idx;
int sent_argc;
int len;
@@ -66,12 +63,7 @@ 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, &args);
-
- parse_treeish_arg(sent_argv + treeish_idx, &args, prefix);
- parse_pathspec_arg(sent_argv + treeish_idx + 1, &args);
-
- return ar->write_archive(&args);
+ return write_archive(sent_argc, sent_argv, prefix, 0);
}
static void error_clnt(const char *fmt, ...)
--
1.6.0.rc0.42.g186458
^ permalink raw reply related
* Re: [PATCH] rebase -i: only automatically amend commit if HEAD did not change
From: Johannes Schindelin @ 2008-07-25 10:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <7vod4muzck.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 25 Jul 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> >> At what point in what valid workflow sequence does HEAD become
> >> different from dotest/amend?
> >
> > $ rebase -i HEAD~5
> >
> > <mark one commit as edit>
> >
> > <Whoa! While editing, I realize that this contains an unrelated
> > bugfix>
> >
> > $ git reset HEAD^
> > $ git add -p
> > $ git commit
> >
> > <Edit a bit here, a bit there>
> >
> > $ git rebase --continue
> >
> > Sure it is a pilot error. It hit this pilot, too.
> > ...
> > In the way that the user certainly did not mean to amend _this_ HEAD.
> > Another HEAD was marked with "edit".
>
> Ok; after this "refraining from incorrectly squashing them", how would the
> user edit the one the user originally intended to edit (I am not
> complaining, but asking for information)?
In this case, the two commits are two commits, the editor popped up with
rebase --continue contains the original commit message, and all is fine.
> So in your workflow example, when there is no pilot error, is this the
> "ideal" sequence?
>
> $ git rebase -i HEAD~5
> .. mark one as edit
> .. ah, the one I wanted to just "edit" actually need to be
> .. split into two because it has some other thing I need to change
> $ git reset HEAD^
> $ git add -p
> $ git stash --keep-index
> .. test to verify the initial part
> $ git commit ;# first part of split commit
> $ git stash pop
> .. test test
> $ git add -p
Without pilot error, here has to come a "git commit -c HEAD@{1}".
> $ git rebase --continue ;# gives you the editor to edit
>
> I wonder if we can make the transcript of the "pilot error" case look like
> this:
>
> $ git rebase -i HEAD~5
> ...
> $ git reset HEAD^
> .. same as above up to...
> .. test to verify the initial part
> $ git rebase --continue ;# oops
> .. gives you the editor to edit the message.
> .. makes a commit, and says:
> committed initial part of the change, stopping.
> .. ah, the command noticed it and did not escape, thanks!
> $ git stash pop
> .. test test
> $ git add -p
> $ git rebase --continue ;# gives you the editor to edit
> .. and goes on this time.
Possible.
The first hunk (IIRC) of my patch would stay the same, but the second hunk
would not fall through to the interactive commit, instead testing if amend
exists, _and_ is different from HEAD, and if both are the case, say
something helpful and exit.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] editor.c: Libify launch_editor()
From: Johannes Schindelin @ 2008-07-25 10:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stephan Beyer, git
In-Reply-To: <7vvdyuuzq2.fsf@gitster.siamese.dyndns.org>
Hi,
On Fri, 25 Jul 2008, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Fri, 18 Jul 2008, Stephan Beyer wrote:
> >
> >> diff --git a/editor.c b/editor.c
> >> index 483b62d..5d7f5f9 100644
> >> --- a/editor.c
> >> +++ b/editor.c
> >> @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
> > ...
> > Why not "return error()"?
> >
> > Rest looks obviously correct to me!
>
> This is a patch to an existing file editor.c???
Yes, Stephan sent the two patches unrelatedly, even if they should have
been marked "1/2" and "2/2". I hope he does so now.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] index-pack: correctly initialize appended objects
From: Johannes Schindelin @ 2008-07-25 10:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Nicolas Pitre, spearce, git
In-Reply-To: <7vy73q4jzp.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 24 Jul 2008, Junio C Hamano wrote:
> The function does not seem to use type (which the patch is also setting)
> nor real_type (which the patch does not set).
>
> However, the code checks objects[nth].real_type all over the place in
> the code. Doesn't the lack of real_type assignment in
> append_obj_to_pack() affect them in any way?
>From staring at the code, I thought that real_type was set in
resolve_delta(), but I may be wrong.
The safer thing would be to set it, but I am not quite sure if we can use
"type" directly, or if type can be "delta" for an object that is used to
complete the pack, and therefore stored as a non-delta.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Preserve cwd in setup_git_directory()
From: Johannes Schindelin @ 2008-07-25 10:04 UTC (permalink / raw)
To: Geoff Russell; +Cc: Nguyen Thai Ngoc Duy, git
In-Reply-To: <93c3eada0807250248l60c81090sd1953df24cca1421@mail.gmail.com>
Hi,
On Fri, 25 Jul 2008, Geoff Russell wrote:
> I was pretty surprised to find that my command line switch (--work-tree)
> was being stored in the config file, which ended up causing the problem.
> Duy thinks he can fix this, which is fine, but as a matter of principle,
> I'm not keen on command line switches being magically saved for me when
> I didn't ask for this to happen.
You were initializing a repository. How on earth could you be surprised
when _important_ things like work-tree git-dir or if it is bare are stored
inside the freshly initialized repository?
> The reason for using a command line switch is because I want to override
> default behaviour on this execution, if I wanted to change the default
> behaviour, then I'd set values in the config file.
No, you would not. Because there is no config file yet.
Htrh,
Dscho
^ permalink raw reply
* [PATCH] bash completion: Add long options for 'git describe'
From: Thomas Rast @ 2008-07-25 10:02 UTC (permalink / raw)
To: git; +Cc: gitster, Shawn O. Pearce
---
It seemed so tedious to type --always so I wrote this instead ;-)
contrib/completion/git-completion.bash | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 3b04934..421431c 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -665,6 +665,17 @@ _git_commit ()
_git_describe ()
{
+ __git_has_doubledash && return
+
+ local cur="${COMP_WORDS[COMP_CWORD]}"
+ case "$cur" in
+ --*)
+ __gitcomp "
+ --all --tags --contains --abbrev= --candidates=
+ --exact-match --debug --long --match --always
+ "
+ return
+ esac
__gitcomp "$(__git_refs)"
}
--
1.6.0.rc0.48.g3fa0a
^ permalink raw reply related
* Re: [PATCH] Preserve cwd in setup_git_directory()
From: Geoff Russell @ 2008-07-25 9:48 UTC (permalink / raw)
To: Nguyen Thai Ngoc Duy; +Cc: Johannes Schindelin, git
In-Reply-To: <fcaeb9bf0807241137h65292d7egad8cc5f797114607@mail.gmail.com>
Hi,
On 7/25/08, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On 7/24/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > Hi,
> >
> > On Thu, 24 Jul 2008, Nguyen Thai Ngoc Duy wrote:
> >
> > > On 7/24/08, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > >
> >
> > > > On Thu, 24 Jul 2008, Nguyen Thai Ngoc Duy wrote:
> > > >
> > > > > When GIT_DIR is not set, cwd is used to determine where .git is. If
> > > > > core.worktree is set, setup_git_directory() needs to jump back to
> > > > > the original cwd in order to calculate worktree, this leads to
> > > > > incorrect .git location later in setup_work_tree().
> > > >
> > > > I do not understand. core.worktree is either absolute, in which case
> > > > there is no problem. Or it is relative to where the config lives, no?
> > >
> > > The problem is GIT_DIR is not absolute in this case. So cwd must stay
> > > where git dir is until it is absolute-ized by setup_work_tree().
> >
> >
> > I do not see GIT_DIR being set in your test case at all.
> >
> > I do not see how get_git_work_tree() ro get_relative_cwd() should ever be
> > allowed to chdir().
> >
> > _If_ they were (which I strongly doubt), they should chdir() back
> > themselves.
> >
> > I now wasted easily 30 minutes just trying to make sense of your patch and
> > your response. And I am still puzzled.
> >
> > Your commit message was of no help.
>
>
> Alright, let's look at the code.
>
> 1. cwd is moved to toplevel working directory by setup_git_directory_gently()
> 2. setup_git_env() by default will set git_dir variable as ".git" as
> part of check_repository_format_gently()
> 3. now in setup_git_directory() finds out core.worktree, it chdir()
> to get relative prefix
> 4. setup_work_tree() sees that git_dir is not absolute path, it makes
> git_dir absolute
>
> If in step 3, it does not chdir(), step 4 will be right. In this case,
> step 3 does chdir() and not going back, access to git repository will
> fail as Geoff Russell discovered.
> --
>
> Duy
>
There seem to be plenty of things happening here which I know nothing
about, which
is really why I hit the problem in the first place. I was pretty
surprised to find that
my command line switch (--work-tree) was being stored in the config file, which
ended up causing the problem. Duy thinks he can fix this, which is
fine, but as a matter of principle, I'm not keen on command line switches
being magically saved for me when I didn't ask for this to happen.
The reason for
using a command line switch is because I want to override default
behaviour on this execution, if I wanted to change the default behaviour, then
I'd set values in the config file.
Whatever you decide, I reckon its pretty magical to have a software
problem, send
an email to a list and get the solution in 45 minutes. I've never got that
service from any other piece of software (or hardware)!
Cheers,
Geoff Russell
^ permalink raw reply
* Re: [PATCH 7/9] builtin-checkout-index.c: use parse_options()
From: René Scharfe @ 2008-07-25 9:01 UTC (permalink / raw)
To: sverre; +Cc: Michele Ballabio, Johannes Schindelin, git, gitster
In-Reply-To: <bd6139dc0807241335i3ab5280aq6a46325428ccc70f@mail.gmail.com>
Sverre Rabbelier schrieb:
> On Thu, Jul 24, 2008 at 10:08 PM, Michele Ballabio
> <barra_cuda@katamail.com> wrote:
>> On Thursday 24 July 2008, Johannes Schindelin wrote:
>>> On Wed, 23 Jul 2008, Michele Ballabio wrote:
>>>
>>>> + { OPTION_CALLBACK, 'f', "force", &state, NULL,
>>>> + "force overwrite of existing files",
>>>> + PARSE_OPT_NOARG, parse_state_force_cb, 0 },
>>> I wonder if this could not be written as
>>>
>>> OPT_BOOLEAN('f', "force", &state.force,
>>> "force overwrite of existing files"),
>> I did it that way because 'force' is a bitfield.
>
> I thought there is an OPT_BIT?
OPT_BIT is for flags and bitmasks, not for bitfields.
Since you can't get the address of a bitfield member, a function that
wants to change its value needs to know its name. Switching to bitmasks
would make the option parsing code look cleaner, but you'd have to
change all those bitfield accesses to explicit bitmask operations, e.g.:
if (state.force)
state.force = 0;
vs.
if (state.flags & CHECKOUT_FORCE)
state.flags &= ~CHECKOUT_FORCE;
In the case of struct checkout, though, we could simply make the
bitfield members full ints, because there are only a few instances of
this structure in memory at any given time. Wasting a few bytes of RAM
in order to gain much simpler code is OK in this case, I think.
OPT_BOOLEAN looks a lot nicer than a callback.
René
^ permalink raw reply
* Re: sparse fetch, was Re: [PATCH 08/12] git-clone: support --path to do sparse clone
From: Sverre Rabbelier @ 2008-07-25 8:54 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, Johannes Schindelin,
Nguyễn Thái Ngọc Duy, git
In-Reply-To: <7v7ibauz94.fsf@gitster.siamese.dyndns.org>
On Fri, Jul 25, 2008 at 10:47, Junio C Hamano <gitster@pobox.com> wrote:
> For many projects that has src/ and doc/ (git.git being one of them), it
We are? That's great, that'd mean I can actually do a 'ls' in the
git.git root! Oh wait...
$ ls | wc -l
693
--
Cheers,
Sverre Rabbelier
^ 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