many: pass device/model info to configcore via sysconfig.Device interface #10446
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
introduce sysconfig.Device (similar to boot.Device) have sysconfig.ApplyFilesystemOnlyDefaults and sysconfig.ConfigureTargetSystem take the model and pass down a sysconfig.Device for these two we assume that mode is not run
adjust the affected code
the information is passed via sysconfig.Device now
this gives us more information more cleanly than the current approach reading the kernel command line directly
mvo5
approved these changes
Jun 24, 2021
| c.Check(err, ErrorMatches, `cannot set "core.unknown.option": unsupported system option`) | ||
| } | ||
|
|
||
| type mockDev struct { |
| } | ||
|
|
||
| var ( | ||
| coreDev = mockDev{classic: false} |
| sysconfig.ApplyFilesystemOnlyDefaultsImpl = func(rootDir string, defaults map[string]interface{}, options *sysconfig.FilesystemOnlyApplyOptions) error { | ||
| return filesystemOnlyApply(rootDir, defaults, options) | ||
| } | ||
| sysconfig.ApplyFilesystemOnlyDefaultsImpl = filesystemOnlyApply |
| defer restore() | ||
|
|
||
| // mock the device as uc20 run mode | ||
| mockCmdline := filepath.Join(dirs.GlobalRootDir, "cmdline") |
There was a problem hiding this comment.
Seeing how much less mocking is needed with this change is really nice
| } | ||
|
|
||
| func (di *deviceInfo) RunMode() bool { | ||
| return false |
There was a problem hiding this comment.
(nipick) maybe a small comment here that this is false because in the sysconfig context where this is used we are never in RunMode ?
the functions using configedDevice are not used from run mode (thanks @mvo)
bboozzoo
approved these changes
Jun 24, 2021
| Classic() bool | ||
|
|
||
| Kernel() string | ||
| //Base() string |
There was a problem hiding this comment.
A TODO maybe depending on whether we actually need Base and Model?
There was a problem hiding this comment.
I'll take care of this in a follow up one way or another. I want to cover at least one of the TODO I left in configcore. The one where we still read the kernel command line.
|
The failures are unrelated. |
This was referenced Jun 25, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is similar to boot.Device.
Use this which is cleaner and delivers more information in the pi options code as well.
Leaves a couple of TODO about where this could also be leveraged as well.