Remove validator_types dep from main validator crate.

[rosslannen]

This removes the transitive dependency on proc_macro, allowing the
crate to be cross compiled when a target proc_macro is not available.

[rosslannen]

This is a breaking API change, as it removes the Validator type from the public API of the crate.

However, this crate currently does not build in some cross compilation environments, failing with this error:

| error[E0463]: can't find crate for `proc_macro`
|   --> /path/redacted/proc-macro2-1.0.27/src/lib.rs:88:1
|    |
| 88 | extern crate proc_macro;
|    | ^^^^^^^^^^^^^^^^^^^^^^^^ can't find crate
| 
| error: aborting due to previous error
| 
| For more information about this error, try `rustc --explain E0463`.
| error: could not compile `proc-macro2`

As far as I can tell, there was no usage of the Validator type anywhere outside of the derive crates, and there is no need to export the type on the main crate.

[Keats]

I think the change makes sense. I can't remember why it's being exported.

[rosslannen]

Awesome! If you could merge this, that would be fantastic - I don't believe I have permissions to do so.

Do you have any idea of when this would make it into a version on crates.io?

[Keats]

Not sure, I'd like to see if someone knows why it was exported and was using it.

[rosslannen]

According to commit 501fe7914222d35dcc42e7cd160ecbd703ecbe39:

• Addition of the validator_types crate for the core type to break the
  cyclic dependency

Is it possibly necessary for implementing some functionality if the derive feature is not enabled? Is anyone using it like that?

[Keats]

cc @ELD if you can remember anything

[bradleyess]

According to commit 501fe7914222d35dcc42e7cd160ecbd703ecbe39:

• Addition of the validator_types crate for the core type to break the
  cyclic dependency

Is it possibly necessary for implementing some functionality if the derive feature is not enabled? Is anyone using it like that?

@rosslannen It's probably without saying that this will be difficult (impossible) for us to prove how all consumers are using this, and introducing a breaking change like this would require Validator to hit its first major release milestone.

1. I'm not sure the library is ready to be considered at V1.
2. Are you able to prove what scenarios would be impacted by this change?

[Keats]

and introducing a breaking change like this would require Validator to hit its first major release milestone.

We're below 1.0, we can have as many breaking changes as we want ;)

[ELD]

Sorry for the late reply (3 days later?), but I just remembered to get back to this.

I can't remember anything in particular, but I am curious if there are other ways to shuffle things around to keep Validator in the public interface, but not have a hard dependency on proc-macro unless a feature is enabled.

I think if this unblocks the user in the immediate future, it makes sense to merge it, breaking change or otherwise (let's just bump the minor version so that folks don't unintentionally pull in the change). Otherwise, I can hack around and see if I can come up with a better solution.

@rosslannen can you shed more light on the cross compilation targets where you see this error? I would like to replicate this, if possible, to find other alternative solutions. Also, fun to find another Mines Alum on here, using Rust, by total happenstance! 😁

[rosslannen]

I am completely OK keeping the Validator type itself in the API, as long as there is a way to get rid of the transitive dependency on the actual proc_macro crate. Luckily it looks like both proc_macro2 and syn have features that can control that. A feature on the validator_types crate that propagates those through would help with that.

That all being said, I really can't see a reason for Validator to be in the public API. It doesn't appear to be mentioned in the documentation, consumed by any other area of the public API, or have any other uses to adding validation to crates. Even custom validation is done through free functions and doesn't appear to need this type.

Right now the issue is showing up in custom yocto linux builds using meta-rust. That is somewhat difficult for others to replicate, so I'm currently exploring other options with Docker and/or cargo-cross. For the immediate future we have our builds pointed at my branch, so we can take the time to get this right.

@ELD I think we've met! I believe you managed my field session project at Pivotal a few years back. Small world.

[ELD]

These proc macro changes I made way back when were when I had a much more elementary understanding of them. I'm sure there is another solution that doesn't change the public interface, but it's reasonable to question to the need for the Validator type to be in the public API. I think it was a solution to get the procedural macro to work as a reexport and feature of the crate. Admittedly, the change that was made was a rather large one (reexport the macro, migrate to trybuild, etc) — so that's a little silly on my part to have jammed so many changes together (with some being relatively unrelated).

I appreciate the info on the cross compilation woes. I'll see what I can pick apart there — but that might be a red herring for investigation. It's probably better to just go with your suggestion to remove the transitive dependency on proc_macro unless a feature is enabled.

@rosslannen Oh yeah! I'm remembering now. You and your team worked on the team health check project. I'm sorry it slipped my mind at first. Hope you've been well!

[rosslannen]

I created a WIP solution that doesn't touch public API here: #208. It still needs some testing, but its another solution to the problem. It is up to you folks to figure out which route you want to take, both work for us.

I still think that removing the type should be revisited before the crate would hit 1.0. No better way to help an API from breaking than getting rid of stuff from it. But that can be a future problem to solve.

@ELD yup! Fun project. I've been doing dandy, hope the same for you 😃

[Keats]

Having thought about it, I would prefer removing it and see if anyone complains. The tried and true method!

[rosslannen]

@Keats that sounds great. I believe now it is all in your hands to get this merged in.