Skip to content

Add the Arcade-powered source-build wrapper.#4105

Merged
zivkan merged 19 commits intoNuGet:devfrom
crummel:arPowWrapper
Jun 23, 2021
Merged

Add the Arcade-powered source-build wrapper.#4105
zivkan merged 19 commits intoNuGet:devfrom
crummel:arPowWrapper

Conversation

@crummel
Copy link
Contributor

@crummel crummel commented Jun 10, 2021 •
edited
Loading

This adds a source-build project in eng/source-build. This project pulls in a recent version of Arcade that supports the ArPow targets, restores arcade tooling (so we make source-build tasks available), builds NuGet in the source-build fashion (Restore/Build/Pack XPlat), and then collects the built NuGet packages into source-build intermediate packages.
Remaining work:

  • publishing the intermediate packages.
  • restoring dependencies from source-build intermediate packages (this requires updates to eng/Version.Details.xml as well to make sure source-build knows what the real dependencies are).
    I wanted to get this in as an intermediate step because the dev workflow for source-build does work now.

There are a couple NuGet source changes I made:

  • Added a global.json in the root and renamed NuGet.Config to NuGet.config (Arcade looks for these in the root and the paths can't be easily changed. I can try to find a workaround if this is an issue).
  • Excluded trying to import some VS targets in source-build.

Bug

Fixes: dotnet/source-build#2069.

Regression? No. Last working version: N/A

Description

PR Checklist

  • PR has a meaningful title

  • PR has a linked issue.

  • Described changes

  • Tests

    • Automated tests added
    • OR
    • Test exception
    • OR
    • N/A
      This adds a source-build CI leg which is effectively the test of the source-build changes.

  • Documentation

    • Documentation PR or issue filled
    • OR
    • N/A

@crummel
Copy link
Contributor Author

crummel commented Jun 10, 2021

@dominoFire Could I get the CI label on this please?

@crummel
Copy link
Contributor Author

crummel commented Jun 11, 2021

ping @nkolev92 @dominoFire for the CI label please.

MichaelSimons
MichaelSimons reviewed Jun 14, 2021
View reviewed changes
@MichaelSimons MichaelSimons mentioned this pull request Jun 14, 2021
Closed
8 tasks
@crummel crummel mentioned this pull request Jun 15, 2021
Closed
5 tasks
@crummel crummel force-pushed the arPowWrapper branch 5 times, most recently from fc7d2f5 to 00a41fe Compare June 15, 2021 16:48
@crummel crummel force-pushed the arPowWrapper branch 2 times, most recently from fc5e970 to 64135a2 Compare June 15, 2021 18:16
@crummel crummel marked this pull request as ready for review June 17, 2021 20:00
@crummel crummel requested a review from a team as a code owner June 17, 2021 20:00
@crummel
Copy link
Contributor Author

crummel commented Jun 17, 2021

@dominoFire I merged #3945 into this one so I closed that one. cc @dseefeld and @MichaelSimons if you'd like to review.

@crummel crummel changed the title Ar pow wrapper Add the Arcade-powered source-build wrapper. Jun 17, 2021
nkolev92
nkolev92 reviewed Jun 17, 2021
View reviewed changes
MichaelSimons
MichaelSimons reviewed Jun 17, 2021
View reviewed changes
@crummel
Copy link
Contributor Author

crummel commented Jun 21, 2021

@dominoFire @nkolev92 @zivkan Is there any other changes that need to be made before you can merge this?

zivkan
zivkan reviewed Jun 21, 2021
View reviewed changes
@zivkan
Copy link
Member

zivkan commented Jun 21, 2021

@crummel Out of curiosity, why is NuGet.Config being renamed to NuGet.config? nuget.config is actually the preferred style on Linux, but all 3 are read.

Furthermore, this repo is mirrored to Azure DevOps' DevDiv instance, which has "Case enforcement" enabled, and therefore this change might break mirroring.

@crummel
Copy link
Contributor Author

crummel commented Jun 21, 2021

This is due to an Arcade issue. I've reverted this for now and put in a source-build patch that does it instead - I can remove this patch when we bring in intermediate package publishing to Nuget.

@crummel crummel requested review from nkolev92 and zivkan June 22, 2021 20:03
zivkan
zivkan approved these changes Jun 22, 2021
View reviewed changes
Copy link
Member

@zivkan zivkan left a comment

Choose a reason for hiding this comment

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

Let's hope we can get a non-flaky CI build soon.

@zivkan zivkan added the Community PRs created by someone not in the NuGet team label Jun 22, 2021
@zivkan zivkan self-assigned this Jun 22, 2021
@zivkan zivkan merged commit 55e6392 into NuGet:dev Jun 23, 2021
@crummel
Copy link
Contributor Author

crummel commented Jun 23, 2021

Thanks for all your help getting this in!

This was referenced Jul 21, 2021
Open
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nkolev92 nkolev92 nkolev92 approved these changes

@zivkan zivkan zivkan approved these changes

+1 more reviewer

@MichaelSimons MichaelSimons MichaelSimons approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@zivkan zivkan

Labels

Community PRs created by someone not in the NuGet team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[ArPow] Onboard NuGet.client to arcade-powered source-build

4 participants

@crummel @zivkan @nkolev92 @MichaelSimons