Skip to content

rustpkg: Register correct dependencies for built and installed files #9322

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/librustpkg/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use target::*;
use version::Version;
use workcache_support::*;

use std::os;
use extra::arc::{Arc,RWArc};
use extra::workcache;
use extra::workcache::{Database, Logger, FreshnessMap};
Expand All @@ -40,11 +41,13 @@ pub fn new_default_context(c: workcache::Context, p: Path) -> BuildContext {
}

fn file_is_fresh(path: &str, in_hash: &str) -> bool {
in_hash == digest_file_with_date(&Path(path))
let path = Path(path);
os::path_exists(&path) && in_hash == digest_file_with_date(&path)
}

fn binary_is_fresh(path: &str, in_hash: &str) -> bool {
in_hash == digest_only_date(&Path(path))
let path = Path(path);
os::path_exists(&path) && in_hash == digest_only_date(&path)
}

pub fn new_workcache_context(p: &Path) -> workcache::Context {
Expand Down
2 changes: 1 addition & 1 deletion src/librustpkg/exit_codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

pub static copy_failed_code: int = 65;
pub static COPY_FAILED_CODE: int = 65;
1 change: 0 additions & 1 deletion src/librustpkg/path_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ fn target_bin_dir(workspace: &Path) -> Path {
/// directory is, and if the file exists, return it.
pub fn built_executable_in_workspace(pkgid: &PkgId, workspace: &Path) -> Option<Path> {
let mut result = target_build_dir(workspace);
// should use a target-specific subdirectory
result = mk_output_path(Main, Build, pkgid, result);
debug!("built_executable_in_workspace: checking whether %s exists",
result.to_str());
Expand Down
6 changes: 3 additions & 3 deletions src/librustpkg/rustpkg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ use package_source::PkgSrc;
use target::{WhatToBuild, Everything, is_lib, is_main, is_test, is_bench};
// use workcache_support::{discover_outputs, digest_only_date};
use workcache_support::digest_only_date;
use exit_codes::copy_failed_code;
use exit_codes::COPY_FAILED_CODE;

pub mod api;
mod conditions;
Expand Down Expand Up @@ -789,10 +789,10 @@ pub fn main_args(args: &[~str]) {
}.run(sub_cmd, rm_args.clone())
};
// FIXME #9262: This is using the same error code for all errors,
// and at least one test case succeeds if rustpkg returns copy_failed_code,
// and at least one test case succeeds if rustpkg returns COPY_FAILED_CODE,
// when actually, it might set the exit code for that even if a different
// unhandled condition got raised.
if result.is_err() { os::set_exit_status(copy_failed_code); }
if result.is_err() { os::set_exit_status(COPY_FAILED_CODE); }

}

Expand Down
53 changes: 53 additions & 0 deletions src/librustpkg/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,13 @@ fn executable_exists(repo: &Path, short_name: &str) -> bool {
os::path_exists(&exec) && is_rwx(&exec)
}

fn remove_executable_file(p: &PkgId, workspace: &Path) {
let exec = target_executable_in_workspace(&PkgId::new(p.short_name), workspace);
if os::path_exists(&exec) {
assert!(os::remove_file(&exec));
}
}

fn assert_built_executable_exists(repo: &Path, short_name: &str) {
assert!(built_executable_exists(repo, short_name));
}
Expand All @@ -368,6 +375,14 @@ fn built_executable_exists(repo: &Path, short_name: &str) -> bool {
}
}

fn remove_built_executable_file(p: &PkgId, workspace: &Path) {
let exec = built_executable_in_workspace(&PkgId::new(p.short_name), workspace);
match exec {
Some(r) => assert!(os::remove_file(&r)),
None => ()
}
}

fn object_file_exists(repo: &Path, short_name: &str) -> bool {
file_exists(repo, short_name, "o")
}
Expand Down Expand Up @@ -1705,6 +1720,44 @@ fn test_dependencies_terminate() {
command_line_test([~"install", ~"b"], &workspace);
}

#[test]
fn install_after_build() {
let b_id = PkgId::new("b");
let workspace = create_local_package(&b_id);
command_line_test([~"build", ~"b"], &workspace);
command_line_test([~"install", ~"b"], &workspace);
assert_executable_exists(&workspace, b_id.short_name);
assert_lib_exists(&workspace, &b_id.path, NoVersion);
}

#[test]
fn reinstall() {
let b = PkgId::new("b");
let workspace = create_local_package(&b);
// 1. Install, then remove executable file, then install again,
// and make sure executable was re-installed
command_line_test([~"install", ~"b"], &workspace);
assert_executable_exists(&workspace, b.short_name);
assert_lib_exists(&workspace, &b.path, NoVersion);
remove_executable_file(&b, &workspace);
command_line_test([~"install", ~"b"], &workspace);
assert_executable_exists(&workspace, b.short_name);
// 2. Build, then remove build executable file, then build again,
// and make sure executable was re-built.
command_line_test([~"build", ~"b"], &workspace);
remove_built_executable_file(&b, &workspace);
command_line_test([~"build", ~"b"], &workspace);
assert_built_executable_exists(&workspace, b.short_name);
// 3. Install, then remove both executable and built executable,
// then install again, make sure both were recreated
command_line_test([~"install", ~"b"], &workspace);
remove_executable_file(&b, &workspace);
remove_built_executable_file(&b, &workspace);
command_line_test([~"install", ~"b"], &workspace);
assert_executable_exists(&workspace, b.short_name);
assert_built_executable_exists(&workspace, b.short_name);
}

/// Returns true if p exists and is executable
fn is_executable(p: &Path) -> bool {
use std::libc::consts::os::posix88::{S_IXUSR};
Expand Down
23 changes: 22 additions & 1 deletion src/librustpkg/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,28 @@ pub fn compile_input(context: &BuildContext,

debug!("calling compile_crate_from_input, workspace = %s,
building_library = %?", out_dir.to_str(), sess.building_library);
compile_crate_from_input(in_file, exec, context.compile_upto(), &out_dir, sess, crate)
let result = compile_crate_from_input(in_file,
exec,
context.compile_upto(),
&out_dir,
sess,
crate);
// Discover the output
let discovered_output = if what == Lib {
installed_library_in_workspace(&pkg_id.path, workspace)
}
else {
result
};
debug!("About to discover output %s", discovered_output.to_str());
for p in discovered_output.iter() {
if os::path_exists(p) {
exec.discover_output("binary", p.to_str(), digest_only_date(p));
}
// Nothing to do if it doesn't exist -- that could happen if we had the
// -S or -emit-llvm flags, etc.
}
discovered_output
}

// Should use workcache to avoid recompiling when not necessary
Expand Down