MsgpackHandle.RawToString in composite literal no longer works

[dcarbone]

This commit: https://github.com/ugorji/go-codec/commit/a70535d8491cc93f3813e0946f4399501f3cb47f breaks backwards compatibility for anybody who attempted to construct MsgpackHandle as a literal.

Take this example code:

package main

import (
    "fmt"
    "github.com/ugorji/go/codec"
)

func main() {
    fmt.Println(&codec.MsgpackHandle{RawToString: true})
}

Prior to this commit, this worked as expected. Now, however, this no longer works as the location of RawToString has been moved to reside within BasicHandle, which in turns embeds DecodeOptions, which finally has the RawToString field.

This is not the end of the world if you are the direct maintainer of the above code, but when the codec package is only included as a dep of a dep, this becomes a problem.

The inability to properly constrain this repo to 1.1.2 due to package structure only exacerbates this issue.

[ugorji]

Thanks for filing this.

This is limited to

• users of MsgpackHandle
• who configure RawToString
• using composite literals

I can try to re-introduce the RawToString field directly in MsgpackHandle and hack it so that it is kept in sync with the value in DecodeOptions. But I don't want to do that unless there is a clear need for it. If the user base is small and we can get the users to make changes to accommodate, then it may not be worth bastardizing it.

Can you point to the package(s) that uses this, and let me see if they are willing to make changes to their code?

We will also keep this bug open for a bit to see who else clamors for it.

[dcarbone]

I was directly impacted by this while developing a product based on https://github.com/gammazero/nexus.

I have an accepted PR that addresses this: https://github.com/gammazero/nexus/pull/168 but no release has been made.

It was only an issue for me at the time due to the issue I raised in #291. I have since decided that modules is a feature not ready for prime time and have gone back to dep, which does properly determine which version of this package to use based on the govendor file checked in with the nexus repo.

I agree that it is not a huge deal, it was only made an issue for me by the problem presented by #291.

[ugorji]

Ok. Let's leave this open for some weeks and see who else expresses issues with it. We may also be able to get at a more robust solution.

Thanks.

[dcarbone]

@ugorji: nexus has since created a release with my fix in place, so this is no longer an issue for myself. Given how few people have responded I'm going to assume this is a very remote edge case, and unless you say otherwise will close this issue later today.

Eager to see what happens with #299.

[ugorji]

Thanks @dcarbone . I am ok with closing it. Thanks.