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

Revert "Revert "Reduce CoreCLR PAL"" #76972

Merged
merged 2 commits into from
Oct 13, 2022

Conversation

janvorli
Copy link
Member

Reverts #76860

@janvorli janvorli added NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it labels Oct 12, 2022
@janvorli janvorli self-assigned this Oct 12, 2022
@janvorli
Copy link
Member Author

@am11 this would not work. The PAL maintains its own copy of the environment, so the CreateProcessW would not see the changes added by the OS putenv.

@janvorli janvorli removed NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) NO-REVIEW Experimental/testing PR, do NOT review it labels Oct 13, 2022
@janvorli janvorli added this to the 8.0.0 milestone Oct 13, 2022
@janvorli janvorli requested a review from jkotas October 13, 2022 00:23
@janvorli
Copy link
Member Author

@jkotas the _putenv turned out to be the only issue, so I've put it back. Can you please take a look at this revert of revert?
Please note I've left a dummy change in one of the files in the eng so that this PR verifies the mono build. I'll remove it before merging the change.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

LGTM!

@am11
Copy link
Member

am11 commented Oct 13, 2022

@janvorli, Mono's llvmaot leg is failing with:

  [ 88%] Building CXX object dlls/mscordbi/utilcode/CMakeFiles/utilcode_obj.dir/configuration.cpp.o
  /__w/1/s/src/mono/dlls/mscordbi/cordb-value.cpp:351:44: error: allocating an object of abstract class type 'CordbArrayValue'
          CordbArrayValue* objectValue = new CordbArrayValue(conn, m_pCordbType, m_debuggerId, m_pClass);
                                             ^
  /__w/1/s/src/coreclr/pal/prebuilt/inc/cordebug.h:15647:43: note: unimplemented pure virtual method 'HasBaseIndicies' in 'CordbArrayValue'
          virtual HRESULT STDMETHODCALLTYPE HasBaseIndicies(
                                            ^
  /__w/1/s/src/coreclr/pal/prebuilt/inc/cordebug.h:15650:43: note: unimplemented pure virtual method 'GetBaseIndicies' in 'CordbArrayValue'
          virtual HRESULT STDMETHODCALLTYPE GetBaseIndicies(
                                            ^
  1 error generated.

https://dev.azure.com/dnceng-public/public/_build/results?buildId=51175&view=logs&jobId=cc858e6b-b6f5-5c08-4b62-8cd383bd096a&j=cc858e6b-b6f5-5c08-4b62-8cd383bd096a&t=74861b75-1f67-5a94-490e-d5eb4723f5a5

Looks like dummy change was needed in src/tests as well.

@janvorli
Copy link
Member Author

@am11 this doesn't seem to be related to my changes. I have not removed anything that could result in this error. And yesterday I've built using the same command line that the failing build is using locally and it passed.

@janvorli
Copy link
Member Author

@jkotas seems like it was caused by your #76966.

@am11
Copy link
Member

am11 commented Oct 13, 2022

You are right, it is caused by #76966.

@jkotas
Copy link
Member

jkotas commented Oct 13, 2022

Ugh. #77002

@ghost ghost locked as resolved and limited conversation to collaborators Nov 17, 2022
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.

3 participants