bazelisk icon indicating copy to clipboard operation
bazelisk copied to clipboard

migrate mode tests flags which are not yet available

Open siddharthab opened this issue 3 years ago • 11 comments

For example, as of today, with bazelisk version 1.5.0 and bazel version 3.4.1, the --migrate mode tries these flags which are not defined for bazel 3.4.1:

  --//tools/build_defs/pkg:incompatible_no_build_defs_pkg (Bazel 4.0: https://github.com/bazelbuild/bazel/issues/8857)
  --//tools/build_defs/pkg:incompatible_no_build_defs_pkg_deb (Bazel 4.0: https://github.com/bazelbuild/bazel/issues/11217)
  --//tools/build_defs/pkg:incompatible_no_build_defs_pkg_rpm (Bazel 4.0: https://github.com/bazelbuild/bazel/issues/11218)
  --incompatible_proto_output_v2 (Bazel 4.0: https://github.com/bazelbuild/bazel/issues/10358)

How are other people using the --migrate flag in practice?

See this Github Actions log for a real life example.

To reproduce:

git clone [email protected]:grailbio/bazel-compilation-database.git
cd bazel-compilation-database
tests/check_migration.sh

siddharthab avatar Aug 01 '20 07:08 siddharthab

Pinging the flag owners @aiuto and @joeleba - if I understand correctly, the flags should be available in Bazel 3.4.1, right? Then why does it fail (see the log)? 🤔

philwo avatar Aug 01 '20 14:08 philwo

Which flag, what log?

And why 3.4.1? I thought the next release was 3.5?

On Sat, Aug 1, 2020 at 10:37 AM Philipp Wollermann [email protected] wrote:

Pinging the flag owners @aiuto https://github.com/aiuto and @joeleba https://github.com/joeleba - if I understand correctly, the flags should be available in Bazel 3.4.1, right? Then why does it fail (see the log)? 🤔

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazelisk/issues/147#issuecomment-667541608, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXHHHEDTW2ZO77PHWQNECDR6QSCRANCNFSM4PRQV5FA .

aiuto avatar Aug 02 '20 01:08 aiuto

The problem is that these issues (e.g.: https://github.com/bazelbuild/bazel/issues/10358, https://github.com/bazelbuild/bazel/issues/11218) are labelled as migration-3.4 which makes bazelisk think that these flags are available in bazel-3.4.z, and so it tries to use these flags when running in --migrate and --strict mode. But these flags are actually not available in bazel-3.4.z, making the run fail.

What are the semantics of labelling an issue with migration-x.y?

This is an example log describing the failure.

siddharthab avatar Aug 02 '20 02:08 siddharthab

That's strange. I don't know why the rpm flag is not there. I can't speak about the other. I removed the label. It will be removed in 4.0 in any case, without a flagged migration or not. No one should be using it since the one in rules_pkg is more capable and supported.

On Sat, Aug 1, 2020 at 10:30 PM Siddhartha Bagaria [email protected] wrote:

The problem is that these issues (e.g.: bazelbuild/bazel#10358 https://github.com/bazelbuild/bazel/issues/10358, bazelbuild/bazel#11218 https://github.com/bazelbuild/bazel/issues/11218) are labelled as migration-3.4 which makes bazelisk think that these flags are available in bazel-3.4.z, and so it tries to use these flags when running in --migrate and --strict mode. But these flags are actually not available in bazel-3.4.z, making the run fail.

What are the semantics of labelling an issue with migration-x.y?

This is an example log https://github.com/grailbio/bazel-compilation-database/runs/934784679?check_suite_focus=true describing the failure.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/bazelbuild/bazelisk/issues/147#issuecomment-667615121, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAXHHHFVWDQFXJIAFA2X2DDR6TFURANCNFSM4PRQV5FA .

aiuto avatar Aug 02 '20 03:08 aiuto

--incompatible_proto_output_v2 isn't a build flag, but an aquery flag, so the failure is expected.

joeleba avatar Aug 03 '20 06:08 joeleba

@philwo What do you think about making the flag placement logic a little more intelligent?

I can send in a PR to help. I was thinking we can check whether or not to use the flag based on the command that's being used, and checking for that in bazel's help output for that command. Happy to go with whatever you suggest?

Without this, migration check in CIs has always been noisy.

siddharthab avatar Aug 04 '20 05:08 siddharthab

As for the starlark flags, they need to be prefixed with @bazel_tools, so they should be used as --@bazel_tools//tools/build_defs/pkg:incompatible_no_build_defs_pkg. Maybe we should rename the github issues that correspond to them?

siddharthab avatar Aug 04 '20 06:08 siddharthab

@siddharthab That sounds like an interesting idea. 🤔 If you can come up with a way that's not too complex, I'd be happy to review a PR.

Regarding the Starlark flags, if they have to be prefixed with @bazel_tools, I think we should rename the GitHub issue titles, yes.

philwo avatar Aug 04 '20 12:08 philwo

I would also be interested on knowing how people is supposed to use strict and migrate. I just naively run bazelisk --strict build //... and I also saw this error message ERROR: --incompatible_proto_output_v2 :: Unrecognized option: --incompatible_proto_output_v2

limdor avatar Nov 13 '20 18:11 limdor

This has not been an issue in the past several months, so we can close this.

siddharthab avatar Nov 09 '21 16:11 siddharthab

I would like if before closing it someone from Bazel team could comment on it. What is the official way for the user to migrate to a new bazel version? (thinking specially from an LTS to the next LTS). I assume that it would be to use --strict now that --all_incompatible_changes has been removed in https://github.com/bazelbuild/bazel/issues/13892.

limdor avatar Nov 09 '21 19:11 limdor

Closing since this looks more like a Bazel issue.

fweikert avatar Feb 06 '24 17:02 fweikert