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

implement support for local extensions #237

Merged
merged 13 commits into from Apr 4, 2023
Merged

implement support for local extensions #237

merged 13 commits into from Apr 4, 2023

Conversation

Chilipp
Copy link
Contributor

@Chilipp Chilipp commented Feb 24, 2023

Thanks for this awesome package!

This PR aims to close #172 and implement support for local extensions. It mimics the original implementation in cookiecutter (cookiecutter/cookiecutter#1240) that unfortunately seems not to be part of the python API

I also implemented two tests (d1355db), one for the creation and one for the update. But currently I am using a fork of the cookiecutter-test repo. You need to copy the local-extension and local-extension-update from my fork to the original repository (or create the two branches in the original repository and I make a PR).

one question remains though: Do we really want to use

from cookiecutter.main import _patch_import_path_for_repo

or should we rather copy that small code snippet from cookiecutter (see https://github.com/cookiecutter/cookiecutter/blob/cf81d63bf3d82e1739db73bcbed6f1012890e33e/cookiecutter/main.py#L139)

ToDos:

  • create test branches in cookiecutter-test repo
  • clarify strategy for _patch_import_path_for_repo

@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (f09db34) 99.67% compared to head (652a5f8) 99.68%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #237   +/-   ##
=======================================
  Coverage   99.67%   99.68%           
=======================================
  Files          21       21           
  Lines         937      964   +27     
=======================================
+ Hits          934      961   +27     
  Misses          3        3           
Impacted Files Coverage Δ
cruft/_commands/check.py 100.00% <100.00%> (ø)
cruft/_commands/create.py 100.00% <100.00%> (ø)
cruft/_commands/diff.py 100.00% <100.00%> (ø)
cruft/_commands/link.py 100.00% <100.00%> (ø)
cruft/_commands/update.py 98.30% <100.00%> (+0.05%) ⬆️
cruft/_commands/utils/generate.py 98.64% <100.00%> (ø)
cruft/_commands/utils/iohelper.py 100.00% <100.00%> (ø)
tests/test_cli.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@Chilipp Chilipp marked this pull request as ready for review February 24, 2023 11:03
@Chilipp
Copy link
Contributor Author

Chilipp commented Mar 23, 2023

@timothycrosley is there any chance to get this merged or anything else that I can do to speed this up?

@MichalKubek
Copy link

@Chilipp I thing will be better to copy that functionality to this repository, because it is private function.
also we can fix this problem with this implementation MichalKubek@5e3ca75

@Chilipp
Copy link
Contributor Author

Chilipp commented Mar 24, 2023

True! Sounds good, thanks @MichalKubek do you want to implement your approach in this PR or shall I do it (will take about an hour until I can do it)

@MichalKubek
Copy link

@Chilipp please you do it. It is your PR.

Thanks

@Chilipp
Copy link
Contributor Author

Chilipp commented Mar 24, 2023

alright @MichalKubek I reverted the original commit (1fd1548) and implemented your solution in 7c63ae4

@Chilipp
Copy link
Contributor Author

Chilipp commented Mar 24, 2023

seems not to work. I'll have a look

@Chilipp
Copy link
Contributor Author

Chilipp commented Mar 28, 2023

hey @MichalKubek! sorry, was busy with a lot of other stuff. But I fixed it now. Although your solution is nicer, I had to add support for a directory parameter in the AltTemporaryDirectory. And as this class is used in many places, I had to touch quite some files (see https://github.com/cruft/cruft/compare/a18aa92..7c63ae4)

@Chilipp
Copy link
Contributor Author

Chilipp commented Mar 30, 2023

Hmm, when I compare the implementation with AltTemporaryDirectory

https://github.com/cruft/cruft/compare/808842d..0c681ef

to the original implementation I suggested,

https://github.com/cruft/cruft/compare/35a2fbf..0c681ef

then my original implementation involved much less code modification and looks less hacky to me. what do you think @MichalKubek ?

@samj1912
Copy link
Member

samj1912 commented Mar 30, 2023

@Chilipp could you please create a PR to https://github.com/cruft/cookiecutter-test with your changes? I have created an extensions branch which you should be able to PR to - https://github.com/cruft/cookiecutter-test/tree/extensions

@Chilipp
Copy link
Contributor Author

Chilipp commented Mar 30, 2023

alright @samj1912 , but I need two branches. one for the creation (https://github.com/Chilipp/cookiecutter-test/tree/local-extension) and one for the update (https://github.com/Chilipp/cookiecutter-test/tree/local-extension-update)

@samj1912
Copy link
Member

@Chilipp
Copy link
Contributor Author

Chilipp commented Apr 1, 2023

alright, I updated the tests. so if you are ok with the AltTemporaryDirectory implementation (that I do not like so much, see #237 (comment)), feel free to merge this PR 😃

@samj1912
Copy link
Member

samj1912 commented Apr 4, 2023

Thanks for this PR!

@samj1912 samj1912 merged commit f0126c5 into cruft:master Apr 4, 2023
17 checks passed
@Chilipp
Copy link
Contributor Author

Chilipp commented Apr 4, 2023

You're welcome! Thanks for working on cruft 😃

@tmeckel
Copy link

tmeckel commented Apr 6, 2023

@Chilipp still does not work. @samj1912

Traceback (most recent call last):

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cookiecutter/environment.py", line 35, in __init__
    super().__init__(extensions=extensions, **kwargs)

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/jinja2/environment.py", line 363, in __init__
    self.extensions = load_extensions(self, extensions)

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/jinja2/environment.py", line 117, in load_extensions
    extension = t.cast(t.Type["Extension"], import_string(extension))

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/jinja2/utils.py", line 149, in import_string
    return getattr(__import__(module, None, None, [obj]), obj)

ModuleNotFoundError: No module named 'local_extensions'


During handling of the above exception, another exception occurred:


Traceback (most recent call last):

  File "/tmp/venv-cookiecutter/bin/cruft", line 8, in <module>
    sys.exit(app())

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cruft/_cli.py", line 282, in diff
    if not _commands.diff(project_dir=project_dir, exit_code=exit_code, checkout=checkout):

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cruft/_commands/diff.py", line 38, in diff
    utils.generate.cookiecutter_template(

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cruft/_commands/utils/generate.py", line 44, in cookiecutter_template
    context = _generate_output(cruft_state, Path(repo.working_dir), cookiecutter_input, output_dir)

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cruft/_commands/utils/generate.py", line 75, in _generate_output
    new_context = generate_cookiecutter_context(

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cruft/_commands/utils/cookiecutter.py", line 98, in generate_cookiecutter_context
    context["cookiecutter"] = prompt_for_config(context, no_input)

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cookiecutter/prompt.py", line 185, in prompt_for_config
    env = StrictEnvironment(context=context)

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cookiecutter/environment.py", line 65, in __init__
    super().__init__(undefined=StrictUndefined, **kwargs)

  File "/tmp/venv-cookiecutter/lib/python3.10/site-packages/cookiecutter/environment.py", line 37, in __init__
    raise UnknownExtension(f'Unable to load extension: {err}')

cookiecutter.exceptions.UnknownExtension: Unable to load extension: No module named 'local_extensions'

image

.cruft.json:

{
  "template": "/home/vagrant/work/pulumi-tf-provider-cookiecutter",
  "commit": "6ca0d9b24dd7fbff7ec9380c3759a5311ac766e9",
  "checkout": null,
  "context": {
    "cookiecutter": {
      [...]
      "_copy_without_render": [
        ".github/workflows/release.yml"
      ],
      "_extensions": [
        "local_extensions.FilterExtension"
      ],
      "_template": "/home/vagrant/work/pulumi-tf-provider-cookiecutter"
    }
  },
  "directory": null
}

@Chilipp
Copy link
Contributor Author

Chilipp commented Apr 6, 2023

Weird! @tmeckel Can you please try

cruft create https://github.com/cruft/cookiecutter-test --checkout extensions --directory dir

Does that work? How is your setup different from that branch?

@tmeckel
Copy link

tmeckel commented Apr 7, 2023

@Chilipp my template (https://github.com/tmeckel/pulumi-tf-provider-cookiecutter) uses a local extension module local_extensions.py in the top-level directory.

For testing I changed the cruft module locally and added the following code:

_commands/utils/generate.py:90:        sys.path.append(tmpdir)
_commands/diff.py:36:            sys.path.append(cruft_state["template"])
_commands/create.py:46:        sys.path.append(cookiecutter_template_dir)

After that cruft was able to create/update and diff.

As far as I understood the code

_commands/utils/iohelper.py:16:            sys.path.append(name)

should've done the trick. But I didn't.

@Chilipp
Copy link
Contributor Author

Chilipp commented Apr 8, 2023

hey @tmeckel! just tested it with

git clone https://github.com/tmeckel/pulumi-tf-provider-cookiecutter
git -C pulumi-tf-provider-cookiecutter rm -r hooks
git -C pulumi-tf-provider-cookiecutter commit -m "remove hooks"
cruft create pulumi-tf-provider-cookiecutter --no-input

and everything went well. Does the command I wrote in my previous comment #237 (comment) work for you? If not, and sorry for the stupid question, are you sure you are using the correct version? What do you get when you run

python -c "import cruft; print(cruft.__version__)"

I am getting 2.14.0

Alternatively I'd say that you should open a new issue (but I am not a maintainer of cruft and so I am not moderating this PR 😉 )

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

Successfully merging this pull request may close these issues.

Local extensions don't work with (Cookiecutter 2.1.1)
4 participants