From 5e3473f7026b4030a9b6ef94547567b09de63fbe Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 29 Jul 2024 14:29:57 +0200 Subject: [PATCH 1/4] Build all cpp binaries of test_wheels ci job in the `wheel-test-min` environment Similar to https://github.com/rerun-io/rerun/pull/6911 May fix Windows issues or at least make it easier to diagnose --- .github/workflows/reusable_test_wheels.yml | 14 +++++---- tests/roundtrips.py | 34 +++++++--------------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/.github/workflows/reusable_test_wheels.yml b/.github/workflows/reusable_test_wheels.yml index e1243ff5923c..b574238740e0 100644 --- a/.github/workflows/reusable_test_wheels.yml +++ b/.github/workflows/reusable_test_wheels.yml @@ -183,14 +183,19 @@ jobs: # Only check that we have the archetype roundtrip tests, but don't spend time actually running them run: pixi run -e wheel-test-min RUST_LOG=debug python tests/roundtrips.py --no-run + - name: Build C++ roundtrips + if: ${{ !inputs.FAST }} + # Separated out of roundtrips.py run so we control the pixi environment. + # This used to cause issues on Windows during the setup of the pixi environment when running from inside `roundtrips.py`. + run: pixi run -e wheel-test-min cpp-build-roundtrips + - name: Run tests/roundtrips.py if: ${{ !inputs.FAST }} - # --release so we can inherit from some of the artifacts that maturin has just built before # explicit target because otherwise cargo loses the target cache… even though this is the target anyhow… # --no-py-build because rerun-sdk is already built and installed - run: pixi run -e wheel-test-min RUST_LOG=debug python tests/roundtrips.py --release --target ${{ needs.set-config.outputs.TARGET }} --no-py-build + run: pixi run -e wheel-test-min RUST_LOG=debug python tests/roundtrips.py --target ${{ needs.set-config.outputs.TARGET }} --no-py-build --no-cpp-build - - name: Build C++ examples + - name: Build C++ snippets if: ${{ !inputs.FAST }} # Separated out of compare_snippet_output.py run so we control the pixi environment. # This used to cause issues on Windows during the setup of the pixi environment when running from inside `compare_snippet_output.py`. @@ -198,7 +203,6 @@ jobs: - name: Run docs/snippets/compare_snippet_output.py if: ${{ !inputs.FAST }} - # --release so we can inherit from some of the artifacts that maturin has just built before # explicit target because otherwise cargo loses the target cache… even though this is the target anyhow… # --no-py-build because rerun-sdk is already built and installed - run: pixi run -e wheel-test-min RUST_LOG=debug python docs/snippets/compare_snippet_output.py --release --target ${{ needs.set-config.outputs.TARGET }} --no-py-build --no-cpp-build + run: pixi run -e wheel-test-min RUST_LOG=debug python docs/snippets/compare_snippet_output.py --target ${{ needs.set-config.outputs.TARGET }} --no-py-build --no-cpp-build diff --git a/tests/roundtrips.py b/tests/roundtrips.py index cae0cc63373e..fea1baaa7409 100755 --- a/tests/roundtrips.py +++ b/tests/roundtrips.py @@ -90,56 +90,44 @@ def main() -> None: print("----------------------------------------------------------") print("Building rerun-sdk for Python…") start_time = time.time() - run(["pixi", "run", "py-build", "--quiet"], env=build_env) + run(["pixi", "run", "-e", "py", "py-build", "--quiet"], env=build_env) elapsed = time.time() - start_time print(f"rerun-sdk for Python built in {elapsed:.1f} seconds") print("") if args.no_cpp_build: - print("Skipping cmake configure & build for rerun_c & rerun_cpp - assuming it is already built and up-to-date!") + print("Skipping cmake configure & build - assuming all tests are already built and up-to-date!") else: print("----------------------------------------------------------") - print("Build rerun_c & rerun_cpp…") + print("Build roundtrips for C++…") start_time = time.time() cmake_configure(args.release, build_env) - cmake_build("rerun_sdk", args.release) + cmake_build("roundtrips", args.release) elapsed = time.time() - start_time - print(f"rerun-sdk for C++ built in {elapsed:.1f} seconds") + print(f"C++ roundtrips built in {elapsed:.1f} seconds") print("") print("----------------------------------------------------------") print(f"Building {len(archetypes)} archetypes…") - # Running CMake in parallel causes failures during rerun_sdk & arrow build. - # TODO(andreas): Tell cmake in a single command to build everything at once. - if not args.no_cpp_build: - start_time = time.time() - for arch in archetypes: - arch_opt_out = opt_out.get(arch, []) - if "cpp" in arch_opt_out: - continue - build(arch, "cpp", args) - elapsed = time.time() - start_time - print(f"C++ examples compiled and ran in {elapsed:.1f} seconds") - with multiprocessing.Pool() as pool: start_time = time.time() jobs = [] for arch in archetypes: arch_opt_out = opt_out.get(arch, []) - for language in ["python", "rust"]: + for language in ["python", "rust", "cpp"]: if language in arch_opt_out: continue - job = pool.apply_async(build, (arch, language, args)) + job = pool.apply_async(run_roundtrips, (arch, language, args)) jobs.append(job) print(f"Waiting for {len(jobs)} build jobs to finish…") for job in jobs: job.get() elapsed = time.time() - start_time - print(f"Python and Rust examples ran in {elapsed:.1f} seconds") + print(f"C++, Python and Rust examples ran in {elapsed:.1f} seconds") print("----------------------------------------------------------") - print(f"Comparing recordings for{len(archetypes)} archetypes…") + print(f"Comparing recordings for {len(archetypes)} archetypes…") start_time = time.time() for arch in archetypes: @@ -168,7 +156,7 @@ def main() -> None: print("All tests passed!") -def build(arch: str, language: str, args: argparse.Namespace) -> None: +def run_roundtrips(arch: str, language: str, args: argparse.Namespace) -> None: if language == "cpp": run_roundtrip_cpp(arch, args.release) elif language == "python": @@ -221,8 +209,6 @@ def run_roundtrip_cpp(arch: str, release: bool) -> str: target_name = f"roundtrip_{arch}" output_path = f"tests/cpp/roundtrips/{arch}/out.rrd" - cmake_build(target_name, release) - config_dir = "Release" if release else "Debug" target_path = f"{config_dir}/{target_name}.exe" if os.name == "nt" else target_name From ba8e2df3d95c4fc84a3decf930b955eb204e15ce Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 29 Jul 2024 15:53:18 +0200 Subject: [PATCH 2/4] fix execution path --- docs/snippets/compare_snippet_output.py | 6 +----- tests/roundtrips.py | 14 ++++---------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/docs/snippets/compare_snippet_output.py b/docs/snippets/compare_snippet_output.py index 55b40e83d962..122f829dcd86 100755 --- a/docs/snippets/compare_snippet_output.py +++ b/docs/snippets/compare_snippet_output.py @@ -79,11 +79,7 @@ def main() -> None: help="Skip cmake configure and ahead of time build for rerun_c & rerun_cpp", ) parser.add_argument("--full-dump", action="store_true", help="Dump both rrd files as tables") - parser.add_argument( - "--release", - action="store_true", - help="Run cargo invocations with --release and CMake with `-DCMAKE_BUILD_TYPE=Release` & `--config Release`", - ) + parser.add_argument("--release", action="store_true", help="Run cargo invocations with --release") parser.add_argument("--target", type=str, default=None, help="Target used for cargo invocations") parser.add_argument("--target-dir", type=str, default=None, help="Target directory used for cargo invocations") parser.add_argument("example", nargs="*", type=str, default=None, help="Run only the specified examples") diff --git a/tests/roundtrips.py b/tests/roundtrips.py index fea1baaa7409..456d7367b012 100755 --- a/tests/roundtrips.py +++ b/tests/roundtrips.py @@ -42,11 +42,7 @@ def main() -> None: help="Skip cmake configure and ahead of time build for rerun_c & rerun_cpp", ) parser.add_argument("--full-dump", action="store_true", help="Dump both rrd files as tables") - parser.add_argument( - "--release", - action="store_true", - help="Run cargo invocations with --release and CMake with `-DCMAKE_BUILD_TYPE=Release` & `--config Release`", - ) + parser.add_argument("--release", action="store_true", help="Run cargo invocations with --release") parser.add_argument("--target", type=str, default=None, help="Target used for cargo invocations") parser.add_argument("--target-dir", type=str, default=None, help="Target directory used for cargo invocations") parser.add_argument("archetype", nargs="*", type=str, default=None, help="Run only the specified archetypes") @@ -207,12 +203,10 @@ def run_roundtrip_rust(arch: str, release: bool, target: str | None, target_dir: def run_roundtrip_cpp(arch: str, release: bool) -> str: target_name = f"roundtrip_{arch}" - output_path = f"tests/cpp/roundtrips/{arch}/out.rrd" - - config_dir = "Release" if release else "Debug" + output_path = f"build/debug/tests/cpp/roundtrips/{arch}/out.rrd" - target_path = f"{config_dir}/{target_name}.exe" if os.name == "nt" else target_name - cmd = [f"{cpp_build_dir}/tests/cpp/roundtrips/{target_path}", output_path] + extension = ".exe" if os.name == "nt" else "" + cmd = [f"./build/debug/docs/roundtrips/{target_name}{extension}"] run(cmd, env=roundtrip_env(), timeout=12000) return output_path From 5246c2b1cb88c89cf82efbe8363b86fb102dbd94 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 29 Jul 2024 16:19:14 +0200 Subject: [PATCH 3/4] fix path --- tests/roundtrips.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/roundtrips.py b/tests/roundtrips.py index 456d7367b012..d37fd3ae37f0 100755 --- a/tests/roundtrips.py +++ b/tests/roundtrips.py @@ -206,7 +206,7 @@ def run_roundtrip_cpp(arch: str, release: bool) -> str: output_path = f"build/debug/tests/cpp/roundtrips/{arch}/out.rrd" extension = ".exe" if os.name == "nt" else "" - cmd = [f"./build/debug/docs/roundtrips/{target_name}{extension}"] + cmd = [f"./build/debug/tests/cpp/roundtrips/{target_name}{extension}"] run(cmd, env=roundtrip_env(), timeout=12000) return output_path From a04c92b8e3baa947692dfe92a5d8153eb800b2c0 Mon Sep 17 00:00:00 2001 From: Andreas Reich Date: Mon, 29 Jul 2024 18:15:37 +0200 Subject: [PATCH 4/4] forgot to pass output path.. --- tests/roundtrips.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/roundtrips.py b/tests/roundtrips.py index d37fd3ae37f0..0d8e51ee011c 100755 --- a/tests/roundtrips.py +++ b/tests/roundtrips.py @@ -203,10 +203,10 @@ def run_roundtrip_rust(arch: str, release: bool, target: str | None, target_dir: def run_roundtrip_cpp(arch: str, release: bool) -> str: target_name = f"roundtrip_{arch}" - output_path = f"build/debug/tests/cpp/roundtrips/{arch}/out.rrd" + output_path = f"tests/cpp/roundtrips/{arch}/out.rrd" extension = ".exe" if os.name == "nt" else "" - cmd = [f"./build/debug/tests/cpp/roundtrips/{target_name}{extension}"] + cmd = [f"./build/debug/tests/cpp/roundtrips/{target_name}{extension}", output_path] run(cmd, env=roundtrip_env(), timeout=12000) return output_path