#problems with object-safety, alternative solutions

24 messages · Page 1 of 1 (latest)

swift flint
#

disclaimer im a bit of a rust noob and very much feeling like i'm fighting against the compiler here, so the chance that i'm just doing something very wrong is like, certain.

I'm trying to write what is basically a package manager for some tools. currently, the way i've been trying to do it is to have a trait, Package, and implement methods/associated functions on it. here's what i've got so far:

pub struct Machine {
    // Store and retrieve from manifest.toml upon save and load
    pub installed_packages: HashMap<String, Box<dyn Package>>,
}

#[derive(EnumIter, Serialize, Deserialize)]
pub enum PackageType {
    PackageOne,
    ConflictingPackage,
}
impl PackageType {
    // I dont know how I feel about returning an option here instead of a Result<Option> here.
    // This function is for if installed_packages is not populated for some reason - we need to
    // make best efforts to populate it.
    fn try_discover(&self, config: &Config) -> Option<impl Package> {
        match self {
            Self::PackageOne => {
                let dir = &config.package_paths.package_one;
                todo!();
            }
            &_ => {
                todo!();
            }
        }
    }
}

impl Machine {
    pub fn has<T: Package>(&self, package: &T) -> bool {
        self.installed_packages.contains_key(package.name())
    }

    pub fn install<T: Package>(&mut self, package: &T) -> Result<()> {
        if self.has(package) {
            bail!("{} is already installed.", package.name());
        };
        package.install(self)
    }

    pub fn try_discover_packages(&mut self, config: &Config) -> Result<()> {
        for package_type in PackageType::iter() {
            let package_opt = package_type.try_discover(config);
            if let Some(package) = package_opt {
                self.installed_packages
                    .insert(package.name().to_string(), Box::new(package));
            }
        }
        Ok(())
    }
}

pub trait Package {
    fn name(&self) -> &str;
    fn version(&self) -> Version;
    fn install(&self, machine: &mut Machine) -> Result<()>;
    fn conflicts() -> Vec<PackageType>;
    fn kind() -> PackageType;
}

#[derive(Serialize, Deserialize)]
pub struct PackageOne {
    version: Version,
}
impl Package for PackageOne {
    fn name(&self) -> &str {
        "packageone"
    }
    fn version(&self) -> Version {
        self.version.to_owned()
    }
    fn kind(&self) -> PackageType {
        PackageType::PackageOne
    }
    fn install(&self, machine: &mut Machine) -> Result<()> {
        todo!();
    }
    fn conflicts(&self) -> Vec<PackageType> {
        vec![PackageType::ConflictingPackage]
    }
}```
the list of packages is going to be loaded/saved from/to a manifesto.toml file. this has ended up being the biggest roadblocking step, as it seems like i'll need serialization/deserialization capabilities on my Package trait for that, and these traits don't seem to be compatible with object safety.
so i guess the question is, is there a way to get what i want out of this current structure? and if not, what's the alternative? 

in particular, i'm a big fan of this bit of code:
```rust

impl Machine {
    pub fn install<T: Package>(&mut self, package: &T) -> Result<()> {
        if self.has(package) {
            bail!("{} is already installed.", package.name());
        };
        package.install(self)
    }
}
pub trait Package {
    fn install(&self, machine: &mut Machine) -> Result<()>;
}
#

i feel like it very much encapsulates the fact that the install function should be flexible between packages, but at the end of the day it's an operation changing the state of the machine so it should be a method on the machine.

what i've read from my bemused googling of this suggests that enums with associated data are a way to do this. my problem with that is that i really dont like the idea of some monumental match statement on the enum of all my possible packages, and also i like the idea of being able to put in the "conflicts()" method that we conflict with a given enum variant, without having to have some associated data initialised for that given variant (especially seeing as "version" has no default, and it shouldnt have.).

any ideas?

#

problems with object-safety, alternative solutions

fast girder
#

Truth be told, my first instinctive question is "why is Package a trait?"

struct Package {
  name: String,
  version: Version,
  install: Box<dyn Fn(&Self, &mut Machine) -> Result<()>,
  conflicts: Vec<PackageType>,
  kind: PackageType,
}
```The code you're a big fan of can be modified as such:
```rs
impl Machine {
    pub fn install(&mut self, package: &Package) -> Result<()> {
        if self.has(package) {
            bail!("{} is already installed.", package.name);
        };
        (package.install)(&package, self)
    }
}
```Now, about ser/de. Most of this struct is trivially ser/de-able. My only question is, what does it mean to serialize/deserialize `install`?
#

You can, with a small change, make it possible to ser/de every other field and then (in the deserialization path) add an installation method manually. You don't even need options.
You could also have a set of supported installation methods and make the struct store install: SupportedInstallationMethod with enum SupportedInstallationMethod, and now it's fully ser/de compatible

swift flint
#

all that i really wanted to say is that any struct that implements Package should be (de)serializable, but i want that to be enforceable by the compiler (i.e. if something is a package, it shoud always be (de)serialisable, and there shouldnt be a way for me to forget to annotate a given package with the deserialize/serialize derive).

#

the methods shouldn't be (de)serializable, because yeah that makes no sense to me.

#

that struct Package idea seems interesting. not really sure how i feel with having the install fn be a field though, i'd prefer it to be a method

fast girder
#

Why is Package a trait to begin with?

#

A trait should generally describe behaviour, and with the sole exception of install, your Package trait is data, It's four functions to get information about what a given package is, rather than to do something with it.

swift flint
#

i suppose it's a good question yeah

fast girder
#

For comparison:

#[derive(Serialize, Deserialize)] //can't forget to annotate a package with these, since all packages are the same type
struct PackageData {
  name: String,
  version: Version,
  conflicts: Vec<PackageType>,
  kind: PackageType,
}
 struct Package { //this one contains a function, which is of course not ser/de-able
  data: PackageData,
  install: Box<dyn Fn(&Self, &mut Machine) -> Result<()>,
}
swift flint
#

the issue i'm thinking about with the package idea is, how do i get from the PackageData saved in my manifest.toml to the Package struct which has the associated functions? when i'm loading from the file

#

given that Package would end up with more fields than just install - e.g. uninstall, get_status or whatever other things i want to be able to do with it

fast girder
swift flint
#

no no i must have badly explained myself

#

so in the manifest.toml we save PackageData, cool and good. but we saved that PackageData after we installed that package using the instructions contained in the Package struct. at some point we're going to want the information in that Package struct again, probably based on the PackageType. but it seems like the only good way to do that mapping is through some sort of get_package function (or method on PackageData ofc), where we use PackageData as an argument and return a Package, which is fetched with some match statement

and idk, match statements have never seemed suited for that sort of thing - it feels like you end up with this unreadable mess once you're matching more than a handful of things (PackageKind variants, in this instance), plus the tendency for them to creep rightwards. which is why it felt like it would be best to do this sort of thing with a method implemented on the type. but then we're back to the trait-on-a-struct idea again (which i accept doesn't work)

fast girder
# swift flint so in the manifest.toml we save PackageData, cool and good. but we saved that Pa...

at some point we're going to want the information in that Package struct again
Everything serializable can go into PackageData, so you can get it back just fine. Everything not serializable you'll have to get some other way.

With some effort, more or less everything is serializable except functions, so unless you expect to be able to load package information from a file and learn how to uninstall it in the process, it should be enough?

swift flint
fast girder
#

You may want to switch to a serializable representation for functions

#

Such as, say, an install/uninstall script

#

A package comes with such a script, and you run it. If you like making interpreters, it's a fun excuse, else you can shell out to your scripting language of choice