-
Notifications
You must be signed in to change notification settings - Fork 140
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
Remove space-in-path replacement #896
base: master
Are you sure you want to change the base?
Conversation
Seems I'll need to find a way to escape some paths in tool invocations, and if not possible give up |
What issues does it cause? |
Unit files have a hash based on the object file path. When we import unit files we need to make the object file path equal to what it would be if Xcode generates them. Xcode will generate these paths with spaces in their name, while rules_swift will replace the spaces. So during an import we would have to undo this, but with index-import (which is just regex) we can't. Even if we could, theoretically there are some paths out there with |
And after saying all of that, I think I'll add something to index-import to address this for the time being. |
MobileNativeFoundation/index-import#65 We can merge this PR once we drop Bazel 5.x support. |
This replacement causes issues with importing indexstores. In 965c373 this replacement was added to work around issues with `ar_wrapper`. I don't see `ar_wrapper` used anywhere in Bazel, so I believe this is safe to remove.
a7a975a
to
6755b56
Compare
Looks like there are new failures. I'll see if I'm able to address them. |
So clang (at least on linux) requires quotes for spaces in the params file. Seeing if I can influence that, or if it will require a Bazel patch. |
It looks like enabling the |
This replacement causes issues with importing indexstores.
In 965c373 this replacement was added to work around issues with
ar_wrapper
. I don't seear_wrapper
used anywhere in Bazel, so I believe this is safe to remove.