fix built-in target detection by japaric · Pull Request #33363 · rust-lang/rust

japaric · 2016-05-03T00:03:09Z

previously the logic was accepting wrong triples (like
x86_64_unknown-linux-musl) as valid ones (like x86_64-unknown-linux-musl) if
they contained an underscore instead of a dash.

fixes#33329

r? @brson

I wanted to use a compile-fail test at first. But, you can't pass an extra --target flag to rustc for those because they already call rustc --target $HOST so you get a error: Option 'target' given more than once.. The run-make test used here works fine though.

previously the logic was accepting wrong triples (like `x86_64_unknown-linux-musl`) as valid ones (like `x86_64-unknown-linux-musl`) if they contained an underscore instead of a dash. fixesrust-lang#33329

rust-highfive · 2016-05-03T00:03:23Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @brson (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

brson · 2016-05-03T00:06:59Z

@bors r+

bors · 2016-05-03T00:07:01Z

📌 Commit 5943507 has been approved by brson

durka · 2016-05-03T01:48:35Z

@@ -71,10 +71,9 @@ macro_rules! supported_targets {

 // this would use a match if stringify! were allowed in pattern position


is this comment stale now?

(it wasn't true anyway)

(it wasn't true anyway)
Actually it would look like this: http://is.gd/ot9Ta2
Now that we are using $triple instead of $module we can change this into a match. I'll send a follow up PR because this one already got pulled into the rollup.

looks like no longer in the rollup

Rollup of 14 pull requests - Successful merges: #33277, #33294, #33314, #33322, #33333, #33338, #33339, #33340, #33343, #33357, #33363, #33365, #33371, #33372 - Failed merges:

japaric · 2016-05-05T03:17:47Z

Pushed a commit with the if -> match refactor

@bors r=brson

bors · 2016-05-05T03:17:49Z

📌 Commit ec616d5 has been approved by brson

bors · 2016-05-08T03:04:01Z

⌛ Testing commit ec616d5 with merge c9ef2c4...

bors · 2016-05-08T04:03:53Z

💔 Test failed - auto-win-msvc-64-opt-rustbuild

japaric · 2016-06-17T00:39:41Z

I had forgotten about this PR 😅. Hmm, I guess this failed because there is no grep on windows? What's a cross platform way to grep files? cc @alexcrichton

alexcrichton · 2016-06-17T10:59:56Z

Oh I think that Windows has grep (part of MinGW), but the grep looks like it may have just failed. Maybe an error string difference?

japaric · 2016-06-18T13:51:39Z

@alexcrichton Does the msvc bot also have grep installed? And does it run the tests under the mingw shell? AFAICT my grep string has no special characters... just alphanum chars and ':'.

alexcrichton · 2016-06-19T21:33:16Z

They should yeah, currently all those bots run in a MinGW shell

brson · 2016-07-11T22:30:38Z

@bors retry

Just retrying to see the error again.

bors · 2016-07-11T22:52:33Z

⌛ Testing commit ec616d5 with merge 7240b18...

brson · 2016-07-21T21:20:48Z

Error was:


---- [run-make] run-make\issue-33329 stdout ----
error: make failed
status: exit code: 2
command: "make"
stdout:
------------------------------------------
make[1]: Entering directory '/c/bot/slave/auto-win-msvc-64-opt-rustbuild/build/src/test/run-make/issue-33329'
C:\bot\slave\auto-win-msvc-64-opt-rustbuild\build\obj\build\x86_64-pc-windows-msvc\stage2\bin\rustc.exe --target x86_64_unknown-linux-musl main.rs 2>&1 | \
grep "error: Error loading target specification: Could not find specification for target"
Makefile:2: recipe for target 'all' failed
make[1]: Leaving directory '/c/bot/slave/auto-win-msvc-64-opt-rustbuild/build/src/test/run-make/issue-33329'
------------------------------------------
stderr:
------------------------------------------
make[1]: *** [all] Error 1
------------------------------------------
thread '[run-make] run-make\issue-33329' panicked at 'explicit panic', C:\bot\slave\auto-win-msvc-64-opt-rustbuild\build\src\tools\compiletest\src\runtest.rs:2243
note: Run with `RUST_BACKTRACE=1` for a backtrace.
failures:
[run-make] run-make\issue-33329
test result: FAILED. 129 passed; 1 failed; 0 ignored; 0 measured
command did not execute successfully: "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\stage2-tools\\x86_64-pc-windows-msvc\\release\\compiletest.exe" "--compile-lib-path" "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\stage2\\bin" "--run-lib-path" "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "--rustc-path" "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\stage2\\bin\\rustc.exe" "--rustdoc-path" "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\stage2\\bin\\rustdoc.exe" "--src-base" "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\src/test\\run-make" "--build-base" "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\test\\run-make" "--stage-id" "stage2-x86_64-pc-windows-msvc" "--mode" "run-make" "--target" "x86_64-pc-windows-msvc" "--host" "x86_64-pc-windows-msvc" "--llvm-filecheck" "C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\llvm\\build\\Release/bin\\FileCheck.exe" "--host-rustcflags" "-Crpath -O" "--target-rustcflags" "-Crpath -O -Lnative=C:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\rust-test-helpers" "--docck-python" "python" "--lldb-python" "python" "--gdb-version" "GNU gdb (GDB) 7.8" "--cc" "C:\\Program Files (x86)\\Microsoft Visual Studio 12.0\\VC/bin\\amd64\\cl.exe" "--cxx" "C:\\Program Files (x86)\\Microsoft Visual Studio 12.0\\VC/bin\\amd64\\cl.exe" "--cflags" "/nologo /MD" "--llvm-components" "aarch64 aarch64asmparser aarch64asmprinter aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils all all-targets analysis arm armasmparser armasmprinter armcodegen armdesc armdisassembler arminfo asmparser asmprinter bitreader bitwriter codegen core debuginfocodeview debuginfodwarf debuginfopdb engine executionengine instcombine instrumentation interpreter ipo irreader libdriver lineeditor linker lto mc mcdisassembler mcjit mcparser mips mipsasmparser mipsasmprinter mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser native nativecodegen objcarcopts object option orcjit passes powerpc powerpcasmparser powerpcasmprinter powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata runtimedyld scalaropts selectiondag support symbolize tablegen target transformutils vectorize x86 x86asmparser x86asmprinter x86codegen x86desc x86disassembler x86info x86utils" "--llvm-cxxflags" "-IC:\\bot\\slave\\auto-win-msvc-64-opt-rustbuild\\build\\obj\\build\\x86_64-pc-windows-msvc\\llvm/include /nologo /MD /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4324 -w14062 -we4238 /Zc:inline /MD /O2 /Ob2 /D NDEBUG /EHs-c- /GR- /MP -D_DEBUG_POINTER_IMPL= -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS" "--adb-path" "adb" "--adb-test-dir" "/data/tmp" "--android-cross-path" ""
thread 'main' panicked at 'Some tests failed', C:\bot\slave\auto-win-msvc-64-opt-rustbuild\build\src\tools\compiletest\src\main.rs:293
expected success, got: exit code: 101

brson · 2016-07-21T21:21:43Z

@japaric Can we just xfail this on windows and push it through?

japaric · 2016-07-22T01:34:15Z

@brson By "xfail" do you mean ignoring this test on windows? That would be fine by me. Is this the standard way to do that?

brson · 2016-07-22T17:57:48Z

@japaric Yep, and that snippet looks ok to me (but I've never ignored a run-make test before).

japaric · 2016-07-22T22:13:53Z

I just realized my Makefile is missing the -include ../tools.mk that all the other run-make Makefiles have ... so I'm going to first check if that was the problem.

@bors try

bors · 2016-07-22T22:13:56Z

⌛ Trying commit 94ae8bb with merge 756564a...

@brson

fix built-in target detection previously the logic was accepting wrong triples (like `x86_64_unknown-linux-musl`) as valid ones (like `x86_64-unknown-linux-musl`) if they contained an underscore instead of a dash. fixes#33329 --- r? @brson I wanted to use a compile-fail test at first. But, you can't pass an extra `--target` flag to `rustc` for those because they already call `rustc --target $HOST` so you get a `error: Option 'target' given more than once.`. The run-make test used here works fine though.

bors · 2016-07-23T08:14:36Z

💥 Test timed out

alexcrichton · 2016-07-23T17:24:51Z

Ah unfortunately @bors: try doesn't actually work, but let's see if 94ae8bb works on bors

@bors: r+ 94ae8bb

japaric · 2016-07-25T06:24:12Z

This was stuck in the queue as failed (because of the previous try). Rebased so I can r+ it again.

@bors: r+

bors · 2016-07-25T06:24:13Z

📌 Commit f438801 has been approved by japaric

bors · 2016-07-25T06:32:15Z

⌛ Testing commit f438801 with merge 5033302...

alexcrichton · 2016-07-25T06:46:09Z

@bors: retry force clean

restarted buildbot

alexcrichton · 2016-07-25T20:17:37Z

@bors: retry force clean

bors · 2016-07-25T21:41:08Z

⌛ Testing commit f438801 with merge 31db8e2...

bors · 2016-07-25T21:45:09Z

💔 Test failed - auto-win-gnu-32-opt

alexcrichton · 2016-07-25T22:15:15Z

@bors: retry

On Mon, Jul 25, 2016 at 2:45 PM, bors notifications@github.com wrote:

💔 Test failed - auto-win-gnu-32-opt
https://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt/builds/5067
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#33363 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95JsWjEYoFRvExsLFPCnpxNy_2Jhtks5qZS5ogaJpZM4IV058
.

bors · 2016-07-26T09:58:31Z

⌛ Testing commit f438801 with merge c1b278a...

bors · 2016-07-26T10:02:35Z

💔 Test failed - auto-win-gnu-32-opt-rustbuild

alexcrichton · 2016-07-26T17:20:39Z

@bors: retry

On Tue, Jul 26, 2016 at 3:02 AM, bors notifications@github.com wrote:

💔 Test failed - auto-win-gnu-32-opt-rustbuild
https://buildbot.rust-lang.org/builders/auto-win-gnu-32-opt-rustbuild/builds/1948
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#33363 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAD95HrVYYpYJGJMdY_p4VjYYSyzs7Rlks5qZds9gaJpZM4IV058
.

bors · 2016-07-27T12:50:27Z

⌛ Testing commit f438801 with merge a373b84...

@brson

fix built-in target detection previously the logic was accepting wrong triples (like `x86_64_unknown-linux-musl`) as valid ones (like `x86_64-unknown-linux-musl`) if they contained an underscore instead of a dash. fixes#33329 --- r? @brson I wanted to use a compile-fail test at first. But, you can't pass an extra `--target` flag to `rustc` for those because they already call `rustc --target $HOST` so you get a `error: Option 'target' given more than once.`. The run-make test used here works fine though.

bors · 2016-07-27T17:16:26Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild
Approved by: japaric
Pushing a373b84 to master...

fix built-in target detection
5943507
previously the logic was accepting wrong triples (like `x86_64_unknown-linux-musl`) as valid ones (like `x86_64-unknown-linux-musl`) if they contained an underscore instead of a dash. fixesrust-lang#33329

rust-highfive assigned brson May 3, 2016

durka reviewed May 3, 2016
View reviewed changes

Manishearth mentioned this pull request May 3, 2016
Rollup of 14 pull requests #33376
Merged

refactor: if -> match
ec616d5

add include ../tools.mk to the Makefile
f438801

japaric force-pushed the target branch from 94ae8bb to f438801Compare July 25, 2016 06:23

bors merged commit f438801 into rust-lang:masterJul 27, 2016

bors mentioned this pull request Jul 27, 2016
Convert built-in targets to JSON #34980
Merged

		@@ -71,10 +71,9 @@ macro_rules! supported_targets {

		// this would use a match if stringify! were allowed in pattern position

Uh oh!

Uh oh!

Conversation

japaric commented May 3, 2016

Uh oh!

rust-highfive commented May 3, 2016

Uh oh!

brson commented May 3, 2016

Uh oh!

bors commented May 3, 2016

Uh oh!

durkaMay 3, 2016

Choose a reason for hiding this comment

Uh oh!

durkaMay 3, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

japaricMay 3, 2016

Choose a reason for hiding this comment

Uh oh!

tamirdMay 5, 2016

Choose a reason for hiding this comment

Uh oh!

japaric commented May 5, 2016

Uh oh!

bors commented May 5, 2016

Uh oh!

bors commented May 8, 2016

Uh oh!

bors commented May 8, 2016

Uh oh!

japaric commented Jun 17, 2016

Uh oh!

alexcrichton commented Jun 17, 2016

Uh oh!

japaric commented Jun 18, 2016

Uh oh!

alexcrichton commented Jun 19, 2016

Uh oh!

brson commented Jul 11, 2016

Uh oh!

bors commented Jul 11, 2016

Uh oh!

brson commented Jul 21, 2016

Uh oh!

brson commented Jul 21, 2016

Uh oh!

japaric commented Jul 22, 2016

Uh oh!

brson commented Jul 22, 2016

Uh oh!

japaric commented Jul 22, 2016

Uh oh!

bors commented Jul 22, 2016

Uh oh!

bors commented Jul 23, 2016

Uh oh!

alexcrichton commented Jul 23, 2016

Uh oh!

japaric commented Jul 25, 2016

Uh oh!

bors commented Jul 25, 2016

Uh oh!

bors commented Jul 25, 2016

Uh oh!

alexcrichton commented Jul 25, 2016

Uh oh!

alexcrichton commented Jul 25, 2016

Uh oh!

bors commented Jul 25, 2016

Uh oh!

bors commented Jul 25, 2016

Uh oh!

alexcrichton commented Jul 25, 2016

Uh oh!

bors commented Jul 26, 2016

Uh oh!

bors commented Jul 26, 2016

Uh oh!

durkaMay 3, 2016•
edited
Loading