Skip to content

wip: config flattening #19

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

wip: config flattening #19

wants to merge 2 commits into from

Conversation

sethp
Copy link
Owner

@sethp sethp commented Aug 16, 2020

Wherein we attempt to convert a Vec<SSHOption> into a form that provides a unique answer for any given config parameter. That structure should have three properties:

  1. It should be easy to query for a particular directive, e.g. User
  2. It should provide a vehicle for expressing the "merge-ability" of options like SendEnv and IdentityFile
  3. It should offer convenient iteration over the whole of the config to "apply" to an ssh session

@sethp
Copy link
Owner Author

sethp commented Aug 16, 2020

In 79318c3 that structure is HashMap<std::mem::Discriminant<SSHOption>, SSHOption>, but offers some caveats on the above:

For 1: In order to query for User, one must construct a User variant with some arbitrary contained data to pass it off to std::mem::discriminant(..)
For 2: The implementation here requires a NxN merge matrix (where N is the number of variants of the SSHOption enum), but only the diagonal is meaningful (we can't merge a User and a Port, for example). Applying that merge function is also somewhat awkward as written: in the fairly common "merge" case requiring two inserts and a remove.
The third point seems well satisfied by the .values() function, at least.

@sethp sethp mentioned this pull request Aug 16, 2020
23 tasks
@sethp
Copy link
Owner Author

sethp commented Aug 16, 2020

Ok, so I found https://www.reddit.com/r/rust/comments/8ls25e/blog_post_typed_key_pattern/ and tried to implement something roughly like it in 5aad6f7. It kind of worked, but storing the types in a map led to conflict: since we no longer have enum-style "reserve max space" semantics (because, at least as far as the compiler's aware at this point, the Kind type is an open set), how do we store the now-heterogeneous things? Something something boxes and Anys as hinted at in that reddit thread, which is very similar to how Java HashMaps work.

At that point I was remembering that I implemented this structure in Java, with a lot of Ts and Class<T>s and casting. Sure enough, the link to "typesafe heterogeneous container" nearly at the bottom of the thread is a link to a Java blog describing basically (exactly?) what I'd done. Before diving into implementing the same thing in Rust, I happened to search for "rust typesafe heterogenous container," which led me to https://www.reddit.com/r/rust/comments/2923m4/heterogeneous_containers/ which pointed the way to https://github.com/chris-morgan/anymap , which looks very promising.

We'd have to promote all of the options from enum variants to newtypes, but then SSHConfig could wrap an AnyMap. That should line up with our criteria something like:

  1. config.find::<User>() or let user: User = config.find()
  2. There's some possibly very convenient methods around entries that, with sufficient bounds (i.e. Any + Merge?), may get us to neater insert-or-update behavior
  3. Unfortunately, iterating values is awkward at best. Probably the best we can try to do is an on-the-fly conversion to a trait object and some dynamic dispatch to "Apply" the config to a session

Some things we'd be trading away with this approach (at least initially):

  • "compactness" of representation: instead of a single closed type that represents all possible config values, we've now got an open + therefore fundamentally unsized set of possibilities. We're going to do a lot more work with pointers into the heap.
  • Easy pattern matching to pull various options out of a stream of what's probably now trait objects
  • Implementing a new option has gone from minimum two lines (variant + try_from) to maybe a dozen? (type + derive + docs or allow(missing_docs) + trait impls)

All that said, dealing with options as reified top-level types does open up some new possibilities:

  • Merge becomes a trait that's really easy to write, and possibly gives us the right ownership semantics (dropping duplicates of some options, "moving" ownership into one for others)
  • Grouping common behaviors across a subset of options with traits ("as_str"?)
  • Maybe more convenient stringly-typed helpers a la the original "typed key pattern" for mapping user to the User type?
  • Implementing slightly more ergonomic constructors, e.g. User::new("alice") vs User(String::From("bob"))

cc @dougli1sqrd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant