Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

omajid
Copy link
Member

@omajid omajid commented Aug 9, 2018

build.sh and build.cmd contain logic to identify a working version of python to use. src/build.proj ignores that and directly invokes 'python', which may not work, or even execute a different program.

See #19043 and #19356

cc @BruceForstall

build.sh and build.cmd contain logic to identify a working version of
python to use. src/build.proj ignores that and directly invokes
'python', which may not work, or even execute a different program.
@BruceForstall BruceForstall requested a review from brianrob August 9, 2018 20:45
@brianrob
Copy link
Member

brianrob commented Aug 9, 2018

LGTM. Can you validate that this works offline by building release with /p:__EnforcePgo=1? There aren't any CI legs that validate that this works - the official builds are the only builds that exercise this path.

@omajid
Copy link
Member Author

omajid commented Aug 9, 2018

Can you validate that this works offline by building release with /p:__EnforcePgo=1? There aren't any CI legs that validate that this works - the official builds are the only builds that exercise this path.

I tried it on Linux where it calls the python script which then fails because the script can not find clrjit.dll.

Checking if the following DLLs are properly compiled with PGO
EXEC : error : Could not find file(s) /home/omajid/devel/dotnet/coreclr/bin/Product/Linux.x64.Debug/coreclr.dll [/home/omajid/devel/dotnet/coreclr/src/build.proj]
/tmp/tmpb59d781a3a42451cb5e04155df797bd8.exec.cmd: line 2: /home/omajid/devel/dotnet/coreclr/bin/Product/Linux.x64.Debug/clrjit.dll: No such file or directory

@brianrob
Copy link
Member

brianrob commented Aug 9, 2018

Thanks @omajid. Can you check this on Windows as well please?

@omajid
Copy link
Member Author

omajid commented Aug 9, 2018

Sorry, I don't have access to a Windows machine :(

@omajid
Copy link
Member Author

omajid commented Aug 9, 2018

FWIW:

$ pylint --py3k  src/scripts/pgocheck.py 
No config file found, using default configuration

------------------------------------
Your code has been rated at 10.00/10

@brianrob
Copy link
Member

brianrob commented Aug 9, 2018

@omajid, I grabbed your PR and tested it on my machine. It looks like at least on my machine, the version of python that gets picked up by build.cmd is different, and so I get this failure:

EnforcePGO:
  Checking if the following DLLs are properly compiled with PGO
  "C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64\python.exe" c:\src\coreclr\src\scripts\pgocheck.py
   c:\src\coreclr\bin\Product\Windows_NT.x64.Release\coreclr.dll;c:\src\coreclr\bin\Product\Windows_NT.x64.Release\clrj
  it.dll
  Traceback (most recent call last):
    File "c:\src\coreclr\src\scripts\pgocheck.py", line 64, in <module>
      result, tech = was_compiled_with_pgo(filename)
    File "c:\src\coreclr\src\scripts\pgocheck.py", line 31, in was_compiled_with_pgo
      match = pgo_pattern.search(headers)
  TypeError: cannot use a string pattern on a bytes-like object

While I think that this change makes sense, it will need to be validated on an official build machine config to validate that it won't break (or figure out what the incompatibility is). I suspect this is a python2 vs. python3 thing, but that is a guess.

@omajid
Copy link
Member Author

omajid commented Aug 10, 2018

Thanks for testing this. This definitely looks like a python 2 vs python 3 issue. I am going to close this PR since it's not very useful to break the official build configuration, and I can't exercise/verify the bits that break myself. Thanks again for your time.

@omajid omajid closed this Aug 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants