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.
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.
curious; would this mean that if I have set
SSL_CERT_FILE
as a user, that it would be ignored?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.
@thaJeztah yes, it would. It appears that PyInstaller sets it so this is just reversing what it does. We only use this for the environment that we shell out to the SSH client with and from what I can see, the SSH client doesn't care about this env var
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.
@chris-crone you posted this link on slack; https://pyinstaller.readthedocs.io/en/stable/runtime-information.html#ld-library-path-libpath-considerations, which mentions;
Does that only apply to
LD_LIBRARY_PATH
, or also toSSL_CERT_FILE
? (if the latter, we could restore the value by settingSSL_CERT_FILE
back toSSL_CERT_FILE_ORIG
, correct?)That's w.r.t
LD_LIBRARY_PATH
orSSL_CERT_FILE
? (or both?)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.
From what I can tell it's not actually setting
_ORIG
forLD_LIBRARY_PATH
orSSL_CERT_FILE
. Test rig:docker run --rm -it ubuntu:20.04
DOCKER_HOST=ssh://localhost
and add ssh script toPATH
docker-compose.yml
./docker-compose ps
env
[2][1] ssh script:
[2] env:
Original env:
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.
Do you still have your environment set up? Could you do a quick test and before
./docker-compose ps
doThen check if it perhaps sets
_ORIG
conditionally (so only if it was set before?)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 same for
LD_LIBRARY_PATH
, soexport LD_LIBRARY_PATH=/some/other/path
before running)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.
Ah yes, that would've been a better test.
I don't have it setup unfortunately :( Since this is us providing SSH using the user's SSH client, I think it's not unreasonable to unset these variables though and rely on the system's settings.