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

add CacheGet and CacheSet tasks #10418

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

adamralph
Copy link

Context

To allow MSBuild target authors to take advantage of the build cache without having to write custom tasks.

Changes Made

Added Microsoft.Build.Tasks.CacheGet and Microsoft.Build.Tasks.CacheSet.

Testing

None yet.

Notes

I'm sure there's plenty to be done to get this into an acceptable state, but I wanted to start a discussion about the feasibility of this feature before proceeding further, and a draft PR seemed like the easiest way to demonstrate what I'm proposing.

@rainersigwald
Copy link
Member

Interesting idea. Can you provide some context about how you'd expect to use this?

@adamralph
Copy link
Author

adamralph commented Jul 22, 2024

@rainersigwald sure, see adamralph/minver#1021

At the moment I'm including my own versions of these classes in the PR.

The use case is: the targets file spawns an external process to do some resource intensive work, based on a number of inputs. The output is always the same for a given set of inputs. Before spawning that process, I want to see if there is already a cached output value for the given inputs. If there is, I skip the resource intensive process and use that value. If there isn't, I run the resource intensive process and cache the output value. The target is embedded in project files, so in a given solution, with many projects, it will be executed many times, and usually with the same inputs.

@baronfel
Copy link
Member

Really the only problem I have with this is a usability one - as proposed we'd only be able to store strings in the cache. Would we want to be able to store other data types that Tasks can accept, like bool, ITaskItem, arrays of these?

@adamralph
Copy link
Author

adamralph commented Jul 22, 2024

@baronfel assuming MSBuild can marshal as task parameters of type object, we can change the types of both the key and the value to object (and remove the ?? "" in CacheGet), since that's what GetRegisteredTaskObject and RegisterTaskObject both use.

Also, I was wondering if either of lifetime and allowEarlyCollection should be exposed as task parameters, but that could come later. see adamralph/minver#1021 (comment)

Also, I was wondering if lifetimeshould be exposed as a task parameter, but that could come later.

@adamralph
Copy link
Author

FYI I tried to change the types to object but I got:

error MSB4069: The "System.Object" type of the "Key" parameter of the "CacheGet" task is not supported by MSBuild.

So I'm not sure how to support those other types. Maybe we would need separate tasks for each type? Personally, I think supporting strings is a very good start, and would probably cover most cases. Also, a lot of types can be converted to and from strings by the calling targets. These two tasks could have String in their names to make the type support clear. E.g. StringCacheGet and StringCacheSet.

@baronfel
Copy link
Member

String is probably fine for a first pass - I think the way this would be tackled without object support in MSBuild would be separately-named input and output parameters, one for each kind of input/output. Super gross.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants