Skip to content

Teach coreclr/src/tools about Checked configuration - #43017

Merged
trylek merged 1 commit into
dotnet:masterfrom
MichalStrehovsky:MichalStrehovsky-patch-1
Nov 27, 2020
Merged

Teach coreclr/src/tools about Checked configuration#43017
trylek merged 1 commit into
dotnet:masterfrom
MichalStrehovsky:MichalStrehovsky-patch-1

Conversation

@MichalStrehovsky

Copy link
Copy Markdown
Member

We never test managed tools in Debug configuration because we never build the runtime partition in the Debug configuration - we only build Checked.

But managed tools don't know what Checked means because it's not an MSBuild concept. So we never run the tools with asserts turned on.

We never test managed tools in Debug configuration because we never build the runtime partition in the Debug configuration - we only build Checked.

But managed tools don't know what Checked means because it's not an MSBuild concept. So we never run the tools with asserts turned on.
@ghost

ghost commented Oct 4, 2020

Copy link
Copy Markdown

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

@MichalStrehovsky

Copy link
Copy Markdown
Member Author

Cc @dotnet/crossgen-contrib. This mostly affects crossgen2. Some other managed tools also do Debug.Assert, but crossgen2 is likely going to be most problematic.

/azp run runtime-coreclr crossgen2

@MichalStrehovsky

Copy link
Copy Markdown
Member Author

/azp run runtime-coreclr crossgen2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

@MichalStrehovsky

Copy link
Copy Markdown
Member Author

Azure Pipelines could not run because the pipeline triggers exclude this branch/path.

Thanks for nothing azure-pipelines. I triggered it manually here: https://dev.azure.com/dnceng/public/_build/results?buildId=841304&view=results

@trylek

trylek commented Oct 4, 2020

Copy link
Copy Markdown
Member

For the inability to use /azp run to trigger general pipelines like CG2, I have a PR out to fix this, it's just waiting on code review right now:

#42933

@MichalStrehovsky

Copy link
Copy Markdown
Member Author

@trylek do the crossgen2 failures look about expected? I don't see an obvious assert being hit.

@trylek

trylek commented Oct 5, 2020

Copy link
Copy Markdown
Member

Hmm, the "incomplete instantiation in GetArrayDataReference" is a known issue unrelated to your change. It started reproing around September 25, JanV originally speculated that it might be related to a small semantical change he inadvertently made when switching over framework compilation using crossgen2 to use r2rtest; but he's already fixed that and apparently the bugs are still there. I'm investigating them now as I believe that to be our largest CQ regression in quite a while.

@AaronRobinsonMSFT

Copy link
Copy Markdown
Member

This looks good to me. I don't know if there are other properties we need to update but this seems like a good start.

/cc @jkoritzinsky

@jkotas

jkotas commented Nov 26, 2020

Copy link
Copy Markdown
Member

@trylek Are the crossgen2 failures known issues? Can this be merged?

@trylek

trylek commented Nov 26, 2020

Copy link
Copy Markdown
Member

@jkotas - Hmm, I cannot access the AzDO page for the CG2 run, it claims that "Page not found". I'll retry in a bit to see whether it's just temporary hiccup, otherwise I see no other way of double-checking CG2 results than rerunning the CG2 job.

@trylek

trylek commented Nov 27, 2020

Copy link
Copy Markdown
Member

OK, let me put it like this: the CG2 failures don't seem related to this change so I believe it's safe to merge.

@trylek
trylek merged commit 3bd3a07 into dotnet:master Nov 27, 2020
@MichalStrehovsky
MichalStrehovsky deleted the MichalStrehovsky-patch-1 branch November 27, 2020 08:44
@ghost ghost locked as resolved and limited conversation to collaborators Dec 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Infrastructure-coreclr Only use for closed issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants