Skip to content

Commit 1bfa45c

Browse files
borsbors
committed
auto merge of #10342 : catamorphism/rust/rustpkg-dir-checking-and-monitor, r=erickt
r? @brson
2 parents 2d6952e + 65aacd0 commit 1bfa45c

File tree

4 files changed

+71
-22
lines changed

4 files changed

+71
-22
lines changed

‎src/librustpkg/package_source.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,8 @@ impl PkgSrc {
118118

119119
debug!("Checking dirs: {:?}", to_try.map(|p| p.display().to_str()).connect(":"));
120120

121-
let path = to_try.iter().find(|&d| d.exists());
121+
let path = to_try.iter().find(|&d| d.is_dir()
122+
&& dir_has_crate_file(d));
122123

123124
// See the comments on the definition of PkgSrc
124125
let mut build_in_destination = use_rust_path_hack;

‎src/librustpkg/path_util.rs

+2
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ pub fn versionize(p: &Path, v: &Version) -> Path {
461461
}
462462

463463
#[cfg(target_os = "win32")]
464+
#[fixed_stack_segment]
464465
pub fn chmod_read_only(p: &Path) -> bool {
465466
unsafe {
466467
do p.with_c_str |src_buf| {
@@ -470,6 +471,7 @@ pub fn chmod_read_only(p: &Path) -> bool {
470471
}
471472

472473
#[cfg(not(target_os = "win32"))]
474+
#[fixed_stack_segment]
473475
pub fn chmod_read_only(p: &Path) -> bool {
474476
unsafe {
475477
do p.with_c_str |src_buf| {

‎src/librustpkg/tests.rs

+49-15
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,8 @@ fn rustpkg_exec() -> Path {
239239
fn command_line_test(args: &[~str], cwd: &Path) -> ProcessOutput {
240240
match command_line_test_with_env(args, cwd, None) {
241241
Success(r) => r,
242-
Fail(error) => fail!("Command line test failed with error {}", error)
242+
Fail(error) => fail!("Command line test failed with error {}",
243+
error.status)
243244
}
244245
}
245246

@@ -253,15 +254,15 @@ fn command_line_test_expect_fail(args: &[~str],
253254
expected_exitcode: int) {
254255
match command_line_test_with_env(args, cwd, env) {
255256
Success(_) => fail!("Should have failed with {}, but it succeeded", expected_exitcode),
256-
Fail(error) if error.matches_exit_status(expected_exitcode) => (), // ok
257+
Fail(ref error) if error.status.matches_exit_status(expected_exitcode) => (), // ok
257258
Fail(other) => fail!("Expected to fail with {}, but failed with {} instead",
258-
expected_exitcode, other)
259+
expected_exitcode, other.status)
259260
}
260261
}
261262

262263
enum ProcessResult {
263264
Success(ProcessOutput),
264-
Fail(ProcessExit)
265+
Fail(ProcessOutput)
265266
}
266267

267268
/// Runs `rustpkg` (based on the directory that this executable was
@@ -295,7 +296,7 @@ fn command_line_test_with_env(args: &[~str], cwd: &Path, env: Option<~[(~str, ~s
295296
debug!("Command {} {:?} failed with exit code {:?}; its output was --- {} ---",
296297
cmd, args, output.status,
297298
str::from_utf8(output.output) + str::from_utf8(output.error));
298-
Fail(output.status)
299+
Fail(output)
299300
}
300301
else {
301302
Success(output)
@@ -1093,7 +1094,7 @@ fn no_rebuilding() {
10931094
10941095
match command_line_test_partial([~"build", ~"foo"], workspace) {
10951096
Success(*) => (), // ok
1096-
Fail(status) if status.matches_exit_status(65) =>
1097+
Fail(ref status) if status.status.matches_exit_status(65) =>
10971098
fail!("no_rebuilding failed: it tried to rebuild bar"),
10981099
Fail(_) => fail!("no_rebuilding failed for some other reason")
10991100
}
@@ -1112,7 +1113,8 @@ fn no_recopying() {
11121113
11131114
match command_line_test_partial([~"install", ~"foo"], workspace) {
11141115
Success(*) => (), // ok
1115-
Fail(process::ExitStatus(65)) => fail!("no_recopying failed: it tried to re-copy foo"),
1116+
Fail(ref status) if status.status.matches_exit_status(65) =>
1117+
fail!("no_recopying failed: it tried to re-copy foo"),
11161118
Fail(_) => fail!("no_copying failed for some other reason")
11171119
}
11181120
}
@@ -1130,7 +1132,7 @@ fn no_rebuilding_dep() {
11301132
assert!(chmod_read_only(&bar_lib));
11311133
match command_line_test_partial([~"build", ~"foo"], workspace) {
11321134
Success(*) => (), // ok
1133-
Fail(status) if status.matches_exit_status(65) =>
1135+
Fail(ref r) if r.status.matches_exit_status(65) =>
11341136
fail!("no_rebuilding_dep failed: it tried to rebuild bar"),
11351137
Fail(_) => fail!("no_rebuilding_dep failed for some other reason")
11361138
}
@@ -1151,7 +1153,7 @@ fn do_rebuild_dep_dates_change() {
11511153
11521154
match command_line_test_partial([~"build", ~"foo"], workspace) {
11531155
Success(*) => fail!("do_rebuild_dep_dates_change failed: it didn't rebuild bar"),
1154-
Fail(status) if status.matches_exit_status(65) => (), // ok
1156+
Fail(ref r) if r.status.matches_exit_status(65) => (), // ok
11551157
Fail(_) => fail!("do_rebuild_dep_dates_change failed for some other reason")
11561158
}
11571159
}
@@ -1172,7 +1174,7 @@ fn do_rebuild_dep_only_contents_change() {
11721174
// should adjust the datestamp
11731175
match command_line_test_partial([~"build", ~"foo"], workspace) {
11741176
Success(*) => fail!("do_rebuild_dep_only_contents_change failed: it didn't rebuild bar"),
1175-
Fail(status) if status.matches_exit_status(65) => (), // ok
1177+
Fail(ref r) if r.status.matches_exit_status(65) => (), // ok
11761178
Fail(_) => fail!("do_rebuild_dep_only_contents_change failed for some other reason")
11771179
}
11781180
}
@@ -2148,7 +2150,7 @@ fn test_rebuild_when_needed() {
21482150
chmod_read_only(&test_executable);
21492151
match command_line_test_partial([~"test", ~"foo"], foo_workspace) {
21502152
Success(*) => fail!("test_rebuild_when_needed didn't rebuild"),
2151-
Fail(status) if status.matches_exit_status(65) => (), // ok
2153+
Fail(ref r) if r.status.matches_exit_status(65) => (), // ok
21522154
Fail(_) => fail!("test_rebuild_when_needed failed for some other reason")
21532155
}
21542156
}
@@ -2168,7 +2170,7 @@ fn test_no_rebuilding() {
21682170
chmod_read_only(&test_executable);
21692171
match command_line_test_partial([~"test", ~"foo"], foo_workspace) {
21702172
Success(*) => (), // ok
2171-
Fail(status) if status.matches_exit_status(65) =>
2173+
Fail(ref r) if r.status.matches_exit_status(65) =>
21722174
fail!("test_no_rebuilding failed: it rebuilt the tests"),
21732175
Fail(_) => fail!("test_no_rebuilding failed for some other reason")
21742176
}
@@ -2296,7 +2298,7 @@ fn test_compile_error() {
22962298
let result = command_line_test_partial([~"build", ~"foo"], foo_workspace);
22972299
match result {
22982300
Success(*) => fail!("Failed by succeeding!"), // should be a compile error
2299-
Fail(status) => {
2301+
Fail(ref status) => {
23002302
debug!("Failed with status {:?}... that's good, right?", status);
23012303
}
23022304
}
@@ -2364,7 +2366,7 @@ fn test_c_dependency_no_rebuilding() {
23642366
23652367
match command_line_test_partial([~"build", ~"cdep"], dir) {
23662368
Success(*) => (), // ok
2367-
Fail(status) if status.matches_exit_status(65) =>
2369+
Fail(ref r) if r.status.matches_exit_status(65) =>
23682370
fail!("test_c_dependency_no_rebuilding failed: \
23692371
it tried to rebuild foo.c"),
23702372
Fail(_) =>
@@ -2403,11 +2405,43 @@ fn test_c_dependency_yes_rebuilding() {
24032405
match command_line_test_partial([~"build", ~"cdep"], dir) {
24042406
Success(*) => fail!("test_c_dependency_yes_rebuilding failed: \
24052407
it didn't rebuild and should have"),
2406-
Fail(status) if status.matches_exit_status(65) => (),
2408+
Fail(ref r) if r.status.matches_exit_status(65) => (),
24072409
Fail(_) => fail!("test_c_dependency_yes_rebuilding failed for some other reason")
24082410
}
24092411
}
24102412
2413+
// n.b. This might help with #10253, or at least the error will be different.
2414+
#[test]
2415+
fn correct_error_dependency() {
2416+
// Supposing a package we're trying to install via a dependency doesn't
2417+
// exist, we should throw a condition, and not ICE
2418+
let dir = create_local_package(&PkgId::new("badpkg"));
2419+
2420+
let dir = dir.path();
2421+
writeFile(&dir.join_many(["src", "badpkg-0.1", "main.rs"]),
2422+
"extern mod p = \"some_package_that_doesnt_exist\";
2423+
fn main() {}");
2424+
2425+
match command_line_test_partial([~"build", ~"badpkg"], dir) {
2426+
Fail(ProcessOutput{ error: error, output: output, _ }) => {
2427+
assert!(str::is_utf8(error));
2428+
assert!(str::is_utf8(output));
2429+
let error_str = str::from_utf8(error);
2430+
let out_str = str::from_utf8(output);
2431+
debug!("ss = {}", error_str);
2432+
debug!("out_str = {}", out_str);
2433+
if out_str.contains("Package badpkg depends on some_package_that_doesnt_exist") &&
2434+
!error_str.contains("nonexistent_package") {
2435+
// Ok
2436+
()
2437+
} else {
2438+
fail!("Wrong error");
2439+
}
2440+
}
2441+
Success(*) => fail!("Test passed when it should have failed")
2442+
}
2443+
}
2444+
24112445
/// Returns true if p exists and is executable
24122446
fn is_executable(p: &Path) -> bool {
24132447
p.exists() && p.stat().perm & io::UserExecute == io::UserExecute

‎src/librustpkg/util.rs

+18-6
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ pub use target::{Target, Build, Install};
3636
use extra::treemap::TreeMap;
3737
pub use target::{lib_name_of, lib_crate_filename, WhatToBuild, MaybeCustom, Inferred};
3838
use workcache_support::{digest_file_with_date, digest_only_date};
39+
use messages::error;
3940

4041
// It would be nice to have the list of commands in just one place -- for example,
4142
// you could update the match in rustpkg.rc but forget to update this list. I think
@@ -430,6 +431,8 @@ struct ViewItemVisitor<'self> {
430431

431432
impl<'self> Visitor<()> for ViewItemVisitor<'self> {
432433
fn visit_view_item(&mut self, vi: &ast::view_item, env: ()) {
434+
use conditions::nonexistent_package::cond;
435+
433436
match vi.node {
434437
// ignore metadata, I guess
435438
ast::view_item_extern_mod(lib_ident, path_opt, _, _) => {
@@ -490,12 +493,21 @@ impl<'self> Visitor<()> for ViewItemVisitor<'self> {
490493
// and the `PkgSrc` constructor will detect that;
491494
// or else it's already in a workspace and we'll build into that
492495
// workspace
493-
let pkg_src = PkgSrc::new(source_workspace,
494-
dest_workspace,
495-
// Use the rust_path_hack to search for dependencies iff
496-
// we were already using it
497-
self.context.context.use_rust_path_hack,
498-
pkg_id.clone());
496+
let pkg_src = do cond.trap(|_| {
497+
// Nonexistent package? Then print a better error
498+
error(format!("Package {} depends on {}, but I don't know \
499+
how to find it",
500+
self.parent.path.display(),
501+
pkg_id.path.display()));
502+
fail!()
503+
}).inside {
504+
PkgSrc::new(source_workspace.clone(),
505+
dest_workspace.clone(),
506+
// Use the rust_path_hack to search for dependencies iff
507+
// we were already using it
508+
self.context.context.use_rust_path_hack,
509+
pkg_id.clone())
510+
};
499511
let (outputs_disc, inputs_disc) =
500512
self.context.install(
501513
pkg_src,

0 commit comments

Comments
 (0)