Skip to content

Vendorruntimetools #1007

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.

Sign up for GitHub

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

Jump to bottom
Closed
baude wants to merge 3 commits into from
Closed

Vendorruntimetools #1007

baude wants to merge 3 commits into from

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
@baude
Vendor in latest runtime-tools
8e22d87 
Newer runtime tools separates syscalls by OS so we can build darwin.

Signed-off-by: baude <bbaude@redhat.com>
@baude
Vendor in latest buildah
dc112dc 
Signed-off-by: baude <bbaude@redhat.com>
@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

wking
wking reviewed Jun 27, 2018
View reviewed changes
pkg/spec/spec.go Outdated
Copy link
Contributor

@wking wking Jun 27, 2018

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

umohnani8 commented Jun 27, 2018

LGTM, but tests are unhappy

@baude
generator.New() requires an OS string input variable
1110e73 
Signed-off-by: baude <bbaude@redhat.com>
@baude baude force-pushed the vendorruntimetools branch from efd1b16 to 1110e73 Compare June 27, 2018 14:45
wking
wking reviewed Jun 27, 2018
View reviewed changes
libpod/common_test.go
if err != nil {
return nil, err
}
ctr.config.Spec = g.Spec()
Copy link
Contributor

@wking wking Jun 27, 2018

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

@wking wking Jun 27, 2018

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

rh-atomic-bot commented Jun 27, 2018

📌 Commit 1110e73 has been approved by baude

@rh-atomic-bot
Copy link
Collaborator

rh-atomic-bot commented Jun 27, 2018

⚡ Test exempted: merge already tested.

rh-atomic-bot pushed a commit that referenced this pull request Jun 27, 2018
@baude @rh-atomic-bot
Vendor in latest buildah
e1b47c1 
Signed-off-by: baude <bbaude@redhat.com>

Closes: #1007
Approved by: baude
rh-atomic-bot pushed a commit that referenced this pull request Jun 27, 2018
@baude @rh-atomic-bot
generator.New() requires an OS string input variable
c32c491 
Signed-off-by: baude <bbaude@redhat.com>

Closes: #1007
Approved by: baude
@wking wking mentioned this pull request Jun 27, 2018
Closed
@wking
Copy link
Contributor

wking commented Jun 27, 2018 •
edited
Loading

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
@wking
*: Replace Generator.Spec() with Generator.Config
4e7d15f 
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 <wking@tremily.us>
rh-atomic-bot pushed a commit that referenced this pull request Jun 27, 2018
@wking @rh-atomic-bot
*: Replace Generator.Spec() with Generator.Config
fd12c89 
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 <wking@tremily.us>

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.

Reviewers

1 more reviewer

@wking wking wking left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@baude @rhatdan @mheon @umohnani8 @rh-atomic-bot @wking