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

run-make test: using single quotes to not trigger the shell #110948

Merged
merged 1 commit into from
Apr 29, 2023

Conversation

TimNN
Copy link
Contributor

@TimNN TimNN commented Apr 28, 2023

This test got added in #110801.

I'm no expert on Makefiles, but IIUC this command is passed to the shell, which usually tries to execute commands specified in between backticks in double-quoted strings.

Using single quotes should fix this, I think. (Note: Waiting for CI to test this, since I only have a web browser available right now).

r? @jyn514

cc @WaffleLapkin

Since this is breaking our build bot, even if it is not directly LLVM related: @rustbot label: +llvm-main

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) labels Apr 28, 2023
@WaffleLapkin
Copy link
Member

r? @WaffleLapkin
r=me with CI green

@rustbot rustbot assigned WaffleLapkin and unassigned jyn514 Apr 28, 2023
@jyn514
Copy link
Member

jyn514 commented Apr 28, 2023

Hmm, I'm surprised CI didn't catch this :/

@TimNN
Copy link
Contributor Author

TimNN commented Apr 28, 2023

Hmm, I'm surprised CI didn't catch this :/

I was also exceedingly surprised that this worked in the first place. I have no idea what kind of magically special circumstances allowed this to work...

Maybe make is rewriting quotes in some way somewhere? (Though if so, I'm not sure if this PR is going to change anything).

@TimNN
Copy link
Contributor Author

TimNN commented Apr 28, 2023

@WaffleLapkin: CI is green.

@jyn514
Copy link
Member

jyn514 commented Apr 28, 2023

@bors r=WaffleLapkin rollup

I want to investigate why CI didn't catch this, but that doesn't have to block the fix.

@bors
Copy link
Contributor

bors commented Apr 28, 2023

📌 Commit f1b88ea has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 28, 2023

🌲 The tree is currently closed for pull requests below priority 50. This pull request will be tested once the tree is reopened.

@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 Apr 28, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 28, 2023
run-make test: using single quotes to not trigger the shell

This test got added in rust-lang#110801.

I'm no expert on Makefiles, but IIUC this command is passed to the shell, which usually tries to execute commands specified in between backticks in double-quoted strings.

Using single quotes should fix this, I think. (Note: Waiting for CI to test this, since I only have a web browser available right now).

r? `@jyn514`

cc `@WaffleLapkin`

Since this is breaking our build bot, even if it is not directly LLVM related: `@rustbot` label: +llvm-main
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 28, 2023
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#110877 (Provide better type hints when a type doesn't support a binary operator)
 - rust-lang#110917 (only error combining +whole-archive and +bundle for rlibs)
 - rust-lang#110921 (Use `NonNull::new_unchecked` and `NonNull::len` in `rustc_arena`.)
 - rust-lang#110927 (Encoder/decoder cleanups)
 - rust-lang#110944 (share BinOp::Offset between CTFE and Miri)
 - rust-lang#110948 (run-make test: using single quotes to not trigger the shell)
 - rust-lang#110957 (Fix an ICE in conflict error diagnostics)
 - rust-lang#110960 (fix false negative for `unused_mut`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit afbb188 into rust-lang:master Apr 29, 2023
@rustbot rustbot added this to the 1.71.0 milestone Apr 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm-main Marks PRs that are making Rust work with LLVM main (this label is consumed by CI tooling) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants