martinvonz / jj

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

If `jj init --git-repo=.` fails, it leaves a non-usable `.jj` directory in place #1794

Closed chriskrycho closed 1 year ago

chriskrycho commented 1 year ago

Description

If you run jj init --git-repo=. in a directory which does not itself contain a .git repository, the operation (quite reasonably!) fails and exits, reporting the issue to the user. However, it leaves the .jj directory in place, which means that jj <command> in that directory or any of its children will fail, because there is no valid Jujutsu directory to work with: initialization failed. The surfacing symptoms are as described below (in the "original write-up" sections).

Original write-up: With Git, you can perform operations in any directory which is a child of a working copy. If you try that today with Jujutsu, it fails with a panic.

Steps to Reproduce the Problem

  1. Create an empty directory.
  2. Run jj init --git-repo=..
  3. Run any jj command (e.g. jj status).

Original write-up:

  1. Checkout any Git project.
  2. cd into a directory that is not the root.
  3. jj status.

Expected Behavior

When initialization fails, it should clean up after itself!

Original write-up: It prints the status, or at least a "Hey, that's not implemented yet, whoops!" kind of message.

Actual Behavior

It doesn't, and running commands results in output like this:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 2, kind: NotFound, message: "No such file or directory" }', lib/src/git_backend.rs:90:79
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Specifications

chriskrycho commented 1 year ago

I notice that the behavior here is documented to do the right thing:

  -R, --repository <REPOSITORY>
          Path to repository to operate on

          By default, Jujutsu searches for the closest .jj/ directory in an ancestor of the current working directory.

I can confirm that setting it explicitly does indeed work, but not “the closest .jj/ directory in an ancestor of the current working directory”.

yuja commented 1 year ago

It's supposed to work. Can you check which directories jj is looking into?

I don't know about macOS, but on Linux, strace -e '%%stat' printed enough information.

strace jj status --ignore-working-copy 2>&1 | egrep '\.jj|git' 
chriskrycho commented 1 year ago

I'm mucking with dtrace (and its wrapper dtruss) trying to answer that; but/and I'm also going to see what happens when I just insert a log here and run it, as I think that's the actual place this is intended to happen:

https://github.com/martinvonz/jj/blob/22a84c8e97e286cf241596e194c9fc4b96e566f9/src/cli_util.rs#L1365-L1368

chriskrycho commented 1 year ago

Ah, an interesting bit of follow-up: the above code does work as expected, as does (ultimately) the lookup code in git_backend, but throwing those explicit logs in helped me figure out what went wrong here: I had somehow ended up with a nested .jj directory, which was not colocated with the .git directory, and so the logic was identifying that directory as the one in which to resolve the Git store, and failing.

I think this was a result of my initial tooling around with jj; I probably did a jj init --git-repo . there, got an error, and did it in the correct directory and moved on—but it turns out that the .jj directory is created anyway in that circumstance, which then leads to this error later.

Net: mostly PEBKAC, but probably jj init should avoid creating the .jj directory tree at all in this case, or should clean it up if creation fails? I'm happy to repurpose this issue to track that, or to create another one from this, whatever is easiest/best for you all!

yuja commented 1 year ago

I had somehow ended up with a nested .jj directory, which was not colocated with the .git directory,

Good find. I think error reporting in such situation should be improved by the next version.

chriskrycho commented 1 year ago

Now known to be a duplicate of #1305. ✅