diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a21cf5a4cb7..ff8eb92c0b4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -578,11 +578,14 @@ jobs: for crate in "${crates[@]}"; do cargo build -p "$crate" --target "$TARGET" done - - name: features of gix-features + - name: features of gix-parallel and gix-utils run: | set -x - for feature in progress parallel io-pipe crc32 cache-efficiency-debug; do - cargo build -p gix-features --features "$feature" --target "$TARGET" + for feature in '' parallel once_cell parallel,once_cell; do + cargo build -p gix-parallel --no-default-features --features "$feature" --target "$TARGET" + done + for feature in '' progress progress,progress-unit-bytes progress,progress-unit-human-numbers interrupt io-pipe cache-efficiency-debug; do + cargo build -p gix-utils --no-default-features --features "$feature" --target "$TARGET" done - name: crates with 'sha1' and 'wasm' feature run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ce4053c4121..dacc0dd6ea0 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,22 +9,22 @@ on: # branches: # - fix-releases tags: - # For now, real releases always use `workflow_dispatch`, and running the workflow on tag pushes - # is only done in testing. This is because we usually push too many tags at once for the `push` - # event to be triggered, since there are usually more than 3 crates tagged together. So the - # `push` trigger doesn't usually work. If we allow it, we risk running the workflow twice if - # it is also manually triggered based on the assumption that it would not run. See #1970 for - # details. See also the `run-release-workflow` and `roll-release` recipes in the `justfile`. - # - 'v*' - - 'v*-DO-NOT-USE' # Pattern for tags used to test the workflow (usually done in a fork). + # For now, real releases always use `workflow_dispatch`, and running the workflow on tag pushes + # is only done in testing. This is because we usually push too many tags at once for the `push` + # event to be triggered, since there are usually more than 3 crates tagged together. So the + # `push` trigger doesn't usually work. If we allow it, we risk running the workflow twice if + # it is also manually triggered based on the assumption that it would not run. See #1970 for + # details. See also the `run-release-workflow` and `roll-release` recipes in the `justfile`. + # - 'v*' + - "v*-DO-NOT-USE" # Pattern for tags used to test the workflow (usually done in a fork). workflow_dispatch: permissions: - contents: read # This is set more permissively in jobs that need `write`. + contents: read # This is set more permissively in jobs that need `write`. defaults: run: - shell: bash # Use `bash` even in the Windows jobs. + shell: bash # Use `bash` even in the Windows jobs. jobs: # Create a draft release, initially with no binary assets attached. @@ -32,11 +32,11 @@ jobs: runs-on: ubuntu-slim permissions: - contents: write # Allows the use of `gh release create`. + contents: write # Allows the use of `gh release create`. -# env: -# # Set to force version number, e.g., when no tag exists. -# VERSION: TEST-0.0.0 + # env: + # # Set to force version number, e.g., when no tag exists. + # VERSION: TEST-0.0.0 steps: - name: Checkout repository @@ -85,7 +85,7 @@ jobs: # Build for a particular feature and target, and attach an archive for it. build-release: - needs: [ create-release ] + needs: [create-release] strategy: matrix: @@ -108,7 +108,7 @@ jobs: - i686-pc-windows-msvc - aarch64-pc-windows-msvc # When changing these features, make the same change in build-macos-universal2-release. - feature: [ small, lean, max, max-pure ] + feature: [small, lean, max, max-pure] include: - rust: stable - target: x86_64-unknown-linux-musl @@ -219,13 +219,13 @@ jobs: runs-on: ${{ matrix.os }} permissions: - contents: write # Allows the use of `gh release upload`. + contents: write # Allows the use of `gh release upload`. env: - RUST_BACKTRACE: '1' # Emit backtraces on panics. + RUST_BACKTRACE: "1" # Emit backtraces on panics. CARGO_TERM_COLOR: always - CLICOLOR: '1' - CARGO: cargo # On Linux, this will be changed to `cross` in a later step. + CLICOLOR: "1" + CARGO: cargo # On Linux, this will be changed to `cross` in a later step. FEATURE: ${{ matrix.feature }} VERSION: ${{ needs.create-release.outputs.version }} TARGET: ${{ matrix.target }} @@ -269,7 +269,17 @@ jobs: - name: Build release binary (with extra optimizations) run: | - "$CARGO" build --verbose --profile="$PROFILE" "$TARGET_FLAGS" --no-default-features --features="$FEATURE" + bash etc/scripts/remap-release-paths.sh build "$CARGO" build --verbose --profile="$PROFILE" "$TARGET_FLAGS" --no-default-features --features="$FEATURE" + + - name: Check release binaries for local build paths + run: | + suffix='' + if [[ "$TARGET" == *-windows-* ]]; then + suffix='.exe' + fi + if ! bash etc/scripts/remap-release-paths.sh check "$TARGET_DIR/$PROFILE/gix$suffix" "$TARGET_DIR/$PROFILE/ein$suffix"; then + echo '::warning::Release binary path scan failed; see the output above. Continuing with packaging.' + fi - name: Determine archive basename run: echo "ARCHIVE=gitoxide-$FEATURE-$VERSION-$TARGET" >> "$GITHUB_ENV" @@ -308,15 +318,15 @@ jobs: build-macos-universal2-release: runs-on: macos-latest - needs: [ create-release, build-release ] + needs: [create-release, build-release] strategy: matrix: # These features need to be exactly the same as the features in build-release. - feature: [ small, lean, max, max-pure ] + feature: [small, lean, max, max-pure] permissions: - contents: write # Allows the use of `gh release upload`. + contents: write # Allows the use of `gh release upload`. env: BASH_ENV: ./helpers.sh @@ -375,10 +385,10 @@ jobs: publish-release: runs-on: ubuntu-slim - needs: [ create-release, build-release, build-macos-universal2-release ] + needs: [create-release, build-release, build-macos-universal2-release] permissions: - contents: write # Allows use of `gh release` for `upload`, `delete-asset`, and `edit`. + contents: write # Allows use of `gh release` for `upload`, `delete-asset`, and `edit`. env: REPOSITORY: ${{ github.repository }} @@ -444,11 +454,11 @@ jobs: announce-release: runs-on: ubuntu-slim - needs: [ create-release, publish-release ] + needs: [create-release, publish-release] permissions: - contents: write # Needed to distinguish unpublished (still draft) from missing releases. - discussions: write # For adding a comment in the announcement discussion. + contents: write # Needed to distinguish unpublished (still draft) from missing releases. + discussions: write # For adding a comment in the announcement discussion. env: REPOSITORY: ${{ github.repository }} @@ -520,7 +530,7 @@ jobs: installation: strategy: matrix: - build: [ win-msvc, win-gnu, win32-msvc, win32-gnu ] + build: [win-msvc, win-gnu, win32-msvc, win32-gnu] include: - build: win-msvc os: windows-latest @@ -559,7 +569,7 @@ jobs: msystem: MINGW${{ startsWith(matrix.target, 'i686-') && '32' || '64' }} pacboy: cc:p path-type: inherit - - name: 'Installation from crates.io: gitoxide' + - name: "Installation from crates.io: gitoxide" run: | cargo +"$RUST" install --target "$TARGET" --no-default-features \ --features max-pure --target-dir install-artifacts --debug --force gitoxide diff --git a/AGENTS.md b/AGENTS.md index e7b19c995b7..e437159565d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -105,6 +105,7 @@ Follow "purposeful conventional commits" style: ### Code Style +- Do not create persistent Python scripts in this repository. Use Bash for repository automation and script-based tests unless the user explicitly requests otherwise. - Follow existing patterns in the codebase - Skip stylistic rewrites that increase SLOC after formatting; keep simpler existing forms instead of applying style rules unconditionally. - Start new Rust modules as `foo.rs`. Create a module directory only when it contains multiple module files; then use `foo/mod.rs` rather than a sibling `foo.rs` file. @@ -112,7 +113,7 @@ Follow "purposeful conventional commits" style: - Prefer references in plumbing crates to avoid expensive clones - Avoid calling `.detach()` unless an owned value is explicitly required. Many `gix` APIs accept attached ids and references directly, so prefer keeping repository-backed handles like `gix::Id` when possible. - Name variables holding untyped Git object IDs `_id` or `*__id` (for example, `commit_id`, `root_tree_id`, or `note_blob_id`) so the object kind is always explicit. -- Use `gix_features::threading::*` for interior mutability primitives +- Use `gix_parallel::*` for interior mutability primitives ### Path Handling diff --git a/Cargo.lock b/Cargo.lock index cb8be04a2f1..d9dba8d16cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1476,7 +1476,6 @@ dependencies = [ "futures-lite", "gitoxide-core", "gix", - "gix-features", "gix-tix", "prodash", "serde_derive", @@ -1499,6 +1498,7 @@ dependencies = [ "fs-err", "futures-io", "futures-lite", + "gitoxide-core", "gix", "gix-archive", "gix-fsck", @@ -1508,6 +1508,7 @@ dependencies = [ "gix-trace", "gix-transport", "gix-url", + "insta", "layout-rs", "open", "parking_lot", @@ -1542,7 +1543,6 @@ dependencies = [ "gix-dir", "gix-discover", "gix-error", - "gix-features", "gix-filter", "gix-fs", "gix-glob", @@ -1558,6 +1558,7 @@ dependencies = [ "gix-object", "gix-odb", "gix-pack", + "gix-parallel", "gix-path", "gix-pathspec", "gix-prompt", @@ -1642,9 +1643,9 @@ dependencies = [ "criterion", "document-features", "gix-error", - "gix-features", "gix-fs", "gix-glob", + "gix-parallel", "gix-path", "gix-quote", "gix-testtools", @@ -1737,12 +1738,13 @@ dependencies = [ "gix-config", "gix-config-value", "gix-error", - "gix-features", "gix-glob", + "gix-parallel", "gix-path", "gix-ref", "gix-sec", "gix-testtools", + "gix-trace", "gix-utils", "insta", "serde", @@ -1785,6 +1787,7 @@ dependencies = [ "gix-trace", "gix-url", "insta", + "percent-encoding", "serde", ] @@ -1887,29 +1890,6 @@ dependencies = [ "insta", ] -[[package]] -name = "gix-features" -version = "0.50.0" -dependencies = [ - "bytes", - "bytesize", - "crc32fast", - "crossbeam-channel", - "document-features", - "gix-error", - "gix-path", - "gix-testtools", - "gix-trace", - "gix-utils", - "insta", - "libc", - "once_cell", - "parking_lot", - "prodash", - "tracing-subscriber", - "walkdir", -] - [[package]] name = "gix-fetchhead" version = "0.0.0" @@ -1943,15 +1923,18 @@ version = "0.23.0" dependencies = [ "bstr", "crossbeam-channel", + "document-features", "gix-error", - "gix-features", + "gix-parallel", "gix-path", "gix-testtools", "gix-utils", "insta", "is_ci", + "libc", "serde", "tempfile", + "walkdir", ] [[package]] @@ -1974,9 +1957,10 @@ dependencies = [ "bstr", "criterion", "document-features", - "gix-features", + "gix-fs", "gix-path", "gix-testtools", + "gix-trace", "gix-utils", "serde", ] @@ -1989,9 +1973,9 @@ dependencies = [ "document-features", "faster-hex", "gix-error", - "gix-features", "gix-hash", "gix-testtools", + "gix-utils", "insta", "serde", "sha1dc", @@ -2048,14 +2032,15 @@ dependencies = [ "fnv", "gix-bitmap", "gix-error", - "gix-features", "gix-fs", "gix-hash", "gix-hashtable", "gix-lock", "gix-object", "gix-odb", + "gix-parallel", "gix-testtools", + "gix-trace", "gix-traverse", "gix-utils", "gix-validate", @@ -2186,7 +2171,6 @@ dependencies = [ "gix-command", "gix-date", "gix-error", - "gix-features", "gix-hash", "gix-hashtable", "gix-object", @@ -2209,22 +2193,25 @@ name = "gix-odb" version = "0.85.0" dependencies = [ "arc-swap", + "crc32fast", "crossbeam-channel", "document-features", "filetime", "gix-actor", "gix-date", "gix-error", - "gix-features", "gix-fs", "gix-hash", "gix-hashtable", "gix-object", "gix-odb", "gix-pack", + "gix-parallel", "gix-path", "gix-quote", "gix-testtools", + "gix-trace", + "gix-utils", "gix-zlib", "insta", "maplit", @@ -2241,27 +2228,31 @@ version = "0.75.0" dependencies = [ "bstr", "clru", + "crc32fast", "criterion", "crossbeam-deque", "document-features", "gix-chunk", "gix-diff", "gix-error", - "gix-features", "gix-hash", "gix-hashtable", "gix-object", "gix-odb", "gix-pack", + "gix-parallel", "gix-path", "gix-tempfile", "gix-testtools", + "gix-trace", "gix-traverse", + "gix-utils", "gix-zlib", "insta", "maplit", "memmap2", "parking_lot", + "prodash", "serde", "smallvec", "uluru", @@ -2289,6 +2280,20 @@ dependencies = [ "serde", ] +[[package]] +name = "gix-parallel" +version = "0.50.0" +dependencies = [ + "crossbeam-channel", + "document-features", + "gix-testtools", + "gix-trace", + "insta", + "once_cell", + "parking_lot", + "tracing-subscriber", +] + [[package]] name = "gix-path" version = "0.13.0" @@ -2349,7 +2354,6 @@ dependencies = [ "gix-credentials", "gix-date", "gix-error", - "gix-features", "gix-hash", "gix-lock", "gix-macros", @@ -2392,12 +2396,12 @@ dependencies = [ "gix-date", "gix-discover", "gix-error", - "gix-features", "gix-fs", "gix-hash", "gix-lock", "gix-object", "gix-odb", + "gix-parallel", "gix-path", "gix-tempfile", "gix-testtools", @@ -2499,17 +2503,19 @@ dependencies = [ "gix-diff", "gix-dir", "gix-error", - "gix-features", "gix-filter", "gix-fs", "gix-hash", "gix-index", "gix-object", "gix-odb", + "gix-parallel", "gix-path", "gix-pathspec", "gix-status", "gix-testtools", + "gix-trace", + "gix-utils", "gix-worktree", "hashbrown 0.16.1", "insta", @@ -2525,7 +2531,7 @@ dependencies = [ "bstr", "gix-config", "gix-error", - "gix-features", + "gix-fs", "gix-path", "gix-pathspec", "gix-refspec", @@ -2635,7 +2641,6 @@ dependencies = [ "gix-command", "gix-credentials", "gix-error", - "gix-features", "gix-hash", "gix-macros", "gix-pack", @@ -2644,8 +2649,10 @@ dependencies = [ "gix-quote", "gix-sec", "gix-testtools", + "gix-trace", "gix-transport", "gix-url", + "gix-utils", "insta", "parking_lot", "pin-project-lite", @@ -2699,10 +2706,17 @@ name = "gix-utils" version = "0.4.0" dependencies = [ "bstr", + "bytes", + "bytesize", "criterion", + "document-features", "fastrand", "getrandom 0.4.3", + "gix-error", + "gix-testtools", "gix-utils", + "insta", + "prodash", "unicode-normalization", ] @@ -2726,7 +2740,6 @@ dependencies = [ "gix-attributes", "gix-discover", "gix-error", - "gix-features", "gix-fs", "gix-glob", "gix-hash", @@ -2734,8 +2747,11 @@ dependencies = [ "gix-index", "gix-object", "gix-odb", + "gix-parallel", "gix-path", "gix-testtools", + "gix-trace", + "gix-utils", "gix-validate", "gix-worktree", "insta", @@ -2748,15 +2764,17 @@ version = "0.35.0" dependencies = [ "bstr", "gix-error", - "gix-features", "gix-filter", "gix-fs", "gix-hash", "gix-index", "gix-object", "gix-odb", + "gix-parallel", "gix-path", "gix-testtools", + "gix-trace", + "gix-utils", "gix-worktree", "gix-worktree-state", "insta", @@ -2770,7 +2788,6 @@ version = "0.37.0" dependencies = [ "gix-attributes", "gix-error", - "gix-features", "gix-filter", "gix-fs", "gix-hash", @@ -2778,7 +2795,9 @@ dependencies = [ "gix-odb", "gix-path", "gix-testtools", + "gix-trace", "gix-traverse", + "gix-utils", "gix-worktree", "gix-worktree-stream", "insta", diff --git a/Cargo.toml b/Cargo.toml index f32a05543f5..3bcfc874a4d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,7 @@ test = false doctest = false [features] -default = ["max", "auto-chain-error"] +default = ["max"] #! ### Build Configuration #! These combine common choices of building blocks to represent typical builds. @@ -84,14 +84,12 @@ lean-async = ["hashes", "fast", "tracing", "pretty-cli", "tix", "gitoxide-core-t #! ### Package Maintainers #! `*-control` features leave it to you to configure C libraries, involving choices for HTTP transport implementation. #! -#! Additional features *can* be provided with `--features` and are handled by the [`gix-features` crate](https://docs.rs/gix-features/latest). +#! Additional features *can* be provided with `--features` and are handled by the [`gix` crate](https://docs.rs/gix/latest/gix/#feature-flags). #! Note that only one HTTP transport can be enabled at a time. See the *Building Blocks for mutually exclusive networking* headline. #! ### Building Blocks #! Typical combinations of features of our dependencies, some of which are referred to in the `gitoxide` crate's code for conditional compilation. -## Flatten error trees into chains with source locations in error reports. -auto-chain-error = ["gix/auto-chain-error"] ## Provide support for all known hashes. If this isn't enabled, select an individual hash algorithms. hashes = ["sha1", "sha256"] @@ -111,7 +109,7 @@ fast = ["gix/max-performance", "gix/comfort"] fast-safe = ["fast"] ## Enable tracing in `gitoxide-core`. -tracing = ["dep:anstream", "dep:tracing", "gitoxide-core/tracing", "gix-features/tracing", "gix-features/tracing-detail"] +tracing = ["dep:anstream", "dep:tracing", "gitoxide-core/tracing", "gix/tracing-detail"] ## Also measure user and kernel CPU time in trace trees on supported Unix platforms. ## Sampling adds overhead on span entry and exit. @@ -129,7 +127,7 @@ prodash-render-line = ["prodash/render-line", "prodash-render-line-crossterm", " ## Prints statistical information to inform about cache efficiency when those are dropped. ## Use this as a way to understand if bigger caches actually produce greater yields. -cache-efficiency-debug = ["gix-features/cache-efficiency-debug"] +cache-efficiency-debug = ["gix/cache-efficiency-debug"] ## A way to enable most `gitoxide-core` tools found in `ein tools`, namely `organize` and `estimate hours`. gitoxide-core-tools = ["gitoxide-core/organize", "gitoxide-core/estimate-hours", "gitoxide-core-tools-archive", "gitoxide-core-tools-clean"] @@ -171,8 +169,7 @@ gitoxide-core-async-client = ["gitoxide-core/async-client", "futures-lite"] [dependencies] gitoxide-core = { version = "^0.62.0", path = "gitoxide-core" } -gix-features = { version = "^0.50.0", path = "gix-features" } -gix = { version = "^0.88.0", path = "gix", default-features = false } +gix = { version = "^0.88.0", path = "gix", default-features = false, features = ["error-print-location"] } gix-tix = { version = "^0.4.0", path = "gix-tix", default-features = false, optional = true } clap = { version = "4.5.42", features = ["derive", "cargo"] } @@ -238,7 +235,7 @@ members = [ "gix-config", "gix-config-value", "gix-discover", - "gix-features", + "gix-parallel", "gix-zlib", "gix-trace", "gix-commitgraph", diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index cad48429a83..78c6113c73c 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -10,6 +10,24 @@ Run `just` to browse commands grouped by purpose, with everyday development task first. Use `just --groups` to list the groups, or filter the overview with, for example, `just --list --group 'Dependencies and SBOMs'`. +## Release source paths + +The GitHub release workflow builds `gix` and `ein` through +`etc/scripts/remap-release-paths.sh`. It appends rustc's `--remap-path-prefix` +flags for all target crates, including dependencies, using stable virtual roots: +`/gitoxide` for the checkout, `/cargo` for Cargo sources, `/rust` for the selected +Rust toolchain, and `/generated` for build output. Home, Rustup, and temporary +paths are also remapped. Native host paths, canonical aliases, Windows separator +variants, and `cross` container mounts are covered. Normal +local development builds are unchanged. + +To opt into the same policy locally, run these commands from the workspace root: + +```sh +bash etc/scripts/remap-release-paths.sh build cargo build --release --locked -p gitoxide --bins +bash etc/scripts/remap-release-paths.sh check target/release/gix target/release/ein +``` + ## Practices * **test-first development** @@ -165,8 +183,8 @@ including feature selection and matching inventories in both formats. * `blocking` can be used to make `Read` and `Iterator` async, or move any operation onto a thread which blends it into the async world. * Most operations are fast and 'interrupting' them is as easy as ignoring their result by cancelling their task. - * Long-running operations can be roughly interacted with using `gix_features::interrupt::trigger()` function, and after a moment - of waiting the flag can be unset with the `…::uninterrupt()` function to allow new long-running operations to work. + * Long-running operations can be roughly interacted with using the `gix::interrupt::trigger()` function, and after a moment + of waiting the flag can be unset with the `gix::interrupt::reset()` function to allow new long-running operations to work. Every long running operation supports this. * **server-side** * ~~Building a pack is CPU and at some point, IO bound, and it makes no sense to use async to handle more connections - git @@ -185,7 +203,7 @@ including feature selection and matching inventories in both formats. control of the name to the caller. However, call `.init(…)` to configure the iteration. * and when calling `add_child(…)` don't use the parent progress instance for anything else. * **interruption of long-running operations** - * Use `gix-features::interrupt::*` for building support for interruptions of long-running operations only. + * Use `gix_utils::interrupt::*` for building support for interruptions of long-running operations only. * It's up to the author to decide how to best integrate it, generally we use a poll-based mechanism to check whether an interrupt flag is set. * **this is a must if…** @@ -207,14 +225,14 @@ including feature selection and matching inventories in both formats. - in **plumbing**, do not use it at all but instead provide the mutable part (like caches, buffers) as arguments, pushing their handling entirely to the caller. - Set on top an optional abstraction that manages the above for you using **interior mutability only if part of the mutable state has to be returned as borrow** or if otherwise it wouldn't be possible to borrowcheck. Or in other words: start without interior mutability and try to do it the standard way, but switch when needed. - - When using primitives to support interior mutability, use the provided ones and utility functions in `gix_features::threading::*` exclusively to allow switching between + - When using primitives to support interior mutability, use the provided ones and utility functions in `gix_parallel::*` exclusively to allow switching between thread-safe and none-threadsafe versions at compile time. - The preferred way of using it is to start out as upgradable reader, and upgrading to write if needed, keeping contention to a minimum. - If _shared ownership_ is involved, one always needs _interior mutability_, but may still decide to use an API that requires `&mut self` if locally stored caches are involved. - - Types that are not thread-local must be `Sync`, but only if the `gix-features/parallel` is enabled due to the usage of `gix_features::threading::…` primitives which won't + - Types that are not thread-local must be `Sync`, but only if the `gix-parallel/parallel` is enabled due to the usage of `gix_parallel::…` primitives which won't be thread-safe without the feature. * **when to use shared ownership** - - Use `gix_features::threading::OwnShared` particularly when shared resources supposed to be used by thread-local handles. Going through a wrapper for shared ownership is fast + - Use `gix_parallel::OwnShared` particularly when shared resources supposed to be used by thread-local handles. Going through a wrapper for shared ownership is fast and won't be the bottleneck, as it's only about 16% slower than going through a shared reference on a single core. * **Path encoding** - For `git`, paths are just bytes no matter on which platform. We assume that on windows its path handling goes through some abstraction layer like `MSYS2` diff --git a/README.md b/README.md index 5677085b12a..b95c8e93c9d 100644 --- a/README.md +++ b/README.md @@ -110,7 +110,7 @@ is usable to some extent. * [gix-commitgraph](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-commitgraph) * [gix-diff](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-diff) * [gix-traverse](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-traverse) - * [gix-features](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-features) + * [gix-parallel](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-parallel) * [gix-credentials](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-credentials) * [gix-sec](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-sec) * [gix-quote](https://github.com/GitoxideLabs/gitoxide/blob/main/crate-status.md#gix-quote) @@ -342,7 +342,7 @@ Project non-goals can change over time as we learn more, and they can be challen * **use async IO everywhere** * for the most part, git operations are heavily reliant on memory mapped IO as well as CPU to decompress data, which doesn't lend itself well to async IO out of the box. - * Use `blocking` as well as `gix-features::interrupt` to bring operations into the async world and to control + * Use `blocking` as well as `gix_utils::interrupt` to bring operations into the async world and to control long running operations. * When connecting or streaming over TCP connections, especially when receiving on the server, async seems like a must though, but behind a feature flag. diff --git a/STABILITY.md b/STABILITY.md index 503fc073d17..ae56ed8cf39 100644 --- a/STABILITY.md +++ b/STABILITY.md @@ -64,7 +64,7 @@ The following schematic helps to visualize what follows. ║ │ │ gix-object │ │ gix-lock │ │ ║ ║ │ └─────────────┘ └─────────────┘ │ ║ │ ║ │ ┌───────────────────────────────┐ │ ║ - ║ │ │ gix-features │ │ ║ │ + ║ │ │ gix-parallel │ │ ║ │ ║ │ └───────────────────────────────┘ │ ║ ║ └───────────────────────────────────┘ ║ │ ║ ║ diff --git a/crate-status.md b/crate-status.md index 6c1dd9f817c..5e3169edcba 100644 --- a/crate-status.md +++ b/crate-status.md @@ -240,10 +240,9 @@ The top-level crate that acts as hub to all functionality provided by the `gix-* * [x] hashset ### gix-utils -* **filesystem** - * [x] probe capabilities - * [x] symlink creation and removal - * [x] file snapshots +* Shared progress, interruption, decoding, iterator, and cache diagnostic utilities. +* Optional **progress**, **interrupt**, and **io-pipe** features enable their dependencies. +* Optional progress unit features select human-readable number and byte formatting. * [ ] **BString Interner with Arena-Backing and arbitrary value association** - probably based on [`internment`](https://docs.rs/internment/latest/internment/struct.Arena.html#), but needs `bumpalo` support to avoid item allocations/boxing, and avoid internal `Mutex`. (key type is pointer based). @@ -1059,9 +1058,8 @@ Provide a reftable backend for refs and reflogs as part of Git 3.0 compatibility [quarantine]: https://github.com/git/git/blob/master/Documentation/git-receive-pack.txt#L223:L223 -### gix-features -* **io-pipe** feature toggle - * a unix like pipeline for bytes +### gix-parallel +* Shared ownership and mutation primitives switch between `Rc`/`RefCell` and `Arc`/locks. * **parallel** feature toggle * _When on…_ * `in_parallel` diff --git a/etc/discovery/odb.md b/etc/discovery/odb.md index 8ce92bb1b06..d8c247f93e2 100644 --- a/etc/discovery/odb.md +++ b/etc/discovery/odb.md @@ -509,7 +509,7 @@ Please note that these are based on the following value system: the latter doesn't even scale that well due to the required dashmap to check for existing objects. In other words, currently there seems to be no actual benchmark for parallel usage. - In single-threaded operation the trait-bounds would prevent creation of packs unless they are adjusted as well, leading to `gix-pack` requiring its own feature toggle - which we really try hard to avoid, but probably can be placed on application level, which has to use that to setup gix-features accordingly, making it bearable. This + which we really try hard to avoid, but probably can be placed on application level, which has to use that to setup gix-parallel accordingly, making it bearable. This means though that we need to implement single-threaded and multi-threaded versions of everything important, like pack generation based on the count (which already has a single-threaded version). - Maybe… after some benchmarks, we can entirely drop the single-threaded version if it's not significantly faster on a single thread (without thread primiives) than the diff --git a/etc/scripts/cargo-check-all.sh b/etc/scripts/cargo-check-all.sh index 8e21336637e..0481d40d499 100755 --- a/etc/scripts/cargo-check-all.sh +++ b/etc/scripts/cargo-check-all.sh @@ -77,13 +77,21 @@ cargo check -p gix-revision --no-default-features --features sha1,describe cargo check -p gix-mailmap --features serde cargo check -p gix-url --all-features cargo check -p gix-status --all-features -cargo check -p gix-features --all-features -cargo check -p gix-features --features parallel -cargo check -p gix-features --features fs-read-dir -cargo check -p gix-features --features progress -cargo check -p gix-features --features io-pipe -cargo check -p gix-features --features crc32 -cargo check -p gix-features --features cache-efficiency-debug +cargo check -p gix-parallel --no-default-features +cargo check -p gix-parallel --all-features +cargo check -p gix-parallel --features parallel +cargo check -p gix-parallel --features once_cell +cargo check -p gix-utils --no-default-features +cargo check -p gix-utils --all-features +cargo check -p gix-utils --features progress +cargo check -p gix-utils --features progress,progress-unit-bytes +cargo check -p gix-utils --features progress,progress-unit-human-numbers +cargo check -p gix-utils --features interrupt +cargo check -p gix-utils --features io-pipe +cargo check -p gix-utils --features cache-efficiency-debug +cargo check -p gix-fs --no-default-features +cargo check -p gix-fs --features walkdir +cargo check -p gix-fs --all-features cargo check -p gix-commitgraph --all-features cargo check -p gix-config-value --all-features cargo check -p gix-config --all-features diff --git a/etc/scripts/check-package-size.sh b/etc/scripts/check-package-size.sh index 3a05c237c0e..84bb9cbc38b 100755 --- a/etc/scripts/check-package-size.sh +++ b/etc/scripts/check-package-size.sh @@ -19,7 +19,7 @@ echo 'in root: gitoxide CLI' (enter gix-actor && indent cargo diet -n --package-size-limit 10KB) (enter gix-archive && indent cargo diet -n --package-size-limit 10KB) (enter gix-worktree-stream && indent cargo diet -n --package-size-limit 40KB) -(enter gix-utils && indent cargo diet -n --package-size-limit 10KB) +(enter gix-utils && indent cargo diet -n --package-size-limit 20KB) (enter gix-fs && indent cargo diet -n --package-size-limit 15KB) (enter gix-pathspec && indent cargo diet -n --package-size-limit 30KB) (enter gix-refspec && indent cargo diet -n --package-size-limit 30KB) @@ -38,7 +38,7 @@ echo 'in root: gitoxide CLI' (enter gix-command && indent cargo diet -n --package-size-limit 10KB) (enter gix-hash && indent cargo diet -n --package-size-limit 30KB) (enter gix-chunk && indent cargo diet -n --package-size-limit 15KB) -(enter gix-features && indent cargo diet -n --package-size-limit 65KB) +(enter gix-parallel && indent cargo diet -n --package-size-limit 65KB) (enter gix-ref && indent cargo diet -n --package-size-limit 55KB) (enter gix-diff && indent cargo diet -n --package-size-limit 35KB) (enter gix-traverse && indent cargo diet -n --package-size-limit 15KB) diff --git a/etc/scripts/remap-release-paths.sh b/etc/scripts/remap-release-paths.sh new file mode 100755 index 00000000000..b01ed5f499b --- /dev/null +++ b/etc/scripts/remap-release-paths.sh @@ -0,0 +1,130 @@ +#!/usr/bin/env bash + +# Remap local Rust source paths to stable virtual roots for release builds. +# Run from the workspace root; requires Bash, cargo, rustc, and jq: +# bash etc/scripts/remap-release-paths.sh build cargo build --release --locked --bins +# bash etc/scripts/remap-release-paths.sh check target/release/gix target/release/ein +# `build` also accepts cross; `check` returns nonzero for known local path leaks +# or scan errors (warning-only in release CI). This is best effort; see DEVELOPMENT.md. + +set -euo pipefail + +release_path_aliases() { + local path="$1" destination="$2" canonical + if command -v cygpath >/dev/null 2>&1; then + path="$(cygpath -am -- "$path")" || return + fi + printf '%s\t%s\n' "$path" "$destination" + printf '%s\t%s\n' "${path//\//\\}" "$destination" + if [[ -d "$path" ]]; then + canonical="$(cd -- "$path" && pwd -P)" || return + if command -v cygpath >/dev/null 2>&1; then + canonical="$(cygpath -am -- "$canonical")" || return + fi + printf '%s\t%s\n' "$canonical" "$destination" + printf '%s\t%s\n' "${canonical//\//\\}" "$destination" + fi +} + +release_local_roots() { + local home="${USERPROFILE:-$HOME}" target_dir sysroot + target_dir="$(cargo metadata --locked --offline --no-deps --format-version=1 | jq -br .target_directory)" || return + sysroot="$(rustc --print sysroot)" || return + sysroot="${sysroot//$'\r'/}" + release_path_aliases "$home" /build-home || return + release_path_aliases "${RUSTUP_HOME:-$home/.rustup}" /rustup || return + release_path_aliases "${CARGO_HOME:-$home/.cargo}" /cargo || return + release_path_aliases "$PWD" /gitoxide || return + if [[ -n "${GITHUB_WORKSPACE:-}" ]]; then + release_path_aliases "$GITHUB_WORKSPACE" /gitoxide || return + fi + release_path_aliases "$sysroot" /rust || return + release_path_aliases "$target_dir" /generated +} + +release_remap_flags() { + local roots="$1" source destination + { + if [[ -n "$roots" ]]; then + printf '%s\n' "$roots" + fi + release_path_aliases "${TMPDIR:-${TEMP:-/tmp}}" /build-tmp || return + # cross 0.2.5 container mounts; newer cross versions may retain host paths. + printf '%s\n' $'/project\t/gitoxide' $'/cargo\t/cargo' $'/rust\t/rust' $'/target\t/generated' $'target\t/generated' + } | awk -F '\t' '{ print length($1) "\t" $0 }' | sort -n | cut -f2- | + while IFS=$'\t' read -r source destination; do + # rustc uses the last matching rule, so nested roots must follow parents. + printf '%s\n' "--remap-path-prefix=$source=$destination" + done +} + +release_build() { + local roots="$1" flags flag remaps + shift + if [[ "${CARGO_ENCODED_RUSTFLAGS+x}" ]]; then + flags="$CARGO_ENCODED_RUSTFLAGS" + else + # Cargo splits RUSTFLAGS on whitespace; shell quotes are not interpreted. + flags="$(printf '%s' "${RUSTFLAGS:-}" | awk '{ for (i = 1; i <= NF; i++) { printf "%s%s", sep, $i; sep = "\037" } }')" || return + fi + remaps="$(release_remap_flags "$roots")" || return + while IFS= read -r flag; do + if [[ -n "$flags" ]]; then + flags+=$'\x1f' + fi + flags+="$flag" + done <<< "$remaps" + # Keep the parent environment unchanged, including flags used to install cross. + # Git Bash must not rewrite virtual roots when launching native Cargo on Windows. + MSYS2_ENV_CONV_EXCL="${MSYS2_ENV_CONV_EXCL:+$MSYS2_ENV_CONV_EXCL;}CARGO_ENCODED_RUSTFLAGS" \ + CARGO_ENCODED_RUSTFLAGS="$flags" "$@" +} + +release_check() { + local roots="$1" binary source destination prefix grep_status status=0 + shift + for binary in "$@"; do + [[ -f "$binary" ]] || { printf 'missing release binary: %s\n' "$binary" >&2; return 1; } + while IFS=$'\t' read -r source destination; do + prefix="${source%/}/" + if [[ "$source" == *\\* ]]; then + prefix="${source%\\}\\" + fi + if LC_ALL=C grep -aFq -- "$prefix" "$binary"; then + printf '%s: embedded local build path prefix: %s\n' "$binary" "$prefix" >&2 + status=1 + else + grep_status=$? + if [[ "$grep_status" -ne 1 ]]; then + printf '%s: could not scan release binary\n' "$binary" >&2 + return "$grep_status" + fi + fi + done < <( + printf '%s\n' "$roots" + printf '%s\n' $'/project\t/gitoxide' + # `/target/` alone is also a legitimate Git ignore-pattern literal in ein. + printf '%s\t/generated\n' "/target/${TARGET:-release-github}" /target/debug /target/release /target/release-github /target/build + ) + done + return "$status" +} + +release_paths_main() { + local operation="${1:-}" roots + if [[ "$#" -lt 2 ]] || [[ "$operation" != build && "$operation" != check ]]; then + printf 'usage: %s build COMMAND [ARGS...] | check BINARY...\n' "$0" >&2 + return 2 + fi + shift + roots="$(release_local_roots)" || return + if [[ "$operation" == build ]]; then + release_build "$roots" "$@" + else + release_check "$roots" "$@" + fi +} + +if [[ "${BASH_SOURCE[0]}" == "$0" ]]; then + release_paths_main "$@" +fi diff --git a/gitoxide-core/Cargo.toml b/gitoxide-core/Cargo.toml index 808264e26a6..cac8ed26ef2 100644 --- a/gitoxide-core/Cargo.toml +++ b/gitoxide-core/Cargo.toml @@ -105,8 +105,12 @@ open = "5.3.5" document-features = { version = "0.2.0", optional = true } [dev-dependencies] +# Exercise optional functionality in tests without enabling it for dependents. +# Networking remains opt-in as its async and blocking implementations are mutually exclusive. +gitoxide-core = { path = ".", features = ["organize", "estimate-hours", "query", "corpus", "archive", "clean", "tracing", "tracing-cpu-time", "serde"] } gix-testtools = { path = "../tests/tools", default-features = false } gix = { path = "../gix", default-features = false, features = ["sha1", "parallel"] } +insta = "1.46.3" [package.metadata.docs.rs] features = ["document-features", "blocking-client", "organize", "estimate-hours", "serde", "gix/sha1", "gix/parallel"] diff --git a/gitoxide-core/src/corpus/engine.rs b/gitoxide-core/src/corpus/engine.rs index be60a08affa..8068b24f957 100644 --- a/gitoxide-core/src/corpus/engine.rs +++ b/gitoxide-core/src/corpus/engine.rs @@ -154,9 +154,8 @@ impl Engine { } else { let counter = repo_progress.counter(); let num_errors = AtomicUsize::default(); - let repo_progress = gix::threading::OwnShared::new(gix::threading::Mutable::new( - repo_progress.add_child("in parallel"), - )); + let repo_progress = + gix::parallel::OwnShared::new(gix::parallel::Mutable::new(repo_progress.add_child("in parallel"))); gix::parallel::in_parallel_with_slice( &mut repos, Some(threads), @@ -164,7 +163,7 @@ impl Engine { let shared_repo_progress = repo_progress.clone(); let db_path = db_path.clone(); move |tid| { - let mut progress = gix::threading::lock(&shared_repo_progress); + let mut progress = gix::parallel::lock(&shared_repo_progress); let lane_progress = progress.add_child(format!("{tid}")); let guard = tracing::dispatcher::set_default(subscriber); (guard, lane_progress, rusqlite::Connection::open(&db_path)) @@ -201,7 +200,7 @@ impl Engine { || (!gix::interrupt::is_triggered()).then(|| Duration::from_millis(100)), drop, )?; - let repo_progress = gix::threading::lock(&repo_progress); + let repo_progress = gix::parallel::lock(&repo_progress); repo_progress.show_throughput(task_start); let num_errors = num_errors.load(Ordering::Relaxed); if num_errors != 0 { diff --git a/gitoxide-core/src/corpus/run.rs b/gitoxide-core/src/corpus/run.rs index 2c8db5c577a..5cdd89dba0d 100644 --- a/gitoxide-core/src/corpus/run.rs +++ b/gitoxide-core/src/corpus/run.rs @@ -86,8 +86,8 @@ impl Execute for WorktreeStream { progress.init(None, gix::progress::bytes()); std::io::copy( &mut stream.into_read(), - &mut gix::features::interrupt::Write { - inner: gix::features::progress::Write { + &mut gix::utils::interrupt::Write { + inner: gix::progress::Write { inner: std::io::sink(), progress, }, diff --git a/gitoxide-core/src/hours/mod.rs b/gitoxide-core/src/hours/mod.rs index 3ecdf3dc69f..3c3a1cee585 100644 --- a/gitoxide-core/src/hours/mod.rs +++ b/gitoxide-core/src/hours/mod.rs @@ -122,7 +122,7 @@ where let commit_id = repo.rev_parse_single(rev_spec)?.detach(); let mut string_heap = BTreeSet::<&'static [u8]>::new(); let needs_stats = file_stats || line_stats; - let threads = gix::features::parallel::num_threads(threads); + let threads = gix::parallel::num_threads(threads); let (commit_authors, stats, is_shallow, skipped_merge_commits, num_commits) = { std::thread::scope(|scope| -> Result<_> { diff --git a/gitoxide-core/src/lib.rs b/gitoxide-core/src/lib.rs index 961eb854a79..4495c5c5a60 100644 --- a/gitoxide-core/src/lib.rs +++ b/gitoxide-core/src/lib.rs @@ -88,8 +88,6 @@ pub mod repository; #[cfg(feature = "tracing")] pub mod trace; -mod output; - mod discover; pub use discover::discover; diff --git a/gitoxide-core/src/organize.rs b/gitoxide-core/src/organize.rs index dade6d7408e..73bf4fe07a0 100644 --- a/gitoxide-core/src/organize.rs +++ b/gitoxide-core/src/organize.rs @@ -8,7 +8,7 @@ use std::{ use gix::{ NestedProgress, Progress, Result, - error::{ResultExt, bail}, + error::{Exn, ResultExt, bail, message}, objs::bstr::ByteSlice, progress, }; @@ -145,6 +145,8 @@ fn find_origin_remote(repo: &Path) -> Result> { .transpose() } +/// Confinement validation errors record `destination` (the resolved path) and `destination_root` +/// (the canonicalized allowed root) as path metadata. fn handle( mode: Mode, kind: gix::repository::Kind, @@ -241,6 +243,18 @@ fn handle( } })); + // Unlike canonicalize(), realpath also resolves existing symlinks when the destination leaf + // does not exist yet. Check confinement before creating directories or moving the repository. + let destination = gix::path::realpath(destination)?; + if !destination.starts_with(canonicalized_destination) || destination == canonicalized_destination { + bail!( + "the origin URL resolves outside the repository destination" + .validation() + .with("destination", destination.as_path()) + .with("destination_root", canonicalized_destination) + ); + } + match destination.canonicalize() { Ok(destination) if git_workdir.canonicalize().or_error()? == destination => return Ok(()), _ => {} @@ -294,6 +308,10 @@ pub fn discover( Ok(()) } +/// Relocate repositories under `source_dir` to paths derived from their origin URLs below `destination`. +/// +/// Errors retain per-repository causes, including validation classifications and the `destination` +/// and `destination_root` path metadata for origin URLs that resolve outside the allowed root. pub fn run( mode: Mode, source_dir: impl AsRef, @@ -301,7 +319,7 @@ pub fn run( mut progress: P, threads: Option, ) -> Result<()> { - let mut num_errors = 0usize; + let mut errors = Vec::new(); let destination = destination.as_ref().canonicalize().or_error()?; let mut repositories = find_git_repository_workdirs(source_dir, progress.add_child("Searching repositories"), false, threads) @@ -314,12 +332,14 @@ pub fn run( path_to_move.display(), err )); - num_errors += 1; + errors.push(err); } } - if num_errors > 0 { - bail!("Failed to handle {num_errors} repositories") + if !errors.is_empty() { + let num_errors = errors.len(); + let repositories = if num_errors == 1 { "repository" } else { "repositories" }; + Err(Exn::raise_all(errors, message!("Failed to handle {num_errors} {repositories}")).into()) } else { Ok(()) } diff --git a/gitoxide-core/src/output.rs b/gitoxide-core/src/output.rs deleted file mode 100644 index c2ed0b2a6b4..00000000000 --- a/gitoxide-core/src/output.rs +++ /dev/null @@ -1,56 +0,0 @@ -use std::io::Write; - -use gix::{bstr::BStr, utils::AsBStr}; - -/// Write arbitrary bytes losslessly, quoting the entire value with its debug representation if it contains -/// control characters, quotes, backslashes, or invalid UTF-8 that must be escaped. -pub(crate) fn write_bstr(mut out: impl Write, input: &BStr, scratch: &mut Vec) -> std::io::Result<()> { - scratch.clear(); - write!(scratch, "{input:?}").expect("writing to a byte buffer cannot fail"); - let debug_matches_input = scratch - .strip_prefix(b"\"") - .and_then(|debug| debug.strip_suffix(b"\"")) - .is_some_and(|debug| debug == input); - let input = if debug_matches_input { input } else { scratch.as_bstr() }; - out.write_all(input) -} - -#[cfg(test)] -mod tests { - use gix::bstr::ByteSlice; - - use super::write_bstr; - - fn render(input: &[u8], scratch: &mut Vec) -> Vec { - let mut out = Vec::new(); - write_bstr(&mut out, input.as_bstr(), scratch).expect("in-memory writes succeed"); - out - } - - #[test] - fn safe_ascii_and_unicode_remain_unchanged() { - let mut scratch = Vec::new(); - for input in [b"hello world".as_slice(), "hello 💡".as_bytes()] { - assert_eq!( - render(input, &mut scratch), - input, - "safe text should remain easy to read" - ); - } - } - - #[test] - fn terminal_controls_and_ambiguous_bytes_are_quoted_losslessly() { - let mut scratch = Vec::new(); - assert_eq!( - render(b"control-\0\x08\t\n\r\x1b\x7f\"\\end", &mut scratch), - br#""control-\0\x08\t\n\r\x1b\x7f\"\\end""#, - "terminal-active and syntax bytes must be escaped" - ); - assert_eq!( - render(b"invalid-\xff", &mut scratch), - br#""invalid-\xff""#, - "invalid UTF-8 must remain recoverable" - ); - } -} diff --git a/gitoxide-core/src/pack/create.rs b/gitoxide-core/src/pack/create.rs index c1d415c18bc..f904444a6b9 100644 --- a/gitoxide-core/src/pack/create.rs +++ b/gitoxide-core/src/pack/create.rs @@ -242,7 +242,7 @@ where version: Default::default(), compression: pack_compression, }, - )) + )?) }; let mut entries_progress = progress.add_child("consuming"); diff --git a/gitoxide-core/src/pack/multi_index.rs b/gitoxide-core/src/pack/multi_index.rs index 4177a7a2da0..ac0c05e22d8 100644 --- a/gitoxide-core/src/pack/multi_index.rs +++ b/gitoxide-core/src/pack/multi_index.rs @@ -89,6 +89,7 @@ pub fn entries(multi_index_path: PathBuf, format: OutputFormat, mut out: impl st } let file = gix::odb::pack::multi_index::File::at(multi_index_path, None)?; for entry in file.iter() { + let entry = entry?; writeln!(out, "{} {} {}", entry.oid, entry.pack_index, entry.pack_offset).or_error()?; } Ok(()) diff --git a/gitoxide-core/src/pack/receive.rs b/gitoxide-core/src/pack/receive.rs index ab8d73e15e8..5ed7dbf4f79 100644 --- a/gitoxide-core/src/pack/receive.rs +++ b/gitoxide-core/src/pack/receive.rs @@ -255,7 +255,8 @@ fn write_raw_refs(refs: &[Ref], directory: PathBuf) -> std::io::Result<()> { let assure_dir_exists = |path: &BString| { gix::validate::reference::name(path.as_bstr()) .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; - let path = directory.join(gix::path::from_byte_slice(path)); + let path = directory + .join(gix::path::from_byte_slice(path).map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?); std::fs::create_dir_all(path.parent().expect("multi-component path")).map(|_| path) }; for r in refs { diff --git a/gitoxide-core/src/query/engine/command.rs b/gitoxide-core/src/query/engine/command.rs index b2afb34424d..dfbbdf524d3 100644 --- a/gitoxide-core/src/query/engine/command.rs +++ b/gitoxide-core/src/query/engine/command.rs @@ -25,9 +25,9 @@ impl query::Engine { let is_excluded = spec.is_excluded(); let relpath = if spec.signature.contains(gix::pathspec::MagicSignature::TOP) { let root = self.repo.workdir().unwrap_or_else(|| self.repo.git_dir()); - let path = root.join(gix::path::from_bstr(spec.path()).as_ref()); + let path = root.join(gix::path::from_bstr(spec.path())?.as_ref()); self.repo - .normalize_path(gix::path::into_bstr(path).as_ref())? + .normalize_path(gix::path::into_bstr(path)?.as_ref())? .into_owned() } else { self.repo.normalize_path(spec.path())?.into_owned() diff --git a/gitoxide-core/src/query/engine/update.rs b/gitoxide-core/src/query/engine/update.rs index 2aa475fd1e3..4d25a99328a 100644 --- a/gitoxide-core/src/query/engine/update.rs +++ b/gitoxide-core/src/query/engine/update.rs @@ -10,9 +10,9 @@ use gix::{ bstr::{BStr, BString, ByteSlice}, diff::{blob::platform::prepare_diff::Operation, rewrites::CopySource}, error::{ResultExt, bail}, - features::progress, parallel::{InOrderIter, SequenceId}, prelude::ObjectIdExt, + progress, }; use rusqlite::{Statement, Transaction, params}; @@ -30,7 +30,7 @@ pub fn update( }: Options, ) -> Result> { let commit_id = repo.head_id()?.detach(); - let threads = gix::features::parallel::num_threads(threads); + let threads = gix::parallel::num_threads(threads); let mut stat_progress = { let mut p = progress.add_child("extract stats"); diff --git a/gitoxide-core/src/repository/attributes/query.rs b/gitoxide-core/src/repository/attributes/query.rs index 945a745d676..862857d5de7 100644 --- a/gitoxide-core/src/repository/attributes/query.rs +++ b/gitoxide-core/src/repository/attributes/query.rs @@ -41,7 +41,7 @@ pub(crate) mod function { match input { PathsOrPatterns::Paths(paths) => { for path in paths { - let mode = gix::path::from_bstr(Cow::Borrowed(path.as_ref())) + let mode = gix::path::from_bstr(Cow::Borrowed(path.as_ref()))? .metadata() .ok() .map(|m| is_dir_to_mode(m.is_dir())); @@ -88,11 +88,14 @@ pub(crate) mod function { let workdir = repo.workdir(); for pattern in pathspec.search().patterns() { let path = pattern.path(); + let is_dir = match workdir { + Some(wd) => wd.join(gix::path::from_bstr(path)?).is_dir(), + None => false, + }; let entry = cache.at_entry( path, Some(is_dir_to_mode( - workdir.is_some_and(|wd| wd.join(gix::path::from_bstr(path)).is_dir()) - || pattern.signature.contains(gix::pathspec::MagicSignature::MUST_BE_DIR), + is_dir || pattern.signature.contains(gix::pathspec::MagicSignature::MUST_BE_DIR), )), )?; if !entry.matching_attributes(&mut matches) { diff --git a/gitoxide-core/src/repository/attributes/validate_baseline.rs b/gitoxide-core/src/repository/attributes/validate_baseline.rs index 114b500396b..dbfd197e939 100644 --- a/gitoxide-core/src/repository/attributes/validate_baseline.rs +++ b/gitoxide-core/src/repository/attributes/validate_baseline.rs @@ -79,7 +79,7 @@ pub(crate) mod function { let mut progress = progress.add_child("attributes"); move || -> Result<()> { let mut child = - std::process::Command::from(gix::command::prepare(gix::path::env::exe_invocation())) + std::process::Command::try_from(gix::command::prepare(gix::path::env::exe_invocation()))? .args(["check-attr", "--stdin", "-a"]) .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) @@ -132,7 +132,7 @@ pub(crate) mod function { let mut progress = progress.add_child("excludes"); move || -> Result<()> { let mut child = - std::process::Command::from(gix::command::prepare(gix::path::env::exe_invocation())) + std::process::Command::try_from(gix::command::prepare(gix::path::env::exe_invocation()))? .args(["check-ignore", "--stdin", "-nv", "--no-index"]) .stdin(std::process::Stdio::piped()) .stdout(std::process::Stdio::piped()) diff --git a/gitoxide-core/src/repository/blame.rs b/gitoxide-core/src/repository/blame.rs index 178829b4c86..ce9c2797619 100644 --- a/gitoxide-core/src/repository/blame.rs +++ b/gitoxide-core/src/repository/blame.rs @@ -65,7 +65,7 @@ fn start_for_blame<'a>( let Some(workdir) = repo.workdir() else { return Ok(Start::Commit(first_suspect)); }; - let path = workdir.join(gix::path::from_bstr(file)); + let path = workdir.join(gix::path::from_bstr(file)?); let metadata = match std::fs::symlink_metadata(&path) { Ok(metadata) => metadata, Err(err) if gix::fs::io_err::is_not_found(err.kind(), err.raw_os_error()) => { diff --git a/gitoxide-core/src/repository/clean.rs b/gitoxide-core/src/repository/clean.rs index fee7b59ace8..e1cf72891be 100644 --- a/gitoxide-core/src/repository/clean.rs +++ b/gitoxide-core/src/repository/clean.rs @@ -167,7 +167,7 @@ pub(crate) mod function { }; if entry.disk_kind.is_none() { entry.disk_kind = workdir - .join(gix::path::from_bstr(entry.rela_path.as_bstr())) + .join(gix::path::from_bstr(entry.rela_path.as_bstr())?) .symlink_metadata() .ok() .map(|e| e.file_type().into()); @@ -186,7 +186,7 @@ pub(crate) mod function { } if disk_kind == gix::dir::entry::Kind::Directory - && gix::discover::is_git(&workdir.join(gix::path::from_bstr(entry.rela_path.as_bstr()))).is_ok() + && gix::discover::is_git(&workdir.join(gix::path::from_bstr(entry.rela_path.as_bstr())?)).is_ok() { if debug { writeln!( @@ -228,7 +228,7 @@ pub(crate) mod function { } let is_ignored = matches!(entry.status, gix::dir::entry::Status::Ignored(_)); - let entry_path = gix::path::from_bstr(entry.rela_path); + let entry_path = gix::path::from_bstr(entry.rela_path)?; let display_path = gix::path::relativize_with_prefix(&entry_path, prefix); if disk_kind == gix::dir::entry::Kind::Directory { saw_ignored_directory |= is_ignored; diff --git a/gitoxide-core/src/repository/clone.rs b/gitoxide-core/src/repository/clone.rs index 2087cea9590..d7f158bc732 100644 --- a/gitoxide-core/src/repository/clone.rs +++ b/gitoxide-core/src/repository/clone.rs @@ -53,7 +53,7 @@ pub(crate) mod function { let url: gix::Url = url.as_ref().try_into()?; let directory = directory.map_or_else( || { - let path = gix::path::from_bstr(Cow::Borrowed(url.path.as_ref())); + let path = gix::path::from_bstr(Cow::Borrowed(url.path.as_ref()))?; if !bare && path.extension() == Some(OsStr::new("git")) { path.file_stem().map(Into::into) } else { diff --git a/gitoxide-core/src/repository/diff.rs b/gitoxide-core/src/repository/diff.rs index ceae29ad963..57db038e83e 100644 --- a/gitoxide-core/src/repository/diff.rs +++ b/gitoxide-core/src/repository/diff.rs @@ -130,9 +130,7 @@ fn resolve_revspec( let not_found = err.downcast_any_ref::(); if let Some(gix::refs::file::find::NotFound { name }) = not_found { let root = repo.workdir().map(ToOwned::to_owned); - let name = gix::path::os_string_into_bstring(name.into())?; - - Ok((ObjectId::null(gix::hash::Kind::Sha1), root, name)) + Ok((ObjectId::null(gix::hash::Kind::Sha1), root, name.clone())) } else { Err(err) } diff --git a/gitoxide-core/src/repository/editor.rs b/gitoxide-core/src/repository/editor.rs index 6307db22ff6..dec2541a534 100644 --- a/gitoxide-core/src/repository/editor.rs +++ b/gitoxide-core/src/repository/editor.rs @@ -10,7 +10,7 @@ pub fn function(repo: gix::Repository, paths: Vec) -> Result<()> { .or_raise(|| message("Could not prepare editor"))? .ok_or_raise(|| unsupported("No editor is configured and the terminal is not capable of running one"))?; let editor_display = editor.command.to_string_lossy().into_owned(); - let mut command: std::process::Command = editor.args(paths).into(); + let mut command: std::process::Command = editor.args(paths).try_into()?; // Program metadata already names a directly launched editor. For shell commands, it only names // the shell, so retain the configured editor command in the message instead of losing that detail. let editor_display = if editor_display == command.get_program().to_string_lossy() { diff --git a/gitoxide-core/src/repository/exclude.rs b/gitoxide-core/src/repository/exclude.rs index 8fc6b0ad024..29995961b80 100644 --- a/gitoxide-core/src/repository/exclude.rs +++ b/gitoxide-core/src/repository/exclude.rs @@ -52,7 +52,7 @@ pub fn query( PathsOrPatterns::Patterns(paths) => Box::new(paths.into_iter()), }; for path in paths { - let mode = gix::path::from_bstr(Cow::Borrowed(path.as_ref())) + let mode = gix::path::from_bstr(Cow::Borrowed(path.as_ref()))? .metadata() .ok() .map(|m| is_dir_to_mode(m.is_dir())) diff --git a/gitoxide-core/src/repository/index/entries.rs b/gitoxide-core/src/repository/index/entries.rs index 59ed5103d16..9ebaf7de797 100644 --- a/gitoxide-core/src/repository/index/entries.rs +++ b/gitoxide-core/src/repository/index/entries.rs @@ -383,7 +383,7 @@ pub(crate) mod function { path: &BStr, buf: &mut Vec, ) -> std::io::Result<()> { - crate::output::write_bstr(&mut *out, path, buf)?; + out.write_all(gix::quote::for_display(path, buf))?; match attrs { Some(attrs) => out.write_all(print_attrs(Some(attrs), entry.mode).as_bytes()), None => Ok(()), @@ -415,7 +415,7 @@ pub(crate) mod function { entry.mode, entry.id, )?; - crate::output::write_bstr(&mut *out, path, buf)?; + out.write_all(gix::quote::for_display(path, buf))?; out.write_all(print_attrs(attrs, entry.mode).as_bytes())?; out.write_all(b"\n") } diff --git a/gitoxide-core/src/repository/index/mod.rs b/gitoxide-core/src/repository/index/mod.rs index f8264f92791..f60267df25b 100644 --- a/gitoxide-core/src/repository/index/mod.rs +++ b/gitoxide-core/src/repository/index/mod.rs @@ -61,7 +61,7 @@ pub fn from_list( .validation(); bail!(err) } - let path = gix::path::into_bstr(path); + let path = gix::path::into_bstr(path)?; index.dangerously_push_entry( gix::index::entry::Stat::default(), gix::hash::ObjectId::empty_blob(object_hash), diff --git a/gitoxide-core/src/repository/odb.rs b/gitoxide-core/src/repository/odb.rs index e54f44db65e..1452cc8a515 100644 --- a/gitoxide-core/src/repository/odb.rs +++ b/gitoxide-core/src/repository/odb.rs @@ -174,7 +174,7 @@ pub fn statistics( let mut stats = if gix::parallel::num_threads(thread_limit) > 1 { gix::parallel::in_parallel( gix::interrupt::Iter::new( - gix::features::iter::Chunks { + gix::utils::iter::Chunks { inner: object_ids, size: chunk_size, }, diff --git a/gitoxide-core/src/repository/status.rs b/gitoxide-core/src/repository/status.rs index 221b8310e50..f786fe7a406 100644 --- a/gitoxide-core/src/repository/status.rs +++ b/gitoxide-core/src/repository/status.rs @@ -1,4 +1,4 @@ -use std::path::Path; +use std::{io, path::Path}; use gix::{ Result, @@ -133,7 +133,7 @@ pub fn show( gix::diff::index::Change::Rewrite { ref source_location, .. } => { - let source_location = gix::path::from_bstr(source_location.as_ref()); + let source_location = gix::path::from_bstr(source_location.as_ref())?; let source_location = gix::path::relativize_with_prefix(&source_location, prefix); writeln!( out, @@ -141,7 +141,7 @@ pub fn show( status = "R", source_rela_path = source_location.display(), dest_rela_path = - gix::path::relativize_with_prefix(&gix::path::from_bstr(location), prefix).display(), + gix::path::relativize_with_prefix(&gix::path::from_bstr(location)?, prefix).display(), ) .or_error()?; continue; @@ -150,7 +150,7 @@ pub fn show( writeln!( out, "{status: >2} {rela_path}", - rela_path = gix::path::relativize_with_prefix(&gix::path::from_bstr(location), prefix).display(), + rela_path = gix::path::relativize_with_prefix(&gix::path::from_bstr(location)?, prefix).display(), ) .or_error()?; } @@ -169,8 +169,8 @@ pub fn show( out, "{status: >3} {rela_path}{slash}", status = "?", - rela_path = - gix::path::relativize_with_prefix(&gix::path::from_bstr(entry.rela_path), prefix).display(), + rela_path = gix::path::relativize_with_prefix(&gix::path::from_bstr(entry.rela_path)?, prefix) + .display(), slash = if entry.disk_kind.unwrap_or(gix::dir::entry::Kind::File).is_dir() { "/" } else { @@ -189,9 +189,9 @@ pub fn show( "{status: >3} {source_rela_path} → {dest_rela_path}", status = "R", source_rela_path = - gix::path::relativize_with_prefix(&gix::path::from_bstr(source.rela_path()), prefix).display(), + gix::path::relativize_with_prefix(&gix::path::from_bstr(source.rela_path())?, prefix).display(), dest_rela_path = gix::path::relativize_with_prefix( - &gix::path::from_bstr(dirwalk_entry.rela_path.as_bstr()), + &gix::path::from_bstr(dirwalk_entry.rela_path.as_bstr())?, prefix ) .display(), @@ -239,7 +239,7 @@ fn print_index_entry_status( EntryStatus::IntentToAdd => "A", }; - let rela_path = gix::path::from_bstr(rela_path); + let rela_path = gix::path::from_bstr(rela_path).map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; let display_path = gix::path::relativize_with_prefix(&rela_path, prefix); writeln!(out, "{status: >3} {}", display_path.display()) } diff --git a/gitoxide-core/tests/fixtures/generated-archives/.gitignore b/gitoxide-core/tests/fixtures/generated-archives/.gitignore new file mode 100644 index 00000000000..18510b6866b --- /dev/null +++ b/gitoxide-core/tests/fixtures/generated-archives/.gitignore @@ -0,0 +1,3 @@ +# These small fixtures can be regenerated; no archived output is needed. +*.tar +*.tar.xz diff --git a/gitoxide-core/tests/fixtures/organize.sh b/gitoxide-core/tests/fixtures/organize.sh new file mode 100755 index 00000000000..f7581e9e5d5 --- /dev/null +++ b/gitoxide-core/tests/fixtures/organize.sh @@ -0,0 +1,10 @@ +#!/usr/bin/env bash +set -eu -o pipefail + +# A standalone worktree keeps organization tests independent of the source checkout. +# HEAD and config suffice for discovery; the payload verifies that the worktree moves intact. +# Tests override origin only in their disposable copies to exercise path confinement. +git init -q source +git -C source config remote.origin.url https://example.com/owner/repository.git +printf '%s' 'repository contents' > source/payload +mkdir destination diff --git a/gitoxide-core/tests/organize.rs b/gitoxide-core/tests/organize.rs new file mode 100644 index 00000000000..a98119b50c0 --- /dev/null +++ b/gitoxide-core/tests/organize.rs @@ -0,0 +1,149 @@ +use gitoxide_core::organize::{Mode, run}; +use gix_testtools::TestResult; + +#[cfg(unix)] +use gix::error::MetadataValue; + +#[test] +fn url_components_cannot_move_repositories_outside_the_destination() -> TestResult { + for url in [ + "https://example.com/%2e%2e/%2e%2e/escaped", + "https://../escaped", + "ssh://example.com/../../escaped", + ] { + for mode in [Mode::Execute, Mode::Simulate] { + let fixture = gix_testtools::scripted_fixture_writable("organize.sh")?; + let source = fixture.path().join("source"); + let destination = fixture.path().join("destination"); + gix_testtools::git(&source, &format!("config remote.origin.url {url}"))?; + + let err = run(mode, &source, &destination, gix::progress::Discard, Some(1)) + .expect_err("organizing must reject a URL-derived path escape, even in simulation"); + assert!( + err.is_validation(), + "URL-derived destination escapes are validation failures: {url}" + ); + let diagnostic = gix_testtools::redact_debug_snapshot( + &err, + &[(&fixture.path().canonicalize()?.to_string_lossy(), "")], + ); + insta::allow_duplicates! { + insta::assert_debug_snapshot!( + diagnostic, + "execute and simulate preserve the confinement cause and resolved destination for every URL", + @r#" + Failed to handle 1 repository + + Caused by: + 0: the origin URL resolves outside the repository destination, destination="/escaped", destination_root="/destination" + "# + ); + } + assert_eq!( + std::fs::read_to_string(source.join("payload"))?, + "repository contents", + "a rejected URL leaves the source contents unchanged: {url}" + ); + assert!( + !fixture.path().join("escaped").exists(), + "no destination is created outside the requested root: {url}" + ); + assert_eq!( + std::fs::read_dir(&destination)?.count(), + 0, + "a rejected URL creates no intermediate destination directories: {url}" + ); + } + } + Ok(()) +} + +#[test] +fn ordinary_urls_still_move_repositories_below_the_destination() -> TestResult { + let fixture = gix_testtools::scripted_fixture_writable("organize.sh")?; + let source = fixture.path().join("source"); + let destination = fixture.path().join("destination"); + run(Mode::Execute, &source, &destination, gix::progress::Discard, Some(1))?; + + let moved = destination.join("example.com/owner/repository"); + assert_eq!( + std::fs::read_to_string(moved.join("payload"))?, + "repository contents", + "normal URL organization retains its layout and repository contents" + ); + assert!( + moved.join(".git/HEAD").is_file(), + "organization moves Git metadata along with the worktree" + ); + assert!(!source.exists(), "successful organization moves the repository"); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn an_existing_symlink_cannot_redirect_the_destination() -> TestResult { + for mode in [Mode::Execute, Mode::Simulate] { + let fixture = gix_testtools::scripted_fixture_writable("organize.sh")?; + let source = fixture.path().join("source"); + let destination = fixture.path().join("destination"); + let outside = fixture.path().join("outside"); + gix_testtools::git(&source, "config remote.origin.url https://example.com/repository.git")?; + std::fs::create_dir(&outside)?; + std::fs::write(outside.join("sentinel"), "outside contents")?; + std::os::unix::fs::symlink(&outside, destination.join("example.com"))?; + + let err = run(mode, &source, &destination, gix::progress::Discard, Some(1)) + .expect_err("existing path components must stay inside the destination, even in simulation"); + assert!(err.is_validation(), "destination escapes are validation failures"); + let diagnostic = gix_testtools::redact_debug_snapshot( + &err, + &[(&fixture.path().canonicalize()?.to_string_lossy(), "")], + ); + insta::allow_duplicates! { + insta::assert_debug_snapshot!( + diagnostic, + "execute and simulate report the symlink target outside the allowed destination root", + @r#" + Failed to handle 1 repository + + Caused by: + 0: the origin URL resolves outside the repository destination, destination="/outside/repository", destination_root="/destination" + "# + ); + } + let metadata = err + .metadata() + .next() + .expect("confinement errors include path metadata through the public API"); + assert_eq!( + metadata["destination"], + MetadataValue::Path(outside.canonicalize()?.join("repository")), + "metadata identifies the resolved path outside the allowed root" + ); + assert_eq!( + metadata["destination_root"], + MetadataValue::Path(destination.canonicalize()?), + "metadata identifies the canonicalized allowed root" + ); + assert_eq!( + std::fs::read_to_string(source.join("payload"))?, + "repository contents", + "a rejected destination leaves the source contents unchanged" + ); + assert!( + !outside.join("repository").exists(), + "a symlink cannot redirect the move" + ); + assert_eq!( + std::fs::read_to_string(outside.join("sentinel"))?, + "outside contents", + "confinement validation leaves pre-existing outside contents unchanged" + ); + assert_eq!( + std::fs::read_dir(&outside)?.count(), + 1, + "a rejected destination creates nothing outside the allowed root" + ); + } + Ok(()) +} diff --git a/gix-actor/tests/actor/identity.rs b/gix-actor/tests/actor/identity.rs index 6a3a96bfe96..3f2178acf40 100644 --- a/gix-actor/tests/actor/identity.rs +++ b/gix-actor/tests/actor/identity.rs @@ -2,7 +2,7 @@ use bstr::ByteSlice; use gix_actor::Identity; #[test] -fn round_trip() -> gix_testtools::Result { +fn round_trip() -> gix_testtools::TestResult { static DEFAULTS: &[&[u8]] = &[ b"Sebastian Thiel ", b"Sebastian Thiel < byronimo@gmail.com>", @@ -12,7 +12,7 @@ fn round_trip() -> gix_testtools::Result { b".. whitespace \t is explicitly allowed - unicode aware trimming must be done elsewhere " ]; for input in DEFAULTS { - let signature: Identity = gix_actor::IdentityRef::from_bytes(input).unwrap().into(); + let signature: Identity = gix_actor::IdentityRef::from_bytes(input)?.into(); let mut output = Vec::new(); signature.write_to(&mut output)?; assert_eq!(output.as_bstr(), input.as_bstr()); @@ -21,7 +21,7 @@ fn round_trip() -> gix_testtools::Result { } #[test] -fn lenient_parsing() -> gix_testtools::Result { +fn lenient_parsing() -> gix_testtools::TestResult { let mut error_snapshots = Vec::new(); for (input, expected_email) in [ ( @@ -48,11 +48,11 @@ fn lenient_parsing() -> gix_testtools::Result { [ Custom { kind: Other, - error: Signature name or email must not contain '<', '>' or \n, "input"="fl > ", + error: Signature name or email must not contain '<', '>' or \n, input="fl > ", }, Custom { kind: Other, - error: Signature name or email must not contain '<', '>' or \n, "input"="fl ' or \n, input="fl ' or \n, "input"="invalid < middlename", + error: Signature name or email must not contain '<', '>' or \n, input="invalid < middlename", } "#); } @@ -28,7 +28,7 @@ mod write_to { insta::assert_debug_snapshot!(signature.write_to(&mut Vec::new()).expect_err("the signature is invalid"), "signature email addresses reject angle brackets", @r#" Custom { kind: Other, - error: Signature name or email must not contain '<', '>' or \n, "input"="server>.example.com", + error: Signature name or email must not contain '<', '>' or \n, input="server>.example.com", } "#); } @@ -43,7 +43,7 @@ mod write_to { insta::assert_debug_snapshot!(signature.write_to(&mut Vec::new()).expect_err("the signature is invalid"), "signature names reject newlines", @r#" Custom { kind: Other, - error: Signature name or email must not contain '<', '>' or \n, "input"="hello\nnewline", + error: Signature name or email must not contain '<', '>' or \n, input="hello\nnewline", } "#); } @@ -62,7 +62,7 @@ fn trim() { } #[test] -fn round_trip() -> Result<(), Box> { +fn round_trip() -> gix_testtools::TestResult { static DEFAULTS: &[&[u8]] = &[ b"Sebastian Thiel 1 -0030", b"Sebastian Thiel -1500 -0030", @@ -71,7 +71,7 @@ fn round_trip() -> Result<(), Box> { ]; for input in DEFAULTS { - let signature: Signature = gix_actor::SignatureRef::from_bytes(input).unwrap().into(); + let signature: Signature = gix_actor::SignatureRef::from_bytes(input)?.into(); let mut output = Vec::new(); signature.write_to(&mut output)?; assert_eq!(output.as_bstr(), input.as_bstr()); @@ -80,9 +80,9 @@ fn round_trip() -> Result<(), Box> { } #[test] -fn signature_ref_round_trips_with_seconds_in_offset() -> Result<(), Box> { +fn signature_ref_round_trips_with_seconds_in_offset() -> gix_testtools::TestResult { let input = b"Sebastian Thiel 1313584730 +051800"; // Seen in the wild - let signature: SignatureRef = gix_actor::SignatureRef::from_bytes(input).unwrap(); + let signature: SignatureRef = gix_actor::SignatureRef::from_bytes(input)?; let mut output = Vec::new(); signature.write_to(&mut output)?; assert_eq!(output.as_bstr(), input.as_bstr()); @@ -90,9 +90,8 @@ fn signature_ref_round_trips_with_seconds_in_offset() -> Result<(), Box 1312735823 +051800") - .expect("deal with trailing zeroes in timestamp by discarding it"); +fn parse_timestamp_with_trailing_digits() -> gix_testtools::TestResult { + let signature = gix_actor::SignatureRef::from_bytes(b"first last 1312735823 +051800")?; assert_eq!( signature, SignatureRef { @@ -102,8 +101,7 @@ fn parse_timestamp_with_trailing_digits() { } ); - let signature = gix_actor::SignatureRef::from_bytes(b"first last 1312735823 +0518") - .expect("this naturally works as the timestamp does not have trailing zeroes"); + let signature = gix_actor::SignatureRef::from_bytes(b"first last 1312735823 +0518")?; assert_eq!( signature, SignatureRef { @@ -112,12 +110,12 @@ fn parse_timestamp_with_trailing_digits() { time: "1312735823 +0518", } ); + Ok(()) } #[test] -fn parse_missing_timestamp() { - let signature = gix_actor::SignatureRef::from_bytes(b"first last ") - .expect("deal with missing timestamp in signature by zeroing it"); +fn parse_missing_timestamp() -> gix_testtools::TestResult { + let signature = gix_actor::SignatureRef::from_bytes(b"first last ")?; assert_eq!( signature, SignatureRef { @@ -126,4 +124,5 @@ fn parse_missing_timestamp() { time: "" } ); + Ok(()) } diff --git a/gix-archive/src/write.rs b/gix-archive/src/write.rs index 06edb8a111c..dab4e5f8a08 100644 --- a/gix-archive/src/write.rs +++ b/gix-archive/src/write.rs @@ -307,12 +307,12 @@ fn append_tar_entry( buf.clear(); std::io::copy(&mut entry, buf).or_raise(|| message("Could not read entry data"))?; - let path = gix_path::from_bstr(add_prefix(entry.relative_path(), opts.tree_prefix.as_ref())); + let path = gix_path::from_bstr(add_prefix(entry.relative_path(), opts.tree_prefix.as_ref()))?; header.set_size(buf.len() as u64); if entry.mode.is_link() { use bstr::ByteSlice; - let target = gix_path::from_bstr(buf.as_bstr()); + let target = gix_path::from_bstr(buf.as_bstr())?; header.set_entry_type(tar::EntryType::Symlink); header.set_size(0); ar.append_link(&mut header, path, target) diff --git a/gix-archive/tests/archive.rs b/gix-archive/tests/archive.rs index d2cc21a9a17..0f12d86c439 100644 --- a/gix-archive/tests/archive.rs +++ b/gix-archive/tests/archive.rs @@ -52,14 +52,14 @@ mod from_tree { use gix_attributes::glob::pattern::Case; use gix_object::tree::EntryKind; - use gix_testtools::bstr::ByteSlice; + use gix_testtools::{TestResult, bstr::ByteSlice}; use gix_worktree::stack::state::attributes::Source; use crate::hex_to_id; #[test] - fn basic_usage_internal() -> gix_testtools::Result { - basic_usage(gix_archive::Format::InternalTransientNonPersistable, |buf| { + fn basic_usage_internal() -> TestResult { + Ok(basic_usage(Format::InternalTransientNonPersistable, |buf| { #[cfg(target_pointer_width = "64")] let expected_buffer_length = match gix_testtools::object_hash() { gix_hash::Kind::Sha1 => 551, @@ -137,13 +137,13 @@ mod from_tree { ] ); Ok(()) - }) + })?) } #[test] #[cfg(feature = "tar")] - fn basic_usage_tar() -> gix_testtools::Result { - basic_usage(gix_archive::Format::Tar, |buf| { + fn basic_usage_tar() -> TestResult { + Ok(basic_usage(Format::Tar, |buf| { use tar::EntryType; let mut ar = tar::Archive::new(buf.as_slice()); let mut out = Vec::new(); @@ -187,14 +187,14 @@ mod from_tree { .collect::>() ); Ok(()) - }) + })?) } #[test] #[cfg(feature = "tar_gz")] - fn basic_usage_tar_gz() -> gix_testtools::Result { - basic_usage( - gix_archive::Format::TarGz { + fn basic_usage_tar_gz() -> TestResult { + Ok(basic_usage( + Format::TarGz { compression_level: Some(9), }, |buf| { @@ -205,14 +205,14 @@ mod from_tree { ); Ok(()) }, - ) + )?) } #[test] #[cfg(feature = "zip")] - fn basic_usage_zip() -> gix_testtools::Result { - basic_usage( - gix_archive::Format::Zip { + fn basic_usage_zip() -> TestResult { + Ok(basic_usage( + Format::Zip { compression_level: Some(9), }, |buf| { @@ -270,7 +270,7 @@ mod from_tree { assert!(found_link, "symlink entry should be found"); Ok(()) }, - ) + )?) } fn basic_usage( diff --git a/gix-attributes/Cargo.toml b/gix-attributes/Cargo.toml index cd6959ff741..8789a5209be 100644 --- a/gix-attributes/Cargo.toml +++ b/gix-attributes/Cargo.toml @@ -23,10 +23,10 @@ path = "./benches/lookup.rs" ## Data structures implement `serde::Serialize` and `serde::Deserialize`. serde = ["dep:serde", "bstr/serde", "gix-glob/serde"] ## Enable thread-safety, otherwise the Search cannot be passed between threads. -parallel = ["gix-features/parallel"] +parallel = ["gix-parallel/parallel"] [dependencies] -gix-features = { version = "^0.50.0", path = "../gix-features" } +gix-parallel = { version = "^0.50.0", path = "../gix-parallel" } gix-error = { version = "^0.4.0", path = "../gix-error", features = ["bstr"] } gix-path = { version = "^0.13.0", path = "../gix-path" } gix-quote = { version = "^0.9.0", path = "../gix-quote" } diff --git a/gix-attributes/benches/lookup.rs b/gix-attributes/benches/lookup.rs index 7b40e1c787e..ca2226ff0f8 100644 --- a/gix-attributes/benches/lookup.rs +++ b/gix-attributes/benches/lookup.rs @@ -33,13 +33,15 @@ fn single_file(c: &mut Criterion) { for num_entries in [1, 10, 100, 1_000] { let mut search = Search::default(); let mut collection = MetadataCollection::default(); - search.add_patterns_buffer( - &attributes(num_entries, 0), - ".gitattributes".into(), - None, - &mut collection, - true, - ); + search + .add_patterns_buffer( + &attributes(num_entries, 0), + ".gitattributes".into(), + None, + &mut collection, + true, + ) + .expect("benchmark pattern sources are valid UTF-8 paths"); assert_eq!(lookup(&search, &collection, "file.rs"), num_entries); group.throughput(Throughput::Elements(num_entries as u64)); @@ -73,13 +75,15 @@ fn five_file_hierarchy(c: &mut Criterion) { } else { Path::new(directory).join(".gitattributes") }; - search.add_patterns_buffer( - &attributes(num_entries, file_index), - source, - Some(Path::new("")), - &mut collection, - true, - ); + search + .add_patterns_buffer( + &attributes(num_entries, file_index), + source, + Some(Path::new("")), + &mut collection, + true, + ) + .expect("benchmark pattern sources are valid UTF-8 paths"); } let num_entries = ENTRIES.into_iter().sum::(); diff --git a/gix-attributes/fuzz/fuzz_targets/fuzz_search.rs b/gix-attributes/fuzz/fuzz_targets/fuzz_search.rs index 21827b4a5ae..fe7443b515e 100644 --- a/gix-attributes/fuzz/fuzz_targets/fuzz_search.rs +++ b/gix-attributes/fuzz/fuzz_targets/fuzz_search.rs @@ -1,6 +1,6 @@ #![no_main] -use gix_error::Result; +use gix_error::{Result, ResultExt}; use libfuzzer_sys::fuzz_target; use std::hint::black_box; @@ -34,13 +34,15 @@ fn fuzz(Ctx { pattern, case }: Ctx) -> Result<()> { let mut search = Search::default(); let mut collection = MetadataCollection::default(); - search.add_patterns_buffer( - format!("{pattern} attr").as_bytes(), - Default::default(), - None, - &mut collection, - true, - ); + search + .add_patterns_buffer( + format!("{pattern} attr").as_bytes(), + Default::default(), + None, + &mut collection, + true, + ) + .or_error()?; let mut out = Outcome::default(); out.initialize(&collection); _ = black_box(search.pattern_matching_relative_path("relative/path".into(), case, None, &mut out)); diff --git a/gix-attributes/src/lib.rs b/gix-attributes/src/lib.rs index a4d9112c30b..b825cd7b242 100644 --- a/gix-attributes/src/lib.rs +++ b/gix-attributes/src/lib.rs @@ -14,7 +14,7 @@ //! None, //! &mut collection, //! true, -//! ); +//! )?; //! //! let mut out = Outcome::default(); //! out.initialize_with_selection(&collection, ["text", "eol"]); @@ -25,6 +25,7 @@ //! .map(|m| m.assignment.to_string()) //! .collect::>(); //! assert_eq!(assignments, vec!["text", "eol=lf"]); +//! # Ok::<(), std::io::Error>(()) //! ``` //! //! ## Feature Flags @@ -97,7 +98,7 @@ pub enum State { /// Enable the `parallel` feature to make this type thread-safe. Without it, the name is backed by an `Rc` and is /// neither `Send` nor `Sync`; with `parallel`, it is backed by an `Arc` instead. #[derive(PartialEq, Eq, Debug, Hash, Ord, PartialOrd, Clone)] -pub struct Name(pub(crate) gix_features::threading::OwnShared); +pub struct Name(pub(crate) gix_parallel::OwnShared); /// Holds a validated attribute name as a reference #[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, Ord, PartialOrd)] diff --git a/gix-attributes/src/name.rs b/gix-attributes/src/name.rs index 13e4626cb19..023b3325f7d 100644 --- a/gix-attributes/src/name.rs +++ b/gix-attributes/src/name.rs @@ -2,7 +2,7 @@ use std::borrow::Borrow; use bstr::{BStr, ByteSlice}; use gix_error::{OptionExt, Result, validation}; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use crate::{Name, NameRef}; diff --git a/gix-attributes/src/search/attributes.rs b/gix-attributes/src/search/attributes.rs index b3155d4d722..dfa67c9a970 100644 --- a/gix-attributes/src/search/attributes.rs +++ b/gix-attributes/src/search/attributes.rs @@ -32,7 +32,7 @@ impl Search { None, collection, true, /* allow macros */ - ); + )?; for path in files.into_iter() { group.add_patterns_file(path.into(), true, None, buf, collection, true /* allow macros */)?; @@ -81,15 +81,16 @@ impl Search { root: Option<&Path>, collection: &mut MetadataCollection, allow_macros: bool, - ) { + ) -> std::io::Result<()> { self.patterns - .push(pattern::List::from_bytes(bytes, source, root, Attributes)); + .push(pattern::List::from_bytes(bytes, source, root, Attributes)?); let last = self.patterns.last_mut().expect("just added"); if !allow_macros { last.patterns .retain(|p| !matches!(p.value, Value::MacroAssignments { .. })); } collection.update_from_list(last); + Ok(()) } /// Pop the last attribute patterns list from our queue. diff --git a/gix-attributes/src/search/mod.rs b/gix-attributes/src/search/mod.rs index 8e8fbf3e4c2..fd412352c06 100644 --- a/gix-attributes/src/search/mod.rs +++ b/gix-attributes/src/search/mod.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use smallvec::SmallVec; use crate::{Assignment, AssignmentRef, Name}; diff --git a/gix-attributes/src/search/outcome.rs b/gix-attributes/src/search/outcome.rs index ecd7d4b8c82..b4f838c436d 100644 --- a/gix-attributes/src/search/outcome.rs +++ b/gix-attributes/src/search/outcome.rs @@ -1,6 +1,6 @@ use bstr::{BString, ByteSlice}; -use gix_features::threading::OwnShared; use gix_glob::Pattern; +use gix_parallel::OwnShared; use crate::{ AssignmentRef, NameRef, StateRef, diff --git a/gix-attributes/tests/attributes/parse.rs b/gix-attributes/tests/attributes/parse.rs index 44beeec9e51..087cab162cb 100644 --- a/gix-attributes/tests/attributes/parse.rs +++ b/gix-attributes/tests/attributes/parse.rs @@ -96,9 +96,9 @@ fn exclamation_marks_must_be_escaped_or_error_unlike_gitignore() { line(r"\!hello"), (pattern(r"!hello", Mode::NO_SUB_DIR, None), vec![], 1) ); - insta::assert_debug_snapshot!(assert_validation(try_line(r"!hello")), "exclamation marks must be escaped or error unlike gitignore", @r#"Line 1 has a negative pattern, for literal characters use \!, "input"="!hello""#); + insta::assert_debug_snapshot!(assert_validation(try_line(r"!hello")), "exclamation marks must be escaped or error unlike gitignore", @r#"Line 1 has a negative pattern, for literal characters use \!, input="!hello""#); assert!(lenient_lines(r#"!hello"#).is_empty()); - insta::assert_debug_snapshot!(assert_validation(try_line(r#""!hello""#)), "even in quotes they trigger…", @r#"Line 1 has a negative pattern, for literal characters use \!, "input"="!hello""#); + insta::assert_debug_snapshot!(assert_validation(try_line(r#""!hello""#)), "even in quotes they trigger…", @r#"Line 1 has a negative pattern, for literal characters use \!, input="!hello""#); assert!(lenient_lines(r#""!hello""#).is_empty()); assert_eq!( line(r#""\\!hello""#), @@ -192,26 +192,26 @@ fn custom_macros_must_be_valid_attribute_names() { Macro in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="-prefixdash" + 0: Attribute has non-ascii characters or starts with '-', input="-prefixdash" "#); assert!(lenient_lines(r"[attr]-prefixdash").is_empty()); insta::assert_debug_snapshot!(assert_validation(try_line(r"[attr]!exclamation")), "custom macros must be valid attribute names", @r#" Macro in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="!exclamation" + 0: Attribute has non-ascii characters or starts with '-', input="!exclamation" "#); insta::assert_debug_snapshot!(assert_validation(try_line(r"[attr]assignment=value")), "custom macros must be valid attribute names", @r#" Macro in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="assignment=value" + 0: Attribute has non-ascii characters or starts with '-', input="assignment=value" "#); insta::assert_debug_snapshot!(assert_validation(try_line(r"[attr]你好")), "custom macros must be valid attribute names", @r#" Macro in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="你好" + 0: Attribute has non-ascii characters or starts with '-', input="你好" "#); assert!(lenient_lines(r"[attr]你好").is_empty()); } @@ -235,11 +235,11 @@ fn invalid_names_retain_line_context_and_the_validation_cause() { Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="你好", + 0: Attribute has non-ascii characters or starts with '-', input="你好", Macro in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="你好", + 0: Attribute has non-ascii characters or starts with '-', input="你好", ] "#); } @@ -250,21 +250,21 @@ fn attribute_names_must_not_begin_with_dash_and_must_be_ascii_only() { Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="-a" + 0: Attribute has non-ascii characters or starts with '-', input="-a" "#); assert!(lenient_lines(r"p !-a").is_empty()); insta::assert_debug_snapshot!(assert_validation(try_line(r#"p !!a"#)), "exclamation marks aren't allowed either", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="!a" + 0: Attribute has non-ascii characters or starts with '-', input="!a" "#); assert!(lenient_lines(r#"p !!a"#).is_empty()); insta::assert_debug_snapshot!(assert_validation(try_line(r#"p 你好"#)), "nor is utf-8 encoded characters - gitoxide could consider to relax this when established", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="你好" + 0: Attribute has non-ascii characters or starts with '-', input="你好" "#); assert!(lenient_lines(r#"p 你好"#).is_empty()); } @@ -275,25 +275,25 @@ fn attribute_names_must_not_be_empty() { Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="" + 0: Attribute has non-ascii characters or starts with '-', input="" "#); insta::assert_debug_snapshot!(assert_validation(try_line(r"p =")), "an assignment that is nothing but an equals sign has no name either", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="" + 0: Attribute has non-ascii characters or starts with '-', input="" "#); insta::assert_debug_snapshot!(assert_validation(try_line(r"p -")), "prefixes need a name to apply to", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="" + 0: Attribute has non-ascii characters or starts with '-', input="" "#); insta::assert_debug_snapshot!(assert_validation(try_line(r"p !")), "the unspecified prefix needs one as well", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="" + 0: Attribute has non-ascii characters or starts with '-', input="" "#); assert!( gix_attributes::NameRef::try_from(bstr::BStr::new(b"")).is_err(), @@ -307,20 +307,20 @@ fn attribute_names_must_not_use_the_reserved_builtin_prefix() { Attribute in line 1 has an invalid name Caused by: - 0: Attribute name uses the reserved 'builtin_' prefix, "input"="builtin_objectmode" + 0: Attribute name uses the reserved 'builtin_' prefix, input="builtin_objectmode" "#); assert!(lenient_lines(r"p builtin_objectmode").is_empty()); insta::assert_debug_snapshot!(assert_validation(try_line(r"p -builtin_objectmode")), "the prefix is checked after '-' and '!' are stripped, just like in `parse_attr()`", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute name uses the reserved 'builtin_' prefix, "input"="builtin_objectmode" + 0: Attribute name uses the reserved 'builtin_' prefix, input="builtin_objectmode" "#); insta::assert_debug_snapshot!(assert_validation(try_line(r"[attr]builtin_macro -text")), "macro names are checked against the reserved namespace as well", @r#" Macro in line 1 has an invalid name Caused by: - 0: Attribute name uses the reserved 'builtin_' prefix, "input"="builtin_macro" + 0: Attribute name uses the reserved 'builtin_' prefix, input="builtin_macro" "#); assert_eq!( line(r"p builtin"), @@ -383,25 +383,25 @@ fn only_ascii_blanks_separate_attributes() { Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="text\u{a0}eol" + 0: Attribute has non-ascii characters or starts with '-', input="text\u{a0}eol" "#); insta::assert_debug_snapshot!(assert_validation(try_line("p a\u{b}b")), "a vertical tab is part of the name, not a separator", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="a\x0bb" + 0: Attribute has non-ascii characters or starts with '-', input="a\x0bb" "#); insta::assert_debug_snapshot!(assert_validation(try_line("p a\u{c}b")), "a form feed is part of the name, not a separator", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="a\x0cb" + 0: Attribute has non-ascii characters or starts with '-', input="a\x0cb" "#); insta::assert_debug_snapshot!(assert_validation(try_line("p a\u{2028}b")), "vertical tabs, form feeds and unicode line separators aren't blanks either", @r#" Attribute in line 1 has an invalid name Caused by: - 0: Attribute has non-ascii characters or starts with '-', "input"="a\u{2028}b" + 0: Attribute has non-ascii characters or starts with '-', input="a\u{2028}b" "#); } diff --git a/gix-attributes/tests/attributes/search.rs b/gix-attributes/tests/attributes/search.rs index b74d29bd2d8..023bd151a2d 100644 --- a/gix-attributes/tests/attributes/search.rs +++ b/gix-attributes/tests/attributes/search.rs @@ -42,13 +42,15 @@ mod specials { fn search_case(pattern: &str, path: &str, rela_containing_dir: Option<&str>, case: Case) -> bool { let mut search = Search::default(); let mut collection = MetadataCollection::default(); - search.add_patterns_buffer( - format!("{pattern} test").as_bytes(), - rela_containing_dir.map_or_else(|| Path::new("").into(), |d| Path::new(d).join("filename")), - rela_containing_dir.map(|_| Path::new("")), - &mut collection, - true, - ); + search + .add_patterns_buffer( + format!("{pattern} test").as_bytes(), + rela_containing_dir.map_or_else(|| Path::new("").into(), |d| Path::new(d).join("filename")), + rela_containing_dir.map(|_| Path::new("")), + &mut collection, + true, + ) + .expect("UTF-8 pattern source has a valid base"); let mut out = Outcome::default(); out.initialize(&collection); search.pattern_matching_relative_path(path.into(), case, None, &mut out) @@ -63,7 +65,7 @@ mod specials { } #[test] -fn baseline() -> gix_error::TestResult { +fn baseline() -> gix_testtools::TestResult { let mut buf = Vec::new(); // Due to the way our setup differs from gits dynamic stack (which involves trying to read files from disk // by path) we can only test one case baseline, so we require multiple platforms (or filesystems) to run this. @@ -131,7 +133,7 @@ fn assert_references(out: &Outcome) { } #[test] -fn all_attributes_are_listed_in_declaration_order() -> gix_error::TestResult { +fn all_attributes_are_listed_in_declaration_order() -> gix_testtools::TestResult { let (mut group, mut collection, base, input) = baseline::user_attributes("lookup-order")?; let mut buf = Vec::new(); @@ -228,7 +230,7 @@ fn all_attributes_are_listed_in_declaration_order() -> gix_error::TestResult { } #[test] -fn given_attributes_are_made_available_in_given_order() -> gix_error::TestResult { +fn given_attributes_are_made_available_in_given_order() -> gix_testtools::TestResult { let (mut group, mut collection, base, input) = baseline::user_attributes_named_baseline("lookup-order", "baseline.selected")?; @@ -270,13 +272,13 @@ fn given_attributes_are_made_available_in_given_order() -> gix_error::TestResult } #[test] -fn macro_attributes_expand_only_when_macro_is_set() -> gix_error::TestResult { +fn macro_attributes_expand_only_when_macro_is_set() -> gix_testtools::TestResult { assert_baseline("macro-expansion")?; Ok(()) } #[test] -fn attribute_tokenisation_matches_git() -> gix_error::TestResult { +fn attribute_tokenisation_matches_git() -> gix_testtools::TestResult { assert_baseline("tokenisation")?; Ok(()) } diff --git a/gix-attributes/tests/attributes/source.rs b/gix-attributes/tests/attributes/source.rs index ccc28ce8124..26459fa65d3 100644 --- a/gix-attributes/tests/attributes/source.rs +++ b/gix-attributes/tests/attributes/source.rs @@ -1,23 +1,23 @@ use gix_attributes::Source; #[test] -fn system_attributes_with_mingw64() -> gix_testtools::Result { - system_attributes("mingw64", false) +fn system_attributes_with_mingw64() -> gix_testtools::TestResult { + system_attributes("mingw64", false).map_err(Into::into) } #[test] -fn system_attributes_with_ucrt64() -> gix_testtools::Result { - system_attributes("ucrt64", false) +fn system_attributes_with_ucrt64() -> gix_testtools::TestResult { + system_attributes("ucrt64", false).map_err(Into::into) } #[test] -fn system_attributes_with_mixed_prefixes_and_mingw64_active() -> gix_testtools::Result { - system_attributes("mingw64", true) +fn system_attributes_with_mixed_prefixes_and_mingw64_active() -> gix_testtools::TestResult { + system_attributes("mingw64", true).map_err(Into::into) } #[test] -fn system_attributes_with_mixed_prefixes_and_ucrt64_active() -> gix_testtools::Result { - system_attributes("ucrt64", true) +fn system_attributes_with_mixed_prefixes_and_ucrt64_active() -> gix_testtools::TestResult { + system_attributes("ucrt64", true).map_err(Into::into) } fn system_attributes(runtime: &str, mixed: bool) -> gix_testtools::Result { diff --git a/gix-bitmap/tests/bitmap.rs b/gix-bitmap/tests/bitmap.rs index 1f83d77d718..e4c1cefaa3d 100644 --- a/gix-bitmap/tests/bitmap.rs +++ b/gix-bitmap/tests/bitmap.rs @@ -1,18 +1,18 @@ mod fuzzed { #[test] - fn ewah_artifacts_run_fuzzer() { + fn ewah_artifacts_run_fuzzer() -> gix_testtools::TestResult { for path in artifact_paths("ewah") { - let data = std::fs::read(path).expect("artifact is readable"); + let data = std::fs::read(path)?; let _ = gix_bitmap::ewah::decode(&data); } + Ok(()) } #[test] - fn runaway_run_length_is_rejected() { + fn runaway_run_length_is_rejected() -> gix_testtools::TestResult { let (bitmap, rest) = gix_bitmap::ewah::decode(include_bytes!( "../fuzz/artifacts/ewah/slow-unit-ac817962d1a6c123d4d1f73860f5b779423ed171" - )) - .expect("fixture must decode"); + ))?; assert!(rest.is_empty(), "fixture should be fully consumed"); assert_eq!( @@ -20,28 +20,23 @@ mod fuzzed { None, "impossible run lengths must be rejected instead of iterating unboundedly" ); + Ok(()) } #[test] - fn non_zero_padding_bits_in_last_literal_word_are_rejected() { + fn non_zero_padding_bits_in_last_literal_word_are_rejected() -> gix_testtools::TestResult { let bitmap = gix_bitmap::ewah::Vec::from_bits(&[false]).expect("small test fixtures must fit into u32"); let mut data = Vec::new(); - bitmap - .write_to(&mut data) - .expect("writing a valid test fixture to bytes must succeed"); + bitmap.write_to(&mut data)?; let header_size = 4 + 4; // Skip the fixed-size header and the RLW word, then flip bit 33 in the first literal word // so the bitmap stays logically empty while the serialized padding becomes invalid. let literal_word_offset = header_size + 8; - let mut literal_word = u64::from_be_bytes( - data[literal_word_offset..literal_word_offset + 8] - .try_into() - .expect("literal word"), - ); + let mut literal_word = u64::from_be_bytes(data[literal_word_offset..literal_word_offset + 8].try_into()?); literal_word |= 1u64 << 33; data[literal_word_offset..literal_word_offset + 8].copy_from_slice(&literal_word.to_be_bytes()); - let (bitmap, rest) = gix_bitmap::ewah::decode(&data).expect("fixture must decode"); + let (bitmap, rest) = gix_bitmap::ewah::decode(&data)?; assert!(rest.is_empty(), "fixture should be fully consumed"); assert_eq!( @@ -49,10 +44,11 @@ mod fuzzed { None, "set bits outside the declared bit length must be rejected" ); + Ok(()) } #[test] - fn literal_only_bitmaps_preserve_all_set_bits() { + fn literal_only_bitmaps_preserve_all_set_bits() -> gix_testtools::TestResult { for bits in [ vec![], vec![false], @@ -70,10 +66,8 @@ mod fuzzed { .filter_map(|(idx, bit)| bit.then_some(idx)) .collect(); - bitmap - .write_to(&mut encoded) - .expect("writing a valid test fixture to bytes must succeed"); - let (bitmap, rest) = gix_bitmap::ewah::decode(&encoded).expect("serialized test fixture must decode"); + bitmap.write_to(&mut encoded)?; + let (bitmap, rest) = gix_bitmap::ewah::decode(&encoded)?; let mut actual = Vec::new(); assert!(rest.is_empty(), "serialized test fixture should be fully consumed"); @@ -95,6 +89,7 @@ mod fuzzed { "iteration should report exactly the set bits from the source bitmap" ); } + Ok(()) } #[test] diff --git a/gix-blame/tests/blame.rs b/gix-blame/tests/blame.rs index 2db0ca9bf39..78e1e2642d8 100644 --- a/gix-blame/tests/blame.rs +++ b/gix-blame/tests/blame.rs @@ -249,7 +249,7 @@ impl Fixture { macro_rules! mktest { ($name:ident, $case:expr, $number_of_lines:literal) => { #[test] - fn $name() -> gix_testtools::Result { + fn $name() -> gix_testtools::TestResult { let Fixture { odb, mut resource_cache, @@ -374,7 +374,7 @@ fn diff_algorithm_parity() { } #[test] -fn file_that_was_added_in_two_branches() -> gix_testtools::Result { +fn file_that_was_added_in_two_branches() -> gix_testtools::TestResult { let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_two_roots_repo.sh")?; let Fixture { @@ -405,7 +405,7 @@ fn file_that_was_added_in_two_branches() -> gix_testtools::Result { } #[test] -fn since() -> gix_testtools::Result { +fn since() -> gix_testtools::TestResult { let Fixture { odb, mut resource_cache, @@ -423,9 +423,7 @@ fn since() -> gix_testtools::Result { gix_blame::Options { diff_algorithm: gix_diff::blob::Algorithm::Histogram, ranges: BlameRanges::default(), - since: Some( - gix_date::parse("2025-01-31", None).expect("TODO: should be able to to retrieve inner from Exn"), - ), + since: Some(gix_date::parse("2025-01-31", None)?), rewrites: Some(gix_diff::Rewrites::default()), debug_track_path: false, }, @@ -447,7 +445,7 @@ mod blame_ranges { use gix_blame::BlameRanges; #[test] - fn line_range() -> gix_testtools::Result { + fn line_range() -> gix_testtools::TestResult { let Fixture { odb, mut resource_cache, @@ -483,7 +481,7 @@ mod blame_ranges { } #[test] - fn multiple_ranges_using_add_range() -> gix_testtools::Result { + fn multiple_ranges_using_add_range() -> gix_testtools::TestResult { let Fixture { odb, mut resource_cache, @@ -529,7 +527,7 @@ mod blame_ranges { } #[test] - fn multiple_ranges_using_from_ranges() -> gix_testtools::Result { + fn multiple_ranges_using_from_ranges() -> gix_testtools::TestResult { let Fixture { odb, mut resource_cache, @@ -576,7 +574,7 @@ mod rename_tracking { use crate::{Baseline, Fixture}; #[test] - fn source_file_name_is_tracked_per_hunk() -> gix_testtools::Result { + fn source_file_name_is_tracked_per_hunk() -> gix_testtools::TestResult { let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_rename_tracking_repo.sh")?; let Fixture { @@ -613,7 +611,7 @@ mod rename_tracking { } #[test] - fn rename_and_change_in_merge_commit() -> gix_testtools::Result { + fn rename_and_change_in_merge_commit() -> gix_testtools::TestResult { let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_rename_tracking_repo.sh")?; let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; @@ -649,12 +647,12 @@ mod untracked_changes { use crate::{Baseline, Fixture}; #[test] - fn untracked_lines() -> gix_testtools::Result { + fn untracked_lines() -> gix_testtools::TestResult { let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_repo.sh")?; let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; let source_file_name = "untracked-lines.txt"; - let contents = std::fs::read(worktree_path.join(source_file_name)).expect("file to be present and readable"); + let contents = std::fs::read(worktree_path.join(source_file_name))?; let lines_blamed = fixture .blame_untracked_changes( @@ -681,12 +679,12 @@ mod untracked_changes { } #[test] - fn untracked_file() -> gix_testtools::Result { + fn untracked_file() -> gix_testtools::TestResult { let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_repo.sh")?; let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; let source_file_name = "untracked-file.txt"; - let contents = std::fs::read(worktree_path.join(source_file_name)).expect("file to be present and readable"); + let contents = std::fs::read(worktree_path.join(source_file_name))?; let lines_blamed = fixture .blame_untracked_changes( @@ -713,12 +711,12 @@ mod untracked_changes { } #[test] - fn untracked_lines_with_ranges() -> gix_testtools::Result { + fn untracked_lines_with_ranges() -> gix_testtools::TestResult { let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_repo.sh")?; let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; let source_file_name = "untracked-lines.txt"; - let contents = std::fs::read(worktree_path.join(source_file_name)).expect("file to be present and readable"); + let contents = std::fs::read(worktree_path.join(source_file_name))?; let lines_blamed = fixture .blame_untracked_changes( @@ -750,7 +748,7 @@ mod symlinks { use crate::{Baseline, Fixture}; - fn run_test(source_file_name: &str, baseline_name: &str) -> gix_testtools::Result { + fn run_test(source_file_name: &str, baseline_name: &str) -> gix_testtools::TestResult { let worktree_path = gix_testtools::scripted_fixture_read_only("make_blame_symlinks_repo.sh")?; let mut fixture = Fixture::for_worktree_path(worktree_path.to_path_buf())?; @@ -778,27 +776,27 @@ mod symlinks { } #[test] - fn lines_added_to_file_targeted_by_symlink() -> gix_testtools::Result { + fn lines_added_to_file_targeted_by_symlink() -> gix_testtools::TestResult { run_test("symlink", "symlink.baseline") } #[test] - fn symlink_changing_target() -> gix_testtools::Result { + fn symlink_changing_target() -> gix_testtools::TestResult { run_test("symlink-changing-target", "symlink-changing-target.baseline") } #[test] - fn symlink_renamed() -> gix_testtools::Result { + fn symlink_renamed() -> gix_testtools::TestResult { run_test("symlink-after-rename", "symlink-renamed.baseline") } #[test] - fn file_becomes_symlink() -> gix_testtools::Result { + fn file_becomes_symlink() -> gix_testtools::TestResult { run_test("file-then-symlink", "file-becomes-symlink.baseline") } #[test] - fn symlink_becomes_file() -> gix_testtools::Result { + fn symlink_becomes_file() -> gix_testtools::TestResult { run_test("symlink-then-file", "symlink-becomes-file.baseline") } } diff --git a/gix-command/fuzz/fuzz_targets/prepare.rs b/gix-command/fuzz/fuzz_targets/prepare.rs index ed5066ebaf0..d038a0652f0 100644 --- a/gix-command/fuzz/fuzz_targets/prepare.rs +++ b/gix-command/fuzz/fuzz_targets/prepare.rs @@ -12,7 +12,7 @@ fn inspect_prepare(command: &str) { gix_command::prepare(command).with_shell(), gix_command::prepare(command).with_shell().with_quoted_command(), ] { - let command = std::process::Command::from(prep); + let command = std::process::Command::try_from(prep); _ = black_box(format!("{command:?}")); } } diff --git a/gix-command/src/lib.rs b/gix-command/src/lib.rs index bf3c547aa97..7c0e3f6c2ce 100644 --- a/gix-command/src/lib.rs +++ b/gix-command/src/lib.rs @@ -46,7 +46,7 @@ pub mod shebang { .map_or(line.len(), |space_idx| slash_idx + space_idx); let (interpreter, args) = line.split_at(space_idx); Some(Data { - interpreter: gix_path::try_from_byte_slice(interpreter.trim()).ok()?.to_owned(), + interpreter: gix_path::from_byte_slice(interpreter.trim()).ok()?.to_owned(), args: crate::parse::arguments(args.trim().as_bstr()).unwrap_or_default(), }) } @@ -63,6 +63,10 @@ pub mod shebang { /// A structure to keep settings to use when invoking a command via [`spawn()`][Prepare::spawn()], /// after creating it with [`prepare()`]. +/// +/// Convert it to [`std::process::Command`] with [`TryFrom::try_from()`] to finish preparation +/// without spawning. Preparation reports an error when shell quoting or namespace conversion +/// requires an encoding the platform cannot represent. pub struct Prepare { /// The command to invoke, either directly or with a shell depending on `use_shell`. pub command: OsString, diff --git a/gix-command/src/parse.rs b/gix-command/src/parse.rs index 40fe4fad595..40aa0345171 100644 --- a/gix-command/src/parse.rs +++ b/gix-command/src/parse.rs @@ -198,7 +198,7 @@ fn push_unquoted( } fn into_os_string(value: BString) -> ExnResult { - gix_path::try_from_bstring(value) + gix_path::from_bstring(value) .map(std::path::PathBuf::into_os_string) .or_raise_typed(|| Error::UnrepresentableOsString) } diff --git a/gix-command/src/prepare.rs b/gix-command/src/prepare.rs index a15089d6a5e..266fbea11c0 100644 --- a/gix-command/src/prepare.rs +++ b/gix-command/src/prepare.rs @@ -6,6 +6,7 @@ use std::{ }; use bstr::ByteSlice; +use gix_error::Result; use crate::{Context, Prepare, extract_interpreter, is_bare_command, split_paths, win_path_lookup}; @@ -15,8 +16,8 @@ impl Prepare { /// scripts, and if found will use `sh` to execute it or whatever is set as /// [`with_shell_program()`](Self::with_shell_program()). /// - /// Commands are inspected as bytes, including non-UTF-8 commands on Unix. If the platform - /// cannot represent a command as bytes, it is invoked directly. + /// Commands are inspected using their native encoded bytes, including commands that cannot + /// be represented as UTF-8. /// /// If a shell is used, then arguments given here with [arg()](Self::arg) or /// [args()](Self::args) will be substituted via `"$@"` if it's not already present in the @@ -30,8 +31,11 @@ impl Prepare { /// If neither this method nor [`with_shell()`](Self::with_shell()) is called, commands are /// always executed verbatim and directly, without the use of a shell. pub fn command_may_be_shell_script(mut self) -> Self { - self.use_shell = gix_path::os_str_into_bstr(&self.command) - .is_ok_and(|cmd| cmd.find_byteset(b"|&;<>()$`\\\"' \t\n*?[#~=%").is_some()); + self.use_shell = self + .command + .as_encoded_bytes() + .find_byteset(b"|&;<>()$`\\\"' \t\n*?[#~=%") + .is_some(); self } @@ -168,15 +172,20 @@ impl Prepare { /// Finalization impl Prepare { /// Spawn the command as configured. + /// + /// Encoding errors during preparation are returned with [`std::io::ErrorKind::InvalidInput`]. pub fn spawn(self) -> std::io::Result { - let mut cmd = Command::from(self); + let mut cmd = + Command::try_from(self).map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidInput, err))?; gix_trace::debug!(cmd = ?cmd); cmd.spawn() } } -impl From for Command { - fn from(mut prep: Prepare) -> Command { +impl TryFrom for Command { + type Error = gix_error::Error; + + fn try_from(mut prep: Prepare) -> Result { let mut inline_env = Vec::new(); let mut cmd = if prep.use_shell { let split_args = prep @@ -218,11 +227,10 @@ impl From for Command { .to_os_string(); cmd.arg("-c"); if !prep.args.is_empty() { - if !gix_path::os_str_into_bstr(&prep.command).is_ok_and(|cmd| cmd.contains_str("$@")) { - if prep.quote_command - && let Ok(command) = gix_path::os_str_into_bstr(&prep.command) - { - prep.command = gix_path::from_bstring(gix_quote::single(command)).into(); + if !prep.command.as_encoded_bytes().contains_str("$@") { + if prep.quote_command { + let command = gix_path::os_str_into_bstr(&prep.command)?; + prep.command = gix_path::from_bstring(gix_quote::single(command))?.into(); } prep.command.push(r#" "$@""#); } else { @@ -265,7 +273,7 @@ impl From for Command { cmd.env("GIT_NO_REPLACE_OBJECTS", usize::from(value).to_string()); } if let Some(namespace) = ctx.ref_namespace { - cmd.env("GIT_NAMESPACE", gix_path::from_bstring(namespace)); + cmd.env("GIT_NAMESPACE", gix_path::from_bstring(namespace)?); } if let Some(value) = ctx.literal_pathspecs { cmd.env("GIT_LITERAL_PATHSPECS", usize::from(value).to_string()); @@ -288,7 +296,7 @@ impl From for Command { } } cmd.envs(inline_env); - cmd + Ok(cmd) } } @@ -296,7 +304,7 @@ impl From for Command { /// /// The last `PATH` in `inline_env` overrides the last one in `env`, which overrides the inherited value (of this process). /// The selected `PATH` is searched in order. A resolved shebang script is launched through its interpreter, ignoring shebang -/// arguments. If an explicit `PATH` does not resolve a bare command, the missing program remains anchored in its first entry +/// arguments. If `PATH` does not resolve a bare command, the missing program remains anchored in its first entry /// so Rust's broader Windows lookup cannot find it elsewhere. fn windows_command(command: OsString, env: &[(OsString, OsString)], inline_env: &[(String, OsString)]) -> Command { let explicit_joined_paths = inline_env @@ -316,15 +324,17 @@ fn windows_command(command: OsString, env: &[(OsString, OsString)], inline_env: let looked_up = joined_paths .as_deref() .and_then(|joined_paths| win_path_lookup(command.as_ref(), joined_paths)); - let program: Cow<'_, Path> = match (looked_up, explicit_joined_paths) { + let program: Cow<'_, Path> = match looked_up { // Use the manually resolved path. - (Some(program), _) => Cow::Owned(program), - // An explicit `PATH` miss must not fall back to `std::process::Command` broader Windows search. - (None, Some(explicit_joined_paths)) if is_bare_command(Path::new(&command)) => { - Cow::Owned(prevent_further_path_lookup(command.as_ref(), explicit_joined_paths)) + Some(program) => Cow::Owned(program), + // A bare PATH miss must neither probe a worktree file nor use Rust's broader Windows search. + None if is_bare_command(Path::new(&command)) => { + return Command::new(prevent_further_path_lookup( + command.as_ref(), + joined_paths.as_deref().unwrap_or_else(|| OsStr::new("")), + )); } - // Preserve non-bare commands and let `std::process::Command` resolve bare commands without an explicit `PATH`. - (None, _) => Cow::Borrowed(command.as_ref()), + None => Cow::Borrowed(command.as_ref()), }; if let Some(shebang) = extract_interpreter(program.as_ref()) { let mut cmd = Command::new(shebang.interpreter); @@ -332,18 +342,13 @@ fn windows_command(command: OsString, env: &[(OsString, OsString)], inline_env: cmd.arg(program.as_ref()); cmd } else { - match program { - // Process lookup happens before the child's environment is installed, so an explicitly - // configured PATH must be handled here for ordinary executables as well. - Cow::Owned(program) if explicit_joined_paths.is_some() => Command::new(program), - _ => Command::new(command), - } + Command::new(program.as_ref()) } } -/// Represent the failed lookup of `command` in an explicitly assigned `PATH` without permitting another search. +/// Represent the failed lookup of `command` in `PATH` without permitting another search. /// -/// `joined_paths` is the complete value of the explicit `PATH`, not one of its entries. The first non-empty entry is +/// `joined_paths` is the complete value of `PATH`, not one of its entries. The first non-empty entry is /// joined with `command`, producing a path that Rust's Windows resolver will not look up elsewhere. If there is no such /// entry, a trailing separator makes `command` invalid instead. fn prevent_further_path_lookup(command: &Path, joined_paths: &OsStr) -> PathBuf { @@ -363,7 +368,60 @@ mod tests { use super::*; #[test] - fn explicit_path_lookup_failure_stays_within_that_path() -> gix_testtools::Result { + fn inherited_path_is_used_for_both_probing_and_execution() -> gix_testtools::TestResult { + if gix_testtools::run_in_isolated_process()? { + return Ok(()); + } + let root = gix_testtools::tempfile::tempdir()?; + let bin = root.path().join("bin"); + std::fs::create_dir(&bin)?; + let _cwd = gix_testtools::set_current_dir(root.path())?; + std::fs::write("ssh", b"#!/untrusted/interpreter\n")?; + + for path in [Some(bin.to_str().expect("temporary path is UTF-8")), Some(""), None] { + let environment = gix_testtools::Env::new(); + let _environment = match path { + Some(path) => environment.set("PATH", path), + None => environment.unset("PATH"), + }; + let mut cmd = windows_command("ssh".into(), &[], &[]); + let expected = match path { + Some(path) if !path.is_empty() => bin.join("ssh"), + _ => Path::new("ssh").join(""), + }; + assert_eq!( + cmd.get_program(), + expected, + "a PATH miss cannot probe a worktree file or leave a bare command for another lookup" + ); + assert!(cmd.get_args().next().is_none(), "the planted shebang is never used"); + assert!(cmd.spawn().is_err(), "a missing program fails to spawn"); + } + + let _environment = gix_testtools::Env::new().set("PATH", bin.to_str().expect("temporary path is UTF-8")); + let executable = bin.join("ssh.exe"); + std::fs::write(&executable, b"executable placeholder")?; + assert_eq!( + windows_command("ssh".into(), &[], &[]).get_program(), + executable, + "the executable selected from PATH is also the one passed to the process launcher" + ); + let explicit_script = windows_command("./ssh".into(), &[], &[]); + assert_eq!( + explicit_script.get_program(), + Path::new("/untrusted/interpreter"), + "an explicitly requested script retains shebang support" + ); + assert_eq!( + explicit_script.get_args().collect::>(), + [OsStr::new("./ssh")], + "the interpreter receives the explicitly requested script" + ); + Ok(()) + } + + #[test] + fn explicit_path_lookup_failure_stays_within_that_path() -> gix_testtools::TestResult { let joined_paths = std::env::join_paths(["", "not/a/real/path", "also/not/real"])?; let cmd = windows_command("missing.exe".into(), &[], &[("PATH".into(), joined_paths)]); assert_eq!( diff --git a/gix-command/src/tests.rs b/gix-command/src/tests.rs index 8c5f5ad2710..9dd01f58bd6 100644 --- a/gix-command/src/tests.rs +++ b/gix-command/src/tests.rs @@ -1,7 +1,7 @@ use super::*; #[test] -fn internal_win_path_lookup() -> gix_testtools::Result { +fn internal_win_path_lookup() -> gix_testtools::TestResult { let root = gix_testtools::scripted_fixture_read_only("win_path_lookup.sh")?; let mut paths: Vec<_> = std::fs::read_dir(&root)? .filter_map(Result::ok) diff --git a/gix-command/tests/command/command_line.rs b/gix-command/tests/command/command_line.rs index 0be91766c89..9be96013927 100644 --- a/gix-command/tests/command/command_line.rs +++ b/gix-command/tests/command/command_line.rs @@ -4,7 +4,7 @@ use gix_command::parse::{self, Outcome}; use gix_error::{Result, ResultExt}; #[test] -fn words_are_split_without_expansion() -> gix_testtools::Result { +fn words_are_split_without_expansion() -> gix_testtools::TestResult { assert_eq!( command_line( r#"cmd 'single quoted' "double \"quoted\"" escaped\ word "kept\q" "" # ignored @@ -32,7 +32,7 @@ next"#, } #[test] -fn assignments_are_returned_separately() -> gix_testtools::Result { +fn assignments_are_returned_separately() -> gix_testtools::TestResult { assert_eq!( command_line(r#" FIRST=one SECOND="two words" _THIRD='' command arg"#)?, Outcome { @@ -49,7 +49,7 @@ fn assignments_are_returned_separately() -> gix_testtools::Result { } #[test] -fn invalid_assignment_names_are_arguments() -> gix_testtools::Result { +fn invalid_assignment_names_are_arguments() -> gix_testtools::TestResult { for (input, expected) in [ ("tool-name=value arg", &["tool-name=value", "arg"]), (r#"'FOO'=bar command"#, &["FOO=bar", "command"]), @@ -168,7 +168,7 @@ fn parse_errors_retain_their_classification() { #[test] #[cfg(unix)] -fn non_utf8_input_is_preserved() -> gix_testtools::Result { +fn non_utf8_input_is_preserved() -> gix_testtools::TestResult { use bstr::ByteSlice; use std::os::unix::ffi::OsStringExt; diff --git a/gix-command/tests/command/context.rs b/gix-command/tests/command/context.rs index 73a12d7836d..c45e5815c6f 100644 --- a/gix-command/tests/command/context.rs +++ b/gix-command/tests/command/context.rs @@ -6,69 +6,74 @@ fn winfix(expected: impl Into) -> String { } #[test] -fn git_dir_sets_git_dir_env_and_cwd() { +fn git_dir_sets_git_dir_env_and_cwd() -> gix_testtools::TestResult { let ctx = Context { git_dir: Some(".".into()), ..Default::default() }; - let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx)); + let cmd = std::process::Command::try_from(gix_command::prepare("").with_context(ctx))?; assert_eq!(format!("{cmd:?}"), winfix(r#"GIT_DIR="." """#)); + Ok(()) } #[test] -fn worktree_dir_sets_env_only() { +fn worktree_dir_sets_env_only() -> gix_testtools::TestResult { let ctx = Context { worktree_dir: Some(".".into()), ..Default::default() }; - let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx)); + let cmd = std::process::Command::try_from(gix_command::prepare("").with_context(ctx))?; assert_eq!(format!("{cmd:?}"), winfix(r#"GIT_WORK_TREE="." """#)); + Ok(()) } #[test] -fn no_replace_objects_sets_env_only() { +fn no_replace_objects_sets_env_only() -> gix_testtools::TestResult { for value in [false, true] { let expected = usize::from(value); let ctx = Context { no_replace_objects: Some(value), ..Default::default() }; - let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx)); + let cmd = std::process::Command::try_from(gix_command::prepare("").with_context(ctx))?; assert_eq!( format!("{cmd:?}"), winfix(format!(r#"GIT_NO_REPLACE_OBJECTS="{expected}" """#)) ); } + Ok(()) } #[test] -fn ref_namespace_sets_env_only() { +fn ref_namespace_sets_env_only() -> gix_testtools::TestResult { let ctx = Context { ref_namespace: Some("namespace".into()), ..Default::default() }; - let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx)); + let cmd = std::process::Command::try_from(gix_command::prepare("").with_context(ctx))?; assert_eq!(format!("{cmd:?}"), winfix(r#"GIT_NAMESPACE="namespace" """#)); + Ok(()) } #[test] -fn literal_pathspecs_sets_env_only() { +fn literal_pathspecs_sets_env_only() -> gix_testtools::TestResult { for value in [false, true] { let expected = usize::from(value); let ctx = Context { literal_pathspecs: Some(value), ..Default::default() }; - let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx)); + let cmd = std::process::Command::try_from(gix_command::prepare("").with_context(ctx))?; assert_eq!( format!("{cmd:?}"), winfix(format!(r#"GIT_LITERAL_PATHSPECS="{expected}" """#)) ); } + Ok(()) } #[test] -fn glob_pathspecs_sets_env_only() { +fn glob_pathspecs_sets_env_only() -> gix_testtools::TestResult { for (value, expected) in [ (false, r#"GIT_NOGLOB_PATHSPECS="1""#), (true, r#"GIT_GLOB_PATHSPECS="1""#), @@ -77,23 +82,25 @@ fn glob_pathspecs_sets_env_only() { glob_pathspecs: Some(value), ..Default::default() }; - let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx)); + let cmd = std::process::Command::try_from(gix_command::prepare("").with_context(ctx))?; assert_eq!(format!("{cmd:?}"), winfix(format!(r#"{expected} """#))); } + Ok(()) } #[test] -fn icase_pathspecs_sets_env_only() { +fn icase_pathspecs_sets_env_only() -> gix_testtools::TestResult { for value in [false, true] { let expected = usize::from(value); let ctx = Context { icase_pathspecs: Some(value), ..Default::default() }; - let cmd = std::process::Command::from(gix_command::prepare("").with_context(ctx)); + let cmd = std::process::Command::try_from(gix_command::prepare("").with_context(ctx))?; assert_eq!( format!("{cmd:?}"), winfix(format!(r#"GIT_ICASE_PATHSPECS="{expected}" """#)) ); } + Ok(()) } diff --git a/gix-command/tests/command/main.rs b/gix-command/tests/command/main.rs index fef0a322c4c..497a256693f 100644 --- a/gix-command/tests/command/main.rs +++ b/gix-command/tests/command/main.rs @@ -1,9 +1,9 @@ use std::path::Path; -use gix_testtools::Result; +use gix_testtools::TestResult; #[test] -fn extract_interpreter() -> gix_testtools::Result { +fn extract_interpreter() -> TestResult { let root = gix_testtools::scripted_fixture_read_only("win_path_lookup.sh")?; assert_eq!( gix_command::extract_interpreter(&root.join("b").join("exe")), diff --git a/gix-command/tests/command/prepare.rs b/gix-command/tests/command/prepare.rs index 388c5a9d50a..2a27b776576 100644 --- a/gix-command/tests/command/prepare.rs +++ b/gix-command/tests/command/prepare.rs @@ -1,6 +1,85 @@ -use crate::Result; +use gix_testtools::TestResult; use std::sync::LazyLock; +#[test] +#[cfg(any(unix, windows))] +fn native_program_paths_are_preserved_without_utf8_conversion() -> TestResult { + #[cfg(unix)] + let program = { + use std::os::unix::ffi::OsStringExt; + std::ffi::OsString::from_vec(b"./native-\xff".to_vec()) + }; + #[cfg(windows)] + let program = { + use std::os::windows::ffi::OsStringExt; + std::ffi::OsString::from_wide(&[b'.' as u16, b'/' as u16, 0xd800]) + }; + // An explicit PATH avoids reading the process environment during Windows preparation. + let command = std::process::Command::try_from(gix_command::prepare(&program).env("PATH", ""))?; + assert_eq!( + command.get_program(), + program, + "direct commands preserve native path encoding" + ); + Ok(()) +} + +#[test] +#[cfg(windows)] +fn native_shell_inspection_preserves_bytes_and_quoting_reports_encoding_errors() -> TestResult { + use std::os::windows::ffi::OsStringExt; + + let native = std::ffi::OsString::from_wide(&[0xd800]); + let mut script = native.clone(); + script.push(" $@"); + let prepare = gix_command::prepare(&script).command_may_be_shell_script(); + assert!( + prepare.use_shell, + "ASCII shell syntax is recognized beside an unpaired surrogate" + ); + let command = std::process::Command::try_from(prepare.with_shell_program("unused-shell").arg("arg"))?; + assert_eq!( + command.get_args().nth(1), + Some(script.as_os_str()), + "an existing $@ is detected without changing the native script" + ); + let err = std::process::Command::try_from( + gix_command::prepare(native) + .with_shell() + .with_shell_program("unused-shell") + .with_quoted_command() + .arg("arg"), + ) + .expect_err("shell quoting needs representable bytes"); + assert!(err.is_validation(), "unrepresentable shell input is a validation error"); + Ok(()) +} + +#[test] +#[cfg(windows)] +fn unrepresentable_namespace_is_reported_before_spawning() -> TestResult { + let prepare = || { + gix_command::prepare("./unused-command") + .env("PATH", "") + .with_context(gix_command::Context { + ref_namespace: Some(vec![0xff].into()), + ..Default::default() + }) + }; + let err = std::process::Command::try_from(prepare()).expect_err("a namespace must fit the process environment"); + assert!( + err.is_validation(), + "invalid namespace bytes retain their validation classification" + ); + let err = prepare().spawn().expect_err("invalid input prevents process creation"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + err.get_ref().is_some(), + "the encoding failure remains the I/O error's source" + ); + Ok(()) +} + fn default_shell() -> &'static str { static SH: LazyLock = LazyLock::new(|| gix_path::env::shell_command().get_program().to_owned()); SH.to_str() @@ -14,7 +93,15 @@ fn default_shell() -> &'static str { const SH_BASENAME: &str = if cfg!(windows) { "sh.exe" } else { "sh" }; fn quoted(input: &[&str]) -> String { - input.iter().map(|s| format!("\"{s}\"")).collect::>().join(" ") + // These assertions cover argument parsing. Windows resolves the program before spawning, so + // compare against the same program prepared without any argument splitting. + let (program, args) = input.split_first().expect("a command always includes its program"); + let cmd = std::process::Command::try_from(gix_command::prepare(program)) + .expect("command fixture can be represented by the platform"); + std::iter::once(format!("{cmd:?}")) + .chain(args.iter().map(|s| format!("\"{s}\""))) + .collect::>() + .join(" ") } fn quoted_default_shell(input: &[&str]) -> String { @@ -35,46 +122,51 @@ fn quoted_default_shell(input: &[&str]) -> String { } #[test] -fn empty() { - let cmd = std::process::Command::from(gix_command::prepare("")); +fn empty() -> TestResult { + let cmd = std::process::Command::try_from(gix_command::prepare(""))?; assert_eq!(format!("{cmd:?}"), "\"\""); + Ok(()) } #[test] -fn whitespace_only_without_shell() { - let cmd = std::process::Command::from(gix_command::prepare(" ")); - assert_eq!(format!("{cmd:?}"), "\" \""); +fn whitespace_only_without_shell() -> TestResult { + let cmd = std::process::Command::try_from(gix_command::prepare(" "))?; + assert_eq!(format!("{cmd:?}"), quoted(&[" "])); + Ok(()) } #[test] -fn whitespace_only_commands_with_auto_split_fall_back_to_shell() { - let cmd = std::process::Command::from( +fn whitespace_only_commands_with_auto_split_fall_back_to_shell() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(" ").command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!(format!("{cmd:?}"), quoted_default_shell(&["-c", " ", SH_BASENAME])); + Ok(()) } #[test] -fn single_and_multiple_arguments() { - let cmd = std::process::Command::from(gix_command::prepare("ls").arg("first").args(["second", "third"])); +fn single_and_multiple_arguments() -> TestResult { + let cmd = std::process::Command::try_from(gix_command::prepare("ls").arg("first").args(["second", "third"]))?; assert_eq!(format!("{cmd:?}"), quoted(&["ls", "first", "second", "third"])); + Ok(()) } #[test] -fn multiple_arguments_in_one_line_with_auto_split() { - let cmd = std::process::Command::from( +fn multiple_arguments_in_one_line_with_auto_split() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare("echo first second third").command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted(&["echo", "first", "second", "third"]), "we split by hand which works unless one tries to rely on shell-builtins (which we can't detect)" ); + Ok(()) } #[test] -fn shell_assignments_are_applied_during_manual_splitting() { - let cmd = std::process::Command::from( +fn shell_assignments_are_applied_during_manual_splitting() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(r#" FOO=bar BAR="two words" GIT_DIR=inline command.exe arg"#) .env("FOO", "overridden") .with_context(gix_command::Context { @@ -82,10 +174,10 @@ fn shell_assignments_are_applied_during_manual_splitting() { ..Default::default() }) .command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!( cmd.get_program(), - "command.exe", + std::process::Command::try_from(gix_command::prepare("command.exe"))?.get_program(), "non-PATH assignments don't prevent manual splitting" ); assert_eq!(cmd.get_args().collect::>(), ["arg"], "arguments are retained"); @@ -109,20 +201,21 @@ fn shell_assignments_are_applied_during_manual_splitting() { Some(std::ffi::OsStr::new("inline")), "inline assignments have shell precedence over context values" ); + Ok(()) } #[test] #[cfg(windows)] -fn inline_path_assignment_controls_lookup() -> gix_testtools::Result { +fn inline_path_assignment_controls_lookup() -> TestResult { let root = gix_testtools::scripted_fixture_read_only("win_path_lookup.sh")?; let joined_paths = root.join("a").to_string_lossy().replace('\\', "/"); let program = format!("{joined_paths}/x.exe"); let input = format!(r#"PATH="{joined_paths}" x.exe arg"#); - let cmd = std::process::Command::from( + let cmd = std::process::Command::try_from( gix_command::prepare(input) .env("PATH", "builder path must be overridden") .command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!(cmd.get_program(), std::path::Path::new(&program)); assert_eq!( cmd.get_args().collect::>(), @@ -141,11 +234,11 @@ fn inline_path_assignment_controls_lookup() -> gix_testtools::Result { #[test] #[cfg(windows)] -fn builder_path_controls_lookup() -> gix_testtools::Result { +fn builder_path_controls_lookup() -> TestResult { let root = gix_testtools::scripted_fixture_read_only("win_path_lookup.sh")?; let joined_paths = root.join("b").to_string_lossy().replace('\\', "/"); let script = std::path::PathBuf::from(&joined_paths).join("exe"); - let cmd = std::process::Command::from(gix_command::prepare("exe").env("Path", joined_paths.as_str())); + let cmd = std::process::Command::try_from(gix_command::prepare("exe").env("Path", joined_paths.as_str()))?; assert_eq!(cmd.get_program(), std::path::Path::new("/b/exe")); assert_eq!( cmd.get_args().collect::>(), @@ -157,13 +250,13 @@ fn builder_path_controls_lookup() -> gix_testtools::Result { #[test] #[cfg(windows)] -fn manually_split_commands_retain_shebang_dispatch() -> gix_testtools::Result { +fn manually_split_commands_retain_shebang_dispatch() -> TestResult { let root = gix_testtools::scripted_fixture_read_only("win_path_lookup.sh")?; let script = root.join("b").join("exe").to_string_lossy().replace('\\', "/"); let input = format!(r#"FOO=bar "{script}" arg"#); - let cmd = std::process::Command::from( + let cmd = std::process::Command::try_from( gix_command::prepare(input).command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!(cmd.get_program(), std::path::Path::new("/b/exe")); assert_eq!( cmd.get_args().collect::>(), @@ -181,34 +274,40 @@ fn manually_split_commands_retain_shebang_dispatch() -> gix_testtools::Result { } #[test] -fn only_unambiguous_shell_assignments_are_applied() { +fn only_unambiguous_shell_assignments_are_applied() -> TestResult { for (input, program, args) in [ ("tool-name=value arg", "tool-name=value", &["arg"][..]), (r#"'FOO'=bar command"#, "FOO=bar", &["command"][..]), ] { - let cmd = std::process::Command::from( + let cmd = std::process::Command::try_from( gix_command::prepare(input).command_may_be_shell_script_allow_manual_argument_splitting(), + )?; + assert_eq!( + cmd.get_program(), + std::process::Command::try_from(gix_command::prepare(program))?.get_program(), + "{input:?} is not an assignment prefix" ); - assert_eq!(cmd.get_program(), program, "{input:?} is not an assignment prefix"); assert_eq!(cmd.get_args().collect::>(), args, "arguments are retained"); assert_eq!(cmd.get_envs().count(), 0, "the environment is unchanged"); } + Ok(()) } #[test] -fn assignment_only_input_is_left_to_the_shell() { - let cmd = std::process::Command::from( +fn assignment_only_input_is_left_to_the_shell() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare("tool=name").command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", "tool=name", SH_BASENAME]) ); + Ok(()) } #[test] #[cfg(unix)] -fn invalid_utf8_commands_are_checked_for_shell_syntax() { +fn invalid_utf8_commands_are_checked_for_shell_syntax() -> TestResult { use std::os::unix::ffi::{OsStrExt, OsStringExt}; assert!( @@ -224,20 +323,21 @@ fn invalid_utf8_commands_are_checked_for_shell_syntax() { "shell syntax is detected without requiring UTF-8" ); - let cmd = std::process::Command::from( + let cmd = std::process::Command::try_from( gix_command::prepare(std::ffi::OsString::from_vec(vec![0xff, b' ', 0xfe])) .command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!(cmd.get_program().as_bytes(), [0xff]); assert_eq!( cmd.get_args().map(OsStrExt::as_bytes).collect::>(), [&[0xfe][..]], "manual splitting preserves invalid UTF-8" ); + Ok(()) } #[test] -fn relative_existing_paths_with_shell_syntax_still_use_the_shell() -> Result { +fn relative_existing_paths_with_shell_syntax_still_use_the_shell() -> TestResult { let temp = gix_testtools::tempfile::Builder::new() .prefix("$HOME") .tempdir_in(".")?; @@ -253,18 +353,20 @@ fn relative_existing_paths_with_shell_syntax_still_use_the_shell() -> Result { } #[test] -fn single_and_multiple_arguments_as_part_of_command() { - let cmd = std::process::Command::from(gix_command::prepare("ls first second third")); +fn single_and_multiple_arguments_as_part_of_command() -> TestResult { + let cmd = std::process::Command::try_from(gix_command::prepare("ls first second third"))?; assert_eq!( format!("{cmd:?}"), quoted(&["ls first second third"]), "without shell, this is an invalid command" ); + Ok(()) } #[test] -fn single_and_multiple_arguments_as_part_of_command_with_shell() { - let cmd = std::process::Command::from(gix_command::prepare("ls first second third").command_may_be_shell_script()); +fn single_and_multiple_arguments_as_part_of_command_with_shell() -> TestResult { + let cmd = + std::process::Command::try_from(gix_command::prepare("ls first second third").command_may_be_shell_script())?; assert_eq!( format!("{cmd:?}"), if cfg!(windows) { @@ -274,15 +376,16 @@ fn single_and_multiple_arguments_as_part_of_command_with_shell() { }, "with shell, this works as it performs word splitting" ); + Ok(()) } #[test] -fn single_and_multiple_arguments_as_part_of_command_with_given_shell() { - let cmd = std::process::Command::from( +fn single_and_multiple_arguments_as_part_of_command_with_given_shell() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare("ls first second third") .command_may_be_shell_script() .with_shell_program("/somepath/to/bash"), - ); + )?; assert_eq!( format!("{cmd:?}"), if cfg!(windows) { @@ -292,15 +395,16 @@ fn single_and_multiple_arguments_as_part_of_command_with_given_shell() { }, "with shell, this works as it performs word splitting on Windows, but on linux (or without splitting) it uses the given shell" ); + Ok(()) } #[test] -fn single_and_complex_arguments_as_part_of_command_with_shell() { - let cmd = std::process::Command::from( +fn single_and_complex_arguments_as_part_of_command_with_shell() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(r#"ls --foo "a b""#) .arg("additional") .command_may_be_shell_script(), - ); + )?; assert_eq!( format!("{cmd:?}"), if cfg!(windows) { @@ -311,63 +415,68 @@ fn single_and_complex_arguments_as_part_of_command_with_shell() { }, "with shell, this works as it performs word splitting, on windows we can avoid the shell" ); + Ok(()) } #[test] -fn single_and_complex_arguments_with_auto_split() { - let cmd = std::process::Command::from( +fn single_and_complex_arguments_with_auto_split() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(r#"ls --foo="a b""#).command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!( format!("{cmd:?}"), - r#""ls" "--foo=a b""#, + quoted(&["ls", "--foo=a b"]), "splitting can also handle quotes" ); + Ok(()) } #[test] -fn single_and_complex_arguments_without_auto_split() { - let cmd = std::process::Command::from( +fn single_and_complex_arguments_without_auto_split() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(r#"ls --foo="a b""#).command_may_be_shell_script_disallow_manual_argument_splitting(), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", r#"ls --foo=\"a b\""#, SH_BASENAME]) ); + Ok(()) } #[test] -fn single_and_simple_arguments_without_auto_split_with_shell() { - let cmd = std::process::Command::from(gix_command::prepare("ls").arg("--foo=a b").with_shell()); +fn single_and_simple_arguments_without_auto_split_with_shell() -> TestResult { + let cmd = std::process::Command::try_from(gix_command::prepare("ls").arg("--foo=a b").with_shell())?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", r#"ls \"$@\""#, SH_BASENAME, "--foo=a b"]) ); + Ok(()) } #[test] -fn quoted_command_without_argument_splitting() { - let cmd = std::process::Command::from( +fn quoted_command_without_argument_splitting() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare("ls") .arg("--foo=a b") .with_shell() .with_quoted_command(), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", r#"'ls' \"$@\""#, SH_BASENAME, "--foo=a b"]), "looks strange thanks to debug printing, but is the right amount of quotes actually" ); + Ok(()) } #[test] -fn quoted_windows_command_without_argument_splitting() { - let cmd = std::process::Command::from( +fn quoted_windows_command_without_argument_splitting() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(r"C:\Users\O'Shaughnessy\with space.exe") .arg("--foo='a b'") .with_shell() .with_quoted_command(), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&[ @@ -378,58 +487,63 @@ fn quoted_windows_command_without_argument_splitting() { ]), "again, a lot of extra backslashes, but it's correct outside of the debug formatting" ); + Ok(()) } #[test] -fn single_and_complex_arguments_will_not_auto_split_on_special_characters() { - let cmd = std::process::Command::from( +fn single_and_complex_arguments_will_not_auto_split_on_special_characters() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare("ls --foo=~/path").command_may_be_shell_script_allow_manual_argument_splitting(), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", "ls --foo=~/path", SH_BASENAME]), "splitting can also handle quotes" ); + Ok(()) } #[test] -fn tilde_path_and_multiple_arguments_as_part_of_command_with_shell() { - let cmd = - std::process::Command::from(gix_command::prepare(r#"~/bin/exe --foo "a b""#).command_may_be_shell_script()); +fn tilde_path_and_multiple_arguments_as_part_of_command_with_shell() -> TestResult { + let cmd = std::process::Command::try_from( + gix_command::prepare(r#"~/bin/exe --foo "a b""#).command_may_be_shell_script(), + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", r#"~/bin/exe --foo \"a b\""#, SH_BASENAME]), "this always needs a shell as we need tilde expansion" ); + Ok(()) } #[test] -fn script_with_dollar_at() { - let cmd = std::process::Command::from( +fn script_with_dollar_at() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(r#"echo "$@" >&2"#) .command_may_be_shell_script() .arg("store"), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", r#"echo \"$@\" >&2"#, SH_BASENAME, "store"]), "this is how credential helpers have to work as for some reason they don't get '$@' added in Git.\ We deal with it by not doubling the '$@' argument, which seems more flexible." ); + Ok(()) } #[test] #[cfg(unix)] -fn non_utf8_script_with_dollar_at_does_not_duplicate_arguments() { +fn non_utf8_script_with_dollar_at_does_not_duplicate_arguments() -> TestResult { use bstr::ByteSlice; use std::os::unix::ffi::{OsStrExt, OsStringExt}; let script = std::ffi::OsString::from_vec(b"echo \xff \"$@\"".to_vec()); - let cmd = std::process::Command::from( + let cmd = std::process::Command::try_from( gix_command::prepare(script.clone()) .command_may_be_shell_script() .arg("argument"), - ); + )?; assert_eq!( cmd.get_args() .nth(1) @@ -439,24 +553,26 @@ fn non_utf8_script_with_dollar_at_does_not_duplicate_arguments() { script.as_bytes().as_bstr(), "the existing byte-encoded $@ is retained without appending another one" ); + Ok(()) } #[test] -fn script_with_dollar_at_has_no_quoting() { - let cmd = std::process::Command::from( +fn script_with_dollar_at_has_no_quoting() -> TestResult { + let cmd = std::process::Command::try_from( gix_command::prepare(r#"echo "$@" >&2"#) .command_may_be_shell_script() .with_quoted_command() .arg("store"), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted_default_shell(&["-c", r#"echo \"$@\" >&2"#, SH_BASENAME, "store"]) ); + Ok(()) } #[test] -fn shell_program_with_no_basename_uses_underscore_placeholder() { +fn shell_program_with_no_basename_uses_underscore_placeholder() -> TestResult { // Defensive fallback for degenerate input that should not occur in // practice. If a caller passes a shell path whose `file_name()` is // `None` (empty string, `/`, etc.), the `command_name` operand falls @@ -464,14 +580,15 @@ fn shell_program_with_no_basename_uses_underscore_placeholder() { // in shell one-liners. Such a "shell" would not produce a runnable // command — the fallback only keeps the construction total in the // face of bad input, without making a false claim about the shell. - let cmd = std::process::Command::from( + let cmd = std::process::Command::try_from( gix_command::prepare("echo hi") .command_may_be_shell_script_disallow_manual_argument_splitting() .with_shell_program(""), - ); + )?; assert_eq!( format!("{cmd:?}"), quoted(&["", "-c", "echo hi", "_"]), "with no basename available, the command_name operand is '_', not a guessed shell name" ); + Ok(()) } diff --git a/gix-command/tests/command/spawn.rs b/gix-command/tests/command/spawn.rs index ed96f4ef86b..d8423e6cd89 100644 --- a/gix-command/tests/command/spawn.rs +++ b/gix-command/tests/command/spawn.rs @@ -1,8 +1,8 @@ -use crate::Result; +use crate::TestResult; use bstr::ByteSlice; #[test] -fn environment_variables_are_passed_one_by_one() -> Result { +fn environment_variables_are_passed_one_by_one() -> TestResult { let out = gix_command::prepare("echo $FIRST $SECOND") .env("FIRST", "first") .env("SECOND", "second") @@ -14,7 +14,7 @@ fn environment_variables_are_passed_one_by_one() -> Result { } #[test] -fn disallow_shell() -> Result { +fn disallow_shell() -> TestResult { let out = gix_command::prepare("PATH= echo hi") .command_may_be_shell_script_disallow_manual_argument_splitting() .spawn()? @@ -24,7 +24,7 @@ fn disallow_shell() -> Result { let mut cmd: std::process::Command = gix_command::prepare("echo hi") .command_may_be_shell_script() .without_shell() - .into(); + .try_into()?; assert!( cmd.env_remove("PATH").spawn().is_err(), "no command named 'echo hi' exists" @@ -33,12 +33,12 @@ fn disallow_shell() -> Result { } #[test] -fn script_with_dollar_at() -> Result { - let out = std::process::Command::from( +fn script_with_dollar_at() -> TestResult { + let out = std::process::Command::try_from( gix_command::prepare(r#"echo "$@""#) .command_may_be_shell_script() .arg("arg"), - ) + )? .spawn()? .wait_with_output()?; assert_eq!( @@ -50,7 +50,7 @@ fn script_with_dollar_at() -> Result { } #[test] -fn direct_command_execution_searches_in_path() -> Result { +fn direct_command_execution_searches_in_path() -> TestResult { assert!( gix_command::prepare(if cfg!(unix) { "ls" } else { "attrib.exe" }) .spawn()? @@ -62,17 +62,17 @@ fn direct_command_execution_searches_in_path() -> Result { #[cfg(unix)] #[test] -fn direct_command_with_absolute_command_path() -> Result { +fn direct_command_with_absolute_command_path() -> TestResult { assert!(gix_command::prepare("/usr/bin/env").spawn()?.wait()?.success()); Ok(()) } mod with_shell { - use crate::Result; + use crate::TestResult; use gix_testtools::bstr::ByteSlice; #[test] - fn command_in_path_with_args() -> Result { + fn command_in_path_with_args() -> TestResult { // `ls` is occasionaly a builtin, as in busybox ash, but it is usually external. assert!( gix_command::prepare(if cfg!(unix) { "ls -l" } else { "attrib.exe /d" }) @@ -86,7 +86,7 @@ mod with_shell { #[cfg(unix)] #[test] - fn shell_builtin_or_command_in_path() -> Result { + fn shell_builtin_or_command_in_path() -> TestResult { let out = gix_command::prepare("echo") .command_may_be_shell_script() .spawn()? @@ -98,7 +98,7 @@ mod with_shell { #[cfg(unix)] #[test] - fn shell_builtin_or_command_in_path_with_single_extra_arg() -> Result { + fn shell_builtin_or_command_in_path_with_single_extra_arg() -> TestResult { let out = gix_command::prepare("printf") .command_may_be_shell_script() .arg("1") @@ -111,7 +111,7 @@ mod with_shell { #[cfg(unix)] #[test] - fn shell_builtin_or_command_in_path_with_multiple_extra_args() -> Result { + fn shell_builtin_or_command_in_path_with_multiple_extra_args() -> TestResult { let out = gix_command::prepare("printf") .command_may_be_shell_script() .arg("%s") @@ -124,7 +124,7 @@ mod with_shell { } #[test] - fn force_shell_builtin() -> Result { + fn force_shell_builtin() -> TestResult { let out = gix_command::prepare("echo").with_shell().spawn()?.wait_with_output()?; assert!(out.status.success()); assert_eq!(out.stdout.as_bstr(), "\n"); @@ -132,7 +132,7 @@ mod with_shell { } #[test] - fn force_shell_builtin_with_single_extra_arg() -> Result { + fn force_shell_builtin_with_single_extra_arg() -> TestResult { let out = gix_command::prepare("printf") .with_shell() .arg("1") @@ -144,7 +144,7 @@ mod with_shell { } #[test] - fn force_shell_builtin_with_multiple_extra_args() -> Result { + fn force_shell_builtin_with_multiple_extra_args() -> TestResult { let out = gix_command::prepare("printf") .with_shell() .arg("%s") @@ -157,7 +157,7 @@ mod with_shell { } #[test] - fn sh_shell_specific_script_code() -> Result { + fn sh_shell_specific_script_code() -> TestResult { assert!( gix_command::prepare(":;:;:") .command_may_be_shell_script() @@ -169,7 +169,7 @@ mod with_shell { } #[test] - fn sh_shell_specific_script_code_with_single_extra_arg() -> Result { + fn sh_shell_specific_script_code_with_single_extra_arg() -> TestResult { let out = gix_command::prepare(":;printf") .command_may_be_shell_script() .arg("1") @@ -181,7 +181,7 @@ mod with_shell { } #[test] - fn sh_shell_specific_script_code_with_multiple_extra_args() -> Result { + fn sh_shell_specific_script_code_with_multiple_extra_args() -> TestResult { let out = gix_command::prepare(":;printf") .command_may_be_shell_script() .arg("%s") @@ -195,7 +195,7 @@ mod with_shell { #[cfg(unix)] #[test] - fn dollar_zero_in_minus_c_is_basename_of_default_shell() -> Result { + fn dollar_zero_in_minus_c_is_basename_of_default_shell() -> TestResult { let out = gix_command::prepare(r#"printf %s "$0""#) .command_may_be_shell_script() .spawn()? @@ -211,12 +211,12 @@ mod with_shell { #[cfg(unix)] #[test] - fn dollar_zero_in_minus_c_reflects_with_shell_program() -> Result { - let out = std::process::Command::from( + fn dollar_zero_in_minus_c_reflects_with_shell_program() -> TestResult { + let out = std::process::Command::try_from( gix_command::prepare(r#"printf %s "$0""#) .command_may_be_shell_script() .with_shell_program(gix_testtools::bash_program()), - ) + )? .spawn()? .wait_with_output()?; assert_eq!( diff --git a/gix-commitgraph/tests/commitgraph.rs b/gix-commitgraph/tests/commitgraph.rs index 855192d4b9d..cbe54f9d959 100644 --- a/gix-commitgraph/tests/commitgraph.rs +++ b/gix-commitgraph/tests/commitgraph.rs @@ -12,7 +12,7 @@ use gix_testtools::scripted_fixture_read_only; mod access; #[test] -fn missing_path_is_not_found() -> gix_testtools::Result { +fn missing_path_is_not_found() -> gix_testtools::TestResult { let dir = gix_testtools::tempfile::tempdir()?; let err = gix_commitgraph::at(dir.path().join("missing")) .err() @@ -31,7 +31,7 @@ fn missing_path_is_not_found() -> gix_testtools::Result { } #[test] -fn checksum_mismatches_retain_their_classification() -> gix_testtools::Result { +fn checksum_mismatches_retain_their_classification() -> gix_testtools::TestResult { let repo = gix_testtools::scripted_fixture_writable("single_commit.sh")?; let mut data = std::fs::read(repo.path().join(".git/objects/info/commit-graph"))?; *data.last_mut().expect("the graph has a checksum trailer") ^= 1; diff --git a/gix-config-value/src/path.rs b/gix-config-value/src/path.rs index fc710e449ec..accd4867452 100644 --- a/gix-config-value/src/path.rs +++ b/gix-config-value/src/path.rs @@ -161,10 +161,9 @@ impl Path { if self.starts_with(PREFIX) { let git_install_dir = git_install_dir.ok_or_raise(|| not_found("git install dir is missing"))?; let (_prefix, path_without_trailing_slash) = self.split_at(PREFIX.len()); - let path_without_trailing_slash = - gix_path::try_from_bstring(path_without_trailing_slash).or_raise(|| { - validation("Ill-formed UTF-8 in path past %(prefix)").with_input(path_without_trailing_slash) - })?; + let path_without_trailing_slash = gix_path::from_bstring(path_without_trailing_slash).or_raise(|| { + validation("Ill-formed UTF-8 in path past %(prefix)").with_input(path_without_trailing_slash) + })?; Ok(git_install_dir.join(path_without_trailing_slash)) } else if let Some(val) = self.strip_prefix(b"~") { let (username, path) = match val.split_once_str(b"/") { @@ -187,13 +186,13 @@ impl Path { }; if let Some(path) = path { home.push( - gix_path::try_from_byte_slice(path) + gix_path::from_byte_slice(path) .or_raise(|| validation(format!("Ill-formed UTF-8 in {what}")).with_input(path))?, ); } Ok(home) } else { - Ok(gix_path::from_bstr(self.value.as_bstr()).into_owned()) + Ok(gix_path::from_bstr(self.value.as_bstr())?.into_owned()) } } diff --git a/gix-config-value/tests/value/boolean.rs b/gix-config-value/tests/value/boolean.rs index 58799158b90..147c9d9a79d 100644 --- a/gix-config-value/tests/value/boolean.rs +++ b/gix-config-value/tests/value/boolean.rs @@ -1,8 +1,8 @@ use gix_config_value::Boolean; -use gix_error::Result; +use gix_testtools::TestResult; #[test] -fn from_utf8_str() -> Result { +fn from_utf8_str() -> TestResult { assert_eq!( Boolean::try_from("yes")?, Boolean(true), @@ -12,7 +12,7 @@ fn from_utf8_str() -> Result { } #[test] -fn from_str_false() -> Result { +fn from_str_false() -> TestResult { assert!(!Boolean::try_from("no")?.0); assert!(!Boolean::try_from("off")?.0); assert!(!Boolean::try_from("false")?.0); @@ -22,7 +22,7 @@ fn from_str_false() -> Result { } #[test] -fn from_str_true() -> Result { +fn from_str_true() -> TestResult { assert!(Boolean::try_from("yes")?.0); assert!(Boolean::try_from("on")?.0); assert!(Boolean::try_from("true")?.0); @@ -33,19 +33,18 @@ fn from_str_true() -> Result { } #[test] -fn ignores_case() { +fn ignores_case() -> gix_testtools::TestResult { // Random subset for word in &["no", "yes", "on", "off", "true", "false"] { - let first: bool = Boolean::try_from(*word).expect("valid boolean").into(); - let second: bool = Boolean::try_from(word.to_uppercase().as_str()) - .expect("valid boolean") - .into(); + let first: bool = Boolean::try_from(*word)?.into(); + let second: bool = Boolean::try_from(word.to_uppercase().as_str())?.into(); assert_eq!(first, second); } + Ok(()) } #[test] -fn numbers_are_parsed_as_integers() -> Result { +fn numbers_are_parsed_as_integers() -> TestResult { // Use the same bases, suffixes, and full `i64` range as `Integer`. for (input, expected) in [ ("0x10", true), diff --git a/gix-config-value/tests/value/color.rs b/gix-config-value/tests/value/color.rs index 112e4b261d7..3a55009fb22 100644 --- a/gix-config-value/tests/value/color.rs +++ b/gix-config-value/tests/value/color.rs @@ -1,8 +1,8 @@ use gix_config_value::Color; -use gix_error::Result; +use gix_testtools::TestResult; #[test] -fn from_utf8_str() -> Result { +fn from_utf8_str() -> TestResult { assert_eq!( Color::try_from("red bold")?.to_string(), "red bold", diff --git a/gix-config-value/tests/value/integer.rs b/gix-config-value/tests/value/integer.rs index 22c886cc5be..fb95fe8c1ea 100644 --- a/gix-config-value/tests/value/integer.rs +++ b/gix-config-value/tests/value/integer.rs @@ -2,10 +2,10 @@ use std::borrow::Cow; use bstr::{BStr, BString}; use gix_config_value::{Integer, integer::Suffix}; -use gix_error::{MetadataValue, Result}; +use gix_error::MetadataValue; #[test] -fn from_utf8_str() -> Result { +fn from_utf8_str() -> gix_testtools::TestResult { assert_eq!( Integer::try_from("1k")?, Integer { @@ -70,7 +70,7 @@ fn invalid_from_str() { } #[test] -fn from_bytes_accepts_common_inputs() -> Result { +fn from_bytes_accepts_common_inputs() -> gix_testtools::TestResult { let owned = BString::from("0X800"); for actual in [ Integer::from_bytes::("2k")?, @@ -94,7 +94,7 @@ fn from_bytes_accepts_common_inputs() -> Result { } #[test] -fn from_bytes_applies_suffixes_before_converting_to_the_target() -> Result { +fn from_bytes_applies_suffixes_before_converting_to_the_target() -> gix_testtools::TestResult { for (input, expected) in [ ("12", 12), ("13k", 13 * 1024), diff --git a/gix-config-value/tests/value/path.rs b/gix-config-value/tests/value/path.rs index 311b7d04e58..ba0a67362b3 100644 --- a/gix-config-value/tests/value/path.rs +++ b/gix-config-value/tests/value/path.rs @@ -6,7 +6,7 @@ mod interpolate { use gix_config_value::path; #[test] - fn backslash_is_not_special_and_they_are_not_escaping_anything() -> Result { + fn backslash_is_not_special_and_they_are_not_escaping_anything() -> gix_testtools::TestResult { for path in [r"C:\foo\bar", "/foo/bar"] { let actual = gix_config_value::Path::from(path).interpolate(Default::default())?; assert_eq!(actual, Path::new(path)); @@ -22,42 +22,40 @@ mod interpolate { } #[test] - fn prefix_substitutes_git_install_dir() { + fn prefix_substitutes_git_install_dir() -> gix_testtools::TestResult { for git_install_dir in &["/tmp/git", r"C:\git"] { for (val, expected) in &[("%(prefix)/foo/bar", "foo/bar"), (r"%(prefix)/foo\bar", r"foo\bar")] { let expected = std::path::PathBuf::from(format!("{}{}{}", git_install_dir, std::path::MAIN_SEPARATOR, expected)); assert_eq!( - gix_config_value::Path::from(*val) - .interpolate(path::interpolate::Context { - git_install_dir: Path::new(git_install_dir).into(), - ..Default::default() - }) - .expect("valid interpolation"), + gix_config_value::Path::from(*val).interpolate(path::interpolate::Context { + git_install_dir: Path::new(git_install_dir).into(), + ..Default::default() + })?, expected, "prefix interpolation keeps separators as they are" ); } } + Ok(()) } #[test] - fn prefix_substitution_skipped_with_dot_slash() { + fn prefix_substitution_skipped_with_dot_slash() -> gix_testtools::TestResult { let path = "./%(prefix)/foo/bar"; let git_install_dir = "/tmp/git"; assert_eq!( - gix_config_value::Path::from(path) - .interpolate(path::interpolate::Context { - git_install_dir: Path::new(git_install_dir).into(), - ..Default::default() - }) - .expect("valid interpolation"), + gix_config_value::Path::from(path).interpolate(path::interpolate::Context { + git_install_dir: Path::new(git_install_dir).into(), + ..Default::default() + })?, Path::new(path) ); + Ok(()) } #[test] - fn tilde_alone_substitutes_current_user() -> Result { + fn tilde_alone_substitutes_current_user() -> gix_testtools::TestResult { let home = std::env::current_dir().expect("current directory is available"); assert_eq!( gix_config_value::Path::from("~").interpolate(path::interpolate::Context { @@ -80,8 +78,8 @@ mod interpolate { } #[test] - fn tilde_slash_substitutes_current_user() -> Result { - let home = std::env::current_dir().expect("current directory is available"); + fn tilde_slash_substitutes_current_user() -> gix_testtools::TestResult { + let home = std::env::current_dir()?; for suffix in ["", "user/bar", r"user\bar", "/user/bar"] { let actual = gix_config_value::Path::from(format!("~/{suffix}").as_str()).interpolate( path::interpolate::Context { @@ -100,7 +98,7 @@ mod interpolate { } #[test] - fn tilde_with_given_user() -> Result { + fn tilde_with_given_user() -> gix_testtools::TestResult { let mut error_snapshots = Vec::new(); let home = std::env::current_dir().expect("current directory is available"); @@ -143,7 +141,7 @@ mod interpolate { }) .expect_err("the username is not UTF-8"); insta::assert_debug_snapshot!(err, "malformed usernames are validation errors with the utf8 cause", @r#" - Ill-formed UTF-8 in username, "input"="\xff" + Ill-formed UTF-8 in username, input="\xff" Caused by: 0: invalid utf-8 sequence of 1 bytes from index 0 diff --git a/gix-config/Cargo.toml b/gix-config/Cargo.toml index 6db62239658..e28e95580a1 100644 --- a/gix-config/Cargo.toml +++ b/gix-config/Cargo.toml @@ -22,7 +22,8 @@ sha256 = ["gix-ref/sha256"] serde = ["dep:serde", "bstr/serde", "gix-sec/serde", "gix-ref/serde", "gix-glob/serde", "gix-config-value/serde"] [dependencies] -gix-features = { version = "^0.50.0", path = "../gix-features" } +gix-trace = { version = "^0.2.0", path = "../gix-trace" } +gix-parallel = { version = "^0.50.0", path = "../gix-parallel" } gix-config-value = { version = "^0.20.0", path = "../gix-config-value" } gix-error = { version = "^0.4.0", path = "../gix-error", features = ["bstr"] } gix-path = { version = "^0.13.0", path = "../gix-path" } diff --git a/gix-config/src/file/access/mutate.rs b/gix-config/src/file/access/mutate.rs index 614a751f037..a9d7067f418 100644 --- a/gix-config/src/file/access/mutate.rs +++ b/gix-config/src/file/access/mutate.rs @@ -1,6 +1,6 @@ use bstr::BStr; use gix_error::{Result, ResultExt}; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use crate::{ AsBStrOpt, File, @@ -124,7 +124,11 @@ impl File { .section_mut_from_id(id, nl) .expect("BUG: Section did not have id from lookup")) } - None => self.new_section_inner(name, subsection_name.map(bstr::BString::from)), + None => self.new_section_inner( + name, + subsection_name.map(bstr::BString::from), + OwnShared::clone(&self.meta), + ), } } @@ -173,6 +177,9 @@ impl File { /// the generated header will use the modern subsection syntax. /// Returns a reference to the new section for immediate editing. /// + /// The section inherits this file's [metadata][Self::meta()]. Use + /// [`Self::new_section_with_meta()`] to provide a different origin for just this section. + /// /// # Examples /// /// Creating a new empty section: @@ -204,11 +211,30 @@ impl File { /// # Ok::<(), Box>(()) /// ``` pub fn new_section(&mut self, name: impl AsRef, subsection: impl IntoBStringOpt) -> Result> { - self.new_section_inner(name.as_ref(), subsection.into_bstring_opt()) + self.new_section_with_meta(name, subsection, OwnShared::clone(&self.meta)) } - fn new_section_inner(&mut self, name: &str, subsection: Option) -> Result> { - let section = file::SectionData::new(name, subsection, OwnShared::clone(&self.meta), &mut self.backing)?; + /// Like [`Self::new_section()`], but attaches the given `meta`data to the new section instead of + /// inheriting this file's metadata. + /// + /// This leaves the file's metadata and that of existing sections unchanged. Subsequent sections + /// created with [`Self::new_section()`] still inherit the file's metadata. + pub fn new_section_with_meta( + &mut self, + name: impl AsRef, + subsection: impl IntoBStringOpt, + meta: impl Into>, + ) -> Result> { + self.new_section_inner(name.as_ref(), subsection.into_bstring_opt(), meta.into()) + } + + fn new_section_inner( + &mut self, + name: &str, + subsection: Option, + meta: OwnShared, + ) -> Result> { + let section = file::SectionData::new(name, subsection, meta, &mut self.backing)?; let id = self.push_section_internal(section); let nl = self.detect_newline_style_smallvec(); let mut section = self.section_mut_from_id(id, nl).expect("each id yields a section"); diff --git a/gix-config/src/file/access/read_only.rs b/gix-config/src/file/access/read_only.rs index 7bdfdd7f794..4e061f50ad3 100644 --- a/gix-config/src/file/access/read_only.rs +++ b/gix-config/src/file/access/read_only.rs @@ -1,6 +1,6 @@ use bstr::{BStr, BString, ByteSlice}; use gix_error::Result; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use smallvec::SmallVec; use crate::{ @@ -426,7 +426,7 @@ impl File { /// Return this file's metadata, typically set when it was first created to indicate its origins. /// - /// It will be used in all newly created sections to identify them. + /// Newly created sections inherit it unless created with [`File::new_section_with_meta()`]. /// Change it with [`File::set_meta()`]. pub fn meta(&self) -> &Metadata { &self.meta @@ -434,7 +434,8 @@ impl File { /// Change the origin of this instance to be the given `meta`data. /// - /// This is useful to control what origin about-to-be-added sections receive. + /// This is useful to control what origin about-to-be-added sections receive by default. + /// Use [`File::new_section_with_meta()`] to set the origin of a single new section instead. pub fn set_meta(&mut self, meta: impl Into>) -> &mut Self { self.meta = meta.into(); self diff --git a/gix-config/src/file/includes/mod.rs b/gix-config/src/file/includes/mod.rs index ea12c268c39..b02def33a80 100644 --- a/gix-config/src/file/includes/mod.rs +++ b/gix-config/src/file/includes/mod.rs @@ -2,7 +2,7 @@ use std::path::{Path, PathBuf}; use bstr::{BStr, BString, ByteSlice, ByteVec}; use gix_error::{OptionExt, Result, ResultExt, message, not_found}; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use gix_ref::Category; use crate::{ @@ -248,7 +248,7 @@ fn gitdir_matches( let git_dir = gix_path::to_unix_separators_on_windows(gix_path::into_bstr(git_dir.ok_or_raise(|| { not_found("The git directory must be provided to support `gitdir:` conditional includes") - })?)); + })?)?); let mut pattern_path = match check_interpolation_result( err_on_interpolation_failure, @@ -256,7 +256,7 @@ fn gitdir_matches( ) .or_raise(|| message("Could not interpolate conditional include path"))? { - Some(path) => gix_path::into_bstr(path).into_owned(), + Some(path) => gix_path::into_bstr(path)?.into_owned(), // Git keeps the original condition pattern when interpolation fails. None => condition_path.to_owned(), }; @@ -275,7 +275,7 @@ fn gitdir_matches( })? .parent() .expect("config path can never be /"); - let mut joined_path = gix_path::to_unix_separators_on_windows(gix_path::into_bstr(parent_dir)).into_owned(); + let mut joined_path = gix_path::to_unix_separators_on_windows(gix_path::into_bstr(parent_dir)?).into_owned(); joined_path.push(b'/'); joined_path.extend_from_slice(relative_pattern_path); pattern_path = joined_path; @@ -283,7 +283,7 @@ fn gitdir_matches( // NOTE: this special handling of leading backslash is needed to do it like git does if pattern_path.iter().next() != Some(&(std::path::MAIN_SEPARATOR as u8)) - && !gix_path::from_bstr(pattern_path.clone()).is_absolute() + && !gix_path::from_bstr(pattern_path.clone())?.is_absolute() { pattern_path.insert_str(0, "**/"); } @@ -298,9 +298,9 @@ fn gitdir_matches( } let expanded_git_dir = gix_path::to_unix_separators_on_windows(gix_path::into_bstr( - gix_path::realpath(gix_path::from_byte_slice(&git_dir)) + gix_path::realpath(gix_path::from_byte_slice(&git_dir)?) .or_raise(|| message("Could not resolve the git directory to its real path"))?, - )); + )?); Ok(gix_glob::wildmatch( pattern_path.as_bstr(), expanded_git_dir.as_ref(), diff --git a/gix-config/src/file/init/from_paths.rs b/gix-config/src/file/init/from_paths.rs index b7cb36f6388..a6992bda24c 100644 --- a/gix-config/src/file/init/from_paths.rs +++ b/gix-config/src/file/init/from_paths.rs @@ -82,7 +82,7 @@ impl File { path.display() )); if options.ignore_io_errors { - gix_features::trace::warn!("ignoring: {err:#?}"); + gix_trace::warn!("ignoring: {err:#?}"); continue; } else { return Err(err); @@ -98,7 +98,7 @@ impl File { path.display() )); if options.ignore_io_errors { - gix_features::trace::warn!("ignoring: {err:#?}"); + gix_trace::warn!("ignoring: {err:#?}"); buf.clear(); } else { return Err(err); diff --git a/gix-config/src/file/init/mod.rs b/gix-config/src/file/init/mod.rs index bec8c813b35..aca5587cd8e 100644 --- a/gix-config/src/file/init/mod.rs +++ b/gix-config/src/file/init/mod.rs @@ -1,5 +1,5 @@ use gix_error::Result; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use crate::{ File, diff --git a/gix-config/src/file/mod.rs b/gix-config/src/file/mod.rs index fc6a00bbade..56e016d4a47 100644 --- a/gix-config/src/file/mod.rs +++ b/gix-config/src/file/mod.rs @@ -6,7 +6,7 @@ use std::{ }; use bstr::BString; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; mod mutable; pub use mutable::{multi_value::MultiValueMut, section::SectionMut, value::ValueMut}; diff --git a/gix-config/src/file/section/mod.rs b/gix-config/src/file/section/mod.rs index b6b918eb0a6..91d1a3ba9ba 100644 --- a/gix-config/src/file/section/mod.rs +++ b/gix-config/src/file/section/mod.rs @@ -12,7 +12,7 @@ use crate::{ pub(crate) mod body; pub(crate) use body::BodyData; pub use body::{BodyRef, BodyRefIter}; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use crate::file::{SectionId, write::platform_newline}; diff --git a/gix-config/src/types.rs b/gix-config/src/types.rs index 923041635bc..05878cefea9 100644 --- a/gix-config/src/types.rs +++ b/gix-config/src/types.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use gix_features::threading::OwnShared; +use gix_parallel::OwnShared; use crate::{ file, diff --git a/gix-config/tests/config/file/access/mutate.rs b/gix-config/tests/config/file/access/mutate.rs index c9ee4fe6d20..3a44d1b055e 100644 --- a/gix-config/tests/config/file/access/mutate.rs +++ b/gix-config/tests/config/file/access/mutate.rs @@ -1,8 +1,8 @@ mod new_section { - use crate::Result; + use crate::TestResult; #[test] - fn accepts_a_borrowed_subsection_name() -> Result { + fn accepts_a_borrowed_subsection_name() -> TestResult { let mut file = gix_config::File::default(); file.new_section("remote", "origin")?; file.new_section("branch", "main")?; @@ -17,15 +17,89 @@ mod new_section { } #[test] - fn owned_sections_accept_a_borrowed_subsection_name() -> Result { + fn owned_sections_accept_a_borrowed_subsection_name() -> TestResult { let section = gix_config::file::Section::new("remote", "origin", gix_config::file::Metadata::default())?; assert_eq!(section.to_ref().header().subsection_name(), Some("origin".into())); Ok(()) } } +mod new_section_with_meta { + use gix_config::{File, Source, file::Metadata}; + use gix_parallel::OwnShared; + use gix_testtools::TestResult; + + #[test] + fn metadata_is_specific_to_the_new_section() -> TestResult { + let default_meta = Metadata::from(Source::Local).at("repository.config"); + let mut file = File::new(default_meta.clone()); + file.new_section("core", None)?; + let meta = Metadata { + level: 2, + ..Metadata::from(Source::User) + .at("user.config") + .with(gix_sec::Trust::Reduced) + }; + { + let mut section = file.new_section_with_meta("remote", "origin", meta.clone())?; + assert_eq!(section.meta(), &meta, "the editable section receives explicit metadata"); + section.push("url", Some("example".into()))?; + } + file.new_section_with_meta("user", None, OwnShared::new(meta.clone()))?; + file.new_section("core", None)?; + + assert_eq!( + file.meta(), + &default_meta, + "explicit metadata does not change the file's origin" + ); + assert_eq!( + file.sections().map(|section| section.meta()).collect::>(), + [&default_meta, &meta, &meta, &default_meta], + "owned and shared metadata persist without affecting existing or subsequent sections" + ); + assert_eq!( + file.section("remote", "origin")?.meta(), + &meta, + "lookup retains the section's explicit origin" + ); + let nl = if cfg!(windows) { "\r\n" } else { "\n" }; + assert_eq!( + file.to_string(), + format!("[core]{nl}[remote \"origin\"]{nl}\turl = example{nl}[user]{nl}[core]{nl}"), + "explicit metadata preserves normal section formatting and value insertion" + ); + Ok(()) + } + + #[test] + fn invalid_names_leave_the_file_unchanged() -> TestResult { + let mut file = File::default(); + file.new_section("core", None)?; + let before = file.to_string(); + for (name, subsection) in [("invalid.name", None), ("remote", Some("invalid\nsubsection"))] { + assert!( + file.new_section_with_meta(name, subsection.map(bstr::BString::from), Metadata::from(Source::Local)) + .is_err(), + "explicit metadata does not bypass section-header validation" + ); + assert_eq!( + file.to_string(), + before, + "invalid headers do not change the file's contents" + ); + assert_eq!( + file.meta(), + &Metadata::api(), + "errors do not change the file's metadata" + ); + } + Ok(()) + } +} + mod remove_section { - use crate::Result; + use crate::TestResult; #[test] fn removal_of_all_sections_programmatically_with_sections_and_ids_by_name() { @@ -53,8 +127,8 @@ mod remove_section { } #[test] - fn removal_is_complete_and_sections_can_be_read() { - let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap(); + fn removal_is_complete_and_sections_can_be_read() -> gix_testtools::TestResult { + let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2")?; assert_eq!(file.sections().count(), 2); let removed = file.remove_section("core", None).expect("removed correct section"); @@ -69,13 +143,13 @@ mod remove_section { assert_eq!(file.sections().count(), 0); assert!(file.remove_section("core", "name").is_none()); - file.section_mut_or_create_new("core", None).expect("creation succeeds"); - file.section_mut_or_create_new("core", "name") - .expect("creation succeeds"); + file.section_mut_or_create_new("core", None)?; + file.section_mut_or_create_new("core", "name")?; + Ok(()) } #[test] - fn removing_lookup_buckets_preserves_siblings_and_drops_the_final_name() -> Result { + fn removing_lookup_buckets_preserves_siblings_and_drops_the_final_name() -> TestResult { let mut file = gix_config::File::try_from( "[core] key=plain\n\ [core \"a\"] key=a\n\ @@ -99,7 +173,7 @@ mod remove_section { } #[test] - fn removed_sections_can_be_mutated_and_reinserted() -> Result { + fn removed_sections_can_be_mutated_and_reinserted() -> TestResult { let mut file = gix_config::File::try_from("[core]\na = b\n")?; let mut section = file.remove_section("core", None).expect("section is present"); let removed_id = section.to_ref().id(); @@ -116,8 +190,8 @@ mod remove_section { } mod remove_section_filter { #[test] - fn removal_of_section_is_complete() { - let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2").unwrap(); + fn removal_of_section_is_complete() -> gix_testtools::TestResult { + let mut file = gix_config::File::try_from("[core] \na = b\nb=c\n\n[core \"name\"]\nd = 1\ne = 2")?; assert_eq!(file.sections().count(), 2); let removed = file @@ -137,29 +211,29 @@ mod remove_section_filter { ); assert!(file.remove_section_filter("core", "name", |_| true).is_none()); - file.section_mut_or_create_new("core", None).expect("creation succeeds"); - file.section_mut_or_create_new("core", "name") - .expect("creation succeeds"); + file.section_mut_or_create_new("core", None)?; + file.section_mut_or_create_new("core", "name")?; + Ok(()) } } mod rename_section { - use crate::Result; + use crate::TestResult; #[test] fn section_renaming_validates_new_name() { let mut file = gix_config::File::try_from("[core] a = b").unwrap(); let err = file.rename_section("core", None, "new_core", None).unwrap_err(); assert!(err.is_validation()); - insta::assert_debug_snapshot!(err, "section renaming validates new name", @r#"section names can only be ascii, '-', "input"="new_core""#); + insta::assert_debug_snapshot!(err, "section renaming validates new name", @r#"section names can only be ascii, '-', input="new_core""#); let err = file.rename_section("core", None, "new-core", "a\nb").unwrap_err(); assert!(err.is_validation()); - insta::assert_debug_snapshot!(err, "section renaming validates new name", @r#"sub-section names must not contain newlines or null bytes, "input"="a\nb""#); + insta::assert_debug_snapshot!(err, "section renaming validates new name", @r#"sub-section names must not contain newlines or null bytes, input="a\nb""#); } #[test] - fn accepts_borrowed_new_subsection_names() -> Result { + fn accepts_borrowed_new_subsection_names() -> TestResult { let mut file = gix_config::File::try_from("[core] a = b")?; file.rename_section("core", None, "remote", "origin")?; assert_eq!( @@ -177,7 +251,7 @@ mod rename_section { } #[test] - fn all_matching_sections_are_renamed_and_target_collisions_are_preserved() -> Result { + fn all_matching_sections_are_renamed_and_target_collisions_are_preserved() -> TestResult { let mut file = gix_config::File::try_from( "[branch \"source\"] key = one\n\ [some \"gar\"] key = unrelated\n\ @@ -201,7 +275,7 @@ mod rename_section { } #[test] - fn filter_renames_every_accepted_section() -> Result { + fn filter_renames_every_accepted_section() -> TestResult { let mut file = gix_config::File::try_from( "[branch \"source\"] key = one\n\ [branch \"source\"] key = two\n\ @@ -247,7 +321,7 @@ mod rename_section { } #[test] - fn renaming_to_the_same_identity_updates_all_headers() -> Result { + fn renaming_to_the_same_identity_updates_all_headers() -> TestResult { let mut file = gix_config::File::try_from( "[branch.source] one = 1\n\ [branch.source] two = 2\n", @@ -263,7 +337,7 @@ mod rename_section { } #[test] - fn an_empty_lookup_bucket_is_reported_as_missing() -> Result { + fn an_empty_lookup_bucket_is_reported_as_missing() -> TestResult { let mut file = gix_config::File::try_from("[core] key = value\n")?; file.remove_section("core", None).expect("section exists"); let err = file.rename_section("core", None, "other", None).unwrap_err(); @@ -273,11 +347,11 @@ mod rename_section { } } mod set_meta { - use crate::Result; + use crate::TestResult; use gix_config::file; #[test] - fn affects_newly_added_sections() -> Result { + fn affects_newly_added_sections() -> TestResult { let mut file = gix_config::File::default(); let expected = &file::Metadata::api(); assert_eq!(file.meta(), expected); diff --git a/gix-config/tests/config/file/access/raw/raw_multi_value.rs b/gix-config/tests/config/file/access/raw/raw_multi_value.rs index dca64d1c664..9dcd547750c 100644 --- a/gix-config/tests/config/file/access/raw/raw_multi_value.rs +++ b/gix-config/tests/config/file/access/raw/raw_multi_value.rs @@ -1,24 +1,24 @@ -use crate::Result; +use crate::TestResult; use gix_config::File; use crate::file::bstring; #[test] -fn single_value_is_identical_to_single_value_query() -> Result { +fn single_value_is_identical_to_single_value_query() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; assert_eq!(vec![config.raw_value("core.a")?], config.raw_values("core.a")?); Ok(()) } #[test] -fn multi_value_in_section() -> Result { +fn multi_value_in_section() -> TestResult { let config = File::try_from("[core]\na=b\na=c")?; assert_eq!(config.raw_values("core.a")?, vec![bstring("b"), bstring("c")]); Ok(()) } #[test] -fn multi_value_across_sections() -> Result { +fn multi_value_across_sections() -> TestResult { let config = File::try_from( "[core]\n\ a=b\n\ @@ -33,7 +33,7 @@ fn multi_value_across_sections() -> Result { } #[test] -fn values_with_sections_identify_each_values_section_in_file_order() -> Result { +fn values_with_sections_identify_each_values_section_in_file_order() -> TestResult { let config = File::try_from( "[core]\n\ a=b\n\ @@ -62,7 +62,7 @@ fn values_with_sections_identify_each_values_section_in_file_order() -> Result { } #[test] -fn values_with_sections_filter_returns_values_from_accepted_sections() -> Result { +fn values_with_sections_filter_returns_values_from_accepted_sections() -> TestResult { let config = File::try_from( "[core]\n\ a=b\n\ @@ -93,7 +93,7 @@ fn values_with_sections_filter_returns_values_from_accepted_sections() -> Result } #[test] -fn section_not_found() -> Result { +fn section_not_found() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; let err = config.raw_values("foo.a").unwrap_err(); assert!(err.is_not_found()); @@ -102,7 +102,7 @@ fn section_not_found() -> Result { } #[test] -fn subsection_not_found() -> Result { +fn subsection_not_found() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; let err = config.raw_values("core.a.a").unwrap_err(); assert!(err.is_not_found()); @@ -111,7 +111,7 @@ fn subsection_not_found() -> Result { } #[test] -fn key_not_found() -> Result { +fn key_not_found() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; let err = config.raw_values("core.aaaaaa").unwrap_err(); assert!(err.is_not_found()); @@ -120,7 +120,7 @@ fn key_not_found() -> Result { } #[test] -fn subsection_must_be_respected() -> Result { +fn subsection_must_be_respected() -> TestResult { let config = File::try_from("[core]a=b\n[core.a]a=c")?; assert_eq!(config.raw_values("core.a")?, vec![bstring("b")]); assert_eq!(config.raw_values("core.a.a")?, vec![bstring("c")]); @@ -128,7 +128,7 @@ fn subsection_must_be_respected() -> Result { } #[test] -fn non_relevant_subsection_is_ignored() -> Result { +fn non_relevant_subsection_is_ignored() -> TestResult { let config = File::try_from("[core]\na=b\na=c\n[core]a=d\n[core]g=g")?; assert_eq!( config.raw_values("core.a")?, diff --git a/gix-config/tests/config/file/access/raw/raw_value.rs b/gix-config/tests/config/file/access/raw/raw_value.rs index d3520902b3d..920f902ee38 100644 --- a/gix-config/tests/config/file/access/raw/raw_value.rs +++ b/gix-config/tests/config/file/access/raw/raw_value.rs @@ -1,8 +1,8 @@ -use crate::Result; +use crate::TestResult; use gix_config::File; #[test] -fn single_section() -> Result { +fn single_section() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; assert_eq!(config.raw_value("core.a")?, "b"); assert_eq!(config.raw_value_by("core", None, "c")?, "d"); @@ -10,28 +10,28 @@ fn single_section() -> Result { } #[test] -fn global_property_uses_empty_section_name() -> Result { +fn global_property_uses_empty_section_name() -> TestResult { let config = File::try_from("a=b\n[core]\na=c")?; insta::assert_debug_snapshot!(config.raw_value_by("", None, "a").expect_err("these are not readable because the supporting this adds a lot of complexity"), "these are not readable because the supporting this adds a lot of complexity", @"The requested section does not exist"); Ok(()) } #[test] -fn last_one_wins_respected_in_section() -> Result { +fn last_one_wins_respected_in_section() -> TestResult { let config = File::try_from("[core]\na=b\na=d")?; assert_eq!(config.raw_value("core.a")?, "d"); Ok(()) } #[test] -fn last_one_wins_respected_across_section() -> Result { +fn last_one_wins_respected_across_section() -> TestResult { let config = File::try_from("[core]\na=b\n[core]\na=d")?; assert_eq!(config.raw_value("core.a")?, "d"); Ok(()) } #[test] -fn value_with_section_identifies_the_section_containing_the_resolved_value() -> Result { +fn value_with_section_identifies_the_section_containing_the_resolved_value() -> TestResult { let config = File::try_from( "[core]\n\ a=first\n\ @@ -51,7 +51,7 @@ fn value_with_section_identifies_the_section_containing_the_resolved_value() -> } #[test] -fn value_with_section_filter_identifies_the_section_containing_the_resolved_value() -> Result { +fn value_with_section_filter_identifies_the_section_containing_the_resolved_value() -> TestResult { let config = File::try_from( "[core]\n\ a=first\n\ @@ -73,7 +73,7 @@ fn value_with_section_filter_identifies_the_section_containing_the_resolved_valu } #[test] -fn mutable_value_filters_have_key_and_component_variants() -> Result { +fn mutable_value_filters_have_key_and_component_variants() -> TestResult { let mut config = File::try_from( "[core]\n\ a=first\n\ @@ -110,7 +110,7 @@ fn mutable_value_filters_have_key_and_component_variants() -> Result { } #[test] -fn section_not_found() -> Result { +fn section_not_found() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; let err = config.raw_value("foo.a").unwrap_err(); assert!(err.is_not_found()); @@ -119,7 +119,7 @@ fn section_not_found() -> Result { } #[test] -fn subsection_not_found() -> Result { +fn subsection_not_found() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; let err = config.raw_value("core.a.a").unwrap_err(); assert!(err.is_not_found()); @@ -128,7 +128,7 @@ fn subsection_not_found() -> Result { } #[test] -fn key_not_found() -> Result { +fn key_not_found() -> TestResult { let config = File::try_from("[core]\na=b\nc=d")?; let err = config.raw_value("core.aaaaaa").unwrap_err(); assert!(err.is_not_found()); @@ -137,19 +137,19 @@ fn key_not_found() -> Result { } #[test] -fn invalid_value_names_are_reported_by_mutable_lookups() -> Result { +fn invalid_value_names_are_reported_by_mutable_lookups() -> TestResult { let mut config = File::try_from("[core]\na=b")?; let err = config.raw_value_mut_by("core", None, "1invalid").unwrap_err(); assert!(err.is_validation()); - insta::assert_debug_snapshot!(err, "invalid value names are reported by mutable lookups", @r#"Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character., "input"="1invalid""#); + insta::assert_debug_snapshot!(err, "invalid value names are reported by mutable lookups", @r#"Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character., input="1invalid""#); let err = config.raw_values_mut_by("core", None, "contains.dot").unwrap_err(); assert!(err.is_validation()); - insta::assert_debug_snapshot!(err, "invalid value names are reported by mutable lookups", @r#"Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character., "input"="contains.dot""#); + insta::assert_debug_snapshot!(err, "invalid value names are reported by mutable lookups", @r#"Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character., input="contains.dot""#); Ok(()) } #[test] -fn subsection_must_be_respected() -> Result { +fn subsection_must_be_respected() -> TestResult { let config = File::try_from("[core]a=b\n[core.a]a=c")?; assert_eq!(config.raw_value("core.a")?, "b"); assert_eq!(config.raw_value("core.a.a")?, "c"); diff --git a/gix-config/tests/config/file/access/raw/set_existing_raw_value.rs b/gix-config/tests/config/file/access/raw/set_existing_raw_value.rs index e3e0cd5f682..134a8aa3b49 100644 --- a/gix-config/tests/config/file/access/raw/set_existing_raw_value.rs +++ b/gix-config/tests/config/file/access/raw/set_existing_raw_value.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; fn file(input: &str) -> gix_config::File { input.parse().unwrap() @@ -23,7 +23,7 @@ fn single_line() { } #[test] -fn global_property_uses_empty_section_name() -> Result { +fn global_property_uses_empty_section_name() -> TestResult { let mut file = file("a=b\n[core]\na=c"); let err = file.set_existing_raw_value_by("", None, "a", "d").unwrap_err(); insta::assert_debug_snapshot!(err, "cannot set global values", @"The requested section does not exist"); diff --git a/gix-config/tests/config/file/access/raw/set_raw_value.rs b/gix-config/tests/config/file/access/raw/set_raw_value.rs index 9f7a3a45f1d..2bdfca0cf19 100644 --- a/gix-config/tests/config/file/access/raw/set_raw_value.rs +++ b/gix-config/tests/config/file/access/raw/set_raw_value.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; fn file(input: &str) -> gix_config::File { input.parse().unwrap() @@ -51,7 +51,7 @@ fn comment_included() { } #[test] -fn non_existing_values_cannot_be_set() -> Result { +fn non_existing_values_cannot_be_set() -> TestResult { let mut file = gix_config::File::default(); file.set_raw_value_by("new", None, "key", "value")?; file.set_raw_value_by("new", "subsection", "key", "subsection-value")?; @@ -66,7 +66,7 @@ fn non_existing_values_cannot_be_set() -> Result { } #[test] -fn accepts_short_lived_keys() -> Result { +fn accepts_short_lived_keys() -> TestResult { let mut file = gix_config::File::default(); let key = String::from("new.key"); @@ -81,6 +81,6 @@ fn invalid_value_names_fail_without_creating_a_section() { let mut file = gix_config::File::default(); let err = file.set_raw_value_by("new", None, "not.valid", "value").unwrap_err(); assert!(err.is_validation()); - insta::assert_debug_snapshot!(err, "invalid value names fail without creating a section", @r#"Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character., "input"="not.valid""#); + insta::assert_debug_snapshot!(err, "invalid value names fail without creating a section", @r#"Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character., input="not.valid""#); assert_eq!(file.sections().count(), 0, "validation precedes section creation"); } diff --git a/gix-config/tests/config/file/access/read_only.rs b/gix-config/tests/config/file/access/read_only.rs index 0f5be78f0b3..b577196cddd 100644 --- a/gix-config/tests/config/file/access/read_only.rs +++ b/gix-config/tests/config/file/access/read_only.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; use std::fs; use bstr::{BString, ByteSlice}; @@ -15,7 +15,7 @@ fn lookup_error(err: gix_config::lookup::Error) -> gix_error:: } #[test] -fn typed_lookup_errors_can_be_erased() -> Result { +fn typed_lookup_errors_can_be_erased() -> TestResult { let mut error_snapshots = Vec::new(); use gix_error::ResultExt; @@ -44,19 +44,19 @@ fn typed_lookup_errors_can_be_erased() -> Result { assert!(err.is_not_found(), "erasure retains missing-value classification"); insta::assert_debug_snapshot!(error_snapshots, "typed lookup errors can be erased", @r#" [ - Booleans need to be 'no', 'off', 'false', '' or 'yes', 'on', 'true' or any number, "input"="invalid", - Integers needs to be positive or negative numbers which may have a suffix like 1k, 42, or 50G, "input"="invalid", - Colors are specific color values and their attributes, like 'brightred', or 'blue', "input"="invalid", - Booleans need to be 'no', 'off', 'false', '' or 'yes', 'on', 'true' or any number, "input"="invalid", - Integers needs to be positive or negative numbers which may have a suffix like 1k, 42, or 50G, "input"="invalid", - Colors are specific color values and their attributes, like 'brightred', or 'blue', "input"="invalid", + Booleans need to be 'no', 'off', 'false', '' or 'yes', 'on', 'true' or any number, input="invalid", + Integers needs to be positive or negative numbers which may have a suffix like 1k, 42, or 50G, input="invalid", + Colors are specific color values and their attributes, like 'brightred', or 'blue', input="invalid", + Booleans need to be 'no', 'off', 'false', '' or 'yes', 'on', 'true' or any number, input="invalid", + Integers needs to be positive or negative numbers which may have a suffix like 1k, 42, or 50G, input="invalid", + Colors are specific color values and their attributes, like 'brightred', or 'blue', input="invalid", ] "#); Ok(()) } #[test] -fn integer_accessors_apply_suffixes() -> Result { +fn integer_accessors_apply_suffixes() -> TestResult { let config = File::try_from("[core]\nvalue = -2k\nvalue = 0x10m\n")?; assert_eq!( config.integer("core.value")?, @@ -84,7 +84,7 @@ fn integer_accessors_apply_suffixes() -> Result { } #[test] -fn integer_accessors_retain_classification_and_input() -> Result { +fn integer_accessors_retain_classification_and_input() -> TestResult { for input in [ b"invalid".as_slice(), b"9223372036854775808", @@ -113,7 +113,7 @@ fn integer_accessors_retain_classification_and_input() -> Result { } #[test] -fn parsed_section_header_legacy_check_uses_backing_buffer() -> Result { +fn parsed_section_header_legacy_check_uses_backing_buffer() -> TestResult { let config = File::try_from( "[remote.origin]\n\turl = https://example.com\n[remote \"upstream\"]\n\turl = https://example.com\n", )?; @@ -127,7 +127,7 @@ fn parsed_section_header_legacy_check_uses_backing_buffer() -> Result { /// Asserts we can cast into all variants of our type #[test] -fn get_value_for_all_provided_values() -> Result { +fn get_value_for_all_provided_values() -> TestResult { let config = r#" [core] other-quoted = "hello" @@ -295,7 +295,7 @@ fn get_value_for_all_provided_values() -> Result { } #[test] -fn get_value_looks_up_all_sections_before_failing() -> Result { +fn get_value_looks_up_all_sections_before_failing() -> TestResult { let config = r#" [core] bool-explicit = false @@ -325,7 +325,7 @@ fn get_value_looks_up_all_sections_before_failing() -> Result { } #[test] -fn interpreted_values_can_be_returned_with_their_sections() -> Result { +fn interpreted_values_can_be_returned_with_their_sections() -> TestResult { let file = File::try_from( "[core]\n\ a=1\n\ @@ -360,7 +360,7 @@ fn interpreted_values_can_be_returned_with_their_sections() -> Result { } #[test] -fn section_names_are_case_insensitive() -> Result { +fn section_names_are_case_insensitive() -> TestResult { let config = "[core] a=true"; let file = File::try_from(config)?; assert_eq!( @@ -372,7 +372,7 @@ fn section_names_are_case_insensitive() -> Result { } #[test] -fn value_names_are_case_insensitive() -> Result { +fn value_names_are_case_insensitive() -> TestResult { let config = "[core] a = true A = false"; @@ -387,7 +387,7 @@ fn value_names_are_case_insensitive() -> Result { } #[test] -fn section_value_access_is_case_insensitive() -> Result { +fn section_value_access_is_case_insensitive() -> TestResult { let file = File::try_from("[core]\nMixedCase = one\nMIXEDCASE = two")?; let section = file.section("core", None)?; @@ -415,7 +415,7 @@ fn single_section() { } #[test] -fn sections_by_name() -> Result { +fn sections_by_name() -> TestResult { let config = r#" [core] repositoryformatversion = 0 @@ -434,7 +434,7 @@ fn sections_by_name() -> Result { } #[test] -fn sections_by_name_ignores_subsections_and_preserves_file_order() -> Result { +fn sections_by_name_ignores_subsections_and_preserves_file_order() -> TestResult { let config = File::try_from( "[remote] marker=plain\n\ [other] marker=unrelated\n\ @@ -462,7 +462,7 @@ fn sections_by_name_ignores_subsections_and_preserves_file_order() -> Result { } #[test] -fn unknown_section() -> Result { +fn unknown_section() -> TestResult { let config = File::default(); let err = config.section("missing", None).unwrap_err(); assert!(err.is_not_found()); @@ -598,7 +598,7 @@ fn multi_line_value_with_empty_continuation_line() { } #[test] -fn multi_line_value_starting_on_a_continuation_line_is_not_indented() -> Result { +fn multi_line_value_starting_on_a_continuation_line_is_not_indented() -> gix_testtools::TestResult { let baseline = crate::scripted_fixture_read_only("make_value_whitespace_baseline.sh")?; let baseline = fs::read(baseline.join("baseline.git"))?; let baseline = baseline @@ -607,7 +607,7 @@ fn multi_line_value_starting_on_a_continuation_line_is_not_indented() -> Result let mut records = baseline.split(|byte| *byte == 0); while let Some(description) = records.next() { - let description = std::str::from_utf8(description).expect("fixture descriptions must be valid UTF-8"); + let description = std::str::from_utf8(description)?; let config = records.next().expect("each description must be followed by a config"); let expected = records.next().expect("each config must be followed by Git's value"); let expected = BString::from(expected); @@ -627,22 +627,19 @@ fn multi_line_value_starting_on_a_continuation_line_is_not_indented() -> Result } #[test] -fn overrides_with_implicit_booleans_work_in_single_section() { +fn overrides_with_implicit_booleans_work_in_single_section() -> gix_testtools::TestResult { let config = r#" [a] b = false b "#; - let config = File::try_from(config).expect("valid config"); - assert_eq!( - config.boolean("a.b").expect("valid boolean"), - Some(true), - "empty implicit booleans " - ); + let config = File::try_from(config)?; + assert_eq!(config.boolean("a.b")?, Some(true), "empty implicit booleans "); + Ok(()) } #[test] -fn implicit_booleans_may_be_followed_by_whitespace() -> Result { +fn implicit_booleans_may_be_followed_by_whitespace() -> TestResult { for config in [ "[a]\n\tb \n", "[a]\n\tb\t\n", @@ -683,17 +680,14 @@ fn implicit_booleans_may_be_followed_by_whitespace() -> Result { } #[test] -fn overrides_with_implicit_booleans_work_across_sections() { +fn overrides_with_implicit_booleans_work_across_sections() -> gix_testtools::TestResult { let config = r#" [a] b = false [a] b "#; - let config = File::try_from(config).expect("valid config"); - assert_eq!( - config.boolean("a.b").expect("valid boolean"), - Some(true), - "empty implicit booleans " - ); + let config = File::try_from(config)?; + assert_eq!(config.boolean("a.b")?, Some(true), "empty implicit booleans "); + Ok(()) } diff --git a/gix-config/tests/config/file/impls.rs b/gix-config/tests/config/file/impls.rs index fdc101e46bd..cd9ab292837 100644 --- a/gix-config/tests/config/file/impls.rs +++ b/gix-config/tests/config/file/impls.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; use gix_config::File; #[test] @@ -71,7 +71,7 @@ fn can_reconstruct_configs_without_whitespace_in_middle() { } #[test] -fn equality_ignores_section_and_value_name_case_but_not_subsection_case() -> Result { +fn equality_ignores_section_and_value_name_case_but_not_subsection_case() -> TestResult { let mixed_case = File::try_from("[Core]\nMixedCase = value\n[Remote \"Origin\"]\nURL = location\n")?; let equivalent = File::try_from("[core]\nmixedcase = value\n[remote \"Origin\"]\nurl = location\n")?; assert_eq!(mixed_case, equivalent, "section and value names are case-insensitive"); diff --git a/gix-config/tests/config/file/init/comfort.rs b/gix-config/tests/config/file/init/comfort.rs index a7a5f4cd8f6..1542575cb79 100644 --- a/gix-config/tests/config/file/init/comfort.rs +++ b/gix-config/tests/config/file/init/comfort.rs @@ -1,11 +1,11 @@ -use crate::Result; +use crate::TestResult; use gix_config::source; use serial_test::serial; #[test] #[serial] -fn from_globals() -> Result { +fn from_globals() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?; let worktree_dir = crate::scripted_fixture_read_only("make_config_repo.sh")?.canonicalize()?; let _environment = _environment.set( @@ -23,7 +23,7 @@ fn from_globals() -> Result { #[test] #[serial] -fn from_environment_overrides() -> Result { +fn from_environment_overrides() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?.set("GIT_CONFIG_COUNT", "0"); let config = gix_config::File::from_environment_overrides()?; assert!(config.is_void()); @@ -32,7 +32,7 @@ fn from_environment_overrides() -> Result { #[test] #[serial] -fn from_git_dir() -> Result { +fn from_git_dir() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?; let worktree_dir = crate::scripted_fixture_read_only("make_config_repo.sh")?; let git_dir = worktree_dir.join(".git"); @@ -97,7 +97,7 @@ fn from_git_dir() -> Result { #[test] #[serial] -fn from_git_dir_with_worktree_extension() -> Result { +fn from_git_dir_with_worktree_extension() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?; let git_dir = crate::scripted_fixture_read_only("config_with_worktree_extension.sh")? .join("main-worktree") diff --git a/gix-config/tests/config/file/init/from_env.rs b/gix-config/tests/config/file/init/from_env.rs index e2596a3fdd1..38ef5a40078 100644 --- a/gix-config/tests/config/file/init/from_env.rs +++ b/gix-config/tests/config/file/init/from_env.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; use std::fs; use gix_config::{ @@ -12,7 +12,7 @@ use crate::file::init::from_paths::escape_backslashes; #[test] #[serial] -fn empty_without_relevant_environment() -> Result { +fn empty_without_relevant_environment() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?.unset("GIT_CONFIG_COUNT"); let config = File::from_env(Default::default())?; assert!(config.is_none()); @@ -21,7 +21,7 @@ fn empty_without_relevant_environment() -> Result { #[test] #[serial] -fn empty_with_zero_count() -> Result { +fn empty_with_zero_count() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?.set("GIT_CONFIG_COUNT", "0"); let config = File::from_env(Default::default())?; assert!(config.is_none()); @@ -30,12 +30,12 @@ fn empty_with_zero_count() -> Result { #[test] #[serial] -fn parse_error_with_invalid_count() -> Result { +fn parse_error_with_invalid_count() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?.set("GIT_CONFIG_COUNT", "invalid"); let err = File::from_env(Default::default()).expect_err("the configuration count is not an integer"); assert!(err.is_validation(), "invalid counts are validation errors"); insta::assert_debug_snapshot!(err, "parse error with invalid count", @r#" - GIT_CONFIG_COUNT was not a positive integer, "input"="invalid" + GIT_CONFIG_COUNT was not a positive integer, input="invalid" Caused by: 0: invalid digit found in string @@ -45,7 +45,7 @@ fn parse_error_with_invalid_count() -> Result { #[test] #[serial] -fn single_key_value_pair() -> Result { +fn single_key_value_pair() -> TestResult { let _environment = gix_testtools::isolate_git_environment()? .set("GIT_CONFIG_COUNT", "1") .set("GIT_CONFIG_KEY_0", "core.key") @@ -64,7 +64,7 @@ fn single_key_value_pair() -> Result { #[test] #[serial] -fn multiple_key_value_pairs() -> Result { +fn multiple_key_value_pairs() -> TestResult { let _environment = gix_testtools::isolate_git_environment()? .set("GIT_CONFIG_COUNT", "3") .set("GIT_CONFIG_KEY_0", "core.a") @@ -85,7 +85,7 @@ fn multiple_key_value_pairs() -> Result { #[test] #[serial] -fn error_on_relative_paths_in_include_paths() -> Result { +fn error_on_relative_paths_in_include_paths() -> TestResult { let _environment = gix_testtools::isolate_git_environment()? .set("GIT_CONFIG_COUNT", "1") .set("GIT_CONFIG_KEY_0", "include.path") @@ -114,7 +114,7 @@ fn error_on_relative_paths_in_include_paths() -> Result { #[test] #[serial] -fn follow_include_paths() -> Result { +fn follow_include_paths() -> TestResult { let _environment = gix_testtools::isolate_git_environment()?; let dir = tempdir().unwrap(); let a_path = dir.path().join("a"); diff --git a/gix-config/tests/config/file/init/from_paths/includes/conditional/gitdir/mod.rs b/gix-config/tests/config/file/init/from_paths/includes/conditional/gitdir/mod.rs index 7cf56ae812f..8e6e31970ac 100644 --- a/gix-config/tests/config/file/init/from_paths/includes/conditional/gitdir/mod.rs +++ b/gix-config/tests/config/file/init/from_paths/includes/conditional/gitdir/mod.rs @@ -1,6 +1,6 @@ mod util; -use crate::Result; +use crate::TestResult; use gix_testtools::Env; use serial_test::serial; use util::{Condition, GitEnv, assert_section_value}; @@ -8,31 +8,31 @@ use util::{Condition, GitEnv, assert_section_value}; use crate::file::init::from_paths::escape_backslashes; #[test] -fn relative_path_with_trailing_slash_matches_like_star_star() -> Result { - assert_section_value(Condition::new("gitdir:worktree/"), GitEnv::repo_name("worktree")?) +fn relative_path_with_trailing_slash_matches_like_star_star() -> TestResult { + assert_section_value(Condition::new("gitdir:worktree/"), GitEnv::repo_name("worktree")?).map_err(Into::into) } #[test] -fn relative_path_without_trailing_slash_does_not_match() -> Result { - assert_section_value( +fn relative_path_without_trailing_slash_does_not_match() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir:worktree").expect_original_value(), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn relative_path_without_trailing_slash_and_dot_git_suffix_matches() -> Result { - assert_section_value(Condition::new("gitdir:worktree/.git"), GitEnv::repo_name("worktree")?) +fn relative_path_without_trailing_slash_and_dot_git_suffix_matches() -> TestResult { + assert_section_value(Condition::new("gitdir:worktree/.git"), GitEnv::repo_name("worktree")?).map_err(Into::into) } #[test] -fn tilde_slash_expands_the_current_user_home() -> Result { +fn tilde_slash_expands_the_current_user_home() -> TestResult { let env = GitEnv::repo_name(std::path::Path::new("subdir").join("worktree"))?; - assert_section_value(Condition::new("gitdir:~/subdir/worktree/"), env) + assert_section_value(Condition::new("gitdir:~/subdir/worktree/"), env).map_err(Into::into) } #[test] -fn failed_user_expansion_matches_the_literal_pattern() -> Result { +fn failed_user_expansion_matches_the_literal_pattern() -> TestResult { let temp = gix_testtools::tempfile::tempdir()?; let name = format!( "~gix-config-{}", @@ -87,63 +87,63 @@ path = included.config } #[test] -fn tilde_alone_does_not_match_even_if_home_is_git_directory() -> Result { +fn tilde_alone_does_not_match_even_if_home_is_git_directory() -> TestResult { let env = GitEnv::repo_in_home()?; - assert_section_value(Condition::new("gitdir:~").expect_original_value(), env) + assert_section_value(Condition::new("gitdir:~").expect_original_value(), env).map_err(Into::into) } #[test] -fn explicit_star_star_prefix_and_suffix_match_zero_or_more_path_components() -> Result { - assert_section_value(Condition::new("gitdir:**/worktree/**"), GitEnv::repo_name("worktree")?) +fn explicit_star_star_prefix_and_suffix_match_zero_or_more_path_components() -> TestResult { + assert_section_value(Condition::new("gitdir:**/worktree/**"), GitEnv::repo_name("worktree")?).map_err(Into::into) } #[test] -fn double_slash_does_not_match() -> Result { - assert_section_value( +fn double_slash_does_not_match() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir://worktree").expect_original_value(), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn absolute_git_dir_with_os_separators_match() -> Result { - assert_section_value( +fn absolute_git_dir_with_os_separators_match() -> TestResult { + Ok(assert_section_value( original_value_on_windows(Condition::new("gitdir:$gitdir")), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn absolute_worktree_dir_with_os_separators_does_not_match_if_trailing_slash_is_missing() -> Result { - assert_section_value( +fn absolute_worktree_dir_with_os_separators_does_not_match_if_trailing_slash_is_missing() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir:$worktree").expect_original_value(), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn absolute_worktree_dir_with_os_separators_matches_with_trailing_glob() -> Result { - assert_section_value( +fn absolute_worktree_dir_with_os_separators_matches_with_trailing_glob() -> TestResult { + Ok(assert_section_value( original_value_on_windows(Condition::new(format!( "gitdir:$worktree{}**", std::path::MAIN_SEPARATOR ))), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn dot_slash_path_is_replaced_with_directory_containing_the_including_config_file() -> Result { - assert_section_value( +fn dot_slash_path_is_replaced_with_directory_containing_the_including_config_file() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir:./").set_user_config_instead_of_repo_config(), GitEnv::repo_name("worktree")?, // the user configuration is in $HOME, which is parent to $HOME/worktree, and the pattern path ends up being $HOME/**, including worktree/.git - ) + )?) } #[test] #[serial] -fn dot_slash_from_environment_causes_error() -> Result { +fn dot_slash_from_environment_causes_error() -> TestResult { let _isolated_environment = gix_testtools::isolate_git_environment()?; let env = GitEnv::repo_name("worktree")?; // Only slashes can be used as matches, even on Windows. @@ -204,101 +204,101 @@ fn dot_slash_from_environment_causes_error() -> Result { } #[test] -fn dot_dot_slash_prefixes_are_not_special_and_are_not_what_you_want() -> Result { - assert_section_value( +fn dot_dot_slash_prefixes_are_not_special_and_are_not_what_you_want() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir:../") .set_user_config_instead_of_repo_config() .expect_no_value(), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn leading_dots_are_not_special() -> Result { - assert_section_value(Condition::new("gitdir:.hidden/"), GitEnv::repo_name(".hidden")?) +fn leading_dots_are_not_special() -> TestResult { + assert_section_value(Condition::new("gitdir:.hidden/"), GitEnv::repo_name(".hidden")?).map_err(Into::into) } #[test] -fn dot_slash_path_with_dot_git_suffix_matches() -> Result { - assert_section_value( +fn dot_slash_path_with_dot_git_suffix_matches() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir:./worktree/.git").set_user_config_instead_of_repo_config(), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn globbing_and_wildcards() -> Result { - assert_section_value( +fn globbing_and_wildcards() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir:stan?ard/glo*ng/[xwz]ildcards/.git").set_user_config_instead_of_repo_config(), GitEnv::repo_name("standard/globbing/wildcards")?, - ) + )?) } #[test] -fn case_insensitive_matches_any_case() -> Result { +fn case_insensitive_matches_any_case() -> TestResult { assert_section_value(Condition::new("gitdir/i:WORKTREE/"), GitEnv::repo_name("worktree")?)?; - assert_section_value( + Ok(assert_section_value( Condition::new("gitdir:WORKTREE/").expect_original_value(), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn pattern_with_escaped_backslash() -> Result { - assert_section_value( +fn pattern_with_escaped_backslash() -> TestResult { + Ok(assert_section_value( original_value_on_windows(Condition::new(r"gitdir:\\work\\tree\\/")), GitEnv::repo_name("worktree")?, - ) + )?) } #[test] -fn pattern_with_backslash() -> Result { - assert_section_value(Condition::new(r"gitdir:work\tree/"), GitEnv::repo_name("worktree")?) +fn pattern_with_backslash() -> TestResult { + assert_section_value(Condition::new(r"gitdir:work\tree/"), GitEnv::repo_name("worktree")?).map_err(Into::into) } #[test] -fn star_star_in_the_middle() -> Result { - assert_section_value( +fn star_star_in_the_middle() -> TestResult { + Ok(assert_section_value( Condition::new("gitdir:**/dir/**/worktree/**"), GitEnv::repo_name("dir/worktree")?, - ) + )?) } #[test] #[cfg(not(windows))] -fn tilde_expansion_with_symlink() -> Result { +fn tilde_expansion_with_symlink() -> TestResult { let env = util::git_env_with_symlinked_repo()?; - assert_section_value(Condition::new("gitdir:~/worktree/"), env) + assert_section_value(Condition::new("gitdir:~/worktree/"), env).map_err(Into::into) } #[test] #[cfg(not(windows))] -fn dot_path_with_symlink() -> Result { +fn dot_path_with_symlink() -> TestResult { let env = util::git_env_with_symlinked_repo()?; - assert_section_value( + Ok(assert_section_value( Condition::new("gitdir:./symlink-worktree/.git").set_user_config_instead_of_repo_config(), env, - ) + )?) } #[test] #[cfg(not(windows))] -fn relative_path_matching_symlink() -> Result { +fn relative_path_matching_symlink() -> TestResult { let env = util::git_env_with_symlinked_repo()?; - assert_section_value( + Ok(assert_section_value( Condition::new("gitdir:symlink-worktree/").set_user_config_instead_of_repo_config(), env, - ) + )?) } #[test] #[cfg(not(windows))] -fn dot_path_matching_symlink_with_icase() -> Result { +fn dot_path_matching_symlink_with_icase() -> TestResult { let env = util::git_env_with_symlinked_repo()?; - assert_section_value( + Ok(assert_section_value( Condition::new("gitdir/i:SYMLINK-WORKTREE/").set_user_config_instead_of_repo_config(), env, - ) + )?) } fn original_value_on_windows(c: Condition) -> Condition { diff --git a/gix-config/tests/config/file/init/from_paths/includes/conditional/hasconfig.rs b/gix-config/tests/config/file/init/from_paths/includes/conditional/hasconfig.rs index 5a977ac9698..eed0dbd4ee4 100644 --- a/gix-config/tests/config/file/init/from_paths/includes/conditional/hasconfig.rs +++ b/gix-config/tests/config/file/init/from_paths/includes/conditional/hasconfig.rs @@ -1,10 +1,10 @@ -use crate::Result; +use crate::{Result, TestResult}; use std::path::{Path, PathBuf}; use gix_config::file::{includes, init}; #[test] -fn simple() -> Result { +fn simple() -> TestResult { let (config, root) = config_with_includes("basic")?; compare_baseline(&config, "user.this", root.join("expected")); assert_eq!(config.string("user.that"), None); @@ -12,7 +12,7 @@ fn simple() -> Result { } #[test] -fn inclusion_order() -> Result { +fn inclusion_order() -> TestResult { let (config, root) = config_with_includes("inclusion-order")?; for key in ["one", "two", "three"] { compare_baseline(&config, format!("user.{key}"), root.join(format!("expected.{key}"))); @@ -21,7 +21,7 @@ fn inclusion_order() -> Result { } #[test] -fn globs() -> Result { +fn globs() -> TestResult { let (config, root) = config_with_includes("globs")?; for key in ["dss", "dse", "dsm", "ssm"] { compare_baseline(&config, format!("user.{key}"), root.join(format!("expected.{key}"))); @@ -31,7 +31,7 @@ fn globs() -> Result { } #[test] -fn cycle_breaker() -> Result { +fn cycle_breaker() -> TestResult { for name in ["cycle-breaker-direct", "cycle-breaker-indirect"] { let (_config, _root) = config_with_includes(name)?; } @@ -40,7 +40,7 @@ fn cycle_breaker() -> Result { } #[test] -fn no_cycle() -> Result { +fn no_cycle() -> TestResult { let (config, root) = config_with_includes("no-cycle")?; compare_baseline(&config, "user.name", root.join("expected")); Ok(()) diff --git a/gix-config/tests/config/file/init/from_paths/includes/conditional/mod.rs b/gix-config/tests/config/file/init/from_paths/includes/conditional/mod.rs index 00c741f23d5..a3e82bc64b7 100644 --- a/gix-config/tests/config/file/init/from_paths/includes/conditional/mod.rs +++ b/gix-config/tests/config/file/init/from_paths/includes/conditional/mod.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::{Result, TestResult}; use std::{fs, path::Path, str::FromStr}; use gix_config::{ @@ -15,7 +15,7 @@ mod hasconfig; mod onbranch; #[test] -fn include_and_includeif_correct_inclusion_order_and_delayed_resolve_include() -> Result { +fn include_and_includeif_correct_inclusion_order_and_delayed_resolve_include() -> TestResult { let dir = tempdir()?; let config_path = dir.path().join("root"); let first_include_path = dir.path().join("first-incl"); diff --git a/gix-config/tests/config/file/init/from_paths/includes/conditional/onbranch.rs b/gix-config/tests/config/file/init/from_paths/includes/conditional/onbranch.rs index 3bd48eb9479..8884bf3a68e 100644 --- a/gix-config/tests/config/file/init/from_paths/includes/conditional/onbranch.rs +++ b/gix-config/tests/config/file/init/from_paths/includes/conditional/onbranch.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::{Result, TestResult}; use std::fs; use bstr::{BString, ByteSlice}; @@ -13,7 +13,7 @@ use gix_testtools::tempfile::tempdir; use crate::file::{bstring, init::from_paths::includes::conditional::git_init}; #[test] -fn literal_branch_names_match() -> Result { +fn literal_branch_names_match() -> TestResult { assert_section_value( Options { condition: "literal-match", @@ -26,7 +26,7 @@ fn literal_branch_names_match() -> Result { } #[test] -fn full_ref_names_do_not_match() -> Result { +fn full_ref_names_do_not_match() -> TestResult { assert_section_value( Options { condition: "refs/heads/simple", @@ -39,7 +39,7 @@ fn full_ref_names_do_not_match() -> Result { } #[test] -fn non_branches_never_match() -> Result { +fn non_branches_never_match() -> TestResult { assert_section_value( Options { condition: "good", @@ -52,7 +52,7 @@ fn non_branches_never_match() -> Result { } #[test] -fn patterns_ending_with_slash_match_subdirectories_recursively() -> Result { +fn patterns_ending_with_slash_match_subdirectories_recursively() -> TestResult { let mut env = GitEnv::new()?; assert_section_value( Options { @@ -83,7 +83,7 @@ fn patterns_ending_with_slash_match_subdirectories_recursively() -> Result { } #[test] -fn simple_glob_patterns() -> Result { +fn simple_glob_patterns() -> TestResult { let mut env = GitEnv::new()?; assert_section_value( Options { @@ -131,7 +131,7 @@ fn simple_glob_patterns() -> Result { } #[test] -fn simple_globs_do_not_cross_component_boundary() -> Result { +fn simple_globs_do_not_cross_component_boundary() -> TestResult { let mut env = GitEnv::new()?; assert_section_value( Options { @@ -154,7 +154,7 @@ fn simple_globs_do_not_cross_component_boundary() -> Result { } #[test] -fn double_star_globs_cross_component_boundaries() -> Result { +fn double_star_globs_cross_component_boundaries() -> TestResult { assert_section_value( Options { condition: "feature/**/start", diff --git a/gix-config/tests/config/file/init/from_paths/includes/unconditional.rs b/gix-config/tests/config/file/init/from_paths/includes/unconditional.rs index 6a1c6f71731..d80c71c50be 100644 --- a/gix-config/tests/config/file/init/from_paths/includes/unconditional.rs +++ b/gix-config/tests/config/file/init/from_paths/includes/unconditional.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; use std::fs; use gix_config::{ @@ -28,7 +28,7 @@ fn assert_include_depth(err: gix_error::Error) -> gix_error::Error { } #[test] -fn multiple() -> Result { +fn multiple() -> TestResult { let dir = tempdir()?; let a_path = dir.path().join("a"); @@ -95,7 +95,7 @@ fn multiple() -> Result { } #[test] -fn respect_max_depth() -> Result { +fn respect_max_depth() -> TestResult { let dir = tempdir()?; // 0 includes 1 - base level @@ -194,7 +194,7 @@ fn respect_max_depth() -> Result { } #[test] -fn simple() -> Result { +fn simple() -> TestResult { let dir = tempdir()?; let a_path = dir.path().join("a"); @@ -230,7 +230,7 @@ fn simple() -> Result { } #[test] -fn cycle_detection() -> Result { +fn cycle_detection() -> TestResult { let dir = tempdir()?; let a_path = dir.path().join("a"); @@ -291,7 +291,7 @@ fn cycle_detection() -> Result { } #[test] -fn nested() -> Result { +fn nested() -> TestResult { let dir = tempdir()?; let a_path = dir.path().join("a"); diff --git a/gix-config/tests/config/file/init/from_paths/mod.rs b/gix-config/tests/config/file/init/from_paths/mod.rs index a6be638197f..eb8c6489a3c 100644 --- a/gix-config/tests/config/file/init/from_paths/mod.rs +++ b/gix-config/tests/config/file/init/from_paths/mod.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; use std::{fs, path::PathBuf}; use gix_config::{File, Source}; @@ -55,7 +55,7 @@ mod from_path_no_includes { } #[test] -fn multiple_paths_single_value() -> Result { +fn multiple_paths_single_value() -> TestResult { let dir = tempdir()?; let a_path = dir.path().join("a"); @@ -83,7 +83,7 @@ fn multiple_paths_single_value() -> Result { } #[test] -fn frontmatter_is_maintained_in_multiple_files() -> Result { +fn frontmatter_is_maintained_in_multiple_files() -> TestResult { let dir = tempdir()?; let a_path = dir.path().join("a"); @@ -144,7 +144,7 @@ fn frontmatter_is_maintained_in_multiple_files() -> Result { } #[test] -fn multiple_paths_multi_value_and_filter() -> Result { +fn multiple_paths_multi_value_and_filter() -> TestResult { let dir = tempdir()?; let a_path = dir.path().join("a"); diff --git a/gix-config/tests/config/file/init/from_str.rs b/gix-config/tests/config/file/init/from_str.rs index 6ceb2464e22..46481e0f55e 100644 --- a/gix-config/tests/config/file/init/from_str.rs +++ b/gix-config/tests/config/file/init/from_str.rs @@ -1,7 +1,7 @@ -use crate::Result; +use crate::TestResult; #[test] -fn empty_yields_default_file() -> Result { +fn empty_yields_default_file() -> TestResult { let a: gix_config::File = "".parse()?; assert_eq!(a, gix_config::File::default()); assert_eq!(a.to_string(), ""); @@ -9,7 +9,7 @@ fn empty_yields_default_file() -> Result { } #[test] -fn whitespace_without_section_contains_front_matter() -> Result { +fn whitespace_without_section_contains_front_matter() -> TestResult { let input = " \t"; let a: gix_config::File = input.parse()?; assert_eq!(a.to_string(), input); diff --git a/gix-config/tests/config/file/mod.rs b/gix-config/tests/config/file/mod.rs index 1c017039269..798c6346b8f 100644 --- a/gix-config/tests/config/file/mod.rs +++ b/gix-config/tests/config/file/mod.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; use std::path::PathBuf; use bstr::BString; @@ -74,7 +74,7 @@ fn fuzzed_stackoverflow() { } #[test] -fn fuzzed_long_runtime() -> Result { +fn fuzzed_long_runtime() -> TestResult { let config = std::fs::read(fixture_path("fuzzed/long-parsetime.config"))?; let file = File::from_bytes_no_includes(&config, gix_config::file::Metadata::default(), Default::default())?; assert_eq!(file.sections().count(), 52); diff --git a/gix-config/tests/config/file/mutable/multi_value.rs b/gix-config/tests/config/file/mutable/multi_value.rs index 1f141a8783a..16d14b339ab 100644 --- a/gix-config/tests/config/file/mutable/multi_value.rs +++ b/gix-config/tests/config/file/mutable/multi_value.rs @@ -1,9 +1,9 @@ mod get { - use crate::Result; + use crate::TestResult; use crate::file::{bstring, mutable::multi_value::init_config}; #[test] - fn single_lines() -> Result { + fn single_lines() -> TestResult { let mut config = init_config(); let value = config.raw_values_mut_by("core", None, "a")?; @@ -12,7 +12,7 @@ mod get { } #[test] - fn multi_line() -> Result { + fn multi_line() -> TestResult { let mut config: gix_config::File = r#"[core] a=b\ "100" @@ -37,7 +37,7 @@ c } #[test] - fn value_names_are_case_insensitive() -> Result { + fn value_names_are_case_insensitive() -> TestResult { let mut config: gix_config::File = "[core]\nMixedCase = one\nMIXEDCASE = two".parse()?; assert_eq!( config.raw_values_mut_by("core", None, "mixedcase")?.get()?, @@ -48,11 +48,11 @@ c } mod access { - use crate::Result; + use crate::TestResult; use crate::file::mutable::multi_value::init_config; #[test] - fn non_empty_sizes() -> Result { + fn non_empty_sizes() -> TestResult { let mut config = init_config(); assert_eq!(config.raw_values_mut_by("core", None, "a")?.len(), 3); assert!(!config.raw_values_mut_by("core", None, "a")?.is_empty()); @@ -61,11 +61,11 @@ mod access { } mod set { - use crate::Result; + use crate::TestResult; use crate::file::{bstring, mutable::multi_value::init_config}; #[test] - fn values_are_escaped() -> Result { + fn values_are_escaped() -> TestResult { for value in ["a b", " a b", "a b\t", ";c", "#c", "a\nb\n\tc"] { let mut config = init_config(); let mut values = config.raw_values_mut_by("core", None, "a")?; @@ -83,7 +83,7 @@ mod set { } #[test] - fn single_at_start() -> Result { + fn single_at_start() -> TestResult { let mut config = init_config(); let mut values = config.raw_values_mut_by("core", None, "a")?; values.set_string_at(0, "Hello")?; @@ -95,7 +95,7 @@ mod set { } #[test] - fn single_at_end() -> Result { + fn single_at_end() -> TestResult { let mut config = init_config(); let mut values = config.raw_values_mut_by("core", None, "a")?; values.set_string_at(2, "Hello")?; @@ -107,7 +107,7 @@ mod set { } #[test] - fn all() -> Result { + fn all() -> TestResult { let mut config = init_config(); let mut values = config.raw_values_mut_by("core", None, "a")?; values.set_all("Hello")?; @@ -119,7 +119,7 @@ mod set { } #[test] - fn all_empty() -> Result { + fn all_empty() -> TestResult { let mut config = init_config(); let mut values = config.raw_values_mut_by("core", None, "a")?; values.set_all("")?; @@ -132,11 +132,11 @@ mod set { } mod delete { - use crate::Result; + use crate::TestResult; use crate::file::mutable::multi_value::init_config; #[test] - fn single_at_start_and_end() -> Result { + fn single_at_start_and_end() -> TestResult { let mut config = init_config(); { let mut values = config.raw_values_mut_by("core", None, "a")?; @@ -154,7 +154,7 @@ mod delete { } #[test] - fn all() -> Result { + fn all() -> TestResult { let mut config = init_config(); let mut values = config.raw_values_mut_by("core", None, "a")?; values.delete_all(); diff --git a/gix-config/tests/config/file/mutable/section.rs b/gix-config/tests/config/file/mutable/section.rs index b53f1ccd03d..4a068af9099 100644 --- a/gix-config/tests/config/file/mutable/section.rs +++ b/gix-config/tests/config/file/mutable/section.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; #[test] fn section_mut_must_exist_as_section_is_not_created_automatically() { @@ -7,7 +7,7 @@ fn section_mut_must_exist_as_section_is_not_created_automatically() { } #[test] -fn section_mut_or_create_new_is_infallible() -> Result { +fn section_mut_or_create_new_is_infallible() -> TestResult { let mut config = multi_value_section(); let section = config.section_mut_or_create_new("name", "subsection")?; assert_eq!(section.header().name(), "name"); @@ -16,7 +16,7 @@ fn section_mut_or_create_new_is_infallible() -> Result { } #[test] -fn section_mut_or_create_new_filter_may_reject_existing_sections() -> Result { +fn section_mut_or_create_new_filter_may_reject_existing_sections() -> TestResult { let mut config = multi_value_section(); let section = config.section_mut_or_create_new_filter("a", None, |_| false)?; assert_eq!(section.header().name(), "a"); @@ -40,11 +40,11 @@ fn section_mut_by_id() { } mod rename { - use crate::Result; + use crate::TestResult; use bstr::ByteSlice; #[test] - fn detached_sections_can_be_renamed() -> Result { + fn detached_sections_can_be_renamed() -> TestResult { let mut section = gix_config::file::Section::new("remote", "origin", gix_config::file::Metadata::default())?; section.to_mut().rename("branch", "main")?; @@ -55,7 +55,7 @@ mod rename { } #[test] - fn attached_sections_are_renamed_unambiguously_and_update_lookups() -> Result { + fn attached_sections_are_renamed_unambiguously_and_update_lookups() -> TestResult { let mut file = gix_config::File::try_from( "[target \"same\"] key = first\n\ [source \"old\"] key = selected\n\ @@ -93,7 +93,7 @@ mod rename { } #[test] - fn invalid_names_leave_attached_sections_unchanged() -> Result { + fn invalid_names_leave_attached_sections_unchanged() -> TestResult { let mut file = gix_config::File::try_from("[core] key = value\n")?; assert!(file.section_mut("core", None)?.rename("not_valid", None).is_err()); assert_eq!( @@ -111,10 +111,10 @@ mod rename { mod remove { use super::multi_value_section; - use crate::Result; + use crate::TestResult; #[test] - fn all() -> Result { + fn all() -> TestResult { let mut config = multi_value_section(); let mut section = config.section_mut("a", None)?; @@ -138,10 +138,10 @@ mod remove { mod pop { use super::multi_value_section; - use crate::Result; + use crate::TestResult; #[test] - fn all() -> Result { + fn all() -> TestResult { let mut config = multi_value_section(); let mut section = config.section_mut_by_key("a")?; @@ -165,10 +165,10 @@ mod pop { mod set { use super::multi_value_section; - use crate::Result; + use crate::TestResult; #[test] - fn various_escapes_onto_various_kinds_of_values() -> Result { + fn various_escapes_onto_various_kinds_of_values() -> TestResult { let mut config = multi_value_section(); let mut section = config.section_mut("a", None)?; let values = vec!["", " a", "b\t", "; comment", "a\n\tc d\\ \"x\""]; @@ -195,10 +195,10 @@ mod set { } mod value_name_validation { - use crate::Result; + use crate::TestResult; #[test] - fn mutations_validate_names_and_leave_the_section_unchanged_on_error() -> Result { + fn mutations_validate_names_and_leave_the_section_unchanged_on_error() -> TestResult { let mut config = gix_config::File::default(); let mut section = config.new_section("core", None)?; @@ -210,7 +210,7 @@ mod value_name_validation { Message { message: "Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character.", class: Validation, - values: {"input": Bytes("not.valid")}, + values: {input: Bytes("not.valid")}, } "#); let err = section @@ -223,7 +223,7 @@ mod value_name_validation { Message { message: "Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character.", class: Validation, - values: {"input": Bytes("1invalid")}, + values: {input: Bytes("1invalid")}, } "#); let err = section.set("also invalid", "value").unwrap_err(); @@ -234,7 +234,7 @@ mod value_name_validation { Message { message: "Valid value names consist of alphanumeric characters or dashes, starting with an alphabetic character.", class: Validation, - values: {"input": Bytes("also invalid")}, + values: {input: Bytes("also invalid")}, } "#); assert_eq!(section.num_values(), 0, "validation happens before mutation"); @@ -242,7 +242,7 @@ mod value_name_validation { } #[test] - fn names_returned_by_public_apis_are_strings() -> Result { + fn names_returned_by_public_apis_are_strings() -> TestResult { let mut config = super::multi_value_section(); let mut section = config.section_mut("a", None)?; let names: Vec = section.value_names().collect(); @@ -255,11 +255,11 @@ mod value_name_validation { } mod push { - use crate::Result; + use crate::TestResult; use crate::file::bstring; #[test] - fn none_as_value_omits_the_key_value_separator() -> Result { + fn none_as_value_omits_the_key_value_separator() -> TestResult { let mut file = gix_config::File::default(); let mut section = file.section_mut_or_create_new("a", "sub")?; section.push("key", None)?; @@ -275,7 +275,7 @@ mod push { } #[test] - fn whitespace_is_derived_from_whitespace_before_first_value() -> Result { + fn whitespace_is_derived_from_whitespace_before_first_value() -> TestResult { for (input, expected_pre_key, expected_sep) in [ ("[a]\n\t\tb=c", Some("\t\t".into()), (None, None)), ("[a]\nb= c", None, (None, Some(" "))), @@ -308,7 +308,7 @@ mod push { } #[test] - fn values_are_escaped() { + fn values_are_escaped() -> gix_testtools::TestResult { for (value, expected) in [ ("a b", "$head\tk = a b$nl"), (" a b", "$head\tk = \" a b\"$nl"), @@ -318,22 +318,21 @@ mod push { ("a\nb\n\tc", "$head\tk = a\\nb\\n\\tc$nl"), ] { let mut config = gix_config::File::default(); - let mut section = config.new_section("a", None).unwrap(); + let mut section = config.new_section("a", None)?; section.set_implicit_newline(false); - section - .push("k", Some(value.into())) - .expect("the fixture fits into the backing buffer"); + section.push("k", Some(value.into()))?; let expected = expected .replace("$head", &format!("[a]{nl}", nl = section.newline())) .replace("$nl", §ion.newline().to_string()); assert_eq!(config.to_bstring(), expected); } + Ok(()) } } mod push_with_comment { #[test] - fn various_comments_and_escaping() { + fn various_comments_and_escaping() -> gix_testtools::TestResult { for (comment, expected) in [ ("", "$head\tk = v #$nl"), ("this is v!", "$head\tk = v # this is v!$nl"), @@ -349,24 +348,23 @@ mod push_with_comment { ), ] { let mut config = gix_config::File::default(); - let mut section = config.new_section("a", None).unwrap(); + let mut section = config.new_section("a", None)?; section.set_implicit_newline(false); - section - .push_with_comment("k", Some("v".into()), comment) - .expect("the fixture fits into the backing buffer"); + section.push_with_comment("k", Some("v".into()), comment)?; let expected = expected .replace("$head", &format!("[a]{nl}", nl = section.newline())) .replace("$nl", §ion.newline().to_string()); assert_eq!(config.to_bstring(), expected); } + Ok(()) } } mod set_leading_whitespace { - use crate::Result; + use crate::TestResult; #[test] - fn any_whitespace_is_ok() -> Result { + fn any_whitespace_is_ok() -> TestResult { let mut config = gix_config::File::default(); let mut section = config.new_section("core", None)?; diff --git a/gix-config/tests/config/file/mutable/value.rs b/gix-config/tests/config/file/mutable/value.rs index ce175671924..c8a0d4d2dc8 100644 --- a/gix-config/tests/config/file/mutable/value.rs +++ b/gix-config/tests/config/file/mutable/value.rs @@ -1,5 +1,5 @@ mod get { - use crate::Result; + use crate::TestResult; use bstr::BString; use crate::file::mutable::value::init_config; @@ -30,7 +30,7 @@ mod get { } #[test] - fn value_is_correct() -> Result { + fn value_is_correct() -> TestResult { let mut config = init_config(); let value = config.raw_value_mut_by("core", None, "a")?; @@ -39,7 +39,7 @@ mod get { } #[test] - fn value_names_are_case_insensitive() -> Result { + fn value_names_are_case_insensitive() -> TestResult { let mut config: gix_config::File = "[core]\nMixedCase = value".parse()?; assert_eq!(config.raw_value_mut_by("core", None, "mIxEdCaSe")?.get()?, "value"); Ok(()) @@ -47,7 +47,7 @@ mod get { } mod set_string { - use crate::Result; + use crate::TestResult; use crate::file::mutable::value::init_config; fn assert_set_string(expected: &str) { @@ -127,7 +127,7 @@ mod set_string { } #[test] - fn unquoted_comments_end_continued_values_and_survive_replacement() -> Result { + fn unquoted_comments_end_continued_values_and_survive_replacement() -> TestResult { for newline in ["\n", "\r\n"] { for comment in ["# comment", "; comment"] { let mut config: gix_config::File = @@ -146,7 +146,7 @@ mod set_string { } #[test] - fn quoted_comment_markers_in_continued_values_are_value_content() -> Result { + fn quoted_comment_markers_in_continued_values_are_value_content() -> TestResult { let mut config: gix_config::File = r#"[a] k="one\ #not;comments" @@ -168,7 +168,7 @@ next=value"# } #[test] - fn simple_value_and_empty_string() -> Result { + fn simple_value_and_empty_string() -> TestResult { let mut config = init_config(); let mut value = config.raw_value_mut_by("core", None, "a")?; @@ -200,10 +200,10 @@ next=value"# mod delete { use super::init_config; - use crate::Result; + use crate::TestResult; #[test] - fn single_line_value() -> Result { + fn single_line_value() -> TestResult { let mut config = init_config(); let mut value = config.raw_value_mut_by("core", None, "a")?; @@ -223,7 +223,7 @@ mod delete { } #[test] - fn get_value_after_deleted() -> Result { + fn get_value_after_deleted() -> TestResult { let mut config = init_config(); let mut value = config.raw_value_mut_by("core", None, "a")?; @@ -233,7 +233,7 @@ mod delete { } #[test] - fn set_string_after_deleted() -> Result { + fn set_string_after_deleted() -> TestResult { let mut config = init_config(); let mut value = config.raw_value_mut_by("core", None, "a")?; @@ -252,7 +252,7 @@ mod delete { } #[test] - fn idempotency() -> Result { + fn idempotency() -> TestResult { let mut config = init_config(); let mut value = config.raw_value_mut_by("core", None, "a")?; @@ -267,7 +267,7 @@ mod delete { } #[test] - fn multi_line_value() -> Result { + fn multi_line_value() -> TestResult { let mut config: gix_config::File = r#"[core] a=b"100"\ c\ diff --git a/gix-config/tests/config/file/resolve_includes.rs b/gix-config/tests/config/file/resolve_includes.rs index a91ec08a5c2..f6a990501cf 100644 --- a/gix-config/tests/config/file/resolve_includes.rs +++ b/gix-config/tests/config/file/resolve_includes.rs @@ -1,8 +1,8 @@ -use crate::Result; +use crate::TestResult; use gix_config::{file, file::init}; #[test] -fn missing_includes_are_ignored_by_default() -> Result { +fn missing_includes_are_ignored_by_default() -> TestResult { let input = r#" [include] path = /etc/absolute/missing.config diff --git a/gix-config/tests/config/file/write.rs b/gix-config/tests/config/file/write.rs index 5355c47257b..52bac857977 100644 --- a/gix-config/tests/config/file/write.rs +++ b/gix-config/tests/config/file/write.rs @@ -1,4 +1,4 @@ -use crate::Result; +use crate::TestResult; use bstr::ByteVec; use gix_config::file::{Metadata, init}; @@ -17,7 +17,7 @@ fn empty_sections_roundtrip() { } #[test] -fn empty_sections_with_comments_roundtrip() { +fn empty_sections_with_comments_roundtrip() -> gix_testtools::TestResult { let input = r#"; pre-a [a] # side a ; post a @@ -27,14 +27,11 @@ fn empty_sections_with_comments_roundtrip() { [d] # side d "#; - let mut config = gix_config::File::try_from(input).unwrap(); + let mut config = gix_config::File::try_from(input)?; let mut single_string = config.to_bstring(); assert_eq!(single_string, input); assert_eq!( - config - .append(config.clone()) - .expect("the small fixture fits into the backing buffer") - .to_string(), + config.append(config.clone())?.to_string(), { let clone = single_string.clone(); single_string.push_str(&clone); @@ -42,16 +39,15 @@ fn empty_sections_with_comments_roundtrip() { }, "string-duplication is the same as data structure duplication" ); + Ok(()) } #[test] -fn decoded_subsection_names_keep_their_raw_spelling() { +fn decoded_subsection_names_keep_their_raw_spelling() -> gix_testtools::TestResult { let input = r#"[remote "single \t \0"] "#; - let config = gix_config::File::try_from(input).expect("valid config"); - let section = config - .section("remote", Some("single t 0".into())) - .expect("the decoded subsection name is used for lookup"); + let config = gix_config::File::try_from(input)?; + let section = config.section("remote", Some("single t 0".into()))?; assert_eq!( section.header().subsection_name(), @@ -63,6 +59,7 @@ fn decoded_subsection_names_keep_their_raw_spelling() { input, "serialization uses the original raw subsection spelling" ); + Ok(()) } #[test] @@ -78,7 +75,7 @@ fn inserted_newlines_use_each_sections_newline_style() { } #[test] -fn crlf_after_a_comment_is_detected_and_used_for_insertions() -> Result { +fn crlf_after_a_comment_is_detected_and_used_for_insertions() -> TestResult { let input = "; root\r\n[core]\nkey=value\n"; let mut config = gix_config::File::try_from(input)?; assert_eq!( @@ -164,12 +161,12 @@ fn complex_lossless_roundtrip() { } mod to_filter { - use crate::Result; + use crate::TestResult; use bstr::ByteSlice; use gix_config::file::Metadata; #[test] - fn allows_only_selected_sections() -> Result { + fn allows_only_selected_sections() -> TestResult { let mut config = gix_config::File::new(Metadata::api()); config.set_raw_value_by("a", None, "b", "c")?; diff --git a/gix-config/tests/config/main.rs b/gix-config/tests/config/main.rs index dafdcecf942..fa1586a6cb9 100644 --- a/gix-config/tests/config/main.rs +++ b/gix-config/tests/config/main.rs @@ -1,4 +1,4 @@ -pub use gix_testtools::{Result, scripted_fixture_read_only}; +pub use gix_testtools::{Result, TestResult, scripted_fixture_read_only}; mod file; mod format; diff --git a/gix-config/tests/config/parse/from_bytes.rs b/gix-config/tests/config/parse/from_bytes.rs index 1afbfc01d08..985a75fb7be 100644 --- a/gix-config/tests/config/parse/from_bytes.rs +++ b/gix-config/tests/config/parse/from_bytes.rs @@ -16,12 +16,11 @@ fn fuzz() { } #[test] -fn filters_receive_event_refs_for_content_access() { +fn filters_receive_event_refs_for_content_access() -> gix_testtools::TestResult { fn reject_drop_values(event: EventRef<'_>) -> bool { !matches!(event, EventRef::Value(value) if value == b"drop".as_slice()) } - let events = Events::from_bytes(b"[core]\nkeep = keep\ndrop = drop\n", Some(reject_drop_values)) - .expect("content-based filters can inspect event bytes through views"); + let events = Events::from_bytes(b"[core]\nkeep = keep\ndrop = drop\n", Some(reject_drop_values))?; assert!( !events @@ -29,6 +28,7 @@ fn filters_receive_event_refs_for_content_access() { .any(|event| matches!(event, EventRef::Value(value) if value == b"drop".as_slice())), "the filter can reject events by inspecting their value bytes" ); + Ok(()) } #[test] diff --git a/gix-config/tests/config/parse/section.rs b/gix-config/tests/config/parse/section.rs index c2ddd20a4da..b96036942c3 100644 --- a/gix-config/tests/config/parse/section.rs +++ b/gix-config/tests/config/parse/section.rs @@ -19,18 +19,18 @@ mod header { } mod write_to { - use crate::Result; + use crate::TestResult; use crate::parse::section::header::serialized; #[test] - fn subsection_backslashes_and_quotes_are_escaped() -> Result { + fn subsection_backslashes_and_quotes_are_escaped() -> TestResult { assert_eq!(serialized("core", r"a\b")?, r#"[core "a\\b"]"#); assert_eq!(serialized("core", r#"a:"b""#)?, r#"[core "a:\"b\""]"#); Ok(()) } #[test] - fn everything_is_allowed() -> Result { + fn everything_is_allowed() -> TestResult { assert_eq!(serialized("core", "a/b \t\t a\\b")?, "[core \"a/b \t\t a\\\\b\"]"); Ok(()) } @@ -49,10 +49,10 @@ mod header { } insta::assert_debug_snapshot!(message_diagnostics, "names must be mostly ascii", @r#" [ - section names can only be ascii, '-', "input"="🤗", - section names can only be ascii, '-', "input"="x.y", - section names can only be ascii, '-', "input"="x y", - section names can only be ascii, '-', "input"="x\ny", + section names can only be ascii, '-', input="🤗", + section names can only be ascii, '-', input="x.y", + section names can only be ascii, '-', input="x y", + section names can only be ascii, '-', input="x\ny", ] "#); } @@ -68,8 +68,8 @@ mod header { } insta::assert_debug_snapshot!(message_diagnostics, "subsections with newlines and null bytes are rejected", @r#" [ - sub-section names must not contain newlines or null bytes, "input"="a\nb", - sub-section names must not contain newlines or null bytes, "input"="a\0b", + sub-section names must not contain newlines or null bytes, input="a\nb", + sub-section names must not contain newlines or null bytes, input="a\0b", ] "#); } diff --git a/gix-credentials/Cargo.toml b/gix-credentials/Cargo.toml index 19d181a9db7..af07e17837a 100644 --- a/gix-credentials/Cargo.toml +++ b/gix-credentials/Cargo.toml @@ -32,6 +32,7 @@ gix-trace = { version = "^0.2.0", path = "../gix-trace" } serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"] } bstr = { version = "1.12.0", default-features = false, features = ["std"] } +percent-encoding = "2.3.1" diff --git a/gix-credentials/src/helper/cascade.rs b/gix-credentials/src/helper/cascade.rs index ef9d3af400f..ae68187914f 100644 --- a/gix-credentials/src/helper/cascade.rs +++ b/gix-credentials/src/helper/cascade.rs @@ -40,7 +40,11 @@ impl Cascade { } else { None } - .map(|name| vec![Program::from_custom_definition(name)]) + .map(|name| { + vec![Program::from_kind(crate::program::Kind::ExternalName { + name_and_args: name.into(), + })] + }) .unwrap_or_default() } } diff --git a/gix-credentials/src/program/mod.rs b/gix-credentials/src/program/mod.rs index 658b817a453..24795baa5b4 100644 --- a/gix-credentials/src/program/mod.rs +++ b/gix-credentials/src/program/mod.rs @@ -4,6 +4,7 @@ use std::{ }; use bstr::{BString, ByteSlice, ByteVec}; +use gix_error::Result; use crate::{Program, helper}; @@ -12,15 +13,15 @@ fn external_name_command( git_program: &Path, name_and_args: &bstr::BStr, action: &helper::Action, -) -> std::process::Command { - let git_program = gix_path::to_unix_separators_on_windows(gix_path::into_bstr(git_program)); +) -> Result { + let git_program = gix_path::to_unix_separators_on_windows(gix_path::into_bstr(git_program)?); let mut args = gix_quote::single(git_program.as_ref()); args.push_str(" credential-"); args.push_str(name_and_args); - gix_command::prepare(gix_path::from_bstr(args.as_bstr()).into_owned()) + gix_command::prepare(gix_path::from_bstr(args.as_bstr())?.into_owned()) .arg(action.as_arg(true)) .command_may_be_shell_script_allow_manual_argument_splitting() - .into() + .try_into() } /// The kind of helper program to use. @@ -58,8 +59,10 @@ impl Program { /// Parse the given input as per the custom helper definition, supporting `!