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

Don't check .gitignore files in parent directories #2158

Merged
merged 1 commit into from
Jul 28, 2024

Conversation

jameshilliard
Copy link
Contributor

@jameshilliard jameshilliard commented Jul 23, 2024

When building inside a .gitignore'd output directory as part of another build system maturin may accidentially pick up the parent .gitignore. Lets just disable searching for parent .gitignore files since this functionality is broken anyways as the logic does not match git's .gitignore logic anyways.

For example in buildroot we have an output directory that has its contents gitignored in which maturin will build packages, changing some minor implementation details in how we ignore this directory triggered this bug.

buildroot/buildroot@a14c862

As the output dir files are supposed to be ignored both before and after this change this indicates that ignore::WalkBuilder parent matching is fundamentally broken and can not be used reliably in the first place as it does not match git's ignore logic.

Also disable git exclude and git global excludes which may cause problems.

Copy link

netlify bot commented Jul 23, 2024

Deploy Preview for maturin-guide ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit e0db05c
🔍 Latest deploy log https://app.netlify.com/sites/maturin-guide/deploys/66a368bf2aea2c0007bad2d7
😎 Deploy Preview https://deploy-preview-2158--maturin-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@messense
Copy link
Member

Also disable git exclude and git global excludes which may cause problems.

I'm not sure about disabling git global excludes, I often put things in it (like .vscode/.idea etc.) to avoid writing in every repo myself.

@jameshilliard
Copy link
Contributor Author

I'm not sure about disabling git global excludes, I often put things in it (like .vscode/.idea etc.) to avoid writing in every repo myself.

Hmm, why would those be getting picked up in the build though? They shouldn't be part of the package paths right?

@jameshilliard
Copy link
Contributor Author

By the way any idea why tests are failing on windows? I don't really have a windows system set up for testing right now.

@messense
Copy link
Member

By the way any idea why tests are failing on windows? I don't really have a windows system set up for testing right now.

Probably because the disk is full.

@messense
Copy link
Member

By the way any idea why tests are failing on windows? I don't really have a windows system set up for testing right now.

Probably because the disk is full.

no, it's because this change breaks something because it's not ignoring a .pyd file.

Cause: File pyo3_mixed_py_subdir\_pyo3_mixed.cp312-win_amd64.pyd was already added from D:\a\maturin\maturin\test-crates\pyo3-mixed-py-subdir\python\pyo3_mixed_py_subdir\_pyo3_mixed.cp312-win_amd64.pyd, can't added it from D:\a\maturin\maturin\test-crates/targets/integration-pyo3-mixed-py-subdir\debug\maturin\pyo3_mixed_py_subdir.dll
Cause: Failed to write to pyo3_mixed_py_subdir\_pyo3_mixed.cp312-win_amd64.pyd
Cause: Failed to add the files to the wheel

@jameshilliard jameshilliard force-pushed the fix-ignore branch 2 times, most recently from cc575a2 to 396c492 Compare July 24, 2024 04:01
@jameshilliard
Copy link
Contributor Author

What's the correct way to get the directory containing pyproject.toml from write_python_part?

@jameshilliard jameshilliard force-pushed the fix-ignore branch 2 times, most recently from 29f30ea to f5d8dfd Compare July 26, 2024 05:10
@jameshilliard jameshilliard force-pushed the fix-ignore branch 4 times, most recently from 8b452cb to 570b5bd Compare July 26, 2024 06:52
@jameshilliard
Copy link
Contributor Author

no, it's because this change breaks something because it's not ignoring a .pyd file.

I think I have this fixed(along with some other random issues such as windows path length issues).

When building inside a .gitignore'd output directory as part of
another build system maturin may accidentially pick up the parent
.gitignore. Lets just disable searching for parent .gitignore
files since this functionality is broken anyways as the logic does
not match git's .gitignore logic anyways.

For example in buildroot we have an output directory that has its
contents gitignored in which maturin will build packages, changing
some minor implementation details in how we ignore this directory
triggered this bug.

buildroot/buildroot@a14c862

As the output dir files are supposed to be ignored both before and
after this change this indicates that ignore::WalkBuilder parent
matching is fundamentally broken and can not be used reliably
in the first place as it does not match git's ignore logic.

Also disable git exclude and git global excludes which may cause
problems.

Signed-off-by: James Hilliard <[email protected]>
@jameshilliard
Copy link
Contributor Author

@messense Does this look ok to merge? The only test failure I'm seeing appears to be spurious.

@messense messense enabled auto-merge July 28, 2024 06:26
@messense messense added this pull request to the merge queue Jul 28, 2024
@messense messense removed this pull request from the merge queue due to a manual request Jul 28, 2024
@messense messense merged commit 64598c4 into PyO3:main Jul 28, 2024
28 checks passed
@jameshilliard jameshilliard deleted the fix-ignore branch July 28, 2024 07:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants