-
Notifications
You must be signed in to change notification settings - Fork 20
Add image build info #146
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
Add image build info #146
Conversation
basak-qcom
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - you found specified keys in os-release that work for us? Nice!
But, /etc/os-release is a symlink to /usr/lib/os-release that is shipped by base-files . Modifying /usr doesn't seem like a good idea, but also this will get stepped on any time base-files is updated, which we do not control. A completely unrelated update to base-files in Debian would wipe out these changes. If you want to use /etc/os-release still, I think we need a better packaging solution to achieve what we want that won't get overwritten, that either involves us carrying a delta/overlay for base-files, or uses some other packaging messaging (diverts?)
bbe2884 to
6101a1d
Compare
Add a buildid variable that will be captured in /etc/buildinfo along with the variant/flavor (console or xfce) of the image. Fixes: qualcomm-linux#37 Signed-off-by: Loïc Minier <[email protected]>
Signed-off-by: Loïc Minier <[email protected]>
6101a1d to
769d856
Compare
Most people I asked expected the info directly in os-release, and until your comment I thought it was a conffile. I was ok with the conffile patching as I thought that a dist upgrade would replace the contents, and that seemed proper, but I don't like patching a regular file which has an expected debsum etc. So I've pushed a new version that creates /etc/buildinfo, as Yocto does; that seems to be the second most popular expectation (with variations of the name in /etc). Kudos to the debos upstream for suggesting /etc/debos-release too :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was ok with the conffile patching as I thought that a dist upgrade would replace the contents, and that seemed proper, but I don't like patching a regular file which has an expected debsum etc.
FWIW, I wouldn't be happy with that either, since it would mean that the user will receive a conffile prompt asking them to merge the changes on every [edit: release] upgrade, which they won't understand. But it's moot if you use /etc/buildinfo instead.
As we've discussed extensively elsewhere, I'm unhappy with using /etc for this now that it's not a defined field from the /etc/os-release spec (that seems more suitable for an image-based distribution like Yocto or other immutable distros). I think /var/log should be used instead. But it's more important that we have a decision to make progress and you've made it, so fine :)
As you've probably noticed I have a strong preference to add tests for everything that we change where possible, but pragmatically have been asking to validate changes PRs manually instead in cases where we don't have an appropriately straightforward way to add a test yet.
But maybe in this we can add a test? How hard would this be? I think we should check that the fields exist in the expected place in a test here, for example, and are of the form that we expect (eg. a regexp for each expected value). If our test infrastructure doesn't make this easy yet, I think it's OK to skip for now if we just validate the result manually instead.
|
I like the idea of a test; it would be nice to have a qemu test framework in place, we could also test this in LAVA, it's not very device specific though, so not sure there is good value in running a generic test like this on every device Could we (could you :-p) resurrect this PR #74? I can help with workflows if needed, and add a test for the buildinfo stuff |
|
I downloaded rootfs.tar.gz from CI and unpacked etc/buildinfo: |
Was addressed as part of qualcomm-linux#146 Signed-off-by: Loïc Minier <[email protected]>
Add image build information when running debos. Specifically, add build id and variant/flavor to /etc/buildinfo.
Fixes: #37