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

Add support for symlinks in cookiecutter templates #868

Closed
wants to merge 0 commits into from

Conversation

pjbull
Copy link
Contributor

@pjbull pjbull commented Dec 7, 2016

  • Support copying symlinks as symlinks for both rendered and
    non-rendered directories
  • Support symlinks that link to a rendered directory
  • Support symlinks that get rendered

@pjbull
Copy link
Contributor Author

pjbull commented Dec 7, 2016

This PR aims to tackle #865

@pjbull
Copy link
Contributor Author

pjbull commented Dec 8, 2016

Sorry for the noise here--don't have a Windows+Python 2.7 environment, so was letting AppVeyor test things for me.

This fix generally works, but there is a complication: Windows+Python2.7 doesn't support symlinks out of the box. To make this work, I've added the win_utils.py file which pulls together a bunch of kernel32 functions to enable symlinks. It looks to me like there's one package on PyPI that would enable this similar functions jaraco.windows if the preference is to take an external dependency on this pacakge in the Windows+Python2.7 case.

I have a little work left to re-implement rmtree in a way that supports symlinks in Windows in the tests. Also, letting flake8 fail on the win_utils.py until we settle whether to use those versions of take the jaraco.windows dependency.

@pjbull
Copy link
Contributor Author

pjbull commented Dec 10, 2016

Finally got a Windows VM to develop against so could get symlinks working there. Separated out the posix and the Windows changes into separate commits so we can reason about them separately.

For background, most concise summary of symlinks in Windows and Python <3.2 is here:

@michaeljoseph michaeljoseph added the needs-review PR Only: This PR require review from other developer label Dec 31, 2016
@bittner
Copy link
Contributor

bittner commented Apr 15, 2017

Can we merge this PR? - It fixes a problem that blocks my work, currently.

I understand it's a bit large, mainly due to adding symlink support for Windows. If this is a problem can we get the PR merged earlier if we move the Windows-related changes to a separate PR?

@hackebrot
Copy link
Member

@pjbull @bittner I'm not really confident merging and maintaining this. It adds a lot of complexity and it's hard for me to test as I don't usually use windows.

I don't object merging this, but we need someone with windows expertise to review this.

@bittner
Copy link
Contributor

bittner commented Apr 15, 2017

Frankly, symlinks on Windows feels rather artificial to me. I wouldn't know how to create them using the file manager or cmd. Do we really have an issue with symbolic links on Windows? If we have cookiecutters (templates) that have symbolic links (e.g. created on Unix OSes) under version control, do they have a meaning under Windows? -- Template authors should probably take that into consideration, not the generator software.

@hackebrot If the complexity that Windows introduces were removed in this PR would you be willing to merge this PR right away?

@bittner
Copy link
Contributor

bittner commented Apr 22, 2017

@hackebrot Should the complexity of Windows be removed from this PR to make the decision of merging easier?

@pjbull pjbull mentioned this pull request Apr 23, 2017
@pjbull
Copy link
Contributor Author

pjbull commented Apr 23, 2017

Added just the PR for posix systems and newer Python, which is cleaner: #934

I'll create an issue for Windows with Python < 3.2, which we may end up won't fixing given the complexity it introduces.

@insspb insspb added unresolved conflict PR Only: This PR has merge conflict, that should be resolved by author and removed needs-review PR Only: This PR require review from other developer labels Jul 1, 2019
@insspb
Copy link
Member

insspb commented Jul 1, 2019

@pjbull, unfortunately, it was not reviewed in time. Currently, we are making cookiecooter issues and prs cleanup after some time without project maintenance. If you can adapt this code to currentl project code state it will be reviewed much faster again.

@ssbarnea ssbarnea closed this Jan 18, 2020
@cookiecutter cookiecutter deleted a comment from codecov-io May 30, 2020
@insspb insspb reopened this May 30, 2020
@insspb
Copy link
Member

insspb commented May 30, 2020

@ssbarnea Please stop closing pull requests without any resolution. At least for problematic functions like symlinks.

It is hard to find them again. And this problem not yet solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unresolved conflict PR Only: This PR has merge conflict, that should be resolved by author
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Question: Symbolic links
6 participants