Fix directory matching if string doesn’t start with bazel-out/ or external/ #639
Conversation
BalestraPatrick
force-pushed
the
pb/replace-linker-inputs-paths
branch
from
3dde58e
to
d3c6077
Compare
| opt = "$(BUILD_DIR)/" + opt | ||
| elif opt.startswith("external/"): | ||
| opt = "$(LINKS_DIR)/" + opt | ||
| if "bazel-out/" in opt: |
There was a problem hiding this comment.
We need this to be startswith to not over replace. Instead we need to split the flag better so we know what the start of the path is.
There was a problem hiding this comment.
Would splitting on = be sufficient? Or what else were you thinking?
There was a problem hiding this comment.
I believe so. Basically match the logic in opts.bzl more completely.
There was a problem hiding this comment.
So for example -Wl,-add_ast_path,bazel-out/darwin_x86_64-dbg-ST-5534cb307cb8/ would still be handled as before, but for flags that don't use -Wl but are instead passed directly as --ld-path=external/..., we would handle them differently? Not sure what other cases there could be.
There was a problem hiding this comment.
I think that's the only two cases we have to worry about.
|
Can you add an example for this as well? |
|
Looking into adding an example with |
| @@ -366,6 +366,13 @@ def _process_linkopt(linkopt, *, triple): | |||
| if opt.endswith(".swiftmodule"): | |||
| opt = opt + "/{}.swiftmodule".format(triple) | |||
|
|
|||
| # Process paths in the --flag=value format | |||
| flag, sep, value = opt.partition("=") | |||
| if value: | |||
There was a problem hiding this comment.
I think you can move this up and only do the path prepend logic once?
There was a problem hiding this comment.
Not sure if how I changed it is like you meant it, but I couldn't find a way to make it more readable. Let me know how it looks!
Fixes #638. This correctly matches and replaces file paths independently from where they are in the flag. This fixes building with BwX when using something like rules_apple_linker that passes the custom linker via the
--ld-pathflag.