bazel-contrib / target-determinator

Determines which Bazel targets were affected between two git commits.
Apache License 2.0
144 stars 22 forks source link

Unclean checkout problem #90

Closed eposinitskiy closed 4 months ago

eposinitskiy commented 4 months ago

Hi, I'm trying to configure target-determinator in CI and facing unclean checkout problem, where my MODULE.bazel.lock is being updated in place due to change in execution platform. Our devs all have MacOS and our CI is running on Ubuntu.

This seems to be a generally expected behavior of Bazel (https://github.com/bazelbuild/bazel/issues/21491) and --lockfile_mode= update is the default one.

As we are currently using bazel-diff where we control the revision checkout process, I was using git checkout --force to ignore any changes in lockfile, but this seems to be not an option in target-determinator. Using flag -ignore-file=MODULE.bazel.lock also doesn't help.

A query error occurred querying revision 'before' (sha: 3bf2816ad143f309f1732429481e99ed514502c8) - ignoring the error and treating all matching targets from the 'after' revision as affected. Error querying: failed to check out original commit during cleanup: failed to check out revision 'after' ([branch-name-redacted], sha: cc6f70650d523720ff615c7b82d4fb0e2321f2e8): exit status 1. Output: error: Your local changes to the following files would be overwritten by checkout:
    MODULE.bazel.lock
Please commit your changes or stash them before you switch branches.
Aborting
....
....
failed to process change: failed to check out original commit during cleanup: failed to check out revision 'after' ([branch-name-redacted], sha: cc6f70650d523720ff615c7b82d4fb0e2321f2e8): exit status 1. Output: error: Your local changes to the following files would be overwritten by checkout:
Target Determinator invocation Error
    MODULE.bazel.lock
Please commit your changes or stash them before you switch branches.
Aborting

My question is what is the recommended way of dealing with such a problem? Should I make sure to run build on all potential execution platforms in advance (is there a way to do it from a host platform?), or maybe I could contribute to add checkout --force making it a configurable behavior for target-determinator?

eposinitskiy commented 4 months ago

I've figured out how to workaround the problem.

Creating a dummy file in the working tree, which in turn allows target-determinator to find the tree is unclean and create new working dir, works perfectly as expected.

My mistake was using target-determinator with flag -ignore-file, this actually makes it think the tree is clean and proceed to checkout without -f flag in the same working directory.

Maybe for the sake of clarity it makes sense to mention this case in the docs for anyone else running into the same issue.

illicitonion commented 4 months ago

This is an interesting corner - we definitely put target-determinator together assuming that running Bazel itself doesn't modify the workspace. I know @fmeum has thoughts/plans around making MODULE.bazel.lock more platform-agnostic and consistent - @fmeum do you view this as a temporary bug we should be working around for now, or as the long-term state of things we should be building systems around?

fmeum commented 4 months ago

CI builds should run (and pass) with --lockfile_mode=error, which would also tell what isn't up-to-date in this case. Lockfile contents should be platform-agnostic if possible and I think that e.g. the latest version of rules_python achieves this.

I would consider this either a temporary bug (in module extensions provided by rulesets) or inherent complexity (if an extension legitimately depends on the host platform). In either case, special handling in target-determinator should not be needed.

eposinitskiy commented 4 months ago

Thank you for the feedback.

I did run with --lockfile_mode=error, and here's the error I've got

ERROR: The module extension 'ModuleExtensionId{bzlFileLabel=@@rules_go~//go:extensions.bzl, extensionName=go_sdk, isolationKey=Optional.empty}' for platform os:linux,arch:amd64 does not exist in the lockfile

Unfortunately I was not able to find a solution, how this should be resolved properly. Running build / mod tidy on MacOS doesn't change lock file, while running on CI this clearly produces changes into the lock file. (tried Bazel 7.1.1, 7.2.0).

I'm happy to follow the advice on how this issue should be handled. For now creating dummy file into the work tree is a workaround that works fine for us.

fmeum commented 4 months ago

This has been fixed in https://github.com/bazelbuild/rules_go/commit/a5bc53135c9ba286cdf7c5c78749a14bb70518f0, so updating rules_go should be all it takes.