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

Change Args file format from multiline to shell #61

Closed
wants to merge 2 commits into from

Conversation

keith
Copy link
Member

@keith keith commented Sep 25, 2018

With the shell file format arguments on each line are quoted, which
makes these use cases compatible with files with spaces in their paths.

With the shell file format arguments on each line are quoted, which
makes these use cases compatible with files with spaces in their paths.
@keith
Copy link
Member Author

keith commented Sep 25, 2018

So I actually found a problem with this a little further down the chain. Setting this results in this flag being wrapped in quotes '-DDEBUG=1' which ends up making clang assume this is a file path, resulting in an error because a swift frontend invocation attempts to exec clang only once, but it ends up assuming 2 files need to be handled. I'm not exactly sure what the workaround is here.

@keith
Copy link
Member Author

keith commented Sep 25, 2018

The specific clang command (found using -Xfrontend -dump-clang-diagnostics is:

'clang' '-fsyntax-only' '-fblocks' '-D__swift__=40200' '-fretain-comments-from-system-headers' '-isystem' '/Applications/Xcode-10.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift' '-fPIC' '-fmodules' '-Xclang' '-fmodule-feature' '-Xclang' 'swift' '-Werror=non-modular-include-in-framework-module' '-x' 'objective-c' '-std=gnu11' '-fobjc-arc' '-DSWIFT_CLASS_EXTRA=__attribute__((annotate("__swift native")))' '-DSWIFT_PROTOCOL_EXTRA=__attribute__((annotate("__swift native")))' '-DSWIFT_EXTENSION_EXTRA=__attribute__((annotate("__swift native")))' '-DSWIFT_ENUM_EXTRA=__attribute__((annotate("__swift native")))' '-D_ISO646_H_' '-D__ISO646_H' '-DSWIFT_SDK_OVERLAY_APPKIT_EPOCH=2' '-DSWIFT_SDK_OVERLAY_FOUNDATION_EPOCH=8' '-DSWIFT_SDK_OVERLAY2_SCENEKIT_EPOCH=3' '-DSWIFT_SDK_OVERLAY_GAMEPLAYKIT_EPOCH=1' '-DSWIFT_SDK_OVERLAY_SPRITEKIT_EPOCH=1' '-DSWIFT_SDK_OVERLAY_COREIMAGE_EPOCH=2' '-DSWIFT_SDK_OVERLAY_DISPATCH_EPOCH=2' '-DSWIFT_SDK_OVERLAY_PTHREAD_EPOCH=1' '-DSWIFT_SDK_OVERLAY_COREGRAPHICS_EPOCH=0' '-DSWIFT_SDK_OVERLAY_UIKIT_EPOCH=2' '-D__SWIFT_COMPILER_VERSION=1000037001000' '-isysroot' '/Applications/Xcode-10.0.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator12.0.sdk' '-fmodules-validate-system-headers' '-Xclang' '-fmodule-format=obj' '-fapinotes-modules' '-iapinotes-modules' '/Applications/Xcode-10.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/iphonesimulator/x86_64' '-fapinotes-swift-version=4.2' '-target' 'x86_64-apple-ios10.0' '<swift-imported-modules>' '-resource-dir' '/Applications/Xcode-10.0.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/clang' '-working-directory' '/private/var/tmp/_bazel_ksmiley/0b4aec06d0a277d8a5aa9cd2752bc453/execroot/__main__' '-v' '-iquotebazel-out/darwin-fastbuild/genfiles' '-iquote.' '-O0' ''-DDEBUG=1''

Which results in this error:

<unknown>:0: error: unable to handle compilation, expected exactly one compiler job in
...
<unknown>:0: error: clang importer creation failed

Because both ''-DDEBUG=1'' and <swift-imported-modules> create invocations to clang from the frontend

@keith
Copy link
Member Author

keith commented Sep 25, 2018

I am able to workaround this by setting experimental_objc_fastbuild_options to nothing, since that's where the debug flag is coming from https://github.com/bazelbuild/bazel/blob/d65ed23beb4b97c6b33eb7d625f98da7e0893b58/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommandLineOptions.java#L140-L141

@keith
Copy link
Member Author

keith commented Sep 25, 2018

Thinking more about this, this might be because of how we're forwarding the arguments to python's check_output. Because this only happens when going through our WIP persistent worker. I'll investigate further

@keith
Copy link
Member Author

keith commented Sep 25, 2018

It actually looks like libtool handles spaces in file paths in these files, so quoting it is redundant and invalid. To be safe I think I'll change this to only quote the Swift place as long as this builds otherwise

allevato pushed a commit that referenced this pull request Oct 2, 2018
With the shell file format arguments on each line are quoted, which
makes these use cases compatible with files with spaces in their paths.

Closes #61.

PiperOrigin-RevId: 215446371
@allevato allevato closed this in 93ac29a Oct 2, 2018
@keith keith deleted the ks/shell-quote branch October 2, 2018 20:31
tymurmustafaiev pushed a commit to tymurmustafaiev/rules_swift that referenced this pull request Jul 19, 2023
With the shell file format arguments on each line are quoted, which
makes these use cases compatible with files with spaces in their paths.

Closes bazelbuild#61.

PiperOrigin-RevId: 215446371
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants