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

specs-go/config: Make Linux and Solaris omitempty (again) #502

Merged
merged 1 commit into from
Jun 24, 2016

Conversation

wking
Copy link
Contributor

@wking wking commented Jun 18, 2016

I'd added some omitempties in 5c2193f (specs-go/config: Make Linux and Solaris omitempty, 2016-05-06, #431), but it turns out to not have the intended effect unless the field is also a pointer type (even after I shifted the omitempty from the platform tag to the json tag ;). Before this commit:

$ ./ocitools generate --template <(echo '{}')
$ jq . config.json
{
  "ociVersion": "1.0.0-rc1-dev",
  "platform": {
    "os": "linux",
    "arch": "amd64"
  },
  "process": {
    "user": {
      "uid": 0,
      "gid": 0
    },
    "args": [],
    "cwd": "/"
  },
  "root": {
    "path": "rootfs"
  },
  "hooks": {},
  "linux": {
    "cgroupsPath": ""
  },
  "solaris": {
    "cappedCPU": {},
    "cappedMemory": {}
  }
}

And after this commit:

$ ./ocitools generate --template <(echo '{}')
$ jq . config.json
{
  "ociVersion": "1.0.0-rc1-dev",
  "platform": {
    "os": "linux",
    "arch": "amd64"
  },
  "process": {
    "user": {
      "uid": 0,
      "gid": 0
    },
    "args": [],
    "cwd": "/"
  },
  "root": {
    "path": "rootfs"
  },
  "hooks": {},
}

The remaining useless properties are addressed by other in-flight pull requests:

So I've left them alone here.

I'd added some omitempties in 5c2193f (specs-go/config: Make Linux
and Solaris omitempty, 2016-05-06, opencontainers#431), but it turns out to not have
the intended effect unless the field is also a pointer type (even
after I shifted the 'omitempty' from the platform tag to the json
tag).  Before this commit:

  $ ./ocitools generate --template <(echo '{}')
  $ jq . config.json
  {
    "ociVersion": "1.0.0-rc1-dev",
    "platform": {
      "os": "linux",
      "arch": "amd64"
    },
    "process": {
      "user": {
        "uid": 0,
        "gid": 0
      },
      "args": [],
      "cwd": "/"
    },
    "root": {
      "path": "rootfs"
    },
    "hooks": {},
    "linux": {
      "cgroupsPath": ""
    },
    "solaris": {
      "cappedCPU": {},
      "cappedMemory": {}
    }
  }

And after this commit:

  $ ./ocitools generate --template <(echo '{}')
  $ jq . config.json
  {
    "ociVersion": "1.0.0-rc1-dev",
    "platform": {
      "os": "linux",
      "arch": "amd64"
    },
    "process": {
      "user": {
        "uid": 0,
        "gid": 0
      },
      "args": [],
      "cwd": "/"
    },
    "root": {
      "path": "rootfs"
    },
    "hooks": {},
  }

The remaining useless properties are addressed by other in-flight pull
requests:

* 5ca74df (config: Make 'process.args' optional, 2016-06-04, opencontainers#489)
* ad33f9c (config: Explicitly list 'hooks' as optional, 2016-05-06,
  opencontainers#427)

So I've left them alone here.

Signed-off-by: W. Trevor King <wking@tremily.us>
@dqminh
Copy link
Contributor

dqminh commented Jun 22, 2016

LGTM

Approved with PullApprove

@crosbymichael
Copy link
Member

crosbymichael commented Jun 24, 2016

LGTM

Approved with PullApprove

@crosbymichael crosbymichael merged commit b45aa77 into opencontainers:master Jun 24, 2016
@wking wking deleted the optional-linux-solaris branch August 18, 2016 04:54
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Sep 21, 2016
Otherwise we it will never be omitted when empty.  This is the same
case as 6323157 (specs-go/config: Make Linux and Solaris omitempty
(again), 2016-06-17, opencontainers#502).

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

3 participants