pijul_org / pijul

#190 panic on checkout (dirty working dir)

Opened by chklauser, on September 18, 2017
chklauser commented on September 18, 2017

I got a panic when I unrecorded an intermediate patch and then tried to checkout to an alternate branch which still has all of the patches applied. The working directory is dirty when I do the checkout.

If necessary, I can send you the repository.

klauser@pjt$ RUST_BACKTRACE=1 RUST_LOG='pijul=DEBUG,libpijul=DEBUG' pijul checkout master
DEBUG:pijul::commands: get_wd: None
DEBUG:libpijul::backend: repository_size = 2564096
DEBUG:libpijul::output: begin output repository
DEBUG:libpijul::output: applying pending patch
DEBUG:libpijul::backend::hash: hash to_base64
DEBUG:libpijul::patch: save, path "/tmp/pijul.Pscm0Wnx8tkS/AdGWOEskJciTAG-twBLaFbwnw5xxT7fCUoQTq7S7GTVSbqqISpazZsDvVKws-jBLfehcChVbJmjTPFWJ7f9H9oE.gz"
DEBUG:libpijul::patch: created
DEBUG:libpijul::patch: saved
INFO:libpijul::apply: registering a patch with 0 changes: Unsigned(UnsignedPatch { flags: (empty), header: PatchHeader { authors: [], name: "", description: None, timestamp: 2017-09-18T21:21:10.328812700Z }, dependencies: [], changes: [] })
DEBUG:libpijul::apply: applying patch
DEBUG:libpijul::apply: apply_raw
DEBUG:libpijul::apply: synchronizing tree: []
DEBUG:libpijul::apply: committing branch
DEBUG:libpijul::backend: Commit_branch. This is not too safe.
DEBUG:libpijul::backend: Commit_branch, dbs_branches = Db(98304, PhantomData)
DEBUG:libpijul::backend: Commit_branch, dbs_branches = Db(36864, PhantomData)
DEBUG:libpijul::backend: Commit_branch, self.dbs.branches = Db(36864, PhantomData)
DEBUG:libpijul::output: applied
DEBUG:libpijul::output: b=Edge { flag: FOLDER_EDGE, dest: Key { patch: PatchId(UyWuGP21j3g), line: LineId(0x0100000000000000) }, introduced_by: PatchId(UyWuGP21j3g) }
DEBUG:libpijul::output: filename: FileMetadata(438) "otherfile"
DEBUG:libpijul::output: /b
DEBUG:libpijul::output: b=Edge { flag: FOLDER_EDGE, dest: Key { patch: PatchId(55tWfXidW34), line: LineId(0x0100000000000000) }, introduced_by: PatchId(55tWfXidW34) }
thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', /checkout/src/libcore/option.rs:335:20
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:396
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: rust_begin_unwind
             at /checkout/src/libstd/panicking.rs:497
   8: core::panicking::panic_fmt
             at /checkout/src/libcore/panicking.rs:92
   9: core::panicking::panic
             at /checkout/src/libcore/panicking.rs:51
  10: libpijul::output::, T>>::collect_children
  11: libpijul::output::, T>>::output_repository_assuming_no_pending_patch
  12: libpijul::output::, T>>::output_repository
  13: pijul::main
  14: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
  15: std::rt::lang_start
             at /checkout/src/libstd/panicking.rs:458
             at /checkout/src/libstd/panic.rs:361
             at /checkout/src/libstd/rt.rs:59
  16: __libc_start_main
  17: _start

This is pijul 0.8.0, running in Bash on Windows.

lthms commented on September 19, 2017

Do the patch add files or directories?

chklauser commented on September 20, 2017

Indeed, the added file is the culprint. I managed to reduce a reproduction to this

mkdir repo
cd repo
pijul init
echo 'A' >file
pijul add file
pijul record --all --message 'A'
pijul fork f
pijul unrecord "hash of patch A"
pijul checkout master # panics

I would expect some error message saying that 'file' is untracked and would be overwritten by the checkout. Instead, the checkout panics.

lthms commented on September 20, 2017

Thanks for the reproduction procedure! It is always very useful, and it can eventually be turned into a regression test. :)

About the behaviour of checkout, there is a pending change that, well, changes it, see #184. The first step is to prevent checkout to overwrite anything (that is, in case of pending changes, it asks for the user to revert or record its change). A second step will probably to add a --force flag.

If the --force flag acts like pijul revert -a, then maybe it would address your issue. But I suspect this is just a manifestation of an underlying problem, as it is not the first time pijul does not act well when the filesystem is not in the state it thinks it is.

pmeunier commented on September 20, 2017

I believe #AUdA2YYY2JXfPsquErM68sjq8GP-xlMLiSUBlmLlRZtbdTP56Ik7_dQuxB6CBvgDbIE0tewpbSfw26_2uxfk5i8 fixes this.

chklauser commented on September 20, 2017

I cloned and compiled pijul on my machine, then re-ran the test with that build. Well the panic is gone, but the result is even worse in my opinion. Afterwards:

klauser@~$ pijul status --repository panic-repro

Changes not yet recorded:
  (use "pijul record ..." to record a new patch)

        new file:  file
klauser@~$ pijul branches --repository panic-repro
* master
klauser@~$ pijul changes --repository panic-repro
Hash: ASePkgBJmTtxsZIkwHM1vuqCqgi-nyhHfEKWklvHMCAKoyWemqKvI3Vvg-f1F9-CqgINNDBGcZAz83eeq9BY78Y
Internal id: J4-SAEmZO3E
Authors: ["christianklauser@outlook.com"]
Timestamp: 2017-09-20 18:09:35.379561 UTC

klauser@~$ pijul ls --repository panic-repro/

pijul now suddenly claims that

  • master is successfully checked out (we did the unrecord on branch 'f')
  • patch A is applied again (consistent with being on the master branch)
  • the working copy is dirty (file untracked)
  • file is tracked twice (!?)

That seems completely broken to me.

pmeunier commented on September 20, 2017

Ok, got it, thanks for checking this out. I wouldn't say it's "completely broken". What happens is:

  • After the unrecord, Pijul knows it is tracking a file named "file", and that this file doesn't come from any patch.

  • When you do pijul checkout master, Pijul adds all the new files, and doesn't remove the files it's currently tracking, that have not been recorded yet.

There are two solutions:

  1. Checkout should maybe check that there are no changes before doing its job.
  2. Or we could just check that there are no new files before running checkout.

Ultimately, I think we need both, and (2) would be called "--force". I'll implement (2) for now, and maybe (1).

lthms added tag
on September 21, 2017
lthms commented on October 4, 2017

Should be fixed (at least, hidden) by #184.

pmeunier commented on October 4, 2017

This is actually fixed. I'm going to add option 2 to your patch.

lthms closed this discussion on October 4, 2017