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.

Sign up for GitHub

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

Jump to bottom

Build scripts while copying native binaries #36482

Merged
merged 4 commits into from
May 16, 2020
Merged

Build scripts while copying native binaries #36482

merged 4 commits into from
May 16, 2020

Conversation

sdmaclea
Copy link
Contributor

Stop building test scripts during the managed build phase
Build after copying the native targets
Only build scripts for the current platform.

Remove issues relative to building scripts on OSX

@sdmaclea sdmaclea added this to the 5.0 milestone May 15, 2020
@sdmaclea sdmaclea self-assigned this May 15, 2020
trylek
trylek approved these changes May 15, 2020
View reviewed changes
Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

LGTM

@sdmaclea sdmaclea force-pushed the infra/buildSciptWithNative branch from 19e30d2 to 70256c9 Compare May 15, 2020 14:35
@sdmaclea
Build scripts while copying native binaries
57bfd7f 
Stop building test scripts during the managed build phase
Build after copying the native targets

Remove issues relative to building scripts on OSX
@sdmaclea sdmaclea force-pushed the infra/buildSciptWithNative branch from 70256c9 to 57bfd7f Compare May 15, 2020 15:19
jashook
jashook approved these changes May 15, 2020
View reviewed changes
Copy link
Contributor

@jashook jashook left a comment

Choose a reason for hiding this comment

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

Lgtm thank you for the change

@jashook
Copy link
Contributor

jashook commented May 15, 2020

Looks like some tests are missing their cmd/sh files

@jashook
Copy link
Contributor

jashook commented May 15, 2020

Ah I think I know the problem, superpmi I think copied these files and expects them already to be on disk.

@jashook
Copy link
Contributor

jashook commented May 15, 2020 

  <ItemGroup>
    <_SpmiTestProjects Include="..\Performance\CodeQuality\Bytemark\Bytemark.csproj" />
    <_SpmiTestProjects Include="..\Methodical\fp\exgen\10w5d_cs_do.csproj" />
    <_SpmiTestProjects Update="@(_SpmiTestProjects)">
      <Properties>OutputPath=$(OutputPath)\%(FileName)</Properties>
    </_SpmiTestProjects>
  </ItemGroup>

@jashook
Copy link
Contributor

jashook commented May 15, 2020

@sdmaclea this test I believe can just be marked as platform specific to avoid this problem

@jashook
Copy link
Contributor

jashook commented May 15, 2020

/cc @dotnet/jit-contrib

@sdmaclea
Copy link
Contributor Author

I just pushed the fix for the superpmi build scripts. I spent the better part of a day trying to figure out how to add a project target that depended on our other targets. I think this solution is clean.

@BruceForstall
Copy link
Member

Is there a dev innerloop impact with this change? E.g., when are the wrapper scripts generated? During test build time? Some kind of on-platform pre-test-run time?

@jaredpar jaredpar mentioned this pull request May 15, 2020
Closed
@sdmaclea
Copy link
Contributor Author

@BruceForstall There is not intended to be a significant impact to normal dev experience.

The intention is for the scripts to be generated at the same time as the native binaries are built, because both are target specific.

  • So the intended change is for the scripts to build unless skipnative is selected.
  • Previously they would build unless skipmanaged was selected.

@sdmaclea sdmaclea mentioned this pull request May 16, 2020
Closed
@sdmaclea sdmaclea merged commit 1d9846a into dotnet:dev/infrastructure May 16, 2020
@sdmaclea sdmaclea mentioned this pull request May 16, 2020
Closed
jashook pushed a commit to jashook/runtime that referenced this pull request May 26, 2020
@sdmaclea
Build scripts while copying native binaries (dotnet#36482)
df0c682 
* Build scripts while copying native binaries

Stop building test scripts during the managed build phase
Build after copying the native targets

Fixes issues relative to building scripts on OSX

* Fix superpmi script generation

* Cleanup issues.targets

* Keep 6 failing IJW tests
jashook pushed a commit that referenced this pull request Jun 4, 2020
@sdmaclea @trylek
@safern @akoeplinger @dotnet-maestro @AndyAyersMS @am11 @Gnbrkm41 @monojenkins @thaystg @ViktorHofer @AaronRobinsonMSFT @vcsjones @bartonjs @EgorBo @nattress @lambdageek @krwq @kunalspathak @CarolEidt @weitzhandler @stephentoub @layomia @danmoseley @eiriktsarpalis @jkoritzinsky @joperezr @flamencist @MihaZupan @NikolaMilosavljevic @steveisok @davidwrighton @buyaa-n @VSadov @lpereira @swaroop-sridhar @ManickaP @MichalStrehovsky @mangod9 @BruceForstall @alexischr @BrzVlad @alfredmyers @NickCraver @YohDeadfall @eerhardt @directhex @janvorli @Anipik @Marusyk @vargaz @Youssef1313 @benaadams @marek-safar @AntonLapounov @AraHaan @tmds @wfurt @jozkee @sywhang @GrabYourPitchforks @davmason @UnityAlex @fanyang-mono @tannergooding @echesakov @radical @ivdiazsa @gbalykov @shadow-cs @ericstj @skill-bit @CoffeeFlux @sakno @saucecontrol @ww898 @jkotas @w-flo @PeterSolMS @grendello @lateralusX @eanova @mmitche @JoshSchreuder @cshung @erozenfeld @kjpou1 @0xfk0
Dev infra master merge1 (#37274)
9e09fe3 
* Build all managed coreclr tests on OSX in CI (#36253)

Remove concept of targetGeneric and targetSpecific

Add OSX corelcr managed tests builds to all pipes
Remove all other platform coreclr managed test build

Remove property `TestUnsupportedOutsideWindows`
Rename conditional `DisableProjectBuild` to `CLRTestTargetUnsupported`

Refactor tests to allow all managed tests to be built on OSX.

Split managed tests which based on target properties conditionally:
+ `<Compile Include/>`
+ `<DefineConstant/>`

This creates a separate test for each target configuration permutation.

Add `<CLRTestTargetUnsupported/>` conditions to select these at build and/or
run time

Append split tests with `_Target*` to identify intended test target

For tests which depend on target specific internal details od System.Private.Corlib,
expose a dummy implementation for unsupported targets.

Clean up

Remove <TraitTags/>
Remove <CLRTestNeedTarget/>
Remove managedOSXBuild
Remove managedTestBuildOsGroup
Remove managedTestBuildOsSubGroup

Use issues.target to disable expected failures

Some tests are configured to run on a targets with
specific charecteristics.

Use issues.targets to conditionally disable tests
not intended to run on all targets.

* Setup pr and batch runs for dev/infrsatructure (#36218) (#36299)

Co-authored-by: Jarret Shook <[email protected]>

* Add alias CoreClrTestBuildHost (#36256)

* Add alias CoreClrTestBuildHost

Intended to allow easy switching of test build host between OSX_x64 & Linux_x64
depending on availablility and reliability.

* Add missing issues.targets from #36253 (#36351)

* Add missing issues.targets from #36253

PR #36253 enabled building all tests on OSX. It was intended to include this list of
tests to disable on unsupported platforms. This was dropped as part of rebasing for
dev/infra branch.

Lack of CI on dev/infra allowed this to be missed.

* Revise issues.targets

* Add src/coreclr/tests/src/baseservices/typeequivalence/simple

* Remove #if Windows from DllImportPathTest

* Build scripts while copying native binaries (#36482)

* Build scripts while copying native binaries

Stop building test scripts during the managed build phase
Build after copying the native targets

Fixes issues relative to building scripts on OSX

* Fix superpmi script generation

* Cleanup issues.targets

* Keep 6 failing IJW tests

* Fix IJW when managed tests are built on OSX (#36711)

* Fix IJW when managed tests are built on OSX

- Refactor Interop.setting.targets to Directory.Build.{targets,props}
- Remove include Interop.setting.targets from interop projects rely on SDK including
    Directory.Build.* automatically
- Add new CopyInteropNativeRuntimeDependencies target for copying IJW dependencies
  * Add <Copy/> task
- Delete obsolete instruction in Interop/ReadMe.md

* Cleanup Readme.md

* Optional stress dependencies & don't populate CORE_ROOT twice (#36851)

Bruce noticed that we're downloading and installing stress
runtime dependencies several times during build. I have added a new
option to suppress this behavior in build-test and I fixed the
invocations of build-test based on Bruce's analysis. I have also
patched the build scripts to avoid populating CORE_ROOT in
"copynativeonly" mode.

Thanks

Tomas

Fixes: #36797

* Build test scripts whenever CopyNativeProjectBinaries builds (#37028)

* Fix pri0 test build.

* Clean pri1 test build (#37266)

* Clean pri1 test build

* Undo testing comment

* Make sure weakreferencetest does not run on unix

* Fixup WinRT proj

* Fix path

Co-authored-by: Steve MacLean <[email protected]>
Co-authored-by: Tomáš Rylek <[email protected]>
Co-authored-by: Santiago Fernandez Madero <[email protected]>
Co-authored-by: Alexander Köplinger <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Andy Ayers <[email protected]>
Co-authored-by: Adeel Mujahid <[email protected]>
Co-authored-by: Ganbarukamo41 <[email protected]>
Co-authored-by: monojenkins <[email protected]>
Co-authored-by: thaystg <[email protected]>
Co-authored-by: Viktor Hofer <[email protected]>
Co-authored-by: Aaron Robinson <[email protected]>
Co-authored-by: Kevin Jones <[email protected]>
Co-authored-by: Jeremy Barton <[email protected]>
Co-authored-by: Egor Bogatov <[email protected]>
Co-authored-by: Simon Nattress <[email protected]>
Co-authored-by: Aleksey Kliger (λgeek) <[email protected]>
Co-authored-by: Krzysztof Wicher <[email protected]>
Co-authored-by: Kunal Pathak <[email protected]>
Co-authored-by: Carol Eidt <[email protected]>
Co-authored-by: Shimmy <[email protected]>
Co-authored-by: Stephen Toub <[email protected]>
Co-authored-by: Layomi Akinrinade <[email protected]>
Co-authored-by: Dan Moseley <[email protected]>
Co-authored-by: Eirik Tsarpalis <[email protected]>
Co-authored-by: Jeremy Koritzinsky <[email protected]>
Co-authored-by: Jose Perez Rodriguez <[email protected]>
Co-authored-by: Alexander Chermyanin <[email protected]>
Co-authored-by: Miha Zupan <[email protected]>
Co-authored-by: Sergey Andreenko <[email protected]>
Co-authored-by: Nikola Milosavljevic <[email protected]>
Co-authored-by: Steve Pfister <[email protected]>
Co-authored-by: David Wrighton <[email protected]>
Co-authored-by: buyaa-n <[email protected]>
Co-authored-by: Krzysztof Wicher <[email protected]>
Co-authored-by: Vladimir Sadov <[email protected]>
Co-authored-by: Leandro Pereira <[email protected]>
Co-authored-by: Swaroop Sridhar <[email protected]>
Co-authored-by: Marie Píchová <[email protected]>
Co-authored-by: Michal Strehovský <[email protected]>
Co-authored-by: Manish Godse <[email protected]>
Co-authored-by: Bruce Forstall <[email protected]>
Co-authored-by: Alexis Christoforides <[email protected]>
Co-authored-by: BrzVlad <[email protected]>
Co-authored-by: Alfred Myers <[email protected]>
Co-authored-by: Nick Craver <[email protected]>
Co-authored-by: Nathan Ricci <[email protected]>
Co-authored-by: Thays Grazia <[email protected]>
Co-authored-by: Yoh Deadfall <[email protected]>
Co-authored-by: Eric Erhardt <[email protected]>
Co-authored-by: Jo Shields <[email protected]>
Co-authored-by: Jan Vorlicek <[email protected]>
Co-authored-by: Anirudh Agnihotry <[email protected]>
Co-authored-by: Roman Marusyk <[email protected]>
Co-authored-by: Zoltan Varga <[email protected]>
Co-authored-by: Youssef Victor <[email protected]>
Co-authored-by: Ben Adams <[email protected]>
Co-authored-by: Marek Safar <[email protected]>
Co-authored-by: Anton Lapounov <[email protected]>
Co-authored-by: AraHaan <[email protected]>
Co-authored-by: Tom Deseyn <[email protected]>
Co-authored-by: Tomas Weinfurt <[email protected]>
Co-authored-by: Tomas Weinfurt <[email protected]>
Co-authored-by: David Cantu <[email protected]>
Co-authored-by: Sung Yoon Whang <[email protected]>
Co-authored-by: Levi Broderick <[email protected]>
Co-authored-by: vargaz <[email protected]>
Co-authored-by: David Mason <[email protected]>
Co-authored-by: UnityAlex <[email protected]>
Co-authored-by: Fan Yang <[email protected]>
Co-authored-by: Tanner Gooding <[email protected]>
Co-authored-by: Egor Chesakov <[email protected]>
Co-authored-by: radical <[email protected]>
Co-authored-by: Ivan Diaz Sanchez <[email protected]>
Co-authored-by: Gleb Balykov <[email protected]>
Co-authored-by: Honza Rameš <[email protected]>
Co-authored-by: Eric StJohn <[email protected]>
Co-authored-by: Ivan <[email protected]>
Co-authored-by: Ryan Lucia <[email protected]>
Co-authored-by: SRV <[email protected]>
Co-authored-by: Clinton Ingram <[email protected]>
Co-authored-by: Mikhail Pilin <[email protected]>
Co-authored-by: Jan Kotas <[email protected]>
Co-authored-by: w-flo <[email protected]>
Co-authored-by: Peter Sollich <[email protected]>
Co-authored-by: grendello <[email protected]>
Co-authored-by: Johan Lorensson <[email protected]>
Co-authored-by: Erhan Atesoglu <[email protected]>
Co-authored-by: Matt Mitchell <[email protected]>
Co-authored-by: Josh Schreuder <[email protected]>
Co-authored-by: Josh Schreuder <[email protected]>
Co-authored-by: Andrew Au <[email protected]>
Co-authored-by: Eugene Rozenfeld <[email protected]>
Co-authored-by: Vlad Brezae <[email protected]>
Co-authored-by: Kenneth Pouncey <[email protected]>
Co-authored-by: Kirill Frolov <[email protected]>
Co-authored-by: Tomas Weinfurt <[email protected]>
@sdmaclea sdmaclea deleted the infra/buildSciptWithNative branch September 26, 2020 16:53
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jashook jashook jashook approved these changes

@swaroop-sridhar swaroop-sridhar swaroop-sridhar approved these changes

@trylek trylek trylek approved these changes

@jkoritzinsky jkoritzinsky Awaiting requested review from jkoritzinsky

Assignees

@sdmaclea sdmaclea

Labels
area-Infrastructure-coreclr
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@sdmaclea @jashook @BruceForstall @swaroop-sridhar @trylek