add source-build pre-built detection #933
Conversation
|
Will the title of this PR be copied to the release notes as is? If so, please include "source-build" in it. I totally expected "pre-build detection" to be a new feature that detects if the CI pipeline has run Uh I suppose I got confused about the repository as well. |
oleksandr-didyk
changed the title
oleksandr-didyk
changed the title
oleksandr-didyk
force-pushed
the
feat-enable-pre-build-detection
branch
from
fcab584 to
c616e6c
Compare
oleksandr-didyk
force-pushed
the
feat-enable-pre-build-detection
branch
from
c6134d4 to
f0df3de
Compare
|
@oleksandr-didyk - Can you give a status on this please? Is there in-progress work or is it blocked or just pending work? Thanks |
oleksandr-didyk
force-pushed
the
feat-enable-pre-build-detection
branch
from
f0df3de to
a2e7374
Compare
It was initially waiting for the Arcade bump to flow in. The change got into |
oleksandr-didyk
force-pushed
the
feat-enable-pre-build-detection
branch
from
9e2f00b to
02f5032
Compare
|
OK there's one more PR that I just submitted that will again make things easier and this PRs diff much smaller: #1003. I suggest to also wait for that one. |
oleksandr-didyk
force-pushed
the
feat-enable-pre-build-detection
branch
3 times, most recently
from
c9d7ba3 to
99d65a8
Compare
| @@ -4,6 +4,7 @@ | |||
| <TargetFrameworks>$(NetCurrent)</TargetFrameworks> | |||
| <!-- Allow tool to roll forward to a newer major version. --> | |||
| <RollForward>Major</RollForward> | |||
| <ExcludeFromSourceBuild>true</ExcludeFromSourceBuild> | |||
There was a problem hiding this comment.
Why does this tool need to be excluded from source build? Please add a comment as it isn't clear just from reading the code.
There was a problem hiding this comment.
I did it based on the project's description:
<Description>Command line tool for SourceLink testing.</Description>Since I thought it is a tool used for testing, but reading it again I might be wrong here. Maybe @tmat can clarify it
Additionally
<PackAsToolShimRuntimeIdentifiers>win-x64;win-x86;osx-x64</PackAsToolShimRuntimeIdentifiers>causes some additional packages to get pulled in that are marked as pre-builts:
<Usage Id="Microsoft.AspNetCore.App.Runtime.osx-x64" Version="8.0.0-preview.3.23177.8" />
<Usage Id="Microsoft.AspNetCore.App.Runtime.win-x64" Version="8.0.0-preview.3.23177.8" />
<Usage Id="Microsoft.AspNetCore.App.Runtime.win-x86" Version="8.0.0-preview.3.23177.8" />
<Usage Id="Microsoft.NETCore.App.Host.osx-x64" Version="8.0.0-preview.3.23174.8" />
<Usage Id="Microsoft.NETCore.App.Host.win-x64" Version="8.0.0-preview.3.23174.8" />
<Usage Id="Microsoft.NETCore.App.Host.win-x86" Version="8.0.0-preview.3.23174.8" />
<Usage Id="Microsoft.NETCore.App.Runtime.osx-x64" Version="8.0.0-preview.3.23174.8" />
<Usage Id="Microsoft.NETCore.App.Runtime.win-x64" Version="8.0.0-preview.3.23174.8" />
<Usage Id="Microsoft.NETCore.App.Runtime.win-x86" Version="8.0.0-preview.3.23174.8" />
There was a problem hiding this comment.
It should be included. This is not a shipping product yet but it is meant to be. It's for customers to test that their binaries have correct Source Link.
There was a problem hiding this comment.
This is not a shipping product yet
Can this be done as a separate issue to not block this PR? Or it being excluded for some time is a no-go?
Specifically since the assets pulled in by PackAsToolShimRuntimeIdentifiers are non-Linux; or can this property be simply excluded? (as source-build is Linux-only for now)
There was a problem hiding this comment.
IMO if source build repositories currently don't depend on the tool, it shouldn't be an issue to exclude it for now and track enabling it.
There was a problem hiding this comment.
Created an issue for it -> #1028
oleksandr-didyk
force-pushed
the
feat-enable-pre-build-detection
branch
from
d790a53 to
81716f5
Compare
Resolves #932
Additionally created a subscription for the dependency on
runtime(disabled until this PR is merged):CC: @MichaelSimons @crummel