Skip to content

skip link step when running clippy #455

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

Closed
wants to merge 1 commit into from

Conversation

dae
Copy link
Contributor

@dae dae commented Oct 20, 2020

I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:

rustc_flags = selects.with_or({
    (
        "@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
    ): [
        "-Clink-arg=-undefined",
        "-Clink-arg=dynamic_lookup",
    ],
    "//conditions:default": [],
}),

Without them, the linker on macOS fails, as the Python API is not
available until runtime.

rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.

Tangentially related to #428 and #421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.

@google-cla google-cla bot added the cla: yes label Oct 20, 2020
@dae dae force-pushed the clippy-link branch 2 times, most recently from e364d38 to 66e2c10 Compare November 5, 2020 20:28
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:

    rustc_flags = selects.with_or({
        (
            "@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
        ): [
            "-Clink-arg=-undefined",
            "-Clink-arg=dynamic_lookup",
        ],
        "//conditions:default": [],
    }),

Without them, the linker on macOS fails, as the Python API is not
available until runtime.

rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.

Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
@UebelAndre
Copy link
Collaborator

Any updates here? Looks like it didn't even get a review yet

@dae
Copy link
Contributor Author

dae commented Dec 3, 2020

Patiently awaiting a merge :-)

@UebelAndre
Copy link
Collaborator

UebelAndre commented Dec 5, 2020

@dfreese Btw there's a similar PR to this

#526

@dfreese
Copy link
Collaborator

dfreese commented Dec 5, 2020

Closed in favor of #526. Sorry for the delay on the review @dae.

@dfreese dfreese closed this Dec 5, 2020
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.

3 participants