Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 5fad501a authored by Liz Kammer's avatar Liz Kammer
Browse files

bp2build: Split export_{includes,system_includes} 

The specification of exporting includes vs system includes has an impact
on inclusion sort order. Conflating the two caused some symbols to not
be resolved correctly.

Bug: 198403271
Test: build/bazel/ci/bp2build.sh
Test: USE_BAZEL_ANALYSIS=1 m libbacktrace_no_dex succeeds with libc++_*
      modules removed from mixed build denylist (would fail otherwise)
Change-Id: I08aff253d8962dc678ed10214b1c171330e0fe19
parent 4011ebb1

bazel/properties.go

+25 −0
Original line number Diff line number Diff line
@@ -743,6 +743,31 @@ func (sla *StringListAttribute) SortedConfigurationAxes() []ConfigurationAxis { 
	return keys

}



// DeduplicateAxesFromBase ensures no duplication of items between the no-configuration value and

// configuration-specific values. For example, if we would convert this StringListAttribute as:

// ["a", "b", "c"] + select({

//    "//condition:one": ["a", "d"],

//    "//conditions:default": [],

// })

// after this function, we would convert this StringListAttribute as:

// ["a", "b", "c"] + select({

//    "//condition:one": ["d"],

//    "//conditions:default": [],

// })

func (sla *StringListAttribute) DeduplicateAxesFromBase() {

	base := sla.Value

	for axis, configToList := range sla.ConfigurableValues {

		for config, list := range configToList {

			remaining := SubtractStrings(list, base)

			if len(remaining) == 0 {

				delete(sla.ConfigurableValues[axis], config)

			} else {

				sla.ConfigurableValues[axis][config] = remaining

			}

		}

	}

}



// TryVariableSubstitution, replace string substitution formatting within each string in slice with

// Starlark string.format compatible tag for productVariable.

func TryVariableSubstitutions(slice []string, productVariable string) ([]string, bool) {

bazel/properties_test.go

+71 −0
Original line number Diff line number Diff line
@@ -293,3 +293,74 @@ func TestResolveExcludes(t *testing.T) { 
		}

	}

}



func TestDeduplicateAxesFromBase(t *testing.T) {

	attr := StringListAttribute{

		Value: []string{

			"all_include",

			"arm_include",

			"android_include",

			"linux_x86_include",

		},

		ConfigurableValues: configurableStringLists{

			ArchConfigurationAxis: stringListSelectValues{

				"arm": []string{"arm_include"},

				"x86": []string{"x86_include"},

			},

			OsConfigurationAxis: stringListSelectValues{

				"android": []string{"android_include"},

				"linux":   []string{"linux_include"},

			},

			OsArchConfigurationAxis: stringListSelectValues{

				"linux_x86": {"linux_x86_include"},

			},

			ProductVariableConfigurationAxis("a"): stringListSelectValues{

				"a": []string{"not_in_value"},

			},

		},

	}



	attr.DeduplicateAxesFromBase()



	expectedBaseIncludes := []string{

		"all_include",

		"arm_include",

		"android_include",

		"linux_x86_include",

	}

	if !reflect.DeepEqual(expectedBaseIncludes, attr.Value) {

		t.Errorf("Expected Value includes %q, got %q", attr.Value, expectedBaseIncludes)

	}

	expectedConfiguredIncludes := configurableStringLists{

		ArchConfigurationAxis: stringListSelectValues{

			"x86": []string{"x86_include"},

		},

		OsConfigurationAxis: stringListSelectValues{

			"linux": []string{"linux_include"},

		},

		OsArchConfigurationAxis: stringListSelectValues{},

		ProductVariableConfigurationAxis("a"): stringListSelectValues{

			"a": []string{"not_in_value"},

		},

	}

	for _, axis := range attr.SortedConfigurationAxes() {

		if _, ok := expectedConfiguredIncludes[axis]; !ok {

			t.Errorf("Found unexpected axis %s", axis)

			continue

		}

		expectedForAxis := expectedConfiguredIncludes[axis]

		gotForAxis := attr.ConfigurableValues[axis]

		if len(expectedForAxis) != len(gotForAxis) {

			t.Errorf("Expected %d configs for %s, got %d: %s", len(expectedForAxis), axis, len(gotForAxis), gotForAxis)

		}

		for config, value := range gotForAxis {

			if expected, ok := expectedForAxis[config]; ok {

				if !reflect.DeepEqual(expected, value) {

					t.Errorf("For %s, expected: %#v, got %#v", axis, expected, value)

				}

			} else {

				t.Errorf("Got unexpected config %q for %s", config, axis)

			}

		}

	}

}

bp2build/cc_library_conversion_test.go

+1 −1
Original line number Diff line number Diff line
@@ -122,8 +122,8 @@ cc_library { 
        "-I.",

        "-I$(BINDIR)/.",

    ],

    export_includes = ["foo-dir"],

    implementation_deps = [":some-headers"],

    includes = ["foo-dir"],

    linkopts = ["-Wl,--exclude-libs=bar.a"] + select({

        "//build/bazel/platforms/arch:x86": ["-Wl,--exclude-libs=baz.a"],

        "//build/bazel/platforms/arch:x86_64": ["-Wl,--exclude-libs=qux.a"],

bp2build/cc_library_headers_conversion_test.go

+9 −9
Original line number Diff line number Diff line
@@ -132,11 +132,7 @@ cc_library_headers { 
        "-I.",

        "-I$(BINDIR)/.",

    ],

    implementation_deps = [

        ":lib-1",

        ":lib-2",

    ],

    includes = [

    export_includes = [

        "dir-1",

        "dir-2",

    ] + select({
@@ -145,20 +141,24 @@ cc_library_headers { 
        "//build/bazel/platforms/arch:x86_64": ["arch_x86_64_exported_include_dir"],

        "//conditions:default": [],

    }),

    implementation_deps = [

        ":lib-1",

        ":lib-2",

    ],

)`, `cc_library_headers(

    name = "lib-1",

    copts = [

        "-I.",

        "-I$(BINDIR)/.",

    ],

    includes = ["lib-1"],

    export_includes = ["lib-1"],

)`, `cc_library_headers(

    name = "lib-2",

    copts = [

        "-I.",

        "-I$(BINDIR)/.",

    ],

    includes = ["lib-2"],

    export_includes = ["lib-2"],

)`},

	})

}
@@ -337,7 +337,7 @@ func TestCcLibraryHeadersArchAndTargetExportSystemIncludes(t *testing.T) { 
        "-I.",

        "-I$(BINDIR)/.",

    ],

    includes = ["shared_include_dir"] + select({

    export_system_includes = ["shared_include_dir"] + select({

        "//build/bazel/platforms/arch:arm": ["arm_include_dir"],

        "//build/bazel/platforms/arch:x86_64": ["x86_64_include_dir"],

        "//conditions:default": [],
@@ -382,7 +382,7 @@ cc_library_headers { 
        "-I.",

        "-I$(BINDIR)/.",

    ],

    includes = ["lib-1"],

    export_includes = ["lib-1"],

)`},

	})

}

bp2build/cc_library_static_conversion_test.go

+7 −7
Original line number Diff line number Diff line
@@ -192,16 +192,16 @@ cc_library_static { 
        "-I.",

        "-I$(BINDIR)/.",

    ],

    export_includes = [

        "export_include_dir_1",

        "export_include_dir_2",

    ],

    implementation_deps = [

        ":header_lib_1",

        ":header_lib_2",

        ":static_lib_1",

        ":static_lib_2",

    ],

    includes = [

        "export_include_dir_1",

        "export_include_dir_2",

    ],

    linkstatic = True,

    srcs = [

        "foo_static1.cc",
@@ -312,7 +312,7 @@ cc_library_static { 
        "-I.",

        "-I$(BINDIR)/.",

    ],

    includes = ["subpackage"],

    export_includes = ["subpackage"],

    linkstatic = True,

)`},

	})
@@ -341,7 +341,7 @@ cc_library_static { 
        "-I.",

        "-I$(BINDIR)/.",

    ],

    includes = ["subpackage"],

    export_system_includes = ["subpackage"],

    linkstatic = True,

)`},

	})
@@ -391,7 +391,7 @@ cc_library_static { 
        "-Isubpackage",

        "-I$(BINDIR)/subpackage",

    ],

    includes = ["./exported_subsubpackage"],

    export_includes = ["./exported_subsubpackage"],

    linkstatic = True,

)`},

	})