CDRIVER-4519 Replace manual OCSP venv creation with activate-ocspvenv.sh #1163
Conversation
| @@ -1055,7 +1055,7 @@ def to_dict(self): | |||
| test_column = 'TEST_4' if self.test == 'cache' else self.test.upper() | |||
|
|
|||
| commands.append(shell_mongoc( | |||
| 'TEST_COLUMN=%s CERT_TYPE=%s USE_DELEGATE=%s sh .evergreen/run-ocsp-responder.sh' % ( | |||
| 'TEST_COLUMN=%s CERT_TYPE=%s USE_DELEGATE=%s bash .evergreen/run-ocsp-responder.sh' % ( | |||
There was a problem hiding this comment.
This actually tripped me up while working on mongodb/mongo-php-driver#1391. I was wondering why neither the shell: bash option on the shell.exec command nor the #!/usr/bin/env bash line in the script were still leading to errors about unsupported shell syntax in the venv and find-python scripts.
On a separate note, I did some significant refactoring to the run-ocsp-responder.sh script in that PR if you'd like to adapt it here. I imagine libmongoc previously clones the drivers-evergreen-tools repo, so I decided to reference $DRIVERS_TOOLS and $PROJECT_DIRECTORY directly in that script.
Writing this comment reminded me that I neglected to add $DRIVERS_TOOLS and $PROJECT_DIRECTORY and to the script's doc header as required env vars, so I'll make sure I do so before merging.
I think your script has the same issue with referring to $CDRIVER_ROOT and $CDRIVER_BUILD.
There was a problem hiding this comment.
I think I will prefer to leave the changes minimal for now.
$CDRIVER_ROOT and $CDRIVER_BUILD are defined at the top of the file and defaults to the current working directory if not already set. As far as I can tell, these variables are never set prior to invocation of this script from within the mongoc directory in all C Driver Evergreen tests, so the assumptions used here should not be an issue, unlike what may have been in the PHP Driver.
I imagine libmongoc previously clones the drivers-evergreen-tools repo
This is not always guaranteed; it depends on the given task and functions invoked, which is why this PR uses a check+clone.
There was a problem hiding this comment.
$CDRIVER_ROOT and $CDRIVER_BUILD are defined at the top of the file...
Ah, missed that since it was outside the diff. Sounds like a good approach I might actually want to incorporate into PHP then :)
Related to CDRIVER-4519, followup to #1155.
Replaces manual Python lookup and package upgrade routines with a sourcing of the
activate-ocspvenv.shscript and removes the unusedSKIP_PIP_INSTALLvariable.Verified by this patch.