Skip to content

compiletest: add a proper supports-crate-type: xxx directive #132309

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.

Sign up for GitHub

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

Jump to bottom
Closed
jieyouxu opened this issue Oct 29, 2024 · 1 comment · Fixed by #139469
Closed

compiletest: add a proper supports-crate-type: xxx directive #132309

jieyouxu opened this issue Oct 29, 2024 · 1 comment · Fixed by #139469
Labels
A-compiletest Area: The compiletest test runner C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)

Comments

@jieyouxu
Copy link
Member

jieyouxu commented Oct 29, 2024 •
edited
Loading

Apparently needs-dynamic-linking is not equivalent to checking if dylib or cdylib crate types are supported.

  • In compiletest, needs-dynamic-linking performs a check based on target cfg's dynamic_linking field + --print=cfg --target $TARGET.
  • However, target cfg has an additional field only_cdylib which, if dynamic_linking is true, indicates that only cdylib crate type is supported and not dylib.

    rust/compiler/rustc_target/src/spec/mod.rs

    Lines 2148 to 2153 in f2becdf

    /// Whether dynamic linking is available on this target. Defaults to false.
    pub dynamic_linking: bool,
    /// Whether dynamic linking can export TLS globals. Defaults to true.
    pub dll_tls_export: bool,
    /// If dynamic linking is available, whether only cdylibs are supported.
    pub only_cdylib: bool,
    • This is the case for wasm base, dynamic linking is supported but not dylib crate type, only cdylib is supported.

      rust/compiler/rustc_target/src/spec/base/wasm.rs

      Lines 58 to 62 in f2becdf

      // we allow dynamic linking, but only cdylibs. Basically we allow a
      // final library artifact that exports some symbols (a wasm module) but
      // we don't allow intermediate `dylib` crate types
      dynamic_linking: true,
      only_cdylib: true,

Originally posted by @jieyouxu in #130860 (comment)
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 29, 2024
@jieyouxu jieyouxu added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-compiletest Area: The compiletest test runner E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. labels Oct 29, 2024
@jieyouxu
Copy link
Member Author

Related: #132350

@jieyouxu jieyouxu mentioned this issue Oct 30, 2024
Closed
@jieyouxu jieyouxu changed the title compiletest: add a proper supports-crate-type: dylib directive compiletest: add a proper supports-crate-type: xxx directive Dec 29, 2024
@jieyouxu jieyouxu mentioned this issue Dec 29, 2024
Merged
This was referenced Jan 8, 2025
Merged
Merged
@jieyouxu jieyouxu mentioned this issue Feb 14, 2025
Closed
3 tasks
@jieyouxu jieyouxu mentioned this issue Apr 7, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Apr 7, 2025
@bors
Auto merge of rust-lang#139469 - jieyouxu:compiletest-supports-crate-…
659206b 
…type, r=<try>

Introduce a `//@ needs-crate-type` compiletest directive

> [!CAUTION]
>
> Blocked on bootstrap compiler bump rust-lang#139279 so I can get rid of the `cfg(not(bootstrap))` on the compiletest directive unit test.

The `//@ needs-crate-type: $crate_types...` directive takes a comma-separated list of crate types that the target platform must support in order for the test to be run. This allows the test writer to semantically convey that the ignore condition is based on target crate type needs, instead of using a general purpose `//@ ignore-$target` directive (often without comment).

Fixes rust-lang#132309.

### Example

```rs
//@ needs-crate-type: dylib (ignored on e.g. wasm32-unknown-unknown)
//@ compile-flags: --crate-type=dylib

fn foo() {}
```

### Review advice

- Best reviewed commit-by-commit.
- The impl is not very clean, I briefly attempted to clean up the directive handling but found that more invasive changes are needed, so I'd like to not block on the cleanup for now.

try-job: test-various
try-job: armhf-gnu
@bors bors closed this as completed in ea1a31b Apr 11, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 11, 2025
@rust-timer
Unrolled build for rust-lang#139469
506803a 
Rollup merge of rust-lang#139469 - jieyouxu:compiletest-supports-crate-type, r=onur-ozkan

Introduce a `//@ needs-crate-type` compiletest directive

The `//@ needs-crate-type: $crate_types...` directive takes a comma-separated list of crate types that the target platform must support in order for the test to be run. This allows the test writer to semantically convey that the ignore condition is based on target crate type needs, instead of using a general purpose `//@ ignore-$target` directive (often without comment).

Fixes rust-lang#132309.

### Example

```rs
//@ needs-crate-type: dylib (ignored on e.g. wasm32-unknown-unknown)
//@ compile-flags: --crate-type=dylib

fn foo() {}
```

### Review advice

- Best reviewed commit-by-commit.
- The impl is not very clean, I briefly attempted to clean up the directive handling but found that more invasive changes are needed, so I'd like to not block on the cleanup for now.

try-job: test-various
try-job: armhf-gnu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-compiletest Area: The compiletest test runner C-enhancement Category: An issue proposing an enhancement or a PR with one. E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Introduce a //@ needs-crate-type compiletest directive jieyouxu/rust
2 participants
@jieyouxu @rustbot