paketo-buildpacks/packit

Simplify buildpack.toml encoding/decoding

sophiewigmore opened this issue · 4 comments

The code in cargo/config.go is responsible for decoding buildpack.toml files into Go structs, as well was encoding structs into TOML. This code is integral to the jam tool.

There are a couple of concerns around this code, given how integral it is to our tools.

  1. The TOML decoding/encoding is done with the BurntSushi/toml package, which is in an unmaintained/archived status. It's worrisome to use a package in this state.
  2. Due to the nature of the structs involved and the usage of the BurntSushi/toml package we have to convert into JSON as an intermittent step in order to decode/encode correctly. This makes for extremely complicated and hard to maintain code.

While all of this functionality works for our use case, this issue is here to document the technical debt associated with this code.

There is a newer TOML library, github.com/pelletier/go-toml that looks like a promising alternative for our use case. Upon initial investigation, there are a couple outstanding issues around using custom TOML marshalling that prevent us from using it currently. Upon the fix of theses issues this could be an option to resolve this.

fg-j commented

@sophiewigmore Is this still blocked on improvements to pelletier/go-toml? If so, can you link to the outstanding issues? If not, feel free to remove the status/blocked label.

fg-j commented

Related to this issue, I notice that buildpack_info.go and cargo/config.go both contain structs that represent the contents of a buildpack.toml. @paketo-buildpacks/tooling-maintainers , why do we have two separate representations? When the spec for buildpack.toml contents changes, we end up having to make changes in two places, as with recent SBOM work.

@fg-j Interestingly enough, BurntSushi doesn't appear to be unmaintained or archived anymore, so this is less of a worrisome issue. However, the issue of complicated code still stands.

I've been digging around and I can't find the issues in go-toml I mentioned in the original issue (WHY didn't I link it then?) @ForestEckhardt we worked on this together, do you recall? Looking at releases for Go Toml, it seems like the new v2 release of the library has a lot of changes to the encoder/decoder. I wish I had been clearer in this issue about the actual problem we were seeing when we tried to switch over. For now, I wouldn't consider this blocked until we can determine that.

@fg-j RE: the need for buildpack_info.go and cargo/config.go my hunch is that we need Cargo's version to abstract away Paketo's opinionated choices in the structs from the top-level packit package. The fields in buildpack_info.go are directly defined in the spec, so they can live in the top-level. It's still confusing, though. It might make more sense to pull in the top-level packit BuildpackInfo struct to replace ConfigBuildpack.