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

Enable arcade support #113

Merged
merged 13 commits into from
May 24, 2019
Merged

Enable arcade support #113

merged 13 commits into from
May 24, 2019

Conversation

eerhardt
Copy link
Member

@eerhardt eerhardt commented May 20, 2019

This is the first round of using dotnet/arcade as the build infrastructure.

It doesn't make use of everything in arcade, but it is the first step to using arcade, and we can incrementally make changes as we go to make use of more and more arcade functionality (for example the signtool which will sign nugets and their contained assemblies in a single step).

It is easiest to review by ignoring the first commit "Copy eng\common from Aracde repo". It is a strict copy of the eng\common folder that is defined by the arcade repo. It will get sync'd automatically when we start taking new versions of arcade using the dependency flow infrastructure.

This should solve #40.

@imback82 imback82 requested review from imback82, suhsteve and rapoth May 20, 2019 23:11
@eerhardt eerhardt marked this pull request as ready for review May 21, 2019 22:03
@eerhardt eerhardt requested a review from safern May 21, 2019 22:03
@eerhardt
Copy link
Member Author

OK, this is now ready for review. Please take a look and let me know what you think.

@eerhardt eerhardt force-pushed the EnableArcade branch 7 times, most recently from 6611ce0 to c4e4116 Compare May 22, 2019 21:06
- Set official build id parameter
- Build nuget package on Build leg
- Publish the worker using MSBuild instead of cmd
azure-pipelines.yml Outdated Show resolved Hide resolved
@eerhardt
Copy link
Member Author

- task: DotNetCoreInstaller@0

I think this can now be removed on this leg. We no longer invoke DotNetCoreCLI tasks on this leg.


Refers to: azure-pipelines.yml:155 in 31c6cae. [](commit_id = 31c6cae, deletion_comment = False)

eng/Versions.props Outdated Show resolved Hide resolved
eerhardt added 2 commits May 22, 2019 17:28
…ed directly in teh Packages folder.

Clean up official build definition.
safern
safern previously approved these changes May 23, 2019
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

Left some comments but looks great. Thanks @eerhardt great cleanup as well.

@imback82
Copy link
Contributor

imback82 commented May 23, 2019

It is easiest to review by ignoring the first commit "Copy eng\common from Aracde repo". It is a strict copy of the eng\common folder that is defined by the arcade repo. It will get sync'd automatically when we start taking new versions of arcade using the dependency flow infrastructure.

So do we plan to remove files under eng\common? Also, do you have an internal build to share so I can double check the nuget and worker binaries?

@imback82
Copy link
Contributor

I see this error message in https://dev.azure.com/dnceng/public/_build/results?buildId=198008, but the build task is marked as success. Is this expected?

D:\a\1\s\.packages\microsoft.dotnet.arcade.sdk\1.0.0-beta.19257.7\tools\SymStore.targets(61,5): error MSB3073: The command ""D:\a\1\s\.packages\microsoft.diasymreader.pdb2pdb\1.1.0-beta1-62506-02\tools\Pdb2Pdb.exe" "D:\a\1\s\artifacts\bin\Microsoft.Spark.FSharp.Examples\Release\net461\Microsoft.Spark.FSharp.Examples.exe" /out "D:\a\1\s\artifacts\SymStore\Release\Microsoft.Spark.FSharp.Examples\net461\Microsoft.Spark.FSharp.Examples.pdb" /verbose /srcsvrvar SRC_INDEX=public" exited with code 2. [D:\a\1\s\examples\Microsoft.Spark.FSharp.Examples\Microsoft.Spark.FSharp.Examples.fsproj]

@eerhardt
Copy link
Member Author

So do we plan to remove files under eng\common?

No, these stay in the repo. But you don't modify them here- the "master" version is located in https://github.com/dotnet/arcade/tree/master/eng/common. When we take new versions of arcade, these files are updated automatically.

Also, do you have an internal build to share so I can double check the nuget and worker binaries?

https://dev.azure.com/dnceng/internal/_build/results?buildId=198161

I see this error message in https://dev.azure.com/dnceng/public/_build/results?buildId=198008, but the build task is marked as success. Is this expected?

No, looking into it now.

@eerhardt
Copy link
Member Author

OK, all feedback is now addressed, and I've updated the branch to the latest code. This should be ready for a final review.

I see this error message in https://dev.azure.com/dnceng/public/_build/results?buildId=198008, but the build task is marked as success. Is this expected?

The issue here is caused by an F# bug: dotnet/fsharp#6832. But also, our Examples projects were defaulting to IsShipping=true in the arcade SDK, which was causing some extra processing during the build. Setting IsShipping=false on the projects that aren't shipping opts out of the processing that was causing the issue. And since we only have F# examples, and not shipping binaries, we are able to work around the issue.

@imback82
Copy link
Contributor

imback82 commented May 23, 2019

Also, do you have an internal build to share so I can double check the nuget and worker binaries?

https://dev.azure.com/dnceng/internal/_build/results?buildId=198161

I checked the nupkg and worker binaries and the names are appended with "-prerelease.19272.6". What's the recommended way of releasing from now on? We were planning to release without the prerelease tag.

targetFolder: $(Build.ArtifactStagingDirectory)/BuildArtifacts/src/csharp/Microsoft.Spark/bin/$(buildConfiguration)
**/*.nupkg
**/*.snupkg
targetFolder: $(Build.ArtifactStagingDirectory)/BuildArtifacts/artifacts/packages/$(buildConfiguration)/Shipping
Copy link
Member

Choose a reason for hiding this comment

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

If the target folder is $(Build.ArtifactStagingDirectory)/BuildArtifacts and we make the contents be: **/Shipping/**/*.nupkg and **/Shipping/**/*.snupkg and then the sourceFolder being (Build.SourcesDirectory) it should preserve the folder structure, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I think so. I can try changing it that way tomorrow morning.

Copy link
Member

Choose a reason for hiding this comment

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

Not required to merge but it’s a nice cleanup.

@eerhardt
Copy link
Member Author

What's the recommended way of releasing from now on? We were planning to release without the prerelease tag.

Daily builds get a build number appended to the end. That’s how you know it isn’t an officially released NuGet package - it has a daily build number. We can push those daily builds to feeds that aren’t NuGet.org. When you are ready to ship, you queue a build with a variable set to build without the build number at the end. And then you release that package.

Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the great work @eerhardt!

@eerhardt eerhardt merged commit 0a58c1e into dotnet:master May 24, 2019
@eerhardt eerhardt deleted the EnableArcade branch May 24, 2019 13:59
@imback82 imback82 mentioned this pull request May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants