{"thread":{"id":"14540","subject":"[PATCH v2 1/4] builtin-add.c: restructure the code for maintainability","startedAt":"2008-07-20T06:09:01Z","lastAt":"2008-07-23T19:11:41Z","messageCount":16,"participants":["Junio C Hamano","Johannes Schindelin","Pierre Habouzit"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"83995","messageId":"1216534144-23826-1-git-send-email-gitster@pobox.com","threadId":"14540","inReplyTo":null,"subject":"[PATCH v2 1/4] builtin-add.c: restructure the code for maintainability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-20T06:09:01Z","receivedAt":"2008-07-20T06:09:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The implementation of \"git add\" has four major codepaths that are mutually\nexclusive:\n\n - if \"--interactive\" or \"--patch\" is given, spawn \"git add--interactive\"\n   and exit without doing anything else.  Otherwise things are handled\n   internally in this C code;\n\n - if \"--update\" is given, update the modified files and exit without\n   doing anything else;\n\n - if \"--refresh\" is given, do refresh and exit without doing anything\n   else;\n\n - otherwise, find the paths that match pathspecs and stage their\n   contents.\n\nIt led to an unholy mess in the code structure; each of the latter three\ncodepaths has a separate call to read_cache(), even though they are all\nabout \"read the current index, update it and write it back\", and logically\nthey should read the index once _anyway_.\n\nThis cleans up the latter three cases by introducing a pair of helper\nvariables:\n\n - \"add_new_files\" is set if we need to scan the working tree for paths\n   that match the pathspec.  This variable is false for \"--update\" and\n   \"--refresh\", because they only work on already tracked files.\n\n - \"require_pathspec\" is set if the user must give at least one pathspec.\n   \"--update\" does not need it but all the other cases do.\n\nThis is in preparation for introducing a new option \"--all\", that does the\nequivalent of \"git add -u && git add .\" (aka \"addremove\").\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin-add.c |   75 ++++++++++++++++++++++++++++++++------------------------\n 1 files changed, 43 insertions(+), 32 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex bf13aa3..9b2ee8c 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -140,8 +140,6 @@ static void refresh(int verbose, const char **pathspec)\n \tfor (specs = 0; pathspec[specs];  specs++)\n \t\t/* nothing */;\n \tseen = xcalloc(specs, 1);\n-\tif (read_cache() < 0)\n-\t\tdie(\"index file corrupt\");\n \trefresh_index(&the_index, verbose ? 0 : REFRESH_QUIET, pathspec, seen);\n \tfor (i = 0; i < specs; i++) {\n \t\tif (!seen[i])\n@@ -216,13 +214,36 @@ static int add_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n+static int add_files(struct dir_struct *dir, int flags)\n+{\n+\tint i, exit_status = 0;\n+\n+\tif (dir->ignored_nr) {\n+\t\tfprintf(stderr, ignore_error);\n+\t\tfor (i = 0; i < dir->ignored_nr; i++)\n+\t\t\tfprintf(stderr, \"%s\\n\", dir->ignored[i]->name);\n+\t\tfprintf(stderr, \"Use -f if you really want to add them.\\n\");\n+\t\tdie(\"no files added\");\n+\t}\n+\n+\tfor (i = 0; i < dir->nr; i++)\n+\t\tif (add_file_to_cache(dir->entries[i]->name, flags)) {\n+\t\t\tif (!ignore_add_errors)\n+\t\t\t\tdie(\"adding files failed\");\n+\t\t\texit_status = 1;\n+\t\t}\n+\treturn exit_status;\n+}\n+\n int cmd_add(int argc, const char **argv, const char *prefix)\n {\n \tint exit_status = 0;\n-\tint i, newfd;\n+\tint newfd;\n \tconst char **pathspec;\n \tstruct dir_struct dir;\n \tint flags;\n+\tint add_new_files;\n+\tint require_pathspec;\n \n \targc = parse_options(argc, argv, builtin_add_options,\n \t\t\t  builtin_add_usage, 0);\n@@ -233,53 +254,43 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tgit_config(add_config, NULL);\n \n+\tadd_new_files = !take_worktree_changes && !refresh_only;\n+\trequire_pathspec = !take_worktree_changes;\n+\n \tnewfd = hold_locked_index(&lock_file, 1);\n \n \tflags = ((verbose ? ADD_CACHE_VERBOSE : 0) |\n \t\t (show_only ? ADD_CACHE_PRETEND : 0) |\n \t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0));\n \n-\tif (take_worktree_changes) {\n-\t\tconst char **pathspec;\n-\t\tif (read_cache() < 0)\n-\t\t\tdie(\"index file corrupt\");\n-\t\tpathspec = get_pathspec(prefix, argv);\n-\t\texit_status = add_files_to_cache(prefix, pathspec, flags);\n-\t\tgoto finish;\n-\t}\n-\n-\tif (argc == 0) {\n+\tif (require_pathspec && argc == 0) {\n \t\tfprintf(stderr, \"Nothing specified, nothing added.\\n\");\n \t\tfprintf(stderr, \"Maybe you wanted to say 'git add .'?\\n\");\n \t\treturn 0;\n \t}\n \tpathspec = get_pathspec(prefix, argv);\n \n-\tif (refresh_only) {\n-\t\trefresh(verbose, pathspec);\n-\t\tgoto finish;\n-\t}\n-\n-\tfill_directory(&dir, pathspec, ignored_too);\n+\t/*\n+\t * If we are adding new files, we need to scan the working\n+\t * tree to find the ones that match pathspecs; this needs\n+\t * to be done before we read the index.\n+\t */\n+\tif (add_new_files)\n+\t\tfill_directory(&dir, pathspec, ignored_too);\n \n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\n \n-\tif (dir.ignored_nr) {\n-\t\tfprintf(stderr, ignore_error);\n-\t\tfor (i = 0; i < dir.ignored_nr; i++) {\n-\t\t\tfprintf(stderr, \"%s\\n\", dir.ignored[i]->name);\n-\t\t}\n-\t\tfprintf(stderr, \"Use -f if you really want to add them.\\n\");\n-\t\tdie(\"no files added\");\n+\tif (refresh_only) {\n+\t\trefresh(verbose, pathspec);\n+\t\tgoto finish;\n \t}\n \n-\tfor (i = 0; i < dir.nr; i++)\n-\t\tif (add_file_to_cache(dir.entries[i]->name, flags)) {\n-\t\t\tif (!ignore_add_errors)\n-\t\t\t\tdie(\"adding files failed\");\n-\t\t\texit_status = 1;\n-\t\t}\n+\tif (take_worktree_changes)\n+\t\texit_status |= add_files_to_cache(prefix, pathspec, flags);\n+\n+\tif (add_new_files)\n+\t\texit_status |= add_files(&dir, flags);\n \n  finish:\n \tif (active_cache_changed) {\n-- \n1.5.6.4.570.g052e6\n"},{"id":"83996","messageId":"1216534144-23826-2-git-send-email-gitster@pobox.com","threadId":"14540","inReplyTo":"1216534144-23826-1-git-send-email-gitster@pobox.com","subject":"[PATCH v2 2/4] git-add --all: add all files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-20T06:09:02Z","receivedAt":"2008-07-20T06:09:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"People sometimes find that \"git add -u && git add .\" are 13 keystrokes too\nmany.  This reduces it by nine.\n\nThe support of this has been very low priority for me personally, because\nI almost never do \"git add .\" in a directory with already tracked files,\nand in a new directory, there is no point saying \"git add -u\".\n\nHowever, for two types of people (that are very different from me), this\nmode of operation may make sense and there is no reason to leave it\nunsupported.  That is:\n\n (1) If you are extremely well disciplined and keep perfect .gitignore, it\n     always is safe to say \"git add .\"; or\n\n (2) If you are extremely undisciplined and do not even know what files\n     you created, and you do not very much care what goes in your history,\n     it does not matter if \"git add .\" included everything.\n\nSo there it is, although I suspect I will not use it myself, ever.\n\nIt will be too much of a change that is against the expectation of the\nexisting users to allow \"git commit -a\" to include untracked files, and\nit would be inconsistent if we named this new option \"-a\", so the short\noption is \"-A\".  We _might_ want to later add \"git commit -A\" but that is\na separate topic.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This option is different from what \"commit -a\" does, so it must be\n   named differently.  v2 patch uses -A as the short option.\n\n builtin-add.c |   13 +++++++++++--\n 1 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 9b2ee8c..6f5672a 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -190,7 +190,7 @@ static const char ignore_error[] =\n \"The following paths are ignored by one of your .gitignore files:\\n\";\n \n static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;\n-static int ignore_add_errors;\n+static int ignore_add_errors, addremove;\n \n static struct option builtin_add_options[] = {\n \tOPT__DRY_RUN(&show_only),\n@@ -200,6 +200,7 @@ static struct option builtin_add_options[] = {\n \tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactive patching\"),\n \tOPT_BOOLEAN('f', \"force\", &ignored_too, \"allow adding otherwise ignored files\"),\n \tOPT_BOOLEAN('u', \"update\", &take_worktree_changes, \"update tracked files\"),\n+\tOPT_BOOLEAN('A', \"all\", &addremove, \"add all, noticing removal of tracked files\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\n \tOPT_BOOLEAN( 0 , \"ignore-errors\", &ignore_add_errors, \"just skip files which cannot be added because of errors\"),\n \tOPT_END(),\n@@ -254,6 +255,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tgit_config(add_config, NULL);\n \n+\tif (addremove && take_worktree_changes)\n+\t\tdie(\"-A and -u are mutually incompatible\");\n+\tif (addremove && !argc) {\n+\t\tstatic const char *here[2] = { \".\", NULL };\n+\t\targc = 1;\n+\t\targv = here;\n+\t}\n+\n \tadd_new_files = !take_worktree_changes && !refresh_only;\n \trequire_pathspec = !take_worktree_changes;\n \n@@ -286,7 +295,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\tgoto finish;\n \t}\n \n-\tif (take_worktree_changes)\n+\tif (take_worktree_changes || addremove)\n \t\texit_status |= add_files_to_cache(prefix, pathspec, flags);\n \n \tif (add_new_files)\n-- \n1.5.6.4.570.g052e6\n"},{"id":"83997","messageId":"1216534144-23826-3-git-send-email-gitster@pobox.com","threadId":"14540","inReplyTo":"1216534144-23826-2-git-send-email-gitster@pobox.com","subject":"[PATCH v2 3/4] git-add --all: tests","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-20T06:09:03Z","receivedAt":"2008-07-20T06:09:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"And here is a small test script that makes sure that:\n\n - both modified and new files are included,\n - removed file is noticed, and\n - no ignored file is included.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * This option has to be a superset of -u, so it must also notice removed\n   files.  v2 adds that to the test.\n\n t/t2202-add-addremove.sh |   44 ++++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 44 insertions(+), 0 deletions(-)\n create mode 100755 t/t2202-add-addremove.sh\n\ndiff --git a/t/t2202-add-addremove.sh b/t/t2202-add-addremove.sh\nnew file mode 100755\nindex 0000000..6a81510\n--- /dev/null\n+++ b/t/t2202-add-addremove.sh\n@@ -0,0 +1,44 @@\n+#!/bin/sh\n+\n+test_description='git add --all'\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\t(\n+\t\techo .gitignore\n+\t\techo will-remove\n+\t) >expect &&\n+\t(\n+\t\techo actual\n+\t\techo expect\n+\t\techo ignored\n+\t) >.gitignore &&\n+\t>will-remove &&\n+\tgit add --all &&\n+\ttest_tick &&\n+\tgit commit -m initial &&\n+\tgit ls-files >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'git add --all' '\n+\t(\n+\t\techo .gitignore\n+\t\techo not-ignored\n+\t\techo \"M\t.gitignore\"\n+\t\techo \"A\tnot-ignored\"\n+\t\techo \"D\twill-remove\"\n+\t) >expect &&\n+\t>ignored &&\n+\t>not-ignored &&\n+\techo modification >>.gitignore &&\n+\trm -f will-remove &&\n+\tgit add --all &&\n+\tgit update-index --refresh &&\n+\tgit ls-files >actual &&\n+\tgit diff-index --name-status --cached HEAD >>actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\n-- \n1.5.6.4.570.g052e6\n"},{"id":"83998","messageId":"1216534144-23826-4-git-send-email-gitster@pobox.com","threadId":"14540","inReplyTo":"1216534144-23826-3-git-send-email-gitster@pobox.com","subject":"[PATCH v2 4/4] git-add --all: documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-20T06:09:04Z","receivedAt":"2008-07-20T06:09:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Signed-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/git-add.txt |    8 +++++++-\n 1 files changed, 7 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 3558905..2b6d6c8 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -9,7 +9,7 @@ SYNOPSIS\n --------\n [verse]\n 'git add' [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]\n-\t  [--update | -u] [--refresh] [--ignore-errors] [--]\n+\t  [--all | [--update | -u]] [--refresh] [--ignore-errors] [--]\n \t  <filepattern>...\n \n DESCRIPTION\n@@ -86,6 +86,12 @@ OPTIONS\n \tcommand line. If no paths are specified, all tracked files in the\n \tcurrent directory and its subdirectories are updated.\n \n+-A::\n+--all::\n+\tUpdate files that git already knows about (same as '\\--update')\n+\tand add all untracked files that are not ignored by '.gitignore'\n+\tmechanism.\n+\n --refresh::\n \tDon't add the file(s), but only refresh their stat()\n \tinformation in the index.\n-- \n1.5.6.4.570.g052e6\n"},{"id":"84118","messageId":"alpine.DEB.1.00.0807201529150.3305@eeepc-johanness","threadId":"14540","inReplyTo":"1216534144-23826-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH v2 1/4] builtin-add.c: restructure the code for maintainability","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-21T00:56:35Z","receivedAt":"2008-07-21T00:56:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 19 Jul 2008, Junio C Hamano wrote:\n\n\n> diff --git a/builtin-add.c b/builtin-add.c\n> index bf13aa3..9b2ee8c 100644\n> --- a/builtin-add.c\n> +++ b/builtin-add.c\n> [...]\n> +\t/*\n> +\t * If we are adding new files, we need to scan the working\n> +\t * tree to find the ones that match pathspecs; this needs\n> +\t * to be done before we read the index.\n> +\t */\n\nThis comment left me scratching my head.  While I do see a breakage when \nreading the index first, I had the impression that it should not.\n\nI can only imagine that the other users of read_directory() depend on some \nfunny interaction between the index and treat_directory().\n\nCiao,\nDscho\n"},{"id":"84120","messageId":"alpine.DEB.1.00.0807210256510.3305@eeepc-johanness","threadId":"14540","inReplyTo":"alpine.DEB.1.00.0807201529150.3305@eeepc-johanness","subject":"[PATCH/RFC] git add: do not add files from a submodule","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-21T00:58:40Z","receivedAt":"2008-07-21T00:58:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIt comes quite as a surprise to an unsuspecting Git user that calling\n\"git add submodule/file\" (which is a mistake, alright) _removes_\nthe submodule in the index, and adds the file.  Instead, complain loudly.\n\nWhile at it, be nice when the user said \"git add submodule/\" which is\nmost likely the consequence of tab-completion, and stage the submodule,\ninstead of trying to add the contents of that directory.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tThis would need a companion patch for \"git diff submodule/\",\n\tobviously.  However, revision.c does not need a valid cache,\n\tusually.  So I am hesitant.\n\n\tOh, and I am sure somebody will come up with a more elegant \n\tsolution to this problem.  I sure do not, having smashed my head \n\tagainst the wall for a few hours.\n\n builtin-add.c              |   42 +++++++++++++++++++++++++++++++++++++++---\n t/t7400-submodule-basic.sh |   18 ++++++++++++++++++\n 2 files changed, 57 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 6f5672a..9453557 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -50,6 +50,33 @@ static void prune_directory(struct dir_struct *dir, const char **pathspec, int p\n         free(seen);\n }\n \n+static void treat_gitlinks(const char **pathspec, struct index_state *index)\n+{\n+\tint i;\n+\n+\tif (!pathspec || !*pathspec)\n+\t\treturn;\n+\n+\tfor (i = 0; i < index->cache_nr; i++) {\n+\t\tstruct cache_entry *ce = index->cache[i];\n+\t\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\t\tint len = ce_namelen(ce), j;\n+\t\t\tfor (j = 0; pathspec[j]; j++) {\n+\t\t\t\tint len2 = strlen(pathspec[j]);\n+\t\t\t\tif (len2 <= len || pathspec[j][len] != '/' ||\n+\t\t\t\t    memcmp(ce->name, pathspec[j], len))\n+\t\t\t\t\tcontinue;\n+\t\t\t\tif (len2 == len + 1)\n+\t\t\t\t\t/* strip trailing slash */\n+\t\t\t\t\tpathspec[j] = xstrndup(ce->name, len);\n+\t\t\t\telse\n+\t\t\t\t\tdie (\"Path '%s' is in submodule '%.*s'\",\n+\t\t\t\t\t\tpathspec[j], len, ce->name);\n+\t\t\t}\n+\t\t}\n+\t}\n+}\n+\n static void fill_directory(struct dir_struct *dir, const char **pathspec,\n \t\tint ignored_too)\n {\n@@ -245,6 +272,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tint flags;\n \tint add_new_files;\n \tint require_pathspec;\n+\tstruct index_state index;\n \n \targc = parse_options(argc, argv, builtin_add_options,\n \t\t\t  builtin_add_usage, 0);\n@@ -283,12 +311,20 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t * If we are adding new files, we need to scan the working\n \t * tree to find the ones that match pathspecs; this needs\n \t * to be done before we read the index.\n+\t *\n+\t * However, to avoid adding files from submodules, we have to\n+\t * read the index first.  So read the index into a local\n+\t * variable, and set the global index after fill_directory().\n \t */\n-\tif (add_new_files)\n-\t\tfill_directory(&dir, pathspec, ignored_too);\n \n-\tif (read_cache() < 0)\n+\tmemset(&index, 0, sizeof(index));\n+\tif (read_index(&index) < 0)\n \t\tdie(\"index file corrupt\");\n+\ttreat_gitlinks(pathspec, &index);\n+\n+\tif (add_new_files)\n+\t\tfill_directory(&dir, pathspec, ignored_too);\n+\tthe_index = index;\n \n \tif (refresh_only) {\n \t\trefresh(verbose, pathspec);\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex cbc0c34..6da2545 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -209,4 +209,22 @@ test_expect_success 'update --init' '\n \n '\n \n+test_expect_success 'do not add files from a submodule' '\n+\n+\tgit reset --hard &&\n+\ttest_must_fail git add init/a\n+\n+'\n+\n+test_expect_success 'gracefully add submodule with a trailing slash' '\n+\n+\tcommit=$(cd init &&\n+\t echo b > a &&\n+\t git commit -m update a >/dev/null &&\n+\t git rev-parse HEAD) &&\n+\tgit add init/ &&\n+\ttest_must_fail git diff --exit-code --cached init\n+\n+'\n+\n test_done\n-- \n1.5.6.2.516.g22071\n"},{"id":"84138","messageId":"7v3am3yfph.fsf@gitster.siamese.dyndns.org","threadId":"14540","inReplyTo":"alpine.DEB.1.00.0807201529150.3305@eeepc-johanness","subject":"Re: [PATCH v2 1/4] builtin-add.c: restructure the code for maintainability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-21T05:22:02Z","receivedAt":"2008-07-21T05:22:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> This comment left me scratching my head.  While I do see a breakage when \n> reading the index first, I had the impression that it should not.\n\nThe directory traversal that originates from git-ls-files (not the\n\"show-index\" mode which the command originally was about, but the \"show\nothers\" part of the feature that came much later) were primarily designed\nfor collecting paths that do not appear in the active_cache[].  Very early\nversion of git-add was about adding untracked paths (and update-index was\nto stage changes to tracked files), and did have the fill_directory()\nafter we have read the index for this exact reason.\n\nThat changed late 2006 when Nico allowed git-add to stage already tracked\nfiles as well.  We collect the paths in the work tree that match given\npathspec, and for the directory traverser to do that job, you would need\nan empty index.\n\n\tSide note: 366bfcb (make 'git add' a first class user friendly\n\tinterface to the index, 2006-12-04) is something to marvel at.  It\n\tdescribes the change with its documentation update fully, changes\n\tthe semantics in a drastic way, with so little change.\n\n        Documentation/git-add.txt  |   53 ++++++++++++-----------\n        Documentation/tutorial.txt |   46 ++++++++++++++++++---\n        builtin-add.c              |    6 +-\n        wt-status.c                |    2 +-\n        4 files changed, 72 insertions(+), 35 deletions(-)\n\nPerhaps we can add a bit to the dir_struct we give to the traverser to\ntell it to ignore the index even if we have already read one.  That would\nbe a much cleaner API enhancement than reading the index and setting aside\nwhile calling read_directory() which feels like a you know what I would\ncall it.\n\nPerhaps you can lose that comment like this.\n\n builtin-add.c |   12 ++++--------\n dir.c         |   12 +++++++++---\n dir.h         |    1 +\n 3 files changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex fc3f96e..c6185c3 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -58,6 +58,7 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,\n \n \t/* Set up the default git porcelain excludes */\n \tmemset(dir, 0, sizeof(*dir));\n+\tdir->ignore_index = 1;\n \tif (!ignored_too) {\n \t\tdir->collect_ignored = 1;\n \t\tsetup_standard_excludes(dir);\n@@ -280,17 +281,12 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \tpathspec = get_pathspec(prefix, argv);\n \n-\t/*\n-\t * If we are adding new files, we need to scan the working\n-\t * tree to find the ones that match pathspecs; this needs\n-\t * to be done before we read the index.\n-\t */\n-\tif (add_new_files)\n-\t\tfill_directory(&dir, pathspec, ignored_too);\n-\n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\n \n+\tif (add_new_files)\n+\t\tfill_directory(&dir, pathspec, ignored_too);\n+\n \tif (refresh_only) {\n \t\trefresh(verbose, pathspec);\n \t\tgoto finish;\ndiff --git a/dir.c b/dir.c\nindex 29d1d5b..7447485 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -389,7 +389,7 @@ static struct dir_entry *dir_entry_new(const char *pathname, int len)\n \n struct dir_entry *dir_add_name(struct dir_struct *dir, const char *pathname, int len)\n {\n-\tif (cache_name_exists(pathname, len, ignore_case))\n+\tif (!dir->ignore_index && cache_name_exists(pathname, len, ignore_case))\n \t\treturn NULL;\n \n \tALLOC_GROW(dir->entries, dir->nr+1, dir->alloc);\n@@ -483,8 +483,14 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n \tconst char *dirname, int len,\n \tconst struct path_simplify *simplify)\n {\n-\t/* The \"len-1\" is to strip the final '/' */\n-\tswitch (directory_exists_in_index(dirname, len-1)) {\n+\tenum exist_status in_index;\n+\n+\tif (dir->ignore_index)\n+\t\tin_index = index_nonexistent;\n+\telse\n+\t\t/* The \"len-1\" is to strip the final '/' */\n+\t\tin_index = directory_exists_in_index(dirname, len-1);\n+\tswitch (in_index) {\n \tcase index_directory:\n \t\treturn recurse_into_directory;\n \ndiff --git a/dir.h b/dir.h\nindex 2df15de..4ef1c99 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -38,6 +38,7 @@ struct dir_struct {\n \t\t     show_other_directories:1,\n \t\t     hide_empty_directories:1,\n \t\t     no_gitlinks:1,\n+\t\t     ignore_index:1,\n \t\t     collect_ignored:1;\n \tstruct dir_entry **entries;\n \tstruct dir_entry **ignored;\n"},{"id":"84153","messageId":"7v7ibfvfmh.fsf@gitster.siamese.dyndns.org","threadId":"14540","inReplyTo":"7v3am3yfph.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/4] builtin-add.c: restructure the code for maintainability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-21T07:52:06Z","receivedAt":"2008-07-21T07:52:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> That changed late 2006 when Nico allowed git-add to stage already tracked\n> files as well.  We collect the paths in the work tree that match given\n> pathspec, and for the directory traverser to do that job, you would need\n> an empty index.\n>\n> \tSide note: 366bfcb (make 'git add' a first class user friendly\n> \tinterface to the index, 2006-12-04) is something to marvel at.  It\n> \tdescribes the change with its documentation update fully, changes\n> \tthe semantics in a drastic way, with so little change.\n>\n>         Documentation/git-add.txt  |   53 ++++++++++++-----------\n>         Documentation/tutorial.txt |   46 ++++++++++++++++++---\n>         builtin-add.c              |    6 +-\n>         wt-status.c                |    2 +-\n>         4 files changed, 72 insertions(+), 35 deletions(-)\n>\n> Perhaps we can add a bit to the dir_struct we give to the traverser to\n> tell it to ignore the index even if we have already read one.  That would\n> be a much cleaner API enhancement than reading the index and setting aside\n> while calling read_directory() which feels like a you know what I would\n> call it.\n\nThinking about this issue a bit more, I realize that the earlier \"git add -A\"\nchange was done in a quite inefficient way (i.e. it is as unefficient as\n\"git add -u && git add .\" modulo one fork/exec and read/write index).  For\nthat matter, the original \"git add .\" could probably be more efficient\nthan it currently is.\n\nThe thing is, when the user asks \"git add .\", we do not have to examine\nall paths we encounter and perform the excluded() and dir_add_name()\nprocessing, both of which are slower code and use slower data structure by\ngit standard, especially when the index is already populated.\n\nInstead, we should be able to implement \"git add $pathspec...\" as:\n\n - read the index;\n\n - read_directory() to process untracked, unignored files the current way,\n   that is, recursively doing readdir(), filtering them by pathspec and\n   excluded(), queueing them via dir_add_name() and finally do\n   add_files(); and\n\n - iterate over the index, filtering them by pathspec, and update only the\n   modified/type changed paths but not deleted ones.\n\nAnd \"git add -A\" will become exactly the same as above, modulo:\n\n - missing $pathspec means \".\" instead of being an error; and \n\n - \"interate over the index\" part handles deleted ones as well,\n   i.e. exactly what the current update_callback() in builtin-add.c does.\n\nIt is likely that I am too tired to do this right tonight, so I'll go to\nbed and expect to find a nicely done patch in my mailbox by somebody else\n;-).\n"},{"id":"84157","messageId":"7vr69ntzke.fsf@gitster.siamese.dyndns.org","threadId":"14540","inReplyTo":"7v7ibfvfmh.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v2 1/4] builtin-add.c: restructure the code for maintainability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-21T08:24:17Z","receivedAt":"2008-07-21T08:24:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Thinking about this issue a bit more, I realize that the earlier \"git add -A\"\n> change was done in a quite inefficient way (i.e. it is as unefficient as\n> \"git add -u && git add .\" modulo one fork/exec and read/write index).  For\n> that matter, the original \"git add .\" could probably be more efficient\n> than it currently is.\n> ...\n> It is likely that I am too tired to do this right tonight, so I'll go to\n> bed and expect to find a nicely done patch in my mailbox by somebody else\n> ;-).\n\nWell, I lied.  I couldn't sleep, so here is a preview that seems to pass\nall the test at least.  It needs to be split into two, but I am too tired\nto do that properly tonight.\n\nThe main part of the change is third hunk (preimage ll.258-) to the bottom\nof builtin-add.c and follows the idea outlined in the message this is a\nresponse to.\n\nA private function add_files_to_cache() in builtin-add.c was borrowed by\ncheckout and commit re-implementors without getting properly refactored to\nmore library-ish place.  This does the refactoring, and most of the\nchanges you see in the diffstat are this; it should come before the main\npart of the change.\n\n---\n\n builtin-add.c |   78 ++++++--------------------------------------------------\n cache.h       |    1 +\n read-cache.c  |   61 ++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 71 insertions(+), 69 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex fc3f96e..f9b25f2 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -8,10 +8,6 @@\n #include \"dir.h\"\n #include \"exec_cmd.h\"\n #include \"cache-tree.h\"\n-#include \"diff.h\"\n-#include \"diffcore.h\"\n-#include \"commit.h\"\n-#include \"revision.h\"\n #include \"run-command.h\"\n #include \"parse-options.h\"\n \n@@ -79,59 +75,6 @@ static void fill_directory(struct dir_struct *dir, const char **pathspec,\n \t\tprune_directory(dir, pathspec, baselen);\n }\n \n-struct update_callback_data\n-{\n-\tint flags;\n-\tint add_errors;\n-};\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;\n-\tstruct update_callback_data *data = cbdata;\n-\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(\"unexpected diff status %c\", p->status);\n-\t\tcase DIFF_STATUS_UNMERGED:\n-\t\tcase DIFF_STATUS_MODIFIED:\n-\t\tcase DIFF_STATUS_TYPE_CHANGED:\n-\t\t\tif (add_file_to_cache(path, data->flags)) {\n-\t\t\t\tif (!(data->flags & ADD_CACHE_IGNORE_ERRORS))\n-\t\t\t\t\tdie(\"updating files failed\");\n-\t\t\t\tdata->add_errors++;\n-\t\t\t}\n-\t\t\tbreak;\n-\t\tcase DIFF_STATUS_DELETED:\n-\t\t\tif (!(data->flags & ADD_CACHE_PRETEND))\n-\t\t\t\tremove_file_from_cache(path);\n-\t\t\tif (data->flags & (ADD_CACHE_PRETEND|ADD_CACHE_VERBOSE))\n-\t\t\t\tprintf(\"remove '%s'\\n\", path);\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-}\n-\n-int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n-{\n-\tstruct update_callback_data data;\n-\tstruct rev_info rev;\n-\tinit_revisions(&rev, prefix);\n-\tsetup_revisions(0, NULL, &rev, NULL);\n-\trev.prune_data = pathspec;\n-\trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n-\trev.diffopt.format_callback = update_callback;\n-\tdata.flags = flags;\n-\tdata.add_errors = 0;\n-\trev.diffopt.format_callback_data = &data;\n-\trun_diff_files(&rev, DIFF_RACY_IS_MODIFIED);\n-\treturn !!data.add_errors;\n-}\n-\n static void refresh(int verbose, const char **pathspec)\n {\n \tchar *seen;\n@@ -258,7 +201,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tif (addremove && take_worktree_changes)\n \t\tdie(\"-A and -u are mutually incompatible\");\n-\tif (addremove && !argc) {\n+\tif ((addremove || take_worktree_changes) && !argc) {\n \t\tstatic const char *here[2] = { \".\", NULL };\n \t\targc = 1;\n \t\targv = here;\n@@ -271,7 +214,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tflags = ((verbose ? ADD_CACHE_VERBOSE : 0) |\n \t\t (show_only ? ADD_CACHE_PRETEND : 0) |\n-\t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0));\n+\t\t (ignore_add_errors ? ADD_CACHE_IGNORE_ERRORS : 0) |\n+\t\t (!(addremove || take_worktree_changes)\n+\t\t  ? ADD_CACHE_IGNORE_REMOVAL : 0));\n \n \tif (require_pathspec && argc == 0) {\n \t\tfprintf(stderr, \"Nothing specified, nothing added.\\n\");\n@@ -280,24 +225,19 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t}\n \tpathspec = get_pathspec(prefix, argv);\n \n-\t/*\n-\t * If we are adding new files, we need to scan the working\n-\t * tree to find the ones that match pathspecs; this needs\n-\t * to be done before we read the index.\n-\t */\n-\tif (add_new_files)\n-\t\tfill_directory(&dir, pathspec, ignored_too);\n-\n \tif (read_cache() < 0)\n \t\tdie(\"index file corrupt\");\n \n+\tif (add_new_files)\n+\t\t/* This picks up the paths that are not tracked */\n+\t\tfill_directory(&dir, pathspec, ignored_too);\n+\n \tif (refresh_only) {\n \t\trefresh(verbose, pathspec);\n \t\tgoto finish;\n \t}\n \n-\tif (take_worktree_changes || addremove)\n-\t\texit_status |= add_files_to_cache(prefix, pathspec, flags);\n+\texit_status |= add_files_to_cache(prefix, pathspec, flags);\n \n \tif (add_new_files)\n \t\texit_status |= add_files(&dir, flags);\ndiff --git a/cache.h b/cache.h\nindex 38985aa..6f374ad 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -375,6 +375,7 @@ extern int remove_file_from_index(struct index_state *, const char *path);\n #define ADD_CACHE_VERBOSE 1\n #define ADD_CACHE_PRETEND 2\n #define ADD_CACHE_IGNORE_ERRORS\t4\n+#define ADD_CACHE_IGNORE_REMOVAL 8\n extern int add_to_index(struct index_state *, const char *path, struct stat *, int flags);\n extern int add_file_to_index(struct index_state *, const char *path, int flags);\n extern struct cache_entry *make_cache_entry(unsigned int mode, const unsigned char *sha1, const char *path, int stage, int refresh);\ndiff --git a/read-cache.c b/read-cache.c\nindex a50a851..6833af6 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -8,6 +8,11 @@\n #include \"cache-tree.h\"\n #include \"refs.h\"\n #include \"dir.h\"\n+#include \"tree.h\"\n+#include \"commit.h\"\n+#include \"diff.h\"\n+#include \"diffcore.h\"\n+#include \"revision.h\"\n \n /* Index extensions.\n  *\n@@ -1444,3 +1449,59 @@ int read_index_unmerged(struct index_state *istate)\n \tistate->cache_nr = dst - istate->cache;\n \treturn !!last;\n }\n+\n+struct update_callback_data\n+{\n+\tint flags;\n+\tint add_errors;\n+};\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;\n+\tstruct update_callback_data *data = cbdata;\n+\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(\"unexpected diff status %c\", p->status);\n+\t\tcase DIFF_STATUS_UNMERGED:\n+\t\tcase DIFF_STATUS_MODIFIED:\n+\t\tcase DIFF_STATUS_TYPE_CHANGED:\n+\t\t\tif (add_file_to_index(&the_index, path, data->flags)) {\n+\t\t\t\tif (!(data->flags & ADD_CACHE_IGNORE_ERRORS))\n+\t\t\t\t\tdie(\"updating files failed\");\n+\t\t\t\tdata->add_errors++;\n+\t\t\t}\n+\t\t\tbreak;\n+\t\tcase DIFF_STATUS_DELETED:\n+\t\t\tif (data->flags & ADD_CACHE_IGNORE_REMOVAL)\n+\t\t\t\tbreak;\n+\t\t\tif (!(data->flags & ADD_CACHE_PRETEND))\n+\t\t\t\tremove_file_from_index(&the_index, path);\n+\t\t\tif (data->flags & (ADD_CACHE_PRETEND|ADD_CACHE_VERBOSE))\n+\t\t\t\tprintf(\"remove '%s'\\n\", path);\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+}\n+\n+int add_files_to_cache(const char *prefix, const char **pathspec, int flags)\n+{\n+\tstruct update_callback_data data;\n+\tstruct rev_info rev;\n+\tinit_revisions(&rev, prefix);\n+\tsetup_revisions(0, NULL, &rev, NULL);\n+\trev.prune_data = pathspec;\n+\trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n+\trev.diffopt.format_callback = update_callback;\n+\tdata.flags = flags;\n+\tdata.add_errors = 0;\n+\trev.diffopt.format_callback_data = &data;\n+\trun_diff_files(&rev, DIFF_RACY_IS_MODIFIED);\n+\treturn !!data.add_errors;\n+}\n+\n"},{"id":"84379","messageId":"alpine.DEB.1.00.0807222230490.8986@racer","threadId":"14540","inReplyTo":"alpine.DEB.1.00.0807210256510.3305@eeepc-johanness","subject":"Re: [PATCH/RFC] git add: do not add files from a submodule","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-22T21:32:03Z","receivedAt":"2008-07-22T21:32:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 21 Jul 2008, Johannes Schindelin wrote:\n\n> It comes quite as a surprise to an unsuspecting Git user that calling\n> \"git add submodule/file\" (which is a mistake, alright) _removes_\n> the submodule in the index, and adds the file.  Instead, complain loudly.\n> \n> While at it, be nice when the user said \"git add submodule/\" which is\n> most likely the consequence of tab-completion, and stage the submodule,\n> instead of trying to add the contents of that directory.\n\nDo you plan to apply the split-up builtin-add enhancments you did a few \nnights ago, so that this patch can be elegant?  I think this fix should go \ninto 1.6.0.\n\nCiao,\nDscho\n"},{"id":"84439","messageId":"7vhcahgl2j.fsf@gitster.siamese.dyndns.org","threadId":"14540","inReplyTo":"alpine.DEB.1.00.0807222230490.8986@racer","subject":"Re: [PATCH/RFC] git add: do not add files from a submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-23T06:40:20Z","receivedAt":"2008-07-23T06:40:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Do you plan to apply the split-up builtin-add enhancments you did a few \n> nights ago,...\n\nI have a few updates to that one, I'll be sending them out shortly.\n\nSwitching branches between revs that have and do not have submodule at a\ngiven path has always been broken.  It is not even a \"known breakage\",\nwhich is a word used for something that has a sensible design already is\nworked out but the implementation does not do so.\n\nIf we started the process of diagnosing and fixing these issues earlier,\nand had plausible code to address the issue already in 'next' before the\ncurrent -rc cycle started, the topic would have been an obvious candidate\nfor the coming release and I'd further say it would be even worth delaying\nthe release for a few weeks if it takes more time.  But I have to say it\nis too late for 1.6.0 now if we are just noticing and starting the\ndiscussion.  This comment goes to the issue Pierre raised last night as\nwell.\n\nNobody prevents us from starting the process to discuss and prepare to put\nsomething in 'next' for 1.6.1 cycle now, though.\n"},{"id":"84446","messageId":"20080723081333.GA15243@artemis.madism.org","threadId":"14540","inReplyTo":"7vhcahgl2j.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RFC] git add: do not add files from a submodule","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-23T08:13:33Z","receivedAt":"2008-07-23T08:13:33Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Jul 23, 2008 at 06:40:20AM +0000, Junio C Hamano wrote:\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > Do you plan to apply the split-up builtin-add enhancments you did a few \n> > nights ago,...\n> \n> I have a few updates to that one, I'll be sending them out shortly.\n> \n> Switching branches between revs that have and do not have submodule at a\n> given path has always been broken.  It is not even a \"known breakage\",\n> which is a word used for something that has a sensible design already is\n> worked out but the implementation does not do so.\n> \n> If we started the process of diagnosing and fixing these issues earlier,\n> and had plausible code to address the issue already in 'next' before the\n> current -rc cycle started, the topic would have been an obvious candidate\n> for the coming release and I'd further say it would be even worth delaying\n> the release for a few weeks if it takes more time.  But I have to say it\n> is too late for 1.6.0 now if we are just noticing and starting the\n> discussion.\n\n  Well given that we now use submodules at work, and that git is\nnowadays somewhere in the top 5 of my most consciously (as opposed to\nthe compiler that I rarely call by hand) used software suites (among my\neditor, my MUA, my shell and my tiling WM), I'm very much interested in\ntackling some things about what is (not) done with submodules yet.\n\n  I sent a mail some time ago about those issues, in short words, those\nconcern git-checkout, git-reset, git-fetch and git-push for starters.\nI'm not sure I can help with the code a lot, but I can for sure provide\na comprehensive list of things I would like the porcelain to do with\nsubmodules if that helps.\n\n> This comment goes to the issue Pierre raised last night as well.\n\n  You mean the git checkout issue ? Because the issue is twofold, one\nthere is the missing warnings that users can shoot themselves in the\nfoot unnoticed, and there is the fact that \"git checkout -- $foo\" does\nnot what it should when \"$foo\" is a valid revision (and *that* is a\nreally really bad bug, that if I'm correct changing parse_options flags\nto KEEP_DASHDASH fixes alright).\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"84567","messageId":"7v8wvse9l7.fsf@gitster.siamese.dyndns.org","threadId":"14540","inReplyTo":"20080723081333.GA15243@artemis.madism.org","subject":"Re: [PATCH/RFC] git add: do not add files from a submodule","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-23T18:31:16Z","receivedAt":"2008-07-23T18:31:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n> On Wed, Jul 23, 2008 at 06:40:20AM +0000, Junio C Hamano wrote:\n> ...\n>> If we started the process of diagnosing and fixing these issues earlier,\n>> and had plausible code to address the issue already in 'next' before the\n>> current -rc cycle started, the topic would have been an obvious candidate\n>> for the coming release and I'd further say it would be even worth delaying\n>> the release for a few weeks if it takes more time.  But I have to say it\n>> is too late for 1.6.0 now if we are just noticing and starting the\n>> discussion.\n>\n>   Well given that we now use submodules at work, and that git is\n> nowadays somewhere in the top 5 of my most consciously (as opposed to\n> the compiler that I rarely call by hand) used software suites (among my\n> editor, my MUA, my shell and my tiling WM), I'm very much interested in\n> tackling some things about what is (not) done with submodules yet.\n\nSurely the effort is appreciated.\n\n>> This comment goes to the issue Pierre raised last night as well.\n>\n>   You mean the git checkout issue?\n\nOh, no; that misuse of parse_opt() that forgot KEEP_DASHDASH one was not\nwhat I had in mind.  I meant to say that your \"switch branches between an\nold pre-submodule rev and a new one that has a submodule at where a blob\nor directory used to be\" issue with a good explanation material was a good\nstarting point for submodule improvements for the next cycle.\n\nI'd like the release schedule not too heavily based on \"per feature\", but\nmore time-based.\n"},{"id":"84576","messageId":"20080723190227.GF20614@artemis.madism.org","threadId":"14540","inReplyTo":"7v8wvse9l7.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH/RFC] git add: do not add files from a submodule","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-23T19:02:27Z","receivedAt":"2008-07-23T19:02:27Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Jul 23, 2008 at 06:31:16PM +0000, Junio C Hamano wrote:\n> Pierre Habouzit <madcoder@debian.org> writes:\n> \n> > On Wed, Jul 23, 2008 at 06:40:20AM +0000, Junio C Hamano wrote:\n> > ...\n> >> If we started the process of diagnosing and fixing these issues earlier,\n> >> and had plausible code to address the issue already in 'next' before the\n> >> current -rc cycle started, the topic would have been an obvious candidate\n> >> for the coming release and I'd further say it would be even worth delaying\n> >> the release for a few weeks if it takes more time.  But I have to say it\n> >> is too late for 1.6.0 now if we are just noticing and starting the\n> >> discussion.\n> >\n> >   Well given that we now use submodules at work, and that git is\n> > nowadays somewhere in the top 5 of my most consciously (as opposed to\n> > the compiler that I rarely call by hand) used software suites (among my\n> > editor, my MUA, my shell and my tiling WM), I'm very much interested in\n> > tackling some things about what is (not) done with submodules yet.\n> \n> Surely the effort is appreciated.\n\n  okay I'll try to work on this on the git wiki so that collaboration is\npossible.\n\n> \n> >> This comment goes to the issue Pierre raised last night as well.\n> >\n> >   You mean the git checkout issue?\n> \n> Oh, no; that misuse of parse_opt() that forgot KEEP_DASHDASH one was not\n> what I had in mind.  I meant to say that your \"switch branches between an\n> old pre-submodule rev and a new one that has a submodule at where a blob\n> or directory used to be\" issue with a good explanation material was a good\n> starting point for submodule improvements for the next cycle.\n\n  ohh that :)\n\n> I'd like the release schedule not too heavily based on \"per feature\", but\n> more time-based.\n\n  Yeah, we've seen in the past how it makes a release slip. Though it'd\nbe great to say e.g. that we won't do a 1.7.0 release[0] until we have\nan UI for submodules we are prood of. It doesn't mean that we won't have\na 1.6.21 because it's slow to get into shape ;)\n\n  IOW I'm all for time based releases, with some big milestones that\nwhen completed bump the git version significantly. And hinting on what\nare those milestones would probably be quite nice. I mean git\ndeveloppement is kind of a hit and run thing: people have an issue, come\nwith patches, and go back to their lives (except for a mere 20 regular\ncontributors with more than 50 patches). Maybe we should hint some\ndirection we would like to see a bit more. examples of such big\ndirections are:\n  * submodules UI ;\n  * sparse checkouts ;\n  * ...\n\n\n  [0] I don't really *mean* we must do it for 1.7.0, It's merely an\n      example.\n"},{"id":"84582","messageId":"alpine.DEB.1.00.0807232009310.8986@racer","threadId":"14540","inReplyTo":"20080723190227.GF20614@artemis.madism.org","subject":"Re: [PATCH/RFC] git add: do not add files from a submodule","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-23T19:10:09Z","receivedAt":"2008-07-23T19:10:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 23 Jul 2008, Pierre Habouzit wrote:\n\n> Though it'd be great to say e.g. that we won't do a 1.7.0 release[0] \n> until we have an UI for submodules we are prood of.\n\nOr until we have everything ported over to parse_options?\n\nCiao,\nDscho\n"},{"id":"84583","messageId":"20080723191141.GH20614@artemis.madism.org","threadId":"14540","inReplyTo":"alpine.DEB.1.00.0807232009310.8986@racer","subject":"Re: [PATCH/RFC] git add: do not add files from a submodule","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-07-23T19:11:41Z","receivedAt":"2008-07-23T19:11:41Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Jul 23, 2008 at 07:10:09PM +0000, Johannes Schindelin wrote:\n> Hi,\n> \n> On Wed, 23 Jul 2008, Pierre Habouzit wrote:\n> \n> > Though it'd be great to say e.g. that we won't do a 1.7.0 release[0] \n> > until we have an UI for submodules we are prood of.\n> \n> Or until we have everything ported over to parse_options?\n\n  hahah, okay, fair enough, I deserve this one ;)\n"}]}