search

martinvonz / jj

A Git-compatible VCS that is both simple and powerful
https://martinvonz.github.io/jj/
Apache License 2.0
8.32k stars 284 forks source link

Managed to checkout hidden change, escalated into panic #4446

Closed Veykril closed 2 weeks ago

Veykril commented 2 weeks ago

Description

I managed to somehow have a hidden change checked out, but I do not know how I managed to do so (and forgot to snapshot my op log at the tim). Then for fun I tried jj edit @ which caused the following panic:

workspace\rust\rust-analyzer (08c7bbc) 
❯ jj edit nxv
thread 'main' panicked at C:\Users\lukas\.cargo\registry\src\index.crates.io-6f17d22bba15001f\jj-lib-0.19.0\src\id_prefix.rs:176:89:
Change ids present in narrower search set should be present globally.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Steps to Reproduce the Problem

Unfortunately I don't know what exactly I did aside from trying to resolve conflicts between two remote branches.

Expected Behavior

Afaik one shouldnt be able to check out hidden changes

Actual Behavior

Managed to check out a hidden change

Specifications

martinvonz commented 2 weeks ago

4427 might have fixed this bug, but maybe more likely it would just make it harder to trigger

yuja commented 2 weeks ago

Perhaps, we'll need to remove the assertion anyway. User can configure short-prefixes to include e.g. remote_branches().

ilyagr commented 2 weeks ago

Hidden changes should never be addressable by their change id, I think. Unless that check can be hit with a commit id, I think hitting the check is always a bug.

It might be reachable because of https://github.com/martinvonz/jj/issues/3861 and other versions of #2193, though.

So, I don't know if the check should stay, but off the top of my head, I think that if including remote_branches() in the short prefixes list causes it to be hit, it's a bug.