Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SNAP FORK][UPSTREAMED] Implement namespaced R class #66

Open
wants to merge 1 commit into
base: pre-alpha
Choose a base branch
from

Conversation

mauriciogg
Copy link
Contributor

@mauriciogg mauriciogg commented Jan 18, 2023

The native version of the rules have this optimization (in our internal fork).

native version uptreamed: bazelbuild/bazel#11385
Enable rudimentary R class namespacing where each library only contains
references to the resources it declares instead of declarations plus all
transitive dependency references.

  • No resource merging for android_library targets
  • Smaller R.{class,txt}
  • Less desugaring + dexing

i think this addresses #10

The native version of the rules have this optimization (in our internal
fork). This is that implementationm, but in starlark.

See https://github.sc-corp.net/Snapchat/bazel/pull/126 for more details
args.add("--androidJar", android_jar)
optional_inputs.append(android_jar)

_disable_warnings(args)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

disable_warnings() doesn't seem to exist on my end. Where is this function from?

@@ -641,14 +645,33 @@ def _compile(
)
args.add("--output", out_file)

optional_outputs = []
if out_class_jar:
args.add("--classJarOutput", out_class_jar)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is throwing the following error on internal tests:

Error parsing command line: Unrecognized option: --classJarOutput

Which is kind of surprising. I'll look into this a bit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need something like this, I think:

    optional_outputs = []
    optional_args = []
    if out_class_jar:
        optional_args.add("--classJarOutput", out_class_jar)
        optional_outputs.append(out_class_jar)
    if out_r_txt:
        optional_args.add("--rTxtOut", out_r_txt)
        optional_outputs.append(out_r_txt)

    if len(optional_outputs) > 1:
        args.extend(["--"], optional_args)

@ted-xie
Copy link
Contributor

ted-xie commented Mar 21, 2023

In addition to changes above, would you mind rebasing this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants