Skip to content
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

Optional TOML Support (disabled by default) #208

Closed
wants to merge 1 commit into from

Conversation

akutz
Copy link
Collaborator

@akutz akutz commented Jul 18, 2016

This patch makes TOML support optional, disabling it by default. To enable support include the Golang build tag toml. For example, the following command builds Viper without TOML support:

$ go build

Whereas this command will build Viper, along with support for parsing TOML:

$ go build -tags toml

The reason for making TOML optional is due to the dependency upon the BurntSushi package that is used to unmarshal TOML content. The package is licensed under the WTFPL license and incompatible with the Kubernetes project. Pull requests submitted to K8 that transitively include a dependency upon packages licensed under the WTFPL license are denied. The reason given links to this discussion.

To this end, until such time an alternative package is deemed acceptable for parsing TOML content, this patch would make TOML an optional component of Viper, enabled only when explicitly requested via the build tag toml.

To prove no TOML is present, from the root of the branch execute:

$ go list -f '{{join .Deps "\n"}}' | grep toml

No matching packages are found. Using the build tag toml will reintroduce the now optional component:

$ go list -tags toml -f '{{join .Deps "\n"}}' | grep toml
github.com/BurntSushi/toml

@akutz
Copy link
Collaborator Author

akutz commented Jul 18, 2016

Hi @spf13,

Doh! It looks like I did not account for the tests that expect the TOML parser to have executed. I will update the PR.

This patch makes TOML support optional, disabling it by default. To
enable support include the Golang build tag `toml`. For example, the
following command builds Viper without TOML support:

        $ go build

Whereas this command will build Viper, along with support for parsing
TOML:

        $ go build -tags toml

The reason for making TOML optional is due to the dependency upon the
BurntSushi package that is used to unmarshal TOML content. The package
is licensed under the WTFPL license (http://www.wtfpl.net/) and
incompatible with the Kubernetes project. Pull requests submitted to K8
that transitively include a dependency upon packages licensed under the
WTFPL license are denied.

To this end, until such time an alternative package is deemed acceptable
for parsing TOML content, this patch would make TOML an optional
component of Viper, enabled only when explicitly requested via the build
tag `toml`.
@akutz
Copy link
Collaborator Author

akutz commented Jul 18, 2016

Hi @spf13,

I can provide far more information as to why this is necessary; one of my colleagues typed up quite a summary relating to the issues of the transitive license in question. I bring this up because in my work to update the tests, more work than with which I felt comfortable performing was required, and I'm not sure it meets your standards for this sort of behavior.

@akutz
Copy link
Collaborator Author

akutz commented Jul 26, 2016

Hi @spf13,

Any thoughts on this? If nothing else, could we make this a branch in this repository so projects that use GoDeps can point to this import path and the Git ref of the branch's tip with this change? As it is, it doesn't appear that something like K8 can use this PR from my fork since K8 uses GoDeps, and that seems to only work with the proper import path (unlike Glide which enables you to point to a fork).

akutz added a commit to akutz/viper that referenced this pull request Aug 2, 2016
This patch updates the package used for parsing TOML content from
"github.com/BurntSushi/toml" to "github.com/pelletier/go-toml" as the
latter uses a more accepted OSS license (MIT), enabling the inclusion of
Viper or projects that depend on Viper in projects that have licensing
requirements incongruent with the license of the previous TOML package.

This patch replaces the PR spf13#208 after
discussing the matter with @spf13 and deciding to update the TOML parser
instead of making TOML build-optional.
@akutz akutz mentioned this pull request Aug 2, 2016
@akutz
Copy link
Collaborator Author

akutz commented Aug 2, 2016

Closed after discussion with @spf13 in favor of #217.

@akutz akutz closed this Aug 2, 2016
bep pushed a commit that referenced this pull request Aug 5, 2016
This patch updates the package used for parsing TOML content from
"github.com/BurntSushi/toml" to "github.com/pelletier/go-toml" as the
latter uses a more accepted OSS license (MIT), enabling the inclusion of
Viper or projects that depend on Viper in projects that have licensing
requirements incongruent with the license of the previous TOML package.

This patch replaces the PR #208 after
discussing the matter with @spf13 and deciding to update the TOML parser
instead of making TOML build-optional.
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.

None yet

1 participant