search

microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.31k stars 1.14k forks source link

Stop using `/p:RestoreForceEvaluate=true` in CI #12004

Open jonthysell opened 1 year ago

jonthysell commented 1 year ago

Problem Description

The whole point of using (NuGet) dependency lock files is to ensure reliable builds by locking to the dependencies in the lock file.

However, we let the PR/CI re-evaluate the dependencies at build time, which is a big no-no. Worst case scenario we download and use a hijacked dependency package, ignoring that the version/hash doesn't match what's in our (trusted) lock file.

Steps To Reproduce

See PR/CI issues such as #11998 for examples of PR/CI using dependencies we didn't expect.

Expected Results

No response

CLI version

npx react-native -v

Environment

npx react-native info

Target Platform Version

None

Target Device(s)

Desktop

Visual Studio Version

Visual Studio 2022

Build Configuration

None

Snack, code example, screenshot, or link to a repository

No response

jonthysell commented 1 year ago

Note, we can't just turn this off because of our mixing of C## and C++ projects. From https://github.com/microsoft/react-native-windows/issues/11998#issuecomment-1673859527:

... because we have UWP C# projects (such as Microsoft.ReactNative.Managed) that depend on C++/WinRT projects (such as Microsoft.ReactNative), and we're using PackageReferences in both, NuGet restore always fails if we don't use force evaluate. We get these errors:

D:\a\_work\1\s\vnext\Microsoft.ReactNative.Managed\Microsoft.ReactNative.Managed.csproj : error NU1004: The project Microsoft.ReactNative has no compatible target framework. The packages lock file is inconsistent with the project dependencies so restore can't be run in locked mode. Disable the RestoreLockedMode MSBuild property or pass an explicit --force-evaluate option to run restore to update the lock file. [D:\a\_work\1\s\vnext\Microsoft.ReactNative.sln]
D:\a\_work\1\s\vnext\Microsoft.ReactNative.Managed.UnitTests\Microsoft.ReactNative.Managed.UnitTests.csproj : error NU1004: The project Microsoft.ReactNative has no compatible target framework. The packages lock file is inconsistent with the project dependencies so restore can't be run in locked mode. Disable the RestoreLockedMode MSBuild property or pass an explicit --force-evaluate option to run restore to update the lock file. [D:\a\_work\1\s\vnext\Microsoft.ReactNative.sln]
D:\a\_work\1\s\vnext\Microsoft.ReactNative.Managed.IntegrationTests\Microsoft.ReactNative.Managed.IntegrationTests.csproj : error NU1004: The project Microsoft.ReactNative has no compatible target framework. The packages lock file is inconsistent with the project dependencies so restore can't be run in locked mode. Disable the RestoreLockedMode MSBuild property or pass an explicit --force-evaluate option to run restore to update the lock file. [D:\a\_work\1\s\vnext\Microsoft.ReactNative.sln]

This is a bug in VS/NuGet. Related issues/comments:

So we're stuck in a spot where we have to force the evaluation on every restore, even if everything is correct and we're happy with the state of our lock files.

acoates-ms commented 1 year ago

Is this something you are going to look at @jonthysell or should @JunielKatarn look into it?

jonthysell commented 5 months ago

This is technically important, but we can't really implement this without ONE of the following:

  1. Every build requests the same dependencies even if they don't use them (i.e. no variations where projects do/don't use WinAppSDK)
  2. We only have one set of binaries and therefore one set of dependencies (i.e. retire react-native-win32, RNW Paper, etc).

Putting this on the backlog until we get some kind of alert.