fix(vcs): prevent local clone from being stuck by gpg prompts

[noirbizarre]

A quick and simple PR preventing the copy command to get stuck if you have commit.gpgSign and try to run the copy command on WIP local project for trial.

[sisp]

Thanks, @noirbizarre! 🙏

I think you're aiming for a subset of #856, which unfortunately seems abandoned. So, I anticipate updating a generated project with GPG commit signing enabled would be a problem for you, too. Would you be interested in finishing #856 here? I.e., add the git config line in main.py and adapt the tests accordingly? This would be in line with @yajo's pragmatic suggestion:

Another option is forget about that and just add the --no-gpg-sign flag wherever needed. I don't think this would be too complicated honestly.

— #1224 (comment)

Related to #1224.

[noirbizarre]

Thanks, I didn't saw it (my bad). I'll resubmit the PR integrating #1224 and #856 👍🏼

[codecov]

Codecov Report

Merging #1360 (92249e9) into master (281a229) will increase coverage by 0.21%.
Report is 2 commits behind head on master.
The diff coverage is 98.00%.

@@            Coverage Diff             @@
##           master    #1360      +/-   ##
==========================================
+ Coverage   97.19%   97.41%   +0.21%     
==========================================
  Files          48       48              
  Lines        4281     4327      +46     
==========================================
+ Hits         4161     4215      +54     
+ Misses        120      112       -8     

Flag	Coverage Δ
unittests	97.41% <98.00%> (+0.21%)	⬆️

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

Files	Coverage Δ
copier/main.py	96.21% <100.00%> (ø)
copier/vcs.py	97.32% <ø> (ø)
tests/test_dirty_local.py	100.00% <100.00%> (ø)
tests/conftest.py	93.54% <93.33%> (-0.57%)	⬇️

... and 1 file with indirect coverage changes

[yajo]

There are still more git commits issued. Maybe it's the time to fix all?

copier/copier/main.py

Lines 946 to 947 in 1196644

	git("commit", "--allow-empty", "-am", "dumb commit 1", retcode=None)
	git("commit", "--allow-empty", "-am", "dumb commit 2", "--no-verify")

copier/tests/helpers.py

Line 149 in 1196644

git("commit", "-m", message)

There are still more in the tests suite. See #616.

[noirbizarre]

OK, there is in fact 2 distinct issues in this:

• tests performs their git-based preparation with the user config which might conflict/interfere with the tests
• copier is also commiting with the user config but this is legit and should be properly handled

PR updated with the following:

• tests use a sandboxed .gitconfig using pytest-gitconfig, tests are left untouched, the git sandboxing fixture is just always loaded
• a specific and explicit test case has been added for the copy operation with user.gpgsigninkey/commit.gpgsign to ensure copier is properly handling this case
• a specific and explicit test case has been added for the update operation with user.gpgsigninkey/commit.gpgsign to ensure copier is properly handling this case
• --no-gpg-sign has been added to copier commit calls only (it is faster and easier to maintain than trying to monkeypatch plumbum or gitconfig, there was only 3 calls)

If there are some new or untested cases which might conflict with user.gpgsigninkey/commit.gpgsign, an explicit test case for this case should be added on the same model than tests/test_dirty_local.py::test_copy_dirty_head_with_gpg

[yajo]

Awesome!

[yajo]

I made a push including nix-community/poetry2nix#1342 to see if that fixes the build.

[noirbizarre]

I have no expertise on Nix and I really can't help on this part.

TBH I was surprise with the .envrc automatically building using Nix for 30min and I wonder if this part should be optin and excluded from CI ? It feels like artificially raising the complexity with an hard system dependency. NiX is meant for reproducibility but I can't reproduce this error, build pass as well as pre-commit checks here 🤷‍♂️ (I didn't modified anything in the setup, so I can't say why it is working fine here and failing in CI)

However I am the pytest-gitconfig author so if something is needed for NiX compatibility (given the module it would be surprizing, but again, I am not knowledgeable on NiX), tell me, I'll do the change (this is the first time I encounter an error on this one)

EDIT: I just saw the upstream poetry2nix contribution. Does it mean that for each python package there should be an entry in this file ? Isn't it defeating PEP517 ?
Anyway it seems my latest build failed with the same error on Python 3.8. Investigating in order to fix. But other version error (which I can't see for the moment), I don't have them locally. Investigating too

[yajo]

Don't worry, it's just a common routine for projects like this one that use poetry2nix. There's nothing special for you to do in pytest-gitconfig to add that compatibility.

Using nix is just because it's great! Most concerns about it is "it's complex", but in reality, once you learn it, it removes lots of complexities. It's obsoleting containers. I just can't live without it. 🤷🏼‍♂️

Regarding test failures, I see mypy failures (probably typing errors in some test) and also gpg signinig failures. It seems like, when you wanted to skip gpg, you actually enabled it everywhere! But you can see that all versions of CI (even those that don't use Nix) are broken, so it doesn't seem like a Nix problem at first sight.

I wouldn't like to go too off-topic about nix here, but if you have any questions please feel free to open thread(s) in the forum. 😊

[noirbizarre]

Last failures were due to:

• a pytest-gitconfig compatibility issue with 3.8
• plumbum.local.env being snapshotted once at start and requiring its own monkeypatching has changes to os.environ are ignored

PR (and pytest-gitconfig) updated and should be fixed 👍🏼

[noirbizarre]

Sorry, comment posted in the wrong place.

PR updated 👍🏼

Note: @yajo for nix it was just bad timing, latest update was broken for 3 days on archlinux. It's now fixed and it works 👍🏼

EDIT: I think I also found the Windows issue: some git parameter were set from outside in CI (and so ignored by pytest-gitconfig doing it's job). Given user.name and user.email are already automatically set by pytest-gitconfig, I think windows was just missing the core.autocrlf=input. Ci should pass (unless there are other Windows specific issues)

All attended