containers / toolbox

Tool for interactive command line environments on Linux
https://containertoolbx.org/
Apache License 2.0
2.38k stars 208 forks source link

profile.d: don't try to source non-existent os-release file #1475

Open ptalbert opened 3 months ago

ptalbert commented 3 months ago

When /etc/os-release does not exist we directly try to source /usr/lib/os-release. This causes a 'No such file or directory' error when entering a toolbox which has neither.

Fix this by checking for the presence of /usr/lib/os-release before sourcing it, just like what is done for /etc/os-release.

softwarefactory-project-zuul[bot] commented 3 months ago

Build failed. https://softwarefactory-project.io/zuul/t/local/buildset/5663ec31cedd4dd496124fcd2eac3897

:heavy_check_mark: unit-test SUCCESS in 4m 57s :heavy_check_mark: unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 38s :heavy_check_mark: unit-test-restricted SUCCESS in 4m 01s :x: system-test-fedora-rawhide TIMED_OUT in 1h 20m 30s :heavy_check_mark: system-test-fedora-39 SUCCESS in 35m 17s :heavy_check_mark: system-test-fedora-38 SUCCESS in 35m 21s

mh21 commented 3 months ago

@ptalbert should it set some defaults for the expected variables that now are unset?

ptalbert commented 3 months ago

On Tue, Apr 2, 2024 at 5:49 PM Michael Hofmann @.***> wrote:

@ptalbert https://github.com/ptalbert should it set some defaults for the expected variables that now are unset?

I don't see this script doing anything special which requires some default value. In the case where neither os-release file exists the behaviour of the script will not be any different after this change.

— Reply to this email directly, view it on GitHub https://github.com/containers/toolbox/pull/1475#issuecomment-2032426745, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABQY3SB6SOHTYNC4VLS5NGDY3LHPNAVCNFSM6AAAAABFTGNLVGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMZSGQZDMNZUGU . You are receiving this because you were mentioned.Message ID: @.***>

mh21 commented 2 months ago

This brings me to the question: how did you end up without any os-release(5)?

By using a RHEL6-based container image 😂. But if this is really going to ripple through multiple pieces of code here, maybe it would be easier to just fake an os-release file in the container image -> https://gitlab.com/cki-project/containers/-/merge_requests/539

ptalbert commented 2 months ago

@debarshiray like Michael said, I noticed this when using a rhel6 container image with toolbox. Every time you enter the toolbox (or source your profile in the container) there is this warning about a missing /usr/lib/os-release.

For our purposes I am happy to take Michael's solution but maybe there are a few unrelated people out there using toolbox with containers that predate os-release 🤷. If there is something else I can test please let me know.