Skip to content
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

Respect property to disable targeting pack caching for runtime build #22834

Open
rainersigwald opened this issue Nov 30, 2021 · 6 comments
Open
Assignees
Labels
Area-NetSDK untriaged Request triage from a team member

Comments

@rainersigwald
Copy link
Member

#19570 introduced caching in ResolveTargetingPackAssets. I'm not aware of complaints from "normal" users, but for the dotnet/runtime repo, which produces those targeting packs during its build, the caching can preserve stale data.

I think this is becoming an issue in between using dotnet build on source projects where the caching feature is not disabled as the env var is only set on the build scripts and then using dotnet build or dotnet test on a test project where the platform manifest is needed.

@rainersigwald is it possible that we can also support an MSBuild property to disable the targeting pack caching?

Originally posted by @safern in dotnet/runtime#61451 (comment)

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Request triage from a team member labels Nov 30, 2021
@rainersigwald
Copy link
Member Author

Yeah, we can add an input to the task and pass a property through to it. It would require an SDK update. @marcpopMSFT would you have any objection to adding such a feature to an SDK task when the only expected consumer is the runtime build? I personally don't . . .

Originally posted by @rainersigwald in dotnet/runtime#61451 (comment)

@ericstj
Copy link
Member

ericstj commented Dec 2, 2021

Is it possible that other scenarios like dependency flow or servicing (both install new side-by-side targeting packs) would hit this? I didn't look at the cache keys to see if they would or not.

IMO it'd be better if we didn't have to disable this, but something in the cache-key captured the "change" that happens in dotnet/runtime that causes it to break. cc @safern @eerhardt

@rainersigwald
Copy link
Member Author

captured the "change" that happens in dotnet/runtime that causes it to break

Yeah, is this just "files magically appear in those directories" or is it more complicated?

@ericstj
Copy link
Member

ericstj commented Dec 2, 2021

I'm not 100% certain as I haven't looked at a repro personally. I can imagine things like file existence and modified time might be interesting inputs to capture in the cache key, similar to target inputs/outputs.

@rainersigwald
Copy link
Member Author

We opted not to have existence/timestamp checks for cache invalidation for this particular cache because it doesn't generally change for users during a process lifetime since everything in it is part of the .NET SDK. Timestamp checks can be surprisingly expensive.

We can add them, but I'd prefer not to if we can get away with opting into "unstable targeting pack mode" in limited circumstances like "the runtime build".

@ericstj
Copy link
Member

ericstj commented Dec 2, 2021

One final thought here. Runtime might want to benefit from caching too. Perhaps if you provided some way for us to control the cache key or change something about it so that we could differentiate this scenario. Could be a way to have cake and eat it too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

3 participants