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

Improve cruft update's rejection fallback #49

Open
samj1912 opened this issue Aug 16, 2020 · 9 comments
Open

Improve cruft update's rejection fallback #49

samj1912 opened this issue Aug 16, 2020 · 9 comments
Assignees
Labels
enhancement New feature or request

Comments

@samj1912
Copy link
Member

Currently, if a user modifies a cookiecutter template's file in a manner that it is no longer similar to the original version, cruft update fails to apply the patch using git 3 way merge and instead falls back to git apply --reject, which produces a bunch of*.rej files. This is not the best way for an end user to manually fix updates. We should look into how we can improve this experience. For more details about the fallback, see #47 and #48

@nickderobertis
Copy link

I agree there is room for improvement, I just recently upgraded cruft and have been trying to figure out a good workflow with .rej files but so far to no avail.

For my usage, I am always using a git repo, and I like how cruft < 1.4 created merge conflict markers in the files because there are lots of GUIs for handling these, and IMO more straightforward when the original and changes are in the same file. I would just delete all the .orig files (they are not particularly useful when everything is tracked in git) and resolve the conflicts.

According to #35 cruft went to using git apply over patch to handle renames and deletes which is certainly an improvement. But I feel that it took a step backwards in terms of user experience in updates for git repos.

I did a quick experiment, where cruft falls back to the patch behavior patch -p1 --merge only for the rejections (as git apply --reject is being used right now). This seems to reproduce the original behavior but I have not tested whether it still properly handles renames and deletes. It seems logical it would still be working for git repos since it tries git apply -3 first. Perhaps only for git repos we could enable fallback to patch and still use git apply --reject for non-git, but that does have the downside of different UX for git vs. non-git. I noticed also adding the --no-backup-if-mismatch flag will prevent it from creating the .orig files, which would be the ideal workflow for me.

One strategy I'm thinking which could may resolve this and give the same UX across git and non git would be:

Project Type First Pass Second Pass
Git git apply -3 patch -p1 --merge --no-backup-if-mismatch
Non-git git apply --reject patch -p1 --merge

I'm curious your thoughts on this approach. I'm also curious whether you would consider having command line options in the library to customize this behavior, e.g. adding a flag for --no-backup-if-mismatch, or perhaps even giving the user control to override specific commands which are being used.

I'm happy to experiment more with this and submit a PR depending on what you would consider should be included in cruft.

@tekumara
Copy link

I like the workflow you've suggested @nickderobertis and would use it over the current workflow.

@samj1912
Copy link
Member Author

The above looks good to me. The only thing to consider is the behavior of patch in Linux vs macOS. IIRC macOS doesn't have the gnu version by default and it would be nice if we could vendor it.

@mikeoconnor0308
Copy link

Any update on this? The user experience of conflicts is the main pain point in my use of cruft.

@woile
Copy link

woile commented Aug 9, 2021

I'm having the same issue. In my case, it has to replace a single line a Makefile and this error appears.
Anyone knows how could I prevent this and how to fix it?

@jonzarecki
Copy link
Contributor

@samj1912 what does a non-git project type mean?
Are there any tests present for 3-way merge creating a merge conflict instead of a .rej file?

@sharadpattanshetti
Copy link

sharadpattanshetti commented Nov 12, 2021

Hi,
I am having this below error scenario during cruft update and wanted to know if it is related to this thread.
Scenario:
When I introduce a new injected cookiecutter variables in my template then the update fails saying certain attribute is not found. Below is the error:
Traceback (most recent call last): File "c:\users\user\appdata\local\programs\python\python38\lib\site-packages\cookiecutter\generate.py", line 352, in generate_files generate_file( ......... raise rewrite_traceback_stack(source=source) File ".\template.yaml", line 20, in top-level template code variable1: '2588237' jinja2.exceptions.UndefinedError: 'collections.OrderedDict object' has no attribute 'variable1'

@sharadpattanshetti
Copy link

Hi,
Any update on this? For newly added cookie cutter fields, the cruft update fails.

@ibackus-convoy
Copy link

ibackus-convoy commented May 9, 2022

We could definitely use improvements to this for our projects. We have lots of repos built off a template maintained by people who are not git experts and this can cause lots of confusion.

Edit: after using cruft a little longer, this would be a game changer!

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

No branches or pull requests

8 participants