Skip to content

fix: 🐛 fix copier update#144

Closed
martonvago wants to merge 4 commits into
mainfrom
fix/copier-update
Closed

fix: 🐛 fix copier update#144
martonvago wants to merge 4 commits into
mainfrom
fix/copier-update

Conversation

@martonvago
Copy link
Copy Markdown
Contributor

Description

This PR fixes and improves copier update.

Closes #143

This PR needs a medium-depth review.

Checklist

  • Added or updated tests
  • Updated documentation
  • Ran just run-all -- will fail until some other PRs are merged in

@martonvago martonvago self-assigned this Aug 15, 2025
@martonvago martonvago moved this from Todo to In Review in Product development Aug 15, 2025

- name: Pull request with updates from template
run: |
just update-from-template
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed this to use the just recipe

Comment thread template/justfile.jinja
update-from-template:
uvx copier update --trust --defaults
# Do not update existing source files
uvx copier update --trust --defaults $(find src/{{ github_repo_snake_case }} -type f -printf "--exclude %p ")
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem was that running copier update ... deleted some files in the src/{{ github_repo_snake_case }} folder. Maybe because the folder name is templated?

In any case, there is a _skip_if_exists property in copier.yml, where you can set which files not to touch if they exist. Setting this worked for some test files I put in the folder, but interestingly not for __init__.py and py.typed. Even skipping all files in the folder didn't catch these two 🤔 .

The _exclude property is similar, but it stays in force regardless of the file existing, so a deleted file will not be recreated.

This version with the --exclude flag is a workaround: it excludes all files in the folder that exist, recreating the behaviour of _skip_if_exists in copier.yml. We could narrow it to only the two problematic files if this is too broad.

Any other ideas welcome!!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Yea, I had been wondering about this.. We'll have to see how we use all of this stuff in practice, will be interesting!

@@ -1,2 +1,2 @@
# Changes here will be overwritten by Copier; NEVER EDIT MANUALLY
{{ dict(_copier_answers, copyright_year=copyright_year) | to_nice_yaml -}}
{{ dict(_copier_answers, github_repo=github_repo, copyright_year=copyright_year) | to_nice_yaml -}}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise copier will use a temp folder name it uses under the hood

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change and how it relates to this PR. What is this doing?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

github_repo defaults to the folder name. When updating, copier seems to keep the project in a temporary folder when it resolves the variables and ends up setting the name of this folder as github_repo. If we save the correct github_repo in the answers file, it will be read from there instead.

@martonvago martonvago marked this pull request as ready for review August 15, 2025 13:03
@martonvago martonvago requested a review from a team August 15, 2025 13:03
Copy link
Copy Markdown
Member

@lwjohnst86 lwjohnst86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you keep a focus on making PRs atomic? The changes in this PR should be done in #134, not as a separate PR. I've made the #134 atomic, so now you can join this PR into that one and we can close it after. ☺️

@@ -1,2 +1,2 @@
# Changes here will be overwritten by Copier; NEVER EDIT MANUALLY
{{ dict(_copier_answers, copyright_year=copyright_year) | to_nice_yaml -}}
{{ dict(_copier_answers, github_repo=github_repo, copyright_year=copyright_year) | to_nice_yaml -}}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change and how it relates to this PR. What is this doing?

Comment thread template/justfile.jinja
update-from-template:
uvx copier update --trust --defaults
# Do not update existing source files
uvx copier update --trust --defaults $(find src/{{ github_repo_snake_case }} -type f -printf "--exclude %p ")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Yea, I had been wondering about this.. We'll have to see how we use all of this stuff in practice, will be interesting!

@github-project-automation github-project-automation Bot moved this from In Review to In Progress in Product development Aug 24, 2025
@martonvago
Copy link
Copy Markdown
Contributor Author

Moved this over to #134 . It does depend on the latest changes to copier.yaml, so tests will be a bit broken until #140 is merged in.

@martonvago martonvago closed this Aug 25, 2025
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Product development Aug 25, 2025
@lwjohnst86 lwjohnst86 deleted the fix/copier-update branch August 26, 2025 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Refine update-from-template workflow

2 participants