'gpl' cabal flag to switch between HsYAML and aeson-yaml

[patrickmn]

Since the primary/only concern is redistributing GPL components, it seemed to make sense to name the flag 'gpl', and have that flag be opt-in rather than opt-out. That way, the user controls whether they want to be subject to the GPL.

• Adds a cabal flag: gpl (off by default)
• When on, HsYAML is used, and the dhall-to-yaml binary is buildable

Relating to aeson-yaml: A couple of test cases are failing due to aeson-yaml double-quoting all strings.

  ./tasty/data/normal:                FAIL
    tasty/Main.hs:155:
    Conversion to YAML did not generate the expected output
    expected: "bool_value: true\nint_value: 1\nstring_value: 2000-01-01\ntext: |\n  Plain text\n"
     but got: "bool_value: true\nint_value: 1\nstring_value: \"2000-01-01\"\ntext: \"Plain text\\n\""
  ./tasty/data/quoted:                FAIL
    tasty/Main.hs:155:
    Conversion to YAML did not generate the expected output
    expected: "'bool_value': true\n'int_value': 1\n'string_value': '2000-01-01'\n'text': |\n  Plain text\n"
     but got: "bool_value: true\nint_value: 1\nstring_value: \"2000-01-01\"\ntext: \"Plain text\\n\""

I will add quoting of keys to aeson-yaml and update this PR, but unquoted scalar support would significantly complicate things: https://stackoverflow.com/questions/3790454/how-do-i-break-a-string-over-multiple-lines/21699210#21699210

Thoughts?

[sjakobi]

So far this is pretty small change, which is nice! :)

I assume the change in the dhall/dhall-lang submodule was not intended?

@patrickmn Can you show us a diff of some YAML generated with both versions of dhall-to-yaml? Maybe one of the example configs from dhall-kubernetes?

It would be good to have tests for both variants, and to run them in CI. As long as the output differs between the variants, the output files produced with -fgpl could have the file extension .yaml-gpl.

Does this allow us to fully get rid of the Eta-specific code in dhall-json?

[patrickmn]

I assume the change in the dhall/dhall-lang submodule was not intended?

Indeed, submodule change was unintended. Think I fixed it.

@patrickmn Can you show us a diff of some YAML generated with both versions of dhall-to-yaml? Maybe one of the example configs from dhall-kubernetes?

@sjakobi Here's an example of aeson-yaml yaml: https://github.com/clovyr/aeson-yaml/blob/master/test/Test/Data/Aeson/Yaml.hs#L94-L126

Will post a diff and add a gpl cpp flag in a bit.

Does this allow us to fully get rid of the Eta-specific code in dhall-json?

HsYAML doesn't build on eta, but if you're ok with only encoding and not parsing on eta, I can certainly strip that out.

[patrickmn]

HsYAML

apiVersion: apps/v1
kind: Deployment
spec:
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app: nginx
  strategy:
    rollingUpdate:
      maxSurge: 5
      maxUnavailable: 0
    type: RollingUpdate
  template:
    spec:
      containers:
      - image: nginx:1.15.3
        imagePullPolicy: Always
        resources:
          limits:
            cpu: 500m
          requests:
            cpu: 10m
        name: nginx
        ports:
        - containerPort: 80
    metadata:
      name: nginx
      labels:
        app: nginx
  replicas: 2
metadata:
  name: nginx

aeson-yaml

apiVersion: apps/v1
kind: Deployment
metadata:
  name: nginx
spec:
  replicas: 2
  revisionHistoryLimit: 10
  selector:
    matchLabels:
      app: nginx
  strategy:
    rollingUpdate:
      maxSurge: 5
      maxUnavailable: 0
    type: RollingUpdate
  template:
    metadata:
      labels:
        app: nginx
      name: nginx
    spec:
      containers:
        - image: nginx:1.15.3
          imagePullPolicy: Always
          name: nginx
          ports:
            - containerPort: 80
          resources:
            limits:
              cpu: 500m
            requests:
              cpu: 10m

One syntax difference (list indentation), and aeson-yaml sorts objects (hashmaps) by key before outputting.

[patrickmn]

I guess I could add some checks to aeson-yaml to output simple strings unquoted. Will mess with it tomorrow.

[jneira]

HsYAML doesn't build on eta, but if you're ok with only encoding and not parsing on eta, I can certainly strip that out.

I got to build it some time ago, what was the problem when trying to build it?

[ocharles]

Why a manual flag? Why not like Cabal choose whatever if it's unspecified? This opens the door to more potential build plans, if you don't care.

[patrickmn]

That didn't seem like an explicit opt-in.

How can you not care about something that will cause you to have to relicense your project GPL? If you're using dhall-json as a library, that's something you have to consider. And if you're not, it's easy to do cabal v2-install -fgpl dhall-json.

I personally wouldn't expect a Dhall (which is otherwise BSD3) utility package to semi-randomly have this effect. Curious what others think.

[Gabriella439]

I agree with @patrickmn that this flag should definitely be manual (since we don't want cabal's solver to govern the decision of what license to use) and off by default (since I imagine most users will prefer a BSD license but they can enable the GPL version if they better YAML support and are willing to convince management/customers that it's safe)

[ocharles]

But I say you do want Cabal to choose unless you actually care. But it seems like we disagree there, and that's ok - we've had the conversation which is more important

[patrickmn]

FWIW there shouldn't be a case where aeson-yaml won't build; it only depends on aeson, bytestring, text, unordered-containers and vector.

[patrickmn]

HsYAML doesn't build on eta, but if you're ok with only encoding and not parsing on eta, I can certainly strip that out.

I got to build it some time ago, what was the problem when trying to build it?

Will try it again and strip the FFI stuff if it works.

Added simple unquoted strings and (always) quoted encoding to aeson-yaml, and updated the diff above. Will add multi-line literal block scalars as well; then the only difference in the tests should be double- vs single quoting.

[Gabriella439]

@patrickmn: I have a pull request against your branch to enable the -fgpl flag when being built by Nix: https://github.com/patrickmn/dhall-haskell/pull/1

[patrickmn]

I pushed a new aeson-yaml release that uses single quotes for simple strings, and now the yaml tests pass without modification.

[patrickmn]

@jneira On second thought: since this works, let's rip out the Eta FFI stuff in its own PR?