Skip to content

Conversation

adincebic
Copy link
Contributor

Addresses #1511

@adincebic adincebic force-pushed the adin/implement-additiona_linker_inputs-attr branch from 5e49b82 to ac269f8 Compare September 28, 2025 13:19
ctx,
ctx.attr.linkopts,
ctx.attr.swiftc_inputs,
ctx.attr.swiftc_inputs + ctx.attr.additional_linker_inputs,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to remove swiftc_inputs here?

expected_inputs = ctx.attr.expected_additional_inputs
expected_linkopts = ctx.attr.expected_linkopts

action_input_paths = set([input.short_path for input in link_action.inputs.to_list()])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like we can't use set yet in our tests, since they run on Bazel 7.

create_linking_context_from_compilation_outputs(
actions = ctx.actions,
additional_inputs = ctx.files.swiftc_inputs,
additional_inputs = ctx.files.swiftc_inputs + ctx.files.additional_linker_inputs,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we want to remove swiftc_inputs here?

@adincebic
Copy link
Contributor Author

@brentleyjones I updated the PR. I left swiftc_inputs in RunEnvironmentInfo since we probably want to be able to reference swiftc_inputs?

RunEnvironmentInfo(
            environment = expand_locations(
                ctx,
                ctx.attr.env,
                ctx.attr.swiftc_inputs + ctx.attr.additional_linker_inputs,
            ),

Copy link
Collaborator

@brentleyjones brentleyjones left a comment

Choose a reason for hiding this comment

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

While this is technically a breaking change, in a way that could be hard to fix in non-root modules, it doesn't look like any public modules was using swiftc_inputs for linker inputs. So I'm fine with this if @keith is.

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.

2 participants