Hi Torsten
Thanks for working on this. I've cc'd Junio for his unpack_trees()
knowledge.
On 08/08/2023 18:26, tboegi@xxxxxx wrote:
From: Torsten Bögershausen <tboegi@xxxxxx>
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 <rev> README
will currently fail if <rev>:README is a blob but will succeed and
remove the untracked file if <rev>: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 <friebetill@xxxxxxxxx>
Signed-off-by: Torsten Bögershausen <tboegi@xxxxxx>
---
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
