For RAR load, check LD_LIBRARY_PATH before checking install path #1258
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
micahsnyder
changed the title
micahsnyder
force-pushed
the
gh-1249-unrar-module-load-order
branch
from
dd70e1e to
1f0caa2
Compare
micahsnyder
changed the title
TheRaynMan
approved these changes
Apr 29, 2024
micahsnyder
force-pushed
the
gh-1249-unrar-module-load-order
branch
from
1f0caa2 to
2676217
Compare
ClamAV initalization's rarload() function tries to load libclamunrar_iface from the install path before checking under LD_LIBRARY_PATH. This means the unit tests will use the wrong unrar library if testing on a system where ClamAV is already installed. In the event there is an ABI break between versions, this will cause a bunch of tests to fail. This commit fixes the issue by checking for libclamunrar_iface under LD_LIBRARY_PATH *first* before checking in the install lib directory. Note in the previous version we were also checking LD_LIBRARY_PATH on Windows, which is not a thing. I removed this. Fixes: Cisco-Talos#1249 Also removed check for WARN_DLOPEN_FAIL define, which was not used, and mistakenly set for the unrar library build target.
micahsnyder
force-pushed
the
gh-1249-unrar-module-load-order
branch
from
2676217 to
5c6b2d6
Compare
|
@TheRaynMan could use re-review. I tidied it up further while trying to figure out why there were test failures in Jenkins on Windows. (didn't figure it out, seemed fine to me). Will continue to investigate. |
micahsnyder
force-pushed
the
gh-1249-unrar-module-load-order
branch
from
d560f1d to
7502682
Compare
|
Apparently GitHub actions on macOS fails today as well for a couple different reasons. I'm fixing them in this PR as well. |
micahsnyder
force-pushed
the
gh-1249-unrar-module-load-order
branch
10 times, most recently
from
9f2ca94 to
f6bff52
Compare
Upgrade macOS OpenSSL dependency to use 3 instead of 1.1.
Python's pip from Homebrew now refuses to isntall globally:
error: externally-managed-environment
× This environment is externally managed
╰─> To install Python packages system-wide, try brew install
xyz, where xyz is the package you are trying to
install.
If you wish to install a Python library that isn't in Homebrew,
use a virtual environment:
python3 -m venv path/to/venv
source path/to/venv/bin/activate
python3 -m pip install xyz
If you wish to install a Python application that isn't in Homebrew,
it may be easiest to use 'pipx install xyz', which will manage a
virtual environment for you. You can install pipx with
brew install pipx
You may restore the old behavior of pip by passing
the '--break-system-packages' flag to pip, or by adding
'break-system-packages = true' to your pip.conf file. The latter
will permanently disable this error.
If you disable this error, we STRONGLY recommend that you additionally
pass the '--user' flag to pip, or set 'user = true' in your pip.conf
file. Failure to do this can result in a broken Homebrew installation.
Read more about this behavior here: <https://peps.python.org/pep-0668/>
Using Pipx instead. Making the same change for Ubuntu just in case.
micahsnyder
force-pushed
the
gh-1249-unrar-module-load-order
branch
from
f6bff52 to
68b4ab4
Compare
|
UGH finally found the correct path for brew library installs after github's change to the macOS runners: I also fixed the windows build issues which had to do with the length of the path/commands in our Jenkins and not an actual bug in our code afaict. I solved that by relocating the build directory to be next to the source dir and to use a short directory name. Previously had been longer and in a subdirectory of the source directory. I should be done fussing with this now. |
TheRaynMan
approved these changes
May 3, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ClamAV initalization's rarload() function tries to load libclamunrar_iface from the install path before checking under LD_LIBRARY_PATH.
This means the unit tests will use the wrong unrar library if testing on a system where ClamAV is already installed.
In the event there is an ABI break between versions, this will cause a bunch of tests to fail.
This commit fixes the issue by checking for libclamunrar_iface under LD_LIBRARY_PATH first before checking in the install lib directory.
Note in the previous version we were also checking LD_LIBRARY_PATH on Windows, which is not a thing. I removed this.
Fixes: #1249
Also removed check for WARN_DLOPEN_FAIL define, which was not used, and mistakenly set for the unrar library build target.