bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.21k stars 4.06k forks source link

objc_import missing data argument #17018

Open xiemotongye opened 1 year ago

xiemotongye commented 1 year ago

Description of the feature request:

Hi wonderful Bazel folks,

This is a feature request.

I'd noticed that objc_import--unlike most other cc rules(cc_import, cc_library, objc_library)--doesn't allow you to specify data. I think objc_import should be equivalent to objc_library. It's so weird that objc_import does not support the data argument.

What underlying problem are you trying to solve with this feature?

I'm using a .a file with some resources. When I specify data to objc_import, bazel told me: no such attribute 'data' in 'objc_import' rule.

Which operating system are you running Bazel on?

macOS 12.6

What is the output of bazel info release?

release 6.0.0rc4

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

git@github.com:bazelbuild/bazel.git
89eba259031ff29136fdb59351adc1e754004672
7ccc66108f08f7b6c6f6e5229f70f29962ea19ce

Have you found anything relevant by searching the web?

Yes. Just found objc_import missing deps argument. This issue only mentioned deps arguments, but no mention for data arguments.

Any other information, logs, or outputs that you want to share?

No response

cpsauer commented 1 year ago

+1 on the utility of this

[As food for thought: Might it just make sense to mostly unify these rules?]

cpsauer commented 10 months ago

(objc_import is starlarkified, right? Perhaps that makes this a good time... I started to look into, but wasn't sure how the data attrs were getting gathered up into providers in the cc_import equivalent--perhaps they're traversed with an aspect?--nor what the SKIP_CONSTRAINTS_OVERRIDE was about. If anyone knows--or knows who would know--please say.)

keith commented 10 months ago

As far as how data is used in apple targets that is from an aspect in rules_apple, so I would try adding the same attr as objc_library (this one https://github.com/bazelbuild/bazel/blob/0acc81724dc16e66bab946422ca2f95b6511e252/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl#L124) and seeing if that "just works" the only place I can see objc_library using that directly is for this https://github.com/bazelbuild/bazel/blob/0acc81724dc16e66bab946422ca2f95b6511e252/src/main/starlark/builtins_bzl/common/objc/objc_library.bzl#L94 which objc_import doesn't have

cpsauer commented 10 months ago

Thanks Keith!

Gave that a quick whirl, but the data don't get picked up and bundled as they would with objc_library. Any ideas?

(Figured I had comparative advantage if it were easy, given a real world test case, and already knowing how to use --experimental_builtins_bzl_path="%workspace%", but I think you (and others) definitely have advantage wrt understanding the gather-up process.)

keith commented 10 months ago

Ah yea need to add it here too https://github.com/bazelbuild/rules_apple/blob/d5d6d58af8b96365e9b9a9d093d4fb4b2f4dd912/apple/internal/aspects/resource_aspect.bzl#L134-L135

cpsauer commented 10 months ago

Got it. Looks like resources.collect no-ops gracefully if the attrdoesn't exist--so that change is our first move.

Should I be concerned also about, e.g., cc_binaries failing to collect the data? (I was surprised to see these not getting passed up e.g. via some cc provider.)

cpsauer commented 10 months ago

It does still does seem like objc_import is so very close to being able to just be implemented as a facade around objc_library. objc_library would just need to be able to take .a files, like cc_library.

cpsauer commented 10 months ago

Prerequisite change merged into rules_apple today.

Added the core change in https://github.com/bazelbuild/bazel/pull/20716 (have ofc verified that they work together.)