The sound distributed version control system

#846 Empty range in Builder::record

Opened by loewenheim on December 19, 2023
loewenheim on December 19, 2023

These are the contents of lines 306-346 in libpijul/src/record.rs:

        for t in 0..0 {
            let working_copy = working_copy.clone();
            let changes = changes.clone();
            let channel = channel.clone();
            let work = work.clone();
            let txn = txn.clone();
            let sep: regex::bytes::Regex = diff_separator.clone();
            let cl = move || {
                loop {
                    let (w, stop) = {
                        let mut work = work.lock();
                        (work.t.pop_front(), work.stop)
                    };
                    if let Some((item, vertex, rec, new_papa)) = w {
                        // This parent has changed.
                        info!("record existing file {:?} on thread {:?}", item, t);
                        rec.lock().record_existing_file(
                            &txn,
                            diff_algorithm,
                            stop_early,
                            &sep,
                            &channel,
                            working_copy.clone(),
                            &changes,
                            &item,
                            new_papa,
                            vertex,
                        )?;
                    } else if stop {
                        info!("stop {:?}", t);
                        break;
                    } else {
                        info!("yield {:?}", t);
                        std::thread::park_timeout(std::time::Duration::from_secs(1));
                    }
                }
                Ok::<_, RecordError<C::Error, W::Error, T>>(())
            };
            workers.push(std::thread::spawn(cl.clone()))
        }

As clippy rightly points out, all of that is dead code because the range is empty. Surely this cannot possibly be working as intended?

pmeunier on February 7, 2024

This is actually working as intended: parallel record works with a single worker doing the “exploratory” work, traversing the repo and collecting things to do. Then, the helper workers pop that “todo list” and do it. At the end the initial worker contributes to that effort.

This here is just meant to disable concurrency, which I did some time ago because it was hard to get deterministic patches. I should definitely get back to it at some point. Feel free to try and enable it if you’re interested.