Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix PYTHONPATH on Linux #57701

Merged
merged 7 commits into from
Aug 20, 2021
Merged

Conversation

Lxiamail
Copy link
Member

@ghost
Copy link

ghost commented Aug 18, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

for issue dotnet/performance#1928

Author: Lxiamail
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@@ -7,7 +7,7 @@
<CoreRun>%HELIX_CORRELATION_PAYLOAD%\Core_Root\CoreRun.exe</CoreRun>
<BaselineCoreRun>%HELIX_CORRELATION_PAYLOAD%\Baseline_Core_Root\CoreRun.exe</BaselineCoreRun>

<HelixPreCommands>$(HelixPreCommands);call %HELIX_CORRELATION_PAYLOAD%\performance\tools\machine-setup.cmd;set PYTHONPATH=%HELIX_WORKITEM_PAYLOAD%\scripts%3B%HELIX_WORKITEM_PAYLOAD%</HelixPreCommands>
<HelixPreCommands>$(HelixPreCommands);call %HELIX_CORRELATION_PAYLOAD%\performance\tools\machine-setup.cmd;export PYTHONPATH=$HELIX_WORKITEM_PAYLOAD/scripts:$HELIX_WORKITEM_PAYLOAD</HelixPreCommands>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are flipped. This block is the Windows path, so should be set, and the other should be export.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

@billwert
Copy link
Member

Looked into this a little further. I think this is not the fix that will fix the helix issue.

Earlier in our automation we enable a venv:

+ python3 -m venv /home/helixbot/work/A3AF0971/w/A711092F/u/.venv
+ source /home/helixbot/work/A3AF0971/w/A711092F/u/.venv/Scripts/activate

The line you're changing is built including this variable HelixPreCommandStemLinux, so it should be there:

- HelixPreCommandStemLinux: 'python3 -m pip install -U pip;sudo apt-get -y install python3-venv;python3 -m venv $HELIX_WORKITEM_PAYLOAD/.venv;source $HELIX_WORKITEM_PAYLOAD/.venv/Scripts/activate;export PYTHONPATH=;python3 -m pip install -U pip;pip3 install azure.storage.blob==12.0.0;pip3 install azure.storage.queue==12.0.0;sudo apt-get update;sudo apt -y install curl dirmngr apt-transport-https lsb-release ca-certificates;curl -sL https://deb.nodesource.com/setup_12.x | sudo -E bash -;sudo apt-get -y install nodejs;sudo apt-get -y install npm;npm install --prefix $HELIX_WORKITEM_PAYLOAD jsvu -g;$HELIX_WORKITEM_PAYLOAD/bin/jsvu --os=linux64 --engines=v8,javascriptcore;export PERFLAB_UPLOAD_TOKEN="$(PerfCommandUploadTokenLinux)"'

By the time the failure noted in dotnet/performance#1928 happens we have exited the venv. This actually seems to me to be an issue with helix or something else having gone wrong on our machines.

(The places added to PYTHONPATH here are solely to support our scenario runs, which is why the fact that it was out of sync here never mattered.)

We need to figure out what is wrong with the machine environment. First Responders may be the right first step.

@Lxiamail
Copy link
Member Author

@billwert based on the test run at https://dev.azure.com/dnceng/internal/_build/results?buildId=1304705&view=results, looks like the fix works. Even though still have some failures on linux, those needs additional investigation and fixes.

@Lxiamail
Copy link
Member Author

Added the same fix for crossgen scenario

@@ -81,6 +80,7 @@

<PropertyGroup>
<PartitionCount>30</PartitionCount>
<EnableAzurePipelinesReporter>false</EnableAzurePipelinesReporter>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently due to some helix recent change, helix reporter crashes due to perf test alters PYTHONPATH. Set EnableAzurePipelinesReporter to false since perf test doesn't use helix reporter to upload test results, instead using perf specific scripts.

@billwert billwert merged commit ec25238 into dotnet:main Aug 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants