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

Fasten the excluded file check #604

Merged

Conversation

Lothiraldan
Copy link
Contributor

Before we build a potentially very big list of all the excluded file matching
every exclude pattern, which could be a lot and then do a containment check on
the resulting list.

A first step to speed up the process is to returns a set instead of a list so
containment checks will not be dependent to the number of matching files
anymore.

If the set still ends up being too large, introducing an intermediary solution
that check if a file name match a glob pattern instead of expanding the glob
pattern might be a good solution. Right now, it seems overkill in comparison
of this patch.

With this patch, building my project that exclude nodes_modules/**/* went
from 4 minutes to 7 seconds.

Fix #603

Pull Request Check List

This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. But please read our contribution guide at least once, it will save you unnecessary review cycles!

  • Added tests for changed code.
  • Updated documentation for changed code.

Note: If your Pull Request introduces a new feature or changes the current behavior, it should be based
on the develop branch. If it's a bug fix or only a documentation update, it should be based on the master branch.

If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!

Before we build a potentially very big list of all the excluded file matching
every exclude pattern, which could be a lot and then do a containment check on
the resulting list.

A first step to speed up the process is to returns a set instead of a list so
containment checks will not be dependent to the number of matching files
anymore.

If the set still ends up being too large, introducing an intermediary solution
that check if a file name match a glob pattern instead of expanding the glob
pattern might be a good solution. Right now, it seems overkill in comparison
of this patch.

With this patch, building my project that exclude `nodes_modules/**/*` went
from 4 minutes to 7 seconds.

Fix python-poetry#603
@sdispater sdispater merged commit 4eb7b87 into python-poetry:master Nov 8, 2018
@sdispater
Copy link
Member

Thanks!

For now I agree this is enough as a quick fix for the issue.

@Lothiraldan
Copy link
Contributor Author

Thank you for merging this PR, the build is likely failing because of Teemu/pytest-sugar#159, I wanted to send a PR for forcing an older of pytest, but didn't had time at the moment

@jaraco
Copy link

jaraco commented Nov 8, 2018

Easier than pinning an older pytest is to add the workaround pytest-sugar-bugfix159 to your deps.

Copy link

github-actions bot commented Mar 1, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Building sdist is slow with wide exclude pattern
3 participants