Skip to content
This repository has been archived by the owner on Apr 3, 2023. It is now read-only.

Add support for setting defines of declared swift_library #171

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

gferon
Copy link

@gferon gferon commented Sep 14, 2022

While trying to use Auth0.swift I realized it failed to compile because WEB_AUTH_PLATFORM wasn't defined.

This patch adds support to adding the output of manifest.settings.kind["define"] to the defines of the generated swift_library (I use spm_repositories in bazel mode).

EDIT: I haven't added support to the spm_swift_library counterpart, and I could give it a try if you'd like me to. this is obviously not necessary because spm handles this properly.

@cgrindel
Copy link
Owner

Thanks for the contribution! What do you think would be a good test for this functionality?

@gferon
Copy link
Author

gferon commented Sep 14, 2022

Good point. I guess I could expand the JSON from https://github.com/cgrindel/rules_spm/blob/main/test/json_test_data.bzl or introduce another one and check that the final target compiles with the right defines?

@cgrindel
Copy link
Owner

Is it possible to add Auth0.swift to the interesting_deps example where it uses a define?

@gferon
Copy link
Author

gferon commented Sep 14, 2022

Is it possible to add Auth0.swift to the interesting_deps example where it uses a define?

I just did this, it should be good enough as a test (the extra example wouldn't build without the extra defines).

Copy link
Owner

@cgrindel cgrindel left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks!

@cgrindel
Copy link
Owner

To fix the failing CI tests, run bazel run //:update_all, then commit and push any changes that have been made.

@gferon
Copy link
Author

gferon commented Sep 19, 2022

@cgrindel it should be fine now!

@cgrindel
Copy link
Owner

@gferon I just used GitHub to update your PR with the AsyncMain fix. I hope that this will address the failing CI job.

@cgrindel cgrindel enabled auto-merge (squash) September 20, 2022 15:27
@gferon
Copy link
Author

gferon commented Oct 31, 2022

@cgrindel I'm not exactly sure which steps I need to take to get CI to pass, could you point me in the right direction? Thanks!

@@ -38,6 +38,7 @@ load("@cgrindel_rules_spm//spm:defs.bzl", "spm_pkg", "spm_repositories")

spm_repositories(
name = "swift_pkgs",
build_mode = "bazel",
Copy link
Owner

Choose a reason for hiding this comment

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

Unfortunately, bazel build mode is not quite ready for primetime. Try removing this. Then, we can chase down any CI issues.

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

Successfully merging this pull request may close these issues.

2 participants