fix(vcs): git checkout after clone must contains `-- .`

[kamontat]

Without -- . on the end cause git to not checkout latest changes from .git directory.

[fixes #1887]

[kamontat]

@sisp I actually cannot reproduce on linux machine. The problem seem to occurred only on MacOS.

Here I tried on gitpod (follow CONTRIBUTING.md). It checkout correctly.

However, I do same thing on MacOS. It doesn't work.

Because of this I don't think test will help anything as it always pass when test on linux machine. I added anyway tho.

[kamontat]

btw: those 2 commits were create from gitpod website so it not signed.

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 97.73%. Comparing base (3770a4d) to head (6d5b1e8). :warning: Report is 154 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2151      +/-   ##
==========================================
- Coverage   97.91%   97.73%   -0.19%     
==========================================
  Files          55       55              
  Lines        5949     6088     +139     
==========================================
+ Hits         5825     5950     +125     
- Misses        124      138      +14     

Flag	Coverage Δ
unittests	97.73% <100.00%> (-0.19%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[sisp]

Odd, the test case is passing on CI macOS although the fix is commented out. So, the test case doesn't seem to reproduce the problem correctly. Does the test fail on your local macOS?

[kamontat]

@sisp I cannot find why, but when I run clone() from test it always passed. It's not the case when I execute copier copy command.

Here is example I ran on my local machine:

Please ignore the error, I need to force to error so I can view print() result.

The output directory from tests can be checkout correctly.

However when I run copy command on dirty repository, It didn't copy template correctly.

Which after I remove dirtiness, the template is back to correct again.

Note: The red box is the print() output from _vcs#clone() function I added to verify if parameter is the same. Note 2: I never work with python before, so now I just stuck. It seem to be some misconfigure between test environment and real environment. Note 3: I create bash script to simulate the checkout command here

#!/usr/bin/env bash

## script.sh /path/to/template
## Expected ls -la result to contains more than .git directory

set -e

INPUT="$1"
OUTPUT="$(mktemp -d)"

rm -rf $OUTPUT

set -x

git="/usr/bin/git"

$git clone --no-checkout "$INPUT" "$OUTPUT"
cd "$OUTPUT"

$git --git-dir=.git --work-tree=$INPUT add -A
$git --git-dir=.git --work-tree=$INPUT commit -m "Copier automated commit for draft changes" --no-verify --no-gpg-sign

$git checkout -f HEAD

ls -la

Not sure if matter or not, but I have 2 git installed on my machine which I tested both of them have same issue.

[kamontat]

@sisp Do you need any help from me? I would love get this merge because I have to manually update copier code every time I install it.

[sisp]

@kamontat Sorry for the delay. I was hoping we'd find a way to test this on the macOS runner. I find it a little suspicious why running the Copier CLI is producing this error for you while the test isn't. :thinking: In the test suite, we're configuring an isolated Git config to avoid side effects from the global/system Git config. I wonder whether this might explain the difference on your machine. Not sure whether the test suite will run properly, but could you try commenting out the default_gitconfig fixture and running only your added test case?

[kamontat]

@sisp You right, now I able to reproduce failed test when core.fsmonitor is set to true. If you re-run the tests, it should fail now.

[sisp]

Tests on macOS and Windows are failing now – great! :tada: Thanks for figuring this out! :bow:

You can enable the fix now, and when the tests pass, we can merge it.

[kamontat]

@sisp Applied hopefully the ci will pass. Thank you for your help.

[sisp]

Now, several other tests are failing. 🤔

[kamontat]

Now, several other tests are failing. 🤔

Any suggestion?

[sisp]

How about disabling core.fsmonitor in the local config of the cloned repository? IIUC, this feature isn't important for our use of Git.

[kamontat]

@sisp I need core.fsmonitor on some of my project as it significantly improve on performance. Is it ok, if I implement try-then-fallback if repo is not clone correctly?

So it will be...

• First then without --, then check
• If check failed, try again with --; then check
• If check failed again, then return error

[sisp]

Not sure if we're talking about the same approach – I might have expressed my suggestion badly. This is what I meant:

We could disable core.fsmonitor in Copier's internal template repository clone after

https://github.com/copier-org/copier/blob/1e16b65e8a4c0de9bcdaf527e1a9f8527c3ef309/copier/_vcs.py#L190

like this:

 _clone()
+git("-C", location, "config", "--local", "core.fsmonitor", "false")

Thinking about it some more, perhaps it might be even better to disable core.fsmonitor only for the git checkout call like this:

-git("checkout", "-f", ref or "HEAD")
+git("-c", "core.fsmonitor=false", "checkout", "-f", ref or "HEAD")

WDYT?

[kamontat]

@sisp Oh, I see. I updated code to overrides core.fsmonitor to false on checkout command. Please rerun the test, thank you.

[sisp]

Looks like this is working. :+1: Could you add an inline comment above that line with a link to this issue, so we'll remember why the core.fsmonitor=false setting is used there? Then, I think we can merge this PR. :tada:

[kamontat]

Looks like this is working. 👍 Could you add an inline comment above that line with a link to this issue, so we'll remember why the core.fsmonitor=false setting is used there? Then, I think we can merge this PR. 🎉

@sisp Added