Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent update behavior when a generated file was deleted #1721

Closed
lkubb opened this issue Aug 4, 2024 · 5 comments · Fixed by #1719
Closed

Inconsistent update behavior when a generated file was deleted #1721

lkubb opened this issue Aug 4, 2024 · 5 comments · Fixed by #1719

Comments

@lkubb
Copy link
Contributor

lkubb commented Aug 4, 2024

Describe the problem

Having deleted a file in a generated project, copier update behaves depending on the template's evolution:

  1. As long as no changes have been made to the file in question in the template repository, the generated file stays absent after the update (as expected).
  2. Any change in the template file causes the rendered file to be regenerated as if a (re)copy operation was requested.

Template

https://github.com/salt-extensions/salt-extension-copier (sadly requires --trust, but this could be reproduced easily with a simple demo template)

To Reproduce

  1. copier copy --trust --defaults --data project_name=foo --data author='Foo Bar' --data author_email='foo@b.ar' --data ssh_fixtures=true --vcs-ref=0.3.4 https://github.com/salt-extensions/salt-extension-copier foo
  2. cd foo; git init && git add . && git commit -m "initial commit"
  3. rm tests/conftest.py setup.py && git commit -am rm
  4. copier update --trust --skip-answered
  5. git status [shows new, untracked tests/conftest.py, but not setup.py]

Logs

No response

Expected behavior

(2) is surprising imho. I would expect one of the following behaviors:

a) Don't regenerate the file.
b) Warn about a merge conflict.

(a) makes the most sense to me here. It was intentionally deleted and we're running update, not recopy.

Screenshots/screencasts/logs

No response

Operating system

macOS

Operating system distribution and version

14.6

Copier version

9.1.0

Python version

CPython 3.11

Installation method

pipx+pypi

Additional context

This came up during the discussion of #1718.
The difference between this issue and the mentioned one is that paths affected by skip_if_exists are always recreated on update, regardless of the template's evolution.

@lkubb lkubb added bug triage Trying to make sure if this is valid or not labels Aug 4, 2024
@yajo
Copy link
Member

yajo commented Aug 7, 2024

In case 2, if Copier would just leave the file absent, you'd never know it changed upstream. A better solution would be a merge conflict indeed, but you're not expected to trust updates blindly. You should review them before commiting, because obviously there can be conflicts.

If you want to automate the updates, then do ˋcopier update --exclude the/path/i/dont/care/in/template.txtˋ and it will work as you expect.

@lkubb
Copy link
Contributor Author

lkubb commented Aug 7, 2024

First, thanks a lot for taking the time to look into my issues/proposal (and for maintaining Copier)!

you'd never know it changed upstream. A better solution would be a merge conflict indeed

I can see arguments for either.

For (a):

I deleted the file intentionally and thereby declared I'm not interested in the file, neither in updates to it. For the usual template, I wouldn't expect any interdependencies between separate files (at least not between those that projects would delete intentionally). If the deletion breaks something down the line or I change my mind, I can recover the necessary files by just running copier recopy. No need for copier update to take care of that.

For (b):
I deleted the contents of the file at the time, but (at least in theory) an update might make it relevant again. I would miss this update, unless a merge conflict was reported. It doesn't hurt to ask (unless it converts a mostly automated process into a very manual one, maybe behavior (a) could be allowed to be enabled explicitly by the project/template/user if (b) is chosen as the default solution).

You should review them before commiting, because obviously there can be conflicts.

Of course. In contrast to the skip_if_exists behavior, which is truly unnerving in the mentioned template's case since each update often recreates 10+ boilerplate files, this is just a surprising inconsistency to me. Silently recreating it if it changed in the template is still not the correct behavior imo since it disrespects the user's intentions.

If you want to automate the updates

Yes, this is a substantial motivation here - there are currently around 36 projects generated from the mentioned template (soon likely several times that), which all need to be kept up to date (using Renovate, similar to Dependabot). Renovate does not allow specifying Copier CLI arguments though (I wrote the feature PR initially with support for that, in part because of the skip_if_exists issue, but the Renovate maintainers did not want to add Copier-specific configuration).

Something that would help alleviate the immediate automation problems is if Copier respected exclude from the generated project, as proposed here: #1718 (comment)

@yajo
Copy link
Member

yajo commented Aug 12, 2024

I wouldn't like to make updates interactive again. Life is better since we turned off that.

For (a):

I deleted the file intentionally and thereby declared I'm not interested in the file, neither in updates to it. For the usual template, I wouldn't expect any interdependencies between separate files (at least not between those that projects would delete intentionally). If the deletion breaks something down the line or I change my mind, I can recover the necessary files by just running copier recopy. No need for copier update to take care of that.

After a 2nd thought, this is probably the right solution. Thank you!

@yajo yajo added enhancement and removed triage Trying to make sure if this is valid or not bug labels Aug 12, 2024
@yajo yajo added this to the Community contribution milestone Aug 12, 2024
@lkubb
Copy link
Contributor Author

lkubb commented Aug 12, 2024

I wouldn't like to make updates interactive again.

Agreed. I'm not sure which part of my proposal indicated making updates interactive though. :)

After a 2nd thought, this is probably the right solution.

👍 Since you closed #1718 as not planned, should files listed in skip_if_exists be excluded from update exclusion then (i.e. always be recreated if missing)? I will adjust my PR in that case. (Edit: Already adjusted.)

@yajo
Copy link
Member

yajo commented Aug 16, 2024

Sorry I've had like 4 different requests about the same subject at the same time and it's been confusing to me.

Let's keep the case for "skip if exists" out of the picture for now. It's probably unrelated. You were trying to use that feature for what it isn't meant, so the behavior was surprising.

Once we fix this, you will probably don't need to care about skip if exists. And if you do, then the issue will probably be more clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants