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

panic on jj file annotate on an unshallow'd repository #5909

Open
jedesroches opened this issue Mar 7, 2025 · 5 comments
Open

panic on jj file annotate on an unshallow'd repository #5909

jedesroches opened this issue Mar 7, 2025 · 5 comments
Labels
🐛bug Something isn't working

Comments

@jedesroches
Copy link
Contributor

jedesroches commented Mar 7, 2025

Description

When using jj inside a repo that was cloned with a shallow depth and then unshallow'd, and then running jj file annotate, jj will panic because it expect's an empty initial commit.

Faulty line is here

Steps to Reproduce the Problem

#!/bin/sh

set eu

for cmd in git jj realpath;
do
  if ! command -v $cmd >/dev/null 2>&1;
  then
    >&2 echo "Missing command: $cmd"
    exit 127;
  fi
done

# Setup a repository with a non-empty file committed at depth > 1
mkdir example1
cd example1 || exit 1
git init .
echo "hello bug" > file1 # bug requires a non-empty file
git add file1; git commit -m "file1"
touch file2; git add file2; git commit -m "file2"

# Make a shallow clone
cd .. || exit 1
git clone file://"$(realpath example1)" --depth=1 example2

# Init the repo
cd example2 || exit 1
jj git init --colocate .

# Here, jj file annotate will work, considering all changes to
# come from the most recent commit

# Un-shallow the clone
git fetch --unshallow

echo "------ FAIL HERE: ------"

# Now jjfa fails:
RUST_BACKTRACE=1 jj file annotate file1

Expected Behavior

The file is annotated.

Actual Behavior

A panic occurs.

Specifications

  • Platform: Darwin excalibur.local 24.3.0 Darwin Kernel Version 24.3.0: Thu Jan 2 20:22:00 PST 2025; root:xnu-11215.81.4~3/RELEASE_X86_64 x86_64
  • Version: jj 0.26.0
@yuja
Copy link
Contributor

yuja commented Mar 7, 2025

Perhaps, we'll have to address this when adding support for revision ranges (or depth) to annotate. Such option will help write tests.

@yuja yuja added the 🐛bug Something isn't working label Mar 7, 2025
@jedesroches
Copy link
Contributor Author

I don't think this is an issue about depth, it is an issue with the import of the underlying git structure.

On a shallow repository, jj behaves correctly (or at least, like git), and blames the oldest fetched commit for the changes.

The issue occurs once we unshallow the repository, where jj does not import the commits between zzzzzz and the last head, even though git has filled them in. Using the above example, one can clearly see the difference between git log and jj log -r all().

@yuja
Copy link
Contributor

yuja commented Mar 10, 2025

The issue occurs once we unshallow the repository, where jj does not import the commits between zzzzzz and the last head, even though git has filled them in.

Oh, you mean this isn't a problem specific to annotate? jj doesn't support expanding the shallow boundary. jj debug reindex might fix the problem, but if it didn't, you'll need to reclone the repo.

@jedesroches
Copy link
Contributor Author

Ok, thanks. Thinking about this, there are actually two issues:

  • The fact that jj file annotate panics - this could hopefully be mitigated somehow (maybe passing the default 0000000 commit id to the a unwrap_or call instead of an expect ?)
  • Not importing commits that were fetched with a git fetch --unshallow, which I imagine is what you mean by "expanding the shallow boundary". Probably this issue can stay focused on the first problem then. Would replacing the expect call with a default of the initial change be an acceptable solution ?

@yuja
Copy link
Contributor

yuja commented Mar 10, 2025

  • The fact that jj file annotate panics - this could hopefully be mitigated somehow (maybe passing the default 0000000 commit id to the a unwrap_or call instead of an expect ?)

I think annotate can show the bottom commit with some marker meaning that the originating commit cannot be determined.

  • Not importing commits that were fetched with a git fetch --unshallow, which I imagine is what you mean by "expanding the shallow boundary".

Yes. I believe various jj subcommands would panic due to the data inconsistency.

jedesroches added a commit to jedesroches/jj that referenced this issue Mar 10, 2025
As described in jj-vcs#5909, in the case where jj was initialized in a
shallowly cloned repository which was then unshallow'd, jj does not
import the fetched commits that were outside the shallow boundary.

However, it does import the ones after the boundary, which after
unshallowing do not contain the changes made before the boundary.

Therefore, when running annotate in such a case, jj would panic because
it does not find the commit from which a line originates. This simply
sets the empty commit as carrying the blame for that line.
jedesroches added a commit to jedesroches/jj that referenced this issue Mar 10, 2025
As described in jj-vcs#5909, in the case where jj was initialized in a
shallowly cloned repository which was then unshallow'd, jj does not
import the fetched commits that were outside the shallow boundary.

However, it does import the ones after the boundary, which after
unshallowing do not contain the changes made before the boundary.

Therefore, when running annotate in such a case, jj would panic because
it does not find the commit from which a line originates. This sets the
empty commit as carrying the blame for that line.
jedesroches added a commit to jedesroches/jj that referenced this issue Mar 11, 2025
As described in jj-vcs#5909, in the case where jj was initialized in a
shallowly cloned repository which was then unshallow'd, jj does not
import the fetched commits that were outside the shallow boundary.

However, it does import the ones after the boundary, which after
unshallowing do not contain the changes made before the boundary.

Therefore, when running annotate in such a case, jj would panic because
it does not find the commit from which a line originates. This sets the
empty commit as carrying the blame for that line.
jedesroches added a commit to jedesroches/jj that referenced this issue Mar 12, 2025
As described in jj-vcs#5909, in the case where jj was initialized in a
shallowly cloned repository which was then unshallow'd, jj does not
import the fetched commits that were outside the shallow boundary.

However, it does import the ones after the boundary, which after
unshallowing do not contain the changes made before the boundary.

Therefore, when running annotate in such a case, jj would panic because
it does not find the commit from which a line originates. This sets the
empty commit as carrying the blame for that line.
jedesroches added a commit to jedesroches/jj that referenced this issue Mar 12, 2025
As described in jj-vcs#5909, in the case where jj was initialized in a
shallowly cloned repository which was then unshallow'd, jj does not
import the fetched commits that were outside the shallow boundary.

However, it does import the ones after the boundary, which after
unshallowing do not contain the changes made before the boundary.

Therefore, when running annotate in such a case, jj would panic because
it does not find the commit from which a line originates. This sets the
empty commit as carrying the blame for that line.
github-merge-queue bot pushed a commit that referenced this issue Mar 12, 2025
As described in #5909, in the case where jj was initialized in a
shallowly cloned repository which was then unshallow'd, jj does not
import the fetched commits that were outside the shallow boundary.

However, it does import the ones after the boundary, which after
unshallowing do not contain the changes made before the boundary.

Therefore, when running annotate in such a case, jj would panic because
it does not find the commit from which a line originates. This sets the
empty commit as carrying the blame for that line.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants