{"thread":{"id":"8810","subject":"[RFC] Update on builtin-commit","startedAt":"2007-07-02T14:22:43Z","lastAt":"2007-07-02T17:57:30Z","messageCount":6,"participants":["Kristian Høgsberg","Johannes Schindelin","Jeffrey C. Ollie"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"46238","messageId":"11833861634103-git-send-email-krh@redhat.com","threadId":"8810","inReplyTo":null,"subject":"[RFC] Update on builtin-commit","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-07-02T14:22:43Z","receivedAt":"2007-07-02T14:22:43Z","isPatch":false,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"Hi,\n\nHere's an update on the work so far, and I've attached the current state\nof the work as a patch.  I'm pretty close to making my first commit with\nthis, and some of the work here is ready to be factored out into a few\nstand-alone patches.  I'm thinking of the wt-status.[ch] changes and the\nread_fd and read_path changes.  My plan is to finish this first-pass\nporting of the script and then go back and review and split the patch into\na series of commits, but if somebody wants to pick that up now, that'd be\ngreat.\n\nThe only steps missing here are\n\n\t1) Call git-write-tree with the new index\n\t2) Run the post-commit hook\n\nand I need to look into the reflog stuff.  But other than that, the port is\nmostly complete.\n\nKristian\n\n---\n Makefile         |    9 +-\n builtin-commit.c |  567 ++++++++++++++++++++++++++++++++++++++++++++++++++++++\n builtin.h        |    3 +-\n cache.h          |    3 +-\n color.c          |   18 +-\n color.h          |    4 +-\n git.c            |    3 +-\n mktag.c          |    8 +-\n sha1_file.c      |   44 +++--\n wt-status.c      |   84 ++++----\n wt-status.h      |    4 +\n 11 files changed, 667 insertions(+), 80 deletions(-)\n create mode 100644 builtin-commit.c\n\ndiff --git a/Makefile b/Makefile\nindex 0f75955..967d5a5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -198,7 +198,7 @@ BASIC_LDFLAGS =\n \n SCRIPT_SH = \\\n \tgit-bisect.sh git-checkout.sh \\\n-\tgit-clean.sh git-clone.sh git-commit.sh \\\n+\tgit-clean.sh git-clone.sh \\\n \tgit-fetch.sh \\\n \tgit-ls-remote.sh \\\n \tgit-merge-one-file.sh git-mergetool.sh git-parse-remote.sh \\\n@@ -257,7 +257,7 @@ EXTRA_PROGRAMS =\n BUILT_INS = \\\n \tgit-format-patch$X git-show$X git-whatchanged$X git-cherry$X \\\n \tgit-get-tar-commit-id$X git-init$X git-repo-config$X \\\n-\tgit-fsck-objects$X git-cherry-pick$X \\\n+\tgit-fsck-objects$X git-cherry-pick$X git-status$X\\\n \t$(patsubst builtin-%.o,git-%$X,$(BUILTIN_OBJS))\n \n # what 'all' will build and 'install' will install, in gitexecdir\n@@ -332,6 +332,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-check-attr.o \\\n \tbuiltin-checkout-index.o \\\n \tbuiltin-check-ref-format.o \\\n+\tbuiltin-commit.o \\\n \tbuiltin-commit-tree.o \\\n \tbuiltin-count-objects.o \\\n \tbuiltin-describe.o \\\n@@ -367,7 +368,6 @@ BUILTIN_OBJS = \\\n \tbuiltin-rev-parse.o \\\n \tbuiltin-revert.o \\\n \tbuiltin-rm.o \\\n-\tbuiltin-runstatus.o \\\n \tbuiltin-shortlog.o \\\n \tbuiltin-show-branch.o \\\n \tbuiltin-stripspace.o \\\n@@ -791,9 +791,6 @@ $(patsubst %.perl,%,$(SCRIPT_PERL)): % : %.perl\n \tchmod +x $@+ && \\\n \tmv $@+ $@\n \n-git-status: git-commit\n-\t$(QUIET_GEN)cp $< $@+ && mv $@+ $@\n-\n gitweb/gitweb.cgi: gitweb/gitweb.perl\n \t$(QUIET_GEN)rm -f $@ $@+ && \\\n \tsed -e '1s|#!.*perl|#!$(PERL_PATH_SQ)|' \\\ndiff --git a/builtin-commit.c b/builtin-commit.c\nnew file mode 100644\nindex 0000000..4a68cc0\n--- /dev/null\n+++ b/builtin-commit.c\n@@ -0,0 +1,567 @@\n+/*\n+ * Builtin \"git commit\"\n+ *\n+ * Copyright (c) 2007 Kristian Høgsberg <krh@redhat.com>\n+ * Based on git-commit.sh by Junio C Hamano and Linus Torvalds\n+ */\n+\n+#include <sys/types.h>\n+#include <sys/stat.h>\n+#include <unistd.h>\n+\n+#include \"cache.h\"\n+#include \"builtin.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+#include \"commit.h\"\n+#include \"revision.h\"\n+#include \"wt-status.h\"\n+#include \"run-command.h\"\n+\n+static const char builtin_commit_usage[] =\n+\t\"[-a | --interactive] [-s] [-v] [--no-verify] [-m <message> | -F <logfile> | (-C|-c) <commit> | --amend] [-u] [-e] [--author <author>] [[-i | -o] <path>...]\";\n+\n+static unsigned char head_sha1[20];\n+static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n+\n+enum option_type {\n+    OPTION_NONE,\n+    OPTION_STRING,\n+    OPTION_INTEGER,\n+    OPTION_LAST,\n+};\n+\n+struct option {\n+    enum option_type type;\n+    const char *long_name;\n+    char short_name;\n+    void *value;\n+};\n+\n+static int scan_options(const char ***argv, struct option *options)\n+{\n+\tconst char *value, *eq;\n+\tint i;\n+\n+\tif (**argv == NULL)\n+\t\treturn 0;\n+\tif ((**argv)[0] != '-')\n+\t\treturn 0;\n+\tif (!strcmp(**argv, \"--\"))\n+\t\treturn 0;\n+\n+\tvalue = NULL;\n+\tfor (i = 0; options[i].type != OPTION_LAST; i++) {\n+\t\tif ((**argv)[1] == '-') {\n+\t\t\tif (!prefixcmp(options[i].long_name, **argv + 2)) {\n+\t\t\t\tif (options[i].type != OPTION_NONE)\n+\t\t\t\t\tvalue = *++(*argv);\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\n+\t\t\teq = strchr(**argv + 2, '=');\n+\t\t\tif (eq && options[i].type != OPTION_NONE &&\n+\t\t\t    !strncmp(**argv + 2, \n+\t\t\t\t     options[i].long_name, eq - **argv - 2)) {\n+\t\t\t\tvalue = eq + 1;\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif ((**argv)[1] == options[i].short_name) {\n+\t\t\tif ((**argv)[2] == '\\0') {\n+\t\t\t\tif (options[i].type != OPTION_NONE)\n+\t\t\t\t\tvalue = *++(*argv);\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\n+\t\t\tif (options[i].type != OPTION_NONE) {\n+\t\t\t\tvalue = **argv + 2;\n+\t\t\t\tgoto match;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tusage(builtin_commit_usage);\n+\n+ match:\n+\tswitch (options[i].type) {\n+\tcase OPTION_NONE:\n+\t\t*(int *)options[i].value = 1;\n+\t\tbreak;\n+\tcase OPTION_STRING:\n+\t\tif (value == NULL)\n+\t\t\tdie(\"option %s requires a value.\", (*argv)[-1]);\n+\t\t*(const char **)options[i].value = value;\n+\t\tbreak;\n+\tcase OPTION_INTEGER:\n+\t\tif (value == NULL)\n+\t\t\tdie(\"option %s requires a value.\", (*argv)[-1]);\n+\t\t*(int *)options[i].value = atoi(value);\n+\t\tbreak;\n+\tdefault:\n+\t\tassert(0);\n+\t}\n+\n+\t(*argv)++;\n+\n+\treturn 1;\n+}\n+\n+static char *logfile, *force_author, *message;\n+static char *edit_message, *use_message;\n+static int all, edit_flag, also, interactive, only, no_verify, amend, signoff;\n+static int quiet, verbose, untracked_files;\n+\n+static int no_edit;\n+const char *only_include_assumed;\n+\n+static struct option commit_options[] = {\n+\t{ OPTION_STRING, \"file\", 'F', (void *) &logfile },\n+\t{ OPTION_NONE, \"all\", 'a', &all },\n+\t{ OPTION_STRING, \"author\", 0, (void *) &force_author },\n+\t{ OPTION_NONE, \"edit\", 0, &edit_flag },\n+\t{ OPTION_NONE, \"include\", 'i', &also },\n+\t{ OPTION_NONE, \"interactive\", 0, &interactive },\n+\t{ OPTION_NONE, \"only\", 'o', &only },\n+\t{ OPTION_STRING, \"message\", 'm', &message },\n+\t{ OPTION_NONE, \"no-verify\", 'n', &no_verify },\n+\t{ OPTION_NONE, \"amend\", 0, &amend },\n+\t{ OPTION_STRING, \"reedit-message\", 'c', &edit_message },\n+\t{ OPTION_STRING, \"reuse-message\", 'C', &use_message },\n+\t{ OPTION_NONE, \"signoff\", 's', &signoff },\n+\t{ OPTION_NONE, \"quiet\", 'q', &signoff },\n+\t{ OPTION_NONE, \"verbose\", 'v', &verbose },\n+\t{ OPTION_NONE, \"untracked-files\", 0, &untracked_files },\n+\t{ OPTION_LAST },\n+};\n+\n+/* FIXME: Taken from builtin-add, should be shared. */\n+\n+static void update_callback(struct diff_queue_struct *q,\n+\t\t\t    struct diff_options *opt, void *cbdata)\n+{\n+\tint i, verbose;\n+\n+\tverbose = *((int *)cbdata);\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filepair *p = q->queue[i];\n+\t\tconst char *path = p->one->path;\n+\t\tswitch (p->status) {\n+\t\tdefault:\n+\t\t\tdie(\"unexpacted diff status %c\", p->status);\n+\t\tcase DIFF_STATUS_UNMERGED:\n+\t\tcase DIFF_STATUS_MODIFIED:\n+\t\t\tadd_file_to_cache(path, verbose);\n+\t\t\tbreak;\n+\t\tcase DIFF_STATUS_DELETED:\n+\t\t\tremove_file_from_cache(path);\n+\t\t\tif (verbose)\n+\t\t\t\tprintf(\"remove '%s'\\n\", path);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+static void\n+add_files_to_cache(int fd, const char **files, const char *prefix)\n+{\n+\tstruct rev_info rev;\n+\n+\tinit_revisions(&rev, \"\");\n+\tsetup_revisions(0, NULL, &rev, NULL);\n+\trev.prune_data = get_pathspec(prefix, files);\n+\trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n+\trev.diffopt.format_callback = update_callback;\n+\trev.diffopt.format_callback_data = &verbose;\n+\n+\trun_diff_files(&rev, 0);\n+\n+\tif (write_cache(fd, active_cache, active_nr) || close(fd))\n+\t    die(\"unable to write new index file\");\n+}\n+\n+static char *\n+prepare_index(const char **files, struct lock_file *lk, const char *prefix)\n+{\n+\tint fd;\n+\tstruct tree *tree;\n+\tstruct lock_file *next_index_lock;\n+\n+\tfd = hold_locked_index(lk, 1);\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n+\n+\tif (all) {\n+\t\tadd_files_to_cache(fd, files, NULL);\n+\t\treturn lk->filename;\n+\t} else if (also) {\n+\t\tadd_files_to_cache(fd, files, prefix);\n+\t\treturn lk->filename;\n+\t}\n+\n+\tif (interactive)\n+\t\t/* launch git-add --interactive */;\n+\n+\tif (*files == NULL) {\n+\t\trollback_lock_file(lk);\n+\t\treturn get_index_file();\n+\t}\n+\n+\t/*\n+\t * FIXME: Warn on unknown files.  Shell script does\n+\t *\n+\t *   commit_only=`git-ls-files --error-unmatch -- \"$@\"`\n+\t */\n+\n+\t/*\n+\t * FIXME: shell script does\n+\t *\n+\t *   git-read-tree --index-output=\"$TMP_INDEX\" -i -m HEAD\n+\t *\n+\t * which warns about unmerged files in the index.\n+\t */\n+\n+\t/* update the user index file */\n+\tadd_files_to_cache(fd, files, prefix);\n+\n+\ttree = parse_tree_indirect(head_sha1);\n+\tif (!tree)\n+\t\tdie(\"failed to unpack HEAD tree object\");\n+\tif (read_tree(tree, 0, NULL))\n+\t\tdie(\"failed to read HEAD tree object\");\n+\n+\t/* Uh oh, abusing lock_file to create a garbage collected file */\n+\tnext_index_lock = xmalloc(sizeof(*next_index_lock));\n+\tfd = hold_lock_file_for_update(next_index_lock,\n+\t\t\t\t       git_path(\"next-index-%d\", getpid()), 1);\n+\tadd_files_to_cache(fd, files, prefix);\n+\n+\treturn next_index_lock->filename;\n+}\n+\n+static int strip_lines(char *buffer, int len)\n+{\n+\tint blank_lines, i, j;\n+\tchar *eol;\n+\n+\tblank_lines = 1;\n+\tfor (i = 0, j = 0; i < len; i++) {\n+\t\tif (blank_lines > 0 && buffer[i] == '#') {\n+\t\t\teol = strchr(buffer + i, '\\n');\n+\t\t\tif (!eol)\n+\t\t\t\tbreak;\n+\n+\t\t\ti = eol - buffer;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (buffer[i] == '\\n') {\n+\t\t\tblank_lines++;\n+\t\t\tif (blank_lines > 1)\n+\t\t\t\tcontinue;\n+\t\t} else {\n+\t\t\tif (blank_lines > 2)\n+\t\t\t\tbuffer[j++] = '\\n';\n+\t\t\tblank_lines = 0;\n+\t\t}\n+\n+\t\tbuffer[j++] = buffer[i];\n+\t}\n+\n+\tif (buffer[j - 1] != '\\n')\n+               buffer[j++] = '\\n';\n+\n+\treturn j;\n+}\n+\n+static int run_status(FILE *fp, const char *index_file)\n+{\n+\tstruct wt_status s;\n+\n+\twt_status_prepare(&s);\n+\n+\tif (amend) {\n+\t\ts.amend = 1;\n+\t\ts.reference = \"HEAD^1\";\n+\t}\n+\ts.verbose = verbose;\n+\ts.untracked = untracked_files;\n+\ts.index_file = index_file;\n+\ts.fp = fp;\n+\n+\twt_status_print(&s);\n+\n+\treturn s.commitable;\n+}\n+\n+static const char sign_off_header[] = \"Signed-off-by: \";\n+\n+static int prepare_log_message(const char *index_file)\n+{\n+\tchar *buffer = NULL, *commit;\n+\tstruct stat statbuf;\n+\tint type, commitable;\n+\tunsigned long len, size;\n+\tunsigned char sha1[20];\n+\tFILE *fp;\n+\n+\tif (message) {\n+\t\tbuffer = message;\n+\t\tlen = strlen(message);\n+\t} else if (logfile && !strcmp(logfile, \"-\")) {\n+\t\tif (isatty(0))\n+\t\t\tfprintf(stderr, \"(reading log message from standard input)\\n\");\n+\t\tif (read_fd(0, &buffer, &len))\n+\t\t\tdie(\"could not read log from standard input\");\n+\t} else if (logfile) {\n+\t\tif (read_path(logfile, &buffer, &len))\n+\t\t\tdie(\"could not read log file '%s': %s\",\n+\t\t\t    logfile, strerror(errno));\n+\t} else if (use_message) {\n+\t\t/* git-show unrolled here... or split out to another\n+\t\t * function */\n+\t\t/* encoding */\n+\t\tif (get_sha1(use_message, sha1))\n+\t\t\tdie(\"could not lookup commit %s\", use_message);\n+\t\t/* check it's a commit */\n+\t\tcommit = read_sha1_file(sha1, &type, &size);\n+\t\t/* filter out headers, write to msg fd */\n+\t} else if (!stat(git_path(\"MERGE_MSG\"), &statbuf)) {\n+\t\tif (read_path(git_path(\"MERGE_MSG\"), &buffer, &len))\n+\t\t\tdie(\"could not read MERGE_MSG: %s\", strerror(errno));\n+\t} else if (!stat(git_path(\"SQUASH_MSG\"), &statbuf)) {\n+\t\tif (read_path(git_path(\"SQUASH_MSG\"), &buffer, &len))\n+\t\t\tdie(\"could not read SQUASH_MSG: %s\", strerror(errno));\n+\t}\n+\n+\tif (buffer)\n+\t\tlen = strip_lines(buffer, len);\n+\n+\tfp = fopen(git_path(commit_editmsg), \"w\");\n+\tif (fp == NULL)\n+\t\tdie(\"could not open %s\\n\", git_path(commit_editmsg));\n+\t\t\n+\tif (fwrite(buffer, 1, len, fp) < len)\n+\t\tdie(\"could not write commit template: %s\\n\", strerror(errno));\n+\n+\tif (buffer && signoff) {\n+\t\tchar *bol = strrchr(buffer + len - 1, '\\n');\n+\t\tconst char *info;\n+\n+\t\tinfo = git_committer_info(1);\n+\t\tif (!bol || prefixcmp(bol, sign_off_header))\n+\t\t\tfprintf(fp, \"\\n\");\n+\t\tfprintf(fp, \"Signed-off-by: %s\\n\", git_committer_info(1));\n+\t}\n+\n+\tif (!stat(git_path(\"MERGE_HEAD\"), &statbuf) && !no_edit) {\n+\t\tfprintf(fp,\n+\t\t\t\"#\\n\"\n+\t\t\t\"# It looks like you may be committing a MERGE.\\n\"\n+\t\t\t\"# If this is not correct, please remove the file\\n\"\n+\t\t\t\"#\t%s\\n\"\n+\t\t\t\"# and try again.\\n\"\n+\t\t\t\"#\\n\",\n+\t\t\tgit_path(\"MERGE_HEAD\"));\n+\t}\n+\n+\tfprintf(fp,\n+\t\t\"\\n\"\n+\t\t\"# Please enter the commit message for your changes.\\n\"\n+\t\t\"# (Comment lines starting with '#' will not be included)\\n\");\n+\tif (only_include_assumed)\n+\t\tfprintf(fp, \"# %s\\n\", only_include_assumed);\n+\n+\tcommitable = run_status(fp, index_file);\n+\n+\tfclose(fp);\n+\n+\treturn commitable;\n+}\n+\n+void\n+determine_author(char **name, char **email)\n+{\n+\tif (use_message) {\n+\t\t/* Get author from commit use_message\n+\t\t * parse \"author Name <email> date\"\n+\t\t */\n+\t} else if (force_author) {\n+\t\t/* parse \"Author <email>\" get from force_author */\n+\t}\n+}\n+\n+static void parse_and_validate_options(const char ***argv)\n+{\n+\tint initial_commit = 0, f = 0;\n+\tstruct stat statbuf;\n+\n+\tgit_config(git_status_config);\n+\n+\t(*argv)++;\n+\twhile (scan_options(argv, commit_options))\n+\t\t;\n+\n+\tif (logfile || message || use_message)\n+\t\tno_edit = 1;\n+\n+\tif (get_sha1(\"HEAD\", head_sha1))\n+\t\tinitial_commit = 1;\n+\n+\t/* Sanity check options */\n+\tif (amend && initial_commit)\n+\t\tdie(\"You have nothing to amend.\");\n+\tif (amend && !stat(git_path(\"MERGE_HEAD\"), &statbuf))\n+\t\tdie(\"You are in the middle of a merger -- cannot amend.\");\n+\n+\tif (use_message)\n+\t\tf++;\n+\tif (edit_message)\n+\t\tf++;\n+\tif (logfile)\n+\t\tf++;\n+\tif (amend)\n+\t\tf++;\n+\tif (f > 1)\n+\t\tdie(\"Only one of -c/-C/-F/--amend can be used.\");\n+\tif (message && f > 0)\n+\t\tdie(\"Option -m cannot be combined with -c/-C/-F/--amend.\");\n+\n+\tif (also && only)\n+\t\tdie(\"Only one of --include/--only can be used.\");\n+\tif (!*argv && (also || (only && !amend)))\n+\t\tdie(\"No paths with --include/--only does not make sense.\");\n+\tif (!*argv && only && amend)\n+\t\tonly_include_assumed = \"Clever... amending the last one with dirty index.\";\n+\tif (*argv && !also && !only) {\n+\t\tonly_include_assumed = \"Explicit paths specified without -i nor -o; assuming --only paths...\";\n+\t\talso = 0;\n+\t}\n+\n+\tif (all && interactive)\n+\t\tdie(\"Cannot use -a, --interactive or -i at the same time.\");\n+\telse if (all && **argv)\n+\t\tdie(\"Paths with -a does not make sense.\");\n+\telse if (interactive && **argv)\n+\t\tdie(\"Paths with --interactive does not make sense.\");\n+}\n+\n+int cmd_status(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *index_file;\n+\tstruct lock_file lk;\n+\tint commitable;\n+\n+\tparse_and_validate_options(&argv);\n+\n+\tindex_file = prepare_index(argv, &lk, prefix);\n+\n+\tcommitable = run_status(stdout, index_file);\n+\n+\trollback_lock_file(&lk);\n+\n+\treturn commitable ? 0 : 1;\n+}\n+\n+static void launch_editor(const char *path, char **buffer, unsigned long *len)\n+{\n+\tconst char *editor, *terminal;\n+\tstruct child_process child;\n+\tconst char *args[3];\n+\n+\teditor = getenv(\"VISUAL\");\n+\tif (!editor)\n+\t\teditor = getenv(\"EDITOR\");\n+\n+\tterminal = getenv(\"TERM\");\n+\tif (!editor && (!terminal || !strcmp(terminal, \"dumb\"))) {\n+\t\tfprintf(stderr, \n+\t\t\t\"Terminal is dumb but no VISUAL nor EDITOR defined.\\n\"\n+\t\t\t\"Please supply the commit log message using either\\n\"\n+\t\t\t\"-m or -F option.  A boilerplate log message has\\n\"\n+\t\t\t\"been prepared in $GIT_DIR/COMMIT_EDITMSG\\n\");\n+\t\texit(1);\n+\t}\n+\n+\tif (!editor)\n+\t\teditor = \"vi\";\n+\t    \n+\tmemset(&child, 0, sizeof(child));\n+\tchild.argv = args;\n+\targs[0] = editor;\n+\targs[1] = path;\n+\targs[2] = NULL;\n+\n+\tif (run_command(&child))\n+\t\tdie(\"could not launch editor %s.\", editor);\n+\n+\tif (read_path(path, buffer, len))\n+\t\tdie(\"could not read commit message file '%s': %s\",\n+\t\t    logfile, strerror(errno));\n+\n+\t*len = strip_lines(*buffer, *len);\n+\n+\tfwrite(*buffer, 1, *len, stderr);\n+}\n+\n+static int run_pre_commit_hook(const char *index_file)\n+{\n+\tstatic const char pre_commit_hook[] = \"hooks/pre-commit\";\n+\tstruct child_process hook;\n+\tconst char *argv[2], *env[2];\n+\tchar index[MAXPATH];\n+\n+\tif (access(git_path(pre_commit_hook), X_OK) < 0)\n+\t\treturn 0;\n+\n+\targv[0] = git_path(update_hook);\n+\targv[1] = NULL;\n+\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n+\tenv[0] = index;\n+\tenv[1] = NULL;\n+\n+\tmemset(&hook, 0, sizeof(hook));\n+\thook.argv = argv;\n+\thook.no_stdin = 1;\n+\thook.stdout_to_stderr = 1;\n+\thook.env = env;\n+\n+\treturn run_command(&proc);\n+}\n+\n+int cmd_commit(int argc, const char **argv, const char *prefix)\n+{\n+\tint fd;\n+\tunsigned long len;\n+\tchar *name, *email, *buffer;\n+\tconst char *index_file;\n+\tstruct lock_file lk;\n+\tstruct stat statbuf;\n+\n+\tparse_and_validate_options(&argv);\n+\n+\tindex_file = prepare_index(argv, &lk, prefix);\n+\n+\tif (!prepare_log_message(index_file)) {\n+\t\trun_status(stdout, index_file);\n+\t\tunlink(commit_editmsg);\n+\t\treturn 1;\n+\t}\n+\n+\tdetermine_author(&name, &email);\n+\n+\tif (!no_edit)\n+\t\tlaunch_editor(commit_editmsg, &buffer, &len);\n+\n+\tif (!no_verify && !stat(git_path(\"hooks/pre-commit\"), &statbuf) &&\n+\t    (statbuf.st_mode & S_IFMT) == \n+\t\t\t\t) {\n+\t\t\n+\t}\n+\n+\tif (commit_locked_index(&lk))\n+\t\tdie(\"failed to write new index\");\n+\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex da4834c..7f395da 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -23,6 +23,7 @@ extern int cmd_check_attr(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry(int argc, const char **argv, const char *prefix);\n extern int cmd_cherry_pick(int argc, const char **argv, const char *prefix);\n+extern int cmd_commit(int argc, const char **argv, const char *prefix);\n extern int cmd_commit_tree(int argc, const char **argv, const char *prefix);\n extern int cmd_count_objects(int argc, const char **argv, const char *prefix);\n extern int cmd_describe(int argc, const char **argv, const char *prefix);\n@@ -63,10 +64,10 @@ extern int cmd_rev_list(int argc, const char **argv, const char *prefix);\n extern int cmd_rev_parse(int argc, const char **argv, const char *prefix);\n extern int cmd_revert(int argc, const char **argv, const char *prefix);\n extern int cmd_rm(int argc, const char **argv, const char *prefix);\n-extern int cmd_runstatus(int argc, const char **argv, const char *prefix);\n extern int cmd_shortlog(int argc, const char **argv, const char *prefix);\n extern int cmd_show(int argc, const char **argv, const char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char *prefix);\n+extern int cmd_status(int argc, const char **argv, const char *prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char *prefix);\n extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);\n extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);\ndiff --git a/cache.h b/cache.h\nindex 5e7381e..0403ada 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -245,7 +245,8 @@ extern int ie_match_stat(struct index_state *, struct cache_entry *, struct stat\n extern int ie_modified(struct index_state *, struct cache_entry *, struct stat *, int);\n extern int ce_path_match(const struct cache_entry *ce, const char **pathspec);\n extern int index_fd(unsigned char *sha1, int fd, struct stat *st, int write_object, enum object_type type, const char *path);\n-extern int read_pipe(int fd, char** return_buf, unsigned long* return_size);\n+extern int read_fd(int fd, char** return_buf, unsigned long* return_size);\n+extern int read_path(const char *path, char** return_buf, unsigned long* return_size);\n extern int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object);\n extern int index_path(unsigned char *sha1, const char *path, struct stat *st, int write_object);\n extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st);\ndiff --git a/color.c b/color.c\nindex 09d82ee..124ba33 100644\n--- a/color.c\n+++ b/color.c\n@@ -135,39 +135,39 @@ int git_config_colorbool(const char *var, const char *value)\n \treturn git_config_bool(var, value);\n }\n \n-static int color_vprintf(const char *color, const char *fmt,\n+static int color_vfprintf(FILE *fp, const char *color, const char *fmt,\n \t\tva_list args, const char *trail)\n {\n \tint r = 0;\n \n \tif (*color)\n-\t\tr += printf(\"%s\", color);\n-\tr += vprintf(fmt, args);\n+\t\tr += fprintf(fp, \"%s\", color);\n+\tr += vfprintf(fp, fmt, args);\n \tif (*color)\n-\t\tr += printf(\"%s\", COLOR_RESET);\n+\t\tr += fprintf(fp, \"%s\", COLOR_RESET);\n \tif (trail)\n-\t\tr += printf(\"%s\", trail);\n+\t\tr += fprintf(fp, \"%s\", trail);\n \treturn r;\n }\n \n \n \n-int color_printf(const char *color, const char *fmt, ...)\n+int color_fprintf(FILE *fp, const char *color, const char *fmt, ...)\n {\n \tva_list args;\n \tint r;\n \tva_start(args, fmt);\n-\tr = color_vprintf(color, fmt, args, NULL);\n+\tr = color_vfprintf(fp, color, fmt, args, NULL);\n \tva_end(args);\n \treturn r;\n }\n \n-int color_printf_ln(const char *color, const char *fmt, ...)\n+int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...)\n {\n \tva_list args;\n \tint r;\n \tva_start(args, fmt);\n-\tr = color_vprintf(color, fmt, args, \"\\n\");\n+\tr = color_vfprintf(fp, color, fmt, args, \"\\n\");\n \tva_end(args);\n \treturn r;\n }\ndiff --git a/color.h b/color.h\nindex 88bb8ff..6809800 100644\n--- a/color.h\n+++ b/color.h\n@@ -6,7 +6,7 @@\n \n int git_config_colorbool(const char *var, const char *value);\n void color_parse(const char *var, const char *value, char *dst);\n-int color_printf(const char *color, const char *fmt, ...);\n-int color_printf_ln(const char *color, const char *fmt, ...);\n+int color_fprintf(FILE *fp, const char *color, const char *fmt, ...);\n+int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);\n \n #endif /* COLOR_H */\ndiff --git a/git.c b/git.c\nindex 29b55a1..4018e3c 100644\n--- a/git.c\n+++ b/git.c\n@@ -237,6 +237,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)\n \t\t{ \"check-attr\", cmd_check_attr, RUN_SETUP | NOT_BARE },\n \t\t{ \"cherry\", cmd_cherry, RUN_SETUP },\n \t\t{ \"cherry-pick\", cmd_cherry_pick, RUN_SETUP | NOT_BARE },\n+\t\t{ \"commit\", cmd_commit, RUN_SETUP },\n \t\t{ \"commit-tree\", cmd_commit_tree, RUN_SETUP },\n \t\t{ \"config\", cmd_config },\n \t\t{ \"count-objects\", cmd_count_objects, RUN_SETUP },\n@@ -279,10 +280,10 @@ static void handle_internal_command(int argc, const char **argv, char **envp)\n \t\t{ \"rev-parse\", cmd_rev_parse, RUN_SETUP },\n \t\t{ \"revert\", cmd_revert, RUN_SETUP | NOT_BARE },\n \t\t{ \"rm\", cmd_rm, RUN_SETUP | NOT_BARE },\n-\t\t{ \"runstatus\", cmd_runstatus, RUN_SETUP | NOT_BARE },\n \t\t{ \"shortlog\", cmd_shortlog, RUN_SETUP | USE_PAGER },\n \t\t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n \t\t{ \"show\", cmd_show, RUN_SETUP | USE_PAGER },\n+\t\t{ \"status\", cmd_status, RUN_SETUP | NOT_BARE },\n \t\t{ \"stripspace\", cmd_stripspace },\n \t\t{ \"symbolic-ref\", cmd_symbolic_ref, RUN_SETUP },\n \t\t{ \"tar-tree\", cmd_tar_tree },\ndiff --git a/mktag.c b/mktag.c\nindex b82e377..26b9ebf 100644\n--- a/mktag.c\n+++ b/mktag.c\n@@ -111,8 +111,8 @@ static int verify_tag(char *buffer, unsigned long size)\n \n int main(int argc, char **argv)\n {\n-\tunsigned long size = 4096;\n-\tchar *buffer = xmalloc(size);\n+\tunsigned long size;\n+\tchar *buffer;\n \tunsigned char result_sha1[20];\n \n \tif (argc != 1)\n@@ -120,10 +120,8 @@ int main(int argc, char **argv)\n \n \tsetup_git_directory();\n \n-\tif (read_pipe(0, &buffer, &size)) {\n-\t\tfree(buffer);\n+\tif (read_fd(0, &buffer, &size))\n \t\tdie(\"could not read from stdin\");\n-\t}\n \n \t/* Verify it for some basic sanity: it needs to start with\n \t   \"object <sha1>\\ntype\\ntagger \" */\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 2b86086..91e8854 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -2286,18 +2286,17 @@ int has_sha1_file(const unsigned char *sha1)\n }\n \n /*\n- * reads from fd as long as possible into a supplied buffer of size bytes.\n- * If necessary the buffer's size is increased using realloc()\n+ * reads from fd as long as possible and allocates a buffer to hold\n+ * the contents.  The buffer and size of the contents is returned in\n+ * *return_buf and *return_size.  In case of failure, the allocated\n+ * buffers are freed, otherwise, the buffer must be freed using xfree.\n  *\n  * returns 0 if anything went fine and -1 otherwise\n- *\n- * NOTE: both buf and size may change, but even when -1 is returned\n- * you still have to free() it yourself.\n  */\n-int read_pipe(int fd, char** return_buf, unsigned long* return_size)\n+int read_fd(int fd, char** return_buf, unsigned long* return_size)\n {\n-\tchar* buf = *return_buf;\n-\tunsigned long size = *return_size;\n+\tunsigned long size = 4096;\n+\tchar* buf = xmalloc(size);\n \tssize_t iret;\n \tunsigned long off = 0;\n \n@@ -2315,21 +2314,38 @@ int read_pipe(int fd, char** return_buf, unsigned long* return_size)\n \t*return_buf = buf;\n \t*return_size = off;\n \n-\tif (iret < 0)\n+\tif (iret < 0) {\n+\t\tfree(buf);\n+\t\treturn -1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+int read_path(const char *path, char** return_buf, unsigned long* return_size)\n+{\n+\tint fd; \n+\n+\tfd = open(path, O_RDONLY);\n+\tif (fd < 0)\n+\t\treturn -1;\n+\tif (read_fd(fd, return_buf, return_size)) {\n+\t\tclose(fd);\n \t\treturn -1;\n+\t}\n+\tclose(fd);\n+\n \treturn 0;\n }\n \n int index_pipe(unsigned char *sha1, int fd, const char *type, int write_object)\n {\n-\tunsigned long size = 4096;\n-\tchar *buf = xmalloc(size);\n+\tunsigned long size;\n+\tchar *buf;\n \tint ret;\n \n-\tif (read_pipe(fd, &buf, &size)) {\n-\t\tfree(buf);\n+\tif (read_fd(fd, &buf, &size))\n \t\treturn -1;\n-\t}\n \n \tif (!type)\n \t\ttype = blob_type;\ndiff --git a/wt-status.c b/wt-status.c\nindex 5205420..463ecd7 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -54,29 +54,30 @@ void wt_status_prepare(struct wt_status *s)\n \ts->reference = \"HEAD\";\n }\n \n-static void wt_status_print_cached_header(const char *reference)\n+static void wt_status_print_cached_header(struct wt_status *s)\n {\n \tconst char *c = color(WT_STATUS_HEADER);\n-\tcolor_printf_ln(c, \"# Changes to be committed:\");\n-\tif (reference) {\n-\t\tcolor_printf_ln(c, \"#   (use \\\"git reset %s <file>...\\\" to unstage)\", reference);\n+\tcolor_fprintf_ln(s->fp, c, \"# Changes to be committed:\");\n+\tif (s->reference) {\n+\t\tcolor_fprintf_ln(s->fp, c, \"#   (use \\\"git reset %s <file>...\\\" to unstage)\", s->reference);\n \t} else {\n-\t\tcolor_printf_ln(c, \"#   (use \\\"git rm --cached <file>...\\\" to unstage)\");\n+\t\tcolor_fprintf_ln(s->fp, c, \"#   (use \\\"git rm --cached <file>...\\\" to unstage)\");\n \t}\n-\tcolor_printf_ln(c, \"#\");\n+\tcolor_fprintf_ln(s->fp, c, \"#\");\n }\n \n-static void wt_status_print_header(const char *main, const char *sub)\n+static void wt_status_print_header(struct wt_status *s,\n+\t\t\t\t   const char *main, const char *sub)\n {\n \tconst char *c = color(WT_STATUS_HEADER);\n-\tcolor_printf_ln(c, \"# %s:\", main);\n-\tcolor_printf_ln(c, \"#   (%s)\", sub);\n-\tcolor_printf_ln(c, \"#\");\n+\tcolor_fprintf_ln(s->fp, c, \"# %s:\", main);\n+\tcolor_fprintf_ln(s->fp, c, \"#   (%s)\", sub);\n+\tcolor_fprintf_ln(s->fp, c, \"#\");\n }\n \n-static void wt_status_print_trailer(void)\n+static void wt_status_print_trailer(struct wt_status *s)\n {\n-\tcolor_printf_ln(color(WT_STATUS_HEADER), \"#\");\n+\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"#\");\n }\n \n static const char *quote_crlf(const char *in, char *buf, size_t sz)\n@@ -108,7 +109,8 @@ static const char *quote_crlf(const char *in, char *buf, size_t sz)\n \treturn ret;\n }\n \n-static void wt_status_print_filepair(int t, struct diff_filepair *p)\n+static void wt_status_print_filepair(struct wt_status *s,\n+\t\t\t\t     int t, struct diff_filepair *p)\n {\n \tconst char *c = color(t);\n \tconst char *one, *two;\n@@ -117,36 +119,36 @@ static void wt_status_print_filepair(int t, struct diff_filepair *p)\n \tone = quote_crlf(p->one->path, onebuf, sizeof(onebuf));\n \ttwo = quote_crlf(p->two->path, twobuf, sizeof(twobuf));\n \n-\tcolor_printf(color(WT_STATUS_HEADER), \"#\\t\");\n+\tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n \tswitch (p->status) {\n \tcase DIFF_STATUS_ADDED:\n-\t\tcolor_printf(c, \"new file:   %s\", one);\n+\t\tcolor_fprintf(s->fp, c, \"new file:   %s\", one);\n \t\tbreak;\n \tcase DIFF_STATUS_COPIED:\n-\t\tcolor_printf(c, \"copied:     %s -> %s\", one, two);\n+\t\tcolor_fprintf(s->fp, c, \"copied:     %s -> %s\", one, two);\n \t\tbreak;\n \tcase DIFF_STATUS_DELETED:\n-\t\tcolor_printf(c, \"deleted:    %s\", one);\n+\t\tcolor_fprintf(s->fp, c, \"deleted:    %s\", one);\n \t\tbreak;\n \tcase DIFF_STATUS_MODIFIED:\n-\t\tcolor_printf(c, \"modified:   %s\", one);\n+\t\tcolor_fprintf(s->fp, c, \"modified:   %s\", one);\n \t\tbreak;\n \tcase DIFF_STATUS_RENAMED:\n-\t\tcolor_printf(c, \"renamed:    %s -> %s\", one, two);\n+\t\tcolor_fprintf(s->fp, c, \"renamed:    %s -> %s\", one, two);\n \t\tbreak;\n \tcase DIFF_STATUS_TYPE_CHANGED:\n-\t\tcolor_printf(c, \"typechange: %s\", one);\n+\t\tcolor_fprintf(s->fp, c, \"typechange: %s\", one);\n \t\tbreak;\n \tcase DIFF_STATUS_UNKNOWN:\n-\t\tcolor_printf(c, \"unknown:    %s\", one);\n+\t\tcolor_fprintf(s->fp, c, \"unknown:    %s\", one);\n \t\tbreak;\n \tcase DIFF_STATUS_UNMERGED:\n-\t\tcolor_printf(c, \"unmerged:   %s\", one);\n+\t\tcolor_fprintf(s->fp, c, \"unmerged:   %s\", one);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"bug: unhandled diff status %c\", p->status);\n \t}\n-\tprintf(\"\\n\");\n+\tfprintf(s->fp, \"\\n\");\n }\n \n static void wt_status_print_updated_cb(struct diff_queue_struct *q,\n@@ -160,14 +162,14 @@ static void wt_status_print_updated_cb(struct diff_queue_struct *q,\n \t\tif (q->queue[i]->status == 'U')\n \t\t\tcontinue;\n \t\tif (!shown_header) {\n-\t\t\twt_status_print_cached_header(s->reference);\n+\t\t\twt_status_print_cached_header(s);\n \t\t\ts->commitable = 1;\n \t\t\tshown_header = 1;\n \t\t}\n-\t\twt_status_print_filepair(WT_STATUS_UPDATED, q->queue[i]);\n+\t\twt_status_print_filepair(s, WT_STATUS_UPDATED, q->queue[i]);\n \t}\n \tif (shown_header)\n-\t\twt_status_print_trailer();\n+\t\twt_status_print_trailer(s);\n }\n \n static void wt_status_print_changed_cb(struct diff_queue_struct *q,\n@@ -184,18 +186,18 @@ static void wt_status_print_changed_cb(struct diff_queue_struct *q,\n \t\t\t\tmsg = use_add_rm_msg;\n \t\t\t\tbreak;\n \t\t\t}\n-\t\twt_status_print_header(\"Changed but not updated\", msg);\n+\t\twt_status_print_header(s, \"Changed but not updated\", msg);\n \t}\n \tfor (i = 0; i < q->nr; i++)\n-\t\twt_status_print_filepair(WT_STATUS_CHANGED, q->queue[i]);\n+\t\twt_status_print_filepair(s, WT_STATUS_CHANGED, q->queue[i]);\n \tif (q->nr)\n-\t\twt_status_print_trailer();\n+\t\twt_status_print_trailer(s);\n }\n \n static void wt_read_cache(struct wt_status *s)\n {\n \tdiscard_cache();\n-\tread_cache();\n+\tread_cache_from(s->index_file);\n }\n \n static void wt_status_print_initial(struct wt_status *s)\n@@ -209,13 +211,13 @@ static void wt_status_print_initial(struct wt_status *s)\n \t\twt_status_print_cached_header(NULL);\n \t}\n \tfor (i = 0; i < active_nr; i++) {\n-\t\tcolor_printf(color(WT_STATUS_HEADER), \"#\\t\");\n-\t\tcolor_printf_ln(color(WT_STATUS_UPDATED), \"new file: %s\",\n+\t\tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n+\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_UPDATED), \"new file: %s\",\n \t\t\t\tquote_crlf(active_cache[i]->name,\n \t\t\t\t\t   buf, sizeof(buf)));\n \t}\n \tif (active_nr)\n-\t\twt_status_print_trailer();\n+\t\twt_status_print_trailer(s);\n }\n \n static void wt_status_print_updated(struct wt_status *s)\n@@ -281,12 +283,12 @@ static void wt_status_print_untracked(struct wt_status *s)\n \t\t}\n \t\tif (!shown_header) {\n \t\t\ts->workdir_untracked = 1;\n-\t\t\twt_status_print_header(\"Untracked files\",\n+\t\t\twt_status_print_header(s, \"Untracked files\",\n \t\t\t\t\t       use_add_to_include_msg);\n \t\t\tshown_header = 1;\n \t\t}\n-\t\tcolor_printf(color(WT_STATUS_HEADER), \"#\\t\");\n-\t\tcolor_printf_ln(color(WT_STATUS_UNTRACKED), \"%.*s\",\n+\t\tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n+\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_UNTRACKED), \"%.*s\",\n \t\t\t\tent->len, ent->name);\n \t}\n }\n@@ -316,14 +318,14 @@ void wt_status_print(struct wt_status *s)\n \t\t\tbranch_name = \"\";\n \t\t\ton_what = \"Not currently on any branch.\";\n \t\t}\n-\t\tcolor_printf_ln(color(WT_STATUS_HEADER),\n+\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER),\n \t\t\t\"# %s%s\", on_what, branch_name);\n \t}\n \n \tif (s->is_initial) {\n-\t\tcolor_printf_ln(color(WT_STATUS_HEADER), \"#\");\n-\t\tcolor_printf_ln(color(WT_STATUS_HEADER), \"# Initial commit\");\n-\t\tcolor_printf_ln(color(WT_STATUS_HEADER), \"#\");\n+\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"#\");\n+\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"# Initial commit\");\n+\t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"#\");\n \t\twt_status_print_initial(s);\n \t}\n \telse {\n@@ -337,7 +339,7 @@ void wt_status_print(struct wt_status *s)\n \t\twt_status_print_verbose(s);\n \tif (!s->commitable) {\n \t\tif (s->amend)\n-\t\t\tprintf(\"# No changes\\n\");\n+\t\t\tfprintf(s->fp, \"# No changes\\n\");\n \t\telse if (s->workdir_dirty)\n \t\t\tprintf(\"no changes added to commit (use \\\"git add\\\" and/or \\\"git commit -a\\\")\\n\");\n \t\telse if (s->workdir_untracked)\ndiff --git a/wt-status.h b/wt-status.h\nindex cfea4ae..7744932 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -1,6 +1,8 @@\n #ifndef STATUS_H\n #define STATUS_H\n \n+#include <stdio.h>\n+\n enum color_wt_status {\n \tWT_STATUS_HEADER,\n \tWT_STATUS_UPDATED,\n@@ -19,6 +21,8 @@ struct wt_status {\n \tint commitable;\n \tint workdir_dirty;\n \tint workdir_untracked;\n+\tconst char *index_file;\n+\tFILE *fp;\n };\n \n int git_status_config(const char *var, const char *value);\n-- \n1.5.2.2\n"},{"id":"46251","messageId":"Pine.LNX.4.64.0707021709120.4071@racer.site","threadId":"8810","inReplyTo":"11833861634103-git-send-email-krh@redhat.com","subject":"Re: [RFC] Update on builtin-commit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-02T16:11:11Z","receivedAt":"2007-07-02T16:11:11Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\njust a quick comment on the option parser:\n\nOn most platforms, sizeof(void*)>=sizeof(int). But I would not rely on \nthat. Rather (also because it is prettier), I'd use \"union\".\n\nBesides, your option parser loses order information, correct? IOW, \nsomething like \"--color --no-color --color\" would confuse it.\n\nCiao,\nDscho\n"},{"id":"46254","messageId":"1183395082.30611.16.camel@hinata.boston.redhat.com","threadId":"8810","inReplyTo":"Pine.LNX.4.64.0707021709120.4071@racer.site","subject":"Re: [RFC] Update on builtin-commit","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-07-02T16:51:22Z","receivedAt":"2007-07-02T16:51:22Z","isPatch":false,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Mon, 2007-07-02 at 17:11 +0100, Johannes Schindelin wrote:\n> just a quick comment on the option parser:\n> \n> On most platforms, sizeof(void*)>=sizeof(int). But I would not rely on \n> that. Rather (also because it is prettier), I'd use \"union\".\n\nIn the OPTION_INTEGER case, the 'value' void pointer points to an\ninteger global that's set to the value passed.  In the OPTION_NONE, it\nalso points to an integer, which is set to 1 if the option is seen.  So\nI'm relying on sizeof(void*) == sizeof(int*), but I'm not storing ints\nin pointers.\n\n> Besides, your option parser loses order information, correct? IOW, \n> something like \"--color --no-color --color\" would confuse it.\n\nYes, I don't record the order of options, but in the builtin-commit\ncase, I don't think there are any options where that makes a difference?\nIn cases where order is important or we have an option that negates the\neffect of another option (your --no-color example), we could either 1)\nextend the option struct with a 'disable' name that flips the value back\nto 0 or 2) instead of just setting it to 1, record the index of the\noptions passed and compare the indexes of conflicting options to see\nwhich one was passed last.\n\nKristian\n"},{"id":"46256","messageId":"Pine.LNX.4.64.0707021758090.4071@racer.site","threadId":"8810","inReplyTo":"1183395082.30611.16.camel@hinata.boston.redhat.com","subject":"Re: [RFC] Update on builtin-commit","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-02T17:02:05Z","receivedAt":"2007-07-02T17:02:05Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 2 Jul 2007, Kristian H?gsberg wrote:\n\n> On Mon, 2007-07-02 at 17:11 +0100, Johannes Schindelin wrote:\n> > just a quick comment on the option parser:\n> > \n> > On most platforms, sizeof(void*)>=sizeof(int). But I would not rely on \n> > that. Rather (also because it is prettier), I'd use \"union\".\n> \n> In the OPTION_INTEGER case, the 'value' void pointer points to an \n> integer global that's set to the value passed.  In the OPTION_NONE, it \n> also points to an integer, which is set to 1 if the option is seen.  So \n> I'm relying on sizeof(void*) == sizeof(int*), but I'm not storing ints \n> in pointers.\n\nAh, right.\n\n> > Besides, your option parser loses order information, correct? IOW, \n> > something like \"--color --no-color --color\" would confuse it.\n> \n> Yes, I don't record the order of options, but in the builtin-commit \n> case, I don't think there are any options where that makes a difference?\n\nThat might be correct now. But why not make the option parser general \nenough to (finally!) support something like \"git clone -lns <directory>\", \ni.e. short options a la GNU? This is something that has been wanted for a \nlong time.\n\nBesides, if you make the option parser not general enough to be reused in \nall git programs, I wonder why bother at all? It's not like it is less \ncomplex than a hand-rolled option parser, if it is used only once.\n\n> In cases where order is important or we have an option that negates the \n> effect of another option (your --no-color example), we could either 1) \n> extend the option struct with a 'disable' name that flips the value back \n> to 0 or 2) instead of just setting it to 1, record the index of the \n> options passed and compare the indexes of conflicting options to see \n> which one was passed last.\n\nHmm. Somehow I think that the getopt solution is not so bad at all. We'd \nneed some code in compat/, but since we're GPL, and there are so many \nGPLed getopt versions out there, I don't see any obstacle there.\n\nCiao,\nDscho\n"},{"id":"46260","messageId":"1183397689.10996.11.camel@lt21223.campus.dmacc.edu","threadId":"8810","inReplyTo":"Pine.LNX.4.64.0707021758090.4071@racer.site","subject":"Re: [RFC] Update on builtin-commit","fromName":"Jeffrey C. Ollie","fromEmail":"jeff@ocjtech.us","sentAt":"2007-07-02T17:34:48Z","receivedAt":"2007-07-02T17:34:48Z","isPatch":false,"sender":{"key":"jeff@ocjtech.us","avatar":"https://gravatar.com/avatar/95918a1992f277a811c471ae7275f7e4c9d1a2e517ad290bd6aa93b97e8d34f3?d=mp&s=160"},"body":"On Mon, 2007-07-02 at 18:02 +0100, Johannes Schindelin wrote:\n>\n> Hmm. Somehow I think that the getopt solution is not so bad at all. We'd \n> need some code in compat/, but since we're GPL, and there are so many \n> GPLed getopt versions out there, I don't see any obstacle there.\n\nIf we are going to make this option parser into some complex\ngeneral-purpose option parsing library let's not re-invent the wheel.\nLet's pick one of the GPL'd option parsing libraries and make it a\ndependency of Git.\n\nJeff\n\n"},{"id":"46261","messageId":"1183399050.30611.25.camel@hinata.boston.redhat.com","threadId":"8810","inReplyTo":"1183397689.10996.11.camel@lt21223.campus.dmacc.edu","subject":"Re: [RFC] Update on builtin-commit","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-07-02T17:57:30Z","receivedAt":"2007-07-02T17:57:30Z","isPatch":false,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Mon, 2007-07-02 at 12:34 -0500, Jeffrey C. Ollie wrote:\n> On Mon, 2007-07-02 at 18:02 +0100, Johannes Schindelin wrote:\n> >\n> > Hmm. Somehow I think that the getopt solution is not so bad at all. We'd \n> > need some code in compat/, but since we're GPL, and there are so many \n> > GPLed getopt versions out there, I don't see any obstacle there.\n> \n> If we are going to make this option parser into some complex\n> general-purpose option parsing library let's not re-invent the wheel.\n> Let's pick one of the GPL'd option parsing libraries and make it a\n> dependency of Git.\n\nI don't have much of an opinion here; as I've said before, my goal here\nis to get commit ported to C, and I specifically don't want to block on\nthe option parser discussion reaching consensus.  One thing I do not\nwant to do, though, is to explode the current table driven approach into\na gazillion strcmps.  Other than that I'm open to porting it to an\nexternal getopt dependency, adding the couple of missing features\nJohannes mentioned (bundling and ordering), or just keeping it local to\nbuiltin-commit.c as is.\n\nThat said, we're debating less than 100 lines of code.  Adding the\nbundling of short options and some kind of ordering mechanism would add\nat most 20 more lines.  Is it worth taking a getopt dependency for that?\n\nKristian\n"}]}