diff --git a/.github/workflows/ci-scripts.yml b/.github/workflows/ci-scripts.yml index 9b9124ab2..f7f0dbb07 100644 --- a/.github/workflows/ci-scripts.yml +++ b/.github/workflows/ci-scripts.yml @@ -5,11 +5,15 @@ on: branches: - main paths: + - ci/check_cargo_lock.py + - ci/check_sdist.py - ci/set_lance_version.py - ci/tests/** - .github/workflows/ci-scripts.yml pull_request: paths: + - ci/check_cargo_lock.py + - ci/check_sdist.py - ci/set_lance_version.py - ci/tests/** - .github/workflows/ci-scripts.yml diff --git a/.github/workflows/pypi-publish.yml b/.github/workflows/pypi-publish.yml index d16203979..e71564473 100644 --- a/.github/workflows/pypi-publish.yml +++ b/.github/workflows/pypi-publish.yml @@ -12,6 +12,9 @@ on: - .github/workflows/build_mac_wheel/action.yml - .github/workflows/build_windows_wheel/action.yml - .github/actions/setup-protoc/action.yml + - ci/check_cargo_lock.py + - ci/check_sdist.py + - python/pyproject.toml - Cargo.toml # Change in dependency frequently breaks builds - Cargo.lock @@ -161,10 +164,91 @@ jobs: name: wheels-windows path: target/wheels/lancedb-*.whl if-no-files-found: error + sdist: + name: Python sdist + timeout-minutes: 60 + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v6 + with: + fetch-depth: 0 + lfs: true + - name: Set up Python + uses: actions/setup-python@v6 + with: + python-version: "3.12" + - name: Build sdist + uses: PyO3/maturin-action@v1 + with: + maturin-version: "1.12.4" + command: sdist + working-directory: python + args: --out dist + # maturin ships the nodejs-only pins in Cargo.lock. Remove this step when + # the pinned maturin fixes https://github.com/PyO3/maturin/issues/2609. + - name: Sync Cargo.lock to the packaged workspace + shell: bash + run: | + # A repo lock in sync with every manifest means the prune only drops pins. + cargo metadata --locked --format-version 1 > /dev/null + mkdir "$RUNNER_TEMP/sdist-src" + tar -xzf python/dist/lancedb-*.tar.gz -C "$RUNNER_TEMP/sdist-src" + lock=$(echo "$RUNNER_TEMP"/sdist-src/lancedb-*/Cargo.lock) + cp "$lock" "$RUNNER_TEMP/Cargo.lock.before" + cargo metadata --format-version 1 \ + --manifest-path "$RUNNER_TEMP"/sdist-src/lancedb-*/Cargo.toml > /dev/null + python ci/check_cargo_lock.py "$RUNNER_TEMP/Cargo.lock.before" "$lock" + tar -czf python/dist/lancedb-*.tar.gz -C "$RUNNER_TEMP/sdist-src" \ + "$(basename python/dist/lancedb-*.tar.gz .tar.gz)" + - name: Install sdist check tools + shell: bash + run: python -m pip install twine==7.0.0 pydistcheck==0.11.3 + - name: Check sdist size + shell: bash + run: | + pydistcheck --select distro-too-large-compressed \ + --max-allowed-size-compressed 100M python/dist/lancedb-*.tar.gz + - name: Check sdist contents + shell: bash + run: python ci/check_sdist.py python/dist/lancedb-*.tar.gz + - name: Check sdist with twine + shell: bash + run: python -m twine check --strict python/dist/lancedb-*.tar.gz + - uses: ./.github/actions/setup-protoc + # Build the way `pip install lancedb` does when no wheel matches. The dev + # profile keeps the build within the runner's disk. + - name: Build wheel from sdist + shell: bash + env: + MATURIN_PEP517_ARGS: "--profile dev --locked" + CARGO_PROFILE_DEV_DEBUG: "0" + run: | + python -m pip wheel --no-deps --wheel-dir "$RUNNER_TEMP/sdist-wheel" \ + python/dist/lancedb-*.tar.gz + - name: Smoke test wheel built from sdist + shell: bash + working-directory: ${{ runner.temp }} + run: | + python -m venv sdist-venv + sdist-venv/bin/pip install sdist-wheel/lancedb-*.whl + sdist-venv/bin/python -c ' + import tempfile + import lancedb + db = lancedb.connect(tempfile.mkdtemp()) + table = db.create_table("t", [{"vector": [1.0, 2.0], "id": 1}, {"vector": [3.0, 4.0], "id": 2}]) + assert table.search([1.0, 2.0]).limit(1).to_list()[0]["id"] == 1 + print("lancedb", lancedb.__version__, "built from sdist OK") + ' + - uses: actions/upload-artifact@v7 + if: startsWith(github.ref, 'refs/tags/v') + with: + name: sdist + path: python/dist/lancedb-*.tar.gz + if-no-files-found: error publish: - name: Publish wheels + name: Publish wheels and sdist if: startsWith(github.ref, 'refs/tags/v') - needs: [linux, mac, windows] + needs: [linux, mac, windows, sdist] runs-on: ubuntu-latest permissions: id-token: write @@ -177,7 +261,12 @@ jobs: pattern: wheels-* path: target/wheels merge-multiple: true - - name: List wheels + - name: Download sdist artifact + uses: actions/download-artifact@v8 + with: + name: sdist + path: target/wheels + - name: List packages run: ls -la target/wheels - name: Choose repo id: choose_repo @@ -194,13 +283,18 @@ jobs: run: | shopt -s nullglob WHEELS=(target/wheels/*.whl) + SDISTS=(target/wheels/*.tar.gz) if [[ ${#WHEELS[@]} -eq 0 ]]; then echo "No wheels found in target/wheels/" >&2 exit 1 fi - for WHEEL in "${WHEELS[@]}"; do - echo "Uploading $WHEEL to Fury" - curl -f -F package=@"$WHEEL" "https://$FURY_TOKEN@push.fury.io/lancedb/" + if [[ ${#SDISTS[@]} -ne 1 ]]; then + echo "Expected one sdist in target/wheels/, found ${#SDISTS[@]}" >&2 + exit 1 + fi + for PACKAGE in "${WHEELS[@]}" "${SDISTS[@]}"; do + echo "Uploading $PACKAGE to Fury" + curl -f -F package=@"$PACKAGE" "https://$FURY_TOKEN@push.fury.io/lancedb/" done # NOTE: pypa/gh-action-pypi-publish must be invoked directly from a # workflow file, not from inside a composite action. When called from a @@ -215,7 +309,7 @@ jobs: report-failure: name: Report Workflow Failure runs-on: ubuntu-latest - needs: [linux, mac, windows, publish] + needs: [linux, mac, windows, sdist, publish] permissions: contents: read issues: write diff --git a/ci/check_cargo_lock.py b/ci/check_cargo_lock.py new file mode 100644 index 000000000..551b76794 --- /dev/null +++ b/ci/check_cargo_lock.py @@ -0,0 +1,56 @@ +"""Fail if pruning Cargo.lock did more than remove packages and the edges to them. + +Usage: python ci/check_cargo_lock.py +""" + +import sys +import tomllib +from pathlib import Path + + +def key(package: dict) -> tuple: + return tuple(package.get(k) for k in ("name", "version", "source", "checksum")) + + +def graph(lock: dict) -> dict[tuple, set[tuple]]: + """Map each package to the packages its dependency strings resolve to.""" + by_name = {} + for p in lock["package"]: + by_name.setdefault(p["name"], []).append(p) + + def resolve(dep: str) -> tuple: + # Cargo writes "name", "name version" or "name version (source)". + name, *rest = dep.split(" ", 2) + (match,) = [ + p + for p in by_name[name] + if (not rest or p["version"] == rest[0]) + and (len(rest) < 2 or rest[1] == f"({p.get('source')})") + ] + return key(match) + + return { + key(p): {resolve(d) for d in p.get("dependencies", [])} for p in lock["package"] + } + + +def main() -> None: + before, after = (tomllib.loads(Path(p).read_text()) for p in sys.argv[1:]) + errors = [] + for field in (before.keys() | after.keys()) - {"package"}: + if before.get(field) != after.get(field): + errors.append(f"top-level {field!r} changed") + edges_before, edges_after = graph(before), graph(after) + removed = edges_before.keys() - edges_after.keys() + for package, deps in edges_after.items(): + if package not in edges_before: + errors.append(f"added or changed {package}") + elif deps != edges_before[package] - removed: + errors.append(f"dependencies of {package[:2]} changed") + if errors: + sys.exit("error: the prune changed Cargo.lock:\n" + "\n".join(errors)) + print(f"Cargo.lock prune only removed {len(removed)} packages") + + +if __name__ == "__main__": + main() diff --git a/ci/check_sdist.py b/ci/check_sdist.py new file mode 100644 index 000000000..6c0c88931 --- /dev/null +++ b/ci/check_sdist.py @@ -0,0 +1,42 @@ +"""Fail if the lancedb sdist lacks a git-tracked file of the Python package. + +Usage: python ci/check_sdist.py (run from the repository root) +""" + +import subprocess +import sys +import tarfile +from pathlib import Path + +# Where git tracks the package, and where maturin puts it under the sdist root. +TRACKED_PACKAGE = "python/python/lancedb" +SDIST_PACKAGE = "python/lancedb" + + +def main() -> None: + sdist = Path(sys.argv[1]) + root = sdist.name.removesuffix(".tar.gz") + listing = subprocess.run( + ["git", "ls-files", "-z", "--", TRACKED_PACKAGE], + check=True, + capture_output=True, + text=True, + ).stdout + tracked = [p for p in listing.split("\0") if p] + if not tracked: + sys.exit(f"error: git tracks no files under {TRACKED_PACKAGE}") + + with tarfile.open(sdist) as tar: + members = {m.name for m in tar if m.isfile()} + missing = [ + p + for p in tracked + if f"{root}/{SDIST_PACKAGE}{p.removeprefix(TRACKED_PACKAGE)}" not in members + ] + if missing: + sys.exit(f"error: {sdist.name} lacks tracked sources: {', '.join(missing)}") + print(f"{sdist.name} OK: all {len(tracked)} tracked sources present") + + +if __name__ == "__main__": + main() diff --git a/ci/tests/test_check_cargo_lock.py b/ci/tests/test_check_cargo_lock.py new file mode 100644 index 000000000..82570d087 --- /dev/null +++ b/ci/tests/test_check_cargo_lock.py @@ -0,0 +1,97 @@ +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT = REPO_ROOT / "ci" / "check_cargo_lock.py" +CRATES_IO = "registry+https://github.com/rust-lang/crates.io-index" + + +def package(name, version, deps=(), source=CRATES_IO, checksum="aa"): + text = f'[[package]]\nname = "{name}"\nversion = "{version}"\n' + if source: + text += f'source = "{source}"\nchecksum = "{checksum}"\n' + if deps: + text += f"dependencies = {list(deps)!r}\n".replace("'", '"') + return text + + +LIBC = package("libc", "0.2.1") +UNTRUSTED = package("untrusted", "0.9.0") +# The prune drops napi and untrusted 0.7.1, the edge from app to it, and the +# version qualifier on app's remaining untrusted edge. +BEFORE = "version = 4\n\n" + "".join( + [ + package( + "app", "1.0.0", ["libc", "untrusted 0.7.1", "untrusted 0.9.0"], source=None + ), + package("napi", "3.0.0", ["untrusted 0.7.1"]), + LIBC, + package("untrusted", "0.7.1"), + UNTRUSTED, + ] +) +APP = package("app", "1.0.0", ["libc", "untrusted"], source=None) + + +class CheckCargoLockTest(unittest.TestCase): + def run_check(self, after, header="version = 4\n\n"): + with tempfile.TemporaryDirectory() as tmp: + before_path = Path(tmp, "before.lock") + after_path = Path(tmp, "after.lock") + before_path.write_text(BEFORE) + after_path.write_text(header + after) + return subprocess.run( + [sys.executable, SCRIPT, before_path, after_path], + capture_output=True, + text=True, + ) + + def test_prune_passes(self): + result = self.run_check(APP + LIBC + UNTRUSTED) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_added_entry_fails(self): + result = self.run_check(APP + LIBC + UNTRUSTED + package("ring", "0.17.0")) + self.assertNotEqual(result.returncode, 0) + self.assertIn("ring", result.stderr) + + def test_changed_version_fails(self): + result = self.run_check(APP + package("libc", "0.2.2") + UNTRUSTED) + self.assertNotEqual(result.returncode, 0) + self.assertIn("0.2.2", result.stderr) + + def test_changed_source_fails(self): + libc = package("libc", "0.2.1", source="git+https://example.com/libc") + result = self.run_check(APP + libc + UNTRUSTED) + self.assertNotEqual(result.returncode, 0) + self.assertIn("example.com", result.stderr) + + def test_changed_checksum_fails(self): + libc = package("libc", "0.2.1", checksum="bb") + result = self.run_check(APP + libc + UNTRUSTED) + self.assertNotEqual(result.returncode, 0) + self.assertIn("'bb'", result.stderr) + + def test_added_dependency_fails(self): + libc = package("libc", "0.2.1", ["untrusted"]) + result = self.run_check(APP + libc + UNTRUSTED) + self.assertNotEqual(result.returncode, 0) + self.assertIn("dependencies of ('libc'", result.stderr) + + def test_removed_dependency_fails(self): + app = package("app", "1.0.0", ["untrusted"], source=None) + result = self.run_check(app + LIBC + UNTRUSTED) + self.assertNotEqual(result.returncode, 0) + self.assertIn("dependencies of ('app'", result.stderr) + + def test_changed_lock_version_fails(self): + result = self.run_check(APP + LIBC + UNTRUSTED, header="version = 3\n\n") + self.assertNotEqual(result.returncode, 0) + self.assertIn("top-level 'version'", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/ci/tests/test_check_sdist.py b/ci/tests/test_check_sdist.py new file mode 100644 index 000000000..1bb34306e --- /dev/null +++ b/ci/tests/test_check_sdist.py @@ -0,0 +1,71 @@ +import subprocess +import sys +import tarfile +import tempfile +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[2] +SCRIPT = REPO_ROOT / "ci" / "check_sdist.py" +ROOT = "lancedb-1.0.0" + + +class CheckSdistTest(unittest.TestCase): + def run_check(self, tracked, shipped, links=(), root=ROOT): + """Run the check in a git repo tracking `tracked`, on an sdist holding `shipped` + as files and `links` as symlinks, under the folder `root`.""" + with tempfile.TemporaryDirectory() as tmp: + subprocess.run(["git", "init", "-q", tmp], check=True) + for name in tracked: + path = Path(tmp, "python/python/lancedb", name) + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("x = 1\n") + if tracked: + subprocess.run(["git", "-C", tmp, "add", "."], check=True) + sdist = Path(tmp, f"{ROOT}.tar.gz") + with tarfile.open(sdist, "w:gz") as tar: + for name in shipped: + member = Path(tmp, "member") + member.write_text("x = 1\n") + tar.add(member, arcname=f"{root}/python/lancedb/{name}") + for name in links: + link = tarfile.TarInfo(f"{root}/python/lancedb/{name}") + link.type = tarfile.SYMTYPE + link.linkname = "__init__.py" + tar.addfile(link) + return subprocess.run( + [sys.executable, SCRIPT, sdist], + cwd=tmp, + capture_output=True, + text=True, + ) + + def test_complete_sdist_passes(self): + result = self.run_check(["__init__.py", "db.py"], ["__init__.py", "db.py"]) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_missing_tracked_file_fails(self): + result = self.run_check(["__init__.py", "db.py"], ["__init__.py"]) + self.assertNotEqual(result.returncode, 0) + self.assertIn("python/python/lancedb/db.py", result.stderr) + + def test_tracked_file_shipped_as_symlink_fails(self): + result = self.run_check( + ["__init__.py", "db.py"], ["__init__.py"], links=["db.py"] + ) + self.assertNotEqual(result.returncode, 0) + self.assertIn("python/python/lancedb/db.py", result.stderr) + + def test_files_outside_the_sdist_root_fail(self): + result = self.run_check(["__init__.py"], ["__init__.py"], root="other-1.0.0") + self.assertNotEqual(result.returncode, 0) + self.assertIn("python/python/lancedb/__init__.py", result.stderr) + + def test_no_tracked_files_fails(self): + result = self.run_check([], ["__init__.py"]) + self.assertNotEqual(result.returncode, 0) + self.assertIn("git tracks no files", result.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/python/README.md b/python/README.md index 3a81c486a..f941c9e20 100644 --- a/python/README.md +++ b/python/README.md @@ -8,6 +8,8 @@ A Python library for [LanceDB](https://github.com/lancedb/lancedb). pip install lancedb ``` +If no wheel is published for your platform, pip builds `lancedb` from the source distribution. That build needs a [Rust toolchain](https://rustup.rs/) and [protoc](https://grpc.io/docs/protoc-installation/) (the Protocol Buffers compiler) on your `PATH`, and can take a while. + ### Pre-Haswell x86_64 hosts: `lancedb-compat` The default `lancedb` wheel targets `x86-64-haswell` (AVX2 + FMA + F16C) for full performance on modern hardware. Pre-Haswell hosts — Intel Sandy Bridge / Ivy Bridge / Westmere; AMD Bulldozer / Piledriver / Steamroller — don't have AVX2 and crash with `Illegal instruction` at `import lancedb`.