-
Notifications
You must be signed in to change notification settings - Fork 312
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
Add signing to worker, spark binaries and nuget packages #6
Conversation
ad1df33
to
4d9bb50
Compare
- Build | ||
displayName: Build packages | ||
pool: | ||
name: NetCoreInternal-Int-Pool |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just run the whole build on NetCoreInternal-Int-Pool
and collapse this into a single job? That way you don't have to publish the artifacts and then pull them down. It could be simpler to do it all in 1 job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was the initial approach but the machines in NetCoreInternal-Int-Pool
don't have Maven in it and we need Maven for the scala sources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an FYI - that might be a good comment to have in the .yml file.
Also - can we fix NetCoreInternal-Int-Pool
to have it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@safern the general guidance is to bootstrap anything in builds which can be done in a non-stateful-to-machine way; I'll do some reading on Maven and if it makes sense we can add it to these machines (though it will take a bit to roll out)
- task: NuGetCommand@2 | ||
inputs: | ||
command: pack | ||
packagesToPack: '$(Build.SourcesDirectory)\src\csharp\Microsoft.Spark.nuspec' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't we using dotnet pack
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotnet pack
should be also fine. What will be the benefit over nuget
in this case? Just curious.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is cross plat, you don't need to bootstrap nuget.exe when you already have dotnet, you can package a csproj, so no need to create nuspec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a example build:
https://dnceng.visualstudio.com/internal/_build/results?buildId=167505
For easier reviewing disable spaces in the diff setting
cc: @danmosemsft @eerhardt @rapoth @imback82