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

match_tree doesn't return the symlinks regardless of the followSymlinks parameter #31

Closed
ftrofin opened this issue Jan 17, 2020 · 10 comments

Comments

@ftrofin
Copy link

ftrofin commented Jan 17, 2020

This may be considered a bug or "works as designed", depending on the interpretation of "followSymlinks", but wanted to point out a (design?) flaw in match_tree:

Suppose I want to archive a folder so I specify a very inclusive pattern: "Libs/"

Under 'Libs/' I have a Mac framework named 'Crmsdk'. Mac frameworks are really just directories with a particular structure inside but the interesting aspect for us is that it has symlinks. This is how Crmsdk.framework looks inside (first level):

lrwxr-xr-x  1 flo  staff    23 Oct  1 17:13 CrmSdk -> Versions/Current/CrmSdk
lrwxr-xr-x  1 flo  staff    26 Oct  1 17:13 Resources -> Versions/Current/Resources
drwxr-xr-x  5 flo  staff   160 Jan 15 00:23 Versions

Notice that Resources is a symlink pointing inside the Versions folder.

Now, when I call the path_spec.match_tree() function, regardless of the value of followSymlinks parameter, I can never get the Resources folder as an entry in the result set. Which, someone might argue, is what I should expect because:

  • if followSymlinks is True I will get entries like:
Libs/CrmSdk.framework/Resources/CrmRsc2
...
Libs/CrmSdk.framework/Versions/Current/Resources/CrmRsc1
Libs/CrmSdk.framework/Versions/Current/Resources/CrmRsc2

Which is normal because the code is following the symlinks and Resources is just a symlink to Versions/Current/Resources.

  • if followSymlinks is False, then Resources doesn't even show up in the results list as a folder after CrmSdk.framework, in other words I get only these entries:
Libs/CrmSdk.framework/Versions/Current/Resources/CrmRsc2

I don't get any Libs/CrmSdk.framework/Resources entries.

This puts me in the impossibility to create an archive of the matched entries that I can unzip on another location and have it recreated the same (because the symlinks are missing).

One may claim that this is a bug and when followSymlinks is False, the Resources folder should be returned in the list of results (clients shouldn't make an assumption about the entries returned: they may be files, folders or symlinks - it's their job to properly handle it). Bottom line is that symlinks are missing in the results list regardless of the flag's value.

@ftrofin
Copy link
Author

ftrofin commented Jan 17, 2020

I have a PR for this but I don't have access to this repo. Can you please give me access so I can create the PR?

@ftrofin
Copy link
Author

ftrofin commented Jan 17, 2020

@cpburnz Caleb?

@cpburnz
Copy link
Owner

cpburnz commented Jan 18, 2020

@ftrofin You don't need direct access to create a pull request. Just fork the project, commit your changes there, then submit a pull request.

I think you're right that it's a bug. In my experience most programs treat symlinks as regular files if they're not resolving the target. Git, in particular, I think treats symlinks as files regardless if they point to a file or folder.

@ftrofin
Copy link
Author

ftrofin commented Jan 22, 2020

I tried that. I cloned (using https), created a branch named 'issue_31' and tried to push it back:
$ git push --set-upstream origin issue_31
remote: Permission to cpburnz/python-path-specification.git denied to ftrofin.

@cpburnz
Copy link
Owner

cpburnz commented Jan 23, 2020

@ftrofin Are you familiar with how to fork and make pull requests on Github? On Github you have to use the web interface to perform those actions, not directly using git.

@ftrofin
Copy link
Author

ftrofin commented Jan 23, 2020

I didn't have to do this to repos I don't own. Here is the change, very simple, can you please apply it? Thanks!

image

cpburnz added a commit that referenced this issue Feb 3, 2020
cpburnz added a commit that referenced this issue Feb 3, 2020
@cpburnz
Copy link
Owner

cpburnz commented Feb 3, 2020

@ftrofin I've implemented this on the master branch.

@ftrofin
Copy link
Author

ftrofin commented Mar 26, 2020

Thank you Caleb. Any idea when you're going to make a new release with the latest changes/fixes?

@cpburnz
Copy link
Owner

cpburnz commented Mar 30, 2020

@ftrofin I'll be able to get to it this coming weekend.

@cpburnz
Copy link
Owner

cpburnz commented Apr 9, 2020

v0.8.0 has been released which supports symlinks.

@cpburnz cpburnz closed this as completed Apr 9, 2020
bors bot added a commit to rehandalal/therapist that referenced this issue Apr 20, 2020
118: Update pathspec to 0.8.0 r=rehandalal a=pyup-bot


This PR updates [pathspec](https://pypi.org/project/pathspec) from **0.7.0** to **0.8.0**.



<details>
  <summary>Changelog</summary>
  
  
   ### 0.8.0
   ```
   ------------------

- `Issue 30`_: Expose what patterns matched paths. Added `util.detailed_match_files()`.
- `Issue 31`_: `match_tree()` doesn&#39;t return symlinks.
- `Issue 34`_: Support `pathlib.Path`\ s.
- Add `PathSpec.match_tree_entries` and `util.iter_tree_entries()` to support directories and symlinks.
- API change: `match_tree()` has been renamed to `match_tree_files()`. The old name `match_tree()` is still available as an alias.
- API change: `match_tree_files()` now returns symlinks. This is a bug fix but it will change the returned results.

.. _`Issue 30`: cpburnz/python-pathspec#30
.. _`Issue 31`: cpburnz/python-pathspec#31
.. _`Issue 34`: cpburnz/python-pathspec#34
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/pathspec
  - Changelog: https://pyup.io/changelogs/pathspec/
  - Repo: https://github.com/cpburnz/python-path-specification
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>
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

No branches or pull requests

2 participants