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

FIX: Regression of not excluding files from build #1606

Merged

Conversation

finswimmer
Copy link
Member

This PR changes the way, how it is checked if a file should be excluded from build.

Fixes: #1597

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.

@sdispater
Copy link
Member

@finswimmer Shouldn't this be based on master?

@finswimmer
Copy link
Member Author

Maybe? :) Because my PR #1464 was based on develop I did here the same way.

Any easy way to switch?

@sdispater
Copy link
Member

@finswimmer Oh sorry, I thought it was a bug fix but it was actually a regression fix for your previous PR so develop is good :-) 

@sdispater sdispater changed the base branch from develop to master November 21, 2019 21:21
@sdispater
Copy link
Member

@finswimmer I changed the base branch to master since your previous fix is already on master. You will need to rebase your branch onto master.

@finswimmer
Copy link
Member Author

@sdispater: That was a hard work doing my first rebase. But now I got it (I think) 🚀

@sdispater
Copy link
Member

@finswimmer I don't think your rebase went well since this PR includes commits that are not relevant to your changes. You only need to rebase your 4 commits onto master. You might want to use git rebase --onto in order to change the base branch.

@finswimmer finswimmer force-pushed the issue-01597-regression-exlude-file branch from b62d772 to b3ca03d Compare November 22, 2019 12:01
finswimmer77@gmail.com and others added 4 commits November 22, 2019 13:05
…mplete list of excluded files. Instead when checking if a file is excluded, it is looked up if one of the parent folder is in the excluded list.
@finswimmer finswimmer force-pushed the issue-01597-regression-exlude-file branch from b3ca03d to c73e19f Compare November 22, 2019 12:09
@finswimmer
Copy link
Member Author

finswimmer commented Nov 22, 2019

TIL: Never ever (never!) use the wrong branch!

Better now @sdispater ?

filepath = filepath.as_posix()
exclude_path = Path(filepath)

while True:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not while len(exclude_path.parts) > 1?

I try to avoid while True statements to have a more explicit condition.

Copy link
Member Author

@finswimmer finswimmer Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because - at least in theory - the root folder could be excluded as well. (Which doesn't make sense, if you want to build a package, but people have sometimes strange ideas).

Of course I could check this if I leave the loop. If you would like this more, I could implement it.

For things like this a native do-while implementation in python would be nice. But AFAIK there isn't such a thing 😞 .

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You know what? We can leave it like this for now. I don't think this is too much of an issue.

@sdispater sdispater merged commit 12f981c into python-poetry:master Nov 22, 2019
@mhozza
Copy link

mhozza commented Nov 22, 2019

Can confirm the fix works. Thanks!

@finswimmer finswimmer deleted the issue-01597-regression-exlude-file branch December 13, 2019 18:03
shenek pushed a commit to shenek/poetry that referenced this pull request Dec 31, 2019
* FIX: To find excluded files, no additional glob is used to build a complete list of excluded files. Instead when checking if a file is excluded, it is looked up if one of the parent folder is in the excluded list.

* making isort happy

* revert accidentally made changes to pyproject.toml/poetry.lock

* fix: change condition for breaking loop, as previous version could lead to infinite loop
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.

Exclude is not respected when building package.
3 participants