Commit graph

19 commits

Author SHA1 Message Date
Liz Kammer
e72e1ff98b Merge "Error if visibility specifies a module" 2020-10-19 19:06:49 +00:00
Liz Kammer
873f4b612a Error if visibility specifies a module
Clarify that only __pkg__ and __subpackages__ are valid package
visibility specifications.

Test: go test soong tests
Test: m nothing
Bug: 168926331
Change-Id: I810e9ef92d081d85cd762e7f8c0b0716f0222883
2020-10-15 17:40:10 -07:00
Liz Kammer
e501bb4d48 Expand visiblity error
Expands the error to suggest a potential fix, giving the package path
for the module that needs visiblity.

Test: go test soong tests
Bug: 168926331
Change-Id: Ie95dec03f5e2c3190bfaed1f0e5789eeb0ab1878
2020-10-15 15:38:27 -07:00
Paul Duffin
157f40f056 Add prebuilt_visibility property
Some mainline modules provide an `sdk` and one or more set of
module_exports. When the snapshots are unpacked into separate prebuilts
directories (one per snapshot) then any dependencies from a member of
one snapshot to a member of another may require the latter to have
additional visibility rules.

Previously, these rules had to be added to each source module. This
change allows additional visibility rules to be specified on the
sdk/module_exports that are added to all the prebuilts in its snapshot.

Bug: 155921753
Bug: 168301990
Test: m nothing
Change-Id: Ia3eacb5749981f04770ae9872a8013e43c5c6ef1
2020-10-02 10:59:41 +01:00
Paul Duffin
d99d997238 Explicitly specify visibility in sdk/module_exports snapshot
This change ensures that each prebuilt in a snapshot explicitly
specifies its visibility even when that visibility is the current
default. This is done for two reasons:
1. It simplifies a follow up change that adds visibility rules to an
   existing set of rules.
2. It ensures that the snapshots are independent of the current Soong
   default visibility.

The latter is important because we intend to switch from modules being
visible to everyone by default (i.e. //visibility:public) to only being
visible to modules in the same package (i.e. //visibility:private). By
making the snapshots of modules that do not specify any visibility
explicitly specify that they are "//visibility:public" it ensures that
the snapshots will not need to be changed when the default changes.

Bug: 168301990
Test: m nothing
Change-Id: Ia034f4a1e5124c17f46d73b0e9a6c5f2a251038e
2020-10-01 18:20:13 +01:00
Martin Stjernholm
64aeaaddef Fix special case when appending a visibility rule to //visibility:private.
Test: m nothing
Bug: 151303681
Change-Id: Ibe7e953219e4660c653063e87cb08407a2cb513d
2020-05-14 16:52:24 +01:00
Martin Stjernholm
0641d189d0 Add explicit visibility on the self package in EffectiveVisibilityRules.
Modules in a package is implicitly visible within the same package.
When making effective visibility rules which work from another location
that visibility needs to be mentioned explicitly. In practice this
allows prebuilts to remain visible to other modules in the source
package when the prebuilts are preferred.

Test: m nothing
Test: m conscrypt-module-sdk, check that the snapshot Soong modules give
  visibility to //external/conscrypt
Bug: 151303681
Change-Id: I404f6e5ca7021974a8c7be5e4d6b4982c050b8a5
2020-05-14 16:50:59 +01:00
Martin Stjernholm
01407c525a Don't print :__pkg__ for package rules, since that's the default.
To reduce clutter a little.

Test: m nothing
Change-Id: I0d2d86f00c3d38e603d6f58c4b1dd9a4170bf819
2020-05-14 16:50:59 +01:00
Paul Duffin
51084ff6cf Add //visibility:override to allow control over inheritance
Visibility rules can be 'inherited' in one of two ways. Either from
defaults or from a module that called ctx.CreateModule(...).
Previously, in both cases the inheriting module could only append
additional visibility rules to the end of the inherited rules. That
made it impossible to restrict the visibility by removing or ignore
inherited rules.

The //visibility:override rectifies that by allowing the inheriting
module to ignore all the rules that they would have inherited. It can
only go at the beginning of a list of rules specified in a module but
after defaults are applied it can end up in the middle of a list of
rules. In that case it behaves as if all the rules up to and including
the //visibility:override rule were discarded.

It can be used with //visibility:private to override
//visibility:public and vice versa.

Bug: 155787200
Test: m nothing
Change-Id: I8a9c9c5a1bdceaee387c08864ae2b34629e0d46f
2020-05-12 20:07:46 +01:00
Paul Duffin
0c83aba28e Report visibility errors in both check and gather phases
Previously, errors that were found when splitting visibility rules were
only reported in the check phase and simply ignored during the gather
phase. That was because every visibility property that was processed in
the gather phase had already been checked in the check phase. However,
that is not strictly true as it has always been possible to add a
mutator between the check and gather phases that creates a module with
invalid visibility properties that will just be ignored. Fortunately,
that has not happened.

A follow up commit will add the capability to create modules after the
defaults have been applied which means the chances of invalid visibility
properties being ignored will increase.

This change makes both phases report any errors they find. It will not
have any impact on existing code as if any errors are reported in the
check phase then the build will exit before the gather phase. It will
prevent any invalid visibility errors from being ignored.

Bug: 155295806
Test: m nothing
Change-Id: I101fa6b03d2530b16e4394a9e466fead48be0ff0
2020-05-01 18:35:07 +01:00
Paul Duffin
5ec73ecc08 Reduce duplication in visibility property management
Adds a couple of new methods to manage visibility property instances to
reduce duplication and encapsulate the implementation slightly better.

The AddVisibilityProperty method is exported as it will be needed by
other packages in follow up commits.

Bug: 155295806
Test: m nothing
Change-Id: Ic1a9bb1e151fc6ae65761344fd210b4e4ba74fbc
2020-05-01 18:35:05 +01:00
Paul Duffin
44885e29d6 Simplify visibility rules that include //visibility:public
While it is invalid to mix //visibility:public with other rules in the
visibility property in a .bp file tt was possible, by overriding
defaults, to have //visibility:public mixed in with other rules in the
effective visibility rules. That caused problems when those effective
rules were used in an sdk snapshot.

This change replaces any set of rules that include //visibility:public
with just the //visibility:public rule. That simplifies those rules,
making them cheaper to process and ensures that the effective rules are
valid in the visibility property.

Adding test support required some refactoring of the
effectiveVisibilityRules(BaseModuleContext, ...) and underlying methods
to take a Config instead of BaseModuleContext as the tests do not have
access to BaseModuleContext.

Bug: 142935992
Test: m nothing - new tests failed without change, work with it
      Add dex2oat to art-module-host-exports, build it and check the
      generated Android.bp file in the snapshot to ensure the
	  visibility property for the dex2oat prebuilt does not mix
	  //visibility:public with other rules.
Change-Id: I08e7f0dcb40838d426fe88fedf69eae27b77473c
2020-02-20 11:41:17 +00:00
Paul Duffin
78ac5b962e Exclude source->prebuilt deps from visibility enforcement
When both prebuilt and source versions of a module are present in the
build an implicit dependency is added from source -> prebuilt. This
change excludes that dependency from visibility enforcement as it
provides no value and would otherwise require an sdk snapshot to
rewrite the visibility of the generated prebuilt modules which would
be complicated.

The rewriting would include:
* Detecting //visibility:public and just passing that straight through.
* Detecting //visibility:private and replacing that with the location
  of the source.
* Otherwise, adding the location of the source to the visibility
  property.

This adds a general mechanism to allow any dependency to be excluded
from visibility enforcement by simply using a tag that implements
the ExcludeFromVisibilityEnforcementTag interface.

Bug: 142940300
Test: m nothing
Change-Id: I0668ff5aa798152d17faf3aac1bb8eff8d6350c3
2020-01-15 11:17:57 +00:00
Paul Duffin
593b3c9fb0 Ensure prebuilt modules have same visibility as source modules
Exports visibility and package mutator registration functions so they
can be used in sdk testing. Updates sdk test to support visibility and
package modules.

Adds EffectiveVisibility(...)[]string function to make the effective
visibility rules available to sdk snapshot creation.

Extracts compositeRule.Strings() []string from compositeRule.String()
method so that it can be used by above func.

Adds visibility property to sdk snapshot and prebuilt modules along
with a test to ensure it works properly.

Adds dir parameter to CheckSnapshot so that it can check the snapshot
generated for a non-root package. That is required in order to ensure
that visibility of :__subpackages__ on a source module in package
<pkg> is resolved to an effective visibility of
//<pkg>:__subpackages__ on its corresponding prebuilt.

Test: m conscrypt-module-sdk
Bug: 143678475
Change-Id: Icaacac5b9c04726d28e6fec93e49715ac45df7f4
2019-12-09 13:32:28 +00:00
Paul Duffin
63c6e183d5 Refactor visibility to support visibility on defaults modules
Existing modules, either general one or package ones have a single
visibility property, called visibility in general, and
default_visibility on package, that controls access to that module, or
in the case of package sets the default visibility of all modules in
that package. The property is checked and gathered during the similarly
named phases of visibility processing.

The defaults module will be different as it will have two properties.
The first, visibility, will not affect the visibility of the module, it
only affects the visibility of modules that 'extend' the defaults. So,
it will need checking but not parsing. The second property,
defaults_visibility, will affect the visibility of the module and so
will need both checking and parsing.

The current implementation does not handle those cases because:
1) It does not differentiate between the property that affects the
   module and those that do not. It checks and gathers all of them with
   the last property gathered overriding the rules for the previous
   properties.

2) It relies on overriding methods in MethodBase in order to change the
   default behavior for the package module. That works because
   packageModule embeds ModuleBase but will not work for
   DefaultsModuleBase as it does not embed ModuleBase and instead is
   embedded alongside it so attempting to override a method in
   MethodBase leads to ambiguity.

This change addresses the issues as follows:
1) It adds a new visibility() []string method to get access to the
   primary visibility rules, i.e. the ones that affect the module.

2) It adds two fields, 'visibilityPropertyInfo []visibilityProperty'
   to provide information about all the properties that need checking,
   and 'primaryVisibilityProperty visibilityProperty' to specify the
   property that affects the module.

The PackageFactory() and InitAndroidModule(Module) functions are
modified to initialize the fields. The override of the
visibilityProperties() method for packageModule is removed and the
default implementations of visibilityProperties() and visibility()
on ModuleBase return information from the two new fields.

The InitDefaultsModule is updated to also initialize the two new
fields. It uses nil for primaryVisibilityProperty for now but that
will be changed to return defaults_visibility. It also uses the
commonProperties structure created for the defaults directly instead
of having to search for it through properties().

Changed the visibilityProperty to take a pointer to the property that
can be used to retrieve the value rather than a lambda function.

Bug: 130796911
Test: m nothing
Change-Id: Icadd470a5f692a48ec61de02bf3dfde3e2eea2ef
2019-07-25 11:31:42 +01:00
Paul Duffin
e484f47a63 Inherit default_visibility from parent package
Enhances the visibility mechanism to use the default_visibility
property of the closest ancestor package that has the property
specified.

Bug: 133290645
Test: m droid
Change-Id: I7248e9034a73894ac8d514f913316438c4d7c079
2019-06-27 13:24:59 +01:00
Paul Duffin
e2453c705f Allow default visibility to be set per package
Adds a package module type with a default_visibility property. The
package module type can only be specified once per package.

Bug: 133290645
Test: m droid
Change-Id: Ibb2fb499c9ea88ecaa662d3cd2cbde478e4b9a4b
2019-06-27 13:24:50 +01:00
Martin Stjernholm
226b20dc15 Allow //visibility:public to override other visibility rules.
However only allow it when they are merged from different defaults.

Extend the tests to cover that and other cases with visibilities in
defaults.

Also avoid dumping the whole visibility spec in the error message when a
visibility check fails, because it gets noisy for long visibility lists, and
can be confusing when //visibility:public gets merged with other visibility
rules.

Test: Soong self test
Bug: 112158820
Bug: 130796911
Change-Id: I242513975a3f824b9ea2eab5b94b194b9af2481b
2019-05-24 16:33:50 +01:00
Paul Duffin
2e61fa6e14 Add visibility support
Implementation uploaded for review. Includes unit tests but does not
yet handle prebuilts, that will come in a future change once some
more general issues with prebuilts and namespaces is resolved.

See README.md#Visibility for details of what this does and how to use
it.

Bug: 112158820
Test: add visibility rules for core library modules, make core-tests
Change-Id: I8ec980554398ad6f2d42043ce518f811a35da679
2019-04-26 12:37:37 +01:00