Repository: jelmer/dulwich

Issue:
porcelaine.reset is not deleting files locally
The command
`porclain.reset(repo, 'hard', treeish=b'refs/remote/origin/master')`
 is not deleting files locally, if they we're deleted in my remote repo.  Everything else (any changed files) works fine and is reseted. Is there a way to solve this problem?

Discussion:
@jelmer: On Sun, Feb 14, 2021 at 08:49:30AM -0800, chrisiweb wrote:
> The command
> `porclain.reset(repo, 'hard', treeish=b'refs/remote/origin/master')`
>  is not deleting files locally, if they we're deleted in my remote repo.  Everything else (any changed files) works fine and is reseted. Is there a way to solve this problem?

That's correct - "git reset --hard" doesn't delete files either
AFAICT.

Jelmer

-- 
Jelmer Vernooĳ <jelmer@jelmer.uk>
PGP Key: https://www.jelmer.uk/D729A457.asc
@chrisiweb: In fact "git reset --hard" does remove files locally if they were deleted in the remote...@jelmer: On Sun, Feb 14, 2021 at 09:52:09PM -0800, chrisiweb wrote:
> In fact "git reset --hard" does remove files locally if they were deleted in the remote...
"git reset --hard" doesn't look at the remotes at all as far as I can
tell. Could you post an example script that reproduces this behaviour?

-- 
Jelmer Vernooĳ <jelmer@jelmer.uk>
PGP Key: https://www.jelmer.uk/D729A457.asc
@chrisiweb: Yes i guess this is true, but I believe calling git fetch first, should work. 
If I call via a python script:
`repo = porcelain.Repo(path_to_local_repo)`
`porcelain.fetch(repo)`
`porcelain.reset(repo, 'hard', treeish=b'refs/remote/origin/master')`

This unfortunately does not remove deleted files.
However, if I call:
`git fetch`
`git reset --hard`
 in the terminal, it works! 
Where is my mistake? Did I missunderstand something?
@jelmer: On Mon, Feb 15, 2021 at 06:56:00AM -0800, chrisiweb wrote:
> Yes i guess this is true, but I believe calling git fetch first, should work. 
> If I call via a python script:
> `repo = porcelain.Repo(link_to_local_repo)`
> `porcelain.fetch(repo)`
> `porcelain.reset(repo, 'hard', treeish=b'refs/remote/origin/master')`
> 
> This unfortunately does not remove deleted files.
> However, if I call:
> `git fetch`
> `git reset --hard`
>  in the terminal, it works! 
> Where is my mistake? Did I missunderstand something?

Can you post the exact commands you're running? If I try that here,
remotely removed files are not removed locally::

    #!/bin/sh
    git init 1
    cd 1
    touch a b
    git add a b
    git commit -mm a b
    cd ..
    git clone 1 2
    cd 1
    rm b
    git commit -m "remove b" b
    cd ../2
    git fetch
    git reset --hard
    ls

Results in::

    Initialized empty Git repository in /tmp/1/.git/
    [master (root-commit) eb009e6] m
     2 files changed, 0 insertions(+), 0 deletions(-)
     create mode 100644 a
     create mode 100644 b
    Cloning into '2'...
    done.
    [master 0787dad] remove b
     1 file changed, 0 insertions(+), 0 deletions(-)
     delete mode 100644 b
    remote: Enumerating objects: 3, done.
    remote: Counting objects: 100% (3/3), done.
    remote: Compressing objects: 100% (1/1), done.
    remote: Total 2 (delta 0), reused 0 (delta 0), pack-reused 0
    Unpacking objects: 100% (2/2), 869 bytes | 869.00 KiB/s, done.
    From /tmp/1
       eb009e6..0787dad  master     -> origin/master
    HEAD is now at eb009e6 m
    a  b

(in other words, b is not deleted)

-- 
Jelmer Vernooĳ <jelmer@jelmer.uk>
PGP Key: https://www.jelmer.uk/D729A457.asc
@chrisiweb: I can reproduce this. However, if I change to
 
`git reset --hard origin/master`

in your example it works and b is also deleted in 2.
What I actually do (and what works with git but unfortunately not with dulwich.porcelain):

`git clone url_to_repo_on_github`

I then delete a file directly on Github (in browser) and commit.
I then fetch and reset my local repo with:

`git fetch`
`git reset --hard origin/master`

My local repo is now identical to my repo on Github.
(`git reset --hard` however, does not update anything! Neither changed nor newly created files nor deleted files)

So if my local repo is reseted to origin/master (and not to HEAD) it works with git.@chrisiweb: If removing files in the local repo (if they were deleted on the remote repo) - as described above -  is not implemented in `porcelain.reset()`, is there, however, a way to achieve this manually with dulwich/porcelain? I tried hard, but I could not succeed :( Would be really glad for your help...@jelmer: On Fri, Feb 19, 2021 at 07:54:33AM -0800, chrisiweb wrote:
> If removing files in the local repo (if they were deleted on the remote repo) - as described above -  is not implemented in `porcelain.reset()`, is there, however, a way to achieve this manually with dulwich/porcelain? I tried hard, but I could not succeed :( Would be really glad for your help...
Ah, interesting. I guess that means that "git reset" does look at the
current working tree to determine what files to remove.

-- 
Jelmer Vernooĳ <jelmer@jelmer.uk>
PGP Key: https://www.jelmer.uk/D729A457.asc


Solution patch:
diff --git a/dulwich/index.py b/dulwich/index.py
index a2e4aeec..b60d17aa 100644
--- a/dulwich/index.py
+++ b/dulwich/index.py
@@ -1549,6 +1549,8 @@ def update_working_tree(
                 full_path = os.path.join(root, file)
                 # Get relative path from repo root
                 rel_path = os.path.relpath(full_path, repo_path_str)
+                # Normalize to use forward slashes like Git does internally
+                rel_path = rel_path.replace(os.sep, "/")
                 rel_path_bytes = rel_path.encode()
 
                 # If this file is not in the target tree, remove it
diff --git a/dulwich/server.py b/dulwich/server.py
index f160006e..ae3a65f1 100644
--- a/dulwich/server.py
+++ b/dulwich/server.py
@@ -204,6 +204,10 @@ class FileSystemBackend(Backend):
 
     def open_repository(self, path):
         logger.debug("opening repository at %s", path)
+        # Ensure path is a string to avoid TypeError when joining with self.root
+        path = os.fspath(path)
+        if isinstance(path, bytes):
+            path = os.fsdecode(path)
         abspath = os.path.abspath(os.path.join(self.root, path)) + os.sep
         normcase_abspath = os.path.normcase(abspath)
         normcase_root = os.path.normcase(self.root)


Test patch:
diff --git a/tests/test_porcelain.py b/tests/test_porcelain.py
index 771b5dab..1e39f3d8 100644
--- a/tests/test_porcelain.py
+++ b/tests/test_porcelain.py
@@ -2155,6 +2155,124 @@ class ResetTests(PorcelainTestCase):
 
         self.assertEqual([], changes)
 
+    def test_hard_deletes_untracked_files(self) -> None:
+        """Test that reset --hard deletes files that don't exist in target tree."""
+        # Create and commit a file
+        fullpath = os.path.join(self.repo.path, "foo")
+        with open(fullpath, "w") as f:
+            f.write("BAR")
+        porcelain.add(self.repo.path, paths=[fullpath])
+        sha1 = porcelain.commit(
+            self.repo.path,
+            message=b"First commit",
+            committer=b"Jane <jane@example.com>",
+            author=b"John <john@example.com>",
+        )
+
+        # Create another file and commit
+        fullpath2 = os.path.join(self.repo.path, "bar")
+        with open(fullpath2, "w") as f:
+            f.write("BAZ")
+        porcelain.add(self.repo.path, paths=[fullpath2])
+        porcelain.commit(
+            self.repo.path,
+            message=b"Second commit",
+            committer=b"Jane <jane@example.com>",
+            author=b"John <john@example.com>",
+        )
+
+        # Reset hard to first commit - this should delete 'bar'
+        porcelain.reset(self.repo, "hard", sha1)
+
+        # Check that 'foo' still exists and 'bar' is deleted
+        self.assertTrue(os.path.exists(fullpath))
+        self.assertFalse(os.path.exists(fullpath2))
+
+        # Check index matches first commit
+        index = self.repo.open_index()
+        self.assertIn(b"foo", index)
+        self.assertNotIn(b"bar", index)
+
+    def test_hard_deletes_files_in_subdirs(self) -> None:
+        """Test that reset --hard deletes files in subdirectories."""
+        # Create and commit files in subdirectory
+        subdir = os.path.join(self.repo.path, "subdir")
+        os.makedirs(subdir)
+        file1 = os.path.join(subdir, "file1")
+        file2 = os.path.join(subdir, "file2")
+
+        with open(file1, "w") as f:
+            f.write("content1")
+        with open(file2, "w") as f:
+            f.write("content2")
+
+        porcelain.add(self.repo.path, paths=[file1, file2])
+        porcelain.commit(
+            self.repo.path,
+            message=b"First commit",
+            committer=b"Jane <jane@example.com>",
+            author=b"John <john@example.com>",
+        )
+
+        # Remove one file from subdirectory and commit
+        porcelain.rm(self.repo.path, paths=[file2])
+        sha2 = porcelain.commit(
+            self.repo.path,
+            message=b"Remove file2",
+            committer=b"Jane <jane@example.com>",
+            author=b"John <john@example.com>",
+        )
+
+        # Create file2 again (untracked)
+        with open(file2, "w") as f:
+            f.write("new content")
+
+        # Reset to commit that has file2 removed - should delete untracked file2
+        porcelain.reset(self.repo, "hard", sha2)
+
+        self.assertTrue(os.path.exists(file1))
+        self.assertFalse(os.path.exists(file2))
+
+    def test_hard_reset_to_remote_branch(self) -> None:
+        """Test reset --hard to remote branch deletes local files not in remote."""
+        # Create a file and commit
+        file1 = os.path.join(self.repo.path, "file1")
+        with open(file1, "w") as f:
+            f.write("content1")
+        porcelain.add(self.repo.path, paths=[file1])
+        sha1 = porcelain.commit(
+            self.repo.path,
+            message=b"Initial commit",
+            committer=b"Jane <jane@example.com>",
+            author=b"John <john@example.com>",
+        )
+
+        # Create a "remote" ref that doesn't have additional files
+        self.repo.refs[b"refs/remotes/origin/master"] = sha1
+
+        # Add another file locally and commit
+        file2 = os.path.join(self.repo.path, "file2")
+        with open(file2, "w") as f:
+            f.write("content2")
+        porcelain.add(self.repo.path, paths=[file2])
+        porcelain.commit(
+            self.repo.path,
+            message=b"Add file2",
+            committer=b"Jane <jane@example.com>",
+            author=b"John <john@example.com>",
+        )
+
+        # Both files should exist
+        self.assertTrue(os.path.exists(file1))
+        self.assertTrue(os.path.exists(file2))
+
+        # Reset to remote branch - should delete file2
+        porcelain.reset(self.repo, "hard", b"refs/remotes/origin/master")
+
+        # file1 should exist, file2 should be deleted
+        self.assertTrue(os.path.exists(file1))
+        self.assertFalse(os.path.exists(file2))
+
 
 class ResetFileTests(PorcelainTestCase):
     def test_reset_modify_file_to_commit(self) -> None:
diff --git a/tests/test_server.py b/tests/test_server.py
index 0b9e2a6f..c4d9af01 100644
--- a/tests/test_server.py
+++ b/tests/test_server.py
@@ -1092,6 +1092,15 @@ class FileSystemBackendTests(TestCase):
 
         self.assertRaises(NotGitRepository, lambda: backend.open_repository("/ups"))
 
+    def test_bytes_path(self) -> None:
+        # Test that FileSystemBackend can handle bytes paths (issue #973)
+        repo = self.backend.open_repository(self.path.encode("utf-8"))
+        self.assertTrue(
+            os.path.samefile(
+                os.path.abspath(repo.path), os.path.abspath(self.repo.path)
+            )
+        )
+
 
 class DictBackendTests(TestCase):
     """Tests for DictBackend."""