Skip to content

[Issue #4500] Fix resource bundle accessor issue when testing with the CLI #5921

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Nov 29, 2022
Merged

[Issue #4500] Fix resource bundle accessor issue when testing with the CLI #5921

merged 3 commits into from
Nov 29, 2022

Conversation

ncooke3
Copy link
Contributor

@ncooke3 ncooke3 commented Nov 22, 2022

Fix issue where resource bundle accessor code returns nil when testing a package with the package manager CLI.

Motivation:

Please see #4500 for the context regarding the issue. In short, when testing a package's access to the resource bundle, the resource bundle will be nil when being accessed in an Objective-C context. This is because there is a difference in logic between the resource_bundle_accessor.m used in an Objective-C context and the resource_accessor.swift used in a Swift context.

As seen below, the Swift implementation will try to access the main bundle but fall back to the bundle at the build path (line 834 in below snippet). The Objective-C implementation does not have this code path and therefore returns a nil bundle when the main bundle does not exist. This is the case when testing a package using the CLI.

resource_bundle_accessor.swift

https://github.com/apple/swift-package-manager/blob/87254d26fa4287e45b20c5eba8bd71386942a880/Sources/Build/BuildPlan.swift#L811-L841

resource_bundle_accessor.m

https://github.com/apple/swift-package-manager/blob/87254d26fa4287e45b20c5eba8bd71386942a880/Sources/Build/BuildPlan.swift#L502-L507

Reproducing the issue

The issue can be reproduced by running swift test on a package that expects a non-nil resource bundle in Objective-C tests. I have also created a minimum, reproducible example: https://github.com/ncooke3/Package4500.

This change enables package developers to successfully test packages using the package manager CLI.

Modifications:

Modified the resource_bundle_accessor.m implementation to fall back to the bundle in the build folder when the main bundle does not exist.

Result:

Accessing the module's resource bundle will work when using the package manager CLI.

Fixes #4500

@ncooke3
Fix resource bundle accessor issue for Clang sources
842d33f 
- I noticed an issue when trying to access a Clang package's resources
  when testing the target from the SwiftPM CLI. In short, the resource
  bundle is placed in the same place when building a Clang-only or
  Swift-only package, but the generated resource_bundle_accessor.m's logic
  tries to create an NSBundle for a path that DNE. The difference becomes
  clear when comparing the logic to the resource_bundle_accessor.swift
  that is generated from Swift-only packages with resources.
ncooke3
ncooke3 commented Nov 22, 2022
View reviewed changes
Sources/Build/BuildPlan.swift
Comment on lines +506 to +512

NSBundle *preferredBundle = [NSBundle bundleWithURL:bundleURL];
if (preferredBundle == nil) {
return [NSBundle bundleWithPath:@"\(bundlePath.pathString)"];
}

return preferredBundle;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change looks to be picked up as a public API change. It is breaking the following test:
https://github.com/apple/swift-package-manager/blob/87254d26fa4287e45b20c5eba8bd71386942a880/Tests/CommandsTests/APIDiffTests.swift#L61

Copy link
Contributor Author

@ncooke3 ncooke3 Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This commit is also part of #5919, I pulled it out into this PR since it isn't directly related to mixed language support and was as being tracked by #4500.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, seems pretty odd that this would affect testInvokeAPIDiffDigester since the packaged being tested there doesn't appear to contain resources. Could you share the full test output?

Copy link
Contributor Author

@ncooke3 ncooke3 Nov 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah– I'm not sure what is going on locally. And it did not seem to affect CI (search for testInvokeAPIDiffDigester in the CI logs)

https://github.com/apple/swift-package-manager/blob/87254d26fa4287e45b20c5eba8bd71386942a880/Tests/CommandsTests/APIDiffTests.swift#L61-L74

I printed the error on line 70:

Here's what printed 
▿ CommandExecutionError
  ▿ result : <ProcessResult: exit: terminated(code: 1), output:
 
>
    ▿ arguments : 5 elements
      - 0 : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug/swift-package"
      - 1 : "--package-path"
      - 2 : "/var/folders/hz/7v68p4h95sb2ypb614xp20jc00r7sc/T/Miscellaneous_APIDiff.WZVA19/Foo"
      - 3 : "diagnose-api-breaking-changes"
      - 4 : "1.2.3"
    ▿ environment : 38 elements
      ▿ 0 : 2 elements
        - key : "SHELL"
        - value : "/bin/zsh"
      ▿ 1 : 2 elements
        - key : "SWIFTUI_VIEW_DEBUG"
        - value : "287"
      ▿ 2 : 2 elements
        - key : "SECURITYSESSIONID"
        - value : "186a5"
      ▿ 3 : 2 elements
        - key : "LaunchInstanceID"
        - value : "78B6CBF1-5859-48F5-8434-ABD520D1A5F1"
      ▿ 4 : 2 elements
        - key : "CA_DEBUG_TRANSACTIONS"
        - value : "0"
      ▿ 5 : 2 elements
        - key : "DYLD_FRAMEWORK_PATH"
        - value : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug:/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug/PackageFrameworks"
      ▿ 6 : 2 elements
        - key : "XPC_SERVICE_NAME"
        - value : "application.com.apple.dt.Xcode.157115435.157699474"
      ▿ 7 : 2 elements
        - key : "__CF_USER_TEXT_ENCODING"
        - value : "0xC1F2C:0x0:0x0"
      ▿ 8 : 2 elements
        - key : "DYLD_LIBRARY_PATH"
        - value : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug"
      ▿ 9 : 2 elements
        - key : "SQLITE_EXEMPT_PATH_FROM_VNODE_GUARDS"
        - value : "/Users/nickcooke/Library/WebKit/Databases"
      ▿ 10 : 2 elements
        - key : "UsePerConfigurationBuildLocations"
        - value : "YES"
      ▿ 11 : 2 elements
        - key : "LD_LIBRARY_PATH"
        - value : "/Applications/Xcode_13.3.1.app/Contents/Developer/../SharedFrameworks/"
      ▿ 12 : 2 elements
        - key : "__XPC_DYLD_LIBRARY_PATH"
        - value : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug"
      ▿ 13 : 2 elements
        - key : "PATH"
        - value : "/Applications/Xcode_13.3.1.app/Contents/Developer/usr/bin:/usr/bin:/bin:/usr/sbin:/sbin"
      ▿ 14 : 2 elements
        - key : "SWIFTPM_TESTS_PACKAGECACHE"
        - value : "1"
      ▿ 15 : 2 elements
        - key : "DYLD_FALLBACK_LIBRARY_PATH"
        - value : "/Applications/Xcode_13.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/usr/lib"
      ▿ 16 : 2 elements
        - key : "LOG4J_FORMAT_MSG_NO_LOOKUPS"
        - value : "true"
      ▿ 17 : 2 elements
        - key : "__XCODE_BUILT_PRODUCTS_DIR_PATHS"
        - value : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug"
      ▿ 18 : 2 elements
        - key : "SWIFTPM_TESTS_MODULECACHE"
        - value : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug"
      ▿ 19 : 2 elements
        - key : "LOGNAME"
        - value : "nickcooke"
      ▿ 20 : 2 elements
        - key : "SSH_AUTH_SOCK"
        - value : "/private/tmp/com.apple.launchd.OIddYYmAGx/Listeners"
      ▿ 21 : 2 elements
        - key : "CA_ASSERT_MAIN_THREAD_TRANSACTIONS"
        - value : "0"
      ▿ 22 : 2 elements
        - key : "MallocNanoZone"
        - value : "0"
      ▿ 23 : 2 elements
        - key : "XPC_FLAGS"
        - value : "0x0"
      ▿ 24 : 2 elements
        - key : "__CFBundleIdentifier"
        - value : "com.apple.dt.Xcode"
      ▿ 25 : 2 elements
        - key : "RUN_DESTINATION_DEVICE_PLATFORM_IDENTIFIER"
        - value : "com.apple.platform.macosx"
      ▿ 26 : 2 elements
        - key : "TMPDIR"
        - value : "/var/folders/hz/7v68p4h95sb2ypb614xp20jc00r7sc/T/"
      ▿ 27 : 2 elements
        - key : "COMMAND_MODE"
        - value : "unix2003"
      ▿ 28 : 2 elements
        - key : "USER"
        - value : "nickcooke"
      ▿ 29 : 2 elements
        - key : "SQLITE_ENABLE_THREAD_ASSERTIONS"
        - value : "1"
      ▿ 30 : 2 elements
        - key : "DYLD_FALLBACK_FRAMEWORK_PATH"
        - value : "/Applications/Xcode_13.3.1.app/Contents/SharedFrameworks:/Applications/Xcode_13.3.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Frameworks"
      ▿ 31 : 2 elements
        - key : "RUN_DESTINATION_DEVICE_NAME"
        - value : "My Mac"
      ▿ 32 : 2 elements
        - key : "TOOLCHAINS"
        - value : "/Applications/Xcode_13.3.1.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/swift"
      ▿ 33 : 2 elements
        - key : "OS_ACTIVITY_DT_MODE"
        - value : "YES"
      ▿ 34 : 2 elements
        - key : "PWD"
        - value : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug"
      ▿ 35 : 2 elements
        - key : "RUN_DESTINATION_DEVICE_UDID"
        - value : "33DF4E41-7255-58F9-A7A5-0DECD3298492"
      ▿ 36 : 2 elements
        - key : "HOME"
        - value : "/Users/nickcooke"
      ▿ 37 : 2 elements
        - key : "__XPC_DYLD_FRAMEWORK_PATH"
        - value : "/Users/nickcooke/Library/Developer/Xcode/DerivedData/swift-package-manager-fnxfxyeqhuuelohbuxseapweetoa/Build/Products/Debug"
    ▿ exitStatus : ExitStatus
      ▿ terminated : 1 element
        - code : 1
    ▿ output : Result<Array<UInt8>, Error>
      - success : 0 elements
    ▿ stderrOutput : Result<Array<UInt8>, Error>
      ▿ success : 381 elements
        - 0 : 66
        - 1 : 117
        - 2 : 105
        - 3 : 108
        - 4 : 100
        - 5 : 105
        - 6 : 110
        - 7 : 103
        - 8 : 32
        - 9 : 102
        - 10 : 111
        - 11 : 114
        - 12 : 32
        - 13 : 100
        - 14 : 101
        - 15 : 98
        - 16 : 117
        - 17 : 103
        - 18 : 103
        - 19 : 105
        - 20 : 110
        - 21 : 103
        - 22 : 46
        - 23 : 46
        - 24 : 46
        - 25 : 10
        - 26 : 50
        - 27 : 48
        - 28 : 50
        - 29 : 50
        - 30 : 45
        - 31 : 49
        - 32 : 49
        - 33 : 45
        - 34 : 50
        - 35 : 57
        - 36 : 32
        - 37 : 49
        - 38 : 53
        - 39 : 58
        - 40 : 50
        - 41 : 51
        - 42 : 58
        - 43 : 51
        - 44 : 49
        - 45 : 46
        - 46 : 54
        - 47 : 56
        - 48 : 49
        - 49 : 50
        - 50 : 57
        - 51 : 48
        - 52 : 45
        - 53 : 48
        - 54 : 53
        - 55 : 48
        - 56 : 48
        - 57 : 32
        - 58 : 115
        - 59 : 119
        - 60 : 105
        - 61 : 102
        - 62 : 116
        - 63 : 45
        - 64 : 112
        - 65 : 97
        - 66 : 99
        - 67 : 107
        - 68 : 97
        - 69 : 103
        - 70 : 101
        - 71 : 91
        - 72 : 53
        - 73 : 55
        - 74 : 52
        - 75 : 50
        - 76 : 51
        - 77 : 58
        - 78 : 53
        - 79 : 50
        - 80 : 48
        - 81 : 52
        - 82 : 56
        - 83 : 50
        - 84 : 93
        - 85 : 32
        - 86 : 91
        - 87 : 108
        - 88 : 111
        - 89 : 103
        - 90 : 103
        - 91 : 105
        - 92 : 110
        - 93 : 103
        - 94 : 93
        - 95 : 32
        - 96 : 109
        - 97 : 105
        - 98 : 115
        - 99 : 117
        - 100 : 115
        - 101 : 101
        - 102 : 32
        - 103 : 97
        - 104 : 116
        - 105 : 32
        - 106 : 108
        - 107 : 105
        - 108 : 110
        - 109 : 101
        - 110 : 32
        - 111 : 49
        - 112 : 55
        - 113 : 52
        - 114 : 56
        - 115 : 50
        - 116 : 48
        - 117 : 32
        - 118 : 111
        - 119 : 102
        - 120 : 32
        - 121 : 91
        - 122 : 57
        - 123 : 102
        - 124 : 102
        - 125 : 50
        - 126 : 52
        - 127 : 52
        - 128 : 99
        - 129 : 101
        - 130 : 48
        - 131 : 55
        - 132 : 93
        - 133 : 10
        - 134 : 91
        - 135 : 49
        - 136 : 47
        - 137 : 50
        - 138 : 93
        - 139 : 32
        - 140 : 67
        - 141 : 111
        - 142 : 109
        - 143 : 112
        - 144 : 105
        - 145 : 108
        - 146 : 105
        - 147 : 110
        - 148 : 103
        - 149 : 32
        - 150 : 70
        - 151 : 111
        - 152 : 111
        - 153 : 32
        - 154 : 70
        - 155 : 111
        - 156 : 111
        - 157 : 46
        - 158 : 115
        - 159 : 119
        - 160 : 105
        - 161 : 102
        - 162 : 116
        - 163 : 10
        - 164 : 91
        - 165 : 50
        - 166 : 47
        - 167 : 50
        - 168 : 93
        - 169 : 32
        - 170 : 69
        - 171 : 109
        - 172 : 105
        - 173 : 116
        - 174 : 116
        - 175 : 105
        - 176 : 110
        - 177 : 103
        - 178 : 32
        - 179 : 109
        - 180 : 111
        - 181 : 100
        - 182 : 117
        - 183 : 108
        - 184 : 101
        - 185 : 32
        - 186 : 70
        - 187 : 111
        - 188 : 111
        - 189 : 10
        - 190 : 66
        - 191 : 117
        - 192 : 105
        - 193 : 108
        - 194 : 100
        - 195 : 32
        - 196 : 99
        - 197 : 111
        - 198 : 109
        - 199 : 112
        - 200 : 108
        - 201 : 101
        - 202 : 116
        - 203 : 101
        - 204 : 33
        - 205 : 32
        - 206 : 40
        - 207 : 49
        - 208 : 46
        - 209 : 48
        - 210 : 49
        - 211 : 115
        - 212 : 41
        - 213 : 10
        - 214 : 66
        - 215 : 117
        - 216 : 105
        - 217 : 108
        - 218 : 100
        - 219 : 105
        - 220 : 110
        - 221 : 103
        - 222 : 32
        - 223 : 102
        - 224 : 111
        - 225 : 114
        - 226 : 32
        - 227 : 100
        - 228 : 101
        - 229 : 98
        - 230 : 117
        - 231 : 103
        - 232 : 103
        - 233 : 105
        - 234 : 110
        - 235 : 103
        - 236 : 46
        - 237 : 46
        - 238 : 46
        - 239 : 10
        - 240 : 91
        - 241 : 49
        - 242 : 47
        - 243 : 50
        - 244 : 93
        - 245 : 32
        - 246 : 67
        - 247 : 111
        - 248 : 109
        - 249 : 112
        - 250 : 105
        - 251 : 108
        - 252 : 105
        - 253 : 110
        - 254 : 103
        - 255 : 32
        - 256 : 70
        - 257 : 111
        - 258 : 111
        - 259 : 32
        - 260 : 70
        - 261 : 111
        - 262 : 111
        - 263 : 46
        - 264 : 115
        - 265 : 119
        - 266 : 105
        - 267 : 102
        - 268 : 116
        - 269 : 10
        - 270 : 91
        - 271 : 50
        - 272 : 47
        - 273 : 50
        - 274 : 93
        - 275 : 32
        - 276 : 69
        - 277 : 109
        - 278 : 105
        - 279 : 116
        - 280 : 116
        - 281 : 105
        - 282 : 110
        - 283 : 103
        - 284 : 32
        - 285 : 109
        - 286 : 111
        - 287 : 100
        - 288 : 117
        - 289 : 108
        - 290 : 101
        - 291 : 32
        - 292 : 70
        - 293 : 111
        - 294 : 111
        - 295 : 10
        - 296 : 66
        - 297 : 117
        - 298 : 105
        - 299 : 108
        - 300 : 100
        - 301 : 32
        - 302 : 99
        - 303 : 111
        - 304 : 109
        - 305 : 112
        - 306 : 108
        - 307 : 101
        - 308 : 116
        - 309 : 101
        - 310 : 33
        - 311 : 32
        - 312 : 40
        - 313 : 49
        - 314 : 46
        - 315 : 48
        - 316 : 49
        - 317 : 115
        - 318 : 41
        - 319 : 10
        - 320 : 101
        - 321 : 114
        - 322 : 114
        - 323 : 111
        - 324 : 114
        - 325 : 58
        - 326 : 32
        - 327 : 102
        - 328 : 97
        - 329 : 105
        - 330 : 108
        - 331 : 101
        - 332 : 100
        - 333 : 32
        - 334 : 116
        - 335 : 111
        - 336 : 32
        - 337 : 118
        - 338 : 97
        - 339 : 108
        - 340 : 105
        - 341 : 100
        - 342 : 97
        - 343 : 116
        - 344 : 101
        - 345 : 32
        - 346 : 98
        - 347 : 97
        - 348 : 115
        - 349 : 101
        - 350 : 108
        - 351 : 105
        - 352 : 110
        - 353 : 101
        - 354 : 32
        - 355 : 102
        - 356 : 111
        - 357 : 114
        - 358 : 32
        - 359 : 70
        - 360 : 111
        - 361 : 111
        - 362 : 10
        - 363 : 101
        - 364 : 114
        - 365 : 114
        - 366 : 111
        - 367 : 114
        - 368 : 58
        - 369 : 32
        - 370 : 102
        - 371 : 97
        - 372 : 116
        - 373 : 97
        - 374 : 108
        - 375 : 69
        - 376 : 114
        - 377 : 114
        - 378 : 111
        - 379 : 114
        - 380 : 10
  - stdout : ""
  - stderr : "Building for debugging...\n2022-11-29 15:23:31.681290-0500 swift-package[57423:520482] [logging] misuse at line 174820 of [9ff244ce07]\n[1/2] Compiling Foo Foo.swift\n[2/2] Emitting module Foo\nBuild complete! (1.01s)\nBuilding for debugging...\n[1/2] Compiling Foo Foo.swift\n[2/2] Emitting module Foo\nBuild complete! (1.01s)\nerror: failed to validate baseline for Foo\nerror: fatalError\n"

Could be some local state causing the failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a false flag, I dropped a breakpoint in the code I changed in this file and it never hit.

@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 22, 2022

@swift-ci please smoke test

@ncooke3 ncooke3 mentioned this pull request Nov 22, 2022
Closed
MaxDesiatov
MaxDesiatov reviewed Nov 22, 2022
View reviewed changes
Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Would it be possible to add a test for this to verify that the fix is working and to prevent regressions? Thanks!

@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 22, 2022

Would it be possible to add a test for this to verify that the fix is working and to prevent regressions? Thanks!

Thanks for taking a look @MaxDesiatov. I've added that unit test and verified that it fails without the fix, and passes with the fix.

@MaxDesiatov
Copy link
Contributor

@swift-ci please smoke test

neonichu
neonichu reviewed Nov 28, 2022
View reviewed changes
Tests/CommandsTests/TestToolTests.swift Outdated

// TODO: This test should be moved into `ResourceTests.swift` in the
// `FunctionalTests` scheme when the `FunctionalTests` scheme is re-enabled.
func testResourceBundleInClangPackageWhenRunningSwiftTest() throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can move it as part of this PR, otherwise it can cause the same type of issue that brought us to disable FunctionalTests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in fadd22c

@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 28, 2022

@swift-ci please smoke test

@ncooke3
Copy link
Contributor Author

ncooke3 commented Nov 28, 2022

@MaxDesiatov, I'm not sure if the CI bot listens to me. Can you tell if CI is running and please trigger it if it's not?

@neonichu
Copy link
Contributor

@swift-ci please smoke test

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Nov 28, 2022 •
edited
Loading

I'm not sure if the CI bot listens to me.

It does not, only committers can trigger CI builds.

@tomerd tomerd merged commit 1d23aab into swiftlang:main Nov 29, 2022
@tomerd
Copy link
Contributor

tomerd commented Nov 29, 2022

thanks @ncooke3

@ncooke3 ncooke3 deleted the nc/issue-4500 branch November 30, 2022 15:51
@ncooke3 ncooke3 mentioned this pull request Nov 30, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxDesiatov MaxDesiatov MaxDesiatov left review comments

@neonichu neonichu neonichu approved these changes

@abertelrud abertelrud Awaiting requested review from abertelrud

@tomerd tomerd Awaiting requested review from tomerd

@elsh elsh Awaiting requested review from elsh

Assignees

@neonichu neonichu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[SR-13560] swiftpm: SWIFTPM_MODULE_BUNDLE returning nil
4 participants
@ncooke3 @MaxDesiatov @neonichu @tomerd