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

build against latest core fx packages #254

Merged
merged 6 commits into from Jan 14, 2019

Conversation

Projects
None yet
6 participants
@stevenbrix
Copy link
Member

stevenbrix commented Jan 10, 2019

I've added the darc subscription for the ".NET Core 3 Dev" channel from the CoreFX repo to the WPF (this) repo. This change checks-in the files required by darc to allow Maestro to update these files and get the latest packages that come from the CoreFX build

I've also added other packages that are required for building WPF that come from the prototype branch.

Fixes #248

@danzil

danzil approved these changes Jan 10, 2019

@@ -4,10 +4,32 @@
<VersionPrefix>4.8.0</VersionPrefix>
<PreReleaseVersionLabel>prerelease</PreReleaseVersionLabel>
<AssemblyVersion>4.0.0.0</AssemblyVersion>
<SystemSecurityVersion>4.5.0</SystemSecurityVersion>
<SystemReflectionEmitVersion>4.3.0</SystemReflectionEmitVersion>

This comment has been minimized.

@ericstj

ericstj Jan 10, 2019

Member

Looks like you removed this property entirely. You need to add it back with a subscription.

    <SystemReflectionEmitVersion>4.6.0-preview.19060.1</SystemReflectionEmitVersion>

This comment has been minimized.

@stevenbrix

stevenbrix Jan 11, 2019

Member

I changed it to SystemReflectionEmitPackageVersion so that it will be properly updated by Maestro. Looks like the build failed because I forgot to update a few other files

This comment has been minimized.

@AdamYoblick

AdamYoblick Jan 11, 2019

Member

Also Steven, if you don't want to change the property names in your build, you can just make a property in Versions.props that matches the darc dependency name and just have your existing property wrap it. Like <MyProp>$(PropFromDarc)</MyProp>

We did this for the winforms package in DotNet-Trusted to avoid breaking other stuff.

@ericstj

This comment has been minimized.

Copy link
Member

ericstj commented Jan 10, 2019

In addition to the fix I pointed out above, you may need to update the dotnet version you're building with as was mentioned here: dotnet/corefx#34447 (comment), since those new packages will depend on type in the newer Microsoft.NETCore.App version which your currently getting via implicit reference from the SDK.

@AdamYoblick

This comment has been minimized.

Copy link
Member

AdamYoblick commented Jan 10, 2019

Yeah what Eric said is the tricky part. Taking corefx dependencies through Maestro is nice, but if we don't take coresdk updates at the same time, it could break stuff.

We probably need to add a darc dependency to coresdk as well so they update at the same time when Maestro runs.

@ericstj Adding a dependency to coresdk should be just as easy as the arcadesdk dependency, yes?

@ericstj

This comment has been minimized.

Copy link
Member

ericstj commented Jan 10, 2019

For now you can manually update the version here:

"version": "3.0.100-preview-009764"

but if we don't take coresdk updates at the same time, it could break stuff.

It's a little worse than that. You'll get corefx right away, but core-sdk will be a few steps behind (since at best it would be ingesting corefx at the same time you are, in reality it needs to go through core-setup first). As a result the current way winforms and WPF are setup means you'll be using a newer set of CoreFx packages than the shared framework. Mostly this will be OK but it could occasionally cause issues, in which case you may just wait to merge a corefx update until it flows into core-sdk and you get a build out. Alternatively you could use RuntimeFrameworkVersion and subscribe to MS.NETCore.App out of core-setup, but this will require folks to install that when running tests (and can still be a day behind).

@livarcocc is core-sdk publishing to BAR?

@AdamYoblick

This comment has been minimized.

Copy link
Member

AdamYoblick commented Jan 11, 2019

I checked the default channels with darc get-default-channels and I do see an entry for coresdk: https://github.com/dotnet/sdk @ refs/heads/master -> .NET Core 3 Dev

I checked what I think is the internal coresdk build (https://dnceng.visualstudio.com/internal/_build/results?buildId=70106&view=results). I DO see publishing of asset manifests to the build artifacts, but I do NOT see a Publish to BAR phase, so I think the answer is no. Also the asset manifest doesn't contain any package ID's, which means there are no dependencies to consume.

Livar would be able to answer better :)

Show resolved Hide resolved global.json

stevenbrix added some commits Jan 11, 2019

Show resolved Hide resolved eng/Versions.props Outdated
<SystemWindowsExtensionsPackageVersion>4.6.0-preview.19060.1</SystemWindowsExtensionsPackageVersion>
<!-- Packages that come from https://github.com/dotnet/corefxlab -->
<SystemReflectionTypeLoaderPackageVersion>0.1.0-preview2-181205-2</SystemReflectionTypeLoaderPackageVersion>
<!-- Maintain System.CodeDom PackageVersion at 4.4.0. See https://github.com/Microsoft/msbuild/issues/3627 -->

This comment has been minimized.

@ericstj

ericstj Jan 14, 2019

Member

You may want to avoid hardcoding this here and instead do it in the build-task project itself. Then have your tests reference the latest.

This comment has been minimized.

@vatsan-madhavan

vatsan-madhavan Jan 14, 2019

Member

Or perhaps define another property, $(SystemCodeDomPackageVersionForPresentationBuildTasks), and keep it constant. Then we can subscribe for updates to $(SystemCodeDomPackageVersion) - which will be the default used everywhere except PBT. This way we'd have all these versions defined in the same place, while also ensuring that the latest version System.CodeDom is used everywhere except in PBT.

@stevenbrix

This comment has been minimized.

Copy link
Member

stevenbrix commented Jan 14, 2019

Thanks everyone!

Closes #248

@stevenbrix stevenbrix closed this Jan 14, 2019

@stevenbrix stevenbrix reopened this Jan 14, 2019

@stevenbrix stevenbrix merged commit dbd3bd9 into master Jan 14, 2019

1 check passed

license/cla All CLA requirements met.
Details

@stevenbrix stevenbrix deleted the dev/stevenbrix/buildAgainstCoreFx branch Jan 14, 2019

@AdamYoblick

This comment has been minimized.

Copy link
Member

AdamYoblick commented Jan 15, 2019

@stevenbrix Did you remember to create the maestro subscription from corefx -> wpf so these changes will flow automatically?

Should be something like this (replace winforms with wpf):
darc add-subscription --channel ".NET Core 3 Dev" --source-repo https://github.com/dotnet/corefx --target-repo https://github.com/dotnet/winforms --target-branch master --update-frequency everyDay --ignore-checks WIP,license/cla --all-checks-passed -q

Nevermind, I ran get-subscriptions and I see you have one :)

"vs": {
"version": "15.9"
"version": "16.0"
}
},
"sdk": {

This comment has been minimized.

@AdamYoblick

AdamYoblick Jan 15, 2019

Member

@stevenbrix Pretty sure you want the sdk version to match the dotnet version, no?

@AdamYoblick

This comment has been minimized.

Copy link
Member

AdamYoblick commented Jan 15, 2019

Also @stevenbrix I spoke with @mmitche about where the dependencies should go (in Version.Details.xml) and he said they stop graph traversal under toolset. So if you want to be able to create a dependency graph for wpf, you should put all your dependencies (except things like coresdk and arcadesdk) under ProductDependencies instead of ToolsetDependencies.

Functionally (in terms of darc updates) it doesn't make any difference though, but it might later on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment