Add serde to Public and Private

[Firstyear]

Add support for serde to Public and Private, going through the current Marshall and Unmarshall pathways. This pattern is quite "simple" so it could be possible to duplicate it to other structures, or macro them for types that implement Marshall and Unmarshall.

[Superhepper]

I think it would be a good idea to make serde support as an optional feature.

[Firstyear]

I think it would be a good idea to make serde support as an optional feature.

Serde is already hard-required, so why make it optional?

[Superhepper]

Oh, you are right. That was unfortunate. Then it will not matter for this PR but rather something we should look into doing. Because it is nice in my opinion to make it possible for the user to opt out of non essential functionality.

[wiktor-k]

Agreed with @Superhepper, serde should be optional but it's not an issue with this PR.

[Firstyear]

Looks like I stepped on a land-mine adding these tests.

Following the pattern of the marshall_unmarshall test I added these with a reflexive test using serde_json. serde_json was added as a dev-dependency.

This caused more than 1500 compiler errors - it turns out that once serde_json exists, this confuses the compiler about the partialEq trait in the macros "test_valid_conversion" used throughout the test suite. Because of this, assert_eq! fails to work because there are multiple possible satisfying types for the ::into calls.

An example is:

error[E0283]: type annotations needed
  --> /home/886b45ce-98b2-4d79-9069-c9ff25bc0232/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/macros/mod.rs:40:32
   |
36 |   macro_rules! assert_eq {
   |   ---------------------- in this expansion of `assert_eq!` (#2)
...
40 |                   if !(*left_val == *right_val) {
   |                                  ^^ cannot infer type
   |
  ::: tss-esapi/tests/integration_tests/constants_tests/algorithm_tests.rs:17:1
   |
17 | / macro_rules! test_conversion {
18 | |     ($tpm_alg_id:ident, $algorithm:ident) => {
19 | |         assert_eq!($tpm_alg_id, AlgorithmIdentifier::$algorithm.into());
   | |         --------------------------------------------------------------- in this macro invocation (#2)
20 | |         assert_eq!(
...  |
27 | |     };
28 | | }
   | |_- in this expansion of `test_conversion!` (#1)
...
32 |       test_conversion!(TPM2_ALG_CAMELLIA, Camellia);
   |       --------------------------------------------- in this macro invocation (#1)
   |
   = note: multiple `impl`s satisfying `u16: PartialEq<_>` found in the following crates: `core`, `serde_json`:
           - impl PartialEq for u16;
           - impl PartialEq<serde_json::Value> for u16;

I think this is beyond my ability to solve this, and probably needs some bigger work to clean up.

[Firstyear]

In light of the above issue, what do you think I should do here @ionut-arm ?

[Superhepper]

Have you tried something trivial like:

if !(u16::from(*left_val) == u16::from(*right_val)) {

And see if it helps?

[Firstyear]

Have you tried something trivial like:

if !(u16::from(*left_val).eq(u16::from(*right_val))) {

And see if it helps?

There are hundreds of locations that need this update. I'm not doing it in this PR.

[Superhepper]

I can help you fix some of them.

[Firstyear]

Thanks to @Superhepper I was now able to add the serde tests :)

[Firstyear]

Hey all, what else is needed here? We really want this merged so that we can use it for TPM support for Azure AD and other linux pam authentication subsystems.

[Superhepper]

Not anything I think. I would just want CI to work again so this can without getting some unrelated errors.

[Superhepper]

Try to rebase this on main now. The CI should be fine.

[Firstyear]

All fixed :)

[Firstyear]

Thank you all so much!