-
Notifications
You must be signed in to change notification settings - Fork 136
Add more build artifacts and automatically publish to BAR. #1395
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
Conversation
cp ./bin/obj/x64/Release/*.props /drop/artifacts/source-build/ | ||
cp -r ./bin/git-info /drop/artifacts/source-build/ | ||
cp -r ./bin/obj/*/*/build-info /drop/artifacts/source-build/" | ||
du -h $(rootDirectory) | sort -h | tail -n 50 |
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.
I wonder if it would be better to have in the build itself... this seems like a useful diagnosis artifact someone can give us when source-build doesn't work for them.
The paths also might also be clearer in that context--using the project props rather than writing out full names in multiple places.
displayName: Download CentOS artifacts | ||
inputs: | ||
artifactName: Tarball centos71 Offline | ||
downloadPath: '$(Build.StagingDirectory)/centos' |
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.
IMO you should use an each
and pass a list of objects to this template to keep this out of the implementation yaml. You can get this down to only have a a single input list of objects that you reprocess to create the dependsOn
and the sequence of download steps. The overall point being to keep the platform listings in a single file.
targetType: 'inline' | ||
workingDirectory: '$(Build.StagingDirectory)' | ||
script: | | ||
version=`echo centos/*/Private.SourceBuilt.Artifacts*.tar.gz | sed 's/\.tar\.gz$//' | sed 's/^centos\/.*\/Private\.SourceBuilt\.Artifacts\.//'` |
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.
$(
is preferred over `
generally. But this is also quite complex to be here and something that might belong as build logic (where I would think version would be accessible?).
echo $(Build.BuildId) > .buildid | ||
tar czf ../Private.SourceBuilt.Artifacts.$version.tar.gz * .version .buildnumber .buildid | ||
rm -rf * | ||
mv ../Private.SourceBuilt.Artifacts.*.tar.gz . |
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 isn't clear to me what this artifact will be used for... sounds like more than diag if it's ending in BAR, so I'm a little concerned since point of source-build is that you can't get a combined artifact like this.
<TarballDir>$(RepoRoot)artifacts/tarball/</TarballDir> | ||
<RestoreSources> | ||
$(RestoreSources); | ||
https://dotnetfeed.blob.core.windows.net/dotnet-tools-internal/index.json; |
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.
This might need to be a nuget.config to allow for internal servicing.
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.
Do you know if it will fall back to the root NuGet.config or do we need a separate one for this?
Co-Authored-By: Davis Goodin <dagood@users.noreply.github.com>
c4a8711
to
ba3a2e0
Compare
Example builds: (old) https://dev.azure.com/dnceng/internal/_build/results?buildId=438340, https://dev.azure.com/dnceng/internal/_build/results?buildId=438343, (latest) https://dev.azure.com/dnceng/internal/_build/results?buildId=438639, https://dev.azure.com/dnceng/internal/_build/results?buildId=438643.
@dagood, added you if you don't mind taking a look, I think you're more familiar with this than any of us.