From: Phillip Wood Date: Wed, 09 Aug 2023 13:15:28 GMT Subject: Re: [PATCH v1 1/1] git stash needing mkdir deletes untracked file Message-ID: <6e40eb0b-2331-1e39-bee0-c9720c24d1c8@gmail.com> In-Reply-To: <20230808172624.14205-1-tboegi@web.de> Hi Torsten Thanks for working on this. I've cc'd Junio for his unpack_trees() knowledge. On 08/08/2023 18:26, tboegi@web.de wrote: > From: Torsten Bögershausen > > The following sequence leads to loss of work: > git init > mkdir README > touch README/README > git add . > git commit -m "Init project" > echo "Test" > README/README > mv README/README README2 > rmdir README > mv README2 README > git stash > git stash pop > > The problem is, that `git stash` needs to create the directory README/ > and to be able to do this, the file README needs to be removed. > And this is, where the work was lost. > There are different possibilities preventing this loss of work: > a) > `git stash` does refuse the removel of the untracked file, > when a directory with the same name needs to be created > There is a small problem here: > In the ideal world, the stash would do nothing at all, > and not do anything but complain. > The current code makes this hard to achieve > An other solution could be to do as much stash work as possible, > but stop when the file/directory conflict is detected. > This would create some inconsistent state. > > b) Create the directory as needed, but rename the file before doing that. > This would let the `git stash` proceed as usual and create a "new" file, > which may be surprising for some worlflows. > > This change goes for b), as it seems the most intuitive solution for > Git users. > > Introdue a new function rename_to_untracked_or_warn() and use it > in create_directories() in entry.c Although this change is framed in terms of changes to "git stash push" I think the underlying issue and this patch actually affects all users of unpack_trees(). For example if "README" is untracked then git checkout README will currently fail if :README is a blob but will succeed and remove the untracked file if :README is a tree. I'm far from an expert in this area but I think we might want to understand why unpack_trees() sets state->force when it calls checkout_entry() before making any changes. Best Wishes Phillip > Reported-by: Till Friebe > Signed-off-by: Torsten Bögershausen > --- > entry.c | 25 ++++++++++++++++++++++++- > t/t3903-stash.sh | 23 +++++++++++++++++++++++ > 2 files changed, 47 insertions(+), 1 deletion(-) > > diff --git a/entry.c b/entry.c > index 43767f9043..76d8a0762d 100644 > --- a/entry.c > +++ b/entry.c > @@ -15,6 +15,28 @@ > #include "entry.h" > #include "parallel-checkout.h" > > +static int rename_to_untracked_or_warn(const char *file) > +{ > + const size_t file_name_len = strlen(file); > + const static char *dot_untracked = ".untracked"; > + const size_t dot_un_len = strlen(dot_untracked); > + struct strbuf sb; > + int ret; > + > + strbuf_init(&sb, file_name_len + dot_un_len); > + strbuf_add(&sb, file, file_name_len); > + strbuf_add(&sb, dot_untracked, dot_un_len); > + ret = rename(file, sb.buf); > + > + if (ret) { > + int saved_errno = errno; > + warning_errno(_("unable rename '%s' into '%s'"), file, sb.buf); > + errno = saved_errno; > + } > + strbuf_release(&sb); > + return ret; > +} > + > static void create_directories(const char *path, int path_len, > const struct checkout *state) > { > @@ -48,7 +70,8 @@ static void create_directories(const char *path, int path_len, > */ > if (mkdir(buf, 0777)) { > if (errno == EEXIST && state->force && > - !unlink_or_warn(buf) && !mkdir(buf, 0777)) > + !rename_to_untracked_or_warn(buf) && > + !mkdir(buf, 0777)) > continue; > die_errno("cannot create directory at '%s'", buf); > } > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 0b3dfeaea2..1a210f8a5a 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -1512,4 +1512,27 @@ test_expect_success 'restore untracked files even when we hit conflicts' ' > ) > ' > > +test_expect_success 'stash mkdir README needed - README.untracked created' ' > + git init mkdir_needed_file_untracked && > + ( > + cd mkdir_needed_file_untracked && > + mkdir README && > + touch README/README && > + git add . && > + git commit -m "Add README/README" && > + echo Version2 > README/README && > + mv README/README README2 && > + rmdir README && > + mv README2 README && > + git stash && > + test_path_is_file README.untracked && > + echo Version2 >expect && > + test_cmp expect README.untracked && > + rm expect && > + git stash pop && > + test_path_is_file README.untracked && > + echo Version2 >expect && > + test_cmp expect README.untracked > + ) > +' > test_done > -- > 2.41.0.394.ge43f4fd0bd >