Skip to content
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

Improve Fedora detection #963

Merged
merged 1 commit into from
Jun 20, 2023
Merged

Conversation

mjgarton
Copy link
Contributor

@mjgarton mjgarton commented Jun 15, 2023

Instead of relying on $NAME, which has changed in recent releases, use $ID, which appears more stable.

Instead of relying on NAME, which has changed in recent releases, use
ID, which appears more stable.
Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

Is $ID more stable than $NAME for just Fedora, or for other distros, too?

@mjgarton
Copy link
Contributor Author

mjgarton commented Jun 19, 2023

It seems that only Fedora has changed the NAME value. The others have all remained the same for a long time. I ran the following to check a few distros, in case you want to quickly check this yourself:

IMAGES="debian:7 debian:8 debian:9 debian:10 debian:11 debian:12 \
	ubuntu:12.04 ubuntu:12.10 ubuntu:13.04 ubuntu:13.10 ubuntu:14.04 ubuntu:14.10 ubuntu:15.04 ubuntu:15.10 ubuntu:16.04 ubuntu:16.10 \
       	ubuntu:17.04 ubuntu:17.10 ubuntu:18.04 ubuntu:18.10 ubuntu:19.04 ubuntu:19.10 ubuntu:20.04 ubuntu:20.10 ubuntu:21.04 ubuntu:21.10 ubuntu:22.04 \
	fedora:20 fedora:21 fedora:22 fedora:23 fedora:24 fedora:25 fedora:26 fedora:27 fedora:28 fedora:29 fedora:30 fedora:31 fedora:32 fedora:33 fedora:34 fedora:35 fedora:36 fedora:37 fedora:38"

for i in $IMAGES; do
	echo -n  $i :
	docker run $i grep ^NAME= /etc/os-release
done

for i in $IMAGES; do
	echo -n  $i :
	docker run $i grep ^ID= /etc/os-release
done

Copy link
Contributor

@kkysen kkysen left a comment

Choose a reason for hiding this comment

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

That looks pretty thorough. Using $ID for just Fedora makes sense then, and if any others ever change, we can use $ID for them, too.

@kkysen kkysen merged commit 355647e into immunant:master Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants