dotnet / iot

This repo includes .NET Core implementations for various IoT boards, chips, displays and PCBs.
MIT License
2.12k stars 574 forks source link

Reorganize dependencies and infrastructure for ACS #2278

Closed pgrawehr closed 2 weeks ago

pgrawehr commented 4 months ago

Make sure the release build only uses the official package, so that it can be used in client applications that don't build Iot.Device.Common from scratch. Due to the way the compiler works, the client application must be built against the exactly same library the compiler itself is built.

Also updates some test settings

Microsoft Reviewers: Open in CodeFlow
pgrawehr commented 2 months ago

@joperezr Added extra notices as discussed.

pgrawehr commented 3 weeks ago

/azp run dotnet.iot

azure-pipelines[bot] commented 3 weeks ago
Azure Pipelines successfully started running 1 pipeline(s).
joperezr commented 2 weeks ago

Ok, so I now understand why the build is still producing stable packages even when you applied the property, looks like all projects under tools (and turns out also the ones under devices) aren't using Arcade at all, and I'm working on some changes on top of your PR to fix that but that is unblocking a can of worms unfortunately. I'll be able to push some changes soon, but we may want to reconsider holding the release for this as this would introduce risk as I'm essentially changing how all of these things are built.

joperezr commented 2 weeks ago

cc @pgrawehr ^^

joperezr commented 2 weeks ago

Ok, so did a whole bunch of changes in 36afdf7 to fix tools and devices directories not using Arcade to build. From that, I started getting a bunch of style and analyzer errors, so that's all of the changes in the *.cs files, mainly addressing those.

@pgrawehr could you pull my changes and build locally and validate everything looks good to you? If you want to do a local "Official stable build" you can run build.cmd -pack -ci /p:OfficialBuildId=20240613.99 /p:DotnetFinalVersionKind=Release which should generate stable versions of packages, except for acs package, that should stay unstable.

pgrawehr commented 2 weeks ago

@joperezr Not good. The build now fails with

D:\a\1\s\.dotnet\sdk\7.0.403\Sdks\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets(221,5): error NU5017: Cannot create a package that has no dependencies nor content. [D:\a\1\s\tools\ArduinoCsCompiler\Frontend\Frontend.csproj]

That package shouldn't be empty...

joperezr commented 2 weeks ago

Oh interesting, not sure why I didn't see that locally. That is because it is trying to create a symbol package (as that is the default on the repo) and this is a tool package. My latest commit should fix that.

joperezr commented 2 weeks ago

I do still think we should retarget the tool and have it target 8.0

joperezr commented 2 weeks ago

Not sure why the GH action is failing though, I think for that one it's best if you take a peek.

pgrawehr commented 2 weeks ago

Not sure why the GH action is failing though, I think for that one it's best if you take a peek.

That happened because the location of the bin folder has changed. Previously, the bin folders where below the projects, now they're all below the artifacts folder. Thus the build script was not able to find the binaries to execute.

pgrawehr commented 2 weeks ago

I do still think we should retarget the tool and have it target 8.0

Have you read my comment above? I quickly tried, and it doesn't even build, because the entire project still builds using .NET 7.0 (see global.json). I really wouldn't do such an update as part of this PR.

joperezr commented 2 weeks ago

Oh missed that. Yeah I agree, let's get this merged then and I can put up a follow up PR that moves us to 8.0 infra

pgrawehr commented 2 weeks ago

@joperezr Can you do the merge? I cannot, because the helix build keep failing. We really need to do something about this unstable ADC test.

joperezr commented 2 weeks ago

We should just disable that test as it is not good to setup a pattern to merge on red.