Skip to content

Conversation

@baude
Copy link
Member

@baude baude commented Jun 27, 2018

Update runtime-tools and buildah to account for the the generator functions and Darwin compilability.

baude added 2 commits June 27, 2018 08:59
Newer runtime tools separates syscalls by OS so we can build darwin.

Signed-off-by: baude <[email protected]>
@rhatdan
Copy link
Member

rhatdan commented Jun 27, 2018

LGTM
@mheon @wking @umohnani8 @TomSweeneyRedHat PTAL

@baude baude force-pushed the vendorruntimetools branch from 469ac8a to efd1b16 Compare June 27, 2018 14:13
@mheon
Copy link
Member

mheon commented Jun 27, 2018

bot, retest this please

@mheon
Copy link
Member

mheon commented Jun 27, 2018

LGTM pending tests

pkg/spec/spec.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of CreateConfigToOCISpec is pretty Linux-specific, so I'm fine with this being hard-coded here. But CreateConfigToOCISpec is so opinionated, I'm surprised you need to seed it at all. Do you know what parts of this initial config survive the subsequent edits to be returned to the caller? You might just want to build the *spec.Spec object directly.

@umohnani8
Copy link
Member

LGTM, but tests are unhappy

@baude baude force-pushed the vendorruntimetools branch from efd1b16 to 1110e73 Compare June 27, 2018 14:45
if err != nil {
return nil, err
}
ctr.config.Spec = g.Spec()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have g.Spec() -> g.Config (and similarly in CreateConfigToOCISpec).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have g.Spec() -> g.Config (and similarly in CreateConfigToOCISpec).

I've filed #1008 for this.

@baude
Copy link
Member Author

baude commented Jun 27, 2018

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 1110e73 has been approved by baude

@rh-atomic-bot
Copy link
Collaborator

⚡ Test exempted: merge already tested.

rh-atomic-bot pushed a commit that referenced this pull request Jun 27, 2018
Signed-off-by: baude <[email protected]>

Closes: #1007
Approved by: baude
rh-atomic-bot pushed a commit that referenced this pull request Jun 27, 2018
Signed-off-by: baude <[email protected]>

Closes: #1007
Approved by: baude
@wking
Copy link
Contributor

wking commented Jun 27, 2018

Hmm, looks like this PR also forgot to bump vendor.conf. @baude, did you want to file a fixup with the dependency commits you used? Or should we deal with this if/when I have time to get back to #747?

wking added a commit to wking/libpod that referenced this pull request Jun 27, 2018
Catching up with opencontainers/runtime-tools@84a62c6a (generate: Move
Generator.spec to Generator.Config, 2016-11-06, containers#266, v0.6.0), now
that we've bumped runtime-tools in f6c0fc1 (Vendor in latest
runtime-tools, 2018-06-26, containers#1007).

Signed-off-by: W. Trevor King <[email protected]>
rh-atomic-bot pushed a commit that referenced this pull request Jun 27, 2018
Catching up with opencontainers/runtime-tools@84a62c6a (generate: Move
Generator.spec to Generator.Config, 2016-11-06, #266, v0.6.0), now
that we've bumped runtime-tools in f6c0fc1 (Vendor in latest
runtime-tools, 2018-06-26, #1007).

Signed-off-by: W. Trevor King <[email protected]>

Closes: #1008
Approved by: mheon
@baude baude deleted the vendorruntimetools branch December 22, 2019 19:17
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants