Use a regex filter for the test runner & assorted fixes #13948
Conversation
|
r=me, but looks like there's a failure on travis |
The regex change is fully backwards compatible, since test names are Rust identifiers + `:`, and hence not special regex characters. (See commits for details.)
| @@ -170,7 +180,7 @@ pub fn log_config(config: &config) { | |||
| logv(c, format!("stage_id: {}", config.stage_id)); | |||
| logv(c, format!("mode: {}", mode_str(config.mode))); | |||
| logv(c, format!("run_ignored: {}", config.run_ignored)); | |||
| logv(c, format!("filter: {}", opt_str(&config.filter))); | |||
| logv(c, format!("filter: {}", if config.filter.is_some() { "(regex)" } else { "(none)" })); | |||
There was a problem hiding this comment.
There is a Show implementation for Regex, is there a reason you opted out of it here? (just passing by)
There was a problem hiding this comment.
Oh, I didn't even realise that existed.
|
This build failure is a legitimate problem, and it is a bit of an interesting one. The failure occurs when the compiler opens up the You've updated libtest to have a dependency on libregex, which means that libregex-foo.so is now available in the host directory (it was missing there before), so now the call to dlopen is opening up libregex-foo.so in the host directory, not the target directory. I believe if you change the order of addition here to prepend to |
|
As in, this: diff --git a/src/libstd/unstable/dynamic_lib.rs b/src/libstd/unstable/dynamic_lib.rs
index 68f0aaa..865b9cb 100644
--- a/src/libstd/unstable/dynamic_lib.rs
+++ b/src/libstd/unstable/dynamic_lib.rs
@@ -73,7 +73,7 @@ impl DynamicLibrary {
("LD_LIBRARY_PATH", ':' as u8)
};
let newenv = os::getenv_as_bytes(envvar).unwrap_or(box []);
- let newenv = newenv + &[sep] + path.as_vec();
+ let newenv = path.as_vec() + &[sep] + newenv;
os::setenv(envvar, str::from_utf8(newenv).unwrap());
}
|
|
I've been a bit busy, will get to this tomorrow. |
|
Actually, was easy enough to do now. r? @alexcrichton |
|
I'm confused about this failure, I'll try to investigate soon. |
|
I have been looking at the failure. I can replicate on OS X. Here is what I have established:
|
|
I assume that The reason I ask: we have wanted to support keeping the compile-time crates in a path distinct from the run-time crates, if desired. But that implies that the above kinds of symbol lookups should not attempt to use the Perhaps this is the kind of problem that @alexcrichton has been concerned about for all this time, and I was missing the big picture issue... |
|
Or really, if you think about a model like that used in Racket (see "You Want It When" paper and the sequel), it seems to me that what we probably need is for i.e. that model needs some way for the macro-defining compile-time crate to indicate that it wants symbols from the run-time crate, but it won't be executing any code from that run-time crate. |
|
(i haven't looked very carefully at why this PR exposes the bug described above. You would think we would be seeing it even without this PR...) |
|
Interesting! I believe that a major culprit is this code: https://github.com/mozilla/rust/blob/master/src/librustc/driver/driver.rs#L234-L241. The comment I wrote asserts that rpaths on unix fix everything, but this is not a true statement. The contents of I believe that I'm not sure if requiring a set of compile-time crates and a set of non-compile-time crates is what we want, but I'm still confused by this problem. It appears that the stage2 host regex crate is different than the stage2 target regex crate, but we're semi-guaranteeing convergence by stage2. For example, the stage2 regex target crate depends on the stage2 std target crate, but we're dlopening the stage2 regex target crate into a process which only has the stage2 host std crate (and yet somehow it works)? I feel like we've been toeing the line of whether this arbitrary code at compile time is possible or not. The possibility of it is looking bleaker as time goes on... |
|
And I confuse myself even further. That code in rustc is adding the host directory to the search paths, not the target directory. So it will not have the fix that I was talking about... |
|
Another thing I just noticed that I should point out: Assuming that our makefiles and Take a look at this command line transcript (which I believe corresponds to what In particular, why is that talking about update: perhaps this is exactly why the fix @alexcrichton mentioned two comments up might actually work (even though alex denied that could work one comment up...) |
|
@alexcrichton and yes, I was also surprised that we did not have convergence in terms of the state of But I also think a scheme like the one employed by Racket should actually not require convergence; rather, it requires the macro author to understand the distinction between their run-time symbols (which end up as compile-time for the end-client program using the macro) and their "template"-time symbols (what I also called " |
When the compiler dlopen()s a crate, it uses the target libraries, not the host libraries (reason below)
The compiler can't currently produce an artifact linked against its own artifacts because that would involve linking to code that the compiler didn't generate. It is dlopen()'ing the code though, which is a bit sketchy...
I rescind this comment, this makes sense. New development: http://www.cocoabuilder.com/archive/cocoa/133694-dyld-library-path-etc.html
Our hokey method of adding to the search path at runtime is apparently just that, hokey. It worked for windows, but I don't think that it works for anyone else. Ok, so here's what I think the situation is. As long as we can guarantee convergence by stage, everything should be working today. I believe the regex crate is the only crate that is not converging, due to one use case of Ok, so let's think about a solution. First, I/felix have been talking a lot about search paths and library paths. I don't think those are relevant in this instance, as we can only have a sane world if we rely on convergence. The real issue here is that the regex crate is different at stage1-generation time and stage2-generation time (all libraries should be constant at these two times). I believe the real solution is that we need |
|
I believe I have found why regex is not converging. This has to do with how when we hash an identifier, we hash the name of the identifier, not the contents of the identifier. This means that the order in which identifiers are declared turns out to be important. For example: fn main() {
let foo = 1;
let bar = 3;
let baz = 4;
let _a = (foo, bar, baz);
}What's happening here is that these symbols are getting interned early on in the compilation process due to being on the command line, affecting their values later on. So the problem is not that the regex crate has tl;dr - I have no idea why, but this patch appears to generate the same SVH for stage1/stage2 diff --git a/src/libregex/test/mod.rs b/src/libregex/test/mod.rs
index a4c3a83..b315396 100644
--- a/src/libregex/test/mod.rs
+++ b/src/libregex/test/mod.rs
@@ -9,6 +9,7 @@
// except according to those terms.
#[cfg(not(stage1))]
+#[cfg(stage2)]
#[phase(syntax)]
extern crate regex_macros;
|
|
@alexcrichton I agree that convergence is a sufficient condition for correctness of our overall macro system (that is, if we could get this crate to converge, then a lot of the problems we are seeing on this PR would go away). I am not yet convinced that convergence is a necessary condition for correctness (which I think I stated earlier when I was referring to the various Racket macro papers). But I am also willing to table this debate for now in the interest of getting today's code working. :) |
|
I hypothesize that rebasing this PR atop PR #14145 will remove the issue that @alexcrichton dissected and diagnosed in his earlier comment |
|
I'll rebase and reopen this when #14145 lands. |
|
Responding to myself: the place where convergence is a necessary condition for correctness is when you have macros that generate procedural macros that are then invoked in that same crate. The Racket papers I linked above provide examples where Racket wants to be able to generate procedural macros and invoke those macros within the same compilation unit (effectively), and I am pretty sure that some of those examples cannot be readily re-encoded in a system where there is a forced separation between the compilation unit with the macro-definition and the unit with the macro-use. (But just because Racket needs that feature does not mean that Rust needs it...) |
This is far cheaper than the `.to_str` technique that was used previously.
|
Rebased. r? I kicked off some try builds (I'll probably be asleep by the time they (hopefully) pass):
I'm not sure if the dynamic_lib patch is needed any more? |
This is fully backwards compatible, since test names are Rust identifiers + `:`, and hence not special regex characters. Fixes rust-lang#2866.
Previously the longer hand-written usage string was never being printed: theoretically it was trying to detect when precisely `--help` was passed (but not `-h`), but the getopts framework was considering a check for the presence of `-h` to be a check for that of `--help` too, i.e. the code was always going through the `-h` path. This changes it to print the extended usage for both `-h` and `--help`, meaning that it does actually appear correctly.
The compiler needs to be opening e.g. libregex in the correct directory, which requires getting these in the right order.
This allows writing a regex to filter tests more precisely, rather than having to list long paths e.g. ``` $ ./stdtest-x86_64-unknown-linux-gnu 'vec.*clone' running 2 tests test vec::tests::test_clone ... ok test vec::tests::test_clone_from ... ok test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured ``` The regex change is fully backwards compatible, since test names are Rust identifiers + `:`, and hence not special regex characters. (See commits for details.)
|
Thanks for fixing the SVH, @pnkfelix! |
Make inlay hint location links work for more types
This allows writing a regex to filter tests more precisely, rather than having to list long paths e.g.
The regex change is fully backwards compatible, since test names are Rust
identifiers +
:, and hence not special regex characters.(See commits for details.)