Skip to content
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

tooling for better copy detection #1529

Merged
merged 4 commits into from
Aug 20, 2024
Merged

tooling for better copy detection #1529

merged 4 commits into from
Aug 20, 2024

Conversation

Byron
Copy link
Member

@Byron Byron commented Aug 19, 2024

A PR to add tooling and a test created with it to faciliate changes to the copy-detection algorithm.

Related to #1524

Tasks

  • royal-copy - degenerative copy from index with pathspecs
    • that away, the content of relevant files can be approximated and should still diff similarly
  • content-to-script - here-documents for all blobs of the listed trees
  • setup a fixture by manually creating indices of the respective tree, and commit them in order
  • create a tracked diff and assert on its renames (as they are now)

Documenting the current behaviour would allow changes to be expressed and then implemented.
Unfortuantely, for reasons of convenience, the test is on a higher level than the implementation has to be.

@Byron Byron force-pushed the better-copy-detection branch 2 times, most recently from 7251db7 to f67249b Compare August 19, 2024 19:54
Byron added 2 commits August 20, 2024 09:24
…IDEs.

Given it's marginal effect on the binary size, the negative effects on RustRover
at least, and the added complexity, I think that ultimately it's not worth keeping,
unfortunately.
Its main purpose is to help build test-cases from real-world repositories.
@Byron Byron force-pushed the better-copy-detection branch from f67249b to 4cf6d96 Compare August 20, 2024 11:25
@@ -378,6 +379,84 @@ mod track_rewrites {
Ok(())
}

#[test]
fn jj_realistic_needs_to_be_more_clever() -> crate::Result {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the complexity of setting up the diffing in the tracker, I decided to add the test on gix level instead of on plumbing level, where everything is far more 'simulated'.

The benefit is that here one can define the final result exactly as one would want to see it, but with the downside that the implementation actually happens elsewhere, i.e. in the tracker, which has its own set of tests which should probably also be amended with boiled-down versions of this once the algorithm is adjusted.

One assume that that the algorithm changes will already change the outcome of the existing tests.

… shell script.

The shell-script will reproduce the repository, as long as the history is linear.
@Byron Byron force-pushed the better-copy-detection branch from 4cf6d96 to 1c59e82 Compare August 20, 2024 11:38
@Byron Byron marked this pull request as ready for review August 20, 2024 11:38
@Byron Byron force-pushed the better-copy-detection branch from 1c59e82 to 7a17894 Compare August 20, 2024 12:04
The pathspec list to reduce the set of files was generated with

```
git diff 47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3~1 47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3  --stat --no-renames --name-only
```

The assets and script to reproduce the fixture was created with:

```
cargo run -p internal-tools -- git-to-sh -c2 /Users/byron/dev/github.com/martinvonz/jj assets/jj-trackcopy-1 47bd6f4aa4a7eeef8b01ce168c6c771bdfffcbd3 CHANGELOG.md cli/src/commands/cat.rs cli/src/commands/chmod.rs cli/src/commands/file/chmod.rs cli/src/commands/file/mod.rs cli/src/commands/file/print.rs cli/src/commands/mod.rs cli/tests/[email protected] cli/tests/runner.rs cli/tests/test_acls.rs cli/tests/test_cat_command.rs cli/tests/test_chmod_command.rs cli/tests/test_diffedit_command.rs cli/tests/test_file_chmod_command.rs cli/tests/test_file_print_command.rs cli/tests/test_fix_command.rs cli/tests/test_global_opts.rs cli/tests/test_immutable_commits.rs cli/tests/test_move_command.rs cli/tests/test_new_command.rs cli/tests/test_squash_command.rs cli/tests/test_unsquash_command.rs
```

It's notable that such issues are currently expected as the tracker
implementation isn't as precise as the one in Git, which tracks more
than one candidate.
@Byron Byron force-pushed the better-copy-detection branch from 7a17894 to 7ef1e88 Compare August 20, 2024 14:37
@Byron Byron merged commit 7b7902e into main Aug 20, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant