{"thread":{"id":"20771","subject":"[BUG] git stash refuses to save after \"add -N\"","startedAt":"2009-08-28T11:02:23Z","lastAt":"2009-08-31T08:16:34Z","messageCount":11,"participants":["Yann Dirson","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"121995","messageId":"54e098c45bffbf870bdfcee26b9ddecc.squirrel@intranet.linagora.com","threadId":"20771","inReplyTo":null,"subject":"[BUG] git stash refuses to save after \"add -N\"","fromName":"Yann Dirson","fromEmail":"ydirson@linagora.com","sentAt":"2009-08-28T11:02:23Z","receivedAt":"2009-08-28T11:02:23Z","isPatch":false,"sender":{"key":"ydirson@linagora.com","avatar":null},"body":"$ echo foo > bar\n$ git add -N bar\n$ ./git --exec-path=$PWD stash save\nbar: not added yet\nfatal: git-write-tree: error building trees\nCannot save the current index state\n\n\nMaybe it would require some magic in git-stash to detect/save/restore that\nparticular state, or \"just\" to cause \"add -N\" to insert an empty file\ninstead ?\n\nI suspect that second solution would not be particularly popular, but I\ndon't find the 1st one very appealing, it just looks like breaking the\nwhole idea behind git-stash :)\n"},{"id":"122037","messageId":"20090828190531.GB11488@coredump.intra.peff.net","threadId":"20771","inReplyTo":"54e098c45bffbf870bdfcee26b9ddecc.squirrel@intranet.linagora.com","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-28T19:05:31Z","receivedAt":"2009-08-28T19:05:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 28, 2009 at 01:02:23PM +0200, Yann Dirson wrote:\n\n> $ echo foo > bar\n> $ git add -N bar\n> $ ./git --exec-path=$PWD stash save\n> bar: not added yet\n> fatal: git-write-tree: error building trees\n> Cannot save the current index state\n> \n> Maybe it would require some magic in git-stash to detect/save/restore that\n> particular state, or \"just\" to cause \"add -N\" to insert an empty file\n> instead ?\n\nYes, there needs to be some magic in git-stash to handle this. There are\nactually two calls to write-tree: one to save the index and one to save\nthe working tree. I think the working tree one should be OK, because we\n\"git add -u\" right beforehand, which means \"intent-to-add\" files will\nbe saved properly.\n\nFor the index case, we unfortunately cannot represent the situation in\nthe index using a tree, which means we cannot have a stash that doesn't\nlose information. So we have to choose either dropping those index\nentries, inserting them as blank files, or inserting them with\nworking-tree contents.\n\nWhen you apply the stash, if they were:\n\n  - dropped, then you may be surprised to find that those files are now\n    untracked\n\n  - inserted as working-tree content, then you may not realize that you\n    had not _actually_ added that content to the index earlier, and just\n    commit it\n\n  - inserted as blank files, then you may be a bit surprised by the fact\n    that it looks like you added a blank version, but at least you will\n    still see a diff against the working tree file, alerting you to the\n    fact that maybe they weren't entirely ready for commit.\n\nSo I think of the three, the last one is the least surprising. The other\noption is to die and force the user to resolve the issue, which is what\nwe do now. It does actually tell you the problem \"bar: not added yet\",\nthough we could perhaps improve on that message a bit. I think that\nwould require a new 'ls-files' flag to list intent-to-add files.\n\n-Peff\n"},{"id":"122044","messageId":"20090828192217.GA13023@coredump.intra.peff.net","threadId":"20771","inReplyTo":"20090828190531.GB11488@coredump.intra.peff.net","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-28T19:22:17Z","receivedAt":"2009-08-28T19:22:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Aug 28, 2009 at 03:05:31PM -0400, Jeff King wrote:\n\n> For the index case, we unfortunately cannot represent the situation in\n> the index using a tree, which means we cannot have a stash that doesn't\n> lose information. So we have to choose either dropping those index\n> entries, inserting them as blank files, or inserting them with\n> working-tree contents.\n\nActually, you _could_ try representing the information by shoe-horning\nit into the tree. For example, by allocating a bit of the mode as\n\"intent-to-add\". That seems pretty ugly to me, though.\n\nYou could also try to stick the information in the stash's commit\nmessage and recreate it during \"stash apply\". But again, that feels kind\nof ugly.\n\n-Peff\n"},{"id":"122090","messageId":"7vmy5ixn96.fsf@alter.siamese.dyndns.org","threadId":"20771","inReplyTo":"20090828190531.GB11488@coredump.intra.peff.net","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-29T22:34:45Z","receivedAt":"2009-08-29T22:34:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>   - inserted as blank files, then you may be a bit surprised by the fact\n>     that it looks like you added a blank version, but at least you will\n>     still see a diff against the working tree file, alerting you to the\n>     fact that maybe they weren't entirely ready for commit.\n>\n> So I think of the three, the last one is the least surprising. The other\n> option is to die and force the user to resolve the issue, which is what\n> we do now. It does actually tell you the problem \"bar: not added yet\",\n> though we could perhaps improve on that message a bit.\n\nI am slightly in favor of leaving the things as they are, as the error\nmessage is quite clear.\n\nThe third option would probably loo like this, which is not too bad,\nthough.\n\n builtin-commit.c       |    2 +-\n builtin-write-tree.c   |    3 +++\n cache-tree.c           |   29 ++++++++++++++---------------\n cache-tree.h           |    4 +++-\n git-stash.sh           |    2 +-\n merge-recursive.c      |    2 +-\n test-dump-cache-tree.c |    2 +-\n 7 files changed, 24 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 200ffda..fc97d54 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -595,7 +595,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tif (!active_cache_tree)\n \t\tactive_cache_tree = cache_tree();\n \tif (cache_tree_update(active_cache_tree,\n-\t\t\t      active_cache, active_nr, 0, 0) < 0) {\n+\t\t\t      active_cache, active_nr, 0) < 0) {\n \t\terror(\"Error building trees\");\n \t\treturn 0;\n \t}\ndiff --git a/builtin-write-tree.c b/builtin-write-tree.c\nindex b223af4..6fce25e 100644\n--- a/builtin-write-tree.c\n+++ b/builtin-write-tree.c\n@@ -23,6 +23,9 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)\n \tstruct option write_tree_options[] = {\n \t\tOPT_BIT(0, \"missing-ok\", &flags, \"allow missing objects\",\n \t\t\tWRITE_TREE_MISSING_OK),\n+\t\tOPT_BIT(0, \"intent-as-empty\", &flags,\n+\t\t\t\"write intent-to-add entries as empty blobs\",\n+\t\t\tWRITE_TREE_ADD_INTENT_AS_EMPTY),\n \t\t{ OPTION_STRING, 0, \"prefix\", &prefix, \"<prefix>/\",\n \t\t  \"write tree object for a subdirectory <prefix>\" ,\n \t\t  PARSE_OPT_LITERAL_ARGHELP },\ndiff --git a/cache-tree.c b/cache-tree.c\nindex d917437..7377066 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -148,7 +148,7 @@ void cache_tree_invalidate_path(struct cache_tree *it, const char *path)\n }\n \n static int verify_cache(struct cache_entry **cache,\n-\t\t\tint entries)\n+\t\t\tint entries, int flags)\n {\n \tint i, funny;\n \n@@ -156,7 +156,9 @@ static int verify_cache(struct cache_entry **cache,\n \tfunny = 0;\n \tfor (i = 0; i < entries; i++) {\n \t\tstruct cache_entry *ce = cache[i];\n-\t\tif (ce_stage(ce) || (ce->ce_flags & CE_INTENT_TO_ADD)) {\n+\t\tif (ce_stage(ce) ||\n+\t\t    (!(flags & WRITE_TREE_ADD_INTENT_AS_EMPTY) &&\n+\t\t     (ce->ce_flags & CE_INTENT_TO_ADD))) {\n \t\t\tif (10 < ++funny) {\n \t\t\t\tfprintf(stderr, \"...\\n\");\n \t\t\t\tbreak;\n@@ -237,8 +239,7 @@ static int update_one(struct cache_tree *it,\n \t\t      int entries,\n \t\t      const char *base,\n \t\t      int baselen,\n-\t\t      int missing_ok,\n-\t\t      int dryrun)\n+\t\t      int flags)\n {\n \tstruct strbuf buffer;\n \tint i;\n@@ -284,8 +285,7 @@ static int update_one(struct cache_tree *it,\n \t\t\t\t    cache + i, entries - i,\n \t\t\t\t    path,\n \t\t\t\t    baselen + sublen + 1,\n-\t\t\t\t    missing_ok,\n-\t\t\t\t    dryrun);\n+\t\t\t\t    flags);\n \t\tif (subcnt < 0)\n \t\t\treturn subcnt;\n \t\ti += subcnt - 1;\n@@ -328,7 +328,9 @@ static int update_one(struct cache_tree *it,\n \t\t\tmode = ce->ce_mode;\n \t\t\tentlen = pathlen - baselen;\n \t\t}\n-\t\tif (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1))\n+\t\tif (mode != S_IFGITLINK &&\n+\t\t    !(flags & WRITE_TREE_MISSING_OK) &&\n+\t\t    !has_sha1_file(sha1))\n \t\t\treturn error(\"invalid object %06o %s for '%.*s'\",\n \t\t\t\tmode, sha1_to_hex(sha1), entlen+baselen, path);\n \n@@ -345,7 +347,7 @@ static int update_one(struct cache_tree *it,\n #endif\n \t}\n \n-\tif (dryrun)\n+\tif (flags & WRITE_TREE_DRY_RUN)\n \t\thash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);\n \telse if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) {\n \t\tstrbuf_release(&buffer);\n@@ -365,14 +367,13 @@ static int update_one(struct cache_tree *it,\n int cache_tree_update(struct cache_tree *it,\n \t\t      struct cache_entry **cache,\n \t\t      int entries,\n-\t\t      int missing_ok,\n-\t\t      int dryrun)\n+\t\t      int flags)\n {\n \tint i;\n-\ti = verify_cache(cache, entries);\n+\ti = verify_cache(cache, entries, flags);\n \tif (i)\n \t\treturn i;\n-\ti = update_one(it, cache, entries, \"\", 0, missing_ok, dryrun);\n+\ti = update_one(it, cache, entries, \"\", 0, flags);\n \tif (i < 0)\n \t\treturn i;\n \treturn 0;\n@@ -565,11 +566,9 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)\n \n \twas_valid = cache_tree_fully_valid(active_cache_tree);\n \tif (!was_valid) {\n-\t\tint missing_ok = flags & WRITE_TREE_MISSING_OK;\n-\n \t\tif (cache_tree_update(active_cache_tree,\n \t\t\t\t      active_cache, active_nr,\n-\t\t\t\t      missing_ok, 0) < 0)\n+\t\t\t\t      flags) < 0)\n \t\t\treturn WRITE_TREE_UNMERGED_INDEX;\n \t\tif (0 <= newfd) {\n \t\t\tif (!write_cache(newfd, active_cache, active_nr) &&\ndiff --git a/cache-tree.h b/cache-tree.h\nindex 3df641f..4ee03bb 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -29,11 +29,13 @@ void cache_tree_write(struct strbuf *, struct cache_tree *root);\n struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);\n \n int cache_tree_fully_valid(struct cache_tree *);\n-int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int);\n+int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int);\n \n /* bitmasks to write_cache_as_tree flags */\n #define WRITE_TREE_MISSING_OK 1\n #define WRITE_TREE_IGNORE_CACHE_TREE 2\n+#define WRITE_TREE_ADD_INTENT_AS_EMPTY 4\n+#define WRITE_TREE_DRY_RUN 8\n \n /* error return codes */\n #define WRITE_TREE_UNREADABLE_INDEX (-1)\ndiff --git a/git-stash.sh b/git-stash.sh\nindex d61c9d0..735e511 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -63,7 +63,7 @@ create_stash () {\n \tmsg=$(printf '%s: %s' \"$branch\" \"$head\")\n \n \t# state of the index\n-\ti_tree=$(git write-tree) &&\n+\ti_tree=$(git write-tree --intent-as-empty) &&\n \ti_commit=$(printf 'index on %s\\n' \"$msg\" |\n \t\tgit commit-tree $i_tree -p $b_commit) ||\n \t\tdie \"Cannot save the current index state\"\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 10d7913..dea5400 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -211,7 +211,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n \n \tif (!cache_tree_fully_valid(active_cache_tree) &&\n \t    cache_tree_update(active_cache_tree,\n-\t\t\t      active_cache, active_nr, 0, 0) < 0)\n+\t\t\t      active_cache, active_nr, 0) < 0)\n \t\tdie(\"error building trees\");\n \n \tresult = lookup_tree(active_cache_tree->sha1);\ndiff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c\nindex 1f73f1e..a6ffdf3 100644\n--- a/test-dump-cache-tree.c\n+++ b/test-dump-cache-tree.c\n@@ -59,6 +59,6 @@ int main(int ac, char **av)\n \tstruct cache_tree *another = cache_tree();\n \tif (read_cache() < 0)\n \t\tdie(\"unable to read index file\");\n-\tcache_tree_update(another, active_cache, active_nr, 0, 1);\n+\tcache_tree_update(another, active_cache, active_nr, WRITE_TREE_DRY_RUN);\n \treturn dump_cache_tree(active_cache_tree, another, \"\");\n }\n"},{"id":"122109","messageId":"20090830095509.GB30922@coredump.intra.peff.net","threadId":"20771","inReplyTo":"7vmy5ixn96.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-30T09:55:09Z","receivedAt":"2009-08-30T09:55:09Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Aug 29, 2009 at 03:34:45PM -0700, Junio C Hamano wrote:\n\n> I am slightly in favor of leaving the things as they are, as the error\n> message is quite clear.\n\nHmm. Thinking about it a bit more, I think \"add as empty content\" is\nprobably the best. It scares me a little because it is losing\ninformation during the stash, but consider it from the user's\nperspective.\n\nTheir work-in-progress is being interrupted, so they need to stash. They\ntry \"git stash\" and the current version comes back with an error. Now\nwhat? If they know what to do, they can manually \"git rm --cached\" each\nof the offending files (and I say manually because there isn't a\nparseable list of them anywhere). But they probably don't know what to\ndo, which means trying to find the information in the documentation.\n\nAnd all of this while they are trying to quickly switch contexts to\nwhatever it was that caused them to stash in the first place. So I\nexpect the most useful thing would be a \"git stash -f\" that adds them as\nempty. And it's reasonably safe, because we're not losing information in\nthe transition from index to stash tree without the user first having\nbeen notified.\n\nOn the other hand, it may be sufficient to just do the transformation\nwith a \"-f\", which will save users even more time, and we can put a note\nin the documentation about how stash interacts with -N. I don't know\nwhether people will actually care or not (and your patch already does\nthe unconditional form, so it's less work :) ).\n\n-Peff\n"},{"id":"122128","messageId":"7v63c5f4vs.fsf@alter.siamese.dyndns.org","threadId":"20771","inReplyTo":"20090830095509.GB30922@coredump.intra.peff.net","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-30T20:01:11Z","receivedAt":"2009-08-30T20:01:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sat, Aug 29, 2009 at 03:34:45PM -0700, Junio C Hamano wrote:\n>\n>> I am slightly in favor of leaving the things as they are, as the error\n>> message is quite clear.\n>\n> Hmm. Thinking about it a bit more, I think \"add as empty content\" is\n> probably the best. It scares me a little because it is losing\n> information during the stash, but consider it from the user's\n> perspective.\n> ...\n> And all of this while they are trying to quickly switch contexts to\n> whatever it was that caused them to stash in the first place.\n\nOk, then probably the \"how about\" patch would be a part of the right\nsolution.\n\nOne thing I noticed was that while unstashing without --index, we add full\ncontents to the index of new files.  I think it is because back then when\nstash was written there was no other way, but now we have intent-to-add\nand a way to stash such an entry, I think we should add only the intent to\nadd them in that codepath.\n\nOf course we will not do this when unstashing with --index.\n"},{"id":"122161","messageId":"20090831042724.GA16646@coredump.intra.peff.net","threadId":"20771","inReplyTo":"7v63c5f4vs.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T04:27:25Z","receivedAt":"2009-08-31T04:27:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 30, 2009 at 01:01:11PM -0700, Junio C Hamano wrote:\n\n> > And all of this while they are trying to quickly switch contexts to\n> > whatever it was that caused them to stash in the first place.\n> \n> Ok, then probably the \"how about\" patch would be a part of the right\n> solution.\n\nAnd then something like this on top (assuming you cut the change to\ngit-stash.sh from yours).\n\nMy concerns are:\n\n  - \"-f\" is kind of vague. Would people expect it to force aspects of\n    the stash? Should it be \"--intent-as-empty\"?\n\n  - the error message is still a bit muddled, because you get the \"not\n    yet added\" files _first_, then some failure cruft from write-tree,\n    and _then_ the trying-to-be-helpful message\n\nI dunno. Honestly I am a bit lukewarm about this whole thing, as it\nseems like something that just wouldn't come up that often, and while\nthe current error message is a bit disorganized, I think a user who has\nused \"git add -N\" can figure out that it is related (the only report we\nhave is from Yann, who _did_ figure it out, but wanted to know how to\nmake git handle the situation better).\n\ndiff --git a/git-stash.sh b/git-stash.sh\nindex d61c9d0..963cad0 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -20,6 +20,7 @@ TMP=\"$GIT_DIR/.git-stash.$$\"\n trap 'rm -f \"$TMP-*\"' 0\n \n ref_stash=refs/stash\n+force=\n \n no_changes () {\n \tgit diff-index --quiet --cached HEAD --ignore-submodules -- &&\n@@ -63,7 +64,14 @@ create_stash () {\n \tmsg=$(printf '%s: %s' \"$branch\" \"$head\")\n \n \t# state of the index\n-\ti_tree=$(git write-tree) &&\n+\tif ! i_tree=$(git write-tree ${force:+--intent-as-empty}); then\n+\t\tcase \"$force\" in\n+\t\tt) die 'Cannot save the current index state';;\n+\t\t*) echo >&2 'fatal: unable to create tree; if some files are marked as'\n+\t\t   echo >&2 '\"not added yet\", you may override with \"git stash save -f\"'\n+\t\t   exit 1\n+\t\tesac\n+\tfi\n \ti_commit=$(printf 'index on %s\\n' \"$msg\" |\n \t\tgit commit-tree $i_tree -p $b_commit) ||\n \t\tdie \"Cannot save the current index state\"\n@@ -104,6 +112,9 @@ save_stash () {\n \t\t-q|--quiet)\n \t\t\tGIT_QUIET=t\n \t\t\t;;\n+\t\t-f|--force)\n+\t\t\tforce=t\n+\t\t\t;;\n \t\t*)\n \t\t\tbreak\n \t\t\t;;\ndiff --git a/t/t3904-stash-intent.sh b/t/t3904-stash-intent.sh\nnew file mode 100755\nindex 0000000..ec7dd12\n--- /dev/null\n+++ b/t/t3904-stash-intent.sh\n@@ -0,0 +1,30 @@\n+#!/bin/sh\n+\n+test_description='stash with intent-to-add index entries'\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+\techo content >base &&\n+\tgit add base &&\n+\tgit commit -m base &&\n+\techo foo content >foo &&\n+\techo bar content >bar &&\n+\tgit add foo &&\n+\tgit add -N bar\n+'\n+\n+test_expect_success 'stash save refuses intent-to-add entry' '\n+\ttest_must_fail git stash save\n+'\n+\n+test_expect_success 'stash save -f allows intent-to-add' '\n+\tgit stash save -f &&\n+\tgit show stash^2:foo >foo.stash &&\n+\techo foo content >expect &&\n+\ttest_cmp expect foo.stash &&\n+\t>expect &&\n+\tgit show stash^2:bar >bar.stash &&\n+\ttest_cmp expect bar.stash\n+'\n+\n+test_done\n"},{"id":"122162","messageId":"20090831043610.GB16646@coredump.intra.peff.net","threadId":"20771","inReplyTo":"7v63c5f4vs.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T04:36:10Z","receivedAt":"2009-08-31T04:36:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 30, 2009 at 01:01:11PM -0700, Junio C Hamano wrote:\n\n> One thing I noticed was that while unstashing without --index, we add full\n> contents to the index of new files.  I think it is because back then when\n> stash was written there was no other way, but now we have intent-to-add\n> and a way to stash such an entry, I think we should add only the intent to\n> add them in that codepath.\n> \n> Of course we will not do this when unstashing with --index.\n\nAnd btw, I think your suggestion is reasonable, though I don't feel\nstrongly either way. I don't know that the current behavior is really\nbothering anybody.\n\n-Peff\n"},{"id":"122163","messageId":"7vljl0lgms.fsf@alter.siamese.dyndns.org","threadId":"20771","inReplyTo":"20090831042724.GA16646@coredump.intra.peff.net","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-31T05:03:07Z","receivedAt":"2009-08-31T05:03:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> My concerns are:\n>\n>   - \"-f\" is kind of vague. Would people expect it to force aspects of\n>     the stash? Should it be \"--intent-as-empty\"?\n>\n>   - the error message is still a bit muddled, because you get the \"not\n>     yet added\" files _first_, then some failure cruft from write-tree,\n>     and _then_ the trying-to-be-helpful message\n>\n> I dunno. Honestly I am a bit lukewarm about this whole thing, as it\n> seems like something that just wouldn't come up that often, and while\n> the current error message is a bit disorganized, I think a user who has\n> used \"git add -N\" can figure out that it is related (the only report we\n> have is from Yann, who _did_ figure it out, but wanted to know how to\n> make git handle the situation better).\n\nI am not sure if asking for positive confirmation with \"-f\" is even worth\nit.  As you pointed out in your earlier message, which prompted me to\nrespond with a patch, when this codepath is exercised, the user is in a\nrush, and I do not see what else the user would want to do other than\nincluding it in the stash by rerunning with -f.\n"},{"id":"122164","messageId":"20090831050554.GA17197@coredump.intra.peff.net","threadId":"20771","inReplyTo":"7vljl0lgms.fsf@alter.siamese.dyndns.org","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-08-31T05:05:54Z","receivedAt":"2009-08-31T05:05:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Aug 30, 2009 at 10:03:07PM -0700, Junio C Hamano wrote:\n\n> >   - \"-f\" is kind of vague. Would people expect it to force aspects of\n> >     the stash? Should it be \"--intent-as-empty\"?\n> \n> I am not sure if asking for positive confirmation with \"-f\" is even worth\n> it.  As you pointed out in your earlier message, which prompted me to\n> respond with a patch, when this codepath is exercised, the user is in a\n> rush, and I do not see what else the user would want to do other than\n> including it in the stash by rerunning with -f.\n\nI guess it was just to mitigate my fear that we are somehow creating a\nstash that will confuse people when they apply it. But really that fear\nis probably unjustified.\n\n-Peff\n"},{"id":"122175","messageId":"51ce4af01c8de1adb66a1818baab1f8a.squirrel@intranet.linagora.com","threadId":"20771","inReplyTo":"20090831050554.GA17197@coredump.intra.peff.net","subject":"Re: [BUG] git stash refuses to save after \"add -N\"","fromName":"Yann Dirson","fromEmail":"ydirson@linagora.com","sentAt":"2009-08-31T08:16:34Z","receivedAt":"2009-08-31T08:16:34Z","isPatch":false,"sender":{"key":"ydirson@linagora.com","avatar":null},"body":"> On Sun, Aug 30, 2009 at 10:03:07PM -0700, Junio C Hamano wrote:\n>\n>> >   - \"-f\" is kind of vague. Would people expect it to force aspects of\n>> >     the stash? Should it be \"--intent-as-empty\"?\n>>\n>> I am not sure if asking for positive confirmation with \"-f\" is even\n>> worth\n>> it.  As you pointed out in your earlier message, which prompted me to\n>> respond with a patch, when this codepath is exercised, the user is in a\n>> rush, and I do not see what else the user would want to do other than\n>> including it in the stash by rerunning with -f.\n>\n> I guess it was just to mitigate my fear that we are somehow creating a\n> stash that will confuse people when they apply it. But really that fear\n> is probably unjustified.\n\nWell, indeed each time I use \"add -N\" it is mostly to get \"git diff\" show\nme the contents of the new file as part of my WIP, so it would not make\nmuch difference to me if \"add -N\" would just add the file as empty to the\nindex in the first place.\n\nOut of curiosity, does anyone make any use of the current difference\nbetween not-added-yet and added-as-empty ?\n"}]}