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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

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.

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.
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 24, 2024 11:28am

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.

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 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.

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.

1 participant