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

Update gix to the latest released version for bugfixes. #5657

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

Byron
Copy link
Collaborator

@Byron Byron commented Nov 24, 2024

The fix include:

  • ability to find references that are all uppercase.
  • tree_merge_options() now correctly configure rename tracking.
    • Previously it was deactivated entirely, which would yield worse conflict resolution.

Tasks

  • upgrade to latest release
  • basic performance testing
    • previously rename-tracking wasn't enabled by accident, let's assure this is a free as I think it is. However, I disabled it in all places were possible just to be sure.
    • Most time is still spent in git2 tree-diffing, the next big area of oxidisation.
  • fix CI - needs help: failure due to disk-space exhaustion. Must delete cache named v0-rust-unix-rust-testing-v2-f3cec62a-230fc39e and it should go through.

Notes for the Reviewer

  • It would probably be good to make a test-release for Windows to assure that the lates [email protected] works. Otherwise it could be downgraded with cargo update --precise 1.1.16 -p [email protected].
  • One commit blanket-updates all dependencies with cargo update in order to fix a CI failure on Windows that happens in seemingly unrelated dependencies. This could probably be solved by updating only the right dependency, but generally a blanket update is probably more of a benefit than a disadvantage.

Even on master, when running branch status in the CLI on the gitlab repository, it runs into an error condition.

     Running `target/release/gitbutler-cli -C /Users/byron/dev-gb/gitlab -d branch status`
INFO     gix::dirwalk [ 333ms | 0.32% / 100.00% ]
DEBUG    ┝━ 🐛 [debug]:  | longest_prefix: None | prefix_dir: "" | patterns: []
INFO     ┕━ walk [ 332ms | 99.68% ] root: "/Users/byron/dev-gb/gitlab" | worktree_root: "/Users/byron/dev-gb/gitlab" | options: Options { precompose_unicode: true, ignore_case: false, recurse_repositories: false, emit_pruned: false, emit_ignored: None, for_deletion: None, classify_untracked_bare_repositories: false, emit_tracked: false, emit_untracked: Matching, emit_empty_directories: false, emit_collapsed: None, symlinks_to_directories_are_ignored_like_directories: false, worktree_relative_worktree_dirs: None }
DEBUG       ┕━ 🐛 [debug]:  | statistics: Outcome { read_dir_calls: 16931, returned_entries: 0, seen_entries: 86219 }
INFO     cli-op [ 139s | 0.05% / 100.00% ]
INFO     ┝━ ThreadSafeRepository::discover() [ 2.88ms | 0.00% / 0.00% ]
INFO     │  ┕━ open_from_paths() [ 2.31ms | 0.00% / 0.00% ]
INFO     │     ┕━ gix_odb::Store::at() [ 338µs | 0.00% ]
DEBUG    ┕━ list_virtual_branches_cached [ 139s | 0.00% / 99.95% ]
DEBUG       ┕━ get_applied_status_cached [ 139s | 95.29% / 99.95% ]
DEBUG          ┝━ get_workspace_head [ 125ms | 0.03% / 0.09% ]
INFO           │  ┝━ ThreadSafeRepository::open() [ 250µs | 0.00% / 0.00% ]
INFO           │  │  ┕━ open_from_paths() [ 198µs | 0.00% / 0.00% ]
INFO           │  │     ┕━ gix_odb::Store::at() [ 52.2µs | 0.00% ]
INFO           │  ┝━ gix_index::File::at() [ 9.83ms | 0.01% ]
INFO           │  ┝━ gix_merge::tree [ 57.5ms | 0.04% ] base_tree: Sha1(6ab5488d84bf3076ca5e2d1771a9ec62ced02652) | our_tree: Sha1(6ab5488d84bf3076ca5e2d1771a9ec62ced02652) | their_tree: Sha1(7430f4864a09cef50dda8d45e1469f31e7aa0bc2) | labels: Labels { ancestor: Some("base"), current: Some("ours"), other: Some("theirs") }
INFO           │  ┕━ gix_merge::tree [ 9.94ms | 0.01% ] base_tree: Sha1(6ab5488d84bf3076ca5e2d1771a9ec62ced02652) | our_tree: Sha1(7430f4864a09cef50dda8d45e1469f31e7aa0bc2) | their_tree: Sha1(6ab5488d84bf3076ca5e2d1771a9ec62ced02652) | labels: Labels { ancestor: Some("base"), current: Some("ours"), other: Some("theirs") }
DEBUG          ┕━ workdir [ 6.36s | 4.32% / 4.57% ] commit_oid: 7a851336365c9d53ac450228b5d3e8f3cb897033
DEBUG             ┕━ ignore_large_files_in_diffs [ 342ms | 0.24% / 0.25% ] limit_in_bytes: 50000000
INFO                 ┝━ ThreadSafeRepository::open() [ 257µs | 0.00% / 0.00% ]
INFO                 │  ┕━ open_from_paths() [ 191µs | 0.00% / 0.00% ]
INFO                 │     ┕━ gix_odb::Store::at() [ 48.1µs | 0.00% ]
INFO                 ┕━ gix_index::File::at() [ 8.12ms | 0.01% ]
Error: File recreation must be an addition

The error originates in this repository, and still happens with this PR, which was to be expected.

Copy link

vercel bot commented Nov 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
gitbutler-components ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 26, 2024 6:26am

Copy link

vercel bot commented Nov 24, 2024

@Byron is attempting to deploy a commit to the GitButler Team on Vercel.

A member of the Team first needs to authorize it.

@Byron
Copy link
Collaborator Author

Byron commented Nov 24, 2024

@krlvi What's happening here with CI is that it fails due to running out of disk space. I have seen this issue before whenever…

  • …caches are used and reused from previous PRs, PR-runs or other branches like is the case here.
  • …there was a substantial change to the dependencies which causes a lot of these caches to be invalidated

The compressed cache here clocks in at 2GB, and uncompressed it's probably even larger, which causes the disk to run out of free space when all the new crate-versions are compiled.

A solution would be to delete the shared cache in the admin interface. Alternatively, one could stop sharing caches between PRs.

The run with caches removed seems to go through now, which means that deleting the caches should have the same effect. I would then remove fc1e488 to make this PR mergable.

@krlvi
Copy link
Member

krlvi commented Nov 25, 2024

@krlvi What's happening here with CI is that it fails due to running out of disk space. I have seen this issue before whenever…

  • …caches are used and reused from previous PRs, PR-runs or other branches like is the case here.
  • …there was a substantial change to the dependencies which causes a lot of these caches to be invalidated

The compressed cache here clocks in at 2GB, and uncompressed it's probably even larger, which causes the disk to run out of free space when all the new crate-versions are compiled.

A solution would be to delete the shared cache in the admin interface. Alternatively, one could stop sharing caches between PRs.

The run with caches removed seems to go through now, which means that deleting the caches should have the same effect. I would then remove fc1e488 to make this PR mergable.

Thanks. Just busted the caches via the gh cli gh cache delete --all

@Byron Byron marked this pull request as ready for review November 25, 2024 08:31
@Byron Byron enabled auto-merge November 25, 2024 08:31
@Byron
Copy link
Collaborator Author

Byron commented Nov 25, 2024

Thanks you! And I just validated that I can do that myself in future, too, I wasn't aware.

Now that I have pushed the original state of the branch back up and if CI now passes, this should auto-merge.

@Byron
Copy link
Collaborator Author

Byron commented Nov 25, 2024

Sorry for the delay, @krlvi, it looks like CI went through and with approval, this PR can be merged.

The fix include:

* ability to find references that are all uppercase.
* `tree_merge_options()` now correctly configure rename tracking.
    - Previously it was deactivated entirely, which would yield worse conflict resolution.
The issue itself seems to be unrelated though.
Rename tracking can help with conflict resolution, but in cases
where we only care about detecting conflicts, rename-tracking should be off.
@Byron
Copy link
Collaborator Author

Byron commented Nov 26, 2024

@krlvi and I decided this can wait until after the pending v0.14 release.

@Byron Byron merged commit c2cf72f into gitbutlerapp:master Nov 26, 2024
19 of 20 checks passed
@Byron Byron deleted the gix-update branch November 27, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Pull requests that update Rust code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants