* Re: log and diff family: honor config even from subdirectories @ 2006-07-29 1:17 Linus Torvalds 2006-07-29 1:54 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: Linus Torvalds @ 2006-07-29 1:17 UTC (permalink / raw) To: Junio C Hamano, Git Mailing List Junio, I think your patch is fine as a band-aid, but I wonder if we shouldn't just move the "setup_git_directory()" call out of init_revisions(), and pass it as an argument to init_revision(). Some of the callers have already done setup_git_directory() earlier for their own reasons anyway. And from a quick look it looks like you missed the same bug happening in cmd_format_patch(), which calls git_config(git_format_config) before having done the setup. I'd actually _like_ to do the setup unconditionally inside the git wrapper (early - to make sure that we don't have this bug), but some things (at least "init-db", "clone" and "ls-remote") are obviously not supposed to do it, since they work outside of a git directory. So either we need to do it in each builtin command separately, or we'd need to add a flag in the command descriptor array. Any clever ideas? Linus ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: log and diff family: honor config even from subdirectories 2006-07-29 1:17 log and diff family: honor config even from subdirectories Linus Torvalds @ 2006-07-29 1:54 ` Junio C Hamano 2006-07-29 4:21 ` Call setup_git_directory() early Linus Torvalds 0 siblings, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2006-07-29 1:54 UTC (permalink / raw) To: Linus Torvalds; +Cc: git Linus Torvalds <torvalds@osdl.org> writes: > I'd actually _like_ to do the setup unconditionally inside the git wrapper > (early - to make sure that we don't have this bug), but some things (at > least "init-db", "clone" and "ls-remote") are obviously not supposed to do > it, since they work outside of a git directory. So either we need to do it > in each builtin command separately, or we'd need to add a flag in the > command descriptor array. > > Any clever ideas? > > Linus No clever ideas, but I agree it would be _very_ nice if we could do the setup unconditionally and early. Some commands that call setup want to know prefix, so we would need to introduce a global to hold the prefix for them. I do not do this myself, but it is conceivable that you might want to be able to set GIT_DIR to point at somewhere outside your working tree hierarchy, _and_ still work in a subdirectory. The current setup does not allow you to do that; we could introduce GIT_PROJECT_ROOT environment variable, and when GIT_PROJECT_ROOT exists difference between getcwd() and the project root could become the value of prefix. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Call setup_git_directory() early 2006-07-29 1:54 ` Junio C Hamano @ 2006-07-29 4:21 ` Linus Torvalds 2006-07-29 4:41 ` An alternative: call " Junio C Hamano 2006-07-29 5:06 ` Call " Junio C Hamano 0 siblings, 2 replies; 8+ messages in thread From: Linus Torvalds @ 2006-07-29 4:21 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List Any git command that expects to work in a subdirectory of a project, and that reads the git config files (which is just about all of them) needs to make sure that it does the "setup_git_directory()" call before it tries to read the config file. This means, among other things, that we need to move the call out of "init_revisions()", and into the caller. This does the mostly trivial conversion to do that. Signed-off-by: Linus Torvalds <torvalds@osdl.org> ---- On Fri, 28 Jul 2006, Junio C Hamano wrote: > > No clever ideas, but I agree it would be _very_ nice if we could > do the setup unconditionally and early. Some commands that call > setup want to know prefix, so we would need to introduce a > global to hold the prefix for them. Ho humm. In the meantime, how about this? It fixes the places I noticed, but is mostly just a totally mindless conversion to move the call to "setup_git_directory()" into the caller. diff --git a/blame.c b/blame.c index b04b8f5..76712b5 100644 --- a/blame.c +++ b/blame.c @@ -834,7 +834,7 @@ int main(int argc, const char **argv) } - init_revisions(&rev); + init_revisions(&rev, setup_git_directory()); rev.remove_empty_trees = 1; rev.topo_order = 1; rev.prune_fn = simplify_commit; diff --git a/builtin-diff-files.c b/builtin-diff-files.c index 2e10118..ea2936a 100644 --- a/builtin-diff-files.c +++ b/builtin-diff-files.c @@ -18,7 +18,7 @@ int cmd_diff_files(int argc, const char struct rev_info rev; int silent = 0; - init_revisions(&rev); + init_revisions(&rev, setup_git_directory()); git_config(git_default_config); /* no "diff" UI options */ rev.abbrev = 0; diff --git a/builtin-diff-index.c b/builtin-diff-index.c index dc52c05..eeeee93 100644 --- a/builtin-diff-index.c +++ b/builtin-diff-index.c @@ -15,7 +15,7 @@ int cmd_diff_index(int argc, const char int cached = 0; int i; - init_revisions(&rev); + init_revisions(&rev, setup_git_directory()); git_config(git_default_config); /* no "diff" UI options */ rev.abbrev = 0; diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c index 8957b45..f8215ea 100644 --- a/builtin-diff-tree.c +++ b/builtin-diff-tree.c @@ -67,7 +67,7 @@ int cmd_diff_tree(int argc, const char * static struct rev_info *opt = &log_tree_opt; int read_stdin = 0; - init_revisions(opt); + init_revisions(opt, setup_git_directory()); git_config(git_default_config); /* no "diff" UI options */ nr_sha1 = 0; opt->abbrev = 0; diff --git a/builtin-diff.c b/builtin-diff.c index 7d5ad62..7965c2c 100644 --- a/builtin-diff.c +++ b/builtin-diff.c @@ -227,7 +227,7 @@ int cmd_diff(int argc, const char **argv struct rev_info rev; struct object_array_entry ent[100]; int ents = 0, blobs = 0, paths = 0; - const char *path = NULL; + const char *path = NULL, *prefix; struct blobinfo blob[2]; /* @@ -250,9 +250,9 @@ int cmd_diff(int argc, const char **argv * Other cases are errors. */ - init_revisions(&rev); + prefix = setup_git_directory(); git_config(git_diff_ui_config); - diff_setup(&rev.diffopt); + init_revisions(&rev, prefix); argc = setup_revisions(argc, argv, &rev, NULL); if (!rev.diffopt.output_format) { diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c index f20b27b..338f209 100644 --- a/builtin-fmt-merge-msg.c +++ b/builtin-fmt-merge-msg.c @@ -250,6 +250,7 @@ int cmd_fmt_merge_msg(int argc, char **a const char *sep = ""; unsigned char head_sha1[20]; const char *head, *current_branch; + const char *prefix = setup_git_directory(); git_config(fmt_merge_msg_config); @@ -342,7 +343,7 @@ int cmd_fmt_merge_msg(int argc, char **a struct rev_info rev; head = lookup_commit(head_sha1); - init_revisions(&rev); + init_revisions(&rev, prefix); rev.commit_format = CMIT_FMT_ONELINE; rev.ignore_merges = 1; rev.limited = 1; diff --git a/builtin-log.c b/builtin-log.c index 88c835a..52064cd 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -48,10 +48,10 @@ static int cmd_log_walk(struct rev_info int cmd_whatchanged(int argc, const char **argv, char **envp) { struct rev_info rev; + const char *prefix = setup_git_directory(); - init_revisions(&rev); git_config(git_diff_ui_config); - diff_setup(&rev.diffopt); + init_revisions(&rev, prefix); rev.diff = 1; rev.diffopt.recursive = 1; rev.simplify_history = 0; @@ -64,10 +64,10 @@ int cmd_whatchanged(int argc, const char int cmd_show(int argc, const char **argv, char **envp) { struct rev_info rev; + const char *prefix = setup_git_directory(); - init_revisions(&rev); git_config(git_diff_ui_config); - diff_setup(&rev.diffopt); + init_revisions(&rev, prefix); rev.diff = 1; rev.diffopt.recursive = 1; rev.combine_merges = 1; @@ -82,10 +82,10 @@ int cmd_show(int argc, const char **argv int cmd_log(int argc, const char **argv, char **envp) { struct rev_info rev; + const char *prefix = setup_git_directory(); - init_revisions(&rev); git_config(git_diff_ui_config); - diff_setup(&rev.diffopt); + init_revisions(&rev, prefix); rev.always_show_header = 1; cmd_log_init(argc, argv, envp, &rev); return cmd_log_walk(&rev); @@ -188,6 +188,7 @@ static void get_patch_ids(struct rev_inf struct object *o1, *o2; unsigned flags1, flags2; unsigned char sha1[20]; + const char *prefix = setup_git_directory(); if (rev->pending.nr != 2) die("Need exactly one range."); @@ -206,7 +207,7 @@ static void get_patch_ids(struct rev_inf die("diff_setup_done failed"); /* given a range a..b get all patch ids for b..a */ - init_revisions(&check_rev); + init_revisions(&check_rev, prefix); o1->flags ^= UNINTERESTING; o2->flags ^= UNINTERESTING; add_pending_object(&check_rev, o1, "o1"); @@ -260,9 +261,10 @@ int cmd_format_patch(int argc, const cha char *add_signoff = NULL; char message_id[1024]; char ref_message_id[1024]; + const char *prefix = setup_git_directory(); git_config(git_format_config); - init_revisions(&rev); + init_revisions(&rev, prefix); rev.commit_format = CMIT_FMT_EMAIL; rev.verbose_header = 1; rev.diff = 1; diff --git a/builtin-prune.c b/builtin-prune.c index d196c41..4ed1e1b 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -234,7 +234,7 @@ int cmd_prune(int argc, const char **arg * Set up revision parsing, and mark us as being interested * in all object types, not just commits. */ - init_revisions(&revs); + init_revisions(&revs, setup_git_directory()); revs.tag_objects = 1; revs.blob_objects = 1; revs.tree_objects = 1; diff --git a/builtin-rev-list.c b/builtin-rev-list.c index 8f32871..2b6691c 100644 --- a/builtin-rev-list.c +++ b/builtin-rev-list.c @@ -311,7 +311,7 @@ int cmd_rev_list(int argc, const char ** struct commit_list *list; int i; - init_revisions(&revs); + init_revisions(&revs, setup_git_directory()); revs.abbrev = 0; revs.commit_format = CMIT_FMT_UNSPECIFIED; argc = setup_revisions(argc, argv, &revs, NULL); diff --git a/http-push.c b/http-push.c index 4768619..4021e7d 100644 --- a/http-push.c +++ b/http-push.c @@ -2521,7 +2521,7 @@ int main(int argc, char **argv) commit_argv[3] = old_sha1_hex; commit_argc++; } - init_revisions(&revs); + init_revisions(&revs, setup_git_directory()); setup_revisions(commit_argc, commit_argv, &revs, NULL); free(new_sha1_hex); if (old_sha1_hex) { diff --git a/revision.c b/revision.c index 874e349..a58257a 100644 --- a/revision.c +++ b/revision.c @@ -509,7 +509,7 @@ static int add_parents_only(struct rev_i return 1; } -void init_revisions(struct rev_info *revs) +void init_revisions(struct rev_info *revs, const char *prefix) { memset(revs, 0, sizeof(*revs)); @@ -521,7 +521,7 @@ void init_revisions(struct rev_info *rev revs->pruning.change = file_change; revs->lifo = 1; revs->dense = 1; - revs->prefix = setup_git_directory(); + revs->prefix = prefix; revs->max_age = -1; revs->min_age = -1; revs->max_count = -1; diff --git a/revision.h b/revision.h index e23ec8f..0c3b8d9 100644 --- a/revision.h +++ b/revision.h @@ -87,7 +87,7 @@ #define REV_TREE_DIFFERENT 2 extern int rev_same_tree_as_empty(struct rev_info *, struct tree *t1); extern int rev_compare_tree(struct rev_info *, struct tree *t1, struct tree *t2); -extern void init_revisions(struct rev_info *revs); +extern void init_revisions(struct rev_info *revs, const char *prefix); extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def); extern void prepare_revision_walk(struct rev_info *revs); extern struct commit *get_revision(struct rev_info *revs); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* An alternative: call setup_git_directory() early 2006-07-29 4:21 ` Call setup_git_directory() early Linus Torvalds @ 2006-07-29 4:41 ` Junio C Hamano 2006-07-29 5:06 ` Call " Junio C Hamano 1 sibling, 0 replies; 8+ messages in thread From: Junio C Hamano @ 2006-07-29 4:41 UTC (permalink / raw) To: Linus Torvalds; +Cc: git We classify built-in commands into three classes: ones that require to know where the .git directory is, ones that could use the knowledge of where the .git directory is but can operate without, and ones that should not attempt any .git discovery. Most of the commands fall within the first category. apply, mailsplit and mailinfo falls within the second category (e.g. patches can be applied to non-git files without --index), and the last category includes init-db (which should never do a discovery). Signed-off-by: Junio C Hamano <junkio@cox.net> --- > On Fri, 28 Jul 2006, Junio C Hamano wrote: >> >> No clever ideas, but I agree it would be _very_ nice if we could >> do the setup unconditionally and early. Some commands that call >> setup want to know prefix, so we would need to introduce a >> global to hold the prefix for them. > > Ho humm. In the meantime, how about this? It fixes the places I noticed, > but is mostly just a totally mindless conversion to move the call to > "setup_git_directory()" into the caller. I guess our mails crossed. This seems to pass all the tests but our test do not cover subdirectory operation that much. diff --git a/builtin-add.c b/builtin-add.c index 3a73a17..07a29bc 100644 --- a/builtin-add.c +++ b/builtin-add.c @@ -127,7 +127,7 @@ int cmd_add(int argc, const char **argv, { int i, newfd; int verbose = 0, show_only = 0; - const char *prefix = setup_git_directory(); + const char *prefix = git_prefix; const char **pathspec; struct dir_struct dir; diff --git a/builtin-apply.c b/builtin-apply.c index d4381d9..fb06cc3 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -2370,7 +2370,7 @@ int cmd_apply(int argc, const char **arg } if (check_index && prefix_length < 0) { - prefix = setup_git_directory(); + prefix = git_prefix; prefix_length = prefix ? strlen(prefix) : 0; git_config(git_apply_config); if (!whitespace_option && apply_default_whitespace) diff --git a/builtin-cat-file.c b/builtin-cat-file.c index 4d36817..53b15ef 100644 --- a/builtin-cat-file.c +++ b/builtin-cat-file.c @@ -102,7 +102,6 @@ int cmd_cat_file(int argc, const char ** unsigned long size; int opt; - setup_git_directory(); git_config(git_default_config); if (argc != 3) usage("git-cat-file [-t|-s|-e|-p|<type>] <sha1>"); diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c index ec082bf..cae544b 100644 --- a/builtin-commit-tree.c +++ b/builtin-commit-tree.c @@ -88,8 +88,6 @@ int cmd_commit_tree(int argc, const char unsigned int size; setup_ident(); - setup_git_directory(); - git_config(git_default_config); if (argc < 2) diff --git a/builtin-diff-stages.c b/builtin-diff-stages.c index 9c62702..9630a37 100644 --- a/builtin-diff-stages.c +++ b/builtin-diff-stages.c @@ -58,7 +58,7 @@ static void diff_stages(int stage1, int int cmd_diff_stages(int ac, const char **av, char **envp) { int stage1, stage2; - const char *prefix = setup_git_directory(); + const char *prefix = git_prefix; const char **pathspec = NULL; git_config(git_default_config); /* no "diff" UI options */ diff --git a/builtin-grep.c b/builtin-grep.c index a79bac3..713cca1 100644 --- a/builtin-grep.c +++ b/builtin-grep.c @@ -926,7 +926,7 @@ int cmd_grep(int argc, const char **argv int seen_dashdash = 0; struct grep_opt opt; struct object_array list = { 0, 0, NULL }; - const char *prefix = setup_git_directory(); + const char *prefix = git_prefix; const char **paths = NULL; int i; diff --git a/builtin-ls-files.c b/builtin-ls-files.c index 8dae9f7..49e848e 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -329,7 +329,7 @@ int cmd_ls_files(int argc, const char ** struct dir_struct dir; memset(&dir, 0, sizeof(dir)); - prefix = setup_git_directory(); + prefix = git_prefix; if (prefix) prefix_offset = strlen(prefix); git_config(git_default_config); diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c index b8d0d88..5cd4964 100644 --- a/builtin-ls-tree.c +++ b/builtin-ls-tree.c @@ -90,7 +90,7 @@ int cmd_ls_tree(int argc, const char **a unsigned char sha1[20]; struct tree *tree; - prefix = setup_git_directory(); + prefix = git_prefix; git_config(git_default_config); if (prefix && *prefix) chomp_prefix = strlen(prefix); diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 122b6f1..375347d 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -882,7 +882,6 @@ int cmd_read_tree(int argc, const char * state.quiet = 1; state.refresh_cache = 1; - setup_git_directory(); git_config(git_default_config); newfd = hold_lock_file_for_update(&lock_file, get_index_file()); diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c index b3e4386..cca9b05 100644 --- a/builtin-rev-parse.c +++ b/builtin-rev-parse.c @@ -213,7 +213,7 @@ int cmd_rev_parse(int argc, const char * { int i, as_is = 0, verify = 0; unsigned char sha1[20]; - const char *prefix = setup_git_directory(); + const char *prefix = git_prefix; git_config(git_default_config); diff --git a/builtin-rm.c b/builtin-rm.c index bb810ba..25d0da3 100644 --- a/builtin-rm.c +++ b/builtin-rm.c @@ -47,7 +47,7 @@ int cmd_rm(int argc, const char **argv, { int i, newfd; int verbose = 0, show_only = 0, force = 0; - const char *prefix = setup_git_directory(); + const char *prefix = git_prefix; const char **pathspec; char *seen; diff --git a/builtin-show-branch.c b/builtin-show-branch.c index 82f75b7..43ab1f8 100644 --- a/builtin-show-branch.c +++ b/builtin-show-branch.c @@ -573,7 +573,6 @@ int cmd_show_branch(int ac, const char * int topics = 0; int dense = 1; - setup_git_directory(); git_config(git_show_branch_config); /* If nothing is specified, try the default first */ diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c index e5aaded..3e6b736 100644 --- a/builtin-tar-tree.c +++ b/builtin-tar-tree.c @@ -319,7 +319,6 @@ static int generate_tar(int argc, const current_path.alloc = PATH_MAX; current_path.len = current_path.eof = 0; - setup_git_directory(); git_config(git_tar_config); switch (argc) { diff --git a/builtin-update-index.c b/builtin-update-index.c index 1a4200d..c64506b 100644 --- a/builtin-update-index.c +++ b/builtin-update-index.c @@ -481,7 +481,7 @@ int cmd_update_index(int argc, const cha int i, newfd, entries, has_errors = 0, line_termination = '\n'; int allow_options = 1; int read_from_stdin = 0; - const char *prefix = setup_git_directory(); + const char *prefix = git_prefix; int prefix_length = prefix ? strlen(prefix) : 0; char set_executable_bit = 0; unsigned int refresh_flags = 0; diff --git a/builtin-update-ref.c b/builtin-update-ref.c index 83094ab..8138eba 100644 --- a/builtin-update-ref.c +++ b/builtin-update-ref.c @@ -13,7 +13,6 @@ int cmd_update_ref(int argc, const char int i; setup_ident(); - setup_git_directory(); git_config(git_default_config); for (i = 1; i < argc; i++) { diff --git a/builtin-write-tree.c b/builtin-write-tree.c index 449a4d1..d987d0b 100644 --- a/builtin-write-tree.c +++ b/builtin-write-tree.c @@ -66,8 +66,6 @@ int cmd_write_tree(int argc, const char const char *prefix = NULL; unsigned char sha1[20]; - setup_git_directory(); - while (1 < argc) { const char *arg = argv[1]; if (!strcmp(arg, "--missing-ok")) diff --git a/cache.h b/cache.h index eee5fc9..6c008f9 100644 --- a/cache.h +++ b/cache.h @@ -130,6 +130,8 @@ extern char *get_graft_file(void); #define ALTERNATE_DB_ENVIRONMENT "GIT_ALTERNATE_OBJECT_DIRECTORIES" +extern int gitdir_not_found; +extern const char *git_prefix; extern const char **get_pathspec(const char *prefix, const char **pathspec); extern const char *setup_git_directory_gently(int *); extern const char *setup_git_directory(void); diff --git a/environment.c b/environment.c index 42f39d6..2fb978d 100644 --- a/environment.c +++ b/environment.c @@ -23,6 +23,8 @@ int shared_repository = PERM_UMASK; const char *apply_default_whitespace = NULL; int zlib_compression_level = Z_DEFAULT_COMPRESSION; int pager_in_use; +int gitdir_not_found; +const char *git_prefix; static char *git_dir, *git_object_dir, *git_index_file, *git_refs_dir, *git_graft_file; diff --git a/git.c b/git.c index 885e1ce..a9d6340 100644 --- a/git.c +++ b/git.c @@ -220,45 +220,50 @@ static void handle_internal_command(int static struct cmd_struct { const char *cmd; int (*fn)(int, const char **, char **); + enum use_gitdir { + GITDIR_OPTIONAL, + GITDIR_REQUIRED, + GITDIR_DONTCARE, + } use_gitdir; } commands[] = { - { "version", cmd_version }, - { "help", cmd_help }, - { "log", cmd_log }, - { "whatchanged", cmd_whatchanged }, - { "show", cmd_show }, - { "push", cmd_push }, - { "format-patch", cmd_format_patch }, - { "count-objects", cmd_count_objects }, - { "diff", cmd_diff }, - { "grep", cmd_grep }, - { "rm", cmd_rm }, - { "add", cmd_add }, - { "rev-list", cmd_rev_list }, - { "init-db", cmd_init_db }, - { "get-tar-commit-id", cmd_get_tar_commit_id }, - { "upload-tar", cmd_upload_tar }, - { "check-ref-format", cmd_check_ref_format }, - { "ls-files", cmd_ls_files }, - { "ls-tree", cmd_ls_tree }, - { "tar-tree", cmd_tar_tree }, - { "read-tree", cmd_read_tree }, - { "commit-tree", cmd_commit_tree }, - { "apply", cmd_apply }, - { "show-branch", cmd_show_branch }, - { "diff-files", cmd_diff_files }, - { "diff-index", cmd_diff_index }, - { "diff-stages", cmd_diff_stages }, - { "diff-tree", cmd_diff_tree }, - { "cat-file", cmd_cat_file }, - { "rev-parse", cmd_rev_parse }, - { "write-tree", cmd_write_tree }, - { "mailsplit", cmd_mailsplit }, - { "mailinfo", cmd_mailinfo }, - { "stripspace", cmd_stripspace }, - { "update-index", cmd_update_index }, - { "update-ref", cmd_update_ref }, - { "fmt-merge-msg", cmd_fmt_merge_msg }, - { "prune", cmd_prune }, + { "version", cmd_version, GITDIR_DONTCARE }, + { "help", cmd_help, GITDIR_DONTCARE }, + { "log", cmd_log, GITDIR_REQUIRED }, + { "whatchanged", cmd_whatchanged, GITDIR_REQUIRED }, + { "show", cmd_show, GITDIR_REQUIRED }, + { "push", cmd_push, GITDIR_REQUIRED }, + { "format-patch", cmd_format_patch, GITDIR_REQUIRED }, + { "count-objects", cmd_count_objects, GITDIR_REQUIRED }, + { "diff", cmd_diff, GITDIR_REQUIRED }, + { "grep", cmd_grep, GITDIR_REQUIRED }, + { "rm", cmd_rm, GITDIR_REQUIRED }, + { "add", cmd_add, GITDIR_REQUIRED }, + { "rev-list", cmd_rev_list, GITDIR_REQUIRED }, + { "init-db", cmd_init_db, GITDIR_DONTCARE }, + { "get-tar-commit-id", cmd_get_tar_commit_id, GITDIR_REQUIRED }, + { "upload-tar", cmd_upload_tar, GITDIR_REQUIRED }, + { "check-ref-format", cmd_check_ref_format, GITDIR_DONTCARE }, + { "ls-files", cmd_ls_files, GITDIR_REQUIRED }, + { "ls-tree", cmd_ls_tree, GITDIR_REQUIRED }, + { "tar-tree", cmd_tar_tree, GITDIR_REQUIRED }, + { "read-tree", cmd_read_tree, GITDIR_REQUIRED }, + { "commit-tree", cmd_commit_tree, GITDIR_REQUIRED }, + { "apply", cmd_apply, GITDIR_OPTIONAL }, + { "show-branch", cmd_show_branch, GITDIR_REQUIRED }, + { "diff-files", cmd_diff_files, GITDIR_REQUIRED }, + { "diff-index", cmd_diff_index, GITDIR_REQUIRED }, + { "diff-stages", cmd_diff_stages, GITDIR_REQUIRED }, + { "diff-tree", cmd_diff_tree, GITDIR_REQUIRED }, + { "cat-file", cmd_cat_file, GITDIR_REQUIRED }, + { "rev-parse", cmd_rev_parse, GITDIR_REQUIRED }, + { "write-tree", cmd_write_tree, GITDIR_REQUIRED }, + { "mailsplit", cmd_mailsplit, GITDIR_OPTIONAL }, + { "mailinfo", cmd_mailinfo, GITDIR_OPTIONAL }, + { "stripspace", cmd_stripspace, GITDIR_OPTIONAL }, + { "update-index", cmd_update_index, GITDIR_REQUIRED }, + { "update-ref", cmd_update_ref, GITDIR_REQUIRED }, + { "fmt-merge-msg", cmd_fmt_merge_msg, GITDIR_DONTCARE }, + { "prune", cmd_prune, GITDIR_REQUIRED }, }; int i; @@ -284,6 +289,18 @@ static void handle_internal_command(int fflush(stderr); } + gitdir_not_found = 0; + switch (p->use_gitdir) { + case GITDIR_REQUIRED: + git_prefix = setup_git_directory(); + break; + case GITDIR_OPTIONAL: + git_prefix = + setup_git_directory_gently(&gitdir_not_found); + break; + case GITDIR_DONTCARE: + break; + } exit(p->fn(argc, argv, envp)); } } diff --git a/t/trash/path0 b/t/trash/path0 deleted file mode 100644 index f87290f..0000000 --- a/t/trash/path0 +++ /dev/null @@ -1 +0,0 @@ -hello path0 diff --git a/t/trash/path0sym b/t/trash/path0sym deleted file mode 120000 index 15a9843..0000000 --- a/t/trash/path0sym +++ /dev/null @@ -1 +0,0 @@ -hello path0 \ No newline at end of file diff --git a/t/trash/path2/file2 b/t/trash/path2/file2 deleted file mode 100644 index 3feff94..0000000 --- a/t/trash/path2/file2 +++ /dev/null @@ -1 +0,0 @@ -hello path2/file2 diff --git a/t/trash/path2/file2sym b/t/trash/path2/file2sym deleted file mode 120000 index d8ce161..0000000 --- a/t/trash/path2/file2sym +++ /dev/null @@ -1 +0,0 @@ -hello path2/file2 \ No newline at end of file diff --git a/t/trash/path3/file3 b/t/trash/path3/file3 deleted file mode 100644 index 0aa34ca..0000000 --- a/t/trash/path3/file3 +++ /dev/null @@ -1 +0,0 @@ -hello path3/file3 diff --git a/t/trash/path3/file3sym b/t/trash/path3/file3sym deleted file mode 120000 index 8599103..0000000 --- a/t/trash/path3/file3sym +++ /dev/null @@ -1 +0,0 @@ -hello path3/file3 \ No newline at end of file diff --git a/t/trash/path3/subp3/file3 b/t/trash/path3/subp3/file3 deleted file mode 100644 index 00fb590..0000000 --- a/t/trash/path3/subp3/file3 +++ /dev/null @@ -1 +0,0 @@ -hello path3/subp3/file3 diff --git a/t/trash/path3/subp3/file3sym b/t/trash/path3/subp3/file3sym deleted file mode 120000 index 6649a1e..0000000 --- a/t/trash/path3/subp3/file3sym +++ /dev/null @@ -1 +0,0 @@ -hello path3/subp3/file3 \ No newline at end of file ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Call setup_git_directory() early 2006-07-29 4:21 ` Call setup_git_directory() early Linus Torvalds 2006-07-29 4:41 ` An alternative: call " Junio C Hamano @ 2006-07-29 5:06 ` Junio C Hamano 2006-07-29 5:44 ` Linus Torvalds 1 sibling, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2006-07-29 5:06 UTC (permalink / raw) To: Linus Torvalds; +Cc: git Sorry for the earlier crapoid. The diffs contained there for t/trash stuff were due to my earlier version that marked init-db to be GITDIR_OPTIONAL -- which caused the "optional" discovery to find the .git/ directory two levels up (relative to t/trash) and contaminating the primary index file by mistake (Yuck). Your patch makes a lot of sense. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Call setup_git_directory() early 2006-07-29 5:06 ` Call " Junio C Hamano @ 2006-07-29 5:44 ` Linus Torvalds 2006-07-29 9:12 ` Junio C Hamano 0 siblings, 1 reply; 8+ messages in thread From: Linus Torvalds @ 2006-07-29 5:44 UTC (permalink / raw) To: Junio C Hamano; +Cc: Git Mailing List On Fri, 28 Jul 2006, Junio C Hamano wrote: > > Your patch makes a lot of sense. I don't think they are mutually incompatible. My patch moves the call to "setup_git_directory()" a _bit_ earlier, and if we just keep doing it, we'll end up with at least a subset of your patch. Anyway, this was what I came up with when I did that "incremental" thing. It's a bit like your patch, but a lot more invasive because it changes the calling convention for the builtin commands (but it's cleaner because of it, I think). This is on top of my previous patch. What do you think? It does delete more lines than it adds.. Linus --- 35 files changed, 109 insertions(+), 114 deletions(-) diff --git a/builtin-add.c b/builtin-add.c index 3a73a17..0fa7dc1 100644 --- a/builtin-add.c +++ b/builtin-add.c @@ -123,11 +123,10 @@ static int add_file_to_index(const char static struct lock_file lock_file; -int cmd_add(int argc, const char **argv, char **envp) +int cmd_add(int argc, const char **argv, const char *prefix) { int i, newfd; int verbose = 0, show_only = 0; - const char *prefix = setup_git_directory(); const char **pathspec; struct dir_struct dir; diff --git a/builtin-apply.c b/builtin-apply.c index d924ac3..c1ba99e 100644 --- a/builtin-apply.c +++ b/builtin-apply.c @@ -2222,7 +2222,7 @@ static int git_apply_config(const char * } -int cmd_apply(int argc, const char **argv, char **envp) +int cmd_apply(int argc, const char **argv, const char *prefix) { int i; int read_stdin = 1; diff --git a/builtin-cat-file.c b/builtin-cat-file.c index 4d36817..d796431 100644 --- a/builtin-cat-file.c +++ b/builtin-cat-file.c @@ -94,7 +94,7 @@ static int pprint_tag(const unsigned cha return 0; } -int cmd_cat_file(int argc, const char **argv, char **envp) +int cmd_cat_file(int argc, const char **argv, const char *prefix) { unsigned char sha1[20]; char type[20]; diff --git a/builtin-check-ref-format.c b/builtin-check-ref-format.c index 4a23936..701de43 100644 --- a/builtin-check-ref-format.c +++ b/builtin-check-ref-format.c @@ -6,7 +6,7 @@ #include "cache.h" #include "refs.h" #include "builtin.h" -int cmd_check_ref_format(int argc, const char **argv, char **envp) +int cmd_check_ref_format(int argc, const char **argv, const char *prefix) { if (argc != 2) usage("git check-ref-format refname"); diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c index ec082bf..e6fb16d 100644 --- a/builtin-commit-tree.c +++ b/builtin-commit-tree.c @@ -77,7 +77,7 @@ static int new_parent(int idx) return 1; } -int cmd_commit_tree(int argc, const char **argv, char **envp) +int cmd_commit_tree(int argc, const char **argv, const char *prefix) { int i; int parents = 0; diff --git a/builtin-count.c b/builtin-count.c index 5ee72df..1d3729a 100644 --- a/builtin-count.c +++ b/builtin-count.c @@ -67,7 +67,7 @@ static void count_objects(DIR *d, char * } } -int cmd_count_objects(int ac, const char **av, char **ep) +int cmd_count_objects(int ac, const char **av, const char *prefix) { int i; int verbose = 0; diff --git a/builtin-diff-files.c b/builtin-diff-files.c index ea2936a..da51284 100644 --- a/builtin-diff-files.c +++ b/builtin-diff-files.c @@ -13,7 +13,7 @@ static const char diff_files_usage[] = "git-diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]" COMMON_DIFF_OPTIONS_HELP; -int cmd_diff_files(int argc, const char **argv, char **envp) +int cmd_diff_files(int argc, const char **argv, const char *prefix) { struct rev_info rev; int silent = 0; diff --git a/builtin-diff-index.c b/builtin-diff-index.c index eeeee93..bedb90c 100644 --- a/builtin-diff-index.c +++ b/builtin-diff-index.c @@ -9,7 +9,7 @@ static const char diff_cache_usage[] = "[<common diff options>] <tree-ish> [<path>...]" COMMON_DIFF_OPTIONS_HELP; -int cmd_diff_index(int argc, const char **argv, char **envp) +int cmd_diff_index(int argc, const char **argv, const char *prefix) { struct rev_info rev; int cached = 0; diff --git a/builtin-diff-stages.c b/builtin-diff-stages.c index 9c62702..5960e08 100644 --- a/builtin-diff-stages.c +++ b/builtin-diff-stages.c @@ -55,10 +55,9 @@ static void diff_stages(int stage1, int } } -int cmd_diff_stages(int ac, const char **av, char **envp) +int cmd_diff_stages(int ac, const char **av, const char *prefix) { int stage1, stage2; - const char *prefix = setup_git_directory(); const char **pathspec = NULL; git_config(git_default_config); /* no "diff" UI options */ diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c index f8215ea..816946b 100644 --- a/builtin-diff-tree.c +++ b/builtin-diff-tree.c @@ -59,7 +59,7 @@ static const char diff_tree_usage[] = " --root include the initial commit as diff against /dev/null\n" COMMON_DIFF_OPTIONS_HELP; -int cmd_diff_tree(int argc, const char **argv, char **envp) +int cmd_diff_tree(int argc, const char **argv, const char *prefix) { int nr_sha1; char line[1000]; diff --git a/builtin-diff.c b/builtin-diff.c index 7965c2c..4a592e6 100644 --- a/builtin-diff.c +++ b/builtin-diff.c @@ -221,13 +221,13 @@ void add_head(struct rev_info *revs) add_pending_object(revs, obj, "HEAD"); } -int cmd_diff(int argc, const char **argv, char **envp) +int cmd_diff(int argc, const char **argv, const char *prefix) { int i; struct rev_info rev; struct object_array_entry ent[100]; int ents = 0, blobs = 0, paths = 0; - const char *path = NULL, *prefix; + const char *path = NULL; struct blobinfo blob[2]; /* @@ -250,7 +250,6 @@ int cmd_diff(int argc, const char **argv * Other cases are errors. */ - prefix = setup_git_directory(); git_config(git_diff_ui_config); init_revisions(&rev, prefix); diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c index 338f209..c84224e 100644 --- a/builtin-fmt-merge-msg.c +++ b/builtin-fmt-merge-msg.c @@ -242,7 +242,7 @@ static void shortlog(const char *name, u free_list(&subjects); } -int cmd_fmt_merge_msg(int argc, char **argv, char **envp) +int cmd_fmt_merge_msg(int argc, char **argv, const char *prefix) { int limit = 20, i = 0; char line[1024]; @@ -250,7 +250,6 @@ int cmd_fmt_merge_msg(int argc, char **a const char *sep = ""; unsigned char head_sha1[20]; const char *head, *current_branch; - const char *prefix = setup_git_directory(); git_config(fmt_merge_msg_config); diff --git a/builtin-grep.c b/builtin-grep.c index a79bac3..69b7c48 100644 --- a/builtin-grep.c +++ b/builtin-grep.c @@ -919,14 +919,13 @@ static const char emsg_missing_context_l static const char emsg_missing_argument[] = "option requires an argument -%s"; -int cmd_grep(int argc, const char **argv, char **envp) +int cmd_grep(int argc, const char **argv, const char *prefix) { int hit = 0; int cached = 0; int seen_dashdash = 0; struct grep_opt opt; struct object_array list = { 0, 0, NULL }; - const char *prefix = setup_git_directory(); const char **paths = NULL; int i; diff --git a/builtin-help.c b/builtin-help.c index 335fe5f..7f7701e 100644 --- a/builtin-help.c +++ b/builtin-help.c @@ -221,13 +221,13 @@ static void show_man_page(const char *gi execlp("man", "man", page, NULL); } -int cmd_version(int argc, const char **argv, char **envp) +int cmd_version(int argc, const char **argv, const char *prefix) { printf("git version %s\n", git_version_string); return 0; } -int cmd_help(int argc, const char **argv, char **envp) +int cmd_help(int argc, const char **argv, const char *prefix) { const char *help_cmd = argv[1]; if (!help_cmd) diff --git a/builtin-init-db.c b/builtin-init-db.c index 7fdd2fa..52473ed 100644 --- a/builtin-init-db.c +++ b/builtin-init-db.c @@ -250,7 +250,7 @@ static const char init_db_usage[] = * On the other hand, it might just make lookup slower and messier. You * be the judge. The default case is to have one DB per managed directory. */ -int cmd_init_db(int argc, const char **argv, char **envp) +int cmd_init_db(int argc, const char **argv, const char *prefix) { const char *git_dir; const char *sha1_dir; diff --git a/builtin-log.c b/builtin-log.c index 52064cd..82c69d1 100644 --- a/builtin-log.c +++ b/builtin-log.c @@ -16,7 +16,7 @@ #include <sys/time.h> /* this is in builtin-diff.c */ void add_head(struct rev_info *revs); -static void cmd_log_init(int argc, const char **argv, char **envp, +static void cmd_log_init(int argc, const char **argv, const char *prefix, struct rev_info *rev) { rev->abbrev = DEFAULT_ABBREV; @@ -45,26 +45,24 @@ static int cmd_log_walk(struct rev_info return 0; } -int cmd_whatchanged(int argc, const char **argv, char **envp) +int cmd_whatchanged(int argc, const char **argv, const char *prefix) { struct rev_info rev; - const char *prefix = setup_git_directory(); git_config(git_diff_ui_config); init_revisions(&rev, prefix); rev.diff = 1; rev.diffopt.recursive = 1; rev.simplify_history = 0; - cmd_log_init(argc, argv, envp, &rev); + cmd_log_init(argc, argv, prefix, &rev); if (!rev.diffopt.output_format) rev.diffopt.output_format = DIFF_FORMAT_RAW; return cmd_log_walk(&rev); } -int cmd_show(int argc, const char **argv, char **envp) +int cmd_show(int argc, const char **argv, const char *prefix) { struct rev_info rev; - const char *prefix = setup_git_directory(); git_config(git_diff_ui_config); init_revisions(&rev, prefix); @@ -75,19 +73,18 @@ int cmd_show(int argc, const char **argv rev.always_show_header = 1; rev.ignore_merges = 0; rev.no_walk = 1; - cmd_log_init(argc, argv, envp, &rev); + cmd_log_init(argc, argv, prefix, &rev); return cmd_log_walk(&rev); } -int cmd_log(int argc, const char **argv, char **envp) +int cmd_log(int argc, const char **argv, const char *prefix) { struct rev_info rev; - const char *prefix = setup_git_directory(); git_config(git_diff_ui_config); init_revisions(&rev, prefix); rev.always_show_header = 1; - cmd_log_init(argc, argv, envp, &rev); + cmd_log_init(argc, argv, prefix, &rev); return cmd_log_walk(&rev); } @@ -181,14 +178,13 @@ static int get_patch_id(struct commit *c return diff_flush_patch_id(options, sha1); } -static void get_patch_ids(struct rev_info *rev, struct diff_options *options) +static void get_patch_ids(struct rev_info *rev, struct diff_options *options, const char *prefix) { struct rev_info check_rev; struct commit *commit; struct object *o1, *o2; unsigned flags1, flags2; unsigned char sha1[20]; - const char *prefix = setup_git_directory(); if (rev->pending.nr != 2) die("Need exactly one range."); @@ -244,7 +240,7 @@ static void gen_message_id(char *dest, u (int)(email_end - email_start - 1), email_start + 1); } -int cmd_format_patch(int argc, const char **argv, char **envp) +int cmd_format_patch(int argc, const char **argv, const char *prefix) { struct commit *commit; struct commit **list = NULL; @@ -261,7 +257,6 @@ int cmd_format_patch(int argc, const cha char *add_signoff = NULL; char message_id[1024]; char ref_message_id[1024]; - const char *prefix = setup_git_directory(); git_config(git_format_config); init_revisions(&rev, prefix); @@ -368,7 +363,7 @@ int cmd_format_patch(int argc, const cha } if (ignore_if_in_upstream) - get_patch_ids(&rev, &patch_id_opts); + get_patch_ids(&rev, &patch_id_opts, prefix); if (!use_stdout) realstdout = fdopen(dup(1), "w"); diff --git a/builtin-ls-files.c b/builtin-ls-files.c index 8dae9f7..e906c81 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -322,7 +322,7 @@ static const char ls_files_usage[] = "[ --exclude-per-directory=<filename> ] [--full-name] [--abbrev] " "[--] [<file>]*"; -int cmd_ls_files(int argc, const char **argv, char** envp) +int cmd_ls_files(int argc, const char **argv, const char *prefix) { int i; int exc_given = 0; diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c index b8d0d88..d31efe9 100644 --- a/builtin-ls-tree.c +++ b/builtin-ls-tree.c @@ -18,7 +18,7 @@ static int abbrev = 0; static int ls_options = 0; static const char **pathspec; static int chomp_prefix = 0; -static const char *prefix; +static const char *ls_tree_prefix; static const char ls_tree_usage[] = "git-ls-tree [-d] [-r] [-t] [-z] [--name-only] [--name-status] [--full-name] [--abbrev[=<n>]] <tree-ish> [path...]"; @@ -71,7 +71,7 @@ static int show_tree(const unsigned char return 0; if (chomp_prefix && - (baselen < chomp_prefix || memcmp(prefix, base, chomp_prefix))) + (baselen < chomp_prefix || memcmp(ls_tree_prefix, base, chomp_prefix))) return 0; if (!(ls_options & LS_NAME_ONLY)) @@ -85,13 +85,14 @@ static int show_tree(const unsigned char return retval; } -int cmd_ls_tree(int argc, const char **argv, char **envp) +int cmd_ls_tree(int argc, const char **argv, const char *prefix) { unsigned char sha1[20]; struct tree *tree; prefix = setup_git_directory(); git_config(git_default_config); + ls_tree_prefix = prefix; if (prefix && *prefix) chomp_prefix = strlen(prefix); while (1 < argc && argv[1][0] == '-') { diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c index 05dc1bf..24a4fc6 100644 --- a/builtin-mailinfo.c +++ b/builtin-mailinfo.c @@ -836,7 +836,7 @@ int mailinfo(FILE *in, FILE *out, int ks static const char mailinfo_usage[] = "git-mailinfo [-k] [-u | --encoding=<encoding>] msg patch <mail >info"; -int cmd_mailinfo(int argc, const char **argv, char **envp) +int cmd_mailinfo(int argc, const char **argv, const char *prefix) { /* NEEDSWORK: might want to do the optional .git/ directory * discovery diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c index e2a0058..91a699d 100644 --- a/builtin-mailsplit.c +++ b/builtin-mailsplit.c @@ -138,7 +138,7 @@ out: free(name); return ret; } -int cmd_mailsplit(int argc, const char **argv, char **envp) +int cmd_mailsplit(int argc, const char **argv, const char *prefix) { int nr = 0, nr_prec = 4, ret; int allow_bare = 0; diff --git a/builtin-prune.c b/builtin-prune.c index 4ed1e1b..15bb650 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -217,7 +217,7 @@ static void add_cache_refs(void) add_cache_tree(active_cache_tree); } -int cmd_prune(int argc, const char **argv, char **envp) +int cmd_prune(int argc, const char **argv, const char *prefix) { int i; diff --git a/builtin-push.c b/builtin-push.c index 31cbfd7..a824171 100644 --- a/builtin-push.c +++ b/builtin-push.c @@ -270,7 +270,7 @@ static int do_push(const char *repo) return 0; } -int cmd_push(int argc, const char **argv, char **envp) +int cmd_push(int argc, const char **argv, const char *prefix) { int i; const char *repo = "origin"; /* default repository */ diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 122b6f1..25ca69c 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -870,7 +870,7 @@ static const char read_tree_usage[] = "g static struct lock_file lock_file; -int cmd_read_tree(int argc, const char **argv, char **envp) +int cmd_read_tree(int argc, const char **argv, const char *prefix) { int i, newfd, stage = 0; unsigned char sha1[20]; diff --git a/builtin-rev-list.c b/builtin-rev-list.c index 2b6691c..65b4d0c 100644 --- a/builtin-rev-list.c +++ b/builtin-rev-list.c @@ -306,7 +306,7 @@ static void mark_edges_uninteresting(str } } -int cmd_rev_list(int argc, const char **argv, char **envp) +int cmd_rev_list(int argc, const char **argv, const char *prefix) { struct commit_list *list; int i; diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c index b3e4386..aca4a36 100644 --- a/builtin-rev-parse.c +++ b/builtin-rev-parse.c @@ -209,11 +209,10 @@ static int try_difference(const char *ar return 0; } -int cmd_rev_parse(int argc, const char **argv, char **envp) +int cmd_rev_parse(int argc, const char **argv, const char *prefix) { int i, as_is = 0, verify = 0; unsigned char sha1[20]; - const char *prefix = setup_git_directory(); git_config(git_default_config); diff --git a/builtin-rm.c b/builtin-rm.c index bb810ba..92d205a 100644 --- a/builtin-rm.c +++ b/builtin-rm.c @@ -43,11 +43,10 @@ static int remove_file(const char *name) static struct lock_file lock_file; -int cmd_rm(int argc, const char **argv, char **envp) +int cmd_rm(int argc, const char **argv, const char *prefix) { int i, newfd; int verbose = 0, show_only = 0, force = 0; - const char *prefix = setup_git_directory(); const char **pathspec; char *seen; diff --git a/builtin-show-branch.c b/builtin-show-branch.c index 82f75b7..9c7f9f6 100644 --- a/builtin-show-branch.c +++ b/builtin-show-branch.c @@ -550,7 +550,7 @@ static int omit_in_dense(struct commit * return 0; } -int cmd_show_branch(int ac, const char **av, char **envp) +int cmd_show_branch(int ac, const char **av, const char *prefix) { struct commit *rev[MAX_REVS], *commit; struct commit_list *list = NULL, *seen = NULL; diff --git a/builtin-stripspace.c b/builtin-stripspace.c index 2ce1264..09cc910 100644 --- a/builtin-stripspace.c +++ b/builtin-stripspace.c @@ -54,7 +54,7 @@ void stripspace(FILE *in, FILE *out) fputc('\n', out); } -int cmd_stripspace(int argc, const char **argv, char **envp) +int cmd_stripspace(int argc, const char **argv, const char *prefix) { stripspace(stdin, stdout); return 0; diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c index e5aaded..637d22e 100644 --- a/builtin-tar-tree.c +++ b/builtin-tar-tree.c @@ -308,7 +308,7 @@ int git_tar_config(const char *var, cons return git_default_config(var, value); } -static int generate_tar(int argc, const char **argv, char** envp) +static int generate_tar(int argc, const char **argv, const char *prefix) { unsigned char sha1[20], tree_sha1[20]; struct commit *commit; @@ -402,19 +402,19 @@ static int remote_tar(int argc, const ch return !!ret; } -int cmd_tar_tree(int argc, const char **argv, char **envp) +int cmd_tar_tree(int argc, const char **argv, const char *prefix) { if (argc < 2) usage(tar_tree_usage); if (!strncmp("--remote=", argv[1], 9)) return remote_tar(argc, argv); - return generate_tar(argc, argv, envp); + return generate_tar(argc, argv, prefix); } /* ustar header + extended global header content */ #define HEADERSIZE (2 * RECORDSIZE) -int cmd_get_tar_commit_id(int argc, const char **argv, char **envp) +int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix) { char buffer[HEADERSIZE]; struct ustar_header *header = (struct ustar_header *)buffer; diff --git a/builtin-update-index.c b/builtin-update-index.c index 1a4200d..24dca47 100644 --- a/builtin-update-index.c +++ b/builtin-update-index.c @@ -476,12 +476,11 @@ static int do_reupdate(int ac, const cha return 0; } -int cmd_update_index(int argc, const char **argv, char **envp) +int cmd_update_index(int argc, const char **argv, const char *prefix) { int i, newfd, entries, has_errors = 0, line_termination = '\n'; int allow_options = 1; int read_from_stdin = 0; - const char *prefix = setup_git_directory(); int prefix_length = prefix ? strlen(prefix) : 0; char set_executable_bit = 0; unsigned int refresh_flags = 0; diff --git a/builtin-update-ref.c b/builtin-update-ref.c index 83094ab..acdf39a 100644 --- a/builtin-update-ref.c +++ b/builtin-update-ref.c @@ -5,7 +5,7 @@ #include "builtin.h" static const char git_update_ref_usage[] = "git-update-ref <refname> <value> [<oldval>] [-m <reason>]"; -int cmd_update_ref(int argc, const char **argv, char **envp) +int cmd_update_ref(int argc, const char **argv, const char *prefix) { const char *refname=NULL, *value=NULL, *oldval=NULL, *msg=NULL; struct ref_lock *lock; diff --git a/builtin-upload-tar.c b/builtin-upload-tar.c index d4fa7b5..7b401bb 100644 --- a/builtin-upload-tar.c +++ b/builtin-upload-tar.c @@ -15,7 +15,7 @@ static int nak(const char *reason) return 1; } -int cmd_upload_tar(int argc, const char **argv, char **envp) +int cmd_upload_tar(int argc, const char **argv, const char *prefix) { int len; const char *dir = argv[1]; diff --git a/builtin-write-tree.c b/builtin-write-tree.c index 449a4d1..0f2dd7c 100644 --- a/builtin-write-tree.c +++ b/builtin-write-tree.c @@ -60,7 +60,7 @@ int write_tree(unsigned char *sha1, int return 0; } -int cmd_write_tree(int argc, const char **argv, char **envp) +int cmd_write_tree(int argc, const char **argv, const char *unused_prefix) { int missing_ok = 0, ret; const char *prefix = NULL; diff --git a/builtin.h b/builtin.h index 5339d86..de244cd 100644 --- a/builtin.h +++ b/builtin.h @@ -15,53 +15,53 @@ #ifdef __GNUC__ #endif ; -extern int cmd_help(int argc, const char **argv, char **envp); -extern int cmd_version(int argc, const char **argv, char **envp); +extern int cmd_help(int argc, const char **argv, const char *prefix); +extern int cmd_version(int argc, const char **argv, const char *prefix); -extern int cmd_whatchanged(int argc, const char **argv, char **envp); -extern int cmd_show(int argc, const char **argv, char **envp); -extern int cmd_log(int argc, const char **argv, char **envp); -extern int cmd_diff(int argc, const char **argv, char **envp); -extern int cmd_format_patch(int argc, const char **argv, char **envp); -extern int cmd_count_objects(int argc, const char **argv, char **envp); +extern int cmd_whatchanged(int argc, const char **argv, const char *prefix); +extern int cmd_show(int argc, const char **argv, const char *prefix); +extern int cmd_log(int argc, const char **argv, const char *prefix); +extern int cmd_diff(int argc, const char **argv, const char *prefix); +extern int cmd_format_patch(int argc, const char **argv, const char *prefix); +extern int cmd_count_objects(int argc, const char **argv, const char *prefix); -extern int cmd_prune(int argc, const char **argv, char **envp); +extern int cmd_prune(int argc, const char **argv, const char *prefix); -extern int cmd_push(int argc, const char **argv, char **envp); -extern int cmd_grep(int argc, const char **argv, char **envp); -extern int cmd_rm(int argc, const char **argv, char **envp); -extern int cmd_add(int argc, const char **argv, char **envp); -extern int cmd_rev_list(int argc, const char **argv, char **envp); -extern int cmd_check_ref_format(int argc, const char **argv, char **envp); -extern int cmd_init_db(int argc, const char **argv, char **envp); -extern int cmd_tar_tree(int argc, const char **argv, char **envp); -extern int cmd_upload_tar(int argc, const char **argv, char **envp); -extern int cmd_get_tar_commit_id(int argc, const char **argv, char **envp); -extern int cmd_ls_files(int argc, const char **argv, char **envp); -extern int cmd_ls_tree(int argc, const char **argv, char **envp); -extern int cmd_read_tree(int argc, const char **argv, char **envp); -extern int cmd_commit_tree(int argc, const char **argv, char **envp); -extern int cmd_apply(int argc, const char **argv, char **envp); -extern int cmd_show_branch(int argc, const char **argv, char **envp); -extern int cmd_diff_files(int argc, const char **argv, char **envp); -extern int cmd_diff_index(int argc, const char **argv, char **envp); -extern int cmd_diff_stages(int argc, const char **argv, char **envp); -extern int cmd_diff_tree(int argc, const char **argv, char **envp); -extern int cmd_cat_file(int argc, const char **argv, char **envp); -extern int cmd_rev_parse(int argc, const char **argv, char **envp); -extern int cmd_update_index(int argc, const char **argv, char **envp); -extern int cmd_update_ref(int argc, const char **argv, char **envp); -extern int cmd_fmt_merge_msg(int argc, const char **argv, char **envp); +extern int cmd_push(int argc, const char **argv, const char *prefix); +extern int cmd_grep(int argc, const char **argv, const char *prefix); +extern int cmd_rm(int argc, const char **argv, const char *prefix); +extern int cmd_add(int argc, const char **argv, const char *prefix); +extern int cmd_rev_list(int argc, const char **argv, const char *prefix); +extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix); +extern int cmd_init_db(int argc, const char **argv, const char *prefix); +extern int cmd_tar_tree(int argc, const char **argv, const char *prefix); +extern int cmd_upload_tar(int argc, const char **argv, const char *prefix); +extern int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix); +extern int cmd_ls_files(int argc, const char **argv, const char *prefix); +extern int cmd_ls_tree(int argc, const char **argv, const char *prefix); +extern int cmd_read_tree(int argc, const char **argv, const char *prefix); +extern int cmd_commit_tree(int argc, const char **argv, const char *prefix); +extern int cmd_apply(int argc, const char **argv, const char *prefix); +extern int cmd_show_branch(int argc, const char **argv, const char *prefix); +extern int cmd_diff_files(int argc, const char **argv, const char *prefix); +extern int cmd_diff_index(int argc, const char **argv, const char *prefix); +extern int cmd_diff_stages(int argc, const char **argv, const char *prefix); +extern int cmd_diff_tree(int argc, const char **argv, const char *prefix); +extern int cmd_cat_file(int argc, const char **argv, const char *prefix); +extern int cmd_rev_parse(int argc, const char **argv, const char *prefix); +extern int cmd_update_index(int argc, const char **argv, const char *prefix); +extern int cmd_update_ref(int argc, const char **argv, const char *prefix); +extern int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix); -extern int cmd_write_tree(int argc, const char **argv, char **envp); +extern int cmd_write_tree(int argc, const char **argv, const char *prefix); extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix); -extern int cmd_mailsplit(int argc, const char **argv, char **envp); +extern int cmd_mailsplit(int argc, const char **argv, const char *prefix); extern int split_mbox(const char **mbox, const char *dir, int allow_bare, int nr_prec, int skip); -extern int cmd_mailinfo(int argc, const char **argv, char **envp); +extern int cmd_mailinfo(int argc, const char **argv, const char *prefix); extern int mailinfo(FILE *in, FILE *out, int ks, const char *encoding, const char *msg, const char *patch); -extern int cmd_stripspace(int argc, const char **argv, char **envp); +extern int cmd_stripspace(int argc, const char **argv, const char *prefix); extern void stripspace(FILE *in, FILE *out); #endif diff --git a/git.c b/git.c index ee5a0e8..e810d5c 100644 --- a/git.c +++ b/git.c @@ -157,25 +157,28 @@ static int handle_alias(int *argcp, cons const char git_version_string[] = GIT_VERSION; +#define NEEDS_PREFIX 1 + static void handle_internal_command(int argc, const char **argv, char **envp) { const char *cmd = argv[0]; static struct cmd_struct { const char *cmd; - int (*fn)(int, const char **, char **); + int (*fn)(int, const char **, const char *); + int prefix; } commands[] = { { "version", cmd_version }, { "help", cmd_help }, - { "log", cmd_log }, - { "whatchanged", cmd_whatchanged }, - { "show", cmd_show }, + { "log", cmd_log, NEEDS_PREFIX }, + { "whatchanged", cmd_whatchanged, NEEDS_PREFIX }, + { "show", cmd_show, NEEDS_PREFIX }, { "push", cmd_push }, - { "format-patch", cmd_format_patch }, + { "format-patch", cmd_format_patch, NEEDS_PREFIX }, { "count-objects", cmd_count_objects }, - { "diff", cmd_diff }, - { "grep", cmd_grep }, - { "rm", cmd_rm }, - { "add", cmd_add }, + { "diff", cmd_diff, NEEDS_PREFIX }, + { "grep", cmd_grep, NEEDS_PREFIX }, + { "rm", cmd_rm, NEEDS_PREFIX }, + { "add", cmd_add, NEEDS_PREFIX }, { "rev-list", cmd_rev_list }, { "init-db", cmd_init_db }, { "get-tar-commit-id", cmd_get_tar_commit_id }, @@ -190,17 +193,17 @@ static void handle_internal_command(int { "show-branch", cmd_show_branch }, { "diff-files", cmd_diff_files }, { "diff-index", cmd_diff_index }, - { "diff-stages", cmd_diff_stages }, + { "diff-stages", cmd_diff_stages, NEEDS_PREFIX }, { "diff-tree", cmd_diff_tree }, { "cat-file", cmd_cat_file }, - { "rev-parse", cmd_rev_parse }, + { "rev-parse", cmd_rev_parse, NEEDS_PREFIX }, { "write-tree", cmd_write_tree }, { "mailsplit", cmd_mailsplit }, { "mailinfo", cmd_mailinfo }, { "stripspace", cmd_stripspace }, - { "update-index", cmd_update_index }, + { "update-index", cmd_update_index, NEEDS_PREFIX }, { "update-ref", cmd_update_ref }, - { "fmt-merge-msg", cmd_fmt_merge_msg }, + { "fmt-merge-msg", cmd_fmt_merge_msg, NEEDS_PREFIX }, { "prune", cmd_prune }, }; int i; @@ -213,9 +216,13 @@ static void handle_internal_command(int for (i = 0; i < ARRAY_SIZE(commands); i++) { struct cmd_struct *p = commands+i; + const char *prefix; if (strcmp(p->cmd, cmd)) continue; + prefix = NULL; + if (p->prefix) + prefix = setup_git_directory(); if (getenv("GIT_TRACE")) { int i; fprintf(stderr, "trace: built-in: git"); @@ -227,7 +234,7 @@ static void handle_internal_command(int fflush(stderr); } - exit(p->fn(argc, argv, envp)); + exit(p->fn(argc, argv, prefix)); } } ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Call setup_git_directory() early 2006-07-29 5:44 ` Linus Torvalds @ 2006-07-29 9:12 ` Junio C Hamano 2006-07-29 16:52 ` Linus Torvalds 0 siblings, 1 reply; 8+ messages in thread From: Junio C Hamano @ 2006-07-29 9:12 UTC (permalink / raw) To: Linus Torvalds; +Cc: git Linus Torvalds <torvalds@osdl.org> writes: > On Fri, 28 Jul 2006, Junio C Hamano wrote: >> >> Your patch makes a lot of sense. > > I don't think they are mutually incompatible. I agree. I was apologizing for the crap under t/trash in the previous patch -- the rest were more or less in the same spirit as yours. > This is on top of my previous patch. What do you think? It does delete > more lines than it adds.. Let me understand what is going on here. > +#define NEEDS_PREFIX 1 >... > { "init-db", cmd_init_db }, This is an oddball that wants to always use $GIT_DIR environment or "./.git" if $GIT_DIR is not exported, and should never go looking for GIT_DIR, so we should not give NEEDS_PREFIX, ever. > { "stripspace", cmd_stripspace }, > { "mailsplit", cmd_mailsplit }, > { "get-tar-commit-id", cmd_get_tar_commit_id }, These are the other extreme -- they really do not care about operating in a git context. { "push", cmd_push }, { "count-objects", cmd_count_objects }, { "check-ref-format", cmd_check_ref_format }, One rule seems to be that the commands that work at the while repository level do not have NEEDS_PREFIX -- they will never work from a subdirectory. There is no technical reason not to let count-objects find $GIT_DIR on its own, but the current implementation does not do that and you are keeping the behaviour bug-to-bug compatible. > + { "diff", cmd_diff, NEEDS_PREFIX }, > + { "grep", cmd_grep, NEEDS_PREFIX }, > + { "rm", cmd_rm, NEEDS_PREFIX }, > + { "add", cmd_add, NEEDS_PREFIX }, > + { "update-index", cmd_update_index, NEEDS_PREFIX }, > + { "rev-parse", cmd_rev_parse, NEEDS_PREFIX }, > + { "fmt-merge-msg", cmd_fmt_merge_msg, NEEDS_PREFIX }, Another rule: porcelain-ish should work from subdirectories, and they do not have to call setup_git_directory() on their own. > { "rev-list", cmd_rev_list }, I think we should mark this with NEEDS_PREFIX, and lose the call to setup_git_directory() it has here: ... int cmd_rev_list(int argc, const char **argv, const char *prefix) { struct commit_list *list; int i; - init_revisions(&revs, setup_git_directory()); + init_revisions(&revs, prefix); ... > { "diff-files", cmd_diff_files }, > { "diff-index", cmd_diff_index }, > { "diff-tree", cmd_diff_tree }, > { "prune", cmd_prune }, { "ls-files", cmd_ls_files }, { "ls-tree", cmd_ls_tree }, { "tar-tree", cmd_tar_tree }, { "read-tree", cmd_read_tree }, { "commit-tree", cmd_commit_tree }, Likewise with the above group. > { "show-branch", cmd_show_branch }, > { "cat-file", cmd_cat_file }, > { "write-tree", cmd_write_tree }, > { "update-ref", cmd_update_ref }, These call setup_git_directory() on their own and discard the resulting prefix; they do want to know where $GIT_DIR is to find objects and refs, so maybe we would want to mark these with NEEDS_PREFIX and lose their own setup_git_directory(), like the previous group. Perhaps you were thinking about splitting setup_git_directory() into two independent functions, one for finding out GIT_DIR, another for finding prefix and cd'ing up to the project root? Some of the above take configuration options, so even they are not interested in finding out prefix they do need to know where $GIT_DIR is (e.g. show-branch, update-ref, mailinfo). > { "mailinfo", cmd_mailinfo }, { "apply", cmd_apply }, These are tricky. On the one hand, there is no strong reason to forbid use of mailinfo or apply outside git context, but they do want to use some configuration items when available (commitencoding for mailinfo, whitespace for apply). So here is on top of your two patches to reflect the above comments. Again it seems to pass our tests, so I'll put it in "next" tonight. -- builtin-cat-file.c | 1 - builtin-commit-tree.c | 2 -- builtin-diff-files.c | 2 +- builtin-diff-index.c | 2 +- builtin-diff-tree.c | 2 +- builtin-ls-files.c | 1 - builtin-ls-tree.c | 1 - builtin-prune.c | 2 +- builtin-read-tree.c | 1 - builtin-rev-list.c | 2 +- builtin-show-branch.c | 1 - builtin-tar-tree.c | 1 - builtin-update-ref.c | 1 - builtin-write-tree.c | 2 -- git.c | 28 ++++++++++++++-------------- 15 files changed, 19 insertions(+), 30 deletions(-) diff --git a/builtin-cat-file.c b/builtin-cat-file.c index d796431..814fb07 100644 --- a/builtin-cat-file.c +++ b/builtin-cat-file.c @@ -102,7 +102,6 @@ int cmd_cat_file(int argc, const char ** unsigned long size; int opt; - setup_git_directory(); git_config(git_default_config); if (argc != 3) usage("git-cat-file [-t|-s|-e|-p|<type>] <sha1>"); diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c index e6fb16d..9c98796 100644 --- a/builtin-commit-tree.c +++ b/builtin-commit-tree.c @@ -88,8 +88,6 @@ int cmd_commit_tree(int argc, const char unsigned int size; setup_ident(); - setup_git_directory(); - git_config(git_default_config); if (argc < 2) diff --git a/builtin-diff-files.c b/builtin-diff-files.c index da51284..ac13db7 100644 --- a/builtin-diff-files.c +++ b/builtin-diff-files.c @@ -18,7 +18,7 @@ int cmd_diff_files(int argc, const char struct rev_info rev; int silent = 0; - init_revisions(&rev, setup_git_directory()); + init_revisions(&rev, prefix); git_config(git_default_config); /* no "diff" UI options */ rev.abbrev = 0; diff --git a/builtin-diff-index.c b/builtin-diff-index.c index bedb90c..95a3db1 100644 --- a/builtin-diff-index.c +++ b/builtin-diff-index.c @@ -15,7 +15,7 @@ int cmd_diff_index(int argc, const char int cached = 0; int i; - init_revisions(&rev, setup_git_directory()); + init_revisions(&rev, prefix); git_config(git_default_config); /* no "diff" UI options */ rev.abbrev = 0; diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c index 816946b..24cb2d7 100644 --- a/builtin-diff-tree.c +++ b/builtin-diff-tree.c @@ -67,7 +67,7 @@ int cmd_diff_tree(int argc, const char * static struct rev_info *opt = &log_tree_opt; int read_stdin = 0; - init_revisions(opt, setup_git_directory()); + init_revisions(opt, prefix); git_config(git_default_config); /* no "diff" UI options */ nr_sha1 = 0; opt->abbrev = 0; diff --git a/builtin-ls-files.c b/builtin-ls-files.c index e906c81..79ffe8f 100644 --- a/builtin-ls-files.c +++ b/builtin-ls-files.c @@ -329,7 +329,6 @@ int cmd_ls_files(int argc, const char ** struct dir_struct dir; memset(&dir, 0, sizeof(dir)); - prefix = setup_git_directory(); if (prefix) prefix_offset = strlen(prefix); git_config(git_default_config); diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c index d31efe9..261147f 100644 --- a/builtin-ls-tree.c +++ b/builtin-ls-tree.c @@ -90,7 +90,6 @@ int cmd_ls_tree(int argc, const char **a unsigned char sha1[20]; struct tree *tree; - prefix = setup_git_directory(); git_config(git_default_config); ls_tree_prefix = prefix; if (prefix && *prefix) diff --git a/builtin-prune.c b/builtin-prune.c index 15bb650..6a86eb5 100644 --- a/builtin-prune.c +++ b/builtin-prune.c @@ -234,7 +234,7 @@ int cmd_prune(int argc, const char **arg * Set up revision parsing, and mark us as being interested * in all object types, not just commits. */ - init_revisions(&revs, setup_git_directory()); + init_revisions(&revs, prefix); revs.tag_objects = 1; revs.blob_objects = 1; revs.tree_objects = 1; diff --git a/builtin-read-tree.c b/builtin-read-tree.c index 25ca69c..49c10bf 100644 --- a/builtin-read-tree.c +++ b/builtin-read-tree.c @@ -882,7 +882,6 @@ int cmd_read_tree(int argc, const char * state.quiet = 1; state.refresh_cache = 1; - setup_git_directory(); git_config(git_default_config); newfd = hold_lock_file_for_update(&lock_file, get_index_file()); diff --git a/builtin-rev-list.c b/builtin-rev-list.c index 65b4d0c..0dee173 100644 --- a/builtin-rev-list.c +++ b/builtin-rev-list.c @@ -311,7 +311,7 @@ int cmd_rev_list(int argc, const char ** struct commit_list *list; int i; - init_revisions(&revs, setup_git_directory()); + init_revisions(&revs, prefix); revs.abbrev = 0; revs.commit_format = CMIT_FMT_UNSPECIFIED; argc = setup_revisions(argc, argv, &revs, NULL); diff --git a/builtin-show-branch.c b/builtin-show-branch.c index 9c7f9f6..2a1b848 100644 --- a/builtin-show-branch.c +++ b/builtin-show-branch.c @@ -573,7 +573,6 @@ int cmd_show_branch(int ac, const char * int topics = 0; int dense = 1; - setup_git_directory(); git_config(git_show_branch_config); /* If nothing is specified, try the default first */ diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c index 637d22e..7c48db9 100644 --- a/builtin-tar-tree.c +++ b/builtin-tar-tree.c @@ -319,7 +319,6 @@ static int generate_tar(int argc, const current_path.alloc = PATH_MAX; current_path.len = current_path.eof = 0; - setup_git_directory(); git_config(git_tar_config); switch (argc) { diff --git a/builtin-update-ref.c b/builtin-update-ref.c index acdf39a..5bd7182 100644 --- a/builtin-update-ref.c +++ b/builtin-update-ref.c @@ -13,7 +13,6 @@ int cmd_update_ref(int argc, const char int i; setup_ident(); - setup_git_directory(); git_config(git_default_config); for (i = 1; i < argc; i++) { diff --git a/builtin-write-tree.c b/builtin-write-tree.c index 0f2dd7c..0289f59 100644 --- a/builtin-write-tree.c +++ b/builtin-write-tree.c @@ -66,8 +66,6 @@ int cmd_write_tree(int argc, const char const char *prefix = NULL; unsigned char sha1[20]; - setup_git_directory(); - while (1 < argc) { const char *arg = argv[1]; if (!strcmp(arg, "--missing-ok")) diff --git a/git.c b/git.c index 34328c0..79db43e 100644 --- a/git.c +++ b/git.c @@ -236,32 +236,32 @@ static void handle_internal_command(int { "grep", cmd_grep, NEEDS_PREFIX }, { "rm", cmd_rm, NEEDS_PREFIX }, { "add", cmd_add, NEEDS_PREFIX }, - { "rev-list", cmd_rev_list }, + { "rev-list", cmd_rev_list, NEEDS_PREFIX }, { "init-db", cmd_init_db }, { "get-tar-commit-id", cmd_get_tar_commit_id }, { "upload-tar", cmd_upload_tar }, { "check-ref-format", cmd_check_ref_format }, - { "ls-files", cmd_ls_files }, - { "ls-tree", cmd_ls_tree }, - { "tar-tree", cmd_tar_tree }, - { "read-tree", cmd_read_tree }, - { "commit-tree", cmd_commit_tree }, + { "ls-files", cmd_ls_files, NEEDS_PREFIX }, + { "ls-tree", cmd_ls_tree, NEEDS_PREFIX }, + { "tar-tree", cmd_tar_tree, NEEDS_PREFIX }, + { "read-tree", cmd_read_tree, NEEDS_PREFIX }, + { "commit-tree", cmd_commit_tree, NEEDS_PREFIX }, { "apply", cmd_apply }, - { "show-branch", cmd_show_branch }, - { "diff-files", cmd_diff_files }, - { "diff-index", cmd_diff_index }, + { "show-branch", cmd_show_branch, NEEDS_PREFIX }, + { "diff-files", cmd_diff_files, NEEDS_PREFIX }, + { "diff-index", cmd_diff_index, NEEDS_PREFIX }, { "diff-stages", cmd_diff_stages, NEEDS_PREFIX }, - { "diff-tree", cmd_diff_tree }, - { "cat-file", cmd_cat_file }, + { "diff-tree", cmd_diff_tree, NEEDS_PREFIX }, + { "cat-file", cmd_cat_file, NEEDS_PREFIX }, { "rev-parse", cmd_rev_parse, NEEDS_PREFIX }, - { "write-tree", cmd_write_tree }, + { "write-tree", cmd_write_tree, NEEDS_PREFIX }, { "mailsplit", cmd_mailsplit }, { "mailinfo", cmd_mailinfo }, { "stripspace", cmd_stripspace }, { "update-index", cmd_update_index, NEEDS_PREFIX }, - { "update-ref", cmd_update_ref }, + { "update-ref", cmd_update_ref, NEEDS_PREFIX }, { "fmt-merge-msg", cmd_fmt_merge_msg, NEEDS_PREFIX }, - { "prune", cmd_prune }, + { "prune", cmd_prune, NEEDS_PREFIX }, }; int i; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Call setup_git_directory() early 2006-07-29 9:12 ` Junio C Hamano @ 2006-07-29 16:52 ` Linus Torvalds 0 siblings, 0 replies; 8+ messages in thread From: Linus Torvalds @ 2006-07-29 16:52 UTC (permalink / raw) To: Junio C Hamano; +Cc: git On Sat, 29 Jul 2006, Junio C Hamano wrote: > > > +#define NEEDS_PREFIX 1 > >... > > { "init-db", cmd_init_db }, > > This is an oddball that wants to always use $GIT_DIR environment > or "./.git" if $GIT_DIR is not exported, and should never go > looking for GIT_DIR, so we should not give NEEDS_PREFIX, ever. Well, the way I did things, each command _can_ still decide to call "setup_git_directory()" on their own, and we don't actually say up front whether they will or not. So some commands will just choose to let the git.c wrapper do it for them, and others can do it their own way, which leaves the maximum of flexibility, and doesn't mean that odd-balls like "init-db" really have to even be an issue. > > { "stripspace", cmd_stripspace }, > > { "mailsplit", cmd_mailsplit }, > > { "get-tar-commit-id", cmd_get_tar_commit_id }, > > These are the other extreme -- they really do not care about > operating in a git context. Right. > { "push", cmd_push }, > { "count-objects", cmd_count_objects }, > { "check-ref-format", cmd_check_ref_format }, > > One rule seems to be that the commands that work at the while > repository level do not have NEEDS_PREFIX -- they will never > work from a subdirectory. Well, maybe we will make them do so, and maybe we won't. The way I chose the NEED_PREFIX ones was actually really simple and totally automated: I changed every builtin function to first _not_ have NEEDS_PREFIX, but changed the prototype to take "const char *prefix" instead of the "char **envp" that nobody actually used. I then compiled them, and for each function that _already_ had a "prefix" in their existing implementation (which the compiler warned about), I just removed the old "prefix = setup_git_directory()", and added the NEEDS_PREFIX flag for that command. > There is no technical reason not to let count-objects find > $GIT_DIR on its own, but the current implementation does not do > that and you are keeping the behaviour bug-to-bug compatible. Exactly. My conversions (both the first smaller one, and the second thing) were really totally mindless translations, nothing fancy. I fixed a few bugs as I hit them (ie the first one moved the "setup_git_directory()" call earlier over some "git_config()" calls), but other than those kinds of local changes, I tried to keep not just the patch, but very much the process of me creating it, very straightforward. > > { "rev-list", cmd_rev_list }, > > I think we should mark this with NEEDS_PREFIX, and lose the call > to setup_git_directory() it has here: I think you're right, and the above explanation of how the patch was mindlessly created explains why my patch didn't do it that way ;) When I do re-organizations, I like doing them in two stages: the "mindless" stage that does the bulk of the syntactic stuff and the movement of code, but that pretty much by definition should not introduce any new bugs, and then the second stage is the "ok, now we have the new organization, we can clean up and fix problems". Linus ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2006-07-29 16:53 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-07-29 1:17 log and diff family: honor config even from subdirectories Linus Torvalds 2006-07-29 1:54 ` Junio C Hamano 2006-07-29 4:21 ` Call setup_git_directory() early Linus Torvalds 2006-07-29 4:41 ` An alternative: call " Junio C Hamano 2006-07-29 5:06 ` Call " Junio C Hamano 2006-07-29 5:44 ` Linus Torvalds 2006-07-29 9:12 ` Junio C Hamano 2006-07-29 16:52 ` Linus Torvalds
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox