-
Notifications
You must be signed in to change notification settings - Fork 545
Fix lliurex detection #1882
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
base: dev
Are you sure you want to change the base?
Fix lliurex detection #1882
Conversation
src/detection/os/os_linux.c
Outdated
"/usr/bin/lliurex-version", | ||
NULL, | ||
}) == NULL) // 8.2.2 | ||
ffStrbufTrimRightSpace(&result->versionID); |
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.
You appended the version string to &result->version
, but triming &result->versionID
here?
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.
And you put executable binaries to /etc/
? Shouldn't it be a regular text file?
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.
Hi, the trim was left by mistake.
For the another question there's no binary at /etc. The check for /etc/lliurex-cdd-version is for a regular txt file, the binary to get the lliurex version is at /usr/bin/lliurex-version. First checks for the file, then (if file exists) gets the version through a binary. I could check the binary itself directly but it will fail if the aforementioned file didn't exist so I believe is better to check for the file.
p.s. I've made another commit fixing the trim
NULL, | ||
}) == NULL) // 8.2.2 | ||
ffStrbufTrimRightSpace(&result->versionID); | ||
ffStrbufSetF(&result->prettyName, "LliureX %s", result->version.chars); |
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.
What if ffProcessAppendStdOut failed?
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.
lliurex-version is a basic system command in lliurex, if it fails then something is really wrong, it wold be as if command lsb_release fails. If you want I can add any behaviour on fail but the tests I did seemed to be ok without any issue at all.
Cheers
Hi. Some time ago I did a PR for adding support for the Lliurex distribution. After the PR was accepted with changes I didn't test them (sorry for that) because i was facing some troubles and forgot about this but as lliurex is a mix of Ubuntu LTS and Neon the detection based on XDG_CONFIG_DIRS was failing because conflicted with kubutnu as both use plasma as desktop.
This patch fixes this problem checking for the presence of a lliurex-specific file (/etc/lliurex-cdd-version) and getting the version from another specific command of llurex.
I've included the detection code just before the tests for XDG_CONFIG_DIRS because xdg_config_dirs in lliurex also contains "plasma" so any detection mechanism for lliurex must run before the check for kubuntu; also the old detection code has been removed.
Thanks!