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

CLI support base commit hash file as an input #3

Open
tingilee opened this issue May 23, 2022 · 1 comment
Open

CLI support base commit hash file as an input #3

tingilee opened this issue May 23, 2022 · 1 comment

Comments

@tingilee
Copy link

We are considering trying out target-determinator in Diff CI, but we have some performance constraints.

Can we add CLI that supports taking in an already computed hash file from master to avoid git checkout base commit and perform a cquery on the base commit for comparison? This is a feature in https://github.com/Tinder/bazel-diff#get-impacted-targets-command

With this feature, we can compute hash file in master CI and reuse the result in diff CI and save us seconds to minutes.

@illicitonion
Copy link
Collaborator

Thanks for filing this issue! I think this makes a lot of sense - definitely being able to cache these values across runs is very sensible.

Would you be interested in trying to put together a PR to implement this? The key pieces of code to affect would be:

  1. The entry-point here should optionally take a path to a hash file instead of a RevisionBefore - I'd probably introduce a new struct HashFileOrRevision which has either a HashFile or Revision set, and populate that on config.
  2. This calls into WalkAffectedTargets - accept a HashFileOrRevision for commitishBefore and pass it through to FullyProcess.
  3. In FullyProcess, populate queryInfoBefore by optionally deserializing JSON instead of calling fullyProcessRevision.

As far as I know, everything in QueryResults should be trivially JSON serializable (I think we'd need to add a Marshall implementation to MatchingTargets, but it's just a wrapper around a slice, so should be trivial).

We may want to nil-out any instances of analysis.ConfiguredTarget because they're probably very large and not useful other than for explaining why something changed. In fact, maybe all we want is to serialize MatchingTargets and TargetHashCache.cache, which maybe calls for its own struct purpose made for serialization/deserialization? We may also want to add some schema version number or similar but we can probably worry about that kind of thing in the future.

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

No branches or pull requests

2 participants