Skip to content

fix: resolve path between dst_path and answers_file to handle dst_path not already existing #2352

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.

Sign up for GitHub

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

Open
Aohzan wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: resolve path between dst_path and answers_file to handle dst_path not already existing #2352

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion copier/_user_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,7 +577,8 @@ def load_answersfile_data(
) -> AnyByStrDict:
"""Load answers data from a `$dst_path/$answers_file` file if it exists."""
try:
with Path(dst_path, answers_file).open("rb") as fd:
answers_path = Path(dst_path, answers_file).resolve()
with answers_path.open("rb") as fd:
return yaml.safe_load(fd)
except (FileNotFoundError, IsADirectoryError):
if warn_on_missing:
Expand Down
46 changes: 46 additions & 0 deletions tests/test_answersfile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,3 +333,49 @@ def test_undefined_phase_in_external_data(
copier.run_copy(str(src), dst, defaults=True, overwrite=True)
answers = load_answersfile_data(dst, ".copier-answers.yml")
assert answers["key"] == "value"



def test_answersfile_relative_path_nonexistent_dst(
tmp_path_factory: pytest.TempPathFactory,
) -> None:
"""Test that relative answers_file paths work when dst_path doesn't exist yet."""
src = tmp_path_factory.mktemp("src")
# Create a simple template
build_file_tree(
{
(src / "copier.yml"): (
"""\
project_name:
type: str
"""
),
(src / ".copier-answers.yml"): (
"""\
project_name: "test project"
"""
),
Comment on lines +353 to +357
Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

This setup is not representative of a real one.

Suggested change
(src / ".copier-answers.yml"): (
"""\
project_name: "test project"
"""
),
(src / "{{ _copier_conf.answers_file }}.jinja"): (
"{{ _copier_answers|to_nice_yaml }}"
),

Copy link
Copy Markdown
Author

@Aohzan Aohzan Oct 15, 2025

Choose a reason for hiding this comment

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

where is defined the _copier_answers?

Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a Jinja variable available in our render context.

https://copier.readthedocs.io/en/stable/creating/#_copier_answers

Copy link
Copy Markdown
Author

@Aohzan Aohzan Oct 15, 2025

Choose a reason for hiding this comment

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

yes but where do I define that project_name: "test project"

Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

You can either set it as the default answer in copier.yml

             (src / "copier.yml"): (
                 """\
                 project_name:
                   type: str
+                  default: test project
                 """
             ),

and run

copier.run_copy(..., defaults=True, ...)

(as you're doing already) or instead pass the answer via the data argument:

copier.run_copy(..., data={"project_name": "test project"}, ...)

Copy link
Copy Markdown
Author

@Aohzan Aohzan Oct 15, 2025

Choose a reason for hiding this comment

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

Ok but I don't test my case where I want to be dure that the existing answers file is loaded, no?

Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

Does your use case assume an existing answers file? If so, why? This seems like an unusual pattern. The answers file is generated by Copier to record previous answers plus metadata to enable updating to new template versions. It looks like you're assuming the answers file to exist before the first copier copy call. Are you trying to pass answers to copier copy from a file instead of via interactive prompting or inline answers via the -d,--data flag? If so, perhaps --data-file is what you're looking for.

Copy link
Copy Markdown
Author

@Aohzan Aohzan Oct 15, 2025

Choose a reason for hiding this comment

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

one of our use case is to have a template for the repository, then we can add terraform stack in subdirs, and we call the initial answer file to not repeat common questions

Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

I think --data-file might work for this use case as well although you'd be passing an answers file, which is a superset of a typical data file.

Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

Or _external_data might be worth checking out.

(src / "README.txt.jinja"): (
"Project: {{ project_name }}"
),
}
)
git_save(src, tag="v1")

# Destination path that doesn't exist yet
dst_dir = src / "my" / "dest" / "path"
Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

The result might be same, but this setup is a bit unintuitive because you're generating the project in a subdirectory of the template's sources. I'd follow the usual setup with

src, dst = map(tmp_path_factory.mktemp, ("src", "dst"))

and generate in dst = src / "my" / "dest" / "path" instead.

Copy link
Copy Markdown
Author

@Aohzan Aohzan Oct 15, 2025

Choose a reason for hiding this comment

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

you mean dst_dir = dst / "my" / "dest" / "path" ? or I miss something

Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

Sorry, yes, I mistyped.


# Answers file in parent directory (relative path of dst_dir)
answers_file = "../../../.copier-answers.yml"

# This should work even though dst_dir doesn't exist yet
copier.run_copy(
str(src),
dst_dir,
answers_file=answers_file,
defaults=True,
overwrite=True,
)

# Verify the project was created
assert (dst_dir / "README.txt").read_text() == "Project: test project"
Copy link
Copy Markdown
Member

@sisp sisp Oct 15, 2025

Choose a reason for hiding this comment

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

I think you should also assert that the .copier-answers.yml file exists in the expected location.