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

repos with symlinks not working anymore since #12 #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sbidoul
Copy link

@sbidoul sbidoul commented Feb 22, 2017

Here is a test illustrating a use case that was working with 1.1 and is broken with 1.2.

Reverting 4885c65 makes the test pass.

Now looking for a proper fix.

@sbidoul
Copy link
Author

sbidoul commented Feb 22, 2017

Oh, I just read in #6 that not supporting symlinks was intended.

This is unfortunate as we are using symlinks heavily in the Odoo ecosystem with setuptools-odoo.

@msabramo @stefanholek @sciyoshi Would there be a simple solution to support the simple test case I provide here? I confess I don't yet fully understand #12 and why it breaks this use case.

@sciyoshi
Copy link

@sbidoul #12 was to avoid walking into directories that aren't managed by git. It works by checking the output of git ls-files and only recursing into those directories. Sounds like a possible fix would be to have that be aware of symlinks, but then you need to check and short-circuit any circular symlinks (links that point to parent folders) and probably any external symlinks.

@electroniceagle
Copy link

Just encountered this issue also. It also drops directories that are managed by git but are above setup.py. Here is our use case for Django. We generate multiple packages from the same git repository. This would give us a site_common tree within each of our packages prior to 4885c65.

     common/
          settings/
          templates/
          ...
     site-a/
          setup.py
         manage.py
         site_common -> ../common/
         ...
     site-b/
          setup.py
          manage.py
          site_common -> ../common/

@sbidoul
Copy link
Author

sbidoul commented Apr 8, 2017

In 6498f29 I provide a solution which should not negatively impact performance and properly ignore out-of-git symlinks

@sbidoul
Copy link
Author

sbidoul commented Apr 8, 2017

@electroniceagle would you like to test this for your use case?

@sbidoul
Copy link
Author

sbidoul commented Apr 8, 2017

I just noticed that setuptools_scm has the same feature and is recommanded by the setuptools documentation, so I'll try that one.

@sbidoul
Copy link
Author

sbidoul commented Apr 9, 2017

It turns out setuptools_scm is very good at tag-based versioning, but less advanced than setuptools-git at file finding.

@sciyoshi @msabramo what do you think of this patch?

@sbidoul
Copy link
Author

sbidoul commented May 13, 2018

This is now implemented in setuptools_scm 2.1

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.

3 participants