Don't deduplicate additional_coursier_options (#1596)

Co-authored-by: Nikolay <n.matyushin@tawasal.ae>

private/extensions/maven.bzl

diff --git a/private/extensions/maven.bzl b/private/extensions/maven.bzl
index 1bf3487..d534bbf 100644
--- a/private/extensions/maven.bzl
+++ b/private/extensions/maven.bzl
@@ -578,6 +578,16 @@
             seen.append(item)
     return merged_list
 
+def concat_coursier_options(root_list, non_root_list):
+    """Concatenate coursier options (root first), never deduplicating.
+
+    Unlike the other repo list attributes, `additional_coursier_options` is an
+    ordered argument vector passed verbatim to coursier, not a set. It can
+    legitimately contain repeated identical tokens (e.g. coursier's repeatable
+    `--variant` flag), so deduplicating it would corrupt the argument list.
+    """
+    return root_list + non_root_list
+
 def remove_fields(s):
     """Used for reducing an artifact struct down to only those fields that have values"""
     return {
@@ -694,11 +704,16 @@
             merged_repo["boms"] = _deduplicate_non_root_artifacts(bazel_dep_to_non_root_boms, True)
 
         # For list attributes, concatenate but avoid duplicates (root items first)
-        for list_attr in ["repositories", "excluded_artifacts", "additional_netrc_lines", "additional_coursier_options"]:
+        for list_attr in ["repositories", "excluded_artifacts", "additional_netrc_lines"]:
             root_list = root_repo.get(list_attr, [])
             non_root_list = non_root_repo.get(list_attr, [])
             merged_repo[list_attr] = _merge_repo_lists(root_list, non_root_list)
 
+        merged_repo["additional_coursier_options"] = concat_coursier_options(
+            root_repo.get("additional_coursier_options", []),
+            non_root_repo.get("additional_coursier_options", []),
+        )
+
         repos[repo_name] = merged_repo
 
     # Breaking out the logic for picking lock files, because it's not terribly simple

tests/unit/BUILD

diff --git a/tests/unit/BUILD b/tests/unit/BUILD
index ea20414..7f86a4d 100644
--- a/tests/unit/BUILD
+++ b/tests/unit/BUILD
@@ -1,5 +1,6 @@
 load(":amend_artifact_test.bzl", "amend_artifact_test_suite")
 load(":artifact_utilities_test.bzl", "artifact_utilities_test_suite")
+load(":concat_coursier_options_test.bzl", "concat_coursier_options_test_suite")
 load(":coordinates_test.bzl", "coordinates_test_suite")
 load(":coursier_test.bzl", "coursier_test_suite")
 load(":coursier_utilities_test.bzl", "coursier_utilities_test_suite")
@@ -17,6 +18,8 @@
 
 artifact_utilities_test_suite()
 
+concat_coursier_options_test_suite()
+
 coordinates_test_suite()
 
 coursier_test_suite()

tests/unit/concat_coursier_options_test.bzl

diff --git a/tests/unit/concat_coursier_options_test.bzl b/tests/unit/concat_coursier_options_test.bzl
new file mode 100644
index 0000000..f8d72a6
--- /dev/null
+++ b/tests/unit/concat_coursier_options_test.bzl
@@ -0,0 +1,69 @@
+"""Tests for merging `additional_coursier_options` across modules.
+
+These options form an ordered argument vector passed verbatim to coursier, so
+they must be concatenated (root first) and never deduplicated.
+"""
+
+load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest")
+load("//private/extensions:maven.bzl", "concat_coursier_options")
+
+def _preserves_repeated_tokens_impl(ctx):
+    env = unittest.begin(ctx)
+
+    # coursier's `--variant` flag is repeatable; deduplicating would collapse
+    # the repeated `--variant` tokens and corrupt the argument list.
+    root_options = [
+        "--enable-modules",
+        "--variant",
+        "org.gradle.category=library",
+        "--variant",
+        "org.gradle.usage=runtime",
+    ]
+
+    merged = concat_coursier_options(root_options, [])
+
+    asserts.equals(env, root_options, merged)
+
+    return unittest.end(env)
+
+preserves_repeated_tokens_test = unittest.make(_preserves_repeated_tokens_impl)
+
+def _concatenates_root_first_impl(ctx):
+    env = unittest.begin(ctx)
+
+    root_options = ["--enable-modules"]
+    non_root_options = ["--variant", "org.gradle.usage=runtime"]
+
+    merged = concat_coursier_options(root_options, non_root_options)
+
+    # Root options come first, then non-root, with nothing dropped.
+    asserts.equals(
+        env,
+        ["--enable-modules", "--variant", "org.gradle.usage=runtime"],
+        merged,
+    )
+
+    return unittest.end(env)
+
+concatenates_root_first_test = unittest.make(_concatenates_root_first_impl)
+
+def _keeps_cross_module_duplicates_impl(ctx):
+    env = unittest.begin(ctx)
+
+    # The same flag declared by both a root and a non-root module is kept twice:
+    # we cannot safely deduplicate an ordered, positional argument vector.
+    merged = concat_coursier_options(["--enable-modules"], ["--enable-modules"])
+
+    asserts.equals(env, ["--enable-modules", "--enable-modules"], merged)
+
+    return unittest.end(env)
+
+keeps_cross_module_duplicates_test = unittest.make(_keeps_cross_module_duplicates_impl)
+
+def concat_coursier_options_test_suite():
+    unittest.suite(
+        "concat_coursier_options_tests",
+        preserves_repeated_tokens_test,
+        concatenates_root_first_test,
+        keeps_cross_module_duplicates_test,
+    )