pijul_org / pijul

#165 Pijul asks to revert conflict markers

Opened by lthms, on September 3, 2017
Closed
lthms commented on September 3, 2017

How to reproduce

pijul clone <name>@nest.pijul.com:lthms/pijul test
cd test
pijul pull <name>@nest.pijul.com:pijul_org/pijul # pull all patches
pijul unrecord --patch AWVvxC9uzvtf5Js1cw2fnQ5Q9Qd-p0Rj36LYKPUCXXVlcN2kQ--FxCD8dwAkWUo3ZukKf-L9qomit9fBKdd4o4M
pijul revert
lthms commented on September 3, 2017

The “How to reproduce” became useless when I unrecorded the patch of @pointfree. You can still reproduce the bug by cloning the refactorings branch of his fork instead of my fork on the nest.

Sorry about that.

pmeunier commented on September 3, 2017

By the way, I was thinking, what were you expecting?

Suppose there's a conflict between patches A and B, in file F. F looks like:

>>>>>>
A
======
B
<<<<<<

Then, you unrecord B. Unrecording doesn't change any file, it just forgets about patches in the abstract representation (the graph). The file doesn't change, and there's no more conflict in the graph.

I believe pijul revert should indeed ask you what to do about these conflict markers.

The incorrect thing (maybe) was to not warn you about conflicts when you did revert. What do you think?

lthms commented on September 3, 2017

The thing is: the revert thing does not produce the expected result. Here what I have in src/commands/patch.rs after reverting everything:

use clap::{SubCommand, ArgMatches, Arg};
use commands::{BasicOptions, StaticSubcommand, default_explain};
use libpijul::Hash;
use libpijul::fs_representation::patches_dir;
use super::validate_base64;

use error::Error;
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
use std::io::{stdout, copy};
use std::fs::File;

pub fn invocation() -> StaticSubcommand {
    return SubCommand::with_name("patch")
        .about("Output a patch (in binary)")

        .arg(Arg::with_name("repository")
             .long("repository")
             .help("Path to the repository where the patches will be applied.")
             .takes_value(true))

        .arg(Arg::with_name("patch")
             .help("The hash of the patch to be printed.")
             .takes_value(true)
             .required(true)
             .validator(validate_base64));
}

pub fn run(args: &ArgMatches) -> Result<(), Error> {
    let opts = BasicOptions::from_args(args)?;
    let patch = Hash::from_base64(args.value_of("patch").unwrap()).unwrap();
    let mut patch_path = patches_dir(opts.repo_root).join(&patch.to_base64(URL_SAFE_NO_PAD));
    patch_path.set_extension("gz");
    let mut f = try!(File::open(&patch_path));
    let mut stdout = stdout();
    try!(copy(&mut f, &mut stdout));
    Ok(())
}

pub fn explain(r: Result<(), Error>) {
    default_explain(r)
}
================================
use base64::URL_SAFE_NO_PAD;
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
lthms commented on September 3, 2017

What I have done:

export USER=lthms
mkdir pijul
cd pijul
pijul init
pijul pull ${USER}@nest.pijul.com:pointfree --from-branch refactorings
# pull everything
pijul pull ${USER}@nest.pijul.com:pijul_org/pijul
# pull the base64 validation patch
pijul unrecord --patch AWVvxC9uzvtf5Js1cw2fnQ5Q9Qd-p0Rj36LYKPUCXXVlcN2kQ--FxCD8dwAkWUo3ZukKf-L9qomit9fBKdd4o4M
pijul revert
# say yes to everything
lthms commented on September 3, 2017

Sorry about the spam, but last time I’ve checked edit was still not working on the nest, so for the sake of readability, I’d rather be explicit.

First, the correct procedure, minus the errors:

export USER=lthms
mkdir pijul
cd pijul
pijul init
pijul pull ${USER}@nest.pijul.com:pointfree/pijul --from-branch refactorings
# pull everything
pijul pull ${USER}@nest.pijul.com:pijul_org/pijul
# pull the base64 validation patch
pijul unrecord --patch AWVvxC9uzvtf5Js1cw2fnQ5Q9Qd-p0Rj36LYKPUCXXVlcN2kQ--FxCD8dwAkWUo3ZukKf-L9qomit9fBKdd4o4M
pijul revert
# say yes to everything

Also, I can confirm the conflict is still here after unrecording the patch.

On branch master

Unresolved conflicts:
  (fix conflicts and record the resolution with "pijul record ...")

        pijul/src/commands/patch.rs

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

        modified:  pijul/src/commands/patch.rs
pmeunier commented on September 4, 2017

I can reproduce on a one line repository, and I know exactly what the problem is (and editing comments seems to be working again on the Nest).

lthms commented on September 4, 2017

I can reproduce on a one line repository, and I know exactly what the problem is

Great! Hope it wasn’t too painful to investigate.

editing comments seems to be working again on the Nest

Noted. Thanks for the heads-up.

pmeunier commented on September 4, 2017

This was fun. The problem was in unrecord: when un-removing an edge, unrecord was always introducing its previous version.

This is the correct thing to do only when the edge has not been removed by patches that are still present in the branch.