script.module.inputstreamhelper icon indicating copy to clipboard operation
script.module.inputstreamhelper copied to clipboard

Improve get_lib_version, use as check

Open horstle opened this issue 3 years ago • 6 comments

Based on the code from @CastagnaIT, this improves the functionality of get_lib_version and adds a check to it. It now returns an empty string if the file is corrupt or the architecture doesn't match.

Currently unit tests fail. I guess that's because they are running on x86 hardware, so the aforementioned check fails. So far I only tested Linux x86 manually.

Closes #413.

horstle avatar Dec 23 '20 00:12 horstle

Currently unit tests fail.

Strange, maybe related to this: https://github.community/t/segmentation-fault-when-running-binary-in-a-job/18418

mediaminister avatar Dec 29 '20 13:12 mediaminister

@horstle I found and fixed the problem:

  • the segmentation fault was caused by loading the shared library several times in quick succession. Unloading the library fixes this. There was also a problem with loading the 32-bit ARM lib. It throws an OSError with wrong ELF class: ELFCLASS32

I added the old get_lib_version again as a fallback.

Feel free to make a better fix, but this is good enough for me to get merged.

mediaminister avatar Jan 06 '21 16:01 mediaminister

Ha, while looking at previous discussions I found my original code from a year ago:

https://github.com/emilsvennesson/script.module.inputstreamhelper/issues/248#issuecomment-575961788

dagwieers avatar Jan 07 '21 04:01 dagwieers

@mediaminister It is supposed to fail when there is an arch mismatch, so it won't install the wrong lib. That's what this PR is trying to fix (see #413). Also, the legacy version doesn't detect an arch mismatch, so it can't be used as a check. Instead of writing worse code to pass the tests, we should improve the tests.

I'm currently looking into AWS CodeBuild, so we can run the ARM tests on ARM hardware. I think it makes sense to have separate Github actions for each platform, then the ARM tests could be run via CodeBuild and maybe at some point the Windows tests could run on Windows (it seems that's already possible in Github, but idk if it would cost something, might look into it later). That way closing the lib wouldn't be necessary, too, although I wouldn't mind keeping that change.

horstle avatar Jan 07 '21 10:01 horstle

SonarCloud Quality Gate failed.

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
9.5% 9.5% Duplication

sonarcloud[bot] avatar Jan 07 '21 12:01 sonarcloud[bot]

Okay, I removed the fallback. Tests fail, but now you can see why.

mediaminister avatar Jan 07 '21 12:01 mediaminister