Update install script to refresh Path #3462
Conversation
mikeagun
requested review from
dthaler,
poornagmsft,
Alan-Jowett,
saxena-anurag,
shankarseal,
dv-msft,
gtrevi,
shpalani,
matthewige,
mtfriesen and
rectified95
as code owners
| Write-Log("Copying ebpfapi.dll from '$ebpfApiDllPath' to '$pwd'...") | ||
| Copy-Item -Path $ebpfApiDllPath -Destination $pwd -Force | ||
| # Refresh Path so EbpfApi.dll can be found. | ||
| $machinepath = [system.environment]::getenvironmentvariable("path", [system.environmentvariabletarget]::machine) |
There was a problem hiding this comment.
Is ebpfapi.dll different for debug and release?
There was a problem hiding this comment.
Yes, the binaries are different for debug and release.
I don't think that affects this PR though -- you'd be getting the same dll after this change vs. before. Previously we copied the dll from Program Files to the current working directory; with this change we refresh the path so that the dll in Program Files is seen in the current shell without copying it.
| Copy-Item -Path $ebpfApiDllPath -Destination $pwd -Force | ||
| # Refresh Path so EbpfApi.dll can be found. | ||
| $machinepath = [system.environment]::getenvironmentvariable("path", [system.environmentvariabletarget]::machine) | ||
| $userpath = [system.environment]::getenvironmentvariable("path", [system.environmentvariabletarget]::user) |
There was a problem hiding this comment.
Can you check non-null constraints for those variables ($userpath, $machinepath)
There was a problem hiding this comment.
I'm not sure I understand. If the environment variables aren't set, then this code would behave the same as if they are set to an empty string, so it shouldn't cause any problems.
There was a problem hiding this comment.
Previously, it was copied from Program Files., which ensured that ebpfapi.dll was present. With the new changes, how do you ensure the DLL is present and mandatory, if the userpath is not set?
Also, there are 2 test cases failing in pipeline. Not sure whether it is due to these envn.vars set....
There was a problem hiding this comment.
On the 2 failing test cases: regression_driver_ws2022 also fails in main, so I don't think that is related.
If the installer succeeded, then the DLL should be present and ebpf-for-windows should have been added to the system path, and if it failed then we will have errored and exited before this line.
If a future bug dropped ebpfapi.dll from the installer testing would still fail at the first test which needs the DLL.
Do we need an explicit check for the DLL, or is the implicit testing of trying to use it enough?
Description
Updates Path after install so EbpfApi.dll can be found (instead of copying the dll to current directory). Fixes #3425
Testing
Covered by existing tests.
Documentation
N/A
Installation
N/A