diff --git a/cargo/private/cargo_build_script.bzl b/cargo/private/cargo_build_script.bzl index b1c630a86e..cac1c60fb7 100644 --- a/cargo/private/cargo_build_script.bzl +++ b/cargo/private/cargo_build_script.bzl @@ -110,12 +110,18 @@ def _should_prefix_pwd(path): if paths.is_absolute(path): return False + if path.startswith("${pwd}"): + return False + for placeholder in _BAZEL_PATH_PLACEHOLDERS: if path.startswith(placeholder): return False return True +def _expects_space_separated_arg(flag): + return not (flag.endswith("=") or flag.endswith(":")) + def _prefix_pwd_to_flag(args, flag_variations): """Prefix execroot-relative paths for flags that support both concatenated and space-separated forms (unless it ends with `=` or `:`). @@ -144,18 +150,16 @@ def _prefix_pwd_to_flag(args, flag_variations): handled = False new_prefix_next_arg = False - for flag in flag_variations: + for flag in sorted(flag_variations, key = len, reverse = True): # Check for exact match first if arg == flag: - if flag.endswith("=") or flag.endswith(":"): - # Flag ending with '=' or ':' and empty path: keep as-is - res.append(arg) - handled = True - break - else: + if _expects_space_separated_arg(flag): # Flag without '=' or ':': next arg might be space-separated path new_prefix_next_arg = True - continue + + res.append(arg) + handled = True + break # Check for concatenated form (flag with path) if arg.startswith(flag): @@ -168,8 +172,10 @@ def _prefix_pwd_to_flag(args, flag_variations): handled = True break - # Check for space-separated form (only for flags without '=' or ':') - if not flag.endswith("=") and not flag.endswith(":") and prefix_next_arg and _should_prefix_pwd(arg.strip()): + # Check for space-separated form (only for flags without '=' or ':'). + # A leading '-' means the flag's value was omitted and this is + # actually the next flag, not a path; leave it untouched. + if _expects_space_separated_arg(flag) and prefix_next_arg and not arg.strip().startswith("-") and _should_prefix_pwd(arg.strip()): res.append("${{pwd}}/{}".format(arg.strip())) handled = True break @@ -260,8 +266,21 @@ def _pwd_paths(args): """Prefix execroot-relative paths with ${pwd}.""" return _prefix_pwd_to_paths(args) +_PWD_FLAG_PASSES = [ + _pwd_flags_sysroot, + _pwd_flags_resource_dir, + _pwd_flags_B, + _pwd_flags_L, + _pwd_flags_isystem, + _pwd_flags_fsanitize_ignorelist, + _pwd_flags_imacros, + _pwd_flags_direct_libs, +] + def _pwd_flags(args): - return _pwd_flags_direct_libs(_pwd_flags_imacros(_pwd_flags_fsanitize_ignorelist(_pwd_flags_isystem(_pwd_flags_L(_pwd_flags_B(_pwd_flags_resource_dir(_pwd_flags_sysroot(args)))))))) + for pwd_flags_pass in _PWD_FLAG_PASSES: + args = pwd_flags_pass(args) + return args def _feature_enabled(ctx, feature_name, default = False): """Check if a feature is enabled. diff --git a/cargo/tests/cargo_build_script/cc_args_and_env/BUILD.bazel b/cargo/tests/cargo_build_script/cc_args_and_env/BUILD.bazel index 496dfb5a2b..a2c9cd7c98 100644 --- a/cargo/tests/cargo_build_script/cc_args_and_env/BUILD.bazel +++ b/cargo/tests/cargo_build_script/cc_args_and_env/BUILD.bazel @@ -1,8 +1,10 @@ load( "cc_args_and_env_test.bzl", "bindir_absolute_test", + "bindir_malformed_missing_value_test", "bindir_relative_test", "direct_libs_absolute_test", + "direct_libs_as_flag_operand_test", "direct_libs_relative_test", "fsanitize_ignorelist_absolute_test", "fsanitize_ignorelist_relative_test", @@ -12,10 +14,12 @@ load( "include_mixed_test", "include_relative_test", "isystem_absolute_test", + "isystem_after_relative_test", "isystem_relative_test", "legacy_cc_toolchain_test", "libpath_absolute_test", "libpath_relative_test", + "libpath_separated_relative_test", "resource_dir_absolute_test", "resource_dir_relative_test", "sysroot_absolute_test", @@ -38,6 +42,8 @@ isystem_relative_test(name = "isystem_relative_test") isystem_absolute_test(name = "isystem_absolute_test") +isystem_after_relative_test(name = "isystem_after_relative_test") + xclang_isystem_relative_test(name = "xclang_isystem_relative_test") xclang_isystem_absolute_test(name = "xclang_isystem_absolute_test") @@ -46,6 +52,8 @@ bindir_relative_test(name = "bindir_relative_test") bindir_absolute_test(name = "bindir_absolute_test") +bindir_malformed_missing_value_test(name = "bindir_malformed_missing_value_test") + fsanitize_ignorelist_absolute_test(name = "fsanitize_ignorelist_absolute_test") fsanitize_ignorelist_relative_test(name = "fsanitize_ignorelist_relative_test") @@ -60,6 +68,8 @@ libpath_absolute_test(name = "libpath_absolute_test") libpath_relative_test(name = "libpath_relative_test") +libpath_separated_relative_test(name = "libpath_separated_relative_test") + resource_dir_absolute_test(name = "resource_dir_absolute_test") resource_dir_relative_test(name = "resource_dir_relative_test") @@ -73,3 +83,5 @@ include_mixed_test(name = "include_mixed_test") direct_libs_relative_test(name = "direct_libs_relative_test") direct_libs_absolute_test(name = "direct_libs_absolute_test") + +direct_libs_as_flag_operand_test(name = "direct_libs_as_flag_operand_test") diff --git a/cargo/tests/cargo_build_script/cc_args_and_env/cc_args_and_env_test.bzl b/cargo/tests/cargo_build_script/cc_args_and_env/cc_args_and_env_test.bzl index 6bd74edd06..663650e46b 100644 --- a/cargo/tests/cargo_build_script/cc_args_and_env/cc_args_and_env_test.bzl +++ b/cargo/tests/cargo_build_script/cc_args_and_env/cc_args_and_env_test.bzl @@ -463,6 +463,18 @@ def isystem_absolute_test(name): expected_cflags = ["-isystem", "/test/absolute/path"], ) +def isystem_after_relative_test(name): + """Regression test: a longer flag spelling must win over a shorter prefix.""" + cargo_build_script_with_extra_cc_compile_flags( + name = "%s/cargo_build_script" % name, + extra_cc_compile_flags = ["-isystem-after", "test/relative/path"], + ) + cc_args_and_env_analysis_test( + name = name, + target_under_test = "%s/cargo_build_script" % name, + expected_cflags = ["-isystem-after", "${pwd}/test/relative/path"], + ) + def bindir_relative_test(name): cargo_build_script_with_extra_cc_compile_flags( name = "%s/cargo_build_script" % name, @@ -485,6 +497,18 @@ def bindir_absolute_test(name): expected_cflags = ["-B", "/test/absolute/path"], ) +def bindir_malformed_missing_value_test(name): + """Regression test: `-B` with no path must not swallow the next, unrelated flag.""" + cargo_build_script_with_extra_cc_compile_flags( + name = "%s/cargo_build_script" % name, + extra_cc_compile_flags = ["-B", "-Wall"], + ) + cc_args_and_env_analysis_test( + name = name, + target_under_test = "%s/cargo_build_script" % name, + expected_cflags = ["-B", "-Wall"], + ) + def fsanitize_ignorelist_relative_test(name): cargo_build_script_with_extra_cc_compile_flags( name = "%s/cargo_build_script" % name, @@ -540,6 +564,18 @@ def libpath_absolute_test(name): expected_cflags = ["-L/test/absolute/sysroot", "-L", "/test/absolute/sysroot2", "-LIBPATH:/test/absolute/sysroot3", "-LIBPATH=/test/absolute/sysroot4", "-LIBPATH:", "some_unrelated_arg", "-LIBPATH=", "some_unrelated_arg2"], ) +def libpath_separated_relative_test(name): + """Regression test: bare `-LIBPATH` must not be matched as `-L` + `IBPATH`.""" + cargo_build_script_with_extra_cc_compile_flags( + name = "%s/cargo_build_script" % name, + extra_cc_compile_flags = ["-LIBPATH", "test/relative/sysroot", "-LIBPATH", "/test/absolute/sysroot"], + ) + cc_args_and_env_analysis_test( + name = name, + target_under_test = "%s/cargo_build_script" % name, + expected_cflags = ["-LIBPATH", "${pwd}/test/relative/sysroot", "-LIBPATH", "/test/absolute/sysroot"], + ) + def resource_dir_relative_test(name): cargo_build_script_with_extra_cc_compile_flags( name = "%s/cargo_build_script" % name, @@ -656,3 +692,15 @@ def direct_libs_absolute_test(name): target_under_test = "%s/cargo_build_script" % name, expected_cflags = ["/test/absolute/libclang_rt.builtins.static.a", "/test/absolute/obj.o", "/test/absolute/libfoo.so", "/test/absolute/libbar.dylib", "some_unrelated_arg"], ) + +def direct_libs_as_flag_operand_test(name): + """Regression test: an operand already rewritten must not be rewritten twice.""" + cargo_build_script_with_extra_cc_compile_flags( + name = "%s/cargo_build_script" % name, + extra_cc_compile_flags = ["-imacros", "test/relative/libfoo.a", "-B", "test/relative/obj.o"], + ) + cc_args_and_env_analysis_test( + name = name, + target_under_test = "%s/cargo_build_script" % name, + expected_cflags = ["-imacros", "${pwd}/test/relative/libfoo.a", "-B", "${pwd}/test/relative/obj.o"], + )