Db::servers Lifetime

[fosskers]

This is an experiment to determine whether the previous anonymous lifetime is the source of issues I'm having. If it is, then I'll undraft this.

[fosskers]

Hm, it doesn't seem to fix it.

Do you know why this compiles:

        let names: Vec<_> = dbs.iter().map(|db| (db.name(), db.name())).collect();

but this does not?

        let inner: HashMap<&'a str, Vec<&'a str>> = dbs
            .iter()
            .map(|db| (db.name(), db.servers().iter().collect()))
            .collect();

The error in the second case is:

error[E0515]: cannot return value referencing function parameter `db`
  --> aura-arch/src/lib.rs:75:23
   |
75 |             .map(|db| (db.name(), db.servers().iter().collect()))
   |                       ^^^^^^^^^^^^--^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |                       |           |
   |                       |           `db` is borrowed here
   |                       returns a value referencing data owned by the current function

No, Rust, I promise you I do not own the db in that map.

[Morganamilo]

Works for me. What's the full code?

[fosskers]

Still experimenting, but this is what wasn't working:

pub struct MirrorMap<'a> {
    inner: HashMap<&'a str, Vec<&'a str>>,
}

pub fn new(dbs: AlpmList<'a, Db<'a>>) -> MirrorMap<'a> {
    let inner: HashMap<&'a str, Vec<&'a str>> = dbs
        .iter()
        .map(|db| (db.name(), db.servers().iter().collect()))
        .collect();

    MirrorMap { inner }
}

[Morganamilo]

Where is the lifetime defined in new?

[fosskers]

Fuller example:

impl<'a> MirrorMap<'a> {
    pub fn new(dbs: AlpmList<'a, Db<'a>>) -> MirrorMap<'a> {
        let inner: HashMap<&'a str, Vec<&'a str>> = dbs
            .iter()
            .map(|db| (db.name(), db.servers().iter().collect()))
            .collect();

        MirrorMap { inner }
    }
}

Even if I slap a & on the AlpmList it doesn't like it.

[fosskers]

This is complaining too:

        // Show servers
        for p in ps {
            let mirror = p.db().and_then(|db| db.servers().first());
        }

EDIT: Ah it looks like and_then demands ownership of the innards. Maybe as_ref() would fix that.

EDIT 2: Yes that fixed this example.

[Morganamilo]

Fuller example:

impl<'a> MirrorMap<'a> {
    pub fn new(dbs: AlpmList<'a, Db<'a>>) -> MirrorMap<'a> {
        let inner: HashMap<&'a str, Vec<&'a str>> = dbs
            .iter()
            .map(|db| (db.name(), db.servers().iter().collect()))
            .collect();

        MirrorMap { inner }
    }
}

Even if I slap a & on the AlpmList it doesn't like it.

Works for me still. Are you sure you're depending alpm with this PR included and not just the standard one from crates.io?

[fosskers]

Ahh it was me not accounting for the 3 places that I'm actually declaring alpm as a dependency, and the different versions were fighting. I seem to recall somebody warning me about all the subcrates... now who was that... :thinking:

Okay, with this PR, this now works, with or without the & on the AlpmList arg.

[fosskers]

Shall I add a CHANGELOG?

[Morganamilo]

See https://doc.rust-lang.org/edition-guide/rust-2018/cargo-and-crates-io/replacing-dependencies-with-patch.html That will propagate to deps of deps.

Probably not. This was a bug so there's no change of functionality, it should have always been this way. And as this is a strict wrapper over alpm I don't see us ever making any changes that are not just changes from alpm ittself.

[fosskers]

In that case this should be good to go.

[fosskers]

Cool, specifying this at the workspace root does the trick:

[patch.crates-io]
alpm = { git = "https://github.com/fosskers/alpm.rs", rev = "9072a3b77a2f1d681bc2d9891c65766484cf7b9e" }

I like that, it keeps the rest of the child Cargo.toml clean.