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

Avoid ignore crate finding cache gitignore #2615

Merged
merged 1 commit into from
Mar 22, 2024
Merged

Conversation

konstin
Copy link
Member

@konstin konstin commented Mar 22, 2024

We put a .gitignore with * at the top of our cache. When maturin was building a source distribution inside the cache, it would walk up the tree to find a gitignore, see that and ignore all python files. We now add an (empty) .git directory one directory below, in the root of built-wheels cache. This prevents ignore walking further up (it marks the top level a git repository).

Deptry (from #2490) is a mid sized rust package with additional python packages, so instead of using it in the test i've replaced it with a small (44KB total) reproducer that uses cffi for faster building, the entire test taking <2s on my machine.

Fixes #2490

We put a `.gitignore` with `*` at the top of our cache. When maturin was building a source distribution inside the cache, it would walk up the tree to find a gitignore, see that and ignore all python files. We now add an (empty) `.git` directory one directory below, in the root of built-wheels, which prevents ignore walking further up (it marks the top level a git repository).

Deptry (from #2490) is a mid sized rust package with additional python packages, so instead of using it in the test i've replaced it with a small (44KB total) reproducer that uses cffi for faster building, the entire test taking <2s on my machine.

Fixes #2490
@zanieb
Copy link
Member

zanieb commented Mar 22, 2024

@zanieb
Copy link
Member

zanieb commented Mar 22, 2024

Why doesn't #2466 address this?

@konstin
Copy link
Member Author

konstin commented Mar 22, 2024

As i understand it, ignore walks up until it finds a git repository root, since gitignores extend each other (cc @BurntSushi)

@BurntSushi
Copy link
Member

As i understand it, ignore walks up until it finds a git repository root, since gitignores extend each other (cc @BurntSushi)

Yeah, although you can opt out of .git detection with WalkBuilder::require_git.

You can also explicitly opt out of the "ascend the directory tree" behavior with WalkBuilder::parents. (@konstin I forgot to ask whether using this would be appropriate in this context.)

@@ -0,0 +1,11 @@
[package]
name = "deptry_reproducer"
Copy link
Member

Choose a reason for hiding this comment

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

Can we put this in editable-installs, just because that's where all our other local packages are?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mind if i rename editable installs to packages first in a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that’s fine.

@zanieb
Copy link
Member

zanieb commented Mar 22, 2024

@charliermarsh should I release this?

@charliermarsh
Copy link
Member

@zanieb - Yeah, let's include it. @konstin can rename the directory as a follow-up instead of a precursor PR.

@zanieb zanieb merged commit fca2883 into main Mar 22, 2024
31 checks passed
@zanieb zanieb deleted the konsti/gitignore-on-the-root branch March 22, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Doesn't add package files when installing from sdist on PyPy
4 participants