Skip to content

Can't run/debug tests in VS Code for crates gix-testtools depends on #1989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
EliahKagan opened this issue May 4, 2025 · 4 comments · Fixed by #1992
Closed

Can't run/debug tests in VS Code for crates gix-testtools depends on #1989

EliahKagan opened this issue May 4, 2025 · 4 comments · Fixed by #1992
Labels
acknowledged an issue is accepted as shortcoming to be fixed help wanted Extra attention is needed

Comments

@EliahKagan
Copy link
Member

EliahKagan commented May 4, 2025

Current behavior 😯

Some crates appear twice in the dependency tree. These are the crates that are part of the workspace and also depended on by gix-testtools, either directly or transitively. For those crates, cargo commands that specify a crate name with -p do not easily work, due to the ambiguity of which crate is intended. For example, gix-date is a transitive dependency of gix-testtools, so this does not work:

ek in 🌐 catenary in gitoxide on  main is 📦 v0.44.0 via 🦀 v1.86.0
❯ cargo nextest run -p gix-date
error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
Please re-run this command with one of the following specifications:
  path+file:///home/ek/repos/gitoxide/gix-date#0.10.1
  registry+https://github.com/rust-lang/crates.io-index#[email protected]
error: command `/home/ek/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101

They can be made to work by giving cargo enough information to select the correct crate, but this is cumbersome, it is not always obvious how to do this, and it seems to vary across different commands, and possibly even sometimes not to be feasible.

For commands run manually or through a script in this project that contains the command, this problem is only a minor inconvenience. This is because the command can be run without -p in a subdirectory. For example, I can run cargo nextest on gix-date like this:

ek in 🌐 catenary in gitoxide on  main is 📦 v0.44.0 via 🦀 v1.86.0 took 7s
❯ (cd gix-date; cargo nextest run)
    Finished `test` profile [unoptimized + debuginfo] target(s) in 0.16s
────────────
 Nextest run ID f978e57c-afa3-4b4c-9989-ad1240a49829 with nextest profile: default
    Starting 31 tests across 2 binaries
        PASS [   0.003s] gix-date parse::relative::tests::two_weeks_ago
        PASS [   0.003s] gix-date::date time::format::iso8601_strict
...

This is long-standing. In a number of places in ci.yml and the justfile, where we would prefer to use <cmd> -p <crate> and may even previously have done so (before <crate> made its way into the gix-testtools dependency tree by being added as a dependency of something it dpeends on), this workaround is used already. That is, we already have a number of uses of (cd <crate-dir>; <cmd>), or a variant such as (cd <crate-dir> && <cmd>). This approach is fully general.

The limitation of this workaround, though, is that sometimes we do not write the command. If the command is in an external script that we do not control, or if it is synthesized by an editor/IDE, then there is no easy workaround. The latter case is a practical problem, because VS Code with rust-analyzer composes commands that attempt but fail to run tests. Running tests through VS Code is something that would be convenient to do. The more important functionality, however, is to debug tests through VS Code, since then the editor integration really helps. Both are broken for tests of crates that are present in the workspace and also depended on directly or transitively by gix-testtools.

For example, this shows an attempt to run the fuzz::invalid_but_does_not_cause_panic test in the gix-date test suite in the debugger in VS Code:

Screenshot of an attempt to run the test in the VS Code debugger, showing the code, a breakpoint, and the error message about multiple gix-date packages being available

The message is analogous to the one shown above:

2025-05-04 18:30:19.625 [error] error: There are multiple `gix-date` packages in your project, and the specification `[email protected]` is ambiguous.
Please re-run this command with one of the following specifications:
  path+file:///home/ek/repos/gitoxide/gix-date#0.10.1
  registry+https://github.com/rust-lang/crates.io-index#[email protected]

2025-05-04 18:30:19.628 [error] Cargo invocation has failed: Error: exit code: 101.

Arguably this is a bug in the rust-analyzer extension or some other component used in VS Code. A workaround is to debug the test in RustRover instead.

However, this will happen anytime -p <crate> is used in a command that is not easily modified, either because it is not clear how to modify it (in practice, applying those "specifications" is not always as straightforward as the error message makes it sound), or because the command is synthesized by software and hard or impossible to customize.

Expected behavior 🤔

Commonly used techniques for running tests in a crate within a workspace should be allowed to work as expected, if at all possible.

If it is reasonable to deduplicate the crates in the workspace dependency tree, then that would fix this. I had long assumed that this was the only possible fix, and thus not gotten around to reporting this before. After all, gix-testtools very intentionally depended on older SemVer-breaking versions of its gix-* dependencies. However, since gix-testtools v0.16.1, the versions are now intentionally the same (#1972).

Assuming we are able to keep it that way, it opens the option of depending not only on the same version, but also on the same code. The distinction between what version is used and what code is used is irrelevant outside of the gitoxide workspace. That is, it is irrelevant for project that are separate from gitoxide and that use gix-testtools from crates.io. But this distinction is relevant in crates developed here. When path = is not specified in Cargo.toml, the code from the workspace is not used, even if the versions would match.

Furthermore, it looks like this may--even separately from this issue--be intended. The commit message in 9b12d50, which appears in the v0.16.1 changelog, summarizes the change with the phrase "unify the dependency graph by choosing the right versions". But this actually does not unify the dependency graph. It unifies the versions, but the dependency is still duplicated due to the distinction between the published crate and the workspace crate.

If I understand correctly, the current behavior is that gix-testtools depends on gix-* crates as code published at crates.io, without including changes in the workspace versions (i.e. changes committed to the repository) since then. Assuming that is the case, truly unifying them by adding path = "../../gix-X" in the gix-testtools manifest for each gix-X dependency gix-testtools would have some other advantages besides fixing this issue, but also may have some disadvantages.

I will open a PR that makes this change, to demonstrate that it does actually work, which can then either be merged if the change is suitable, or closed without merging if not. Although there are other tradeoffs to consider, which I'll try to cover in the PR, the main concern I have about this relates to #1972 (comment): would this change further complicate things in the event that cargo-smart-release is not adapted to allow releasing to proceed without extra steps?

Git behavior

Not applicable.

Steps to reproduce 🕹

See "Current behavior" above for details.

The quick reproducer is to see if cargo nextest run -p gix-date works when run from the workspace root.

EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 4, 2025
`gix-testtools` depends on several other `gix-*` crates. Before
version 0.16.1 (GitoxideLabs#1972), `gix-testtools` depended on prior breaking
versions of those crates (as discussed in GitoxideLabs#1510 and GitoxideLabs#1886). Since
then, it depends on the current versions.

When depending on a strictly earlier version, it was necessary to
omit `path =` in the `gix-testtools` manifest for its `gix-*`
dependencies. Now that `gix-testtools` depends on current versions
of those dependencies, it seems feasible to specify both `version`
and `path`, as we do in other cases where one crate developed in
this workspace depends on another crate developed in the workspace.

Aside from improving general consistency (which is a weak rationale
here, since the role of `gix-testools` differs substantially from
that of other `gix-*` crates, in terms of how we're ourselves using
it), the broad benefits here are that:

- Ambiguity in what crate is meant, when an operation is performed
  on a specific `gix-*` crate, is lessened, or maybe even
  eliminated.

- Because the code of the dependency comes from the workspace when
  applicable, i.e. when `gix-testtools` is itself being used in the
  workspace, it should allow new not-yet-published functionality to
  be leveraged in `gix-testtools`, without confusion or breakage.

Before this, some actions we'd prefer to do by `<cmd> -p <crate>`
had to be done by `(cd <crate-dir>; <cmd>)`. This was needed to
operate on `gix-*` crates in the workspace that are also
dependencies, even transitively, of `gix-testtools`.

This affected some commands in `justfile` recipes, some commands
run in CI workflows (indirectly via `just`, or directly in script
steps), and some operations carried out manually. This included
`cargo nextest run` and `cargo check` on various crates.

Here's an example (shown on Windows, but this problem was not
specific to Windows) using `gix-date`, which is not listed in
`tests/tools/Cargo.toml`, but which is a transitive dependency:

    C:\Users\ek\source\repos\gitoxide [main ≡]> cargo nextest run -p gix-date
        Blocking waiting for file lock on package cache
    error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
    Please re-run this command with one of the following specifications:
      path+file:///C:/Users/ek/source/repos/gitoxide/gix-date#0.10.1
      registry+https://github.com/rust-lang/crates.io-index#[email protected]
    error: command `'\\?\C:\Users\ek\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe' test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101

An important special case is that of editor/IDE integration,
especially in VS Code. This couldn't run and (more significantly,
in view of the benefit of integration) couldn't debug some of the
tests.

This happened because synthesized `cargo test -p ...` commands,
used behind the scenes to launch the tests, were ambiguous. For
further details, see GitoxideLabs#1989.

Another benefit is that the lockfile and dependency tree are
simpler, and the dependency tree is truly unified.

However, that points to an important aspect of this change, which
is more than a refactoring and will affect test behavior:

- It shouldn't produce different behavior when `gix-testtools` is
  obtained from crates.io (i.e. when projects developed outside the
  `gitoxide` repository use `gix-testtools`), it can produce
  different behavior here, where `gix-testtools` will use changes
  to its `gix-*` dependencies (and accordingly their own
  dependencies, recursively) that are present in the workspace even
  if not present in the released version that matches `version =`.

- That could be a good thing if it causes new changes to be
  exercised more and earlier. That might help find bugs.

- This is also desirable in that it allows feature changes and
  bugfixes in `gix-*` crates to be used immediately in
  `gix-testtools`, before either those `gix-*` crates or
  `gix-testtools` are published with the changes (GitoxideLabs#1886). But...

- It could be bad if it introduces an undesirable dependency
  ordering for fixing bugs and/or introducing regression tests.

  That is, in principle there could arise two (possibly related)
  bugs, A and B, where there is some reason to fix A before B, but
  where B must be fixed in order for the regression test for A to
  run (to validate that it can catch A), due to B breaking
  `gix-testtools` as used in the test for A or in other tests in
  the crate affected by A.

  Because this would presumably be known--an error would occur,
  likely when building the tests--it could be worked around by
  temporarily (or permanently) reverting this change if and when
  such a problem ever arises, or partially undoing it for the
  specific affected `gix-*` dependency of `gix-testtools`.

- It could be bad if a bug affects a `gix-*` crate and its own
  tests in identical or complementary ways, and this is used to
  establish or check an expectation.

  That is, in principle there could arise a bug in a `gix-*` crate
  that `gix-testtools` uses, and that itself uses `gix-testtools`
  in its tests, that causes a test that should catch that bug
  (either initially or to verify a bugfix) to wrongly report that
  the code is working.

  This scenario is a case of the general problem that duplicated
  logic between code and its tests can cause a bug to appear
  (either in the same form or in different forms) in both, such
  that tests that should catch the bug don't catch it because they
  suffer from the same bug. In the hypothetical case imagined here,
  the duplication of logic would arise from the tests calling and
  using the very code that is under test.

  For the way we are currently using or likely ever to use
  `gix-testtools`, it seems like this would probably not happen.
  But it is hard to be completely sure. Unlike the previously
  described scenario, if this scenario did occur, it would likely
  not be noticed.

Both those scenarios have corresponding scenarios that had already
applied (and which the change here at least slightly *mitigates*):
if the code with the bug has already been published.

Fixes GitoxideLabs#1886
Fixes GitoxideLabs#1989
EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 4, 2025
`gix-testtools` depends on several other `gix-*` crates. Before
version 0.16.1 (GitoxideLabs#1972), `gix-testtools` depended on prior breaking
versions of those crates (as discussed in GitoxideLabs#1510 and GitoxideLabs#1886). Since
then, it depends on the current versions.

When depending on a strictly earlier version, it was necessary to
omit `path =` in the `gix-testtools` manifest for its `gix-*`
dependencies. Now that `gix-testtools` depends on current versions
of those dependencies, it seems feasible to specify both `version`
and `path`, as we do in other cases where one crate developed in
this workspace depends on another crate developed in the workspace.

Aside from improving general consistency (which is a weak rationale
here, since the role of `gix-testools` differs substantially from
that of other `gix-*` crates, in terms of how we're ourselves using
it), the broad benefits here are that:

- Ambiguity in what crate is meant, when an operation is performed
  on a specific `gix-*` crate, is lessened, or maybe even
  eliminated.

- Because the code of the dependency comes from the workspace when
  applicable, i.e. when `gix-testtools` is itself being used in the
  workspace, it should allow new not-yet-published functionality to
  be leveraged in `gix-testtools`, without confusion or breakage.

Before this, some actions we'd prefer to do by `<cmd> -p <crate>`
had to be done by `(cd <crate-dir>; <cmd>)`. This was needed to
operate on `gix-*` crates in the workspace that are also
dependencies, even transitively, of `gix-testtools`.

This affected some commands in `justfile` recipes, some commands
run in CI workflows (indirectly via `just`, or directly in script
steps), and some operations carried out manually. This included
`cargo nextest run` and `cargo check` on various crates.

Here's an example (shown on Windows, but this problem was not
specific to Windows) using `gix-date`, which is not listed in
`tests/tools/Cargo.toml`, but which is a transitive dependency:

    C:\Users\ek\source\repos\gitoxide [main ≡]> cargo nextest run -p gix-date
        Blocking waiting for file lock on package cache
    error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
    Please re-run this command with one of the following specifications:
      path+file:///C:/Users/ek/source/repos/gitoxide/gix-date#0.10.1
      registry+https://github.com/rust-lang/crates.io-index#[email protected]
    error: command `'\\?\C:\Users\ek\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe' test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101

An important special case is that of editor/IDE integration,
especially in VS Code. This couldn't run and (more significantly,
in view of the benefit of integration) couldn't debug some of the
tests.

This happened because synthesized `cargo test -p ...` commands,
used behind the scenes to launch the tests, were ambiguous. For
further details, see GitoxideLabs#1989.

Another benefit is that the lockfile and dependency tree are
simpler, and the dependency tree is truly unified.

That points to an important aspect of this change, which is more
than a refactoring and will affect test behavior:

- It shouldn't produce different behavior when `gix-testtools` is
  obtained from crates.io (i.e. when projects developed outside the
  `gitoxide` repository use `gix-testtools`), it can produce
  different behavior here, where `gix-testtools` will use changes
  to its `gix-*` dependencies (and accordingly their own
  dependencies, recursively) that are present in the workspace even
  if not present in the released version that matches `version =`.

- That could be a good thing if it causes new changes to be
  exercised more and earlier. That might help find bugs.

- This is also desirable in that it allows feature changes and
  bugfixes in `gix-*` crates to be used immediately in
  `gix-testtools`, before either those `gix-*` crates or
  `gix-testtools` are published with the changes (GitoxideLabs#1886). But...

However:

- It could be bad if it introduces an undesirable dependency
  ordering for fixing bugs and/or introducing regression tests.

  That is, in principle there could arise two (possibly related)
  bugs, A and B, where there is some reason to fix A before B, but
  where B must be fixed in order for the regression test for A to
  run (to validate that it can catch A), due to B breaking
  `gix-testtools` as used in the test for A or in other tests in
  the crate affected by A.

  Because this would presumably be known--an error would occur,
  likely when building the tests--it could be worked around by
  temporarily (or permanently) reverting this change if and when
  such a problem ever arises, or partially undoing it for the
  specific affected `gix-*` dependency of `gix-testtools`.

- It could be bad if a bug affects a `gix-*` crate and its own
  tests in identical or complementary ways, and this is used to
  establish or check an expectation.

  That is, in principle there could arise a bug in a `gix-*` crate
  that `gix-testtools` uses, and that itself uses `gix-testtools`
  in its tests, that causes a test that should catch that bug
  (either initially or to verify a bugfix) to wrongly report that
  the code is working.

  This scenario is a case of the general problem that duplicated
  logic between code and its tests can cause a bug to appear
  (either in the same form or in different forms) in both, such
  that tests that should catch the bug don't catch it because they
  suffer from the same bug. In the hypothetical case imagined here,
  the duplication of logic would arise from the tests calling and
  using the very code that is under test.

  For the way we are currently using or likely ever to use
  `gix-testtools`, it seems like this would probably not happen.
  But it is hard to be completely sure. Unlike the previously
  described scenario, if this scenario did occur, it would likely
  not be noticed.

Both those problem scenarios have corresponding scenarios that had
already applied (and which the change here at least slightly
*mitigates*): if the code with the bug has already been published.

Fixes GitoxideLabs#1886
Fixes GitoxideLabs#1989
EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 4, 2025
`gix-testtools` depends on several other `gix-*` crates. Before
version 0.16.1 (GitoxideLabs#1972), `gix-testtools` depended on prior breaking
versions of those crates (as discussed in GitoxideLabs#1510 and GitoxideLabs#1886). Since
then, it depends on the current versions.

When depending on a strictly earlier version, it was necessary to
omit `path =` in the `gix-testtools` manifest for its `gix-*`
dependencies. Now that `gix-testtools` depends on current versions
of those dependencies, it seems feasible to specify both `version`
and `path`, as we do in other cases where one crate developed in
this workspace depends on another crate developed in the workspace.

Aside from improving general consistency (which is a weak rationale
here, since the role of `gix-testools` differs substantially from
that of other `gix-*` crates, in terms of how we're ourselves using
it), the broad benefits here are that:

1. Ambiguity in what crate is meant, when an operation is performed
   on a specific `gix-*` crate, is lessened, or maybe even
   eliminated.

2. Because the code of the dependency comes from the workspace when
   applicable, i.e. when `gix-testtools` is itself being used in
   the workspace, it should allow new not-yet-published
   functionality to be leveraged in `gix-testtools`, without
   confusion or breakage.

Before this, some actions we'd prefer to do by `<cmd> -p <crate>`
had to be done by `(cd <crate-dir>; <cmd>)`. This was needed to
operate on `gix-*` crates in the workspace that are also
dependencies, even transitively, of `gix-testtools`.

This affected some commands in `justfile` recipes, some commands
run in CI workflows (indirectly via `just`, or directly in script
steps), and some operations carried out manually. This included
`cargo nextest run` and `cargo check` on various crates.

Here's an example (shown on Windows, but this problem was not
specific to Windows) using `gix-date`, which is not listed in
`tests/tools/Cargo.toml`, but which is a transitive dependency:

    C:\Users\ek\source\repos\gitoxide [main ≡]> cargo nextest run -p gix-date
        Blocking waiting for file lock on package cache
    error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
    Please re-run this command with one of the following specifications:
      path+file:///C:/Users/ek/source/repos/gitoxide/gix-date#0.10.1
      registry+https://github.com/rust-lang/crates.io-index#[email protected]
    error: command `'\\?\C:\Users\ek\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe' test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101

An important special case is that of editor/IDE integration,
especially in VS Code. This couldn't run and (more significantly,
in view of the benefit of integration) couldn't debug some of the
tests.

This happened because synthesized `cargo test -p ...` commands,
used behind the scenes to launch the tests, were ambiguous. For
further details, see GitoxideLabs#1989.

Another benefit is that the lockfile and dependency tree are
simpler, and the dependency tree is truly unified.

That points to an important aspect of this change, which is more
than a refactoring and will affect test behavior:

- It shouldn't produce different behavior when `gix-testtools` is
  obtained from crates.io (i.e. when projects developed outside the
  `gitoxide` repository use `gix-testtools`), it can produce
  different behavior here, where `gix-testtools` will use changes
  to its `gix-*` dependencies (and accordingly their own
  dependencies, recursively) that are present in the workspace even
  if not present in the released version that matches `version =`.

- That could be a good thing if it causes new changes to be
  exercised more and earlier. That might help find bugs.

- This is also desirable in that it allows feature changes and
  bugfixes in `gix-*` crates to be used immediately in
  `gix-testtools`, before either those `gix-*` crates or
  `gix-testtools` are published with the changes (GitoxideLabs#1886). But...

However:

- It could be bad if it introduces an undesirable dependency
  ordering for fixing bugs and/or introducing regression tests.

  That is, in principle there could arise two (possibly related)
  bugs, A and B, where there is some reason to fix A before B, but
  where B must be fixed in order for the regression test for A to
  run (to validate that it can catch A), due to B breaking
  `gix-testtools` as used in the test for A or in other tests in
  the crate affected by A.

  Because this would presumably be known--an error would occur,
  likely when building the tests--it could be worked around by
  temporarily (or permanently) reverting this change if and when
  such a problem ever arises, or partially undoing it for the
  specific affected `gix-*` dependency of `gix-testtools`.

- It could be bad if a bug affects a `gix-*` crate and its own
  tests in identical or complementary ways, and this is used to
  establish or check an expectation.

  That is, in principle there could arise a bug in a `gix-*` crate
  that `gix-testtools` uses, and that itself uses `gix-testtools`
  in its tests, that causes a test that should catch that bug
  (either initially or to verify a bugfix) to wrongly report that
  the code is working.

  This scenario is a case of the general problem that duplicated
  logic between code and its tests can cause a bug to appear
  (either in the same form or in different forms) in both, such
  that tests that should catch the bug don't catch it because they
  suffer from the same bug. In the hypothetical case imagined here,
  the duplication of logic would arise from the tests calling and
  using the very code that is under test.

  For the way we are currently using or likely ever to use
  `gix-testtools`, it seems like this would probably not happen.
  But it is hard to be completely sure. Unlike the previously
  described scenario, if this scenario did occur, it would likely
  not be noticed.

Both those problem scenarios have corresponding scenarios that had
already applied (and which the change here at least slightly
*mitigates*): if the code with the bug has already been published.

Fixes GitoxideLabs#1886
Fixes GitoxideLabs#1989
EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 5, 2025
`gix-testtools` depends on several other `gix-*` crates. Before
version 0.16.1 (GitoxideLabs#1972), `gix-testtools` depended on prior breaking
versions of those crates (as discussed in GitoxideLabs#1510 and GitoxideLabs#1886). Since
then, it depends on the current versions.

When depending on a strictly earlier version, it was necessary to
omit `path =` in the `gix-testtools` manifest for its `gix-*`
dependencies. Now that `gix-testtools` depends on current versions
of those dependencies, it seems feasible to specify both `version`
and `path`, as we do in other cases where one crate developed in
this workspace depends on another crate developed in the workspace.

Aside from improving general consistency (which is a weak rationale
here, since the role of `gix-testools` differs substantially from
that of other `gix-*` crates, in terms of how we're ourselves using
it), the broad benefits here are that:

1. Ambiguity in what crate is meant, when an operation is performed
   on a specific `gix-*` crate, is lessened, or maybe even
   eliminated. (GitoxideLabs#1989)

2. Because the code of the dependency comes from the workspace when
   applicable, i.e. when `gix-testtools` is itself being used in
   the workspace, it should allow new not-yet-published
   functionality to be leveraged in `gix-testtools`, without
   confusion or breakage. (GitoxideLabs#1886)

Before this, some actions we'd prefer to do by `<cmd> -p <crate>`
had to be done by `(cd <crate-dir>; <cmd>)`. This was needed to
operate on `gix-*` crates in the workspace that are also
dependencies, even transitively, of `gix-testtools`.

This affected some commands in `justfile` recipes, some commands
run in CI workflows (indirectly via `just`, or directly in script
steps), and some operations carried out manually. This included
`cargo nextest run` and `cargo check` on various crates.

Here's an example (shown on Windows, but this problem was not
specific to Windows) using `gix-date`, which is not listed in
`tests/tools/Cargo.toml`, but which is a transitive dependency:

    C:\Users\ek\source\repos\gitoxide [main ≡]> cargo nextest run -p gix-date
        Blocking waiting for file lock on package cache
    error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
    Please re-run this command with one of the following specifications:
      path+file:///C:/Users/ek/source/repos/gitoxide/gix-date#0.10.1
      registry+https://github.com/rust-lang/crates.io-index#[email protected]
    error: command `'\\?\C:\Users\ek\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe' test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101

An important special case is that of editor/IDE integration,
especially in VS Code. This couldn't run and (more significantly,
in view of the benefit of integration) couldn't debug some of the
tests.

This happened because synthesized `cargo test -p ...` commands,
used behind the scenes to launch the tests, were ambiguous. For
further details, see GitoxideLabs#1989.

Another benefit is that the lockfile and dependency tree are
simpler, and the dependency tree is truly unified.

That points to an important aspect of this change, which is more
than a refactoring and will affect test behavior:

- It shouldn't produce different behavior when `gix-testtools` is
  obtained from crates.io (i.e. when projects developed outside the
  `gitoxide` repository use `gix-testtools`), it can produce
  different behavior here, where `gix-testtools` will use changes
  to its `gix-*` dependencies (and accordingly their own
  dependencies, recursively) that are present in the workspace even
  if not present in the released version that matches `version =`.

- That could be a good thing if it causes new changes to be
  exercised more and earlier. That might help find bugs.

- This is also desirable in that it allows feature changes and
  bugfixes in `gix-*` crates to be used immediately in
  `gix-testtools`, before either those `gix-*` crates or
  `gix-testtools` are published with the changes (GitoxideLabs#1886). But...

However:

- It could be bad if it introduces an undesirable dependency
  ordering for fixing bugs and/or introducing regression tests.

  That is, in principle there could arise two (possibly related)
  bugs, A and B, where there is some reason to fix A before B, but
  where B must be fixed in order for the regression test for A to
  run (to validate that it can catch A), due to B breaking
  `gix-testtools` as used in the test for A or in other tests in
  the crate affected by A.

  Because this would presumably be known--an error would occur,
  likely when building the tests--it could be worked around by
  temporarily (or permanently) reverting this change if and when
  such a problem ever arises, or partially undoing it for the
  specific affected `gix-*` dependency of `gix-testtools`.

- It could be bad if a bug affects a `gix-*` crate and its own
  tests in identical or complementary ways, and this is used to
  establish or check an expectation.

  That is, in principle there could arise a bug in a `gix-*` crate
  that `gix-testtools` uses, and that itself uses `gix-testtools`
  in its tests, that causes a test that should catch that bug
  (either initially or to verify a bugfix) to wrongly report that
  the code is working.

  This scenario is a case of the general problem that duplicated
  logic between code and its tests can cause a bug to appear
  (either in the same form or in different forms) in both, such
  that tests that should catch the bug don't catch it because they
  suffer from the same bug. In the hypothetical case imagined here,
  the duplication of logic would arise from the tests calling and
  using the very code that is under test.

  For the way we are currently using or likely ever to use
  `gix-testtools`, it seems like this would probably not happen.
  But it is hard to be completely sure. Unlike the previously
  described scenario, if this scenario did occur, it would likely
  not be noticed.

Both those problem scenarios have corresponding scenarios that had
already applied (and which the change here at least slightly
*mitigates*): if the code with the bug has already been published.

Fixes GitoxideLabs#1886
Fixes GitoxideLabs#1989
EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 5, 2025
`gix-testtools` depends on several other `gix-*` crates. Before
version 0.16.1 (GitoxideLabs#1972), `gix-testtools` depended on prior breaking
versions of those crates (as discussed in GitoxideLabs#1510 and GitoxideLabs#1886). Since
then, it depends on the current versions.

When depending on a strictly earlier version, it was necessary to
omit `path =` in the `gix-testtools` manifest for its `gix-*`
dependencies. Now that `gix-testtools` depends on current versions
of those dependencies, it seems feasible to specify both `version`
and `path`, as we do in other cases where one crate developed in
this workspace depends on another crate developed in the workspace.

Aside from improving general consistency (which is a weak rationale
here, since the role of `gix-testtools` differs substantially from
that of other `gix-*` crates, in terms of how we're ourselves using
it), the broad benefits here are that:

1. Ambiguity in what crate is meant, when an operation is performed
   on a specific `gix-*` crate, is lessened, or maybe even
   eliminated. (GitoxideLabs#1989)

2. Because the code of the dependency comes from the workspace when
   applicable, i.e. when `gix-testtools` is itself being used in
   the workspace, it should allow new not-yet-published
   functionality to be leveraged in `gix-testtools`, without
   confusion or breakage. (GitoxideLabs#1886)

Before this, some actions we'd prefer to do by `<cmd> -p <crate>`
had to be done by `(cd <crate-dir>; <cmd>)`. This was needed to
operate on `gix-*` crates in the workspace that are also
dependencies, even transitively, of `gix-testtools`.

This affected some commands in `justfile` recipes, some commands
run in CI workflows (indirectly via `just`, or directly in script
steps), and some operations carried out manually. This included
`cargo nextest run` and `cargo check` on various crates.

Here's an example (shown on Windows, but this problem was not
specific to Windows) using `gix-date`, which is not listed in
`tests/tools/Cargo.toml`, but which is a transitive dependency:

    C:\Users\ek\source\repos\gitoxide [main ≡]> cargo nextest run -p gix-date
        Blocking waiting for file lock on package cache
    error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
    Please re-run this command with one of the following specifications:
      path+file:///C:/Users/ek/source/repos/gitoxide/gix-date#0.10.1
      registry+https://github.com/rust-lang/crates.io-index#[email protected]
    error: command `'\\?\C:\Users\ek\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe' test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101

An important special case is that of editor/IDE integration,
especially in VS Code. This couldn't run and (more significantly,
in view of the benefit of integration) couldn't debug some of the
tests.

This happened because synthesized `cargo test -p ...` commands,
used behind the scenes to launch the tests, were ambiguous. For
further details, see GitoxideLabs#1989.

Another benefit is that the lockfile and dependency tree are
simpler, and the dependency tree is truly unified.

That points to an important aspect of this change, which is more
than a refactoring and will affect test behavior:

- It shouldn't produce different behavior when `gix-testtools` is
  obtained from crates.io (i.e. when projects developed outside the
  `gitoxide` repository use `gix-testtools`), it can produce
  different behavior here, where `gix-testtools` will use changes
  to its `gix-*` dependencies (and accordingly their own
  dependencies, recursively) that are present in the workspace even
  if not present in the released version that matches `version =`.

- That could be a good thing if it causes new changes to be
  exercised more and earlier. That might help find bugs.

- This is also desirable in that it allows feature changes and
  bugfixes in `gix-*` crates to be used immediately in
  `gix-testtools`, before either those `gix-*` crates or
  `gix-testtools` are published with the changes (GitoxideLabs#1886). But...

However:

- It could be bad if it introduces an undesirable dependency
  ordering for fixing bugs and/or introducing regression tests.

  That is, in principle there could arise two (possibly related)
  bugs, A and B, where there is some reason to fix A before B, but
  where B must be fixed in order for the regression test for A to
  run (to validate that it can catch A), due to B breaking
  `gix-testtools` as used in the test for A or in other tests in
  the crate affected by A.

  Because this would presumably be known--an error would occur,
  likely when building the tests--it could be worked around by
  temporarily (or permanently) reverting this change if and when
  such a problem ever arises, or partially undoing it for the
  specific affected `gix-*` dependency of `gix-testtools`.

- It could be bad if a bug affects a `gix-*` crate and its own
  tests in identical or complementary ways, and this is used to
  establish or check an expectation.

  That is, in principle there could arise a bug in a `gix-*` crate
  that `gix-testtools` uses, and that itself uses `gix-testtools`
  in its tests, that causes a test that should catch that bug
  (either initially or to verify a bugfix) to wrongly report that
  the code is working.

  This scenario is a case of the general problem that duplicated
  logic between code and its tests can cause a bug to appear
  (either in the same form or in different forms) in both, such
  that tests that should catch the bug don't catch it because they
  suffer from the same bug. In the hypothetical case imagined here,
  the duplication of logic would arise from the tests calling and
  using the very code that is under test.

  For the way we are currently using or likely ever to use
  `gix-testtools`, it seems like this would probably not happen.
  But it is hard to be completely sure. Unlike the previously
  described scenario, if this scenario did occur, it would likely
  not be noticed.

Both those problem scenarios have corresponding scenarios that had
already applied (and which the change here at least slightly
*mitigates*): if the code with the bug has already been published.

Fixes GitoxideLabs#1886
Fixes GitoxideLabs#1989
EliahKagan added a commit to EliahKagan/gitoxide that referenced this issue May 5, 2025
`gix-testtools` depends on several other `gix-*` crates. Before
version 0.16.1 (GitoxideLabs#1972), `gix-testtools` depended on prior breaking
versions of those crates (as discussed in GitoxideLabs#1510 and GitoxideLabs#1886). Since
then, it depends on the current versions.

When depending on a strictly earlier version, it was necessary to
omit `path =` in the `gix-testtools` manifest for its `gix-*`
dependencies. Now that `gix-testtools` depends on current versions
of those dependencies, it seems feasible to specify both `version`
and `path`, as we do in other cases where one crate developed in
this workspace depends on another crate developed in the workspace.

Aside from improving general consistency (which is a weak rationale
here, since the role of `gix-testtools` differs substantially from
that of other `gix-*` crates, in terms of how we're ourselves using
it), the broad benefits here are that:

1. Ambiguity in what crate is meant, when an operation is performed
   on a specific `gix-*` crate, is lessened, or maybe even
   eliminated. (GitoxideLabs#1989)

2. Because the code of the dependency comes from the workspace when
   applicable, i.e. when `gix-testtools` is itself being used in
   the workspace, it should allow new not-yet-published
   functionality to be leveraged in `gix-testtools`, without
   confusion or breakage. (GitoxideLabs#1886)

Before this, some actions we'd prefer to do by `<cmd> -p <crate>`
had to be done by `(cd <crate-dir>; <cmd>)`. This was needed to
operate on `gix-*` crates in the workspace that are also
dependencies, even transitively, of `gix-testtools`.

This affected some commands in `justfile` recipes, some commands
run in CI workflows (indirectly via `just`, or directly in script
steps), and some operations carried out manually. This included
`cargo nextest run` and `cargo check` on various crates.

Here's an example (shown on Windows, but this problem was not
specific to Windows) using `gix-date`, which is not listed in
`tests/tools/Cargo.toml`, but which is a transitive dependency:

    C:\Users\ek\source\repos\gitoxide [main ≡]> cargo nextest run -p gix-date
        Blocking waiting for file lock on package cache
    error: There are multiple `gix-date` packages in your project, and the specification `gix-date` is ambiguous.
    Please re-run this command with one of the following specifications:
      path+file:///C:/Users/ek/source/repos/gitoxide/gix-date#0.10.1
      registry+https://github.com/rust-lang/crates.io-index#[email protected]
    error: command `'\\?\C:\Users\ek\.rustup\toolchains\stable-x86_64-pc-windows-msvc\bin\cargo.exe' test --no-run --message-format json-render-diagnostics --package gix-date` exited with code 101

An important special case is that of editor/IDE integration,
especially in VS Code. This couldn't run and (more significantly,
in view of the benefit of integration) couldn't debug some of the
tests.

This happened because synthesized `cargo test -p ...` commands,
used behind the scenes to launch the tests, were ambiguous. For
further details, see GitoxideLabs#1989.

Another benefit is that the lockfile and dependency tree are
simpler, and the dependency tree is truly unified.

That points to an important aspect of this change, which is more
than a refactoring and will affect test behavior:

- It shouldn't produce different behavior when `gix-testtools` is
  obtained from crates.io (i.e. when projects developed outside the
  `gitoxide` repository use `gix-testtools`), it can produce
  different behavior here, where `gix-testtools` will use changes
  to its `gix-*` dependencies (and accordingly their own
  dependencies, recursively) that are present in the workspace even
  if not present in the released version that matches `version =`.

- That could be a good thing if it causes new changes to be
  exercised more and earlier. That might help find bugs.

- This is also desirable in that it allows feature changes and
  bugfixes in `gix-*` crates to be used immediately in
  `gix-testtools`, before either those `gix-*` crates or
  `gix-testtools` are published with the changes (GitoxideLabs#1886). But...

However:

- It could be bad if it introduces an undesirable dependency
  ordering for fixing bugs and/or introducing regression tests.

  That is, in principle there could arise two (possibly related)
  bugs, A and B, where there is some reason to fix A before B, but
  where B must be fixed in order for the regression test for A to
  run (to validate that it can catch A), due to B breaking
  `gix-testtools` as used in the test for A or in other tests in
  the crate affected by A.

  Because this would presumably be known--an error would occur,
  likely when building the tests--it could be worked around by
  temporarily (or permanently) reverting this change if and when
  such a problem ever arises, or partially undoing it for the
  specific affected `gix-*` dependency of `gix-testtools`.

- It could be bad if a bug affects a `gix-*` crate and its own
  tests in identical or complementary ways, and this is used to
  establish or check an expectation.

  That is, in principle there could arise a bug in a `gix-*` crate
  that `gix-testtools` uses, and that itself uses `gix-testtools`
  in its tests, that causes a test that should catch that bug
  (either initially or to verify a bugfix) to wrongly report that
  the code is working.

  This scenario is a case of the general problem that duplicated
  logic between code and its tests can cause a bug to appear
  (either in the same form or in different forms) in both, such
  that tests that should catch the bug don't catch it because they
  suffer from the same bug. In the hypothetical case imagined here,
  the duplication of logic would arise from the tests calling and
  using the very code that is under test.

  For the way we are currently using or likely ever to use
  `gix-testtools`, it seems like this would probably not happen.
  But it is hard to be completely sure. Unlike the previously
  described scenario, if this scenario did occur, it would likely
  not be noticed.

Both those problem scenarios have corresponding scenarios that had
already applied (and which the change here at least slightly
*mitigates*): if the code with the bug has already been published.

Fixes GitoxideLabs#1886
Fixes GitoxideLabs#1989
@Byron Byron added help wanted Extra attention is needed acknowledged an issue is accepted as shortcoming to be fixed labels May 5, 2025
@Byron
Copy link
Member

Byron commented May 5, 2025

Thanks a lot for writing up such a detailed analysis of the matter. Due to using RustRover I never encountered the ambiguity issues while inside of the IDE, but have run into them a couple of times when invoking -p commands manually. Writing scripts, fortunately, is still supported by preferring cd over -p, but there are undoubtedly many third-party scripts which use -p that wouldn’t work here.

It is also true that the commit message claiming a unification of the dependency graph is made by mistake as gix-testtools still uses the released versions of the current crates while everything else in the workspace uses the workspace crates.

I predict that once it’s time to release I will have to reduce the versions of the crates that gix-testtools depends on if I don’t have enough time or don’t manage to make CSR work with it. That should, at least temporarily, fix the issues described here, despite still being a workaround.
Ideally, path-dependencies can be used in gix-testtools as well, which could be enabled by yet another CSR upgrade, one whose feasibility is unclear as well.

@EliahKagan
Copy link
Member Author

EliahKagan commented May 5, 2025

I predict that once it’s time to release I will have to reduce the versions of the crates that gix-testtools depends on if I don’t have enough time or don’t manage to make CSR work with it. That should, at least temporarily, fix the issues described here, despite still being a workaround. Ideally, path-dependencies can be used in gix-testtools as well, which could be enabled by yet another CSR upgrade, one whose feasibility is unclear as well.

Thanks. I've gone ahead and opened #1992, the PR I had said I'd open above, even though based on what you've said, it sounds like that might be valuable only as an experiment rather than being ready to merge at this time (or maybe even at all). I have no objection if that is closed.

@Byron Byron closed this as completed in 7e057f2 May 5, 2025
@EliahKagan
Copy link
Member Author

EliahKagan commented May 5, 2025

By the way, I wonder if this -p issue happened when working in Zed. I think that uses rust-analyzer for Rust, but I am not sure. Currently I have no system on which I can run that. I recommend only checking if you have it or it is otherwise of interest. I thought to mention it because it has recently gained features that may be relevant to look at in terms of what, if anything, then do to operate in an environment where git commands, or commands that are configured to run in git configuration, work properly.

(I have not forgotten #1862 (comment) and I shall reply to it in more depth, hopefully soon. This idea about looking at what Zed is doing does not really speak to that directly, I don't think, but only to the earlier related idea mentioned at the bottom of #1752 (review) of looking at what VS Code is doing.)

@Byron
Copy link
Member

Byron commented May 5, 2025

Thanks for the hint! I never really checked out Zed's Git implementation beyond seeing that they don't go with Gitoxide.

On another note, GitButler is now back to using the Git shell on Windows to run GPG through, while running it directly on Unix systems. The latter works by running $SHELL -i -l -c env to use the interactive login shell environment variables in the current process. Thus far this works well, and there seems to be some compliance around -i -l -e as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants