Skip to content
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

skip sanity check for non-host targets in check builds #121907

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

onur-ozkan
Copy link
Member

For check builds, since we only need to perform a sanity check on the host target, this patch skips target sanity checks on non-host targets.

For more context, see #121519 (comment)

cc @saethlin

@rustbot
Copy link
Collaborator

rustbot commented Mar 2, 2024

r? @albertlarsan68

rustbot has assigned @albertlarsan68.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Mar 2, 2024
// checks, and have a regular flag for skipping the latter. Also see
// <https://github.com/rust-lang/rust/pull/103569#discussion_r1008741742>.
if skip_target_sanity {
if skip_target_sanity && target != &build.build {
Copy link
Member

Choose a reason for hiding this comment

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

Does target != &build.build mean "target is the host"?

Copy link
Member Author

Choose a reason for hiding this comment

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

It means target is not host.

Copy link
Member

@RalfJung RalfJung Mar 10, 2024

Choose a reason for hiding this comment

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

Seems worth a comment somewhere to explain that we interpret skip_target_sanity to mean "skip checks for non-host targets".

It's not clear that build is the "host" target, that seems to diverge from the usual Rust naming conventions. There's no doc comment there either, in fact looking at struct Build it gets even more confusing

    // Targets for which to build
    build: TargetSelection,
    hosts: Vec<TargetSelection>,
    targets: Vec<TargetSelection>,

How can there be more than one host...?

Is this using GCC/autotools naming conventions rather than Rust naming conventions? That would be quite confusing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fields of Build represents these from the configuration:

rust/config.example.toml

Lines 184 to 210 in cdb775c

# Build triple for the pre-compiled snapshot compiler. If `rustc` is set, this must match its host
# triple (see `rustc --version --verbose`; cross-compiling the rust build system itself is NOT
# supported). If `rustc` is unset, this must be a platform with pre-compiled host tools
# (https://doc.rust-lang.org/nightly/rustc/platform-support.html). The current platform must be
# able to run binaries of this build triple.
#
# If `rustc` is present in path, this defaults to the host it was compiled for.
# Otherwise, `x.py` will try to infer it from the output of `uname`.
# If `uname` is not found in PATH, we assume this is `x86_64-pc-windows-msvc`.
# This may be changed in the future.
#build = "x86_64-unknown-linux-gnu" (as an example)
# Which triples to produce a compiler toolchain for. Each of these triples will be bootstrapped from
# the build triple themselves. In other words, this is the list of triples for which to build a
# compiler that can RUN on that triple.
#
# Defaults to just the `build` triple.
#host = [build.build] (list of triples)
# Which triples to build libraries (core/alloc/std/test/proc_macro) for. Each of these triples will
# be bootstrapped from the build triple themselves. In other words, this is the list of triples for
# which to build a library that can CROSS-COMPILE to that triple.
#
# Defaults to `host`. If you set this explicitly, you likely want to add all
# host triples to this list as well in order for those host toolchains to be
# able to compile programs for their native target.
#target = build.host (list of triples)

Seems worth a comment somewhere to explain that we interpret skip_target_sanity to mean "skip checks for non-host targets".

Sure

Copy link
Member

Choose a reason for hiding this comment

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

Fields of Build represents these from the configuration:

Ah, thanks for the link!

@albertlarsan68
Copy link
Member

Thanks for the PR!
@bors r+

@bors
Copy link
Contributor

bors commented Mar 9, 2024

📌 Commit 489dcf2 has been approved by albertlarsan68

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 9, 2024
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 9, 2024
…eck, r=albertlarsan68

skip sanity check for non-host targets in `check` builds

For `check` builds, since we only need to perform a sanity check on the host target, this patch skips target sanity checks on non-host targets.

For more context, see rust-lang#121519 (comment)

cc `@saethlin`
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 9, 2024
…iaskrgr

Rollup of 11 pull requests

Successful merges:

 - rust-lang#121567 (Avoid some interning in bootstrap)
 - rust-lang#121813 (Misc improvements to non local defs lint implementation)
 - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once)
 - rust-lang#121907 (skip sanity check for non-host targets in `check` builds)
 - rust-lang#122160 (Eagerly translate `HelpUseLatestEdition` in parser diagnostics)
 - rust-lang#122178 (ci: add a runner for vanilla LLVM 18)
 - rust-lang#122186 (Remove a workaround for a bug)
 - rust-lang#122187 (Move metadata header and version checks together)
 - rust-lang#122215 (Some tweaks to the parallel query cycle handler)
 - rust-lang#122223 (Fix typo in `VisitorResult`)
 - rust-lang#122232 (library/core: fix a comment, and a cfg(miri) warning)

r? `@ghost`
`@rustbot` modify labels: rollup
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 9, 2024
…eck, r=albertlarsan68

skip sanity check for non-host targets in `check` builds

For `check` builds, since we only need to perform a sanity check on the host target, this patch skips target sanity checks on non-host targets.

For more context, see rust-lang#121519 (comment)

cc ``@saethlin``
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 10, 2024
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#121561 (Detect typos for compiletest test directives)
 - rust-lang#121567 (Avoid some interning in bootstrap)
 - rust-lang#121685 (Fixing shellcheck comments on lvi test script)
 - rust-lang#121833 (Suggest correct path in include_bytes!)
 - rust-lang#121860 (Add a tidy check that checks whether the fluent slugs only appear once)
 - rust-lang#121907 (skip sanity check for non-host targets in `check` builds)
 - rust-lang#122029 (When displaying multispans, ignore empty lines adjacent to `...`)
 - rust-lang#122221 (match lowering: define a convenient struct)
 - rust-lang#122244 (fix: LocalWaker memory leak and some stability attributes)
 - rust-lang#122251 (Add test to check unused_lifetimes don't duplicate "parameter is never used" error)

r? `@ghost`
`@rustbot` modify labels: rollup
@onur-ozkan
Copy link
Member Author

@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 10, 2024
For `check` builds, since we only need to perform a sanity check on
the host target, this patch skips target sanity checks on non-host targets.

Signed-off-by: onur-ozkan <work@onurozkan.dev>
@onur-ozkan
Copy link
Member Author

@bors r=albertlarsan68

@bors
Copy link
Contributor

bors commented Mar 10, 2024

📌 Commit e5e1fa6 has been approved by albertlarsan68

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 10, 2024
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 11, 2024
…eck, r=albertlarsan68

skip sanity check for non-host targets in `check` builds

For `check` builds, since we only need to perform a sanity check on the host target, this patch skips target sanity checks on non-host targets.

For more context, see rust-lang#121519 (comment)

cc `@saethlin`
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
Rollup of 8 pull requests

Successful merges:

 - rust-lang#121148 (Add slice::try_range)
 - rust-lang#121573 (unix_sigpipe: Add test for SIGPIPE disposition in child processes)
 - rust-lang#121633 (Win10: Use `GetSystemTimePreciseAsFileTime` directly)
 - rust-lang#121840 (Expose the Freeze trait again (unstably) and forbid implementing it manually)
 - rust-lang#121907 (skip sanity check for non-host targets in `check` builds)
 - rust-lang#122002 (std::threads: revisit stack address calculation on netbsd.)
 - rust-lang#122108 (Add `target.*.runner` configuration for targets)
 - rust-lang#122298 (RawVec::into_box: avoid unnecessary intermediate reference)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
Rollup of 9 pull requests

Successful merges:

 - rust-lang#121148 (Add slice::try_range)
 - rust-lang#121633 (Win10: Use `GetSystemTimePreciseAsFileTime` directly)
 - rust-lang#121840 (Expose the Freeze trait again (unstably) and forbid implementing it manually)
 - rust-lang#121907 (skip sanity check for non-host targets in `check` builds)
 - rust-lang#122002 (std::threads: revisit stack address calculation on netbsd.)
 - rust-lang#122108 (Add `target.*.runner` configuration for targets)
 - rust-lang#122298 (RawVec::into_box: avoid unnecessary intermediate reference)
 - rust-lang#122315 (Allow multiple `impl Into<{D,Subd}iagMessage>` parameters in a function.)
 - rust-lang#122326 (Optimize `process_heap_alloc`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 1009859 into rust-lang:master Mar 11, 2024
11 checks passed
@rustbot rustbot added this to the 1.78.0 milestone Mar 11, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 11, 2024
Rollup merge of rust-lang#121907 - onur-ozkan:better-target-sanity-check, r=albertlarsan68

skip sanity check for non-host targets in `check` builds

For `check` builds, since we only need to perform a sanity check on the host target, this patch skips target sanity checks on non-host targets.

For more context, see rust-lang#121519 (comment)

cc ``@saethlin``
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants