Script changes enabling composite R2R build with shared framework #34661
Conversation
| if !__FailedToPrecompile! NEQ 0 ( | ||
| echo Failed assemblies: | ||
| @echo Failed assemblies: |
There was a problem hiding this comment.
Curious; why did you add the @ here?
There was a problem hiding this comment.
I think this was needed to make the text display with echo turned off but I must admit I'm not 100% sure.
| set __Command=!__Command! "!CORE_ROOT!\CoreRun.exe" | ||
| set __Command=!__Command! "!CORE_ROOT!\crossgen2\crossgen2.dll" | ||
| set __Command=!__Command! @"!__ResponseFile!" | ||
| set __Command=!__Command! !ExtraCrossGen2Args! |
There was a problem hiding this comment.
Should we echo this in to the response file - it might make reproducing issues easier if it's all in a single file we can paste into VS debug pane.
There was a problem hiding this comment.
Well, sadly this is tricky for several reasons:
-
CoreRun.exe naturally cannot be part of the response file, that's the app to run.
-
I need to double-check whether the app to run (crossgen2.dll) can be specified in a response file, probably yes.
-
I'm not sure about CoreRun parser but for instance the managed command-line parser we're using in Crossgen2, R2RDump and SuperIlc has a different syntax for arguments on the command line vs. in the response file - e.g., file arguments mustn't be quoted in response files, there's just one argument per line etc. - so that emitting !ExtraCrossGen2Args! into the response file is potentially risky.
There was a problem hiding this comment.
Sorry, I meant just the ExtraCrossgen2Args, so when we run crossgen2 in VS we have all the args in one place and don't have to glue extras on potentially.
There was a problem hiding this comment.
Well, yes, I need to double-check whether I can just output the extra args to the response file - in case of the managed parser, you would need to replace spaces (outside of quoted sections) with line breaks in order to make the parser work properly.
There was a problem hiding this comment.
Hmm, so it seems to me that corerun doesn't suppor response files at all and the managed parser in Crossgen2 has exactly the problem I described i.o.w. the extra command-line args have to be constructed based on knowledge whether they are going to be put directly into the command line or into a response file. I think it will be useful to switch even Crossgen1 to use response files and unify the construction of extra args everywhere but I'd prefer to do that as a separate follow-up change if you don't mind.
| Exit /b 1 | ||
| call :TakeLock | ||
| set CrossGen2Status=0 | ||
| if not exist "IL" ( |
There was a problem hiding this comment.
Thanks for the quality of life improvement here for re-running crossgen each time 👍
trylek
force-pushed
the
CompositeFrameworkScriptChanges
branch
from
153b84d
to
adadbaf
Compare
trylek
force-pushed
the
CompositeFrameworkScriptChanges
branch
from
aa5df1a
to
04cdc19
Compare
ghost
locked as resolved and limited conversation to collaborators
This change basically does three things:
Adds code to build-test.cmd/sh to Crossgen2 framework in composite mode;
Adds code to CLRTest.Crossgen.targets to run Crossgen2 in Helix in composite mode when told to do so via the CompositeBuildMode environment variable;
Implements YML fixes for composite pipeline support (sadly the jobs don't pass yet, apparently due to a codegen bug in CoreLib I'm currently investigating).
Thanks
Tomas