Skip to content

[Issue #5923] Fix resource-related build failure for package containing Objective-C and C sources #5924

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

Merged
merged 7 commits into from
Dec 5, 2022

Conversation

ncooke3
Copy link
Contributor

@ncooke3 ncooke3 commented Nov 22, 2022

Fix issue where adding a resource to a package containing Objective-C and C sources causes a build failure. Note: This only occurs when using swift build via the package manager CLI.

Motivation:

Please see #5923 for the context regarding the issue. The package manager will add the resource_bundle_accessor.h as an input when compiling C sources. This causes a build failure because of the Foundation import in resource_bundle_accessor.h.

This PR fixes this issue.

Modifications:

  • Modify build instructions for Clang targets to not include the -include /path/to/resource_bundle_accessor.h compile argument when compiling a C source.
  • Add unit test testClangBundleAccessor in BuildPlanTests.swift. I wrote this with intention of testing the fix but it doesn't actually test anything because those tests don't actually build anything. I was going to delete it, but noticed that resource bundle generation in Clang targets hasn't been tested so maybe it's a good test to keep?
  • Add unit test testResourcesInMixedClangPackage in TestToolTests.swift to test this fix and potential regressions.
  • Add corresponding fixture target for unit testing this fix.

Result:

This fix enables packages to contain mixed Clang sources (e.g. Objective-C and C) and resources.

Fixes #5923.

@MaxDesiatov
Copy link
Contributor

@swift-ci please smoke test

@MaxDesiatov
Copy link
Contributor

According to build logs, BuildPlanTests.testClangBundleAccessor fails in certain scenarios on both macOS and Linux.

@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 28, 2022

According to build logs, BuildPlanTests.testClangBundleAccessor fails in certain scenarios on both macOS and Linux.

@MaxDesiatov, could you share any relevant info from the build logs? I get a 403 forbidden when trying to see the CI failures myself.

I could just remove the test since it's not directly related to this PR's change, but think it'd be nice to have if I can get it working.

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Nov 28, 2022

@ncooke3 ci.swift.org is up after some downtime, can you see the logs now?

@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 28, 2022

@ncooke3 ci.swift.org is up after some downtime, can you see the logs now?

yep– now I can. Will take a look...

@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 28, 2022

Hi @MaxDesiatov, could you kick off CI again?

@MaxDesiatov
Copy link
Contributor

@swift-ci please smoke test

- The order of the Arrays being compared seems to be non-deterministic. To
  work around this sort them before comparing them.
@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 29, 2022

In one of the tests I added, the order of two arrays I'm comparing seems to be non-deterministic. My latest commit should fix the CI failure by sorting both before comparing. Please kick off another round of CI.

@neonichu
Copy link
Contributor

@swift-ci please smoke test

@neonichu
Copy link
Contributor

@swift-ci please smoke test

@neonichu
Copy link
Contributor

neonichu commented Dec 1, 2022

@swift-ci please smoke test

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.

Package with resources will fail to build when package contains Objective-C and C
3 participants