search

cgrindel / rules_swift_package_manager

Collection of utilities and Bazel rules to aid in the development and maintenance of Swift repositories using Bazel.
Apache License 2.0
78 stars 31 forks source link

feat: add string_literal to module identifier allow list #1319

Closed lucasmarcal-faire closed 3 weeks ago

lucasmarcal-faire commented 3 weeks ago

Add string_literal to module identifier allow list. This case seems to be pretty rare, and besides the fact that it's not mentioned on ModuleMap specs, Clang actually accepts that.

It's rare, but you can find examples by searching for module " path:/(^|\/)*\.modulemap$/ on Github

The motivation behind creating this PR is to add support to the swift-llbuild Package build that fails while resolving this modulemap

This PR alone doesn't fix swift-llbuild build. Some follow-up PRs will be created to add SPM include folder handling

cgrindel commented 3 weeks ago

I converted this to a draft while you are still working on it. When you would like a review, feel free to transition it out of the draft state.

cgrindel commented 3 weeks ago

@mergifyio queue

mergify[bot] commented 3 weeks ago

queue

🟠 Waiting for conditions to match

- [ ] any of: [🔀 queue conditions] - [ ] all of: [📌 queue conditions of queue `default`] - [ ] any of: [🛡 GitHub branch protection] - [ ] `check-neutral = all_ci_tests` - [ ] `check-skipped = all_ci_tests` - [ ] `check-success = all_ci_tests` - [ ] any of: [🛡 GitHub branch protection] - [ ] `check-neutral = conventional_commit_check` - [ ] `check-skipped = conventional_commit_check` - [ ] `check-success = conventional_commit_check` - [X] `#approved-reviews-by >= 1` [🛡 GitHub branch protection] - [X] `#changes-requested-reviews-by = 0` [🛡 GitHub branch protection] - [X] `#review-threads-unresolved = 0` [🛡 GitHub branch protection] - [X] `-closed` [📌 queue requirement] - [X] `-conflict` [📌 queue requirement] - [X] `-draft` [📌 queue requirement] - [X] any of: [📌 queue -> configuration change requirements] - [X] `-mergify-configuration-changed` - [ ] `check-success = Configuration changed`
lucasmarcal-faire commented 3 weeks ago

I guess I have to fix some tests, let me push a commit with that

lucasmarcal-faire commented 3 weeks ago

@cgrindel Failing tests are now fixed.

cgrindel commented 3 weeks ago

It looks like you need to run bazel run //:tidy.

lucasmarcal-faire commented 3 weeks ago

@cgrindel done!

cgrindel commented 3 weeks ago

@mergifyio queue

mergify[bot] commented 3 weeks ago

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at *8d4e7134bc13c0b857d83b04c933c93e79809313*