From fca288386454f72440c02f79bc39ec9eb4558625 Mon Sep 17 00:00:00 2001 From: konsti Date: Fri, 22 Mar 2024 20:36:07 +0100 Subject: [PATCH] Avoid ignore crate finding cache gitignore (#2615) 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 --- crates/uv-cache/src/lib.rs | 21 ++++--- crates/uv/tests/pip_install.rs | 56 ++++++++++++++++++ scripts/packages/deptry_reproducer/.gitignore | 1 + scripts/packages/deptry_reproducer/Cargo.lock | 7 +++ scripts/packages/deptry_reproducer/Cargo.toml | 11 ++++ .../deptry_reproducer-0.1.0.tar.gz | Bin 0 -> 917 bytes .../packages/deptry_reproducer/pyproject.toml | 21 +++++++ .../python/deptry_reproducer/__init__.py | 6 ++ .../python/deptry_reproducer/foo.py | 0 scripts/packages/deptry_reproducer/src/lib.rs | 1 + 10 files changed, 115 insertions(+), 9 deletions(-) create mode 100644 scripts/packages/deptry_reproducer/.gitignore create mode 100644 scripts/packages/deptry_reproducer/Cargo.lock create mode 100644 scripts/packages/deptry_reproducer/Cargo.toml create mode 100644 scripts/packages/deptry_reproducer/deptry_reproducer-0.1.0.tar.gz create mode 100644 scripts/packages/deptry_reproducer/pyproject.toml create mode 100644 scripts/packages/deptry_reproducer/python/deptry_reproducer/__init__.py create mode 100644 scripts/packages/deptry_reproducer/python/deptry_reproducer/foo.py create mode 100644 scripts/packages/deptry_reproducer/src/lib.rs diff --git a/crates/uv-cache/src/lib.rs b/crates/uv-cache/src/lib.rs index 10c4f532f..9ab8ec315 100644 --- a/crates/uv-cache/src/lib.rs +++ b/crates/uv-cache/src/lib.rs @@ -251,15 +251,6 @@ impl Cache { Err(err) => return Err(err), } - // Add a phony .git, if it doesn't exist, to ensure that the cache isn't considered to be - // part of a Git repository. (Some packages will include Git metadata (like a hash) in the - // built version if they're in a Git repository, but the cache should be viewed as an - // isolated store.) - fs::OpenOptions::new() - .create(true) - .write(true) - .open(root.join(".git"))?; - // Add an empty .gitignore to the build bucket, to ensure that the cache's own .gitignore // doesn't interfere with source distribution builds. Build backends (like hatchling) will // traverse upwards to look for .gitignore files. @@ -273,6 +264,18 @@ impl Cache { Err(err) => return Err(err), } + // Add a phony .git, if it doesn't exist, to ensure that the cache isn't considered to be + // part of a Git repository. (Some packages will include Git metadata (like a hash) in the + // built version if they're in a Git repository, but the cache should be viewed as an + // isolated store.). + // We have to put this below the gitignore. Otherwise, if the build backend uses the rust + // ignore crate it will walk up to the top level .gitignore and ignore its python source + // files. + fs::OpenOptions::new() + .create(true) + .write(true) + .open(root.join(CacheBucket::BuiltWheels.to_str()).join(".git"))?; + fs::canonicalize(root) } diff --git a/crates/uv/tests/pip_install.rs b/crates/uv/tests/pip_install.rs index a3ece68b2..6dfbd764d 100644 --- a/crates/uv/tests/pip_install.rs +++ b/crates/uv/tests/pip_install.rs @@ -6,10 +6,12 @@ use assert_fs::prelude::*; use base64::{prelude::BASE64_STANDARD as base64, Engine}; use indoc::indoc; use itertools::Itertools; +use std::env::current_dir; use std::process::Command; use url::Url; use common::{uv_snapshot, TestContext, EXCLUDE_NEWER, INSTA_FILTERS}; +use uv_fs::Simplified; use crate::common::get_bin; @@ -2952,3 +2954,57 @@ fn install_site_packages_mtime_updated() -> Result<()> { Ok(()) } + +/// We had a bug where maturin would walk up to the top level gitignore of the cache with a `*` +/// entry (because we want to ignore the entire cache from outside), ignoring all python source +/// files. +#[test] +fn deptry_gitignore() -> Result<()> { + let context = TestContext::new("3.12"); + + let project_root = current_dir()? + .parent() + .unwrap() + .parent() + .unwrap() + .to_path_buf(); + let source_dist_dir = project_root + .join("scripts") + .join("packages") + .join("deptry_reproducer"); + let filter_path = regex::escape( + Url::from_directory_path(source_dist_dir.simplified_display().to_string()) + .unwrap() + .to_string() + .trim_end_matches('/'), + ); + let filters: Vec<_> = [(filter_path.as_str(), "[SOURCE_DIST_DIR]")] + .into_iter() + .chain(INSTA_FILTERS.to_vec()) + .collect(); + + uv_snapshot!(filters, command(&context) + .arg(format!("deptry_reproducer @ {}/deptry_reproducer-0.1.0.tar.gz", source_dist_dir.simplified_display())) + .arg("--strict") + .current_dir(source_dist_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + Downloaded 3 packages in [TIME] + Installed 3 packages in [TIME] + + cffi==1.16.0 + + deptry-reproducer==0.1.0 (from [SOURCE_DIST_DIR]/deptry_reproducer-0.1.0.tar.gz) + + pycparser==2.21 + "### + ); + + // Check that we packed the python source files + context + .assert_command("import deptry_reproducer.foo") + .success(); + + Ok(()) +} diff --git a/scripts/packages/deptry_reproducer/.gitignore b/scripts/packages/deptry_reproducer/.gitignore new file mode 100644 index 000000000..ea8c4bf7f --- /dev/null +++ b/scripts/packages/deptry_reproducer/.gitignore @@ -0,0 +1 @@ +/target diff --git a/scripts/packages/deptry_reproducer/Cargo.lock b/scripts/packages/deptry_reproducer/Cargo.lock new file mode 100644 index 000000000..1c831357b --- /dev/null +++ b/scripts/packages/deptry_reproducer/Cargo.lock @@ -0,0 +1,7 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "deptry_reproducer" +version = "0.1.0" diff --git a/scripts/packages/deptry_reproducer/Cargo.toml b/scripts/packages/deptry_reproducer/Cargo.toml new file mode 100644 index 000000000..d685993bd --- /dev/null +++ b/scripts/packages/deptry_reproducer/Cargo.toml @@ -0,0 +1,11 @@ +[package] +name = "deptry_reproducer" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[lib] +name = "deptry_reproducer" +crate-type = ["cdylib"] + +[dependencies] diff --git a/scripts/packages/deptry_reproducer/deptry_reproducer-0.1.0.tar.gz b/scripts/packages/deptry_reproducer/deptry_reproducer-0.1.0.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..b76ecee2e31ac1f1144d70eccd5654ee6b8e7513 GIT binary patch literal 917 zcmV;G18V#qiwFQknEhn{1MQgKZ`(Ey$NS8`;-Efs8=~K~GH7#AAYF@~TW7SxUJOHE zB+BL@i7H7I)qlUEq&TS!$H`o{-KOu0Z1O}(M}AJc%UG>tn@Cnmku@okw&(bc=Uz~` z5{?#S@#JWEo)=9fV6Ld`tLKGL2;Qy0^QWO7JOS_csSm4Bno4|>W5?NkxqpckHBGPS ziY-Y+O9nHrvh5vVk&jF&E-KU8?GB5ujO(E#@B-f=H#K|Cl`LnHRVi~zTJfAI?W}cKkVV0-?tRKs zQq64L*2Z9ArCE!2SW7|{Xw0e%$K*^c$=7!IH>Djv4*b`xUW>}z>2{OEEM}58b^Fjc z4F6HbzkfdU?p}KZp%4CKdVU`ICK!Mh1kuzR`9FnnDN1m5mKk_i3#s7;LP(Mzs!4)S z?U*@3AK^KysiIoT?{f-0qk?Qz44zK@K{Kxa(G#uCe6dL2~|6BX`@AJPxq}RvK zaq#>%+3@fCld$7I8u>qop28n%t{~?H1H94(Q+}#>iifZkUaVNfOjeb_RSW%=z(ns- z8`DbQrP3y`${-gKFkNeC0bOc{8Qq<%F~l5flW0CN)BM^F`j2b>`41cZVyXTDHK0TM zpNHWs|K0aLns{UV=OkKOHN41d)hf-(C6Vky!zEK)1qA~^BYE|F=AZo>Ad5@V8($&G zh`oVyFtHHuVZYpq_s#HjpI#WN=d;lH(IRO|K&CR%eTR+5$_1{3&|R;vc1 z2gEVF!zHuM?z5QpqvNm3vMyN3Dowj`6}DXTo&S7*c{^|M?j^ZfRq88FdCt*ES&JUv zY1cHwGi<%_k0WX`5-se{t5TXNy?J$X59?kM@6$pQ&fsTU0bXVJw8pZZkMypzm1tzz z`Mqy=VC3A#|NPBs`||Cp-@n8jJ5>M0)BaZdcQT2_`p-%98`CtSn%aN%J2i&D3CUYT zINmMJkd0BdNyxoc03HAUhO^vj literal 0 HcmV?d00001 diff --git a/scripts/packages/deptry_reproducer/pyproject.toml b/scripts/packages/deptry_reproducer/pyproject.toml new file mode 100644 index 000000000..cf8864fb5 --- /dev/null +++ b/scripts/packages/deptry_reproducer/pyproject.toml @@ -0,0 +1,21 @@ +[build-system] +requires = ["maturin>=1,<2.0"] +build-backend = "maturin" + +[project] +name = "deptry_reproducer" +requires-python = ">=3.8" +classifiers = [ + "Programming Language :: Rust", + "Programming Language :: Python :: Implementation :: CPython", + "Programming Language :: Python :: Implementation :: PyPy", +] +dependencies = ["cffi"] +dynamic = ["version"] +[project.optional-dependencies] +tests = [ + "pytest", +] +[tool.maturin] +bindings = "cffi" +python-source = "python" diff --git a/scripts/packages/deptry_reproducer/python/deptry_reproducer/__init__.py b/scripts/packages/deptry_reproducer/python/deptry_reproducer/__init__.py new file mode 100644 index 000000000..a95c6b977 --- /dev/null +++ b/scripts/packages/deptry_reproducer/python/deptry_reproducer/__init__.py @@ -0,0 +1,6 @@ +from .deptry_reproducer import * + + +__doc__ = deptry_reproducer.__doc__ +if hasattr(deptry_reproducer, "__all__"): + __all__ = deptry_reproducer.__all__ diff --git a/scripts/packages/deptry_reproducer/python/deptry_reproducer/foo.py b/scripts/packages/deptry_reproducer/python/deptry_reproducer/foo.py new file mode 100644 index 000000000..e69de29bb diff --git a/scripts/packages/deptry_reproducer/src/lib.rs b/scripts/packages/deptry_reproducer/src/lib.rs new file mode 100644 index 000000000..8b1378917 --- /dev/null +++ b/scripts/packages/deptry_reproducer/src/lib.rs @@ -0,0 +1 @@ +