peru behaves differently when called from a sub-directory (where peru.yaml is located)

[Tweakbert]

Hi,

I experience a different behaviour when peru is called from the same directory as the peru.yaml file compared to when it is called from a sub-directory. I am using peru on Windows 10, I only have peru V1.2.0 installed. To my understanding, when e.g. peru sync is executed from the command shell and peru.yaml can not be found in the current working directory, peru iterates through the parent directories, until either a peru.yaml file is found or there are no more parent directories that can be checked (root).

This works basically, but the behaviour I experience when calling peru from the directory containing peru.yaml is different from when I do the same call from a sub-directory.

Here's a simple example to reproduce the behaviour: I expected that when I call peru sync with the following peru.yaml file, that all the contents in the head revision of the specified repository/subdirectory would be imported into a new generated sub-directory called common. This sub-directory will be created in the folder where peru.yaml is located.

imports:
    txtTest: common

svn module txtTest:
    url: https://<someRepoPath>/dir1_bornRev32

Before trying to reproduce the behaviour, I noticed that it is always a good idea to delete the .peru directory. Let's assume a directory tree with dir0. This directory contains peru.yaml and the sub-directory dir1.

1. I change the working directory to /dir0/dir1
2. I execute peru sync. It returns without error and successfully creates the sub-directory dir0/common and imports all the files from url: https://<someRepoPath>/dir1_bornRev32, as it should.
3. Now I delete one or more random files from the common directory by hand.
4. I I execute peru sync again from /dir0/dir1 expecting it will re-import the files I just deleted. It executes without returning an error just like before, but it did not import the files I just deleted.
5. I change the working directory to the parent /dir0 and do exactly the same as before and call peru sync again. Again peru returns without an error just like before, but now the files I deleted before have been re-imported again.

I assume that the behaviour pointed out in 4. is not desired. Could you have a look at it? Thank you in advance.

Best Regards, Tweakbert

[oconnor663]

Good find!!! I can reproduce it with this example bash script, and I have a possible fix below.

cd `mktemp -d`
pwd
cat <<EOF > peru.yaml
imports:
    peru: perudir

git module peru:
    url: https://github.com/buildinspace/peru
EOF

# Look for an example file. I'm using README.md from the peru repo.

peru sync
if [[ ! -e perudir/README.md ]] ; then
    echo OOPS: expected to find README.md
    exit 1
fi

# Remove that file and re-sync. The file should be recreated.

rm perudir/README.md
peru sync
if [[ ! -e perudir/README.md ]] ; then
    echo OOPS: expected README.md to be recreated
    exit 1
fi

# Now the same thing, but from a subdirectory. BUG: This doesn't work.

rm perudir/README.md
mkdir subdir
cd subdir
peru sync
if [[ ! -e perudir/README.md ]] ; then
    echo OOPS: expected README.md to be recreated from subdir
    exit 1
fi

Here's an example change that seems like it might fix the problem (I've pushed this to the branch https://github.com/buildinspace/peru/tree/example_fix):

diff --git a/peru/cache.py b/peru/cache.py
index c459630..461fd1a 100644
--- a/peru/cache.py
+++ b/peru/cache.py
@@ -51,7 +51,7 @@ class GitSession:
         self.index_file = index_file
         self.working_copy = working_copy

-    async def git(self, *args, input=None, output_mode=TEXT_MODE):
+    async def git(self, *args, input=None, output_mode=TEXT_MODE, cwd=None):
         global DEBUG_GIT_COMMAND_COUNT
         DEBUG_GIT_COMMAND_COUNT += 1
         command = ['git']
@@ -63,6 +63,7 @@ class GitSession:
             input = input.encode()
         process = await asyncio.subprocess.create_subprocess_exec(
             *command,
+            cwd=cwd,
             env=self.git_env(),
             stdin=asyncio.subprocess.PIPE,
             stdout=asyncio.subprocess.PIPE,
@@ -178,7 +179,7 @@ class GitSession:

     async def checkout_files_from_index(self):
         'This recreates any deleted files.'
-        await self.git('checkout-index', '--all')
+        await self.git('checkout-index', '--all', cwd=self.working_copy)

     async def get_info_for_path(self, tree, path):
         # --full-tree makes ls-tree ignore the cwd. As in list_tree_entries,

In short, it seems like the Git plumbing command checkout-index might be sensitive to the directory that it's invoked from, despite the fact that we explicitly pass the --work-tree flag. That's not totally surprising to me, because git checkout has similar behavior. However, these plumbing commands tend to have some tricky corner cases, and I'll want to play around with this carefully (and maybe read some Git source code if I can find the right spot...) before we settle on this particular fix. Notably, this would be the first time we've needed to override the cwd, which is why that parameter needed to be plumbed through above.

@Tweakbert, could you confirm that this branch fixes the problem on your end? The easiest way to test it is to check out the branch and then execute peru using the peru.py script at the root of the repo.

[oconnor663]

Added a test for this in 7322dcd6c6ecb63756d68b2aba14c89c88fc2e2a, not yet pushed to master which I've pushed to master.

[Tweakbert]

Thank you for the quick fix, but I won't be able to check it before tomorrow. I will let you know as soon as possible.

[Tweakbert]

@oconnor663 I just did a quick test, comparing the new example_fix and V1.2.0 on the same folders. With example_fix I was not able to reproduce the issue anymore (the deleted files are now imported in both cases successfully) - looks good to me so far! Thank you.

[oconnor663]

Just pushed the fix to master. I'd like to cut a release soon, though another thing I want to get done first is switching us over to GitHub Actions for CI.

[oconnor663]

Ok all done, version 1.2.1 is out.