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

objc_import missing deps argument #10739

Closed
cpsauer opened this issue Feb 9, 2020 · 12 comments
Closed

objc_import missing deps argument #10739

cpsauer opened this issue Feb 9, 2020 · 12 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@cpsauer
Copy link
Contributor

cpsauer commented Feb 9, 2020

Hi wonderful Bazel folks,

This is a feature request, but hopefully a fairly straightforward one.

I'd noticed that objc_import--unlike most other rules--doesn't allow you to specify deps. But I really think it's worth allowing it to do so. It's not uncommon for .a files to have dependencies, and I think it's a good thing in terms of promoting modularity and reuse.

Consider, for example, a closed-source library that depends on protobuf. Better to allow the library to easily depend on a shared protobuf that might be used elsewhere in the project, rather than encourage all dependencies to already be in the archive.

Thanks for your consideration,
Chris
(ex-Googler)

@cpsauer
Copy link
Contributor Author

cpsauer commented Feb 9, 2020

(Workarounds to this are somewhat complicated by #10738)

@dslomov dslomov added team-Rules-CPP Issues for C++ rules untriaged labels Feb 10, 2020
@oquenchil oquenchil added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Feb 18, 2020
@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) type: feature request and removed P3 We're not considering working on this, but happy to review a PR. (No assignee) labels Nov 19, 2020
@cpsauer
Copy link
Contributor Author

cpsauer commented Apr 16, 2021

From the docs, this seems to have been fixed in master, though not in a yet released version of Bazel.
(Not in 4.0, unsure if in 4.1)

@cpsauer
Copy link
Contributor Author

cpsauer commented May 22, 2021

Whoops! The fix seems to have been reverted! For a brief, glorious moment, it looked like objc_import was on track to have the full complement of objc attributes, like deps and data, so I closed things, but it seems like I jumped the gun.

I'm going to reopen this and tag @googlewalt, since it looks like he's the one doing good work in there and making changes. (Thanks for all the improvements, @googlewalt! They're much appreciated.)

@cpsauer cpsauer reopened this May 22, 2021
@cpsauer
Copy link
Contributor Author

cpsauer commented May 22, 2021

FWIW, if objc_library allowed archives (.a) in srcs, like its cc_library equivalent, I think that'd solve the problem, too. That'd make objc_library general enough to be used to either build or import an objc_library!

@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label May 10, 2023
@cpsauer
Copy link
Contributor Author

cpsauer commented May 11, 2023

@bazelbuild/triage, this is still quite relevant and would be quite useful, IMO.

@github-actions github-actions bot removed the stale Issues or PRs that are stale (no activity for 30 days) label May 11, 2023
@googlewalt
Copy link
Contributor

This should be fixed in 0f19ef3, though I see that the documentation was not updated.

@cpsauer
Copy link
Contributor Author

cpsauer commented May 11, 2023

(@bazelbuild/triage doesn't seem to create a tag (no link) so I'm going to tag @sgowroji and @Pavank1992 manually. Please coach me if you'd have preferred otherwise--and maybe update the bot's instructions)

@cpsauer
Copy link
Contributor Author

cpsauer commented May 11, 2023

Oh, amazing. Thanks @googlewalt! Will test--and leave this open for now to track docs?

@cpsauer
Copy link
Contributor Author

cpsauer commented May 11, 2023

Update: Have confirmed deps has been added as an attribute and seems to work in a quick test.

[Sorry that I'd wrongly assumed the docs would be auto-synced.]

Still think it makes sense to leave this open to track the docs.

[The original use case I'd planned to use this for--using a google library from bazel--is sadly also blocked by https://github.com//issues/17018]

@googlewalt
Copy link
Contributor

I'll fix the documentation.

@cpsauer
Copy link
Contributor Author

cpsauer commented May 11, 2023

Thanks so much @googlewalt! Really appreciate all you do--including knocking this out.

fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
Closes bazelbuild#10739.

PiperOrigin-RevId: 531213244
Change-Id: I0d9ae8733164494995219d6b3d2537ea212cede8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

4 participants