add a reason to `masquerade_as_nightly_cargo` so it is searchable #10868

Muscraft · 2022-07-15T02:37:22Z

When I was working on the stabilization for workspace inheritance, it was very tedious to find all of the places to remove .masquerade_as_nightly_cargo(). I suggested to add a reason to .masquerade_as_nightly_cargo() so that it would be easier to find all of the places to remove it. By adding the reason it makes it easy to search for all places with the features name. This PR adds the reason(s) to all of the places .masquerade_as_nightly_cargo() is called, as well as updates the documentation so it talks about adding a reason when making the call.

rust-highfive · 2022-07-15T02:37:25Z

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

epage · 2022-07-15T12:41:37Z

src/doc/contrib/src/tests/writing.md

@@ -192,12 +192,12 @@ Be sure to check the snapshots to make sure they make sense.
 #### Testing Nightly Features

 If you are testing a Cargo feature that only works on "nightly" Cargo, then
-you need to call `masquerade_as_nightly_cargo` on the process builder like
-this:
+you need to call `masquerade_as_nightly_cargo` on the process builder nd pass


nd -> and

epage · 2022-07-15T12:46:50Z

tests/testsuite/cargo_config.rs

@@ -273,7 +273,7 @@ target.\"cfg(target_os = \\\"linux\\\")\".runner = \"runme\" # [ROOT]/home/.carg

    cargo_process("config get --show-origin build.rustflags -Zunstable-options")
        .cwd(&sub_folder.parent().unwrap())
-        .masquerade_as_nightly_cargo()
+        .masquerade_as_nightly_cargo(&["cargo config"])


Eh, guess its good enough

unstable-options is an umbrella feature

nothing else really to specify

cargo config is unstable which is why I used it. The question I have is should we prefer dashes? as in cargo-config? It might be slightly better

epage · 2022-07-15T12:47:48Z

tests/testsuite/cargo_features.rs

@@ -19,7 +19,7 @@ fn feature_required() {
        .file("src/lib.rs", "")
        .build();
    p.cargo("build")
-        .masquerade_as_nightly_cargo()
+        .masquerade_as_nightly_cargo(&["cargo_features"])


Hmm, unsure what communicates intent best here.

I debated with im-a-teapot or test-dummy-unstable but those don't really cover why. always_nightly or both might be better but I am unsure here as well

epage · 2022-07-15T12:48:42Z

tests/testsuite/check_cfg.rs

    p.cargo("build -v -Zcheck-cfg=features")
-        .masquerade_as_nightly_cargo()
+        .masquerade_as_nightly_cargo(&["check-cfg"])


Haven't dealt with this one before, unsure if we should do it as check-cfg or check-cfg=features

I think check-cfg is better since that is that the unstable feature is called. If certain parts get stabilized before others (like features), at that point it should become check-cfg=features

epage · 2022-07-15T12:50:23Z

tests/testsuite/credential_process.rs

@@ -32,7 +32,7 @@ fn gated() {
        .build();

    p.cargo("publish --no-verify")
-        .masquerade_as_nightly_cargo()
+        .masquerade_as_nightly_cargo(&["credential-process"])


Something is non-obvious here since the -Zcredential-process is unspecified for this and the next test

I'll dig a bit but removing masquerade_as_nightly_cargo allows it to work but the test is called gated. Maybe it should have -Zcredential-process?

I updated everything but this. I honestly have no idea why masquerade_as_nightly_cargo is here.

@ehuss it looks like you wrote this, do you have any ideas?

There's not really a strong reason to have it one way or the other. Switching to nightly mode makes extra sure that it does not implicitly use the feature without the -Z flag. There are some things that behave differently in nightly mode, and I was probably just trying to be extra paranoid here. But it is also unlikely for that to be a problem in practice.

epage · 2022-07-15T12:51:37Z

tests/testsuite/features2.rs

@@ -2012,7 +2012,7 @@ fn minimal_download() {
    // none
    // Should be the same as `-Zfeatures=all`
    p.cargo("check -Zfeatures=compare")
-        .masquerade_as_nightly_cargo()
+        .masquerade_as_nightly_cargo(&["features=compare"])


Should this be always_nightly?

I'm not sure what this should be. This makes it seem like it shouldn't be nightly forever, or that it should at least be moved to its own command. The correct decision might be to tag it with both I am unsure

Either would be fine, this is a permanently unstable option. I'm fine with leaving it as-is.

ehuss · 2022-07-16T18:36:52Z

Thanks! I think I would have preferred just a string to avoid all the extra punctuation, but I don't feel strongly about that. I'm going to go ahead and approve, but if @epage has any other concerns or changes, feel free to comment or post a followup pr.

@bors r+

bors · 2022-07-16T18:36:54Z

📌 Commit c239e40 has been approved by ehuss

It is now in the queue for this repository.

bors · 2022-07-16T18:37:01Z

⌛ Testing commit c239e40 with merge bd5db30...

bors · 2022-07-16T19:23:02Z

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing bd5db30 to master...

Update cargo 9 commits in 8827baaa781b37872134c1ba692a6f0aeb37890e..d8d30a75376f78bb0fabe3d28ee9d87aa8035309 2022-07-14 02:56:51 +0000 to 2022-07-19 13:59:17 +0000 - docs: fixing minor error in the default timing report filename (rust-lang/cargo#10879) - Stabilize `--crate-type` flag for `cargo rustc` (rust-lang/cargo#10838) - Stabilize `-Zmultitarget` (rust-lang/cargo#10766) - Clean up leftover in unstable documentation (rust-lang/cargo#10874) - Normalize path for `cargo vendor` output (rust-lang/cargo#10668) - add a reason to `masquerade_as_nightly_cargo` so it is searchable (rust-lang/cargo#10868) - Allow '.' in workspace.default-members in non-virtual workspaces. (rust-lang/cargo#10784) - remove`.masquerade_as_nightly_cargo()` from various tests the no longer need it (rust-lang/cargo#10867) - remove `.masquerade_as_nightly_cargo()` from build_script_extra_link_arg.rs (rust-lang/cargo#10866)

rust-highfive assigned ehuss Jul 15, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 15, 2022

epage reviewed Jul 15, 2022

View reviewed changes

add a reason to masquerade_as_nightly_cargo so it is searchable

c239e40

Muscraft force-pushed the add-reason-for-nightly-tests branch from 3e4f94d to c239e40 Compare July 16, 2022 02:32

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 Jul 16, 2022

bors merged commit bd5db30 into rust-lang:master Jul 16, 2022

ehuss mentioned this pull request Jul 20, 2022

Update cargo rust-lang/rust#99484

Merged

ehuss added this to the 1.64.0 milestone Jul 20, 2022

Muscraft deleted the add-reason-for-nightly-tests branch August 17, 2022 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a reason to `masquerade_as_nightly_cargo` so it is searchable #10868

add a reason to `masquerade_as_nightly_cargo` so it is searchable #10868

Muscraft commented Jul 15, 2022

rust-highfive commented Jul 15, 2022

epage Jul 15, 2022

epage Jul 15, 2022

Muscraft Jul 15, 2022

epage Jul 15, 2022

Muscraft Jul 15, 2022

epage Jul 15, 2022

Muscraft Jul 15, 2022

epage Jul 15, 2022

Muscraft Jul 15, 2022

Muscraft Jul 16, 2022

ehuss Jul 16, 2022

epage Jul 15, 2022

Muscraft Jul 15, 2022

ehuss Jul 16, 2022

ehuss commented Jul 16, 2022

bors commented Jul 16, 2022

bors commented Jul 16, 2022

bors commented Jul 16, 2022

add a reason to masquerade_as_nightly_cargo so it is searchable #10868

add a reason to masquerade_as_nightly_cargo so it is searchable #10868

Conversation

Muscraft commented Jul 15, 2022

rust-highfive commented Jul 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehuss commented Jul 16, 2022

bors commented Jul 16, 2022

bors commented Jul 16, 2022

bors commented Jul 16, 2022

add a reason to `masquerade_as_nightly_cargo` so it is searchable #10868

add a reason to `masquerade_as_nightly_cargo` so it is searchable #10868